2022-11-18 02:08:46

by Hal Feng

[permalink] [raw]
Subject: [PATCH v2 0/5] Basic pinctrl support for StarFive JH7110 RISC-V SoC

The original patch series "Basic StarFive JH7110 RISC-V SoC support" [1]
is split into 3 patch series. They respectively add basic clock&reset,
pinctrl and device tree support for StarFive JH7110 SoC. These patch
series are independent, but the Visionfive2 board can boot up successfully
only if all these patches series applied. This one adds basic pinctrl
support. This patch series is pulled out from the patch 22~26 of v1 [1].
You can simply get or review the patches at the link [2].

[1]: https://lore.kernel.org/all/[email protected]/
[2]: https://github.com/hal-feng/linux/commits/visionfive2-minimal

Changes since v1:
- Rebased on tag v6.1-rc5.
- Dropped patch 22 and 23 since they were merged in v6.1-rc1.
- Removed some unused macros and register values which do not belong to
bindings. Simplified pinctrl definitions in patch 24. (by Krzysztof)
- Split the bindings into sys pinctrl bindings and aon pinctrl bindings,
and split patch 25 into two patches.
- Made the bindings follow generic pinctrl bindings. (by Krzysztof)
- Fixed some wrong indentation in bindings, and checked it with
`make dt_binding_check`.
- Split the patch 26 into two patches which added sys and aon pinctrl
driver respectively.
- Restructured the pinctrl drivers so made them follow generic pinctrl
bindings. Rewrote `dt_node_to_map` and extracted the public code to make
it clearer.

v1: https://lore.kernel.org/all/[email protected]/

Jianlong Huang (5):
dt-bindings: pinctrl: Add StarFive JH7110 pinctrl definitions
dt-bindings: pinctrl: Add StarFive JH7110 sys pinctrl
dt-bindings: pinctrl: Add StarFive JH7110 aon pinctrl
pinctrl: starfive: Add StarFive JH7110 sys controller driver
pinctrl: starfive: Add StarFive JH7110 aon controller driver

.../pinctrl/starfive,jh7110-aon-pinctrl.yaml | 134 +++
.../pinctrl/starfive,jh7110-sys-pinctrl.yaml | 165 +++
MAINTAINERS | 7 +-
drivers/pinctrl/starfive/Kconfig | 21 +
drivers/pinctrl/starfive/Makefile | 5 +
drivers/pinctrl/starfive/pinctrl-jh7110-aon.c | 192 ++++
drivers/pinctrl/starfive/pinctrl-jh7110-sys.c | 464 +++++++++
drivers/pinctrl/starfive/pinctrl-starfive.c | 972 ++++++++++++++++++
drivers/pinctrl/starfive/pinctrl-starfive.h | 72 ++
.../pinctrl/pinctrl-starfive-jh7110.h | 427 ++++++++
10 files changed, 2456 insertions(+), 3 deletions(-)
create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml
create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml
create mode 100644 drivers/pinctrl/starfive/pinctrl-jh7110-aon.c
create mode 100644 drivers/pinctrl/starfive/pinctrl-jh7110-sys.c
create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive.c
create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive.h
create mode 100644 include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h


base-commit: 094226ad94f471a9f19e8f8e7140a09c2625abaa
prerequisite-patch-id: 6b1b43a55b9773bec61ab6c1bbaa54dccbac0837
prerequisite-patch-id: 09c98554df52d17ba5fd604125f8cdd62cbe80d1
prerequisite-patch-id: 29fe0b0c19b6f0cd31114ee9fe17fe9732047f33
prerequisite-patch-id: c59d9908de90e09ba2b9a81aadbf9fb9f00c8f04
prerequisite-patch-id: 94ac03d518993921bcfc9cc9f58d7da0c3528b51
prerequisite-patch-id: 694f7400375f5b85581fc1821e427334507826f2
prerequisite-patch-id: 699d49c4439dadb4b7cf900857f027d050cd6093
prerequisite-patch-id: 40d773f5a19912f731ee5fd4739ed2e3c2157b07
prerequisite-patch-id: 2bc3fd6df5dda116efe882045863d6c88aa81b3a
prerequisite-patch-id: 735e62255c75801bdc4c0b4107850bce821ff7f5
prerequisite-patch-id: b2a923b922e661fa6085185f33c1f1e733db9110
prerequisite-patch-id: b2bbc28354075432f059344eba5a127a653475cf
prerequisite-patch-id: 70eab7b7eee728afcd90e40f6743d1356f6d81ab
prerequisite-patch-id: 6276b2a23818c65ff2ad3d65b562615690cffee9
--
2.38.1



2022-11-18 02:09:11

by Hal Feng

[permalink] [raw]
Subject: [PATCH v2 4/5] pinctrl: starfive: Add StarFive JH7110 sys controller driver

From: Jianlong Huang <[email protected]>

Add pinctrl driver for StarFive JH7110 SoC sys pinctrl controller.

Co-developed-by: Emil Renner Berthing <[email protected]>
Signed-off-by: Emil Renner Berthing <[email protected]>
Signed-off-by: Jianlong Huang <[email protected]>
Signed-off-by: Hal Feng <[email protected]>
---
MAINTAINERS | 7 +-
drivers/pinctrl/starfive/Kconfig | 21 +
drivers/pinctrl/starfive/Makefile | 5 +
drivers/pinctrl/starfive/pinctrl-jh7110-sys.c | 464 +++++++++
drivers/pinctrl/starfive/pinctrl-starfive.c | 972 ++++++++++++++++++
drivers/pinctrl/starfive/pinctrl-starfive.h | 72 ++
6 files changed, 1538 insertions(+), 3 deletions(-)
create mode 100644 drivers/pinctrl/starfive/pinctrl-jh7110-sys.c
create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive.c
create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive.h

diff --git a/MAINTAINERS b/MAINTAINERS
index ec6647e2772f..a70c1d0f303e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19606,13 +19606,14 @@ F: Documentation/devicetree/bindings/clock/starfive*
F: drivers/clk/starfive/
F: include/dt-bindings/clock/starfive*

-STARFIVE JH7100 PINCTRL DRIVER
+STARFIVE PINCTRL DRIVER
M: Emil Renner Berthing <[email protected]>
+M: Jianlong Huang <[email protected]>
L: [email protected]
S: Maintained
-F: Documentation/devicetree/bindings/pinctrl/starfive,jh7100-pinctrl.yaml
+F: Documentation/devicetree/bindings/pinctrl/starfive*
F: drivers/pinctrl/starfive/
-F: include/dt-bindings/pinctrl/pinctrl-starfive-jh7100.h
+F: include/dt-bindings/pinctrl/pinctrl-starfive*

STARFIVE RESET CONTROLLER DRIVERS
M: Emil Renner Berthing <[email protected]>
diff --git a/drivers/pinctrl/starfive/Kconfig b/drivers/pinctrl/starfive/Kconfig
index 55c514e622f9..c7896a1f7d5a 100644
--- a/drivers/pinctrl/starfive/Kconfig
+++ b/drivers/pinctrl/starfive/Kconfig
@@ -16,3 +16,24 @@ config PINCTRL_STARFIVE_JH7100
This also provides an interface to the GPIO pins not used by other
peripherals supporting inputs, outputs, configuring pull-up/pull-down
and interrupts on input changes.
+
+config PINCTRL_STARFIVE
+ bool
+ select GENERIC_PINCTRL_GROUPS
+ select GENERIC_PINMUX_FUNCTIONS
+ select GENERIC_PINCONF
+ select GPIOLIB
+ select GPIOLIB_IRQCHIP
+ select OF_GPIO
+
+config PINCTRL_STARFIVE_JH7110
+ bool "Pinctrl and GPIO driver for the StarFive JH7110 SoC"
+ depends on SOC_STARFIVE || COMPILE_TEST
+ depends on OF
+ select PINCTRL_STARFIVE
+ default SOC_STARFIVE
+ help
+ Say yes here to support pin control on the StarFive JH7110 SoC.
+ This also provides an interface to the GPIO pins not used by other
+ peripherals supporting inputs, outputs, configuring pull-up/pull-down
+ and interrupts on input changes.
diff --git a/drivers/pinctrl/starfive/Makefile b/drivers/pinctrl/starfive/Makefile
index 0293f26a0a99..404929f760e8 100644
--- a/drivers/pinctrl/starfive/Makefile
+++ b/drivers/pinctrl/starfive/Makefile
@@ -1,3 +1,8 @@
# SPDX-License-Identifier: GPL-2.0

+# Core
+obj-$(CONFIG_PINCTRL_STARFIVE) += pinctrl-starfive.o
+
+# SoC Drivers
obj-$(CONFIG_PINCTRL_STARFIVE_JH7100) += pinctrl-starfive-jh7100.o
+obj-$(CONFIG_PINCTRL_STARFIVE_JH7110) += pinctrl-jh7110-sys.o
diff --git a/drivers/pinctrl/starfive/pinctrl-jh7110-sys.c b/drivers/pinctrl/starfive/pinctrl-jh7110-sys.c
new file mode 100644
index 000000000000..942e0b6e5ac6
--- /dev/null
+++ b/drivers/pinctrl/starfive/pinctrl-jh7110-sys.c
@@ -0,0 +1,464 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Pinctrl / GPIO driver for StarFive JH7110 SoC sys controller
+ *
+ * Copyright (C) 2022 Emil Renner Berthing <[email protected]>
+ * Copyright (C) 2022 StarFive Technology Co., Ltd.
+ */
+
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/gpio/driver.h>
+#include <linux/io.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+#include <linux/spinlock.h>
+
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+
+#include <dt-bindings/pinctrl/pinctrl-starfive-jh7110.h>
+
+#include "../core.h"
+#include "../pinctrl-utils.h"
+#include "../pinmux.h"
+#include "../pinconf.h"
+#include "pinctrl-starfive.h"
+
+#define JH7110_SYS_NGPIO 64
+#define JH7110_SYS_GC_BASE 0
+
+/* registers */
+#define JH7110_SYS_DOEN 0x000
+#define JH7110_SYS_DOUT 0x040
+#define JH7110_SYS_GPI 0x080
+#define JH7110_SYS_GPIOIN 0x118
+
+#define JH7110_SYS_GPIOEN 0x0dc
+#define JH7110_SYS_GPIOIS0 0x0e0
+#define JH7110_SYS_GPIOIS1 0x0e4
+#define JH7110_SYS_GPIOIC0 0x0e8
+#define JH7110_SYS_GPIOIC1 0x0ec
+#define JH7110_SYS_GPIOIBE0 0x0f0
+#define JH7110_SYS_GPIOIBE1 0x0f4
+#define JH7110_SYS_GPIOIEV0 0x0f8
+#define JH7110_SYS_GPIOIEV1 0x0fc
+#define JH7110_SYS_GPIOIE0 0x100
+#define JH7110_SYS_GPIOIE1 0x104
+#define JH7110_SYS_GPIORIS0 0x108
+#define JH7110_SYS_GPIORIS1 0x10c
+#define JH7110_SYS_GPIOMIS0 0x110
+#define JH7110_SYS_GPIOMIS1 0x114
+
+#define SYS_GPO_PDA_0_74_CFG 0x120
+#define SYS_GPO_PDA_89_94_CFG 0x284
+
+static const struct pinctrl_pin_desc jh7110_sys_pins[] = {
+ PINCTRL_PIN(PAD_GPIO0, "GPIO0"),
+ PINCTRL_PIN(PAD_GPIO1, "GPIO1"),
+ PINCTRL_PIN(PAD_GPIO2, "GPIO2"),
+ PINCTRL_PIN(PAD_GPIO3, "GPIO3"),
+ PINCTRL_PIN(PAD_GPIO4, "GPIO4"),
+ PINCTRL_PIN(PAD_GPIO5, "GPIO5"),
+ PINCTRL_PIN(PAD_GPIO6, "GPIO6"),
+ PINCTRL_PIN(PAD_GPIO7, "GPIO7"),
+ PINCTRL_PIN(PAD_GPIO8, "GPIO8"),
+ PINCTRL_PIN(PAD_GPIO9, "GPIO9"),
+ PINCTRL_PIN(PAD_GPIO10, "GPIO10"),
+ PINCTRL_PIN(PAD_GPIO11, "GPIO11"),
+ PINCTRL_PIN(PAD_GPIO12, "GPIO12"),
+ PINCTRL_PIN(PAD_GPIO13, "GPIO13"),
+ PINCTRL_PIN(PAD_GPIO14, "GPIO14"),
+ PINCTRL_PIN(PAD_GPIO15, "GPIO15"),
+ PINCTRL_PIN(PAD_GPIO16, "GPIO16"),
+ PINCTRL_PIN(PAD_GPIO17, "GPIO17"),
+ PINCTRL_PIN(PAD_GPIO18, "GPIO18"),
+ PINCTRL_PIN(PAD_GPIO19, "GPIO19"),
+ PINCTRL_PIN(PAD_GPIO20, "GPIO20"),
+ PINCTRL_PIN(PAD_GPIO21, "GPIO21"),
+ PINCTRL_PIN(PAD_GPIO22, "GPIO22"),
+ PINCTRL_PIN(PAD_GPIO23, "GPIO23"),
+ PINCTRL_PIN(PAD_GPIO24, "GPIO24"),
+ PINCTRL_PIN(PAD_GPIO25, "GPIO25"),
+ PINCTRL_PIN(PAD_GPIO26, "GPIO26"),
+ PINCTRL_PIN(PAD_GPIO27, "GPIO27"),
+ PINCTRL_PIN(PAD_GPIO28, "GPIO28"),
+ PINCTRL_PIN(PAD_GPIO29, "GPIO29"),
+ PINCTRL_PIN(PAD_GPIO30, "GPIO30"),
+ PINCTRL_PIN(PAD_GPIO31, "GPIO31"),
+ PINCTRL_PIN(PAD_GPIO32, "GPIO32"),
+ PINCTRL_PIN(PAD_GPIO33, "GPIO33"),
+ PINCTRL_PIN(PAD_GPIO34, "GPIO34"),
+ PINCTRL_PIN(PAD_GPIO35, "GPIO35"),
+ PINCTRL_PIN(PAD_GPIO36, "GPIO36"),
+ PINCTRL_PIN(PAD_GPIO37, "GPIO37"),
+ PINCTRL_PIN(PAD_GPIO38, "GPIO38"),
+ PINCTRL_PIN(PAD_GPIO39, "GPIO39"),
+ PINCTRL_PIN(PAD_GPIO40, "GPIO40"),
+ PINCTRL_PIN(PAD_GPIO41, "GPIO41"),
+ PINCTRL_PIN(PAD_GPIO42, "GPIO42"),
+ PINCTRL_PIN(PAD_GPIO43, "GPIO43"),
+ PINCTRL_PIN(PAD_GPIO44, "GPIO44"),
+ PINCTRL_PIN(PAD_GPIO45, "GPIO45"),
+ PINCTRL_PIN(PAD_GPIO46, "GPIO46"),
+ PINCTRL_PIN(PAD_GPIO47, "GPIO47"),
+ PINCTRL_PIN(PAD_GPIO48, "GPIO48"),
+ PINCTRL_PIN(PAD_GPIO49, "GPIO49"),
+ PINCTRL_PIN(PAD_GPIO50, "GPIO50"),
+ PINCTRL_PIN(PAD_GPIO51, "GPIO51"),
+ PINCTRL_PIN(PAD_GPIO52, "GPIO52"),
+ PINCTRL_PIN(PAD_GPIO53, "GPIO53"),
+ PINCTRL_PIN(PAD_GPIO54, "GPIO54"),
+ PINCTRL_PIN(PAD_GPIO55, "GPIO55"),
+ PINCTRL_PIN(PAD_GPIO56, "GPIO56"),
+ PINCTRL_PIN(PAD_GPIO57, "GPIO57"),
+ PINCTRL_PIN(PAD_GPIO58, "GPIO58"),
+ PINCTRL_PIN(PAD_GPIO59, "GPIO59"),
+ PINCTRL_PIN(PAD_GPIO60, "GPIO60"),
+ PINCTRL_PIN(PAD_GPIO61, "GPIO61"),
+ PINCTRL_PIN(PAD_GPIO62, "GPIO62"),
+ PINCTRL_PIN(PAD_GPIO63, "GPIO63"),
+ PINCTRL_PIN(PAD_SD0_CLK, "SD0_CLK"),
+ PINCTRL_PIN(PAD_SD0_CMD, "SD0_CMD"),
+ PINCTRL_PIN(PAD_SD0_DATA0, "SD0_DATA0"),
+ PINCTRL_PIN(PAD_SD0_DATA1, "SD0_DATA1"),
+ PINCTRL_PIN(PAD_SD0_DATA2, "SD0_DATA2"),
+ PINCTRL_PIN(PAD_SD0_DATA3, "SD0_DATA3"),
+ PINCTRL_PIN(PAD_SD0_DATA4, "SD0_DATA4"),
+ PINCTRL_PIN(PAD_SD0_DATA5, "SD0_DATA5"),
+ PINCTRL_PIN(PAD_SD0_DATA6, "SD0_DATA6"),
+ PINCTRL_PIN(PAD_SD0_DATA7, "SD0_DATA7"),
+ PINCTRL_PIN(PAD_SD0_STRB, "SD0_STRB"),
+ PINCTRL_PIN(PAD_GMAC1_MDC, "GMAC1_MDC"),
+ PINCTRL_PIN(PAD_GMAC1_MDIO, "GMAC1_MDIO"),
+ PINCTRL_PIN(PAD_GMAC1_RXD0, "GMAC1_RXD0"),
+ PINCTRL_PIN(PAD_GMAC1_RXD1, "GMAC1_RXD1"),
+ PINCTRL_PIN(PAD_GMAC1_RXD2, "GMAC1_RXD2"),
+ PINCTRL_PIN(PAD_GMAC1_RXD3, "GMAC1_RXD3"),
+ PINCTRL_PIN(PAD_GMAC1_RXDV, "GMAC1_RXDV"),
+ PINCTRL_PIN(PAD_GMAC1_RXC, "GMAC1_RXC"),
+ PINCTRL_PIN(PAD_GMAC1_TXD0, "GMAC1_TXD0"),
+ PINCTRL_PIN(PAD_GMAC1_TXD1, "GMAC1_TXD1"),
+ PINCTRL_PIN(PAD_GMAC1_TXD2, "GMAC1_TXD2"),
+ PINCTRL_PIN(PAD_GMAC1_TXD3, "GMAC1_TXD3"),
+ PINCTRL_PIN(PAD_GMAC1_TXEN, "GMAC1_TXEN"),
+ PINCTRL_PIN(PAD_GMAC1_TXC, "GMAC1_TXC"),
+ PINCTRL_PIN(PAD_QSPI_SCLK, "QSPI_SCLK"),
+ PINCTRL_PIN(PAD_QSPI_CS0, "QSPI_CS0"),
+ PINCTRL_PIN(PAD_QSPI_DATA0, "QSPI_DATA0"),
+ PINCTRL_PIN(PAD_QSPI_DATA1, "QSPI_DATA1"),
+ PINCTRL_PIN(PAD_QSPI_DATA2, "QSPI_DATA2"),
+ PINCTRL_PIN(PAD_QSPI_DATA3, "QSPI_DATA3"),
+};
+
+struct jh7110_func_sel {
+ u16 offset;
+ u8 shift;
+ u8 max;
+};
+
+static const struct jh7110_func_sel
+ jh7110_sys_func_sel[ARRAY_SIZE(jh7110_sys_pins)] = {
+ [PAD_GMAC1_RXC] = { 0x29c, 0, 1 },
+ [PAD_GPIO10] = { 0x29c, 2, 3 },
+ [PAD_GPIO11] = { 0x29c, 5, 3 },
+ [PAD_GPIO12] = { 0x29c, 8, 3 },
+ [PAD_GPIO13] = { 0x29c, 11, 3 },
+ [PAD_GPIO14] = { 0x29c, 14, 3 },
+ [PAD_GPIO15] = { 0x29c, 17, 3 },
+ [PAD_GPIO16] = { 0x29c, 20, 3 },
+ [PAD_GPIO17] = { 0x29c, 23, 3 },
+ [PAD_GPIO18] = { 0x29c, 26, 3 },
+ [PAD_GPIO19] = { 0x29c, 29, 3 },
+
+ [PAD_GPIO20] = { 0x2a0, 0, 3 },
+ [PAD_GPIO21] = { 0x2a0, 3, 3 },
+ [PAD_GPIO22] = { 0x2a0, 6, 3 },
+ [PAD_GPIO23] = { 0x2a0, 9, 3 },
+ [PAD_GPIO24] = { 0x2a0, 12, 3 },
+ [PAD_GPIO25] = { 0x2a0, 15, 3 },
+ [PAD_GPIO26] = { 0x2a0, 18, 3 },
+ [PAD_GPIO27] = { 0x2a0, 21, 3 },
+ [PAD_GPIO28] = { 0x2a0, 24, 3 },
+ [PAD_GPIO29] = { 0x2a0, 27, 3 },
+
+ [PAD_GPIO30] = { 0x2a4, 0, 3 },
+ [PAD_GPIO31] = { 0x2a4, 3, 3 },
+ [PAD_GPIO32] = { 0x2a4, 6, 3 },
+ [PAD_GPIO33] = { 0x2a4, 9, 3 },
+ [PAD_GPIO34] = { 0x2a4, 12, 3 },
+ [PAD_GPIO35] = { 0x2a4, 15, 3 },
+ [PAD_GPIO36] = { 0x2a4, 17, 3 },
+ [PAD_GPIO37] = { 0x2a4, 20, 3 },
+ [PAD_GPIO38] = { 0x2a4, 23, 3 },
+ [PAD_GPIO39] = { 0x2a4, 26, 3 },
+ [PAD_GPIO40] = { 0x2a4, 29, 3 },
+
+ [PAD_GPIO41] = { 0x2a8, 0, 3 },
+ [PAD_GPIO42] = { 0x2a8, 3, 3 },
+ [PAD_GPIO43] = { 0x2a8, 6, 3 },
+ [PAD_GPIO44] = { 0x2a8, 9, 3 },
+ [PAD_GPIO45] = { 0x2a8, 12, 3 },
+ [PAD_GPIO46] = { 0x2a8, 15, 3 },
+ [PAD_GPIO47] = { 0x2a8, 18, 3 },
+ [PAD_GPIO48] = { 0x2a8, 21, 3 },
+ [PAD_GPIO49] = { 0x2a8, 24, 3 },
+ [PAD_GPIO50] = { 0x2a8, 27, 3 },
+ [PAD_GPIO51] = { 0x2a8, 30, 3 },
+
+ [PAD_GPIO52] = { 0x2ac, 0, 3 },
+ [PAD_GPIO53] = { 0x2ac, 2, 3 },
+ [PAD_GPIO54] = { 0x2ac, 4, 3 },
+ [PAD_GPIO55] = { 0x2ac, 6, 3 },
+ [PAD_GPIO56] = { 0x2ac, 9, 3 },
+ [PAD_GPIO57] = { 0x2ac, 12, 3 },
+ [PAD_GPIO58] = { 0x2ac, 15, 3 },
+ [PAD_GPIO59] = { 0x2ac, 18, 3 },
+ [PAD_GPIO60] = { 0x2ac, 21, 3 },
+ [PAD_GPIO61] = { 0x2ac, 24, 3 },
+ [PAD_GPIO62] = { 0x2ac, 27, 3 },
+ [PAD_GPIO63] = { 0x2ac, 30, 3 },
+
+ [PAD_GPIO6] = { 0x2b0, 0, 3 },
+ [PAD_GPIO7] = { 0x2b0, 2, 3 },
+ [PAD_GPIO8] = { 0x2b0, 5, 3 },
+ [PAD_GPIO9] = { 0x2b0, 8, 3 },
+};
+
+struct jh7110_vin_group_sel {
+ u16 offset;
+ u8 shift;
+ u8 group;
+};
+
+static const struct jh7110_vin_group_sel
+ jh7110_sys_vin_group_sel[ARRAY_SIZE(jh7110_sys_pins)] = {
+ [PAD_GPIO6] = { 0x2b4, 21, 0 },
+ [PAD_GPIO7] = { 0x2b4, 18, 0 },
+ [PAD_GPIO8] = { 0x2b4, 15, 0 },
+ [PAD_GPIO9] = { 0x2b0, 11, 0 },
+ [PAD_GPIO10] = { 0x2b0, 20, 0 },
+ [PAD_GPIO11] = { 0x2b0, 23, 0 },
+ [PAD_GPIO12] = { 0x2b0, 26, 0 },
+ [PAD_GPIO13] = { 0x2b0, 29, 0 },
+ [PAD_GPIO14] = { 0x2b4, 0, 0 },
+ [PAD_GPIO15] = { 0x2b4, 3, 0 },
+ [PAD_GPIO16] = { 0x2b4, 6, 0 },
+ [PAD_GPIO17] = { 0x2b4, 9, 0 },
+ [PAD_GPIO18] = { 0x2b4, 12, 0 },
+ [PAD_GPIO19] = { 0x2b0, 14, 0 },
+ [PAD_GPIO20] = { 0x2b0, 17, 0 },
+
+ [PAD_GPIO21] = { 0x2b4, 21, 1 },
+ [PAD_GPIO22] = { 0x2b4, 18, 1 },
+ [PAD_GPIO23] = { 0x2b4, 15, 1 },
+ [PAD_GPIO24] = { 0x2b0, 11, 1 },
+ [PAD_GPIO25] = { 0x2b0, 20, 1 },
+ [PAD_GPIO26] = { 0x2b0, 23, 1 },
+ [PAD_GPIO27] = { 0x2b0, 26, 1 },
+ [PAD_GPIO28] = { 0x2b0, 29, 1 },
+ [PAD_GPIO29] = { 0x2b4, 0, 1 },
+ [PAD_GPIO30] = { 0x2b4, 3, 1 },
+ [PAD_GPIO31] = { 0x2b4, 6, 1 },
+ [PAD_GPIO32] = { 0x2b4, 9, 1 },
+ [PAD_GPIO33] = { 0x2b4, 12, 1 },
+ [PAD_GPIO34] = { 0x2b0, 14, 1 },
+ [PAD_GPIO35] = { 0x2b0, 17, 1 },
+
+ [PAD_GPIO36] = { 0x2b4, 21, 2 },
+ [PAD_GPIO37] = { 0x2b4, 18, 2 },
+ [PAD_GPIO38] = { 0x2b4, 15, 2 },
+ [PAD_GPIO39] = { 0x2b0, 11, 2 },
+ [PAD_GPIO40] = { 0x2b0, 20, 2 },
+ [PAD_GPIO41] = { 0x2b0, 23, 2 },
+ [PAD_GPIO42] = { 0x2b0, 26, 2 },
+ [PAD_GPIO43] = { 0x2b0, 29, 2 },
+ [PAD_GPIO44] = { 0x2b4, 0, 2 },
+ [PAD_GPIO45] = { 0x2b4, 3, 2 },
+ [PAD_GPIO46] = { 0x2b4, 6, 2 },
+ [PAD_GPIO47] = { 0x2b4, 9, 2 },
+ [PAD_GPIO48] = { 0x2b4, 12, 2 },
+ [PAD_GPIO49] = { 0x2b0, 14, 2 },
+ [PAD_GPIO50] = { 0x2b0, 17, 2 },
+};
+
+static void jh7110_set_function(struct starfive_pinctrl *sfp,
+ unsigned int pin, u32 func)
+{
+ const struct jh7110_func_sel *fs = &jh7110_sys_func_sel[pin];
+ unsigned long flags;
+ void __iomem *reg;
+ u32 mask;
+
+ if (!fs->offset)
+ return;
+
+ if (func > fs->max)
+ return;
+
+ reg = sfp->base + fs->offset;
+ func = func << fs->shift;
+ mask = 0x3U << fs->shift;
+
+ raw_spin_lock_irqsave(&sfp->lock, flags);
+ func |= readl_relaxed(reg) & ~mask;
+ writel_relaxed(func, reg);
+ raw_spin_unlock_irqrestore(&sfp->lock, flags);
+}
+
+static void jh7110_set_vin_group(struct starfive_pinctrl *sfp,
+ unsigned int pin)
+{
+ const struct jh7110_vin_group_sel *gs = &jh7110_sys_vin_group_sel[pin];
+ unsigned long flags;
+ void __iomem *reg;
+ u32 mask;
+ u32 grp;
+
+ if (!gs->offset)
+ return;
+
+ reg = sfp->base + gs->offset;
+ grp = gs->group << gs->shift;
+ mask = 0x3U << gs->shift;
+
+ raw_spin_lock_irqsave(&sfp->lock, flags);
+ grp |= readl_relaxed(reg) & ~mask;
+ writel_relaxed(grp, reg);
+ raw_spin_unlock_irqrestore(&sfp->lock, flags);
+}
+
+static int jh7110_sys_set_one_pin_mux(struct starfive_pinctrl *sfp,
+ unsigned int pin,
+ unsigned int din, u32 dout,
+ u32 doen, u32 func)
+{
+ if (pin < sfp->gc.ngpio && func == 0)
+ starfive_set_gpiomux(sfp, pin, din, dout, doen);
+
+ jh7110_set_function(sfp, pin, func);
+
+ if (pin < sfp->gc.ngpio && func == 2)
+ jh7110_set_vin_group(sfp, pin);
+
+ return 0;
+}
+
+static int jh7110_sys_get_padcfg_base(struct starfive_pinctrl *sfp,
+ unsigned int pin)
+{
+ if (pin < PAD_GMAC1_MDC)
+ return SYS_GPO_PDA_0_74_CFG;
+ else if (pin > PAD_GMAC1_TXC && pin <= PAD_QSPI_DATA3)
+ return SYS_GPO_PDA_89_94_CFG;
+ else
+ return -1;
+}
+
+static void jh7110_sys_irq_handler(struct irq_desc *desc)
+{
+ struct starfive_pinctrl *sfp = starfive_from_irq_desc(desc);
+ struct irq_chip *chip = irq_desc_get_chip(desc);
+ unsigned long mis;
+ unsigned int pin;
+
+ chained_irq_enter(chip, desc);
+
+ mis = readl_relaxed(sfp->base + JH7110_SYS_GPIOMIS0);
+ for_each_set_bit(pin, &mis, 32)
+ generic_handle_domain_irq(sfp->gc.irq.domain, pin);
+
+ mis = readl_relaxed(sfp->base + JH7110_SYS_GPIOMIS1);
+ for_each_set_bit(pin, &mis, 32)
+ generic_handle_domain_irq(sfp->gc.irq.domain, pin + 32);
+
+ chained_irq_exit(chip, desc);
+}
+
+static int jh7110_sys_init_hw(struct gpio_chip *gc)
+{
+ struct starfive_pinctrl *sfp = container_of(gc,
+ struct starfive_pinctrl, gc);
+
+ /* mask all GPIO interrupts */
+ writel(0U, sfp->base + JH7110_SYS_GPIOIE0);
+ writel(0U, sfp->base + JH7110_SYS_GPIOIE1);
+ /* clear edge interrupt flags */
+ writel(~0U, sfp->base + JH7110_SYS_GPIOIC0);
+ writel(~0U, sfp->base + JH7110_SYS_GPIOIC1);
+ /* enable GPIO interrupts */
+ writel(1U, sfp->base + JH7110_SYS_GPIOEN);
+ return 0;
+}
+
+static const struct starfive_gpio_irq_reg jh7110_sys_irq_reg = {
+ .is_reg_base = JH7110_SYS_GPIOIS0,
+ .ic_reg_base = JH7110_SYS_GPIOIC0,
+ .ibe_reg_base = JH7110_SYS_GPIOIBE0,
+ .iev_reg_base = JH7110_SYS_GPIOIEV0,
+ .ie_reg_base = JH7110_SYS_GPIOIE0,
+ .ris_reg_base = JH7110_SYS_GPIORIS0,
+ .mis_reg_base = JH7110_SYS_GPIOMIS0,
+};
+
+static const struct starfive_pinctrl_soc_info jh7110_sys_pinctrl_info = {
+ .pins = jh7110_sys_pins,
+ .npins = ARRAY_SIZE(jh7110_sys_pins),
+ .ngpios = JH7110_SYS_NGPIO,
+ .gc_base = JH7110_SYS_GC_BASE,
+ .flags = 1,
+ .dout_reg_base = JH7110_SYS_DOUT,
+ .dout_mask = GENMASK(6, 0),
+ .doen_reg_base = JH7110_SYS_DOEN,
+ .doen_mask = GENMASK(5, 0),
+ .gpi_reg_base = JH7110_SYS_GPI,
+ .gpi_mask = GENMASK(6, 0),
+ .gpioin_reg_base = JH7110_SYS_GPIOIN,
+ .irq_reg = &jh7110_sys_irq_reg,
+ .starfive_set_one_pin_mux = jh7110_sys_set_one_pin_mux,
+ .starfive_get_padcfg_base = jh7110_sys_get_padcfg_base,
+ .starfive_gpio_irq_handler = jh7110_sys_irq_handler,
+ .starfive_gpio_init_hw = jh7110_sys_init_hw,
+};
+
+static const struct of_device_id jh7110_sys_pinctrl_of_match[] = {
+ {
+ .compatible = "starfive,jh7110-sys-pinctrl",
+ .data = &jh7110_sys_pinctrl_info,
+ },
+ { /* sentinel */ }
+};
+
+static int jh7110_sys_pinctrl_probe(struct platform_device *pdev)
+{
+ const struct starfive_pinctrl_soc_info *pinctrl_info;
+
+ pinctrl_info = of_device_get_match_data(&pdev->dev);
+ if (!pinctrl_info)
+ return -ENODEV;
+
+ return starfive_pinctrl_probe(pdev, pinctrl_info);
+}
+
+static struct platform_driver jh7110_sys_pinctrl_driver = {
+ .driver = {
+ .name = "starfive-jh7110-sys-pinctrl",
+ .of_match_table = of_match_ptr(jh7110_sys_pinctrl_of_match),
+ },
+ .probe = jh7110_sys_pinctrl_probe,
+};
+
+static int __init jh7110_sys_pinctrl_init(void)
+{
+ return platform_driver_register(&jh7110_sys_pinctrl_driver);
+}
+arch_initcall(jh7110_sys_pinctrl_init);
+
+MODULE_DESCRIPTION("Pinctrl driver for the StarFive JH7110 SoC sys controller");
+MODULE_AUTHOR("Emil Renner Berthing <[email protected]>");
+MODULE_AUTHOR("Jianlong Huang <[email protected]>");
diff --git a/drivers/pinctrl/starfive/pinctrl-starfive.c b/drivers/pinctrl/starfive/pinctrl-starfive.c
new file mode 100644
index 000000000000..944caa599460
--- /dev/null
+++ b/drivers/pinctrl/starfive/pinctrl-starfive.c
@@ -0,0 +1,972 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Pinctrl / GPIO driver for StarFive JH7110 SoC
+ *
+ * Copyright (C) 2022 Emil Renner Berthing <[email protected]>
+ * Copyright (C) 2022 StarFive Technology Co., Ltd.
+ */
+
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/gpio/driver.h>
+#include <linux/io.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+#include <linux/spinlock.h>
+
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+
+#include <dt-bindings/pinctrl/pinctrl-starfive-jh7110.h>
+
+#include "../core.h"
+#include "../pinctrl-utils.h"
+#include "../pinmux.h"
+#include "../pinconf.h"
+#include "pinctrl-starfive.h"
+
+/* pad control bits */
+#define STARFIVE_PADCFG_POS BIT(7)
+#define STARFIVE_PADCFG_SMT BIT(6)
+#define STARFIVE_PADCFG_SLEW BIT(5)
+#define STARFIVE_PADCFG_PD BIT(4)
+#define STARFIVE_PADCFG_PU BIT(3)
+#define STARFIVE_PADCFG_BIAS (STARFIVE_PADCFG_PD | STARFIVE_PADCFG_PU)
+#define STARFIVE_PADCFG_DS_MASK GENMASK(2, 1)
+#define STARFIVE_PADCFG_DS_2MA (0U << 1)
+#define STARFIVE_PADCFG_DS_4MA BIT(1)
+#define STARFIVE_PADCFG_DS_8MA (2U << 1)
+#define STARFIVE_PADCFG_DS_12MA (3U << 1)
+#define STARFIVE_PADCFG_IE BIT(0)
+#define GPIO_NUM_PER_WORD 32
+
+/*
+ * The packed pinmux values from the device tree look like this:
+ *
+ * | 31 - 24 | 23 - 16 | 15 - 10 | 9 - 8 | 7 - 0 |
+ * | din | dout | doen | function | pin |
+ */
+static unsigned int starfive_pinmux_din(u32 v)
+{
+ return (v & GENMASK(31, 24)) >> 24;
+}
+
+static u32 starfive_pinmux_dout(u32 v)
+{
+ return (v & GENMASK(23, 16)) >> 16;
+}
+
+static u32 starfive_pinmux_doen(u32 v)
+{
+ return (v & GENMASK(15, 10)) >> 10;
+}
+
+static u32 starfive_pinmux_function(u32 v)
+{
+ return (v & GENMASK(9, 8)) >> 8;
+}
+
+static unsigned int starfive_pinmux_pin(u32 v)
+{
+ return v & GENMASK(7, 0);
+}
+
+static struct starfive_pinctrl *starfive_from_irq_data(struct irq_data *d)
+{
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+
+ return container_of(gc, struct starfive_pinctrl, gc);
+}
+
+struct starfive_pinctrl *starfive_from_irq_desc(struct irq_desc *desc)
+{
+ struct gpio_chip *gc = irq_desc_get_handler_data(desc);
+
+ return container_of(gc, struct starfive_pinctrl, gc);
+}
+
+#ifdef CONFIG_DEBUG_FS
+static void starfive_pin_dbg_show(struct pinctrl_dev *pctldev,
+ struct seq_file *s, unsigned int pin)
+{
+ struct starfive_pinctrl *sfp = pinctrl_dev_get_drvdata(pctldev);
+ const struct starfive_pinctrl_soc_info *info = sfp->info;
+
+ seq_printf(s, "%s", dev_name(pctldev->dev));
+
+ if (pin < sfp->gc.ngpio) {
+ unsigned int offset = 4 * (pin / 4);
+ unsigned int shift = 8 * (pin % 4);
+ u32 dout = readl_relaxed(sfp->base + info->dout_reg_base + offset);
+ u32 doen = readl_relaxed(sfp->base + info->doen_reg_base + offset);
+ u32 gpi = readl_relaxed(sfp->base + info->gpi_reg_base + offset);
+
+ dout = (dout >> shift) & info->dout_mask;
+ doen = (doen >> shift) & info->doen_mask;
+ gpi = ((gpi >> shift) - 2) & info->gpi_mask;
+
+ seq_printf(s, " dout=%u doen=%u din=%u", dout, doen, gpi);
+ }
+}
+#else
+#define starfive_pin_dbg_show NULL
+#endif
+
+static int starfive_dt_node_to_map(struct pinctrl_dev *pctldev,
+ struct device_node *np,
+ struct pinctrl_map **maps,
+ unsigned int *num_maps)
+{
+ struct starfive_pinctrl *sfp = pinctrl_dev_get_drvdata(pctldev);
+ struct device *dev = sfp->gc.parent;
+ struct device_node *child;
+ struct pinctrl_map *map;
+ const char **pgnames;
+ const char *grpname;
+ int ngroups;
+ int nmaps;
+ int ret;
+
+ ngroups = 0;
+ for_each_child_of_node(np, child)
+ ngroups += 1;
+ nmaps = 2 * ngroups;
+
+ pgnames = devm_kcalloc(dev, ngroups, sizeof(*pgnames), GFP_KERNEL);
+ if (!pgnames)
+ return -ENOMEM;
+
+ map = kcalloc(nmaps, sizeof(*map), GFP_KERNEL);
+ if (!map)
+ return -ENOMEM;
+
+ nmaps = 0;
+ ngroups = 0;
+ mutex_lock(&sfp->mutex);
+ for_each_child_of_node(np, child) {
+ int npins = of_property_count_u32_elems(child, "pinmux");
+ int *pins;
+ u32 *pinmux;
+ int i;
+
+ if (npins < 1) {
+ dev_err(dev,
+ "invalid pinctrl group %pOFn.%pOFn: pinmux not set\n",
+ np, child);
+ ret = -EINVAL;
+ goto put_child;
+ }
+
+ grpname = devm_kasprintf(dev, GFP_KERNEL, "%pOFn.%pOFn", np, child);
+ if (!grpname) {
+ ret = -ENOMEM;
+ goto put_child;
+ }
+
+ pgnames[ngroups++] = grpname;
+
+ pins = devm_kcalloc(dev, npins, sizeof(*pins), GFP_KERNEL);
+ if (!pins) {
+ ret = -ENOMEM;
+ goto put_child;
+ }
+
+ pinmux = devm_kcalloc(dev, npins, sizeof(*pinmux), GFP_KERNEL);
+ if (!pinmux) {
+ ret = -ENOMEM;
+ goto put_child;
+ }
+
+ ret = of_property_read_u32_array(child, "pinmux", pinmux, npins);
+ if (ret)
+ goto put_child;
+
+ for (i = 0; i < npins; i++)
+ pins[i] = starfive_pinmux_pin(pinmux[i]);
+
+ map[nmaps].type = PIN_MAP_TYPE_MUX_GROUP;
+ map[nmaps].data.mux.function = np->name;
+ map[nmaps].data.mux.group = grpname;
+ nmaps += 1;
+
+ ret = pinctrl_generic_add_group(pctldev, grpname,
+ pins, npins, pinmux);
+ if (ret < 0) {
+ dev_err(dev, "error adding group %s: %d\n", grpname, ret);
+ goto put_child;
+ }
+
+ ret = pinconf_generic_parse_dt_config(child, pctldev,
+ &map[nmaps].data.configs.configs,
+ &map[nmaps].data.configs.num_configs);
+ if (ret) {
+ dev_err(dev, "error parsing pin config of group %s: %d\n",
+ grpname, ret);
+ goto put_child;
+ }
+
+ /* don't create a map if there are no pinconf settings */
+ if (map[nmaps].data.configs.num_configs == 0)
+ continue;
+
+ map[nmaps].type = PIN_MAP_TYPE_CONFIGS_GROUP;
+ map[nmaps].data.configs.group_or_pin = grpname;
+ nmaps += 1;
+ }
+
+ ret = pinmux_generic_add_function(pctldev, np->name,
+ pgnames, ngroups, NULL);
+ if (ret < 0) {
+ dev_err(dev, "error adding function %s: %d\n", np->name, ret);
+ goto free_map;
+ }
+ mutex_unlock(&sfp->mutex);
+
+ *maps = map;
+ *num_maps = nmaps;
+ return 0;
+
+put_child:
+ of_node_put(child);
+free_map:
+ pinctrl_utils_free_map(pctldev, map, nmaps);
+ mutex_unlock(&sfp->mutex);
+ return ret;
+}
+
+static const struct pinctrl_ops starfive_pinctrl_ops = {
+ .get_groups_count = pinctrl_generic_get_group_count,
+ .get_group_name = pinctrl_generic_get_group_name,
+ .get_group_pins = pinctrl_generic_get_group_pins,
+ .pin_dbg_show = starfive_pin_dbg_show,
+ .dt_node_to_map = starfive_dt_node_to_map,
+ .dt_free_map = pinctrl_utils_free_map,
+};
+
+void starfive_set_gpiomux(struct starfive_pinctrl *sfp, unsigned int pin,
+ unsigned int din, u32 dout, u32 doen)
+{
+ const struct starfive_pinctrl_soc_info *info = sfp->info;
+
+ unsigned int offset = 4 * (pin / 4);
+ unsigned int shift = 8 * (pin % 4);
+ u32 dout_mask = info->dout_mask << shift;
+ u32 done_mask = info->doen_mask << shift;
+ u32 ival, imask;
+ void __iomem *reg_dout;
+ void __iomem *reg_doen;
+ void __iomem *reg_din;
+ unsigned long flags;
+
+ reg_dout = sfp->base + info->dout_reg_base + offset;
+ reg_doen = sfp->base + info->doen_reg_base + offset;
+ dout <<= shift;
+ doen <<= shift;
+ if (din != GPI_NONE) {
+ unsigned int ioffset = 4 * (din / 4);
+ unsigned int ishift = 8 * (din % 4);
+
+ reg_din = sfp->base + info->gpi_reg_base + ioffset;
+ ival = (pin + 2) << ishift;
+ imask = info->gpi_mask << ishift;
+ } else {
+ reg_din = NULL;
+ }
+
+ raw_spin_lock_irqsave(&sfp->lock, flags);
+ dout |= readl_relaxed(reg_dout) & ~dout_mask;
+ writel_relaxed(dout, reg_dout);
+ doen |= readl_relaxed(reg_doen) & ~done_mask;
+ writel_relaxed(doen, reg_doen);
+ if (reg_din) {
+ ival |= readl_relaxed(reg_din) & ~imask;
+ writel_relaxed(ival, reg_din);
+ }
+ raw_spin_unlock_irqrestore(&sfp->lock, flags);
+}
+
+static int starfive_set_mux(struct pinctrl_dev *pctldev,
+ unsigned int fsel, unsigned int gsel)
+{
+ struct starfive_pinctrl *sfp = pinctrl_dev_get_drvdata(pctldev);
+ const struct starfive_pinctrl_soc_info *info = sfp->info;
+ const struct group_desc *group;
+ const u32 *pinmux;
+ unsigned int i;
+
+ group = pinctrl_generic_get_group(pctldev, gsel);
+ if (!group)
+ return -EINVAL;
+
+ pinmux = group->data;
+ for (i = 0; i < group->num_pins; i++) {
+ u32 v = pinmux[i];
+
+ if (info->starfive_set_one_pin_mux)
+ info->starfive_set_one_pin_mux(sfp,
+ starfive_pinmux_pin(v),
+ starfive_pinmux_din(v),
+ starfive_pinmux_dout(v),
+ starfive_pinmux_doen(v),
+ starfive_pinmux_function(v));
+ }
+
+ return 0;
+}
+
+static const struct pinmux_ops starfive_pinmux_ops = {
+ .get_functions_count = pinmux_generic_get_function_count,
+ .get_function_name = pinmux_generic_get_function_name,
+ .get_function_groups = pinmux_generic_get_function_groups,
+ .set_mux = starfive_set_mux,
+ .strict = true,
+};
+
+static const u8 starfive_drive_strength_mA[4] = { 2, 4, 8, 12 };
+
+static u32 starfive_padcfg_ds_to_mA(u32 padcfg)
+{
+ return starfive_drive_strength_mA[(padcfg >> 1) & 3U];
+}
+
+static u32 starfive_padcfg_ds_from_mA(u32 v)
+{
+ int i;
+
+ for (i = 0; i < 3; i++) {
+ if (v <= starfive_drive_strength_mA[i])
+ break;
+ }
+ return i << 1;
+}
+
+static void starfive_padcfg_rmw(struct starfive_pinctrl *sfp,
+ unsigned int pin, u32 mask, u32 value)
+{
+ const struct starfive_pinctrl_soc_info *info = sfp->info;
+ void __iomem *reg;
+ unsigned long flags;
+ int padcfg_base;
+
+ if (!info->starfive_get_padcfg_base)
+ return;
+
+ padcfg_base = info->starfive_get_padcfg_base(sfp, pin);
+ if (padcfg_base < 0)
+ return;
+
+ reg = sfp->base + padcfg_base + 4 * pin;
+ value &= mask;
+
+ raw_spin_lock_irqsave(&sfp->lock, flags);
+ value |= readl_relaxed(reg) & ~mask;
+ writel_relaxed(value, reg);
+ raw_spin_unlock_irqrestore(&sfp->lock, flags);
+}
+
+static int starfive_pinconf_get(struct pinctrl_dev *pctldev,
+ unsigned int pin, unsigned long *config)
+{
+ struct starfive_pinctrl *sfp = pinctrl_dev_get_drvdata(pctldev);
+ const struct starfive_pinctrl_soc_info *info = sfp->info;
+ int param = pinconf_to_config_param(*config);
+ u32 padcfg, arg;
+ bool enabled;
+ int padcfg_base;
+
+ if (!info->starfive_get_padcfg_base)
+ return 0;
+
+ padcfg_base = info->starfive_get_padcfg_base(sfp, pin);
+ if (padcfg_base < 0)
+ return 0;
+
+ padcfg = readl_relaxed(sfp->base + padcfg_base + 4 * pin);
+ switch (param) {
+ case PIN_CONFIG_BIAS_DISABLE:
+ enabled = !(padcfg & STARFIVE_PADCFG_BIAS);
+ arg = 0;
+ break;
+ case PIN_CONFIG_BIAS_PULL_DOWN:
+ enabled = padcfg & STARFIVE_PADCFG_PD;
+ arg = 1;
+ break;
+ case PIN_CONFIG_BIAS_PULL_UP:
+ enabled = padcfg & STARFIVE_PADCFG_PU;
+ arg = 1;
+ break;
+ case PIN_CONFIG_DRIVE_STRENGTH:
+ enabled = true;
+ arg = starfive_padcfg_ds_to_mA(padcfg);
+ break;
+ case PIN_CONFIG_INPUT_ENABLE:
+ enabled = padcfg & STARFIVE_PADCFG_IE;
+ arg = enabled;
+ break;
+ case PIN_CONFIG_INPUT_SCHMITT_ENABLE:
+ enabled = padcfg & STARFIVE_PADCFG_SMT;
+ arg = enabled;
+ break;
+ case PIN_CONFIG_SLEW_RATE:
+ enabled = true;
+ arg = !!(padcfg & STARFIVE_PADCFG_SLEW);
+ break;
+ default:
+ return -ENOTSUPP;
+ }
+
+ *config = pinconf_to_config_packed(param, arg);
+ return enabled ? 0 : -EINVAL;
+}
+
+static int starfive_pinconf_group_get(struct pinctrl_dev *pctldev,
+ unsigned int gsel,
+ unsigned long *config)
+{
+ const struct group_desc *group;
+
+ group = pinctrl_generic_get_group(pctldev, gsel);
+ if (!group)
+ return -EINVAL;
+
+ return starfive_pinconf_get(pctldev, group->pins[0], config);
+}
+
+static int starfive_pinconf_group_set(struct pinctrl_dev *pctldev,
+ unsigned int gsel,
+ unsigned long *configs,
+ unsigned int num_configs)
+{
+ struct starfive_pinctrl *sfp = pinctrl_dev_get_drvdata(pctldev);
+ const struct group_desc *group;
+ u16 mask, value;
+ int i;
+
+ group = pinctrl_generic_get_group(pctldev, gsel);
+ if (!group)
+ return -EINVAL;
+
+ mask = 0;
+ value = 0;
+ for (i = 0; i < num_configs; i++) {
+ int param = pinconf_to_config_param(configs[i]);
+ u32 arg = pinconf_to_config_argument(configs[i]);
+
+ switch (param) {
+ case PIN_CONFIG_BIAS_DISABLE:
+ mask |= STARFIVE_PADCFG_BIAS;
+ value &= ~STARFIVE_PADCFG_BIAS;
+ break;
+ case PIN_CONFIG_BIAS_PULL_DOWN:
+ if (arg == 0)
+ return -ENOTSUPP;
+ mask |= STARFIVE_PADCFG_BIAS;
+ value = (value & ~STARFIVE_PADCFG_BIAS) | STARFIVE_PADCFG_PD;
+ break;
+ case PIN_CONFIG_BIAS_PULL_UP:
+ if (arg == 0)
+ return -ENOTSUPP;
+ mask |= STARFIVE_PADCFG_BIAS;
+ value = (value & ~STARFIVE_PADCFG_BIAS) | STARFIVE_PADCFG_PU;
+ break;
+ case PIN_CONFIG_DRIVE_STRENGTH:
+ mask |= STARFIVE_PADCFG_DS_MASK;
+ value = (value & ~STARFIVE_PADCFG_DS_MASK) |
+ starfive_padcfg_ds_from_mA(arg);
+ break;
+ case PIN_CONFIG_INPUT_ENABLE:
+ mask |= STARFIVE_PADCFG_IE;
+ if (arg)
+ value |= STARFIVE_PADCFG_IE;
+ else
+ value &= ~STARFIVE_PADCFG_IE;
+ break;
+ case PIN_CONFIG_INPUT_SCHMITT_ENABLE:
+ mask |= STARFIVE_PADCFG_SMT;
+ if (arg)
+ value |= STARFIVE_PADCFG_SMT;
+ else
+ value &= ~STARFIVE_PADCFG_SMT;
+ break;
+ case PIN_CONFIG_SLEW_RATE:
+ mask |= STARFIVE_PADCFG_SLEW;
+ if (arg)
+ value |= STARFIVE_PADCFG_SLEW;
+ else
+ value &= ~STARFIVE_PADCFG_SLEW;
+ break;
+ default:
+ return -ENOTSUPP;
+ }
+ }
+
+ for (i = 0; i < group->num_pins; i++)
+ starfive_padcfg_rmw(sfp, group->pins[i], mask, value);
+
+ return 0;
+}
+
+#ifdef CONFIG_DEBUG_FS
+static void starfive_pinconf_dbg_show(struct pinctrl_dev *pctldev,
+ struct seq_file *s, unsigned int pin)
+{
+ struct starfive_pinctrl *sfp = pinctrl_dev_get_drvdata(pctldev);
+ const struct starfive_pinctrl_soc_info *info = sfp->info;
+ u32 value;
+ int padcfg_base;
+
+ if (!info->starfive_get_padcfg_base)
+ return;
+
+ padcfg_base = info->starfive_get_padcfg_base(sfp, pin);
+ if (padcfg_base < 0)
+ return;
+
+ value = readl_relaxed(sfp->base + padcfg_base + 4 * pin);
+ seq_printf(s, " (0x%02x)", value);
+}
+#else
+#define starfive_pinconf_dbg_show NULL
+#endif
+
+static const struct pinconf_ops starfive_pinconf_ops = {
+ .pin_config_get = starfive_pinconf_get,
+ .pin_config_group_get = starfive_pinconf_group_get,
+ .pin_config_group_set = starfive_pinconf_group_set,
+ .pin_config_dbg_show = starfive_pinconf_dbg_show,
+ .is_generic = true,
+};
+
+static int starfive_gpio_request(struct gpio_chip *gc, unsigned int gpio)
+{
+ return pinctrl_gpio_request(gc->base + gpio);
+}
+
+static void starfive_gpio_free(struct gpio_chip *gc, unsigned int gpio)
+{
+ pinctrl_gpio_free(gc->base + gpio);
+}
+
+static int starfive_gpio_get_direction(struct gpio_chip *gc,
+ unsigned int gpio)
+{
+ struct starfive_pinctrl *sfp = container_of(gc,
+ struct starfive_pinctrl, gc);
+ const struct starfive_pinctrl_soc_info *info = sfp->info;
+ unsigned int offset = 4 * (gpio / 4);
+ unsigned int shift = 8 * (gpio % 4);
+ u32 doen = readl_relaxed(sfp->base + info->doen_reg_base + offset);
+
+ doen = (doen >> shift) & info->doen_mask;
+
+ return doen == GPOEN_ENABLE ?
+ GPIO_LINE_DIRECTION_OUT : GPIO_LINE_DIRECTION_IN;
+}
+
+static int starfive_gpio_direction_input(struct gpio_chip *gc,
+ unsigned int gpio)
+{
+ struct starfive_pinctrl *sfp = container_of(gc,
+ struct starfive_pinctrl, gc);
+ const struct starfive_pinctrl_soc_info *info = sfp->info;
+
+ /* enable input and schmitt trigger */
+ starfive_padcfg_rmw(sfp, gpio,
+ STARFIVE_PADCFG_IE | STARFIVE_PADCFG_SMT,
+ STARFIVE_PADCFG_IE | STARFIVE_PADCFG_SMT);
+
+ if (info->starfive_set_one_pin_mux)
+ info->starfive_set_one_pin_mux(sfp, gpio,
+ GPI_NONE, GPOUT_LOW, GPOEN_DISABLE, 0);
+
+ return 0;
+}
+
+static int starfive_gpio_direction_output(struct gpio_chip *gc,
+ unsigned int gpio, int value)
+{
+ struct starfive_pinctrl *sfp = container_of(gc,
+ struct starfive_pinctrl, gc);
+ const struct starfive_pinctrl_soc_info *info = sfp->info;
+
+ if (info->starfive_set_one_pin_mux)
+ info->starfive_set_one_pin_mux(sfp, gpio,
+ GPI_NONE, value ? GPOUT_HIGH : GPOUT_LOW,
+ GPOEN_ENABLE, 0);
+
+ /* disable input, schmitt trigger and bias */
+ starfive_padcfg_rmw(sfp, gpio,
+ STARFIVE_PADCFG_IE | STARFIVE_PADCFG_SMT
+ | STARFIVE_PADCFG_BIAS, 0);
+ return 0;
+}
+
+static int starfive_gpio_get(struct gpio_chip *gc, unsigned int gpio)
+{
+ struct starfive_pinctrl *sfp = container_of(gc,
+ struct starfive_pinctrl, gc);
+ const struct starfive_pinctrl_soc_info *info = sfp->info;
+ void __iomem *reg = sfp->base + info->gpioin_reg_base
+ + 4 * (gpio / GPIO_NUM_PER_WORD);
+
+ return !!(readl_relaxed(reg) & BIT(gpio % GPIO_NUM_PER_WORD));
+}
+
+static void starfive_gpio_set(struct gpio_chip *gc,
+ unsigned int gpio, int value)
+{
+ struct starfive_pinctrl *sfp = container_of(gc,
+ struct starfive_pinctrl, gc);
+ const struct starfive_pinctrl_soc_info *info = sfp->info;
+ unsigned int offset = 4 * (gpio / 4);
+ unsigned int shift = 8 * (gpio % 4);
+ void __iomem *reg_dout = sfp->base + info->dout_reg_base + offset;
+ u32 dout = (value ? GPOUT_HIGH : GPOUT_LOW) << shift;
+ u32 mask = info->dout_mask << shift;
+ unsigned long flags;
+
+ raw_spin_lock_irqsave(&sfp->lock, flags);
+ dout |= readl_relaxed(reg_dout) & ~mask;
+ writel_relaxed(dout, reg_dout);
+ raw_spin_unlock_irqrestore(&sfp->lock, flags);
+}
+
+static int starfive_gpio_set_config(struct gpio_chip *gc,
+ unsigned int gpio, unsigned long config)
+{
+ struct starfive_pinctrl *sfp = container_of(gc,
+ struct starfive_pinctrl, gc);
+ u32 arg = pinconf_to_config_argument(config);
+ u32 value;
+ u32 mask;
+
+ switch (pinconf_to_config_param(config)) {
+ case PIN_CONFIG_BIAS_DISABLE:
+ mask = STARFIVE_PADCFG_BIAS;
+ value = 0;
+ break;
+ case PIN_CONFIG_BIAS_PULL_DOWN:
+ if (arg == 0)
+ return -ENOTSUPP;
+ mask = STARFIVE_PADCFG_BIAS;
+ value = STARFIVE_PADCFG_PD;
+ break;
+ case PIN_CONFIG_BIAS_PULL_UP:
+ if (arg == 0)
+ return -ENOTSUPP;
+ mask = STARFIVE_PADCFG_BIAS;
+ value = STARFIVE_PADCFG_PU;
+ break;
+ case PIN_CONFIG_DRIVE_PUSH_PULL:
+ return 0;
+ case PIN_CONFIG_INPUT_ENABLE:
+ mask = STARFIVE_PADCFG_IE;
+ value = arg ? STARFIVE_PADCFG_IE : 0;
+ break;
+ case PIN_CONFIG_INPUT_SCHMITT_ENABLE:
+ mask = STARFIVE_PADCFG_SMT;
+ value = arg ? STARFIVE_PADCFG_SMT : 0;
+ break;
+ default:
+ return -ENOTSUPP;
+ }
+
+ starfive_padcfg_rmw(sfp, gpio, mask, value);
+ return 0;
+}
+
+static int starfive_gpio_add_pin_ranges(struct gpio_chip *gc)
+{
+ struct starfive_pinctrl *sfp = container_of(gc,
+ struct starfive_pinctrl, gc);
+
+ sfp->gpios.name = sfp->gc.label;
+ sfp->gpios.base = sfp->gc.base;
+ sfp->gpios.pin_base = 0;
+ sfp->gpios.npins = sfp->gc.ngpio;
+ sfp->gpios.gc = &sfp->gc;
+ pinctrl_add_gpio_range(sfp->pctl, &sfp->gpios);
+ return 0;
+}
+
+static void starfive_irq_ack(struct irq_data *d)
+{
+ struct starfive_pinctrl *sfp = starfive_from_irq_data(d);
+ const struct starfive_gpio_irq_reg *irq_reg = sfp->info->irq_reg;
+ irq_hw_number_t gpio = irqd_to_hwirq(d);
+ void __iomem *ic = sfp->base + irq_reg->ic_reg_base
+ + 4 * (gpio / GPIO_NUM_PER_WORD);
+ u32 mask = BIT(gpio % GPIO_NUM_PER_WORD);
+ unsigned long flags;
+ u32 value;
+
+ raw_spin_lock_irqsave(&sfp->lock, flags);
+ value = readl_relaxed(ic) & ~mask;
+ writel_relaxed(value, ic);
+ writel_relaxed(value | mask, ic);
+ raw_spin_unlock_irqrestore(&sfp->lock, flags);
+}
+
+static void starfive_irq_mask(struct irq_data *d)
+{
+ struct starfive_pinctrl *sfp = starfive_from_irq_data(d);
+ const struct starfive_gpio_irq_reg *irq_reg = sfp->info->irq_reg;
+ irq_hw_number_t gpio = irqd_to_hwirq(d);
+ void __iomem *ie = sfp->base + irq_reg->ie_reg_base
+ + 4 * (gpio / GPIO_NUM_PER_WORD);
+ u32 mask = BIT(gpio % GPIO_NUM_PER_WORD);
+ unsigned long flags;
+ u32 value;
+
+ raw_spin_lock_irqsave(&sfp->lock, flags);
+ value = readl_relaxed(ie) & ~mask;
+ writel_relaxed(value, ie);
+ raw_spin_unlock_irqrestore(&sfp->lock, flags);
+
+ gpiochip_disable_irq(&sfp->gc, d->hwirq);
+}
+
+static void starfive_irq_mask_ack(struct irq_data *d)
+{
+ struct starfive_pinctrl *sfp = starfive_from_irq_data(d);
+ const struct starfive_gpio_irq_reg *irq_reg = sfp->info->irq_reg;
+ irq_hw_number_t gpio = irqd_to_hwirq(d);
+ void __iomem *ie = sfp->base + irq_reg->ie_reg_base
+ + 4 * (gpio / GPIO_NUM_PER_WORD);
+ void __iomem *ic = sfp->base + irq_reg->ic_reg_base
+ + 4 * (gpio / GPIO_NUM_PER_WORD);
+ u32 mask = BIT(gpio % GPIO_NUM_PER_WORD);
+ unsigned long flags;
+ u32 value;
+
+ raw_spin_lock_irqsave(&sfp->lock, flags);
+ value = readl_relaxed(ie) & ~mask;
+ writel_relaxed(value, ie);
+
+ value = readl_relaxed(ic) & ~mask;
+ writel_relaxed(value, ic);
+ writel_relaxed(value | mask, ic);
+ raw_spin_unlock_irqrestore(&sfp->lock, flags);
+}
+
+static void starfive_irq_unmask(struct irq_data *d)
+{
+ struct starfive_pinctrl *sfp = starfive_from_irq_data(d);
+ const struct starfive_gpio_irq_reg *irq_reg = sfp->info->irq_reg;
+ irq_hw_number_t gpio = irqd_to_hwirq(d);
+ void __iomem *ie = sfp->base + irq_reg->ie_reg_base
+ + 4 * (gpio / GPIO_NUM_PER_WORD);
+ u32 mask = BIT(gpio % GPIO_NUM_PER_WORD);
+ unsigned long flags;
+ u32 value;
+
+ gpiochip_enable_irq(&sfp->gc, d->hwirq);
+
+ raw_spin_lock_irqsave(&sfp->lock, flags);
+ value = readl_relaxed(ie) | mask;
+ writel_relaxed(value, ie);
+ raw_spin_unlock_irqrestore(&sfp->lock, flags);
+}
+
+static int starfive_irq_set_type(struct irq_data *d, unsigned int trigger)
+{
+ struct starfive_pinctrl *sfp = starfive_from_irq_data(d);
+ const struct starfive_gpio_irq_reg *irq_reg = sfp->info->irq_reg;
+ irq_hw_number_t gpio = irqd_to_hwirq(d);
+ void __iomem *base = sfp->base + 4 * (gpio / GPIO_NUM_PER_WORD);
+ u32 mask = BIT(gpio % GPIO_NUM_PER_WORD);
+ u32 irq_type, edge_both, polarity;
+ unsigned long flags;
+
+ switch (trigger) {
+ case IRQ_TYPE_EDGE_RISING:
+ irq_type = mask; /* 1: edge triggered */
+ edge_both = 0; /* 0: single edge */
+ polarity = mask; /* 1: rising edge */
+ break;
+ case IRQ_TYPE_EDGE_FALLING:
+ irq_type = mask; /* 1: edge triggered */
+ edge_both = 0; /* 0: single edge */
+ polarity = 0; /* 0: falling edge */
+ break;
+ case IRQ_TYPE_EDGE_BOTH:
+ irq_type = mask; /* 1: edge triggered */
+ edge_both = mask; /* 1: both edges */
+ polarity = 0; /* 0: ignored */
+ break;
+ case IRQ_TYPE_LEVEL_HIGH:
+ irq_type = 0; /* 0: level triggered */
+ edge_both = 0; /* 0: ignored */
+ polarity = mask; /* 1: high level */
+ break;
+ case IRQ_TYPE_LEVEL_LOW:
+ irq_type = 0; /* 0: level triggered */
+ edge_both = 0; /* 0: ignored */
+ polarity = 0; /* 0: low level */
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (trigger & IRQ_TYPE_EDGE_BOTH)
+ irq_set_handler_locked(d, handle_edge_irq);
+ else
+ irq_set_handler_locked(d, handle_level_irq);
+
+ raw_spin_lock_irqsave(&sfp->lock, flags);
+ irq_type |= readl_relaxed(base + irq_reg->is_reg_base) & ~mask;
+ writel_relaxed(irq_type, base + irq_reg->is_reg_base);
+
+ edge_both |= readl_relaxed(base + irq_reg->ibe_reg_base) & ~mask;
+ writel_relaxed(edge_both, base + irq_reg->ibe_reg_base);
+
+ polarity |= readl_relaxed(base + irq_reg->iev_reg_base) & ~mask;
+ writel_relaxed(polarity, base + irq_reg->iev_reg_base);
+ raw_spin_unlock_irqrestore(&sfp->lock, flags);
+ return 0;
+}
+
+static struct irq_chip starfive_irq_chip = {
+ .irq_ack = starfive_irq_ack,
+ .irq_mask = starfive_irq_mask,
+ .irq_mask_ack = starfive_irq_mask_ack,
+ .irq_unmask = starfive_irq_unmask,
+ .irq_set_type = starfive_irq_set_type,
+ .flags = IRQCHIP_IMMUTABLE | IRQCHIP_SET_TYPE_MASKED,
+ GPIOCHIP_IRQ_RESOURCE_HELPERS,
+};
+
+static void starfive_disable_clock(void *data)
+{
+ clk_disable_unprepare(data);
+}
+
+int starfive_pinctrl_probe(struct platform_device *pdev,
+ const struct starfive_pinctrl_soc_info *info)
+{
+ struct device *dev = &pdev->dev;
+ struct starfive_pinctrl *sfp;
+ struct pinctrl_desc *starfive_pinctrl_desc;
+ struct reset_control *rst;
+ struct clk *clk;
+ int ret;
+
+ if (!info || !info->pins || !info->npins) {
+ dev_err(dev, "wrong pinctrl info\n");
+ return -EINVAL;
+ }
+
+ sfp = devm_kzalloc(dev, sizeof(*sfp), GFP_KERNEL);
+ if (!sfp)
+ return -ENOMEM;
+
+ sfp->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(sfp->base))
+ return PTR_ERR(sfp->base);
+
+ clk = devm_clk_get_optional(dev, NULL);
+ if (IS_ERR(clk))
+ return dev_err_probe(dev, PTR_ERR(clk), "could not get clock\n");
+
+ rst = devm_reset_control_get_exclusive(dev, NULL);
+ if (IS_ERR(rst))
+ return dev_err_probe(dev, PTR_ERR(rst), "could not get reset\n");
+
+ /*
+ * we don't want to assert reset and risk undoing pin muxing for the
+ * early boot serial console, but let's make sure the reset line is
+ * deasserted in case someone runs a really minimal bootloader.
+ */
+ ret = reset_control_deassert(rst);
+ if (ret)
+ return dev_err_probe(dev, ret, "could not deassert reset\n");
+
+ if (clk) {
+ ret = clk_prepare_enable(clk);
+ if (ret)
+ return dev_err_probe(dev, ret, "could not enable clock\n");
+
+ ret = devm_add_action_or_reset(dev, starfive_disable_clock, clk);
+ if (ret)
+ return ret;
+ }
+
+ starfive_pinctrl_desc = devm_kzalloc(&pdev->dev,
+ sizeof(*starfive_pinctrl_desc),
+ GFP_KERNEL);
+ if (!starfive_pinctrl_desc)
+ return -ENOMEM;
+
+ starfive_pinctrl_desc->name = dev_name(dev);
+ starfive_pinctrl_desc->pins = info->pins;
+ starfive_pinctrl_desc->npins = info->npins;
+ starfive_pinctrl_desc->pctlops = &starfive_pinctrl_ops;
+ starfive_pinctrl_desc->pmxops = &starfive_pinmux_ops;
+ starfive_pinctrl_desc->confops = &starfive_pinconf_ops;
+ starfive_pinctrl_desc->owner = THIS_MODULE;
+
+ sfp->info = info;
+ sfp->dev = dev;
+ platform_set_drvdata(pdev, sfp);
+ sfp->gc.parent = dev;
+ raw_spin_lock_init(&sfp->lock);
+ mutex_init(&sfp->mutex);
+
+ ret = devm_pinctrl_register_and_init(dev,
+ starfive_pinctrl_desc,
+ sfp, &sfp->pctl);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "could not register pinctrl driver\n");
+
+ sfp->gc.label = dev_name(dev);
+ sfp->gc.owner = THIS_MODULE;
+ sfp->gc.request = starfive_gpio_request;
+ sfp->gc.free = starfive_gpio_free;
+ sfp->gc.get_direction = starfive_gpio_get_direction;
+ sfp->gc.direction_input = starfive_gpio_direction_input;
+ sfp->gc.direction_output = starfive_gpio_direction_output;
+ sfp->gc.get = starfive_gpio_get;
+ sfp->gc.set = starfive_gpio_set;
+ sfp->gc.set_config = starfive_gpio_set_config;
+ sfp->gc.add_pin_ranges = starfive_gpio_add_pin_ranges;
+ sfp->gc.base = info->gc_base;
+ sfp->gc.ngpio = info->ngpios;
+
+ starfive_irq_chip.name = sfp->gc.label;
+ gpio_irq_chip_set_chip(&sfp->gc.irq, &starfive_irq_chip);
+ sfp->gc.irq.parent_handler = info->starfive_gpio_irq_handler;
+ sfp->gc.irq.num_parents = 1;
+ sfp->gc.irq.parents = devm_kcalloc(dev, sfp->gc.irq.num_parents,
+ sizeof(*sfp->gc.irq.parents),
+ GFP_KERNEL);
+ if (!sfp->gc.irq.parents)
+ return -ENOMEM;
+ sfp->gc.irq.default_type = IRQ_TYPE_NONE;
+ sfp->gc.irq.handler = handle_bad_irq;
+ sfp->gc.irq.init_hw = info->starfive_gpio_init_hw;
+
+ ret = platform_get_irq(pdev, 0);
+ if (ret < 0)
+ return ret;
+ sfp->gc.irq.parents[0] = ret;
+
+ ret = devm_gpiochip_add_data(dev, &sfp->gc, sfp);
+ if (ret)
+ return dev_err_probe(dev, ret, "could not register gpiochip\n");
+
+ irq_domain_set_pm_device(sfp->gc.irq.domain, dev);
+
+ dev_info(dev, "StarFive GPIO chip registered %d GPIOs\n", sfp->gc.ngpio);
+
+ return pinctrl_enable(sfp->pctl);
+}
+
+MODULE_DESCRIPTION("Pinctrl driver for the StarFive JH7110 SoC");
+MODULE_AUTHOR("Emil Renner Berthing <[email protected]>");
+MODULE_AUTHOR("Jianlong Huang <[email protected]>");
diff --git a/drivers/pinctrl/starfive/pinctrl-starfive.h b/drivers/pinctrl/starfive/pinctrl-starfive.h
new file mode 100644
index 000000000000..54ffdb6412f1
--- /dev/null
+++ b/drivers/pinctrl/starfive/pinctrl-starfive.h
@@ -0,0 +1,72 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Pinctrl / GPIO driver for StarFive JH7110 SoC
+ *
+ * Copyright (C) 2022 StarFive Technology Co., Ltd.
+ */
+
+#ifndef __DRIVERS_PINCTRL_STARFIVE_H__
+#define __DRIVERS_PINCTRL_STARFIVE_H__
+
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinmux.h>
+
+struct starfive_pinctrl {
+ struct device *dev;
+ struct gpio_chip gc;
+ struct pinctrl_gpio_range gpios;
+ raw_spinlock_t lock;
+ void __iomem *base;
+ struct pinctrl_dev *pctl;
+ /* register read/write mutex */
+ struct mutex mutex;
+ const struct starfive_pinctrl_soc_info *info;
+};
+
+struct starfive_gpio_irq_reg {
+ unsigned int is_reg_base;
+ unsigned int ic_reg_base;
+ unsigned int ibe_reg_base;
+ unsigned int iev_reg_base;
+ unsigned int ie_reg_base;
+ unsigned int ris_reg_base;
+ unsigned int mis_reg_base;
+};
+
+struct starfive_pinctrl_soc_info {
+ const struct pinctrl_pin_desc *pins;
+ unsigned int npins;
+ unsigned int ngpios;
+ unsigned int gc_base;
+ unsigned int flags;
+
+ /* gpio dout/doen/din/gpioinput register */
+ unsigned int dout_reg_base;
+ unsigned int dout_mask;
+ unsigned int doen_reg_base;
+ unsigned int doen_mask;
+ unsigned int gpi_reg_base;
+ unsigned int gpi_mask;
+ unsigned int gpioin_reg_base;
+
+ const struct starfive_gpio_irq_reg *irq_reg;
+
+ /* generic pinmux */
+ int (*starfive_set_one_pin_mux)(struct starfive_pinctrl *sfp,
+ unsigned int pin,
+ unsigned int din, u32 dout,
+ u32 doen, u32 func);
+ /* gpio chip */
+ int (*starfive_get_padcfg_base)(struct starfive_pinctrl *sfp,
+ unsigned int pin);
+ void (*starfive_gpio_irq_handler)(struct irq_desc *desc);
+ int (*starfive_gpio_init_hw)(struct gpio_chip *gc);
+};
+
+void starfive_set_gpiomux(struct starfive_pinctrl *sfp, unsigned int pin,
+ unsigned int din, u32 dout, u32 doen);
+int starfive_pinctrl_probe(struct platform_device *pdev,
+ const struct starfive_pinctrl_soc_info *info);
+struct starfive_pinctrl *starfive_from_irq_desc(struct irq_desc *desc);
+
+#endif /* __DRIVERS_PINCTRL_STARFIVE_H__ */
--
2.38.1


2022-11-18 02:10:10

by Hal Feng

[permalink] [raw]
Subject: [PATCH v2 1/5] dt-bindings: pinctrl: Add StarFive JH7110 pinctrl definitions

From: Jianlong Huang <[email protected]>

Add pinctrl definitions for StarFive JH7110 SoC.

Co-developed-by: Emil Renner Berthing <[email protected]>
Signed-off-by: Emil Renner Berthing <[email protected]>
Signed-off-by: Jianlong Huang <[email protected]>
Signed-off-by: Hal Feng <[email protected]>
---
.../pinctrl/pinctrl-starfive-jh7110.h | 427 ++++++++++++++++++
1 file changed, 427 insertions(+)
create mode 100644 include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h

diff --git a/include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h b/include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h
new file mode 100644
index 000000000000..fb02345caa27
--- /dev/null
+++ b/include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h
@@ -0,0 +1,427 @@
+/* SPDX-License-Identifier: GPL-2.0 OR MIT */
+/*
+ * Copyright (C) 2022 Emil Renner Berthing <[email protected]>
+ * Copyright (C) 2022 StarFive Technology Co., Ltd.
+ */
+
+#ifndef __DT_BINDINGS_PINCTRL_STARFIVE_JH7110_H__
+#define __DT_BINDINGS_PINCTRL_STARFIVE_JH7110_H__
+
+/*
+ * mux bits:
+ * | 31 - 24 | 23 - 16 | 15 - 10 | 9 - 8 | 7 - 0 |
+ * | din | dout | doen | function | gpio nr |
+ *
+ * dout: output signal
+ * doen: output enable signal
+ * din: optional input signal, 0xff = none
+ * function:
+ * gpio nr: gpio number, 0 - 63
+ */
+#define GPIOMUX(n, dout, doen, din) ( \
+ (((din) & 0xff) << 24) | \
+ (((dout) & 0xff) << 16) | \
+ (((doen) & 0x3f) << 10) | \
+ ((n) & 0x3f))
+
+#define PINMUX(n, func) ((1 << 10) | (((func) & 0x3) << 8) | ((n) & 0xff))
+
+/* sys_iomux pin */
+#define PAD_GPIO0 0
+#define PAD_GPIO1 1
+#define PAD_GPIO2 2
+#define PAD_GPIO3 3
+#define PAD_GPIO4 4
+#define PAD_GPIO5 5
+#define PAD_GPIO6 6
+#define PAD_GPIO7 7
+#define PAD_GPIO8 8
+#define PAD_GPIO9 9
+#define PAD_GPIO10 10
+#define PAD_GPIO11 11
+#define PAD_GPIO12 12
+#define PAD_GPIO13 13
+#define PAD_GPIO14 14
+#define PAD_GPIO15 15
+#define PAD_GPIO16 16
+#define PAD_GPIO17 17
+#define PAD_GPIO18 18
+#define PAD_GPIO19 19
+#define PAD_GPIO20 20
+#define PAD_GPIO21 21
+#define PAD_GPIO22 22
+#define PAD_GPIO23 23
+#define PAD_GPIO24 24
+#define PAD_GPIO25 25
+#define PAD_GPIO26 26
+#define PAD_GPIO27 27
+#define PAD_GPIO28 28
+#define PAD_GPIO29 29
+#define PAD_GPIO30 30
+#define PAD_GPIO31 31
+#define PAD_GPIO32 32
+#define PAD_GPIO33 33
+#define PAD_GPIO34 34
+#define PAD_GPIO35 35
+#define PAD_GPIO36 36
+#define PAD_GPIO37 37
+#define PAD_GPIO38 38
+#define PAD_GPIO39 39
+#define PAD_GPIO40 40
+#define PAD_GPIO41 41
+#define PAD_GPIO42 42
+#define PAD_GPIO43 43
+#define PAD_GPIO44 44
+#define PAD_GPIO45 45
+#define PAD_GPIO46 46
+#define PAD_GPIO47 47
+#define PAD_GPIO48 48
+#define PAD_GPIO49 49
+#define PAD_GPIO50 50
+#define PAD_GPIO51 51
+#define PAD_GPIO52 52
+#define PAD_GPIO53 53
+#define PAD_GPIO54 54
+#define PAD_GPIO55 55
+#define PAD_GPIO56 56
+#define PAD_GPIO57 57
+#define PAD_GPIO58 58
+#define PAD_GPIO59 59
+#define PAD_GPIO60 60
+#define PAD_GPIO61 61
+#define PAD_GPIO62 62
+#define PAD_GPIO63 63
+#define PAD_SD0_CLK 64
+#define PAD_SD0_CMD 65
+#define PAD_SD0_DATA0 66
+#define PAD_SD0_DATA1 67
+#define PAD_SD0_DATA2 68
+#define PAD_SD0_DATA3 69
+#define PAD_SD0_DATA4 70
+#define PAD_SD0_DATA5 71
+#define PAD_SD0_DATA6 72
+#define PAD_SD0_DATA7 73
+#define PAD_SD0_STRB 74
+#define PAD_GMAC1_MDC 75
+#define PAD_GMAC1_MDIO 76
+#define PAD_GMAC1_RXD0 77
+#define PAD_GMAC1_RXD1 78
+#define PAD_GMAC1_RXD2 79
+#define PAD_GMAC1_RXD3 80
+#define PAD_GMAC1_RXDV 81
+#define PAD_GMAC1_RXC 82
+#define PAD_GMAC1_TXD0 83
+#define PAD_GMAC1_TXD1 84
+#define PAD_GMAC1_TXD2 85
+#define PAD_GMAC1_TXD3 86
+#define PAD_GMAC1_TXEN 87
+#define PAD_GMAC1_TXC 88
+#define PAD_QSPI_SCLK 89
+#define PAD_QSPI_CS0 90
+#define PAD_QSPI_DATA0 91
+#define PAD_QSPI_DATA1 92
+#define PAD_QSPI_DATA2 93
+#define PAD_QSPI_DATA3 94
+
+/* aon_iomux pin */
+#define PAD_TESTEN 0
+#define PAD_RGPIO0 1
+#define PAD_RGPIO1 2
+#define PAD_RGPIO2 3
+#define PAD_RGPIO3 4
+#define PAD_RSTN 5
+#define PAD_GMAC0_MDC 6
+#define PAD_GMAC0_MDIO 7
+#define PAD_GMAC0_RXD0 8
+#define PAD_GMAC0_RXD1 9
+#define PAD_GMAC0_RXD2 10
+#define PAD_GMAC0_RXD3 11
+#define PAD_GMAC0_RXDV 12
+#define PAD_GMAC0_RXC 13
+#define PAD_GMAC0_TXD0 14
+#define PAD_GMAC0_TXD1 15
+#define PAD_GMAC0_TXD2 16
+#define PAD_GMAC0_TXD3 17
+#define PAD_GMAC0_TXEN 18
+#define PAD_GMAC0_TXC 19
+
+/* sys_iomux dout */
+#define GPOUT_LOW 0
+#define GPOUT_HIGH 1
+#define GPOUT_SYS_WAVE511_UART_TX 2
+#define GPOUT_SYS_CAN0_STBY 3
+#define GPOUT_SYS_CAN0_TST_NEXT_BIT 4
+#define GPOUT_SYS_CAN0_TST_SAMPLE_POINT 5
+#define GPOUT_SYS_CAN0_TXD 6
+#define GPOUT_SYS_USB_DRIVE_VBUS 7
+#define GPOUT_SYS_QSPI_CS1 8
+#define GPOUT_SYS_SPDIF 9
+#define GPOUT_SYS_HDMI_CEC_SDA 10
+#define GPOUT_SYS_HDMI_DDC_SCL 11
+#define GPOUT_SYS_HDMI_DDC_SDA 12
+#define GPOUT_SYS_WATCHDOG 13
+#define GPOUT_SYS_I2C0_CLK 14
+#define GPOUT_SYS_I2C0_DATA 15
+#define GPOUT_SYS_SDIO0_BACK_END_POWER 16
+#define GPOUT_SYS_SDIO0_CARD_POWER_EN 17
+#define GPOUT_SYS_SDIO0_CCMD_OD_PULLUP_EN 18
+#define GPOUT_SYS_SDIO0_RST 19
+#define GPOUT_SYS_UART0_TX 20
+#define GPOUT_SYS_HIFI4_JTAG_TDO 21
+#define GPOUT_SYS_JTAG_TDO 22
+#define GPOUT_SYS_PDM_MCLK 23
+#define GPOUT_SYS_PWM_CHANNEL0 24
+#define GPOUT_SYS_PWM_CHANNEL1 25
+#define GPOUT_SYS_PWM_CHANNEL2 26
+#define GPOUT_SYS_PWM_CHANNEL3 27
+#define GPOUT_SYS_PWMDAC_LEFT 28
+#define GPOUT_SYS_PWMDAC_RIGHT 29
+#define GPOUT_SYS_SPI0_CLK 30
+#define GPOUT_SYS_SPI0_FSS 31
+#define GPOUT_SYS_SPI0_TXD 32
+#define GPOUT_SYS_GMAC_PHYCLK 33
+#define GPOUT_SYS_I2SRX_BCLK 34
+#define GPOUT_SYS_I2SRX_LRCK 35
+#define GPOUT_SYS_I2STX0_BCLK 36
+#define GPOUT_SYS_I2STX0_LRCK 37
+#define GPOUT_SYS_MCLK 38
+#define GPOUT_SYS_TDM_CLK 39
+#define GPOUT_SYS_TDM_SYNC 40
+#define GPOUT_SYS_TDM_TXD 41
+#define GPOUT_SYS_TRACE_DATA0 42
+#define GPOUT_SYS_TRACE_DATA1 43
+#define GPOUT_SYS_TRACE_DATA2 44
+#define GPOUT_SYS_TRACE_DATA3 45
+#define GPOUT_SYS_TRACE_REF 46
+#define GPOUT_SYS_CAN1_STBY 47
+#define GPOUT_SYS_CAN1_TST_NEXT_BIT 48
+#define GPOUT_SYS_CAN1_TST_SAMPLE_POINT 49
+#define GPOUT_SYS_CAN1_TXD 50
+#define GPOUT_SYS_I2C1_CLK 51
+#define GPOUT_SYS_I2C1_DATA 52
+#define GPOUT_SYS_SDIO1_BACK_END_POWER 53
+#define GPOUT_SYS_SDIO1_CARD_POWER_EN 54
+#define GPOUT_SYS_SDIO1_CLK 55
+#define GPOUT_SYS_SDIO1_CMD_OD_PULLUP_EN 56
+#define GPOUT_SYS_SDIO1_CMD 57
+#define GPOUT_SYS_SDIO1_DATA0 58
+#define GPOUT_SYS_SDIO1_DATA1 59
+#define GPOUT_SYS_SDIO1_DATA2 60
+#define GPOUT_SYS_SDIO1_DATA3 61
+#define GPOUT_SYS_SDIO1_DATA4 63
+#define GPOUT_SYS_SDIO1_DATA5 63
+#define GPOUT_SYS_SDIO1_DATA6 64
+#define GPOUT_SYS_SDIO1_DATA7 65
+#define GPOUT_SYS_SDIO1_RST 66
+#define GPOUT_SYS_UART1_RTS 67
+#define GPOUT_SYS_UART1_TX 68
+#define GPOUT_SYS_I2STX1_SDO0 69
+#define GPOUT_SYS_I2STX1_SDO1 70
+#define GPOUT_SYS_I2STX1_SDO2 71
+#define GPOUT_SYS_I2STX1_SDO3 72
+#define GPOUT_SYS_SPI1_CLK 73
+#define GPOUT_SYS_SPI1_FSS 74
+#define GPOUT_SYS_SPI1_TXD 75
+#define GPOUT_SYS_I2C2_CLK 76
+#define GPOUT_SYS_I2C2_DATA 77
+#define GPOUT_SYS_UART2_RTS 78
+#define GPOUT_SYS_UART2_TX 79
+#define GPOUT_SYS_SPI2_CLK 80
+#define GPOUT_SYS_SPI2_FSS 81
+#define GPOUT_SYS_SPI2_TXD 82
+#define GPOUT_SYS_I2C3_CLK 83
+#define GPOUT_SYS_I2C3_DATA 84
+#define GPOUT_SYS_UART3_TX 85
+#define GPOUT_SYS_SPI3_CLK 86
+#define GPOUT_SYS_SPI3_FSS 87
+#define GPOUT_SYS_SPI3_TXD 88
+#define GPOUT_SYS_I2C4_CLK 89
+#define GPOUT_SYS_I2C4_DATA 90
+#define GPOUT_SYS_UART4_RTS 91
+#define GPOUT_SYS_UART4_TX 92
+#define GPOUT_SYS_SPI4_CLK 93
+#define GPOUT_SYS_SPI4_FSS 94
+#define GPOUT_SYS_SPI4_TXD 95
+#define GPOUT_SYS_I2C5_CLK 96
+#define GPOUT_SYS_I2C5_DATA 97
+#define GPOUT_SYS_UART5_RTS 98
+#define GPOUT_SYS_UART5_TX 99
+#define GPOUT_SYS_SPI5_CLK 100
+#define GPOUT_SYS_SPI5_FSS 101
+#define GPOUT_SYS_SPI5_TXD 102
+#define GPOUT_SYS_I2C6_CLK 103
+#define GPOUT_SYS_I2C6_DATA 104
+#define GPOUT_SYS_SPI6_CLK 105
+#define GPOUT_SYS_SPI6_FSS 106
+#define GPOUT_SYS_SPI6_TXD 107
+
+/* aon_iomux dout */
+#define GPOUT_AON_CLK_32K_OUT 2
+#define GPOUT_AON_PTC0_PWM4 3
+#define GPOUT_AON_PTC0_PWM5 4
+#define GPOUT_AON_PTC0_PWM6 5
+#define GPOUT_AON_PTC0_PWM7 6
+#define GPOUT_AON_CLK_GCLK0 7
+#define GPOUT_AON_CLK_GCLK1 8
+#define GPOUT_AON_CLK_GCLK2 9
+
+/* sys_iomux doen */
+#define GPOEN_ENABLE 0
+#define GPOEN_DISABLE 1
+#define GPOEN_SYS_HDMI_CEC_SDA 2
+#define GPOEN_SYS_HDMI_DDC_SCL 3
+#define GPOEN_SYS_HDMI_DDC_SDA 4
+#define GPOEN_SYS_I2C0_CLK 5
+#define GPOEN_SYS_I2C0_DATA 6
+#define GPOEN_SYS_HIFI4_JTAG_TDO 7
+#define GPOEN_SYS_JTAG_TDO 8
+#define GPOEN_SYS_PWM0_CHANNEL0 9
+#define GPOEN_SYS_PWM0_CHANNEL1 10
+#define GPOEN_SYS_PWM0_CHANNEL2 11
+#define GPOEN_SYS_PWM0_CHANNEL3 12
+#define GPOEN_SYS_SPI0_NSSPCTL 13
+#define GPOEN_SYS_SPI0_NSSP 14
+#define GPOEN_SYS_TDM_SYNC 15
+#define GPOEN_SYS_TDM_TXD 16
+#define GPOEN_SYS_I2C1_CLK 17
+#define GPOEN_SYS_I2C1_DATA 18
+#define GPOEN_SYS_SDIO1_CMD 19
+#define GPOEN_SYS_SDIO1_DATA0 20
+#define GPOEN_SYS_SDIO1_DATA1 21
+#define GPOEN_SYS_SDIO1_DATA2 22
+#define GPOEN_SYS_SDIO1_DATA3 23
+#define GPOEN_SYS_SDIO1_DATA4 24
+#define GPOEN_SYS_SDIO1_DATA5 25
+#define GPOEN_SYS_SDIO1_DATA6 26
+#define GPOEN_SYS_SDIO1_DATA7 27
+#define GPOEN_SYS_SPI1_NSSPCTL 28
+#define GPOEN_SYS_SPI1_NSSP 29
+#define GPOEN_SYS_I2C2_CLK 30
+#define GPOEN_SYS_I2C2_DATA 31
+#define GPOEN_SYS_SPI2_NSSPCTL 32
+#define GPOEN_SYS_SPI2_NSSP 33
+#define GPOEN_SYS_I2C3_CLK 34
+#define GPOEN_SYS_I2C3_DATA 35
+#define GPOEN_SYS_SPI3_NSSPCTL 36
+#define GPOEN_SYS_SPI3_NSSP 37
+#define GPOEN_SYS_I2C4_CLK 38
+#define GPOEN_SYS_I2C4_DATA 39
+#define GPOEN_SYS_SPI4_NSSPCTL 40
+#define GPOEN_SYS_SPI4_NSSP 41
+#define GPOEN_SYS_I2C5_CLK 42
+#define GPOEN_SYS_I2C5_DATA 43
+#define GPOEN_SYS_SPI5_NSSPCTL 44
+#define GPOEN_SYS_SPI5_NSSP 45
+#define GPOEN_SYS_I2C6_CLK 46
+#define GPOEN_SYS_I2C6_DATA 47
+#define GPOEN_SYS_SPI6_NSSPCTL 48
+#define GPOEN_SYS_SPI6_NSSP 49
+
+/* aon_iomux doen */
+#define GPOEN_AON_PTC0_OE_N_4 2
+#define GPOEN_AON_PTC0_OE_N_5 3
+#define GPOEN_AON_PTC0_OE_N_6 4
+#define GPOEN_AON_PTC0_OE_N_7 5
+
+/* sys_iomux gin */
+#define GPI_NONE 255
+
+#define GPI_SYS_WAVE511_UART_RX 0
+#define GPI_SYS_CAN0_RXD 1
+#define GPI_SYS_USB_OVERCURRENT 2
+#define GPI_SYS_SPDIF 3
+#define GPI_SYS_JTAG_RST 4
+#define GPI_SYS_HDMI_CEC_SDA 5
+#define GPI_SYS_HDMI_DDC_SCL 6
+#define GPI_SYS_HDMI_DDC_SDA 7
+#define GPI_SYS_HDMI_HPD 8
+#define GPI_SYS_I2C0_CLK 9
+#define GPI_SYS_I2C0_DATA 10
+#define GPI_SYS_SDIO0_CD 11
+#define GPI_SYS_SDIO0_INT 12
+#define GPI_SYS_SDIO0_WP 13
+#define GPI_SYS_UART0_RX 14
+#define GPI_SYS_HIFI4_JTAG_TCK 15
+#define GPI_SYS_HIFI4_JTAG_TDI 16
+#define GPI_SYS_HIFI4_JTAG_TMS 17
+#define GPI_SYS_HIFI4_JTAG_RST 18
+#define GPI_SYS_JTAG_TDI 19
+#define GPI_SYS_JTAG_TMS 20
+#define GPI_SYS_PDM_DMIC0 21
+#define GPI_SYS_PDM_DMIC1 22
+#define GPI_SYS_I2SRX_SDIN0 23
+#define GPI_SYS_I2SRX_SDIN1 24
+#define GPI_SYS_I2SRX_SDIN2 25
+#define GPI_SYS_SPI0_CLK 26
+#define GPI_SYS_SPI0_FSS 27
+#define GPI_SYS_SPI0_RXD 28
+#define GPI_SYS_JTAG_TCK 29
+#define GPI_SYS_MCLK_EXT 30
+#define GPI_SYS_I2SRX_BCLK 31
+#define GPI_SYS_I2SRX_LRCK 32
+#define GPI_SYS_I2STX0_BCLK 33
+#define GPI_SYS_I2STX0_LRCK 34
+#define GPI_SYS_TDM_CLK 35
+#define GPI_SYS_TDM_RXD 36
+#define GPI_SYS_TDM_SYNC 37
+#define GPI_SYS_CAN1_RXD 38
+#define GPI_SYS_I2C1_CLK 39
+#define GPI_SYS_I2C1_DATA 40
+#define GPI_SYS_SDIO1_CD 41
+#define GPI_SYS_SDIO1_INT 42
+#define GPI_SYS_SDIO1_WP 43
+#define GPI_SYS_SDIO1_CMD 44
+#define GPI_SYS_SDIO1_DATA0 45
+#define GPI_SYS_SDIO1_DATA1 46
+#define GPI_SYS_SDIO1_DATA2 47
+#define GPI_SYS_SDIO1_DATA3 48
+#define GPI_SYS_SDIO1_DATA4 49
+#define GPI_SYS_SDIO1_DATA5 50
+#define GPI_SYS_SDIO1_DATA6 51
+#define GPI_SYS_SDIO1_DATA7 52
+#define GPI_SYS_SDIO1_STRB 53
+#define GPI_SYS_UART1_CTS 54
+#define GPI_SYS_UART1_RX 55
+#define GPI_SYS_SPI1_CLK 56
+#define GPI_SYS_SPI1_FSS 57
+#define GPI_SYS_SPI1_RXD 58
+#define GPI_SYS_I2C2_CLK 59
+#define GPI_SYS_I2C2_DATA 60
+#define GPI_SYS_UART2_CTS 61
+#define GPI_SYS_UART2_RX 62
+#define GPI_SYS_SPI2_CLK 63
+#define GPI_SYS_SPI2_FSS 64
+#define GPI_SYS_SPI2_RXD 65
+#define GPI_SYS_I2C3_CLK 66
+#define GPI_SYS_I2C3_DATA 67
+#define GPI_SYS_UART3_RX 68
+#define GPI_SYS_SPI3_CLK 69
+#define GPI_SYS_SPI3_FSS 70
+#define GPI_SYS_SPI3_RXD 71
+#define GPI_SYS_I2C4_CLK 72
+#define GPI_SYS_I2C4_DATA 73
+#define GPI_SYS_UART4_CTS 74
+#define GPI_SYS_UART4_RX 75
+#define GPI_SYS_SPI4_CLK 76
+#define GPI_SYS_SPI4_FSS 77
+#define GPI_SYS_SPI4_RXD 78
+#define GPI_SYS_I2C5_CLK 79
+#define GPI_SYS_I2C5_DATA 80
+#define GPI_SYS_UART5_CTS 81
+#define GPI_SYS_UART5_RX 82
+#define GPI_SYS_SPI5_CLK 83
+#define GPI_SYS_SPI5_FSS 84
+#define GPI_SYS_SPI5_RXD 85
+#define GPI_SYS_I2C6_CLK 86
+#define GPI_SYS_I2C6_DATA 87
+#define GPI_SYS_SPI6_CLK 88
+#define GPI_SYS_SPI6_FSS 89
+#define GPI_SYS_SPI6_RXD 90
+
+/* aon_iomux gin */
+#define GPI_AON_PMU_GPIO_WAKEUP_0 0
+#define GPI_AON_PMU_GPIO_WAKEUP_1 1
+#define GPI_AON_PMU_GPIO_WAKEUP_2 2
+#define GPI_AON_PMU_GPIO_WAKEUP_3 3
+
+#endif
--
2.38.1


2022-11-18 02:11:28

by Hal Feng

[permalink] [raw]
Subject: [PATCH v2 3/5] dt-bindings: pinctrl: Add StarFive JH7110 aon pinctrl

From: Jianlong Huang <[email protected]>

Add pinctrl bindings for StarFive JH7110 SoC aon pinctrl controller.

Signed-off-by: Jianlong Huang <[email protected]>
Signed-off-by: Hal Feng <[email protected]>
---
.../pinctrl/starfive,jh7110-aon-pinctrl.yaml | 134 ++++++++++++++++++
1 file changed, 134 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml

diff --git a/Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml
new file mode 100644
index 000000000000..1dd000e1f614
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml
@@ -0,0 +1,134 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/starfive,jh7110-aon-pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: StarFive JH7110 Aon Pin Controller
+
+description: |
+ Bindings for the JH7110 RISC-V SoC from StarFive Technology Ltd.
+
+ Out of the SoC's many pins only the ones named PAD_GPIO0 to PAD_GPIO4
+ can be multiplexed and have configurable bias, drive strength,
+ schmitt trigger etc.
+ Some peripherals have their I/O go through the 4 "GPIOs". This also
+ includes PWM.
+
+maintainers:
+ - Jianlong Huang <[email protected]>
+
+properties:
+ compatible:
+ const: starfive,jh7110-aon-pinctrl
+
+ reg:
+ maxItems: 1
+
+ reg-names:
+ items:
+ - const: control
+
+ clocks:
+ maxItems: 1
+
+ resets:
+ maxItems: 1
+
+ gpio-controller: true
+
+ "#gpio-cells":
+ const: 2
+
+ interrupts:
+ maxItems: 1
+ description: The GPIO parent interrupt.
+
+ interrupt-controller: true
+
+ "#interrupt-cells":
+ const: 2
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - gpio-controller
+ - "#gpio-cells"
+ - interrupts
+ - interrupt-controller
+ - "#interrupt-cells"
+
+patternProperties:
+ '-[0-9]+$':
+ type: object
+ patternProperties:
+ '-pins$':
+ type: object
+ description: |
+ A pinctrl node should contain at least one subnode representing the
+ pinctrl groups available on the machine. Each subnode will list the
+ pins it needs, and how they should be configured, with regard to
+ muxer configuration, system signal configuration, pin groups for
+ vin/vout module, pin voltage, mux functions for output, mux functions
+ for output enable, mux functions for input.
+
+ properties:
+ pinmux:
+ description: |
+ The list of GPIOs and their mux settings that properties in the
+ node apply to. This should be set using the GPIOMUX macro.
+ $ref: "/schemas/pinctrl/pinmux-node.yaml#/properties/pinmux"
+
+ bias-disable: true
+
+ bias-pull-up:
+ type: boolean
+
+ bias-pull-down:
+ type: boolean
+
+ drive-strength:
+ enum: [ 2, 4, 8, 12 ]
+
+ input-enable: true
+
+ input-disable: true
+
+ input-schmitt-enable: true
+
+ input-schmitt-disable: true
+
+ slew-rate:
+ maximum: 1
+
+ additionalProperties: false
+
+ additionalProperties: false
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/starfive-jh7110.h>
+ #include <dt-bindings/reset/starfive-jh7110.h>
+ #include <dt-bindings/pinctrl/pinctrl-starfive-jh7110.h>
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ gpioa: gpio@17020000 {
+ compatible = "starfive,jh7110-aon-pinctrl";
+ reg = <0x0 0x17020000 0x0 0x10000>;
+ reg-names = "control";
+ resets = <&aoncrg_rst JH7110_AONRST_AON_IOMUX>;
+ interrupts = <85>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ #gpio-cells = <2>;
+ gpio-controller;
+ };
+ };
+
+...
--
2.38.1


2022-11-18 07:29:14

by Hal Feng

[permalink] [raw]
Subject: Re: [PATCH v2 0/5] Basic pinctrl support for StarFive JH7110 RISC-V SoC

On Fri, 18 Nov 2022 09:11:03 +0800, Hal Feng wrote:
> The original patch series "Basic StarFive JH7110 RISC-V SoC support" [1]
> is split into 3 patch series. They respectively add basic clock&reset,
> pinctrl and device tree support for StarFive JH7110 SoC. These patch
> series are independent, but the Visionfive2 board can boot up successfully

Note that this patch series depends on the patch series [1].

[1] https://lore.kernel.org/all/[email protected]/

> only if all these patches series applied. This one adds basic pinctrl


2022-11-21 08:53:03

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] dt-bindings: pinctrl: Add StarFive JH7110 pinctrl definitions

On 21/11/2022 09:38, Krzysztof Kozlowski wrote:
> On 18/11/2022 02:11, Hal Feng wrote:
>> From: Jianlong Huang <[email protected]>
>>
>> Add pinctrl definitions for StarFive JH7110 SoC.
>>
>> Co-developed-by: Emil Renner Berthing <[email protected]>
>> Signed-off-by: Emil Renner Berthing <[email protected]>
>> Signed-off-by: Jianlong Huang <[email protected]>
>> Signed-off-by: Hal Feng <[email protected]>
>> ---
>> .../pinctrl/pinctrl-starfive-jh7110.h | 427 ++++++++++++++++++
>> 1 file changed, 427 insertions(+)
>> create mode 100644 include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h
>>
>> diff --git a/include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h b/include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h
>> new file mode 100644
>> index 000000000000..fb02345caa27
>> --- /dev/null
>> +++ b/include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h
>> @@ -0,0 +1,427 @@
>> +/* SPDX-License-Identifier: GPL-2.0 OR MIT */
>> +/*
>> + * Copyright (C) 2022 Emil Renner Berthing <[email protected]>
>> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
>> + */
>> +
>> +#ifndef __DT_BINDINGS_PINCTRL_STARFIVE_JH7110_H__
>> +#define __DT_BINDINGS_PINCTRL_STARFIVE_JH7110_H__
>> +
>> +/*
>> + * mux bits:
>> + * | 31 - 24 | 23 - 16 | 15 - 10 | 9 - 8 | 7 - 0 |
>> + * | din | dout | doen | function | gpio nr |
>> + *
>> + * dout: output signal
>> + * doen: output enable signal
>> + * din: optional input signal, 0xff = none
>> + * function:
>> + * gpio nr: gpio number, 0 - 63
>> + */
>> +#define GPIOMUX(n, dout, doen, din) ( \
>> + (((din) & 0xff) << 24) | \
>> + (((dout) & 0xff) << 16) | \
>> + (((doen) & 0x3f) << 10) | \
>> + ((n) & 0x3f))
>> +
>
>
> (...)
>
>> +/* sys_iomux doen */
>> +#define GPOEN_ENABLE 0
>> +#define GPOEN_DISABLE 1
>> +#define GPOEN_SYS_HDMI_CEC_SDA 2
>> +#define GPOEN_SYS_HDMI_DDC_SCL 3
>> +#define GPOEN_SYS_HDMI_DDC_SDA 4
>> +#define GPOEN_SYS_I2C0_CLK 5
>> +#define GPOEN_SYS_I2C0_DATA 6
>> +#define GPOEN_SYS_HIFI4_JTAG_TDO 7
>> +#define GPOEN_SYS_JTAG_TDO 8
>> +#define GPOEN_SYS_PWM0_CHANNEL0 9
>> +#define GPOEN_SYS_PWM0_CHANNEL1 10
>> +#define GPOEN_SYS_PWM0_CHANNEL2 11
>> +#define GPOEN_SYS_PWM0_CHANNEL3 12
>> +#define GPOEN_SYS_SPI0_NSSPCTL 13
>> +#define GPOEN_SYS_SPI0_NSSP 14
>> +#define GPOEN_SYS_TDM_SYNC 15
>> +#define GPOEN_SYS_TDM_TXD 16
>> +#define GPOEN_SYS_I2C1_CLK 17
>> +#define GPOEN_SYS_I2C1_DATA 18
>> +#define GPOEN_SYS_SDIO1_CMD 19
>> +#define GPOEN_SYS_SDIO1_DATA0 20
>> +#define GPOEN_SYS_SDIO1_DATA1 21
>> +#define GPOEN_SYS_SDIO1_DATA2 22
>> +#define GPOEN_SYS_SDIO1_DATA3 23
>> +#define GPOEN_SYS_SDIO1_DATA4 24
>> +#define GPOEN_SYS_SDIO1_DATA5 25
>> +#define GPOEN_SYS_SDIO1_DATA6 26
>> +#define GPOEN_SYS_SDIO1_DATA7 27
>> +#define GPOEN_SYS_SPI1_NSSPCTL 28
>> +#define GPOEN_SYS_SPI1_NSSP 29
>> +#define GPOEN_SYS_I2C2_CLK 30
>> +#define GPOEN_SYS_I2C2_DATA 31
>> +#define GPOEN_SYS_SPI2_NSSPCTL 32
>> +#define GPOEN_SYS_SPI2_NSSP 33
>> +#define GPOEN_SYS_I2C3_CLK 34
>> +#define GPOEN_SYS_I2C3_DATA 35
>> +#define GPOEN_SYS_SPI3_NSSPCTL 36
>> +#define GPOEN_SYS_SPI3_NSSP 37
>> +#define GPOEN_SYS_I2C4_CLK 38
>> +#define GPOEN_SYS_I2C4_DATA 39
>> +#define GPOEN_SYS_SPI4_NSSPCTL 40
>> +#define GPOEN_SYS_SPI4_NSSP 41
>> +#define GPOEN_SYS_I2C5_CLK 42
>> +#define GPOEN_SYS_I2C5_DATA 43
>> +#define GPOEN_SYS_SPI5_NSSPCTL 44
>> +#define GPOEN_SYS_SPI5_NSSP 45
>> +#define GPOEN_SYS_I2C6_CLK 46
>> +#define GPOEN_SYS_I2C6_DATA 47
>> +#define GPOEN_SYS_SPI6_NSSPCTL 48
>> +#define GPOEN_SYS_SPI6_NSSP 49
>> +
>> +/* aon_iomux doen */
>> +#define GPOEN_AON_PTC0_OE_N_4 2
>> +#define GPOEN_AON_PTC0_OE_N_5 3
>> +#define GPOEN_AON_PTC0_OE_N_6 4
>> +#define GPOEN_AON_PTC0_OE_N_7 5
>> +
>
> It looks like you add register constants to the bindings. Why? The
> bindings are not the place to represent hardware programming model. Not
> mentioning that there is no benefit in this.

Also: this entire file should be dropped, but if it stays, you have to
name it matching bindings or compatible (vendor,device.h).

Best regards,
Krzysztof


2022-11-21 08:54:20

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] dt-bindings: pinctrl: Add StarFive JH7110 pinctrl definitions

On 18/11/2022 02:11, Hal Feng wrote:
> From: Jianlong Huang <[email protected]>
>
> Add pinctrl definitions for StarFive JH7110 SoC.
>
> Co-developed-by: Emil Renner Berthing <[email protected]>
> Signed-off-by: Emil Renner Berthing <[email protected]>
> Signed-off-by: Jianlong Huang <[email protected]>
> Signed-off-by: Hal Feng <[email protected]>
> ---
> .../pinctrl/pinctrl-starfive-jh7110.h | 427 ++++++++++++++++++
> 1 file changed, 427 insertions(+)
> create mode 100644 include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h
>
> diff --git a/include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h b/include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h
> new file mode 100644
> index 000000000000..fb02345caa27
> --- /dev/null
> +++ b/include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h
> @@ -0,0 +1,427 @@
> +/* SPDX-License-Identifier: GPL-2.0 OR MIT */
> +/*
> + * Copyright (C) 2022 Emil Renner Berthing <[email protected]>
> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
> + */
> +
> +#ifndef __DT_BINDINGS_PINCTRL_STARFIVE_JH7110_H__
> +#define __DT_BINDINGS_PINCTRL_STARFIVE_JH7110_H__
> +
> +/*
> + * mux bits:
> + * | 31 - 24 | 23 - 16 | 15 - 10 | 9 - 8 | 7 - 0 |
> + * | din | dout | doen | function | gpio nr |
> + *
> + * dout: output signal
> + * doen: output enable signal
> + * din: optional input signal, 0xff = none
> + * function:
> + * gpio nr: gpio number, 0 - 63
> + */
> +#define GPIOMUX(n, dout, doen, din) ( \
> + (((din) & 0xff) << 24) | \
> + (((dout) & 0xff) << 16) | \
> + (((doen) & 0x3f) << 10) | \
> + ((n) & 0x3f))
> +


(...)

> +/* sys_iomux doen */
> +#define GPOEN_ENABLE 0
> +#define GPOEN_DISABLE 1
> +#define GPOEN_SYS_HDMI_CEC_SDA 2
> +#define GPOEN_SYS_HDMI_DDC_SCL 3
> +#define GPOEN_SYS_HDMI_DDC_SDA 4
> +#define GPOEN_SYS_I2C0_CLK 5
> +#define GPOEN_SYS_I2C0_DATA 6
> +#define GPOEN_SYS_HIFI4_JTAG_TDO 7
> +#define GPOEN_SYS_JTAG_TDO 8
> +#define GPOEN_SYS_PWM0_CHANNEL0 9
> +#define GPOEN_SYS_PWM0_CHANNEL1 10
> +#define GPOEN_SYS_PWM0_CHANNEL2 11
> +#define GPOEN_SYS_PWM0_CHANNEL3 12
> +#define GPOEN_SYS_SPI0_NSSPCTL 13
> +#define GPOEN_SYS_SPI0_NSSP 14
> +#define GPOEN_SYS_TDM_SYNC 15
> +#define GPOEN_SYS_TDM_TXD 16
> +#define GPOEN_SYS_I2C1_CLK 17
> +#define GPOEN_SYS_I2C1_DATA 18
> +#define GPOEN_SYS_SDIO1_CMD 19
> +#define GPOEN_SYS_SDIO1_DATA0 20
> +#define GPOEN_SYS_SDIO1_DATA1 21
> +#define GPOEN_SYS_SDIO1_DATA2 22
> +#define GPOEN_SYS_SDIO1_DATA3 23
> +#define GPOEN_SYS_SDIO1_DATA4 24
> +#define GPOEN_SYS_SDIO1_DATA5 25
> +#define GPOEN_SYS_SDIO1_DATA6 26
> +#define GPOEN_SYS_SDIO1_DATA7 27
> +#define GPOEN_SYS_SPI1_NSSPCTL 28
> +#define GPOEN_SYS_SPI1_NSSP 29
> +#define GPOEN_SYS_I2C2_CLK 30
> +#define GPOEN_SYS_I2C2_DATA 31
> +#define GPOEN_SYS_SPI2_NSSPCTL 32
> +#define GPOEN_SYS_SPI2_NSSP 33
> +#define GPOEN_SYS_I2C3_CLK 34
> +#define GPOEN_SYS_I2C3_DATA 35
> +#define GPOEN_SYS_SPI3_NSSPCTL 36
> +#define GPOEN_SYS_SPI3_NSSP 37
> +#define GPOEN_SYS_I2C4_CLK 38
> +#define GPOEN_SYS_I2C4_DATA 39
> +#define GPOEN_SYS_SPI4_NSSPCTL 40
> +#define GPOEN_SYS_SPI4_NSSP 41
> +#define GPOEN_SYS_I2C5_CLK 42
> +#define GPOEN_SYS_I2C5_DATA 43
> +#define GPOEN_SYS_SPI5_NSSPCTL 44
> +#define GPOEN_SYS_SPI5_NSSP 45
> +#define GPOEN_SYS_I2C6_CLK 46
> +#define GPOEN_SYS_I2C6_DATA 47
> +#define GPOEN_SYS_SPI6_NSSPCTL 48
> +#define GPOEN_SYS_SPI6_NSSP 49
> +
> +/* aon_iomux doen */
> +#define GPOEN_AON_PTC0_OE_N_4 2
> +#define GPOEN_AON_PTC0_OE_N_5 3
> +#define GPOEN_AON_PTC0_OE_N_6 4
> +#define GPOEN_AON_PTC0_OE_N_7 5
> +

It looks like you add register constants to the bindings. Why? The
bindings are not the place to represent hardware programming model. Not
mentioning that there is no benefit in this.

Best regards,
Krzysztof


2022-11-21 08:54:20

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 3/5] dt-bindings: pinctrl: Add StarFive JH7110 aon pinctrl

On 18/11/2022 02:11, Hal Feng wrote:
> From: Jianlong Huang <[email protected]>
>
> Add pinctrl bindings for StarFive JH7110 SoC aon pinctrl controller.
>
> Signed-off-by: Jianlong Huang <[email protected]>
> Signed-off-by: Hal Feng <[email protected]>
> ---
> .../pinctrl/starfive,jh7110-aon-pinctrl.yaml | 134 ++++++++++++++++++
> 1 file changed, 134 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml
> new file mode 100644
> index 000000000000..1dd000e1f614
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml
> @@ -0,0 +1,134 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/starfive,jh7110-aon-pinctrl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: StarFive JH7110 Aon Pin Controller
> +
> +description: |
> + Bindings for the JH7110 RISC-V SoC from StarFive Technology Ltd.
> +
> + Out of the SoC's many pins only the ones named PAD_GPIO0 to PAD_GPIO4
> + can be multiplexed and have configurable bias, drive strength,
> + schmitt trigger etc.
> + Some peripherals have their I/O go through the 4 "GPIOs". This also
> + includes PWM.
> +
> +maintainers:
> + - Jianlong Huang <[email protected]>
> +
> +properties:
> + compatible:
> + const: starfive,jh7110-aon-pinctrl
> +
> + reg:
> + maxItems: 1
> +
> + reg-names:
> + items:
> + - const: control
> +
> + clocks:
> + maxItems: 1
> +
> + resets:
> + maxItems: 1
> +
> + gpio-controller: true
> +
> + "#gpio-cells":
> + const: 2
> +
> + interrupts:
> + maxItems: 1
> + description: The GPIO parent interrupt.

Same comments apply plus one more.

> +
> + interrupt-controller: true
> +
> + "#interrupt-cells":
> + const: 2
> +
> +required:
> + - compatible
> + - reg
> + - reg-names
> + - gpio-controller
> + - "#gpio-cells"
> + - interrupts
> + - interrupt-controller
> + - "#interrupt-cells"

"required:" goes after patternProperties.

> +
> +patternProperties:
> + '-[0-9]+$':

Same comment.

> + type: object
> + patternProperties:
> + '-pins$':
> + type: object
> + description: |
> + A pinctrl node should contain at least one subnode representing the
> + pinctrl groups available on the machine. Each subnode will list the
> + pins it needs, and how they should be configured, with regard to
> + muxer configuration, system signal configuration, pin groups for
> + vin/vout module, pin voltage, mux functions for output, mux functions
> + for output enable, mux functions for input.
> +
> + properties:
> + pinmux:
> + description: |
> + The list of GPIOs and their mux settings that properties in the
> + node apply to. This should be set using the GPIOMUX macro.
> + $ref: "/schemas/pinctrl/pinmux-node.yaml#/properties/pinmux"
> +
> + bias-disable: true
> +
> + bias-pull-up:
> + type: boolean
> +
> + bias-pull-down:
> + type: boolean
> +
> + drive-strength:
> + enum: [ 2, 4, 8, 12 ]
> +
> + input-enable: true
> +
> + input-disable: true
> +
> + input-schmitt-enable: true
> +
> + input-schmitt-disable: true
> +
> + slew-rate:
> + maximum: 1
> +
> + additionalProperties: false
> +
> + additionalProperties: false
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/starfive-jh7110.h>
> + #include <dt-bindings/reset/starfive-jh7110.h>
> + #include <dt-bindings/pinctrl/pinctrl-starfive-jh7110.h>
> +
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;

Same comment.

> +
> + gpioa: gpio@17020000 {
> + compatible = "starfive,jh7110-aon-pinctrl";
> + reg = <0x0 0x17020000 0x0 0x10000>;
> + reg-names = "control";
> + resets = <&aoncrg_rst JH7110_AONRST_AON_IOMUX>;
> + interrupts = <85>;
> + interrupt-controller;
> + #interrupt-cells = <2>;
> + #gpio-cells = <2>;
> + gpio-controller;
> + };
> + };
> +
> +...

Best regards,
Krzysztof


2022-11-28 01:10:11

by Jianlong Huang

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] dt-bindings: pinctrl: Add StarFive JH7110 pinctrl definitions

On Mon, 21 Nov 2022 09:39:46 +0100, Krzysztof Kozlowski wrote:
> On 21/11/2022 09:38, Krzysztof Kozlowski wrote:
>> On 18/11/2022 02:11, Hal Feng wrote:
>>> From: Jianlong Huang <[email protected]>
>>>
>>> Add pinctrl definitions for StarFive JH7110 SoC.
>>>
>>> Co-developed-by: Emil Renner Berthing <[email protected]>
>>> Signed-off-by: Emil Renner Berthing <[email protected]>
>>> Signed-off-by: Jianlong Huang <[email protected]>
>>> Signed-off-by: Hal Feng <[email protected]>
>>> ---
>>> .../pinctrl/pinctrl-starfive-jh7110.h | 427 ++++++++++++++++++
>>> 1 file changed, 427 insertions(+)
>>> create mode 100644 include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h
>>>
>>> diff --git a/include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h b/include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h
>>> new file mode 100644
>>> index 000000000000..fb02345caa27
>>> --- /dev/null
>>> +++ b/include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h
>>> @@ -0,0 +1,427 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 OR MIT */
>>> +/*
>>> + * Copyright (C) 2022 Emil Renner Berthing <[email protected]>
>>> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
>>> + */
>>> +
>>> +#ifndef __DT_BINDINGS_PINCTRL_STARFIVE_JH7110_H__
>>> +#define __DT_BINDINGS_PINCTRL_STARFIVE_JH7110_H__
>>> +
>>> +/*
>>> + * mux bits:
>>> + * | 31 - 24 | 23 - 16 | 15 - 10 | 9 - 8 | 7 - 0 |
>>> + * | din | dout | doen | function | gpio nr |
>>> + *
>>> + * dout: output signal
>>> + * doen: output enable signal
>>> + * din: optional input signal, 0xff = none
>>> + * function:
>>> + * gpio nr: gpio number, 0 - 63
>>> + */
>>> +#define GPIOMUX(n, dout, doen, din) ( \
>>> + (((din) & 0xff) << 24) | \
>>> + (((dout) & 0xff) << 16) | \
>>> + (((doen) & 0x3f) << 10) | \
>>> + ((n) & 0x3f))
>>> +
>>
>>
>> (...)
>>
>>> +/* sys_iomux doen */
>>> +#define GPOEN_ENABLE 0
>>> +#define GPOEN_DISABLE 1
>>> +#define GPOEN_SYS_HDMI_CEC_SDA 2
>>> +#define GPOEN_SYS_HDMI_DDC_SCL 3
>>> +#define GPOEN_SYS_HDMI_DDC_SDA 4
>>> +#define GPOEN_SYS_I2C0_CLK 5
>>> +#define GPOEN_SYS_I2C0_DATA 6
>>> +#define GPOEN_SYS_HIFI4_JTAG_TDO 7
>>> +#define GPOEN_SYS_JTAG_TDO 8
>>> +#define GPOEN_SYS_PWM0_CHANNEL0 9
>>> +#define GPOEN_SYS_PWM0_CHANNEL1 10
>>> +#define GPOEN_SYS_PWM0_CHANNEL2 11
>>> +#define GPOEN_SYS_PWM0_CHANNEL3 12
>>> +#define GPOEN_SYS_SPI0_NSSPCTL 13
>>> +#define GPOEN_SYS_SPI0_NSSP 14
>>> +#define GPOEN_SYS_TDM_SYNC 15
>>> +#define GPOEN_SYS_TDM_TXD 16
>>> +#define GPOEN_SYS_I2C1_CLK 17
>>> +#define GPOEN_SYS_I2C1_DATA 18
>>> +#define GPOEN_SYS_SDIO1_CMD 19
>>> +#define GPOEN_SYS_SDIO1_DATA0 20
>>> +#define GPOEN_SYS_SDIO1_DATA1 21
>>> +#define GPOEN_SYS_SDIO1_DATA2 22
>>> +#define GPOEN_SYS_SDIO1_DATA3 23
>>> +#define GPOEN_SYS_SDIO1_DATA4 24
>>> +#define GPOEN_SYS_SDIO1_DATA5 25
>>> +#define GPOEN_SYS_SDIO1_DATA6 26
>>> +#define GPOEN_SYS_SDIO1_DATA7 27
>>> +#define GPOEN_SYS_SPI1_NSSPCTL 28
>>> +#define GPOEN_SYS_SPI1_NSSP 29
>>> +#define GPOEN_SYS_I2C2_CLK 30
>>> +#define GPOEN_SYS_I2C2_DATA 31
>>> +#define GPOEN_SYS_SPI2_NSSPCTL 32
>>> +#define GPOEN_SYS_SPI2_NSSP 33
>>> +#define GPOEN_SYS_I2C3_CLK 34
>>> +#define GPOEN_SYS_I2C3_DATA 35
>>> +#define GPOEN_SYS_SPI3_NSSPCTL 36
>>> +#define GPOEN_SYS_SPI3_NSSP 37
>>> +#define GPOEN_SYS_I2C4_CLK 38
>>> +#define GPOEN_SYS_I2C4_DATA 39
>>> +#define GPOEN_SYS_SPI4_NSSPCTL 40
>>> +#define GPOEN_SYS_SPI4_NSSP 41
>>> +#define GPOEN_SYS_I2C5_CLK 42
>>> +#define GPOEN_SYS_I2C5_DATA 43
>>> +#define GPOEN_SYS_SPI5_NSSPCTL 44
>>> +#define GPOEN_SYS_SPI5_NSSP 45
>>> +#define GPOEN_SYS_I2C6_CLK 46
>>> +#define GPOEN_SYS_I2C6_DATA 47
>>> +#define GPOEN_SYS_SPI6_NSSPCTL 48
>>> +#define GPOEN_SYS_SPI6_NSSP 49
>>> +
>>> +/* aon_iomux doen */
>>> +#define GPOEN_AON_PTC0_OE_N_4 2
>>> +#define GPOEN_AON_PTC0_OE_N_5 3
>>> +#define GPOEN_AON_PTC0_OE_N_6 4
>>> +#define GPOEN_AON_PTC0_OE_N_7 5
>>> +
>>
>> It looks like you add register constants to the bindings. Why? The
>> bindings are not the place to represent hardware programming model. Not
>> mentioning that there is no benefit in this.
>
> Also: this entire file should be dropped, but if it stays, you have to
> name it matching bindings or compatible (vendor,device.h).

Thanks your comments.
These macros are used to configure pinctrl in dts, so the file should stay,
and will change the name as "starfive,jh7110-pinctrl.h" to match bindings or compatible.

Best regards,
Jianlong Huang


2022-11-28 01:33:32

by Jianlong Huang

[permalink] [raw]
Subject: Re: [PATCH v2 3/5] dt-bindings: pinctrl: Add StarFive JH7110 aon pinctrl

On Mon, 21 Nov 2022 09:44:00 +0100, Krzysztof Kozlowski wrote:
> On 18/11/2022 02:11, Hal Feng wrote:
>> From: Jianlong Huang <[email protected]>
>>
>> Add pinctrl bindings for StarFive JH7110 SoC aon pinctrl controller.
>>
>> Signed-off-by: Jianlong Huang <[email protected]>
>> Signed-off-by: Hal Feng <[email protected]>
>> ---
>> .../pinctrl/starfive,jh7110-aon-pinctrl.yaml | 134 ++++++++++++++++++
>> 1 file changed, 134 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml
>> new file mode 100644
>> index 000000000000..1dd000e1f614
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml
>> @@ -0,0 +1,134 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/pinctrl/starfive,jh7110-aon-pinctrl.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: StarFive JH7110 Aon Pin Controller
>> +
>> +description: |
>> + Bindings for the JH7110 RISC-V SoC from StarFive Technology Ltd.
>> +
>> + Out of the SoC's many pins only the ones named PAD_GPIO0 to PAD_GPIO4
>> + can be multiplexed and have configurable bias, drive strength,
>> + schmitt trigger etc.
>> + Some peripherals have their I/O go through the 4 "GPIOs". This also
>> + includes PWM.
>> +
>> +maintainers:
>> + - Jianlong Huang <[email protected]>
>> +
>> +properties:
>> + compatible:
>> + const: starfive,jh7110-aon-pinctrl
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + reg-names:
>> + items:
>> + - const: control
>> +
>> + clocks:
>> + maxItems: 1
>> +
>> + resets:
>> + maxItems: 1
>> +
>> + gpio-controller: true
>> +
>> + "#gpio-cells":
>> + const: 2
>> +
>> + interrupts:
>> + maxItems: 1
>> + description: The GPIO parent interrupt.
>
> Same comments apply plus one more.

Will fix, drop this description.

>
>> +
>> + interrupt-controller: true
>> +
>> + "#interrupt-cells":
>> + const: 2
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - reg-names
>> + - gpio-controller
>> + - "#gpio-cells"
>> + - interrupts
>> + - interrupt-controller
>> + - "#interrupt-cells"
>
> "required:" goes after patternProperties.

Will fix.

>
>> +
>> +patternProperties:
>> + '-[0-9]+$':
>
> Same comment.

Will fix.
Keep consistent quotes, use '

>
>> + type: object
>> + patternProperties:
>> + '-pins$':
>> + type: object
>> + description: |
>> + A pinctrl node should contain at least one subnode representing the
>> + pinctrl groups available on the machine. Each subnode will list the
>> + pins it needs, and how they should be configured, with regard to
>> + muxer configuration, system signal configuration, pin groups for
>> + vin/vout module, pin voltage, mux functions for output, mux functions
>> + for output enable, mux functions for input.
>> +
>> + properties:
>> + pinmux:
>> + description: |
>> + The list of GPIOs and their mux settings that properties in the
>> + node apply to. This should be set using the GPIOMUX macro.
>> + $ref: "/schemas/pinctrl/pinmux-node.yaml#/properties/pinmux"
>> +
>> + bias-disable: true
>> +
>> + bias-pull-up:
>> + type: boolean
>> +
>> + bias-pull-down:
>> + type: boolean
>> +
>> + drive-strength:
>> + enum: [ 2, 4, 8, 12 ]
>> +
>> + input-enable: true
>> +
>> + input-disable: true
>> +
>> + input-schmitt-enable: true
>> +
>> + input-schmitt-disable: true
>> +
>> + slew-rate:
>> + maximum: 1
>> +
>> + additionalProperties: false
>> +
>> + additionalProperties: false
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/clock/starfive-jh7110.h>
>> + #include <dt-bindings/reset/starfive-jh7110.h>
>> + #include <dt-bindings/pinctrl/pinctrl-starfive-jh7110.h>
>> +
>> + soc {
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>
> Same comment.

Will fix.
Use 4 spaces for example indentation.

>
>> +
>> + gpioa: gpio@17020000 {
>> + compatible = "starfive,jh7110-aon-pinctrl";
>> + reg = <0x0 0x17020000 0x0 0x10000>;
>> + reg-names = "control";
>> + resets = <&aoncrg_rst JH7110_AONRST_AON_IOMUX>;
>> + interrupts = <85>;
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + #gpio-cells = <2>;
>> + gpio-controller;
>> + };
>> + };
>> +
>> +...
>

Best regards,
Jianlong Huang


2022-11-28 09:05:52

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] dt-bindings: pinctrl: Add StarFive JH7110 pinctrl definitions

On 28/11/2022 01:48, Jianlong Huang wrote:

>>>> +/* aon_iomux doen */
>>>> +#define GPOEN_AON_PTC0_OE_N_4 2
>>>> +#define GPOEN_AON_PTC0_OE_N_5 3
>>>> +#define GPOEN_AON_PTC0_OE_N_6 4
>>>> +#define GPOEN_AON_PTC0_OE_N_7 5
>>>> +
>>>
>>> It looks like you add register constants to the bindings. Why? The
>>> bindings are not the place to represent hardware programming model. Not
>>> mentioning that there is no benefit in this.
>>
>> Also: this entire file should be dropped, but if it stays, you have to
>> name it matching bindings or compatible (vendor,device.h).
>
> Thanks your comments.
> These macros are used to configure pinctrl in dts, so the file should stay,

Why they should stay? What's the reason? If it is not a constant used by
driver, then register values should not be placed in the bindings, so
drop it.

Best regards,
Krzysztof

2022-11-29 02:15:54

by Jianlong Huang

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] dt-bindings: pinctrl: Add StarFive JH7110 pinctrl definitions

On Mon, 28 Nov 2022 09:32:45 +0100, Krzysztof Kozlowski wrote:
> On 28/11/2022 01:48, Jianlong Huang wrote:
>
>>>>> +/* aon_iomux doen */
>>>>> +#define GPOEN_AON_PTC0_OE_N_4 2
>>>>> +#define GPOEN_AON_PTC0_OE_N_5 3
>>>>> +#define GPOEN_AON_PTC0_OE_N_6 4
>>>>> +#define GPOEN_AON_PTC0_OE_N_7 5
>>>>> +
>>>>
>>>> It looks like you add register constants to the bindings. Why? The
>>>> bindings are not the place to represent hardware programming model. Not
>>>> mentioning that there is no benefit in this.
>>>
>>> Also: this entire file should be dropped, but if it stays, you have to
>>> name it matching bindings or compatible (vendor,device.h).
>>
>> Thanks your comments.
>> These macros are used to configure pinctrl in dts, so the file should stay,
>
> Why they should stay? What's the reason? If it is not a constant used by
> driver, then register values should not be placed in the bindings, so
> drop it.
>

Thanks.

These macros in binding header(example, DOUT, DOEN etc) will be used in DTS,
and driver will parse the DT for pinctrl configuration.

Example in dts:
uart0_pins: uart0-0 {
tx-pins {
pinmux = <GPIOMUX(5, GPOUT_SYS_UART0_TX, GPOEN_ENABLE, GPI_NONE)>;
bias-disable;
drive-strength = <12>;
input-disable;
input-schmitt-disable;
slew-rate = <0>;
};

rx-pins {
pinmux = <GPIOMUX(6, GPOUT_LOW, GPOEN_DISABLE, GPI_SYS_UART0_RX)>;
bias-pull-up;
drive-strength = <2>;
input-enable;
input-schmitt-enable;
slew-rate = <0>;
};
};


Best regards,
Jianlong Huang


2022-11-29 08:50:40

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] dt-bindings: pinctrl: Add StarFive JH7110 pinctrl definitions

On 29/11/2022 02:47, Jianlong Huang wrote:
> On Mon, 28 Nov 2022 09:32:45 +0100, Krzysztof Kozlowski wrote:
>> On 28/11/2022 01:48, Jianlong Huang wrote:
>>
>>>>>> +/* aon_iomux doen */
>>>>>> +#define GPOEN_AON_PTC0_OE_N_4 2
>>>>>> +#define GPOEN_AON_PTC0_OE_N_5 3
>>>>>> +#define GPOEN_AON_PTC0_OE_N_6 4
>>>>>> +#define GPOEN_AON_PTC0_OE_N_7 5
>>>>>> +
>>>>>
>>>>> It looks like you add register constants to the bindings. Why? The
>>>>> bindings are not the place to represent hardware programming model. Not
>>>>> mentioning that there is no benefit in this.
>>>>
>>>> Also: this entire file should be dropped, but if it stays, you have to
>>>> name it matching bindings or compatible (vendor,device.h).
>>>
>>> Thanks your comments.
>>> These macros are used to configure pinctrl in dts, so the file should stay,
>>
>> Why they should stay? What's the reason? If it is not a constant used by
>> driver, then register values should not be placed in the bindings, so
>> drop it.
>>
>
> Thanks.
>
> These macros in binding header(example, DOUT, DOEN etc) will be used in DTS,
> and driver will parse the DT for pinctrl configuration.
>
> Example in dts:
> uart0_pins: uart0-0 {
> tx-pins {
> pinmux = <GPIOMUX(5, GPOUT_SYS_UART0_TX, GPOEN_ENABLE, GPI_NONE)>;

This is usage in DTS and is not an argument to store register
addresses/offsets as bindings. What is the usage (of define, not value)
in the driver?


Best regards,
Krzysztof

2022-11-29 15:07:59

by Jianlong Huang

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] dt-bindings: pinctrl: Add StarFive JH7110 pinctrl definitions

On Tue, 29 Nov 2022 08:49:49 +0100, Krzysztof Kozlowski wrote:
> On 29/11/2022 02:47, Jianlong Huang wrote:
>> On Mon, 28 Nov 2022 09:32:45 +0100, Krzysztof Kozlowski wrote:
>>> On 28/11/2022 01:48, Jianlong Huang wrote:
>>>
>>>>>>> +/* aon_iomux doen */
>>>>>>> +#define GPOEN_AON_PTC0_OE_N_4 2
>>>>>>> +#define GPOEN_AON_PTC0_OE_N_5 3
>>>>>>> +#define GPOEN_AON_PTC0_OE_N_6 4
>>>>>>> +#define GPOEN_AON_PTC0_OE_N_7 5
>>>>>>> +
>>>>>>
>>>>>> It looks like you add register constants to the bindings. Why? The
>>>>>> bindings are not the place to represent hardware programming model. Not
>>>>>> mentioning that there is no benefit in this.
>>>>>
>>>>> Also: this entire file should be dropped, but if it stays, you have to
>>>>> name it matching bindings or compatible (vendor,device.h).
>>>>
>>>> Thanks your comments.
>>>> These macros are used to configure pinctrl in dts, so the file should stay,
>>>
>>> Why they should stay? What's the reason? If it is not a constant used by
>>> driver, then register values should not be placed in the bindings, so
>>> drop it.
>>>
>>
>> Thanks.
>>
>> These macros in binding header(example, DOUT, DOEN etc) will be used in DTS,
>> and driver will parse the DT for pinctrl configuration.
>>
>> Example in dts:
>> uart0_pins: uart0-0 {
>> tx-pins {
>> pinmux = <GPIOMUX(5, GPOUT_SYS_UART0_TX, GPOEN_ENABLE, GPI_NONE)>;
>
> This is usage in DTS and is not an argument to store register
> addresses/offsets as bindings. What is the usage (of define, not value)
> in the driver?
>

The existing implementation reuse the macros for DTS and driver.
Do you mean we need to separate the macros, one for DTS and one for driver usage?
Or you have any better suggestion?

These macros are the value of register, not register addresses/offsets,
except for with prefix of GPI.

Drivers rarely reference macros directly, mostly parsing dts and writing them to registers.

Best regards,
Jianlong Huang

2022-11-29 15:17:57

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] dt-bindings: pinctrl: Add StarFive JH7110 pinctrl definitions

On 29/11/2022 15:46, Jianlong Huang wrote:
> On Tue, 29 Nov 2022 08:49:49 +0100, Krzysztof Kozlowski wrote:
>> On 29/11/2022 02:47, Jianlong Huang wrote:
>>> On Mon, 28 Nov 2022 09:32:45 +0100, Krzysztof Kozlowski wrote:
>>>> On 28/11/2022 01:48, Jianlong Huang wrote:
>>>>
>>>>>>>> +/* aon_iomux doen */
>>>>>>>> +#define GPOEN_AON_PTC0_OE_N_4 2
>>>>>>>> +#define GPOEN_AON_PTC0_OE_N_5 3
>>>>>>>> +#define GPOEN_AON_PTC0_OE_N_6 4
>>>>>>>> +#define GPOEN_AON_PTC0_OE_N_7 5
>>>>>>>> +
>>>>>>>
>>>>>>> It looks like you add register constants to the bindings. Why? The
>>>>>>> bindings are not the place to represent hardware programming model. Not
>>>>>>> mentioning that there is no benefit in this.
>>>>>>
>>>>>> Also: this entire file should be dropped, but if it stays, you have to
>>>>>> name it matching bindings or compatible (vendor,device.h).
>>>>>
>>>>> Thanks your comments.
>>>>> These macros are used to configure pinctrl in dts, so the file should stay,
>>>>
>>>> Why they should stay? What's the reason? If it is not a constant used by
>>>> driver, then register values should not be placed in the bindings, so
>>>> drop it.
>>>>
>>>
>>> Thanks.
>>>
>>> These macros in binding header(example, DOUT, DOEN etc) will be used in DTS,
>>> and driver will parse the DT for pinctrl configuration.
>>>
>>> Example in dts:
>>> uart0_pins: uart0-0 {
>>> tx-pins {
>>> pinmux = <GPIOMUX(5, GPOUT_SYS_UART0_TX, GPOEN_ENABLE, GPI_NONE)>;
>>
>> This is usage in DTS and is not an argument to store register
>> addresses/offsets as bindings. What is the usage (of define, not value)
>> in the driver?
>>
>
> The existing implementation reuse the macros for DTS and driver.

Where in the driver? Grep gives zero results.

> Do you mean we need to separate the macros, one for DTS and one for driver usage?

No, if driver uses them it is fine. The problem is I cannot find it
anywhere.

> Or you have any better suggestion?
>
> These macros are the value of register, not register addresses/offsets,
> except for with prefix of GPI.

Still, values are not usually part of bindings.

>
> Drivers rarely reference macros directly, mostly parsing dts and writing them to registers.

So drivers do not use macros? Then there is no reason to store them in
bindings? What do you "bind" if there is no usage (and we do not talk
about DTS...)?

Best regards,
Krzysztof

2022-11-29 16:10:17

by Jianlong Huang

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] dt-bindings: pinctrl: Add StarFive JH7110 pinctrl definitions

On Tue, 29 Nov 2022 15:58:12 +0100, Krzysztof Kozlowski wrote:
> On 29/11/2022 15:46, Jianlong Huang wrote:
>> On Tue, 29 Nov 2022 08:49:49 +0100, Krzysztof Kozlowski wrote:
>>> On 29/11/2022 02:47, Jianlong Huang wrote:
>>>> On Mon, 28 Nov 2022 09:32:45 +0100, Krzysztof Kozlowski wrote:
>>>>> On 28/11/2022 01:48, Jianlong Huang wrote:
>>>>>
>>>>>>>>> +/* aon_iomux doen */
>>>>>>>>> +#define GPOEN_AON_PTC0_OE_N_4 2
>>>>>>>>> +#define GPOEN_AON_PTC0_OE_N_5 3
>>>>>>>>> +#define GPOEN_AON_PTC0_OE_N_6 4
>>>>>>>>> +#define GPOEN_AON_PTC0_OE_N_7 5
>>>>>>>>> +
>>>>>>>>
>>>>>>>> It looks like you add register constants to the bindings. Why? The
>>>>>>>> bindings are not the place to represent hardware programming model. Not
>>>>>>>> mentioning that there is no benefit in this.
>>>>>>>
>>>>>>> Also: this entire file should be dropped, but if it stays, you have to
>>>>>>> name it matching bindings or compatible (vendor,device.h).
>>>>>>
>>>>>> Thanks your comments.
>>>>>> These macros are used to configure pinctrl in dts, so the file should stay,
>>>>>
>>>>> Why they should stay? What's the reason? If it is not a constant used by
>>>>> driver, then register values should not be placed in the bindings, so
>>>>> drop it.
>>>>>
>>>>
>>>> Thanks.
>>>>
>>>> These macros in binding header(example, DOUT, DOEN etc) will be used in DTS,
>>>> and driver will parse the DT for pinctrl configuration.
>>>>
>>>> Example in dts:
>>>> uart0_pins: uart0-0 {
>>>> tx-pins {
>>>> pinmux = <GPIOMUX(5, GPOUT_SYS_UART0_TX, GPOEN_ENABLE, GPI_NONE)>;
>>>
>>> This is usage in DTS and is not an argument to store register
>>> addresses/offsets as bindings. What is the usage (of define, not value)
>>> in the driver?
>>>
>>
>> The existing implementation reuse the macros for DTS and driver.
>
> Where in the driver? Grep gives zero results.
>
>> Do you mean we need to separate the macros, one for DTS and one for driver usage?
>
> No, if driver uses them it is fine. The problem is I cannot find it
> anywhere.
>
>> Or you have any better suggestion?
>>
>> These macros are the value of register, not register addresses/offsets,
>> except for with prefix of GPI.
>
> Still, values are not usually part of bindings.
>
>>
>> Drivers rarely reference macros directly, mostly parsing dts and writing them to registers.
>
> So drivers do not use macros? Then there is no reason to store them in
> bindings? What do you "bind" if there is no usage (and we do not talk
> about DTS...)?
>

Where do you suggest to store these macros used in DTS?

Best regards,
Jianlong Huang

2022-11-29 16:25:03

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] dt-bindings: pinctrl: Add StarFive JH7110 pinctrl definitions

On 29/11/2022 16:58, Jianlong Huang wrote:
>>>
>>> Drivers rarely reference macros directly, mostly parsing dts and writing them to registers.
>>
>> So drivers do not use macros? Then there is no reason to store them in
>> bindings? What do you "bind" if there is no usage (and we do not talk
>> about DTS...)?
>>
>
> Where do you suggest to store these macros used in DTS?

Sometimes they do not need storing. If they are worth, then in the DTS
headers. Few platforms (Samsung, All of NXP, Mediatek, Sama5/Atmel) are
doing it already.

Best regards,
Krzysztof

2022-12-01 09:45:07

by Jianlong Huang

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] dt-bindings: pinctrl: Add StarFive JH7110 pinctrl definitions

On Tue, 29 Nov 2022 15:58:12 +0100, Krzysztof Kozlowski wrote:
> On 29/11/2022 15:46, Jianlong Huang wrote:
>> On Tue, 29 Nov 2022 08:49:49 +0100, Krzysztof Kozlowski wrote:
>>> On 29/11/2022 02:47, Jianlong Huang wrote:
>>>> On Mon, 28 Nov 2022 09:32:45 +0100, Krzysztof Kozlowski wrote:
>>>>> On 28/11/2022 01:48, Jianlong Huang wrote:
>>>>>
>>>>>>>>> +/* aon_iomux doen */
>>>>>>>>> +#define GPOEN_AON_PTC0_OE_N_4 2
>>>>>>>>> +#define GPOEN_AON_PTC0_OE_N_5 3
>>>>>>>>> +#define GPOEN_AON_PTC0_OE_N_6 4
>>>>>>>>> +#define GPOEN_AON_PTC0_OE_N_7 5
>>>>>>>>> +
>>>>>>>>
>>>>>>>> It looks like you add register constants to the bindings. Why? The
>>>>>>>> bindings are not the place to represent hardware programming model. Not
>>>>>>>> mentioning that there is no benefit in this.
>>>>>>>
>>>>>>> Also: this entire file should be dropped, but if it stays, you have to
>>>>>>> name it matching bindings or compatible (vendor,device.h).
>>>>>>
>>>>>> Thanks your comments.
>>>>>> These macros are used to configure pinctrl in dts, so the file should stay,
>>>>>
>>>>> Why they should stay? What's the reason? If it is not a constant used by
>>>>> driver, then register values should not be placed in the bindings, so
>>>>> drop it.
>>>>>
>>>>
>>>> Thanks.
>>>>
>>>> These macros in binding header(example, DOUT, DOEN etc) will be used in DTS,
>>>> and driver will parse the DT for pinctrl configuration.
>>>>
>>>> Example in dts:
>>>> uart0_pins: uart0-0 {
>>>> tx-pins {
>>>> pinmux = <GPIOMUX(5, GPOUT_SYS_UART0_TX, GPOEN_ENABLE, GPI_NONE)>;
>>>
>>> This is usage in DTS and is not an argument to store register
>>> addresses/offsets as bindings. What is the usage (of define, not value)
>>> in the driver?
>>>
>>
>> The existing implementation reuse the macros for DTS and driver.
>
> Where in the driver? Grep gives zero results.
>
>> Do you mean we need to separate the macros, one for DTS and one for driver usage?
>
> No, if driver uses them it is fine. The problem is I cannot find it
> anywhere.
>
>> Or you have any better suggestion?
>>
>> These macros are the value of register, not register addresses/offsets,
>> except for with prefix of GPI.
>
> Still, values are not usually part of bindings.
>
>>
>> Drivers rarely reference macros directly, mostly parsing dts and writing them to registers.
>
> So drivers do not use macros? Then there is no reason to store them in
> bindings? What do you "bind" if there is no usage (and we do not talk
> about DTS...)?
>

These macros are more friendly for configuring dts, so i stay the file.
And change the file path to 'arch/riscv/boot/dts/starfive/',
change the file name to 'jh7110-pinfunc.h'.

Best regards,
Jianlong Huang

2022-12-07 13:38:13

by Emil Renner Berthing

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] dt-bindings: pinctrl: Add StarFive JH7110 pinctrl definitions

On Fri, 18 Nov 2022 at 02:11, Hal Feng <[email protected]> wrote:
>
> From: Jianlong Huang <[email protected]>
>
> Add pinctrl definitions for StarFive JH7110 SoC.
>
> Co-developed-by: Emil Renner Berthing <[email protected]>
> Signed-off-by: Emil Renner Berthing <[email protected]>
> Signed-off-by: Jianlong Huang <[email protected]>
> Signed-off-by: Hal Feng <[email protected]>
> ---
> .../pinctrl/pinctrl-starfive-jh7110.h | 427 ++++++++++++++++++
> 1 file changed, 427 insertions(+)
> create mode 100644 include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h
>
> diff --git a/include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h b/include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h
> new file mode 100644
> index 000000000000..fb02345caa27
> --- /dev/null
> +++ b/include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h
> @@ -0,0 +1,427 @@
> +/* SPDX-License-Identifier: GPL-2.0 OR MIT */
> +/*
> + * Copyright (C) 2022 Emil Renner Berthing <[email protected]>
> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
> + */
> +
> +#ifndef __DT_BINDINGS_PINCTRL_STARFIVE_JH7110_H__
> +#define __DT_BINDINGS_PINCTRL_STARFIVE_JH7110_H__
> +
> +/*
> + * mux bits:
> + * | 31 - 24 | 23 - 16 | 15 - 10 | 9 - 8 | 7 - 0 |
> + * | din | dout | doen | function | gpio nr |
> + *
> + * dout: output signal
> + * doen: output enable signal
> + * din: optional input signal, 0xff = none
> + * function:
> + * gpio nr: gpio number, 0 - 63
> + */
> +#define GPIOMUX(n, dout, doen, din) ( \
> + (((din) & 0xff) << 24) | \
> + (((dout) & 0xff) << 16) | \
> + (((doen) & 0x3f) << 10) | \
> + ((n) & 0x3f))
> +
> +#define PINMUX(n, func) ((1 << 10) | (((func) & 0x3) << 8) | ((n) & 0xff))
> +
> +/* sys_iomux pin */
> +#define PAD_GPIO0 0
> +#define PAD_GPIO1 1
> +#define PAD_GPIO2 2
> +#define PAD_GPIO3 3
> +#define PAD_GPIO4 4
> +#define PAD_GPIO5 5
> +#define PAD_GPIO6 6
> +#define PAD_GPIO7 7
> +#define PAD_GPIO8 8
> +#define PAD_GPIO9 9
> +#define PAD_GPIO10 10
> +#define PAD_GPIO11 11
> +#define PAD_GPIO12 12
> +#define PAD_GPIO13 13
> +#define PAD_GPIO14 14
> +#define PAD_GPIO15 15
> +#define PAD_GPIO16 16
> +#define PAD_GPIO17 17
> +#define PAD_GPIO18 18
> +#define PAD_GPIO19 19
> +#define PAD_GPIO20 20
> +#define PAD_GPIO21 21
> +#define PAD_GPIO22 22
> +#define PAD_GPIO23 23
> +#define PAD_GPIO24 24
> +#define PAD_GPIO25 25
> +#define PAD_GPIO26 26
> +#define PAD_GPIO27 27
> +#define PAD_GPIO28 28
> +#define PAD_GPIO29 29
> +#define PAD_GPIO30 30
> +#define PAD_GPIO31 31
> +#define PAD_GPIO32 32
> +#define PAD_GPIO33 33
> +#define PAD_GPIO34 34
> +#define PAD_GPIO35 35
> +#define PAD_GPIO36 36
> +#define PAD_GPIO37 37
> +#define PAD_GPIO38 38
> +#define PAD_GPIO39 39
> +#define PAD_GPIO40 40
> +#define PAD_GPIO41 41
> +#define PAD_GPIO42 42
> +#define PAD_GPIO43 43
> +#define PAD_GPIO44 44
> +#define PAD_GPIO45 45
> +#define PAD_GPIO46 46
> +#define PAD_GPIO47 47
> +#define PAD_GPIO48 48
> +#define PAD_GPIO49 49
> +#define PAD_GPIO50 50
> +#define PAD_GPIO51 51
> +#define PAD_GPIO52 52
> +#define PAD_GPIO53 53
> +#define PAD_GPIO54 54
> +#define PAD_GPIO55 55
> +#define PAD_GPIO56 56
> +#define PAD_GPIO57 57
> +#define PAD_GPIO58 58
> +#define PAD_GPIO59 59
> +#define PAD_GPIO60 60
> +#define PAD_GPIO61 61
> +#define PAD_GPIO62 62
> +#define PAD_GPIO63 63
> +#define PAD_SD0_CLK 64
> +#define PAD_SD0_CMD 65
> +#define PAD_SD0_DATA0 66
> +#define PAD_SD0_DATA1 67
> +#define PAD_SD0_DATA2 68
> +#define PAD_SD0_DATA3 69
> +#define PAD_SD0_DATA4 70
> +#define PAD_SD0_DATA5 71
> +#define PAD_SD0_DATA6 72
> +#define PAD_SD0_DATA7 73
> +#define PAD_SD0_STRB 74
> +#define PAD_GMAC1_MDC 75
> +#define PAD_GMAC1_MDIO 76
> +#define PAD_GMAC1_RXD0 77
> +#define PAD_GMAC1_RXD1 78
> +#define PAD_GMAC1_RXD2 79
> +#define PAD_GMAC1_RXD3 80
> +#define PAD_GMAC1_RXDV 81
> +#define PAD_GMAC1_RXC 82
> +#define PAD_GMAC1_TXD0 83
> +#define PAD_GMAC1_TXD1 84
> +#define PAD_GMAC1_TXD2 85
> +#define PAD_GMAC1_TXD3 86
> +#define PAD_GMAC1_TXEN 87
> +#define PAD_GMAC1_TXC 88
> +#define PAD_QSPI_SCLK 89
> +#define PAD_QSPI_CS0 90
> +#define PAD_QSPI_DATA0 91
> +#define PAD_QSPI_DATA1 92
> +#define PAD_QSPI_DATA2 93
> +#define PAD_QSPI_DATA3 94
> +
> +/* aon_iomux pin */
> +#define PAD_TESTEN 0
> +#define PAD_RGPIO0 1
> +#define PAD_RGPIO1 2
> +#define PAD_RGPIO2 3
> +#define PAD_RGPIO3 4
> +#define PAD_RSTN 5
> +#define PAD_GMAC0_MDC 6
> +#define PAD_GMAC0_MDIO 7
> +#define PAD_GMAC0_RXD0 8
> +#define PAD_GMAC0_RXD1 9
> +#define PAD_GMAC0_RXD2 10
> +#define PAD_GMAC0_RXD3 11
> +#define PAD_GMAC0_RXDV 12
> +#define PAD_GMAC0_RXC 13
> +#define PAD_GMAC0_TXD0 14
> +#define PAD_GMAC0_TXD1 15
> +#define PAD_GMAC0_TXD2 16
> +#define PAD_GMAC0_TXD3 17
> +#define PAD_GMAC0_TXEN 18
> +#define PAD_GMAC0_TXC 19
> +
> +/* sys_iomux dout */
> +#define GPOUT_LOW 0
> +#define GPOUT_HIGH 1
> +#define GPOUT_SYS_WAVE511_UART_TX 2
> +#define GPOUT_SYS_CAN0_STBY 3
> +#define GPOUT_SYS_CAN0_TST_NEXT_BIT 4
> +#define GPOUT_SYS_CAN0_TST_SAMPLE_POINT 5
> +#define GPOUT_SYS_CAN0_TXD 6
> +#define GPOUT_SYS_USB_DRIVE_VBUS 7
> +#define GPOUT_SYS_QSPI_CS1 8
> +#define GPOUT_SYS_SPDIF 9
> +#define GPOUT_SYS_HDMI_CEC_SDA 10
> +#define GPOUT_SYS_HDMI_DDC_SCL 11
> +#define GPOUT_SYS_HDMI_DDC_SDA 12
> +#define GPOUT_SYS_WATCHDOG 13
> +#define GPOUT_SYS_I2C0_CLK 14
> +#define GPOUT_SYS_I2C0_DATA 15
> +#define GPOUT_SYS_SDIO0_BACK_END_POWER 16
> +#define GPOUT_SYS_SDIO0_CARD_POWER_EN 17
> +#define GPOUT_SYS_SDIO0_CCMD_OD_PULLUP_EN 18
> +#define GPOUT_SYS_SDIO0_RST 19
> +#define GPOUT_SYS_UART0_TX 20
> +#define GPOUT_SYS_HIFI4_JTAG_TDO 21
> +#define GPOUT_SYS_JTAG_TDO 22
> +#define GPOUT_SYS_PDM_MCLK 23
> +#define GPOUT_SYS_PWM_CHANNEL0 24
> +#define GPOUT_SYS_PWM_CHANNEL1 25
> +#define GPOUT_SYS_PWM_CHANNEL2 26
> +#define GPOUT_SYS_PWM_CHANNEL3 27
> +#define GPOUT_SYS_PWMDAC_LEFT 28
> +#define GPOUT_SYS_PWMDAC_RIGHT 29
> +#define GPOUT_SYS_SPI0_CLK 30
> +#define GPOUT_SYS_SPI0_FSS 31
> +#define GPOUT_SYS_SPI0_TXD 32
> +#define GPOUT_SYS_GMAC_PHYCLK 33
> +#define GPOUT_SYS_I2SRX_BCLK 34
> +#define GPOUT_SYS_I2SRX_LRCK 35
> +#define GPOUT_SYS_I2STX0_BCLK 36
> +#define GPOUT_SYS_I2STX0_LRCK 37
> +#define GPOUT_SYS_MCLK 38
> +#define GPOUT_SYS_TDM_CLK 39
> +#define GPOUT_SYS_TDM_SYNC 40
> +#define GPOUT_SYS_TDM_TXD 41
> +#define GPOUT_SYS_TRACE_DATA0 42
> +#define GPOUT_SYS_TRACE_DATA1 43
> +#define GPOUT_SYS_TRACE_DATA2 44
> +#define GPOUT_SYS_TRACE_DATA3 45
> +#define GPOUT_SYS_TRACE_REF 46
> +#define GPOUT_SYS_CAN1_STBY 47
> +#define GPOUT_SYS_CAN1_TST_NEXT_BIT 48
> +#define GPOUT_SYS_CAN1_TST_SAMPLE_POINT 49
> +#define GPOUT_SYS_CAN1_TXD 50
> +#define GPOUT_SYS_I2C1_CLK 51
> +#define GPOUT_SYS_I2C1_DATA 52
> +#define GPOUT_SYS_SDIO1_BACK_END_POWER 53
> +#define GPOUT_SYS_SDIO1_CARD_POWER_EN 54
> +#define GPOUT_SYS_SDIO1_CLK 55
> +#define GPOUT_SYS_SDIO1_CMD_OD_PULLUP_EN 56
> +#define GPOUT_SYS_SDIO1_CMD 57
> +#define GPOUT_SYS_SDIO1_DATA0 58
> +#define GPOUT_SYS_SDIO1_DATA1 59
> +#define GPOUT_SYS_SDIO1_DATA2 60
> +#define GPOUT_SYS_SDIO1_DATA3 61
> +#define GPOUT_SYS_SDIO1_DATA4 63
> +#define GPOUT_SYS_SDIO1_DATA5 63
> +#define GPOUT_SYS_SDIO1_DATA6 64
> +#define GPOUT_SYS_SDIO1_DATA7 65
> +#define GPOUT_SYS_SDIO1_RST 66
> +#define GPOUT_SYS_UART1_RTS 67
> +#define GPOUT_SYS_UART1_TX 68
> +#define GPOUT_SYS_I2STX1_SDO0 69
> +#define GPOUT_SYS_I2STX1_SDO1 70
> +#define GPOUT_SYS_I2STX1_SDO2 71
> +#define GPOUT_SYS_I2STX1_SDO3 72
> +#define GPOUT_SYS_SPI1_CLK 73
> +#define GPOUT_SYS_SPI1_FSS 74
> +#define GPOUT_SYS_SPI1_TXD 75
> +#define GPOUT_SYS_I2C2_CLK 76
> +#define GPOUT_SYS_I2C2_DATA 77
> +#define GPOUT_SYS_UART2_RTS 78
> +#define GPOUT_SYS_UART2_TX 79
> +#define GPOUT_SYS_SPI2_CLK 80
> +#define GPOUT_SYS_SPI2_FSS 81
> +#define GPOUT_SYS_SPI2_TXD 82
> +#define GPOUT_SYS_I2C3_CLK 83
> +#define GPOUT_SYS_I2C3_DATA 84
> +#define GPOUT_SYS_UART3_TX 85
> +#define GPOUT_SYS_SPI3_CLK 86
> +#define GPOUT_SYS_SPI3_FSS 87
> +#define GPOUT_SYS_SPI3_TXD 88
> +#define GPOUT_SYS_I2C4_CLK 89
> +#define GPOUT_SYS_I2C4_DATA 90
> +#define GPOUT_SYS_UART4_RTS 91
> +#define GPOUT_SYS_UART4_TX 92
> +#define GPOUT_SYS_SPI4_CLK 93
> +#define GPOUT_SYS_SPI4_FSS 94
> +#define GPOUT_SYS_SPI4_TXD 95
> +#define GPOUT_SYS_I2C5_CLK 96
> +#define GPOUT_SYS_I2C5_DATA 97
> +#define GPOUT_SYS_UART5_RTS 98
> +#define GPOUT_SYS_UART5_TX 99
> +#define GPOUT_SYS_SPI5_CLK 100
> +#define GPOUT_SYS_SPI5_FSS 101
> +#define GPOUT_SYS_SPI5_TXD 102
> +#define GPOUT_SYS_I2C6_CLK 103
> +#define GPOUT_SYS_I2C6_DATA 104
> +#define GPOUT_SYS_SPI6_CLK 105
> +#define GPOUT_SYS_SPI6_FSS 106
> +#define GPOUT_SYS_SPI6_TXD 107
> +
> +/* aon_iomux dout */
> +#define GPOUT_AON_CLK_32K_OUT 2
> +#define GPOUT_AON_PTC0_PWM4 3
> +#define GPOUT_AON_PTC0_PWM5 4
> +#define GPOUT_AON_PTC0_PWM6 5
> +#define GPOUT_AON_PTC0_PWM7 6
> +#define GPOUT_AON_CLK_GCLK0 7
> +#define GPOUT_AON_CLK_GCLK1 8
> +#define GPOUT_AON_CLK_GCLK2 9
> +
> +/* sys_iomux doen */
> +#define GPOEN_ENABLE 0
> +#define GPOEN_DISABLE 1
> +#define GPOEN_SYS_HDMI_CEC_SDA 2
> +#define GPOEN_SYS_HDMI_DDC_SCL 3
> +#define GPOEN_SYS_HDMI_DDC_SDA 4
> +#define GPOEN_SYS_I2C0_CLK 5
> +#define GPOEN_SYS_I2C0_DATA 6
> +#define GPOEN_SYS_HIFI4_JTAG_TDO 7
> +#define GPOEN_SYS_JTAG_TDO 8
> +#define GPOEN_SYS_PWM0_CHANNEL0 9
> +#define GPOEN_SYS_PWM0_CHANNEL1 10
> +#define GPOEN_SYS_PWM0_CHANNEL2 11
> +#define GPOEN_SYS_PWM0_CHANNEL3 12
> +#define GPOEN_SYS_SPI0_NSSPCTL 13
> +#define GPOEN_SYS_SPI0_NSSP 14
> +#define GPOEN_SYS_TDM_SYNC 15
> +#define GPOEN_SYS_TDM_TXD 16
> +#define GPOEN_SYS_I2C1_CLK 17
> +#define GPOEN_SYS_I2C1_DATA 18
> +#define GPOEN_SYS_SDIO1_CMD 19
> +#define GPOEN_SYS_SDIO1_DATA0 20
> +#define GPOEN_SYS_SDIO1_DATA1 21
> +#define GPOEN_SYS_SDIO1_DATA2 22
> +#define GPOEN_SYS_SDIO1_DATA3 23
> +#define GPOEN_SYS_SDIO1_DATA4 24
> +#define GPOEN_SYS_SDIO1_DATA5 25
> +#define GPOEN_SYS_SDIO1_DATA6 26
> +#define GPOEN_SYS_SDIO1_DATA7 27
> +#define GPOEN_SYS_SPI1_NSSPCTL 28
> +#define GPOEN_SYS_SPI1_NSSP 29
> +#define GPOEN_SYS_I2C2_CLK 30
> +#define GPOEN_SYS_I2C2_DATA 31
> +#define GPOEN_SYS_SPI2_NSSPCTL 32
> +#define GPOEN_SYS_SPI2_NSSP 33
> +#define GPOEN_SYS_I2C3_CLK 34
> +#define GPOEN_SYS_I2C3_DATA 35
> +#define GPOEN_SYS_SPI3_NSSPCTL 36
> +#define GPOEN_SYS_SPI3_NSSP 37
> +#define GPOEN_SYS_I2C4_CLK 38
> +#define GPOEN_SYS_I2C4_DATA 39
> +#define GPOEN_SYS_SPI4_NSSPCTL 40
> +#define GPOEN_SYS_SPI4_NSSP 41
> +#define GPOEN_SYS_I2C5_CLK 42
> +#define GPOEN_SYS_I2C5_DATA 43
> +#define GPOEN_SYS_SPI5_NSSPCTL 44
> +#define GPOEN_SYS_SPI5_NSSP 45
> +#define GPOEN_SYS_I2C6_CLK 46
> +#define GPOEN_SYS_I2C6_DATA 47
> +#define GPOEN_SYS_SPI6_NSSPCTL 48
> +#define GPOEN_SYS_SPI6_NSSP 49
> +
> +/* aon_iomux doen */
> +#define GPOEN_AON_PTC0_OE_N_4 2
> +#define GPOEN_AON_PTC0_OE_N_5 3
> +#define GPOEN_AON_PTC0_OE_N_6 4
> +#define GPOEN_AON_PTC0_OE_N_7 5
> +
> +/* sys_iomux gin */
> +#define GPI_NONE 255
> +
> +#define GPI_SYS_WAVE511_UART_RX 0
> +#define GPI_SYS_CAN0_RXD 1
> +#define GPI_SYS_USB_OVERCURRENT 2
> +#define GPI_SYS_SPDIF 3
> +#define GPI_SYS_JTAG_RST 4
> +#define GPI_SYS_HDMI_CEC_SDA 5
> +#define GPI_SYS_HDMI_DDC_SCL 6
> +#define GPI_SYS_HDMI_DDC_SDA 7
> +#define GPI_SYS_HDMI_HPD 8
> +#define GPI_SYS_I2C0_CLK 9
> +#define GPI_SYS_I2C0_DATA 10
> +#define GPI_SYS_SDIO0_CD 11
> +#define GPI_SYS_SDIO0_INT 12
> +#define GPI_SYS_SDIO0_WP 13
> +#define GPI_SYS_UART0_RX 14
> +#define GPI_SYS_HIFI4_JTAG_TCK 15
> +#define GPI_SYS_HIFI4_JTAG_TDI 16
> +#define GPI_SYS_HIFI4_JTAG_TMS 17
> +#define GPI_SYS_HIFI4_JTAG_RST 18
> +#define GPI_SYS_JTAG_TDI 19
> +#define GPI_SYS_JTAG_TMS 20
> +#define GPI_SYS_PDM_DMIC0 21
> +#define GPI_SYS_PDM_DMIC1 22
> +#define GPI_SYS_I2SRX_SDIN0 23
> +#define GPI_SYS_I2SRX_SDIN1 24
> +#define GPI_SYS_I2SRX_SDIN2 25
> +#define GPI_SYS_SPI0_CLK 26
> +#define GPI_SYS_SPI0_FSS 27
> +#define GPI_SYS_SPI0_RXD 28
> +#define GPI_SYS_JTAG_TCK 29
> +#define GPI_SYS_MCLK_EXT 30
> +#define GPI_SYS_I2SRX_BCLK 31
> +#define GPI_SYS_I2SRX_LRCK 32
> +#define GPI_SYS_I2STX0_BCLK 33
> +#define GPI_SYS_I2STX0_LRCK 34
> +#define GPI_SYS_TDM_CLK 35
> +#define GPI_SYS_TDM_RXD 36
> +#define GPI_SYS_TDM_SYNC 37
> +#define GPI_SYS_CAN1_RXD 38
> +#define GPI_SYS_I2C1_CLK 39
> +#define GPI_SYS_I2C1_DATA 40
> +#define GPI_SYS_SDIO1_CD 41
> +#define GPI_SYS_SDIO1_INT 42
> +#define GPI_SYS_SDIO1_WP 43
> +#define GPI_SYS_SDIO1_CMD 44
> +#define GPI_SYS_SDIO1_DATA0 45
> +#define GPI_SYS_SDIO1_DATA1 46
> +#define GPI_SYS_SDIO1_DATA2 47
> +#define GPI_SYS_SDIO1_DATA3 48
> +#define GPI_SYS_SDIO1_DATA4 49
> +#define GPI_SYS_SDIO1_DATA5 50
> +#define GPI_SYS_SDIO1_DATA6 51
> +#define GPI_SYS_SDIO1_DATA7 52
> +#define GPI_SYS_SDIO1_STRB 53
> +#define GPI_SYS_UART1_CTS 54
> +#define GPI_SYS_UART1_RX 55
> +#define GPI_SYS_SPI1_CLK 56
> +#define GPI_SYS_SPI1_FSS 57
> +#define GPI_SYS_SPI1_RXD 58
> +#define GPI_SYS_I2C2_CLK 59
> +#define GPI_SYS_I2C2_DATA 60
> +#define GPI_SYS_UART2_CTS 61
> +#define GPI_SYS_UART2_RX 62
> +#define GPI_SYS_SPI2_CLK 63
> +#define GPI_SYS_SPI2_FSS 64
> +#define GPI_SYS_SPI2_RXD 65
> +#define GPI_SYS_I2C3_CLK 66
> +#define GPI_SYS_I2C3_DATA 67
> +#define GPI_SYS_UART3_RX 68
> +#define GPI_SYS_SPI3_CLK 69
> +#define GPI_SYS_SPI3_FSS 70
> +#define GPI_SYS_SPI3_RXD 71
> +#define GPI_SYS_I2C4_CLK 72
> +#define GPI_SYS_I2C4_DATA 73
> +#define GPI_SYS_UART4_CTS 74
> +#define GPI_SYS_UART4_RX 75
> +#define GPI_SYS_SPI4_CLK 76
> +#define GPI_SYS_SPI4_FSS 77
> +#define GPI_SYS_SPI4_RXD 78
> +#define GPI_SYS_I2C5_CLK 79
> +#define GPI_SYS_I2C5_DATA 80
> +#define GPI_SYS_UART5_CTS 81
> +#define GPI_SYS_UART5_RX 82
> +#define GPI_SYS_SPI5_CLK 83
> +#define GPI_SYS_SPI5_FSS 84
> +#define GPI_SYS_SPI5_RXD 85
> +#define GPI_SYS_I2C6_CLK 86
> +#define GPI_SYS_I2C6_DATA 87
> +#define GPI_SYS_SPI6_CLK 88
> +#define GPI_SYS_SPI6_FSS 89
> +#define GPI_SYS_SPI6_RXD 90

You seem to have removed the comments documenting what these lines are
called in the documentation. Please don't do that. The names in the
documentation are overly long for macro names, but these comments are
really helpful to map the macro names back to the name used in the
docs.

> +/* aon_iomux gin */
> +#define GPI_AON_PMU_GPIO_WAKEUP_0 0
> +#define GPI_AON_PMU_GPIO_WAKEUP_1 1
> +#define GPI_AON_PMU_GPIO_WAKEUP_2 2
> +#define GPI_AON_PMU_GPIO_WAKEUP_3 3
> +
> +#endif
> --
> 2.38.1
>

2022-12-07 14:00:00

by Emil Renner Berthing

[permalink] [raw]
Subject: Re: [PATCH v2 3/5] dt-bindings: pinctrl: Add StarFive JH7110 aon pinctrl

On Mon, 28 Nov 2022 at 02:15, Jianlong Huang
<[email protected]> wrote:
>
> On Mon, 21 Nov 2022 09:44:00 +0100, Krzysztof Kozlowski wrote:
> > On 18/11/2022 02:11, Hal Feng wrote:
> >> From: Jianlong Huang <[email protected]>
> >>
> >> Add pinctrl bindings for StarFive JH7110 SoC aon pinctrl controller.
> >>
> >> Signed-off-by: Jianlong Huang <[email protected]>
> >> Signed-off-by: Hal Feng <[email protected]>
> >> ---
> >> .../pinctrl/starfive,jh7110-aon-pinctrl.yaml | 134 ++++++++++++++++++
> >> 1 file changed, 134 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml
> >>
> >> diff --git a/Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml
> >> new file mode 100644
> >> index 000000000000..1dd000e1f614
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml
> >> @@ -0,0 +1,134 @@
> >> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> >> +%YAML 1.2
> >> +---
> >> +$id: http://devicetree.org/schemas/pinctrl/starfive,jh7110-aon-pinctrl.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: StarFive JH7110 Aon Pin Controller
> >> +
> >> +description: |
> >> + Bindings for the JH7110 RISC-V SoC from StarFive Technology Ltd.
> >> +
> >> + Out of the SoC's many pins only the ones named PAD_GPIO0 to PAD_GPIO4
> >> + can be multiplexed and have configurable bias, drive strength,
> >> + schmitt trigger etc.
> >> + Some peripherals have their I/O go through the 4 "GPIOs". This also
> >> + includes PWM.
> >> +
> >> +maintainers:
> >> + - Jianlong Huang <[email protected]>
> >> +
> >> +properties:
> >> + compatible:
> >> + const: starfive,jh7110-aon-pinctrl
> >> +
> >> + reg:
> >> + maxItems: 1
> >> +
> >> + reg-names:
> >> + items:
> >> + - const: control

Again this doesn't seem necessary when you only have 1 memory range.

> >> +
> >> + clocks:
> >> + maxItems: 1
> >> +
> >> + resets:
> >> + maxItems: 1
> >> +
> >> + gpio-controller: true
> >> +
> >> + "#gpio-cells":
> >> + const: 2
> >> +
> >> + interrupts:
> >> + maxItems: 1
> >> + description: The GPIO parent interrupt.
> >
> > Same comments apply plus one more.
>
> Will fix, drop this description.
>
> >
> >> +
> >> + interrupt-controller: true
> >> +
> >> + "#interrupt-cells":
> >> + const: 2
> >> +
> >> +required:
> >> + - compatible
> >> + - reg
> >> + - reg-names
> >> + - gpio-controller
> >> + - "#gpio-cells"
> >> + - interrupts
> >> + - interrupt-controller
> >> + - "#interrupt-cells"
> >
> > "required:" goes after patternProperties.
>
> Will fix.
>
> >
> >> +
> >> +patternProperties:
> >> + '-[0-9]+$':
> >
> > Same comment.
>
> Will fix.
> Keep consistent quotes, use '
>
> >
> >> + type: object
> >> + patternProperties:
> >> + '-pins$':
> >> + type: object
> >> + description: |
> >> + A pinctrl node should contain at least one subnode representing the
> >> + pinctrl groups available on the machine. Each subnode will list the
> >> + pins it needs, and how they should be configured, with regard to
> >> + muxer configuration, system signal configuration, pin groups for
> >> + vin/vout module, pin voltage, mux functions for output, mux functions
> >> + for output enable, mux functions for input.
> >> +
> >> + properties:
> >> + pinmux:
> >> + description: |
> >> + The list of GPIOs and their mux settings that properties in the
> >> + node apply to. This should be set using the GPIOMUX macro.
> >> + $ref: "/schemas/pinctrl/pinmux-node.yaml#/properties/pinmux"
> >> +
> >> + bias-disable: true
> >> +
> >> + bias-pull-up:
> >> + type: boolean
> >> +
> >> + bias-pull-down:
> >> + type: boolean
> >> +
> >> + drive-strength:
> >> + enum: [ 2, 4, 8, 12 ]
> >> +
> >> + input-enable: true
> >> +
> >> + input-disable: true
> >> +
> >> + input-schmitt-enable: true
> >> +
> >> + input-schmitt-disable: true
> >> +
> >> + slew-rate:
> >> + maximum: 1
> >> +
> >> + additionalProperties: false
> >> +
> >> + additionalProperties: false
> >> +
> >> +additionalProperties: false
> >> +
> >> +examples:
> >> + - |
> >> + #include <dt-bindings/clock/starfive-jh7110.h>
> >> + #include <dt-bindings/reset/starfive-jh7110.h>
> >> + #include <dt-bindings/pinctrl/pinctrl-starfive-jh7110.h>
> >> +
> >> + soc {
> >> + #address-cells = <2>;
> >> + #size-cells = <2>;

Again these two lines can be dropped..

> >
> > Same comment.
>
> Will fix.
> Use 4 spaces for example indentation.
>
> >
> >> +
> >> + gpioa: gpio@17020000 {
> >> + compatible = "starfive,jh7110-aon-pinctrl";
> >> + reg = <0x0 0x17020000 0x0 0x10000>;

..if you just change this to
reg = <0x17020000 0x10000>;

> >> + reg-names = "control";
> >> + resets = <&aoncrg_rst JH7110_AONRST_AON_IOMUX>;
> >> + interrupts = <85>;
> >> + interrupt-controller;
> >> + #interrupt-cells = <2>;
> >> + #gpio-cells = <2>;
> >> + gpio-controller;
> >> + };
> >> + };
> >> +
> >> +...
> >
>
> Best regards,
> Jianlong Huang
>
>

2022-12-07 14:17:26

by Emil Renner Berthing

[permalink] [raw]
Subject: Re: [PATCH v2 4/5] pinctrl: starfive: Add StarFive JH7110 sys controller driver

On Fri, 18 Nov 2022 at 02:11, Hal Feng <[email protected]> wrote:
>
> From: Jianlong Huang <[email protected]>
>
> Add pinctrl driver for StarFive JH7110 SoC sys pinctrl controller.
>
> Co-developed-by: Emil Renner Berthing <[email protected]>
> Signed-off-by: Emil Renner Berthing <[email protected]>
> Signed-off-by: Jianlong Huang <[email protected]>
> Signed-off-by: Hal Feng <[email protected]>
> ---
> MAINTAINERS | 7 +-
> drivers/pinctrl/starfive/Kconfig | 21 +
> drivers/pinctrl/starfive/Makefile | 5 +
> drivers/pinctrl/starfive/pinctrl-jh7110-sys.c | 464 +++++++++
> drivers/pinctrl/starfive/pinctrl-starfive.c | 972 ++++++++++++++++++
> drivers/pinctrl/starfive/pinctrl-starfive.h | 72 ++
> 6 files changed, 1538 insertions(+), 3 deletions(-)
> create mode 100644 drivers/pinctrl/starfive/pinctrl-jh7110-sys.c
> create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive.c
> create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ec6647e2772f..a70c1d0f303e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19606,13 +19606,14 @@ F: Documentation/devicetree/bindings/clock/starfive*
> F: drivers/clk/starfive/
> F: include/dt-bindings/clock/starfive*
>
> -STARFIVE JH7100 PINCTRL DRIVER
> +STARFIVE PINCTRL DRIVER

There are now more than 1 driver, so please use DRIVERS

> M: Emil Renner Berthing <[email protected]>
> +M: Jianlong Huang <[email protected]>
> L: [email protected]
> S: Maintained
> -F: Documentation/devicetree/bindings/pinctrl/starfive,jh7100-pinctrl.yaml
> +F: Documentation/devicetree/bindings/pinctrl/starfive*
> F: drivers/pinctrl/starfive/
> -F: include/dt-bindings/pinctrl/pinctrl-starfive-jh7100.h
> +F: include/dt-bindings/pinctrl/pinctrl-starfive*
>
> STARFIVE RESET CONTROLLER DRIVERS
> M: Emil Renner Berthing <[email protected]>
> diff --git a/drivers/pinctrl/starfive/Kconfig b/drivers/pinctrl/starfive/Kconfig
> index 55c514e622f9..c7896a1f7d5a 100644
> --- a/drivers/pinctrl/starfive/Kconfig
> +++ b/drivers/pinctrl/starfive/Kconfig
> @@ -16,3 +16,24 @@ config PINCTRL_STARFIVE_JH7100
> This also provides an interface to the GPIO pins not used by other
> peripherals supporting inputs, outputs, configuring pull-up/pull-down
> and interrupts on input changes.
> +
> +config PINCTRL_STARFIVE
> + bool

Using PINCTRL_STARFIVE here is confusing, since it doesn't apply to the JH7100.
Please call this PINCTRL_STARFIVE_JH7110.

Also is there a reason this can't be tristate and the driver compiled
as a module?

> + select GENERIC_PINCTRL_GROUPS
> + select GENERIC_PINMUX_FUNCTIONS
> + select GENERIC_PINCONF
> + select GPIOLIB
> + select GPIOLIB_IRQCHIP
> + select OF_GPIO
> +
> +config PINCTRL_STARFIVE_JH7110
> + bool "Pinctrl and GPIO driver for the StarFive JH7110 SoC"

Here you seem to have 1 Kconfig option for 2 separate drivers:
pinctrl-jh7110-aon and pinctrl-jh7110-sys.

I'd have individual Kconfig options for each module,
PINCTRL_STARFIVE_JH7110_SYS and PINCTRL_STARFIVE_JH7110_AON.

> + depends on SOC_STARFIVE || COMPILE_TEST
> + depends on OF
> + select PINCTRL_STARFIVE
> + default SOC_STARFIVE
> + help
> + Say yes here to support pin control on the StarFive JH7110 SoC.
> + This also provides an interface to the GPIO pins not used by other
> + peripherals supporting inputs, outputs, configuring pull-up/pull-down
> + and interrupts on input changes.
> diff --git a/drivers/pinctrl/starfive/Makefile b/drivers/pinctrl/starfive/Makefile
> index 0293f26a0a99..404929f760e8 100644
> --- a/drivers/pinctrl/starfive/Makefile
> +++ b/drivers/pinctrl/starfive/Makefile
> @@ -1,3 +1,8 @@
> # SPDX-License-Identifier: GPL-2.0
>
> +# Core
> +obj-$(CONFIG_PINCTRL_STARFIVE) += pinctrl-starfive.o

This is confusing since it gives the impression that
pinctrl-starfive.o is used by all starfive drivers, but in reality
it's only used by the jh7110 drivers. Please group the lines by SoC.

> +# SoC Drivers
> obj-$(CONFIG_PINCTRL_STARFIVE_JH7100) += pinctrl-starfive-jh7100.o
> +obj-$(CONFIG_PINCTRL_STARFIVE_JH7110) += pinctrl-jh7110-sys.o

For consistency please call the module pinctrl-starfive-jh7110-sys.

> diff --git a/drivers/pinctrl/starfive/pinctrl-jh7110-sys.c b/drivers/pinctrl/starfive/pinctrl-jh7110-sys.c
> new file mode 100644
> index 000000000000..942e0b6e5ac6
> --- /dev/null
> +++ b/drivers/pinctrl/starfive/pinctrl-jh7110-sys.c
> @@ -0,0 +1,464 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Pinctrl / GPIO driver for StarFive JH7110 SoC sys controller
> + *
> + * Copyright (C) 2022 Emil Renner Berthing <[email protected]>
> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/io.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +#include <linux/spinlock.h>
> +
> +#include <linux/pinctrl/pinctrl.h>
> +#include <linux/pinctrl/pinmux.h>
> +
> +#include <dt-bindings/pinctrl/pinctrl-starfive-jh7110.h>
> +
> +#include "../core.h"
> +#include "../pinctrl-utils.h"
> +#include "../pinmux.h"
> +#include "../pinconf.h"
> +#include "pinctrl-starfive.h"
> +
> +#define JH7110_SYS_NGPIO 64
> +#define JH7110_SYS_GC_BASE 0
> +
> +/* registers */
> +#define JH7110_SYS_DOEN 0x000
> +#define JH7110_SYS_DOUT 0x040
> +#define JH7110_SYS_GPI 0x080
> +#define JH7110_SYS_GPIOIN 0x118
> +
> +#define JH7110_SYS_GPIOEN 0x0dc
> +#define JH7110_SYS_GPIOIS0 0x0e0
> +#define JH7110_SYS_GPIOIS1 0x0e4
> +#define JH7110_SYS_GPIOIC0 0x0e8
> +#define JH7110_SYS_GPIOIC1 0x0ec
> +#define JH7110_SYS_GPIOIBE0 0x0f0
> +#define JH7110_SYS_GPIOIBE1 0x0f4
> +#define JH7110_SYS_GPIOIEV0 0x0f8
> +#define JH7110_SYS_GPIOIEV1 0x0fc
> +#define JH7110_SYS_GPIOIE0 0x100
> +#define JH7110_SYS_GPIOIE1 0x104
> +#define JH7110_SYS_GPIORIS0 0x108
> +#define JH7110_SYS_GPIORIS1 0x10c
> +#define JH7110_SYS_GPIOMIS0 0x110
> +#define JH7110_SYS_GPIOMIS1 0x114
> +
> +#define SYS_GPO_PDA_0_74_CFG 0x120
> +#define SYS_GPO_PDA_89_94_CFG 0x284

These macros suddenly drop the JH7110_ prefix. Why?

> +static const struct pinctrl_pin_desc jh7110_sys_pins[] = {
> + PINCTRL_PIN(PAD_GPIO0, "GPIO0"),
> + PINCTRL_PIN(PAD_GPIO1, "GPIO1"),
> + PINCTRL_PIN(PAD_GPIO2, "GPIO2"),
> + PINCTRL_PIN(PAD_GPIO3, "GPIO3"),
> + PINCTRL_PIN(PAD_GPIO4, "GPIO4"),
> + PINCTRL_PIN(PAD_GPIO5, "GPIO5"),
> + PINCTRL_PIN(PAD_GPIO6, "GPIO6"),
> + PINCTRL_PIN(PAD_GPIO7, "GPIO7"),
> + PINCTRL_PIN(PAD_GPIO8, "GPIO8"),
> + PINCTRL_PIN(PAD_GPIO9, "GPIO9"),
> + PINCTRL_PIN(PAD_GPIO10, "GPIO10"),
> + PINCTRL_PIN(PAD_GPIO11, "GPIO11"),
> + PINCTRL_PIN(PAD_GPIO12, "GPIO12"),
> + PINCTRL_PIN(PAD_GPIO13, "GPIO13"),
> + PINCTRL_PIN(PAD_GPIO14, "GPIO14"),
> + PINCTRL_PIN(PAD_GPIO15, "GPIO15"),
> + PINCTRL_PIN(PAD_GPIO16, "GPIO16"),
> + PINCTRL_PIN(PAD_GPIO17, "GPIO17"),
> + PINCTRL_PIN(PAD_GPIO18, "GPIO18"),
> + PINCTRL_PIN(PAD_GPIO19, "GPIO19"),
> + PINCTRL_PIN(PAD_GPIO20, "GPIO20"),
> + PINCTRL_PIN(PAD_GPIO21, "GPIO21"),
> + PINCTRL_PIN(PAD_GPIO22, "GPIO22"),
> + PINCTRL_PIN(PAD_GPIO23, "GPIO23"),
> + PINCTRL_PIN(PAD_GPIO24, "GPIO24"),
> + PINCTRL_PIN(PAD_GPIO25, "GPIO25"),
> + PINCTRL_PIN(PAD_GPIO26, "GPIO26"),
> + PINCTRL_PIN(PAD_GPIO27, "GPIO27"),
> + PINCTRL_PIN(PAD_GPIO28, "GPIO28"),
> + PINCTRL_PIN(PAD_GPIO29, "GPIO29"),
> + PINCTRL_PIN(PAD_GPIO30, "GPIO30"),
> + PINCTRL_PIN(PAD_GPIO31, "GPIO31"),
> + PINCTRL_PIN(PAD_GPIO32, "GPIO32"),
> + PINCTRL_PIN(PAD_GPIO33, "GPIO33"),
> + PINCTRL_PIN(PAD_GPIO34, "GPIO34"),
> + PINCTRL_PIN(PAD_GPIO35, "GPIO35"),
> + PINCTRL_PIN(PAD_GPIO36, "GPIO36"),
> + PINCTRL_PIN(PAD_GPIO37, "GPIO37"),
> + PINCTRL_PIN(PAD_GPIO38, "GPIO38"),
> + PINCTRL_PIN(PAD_GPIO39, "GPIO39"),
> + PINCTRL_PIN(PAD_GPIO40, "GPIO40"),
> + PINCTRL_PIN(PAD_GPIO41, "GPIO41"),
> + PINCTRL_PIN(PAD_GPIO42, "GPIO42"),
> + PINCTRL_PIN(PAD_GPIO43, "GPIO43"),
> + PINCTRL_PIN(PAD_GPIO44, "GPIO44"),
> + PINCTRL_PIN(PAD_GPIO45, "GPIO45"),
> + PINCTRL_PIN(PAD_GPIO46, "GPIO46"),
> + PINCTRL_PIN(PAD_GPIO47, "GPIO47"),
> + PINCTRL_PIN(PAD_GPIO48, "GPIO48"),
> + PINCTRL_PIN(PAD_GPIO49, "GPIO49"),
> + PINCTRL_PIN(PAD_GPIO50, "GPIO50"),
> + PINCTRL_PIN(PAD_GPIO51, "GPIO51"),
> + PINCTRL_PIN(PAD_GPIO52, "GPIO52"),
> + PINCTRL_PIN(PAD_GPIO53, "GPIO53"),
> + PINCTRL_PIN(PAD_GPIO54, "GPIO54"),
> + PINCTRL_PIN(PAD_GPIO55, "GPIO55"),
> + PINCTRL_PIN(PAD_GPIO56, "GPIO56"),
> + PINCTRL_PIN(PAD_GPIO57, "GPIO57"),
> + PINCTRL_PIN(PAD_GPIO58, "GPIO58"),
> + PINCTRL_PIN(PAD_GPIO59, "GPIO59"),
> + PINCTRL_PIN(PAD_GPIO60, "GPIO60"),
> + PINCTRL_PIN(PAD_GPIO61, "GPIO61"),
> + PINCTRL_PIN(PAD_GPIO62, "GPIO62"),
> + PINCTRL_PIN(PAD_GPIO63, "GPIO63"),
> + PINCTRL_PIN(PAD_SD0_CLK, "SD0_CLK"),
> + PINCTRL_PIN(PAD_SD0_CMD, "SD0_CMD"),
> + PINCTRL_PIN(PAD_SD0_DATA0, "SD0_DATA0"),
> + PINCTRL_PIN(PAD_SD0_DATA1, "SD0_DATA1"),
> + PINCTRL_PIN(PAD_SD0_DATA2, "SD0_DATA2"),
> + PINCTRL_PIN(PAD_SD0_DATA3, "SD0_DATA3"),
> + PINCTRL_PIN(PAD_SD0_DATA4, "SD0_DATA4"),
> + PINCTRL_PIN(PAD_SD0_DATA5, "SD0_DATA5"),
> + PINCTRL_PIN(PAD_SD0_DATA6, "SD0_DATA6"),
> + PINCTRL_PIN(PAD_SD0_DATA7, "SD0_DATA7"),
> + PINCTRL_PIN(PAD_SD0_STRB, "SD0_STRB"),
> + PINCTRL_PIN(PAD_GMAC1_MDC, "GMAC1_MDC"),
> + PINCTRL_PIN(PAD_GMAC1_MDIO, "GMAC1_MDIO"),
> + PINCTRL_PIN(PAD_GMAC1_RXD0, "GMAC1_RXD0"),
> + PINCTRL_PIN(PAD_GMAC1_RXD1, "GMAC1_RXD1"),
> + PINCTRL_PIN(PAD_GMAC1_RXD2, "GMAC1_RXD2"),
> + PINCTRL_PIN(PAD_GMAC1_RXD3, "GMAC1_RXD3"),
> + PINCTRL_PIN(PAD_GMAC1_RXDV, "GMAC1_RXDV"),
> + PINCTRL_PIN(PAD_GMAC1_RXC, "GMAC1_RXC"),
> + PINCTRL_PIN(PAD_GMAC1_TXD0, "GMAC1_TXD0"),
> + PINCTRL_PIN(PAD_GMAC1_TXD1, "GMAC1_TXD1"),
> + PINCTRL_PIN(PAD_GMAC1_TXD2, "GMAC1_TXD2"),
> + PINCTRL_PIN(PAD_GMAC1_TXD3, "GMAC1_TXD3"),
> + PINCTRL_PIN(PAD_GMAC1_TXEN, "GMAC1_TXEN"),
> + PINCTRL_PIN(PAD_GMAC1_TXC, "GMAC1_TXC"),
> + PINCTRL_PIN(PAD_QSPI_SCLK, "QSPI_SCLK"),
> + PINCTRL_PIN(PAD_QSPI_CS0, "QSPI_CS0"),
> + PINCTRL_PIN(PAD_QSPI_DATA0, "QSPI_DATA0"),
> + PINCTRL_PIN(PAD_QSPI_DATA1, "QSPI_DATA1"),
> + PINCTRL_PIN(PAD_QSPI_DATA2, "QSPI_DATA2"),
> + PINCTRL_PIN(PAD_QSPI_DATA3, "QSPI_DATA3"),
> +};
> +
> +struct jh7110_func_sel {
> + u16 offset;
> + u8 shift;
> + u8 max;
> +};
> +
> +static const struct jh7110_func_sel
> + jh7110_sys_func_sel[ARRAY_SIZE(jh7110_sys_pins)] = {
> + [PAD_GMAC1_RXC] = { 0x29c, 0, 1 },
> + [PAD_GPIO10] = { 0x29c, 2, 3 },
> + [PAD_GPIO11] = { 0x29c, 5, 3 },
> + [PAD_GPIO12] = { 0x29c, 8, 3 },
> + [PAD_GPIO13] = { 0x29c, 11, 3 },
> + [PAD_GPIO14] = { 0x29c, 14, 3 },
> + [PAD_GPIO15] = { 0x29c, 17, 3 },
> + [PAD_GPIO16] = { 0x29c, 20, 3 },
> + [PAD_GPIO17] = { 0x29c, 23, 3 },
> + [PAD_GPIO18] = { 0x29c, 26, 3 },
> + [PAD_GPIO19] = { 0x29c, 29, 3 },
> +
> + [PAD_GPIO20] = { 0x2a0, 0, 3 },
> + [PAD_GPIO21] = { 0x2a0, 3, 3 },
> + [PAD_GPIO22] = { 0x2a0, 6, 3 },
> + [PAD_GPIO23] = { 0x2a0, 9, 3 },
> + [PAD_GPIO24] = { 0x2a0, 12, 3 },
> + [PAD_GPIO25] = { 0x2a0, 15, 3 },
> + [PAD_GPIO26] = { 0x2a0, 18, 3 },
> + [PAD_GPIO27] = { 0x2a0, 21, 3 },
> + [PAD_GPIO28] = { 0x2a0, 24, 3 },
> + [PAD_GPIO29] = { 0x2a0, 27, 3 },
> +
> + [PAD_GPIO30] = { 0x2a4, 0, 3 },
> + [PAD_GPIO31] = { 0x2a4, 3, 3 },
> + [PAD_GPIO32] = { 0x2a4, 6, 3 },
> + [PAD_GPIO33] = { 0x2a4, 9, 3 },
> + [PAD_GPIO34] = { 0x2a4, 12, 3 },
> + [PAD_GPIO35] = { 0x2a4, 15, 3 },
> + [PAD_GPIO36] = { 0x2a4, 17, 3 },
> + [PAD_GPIO37] = { 0x2a4, 20, 3 },
> + [PAD_GPIO38] = { 0x2a4, 23, 3 },
> + [PAD_GPIO39] = { 0x2a4, 26, 3 },
> + [PAD_GPIO40] = { 0x2a4, 29, 3 },
> +
> + [PAD_GPIO41] = { 0x2a8, 0, 3 },
> + [PAD_GPIO42] = { 0x2a8, 3, 3 },
> + [PAD_GPIO43] = { 0x2a8, 6, 3 },
> + [PAD_GPIO44] = { 0x2a8, 9, 3 },
> + [PAD_GPIO45] = { 0x2a8, 12, 3 },
> + [PAD_GPIO46] = { 0x2a8, 15, 3 },
> + [PAD_GPIO47] = { 0x2a8, 18, 3 },
> + [PAD_GPIO48] = { 0x2a8, 21, 3 },
> + [PAD_GPIO49] = { 0x2a8, 24, 3 },
> + [PAD_GPIO50] = { 0x2a8, 27, 3 },
> + [PAD_GPIO51] = { 0x2a8, 30, 3 },
> +
> + [PAD_GPIO52] = { 0x2ac, 0, 3 },
> + [PAD_GPIO53] = { 0x2ac, 2, 3 },
> + [PAD_GPIO54] = { 0x2ac, 4, 3 },
> + [PAD_GPIO55] = { 0x2ac, 6, 3 },
> + [PAD_GPIO56] = { 0x2ac, 9, 3 },
> + [PAD_GPIO57] = { 0x2ac, 12, 3 },
> + [PAD_GPIO58] = { 0x2ac, 15, 3 },
> + [PAD_GPIO59] = { 0x2ac, 18, 3 },
> + [PAD_GPIO60] = { 0x2ac, 21, 3 },
> + [PAD_GPIO61] = { 0x2ac, 24, 3 },
> + [PAD_GPIO62] = { 0x2ac, 27, 3 },
> + [PAD_GPIO63] = { 0x2ac, 30, 3 },
> +
> + [PAD_GPIO6] = { 0x2b0, 0, 3 },
> + [PAD_GPIO7] = { 0x2b0, 2, 3 },
> + [PAD_GPIO8] = { 0x2b0, 5, 3 },
> + [PAD_GPIO9] = { 0x2b0, 8, 3 },
> +};
> +
> +struct jh7110_vin_group_sel {
> + u16 offset;
> + u8 shift;
> + u8 group;
> +};
> +
> +static const struct jh7110_vin_group_sel
> + jh7110_sys_vin_group_sel[ARRAY_SIZE(jh7110_sys_pins)] = {
> + [PAD_GPIO6] = { 0x2b4, 21, 0 },
> + [PAD_GPIO7] = { 0x2b4, 18, 0 },
> + [PAD_GPIO8] = { 0x2b4, 15, 0 },
> + [PAD_GPIO9] = { 0x2b0, 11, 0 },
> + [PAD_GPIO10] = { 0x2b0, 20, 0 },
> + [PAD_GPIO11] = { 0x2b0, 23, 0 },
> + [PAD_GPIO12] = { 0x2b0, 26, 0 },
> + [PAD_GPIO13] = { 0x2b0, 29, 0 },
> + [PAD_GPIO14] = { 0x2b4, 0, 0 },
> + [PAD_GPIO15] = { 0x2b4, 3, 0 },
> + [PAD_GPIO16] = { 0x2b4, 6, 0 },
> + [PAD_GPIO17] = { 0x2b4, 9, 0 },
> + [PAD_GPIO18] = { 0x2b4, 12, 0 },
> + [PAD_GPIO19] = { 0x2b0, 14, 0 },
> + [PAD_GPIO20] = { 0x2b0, 17, 0 },
> +
> + [PAD_GPIO21] = { 0x2b4, 21, 1 },
> + [PAD_GPIO22] = { 0x2b4, 18, 1 },
> + [PAD_GPIO23] = { 0x2b4, 15, 1 },
> + [PAD_GPIO24] = { 0x2b0, 11, 1 },
> + [PAD_GPIO25] = { 0x2b0, 20, 1 },
> + [PAD_GPIO26] = { 0x2b0, 23, 1 },
> + [PAD_GPIO27] = { 0x2b0, 26, 1 },
> + [PAD_GPIO28] = { 0x2b0, 29, 1 },
> + [PAD_GPIO29] = { 0x2b4, 0, 1 },
> + [PAD_GPIO30] = { 0x2b4, 3, 1 },
> + [PAD_GPIO31] = { 0x2b4, 6, 1 },
> + [PAD_GPIO32] = { 0x2b4, 9, 1 },
> + [PAD_GPIO33] = { 0x2b4, 12, 1 },
> + [PAD_GPIO34] = { 0x2b0, 14, 1 },
> + [PAD_GPIO35] = { 0x2b0, 17, 1 },
> +
> + [PAD_GPIO36] = { 0x2b4, 21, 2 },
> + [PAD_GPIO37] = { 0x2b4, 18, 2 },
> + [PAD_GPIO38] = { 0x2b4, 15, 2 },
> + [PAD_GPIO39] = { 0x2b0, 11, 2 },
> + [PAD_GPIO40] = { 0x2b0, 20, 2 },
> + [PAD_GPIO41] = { 0x2b0, 23, 2 },
> + [PAD_GPIO42] = { 0x2b0, 26, 2 },
> + [PAD_GPIO43] = { 0x2b0, 29, 2 },
> + [PAD_GPIO44] = { 0x2b4, 0, 2 },
> + [PAD_GPIO45] = { 0x2b4, 3, 2 },
> + [PAD_GPIO46] = { 0x2b4, 6, 2 },
> + [PAD_GPIO47] = { 0x2b4, 9, 2 },
> + [PAD_GPIO48] = { 0x2b4, 12, 2 },
> + [PAD_GPIO49] = { 0x2b0, 14, 2 },
> + [PAD_GPIO50] = { 0x2b0, 17, 2 },
> +};
> +
> +static void jh7110_set_function(struct starfive_pinctrl *sfp,
> + unsigned int pin, u32 func)
> +{
> + const struct jh7110_func_sel *fs = &jh7110_sys_func_sel[pin];
> + unsigned long flags;
> + void __iomem *reg;
> + u32 mask;
> +
> + if (!fs->offset)
> + return;
> +
> + if (func > fs->max)
> + return;
> +
> + reg = sfp->base + fs->offset;
> + func = func << fs->shift;
> + mask = 0x3U << fs->shift;
> +
> + raw_spin_lock_irqsave(&sfp->lock, flags);
> + func |= readl_relaxed(reg) & ~mask;
> + writel_relaxed(func, reg);
> + raw_spin_unlock_irqrestore(&sfp->lock, flags);
> +}
> +
> +static void jh7110_set_vin_group(struct starfive_pinctrl *sfp,
> + unsigned int pin)
> +{
> + const struct jh7110_vin_group_sel *gs = &jh7110_sys_vin_group_sel[pin];
> + unsigned long flags;
> + void __iomem *reg;
> + u32 mask;
> + u32 grp;
> +
> + if (!gs->offset)
> + return;
> +
> + reg = sfp->base + gs->offset;
> + grp = gs->group << gs->shift;
> + mask = 0x3U << gs->shift;
> +
> + raw_spin_lock_irqsave(&sfp->lock, flags);
> + grp |= readl_relaxed(reg) & ~mask;
> + writel_relaxed(grp, reg);
> + raw_spin_unlock_irqrestore(&sfp->lock, flags);
> +}
> +
> +static int jh7110_sys_set_one_pin_mux(struct starfive_pinctrl *sfp,
> + unsigned int pin,
> + unsigned int din, u32 dout,
> + u32 doen, u32 func)
> +{
> + if (pin < sfp->gc.ngpio && func == 0)
> + starfive_set_gpiomux(sfp, pin, din, dout, doen);
> +
> + jh7110_set_function(sfp, pin, func);
> +
> + if (pin < sfp->gc.ngpio && func == 2)
> + jh7110_set_vin_group(sfp, pin);
> +
> + return 0;
> +}
> +
> +static int jh7110_sys_get_padcfg_base(struct starfive_pinctrl *sfp,
> + unsigned int pin)
> +{
> + if (pin < PAD_GMAC1_MDC)
> + return SYS_GPO_PDA_0_74_CFG;
> + else if (pin > PAD_GMAC1_TXC && pin <= PAD_QSPI_DATA3)
> + return SYS_GPO_PDA_89_94_CFG;
> + else
> + return -1;
> +}
> +
> +static void jh7110_sys_irq_handler(struct irq_desc *desc)
> +{
> + struct starfive_pinctrl *sfp = starfive_from_irq_desc(desc);
> + struct irq_chip *chip = irq_desc_get_chip(desc);
> + unsigned long mis;
> + unsigned int pin;
> +
> + chained_irq_enter(chip, desc);
> +
> + mis = readl_relaxed(sfp->base + JH7110_SYS_GPIOMIS0);
> + for_each_set_bit(pin, &mis, 32)
> + generic_handle_domain_irq(sfp->gc.irq.domain, pin);
> +
> + mis = readl_relaxed(sfp->base + JH7110_SYS_GPIOMIS1);
> + for_each_set_bit(pin, &mis, 32)
> + generic_handle_domain_irq(sfp->gc.irq.domain, pin + 32);
> +
> + chained_irq_exit(chip, desc);
> +}
> +
> +static int jh7110_sys_init_hw(struct gpio_chip *gc)
> +{
> + struct starfive_pinctrl *sfp = container_of(gc,
> + struct starfive_pinctrl, gc);
> +
> + /* mask all GPIO interrupts */
> + writel(0U, sfp->base + JH7110_SYS_GPIOIE0);
> + writel(0U, sfp->base + JH7110_SYS_GPIOIE1);
> + /* clear edge interrupt flags */
> + writel(~0U, sfp->base + JH7110_SYS_GPIOIC0);
> + writel(~0U, sfp->base + JH7110_SYS_GPIOIC1);
> + /* enable GPIO interrupts */
> + writel(1U, sfp->base + JH7110_SYS_GPIOEN);
> + return 0;
> +}
> +
> +static const struct starfive_gpio_irq_reg jh7110_sys_irq_reg = {
> + .is_reg_base = JH7110_SYS_GPIOIS0,
> + .ic_reg_base = JH7110_SYS_GPIOIC0,
> + .ibe_reg_base = JH7110_SYS_GPIOIBE0,
> + .iev_reg_base = JH7110_SYS_GPIOIEV0,
> + .ie_reg_base = JH7110_SYS_GPIOIE0,
> + .ris_reg_base = JH7110_SYS_GPIORIS0,
> + .mis_reg_base = JH7110_SYS_GPIOMIS0,
> +};
> +
> +static const struct starfive_pinctrl_soc_info jh7110_sys_pinctrl_info = {
> + .pins = jh7110_sys_pins,
> + .npins = ARRAY_SIZE(jh7110_sys_pins),
> + .ngpios = JH7110_SYS_NGPIO,
> + .gc_base = JH7110_SYS_GC_BASE,
> + .flags = 1,
> + .dout_reg_base = JH7110_SYS_DOUT,
> + .dout_mask = GENMASK(6, 0),
> + .doen_reg_base = JH7110_SYS_DOEN,
> + .doen_mask = GENMASK(5, 0),
> + .gpi_reg_base = JH7110_SYS_GPI,
> + .gpi_mask = GENMASK(6, 0),
> + .gpioin_reg_base = JH7110_SYS_GPIOIN,
> + .irq_reg = &jh7110_sys_irq_reg,
> + .starfive_set_one_pin_mux = jh7110_sys_set_one_pin_mux,
> + .starfive_get_padcfg_base = jh7110_sys_get_padcfg_base,
> + .starfive_gpio_irq_handler = jh7110_sys_irq_handler,
> + .starfive_gpio_init_hw = jh7110_sys_init_hw,
> +};
> +
> +static const struct of_device_id jh7110_sys_pinctrl_of_match[] = {
> + {
> + .compatible = "starfive,jh7110-sys-pinctrl",
> + .data = &jh7110_sys_pinctrl_info,
> + },
> + { /* sentinel */ }
> +};
> +
> +static int jh7110_sys_pinctrl_probe(struct platform_device *pdev)
> +{
> + const struct starfive_pinctrl_soc_info *pinctrl_info;
> +
> + pinctrl_info = of_device_get_match_data(&pdev->dev);
> + if (!pinctrl_info)
> + return -ENODEV;
> +
> + return starfive_pinctrl_probe(pdev, pinctrl_info);
> +}

This wrapper is identical to the wrapper in the -aon driver, so you
can get the match data in the common probe function and drop these
wrappers.

> +
> +static struct platform_driver jh7110_sys_pinctrl_driver = {
> + .driver = {
> + .name = "starfive-jh7110-sys-pinctrl",
> + .of_match_table = of_match_ptr(jh7110_sys_pinctrl_of_match),

Please drop the of_match_ptr macro. This driver already depends on CONFIG_OF.

> + },
> + .probe = jh7110_sys_pinctrl_probe,
> +};
> +
> +static int __init jh7110_sys_pinctrl_init(void)
> +{
> + return platform_driver_register(&jh7110_sys_pinctrl_driver);
> +}
> +arch_initcall(jh7110_sys_pinctrl_init);

Why can't this just be module_platform_driver(jh7110_sys_pinctrl_driver); ?
That works for me, so what problems did you run into that means you
need to load this driver earlier?

> +MODULE_DESCRIPTION("Pinctrl driver for the StarFive JH7110 SoC sys controller");
> +MODULE_AUTHOR("Emil Renner Berthing <[email protected]>");
> +MODULE_AUTHOR("Jianlong Huang <[email protected]>");

I still think this driver should be fine as a module, but if not then
these lines should be removed.

> diff --git a/drivers/pinctrl/starfive/pinctrl-starfive.c b/drivers/pinctrl/starfive/pinctrl-starfive.c
> new file mode 100644
> index 000000000000..944caa599460
> --- /dev/null
> +++ b/drivers/pinctrl/starfive/pinctrl-starfive.c

Please call the file pinctrl-starfive-jh7110.c since it only applies
to the JH7110.

> @@ -0,0 +1,972 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Pinctrl / GPIO driver for StarFive JH7110 SoC
> + *
> + * Copyright (C) 2022 Emil Renner Berthing <[email protected]>
> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/io.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +#include <linux/spinlock.h>
> +
> +#include <linux/pinctrl/pinctrl.h>
> +#include <linux/pinctrl/pinmux.h>
> +
> +#include <dt-bindings/pinctrl/pinctrl-starfive-jh7110.h>
> +
> +#include "../core.h"
> +#include "../pinctrl-utils.h"
> +#include "../pinmux.h"
> +#include "../pinconf.h"
> +#include "pinctrl-starfive.h"
> +
> +/* pad control bits */
> +#define STARFIVE_PADCFG_POS BIT(7)
> +#define STARFIVE_PADCFG_SMT BIT(6)
> +#define STARFIVE_PADCFG_SLEW BIT(5)
> +#define STARFIVE_PADCFG_PD BIT(4)
> +#define STARFIVE_PADCFG_PU BIT(3)
> +#define STARFIVE_PADCFG_BIAS (STARFIVE_PADCFG_PD | STARFIVE_PADCFG_PU)
> +#define STARFIVE_PADCFG_DS_MASK GENMASK(2, 1)
> +#define STARFIVE_PADCFG_DS_2MA (0U << 1)
> +#define STARFIVE_PADCFG_DS_4MA BIT(1)
> +#define STARFIVE_PADCFG_DS_8MA (2U << 1)
> +#define STARFIVE_PADCFG_DS_12MA (3U << 1)
> +#define STARFIVE_PADCFG_IE BIT(0)

Above you use the JH7110_ prefix for macros, so please be consistent
and use the same prefix here.

> +#define GPIO_NUM_PER_WORD 32

I don't really see the value in this macro, but if you really want it
then it should be named something less generic.

> +/*
> + * The packed pinmux values from the device tree look like this:
> + *
> + * | 31 - 24 | 23 - 16 | 15 - 10 | 9 - 8 | 7 - 0 |
> + * | din | dout | doen | function | pin |
> + */
> +static unsigned int starfive_pinmux_din(u32 v)
> +{
> + return (v & GENMASK(31, 24)) >> 24;
> +}
> +
> +static u32 starfive_pinmux_dout(u32 v)
> +{
> + return (v & GENMASK(23, 16)) >> 16;
> +}
> +
> +static u32 starfive_pinmux_doen(u32 v)
> +{
> + return (v & GENMASK(15, 10)) >> 10;
> +}
> +
> +static u32 starfive_pinmux_function(u32 v)
> +{
> + return (v & GENMASK(9, 8)) >> 8;
> +}
> +
> +static unsigned int starfive_pinmux_pin(u32 v)
> +{
> + return v & GENMASK(7, 0);
> +}
> +
> +static struct starfive_pinctrl *starfive_from_irq_data(struct irq_data *d)
> +{
> + struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> +
> + return container_of(gc, struct starfive_pinctrl, gc);
> +}
> +
> +struct starfive_pinctrl *starfive_from_irq_desc(struct irq_desc *desc)
> +{
> + struct gpio_chip *gc = irq_desc_get_handler_data(desc);
> +
> + return container_of(gc, struct starfive_pinctrl, gc);
> +}
> +
> +#ifdef CONFIG_DEBUG_FS
> +static void starfive_pin_dbg_show(struct pinctrl_dev *pctldev,
> + struct seq_file *s, unsigned int pin)
> +{
> + struct starfive_pinctrl *sfp = pinctrl_dev_get_drvdata(pctldev);
> + const struct starfive_pinctrl_soc_info *info = sfp->info;
> +
> + seq_printf(s, "%s", dev_name(pctldev->dev));
> +
> + if (pin < sfp->gc.ngpio) {
> + unsigned int offset = 4 * (pin / 4);
> + unsigned int shift = 8 * (pin % 4);
> + u32 dout = readl_relaxed(sfp->base + info->dout_reg_base + offset);
> + u32 doen = readl_relaxed(sfp->base + info->doen_reg_base + offset);
> + u32 gpi = readl_relaxed(sfp->base + info->gpi_reg_base + offset);
> +
> + dout = (dout >> shift) & info->dout_mask;
> + doen = (doen >> shift) & info->doen_mask;
> + gpi = ((gpi >> shift) - 2) & info->gpi_mask;
> +
> + seq_printf(s, " dout=%u doen=%u din=%u", dout, doen, gpi);
> + }
> +}
> +#else
> +#define starfive_pin_dbg_show NULL
> +#endif
> +
> +static int starfive_dt_node_to_map(struct pinctrl_dev *pctldev,
> + struct device_node *np,
> + struct pinctrl_map **maps,
> + unsigned int *num_maps)
> +{
> + struct starfive_pinctrl *sfp = pinctrl_dev_get_drvdata(pctldev);
> + struct device *dev = sfp->gc.parent;
> + struct device_node *child;
> + struct pinctrl_map *map;
> + const char **pgnames;
> + const char *grpname;
> + int ngroups;
> + int nmaps;
> + int ret;
> +
> + ngroups = 0;
> + for_each_child_of_node(np, child)
> + ngroups += 1;
> + nmaps = 2 * ngroups;
> +
> + pgnames = devm_kcalloc(dev, ngroups, sizeof(*pgnames), GFP_KERNEL);
> + if (!pgnames)
> + return -ENOMEM;
> +
> + map = kcalloc(nmaps, sizeof(*map), GFP_KERNEL);
> + if (!map)
> + return -ENOMEM;
> +
> + nmaps = 0;
> + ngroups = 0;
> + mutex_lock(&sfp->mutex);
> + for_each_child_of_node(np, child) {
> + int npins = of_property_count_u32_elems(child, "pinmux");
> + int *pins;
> + u32 *pinmux;
> + int i;
> +
> + if (npins < 1) {
> + dev_err(dev,
> + "invalid pinctrl group %pOFn.%pOFn: pinmux not set\n",
> + np, child);
> + ret = -EINVAL;
> + goto put_child;
> + }
> +
> + grpname = devm_kasprintf(dev, GFP_KERNEL, "%pOFn.%pOFn", np, child);
> + if (!grpname) {
> + ret = -ENOMEM;
> + goto put_child;
> + }
> +
> + pgnames[ngroups++] = grpname;
> +
> + pins = devm_kcalloc(dev, npins, sizeof(*pins), GFP_KERNEL);
> + if (!pins) {
> + ret = -ENOMEM;
> + goto put_child;
> + }
> +
> + pinmux = devm_kcalloc(dev, npins, sizeof(*pinmux), GFP_KERNEL);
> + if (!pinmux) {
> + ret = -ENOMEM;
> + goto put_child;
> + }
> +
> + ret = of_property_read_u32_array(child, "pinmux", pinmux, npins);
> + if (ret)
> + goto put_child;
> +
> + for (i = 0; i < npins; i++)
> + pins[i] = starfive_pinmux_pin(pinmux[i]);
> +
> + map[nmaps].type = PIN_MAP_TYPE_MUX_GROUP;
> + map[nmaps].data.mux.function = np->name;
> + map[nmaps].data.mux.group = grpname;
> + nmaps += 1;
> +
> + ret = pinctrl_generic_add_group(pctldev, grpname,
> + pins, npins, pinmux);
> + if (ret < 0) {
> + dev_err(dev, "error adding group %s: %d\n", grpname, ret);
> + goto put_child;
> + }
> +
> + ret = pinconf_generic_parse_dt_config(child, pctldev,
> + &map[nmaps].data.configs.configs,
> + &map[nmaps].data.configs.num_configs);
> + if (ret) {
> + dev_err(dev, "error parsing pin config of group %s: %d\n",
> + grpname, ret);
> + goto put_child;
> + }
> +
> + /* don't create a map if there are no pinconf settings */
> + if (map[nmaps].data.configs.num_configs == 0)
> + continue;
> +
> + map[nmaps].type = PIN_MAP_TYPE_CONFIGS_GROUP;
> + map[nmaps].data.configs.group_or_pin = grpname;
> + nmaps += 1;
> + }
> +
> + ret = pinmux_generic_add_function(pctldev, np->name,
> + pgnames, ngroups, NULL);
> + if (ret < 0) {
> + dev_err(dev, "error adding function %s: %d\n", np->name, ret);
> + goto free_map;
> + }
> + mutex_unlock(&sfp->mutex);
> +
> + *maps = map;
> + *num_maps = nmaps;
> + return 0;
> +
> +put_child:
> + of_node_put(child);
> +free_map:
> + pinctrl_utils_free_map(pctldev, map, nmaps);
> + mutex_unlock(&sfp->mutex);
> + return ret;
> +}
> +
> +static const struct pinctrl_ops starfive_pinctrl_ops = {
> + .get_groups_count = pinctrl_generic_get_group_count,
> + .get_group_name = pinctrl_generic_get_group_name,
> + .get_group_pins = pinctrl_generic_get_group_pins,
> + .pin_dbg_show = starfive_pin_dbg_show,
> + .dt_node_to_map = starfive_dt_node_to_map,
> + .dt_free_map = pinctrl_utils_free_map,
> +};

Again this file only applies to the JH7110, so please be consistent
and prefix everything jh7110_ and not the generic starfive_ which
gives the impression works on all StarFive SoCs.

> +
> +void starfive_set_gpiomux(struct starfive_pinctrl *sfp, unsigned int pin,
> + unsigned int din, u32 dout, u32 doen)
> +{
> + const struct starfive_pinctrl_soc_info *info = sfp->info;
> +
> + unsigned int offset = 4 * (pin / 4);
> + unsigned int shift = 8 * (pin % 4);
> + u32 dout_mask = info->dout_mask << shift;
> + u32 done_mask = info->doen_mask << shift;
> + u32 ival, imask;
> + void __iomem *reg_dout;
> + void __iomem *reg_doen;
> + void __iomem *reg_din;
> + unsigned long flags;
> +
> + reg_dout = sfp->base + info->dout_reg_base + offset;
> + reg_doen = sfp->base + info->doen_reg_base + offset;
> + dout <<= shift;
> + doen <<= shift;
> + if (din != GPI_NONE) {
> + unsigned int ioffset = 4 * (din / 4);
> + unsigned int ishift = 8 * (din % 4);
> +
> + reg_din = sfp->base + info->gpi_reg_base + ioffset;
> + ival = (pin + 2) << ishift;
> + imask = info->gpi_mask << ishift;
> + } else {
> + reg_din = NULL;
> + }
> +
> + raw_spin_lock_irqsave(&sfp->lock, flags);
> + dout |= readl_relaxed(reg_dout) & ~dout_mask;
> + writel_relaxed(dout, reg_dout);
> + doen |= readl_relaxed(reg_doen) & ~done_mask;
> + writel_relaxed(doen, reg_doen);
> + if (reg_din) {
> + ival |= readl_relaxed(reg_din) & ~imask;
> + writel_relaxed(ival, reg_din);
> + }
> + raw_spin_unlock_irqrestore(&sfp->lock, flags);
> +}
> +
> +static int starfive_set_mux(struct pinctrl_dev *pctldev,
> + unsigned int fsel, unsigned int gsel)
> +{
> + struct starfive_pinctrl *sfp = pinctrl_dev_get_drvdata(pctldev);
> + const struct starfive_pinctrl_soc_info *info = sfp->info;
> + const struct group_desc *group;
> + const u32 *pinmux;
> + unsigned int i;
> +
> + group = pinctrl_generic_get_group(pctldev, gsel);
> + if (!group)
> + return -EINVAL;
> +
> + pinmux = group->data;
> + for (i = 0; i < group->num_pins; i++) {
> + u32 v = pinmux[i];
> +
> + if (info->starfive_set_one_pin_mux)
> + info->starfive_set_one_pin_mux(sfp,
> + starfive_pinmux_pin(v),
> + starfive_pinmux_din(v),
> + starfive_pinmux_dout(v),
> + starfive_pinmux_doen(v),
> + starfive_pinmux_function(v));
> + }
> +
> + return 0;
> +}
> +
> +static const struct pinmux_ops starfive_pinmux_ops = {
> + .get_functions_count = pinmux_generic_get_function_count,
> + .get_function_name = pinmux_generic_get_function_name,
> + .get_function_groups = pinmux_generic_get_function_groups,
> + .set_mux = starfive_set_mux,
> + .strict = true,
> +};
> +
> +static const u8 starfive_drive_strength_mA[4] = { 2, 4, 8, 12 };
> +
> +static u32 starfive_padcfg_ds_to_mA(u32 padcfg)
> +{
> + return starfive_drive_strength_mA[(padcfg >> 1) & 3U];
> +}
> +
> +static u32 starfive_padcfg_ds_from_mA(u32 v)
> +{
> + int i;
> +
> + for (i = 0; i < 3; i++) {
> + if (v <= starfive_drive_strength_mA[i])
> + break;
> + }
> + return i << 1;
> +}
> +
> +static void starfive_padcfg_rmw(struct starfive_pinctrl *sfp,
> + unsigned int pin, u32 mask, u32 value)
> +{
> + const struct starfive_pinctrl_soc_info *info = sfp->info;
> + void __iomem *reg;
> + unsigned long flags;
> + int padcfg_base;
> +
> + if (!info->starfive_get_padcfg_base)
> + return;
> +
> + padcfg_base = info->starfive_get_padcfg_base(sfp, pin);
> + if (padcfg_base < 0)
> + return;
> +
> + reg = sfp->base + padcfg_base + 4 * pin;
> + value &= mask;
> +
> + raw_spin_lock_irqsave(&sfp->lock, flags);
> + value |= readl_relaxed(reg) & ~mask;
> + writel_relaxed(value, reg);
> + raw_spin_unlock_irqrestore(&sfp->lock, flags);
> +}
> +
> +static int starfive_pinconf_get(struct pinctrl_dev *pctldev,
> + unsigned int pin, unsigned long *config)
> +{
> + struct starfive_pinctrl *sfp = pinctrl_dev_get_drvdata(pctldev);
> + const struct starfive_pinctrl_soc_info *info = sfp->info;
> + int param = pinconf_to_config_param(*config);
> + u32 padcfg, arg;
> + bool enabled;
> + int padcfg_base;
> +
> + if (!info->starfive_get_padcfg_base)
> + return 0;
> +
> + padcfg_base = info->starfive_get_padcfg_base(sfp, pin);
> + if (padcfg_base < 0)
> + return 0;
> +
> + padcfg = readl_relaxed(sfp->base + padcfg_base + 4 * pin);
> + switch (param) {
> + case PIN_CONFIG_BIAS_DISABLE:
> + enabled = !(padcfg & STARFIVE_PADCFG_BIAS);
> + arg = 0;
> + break;
> + case PIN_CONFIG_BIAS_PULL_DOWN:
> + enabled = padcfg & STARFIVE_PADCFG_PD;
> + arg = 1;
> + break;
> + case PIN_CONFIG_BIAS_PULL_UP:
> + enabled = padcfg & STARFIVE_PADCFG_PU;
> + arg = 1;
> + break;
> + case PIN_CONFIG_DRIVE_STRENGTH:
> + enabled = true;
> + arg = starfive_padcfg_ds_to_mA(padcfg);
> + break;
> + case PIN_CONFIG_INPUT_ENABLE:
> + enabled = padcfg & STARFIVE_PADCFG_IE;
> + arg = enabled;
> + break;
> + case PIN_CONFIG_INPUT_SCHMITT_ENABLE:
> + enabled = padcfg & STARFIVE_PADCFG_SMT;
> + arg = enabled;
> + break;
> + case PIN_CONFIG_SLEW_RATE:
> + enabled = true;
> + arg = !!(padcfg & STARFIVE_PADCFG_SLEW);
> + break;
> + default:
> + return -ENOTSUPP;
> + }
> +
> + *config = pinconf_to_config_packed(param, arg);
> + return enabled ? 0 : -EINVAL;
> +}
> +
> +static int starfive_pinconf_group_get(struct pinctrl_dev *pctldev,
> + unsigned int gsel,
> + unsigned long *config)
> +{
> + const struct group_desc *group;
> +
> + group = pinctrl_generic_get_group(pctldev, gsel);
> + if (!group)
> + return -EINVAL;
> +
> + return starfive_pinconf_get(pctldev, group->pins[0], config);
> +}
> +
> +static int starfive_pinconf_group_set(struct pinctrl_dev *pctldev,
> + unsigned int gsel,
> + unsigned long *configs,
> + unsigned int num_configs)
> +{
> + struct starfive_pinctrl *sfp = pinctrl_dev_get_drvdata(pctldev);
> + const struct group_desc *group;
> + u16 mask, value;
> + int i;
> +
> + group = pinctrl_generic_get_group(pctldev, gsel);
> + if (!group)
> + return -EINVAL;
> +
> + mask = 0;
> + value = 0;
> + for (i = 0; i < num_configs; i++) {
> + int param = pinconf_to_config_param(configs[i]);
> + u32 arg = pinconf_to_config_argument(configs[i]);
> +
> + switch (param) {
> + case PIN_CONFIG_BIAS_DISABLE:
> + mask |= STARFIVE_PADCFG_BIAS;
> + value &= ~STARFIVE_PADCFG_BIAS;
> + break;
> + case PIN_CONFIG_BIAS_PULL_DOWN:
> + if (arg == 0)
> + return -ENOTSUPP;
> + mask |= STARFIVE_PADCFG_BIAS;
> + value = (value & ~STARFIVE_PADCFG_BIAS) | STARFIVE_PADCFG_PD;
> + break;
> + case PIN_CONFIG_BIAS_PULL_UP:
> + if (arg == 0)
> + return -ENOTSUPP;
> + mask |= STARFIVE_PADCFG_BIAS;
> + value = (value & ~STARFIVE_PADCFG_BIAS) | STARFIVE_PADCFG_PU;
> + break;
> + case PIN_CONFIG_DRIVE_STRENGTH:
> + mask |= STARFIVE_PADCFG_DS_MASK;
> + value = (value & ~STARFIVE_PADCFG_DS_MASK) |
> + starfive_padcfg_ds_from_mA(arg);
> + break;
> + case PIN_CONFIG_INPUT_ENABLE:
> + mask |= STARFIVE_PADCFG_IE;
> + if (arg)
> + value |= STARFIVE_PADCFG_IE;
> + else
> + value &= ~STARFIVE_PADCFG_IE;
> + break;
> + case PIN_CONFIG_INPUT_SCHMITT_ENABLE:
> + mask |= STARFIVE_PADCFG_SMT;
> + if (arg)
> + value |= STARFIVE_PADCFG_SMT;
> + else
> + value &= ~STARFIVE_PADCFG_SMT;
> + break;
> + case PIN_CONFIG_SLEW_RATE:
> + mask |= STARFIVE_PADCFG_SLEW;
> + if (arg)
> + value |= STARFIVE_PADCFG_SLEW;
> + else
> + value &= ~STARFIVE_PADCFG_SLEW;
> + break;
> + default:
> + return -ENOTSUPP;
> + }
> + }
> +
> + for (i = 0; i < group->num_pins; i++)
> + starfive_padcfg_rmw(sfp, group->pins[i], mask, value);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_DEBUG_FS
> +static void starfive_pinconf_dbg_show(struct pinctrl_dev *pctldev,
> + struct seq_file *s, unsigned int pin)
> +{
> + struct starfive_pinctrl *sfp = pinctrl_dev_get_drvdata(pctldev);
> + const struct starfive_pinctrl_soc_info *info = sfp->info;
> + u32 value;
> + int padcfg_base;
> +
> + if (!info->starfive_get_padcfg_base)
> + return;
> +
> + padcfg_base = info->starfive_get_padcfg_base(sfp, pin);
> + if (padcfg_base < 0)
> + return;
> +
> + value = readl_relaxed(sfp->base + padcfg_base + 4 * pin);
> + seq_printf(s, " (0x%02x)", value);
> +}
> +#else
> +#define starfive_pinconf_dbg_show NULL
> +#endif
> +
> +static const struct pinconf_ops starfive_pinconf_ops = {
> + .pin_config_get = starfive_pinconf_get,
> + .pin_config_group_get = starfive_pinconf_group_get,
> + .pin_config_group_set = starfive_pinconf_group_set,
> + .pin_config_dbg_show = starfive_pinconf_dbg_show,
> + .is_generic = true,
> +};
> +
> +static int starfive_gpio_request(struct gpio_chip *gc, unsigned int gpio)
> +{
> + return pinctrl_gpio_request(gc->base + gpio);
> +}
> +
> +static void starfive_gpio_free(struct gpio_chip *gc, unsigned int gpio)
> +{
> + pinctrl_gpio_free(gc->base + gpio);
> +}
> +
> +static int starfive_gpio_get_direction(struct gpio_chip *gc,
> + unsigned int gpio)
> +{
> + struct starfive_pinctrl *sfp = container_of(gc,
> + struct starfive_pinctrl, gc);
> + const struct starfive_pinctrl_soc_info *info = sfp->info;
> + unsigned int offset = 4 * (gpio / 4);
> + unsigned int shift = 8 * (gpio % 4);
> + u32 doen = readl_relaxed(sfp->base + info->doen_reg_base + offset);
> +
> + doen = (doen >> shift) & info->doen_mask;
> +
> + return doen == GPOEN_ENABLE ?
> + GPIO_LINE_DIRECTION_OUT : GPIO_LINE_DIRECTION_IN;
> +}
> +
> +static int starfive_gpio_direction_input(struct gpio_chip *gc,
> + unsigned int gpio)
> +{
> + struct starfive_pinctrl *sfp = container_of(gc,
> + struct starfive_pinctrl, gc);
> + const struct starfive_pinctrl_soc_info *info = sfp->info;
> +
> + /* enable input and schmitt trigger */
> + starfive_padcfg_rmw(sfp, gpio,
> + STARFIVE_PADCFG_IE | STARFIVE_PADCFG_SMT,
> + STARFIVE_PADCFG_IE | STARFIVE_PADCFG_SMT);
> +
> + if (info->starfive_set_one_pin_mux)
> + info->starfive_set_one_pin_mux(sfp, gpio,
> + GPI_NONE, GPOUT_LOW, GPOEN_DISABLE, 0);
> +
> + return 0;
> +}
> +
> +static int starfive_gpio_direction_output(struct gpio_chip *gc,
> + unsigned int gpio, int value)
> +{
> + struct starfive_pinctrl *sfp = container_of(gc,
> + struct starfive_pinctrl, gc);
> + const struct starfive_pinctrl_soc_info *info = sfp->info;
> +
> + if (info->starfive_set_one_pin_mux)
> + info->starfive_set_one_pin_mux(sfp, gpio,
> + GPI_NONE, value ? GPOUT_HIGH : GPOUT_LOW,
> + GPOEN_ENABLE, 0);
> +
> + /* disable input, schmitt trigger and bias */
> + starfive_padcfg_rmw(sfp, gpio,
> + STARFIVE_PADCFG_IE | STARFIVE_PADCFG_SMT
> + | STARFIVE_PADCFG_BIAS, 0);
> + return 0;
> +}
> +
> +static int starfive_gpio_get(struct gpio_chip *gc, unsigned int gpio)
> +{
> + struct starfive_pinctrl *sfp = container_of(gc,
> + struct starfive_pinctrl, gc);
> + const struct starfive_pinctrl_soc_info *info = sfp->info;
> + void __iomem *reg = sfp->base + info->gpioin_reg_base
> + + 4 * (gpio / GPIO_NUM_PER_WORD);
> +
> + return !!(readl_relaxed(reg) & BIT(gpio % GPIO_NUM_PER_WORD));
> +}
> +
> +static void starfive_gpio_set(struct gpio_chip *gc,
> + unsigned int gpio, int value)
> +{
> + struct starfive_pinctrl *sfp = container_of(gc,
> + struct starfive_pinctrl, gc);
> + const struct starfive_pinctrl_soc_info *info = sfp->info;
> + unsigned int offset = 4 * (gpio / 4);
> + unsigned int shift = 8 * (gpio % 4);
> + void __iomem *reg_dout = sfp->base + info->dout_reg_base + offset;
> + u32 dout = (value ? GPOUT_HIGH : GPOUT_LOW) << shift;
> + u32 mask = info->dout_mask << shift;
> + unsigned long flags;
> +
> + raw_spin_lock_irqsave(&sfp->lock, flags);
> + dout |= readl_relaxed(reg_dout) & ~mask;
> + writel_relaxed(dout, reg_dout);
> + raw_spin_unlock_irqrestore(&sfp->lock, flags);
> +}
> +
> +static int starfive_gpio_set_config(struct gpio_chip *gc,
> + unsigned int gpio, unsigned long config)
> +{
> + struct starfive_pinctrl *sfp = container_of(gc,
> + struct starfive_pinctrl, gc);
> + u32 arg = pinconf_to_config_argument(config);
> + u32 value;
> + u32 mask;
> +
> + switch (pinconf_to_config_param(config)) {
> + case PIN_CONFIG_BIAS_DISABLE:
> + mask = STARFIVE_PADCFG_BIAS;
> + value = 0;
> + break;
> + case PIN_CONFIG_BIAS_PULL_DOWN:
> + if (arg == 0)
> + return -ENOTSUPP;
> + mask = STARFIVE_PADCFG_BIAS;
> + value = STARFIVE_PADCFG_PD;
> + break;
> + case PIN_CONFIG_BIAS_PULL_UP:
> + if (arg == 0)
> + return -ENOTSUPP;
> + mask = STARFIVE_PADCFG_BIAS;
> + value = STARFIVE_PADCFG_PU;
> + break;
> + case PIN_CONFIG_DRIVE_PUSH_PULL:
> + return 0;
> + case PIN_CONFIG_INPUT_ENABLE:
> + mask = STARFIVE_PADCFG_IE;
> + value = arg ? STARFIVE_PADCFG_IE : 0;
> + break;
> + case PIN_CONFIG_INPUT_SCHMITT_ENABLE:
> + mask = STARFIVE_PADCFG_SMT;
> + value = arg ? STARFIVE_PADCFG_SMT : 0;
> + break;
> + default:
> + return -ENOTSUPP;
> + }
> +
> + starfive_padcfg_rmw(sfp, gpio, mask, value);
> + return 0;
> +}
> +
> +static int starfive_gpio_add_pin_ranges(struct gpio_chip *gc)
> +{
> + struct starfive_pinctrl *sfp = container_of(gc,
> + struct starfive_pinctrl, gc);
> +
> + sfp->gpios.name = sfp->gc.label;
> + sfp->gpios.base = sfp->gc.base;
> + sfp->gpios.pin_base = 0;
> + sfp->gpios.npins = sfp->gc.ngpio;
> + sfp->gpios.gc = &sfp->gc;
> + pinctrl_add_gpio_range(sfp->pctl, &sfp->gpios);
> + return 0;
> +}
> +
> +static void starfive_irq_ack(struct irq_data *d)
> +{
> + struct starfive_pinctrl *sfp = starfive_from_irq_data(d);
> + const struct starfive_gpio_irq_reg *irq_reg = sfp->info->irq_reg;
> + irq_hw_number_t gpio = irqd_to_hwirq(d);
> + void __iomem *ic = sfp->base + irq_reg->ic_reg_base
> + + 4 * (gpio / GPIO_NUM_PER_WORD);
> + u32 mask = BIT(gpio % GPIO_NUM_PER_WORD);
> + unsigned long flags;
> + u32 value;
> +
> + raw_spin_lock_irqsave(&sfp->lock, flags);
> + value = readl_relaxed(ic) & ~mask;
> + writel_relaxed(value, ic);
> + writel_relaxed(value | mask, ic);
> + raw_spin_unlock_irqrestore(&sfp->lock, flags);
> +}
> +
> +static void starfive_irq_mask(struct irq_data *d)
> +{
> + struct starfive_pinctrl *sfp = starfive_from_irq_data(d);
> + const struct starfive_gpio_irq_reg *irq_reg = sfp->info->irq_reg;
> + irq_hw_number_t gpio = irqd_to_hwirq(d);
> + void __iomem *ie = sfp->base + irq_reg->ie_reg_base
> + + 4 * (gpio / GPIO_NUM_PER_WORD);
> + u32 mask = BIT(gpio % GPIO_NUM_PER_WORD);
> + unsigned long flags;
> + u32 value;
> +
> + raw_spin_lock_irqsave(&sfp->lock, flags);
> + value = readl_relaxed(ie) & ~mask;
> + writel_relaxed(value, ie);
> + raw_spin_unlock_irqrestore(&sfp->lock, flags);
> +
> + gpiochip_disable_irq(&sfp->gc, d->hwirq);
> +}
> +
> +static void starfive_irq_mask_ack(struct irq_data *d)
> +{
> + struct starfive_pinctrl *sfp = starfive_from_irq_data(d);
> + const struct starfive_gpio_irq_reg *irq_reg = sfp->info->irq_reg;
> + irq_hw_number_t gpio = irqd_to_hwirq(d);
> + void __iomem *ie = sfp->base + irq_reg->ie_reg_base
> + + 4 * (gpio / GPIO_NUM_PER_WORD);
> + void __iomem *ic = sfp->base + irq_reg->ic_reg_base
> + + 4 * (gpio / GPIO_NUM_PER_WORD);
> + u32 mask = BIT(gpio % GPIO_NUM_PER_WORD);
> + unsigned long flags;
> + u32 value;
> +
> + raw_spin_lock_irqsave(&sfp->lock, flags);
> + value = readl_relaxed(ie) & ~mask;
> + writel_relaxed(value, ie);
> +
> + value = readl_relaxed(ic) & ~mask;
> + writel_relaxed(value, ic);
> + writel_relaxed(value | mask, ic);
> + raw_spin_unlock_irqrestore(&sfp->lock, flags);
> +}
> +
> +static void starfive_irq_unmask(struct irq_data *d)
> +{
> + struct starfive_pinctrl *sfp = starfive_from_irq_data(d);
> + const struct starfive_gpio_irq_reg *irq_reg = sfp->info->irq_reg;
> + irq_hw_number_t gpio = irqd_to_hwirq(d);
> + void __iomem *ie = sfp->base + irq_reg->ie_reg_base
> + + 4 * (gpio / GPIO_NUM_PER_WORD);
> + u32 mask = BIT(gpio % GPIO_NUM_PER_WORD);
> + unsigned long flags;
> + u32 value;
> +
> + gpiochip_enable_irq(&sfp->gc, d->hwirq);
> +
> + raw_spin_lock_irqsave(&sfp->lock, flags);
> + value = readl_relaxed(ie) | mask;
> + writel_relaxed(value, ie);
> + raw_spin_unlock_irqrestore(&sfp->lock, flags);
> +}
> +
> +static int starfive_irq_set_type(struct irq_data *d, unsigned int trigger)
> +{
> + struct starfive_pinctrl *sfp = starfive_from_irq_data(d);
> + const struct starfive_gpio_irq_reg *irq_reg = sfp->info->irq_reg;
> + irq_hw_number_t gpio = irqd_to_hwirq(d);
> + void __iomem *base = sfp->base + 4 * (gpio / GPIO_NUM_PER_WORD);
> + u32 mask = BIT(gpio % GPIO_NUM_PER_WORD);
> + u32 irq_type, edge_both, polarity;
> + unsigned long flags;
> +
> + switch (trigger) {
> + case IRQ_TYPE_EDGE_RISING:
> + irq_type = mask; /* 1: edge triggered */
> + edge_both = 0; /* 0: single edge */
> + polarity = mask; /* 1: rising edge */
> + break;
> + case IRQ_TYPE_EDGE_FALLING:
> + irq_type = mask; /* 1: edge triggered */
> + edge_both = 0; /* 0: single edge */
> + polarity = 0; /* 0: falling edge */
> + break;
> + case IRQ_TYPE_EDGE_BOTH:
> + irq_type = mask; /* 1: edge triggered */
> + edge_both = mask; /* 1: both edges */
> + polarity = 0; /* 0: ignored */
> + break;
> + case IRQ_TYPE_LEVEL_HIGH:
> + irq_type = 0; /* 0: level triggered */
> + edge_both = 0; /* 0: ignored */
> + polarity = mask; /* 1: high level */
> + break;
> + case IRQ_TYPE_LEVEL_LOW:
> + irq_type = 0; /* 0: level triggered */
> + edge_both = 0; /* 0: ignored */
> + polarity = 0; /* 0: low level */
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + if (trigger & IRQ_TYPE_EDGE_BOTH)
> + irq_set_handler_locked(d, handle_edge_irq);
> + else
> + irq_set_handler_locked(d, handle_level_irq);
> +
> + raw_spin_lock_irqsave(&sfp->lock, flags);
> + irq_type |= readl_relaxed(base + irq_reg->is_reg_base) & ~mask;
> + writel_relaxed(irq_type, base + irq_reg->is_reg_base);
> +
> + edge_both |= readl_relaxed(base + irq_reg->ibe_reg_base) & ~mask;
> + writel_relaxed(edge_both, base + irq_reg->ibe_reg_base);
> +
> + polarity |= readl_relaxed(base + irq_reg->iev_reg_base) & ~mask;
> + writel_relaxed(polarity, base + irq_reg->iev_reg_base);
> + raw_spin_unlock_irqrestore(&sfp->lock, flags);
> + return 0;
> +}
> +
> +static struct irq_chip starfive_irq_chip = {
> + .irq_ack = starfive_irq_ack,
> + .irq_mask = starfive_irq_mask,
> + .irq_mask_ack = starfive_irq_mask_ack,
> + .irq_unmask = starfive_irq_unmask,
> + .irq_set_type = starfive_irq_set_type,
> + .flags = IRQCHIP_IMMUTABLE | IRQCHIP_SET_TYPE_MASKED,
> + GPIOCHIP_IRQ_RESOURCE_HELPERS,
> +};
> +
> +static void starfive_disable_clock(void *data)
> +{
> + clk_disable_unprepare(data);
> +}
> +
> +int starfive_pinctrl_probe(struct platform_device *pdev,
> + const struct starfive_pinctrl_soc_info *info)
> +{
> + struct device *dev = &pdev->dev;
> + struct starfive_pinctrl *sfp;
> + struct pinctrl_desc *starfive_pinctrl_desc;
> + struct reset_control *rst;
> + struct clk *clk;
> + int ret;
> +
> + if (!info || !info->pins || !info->npins) {
> + dev_err(dev, "wrong pinctrl info\n");
> + return -EINVAL;
> + }
> +
> + sfp = devm_kzalloc(dev, sizeof(*sfp), GFP_KERNEL);
> + if (!sfp)
> + return -ENOMEM;
> +
> + sfp->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(sfp->base))
> + return PTR_ERR(sfp->base);
> +
> + clk = devm_clk_get_optional(dev, NULL);
> + if (IS_ERR(clk))
> + return dev_err_probe(dev, PTR_ERR(clk), "could not get clock\n");
> +
> + rst = devm_reset_control_get_exclusive(dev, NULL);
> + if (IS_ERR(rst))
> + return dev_err_probe(dev, PTR_ERR(rst), "could not get reset\n");
> +
> + /*
> + * we don't want to assert reset and risk undoing pin muxing for the
> + * early boot serial console, but let's make sure the reset line is
> + * deasserted in case someone runs a really minimal bootloader.
> + */
> + ret = reset_control_deassert(rst);
> + if (ret)
> + return dev_err_probe(dev, ret, "could not deassert reset\n");
> +
> + if (clk) {
> + ret = clk_prepare_enable(clk);
> + if (ret)
> + return dev_err_probe(dev, ret, "could not enable clock\n");
> +
> + ret = devm_add_action_or_reset(dev, starfive_disable_clock, clk);
> + if (ret)
> + return ret;
> + }
> +
> + starfive_pinctrl_desc = devm_kzalloc(&pdev->dev,
> + sizeof(*starfive_pinctrl_desc),
> + GFP_KERNEL);
> + if (!starfive_pinctrl_desc)
> + return -ENOMEM;
> +
> + starfive_pinctrl_desc->name = dev_name(dev);
> + starfive_pinctrl_desc->pins = info->pins;
> + starfive_pinctrl_desc->npins = info->npins;
> + starfive_pinctrl_desc->pctlops = &starfive_pinctrl_ops;
> + starfive_pinctrl_desc->pmxops = &starfive_pinmux_ops;
> + starfive_pinctrl_desc->confops = &starfive_pinconf_ops;
> + starfive_pinctrl_desc->owner = THIS_MODULE;
> +
> + sfp->info = info;
> + sfp->dev = dev;
> + platform_set_drvdata(pdev, sfp);
> + sfp->gc.parent = dev;
> + raw_spin_lock_init(&sfp->lock);
> + mutex_init(&sfp->mutex);
> +
> + ret = devm_pinctrl_register_and_init(dev,
> + starfive_pinctrl_desc,
> + sfp, &sfp->pctl);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "could not register pinctrl driver\n");
> +
> + sfp->gc.label = dev_name(dev);
> + sfp->gc.owner = THIS_MODULE;
> + sfp->gc.request = starfive_gpio_request;
> + sfp->gc.free = starfive_gpio_free;
> + sfp->gc.get_direction = starfive_gpio_get_direction;
> + sfp->gc.direction_input = starfive_gpio_direction_input;
> + sfp->gc.direction_output = starfive_gpio_direction_output;
> + sfp->gc.get = starfive_gpio_get;
> + sfp->gc.set = starfive_gpio_set;
> + sfp->gc.set_config = starfive_gpio_set_config;
> + sfp->gc.add_pin_ranges = starfive_gpio_add_pin_ranges;
> + sfp->gc.base = info->gc_base;
> + sfp->gc.ngpio = info->ngpios;
> +
> + starfive_irq_chip.name = sfp->gc.label;
> + gpio_irq_chip_set_chip(&sfp->gc.irq, &starfive_irq_chip);
> + sfp->gc.irq.parent_handler = info->starfive_gpio_irq_handler;
> + sfp->gc.irq.num_parents = 1;
> + sfp->gc.irq.parents = devm_kcalloc(dev, sfp->gc.irq.num_parents,
> + sizeof(*sfp->gc.irq.parents),
> + GFP_KERNEL);
> + if (!sfp->gc.irq.parents)
> + return -ENOMEM;
> + sfp->gc.irq.default_type = IRQ_TYPE_NONE;
> + sfp->gc.irq.handler = handle_bad_irq;
> + sfp->gc.irq.init_hw = info->starfive_gpio_init_hw;
> +
> + ret = platform_get_irq(pdev, 0);
> + if (ret < 0)
> + return ret;
> + sfp->gc.irq.parents[0] = ret;
> +
> + ret = devm_gpiochip_add_data(dev, &sfp->gc, sfp);
> + if (ret)
> + return dev_err_probe(dev, ret, "could not register gpiochip\n");
> +
> + irq_domain_set_pm_device(sfp->gc.irq.domain, dev);
> +
> + dev_info(dev, "StarFive GPIO chip registered %d GPIOs\n", sfp->gc.ngpio);
> +
> + return pinctrl_enable(sfp->pctl);
> +}
> +
> +MODULE_DESCRIPTION("Pinctrl driver for the StarFive JH7110 SoC");
> +MODULE_AUTHOR("Emil Renner Berthing <[email protected]>");
> +MODULE_AUTHOR("Jianlong Huang <[email protected]>");
> diff --git a/drivers/pinctrl/starfive/pinctrl-starfive.h b/drivers/pinctrl/starfive/pinctrl-starfive.h
> new file mode 100644
> index 000000000000..54ffdb6412f1
> --- /dev/null
> +++ b/drivers/pinctrl/starfive/pinctrl-starfive.h
> @@ -0,0 +1,72 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Pinctrl / GPIO driver for StarFive JH7110 SoC
> + *
> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
> + */
> +
> +#ifndef __DRIVERS_PINCTRL_STARFIVE_H__
> +#define __DRIVERS_PINCTRL_STARFIVE_H__
> +
> +#include <linux/pinctrl/pinconf-generic.h>
> +#include <linux/pinctrl/pinmux.h>
> +
> +struct starfive_pinctrl {
> + struct device *dev;
> + struct gpio_chip gc;
> + struct pinctrl_gpio_range gpios;
> + raw_spinlock_t lock;
> + void __iomem *base;
> + struct pinctrl_dev *pctl;
> + /* register read/write mutex */
> + struct mutex mutex;
> + const struct starfive_pinctrl_soc_info *info;
> +};
> +
> +struct starfive_gpio_irq_reg {
> + unsigned int is_reg_base;
> + unsigned int ic_reg_base;
> + unsigned int ibe_reg_base;
> + unsigned int iev_reg_base;
> + unsigned int ie_reg_base;
> + unsigned int ris_reg_base;
> + unsigned int mis_reg_base;
> +};
> +
> +struct starfive_pinctrl_soc_info {
> + const struct pinctrl_pin_desc *pins;
> + unsigned int npins;
> + unsigned int ngpios;
> + unsigned int gc_base;
> + unsigned int flags;

The flag member seems unused. Please drop.

> + /* gpio dout/doen/din/gpioinput register */
> + unsigned int dout_reg_base;
> + unsigned int dout_mask;
> + unsigned int doen_reg_base;
> + unsigned int doen_mask;
> + unsigned int gpi_reg_base;
> + unsigned int gpi_mask;
> + unsigned int gpioin_reg_base;
> +
> + const struct starfive_gpio_irq_reg *irq_reg;
> +
> + /* generic pinmux */
> + int (*starfive_set_one_pin_mux)(struct starfive_pinctrl *sfp,
> + unsigned int pin,
> + unsigned int din, u32 dout,
> + u32 doen, u32 func);
> + /* gpio chip */
> + int (*starfive_get_padcfg_base)(struct starfive_pinctrl *sfp,
> + unsigned int pin);
> + void (*starfive_gpio_irq_handler)(struct irq_desc *desc);
> + int (*starfive_gpio_init_hw)(struct gpio_chip *gc);
> +};
> +
> +void starfive_set_gpiomux(struct starfive_pinctrl *sfp, unsigned int pin,
> + unsigned int din, u32 dout, u32 doen);
> +int starfive_pinctrl_probe(struct platform_device *pdev,
> + const struct starfive_pinctrl_soc_info *info);
> +struct starfive_pinctrl *starfive_from_irq_desc(struct irq_desc *desc);
> +
> +#endif /* __DRIVERS_PINCTRL_STARFIVE_H__ */
> --
> 2.38.1
>