Basic pinctrl support for StarFive JH7110 RISC-V SoC

PATCH In review Hal Feng 6 participants 29 messages Nov 18, 2022 – Dec 9, 2022

Subsystems (5)

Device tree bindings GPIO RISC-V Pin control +1 more subsystems

lore

Share & tools

lore

Hal Feng

author maintainer

2022-11-18 01:11 UTC #0 lore raw

v2 cover 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

Signature · 7 lines

2.38.1


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

Hal Feng

author maintainer

2022-11-18 01:20 UTC #1 lore raw

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

include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h new file +427 -0

new file mode 100644
index 000000000000..fb02345caa27
@@ -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

Signature · 7 lines

2.38.1


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

Krzysztof Kozlowski

2022-11-21 08:38 UTC #2 lore raw

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

Quoted 45 lines · Hal Feng

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

include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h new file +26 -0

new file mode 100644
index 000000000000..fb02345caa27
@@ -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))
+

(...)

Quoted 48 lines

include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h new file +48 -0

+/* 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

include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h line 325

Quoted 10 lines

include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h new file +10 -0

+#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

Krzysztof Kozlowski

2022-11-21 08:39 UTC #3 lore raw

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

Quoted 104 lines · Krzysztof Kozlowski

Quoted 45 lines · Hal Feng

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

include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h new file +26 -0

new file mode 100644
index 000000000000..fb02345caa27
@@ -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))
+

(...)

Quoted 54 lines

include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h new file +54 -0

+/* 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

Quoted 8 lines

include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h line 325
Quoted 4 lines
include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h new file +4 -0
+#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

Jianlong Huang

2022-11-28 00:50 UTC #4 lore raw

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

Quoted 114 lines · Krzysztof Kozlowski

Quoted 104 lines · Krzysztof Kozlowski

Quoted 45 lines · Hal Feng

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

include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h new file +26 -0

new file mode 100644
index 000000000000..fb02345caa27
@@ -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))
+

(...)

Quoted 54 lines

include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h new file +54 -0

+/* 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

Quoted 8 lines

include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h line 325
Quoted 4 lines
include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h new file +4 -0
+#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.

Quoted 2 lines

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

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

Krzysztof Kozlowski

2022-11-28 08:32 UTC #5 lore raw

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

Quoted 11 lines · Jianlong Huang

Quoted 11 lines
Quoted 10 lines
include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h line 325
Quoted 6 lines
include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h new file +6 -0
+/* 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.

Quoted 5 lines

Quoted 2 lines
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

Jianlong Huang

2022-11-29 01:47 UTC #6 lore raw

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

Quoted 16 lines · Krzysztof Kozlowski

Quoted 14 lines · Jianlong Huang
Quoted 13 lines
Quoted 10 lines
include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h line 325
Quoted 6 lines
include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h new file +6 -0
+/* 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).

Quoted 7 lines

Quoted 2 lines
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

Krzysztof Kozlowski

2022-11-29 07:50 UTC #7 lore raw

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

Quoted 27 lines · Jianlong Huang

Quoted 16 lines · Krzysztof Kozlowski
Quoted 14 lines · Jianlong Huang
Quoted 13 lines
Quoted 10 lines
include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h line 325
Quoted 6 lines
include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h new file +6 -0
+/* 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).
Quoted 7 lines
Quoted 2 lines
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.

Quoted 7 lines

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

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

Jianlong Huang

2022-11-29 14:46 UTC #8 lore raw

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

Quoted 31 lines · Krzysztof Kozlowski

Quoted 30 lines · Jianlong Huang
Quoted 16 lines · Krzysztof Kozlowski
Quoted 14 lines · Jianlong Huang
Quoted 13 lines
Quoted 10 lines
Review
Quoted 6 lines
(quoted diff) +6 -0
+/* 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).
Quoted 7 lines
Quoted 2 lines
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.

Quoted 9 lines

Quoted 4 lines
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

Krzysztof Kozlowski

2022-11-29 14:58 UTC #9 lore raw

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

Quoted 37 lines · Jianlong Huang

Quoted 36 lines · Krzysztof Kozlowski
Quoted 34 lines · Jianlong Huang
Quoted 16 lines · Krzysztof Kozlowski
Quoted 14 lines · Jianlong Huang
Quoted 13 lines
Quoted 10 lines
Review
Quoted 6 lines
(quoted diff) +6 -0
+/* 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).
Quoted 7 lines
Quoted 2 lines
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)>;

Quoted 6 lines

Quoted 4 lines
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.

Quoted 1 line

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.

Quoted 4 lines

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.

Quoted 2 lines

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

Jianlong Huang

2022-11-29 15:59 UTC #10 lore raw

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

Quoted 57 lines · Krzysztof Kozlowski

Quoted 37 lines · Jianlong Huang
Quoted 36 lines · Krzysztof Kozlowski
Quoted 34 lines · Jianlong Huang
Quoted 16 lines · Krzysztof Kozlowski
Quoted 14 lines · Jianlong Huang
Quoted 13 lines
Quoted 10 lines
Review
Quoted 6 lines
(quoted diff) +6 -0
+/* 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).
Quoted 7 lines
Quoted 2 lines
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)>;
Quoted 6 lines
Quoted 4 lines
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.
Quoted 1 line
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.
Quoted 4 lines
Or you have any better suggestion?
These macros are the value of register, not register addresses/offsets, except for with prefix of GPI.

Quoted 9 lines

Still, values are not usually part of bindings.
Quoted 2 lines
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

Krzysztof Kozlowski

2022-11-29 16:03 UTC #11 lore raw

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

Quoted 9 lines · Jianlong Huang

Quoted 7 lines
Quoted 2 lines
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

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

Jianlong Huang

2022-12-01 09:31 UTC #12 lore raw

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

In reply to Krzysztof Kozlowski

Quoted 57 lines · Krzysztof Kozlowski

Quoted 37 lines · Jianlong Huang
Quoted 36 lines · Krzysztof Kozlowski
Quoted 34 lines · Jianlong Huang
Quoted 16 lines · Krzysztof Kozlowski
Quoted 14 lines · Jianlong Huang
Quoted 13 lines
Quoted 10 lines
Review
Quoted 6 lines
(quoted diff) +6 -0
+/* 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).
Quoted 7 lines
Quoted 2 lines
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)>;
Quoted 6 lines
Quoted 4 lines
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.
Quoted 1 line
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.
Quoted 4 lines
Or you have any better suggestion?
These macros are the value of register, not register addresses/offsets, except for with prefix of GPI.

Quoted 9 lines

Still, values are not usually part of bindings.
Quoted 2 lines
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

Emil Renner Berthing

2022-12-07 13:15 UTC #13 lore raw

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

In reply to Hal Feng

Quoted 429 lines · Hal Feng

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

include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h new file +409 -0

new file mode 100644
index 000000000000..fb02345caa27
@@ -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

include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h line 419

Quoted 10 lines

include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h new file +10 -0

+#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.

Quoted 10 lines

include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h new file +7 -1

+/* 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

Hal Feng

author maintainer

2022-11-18 01:11 UTC #14 lore raw

PATCH v2 2/5 dt-bindings: pinctrl: Add StarFive JH7110 sys pinctrl

In reply to Hal Feng

From: Jianlong Huang <[email protected]>

Add pinctrl bindings for StarFive JH7110 SoC sys pinctrl controller.

Signed-off-by Jianlong Huang <[email protected]>

Signed-off-by Hal Feng <[email protected]>

---
 .../pinctrl/starfive,jh7110-sys-pinctrl.yaml  | 165 ++++++++++++++++++
 1 file changed, 165 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +165 -0

new file mode 100644
index 000000000000..79623f884a9c
@@ -0,0 +1,165 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/starfive,jh7110-sys-pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: StarFive JH7110 Sys 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_GPIO63
+  can be multiplexed and have configurable bias, drive strength,
+  schmitt trigger etc.
+  Some peripherals have their I/O go through the 64 "GPIOs". This also
+  includes a number of other UARTs, I2Cs, SPIs, PWMs etc.
+  All these peripherals are connected to all 64 GPIOs such that
+  any GPIO can be set up to be controlled by any of the peripherals.
+
+maintainers:
+  - Jianlong Huang <[email protected]>
+
+properties:
+  compatible:
+    const: starfive,jh7110-sys-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
+  - clocks
+  - 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>;
+
+                gpio: gpio@13040000 {
+                        compatible = "starfive,jh7110-sys-pinctrl";
+                        reg = <0x0 0x13040000 0x0 0x10000>;
+                        reg-names = "control";
+                        clocks = <&syscrg_clk JH7110_SYSCLK_IOMUX>;
+                        resets = <&syscrg_rst JH7110_SYSRST_IOMUX>;
+                        interrupts = <86>;
+                        interrupt-controller;
+                        #interrupt-cells = <2>;
+                        #gpio-cells = <2>;
+                        gpio-controller;
+                        status = "okay";
+
+                        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>;
+                                };
+                        };
+                };
+
+                uart0 {
+                        pinctrl-names = "default";
+                        pinctrl-0 = <&uart0_pins>;
+                        status = "okay";
+                };
+        };
+
+...

Signature · 7 lines

2.38.1


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

Rob Herring

maintainer

2022-11-18 03:56 UTC #15 lore raw

Re: [PATCH v2 2/5] dt-bindings: pinctrl: Add StarFive JH7110 sys pinctrl

Quoted 11 lines · Hal Feng

From: Jianlong Huang <[email protected]>
Add pinctrl bindings for StarFive JH7110 SoC sys pinctrl controller.
Signed-off-by Jianlong Huang <[email protected]>
Signed-off-by Hal Feng <[email protected]>
---
 .../pinctrl/starfive,jh7110-sys-pinctrl.yaml  | 165 ++++++++++++++++++
 1 file changed, 165 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.example.dts:21:18: fatal error: dt-bindings/clock/starfive-jh7110.h: No such file or directory
21 | #include <dt-bindings/clock/starfive-jh7110.h>

      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

compilation terminated.
make[1]: *** [scripts/Makefile.lib:406: Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.example.dtb] Error 1 make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1492: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]

This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command.

Hal Feng

author maintainer

2022-11-18 06:57 UTC #16 lore raw

Re: [PATCH v2 2/5] dt-bindings: pinctrl: Add StarFive JH7110 sys pinctrl

Quoted 14 lines · Rob Herring

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.example.dts:21:18: fatal error: dt-bindings/clock/starfive-jh7110.h: No such file or directory
21 | #include <dt-bindings/clock/starfive-jh7110.h>
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [scripts/Makefile.lib:406: Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.example.dtb] Error 1 make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1492: dt_binding_check] Error 2

Quoted 6 lines

doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1.

This patch series depends on the patch series [1].

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

Best regards,
Hal

Quoted 8 lines

If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command.

Krzysztof Kozlowski

2022-11-21 08:43 UTC #17 lore raw

Re: [PATCH v2 2/5] dt-bindings: pinctrl: Add StarFive JH7110 sys pinctrl

In reply to Hal Feng

Quoted 39 lines · Hal Feng

From: Jianlong Huang <[email protected]>

Add pinctrl bindings for StarFive JH7110 SoC sys pinctrl controller.

Signed-off-by Jianlong Huang <[email protected]>

Signed-off-by Hal Feng <[email protected]>

---
 .../pinctrl/starfive,jh7110-sys-pinctrl.yaml  | 165 ++++++++++++++++++
 1 file changed, 165 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +22 -0

new file mode 100644
index 000000000000..79623f884a9c
@@ -0,0 +1,165 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/starfive,jh7110-sys-pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: StarFive JH7110 Sys 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_GPIO63
+  can be multiplexed and have configurable bias, drive strength,
+  schmitt trigger etc.
+  Some peripherals have their I/O go through the 64 "GPIOs". This also
+  includes a number of other UARTs, I2Cs, SPIs, PWMs etc.
+  All these peripherals are connected to all 64 GPIOs such that
+  any GPIO can be set up to be controlled by any of the peripherals.
+
+maintainers:
+  - Jianlong Huang <[email protected]>
+

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml line 32

Quoted 10 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +10 -0
+properties:
+  compatible:
+    const: starfive,jh7110-sys-pinctrl
+
+  reg:
+    maxItems: 1
+
+  reg-names:
+    items:
+      - const: control

Why reg-names for one entry? Perhaps just drop it.

Quoted 5 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +5 -0
+
+  clocks:
+    maxItems: 1
+
+  resets:

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml line 47

Quoted 10 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +10 -0
+    maxItems: 1
+
+  gpio-controller: true
+
+  "#gpio-cells":
+    const: 2
+
+  interrupts:
+    maxItems: 1
+    description: The GPIO parent interrupt.

Drop description, it's obvious.

Quoted 9 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +9 -0
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 2
+
+required:
+  - compatible
+  - reg

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml line 66

Quoted 10 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +10 -0
+  - reg-names
+  - clocks
+  - gpio-controller
+  - "#gpio-cells"
+  - interrupts
+  - interrupt-controller
+  - "#interrupt-cells"
+
+patternProperties:
+  '-[0-9]+$':

Keep consistent quotes, either ' or "

How do you differentiate hogs? The pattern is a bit unspecific.

Quoted 8 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +8 -0

+    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

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml line 84

Quoted 10 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +10 -0

+          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"

Drop quotes.

Quoted 26 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +26 -0

+
+          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

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml line 120

Quoted 10 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +10 -0

+
+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 {

Use 4 spaces for example indentation.

Quoted 5 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +5 -0
+                #address-cells = <2>;
+                #size-cells = <2>;
+
+                gpio: gpio@13040000 {
+                        compatible = "starfive,jh7110-sys-pinctrl";

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml line 135

Quoted 10 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +10 -0

+                        reg = <0x0 0x13040000 0x0 0x10000>;
+                        reg-names = "control";
+                        clocks = <&syscrg_clk JH7110_SYSCLK_IOMUX>;
+                        resets = <&syscrg_rst JH7110_SYSRST_IOMUX>;
+                        interrupts = <86>;
+                        interrupt-controller;
+                        #interrupt-cells = <2>;
+                        #gpio-cells = <2>;
+                        gpio-controller;
+                        status = "okay";

No status in examples.

Quoted 16 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +16 -0

+
+                        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;

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml line 161

Quoted 10 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +10 -0

+                                        input-schmitt-enable;
+                                        slew-rate = <0>;
+                                };
+                        };
+                };
+
+                uart0 {
+                        pinctrl-names = "default";
+                        pinctrl-0 = <&uart0_pins>;
+                        status = "okay";

Drop this node, useless.

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml line 165

Quoted 4 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +4 -0
+                };
+        };
+
+...

Best regards,
Krzysztof

Jianlong Huang

2022-11-28 01:04 UTC #18 lore raw

Re: [PATCH v2 2/5] dt-bindings: pinctrl: Add StarFive JH7110 sys pinctrl

Quoted 44 lines · Krzysztof Kozlowski

Quoted 43 lines · Hal Feng

From: Jianlong Huang <[email protected]>

Add pinctrl bindings for StarFive JH7110 SoC sys pinctrl controller.

Signed-off-by Jianlong Huang <[email protected]>

Signed-off-by Hal Feng <[email protected]>

---
 .../pinctrl/starfive,jh7110-sys-pinctrl.yaml  | 165 ++++++++++++++++++
 1 file changed, 165 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +26 -0

new file mode 100644
index 000000000000..79623f884a9c
@@ -0,0 +1,165 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/starfive,jh7110-sys-pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: StarFive JH7110 Sys 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_GPIO63
+  can be multiplexed and have configurable bias, drive strength,
+  schmitt trigger etc.
+  Some peripherals have their I/O go through the 64 "GPIOs". This also
+  includes a number of other UARTs, I2Cs, SPIs, PWMs etc.
+  All these peripherals are connected to all 64 GPIOs such that
+  any GPIO can be set up to be controlled by any of the peripherals.
+
+maintainers:
+  - Jianlong Huang <[email protected]>
+
+properties:
+  compatible:
+    const: starfive,jh7110-sys-pinctrl
+

Quoted 8 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml line 32
Quoted 6 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +6 -0
+  reg:
+    maxItems: 1
+
+  reg-names:
+    items:
+      - const: control
Why reg-names for one entry? Perhaps just drop it.

Will fix, drop it.

Quoted 10 lines

Quoted 10 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +9 -0
+
+  clocks:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+  gpio-controller: true
+

Quoted 8 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml line 47
Quoted 6 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +6 -0
+  "#gpio-cells":
+    const: 2
+
+  interrupts:
+    maxItems: 1
+    description: The GPIO parent interrupt.
Drop description, it's obvious.

Will fix, drop it.

Quoted 23 lines

Quoted 10 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +9 -0
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 2
+
+required:
+  - compatible
+  - reg
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml line 66
Quoted 10 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +10 -0
+  - reg-names
+  - clocks
+  - gpio-controller
+  - "#gpio-cells"
+  - interrupts
+  - interrupt-controller
+  - "#interrupt-cells"
+
+patternProperties:
+  '-[0-9]+$':
Keep consistent quotes, either ' or "

Quoted 1 line

How do you differentiate hogs? The pattern is a bit unspecific.

Will fix.
Keep consisitent quotes, use '

Quoted 20 lines

Quoted 9 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +8 -0

+    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

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml line 84

Quoted 10 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +10 -0

+          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"

Quoted 1 line

Drop quotes.

Will fix, drop quotes.

Quoted 31 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +30 -0

+
+          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:

Quoted 8 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml line 120
Quoted 6 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +6 -0
+  - |
+    #include <dt-bindings/clock/starfive-jh7110.h>
+    #include <dt-bindings/reset/starfive-jh7110.h>
+    #include <dt-bindings/pinctrl/pinctrl-starfive-jh7110.h>
+
+        soc {
Use 4 spaces for example indentation.

Will fix.

Quoted 17 lines

Quoted 6 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +5 -0
+                #address-cells = <2>;
+                #size-cells = <2>;
+
+                gpio: gpio@13040000 {
+                        compatible = "starfive,jh7110-sys-pinctrl";

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml line 135

Quoted 10 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +10 -0

+                        reg = <0x0 0x13040000 0x0 0x10000>;
+                        reg-names = "control";
+                        clocks = <&syscrg_clk JH7110_SYSCLK_IOMUX>;
+                        resets = <&syscrg_rst JH7110_SYSRST_IOMUX>;
+                        interrupts = <86>;
+                        interrupt-controller;
+                        #interrupt-cells = <2>;
+                        #gpio-cells = <2>;
+                        gpio-controller;
+                        status = "okay";

Quoted 1 line

No status in examples.

Will fix, drop it.

Quoted 21 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +20 -0

+
+                        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>;
+                                };
+                        };

Quoted 8 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml line 161
Quoted 6 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +6 -0
+                };
+
+                uart0 {
+                        pinctrl-names = "default";
+                        pinctrl-0 = <&uart0_pins>;
+                        status = "okay";
Drop this node, useless.

Will fix, drop this node.

Quoted 6 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml line 165
Quoted 5 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +4 -0
+                };
+        };
+
+...

Best regards,
Jianlong Huang

Emil Renner Berthing

2022-12-07 13:19 UTC #19 lore raw

Re: [PATCH v2 2/5] dt-bindings: pinctrl: Add StarFive JH7110 sys pinctrl

Quoted 170 lines · Jianlong Huang

Quoted 44 lines · Krzysztof Kozlowski
Quoted 43 lines · Hal Feng
From: Jianlong Huang <[email protected]>
Add pinctrl bindings for StarFive JH7110 SoC sys pinctrl controller.
Signed-off-by Jianlong Huang <[email protected]>
Signed-off-by Hal Feng <[email protected]>
---
 .../pinctrl/starfive,jh7110-sys-pinctrl.yaml  | 165 ++++++++++++++++++
 1 file changed, 165 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +26 -0
new file mode 100644
index 000000000000..79623f884a9c
@@ -0,0 +1,165 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/starfive,jh7110-sys-pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: StarFive JH7110 Sys 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_GPIO63
+  can be multiplexed and have configurable bias, drive strength,
+  schmitt trigger etc.
+  Some peripherals have their I/O go through the 64 "GPIOs". This also
+  includes a number of other UARTs, I2Cs, SPIs, PWMs etc.
+  All these peripherals are connected to all 64 GPIOs such that
+  any GPIO can be set up to be controlled by any of the peripherals.
+
+maintainers:
+  - Jianlong Huang <[email protected]>
+
+properties:
+  compatible:
+    const: starfive,jh7110-sys-pinctrl
+
Quoted 8 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml line 32
Quoted 6 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +6 -0
+  reg:
+    maxItems: 1
+
+  reg-names:
+    items:
+      - const: control
Why reg-names for one entry? Perhaps just drop it.
Will fix, drop it.
Quoted 10 lines
Quoted 10 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +9 -0
+
+  clocks:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+  gpio-controller: true
+
Quoted 8 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml line 47
Quoted 6 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +6 -0
+  "#gpio-cells":
+    const: 2
+
+  interrupts:
+    maxItems: 1
+    description: The GPIO parent interrupt.
Drop description, it's obvious.
Will fix, drop it.
Quoted 23 lines
Quoted 10 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +9 -0
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 2
+
+required:
+  - compatible
+  - reg
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml line 66
Quoted 10 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +10 -0
+  - reg-names
+  - clocks
+  - gpio-controller
+  - "#gpio-cells"
+  - interrupts
+  - interrupt-controller
+  - "#interrupt-cells"
+
+patternProperties:
+  '-[0-9]+$':
Keep consistent quotes, either ' or "
Quoted 1 line
How do you differentiate hogs? The pattern is a bit unspecific.
Will fix.
Keep consisitent quotes, use '
Quoted 20 lines
Quoted 9 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +8 -0
+    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
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml line 84
Quoted 10 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +10 -0
+          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"
Quoted 1 line
Drop quotes.
Will fix, drop quotes.
Quoted 31 lines
Quoted 31 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +30 -0
+
+          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:
Quoted 8 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml line 120
Quoted 6 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +6 -0
+  - |
+    #include <dt-bindings/clock/starfive-jh7110.h>
+    #include <dt-bindings/reset/starfive-jh7110.h>
+    #include <dt-bindings/pinctrl/pinctrl-starfive-jh7110.h>
+
+        soc {
Use 4 spaces for example indentation.

Quoted 5 lines

Will fix.
Quoted 3 lines
Quoted 3 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +2 -0
+                #address-cells = <2>;
+                #size-cells = <2>;

You can also drop these to lines..

Quoted 4 lines

Quoted 4 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml line 126
Quoted 4 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +4 -0
+
+                gpio: gpio@13040000 {
+                        compatible = "starfive,jh7110-sys-pinctrl";
+                        reg = <0x0 0x13040000 0x0 0x10000>;

..and then just make this
reg = <0x13040000 0x10000>;

Quoted 57 lines

Quoted 11 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml line 135
Quoted 9 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +9 -0
+                        reg-names = "control";
+                        clocks = <&syscrg_clk JH7110_SYSCLK_IOMUX>;
+                        resets = <&syscrg_rst JH7110_SYSRST_IOMUX>;
+                        interrupts = <86>;
+                        interrupt-controller;
+                        #interrupt-cells = <2>;
+                        #gpio-cells = <2>;
+                        gpio-controller;
+                        status = "okay";
No status in examples.
Will fix, drop it.
Quoted 21 lines
Quoted 21 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +20 -0
+
+                        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>;
+                                };
+                        };
Quoted 8 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml line 161
Quoted 6 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +6 -0
+                };
+
+                uart0 {
+                        pinctrl-names = "default";
+                        pinctrl-0 = <&uart0_pins>;
+                        status = "okay";
Drop this node, useless.
Will fix, drop this node.
Quoted 6 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml line 165
Quoted 5 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml new file +4 -0
+                };
+        };
+
+...
Best regards,
Jianlong Huang

Icenowy Zheng

2022-12-09 03:13 UTC #20 lore raw

Re: [PATCH v2 2/5] dt-bindings: pinctrl: Add StarFive JH7110 sys pinctrl

In reply to Hal Feng

Quoted 102 lines · 在 2022-11-18星期五的 09:11 +0800，Hal Feng写道：

From: Jianlong Huang <[email protected]>
Add pinctrl bindings for StarFive JH7110 SoC sys pinctrl controller.
Signed-off-by Jianlong Huang <[email protected]>
Signed-off-by Hal Feng <[email protected]>
---
.../pinctrl/starfive,jh7110-sys-pinctrl.yaml | 165
++++++++++++++++++
 1 file changed, 165 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys- pinctrl.yaml
diff --git a/Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys- pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys- pinctrl.yaml
new file mode 100644
index 000000000000..79623f884a9c
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys- new file +0 -0
pinctrl.yaml
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys- new file +4 -0
@@ -0,0 +1,165 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id:
http://devicetree.org/schemas/pinctrl/starfive,jh7110-sys-pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys- new file +7 -0
+
+title: StarFive JH7110 Sys 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_GPIO63
+ can be multiplexed and have configurable bias, drive strength, + schmitt trigger etc.
+ Some peripherals have their I/O go through the 64 "GPIOs". This also
+ includes a number of other UARTs, I2Cs, SPIs, PWMs etc. + All these peripherals are connected to all 64 GPIOs such that + any GPIO can be set up to be controlled by any of the peripherals.
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys- new file +54 -0
+
+maintainers:
+  - Jianlong Huang <[email protected]>
+
+properties:
+  compatible:
+    const: starfive,jh7110-sys-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
+  - clocks
+  - 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

Quoted 8 lines

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.

Could this handle hard wiring an internal input mux function to high or low?

This feature is needed on the Star64 board to omit the USB overcurrent pin.

Quoted 84 lines

+
+        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"
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys- new file +55 -0
+
+          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>;
+
+                gpio: gpio@13040000 {
+                        compatible = "starfive,jh7110-sys-pinctrl";
+                        reg = <0x0 0x13040000 0x0 0x10000>;
+                        reg-names = "control";
+                        clocks = <&syscrg_clk JH7110_SYSCLK_IOMUX>;
+                        resets = <&syscrg_rst JH7110_SYSRST_IOMUX>;
+                        interrupts = <86>;
+                        interrupt-controller;
+                        #interrupt-cells = <2>;
+                        #gpio-cells = <2>;
+                        gpio-controller;
+                        status = "okay";
+
+                        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>; +                                };
+                        };
+                };

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-

Quoted 9 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys- new file +9 -0

+
+                uart0 {
+                        pinctrl-names = "default";
+                        pinctrl-0 = <&uart0_pins>;
+                        status = "okay";
+                };
+        };
+
+...

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

Hal Feng

author maintainer

2022-11-18 01:11 UTC #21 lore raw

PATCH v2 3/5 dt-bindings: pinctrl: Add StarFive JH7110 aon pinctrl

In reply to Hal Feng

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

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml new file +134 -0

new file mode 100644
index 000000000000..1dd000e1f614
@@ -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;
+                };
+        };
+
+...

Signature · 7 lines

2.38.1


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

Krzysztof Kozlowski

2022-11-21 08:44 UTC #22 lore raw

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

Quoted 52 lines · Hal Feng

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

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml new file +35 -0

new file mode 100644
index 000000000000..1dd000e1f614
@@ -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:

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml line 45

Quoted 10 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml new file +10 -0
+    maxItems: 1
+
+  gpio-controller: true
+
+  "#gpio-cells":
+    const: 2
+
+  interrupts:
+    maxItems: 1
+    description: The GPIO parent interrupt.

Same comments apply plus one more.

Quoted 5 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml new file +5 -0
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 2

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml line 60

Quoted 10 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml new file +10 -0
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - gpio-controller
+  - "#gpio-cells"
+  - interrupts
+  - interrupt-controller
+  - "#interrupt-cells"

"required:" goes after patternProperties.

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml line 63

Quoted 3 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml new file +3 -0
+
+patternProperties:
+  '-[0-9]+$':

Same comment.

Quoted 46 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml new file +46 -0

+    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

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml line 119

Quoted 10 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml new file +10 -0

+
+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.

Quoted 5 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml new file +5 -0

+
+                gpioa: gpio@17020000 {
+                        compatible = "starfive,jh7110-aon-pinctrl";
+                        reg = <0x0 0x17020000 0x0 0x10000>;
+                        reg-names = "control";

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml line 134

Quoted 10 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml new file +10 -0

+                        resets = <&aoncrg_rst JH7110_AONRST_AON_IOMUX>;
+                        interrupts = <85>;
+                        interrupt-controller;
+                        #interrupt-cells = <2>;
+                        #gpio-cells = <2>;
+                        gpio-controller;
+                };
+        };
+
+...

Best regards,
Krzysztof

Jianlong Huang

2022-11-28 01:15 UTC #23 lore raw

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

Quoted 57 lines · Krzysztof Kozlowski

Quoted 56 lines · Hal Feng

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

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml new file +39 -0

new file mode 100644
index 000000000000..1dd000e1f614
@@ -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
+

Quoted 8 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml line 45
Quoted 6 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml new file +6 -0
+  "#gpio-cells":
+    const: 2
+
+  interrupts:
+    maxItems: 1
+    description: The GPIO parent interrupt.
Same comments apply plus one more.

Will fix, drop this description.

Quoted 10 lines

Quoted 10 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml new file +9 -0
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 2
+
+required:
+  - compatible
+  - reg

Quoted 8 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml line 60
Quoted 6 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml new file +6 -0
+  - reg-names
+  - gpio-controller
+  - "#gpio-cells"
+  - interrupts
+  - interrupt-controller
+  - "#interrupt-cells"
"required:" goes after patternProperties.

Will fix.

Quoted 6 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml line 63
Quoted 4 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml new file +3 -0
+
+patternProperties:
+  '-[0-9]+$':
Same comment.

Will fix.
Keep consistent quotes, use '

Quoted 51 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml new file +50 -0

+    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>

Quoted 8 lines

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml line 119
Quoted 6 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml new file +6 -0
+    #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.

Quoted 17 lines

Quoted 6 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml new file +5 -0
+
+                gpioa: gpio@17020000 {
+                        compatible = "starfive,jh7110-aon-pinctrl";
+                        reg = <0x0 0x17020000 0x0 0x10000>;
+                        reg-names = "control";
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml line 134
Quoted 10 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml new file +10 -0
+                        resets = <&aoncrg_rst JH7110_AONRST_AON_IOMUX>;
+                        interrupts = <85>;
+                        interrupt-controller;
+                        #interrupt-cells = <2>;
+                        #gpio-cells = <2>;
+                        gpio-controller;
+                };
+        };
+
+...

Best regards,
Jianlong Huang

Emil Renner Berthing

2022-12-07 13:21 UTC #24 lore raw

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

Quoted 42 lines · Jianlong Huang

Quoted 40 lines · Krzysztof Kozlowski

Quoted 39 lines · Hal Feng

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

Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml new file +22 -0

new file mode 100644
index 000000000000..1dd000e1f614
@@ -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:

Quoted 8 lines

Quoted 8 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml line 30
Quoted 8 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml new file +8 -0
+    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.

Quoted 100 lines

Quoted 9 lines
Quoted 9 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml new file +9 -0
+
+  clocks:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+  gpio-controller: true
+
Quoted 8 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml line 45
Quoted 6 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml new file +6 -0
+  "#gpio-cells":
+    const: 2
+
+  interrupts:
+    maxItems: 1
+    description: The GPIO parent interrupt.
Same comments apply plus one more.
Will fix, drop this description.
Quoted 10 lines
Quoted 10 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml new file +9 -0
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 2
+
+required:
+  - compatible
+  - reg
Quoted 8 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml line 60
Quoted 6 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml new file +6 -0
+  - reg-names
+  - gpio-controller
+  - "#gpio-cells"
+  - interrupts
+  - interrupt-controller
+  - "#interrupt-cells"
"required:" goes after patternProperties.
Will fix.
Quoted 6 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml line 63
Quoted 4 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml new file +3 -0
+
+patternProperties:
+  '-[0-9]+$':
Same comment.
Will fix.
Keep consistent quotes, use '
Quoted 49 lines
Quoted 49 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml new file +48 -0
+    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:

Quoted 8 lines

Quoted 8 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml line 119
Quoted 8 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml new file +8 -0
+  - |
+    #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..

Quoted 11 lines

Quoted 2 lines
Same comment.
Will fix.
Use 4 spaces for example indentation.
Quoted 5 lines
Quoted 5 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml new file +4 -0
+
+                gpioa: gpio@17020000 {
+                        compatible = "starfive,jh7110-aon-pinctrl";
+                        reg = <0x0 0x17020000 0x0 0x10000>;

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

Quoted 17 lines

Quoted 12 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml line 134
Quoted 11 lines
Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml new file +11 -0
+                        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

Hal Feng

author maintainer

2022-11-18 01:11 UTC #25 lore raw

PATCH v2 4/5 pinctrl: starfive: Add StarFive JH7110 sys controller driver

In reply to Hal Feng

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

MAINTAINERS +4 -3

index ec6647e2772f..a70c1d0f303e 100644
@@ -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]>

drivers/pinctrl/starfive/Kconfig +21 -0

index 55c514e622f9..c7896a1f7d5a 100644
@@ -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.

drivers/pinctrl/starfive/Makefile +5 -0

index 0293f26a0a99..404929f760e8 100644
@@ -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

drivers/pinctrl/starfive/pinctrl-jh7110-sys.c new file +464 -0

464 lines not on this page · raw message

drivers/pinctrl/starfive/pinctrl-starfive.c new file +972 -0

972 lines not on this page · raw message

drivers/pinctrl/starfive/pinctrl-starfive.h new file +72 -0

72 lines not on this page · raw message

Signature · 7 lines

2.38.1


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

Emil Renner Berthing

2022-12-07 13:47 UTC #26 lore raw

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

Quoted 26 lines · Hal Feng

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
MAINTAINERS +0 -0
index ec6647e2772f..a70c1d0f303e 100644

MAINTAINERS F: Documentation/devicetree/bindings/clock/starfive* · line 19609

Quoted 5 lines

MAINTAINERS +1 -1

@@ -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

Quoted 17 lines

MAINTAINERS +3 -2

 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]>

drivers/pinctrl/starfive/Kconfig +0 -0

index 55c514e622f9..c7896a1f7d5a 100644

drivers/pinctrl/starfive/Kconfig config PINCTRL_STARFIVE_JH7100 · line 21

Quoted 6 lines

drivers/pinctrl/starfive/Kconfig +3 -0

@@ -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?

drivers/pinctrl/starfive/Kconfig config PINCTRL_STARFIVE_JH7100 · line 30

Quoted 9 lines

drivers/pinctrl/starfive/Kconfig +9 -0

+       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.

Quoted 14 lines

drivers/pinctrl/starfive/Kconfig +9 -0

+       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.

drivers/pinctrl/starfive/Makefile +0 -0

index 0293f26a0a99..404929f760e8 100644

drivers/pinctrl/starfive/Makefile line 4

Quoted 4 lines

drivers/pinctrl/starfive/Makefile +2 -0

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

drivers/pinctrl/starfive/Makefile line 8

Quoted 3 lines

drivers/pinctrl/starfive/Makefile +2 -0

+# 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.

Quoted 55 lines

drivers/pinctrl/starfive/pinctrl-jh7110-sys.c new file +49 -0

new file mode 100644
index 000000000000..942e0b6e5ac6
@@ -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

drivers/pinctrl/starfive/pinctrl-jh7110-sys.c line 59

Quoted 10 lines

drivers/pinctrl/starfive/pinctrl-jh7110-sys.c new file +10 -0

+#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?

Quoted 376 lines

drivers/pinctrl/starfive/pinctrl-jh7110-sys.c new file +376 -0

+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 */ }
+};
+

drivers/pinctrl/starfive/pinctrl-jh7110-sys.c line 446

Quoted 10 lines

drivers/pinctrl/starfive/pinctrl-jh7110-sys.c new file +10 -0

+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.

drivers/pinctrl/starfive/pinctrl-jh7110-sys.c line 451

Quoted 5 lines

drivers/pinctrl/starfive/pinctrl-jh7110-sys.c new file +5 -0

+
+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.

drivers/pinctrl/starfive/pinctrl-jh7110-sys.c line 460

Quoted 9 lines

drivers/pinctrl/starfive/pinctrl-jh7110-sys.c new file +9 -0

+       },
+       .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?

drivers/pinctrl/starfive/pinctrl-jh7110-sys.c line 464

Quoted 3 lines

drivers/pinctrl/starfive/pinctrl-jh7110-sys.c new file +3 -0

+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.

drivers/pinctrl/starfive/pinctrl-starfive.c

Quoted 5 lines

drivers/pinctrl/starfive/pinctrl-starfive.c new file +0 -0
new file mode 100644
index 000000000000..944caa599460

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

Quoted 36 lines

drivers/pinctrl/starfive/pinctrl-starfive.c new file +35 -0

@@ -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)

drivers/pinctrl/starfive/pinctrl-starfive.c line 45

Quoted 10 lines

drivers/pinctrl/starfive/pinctrl-starfive.c new file +10 -0

+#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.

drivers/pinctrl/starfive/pinctrl-starfive.c line 46

Quoted 1 line

drivers/pinctrl/starfive/pinctrl-starfive.c new file +1 -0
+#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.

Quoted 192 lines

drivers/pinctrl/starfive/pinctrl-starfive.c new file +192 -0

+/*
+ * 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;

drivers/pinctrl/starfive/pinctrl-starfive.c line 249

Quoted 10 lines

drivers/pinctrl/starfive/pinctrl-starfive.c new file +10 -0

+}
+
+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.

Quoted 760 lines

drivers/pinctrl/starfive/pinctrl-starfive.c new file +723 -0

+
+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]>");

drivers/pinctrl/starfive/pinctrl-starfive.h new file +31 -0

new file mode 100644
index 000000000000..54ffdb6412f1
@@ -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;

drivers/pinctrl/starfive/pinctrl-starfive.h line 41

Quoted 10 lines

drivers/pinctrl/starfive/pinctrl-starfive.h new file +10 -0

+       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.

Quoted 33 lines

drivers/pinctrl/starfive/pinctrl-starfive.h new file +30 -1

+       /* 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

Hal Feng

author maintainer

2022-11-18 01:11 UTC #27 lore raw

PATCH v2 5/5 pinctrl: starfive: Add StarFive JH7110 aon controller driver

In reply to Hal Feng

From: Jianlong Huang <[email protected]>

Add pinctrl driver for StarFive JH7110 SoC aon 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]>

---
 drivers/pinctrl/starfive/Makefile             |   2 +-
 drivers/pinctrl/starfive/pinctrl-jh7110-aon.c | 192 ++++++++++++++++++
 2 files changed, 193 insertions(+), 1 deletion(-)
 create mode 100644 drivers/pinctrl/starfive/pinctrl-jh7110-aon.c

drivers/pinctrl/starfive/Makefile +1 -1

index 404929f760e8..17cdd1b0e650 100644
@@ -5,4 +5,4 @@ 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
+obj-$(CONFIG_PINCTRL_STARFIVE_JH7110)	+= pinctrl-jh7110-sys.o pinctrl-jh7110-aon.o

drivers/pinctrl/starfive/pinctrl-jh7110-aon.c new file +192 -0

192 lines not on this page · raw message

Signature · 7 lines

2.38.1


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

Hal Feng

author maintainer

2022-11-18 07:17 UTC #28 lore raw

In reply to Hal Feng

Quoted 4 lines · Hal Feng

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]/

Quoted 1 line

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