diff mbox series

[RFC,V2,04/12] dt-bindings: misc: tegra-i2c: config settings

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

Commit Message

Krishna Yarlagadda July 1, 2024, 3:12 p.m. UTC
I2C interface timing registers are configured using config setting
framework. List available field properties for Tegra I2C controllers.

Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
---
 .../misc/nvidia,tegra-config-settings.yaml    | 83 +++++++++++++++++--
 1 file changed, 74 insertions(+), 9 deletions(-)

Comments

Thierry Reding July 5, 2024, 10:51 a.m. UTC | #1
On Wed, Jul 03, 2024 at 02:21:04PM GMT, Rob Herring wrote:
> On Tue, Jul 2, 2024 at 4:29 AM Thierry Reding <thierry.reding@gmail.com> wrote:
> >
> > On Mon, Jul 01, 2024 at 11:42:27AM GMT, Rob Herring wrote:
> > > On Mon, Jul 01, 2024 at 08:42:22PM +0530, Krishna Yarlagadda wrote:
> > > > I2C interface timing registers are configured using config setting
> > > > framework. List available field properties for Tegra I2C controllers.
> > >
> > > How is I2C bus timing parameters specific to NVIDIA? Just because you
> > > have more controls? No. That's no reason to invent a whole new way to
> > > specify parameters. Extend what's already there and make it work for
> > > anyone.
> >
> > This may be applicable to a subset of this, and yes, maybe we can find
> > generalizations for some of these parameters.
> >
> > However, we're also looking for feedback specifically on these config
> > nodes that go beyond individual timing parameters. For example in the
> > case of I2C, how should parameters for different operating modes be
> > described?
> 
> Like what? It all looks like timing to me.

The problem here isn't the individual properties but rather how to group
them. More generally the problem is that we have a set of settings that
need to be applied in different variants. Yes, they are all timings, but
the values differ based on what mode a given controller operates at.

Take for example I2C where we have things like start-hold time or stop-
setup time, which we could describe in a more generic way (i.e. leave
out the vendor prefix). However, depending on the mode that the I2C
controller runs at (could be standard mode, fast mode or fastplus mode)
these values need to be adjusted.

So it's the same set of properties but with different values for each
different operating mode. As far as I can tell there's no good construct
to describe this in DT currently.

> > Would you agree with something along the lines provided in this series?
> 
> When there are multiple users/vendors of it, maybe.
> 
> In general, it goes against the DT design of properties for foo go in
> foo's node. This looks more like how ACPI does things where it's add
> another table for this new thing we need.

Well, that's what Krishna had proposed in the first version of the
series, which you guys rejected. The problem with that is that we cannot
easily group these settings using nodes because subnodes of I2C
controllers are considered to be clients by default. This applies to SPI
and other busses as well.

This approach avoids these issues and can be more easily optimized since
settings could be shared between multiple instances of the controllers.
I have a slight preference of putting this into the controllers' device
tree nodes, but I can't think of a good way of avoiding the above child
node problem other than what we had in v1.

Thierry
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/misc/nvidia,tegra-config-settings.yaml b/Documentation/devicetree/bindings/misc/nvidia,tegra-config-settings.yaml
index 4e5d52504c01..5f4da633e69b 100644
--- a/Documentation/devicetree/bindings/misc/nvidia,tegra-config-settings.yaml
+++ b/Documentation/devicetree/bindings/misc/nvidia,tegra-config-settings.yaml
@@ -38,17 +38,74 @@  patternProperties:
     additionalProperties: false
 
     patternProperties:
-      "^[a-z0-9_]+-cfg$":
-        description:
-          Config profiles applied conditionally.
+      "^i2c-[a-z0-9_]+-cfg$":
+        description: Config settings for I2C devices.
         type: object
-        patternProperties:
-	  "nvidia,[a-z0-9_]+$":
-	    description:
-	      Register field configuration.
-	    $ref: /schemas/types.yaml#/definitions/uint32
+        additionalProperties: false
 
-additionalProperties: true
+        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
+
+additionalProperties: false
 
 examples:
   - |
@@ -58,5 +115,13 @@  examples:
                 nvidia,i2c-hs-sclk-high-period = <0x03>;
                 nvidia,i2c-hs-sclk-low-period = <0x08>;
             };
+            i2c-fast-cfg {
+                nvidia,i2c-clk-divisor-fs-mode = <0x3c>;
+                nvidia,i2c-sclk-high-period = <0x02>;
+            };
+            i2c-fastplus-cfg {
+                nvidia,i2c-clk-divisor-fs-mode = <0x4f>;
+                nvidia,i2c-sclk-high-period = <0x07>;
+            };
         };
     };