Re: [PATCH 01/12] spi: dt-bindings: introduce the ``fifo-depth`` property

[Tudor Ambarus <tudor.ambarus@xxxxxxxxxx>]

+ 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@xxxxxxxxxxxxx>
> 

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>;