On Wed, Nov 3, 2021 at 6:45 AM Peter Rosin [off-list ref] wrote:

On 2021-11-02 23:27, Horatiu Vultur wrote:

quoted

The 11/02/2021 13:37, Rob Herring wrote:

quoted

On Mon, Nov 01, 2021 at 10:32:01PM +0100, Horatiu Vultur wrote:

quoted

The 11/01/2021 15:32, Peter Rosin wrote:

*snip*

quoted

quoted

quoted

+required:
+  - compatible

compatible should not be required here.

quoted

+  - '#address-cells'
+  - '#size-cells'
+
 examples:
   - |
     /*
---

If I have this then my problem is with the required properties because then I
start to get new warnings once I run:

make ARCH=arm CROSS_COMPILE=arm-linux- dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/i2c/i2c-mux.yaml

For example, one of new the warnings is this:

/home/hvultur/linux/arch/arm/boot/dts/am335x-icev2.dt.yaml: mux-mii-hog: 'compatible' is a required property
      From schema: /home/hvultur/linux/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
/home/hvultur/linux/arch/arm/boot/dts/am335x-icev2.dt.yaml: mux-mii-hog: '#address-cells' is a required property
      From schema: /home/hvultur/linux/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
/home/hvultur/linux/arch/arm/boot/dts/am335x-icev2.dt.yaml: mux-mii-hog: '#size-cells' is a required property
      From schema: /home/hvultur/linux/Documentation/devicetree/bindings/i2c/i2c-mux.yaml

There's actually a ton of 'mux' nodes that should be causing warnings too.

quoted

quoted

This is because of the $nodename pattern being pretty lax and matches
on mux-mii-hog by mistake. We have 2 options. Change the nodename
pattern to '^(i2c-?)?mux(@.*)?$' or add 'select: false'. The former
would still match on 'mux' or 'mux@.*' which might still have problems.
For the latter, we just need to make sure all the i2c-mux schemas have a
$ref to this schema. Also, with that change we'd stop checking 'i2c-mux'
nodes that don't yet have a specific schema. That said, I do lean toward
the latter option.

From what I can see there are only two i2c-mux schemas and both of them
have a $ref to this schema [1][2]

[1] https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/i2c/i2c-mux-gpmux.yaml#L33
[2] https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml#L16

I'm a relative yaml bindings newbie, but I assume adding "select: false" will
have the side effect of not enforcing this i2c-mux schema on i2c-muxes that
have not yet been converted to yaml? E.g. i2c-mux-gpio.txt, i2c-mux-pinctrl.txt
etc etc. But there are not too many of those. Is it a prerequisite to update
those bindings to yaml before doing "select: false"?

No. We may be losing some checks temporarily, but we've got plenty of
other warnings to keep busy. And most cases in tree seem to be pca954x
anyways.

Looking further I think
there's a total of about 15-20 drivers doing i2c-muxing (or arbing/gating),
and some of those exist outside the "i2c umbrella".

I wonder if e.g. this one [1] should really reference i2c-controller.yaml as
it is currently doing, or if i2c-mux.yaml is correct?

[1] Documentation/devicetree/bindings/power/supply/sbs,sbs-manager.yaml

Maybe i2c-mux.yaml didn't work in that case because the node names were
"wrong" and did not match the pattern and then someone stuck
i2c-controller.yaml in there simply because that was close enough, and
also happened to work?

While the device does have muxing capability, it does much more and
the use is rather specific. So I think it is fine.

Rob