linux-kernel - Re: [PATCH v2 16/18] arm64: dts: hi3660: Harmonize DWC USB3 DT nodes name

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] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 21 Dec 2020 12:24:11 -0800
From:   John Stultz <john.stultz@...aro.org>
To:     Krzysztof Kozlowski <krzk@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Serge Semin <Sergey.Semin@...kalelectronics.ru>,
        Felipe Balbi <balbi@...nel.org>,
        Florian Fainelli <f.fainelli@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Wei Xu <xuwei5@...ilicon.com>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        Serge Semin <fancer.lancer@...il.com>,
        lkml <linux-kernel@...r.kernel.org>,
        YongQin Liu <yongqin.liu@...aro.org>,
        Amit Pundir <amit.pundir@...aro.org>,
        Todd Kjos <tkjos@...gle.com>
Subject: Re: [PATCH v2 16/18] arm64: dts: hi3660: Harmonize DWC USB3 DT nodes name

On Sat, Dec 19, 2020 at 3:06 AM Krzysztof Kozlowski <krzk@...nel.org> wrote:
> On Fri, Dec 18, 2020 at 09:11:42PM -0800, John Stultz wrote:
> > On Wed, Nov 11, 2020 at 1:22 AM Serge Semin
> > <Sergey.Semin@...kalelectronics.ru> wrote:
> > >
> > > In accordance with the DWC USB3 bindings the corresponding node
> > > name is suppose to comply with the Generic USB HCD DT schema, which
> > > requires the USB nodes to have the name acceptable by the regexp:
> > > "^usb(@.*)?" . Make sure the "snps,dwc3"-compatible nodes are correctly
> > > named.
> > >
> > > Signed-off-by: Serge Semin <Sergey.Semin@...kalelectronics.ru>
> > > Acked-by: Krzysztof Kozlowski <krzk@...nel.org>
> > > ---
> > >  arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> > > index d25aac5e0bf8..aea3800029b5 100644
> > > --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> > > +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> > > @@ -1166,7 +1166,7 @@ usb_phy: usb-phy {
> > >                         };
> > >                 };
> > >
> > > -               dwc3: dwc3@...00000 {
> > > +               dwc3: usb@...00000 {
> > >                         compatible = "snps,dwc3";
> > >                         reg = <0x0 0xff100000 0x0 0x100000>;
> >
> >
> > Oof. So this patch is breaking the usb gadget functionality on HiKey960 w/ AOSP.
> >
> > In order to choose the right controller for gadget mode with AOSP, one
> > sets the "sys.usb.controller" property, which until now for HiKey960
> > has been "ff100000.dwc3".
> > After this patch, the controller isn't found and we would have to
> > change userland to use "ff100000.usb", which would then break booting
> > on older kernels (testing various LTS releases on AOSP is one of the
> > key uses of the HiKey960).
> >
> > So while I understand the desire to unify the schema, as HiKey960
> > really isn't likely to be used outside of AOSP, I wonder if reverting
> > this one change is in the best interest of not breaking existing
> > userland?
>
> The node names are not part of an ABI, are they? I expect only
> compatibles and properties to be stable. If user-space looks for
> something by name, it's a user-space's mistake.  Not mentioning that you
> also look for specific address... Imagine remapping of addresses with
> ranges (for whatever reason) - AOSP also would be broken? Addresses are
> definitely not an ABI.

Though that is how it's exported through sysfs.

In AOSP it is then used to setup the configfs gadget by writing that
value into /config/usb_gadget/g1/UDC.

Given there may be multiple controllers on a device, or even if its
just one and the dummy hcd driver is enabled, I'm not sure how folks
reference the "right" one without the node name?

I understand the fuzziness with sysfs ABI, and I get that having
consistent naming is important, but like the eth0 -> enp3s0 changes,
it seems like this is going to break things.

Greg? Is there some better way AOSP should be doing this?

thanks
-john