| Message ID | 20240208135045.3728927-2-tudor.ambarus@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | spi: s3c64xx: remove OF alias ID dependency | expand |
Hi Tudor, On Thu, Feb 8, 2024 at 2:51 PM Tudor Ambarus <tudor.ambarus@linaro.org> 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. Ha... Current documentation for the Clock-Synchronized Serial Interface with FIFO (MSIOF) on e.g. R-Car Gen2 and later states: FIFO capacity: 32 bits × 64 stages for transmission and 32 bits × 256 stages for reception Initially (many years ago), there was some doubt about the validity of these values (older variants on SH supported 64/64), hence drivers/spi/spi-sh-msiof.c still has .tx_fifo_size = 64, .rx_fifo_size = 64, Probably we should test and revisit this... > --- 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. I think it is prudent to consider the asymmetric case, too. Whether that should be just two properties ("rx-fifo-depth" and "tx-fifo-depth"), or also a third "fifo-depth", I defer to the DT maintainers... > + > num-cs: > $ref: /schemas/types.yaml#/definitions/uint32 > description: Gr{oetje,eeting}s, Geert
On 09/02/2024 18:13, Geert Uytterhoeven wrote: > Hi Tudor, > > On Thu, Feb 8, 2024 at 2:51 PM Tudor Ambarus <tudor.ambarus@linaro.org> 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. > > Ha... > > Current documentation for the Clock-Synchronized Serial Interface with > FIFO (MSIOF) on e.g. R-Car Gen2 and later states: > > FIFO capacity: 32 bits × 64 stages for transmission and 32 bits × > 256 stages for reception > > Initially (many years ago), there was some doubt about the validity > of these values (older variants on SH supported 64/64), hence > drivers/spi/spi-sh-msiof.c still has > > .tx_fifo_size = 64, > .rx_fifo_size = 64, > > Probably we should test and revisit this... > >> --- 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. > > I think it is prudent to consider the asymmetric case, too. > Whether that should be just two properties ("rx-fifo-depth" and > "tx-fifo-depth"), or also a third "fifo-depth", I defer to the DT > maintainers... Since most of the cases FIFO depth tx=rx, we could go with three properties and: allOf: - not: required: - fifo-depth - tx-fifo-depth - not: required: - fifo-depth - rx-fifo-depth and probably dependencies between rx and tx (see example-schema). Best regards, Krzysztof
Hi, Geert, Krzysztof, On 2/11/24 13:49, Krzysztof Kozlowski wrote:> On 09/02/2024 18:13, Geert Uytterhoeven wrote: >> Hi Tudor, >> >> On Thu, Feb 8, 2024 at 2:51 PM Tudor Ambarus <tudor.ambarus@linaro.org> 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. >> >> Ha... >> >> Current documentation for the Clock-Synchronized Serial Interface with >> FIFO (MSIOF) on e.g. R-Car Gen2 and later states: >> >> FIFO capacity: 32 bits × 64 stages for transmission and 32 bits × >> 256 stages for reception >> >> Initially (many years ago), there was some doubt about the validity >> of these values (older variants on SH supported 64/64), hence >> drivers/spi/spi-sh-msiof.c still has >> >> .tx_fifo_size = 64, >> .rx_fifo_size = 64, >> >> Probably we should test and revisit this... >> >>> --- 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. >> >> I think it is prudent to consider the asymmetric case, too. >> Whether that should be just two properties ("rx-fifo-depth" and >> "tx-fifo-depth"), or also a third "fifo-depth", I defer to the DT >> maintainers... Thanks, Geert for the insight! > > Since most of the cases FIFO depth tx=rx, we could go with three > properties and: > > allOf: > - not: > required: > - fifo-depth > - tx-fifo-depth > - not: > required: > - fifo-depth > - rx-fifo-depth > > and probably dependencies between rx and tx (see example-schema). > Great. Thanks, Krzysztof! I'll give it a try. Cheers, ta
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:
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. Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org> --- Documentation/devicetree/bindings/spi/spi-controller.yaml | 5 +++++ 1 file changed, 5 insertions(+)