| Message ID | 20230205230208.58355-2-lars@metafoo.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/2] i2c: cadence: Allow to specify the FIFO depth and maximum transfer length | expand |
On 2/6/23 00:02, Lars-Peter Clausen wrote: > The depth of the FIFO of the Cadence I2C controller IP is a synthesis > configuration parameter. Different instances of the IP can have different > values. For correct operation software needs to be aware of the size of the > FIFO. > > Add the documentation for the devicetree property that describes the FIFO > depth of the IP core. > > The default value of 16 is for backwards compatibility reasons with > existing hardware descriptions where this property is not specified and > software has assumed that the FIFO depth is 16. > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> > --- > Documentation/devicetree/bindings/i2c/cdns,i2c-r1p10.yaml | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/Documentation/devicetree/bindings/i2c/cdns,i2c-r1p10.yaml b/Documentation/devicetree/bindings/i2c/cdns,i2c-r1p10.yaml > index 2e95cda7262a..3daa2fa73257 100644 > --- a/Documentation/devicetree/bindings/i2c/cdns,i2c-r1p10.yaml > +++ b/Documentation/devicetree/bindings/i2c/cdns,i2c-r1p10.yaml > @@ -38,6 +38,12 @@ properties: > description: | > Input clock name. > > + cdns,fifo-depth: > + description: > + Size of the data FIFO in words. > + $ref: "/schemas/types.yaml#/definitions/uint32" > + default: 16 > + > required: > - compatible > - reg > @@ -57,4 +63,6 @@ examples: > clock-frequency = <400000>; > #address-cells = <1>; > #size-cells = <0>; > + nit: Remove this newline. > + cdns,fifo-depth = <8>; > }; This should be 1/2 before the patch which actually use it. M
On 06/02/2023 00:02, Lars-Peter Clausen wrote: > The depth of the FIFO of the Cadence I2C controller IP is a synthesis > configuration parameter. Different instances of the IP can have different > values. For correct operation software needs to be aware of the size of the > FIFO. Cannot this be inferred from compatible? > > Add the documentation for the devicetree property that describes the FIFO > depth of the IP core. > > The default value of 16 is for backwards compatibility reasons with > existing hardware descriptions where this property is not specified and > software has assumed that the FIFO depth is 16. > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> > --- > Documentation/devicetree/bindings/i2c/cdns,i2c-r1p10.yaml | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/Documentation/devicetree/bindings/i2c/cdns,i2c-r1p10.yaml b/Documentation/devicetree/bindings/i2c/cdns,i2c-r1p10.yaml > index 2e95cda7262a..3daa2fa73257 100644 > --- a/Documentation/devicetree/bindings/i2c/cdns,i2c-r1p10.yaml > +++ b/Documentation/devicetree/bindings/i2c/cdns,i2c-r1p10.yaml > @@ -38,6 +38,12 @@ properties: > description: | > Input clock name. > > + cdns,fifo-depth: > + description: > + Size of the data FIFO in words. > + $ref: "/schemas/types.yaml#/definitions/uint32" Drop quotes. > + default: 16 maximum minimum? Best regards, Krzysztof
Thanks for the review. On 2/5/23 23:41, Krzysztof Kozlowski wrote: > On 06/02/2023 00:02, Lars-Peter Clausen wrote: >> The depth of the FIFO of the Cadence I2C controller IP is a synthesis >> configuration parameter. Different instances of the IP can have different >> values. For correct operation software needs to be aware of the size of the >> FIFO. > Cannot this be inferred from compatible? The compatible string currently encodes the version of the IP core. The FIFO depth is an orthogonal setting. You could of course encode both the version and FIFO depth as part of the compatible string. But you'd end up with a combinatorial explosion of compatible strings. Like cdns,i2c-r1p10-fifo-depth-2, ... cdns,i2c-r1p10-fifo-depth-16, ..., cdns,i2c-r1p10-fifo-depth-256, cdns,i2c-r1p14-fifo-depth-2, ... cdns,i2c-r1p14-fifo-depth-16, ..., cdns,i2c-r1p14-fifo-depth-256, > >> Add the documentation for the devicetree property that describes the FIFO >> depth of the IP core. >> >> The default value of 16 is for backwards compatibility reasons with >> existing hardware descriptions where this property is not specified and >> software has assumed that the FIFO depth is 16. >> >> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> >> --- >> Documentation/devicetree/bindings/i2c/cdns,i2c-r1p10.yaml | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/i2c/cdns,i2c-r1p10.yaml b/Documentation/devicetree/bindings/i2c/cdns,i2c-r1p10.yaml >> index 2e95cda7262a..3daa2fa73257 100644 >> --- a/Documentation/devicetree/bindings/i2c/cdns,i2c-r1p10.yaml >> +++ b/Documentation/devicetree/bindings/i2c/cdns,i2c-r1p10.yaml >> @@ -38,6 +38,12 @@ properties: >> description: | >> Input clock name. >> >> + cdns,fifo-depth: >> + description: >> + Size of the data FIFO in words. >> + $ref: "/schemas/types.yaml#/definitions/uint32" > Drop quotes. > >> + default: 16 > maximum > minimum? Will add a enum, since it has to be a power of two.
On Sun, Feb 05, 2023 at 03:02:08PM -0800, Lars-Peter Clausen wrote: > The depth of the FIFO of the Cadence I2C controller IP is a synthesis > configuration parameter. Different instances of the IP can have different > values. For correct operation software needs to be aware of the size of the > FIFO. > > Add the documentation for the devicetree property that describes the FIFO > depth of the IP core. > > The default value of 16 is for backwards compatibility reasons with > existing hardware descriptions where this property is not specified and > software has assumed that the FIFO depth is 16. > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> > --- > Documentation/devicetree/bindings/i2c/cdns,i2c-r1p10.yaml | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/Documentation/devicetree/bindings/i2c/cdns,i2c-r1p10.yaml b/Documentation/devicetree/bindings/i2c/cdns,i2c-r1p10.yaml > index 2e95cda7262a..3daa2fa73257 100644 > --- a/Documentation/devicetree/bindings/i2c/cdns,i2c-r1p10.yaml > +++ b/Documentation/devicetree/bindings/i2c/cdns,i2c-r1p10.yaml > @@ -38,6 +38,12 @@ properties: > description: | > Input clock name. > > + cdns,fifo-depth: We already have: fifo-size rx-fifo-size tx-fifo-size fifo-depth tx-fifo-depth rx-fifo-depth And we have cdns,fifo-depth too (among other vendor specific ones), but pick a non-vendor specific one. > + description: > + Size of the data FIFO in words. What's the word size? Use bytes. > + $ref: "/schemas/types.yaml#/definitions/uint32" > + default: 16 > + > required: > - compatible > - reg > @@ -57,4 +63,6 @@ examples: > clock-frequency = <400000>; > #address-cells = <1>; > #size-cells = <0>; > + > + cdns,fifo-depth = <8>; > }; > -- > 2.30.2 >
diff --git a/Documentation/devicetree/bindings/i2c/cdns,i2c-r1p10.yaml b/Documentation/devicetree/bindings/i2c/cdns,i2c-r1p10.yaml index 2e95cda7262a..3daa2fa73257 100644 --- a/Documentation/devicetree/bindings/i2c/cdns,i2c-r1p10.yaml +++ b/Documentation/devicetree/bindings/i2c/cdns,i2c-r1p10.yaml @@ -38,6 +38,12 @@ properties: description: | Input clock name. + cdns,fifo-depth: + description: + Size of the data FIFO in words. + $ref: "/schemas/types.yaml#/definitions/uint32" + default: 16 + required: - compatible - reg @@ -57,4 +63,6 @@ examples: clock-frequency = <400000>; #address-cells = <1>; #size-cells = <0>; + + cdns,fifo-depth = <8>; };
The depth of the FIFO of the Cadence I2C controller IP is a synthesis configuration parameter. Different instances of the IP can have different values. For correct operation software needs to be aware of the size of the FIFO. Add the documentation for the devicetree property that describes the FIFO depth of the IP core. The default value of 16 is for backwards compatibility reasons with existing hardware descriptions where this property is not specified and software has assumed that the FIFO depth is 16. Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> --- Documentation/devicetree/bindings/i2c/cdns,i2c-r1p10.yaml | 8 ++++++++ 1 file changed, 8 insertions(+)