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

[Andy Shevchenko]

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

> +       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

> +#include <linux/platform_device.h>

Besides that kernel.h for ARRAY_SIZE() init.h for arch_initcall() and
others might be missing.

-- 
With Best Regards,
Andy Shevchenko