Re: [PATCH 2/3] pinctrl: ti: Add TDA54 pin controller driver

[Yemike Abhilash Chandra]

Hi Linus,

On 02/10/26 02:22, Linus Walleij wrote:
> Hi Yemike,
>
> thanks for your patch!

Thank you for the review and apologies for the delayed response.

> On Wed, Sep 30, 2026 at 11:21 AM Yemike Abhilash Chandra
> <y-abhilashchandra@xxxxxx> wrote:
>
> > The TDA54 pin controller splits the pad configuration across two registers:
> > PADCFGA holds the mux mode and the functional pin configuration, PADCFGB
> > holds the wakeup and retention configuration. The controller also supports
> > two proxy addressing regions and claiming of registers. A register claimed
> > through proxy 1 becomes read-only at proxy 0 and can only be written
> > through proxy 1.
> >
> > Add support for the pin controller in the TI TDA54 SoC.
> >
> > Signed-off-by: Yemike Abhilash Chandra <y-abhilashchandra@xxxxxx>
>
> Overall this looks pretty nice.

Thanks !!

> Check the Sashiko reviews, there is always something
> interesting there.

Yes, found a few interesting things there. Will reply to that
separately.

> > +config PINCTRL_TDA54
> > +       tristate "TI TDA54 pinctrl driver"
> > +       depends on OF && (ARCH_K3 || COMPILE_TEST)
> > +       select GENERIC_PINCTRL_GROUPS
> > +       select GENERIC_PINMUX_FUNCTIONS
> > +       select GENERIC_PINCONF
>
> Just
>
> select GENERIC_PINCTRL
>
> and you get all three.

Done. Thanks.

> > +static int tda54_dt_node_to_map(struct pinctrl_dev *pctldev,
> > +                               struct device_node *np,
> > +                               struct pinctrl_map **maps,
> > +                               unsigned int *num_maps)
>
> What is so unique here that you need your own node to map function?
>
> Can't you use pinctrl_generic_pinmux_dt_node_to_map()?

I can, thanks for the pointer. I wasn't aware of the generic helper.
Switched to pinctrl_generic_pinmux_dt_node_to_map() and it works fine.

Thanks and regards,
Yemike Abhilash Chandra

> Please consult the code in drivers/pinctrl/pinctrl-generic.c.
>
> Yours,
> Linus Walleij