mbox series

[00/12] spi: s3c64xx: remove OF alias ID dependency

Message ID 20240208135045.3728927-1-tudor.ambarus@linaro.org
Headers show
Series spi: s3c64xx: remove OF alias ID dependency | expand

Message

Tudor Ambarus Feb. 8, 2024, 1:50 p.m. UTC 
The driver was wrong as it assumed that the alias values in devicetree
have a particular meaning in identifying instances. This immediately
breaks when there is a dtb file that does not use the same alias values,
e.g. because it only needs some of the SPI ports.

Tested gs101 SPI with spi-loopback-test, all went fine. I updated
exynos850 as it uses the same USI.SPI_VERSION as gs101. Maybe Sam can
test exynos850, if not, we can drop that patch.

The patch set has some dependencies. One has to apply first the gs101
addition [1], then the simple cleanup from [2], then this patch set.
[1] https://lore.kernel.org/linux-spi/20240207111516.2563218-1-tudor.ambarus@linaro.org/
[2] https://lore.kernel.org/linux-spi/20240207120431.2766269-1-tudor.ambarus@linaro.org/

Cheers,
ta

Tudor Ambarus (12):
  spi: dt-bindings: introduce the ``fifo-depth`` property
  spi: s3c64xx: define a magic value
  spi: s3c64xx: allow full FIFO masks
  spi: s3c64xx: determine the fifo depth only once
  spi: s3c64xx: retrieve the FIFO depth from the device tree
  spi: s3c64xx: allow FIFO depth to be determined from the compatible
  spi: s3c64xx: let the SPI core determine the bus number
  spi: s3c64xx: introduce s3c64xx_spi_set_port_id()
  spi: s3c64xx: get rid of the OF alias ID dependency
  spi: s3c64xx: deprecate fifo_lvl_mask, rx_lvl_offset and port_id
  spi: s3c64xx: switch gs101 to new port config data
  spi: s3c64xx: switch exynos850 to new port config data

 .../bindings/spi/spi-controller.yaml          |   5 +
 drivers/spi/spi-s3c64xx.c                     | 142 ++++++++++++++----
 2 files changed, 116 insertions(+), 31 deletions(-)

Comments

Tudor Ambarus Feb. 8, 2024, 4:01 p.m. UTC | #1 
On 2/8/24 13:50, Tudor Ambarus wrote:
> Exynos850 integrates 3 SPI

I should have replaced this with "GS101 integrates 16 SPI", as here I'm
updating gs101.
Conor Dooley Feb. 8, 2024, 6:24 p.m. UTC | #2 
On Thu, Feb 08, 2024 at 01:50:34PM +0000, Tudor Ambarus wrote:
> There are instances of the same IP that are configured by the integrator
> with different FIFO depths. Introduce the fifo-depth property to allow
> such nodes to specify their FIFO depth.
> 
> We haven't seen SPI IPs with different FIFO depths for RX and TX, thus
> introduce a single property.

Some citation attached to this would be nice. "We haven't seen" offers
no detail as to what IPs that allow this sort of configuration of FIFO
size that you have actually checked.

I went and checked our IP that we use in FPGA fabric, which has a
configurable fifo depth. It only has a single knob for both RX and TX
FIFOs. The Xilinx xps spi core also has configurable FIFOs, but again RX
and TX sizes are tied there. At least that's a sample size of three.

One of our guys is working on support for the IP I just mentioned and
would be defining a vendor property for this, so
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.

> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
> ---
>  Documentation/devicetree/bindings/spi/spi-controller.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/spi/spi-controller.yaml b/Documentation/devicetree/bindings/spi/spi-controller.yaml
> index 524f6fe8c27b..99272e6f115e 100644
> --- a/Documentation/devicetree/bindings/spi/spi-controller.yaml
> +++ b/Documentation/devicetree/bindings/spi/spi-controller.yaml
> @@ -69,6 +69,11 @@ properties:
>           Should be generally avoided and be replaced by
>           spi-cs-high + ACTIVE_HIGH.
>  
> +  fifo-depth:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Size of the data FIFO in bytes.
> +
>    num-cs:
>      $ref: /schemas/types.yaml#/definitions/uint32
>      description:
> -- 
> 2.43.0.687.g38aa6559b0-goog
>
Tudor Ambarus Feb. 9, 2024, 1:56 p.m. UTC | #3 
+ Geert

On 2/8/24 18:24, Conor Dooley wrote:
> On Thu, Feb 08, 2024 at 01:50:34PM +0000, Tudor Ambarus wrote:
>> There are instances of the same IP that are configured by the integrator
>> with different FIFO depths. Introduce the fifo-depth property to allow
>> such nodes to specify their FIFO depth.
>>
>> We haven't seen SPI IPs with different FIFO depths for RX and TX, thus
>> introduce a single property.
> 
> Some citation attached to this would be nice. "We haven't seen" offers
> no detail as to what IPs that allow this sort of configuration of FIFO
> size that you have actually checked.
> 
> I went and checked our IP that we use in FPGA fabric, which has a
> configurable fifo depth. It only has a single knob for both RX and TX
> FIFOs. The Xilinx xps spi core also has configurable FIFOs, but again RX
> and TX sizes are tied there. At least that's a sample size of three.
> 
> One of our guys is working on support for the IP I just mentioned and
> would be defining a vendor property for this, so
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> 

Thanks, Conor. I had in mind that SPI has a shift register and it's
improbable to have different FIFO depths for RX and TX. At least I don't
see how it would work, I guess it will use the minimum depth between the
two?

I grepped by "fifo" in the spi bindings and I now see that renesas is
using dedicated properties for RX and TX, but I think that there too the
FIFOs have the same depths. Looking into drivers/spi/spi-sh-msiof.c I
see that the of_device_id.data contains 64 bytes FIFOs for RX and TX,
regardless of the compatible.

Geert, any idea if the FIFO depths can differ for RX and TX in
spi-sh-msiof.c?

Anyway, even if there are such imbalanced architectures, I guess we can
consider them when/if they appear? (add rx/tx-fifo-depth dt properties)

Cheers,
ta


----
$ git grep fifo Documentation/devicetree/bindings/spi/
Documentation/devicetree/bindings/spi/atmel,at91rm9200-spi.yaml:
atmel,fifo-size:
Documentation/devicetree/bindings/spi/atmel,at91rm9200-spi.yaml:
atmel,fifo-size = <32>;
Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml:
cdns,fifo-depth:
Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml:
cdns,fifo-depth:
Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml:  cdns,fifo-depth:
Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml:  cdns,fifo-width:
Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml:  - cdns,fifo-depth
Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml:  - cdns,fifo-width
Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml:
cdns,fifo-depth = <128>;
Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml:
cdns,fifo-width = <4>;
Documentation/devicetree/bindings/spi/renesas,sh-msiof.yaml:
renesas,tx-fifo-size:
Documentation/devicetree/bindings/spi/renesas,sh-msiof.yaml:
Override the default TX fifo size.  Unit is words.  Ignored if 0.
Documentation/devicetree/bindings/spi/renesas,sh-msiof.yaml:
renesas,rx-fifo-size:
Documentation/devicetree/bindings/spi/renesas,sh-msiof.yaml:
Override the default RX fifo size.  Unit is words.  Ignored if 0.
Documentation/devicetree/bindings/spi/spi-sifive.yaml:  sifive,fifo-depth:
Documentation/devicetree/bindings/spi/spi-sifive.yaml:
sifive,fifo-depth = <8>;
Conor Dooley Feb. 9, 2024, 4:21 p.m. UTC | #4 
On Fri, Feb 09, 2024 at 01:56:56PM +0000, Tudor Ambarus wrote:
> 
> + Geert
> 
> On 2/8/24 18:24, Conor Dooley wrote:
> > On Thu, Feb 08, 2024 at 01:50:34PM +0000, Tudor Ambarus wrote:
> >> There are instances of the same IP that are configured by the integrator
> >> with different FIFO depths. Introduce the fifo-depth property to allow
> >> such nodes to specify their FIFO depth.
> >>
> >> We haven't seen SPI IPs with different FIFO depths for RX and TX, thus
> >> introduce a single property.
> > 
> > Some citation attached to this would be nice. "We haven't seen" offers
> > no detail as to what IPs that allow this sort of configuration of FIFO
> > size that you have actually checked.
> > 
> > I went and checked our IP that we use in FPGA fabric, which has a
> > configurable fifo depth. It only has a single knob for both RX and TX
> > FIFOs. The Xilinx xps spi core also has configurable FIFOs, but again RX
> > and TX sizes are tied there. At least that's a sample size of three.
> > 
> > One of our guys is working on support for the IP I just mentioned and
> > would be defining a vendor property for this, so
> > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> > 
> 
> Thanks, Conor. I had in mind that SPI has a shift register and it's
> improbable to have different FIFO depths for RX and TX.

IDK, but I've learned to expect the unexpectable, especially when it
comes to the IPs intended for use in FPGAs.

> At least I don't
> see how it would work, I guess it will use the minimum depth between the
> two?

I'm not really sure how it would work other than that in the general
case, but some use case specific configuration could work, but I do
agree that it is

> I grepped by "fifo" in the spi bindings and I now see that renesas is
> using dedicated properties for RX and TX, but I think that there too the
> FIFOs have the same depths. Looking into drivers/spi/spi-sh-msiof.c I
> see that the of_device_id.data contains 64 bytes FIFOs for RX and TX,
> regardless of the compatible.
> 
> Geert, any idea if the FIFO depths can differ for RX and TX in
> spi-sh-msiof.c?
> 
> Anyway, even if there are such imbalanced architectures, I guess we can
> consider them when/if they appear? (add rx/tx-fifo-depth dt properties)

I think so.

> Documentation/devicetree/bindings/spi/renesas,sh-msiof.yaml:
> Override the default TX fifo size.  Unit is words.  Ignored if 0.
> Documentation/devicetree/bindings/spi/renesas,sh-msiof.yaml:
> renesas,rx-fifo-size:
> Documentation/devicetree/bindings/spi/renesas,sh-msiof.yaml:
> Override the default RX fifo size.  Unit is words.  Ignored if 0.

These renesas ones seemed interesting at first glance due to these
comments, but what's missed by grep the is "deprecated" marking on
these. They seem to have been replaced by soc-specific compatibles.
Tudor Ambarus Feb. 9, 2024, 4:55 p.m. UTC | #5 
On 2/9/24 16:21, Conor Dooley wrote:
> On Fri, Feb 09, 2024 at 01:56:56PM +0000, Tudor Ambarus wrote:
>>
>> + Geert
>>
>> On 2/8/24 18:24, Conor Dooley wrote:
>>> On Thu, Feb 08, 2024 at 01:50:34PM +0000, Tudor Ambarus wrote:
>>>> There are instances of the same IP that are configured by the integrator
>>>> with different FIFO depths. Introduce the fifo-depth property to allow
>>>> such nodes to specify their FIFO depth.
>>>>
>>>> We haven't seen SPI IPs with different FIFO depths for RX and TX, thus
>>>> introduce a single property.
>>>
>>> Some citation attached to this would be nice. "We haven't seen" offers
>>> no detail as to what IPs that allow this sort of configuration of FIFO
>>> size that you have actually checked.
>>>
>>> I went and checked our IP that we use in FPGA fabric, which has a
>>> configurable fifo depth. It only has a single knob for both RX and TX
>>> FIFOs. The Xilinx xps spi core also has configurable FIFOs, but again RX
>>> and TX sizes are tied there. At least that's a sample size of three.
>>>
>>> One of our guys is working on support for the IP I just mentioned and
>>> would be defining a vendor property for this, so
>>> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
>>>
>>
>> Thanks, Conor. I had in mind that SPI has a shift register and it's
>> improbable to have different FIFO depths for RX and TX.
> 
> IDK, but I've learned to expect the unexpectable, especially when it
> comes to the IPs intended for use in FPGAs.
> 
>> At least I don't
>> see how it would work, I guess it will use the minimum depth between the
>> two?
> 
> I'm not really sure how it would work other than that in the general
> case, but some use case specific configuration could work, but I do
> agree that it is
> 
>> I grepped by "fifo" in the spi bindings and I now see that renesas is
>> using dedicated properties for RX and TX, but I think that there too the
>> FIFOs have the same depths. Looking into drivers/spi/spi-sh-msiof.c I
>> see that the of_device_id.data contains 64 bytes FIFOs for RX and TX,
>> regardless of the compatible.
>>
>> Geert, any idea if the FIFO depths can differ for RX and TX in
>> spi-sh-msiof.c?
>>
>> Anyway, even if there are such imbalanced architectures, I guess we can
>> consider them when/if they appear? (add rx/tx-fifo-depth dt properties)
> 
> I think so.
> 
>> Documentation/devicetree/bindings/spi/renesas,sh-msiof.yaml:
>> Override the default TX fifo size.  Unit is words.  Ignored if 0.
>> Documentation/devicetree/bindings/spi/renesas,sh-msiof.yaml:
>> renesas,rx-fifo-size:
>> Documentation/devicetree/bindings/spi/renesas,sh-msiof.yaml:
>> Override the default RX fifo size.  Unit is words.  Ignored if 0.
> 
> These renesas ones seemed interesting at first glance due to these
> comments, but what's missed by grep the is "deprecated" marking on
> these. They seem to have been replaced by soc-specific compatibles.

In the driver the renesas,{rx,tx}-fifo-size properties still have the
highest priority:

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git/tree/drivers/spi/spi-sh-msiof.c#n1350

Maybe something for Geert, as I see he was the one marking these
properties as deprecated. I guess he forgot to update the driver.

Anyway, I think we shall be fine, even if we don't hear from Geert.

Cheers,
ta
Mark Brown Feb. 9, 2024, 5:41 p.m. UTC | #6 
On Fri, Feb 09, 2024 at 04:21:16PM +0000, Conor Dooley wrote:
> On Fri, Feb 09, 2024 at 01:56:56PM +0000, Tudor Ambarus wrote:

> > At least I don't
> > see how it would work, I guess it will use the minimum depth between the
> > two?

> I'm not really sure how it would work other than that in the general
> case, but some use case specific configuration could work, but I do
> agree that it is

You do get devices that are single duplex only where the mismatched
sizes wouldn't be a pressing issue.
Geert Uytterhoeven Feb. 12, 2024, 10:38 a.m. UTC | #7 
Hi Tudor,

On Fri, Feb 9, 2024 at 5:55 PM Tudor Ambarus <tudor.ambarus@linaro.org> wrote:
> On 2/9/24 16:21, Conor Dooley wrote:
> > On Fri, Feb 09, 2024 at 01:56:56PM +0000, Tudor Ambarus wrote:
> >> On 2/8/24 18:24, Conor Dooley wrote:
> >>> On Thu, Feb 08, 2024 at 01:50:34PM +0000, Tudor Ambarus wrote:
> >>>> There are instances of the same IP that are configured by the integrator
> >>>> with different FIFO depths. Introduce the fifo-depth property to allow
> >>>> such nodes to specify their FIFO depth.
> >>>>
> >>>> We haven't seen SPI IPs with different FIFO depths for RX and TX, thus
> >>>> introduce a single property.
> >>>
> >>> Some citation attached to this would be nice. "We haven't seen" offers
> >>> no detail as to what IPs that allow this sort of configuration of FIFO
> >>> size that you have actually checked.
> >>>
> >>> I went and checked our IP that we use in FPGA fabric, which has a
> >>> configurable fifo depth. It only has a single knob for both RX and TX
> >>> FIFOs. The Xilinx xps spi core also has configurable FIFOs, but again RX
> >>> and TX sizes are tied there. At least that's a sample size of three.
> >>>
> >>> One of our guys is working on support for the IP I just mentioned and
> >>> would be defining a vendor property for this, so
> >>> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> >>>
> >>
> >> Thanks, Conor. I had in mind that SPI has a shift register and it's
> >> improbable to have different FIFO depths for RX and TX.
> >
> > IDK, but I've learned to expect the unexpectable, especially when it
> > comes to the IPs intended for use in FPGAs.
> >
> >> At least I don't
> >> see how it would work, I guess it will use the minimum depth between the
> >> two?
> >
> > I'm not really sure how it would work other than that in the general
> > case, but some use case specific configuration could work, but I do
> > agree that it is
> >
> >> I grepped by "fifo" in the spi bindings and I now see that renesas is
> >> using dedicated properties for RX and TX, but I think that there too the
> >> FIFOs have the same depths. Looking into drivers/spi/spi-sh-msiof.c I
> >> see that the of_device_id.data contains 64 bytes FIFOs for RX and TX,
> >> regardless of the compatible.
> >>
> >> Geert, any idea if the FIFO depths can differ for RX and TX in
> >> spi-sh-msiof.c?

See my other email
https://lore.kernel.org/all/CAMuHMdU_Hx9PLmHf2Xm1KKTy_OF-TeCv7SzmA5CZWz+PLkbAGA@mail.gmail.com

Note that at one point we did have 64/256 in the driver, but that was
changed in commit fe78d0b7691c0274 ("spi: sh-msiof: Fix FIFO size to
64 word from 256 word").  Diving into the discussion around that patch,
there seem to be two factors at play:
  1. Actual FIFO size,
  2. Maximum transfer size per block
     (up to 2 blocks on R-Car, up to 4 blocks on SH(-Mobile)).
As the driver supports only a single block, it should be limited to
64 on R-Car Gen2/3.  R-Car Gen4 claims to have widened the register
bit field for the maximum transfer size per block, so 256 might be
possible there...

> >> Anyway, even if there are such imbalanced architectures, I guess we can
> >> consider them when/if they appear? (add rx/tx-fifo-depth dt properties)
> >
> > I think so.
> >
> >> Documentation/devicetree/bindings/spi/renesas,sh-msiof.yaml:
> >> Override the default TX fifo size.  Unit is words.  Ignored if 0.
> >> Documentation/devicetree/bindings/spi/renesas,sh-msiof.yaml:
> >> renesas,rx-fifo-size:
> >> Documentation/devicetree/bindings/spi/renesas,sh-msiof.yaml:
> >> Override the default RX fifo size.  Unit is words.  Ignored if 0.
> >
> > These renesas ones seemed interesting at first glance due to these
> > comments, but what's missed by grep the is "deprecated" marking on
> > these. They seem to have been replaced by soc-specific compatibles.
>
> In the driver the renesas,{rx,tx}-fifo-size properties still have the
> highest priority:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git/tree/drivers/spi/spi-sh-msiof.c#n1350
>
> Maybe something for Geert, as I see he was the one marking these
> properties as deprecated. I guess he forgot to update the driver.
>
> Anyway, I think we shall be fine, even if we don't hear from Geert.

The renesas,{rx,tx}-fifo-size properties date back to the early days
of DT an ARM, when it was assumed that slightly different versions of
IP cores could be handled well using a single common compatible value,
and properties describing the (few) differences.  The pitfall here
is the "few differences": too many times people discovered later that
there were more differences, needing more properties, and complicating
backwards-compatibility.

Hence the handling of different FIFO sizes was moved to the driver based
on compatible values, and the renesas,{rx,tx}-fifo-size properties were
deprecated.  See commit beb74bb0875579c4 ("spi: sh-msiof: Add support
for R-Car H2 and M2"), which shows that there were more changes
needed than the anticipated FIFO sizes.  And more were added later,
see later additions to sh_msiof_chipdata.

So unless it is meant for a configurable synthesizable IP core, where
this is a documented parameter of the IP core, I advise against
specifying the FIFO size(s) in DT.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Tudor Ambarus Feb. 12, 2024, 12:01 p.m. UTC | #8 
On 2/12/24 10:38, Geert Uytterhoeven wrote:
> Hi Tudor,

Hi, Geert!

> 
> On Fri, Feb 9, 2024 at 5:55 PM Tudor Ambarus <tudor.ambarus@linaro.org> wrote:
>> On 2/9/24 16:21, Conor Dooley wrote:
>>> On Fri, Feb 09, 2024 at 01:56:56PM +0000, Tudor Ambarus wrote:
>>>> On 2/8/24 18:24, Conor Dooley wrote:
>>>>> On Thu, Feb 08, 2024 at 01:50:34PM +0000, Tudor Ambarus wrote:
>>>>>> There are instances of the same IP that are configured by the integrator
>>>>>> with different FIFO depths. Introduce the fifo-depth property to allow
>>>>>> such nodes to specify their FIFO depth.
>>>>>>
>>>>>> We haven't seen SPI IPs with different FIFO depths for RX and TX, thus
>>>>>> introduce a single property.
>>>>>
>>>>> Some citation attached to this would be nice. "We haven't seen" offers
>>>>> no detail as to what IPs that allow this sort of configuration of FIFO
>>>>> size that you have actually checked.
>>>>>
>>>>> I went and checked our IP that we use in FPGA fabric, which has a
>>>>> configurable fifo depth. It only has a single knob for both RX and TX
>>>>> FIFOs. The Xilinx xps spi core also has configurable FIFOs, but again RX
>>>>> and TX sizes are tied there. At least that's a sample size of three.
>>>>>
>>>>> One of our guys is working on support for the IP I just mentioned and
>>>>> would be defining a vendor property for this, so
>>>>> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
>>>>>
>>>>
>>>> Thanks, Conor. I had in mind that SPI has a shift register and it's
>>>> improbable to have different FIFO depths for RX and TX.
>>>
>>> IDK, but I've learned to expect the unexpectable, especially when it
>>> comes to the IPs intended for use in FPGAs.
>>>
>>>> At least I don't
>>>> see how it would work, I guess it will use the minimum depth between the
>>>> two?
>>>
>>> I'm not really sure how it would work other than that in the general
>>> case, but some use case specific configuration could work, but I do
>>> agree that it is
>>>
>>>> I grepped by "fifo" in the spi bindings and I now see that renesas is
>>>> using dedicated properties for RX and TX, but I think that there too the
>>>> FIFOs have the same depths. Looking into drivers/spi/spi-sh-msiof.c I
>>>> see that the of_device_id.data contains 64 bytes FIFOs for RX and TX,
>>>> regardless of the compatible.
>>>>
>>>> Geert, any idea if the FIFO depths can differ for RX and TX in
>>>> spi-sh-msiof.c?
> 
> See my other email
> https://lore.kernel.org/all/CAMuHMdU_Hx9PLmHf2Xm1KKTy_OF-TeCv7SzmA5CZWz+PLkbAGA@mail.gmail.com
> 

I saw the response, thanks again!

> Note that at one point we did have 64/256 in the driver, but that was
> changed in commit fe78d0b7691c0274 ("spi: sh-msiof: Fix FIFO size to
> 64 word from 256 word").  Diving into the discussion around that patch,
> there seem to be two factors at play:
>   1. Actual FIFO size,
>   2. Maximum transfer size per block
>      (up to 2 blocks on R-Car, up to 4 blocks on SH(-Mobile)).
> As the driver supports only a single block, it should be limited to
> 64 on R-Car Gen2/3.  R-Car Gen4 claims to have widened the register
> bit field for the maximum transfer size per block, so 256 might be
> possible there...

Got it.

> 
>>>> Anyway, even if there are such imbalanced architectures, I guess we can
>>>> consider them when/if they appear? (add rx/tx-fifo-depth dt properties)
>>>
>>> I think so.
>>>
>>>> Documentation/devicetree/bindings/spi/renesas,sh-msiof.yaml:
>>>> Override the default TX fifo size.  Unit is words.  Ignored if 0.
>>>> Documentation/devicetree/bindings/spi/renesas,sh-msiof.yaml:
>>>> renesas,rx-fifo-size:
>>>> Documentation/devicetree/bindings/spi/renesas,sh-msiof.yaml:
>>>> Override the default RX fifo size.  Unit is words.  Ignored if 0.
>>>
>>> These renesas ones seemed interesting at first glance due to these
>>> comments, but what's missed by grep the is "deprecated" marking on
>>> these. They seem to have been replaced by soc-specific compatibles.
>>
>> In the driver the renesas,{rx,tx}-fifo-size properties still have the
>> highest priority:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git/tree/drivers/spi/spi-sh-msiof.c#n1350
>>
>> Maybe something for Geert, as I see he was the one marking these
>> properties as deprecated. I guess he forgot to update the driver.
>>
>> Anyway, I think we shall be fine, even if we don't hear from Geert.
> 
> The renesas,{rx,tx}-fifo-size properties date back to the early days
> of DT an ARM, when it was assumed that slightly different versions of
> IP cores could be handled well using a single common compatible value,
> and properties describing the (few) differences.  The pitfall here
> is the "few differences": too many times people discovered later that
> there were more differences, needing more properties, and complicating
> backwards-compatibility.
> 
> Hence the handling of different FIFO sizes was moved to the driver based
> on compatible values, and the renesas,{rx,tx}-fifo-size properties were
> deprecated.  See commit beb74bb0875579c4 ("spi: sh-msiof: Add support
> for R-Car H2 and M2"), which shows that there were more changes
> needed than the anticipated FIFO sizes.  And more were added later,
> see later additions to sh_msiof_chipdata.
> 
> So unless it is meant for a configurable synthesizable IP core, where
> this is a documented parameter of the IP core, I advise against
> specifying the FIFO size(s) in DT.
> 

I guess I get it now. You marked those properties as deprecated so that
users stop using them and rely on the driver based compatible values,
but at the same time you allowed the devicetree properties to have a
higher priority than the driver based compatible values in case one
really wants/needs to use the dt properties. I don't have a preference
here, I guess it's fine.

Thanks for the explanations!
ta