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

[Romain Gantois <romain.gantois@bootlin.com>]

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,

-- 
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com