Re: [PATCH V7 4/8] pinctrl: qcom: Add IPQ5018 pinctrl driver

[Sricharan Ramabadhran]

Hi Andy,

On 5/20/2023 12:17 AM, Andy Shevchenko wrote:
> On Fri, May 19, 2023 at 3:55 PM Sricharan Ramabadhran
> <quic_srichara@xxxxxxxxxxx> wrote:
> > Add pinctrl definitions for the TLMM of IPQ5018.
>
> A couple of remarks either for the next version of the series or for
> the follow ups.
>
> ...
>
> > +config PINCTRL_IPQ5018
> > +       tristate "Qualcomm Technologies, Inc. IPQ5018 pin controller driver"
>
> > +       depends on GPIOLIB && OF
>
> I'm wondering why OF.
> If it's a functional dependency (I do not see compile-time one) the
> compile test can be added, no?
>
>    depends on GPIOLIB
>    depends on OF || COMPILE_TEST

 Yeah sure. COMPILE_TEST could be standalone. Will fix it and repost.

> > +       select PINCTRL_MSM
> > +       help
> > +         This is the pinctrl, pinmux, pinconf and gpiolib driver for
> > +         the Qualcomm Technologies Inc. TLMM block found on the
> > +         Qualcomm Technologies Inc. IPQ5018 platform. Select this for
> > +         IPQ5018.
>
> ...
>
> > +#include <linux/module.h>
>
> > +#include <linux/of.h>
>
> There is a wrong header (the code doesn't use this one).
> You meant mod_devicetable.h

 ho ok, let me check this part.

> > +#include <linux/platform_device.h>
>
> Besides that kernel.h for ARRAY_SIZE() init.h for arch_initcall() and
> others might be missing.

 init.h is getting included from module.h. Will check for kernel.h and
 update accordingly.

Regards,
 Sricharan