linux-kernel - Re: fw_devlink on will break all snps,dw-apb-gpio users

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:   Wed, 14 Oct 2020 10:29:36 -0700
From:   Saravana Kannan <saravanak@...gle.com>
To:     Jisheng Zhang <Jisheng.Zhang@...aptics.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Frank Rowand <frowand.list@...il.com>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>
Subject: Re: fw_devlink on will break all snps,dw-apb-gpio users

On Wed, Oct 14, 2020 at 4:12 AM Jisheng Zhang
<Jisheng.Zhang@...aptics.com> wrote:
>
> Hi,
>
> If set fw_devlink as on, any consumers of dw apb gpio won't probe.
>
> The related dts looks like:
>
> gpio0: gpio@...0 {
>        compatible = "snps,dw-apb-gpio";
>        #address-cells = <1>;
>        #size-cells = <0>;
>
>        porta: gpio-port@0 {
>               compatible = "snps,dw-apb-gpio-port";
>               gpio-controller;
>               #gpio-cells = <2>;
>               ngpios = <32>;
>               reg = <0>;
>        };
> };
>
> device_foo {
>         status = "okay"
>         ...;
>         reset-gpio = <&porta, 0, GPIO_ACTIVE_HIGH>;
> };
>
> If I change the reset-gpio property to use another kind of gpio phandle,
> e.g gpio expander, then device_foo can be probed successfully.
>
> The gpio expander dt node looks like:
>
>         expander3: gpio@44 {
>                 compatible = "fcs,fxl6408";
>                 pinctrl-names = "default";
>                 pinctrl-0 = <&expander3_pmux>;
>                 reg = <0x44>;
>                 gpio-controller;
>                 #gpio-cells = <2>;
>                 interrupt-parent = <&portb>;
>                 interrupts = <23 IRQ_TYPE_NONE>;
>                 interrupt-controller;
>                 #interrupt-cells = <2>;
>         };
>
> The common pattern looks like the devlink can't cope with suppliers from
> child dt node.

fw_devlink doesn't have any problem dealing with child devices being
suppliers. The problem with your case is that the
drivers/gpio/gpio-dwapb.c driver directly parses the child nodes and
never creates struct devices for them. If you have a node with
compatible string, fw_devlink expects you to create and probe a struct
device for it. So change your driver to add the child devices as
devices instead of just parsing the node directly and doing stuff with
it.

Either that, or stop putting "compatible" string in a node if you
don't plan to actually treat it as a device -- but that's too late for
this driver (it needs to be backward compatible). So change the driver
to add of_platform_populate() and write a driver that probes
"snps,dw-apb-gpio-port".

-Saravana