RE: [RFC PATCH 0/6] Add Pinctrl driver for Starfive JH8100 SoC

[Yuklin Soo <yuklin.soo@xxxxxxxxxxxxxxxx>]

> -----Original Message-----
> From: Linus Walleij <linus.walleij@xxxxxxxxxx>
> Sent: Saturday, December 23, 2023 1:59 AM
> To: Yuklin Soo <yuklin.soo@xxxxxxxxxxxxxxxx>
> Cc: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>; Hal Feng
> <hal.feng@xxxxxxxxxxxxxxxx>; Leyfoon Tan <leyfoon.tan@xxxxxxxxxxxxxxxx>;
> Jianlong Huang <jianlong.huang@xxxxxxxxxxxxxxxx>; Emil Renner Berthing
> <kernel@xxxxxxxx>; Rob Herring <robh@xxxxxxxxxx>; Krzysztof Kozlowski
> <krzysztof.kozlowski+dt@xxxxxxxxxx>; Conor Dooley <conor+dt@xxxxxxxxxx>;
> Drew Fustini <drew@xxxxxxxxxxxxxxx>; linux-gpio@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-
> riscv@xxxxxxxxxxxxxxxxxxx; Paul Walmsley <paul.walmsley@xxxxxxxxxx>;
> Palmer Dabbelt <palmer@xxxxxxxxxxx>; Albert Ou
> <aou@xxxxxxxxxxxxxxxxx>
> Subject: Re: [RFC PATCH 0/6] Add Pinctrl driver for Starfive JH8100 SoC
> 
> Hi Alex,
> 
> thanks for your patch!
> 
> On Thu, Dec 21, 2023 at 9:36 AM Alex Soo <yuklin.soo@xxxxxxxxxxxxxxxx>
> wrote:
> 
> >   pinctrl: starfive: jh8100: add pinctrl driver for sys_east domain
> >   pinctrl: starfive: jh8100: add pinctrl driver for sys_west domain
> >   pinctrl: starfive: jh8100: add pinctrl driver for sys_gmac domain
> >   pinctrl: starfive: jh8100: add pinctrl driver for AON domain
> 
> To my eye it looks like a lot of code is duplicated between the four subdrivers.
> 
> The pattern from other pin controllers is to create a file with all the common
> code and then subdrivers for each sub-pincontroller that have their own
> probe but calls into the library.
> 
> C.f.
> drivers/pinctrl/qcom/pinctrl-apq8064.c:
> 
> static int apq8064_pinctrl_probe(struct platform_device *pdev) {
>         return msm_pinctrl_probe(pdev, &apq8064_pinctrl); }
> 
> And that function is in drivers/pinctrl/qcom/pinctrl-msm.c
> and you find great inspiration in the qcom Kconfig and Makefile and
> drivers/pinctrl/qcom/pinctrl-msm.h
> that you can copypaste to pull this off.
> 
> Maybe you should start with a patch that extract the common stuff from the
> existing jh7100/jh7110 drivers and then reuse that for jh8100?

The duplicated codes in the subdrivers have been moved to the main driver.
The subdrivers will call those functions from the main driver.
The test results are successful and will submit in the next version for review.

> 
> Yours,
> Linus Walleij