diff mbox series

[RFC,04/11] i2c: dt-bindings: configuration settings

Message ID 20240506225139.57647-5-kyarlagadda@nvidia.com
State Rejected
Headers show
Series Introduce Tegra register config settings | expand

Commit Message

Krishna Yarlagadda May 6, 2024, 10:51 p.m. UTC
I2C interface timing registers are configured using config setting
framework. Document available properties for Tegra I2C controllers.

Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
---
 .../bindings/i2c/nvidia,tegra20-i2c.yaml      | 104 ++++++++++++++++++
 1 file changed, 104 insertions(+)

Comments

Krzysztof Kozlowski May 7, 2024, 6:34 a.m. UTC | #1
On 07/05/2024 00:51, Krishna Yarlagadda wrote:
> I2C interface timing registers are configured using config setting
> framework. Document available properties for Tegra I2C controllers.
> 
> Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> ---
>  .../bindings/i2c/nvidia,tegra20-i2c.yaml      | 104 ++++++++++++++++++
>  1 file changed, 104 insertions(+)
> 

You called it RFC, so not ready for review, thus just few remarks.

Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters


> diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
> index 424a4fc218b6..3b22e75e5aa0 100644
> --- a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
> +++ b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
> @@ -119,6 +119,96 @@ properties:
>        - const: rx
>        - const: tx
>  
> +  config:
> +    description: Config settings for I2C devices enlisted with I2C controller.
> +      Config setting is the configuration based on chip/board/system
> +      characterization on interface/controller settings. This is needed for
> +      - making the controller internal configuration to better perform
> +      - making the interface to work proper by setting drive strength, slew
> +        rates etc
> +      - making the low power leakage.
> +      There are two types of recommended configuration settings
> +      - Controller register specific for internal operation of controller.
> +      - Pad control/Pinmux/pincontrol registers for interfacing.
> +      These configurations can further be categorized as static and dynamic.
> +      - Static config does not change until a controller is reset.
> +      - Dynamic config changes based on mode or condition, controller is
> +        operating in.
> +      I2C has configuration based on clock speed and has below modes.
> +      - common is set on all speeds and can be overridden by speed mode.
> +      - high is set when clock mode is high speed.
> +      - fastplus is set when clock mode is fast plus.
> +      - fast is set when clock mode is fast mode.
> +      - standard is set when clock mode is standard mode.
> +    $ref: /schemas/misc/nvidia,tegra-config-settings.yaml
> +    unevaluatedProperties: false
> +    properties:
> +      nvidia,i2c-clk-divisor-hs-mode:
> +        description: I2C clock divisor for HS mode.

So you decided to implement clocks in DT? No.

> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        minimum: 0
> +        maximum: 0xffff

Anyway divisors, clock rates and everything is human-readable so in
decimal, not hex.

There are also several issues further, like using wrong units (time has
a unit), but since this is RFC, I will just NAK.

Best regards,
Krzysztof
Rob Herring May 7, 2024, 12:35 p.m. UTC | #2
On Tue, 07 May 2024 04:21:32 +0530, Krishna Yarlagadda wrote:
> I2C interface timing registers are configured using config setting
> framework. Document available properties for Tegra I2C controllers.
> 
> Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> ---
>  .../bindings/i2c/nvidia,tegra20-i2c.yaml      | 104 ++++++++++++++++++
>  1 file changed, 104 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.example.dts:37.20-50.15: Warning (i2c_bus_reg): /example-0/i2c@7000c000/config: missing or empty reg property

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240506225139.57647-5-kyarlagadda@nvidia.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
index 424a4fc218b6..3b22e75e5aa0 100644
--- a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
@@ -119,6 +119,96 @@  properties:
       - const: rx
       - const: tx
 
+  config:
+    description: Config settings for I2C devices enlisted with I2C controller.
+      Config setting is the configuration based on chip/board/system
+      characterization on interface/controller settings. This is needed for
+      - making the controller internal configuration to better perform
+      - making the interface to work proper by setting drive strength, slew
+        rates etc
+      - making the low power leakage.
+      There are two types of recommended configuration settings
+      - Controller register specific for internal operation of controller.
+      - Pad control/Pinmux/pincontrol registers for interfacing.
+      These configurations can further be categorized as static and dynamic.
+      - Static config does not change until a controller is reset.
+      - Dynamic config changes based on mode or condition, controller is
+        operating in.
+      I2C has configuration based on clock speed and has below modes.
+      - common is set on all speeds and can be overridden by speed mode.
+      - high is set when clock mode is high speed.
+      - fastplus is set when clock mode is fast plus.
+      - fast is set when clock mode is fast mode.
+      - standard is set when clock mode is standard mode.
+    $ref: /schemas/misc/nvidia,tegra-config-settings.yaml
+    unevaluatedProperties: false
+    properties:
+      nvidia,i2c-clk-divisor-hs-mode:
+        description: I2C clock divisor for HS mode.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 0xffff
+      nvidia,i2c-clk-divisor-fs-mode:
+        description: I2C clock divisor for FS mode.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 0xffff
+      nvidia,i2c-hs-sclk-high-period:
+        description: I2C high speed sclk high period.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 0xffff
+      nvidia,i2c-hs-sclk-low-period:
+        description: I2C high speed sclk low period.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 0xffff
+      nvidia,i2c-hs-stop-setup-time:
+        description: I2C high speed stop setup time.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 0xffff
+      nvidia,i2c-hs-start-hold-time:
+        description: I2C high speed start hold time.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 0xffff
+      nvidia,i2c-hs-start-setup-time:
+        description: I2C high speed start setup time.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 0xffff
+      nvidia,i2c-sclk-high-period:
+        description: I2C sclk high period.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 0xffff
+      nvidia,i2c-sclk-low-period:
+        description: I2C sclk low period.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 0xffff
+      nvidia,i2c-bus-free-time:
+        description: I2C bus free time.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 0xffff
+      nvidia,i2c-stop-setup-time:
+        description: I2C stop setup time.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 0xffff
+      nvidia,i2c-start-hold-time:
+        description: I2C start hold time.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 0xffff
+      nvidia,i2c-start-setup-time:
+        description: I2C start setup time.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 0xffff
+
 allOf:
   - $ref: /schemas/i2c/i2c-controller.yaml
   - if:
@@ -189,4 +279,18 @@  examples:
 
         #address-cells = <1>;
         #size-cells = <0>;
+        config {
+            common {
+                nvidia,i2c-hs-sclk-high-period = <0x03>;
+                nvidia,i2c-hs-sclk-low-period = <0x08>;
+            };
+            fast {
+                nvidia,i2c-clk-divisor-fs-mode = <0x3c>;
+                nvidia,i2c-sclk-high-period = <0x02>;
+            };
+            fastplus {
+                nvidia,i2c-clk-divisor-fs-mode = <0x4f>;
+                nvidia,i2c-sclk-high-period = <0x07>;
+            };
+        };
     };