Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20201014191235.7f71fcb4@xhacker.debian>
In-Reply-To: <20201014191235.7f71fcb4@xhacker.debian>
From:   Saravana Kannan <saravanak@google.com>
Date:   Wed, 14 Oct 2020 10:29:36 -0700
Message-ID: <CAGETcx9PiX==mLxB9PO8Myyk6u2vhPVwTMsA5NkD-ywH5xhusw@mail.gmail.com>
Subject: Re: fw_devlink on will break all snps,dw-apb-gpio users
To:     Jisheng Zhang <Jisheng.Zhang@synaptics.com>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Frank Rowand <frowand.list@gmail.com>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@vger.kernel.org>,
        linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Oct 14, 2020 at 4:12 AM Jisheng Zhang
<Jisheng.Zhang@synaptics.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@2400 {
>        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