MIME-Version: 1.0
References: <20211207210823.1975632-1-j.neuschaefer@gmx.net>
 <20211207210823.1975632-6-j.neuschaefer@gmx.net> <CAHp75Vew=M_ofNM5pmeHtTJHXRUbbO4RrtgYAtLBznTBm3CS6Q@mail.gmail.com>
 <YbC6Bv2teZ5CFhFQ@latitude>
In-Reply-To: <YbC6Bv2teZ5CFhFQ@latitude>
From:   Andy Shevchenko <andy.shevchenko@gmail.com>
Date:   Wed, 8 Dec 2021 16:14:38 +0200
Message-ID: <CAHp75VdYtLbCLi7iD0UT7MrPi9oxJLWA9ZCBo7uGbWxNCBdGyw@mail.gmail.com>
Subject: Re: [PATCH v2 5/8] pinctrl: nuvoton: Add driver for WPCM450
To:     =?UTF-8?Q?Jonathan_Neusch=C3=A4fer?= <j.neuschaefer@gmx.net>
Cc:     "linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        Linus Walleij <linus.walleij@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        "openbmc@lists.ozlabs.org" <openbmc@lists.ozlabs.org>,
        Tomer Maimon <tmaimon77@gmail.com>,
        Joel Stanley <joel@jms.id.au>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, Dec 8, 2021 at 3:58 PM Jonathan Neusch=C3=A4fer
<j.neuschaefer@gmx.net> wrote:
> On Wed, Dec 08, 2021 at 01:24:18PM +0200, Andy Shevchenko wrote:
> > On Tuesday, December 7, 2021, Jonathan Neusch=C3=A4fer <j.neuschaefer@g=
mx.net>
> > wrote:

...

> > > +       ours =3D ((1UL << gpio->num_irqs) - 1) << gpio->first_irq_bit=
;
> >
> > BIT()
>
> I'll use it, but in this case, I think it doesn't simplify much the
> whole expression all that much.

It is still better to use in my opinion.

> Is there perhaps a macro that
> constructs a continuous bitmask of N bits, perhaps additionally
> left-shifted by M bits?

> Maybe somewhere in the bitmap_* API...

Maybe, I dunno since I haven't clearly got this code anyway, so up to
you to check and see what to do about it.

...

> > > +       struct fwnode_handle *np;
> >
> >  Either be fully OF, or don=E2=80=99t name =E2=80=98np' here. We usuall=
y use fwnode or
> > =E2=80=98child=E2=80=99 in this case.
>
> Ah, I thought "np" (=3D node pointer) was still appropriate because I'm
> dealing with firmware _nodes_. My intention was indeed to switch fully
> to the fwnode API.

Just a convention "de facto".

...

> > > +               ret =3D gpiochip_add_pin_range(&gpio->gc,
> > > dev_name(pctrl->dev),
> > > +                                            0, bank->base, bank->len=
gth);
> > > +               if (ret) {
> > > +                       dev_err(pctrl->dev, "Failed to add pin range =
for
> > > GPIO bank %u\n", reg);
> > > +                       return ret;
> > > +               }
> >
> > Please move it to the corresponding callback.
>
> What's the corresponding callback?

https://elixir.bootlin.com/linux/latest/source/include/linux/gpio/driver.h#=
L400

--=20
With Best Regards,
Andy Shevchenko