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

[Yuklin Soo]

> -----Original Message-----
> From: Linus Walleij <linus.walleij at linaro.org>
> Sent: Saturday, December 23, 2023 1:59 AM
> To: Yuklin Soo <yuklin.soo at starfivetech.com>
> Cc: Bartosz Golaszewski <bartosz.golaszewski at linaro.org>; Hal Feng
> <hal.feng at starfivetech.com>; Leyfoon Tan <leyfoon.tan at starfivetech.com>;
> Jianlong Huang <jianlong.huang at starfivetech.com>; Emil Renner Berthing
> <kernel at esmil.dk>; Rob Herring <robh at kernel.org>; Krzysztof Kozlowski
> <krzysztof.kozlowski+dt at linaro.org>; Conor Dooley <conor+dt at kernel.org>;
> Drew Fustini <drew at beagleboard.org>; linux-gpio at vger.kernel.org; linux-
> kernel at vger.kernel.org; devicetree at vger.kernel.org; linux-
> riscv at lists.infradead.org; Paul Walmsley <paul.walmsley at sifive.com>;
> Palmer Dabbelt <palmer at dabbelt.com>; Albert Ou
> <aou at eecs.berkeley.edu>
> 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 at starfivetech.com>
> 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