diff mbox series

[1/9] dt-bindings: misc: Describe TI FPC202 dual port controller

Message ID 20241108-fpc202-v1-1-fe42c698bc92@bootlin.com (mailing list archive)
State New
Headers show
Series misc: Support TI FPC202 dual-port controller | expand

Commit Message

Romain Gantois Nov. 8, 2024, 3:36 p.m. UTC
The FPC202 dual port controller serves as a low speed signal aggregator for
common port types, notably SFP. It provides access to I2C and low-speed
GPIO signals of a downstream device through a single upstream control
interface.

Up to two logical I2C addresses can be accessed on each of the FPC202's
ports. The port controller acts as an I2C translator (ATR). It converts
addresses of incoming and outgoing I2C transactions. One use case of this
is accessing two SFP modules at logical address 0x50 from the same upstream
I2C controller, using two different client aliases.

Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
---
 .../devicetree/bindings/misc/ti,fpc202.yaml        | 75 ++++++++++++++++++++++
 1 file changed, 75 insertions(+)

Comments

Conor Dooley Nov. 8, 2024, 6:23 p.m. UTC | #1
On Fri, Nov 08, 2024 at 04:36:53PM +0100, Romain Gantois wrote:
> The FPC202 dual port controller serves as a low speed signal aggregator for
> common port types, notably SFP. It provides access to I2C and low-speed
> GPIO signals of a downstream device through a single upstream control
> interface.
> 
> Up to two logical I2C addresses can be accessed on each of the FPC202's
> ports. The port controller acts as an I2C translator (ATR). It converts
> addresses of incoming and outgoing I2C transactions. One use case of this
> is accessing two SFP modules at logical address 0x50 from the same upstream
> I2C controller, using two different client aliases.
> 
> Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
> ---
>  .../devicetree/bindings/misc/ti,fpc202.yaml        | 75 ++++++++++++++++++++++
>  1 file changed, 75 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/misc/ti,fpc202.yaml b/Documentation/devicetree/bindings/misc/ti,fpc202.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..ad11abe11e68aa266acdd6b43a5b425340bbbba8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/ti,fpc202.yaml
> @@ -0,0 +1,75 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/misc/ti,fpc202.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI FPC202 dual port controller with expanded IOs
> +
> +maintainers:
> +  - Romain Gantois <romain.gantois@bootlin.com>
> +
> +allOf:
> +  - $ref: /schemas/i2c/i2c-atr.yaml#

Gotta say, this looks absolutely nothing like the other i2c-atr user!

> +
> +properties:
> +  compatible:
> +    const: ti,fpc202
> +
> +  reg:
> +    maxItems: 1
> +
> +  gpio-controller: true
> +
> +  "#gpio-cells":
> +    const: 2
> +
> +  enable-gpios:
> +    description:
> +      Specifier for the GPIO connected to the EN pin.
> +    maxItems: 1
> +
> +  port0:

ports usually go in a ports node, and are port@0 not port0. That said,
these are i2c buses, so the node name would usually be i2c@ for those.
In fact, given you have i2c-mux as your node name, the binding for that
expects you to format your child nodes like '^i2c@[0-9a-f]+$'. Is there
a reason you can't just drop this ports business and go with a pattern
property here that restricts the pattern to '^i2c@[0-1]$'?

> +    $ref: /schemas/i2c/i2c-controller.yaml
> +    description:
> +      Device port 0, accessible over I2C.
> +
> +  port1:
> +    $ref: /schemas/i2c/i2c-controller.yaml
> +    description:
> +      Device port 1, accessible over I2C.
> +
> +
> +required:
> +  - compatible
> +  - gpio-controller
> +  - "#gpio-cells"
> +  - reg
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        fpc202: i2c-mux@f {

The label here is uused, you should drop it.

Cheers,
Conor.

> +            compatible = "ti,fpc202";
> +            reg = <0xf>;
> +
> +            gpio-controller;
> +            #gpio-cells = <2>;
> +
> +            port0 {
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +            };
> +
> +            port1 {
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +            };
> +        };
> +    };
> +...
> 
> -- 
> 2.47.0
>
Romain Gantois Nov. 13, 2024, 9:46 a.m. UTC | #2
Hello Conor,

On vendredi 8 novembre 2024 19:23:37 heure normale d’Europe centrale Conor Dooley wrote:
> On Fri, Nov 08, 2024 at 04:36:53PM +0100, Romain Gantois wrote:
...
> > index
> > 0000000000000000000000000000000000000000..ad11abe11e68aa266acdd6b43a5b425
> > 340bbbba8 --- /dev/null
> > +++ b/Documentation/devicetree/bindings/misc/ti,fpc202.yaml
> > @@ -0,0 +1,75 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/misc/ti,fpc202.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: TI FPC202 dual port controller with expanded IOs
> > +
> > +maintainers:
> > +  - Romain Gantois <romain.gantois@bootlin.com>
> > +
> > +allOf:
> > +  - $ref: /schemas/i2c/i2c-atr.yaml#
> 
> Gotta say, this looks absolutely nothing like the other i2c-atr user!

Indeed, the critical difference between the two is that the existing
user has a global alias pool whereas this component doesn't. So
the "i2c-alias-pool" property isn't relevant here, and it's currently
the only property defined by the i2c-atr binding.

We did consider defining a per-channel alias pool in the i2c-atr binding
but the results were quite awkward and it didn't seem like this property
belonged in the device tree at all, since the alias values were hardwired
into the FPC202 and were known in advance.

> 
> > +
> > +properties:
> > +  compatible:
> > +    const: ti,fpc202
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  gpio-controller: true
> > +
> > +  "#gpio-cells":
> > +    const: 2
> > +
> > +  enable-gpios:
> > +    description:
> > +      Specifier for the GPIO connected to the EN pin.
> > +    maxItems: 1
> > +
> 
> > +  port0:
> ports usually go in a ports node, and are port@0 not port0. That said,
> these are i2c buses, so the node name would usually be i2c@ for those.
> In fact, given you have i2c-mux as your node name, the binding for that
> expects you to format your child nodes like '^i2c@[0-9a-f]+$'. Is there
> a reason you can't just drop this ports business and go with a pattern
> property here that restricts the pattern to '^i2c@[0-1]$'?

I didn't think of restricting the pattern in this way, that is indeed more
appropriate than using static port names.

Moreover, I don't think that the "i2c-mux" naming is appropriate here,
as the FPC202 isn't a mux at all. I'll look for a better name for the next
iteration.

Thanks,
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/misc/ti,fpc202.yaml b/Documentation/devicetree/bindings/misc/ti,fpc202.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..ad11abe11e68aa266acdd6b43a5b425340bbbba8
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/ti,fpc202.yaml
@@ -0,0 +1,75 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/misc/ti,fpc202.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI FPC202 dual port controller with expanded IOs
+
+maintainers:
+  - Romain Gantois <romain.gantois@bootlin.com>
+
+allOf:
+  - $ref: /schemas/i2c/i2c-atr.yaml#
+
+properties:
+  compatible:
+    const: ti,fpc202
+
+  reg:
+    maxItems: 1
+
+  gpio-controller: true
+
+  "#gpio-cells":
+    const: 2
+
+  enable-gpios:
+    description:
+      Specifier for the GPIO connected to the EN pin.
+    maxItems: 1
+
+  port0:
+    $ref: /schemas/i2c/i2c-controller.yaml
+    description:
+      Device port 0, accessible over I2C.
+
+  port1:
+    $ref: /schemas/i2c/i2c-controller.yaml
+    description:
+      Device port 1, accessible over I2C.
+
+
+required:
+  - compatible
+  - gpio-controller
+  - "#gpio-cells"
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        fpc202: i2c-mux@f {
+            compatible = "ti,fpc202";
+            reg = <0xf>;
+
+            gpio-controller;
+            #gpio-cells = <2>;
+
+            port0 {
+                #address-cells = <1>;
+                #size-cells = <0>;
+            };
+
+            port1 {
+                #address-cells = <1>;
+                #size-cells = <0>;
+            };
+        };
+    };
+...