diff mbox series

[v2,1/2] dt-bindings: i2c: snps,designware-i2c: add bus-capacitance-pf and clk-freq-optimized

Message ID 20240927042230.277144-2-michael.wu@kneron.us
State New
Headers show
Series Compute HS HCNT and LCNT based on HW parameters | expand

Commit Message

Michael Wu Sept. 27, 2024, 4:22 a.m. UTC
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.

bus-capacitance-pf indicates the bus capacitance in picofarad (pF). It
affects the high and low pulse width of SCL line in high speed mode. The
only legal values for this property are 100 and 400, which are used to
calculate the tHIGH and tLOW periods for high speed mode. This property
corresponds to IC_CAP_LOADING.

clk-freq-optimized indicates that the hardware input clock frequency is
reduced by reducing the internal latency. The property affects the high
period and low period of the SCL line. The property conrresponds to
IC_CLK_FREQ_OPTIMIZATION.

Signed-off-by: Michael Wu <michael.wu@kneron.us>
---
 .../bindings/i2c/snps,designware-i2c.yaml          | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Krzysztof Kozlowski Sept. 27, 2024, 8:35 a.m. UTC | #1
On Fri, Sep 27, 2024 at 12:22:16PM +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.
> 
> bus-capacitance-pf indicates the bus capacitance in picofarad (pF). It
> affects the high and low pulse width of SCL line in high speed mode. The
> only legal values for this property are 100 and 400, which are used to
> calculate the tHIGH and tLOW periods for high speed mode. This property
> corresponds to IC_CAP_LOADING.
> 
> clk-freq-optimized indicates that the hardware input clock frequency is
> reduced by reducing the internal latency. The property affects the high
> period and low period of the SCL line. The property conrresponds to
> IC_CLK_FREQ_OPTIMIZATION.
> 
> Signed-off-by: Michael Wu <michael.wu@kneron.us>
> ---
>  .../bindings/i2c/snps,designware-i2c.yaml          | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
> index 60035a787e5c..fc19e6a8b306 100644
> --- a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
> +++ b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
> @@ -87,6 +87,20 @@ properties:
>        This value is used to compute the tHIGH period.
>      default: 300
>  
> +  bus-capacitance-pf:

Why is this generic property? Is this going to be applied to all I2C
controllers? If not, you miss vendors prefix.

> +    description: |
> +      This property represents the bus capacitance in picofarad (pF). It
> +      affects the high and low pulse width of SCL line in high speed mode.
> +      The only legal values for this property are 100 and 400, which are used
> +      to calculate the tHIGH and tLOW periods for high speed mode.
> +    default: 100
> +
> +  clk-freq-optimized:

This was never tested.

You got this comment already and not much improved.

It does not look like you tested the bindings, at least after quick
look. Please run  (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).
Maybe you need to update your dtschema and yamllint.

> +    description: |

Do not need '|' unless you need to preserve formatting.

Missing vendor prefix. Missing tests.

Also, extend the example with this.


> +      If the hardware input clock frequency is reduced by reducing the
> +      internal latency, this property must be declared in the device tree. It
> +      affects the high period and low period of SCL line.

I assume this is hardware choice, not driver?

Best regards,
Krzysztof
Michael Wu Sept. 28, 2024, 7:07 a.m. UTC | #2
On Fri, Sep 27 2024 at 10:35:09AM +0200, Krzysztof Kozlowski wrote:
> On Fri, Sep 27, 2024 at 12:22:16PM +0800, Michael Wu wrote:
...
> > index 60035a787e5c..fc19e6a8b306 100644
> > --- a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
> > +++ b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
> > @@ -87,6 +87,20 @@ properties:
> >        This value is used to compute the tHIGH period.
> >      default: 300
> >
> > +  bus-capacitance-pf:
> 
> Why is this generic property? Is this going to be applied to all I2C
> controllers? If not, you miss vendors prefix.
>
> > +    description: |
> > +      This property represents the bus capacitance in picofarad (pF). It
> > +      affects the high and low pulse width of SCL line in high speed mode.
> > +      The only legal values for this property are 100 and 400, which are
> used
> > +      to calculate the tHIGH and tLOW periods for high speed mode.
> > +    default: 100
> > +
> > +  clk-freq-optimized:
> 
> This was never tested.
> 
> You got this comment already and not much improved.
> 
> It does not look like you tested the bindings, at least after quick
> look. Please run  (see
> Documentation/devicetree/bindings/writing-schema.rst for instructions).
> Maybe you need to update your dtschema and yamllint.
> 
> > +    description: |
> 
> Do not need '|' unless you need to preserve formatting.
> 
> Missing vendor prefix. Missing tests.
> 
> Also, extend the example with this.
> 

Both are snps,designware-i2c properties, and should be renamed to
snps,bus-capacitance-pf and snps,clk-freq-optimized.

I apologize for my lack of understanding of binding.

> 
> > +      If the hardware input clock frequency is reduced by reducing the
> > +      internal latency, this property must be declared in the device tree. It
> > +      affects the high period and low period of SCL line.
> 
> I assume this is hardware choice, not driver?

Yes, they are hardware properties. The driver must use this information
from the device tree to calculate SCL high and low periods appropriate
for the hardware.

Thanks & Regards,
Michael
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
index 60035a787e5c..fc19e6a8b306 100644
--- a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
@@ -87,6 +87,20 @@  properties:
       This value is used to compute the tHIGH period.
     default: 300
 
+  bus-capacitance-pf:
+    description: |
+      This property represents the bus capacitance in picofarad (pF). It
+      affects the high and low pulse width of SCL line in high speed mode.
+      The only legal values for this property are 100 and 400, which are used
+      to calculate the tHIGH and tLOW periods for high speed mode.
+    default: 100
+
+  clk-freq-optimized:
+    description: |
+      If the hardware input clock frequency is reduced by reducing the
+      internal latency, this property must be declared in the device tree. It
+      affects the high period and low period of SCL line.
+
   dmas:
     items:
       - description: TX DMA Channel