| Message ID | 20241001082937.680372-2-michael.wu@kneron.us |
|---|---|
| State | Changes Requested |
| Delegated to: | Andi Shyti |
| Headers | show |
| Series | Compute HS HCNT and LCNT based on HW parameters | expand |
On Tue, Oct 01, 2024 at 04:29:33PM +0800, Michael Wu wrote: > Since there are no registers controlling the hardware parameters > IC_CAP_LOADING and IC_CLK_FREQ_OPTIMIZATION, their values can only be > declared in the device tree. > > snps,bus-capacitance-pf indicates the bus capacitance in picofarads (pF). > It affects the high and low pulse width of SCL line in high speed mode. > The legal values for this property are 100 and 400 only, and default > value is 100. This property corresponds to IC_CAP_LOADING. > > snps,clk-freq-optimized indicates whether the hardware input clock > frequency is reduced by reducing the internal latency. This property > corresponds to IC_CLK_FREQ_OPTIMIZATION. > > The driver can calculate hs_hcnt and hs_lcnt appropriate for the hardware > based on these two properties. > > Signed-off-by: Michael Wu <michael.wu@kneron.us> > --- > .../bindings/i2c/snps,designware-i2c.yaml | 24 +++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml > index 60035a787e5c..c373f3acd34b 100644 > --- a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml > +++ b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml > @@ -97,6 +97,21 @@ properties: > - const: tx > - const: rx > > + snps,bus-capacitance-pf: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > I asked to drop |, so you replaced it with something else? So drop >... and then you are going to replace it with another one? That's not a cat and mouse. > + This property indicates the bus capacitance in picofarads (pF). > + This value is used to compute the tHIGH and tLOW periods for high speed > + mode. > + default: 100 I asked for some constraints here. min/maximum. I think you never replied to this. > + > + snps,clk-freq-optimized: > + description: > > + This property indicates whether the hardware input clock frequency is > + reduced by reducing the internal latency. This value is used to compute > + the tHIGH and tLOW periods for high speed mode. > + type: boolean > + > unevaluatedProperties: false > > required: > @@ -146,4 +161,13 @@ examples: > interrupts = <8>; > clocks = <&ahb_clk>; > }; > + - | > + i2c@c5000000 { > + compatible = "snps,designware-i2c"; Extend EXISTING example. Not add new example. Best regards, Krzysztof
> On Tue, Oct 01, 2024 at 04:29:33PM +0800, Michael Wu wrote: > > Since there are no registers controlling the hardware parameters > > IC_CAP_LOADING and IC_CLK_FREQ_OPTIMIZATION, their values can only be > > declared in the device tree. > > > > snps,bus-capacitance-pf indicates the bus capacitance in picofarads (pF). > > It affects the high and low pulse width of SCL line in high speed mode. > > The legal values for this property are 100 and 400 only, and default > > value is 100. This property corresponds to IC_CAP_LOADING. > > > > snps,clk-freq-optimized indicates whether the hardware input clock > > frequency is reduced by reducing the internal latency. This property > > corresponds to IC_CLK_FREQ_OPTIMIZATION. > > > > The driver can calculate hs_hcnt and hs_lcnt appropriate for the hardware > > based on these two properties. > > > > Signed-off-by: Michael Wu <michael.wu@kneron.us> > > --- > > .../bindings/i2c/snps,designware-i2c.yaml | 24 > +++++++++++++++++++ > > 1 file changed, 24 insertions(+) > > > > diff --git > a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml > b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml > > index 60035a787e5c..c373f3acd34b 100644 > > --- a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml > > +++ b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml ... > > + This property indicates the bus capacitance in picofarads (pF). > > + This value is used to compute the tHIGH and tLOW periods for high > speed > > + mode. > > + default: 100 > > I asked for some constraints here. min/maximum. I think you never > replied to this. > In I2C DesignWare Databook v2.03a the mandatory option is provided to select whether the bus capacitance is 400pF or 100pF. It presents the description like that: Description: For high speed mode, the bus loading (pF) affects the high and low pulse width of SCL. Values: 100, 400 Default Value: 100 Parameter Name: IC_CAP_LOADING There is no further information describing this option except to the declaration of legal values above, let alone minimum and maximum limits. As a user I don't think I have the right to define a value range for the vendor. From the information provided in the data sheet, I prefer to list the legal values like the following: enum: [100, 400] default: 100 instead of declaring its range. What do you think? In patches v2 I used if (dev->bus_capacitance_pf == 400) {... } else {...} and other statements in the driver code to indicate that the capacitance can only be 400pf or not. Maybe this is a metaphor. I'm sorry that I wasn't more explicit about the constraints. > > + > > + snps,clk-freq-optimized: > > + description: > > > + This property indicates whether the hardware input clock frequency > is > > + reduced by reducing the internal latency. This value is used to > compute > > + the tHIGH and tLOW periods for high speed mode. > > + type: boolean > > + > > unevaluatedProperties: false > > > > required: > > @@ -146,4 +161,13 @@ examples: > > interrupts = <8>; > > clocks = <&ahb_clk>; > > }; > > + - | > > + i2c@c5000000 { > > + compatible = "snps,designware-i2c"; > > Extend EXISTING example. Not add new example. Should I insert these two properties into one or all existing examples? Thanks & Regards Michael
> On Tue, Oct 01, 2024 at 04:29:33PM +0800, Michael Wu wrote: > > Since there are no registers controlling the hardware parameters > > IC_CAP_LOADING and IC_CLK_FREQ_OPTIMIZATION, their values can only be > > declared in the device tree. > > > > snps,bus-capacitance-pf indicates the bus capacitance in picofarads (pF). > > It affects the high and low pulse width of SCL line in high speed mode. > > The legal values for this property are 100 and 400 only, and default > > value is 100. This property corresponds to IC_CAP_LOADING. > > > > snps,clk-freq-optimized indicates whether the hardware input clock > > frequency is reduced by reducing the internal latency. This property > > corresponds to IC_CLK_FREQ_OPTIMIZATION. > > > > The driver can calculate hs_hcnt and hs_lcnt appropriate for the hardware > > based on these two properties. > > > > Signed-off-by: Michael Wu <michael.wu@kneron.us> > > --- > > .../bindings/i2c/snps,designware-i2c.yaml | 24 > +++++++++++++++++++ > > 1 file changed, 24 insertions(+) > > > > diff --git > a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml > b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml > > index 60035a787e5c..c373f3acd34b 100644 > > --- a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml > > +++ b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml ... > > + This property indicates the bus capacitance in picofarads (pF). > > + This value is used to compute the tHIGH and tLOW periods for high > speed > > + mode. > > + default: 100 > > I asked for some constraints here. min/maximum. I think you never > replied to this. > In I2C DesignWare Databook v2.03a the mandatory option is provided to select whether the bus capacitance is 400pF or 100pF. It presents the description like that: Description: For high speed mode, the bus loading (pF) affects the high and low pulse width of SCL. Values: 100, 400 Default Value: 100 Parameter Name: IC_CAP_LOADING There is no further information describing this option except to the declaration of legal values above, let alone minimum and maximum limits. As a user I don't think I have the right to define a value range for the vendor. From the information provided in the data sheet, I prefer to list the legal values like the following: enum: [100, 400] default: 100 instead of declaring its range. What do you think? In patches v2 I used if (dev->bus_capacitance_pf == 400) {... } else {...} and other statements in the driver code to indicate that the capacitance can only be 400pf or not. Maybe this is a metaphor. I'm sorry that I wasn't more explicit about the constraints. > > + > > + snps,clk-freq-optimized: > > + description: > > > + This property indicates whether the hardware input clock frequency > is > > + reduced by reducing the internal latency. This value is used to > compute > > + the tHIGH and tLOW periods for high speed mode. > > + type: boolean > > + > > unevaluatedProperties: false > > > > required: > > @@ -146,4 +161,13 @@ examples: > > interrupts = <8>; > > clocks = <&ahb_clk>; > > }; > > + - | > > + i2c@c5000000 { > > + compatible = "snps,designware-i2c"; > > Extend EXISTING example. Not add new example. Should I insert these two properties into one or all existing examples? Thanks & Regards Michael
On 02/10/2024 11:20, Michael Wu wrote: >> On Tue, Oct 01, 2024 at 04:29:33PM +0800, Michael Wu wrote: >>> Since there are no registers controlling the hardware parameters >>> IC_CAP_LOADING and IC_CLK_FREQ_OPTIMIZATION, their values can only be >>> declared in the device tree. >>> >>> snps,bus-capacitance-pf indicates the bus capacitance in picofarads (pF). >>> It affects the high and low pulse width of SCL line in high speed mode. >>> The legal values for this property are 100 and 400 only, and default >>> value is 100. This property corresponds to IC_CAP_LOADING. >>> >>> snps,clk-freq-optimized indicates whether the hardware input clock >>> frequency is reduced by reducing the internal latency. This property >>> corresponds to IC_CLK_FREQ_OPTIMIZATION. >>> >>> The driver can calculate hs_hcnt and hs_lcnt appropriate for the hardware >>> based on these two properties. >>> >>> Signed-off-by: Michael Wu <michael.wu@kneron.us> >>> --- >>> .../bindings/i2c/snps,designware-i2c.yaml | 24 >> +++++++++++++++++++ >>> 1 file changed, 24 insertions(+) >>> >>> diff --git >> a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml >> b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml >>> index 60035a787e5c..c373f3acd34b 100644 >>> --- a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml >>> +++ b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml > ... >>> + This property indicates the bus capacitance in picofarads (pF). >>> + This value is used to compute the tHIGH and tLOW periods for high >> speed >>> + mode. >>> + default: 100 >> >> I asked for some constraints here. min/maximum. I think you never >> replied to this. >> > > In I2C DesignWare Databook v2.03a the mandatory option is provided to > select whether the bus capacitance is 400pF or 100pF. It presents the > description like that: > > Description: > For high speed mode, the bus loading (pF) affects the high and low > pulse width of SCL. > Values: 100, 400 > Default Value: 100 > Parameter Name: IC_CAP_LOADING > > There is no further information describing this option except to the > declaration of legal values above, let alone minimum and maximum limits. So only two values are valid? Then not min/max but enum. > As a user I don't think I have the right to define a value range for the > vendor. > > From the information provided in the data sheet, I prefer to list the > legal values like the following: > > enum: [100, 400] > default: 100 > > instead of declaring its range. What do you think? > > In patches v2 I used if (dev->bus_capacitance_pf == 400) {... } else {...} > and other statements in the driver code to indicate that the capacitance > can only be 400pf or not. Maybe this is a metaphor. I'm sorry that I > wasn't more explicit about the constraints. > >>> + >>> + snps,clk-freq-optimized: >>> + description: > >>> + This property indicates whether the hardware input clock frequency >> is >>> + reduced by reducing the internal latency. This value is used to >> compute >>> + the tHIGH and tLOW periods for high speed mode. >>> + type: boolean >>> + >>> unevaluatedProperties: false >>> >>> required: >>> @@ -146,4 +161,13 @@ examples: >>> interrupts = <8>; >>> clocks = <&ahb_clk>; >>> }; >>> + - | >>> + i2c@c5000000 { >>> + compatible = "snps,designware-i2c"; >> >> Extend EXISTING example. Not add new example. > > Should I insert these two properties into one or all existing examples? Into any example, where it looks reasonable. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml index 60035a787e5c..c373f3acd34b 100644 --- a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml +++ b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml @@ -97,6 +97,21 @@ properties: - const: tx - const: rx + snps,bus-capacitance-pf: + $ref: /schemas/types.yaml#/definitions/uint32 + description: > + This property indicates the bus capacitance in picofarads (pF). + This value is used to compute the tHIGH and tLOW periods for high speed + mode. + default: 100 + + snps,clk-freq-optimized: + description: > + This property indicates whether the hardware input clock frequency is + reduced by reducing the internal latency. This value is used to compute + the tHIGH and tLOW periods for high speed mode. + type: boolean + unevaluatedProperties: false required: @@ -146,4 +161,13 @@ examples: interrupts = <8>; clocks = <&ahb_clk>; }; + - | + i2c@c5000000 { + compatible = "snps,designware-i2c"; + reg = <0xc5000000 0x1000>; + interrupts = <37 1>; + clock-frequency = <400000>; + snps,bus-capacitance-pf = <400>; + snps,clk-freq-optimized; + }; ...
Since there are no registers controlling the hardware parameters IC_CAP_LOADING and IC_CLK_FREQ_OPTIMIZATION, their values can only be declared in the device tree. snps,bus-capacitance-pf indicates the bus capacitance in picofarads (pF). It affects the high and low pulse width of SCL line in high speed mode. The legal values for this property are 100 and 400 only, and default value is 100. This property corresponds to IC_CAP_LOADING. snps,clk-freq-optimized indicates whether the hardware input clock frequency is reduced by reducing the internal latency. This property corresponds to IC_CLK_FREQ_OPTIMIZATION. The driver can calculate hs_hcnt and hs_lcnt appropriate for the hardware based on these two properties. Signed-off-by: Michael Wu <michael.wu@kneron.us> --- .../bindings/i2c/snps,designware-i2c.yaml | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+)