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

Alex Soo posted 6 patches 2 years ago
There is a newer version of this series
.../pinctrl/starfive,jh8100-aon-pinctrl.yaml  |  183 +++
.../starfive,jh8100-sys-east-pinctrl.yaml     |  188 +++
.../starfive,jh8100-sys-gmac-pinctrl.yaml     |  124 ++
.../starfive,jh8100-sys-west-pinctrl.yaml     |  188 +++
MAINTAINERS                                   |    7 +
arch/riscv/boot/dts/starfive/jh8100-evb.dts   |    5 +
arch/riscv/boot/dts/starfive/jh8100-pinfunc.h |  418 +++++++
arch/riscv/boot/dts/starfive/jh8100.dtsi      |   44 +
drivers/pinctrl/starfive/Kconfig              |   57 +
drivers/pinctrl/starfive/Makefile             |    6 +
.../starfive/pinctrl-starfive-jh8100-aon.c    |  241 ++++
.../pinctrl-starfive-jh8100-sys-east.c        |  326 +++++
.../pinctrl-starfive-jh8100-sys-gmac.c        |  164 +++
.../pinctrl-starfive-jh8100-sys-west.c        |  264 ++++
.../starfive/pinctrl-starfive-jh8100.c        | 1090 +++++++++++++++++
.../starfive/pinctrl-starfive-jh8100.h        |   85 ++
.../pinctrl/starfive,jh8100-pinctrl.h         |  303 +++++
17 files changed, 3693 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh8100-aon-pinctrl.yaml
create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh8100-sys-east-pinctrl.yaml
create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh8100-sys-gmac-pinctrl.yaml
create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh8100-sys-west-pinctrl.yaml
create mode 100644 arch/riscv/boot/dts/starfive/jh8100-pinfunc.h
create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive-jh8100-aon.c
create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive-jh8100-sys-east.c
create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive-jh8100-sys-gmac.c
create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive-jh8100-sys-west.c
create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive-jh8100.c
create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive-jh8100.h
create mode 100644 include/dt-bindings/pinctrl/starfive,jh8100-pinctrl.h
[RFC PATCH 0/6] Add Pinctrl driver for Starfive JH8100 SoC
Posted by Alex Soo 2 years ago
Starfive JH8100 SoC consists of 4 pinctrl domains - sys_east,
sys_west, sys_gmac, and aon. This patch series adds pinctrl
drivers for these 4 pinctrl domains and this patch series is
depending on the JH8100 base patch series in [1] and [2].
The relevant dt-binding documentation for each pinctrl domain has
been updated accordingly.

[1] https://lore.kernel.org/lkml/20231201121410.95298-1-jeeheng.sia@starfivetech.com/
[2] https://lore.kernel.org/lkml/20231206115000.295825-1-jeeheng.sia@starfivetech.com/

Alex Soo (6):
  dt-bindings: pinctrl: starfive: add JH8100 pinctrl bindings
  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
  riscv: dts: starfive: jh8100: add pinctrl device tree nodes

 .../pinctrl/starfive,jh8100-aon-pinctrl.yaml  |  183 +++
 .../starfive,jh8100-sys-east-pinctrl.yaml     |  188 +++
 .../starfive,jh8100-sys-gmac-pinctrl.yaml     |  124 ++
 .../starfive,jh8100-sys-west-pinctrl.yaml     |  188 +++
 MAINTAINERS                                   |    7 +
 arch/riscv/boot/dts/starfive/jh8100-evb.dts   |    5 +
 arch/riscv/boot/dts/starfive/jh8100-pinfunc.h |  418 +++++++
 arch/riscv/boot/dts/starfive/jh8100.dtsi      |   44 +
 drivers/pinctrl/starfive/Kconfig              |   57 +
 drivers/pinctrl/starfive/Makefile             |    6 +
 .../starfive/pinctrl-starfive-jh8100-aon.c    |  241 ++++
 .../pinctrl-starfive-jh8100-sys-east.c        |  326 +++++
 .../pinctrl-starfive-jh8100-sys-gmac.c        |  164 +++
 .../pinctrl-starfive-jh8100-sys-west.c        |  264 ++++
 .../starfive/pinctrl-starfive-jh8100.c        | 1090 +++++++++++++++++
 .../starfive/pinctrl-starfive-jh8100.h        |   85 ++
 .../pinctrl/starfive,jh8100-pinctrl.h         |  303 +++++
 17 files changed, 3693 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh8100-aon-pinctrl.yaml
 create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh8100-sys-east-pinctrl.yaml
 create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh8100-sys-gmac-pinctrl.yaml
 create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh8100-sys-west-pinctrl.yaml
 create mode 100644 arch/riscv/boot/dts/starfive/jh8100-pinfunc.h
 create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive-jh8100-aon.c
 create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive-jh8100-sys-east.c
 create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive-jh8100-sys-gmac.c
 create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive-jh8100-sys-west.c
 create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive-jh8100.c
 create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive-jh8100.h
 create mode 100644 include/dt-bindings/pinctrl/starfive,jh8100-pinctrl.h


base-commit: ddb42e41c0991b443dd6cc97213b478a7324d72b
-- 
2.25.1
Re: [RFC PATCH 0/6] Add Pinctrl driver for Starfive JH8100 SoC
Posted by Krzysztof Kozlowski 2 years ago
On 21/12/2023 09:36, Alex Soo wrote:
> Starfive JH8100 SoC consists of 4 pinctrl domains - sys_east,
> sys_west, sys_gmac, and aon. This patch series adds pinctrl
> drivers for these 4 pinctrl domains and this patch series is
> depending on the JH8100 base patch series in [1] and [2].
> The relevant dt-binding documentation for each pinctrl domain has
> been updated accordingly.

Please explain why this is RFC. Every patch is RFC, so what is special
about here? Usually this means work is not finished and should not be
merged, neither reviewed. If you spelled out here the reasons, it would
be easier for us to understand whether we should complain about broken
and non-building code or not.

Best regards,
Krzysztof
Re: [RFC PATCH 0/6] Add Pinctrl driver for Starfive JH8100 SoC
Posted by Linus Walleij 2 years ago
Hi Alex,

thanks for your patch!

On Thu, Dec 21, 2023 at 9:36 AM Alex Soo <yuklin.soo@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?

Yours,
Linus Walleij