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

[Yemike Abhilash Chandra <y-abhilashchandra@ti.com>]

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@ti.com> 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@ti.com>
> 
> 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