linux-kernel - Re: [PATCH v3 1/2] dt-bindings: i2c-mux: Add property for settle time

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAL_JsqJYLgmPu8ACFcbwdj9GukPiSpMGsZWr6x659WMNyZEL9A@mail.gmail.com>
Date:   Wed, 3 Nov 2021 09:13:19 -0500
From:   Rob Herring <robh@...nel.org>
To:     Peter Rosin <peda@...ntia.se>
Cc:     Horatiu Vultur <horatiu.vultur@...rochip.com>,
        Linux I2C <linux-i2c@...r.kernel.org>,
        devicetree@...r.kernel.org,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 1/2] dt-bindings: i2c-mux: Add property for settle time

On Wed, Nov 3, 2021 at 6:45 AM Peter Rosin <peda@...ntia.se> wrote:
>
> On 2021-11-02 23:27, Horatiu Vultur wrote:
> > The 11/02/2021 13:37, Rob Herring wrote:
> >> On Mon, Nov 01, 2021 at 10:32:01PM +0100, Horatiu Vultur wrote:
> >>> The 11/01/2021 15:32, Peter Rosin wrote:
>
> *snip*
>
> >>>
> >>> +required:
> >>> +  - compatible
> >>
> >> compatible should not be required here.
> >>
> >>> +  - '#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.

> >> 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