2021-12-07 04:17:47

by Wells Lu

[permalink] [raw]
Subject: [PATCH v3 0/2] This is a patch series for pinctrl driver of Sunplus SP7021 SoC.

Sunplus SP7021 is an ARM Cortex A7 (4 cores) based SoC. It integrates
many peripherals (ex: UART, I2C, SPI, SDIO, eMMC, USB, SD card and
etc.) into a single chip. It is designed for industrial control
applications.

Refer to:
https://sunplus-tibbo.atlassian.net/wiki/spaces/doc/overview
https://tibbo.com/store/plus1.html

Wells Lu (2):
dt-bindings: pinctrl: Add dt-bindings for Sunplus SP7021
pinctrl: Add driver for Sunplus SP7021

.../bindings/pinctrl/sunplus,sp7021-pinctrl.yaml | 293 ++++++
MAINTAINERS | 10 +
drivers/pinctrl/Kconfig | 1 +
drivers/pinctrl/Makefile | 1 +
drivers/pinctrl/sunplus/Kconfig | 21 +
drivers/pinctrl/sunplus/Makefile | 5 +
drivers/pinctrl/sunplus/sppctl.c | 1074 ++++++++++++++++++++
drivers/pinctrl/sunplus/sppctl.h | 154 +++
drivers/pinctrl/sunplus/sppctl_sp7021.c | 536 ++++++++++
include/dt-bindings/pinctrl/sppctl-sp7021.h | 173 ++++
include/dt-bindings/pinctrl/sppctl.h | 40 +
11 files changed, 2308 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pinctrl/sunplus,sp7021-pinctrl.yaml
create mode 100644 drivers/pinctrl/sunplus/Kconfig
create mode 100644 drivers/pinctrl/sunplus/Makefile
create mode 100644 drivers/pinctrl/sunplus/sppctl.c
create mode 100644 drivers/pinctrl/sunplus/sppctl.h
create mode 100644 drivers/pinctrl/sunplus/sppctl_sp7021.c
create mode 100644 include/dt-bindings/pinctrl/sppctl-sp7021.h
create mode 100644 include/dt-bindings/pinctrl/sppctl.h

--
2.7.4



2021-12-07 04:17:51

by Wells Lu

[permalink] [raw]
Subject: [PATCH v3 1/2] dt-bindings: pinctrl: Add dt-bindings for Sunplus SP7021

Add dt-bindings header files and documentation for Sunplus SP7021 SoC.

Signed-off-by: Wells Lu <[email protected]>
---
Changes in V3
- Addressed comments of Mr. Rob Herring.

.../bindings/pinctrl/sunplus,sp7021-pinctrl.yaml | 293 +++++++++++++++++++++
MAINTAINERS | 9 +
include/dt-bindings/pinctrl/sppctl-sp7021.h | 173 ++++++++++++
include/dt-bindings/pinctrl/sppctl.h | 40 +++
4 files changed, 515 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pinctrl/sunplus,sp7021-pinctrl.yaml
create mode 100644 include/dt-bindings/pinctrl/sppctl-sp7021.h
create mode 100644 include/dt-bindings/pinctrl/sppctl.h

diff --git a/Documentation/devicetree/bindings/pinctrl/sunplus,sp7021-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/sunplus,sp7021-pinctrl.yaml
new file mode 100644
index 0000000..fe2ab1d
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/sunplus,sp7021-pinctrl.yaml
@@ -0,0 +1,293 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) Sunplus Co., Ltd. 2021
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/sunplus,sp7021-pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sunplus SP7021 Pin Controller Device Tree Bindings
+
+maintainers:
+ - Dvorkin Dmitry <[email protected]>
+ - Wells Lu <[email protected]>
+
+description: |
+ The Sunplus SP7021 pin controller is used to control SoC pins. Please
+ refer to pinctrl-bindings.txt in this directory for details of the common
+ pinctrl bindings used by client devices.
+
+ The device node of pin controller of Sunplus SP7021 has following
+ properties.
+
+properties:
+ compatible:
+ const: sunplus,sp7021-pctl
+
+ gpio-controller: true
+
+ '#gpio-cells':
+ const: 2
+
+ reg:
+ items:
+ - description: the MOON2 registers
+ - description: the GPIOXT registers
+ - description: the GPIOXT2 registers
+ - description: the FIRST registers
+ - description: the MOON1 registers
+
+ reg-names:
+ items:
+ - const: moon2
+ - const: gpioxt
+ - const: gpioxt2
+ - const: first
+ - const: moon1
+
+ clocks:
+ maxItems: 1
+
+ resets:
+ maxItems: 1
+
+patternProperties:
+ '-pins$':
+ if:
+ type: object
+ then:
+ description: |
+ A pinctrl node should contain at least one subnodes representing the
+ pins or function-pins group available on the machine. Each subnode
+ will list the pins it needs, and how they should be configured.
+
+ Pinctrl node's client devices use subnodes for desired pin
+ configuration. Client device subnodes use below standard properties.
+
+ properties:
+ pins:
+ description: |
+ Define pins which are used by pinctrl node's client device.
+
+ It consists of one or more integers which represents the config
+ setting for corresponding pin. Please use macro SPPCTL_IOPAD to
+ define the integers for pins.
+
+ The first argument of the macro is pin number, the second is pin
+ type, the third is type of GPIO, the last is default output state
+ of GPIO.
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+
+ function:
+ description: |
+ Define pin-function which is used by pinctrl node's client device.
+ The name should be one of string in the following enumeration.
+ $ref: "/schemas/types.yaml#/definitions/string"
+ enum: [ SPI_FLASH, SPI_FLASH_4BIT, SPI_NAND, CARD0_EMMC, SD_CARD,
+ UA0, FPGA_IFX, HDMI_TX, LCDIF, USB0_OTG, USB1_OTG ]
+
+ groups:
+ description: |
+ Define pin-group in a specified pin-function.
+ The name should be one of string in the following enumeration.
+ $ref: "/schemas/types.yaml#/definitions/string"
+ enum: [ SPI_FLASH1, SPI_FLASH2, SPI_FLASH_4BIT1, SPI_FLASH_4BIT2,
+ SPI_NAND, CARD0_EMMC, SD_CARD, UA0, FPGA_IFX, HDMI_TX1,
+ HDMI_TX2, HDMI_TX3, LCDIF, USB0_OTG, USB1_OTG ]
+
+ zero_func:
+ description: |
+ Disabled pins which are not used by pinctrl node's client device.
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+
+ additionalProperties: false
+
+ allOf:
+ - if:
+ properties:
+ function:
+ enum:
+ - SPI_FLASH
+ then:
+ properties:
+ groups:
+ enum:
+ - SPI_FLASH1
+ - SPI_FLASH2
+ - if:
+ properties:
+ function:
+ enum:
+ - SPI_FLASH_4BIT
+ then:
+ properties:
+ groups:
+ enum:
+ - SPI_FLASH_4BIT1
+ - SPI_FLASH_4BIT2
+ - if:
+ properties:
+ function:
+ enum:
+ - SPI_NAND
+ then:
+ properties:
+ groups:
+ enum:
+ - SPI_NAND
+ - if:
+ properties:
+ function:
+ enum:
+ - CARD0_EMMC
+ then:
+ properties:
+ groups:
+ enum:
+ - CARD0_EMMC
+ - if:
+ properties:
+ function:
+ enum:
+ - SD_CARD
+ then:
+ properties:
+ groups:
+ enum:
+ - SD_CARD
+ - if:
+ properties:
+ function:
+ enum:
+ - UA0
+ then:
+ properties:
+ groups:
+ enum:
+ - UA0
+ - if:
+ properties:
+ function:
+ enum:
+ - FPGA_IFX
+ then:
+ properties:
+ groups:
+ enum:
+ - FPGA_IFX
+ - if:
+ properties:
+ function:
+ enum:
+ - HDMI_TX
+ then:
+ properties:
+ groups:
+ enum:
+ - HDMI_TX1
+ - HDMI_TX2
+ - HDMI_TX3
+ - if:
+ properties:
+ function:
+ enum:
+ - LCDIF
+ then:
+ properties:
+ groups:
+ enum:
+ - LCDIF
+ - if:
+ properties:
+ function:
+ enum:
+ - USB0_OTG
+ then:
+ properties:
+ groups:
+ enum:
+ - USB0_OTG
+ - if:
+ properties:
+ function:
+ enum:
+ - USB1_OTG
+ then:
+ properties:
+ groups:
+ enum:
+ - USB1_OTG
+
+required:
+ - compatible
+ - reg
+ - "#gpio-cells"
+ - gpio-controller
+ - clocks
+ - resets
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/pinctrl/sppctl-sp7021.h>
+
+ pinctl@9c000100 {
+ compatible = "sunplus,sp7021-pctl";
+ reg = <0x9c000100 0x100>, <0x9c000300 0x80>, <0x9c000380 0x80>,
+ <0x9c0032e4 0x1c>, <0x9c000080 0x20>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ clocks = <&clkc 0x83>;
+ resets = <&rstc 0x73>;
+
+ uart0-pins {
+ function = "UA0";
+ groups = "UA0";
+ };
+
+ spinand0-pins {
+ function = "SPI_NAND";
+ groups = "SPI_NAND";
+ };
+
+ uart1-pins {
+ pins = <
+ SPPCTL_IOPAD(11, SPPCTL_PCTL_G_PMUX, MUXF_UA1_TX, 0)
+ SPPCTL_IOPAD(10, SPPCTL_PCTL_G_PMUX, MUXF_UA1_RX, 0)
+ >;
+ };
+
+ uart2-pins {
+ pins = <
+ SPPCTL_IOPAD(20, SPPCTL_PCTL_G_PMUX, MUXF_UA1_TX, 0)
+ SPPCTL_IOPAD(21, SPPCTL_PCTL_G_PMUX, MUXF_UA1_RX, 0)
+ SPPCTL_IOPAD(22, SPPCTL_PCTL_G_PMUX, MUXF_UA1_RTS, 0)
+ SPPCTL_IOPAD(23, SPPCTL_PCTL_G_PMUX, MUXF_UA1_CTS, 0)
+ >;
+ };
+
+ emmc-pins {
+ function = "CARD0_EMMC";
+ groups = "CARD0_EMMC";
+ };
+
+ sdcard-pins {
+ function = "SD_CARD";
+ groups = "SD_CARD";
+ pins = < SPPCTL_IOPAD(91, SPPCTL_PCTL_G_GPIO, 0, 0) >;
+ };
+
+ hdmi_A_tx1-pins {
+ function = "HDMI_TX";
+ groups = "HDMI_TX1";
+ };
+ hdmi_A_tx2-pins {
+ function = "HDMI_TX";
+ groups = "HDMI_TX2";
+ };
+ hdmi_A_tx3-pins {
+ function = "HDMI_TX";
+ groups = "HDMI_TX3";
+ };
+ };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index faa9c34..723bbe3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15128,6 +15128,15 @@ L: [email protected]
S: Maintained
F: drivers/pinctrl/pinctrl-single.c

+PIN CONTROLLER - SUNPLUS / TIBBO
+M: Dvorkin Dmitry <[email protected]>
+M: Wells Lu <[email protected]>
+L: [email protected] (moderated for non-subscribers)
+S: Maintained
+W: https://sunplus-tibbo.atlassian.net/wiki/spaces/doc/overview
+F: Documentation/devicetree/bindings/pinctrl/sunplus,*
+F: include/dt-bindings/pinctrl/sppctl*
+
PKTCDVD DRIVER
M: [email protected]
S: Orphan
diff --git a/include/dt-bindings/pinctrl/sppctl-sp7021.h b/include/dt-bindings/pinctrl/sppctl-sp7021.h
new file mode 100644
index 0000000..c9e6a07
--- /dev/null
+++ b/include/dt-bindings/pinctrl/sppctl-sp7021.h
@@ -0,0 +1,173 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/* Sunplus SP7021 dt-bindings Pinctrl header file
+ * Copyright (C) Sunplus Tech/Tibbo Tech. 2020
+ * Author: Dvorkin Dmitry <[email protected]>
+ */
+
+#ifndef __DT_BINDINGS_PINCTRL_SPPCTL_SP7021_H__
+#define __DT_BINDINGS_PINCTRL_SPPCTL_SP7021_H__
+
+#include <dt-bindings/pinctrl/sppctl.h>
+
+/* Please don't change the order of the
+ * following defines. They are based on
+ * order of control register define of
+ * MOON2 and MOON4 registers.
+ */
+#define MUXF_GPIO 0
+#define MUXF_IOP 1
+#define MUXF_L2SW_CLK_OUT 2
+#define MUXF_L2SW_MAC_SMI_MDC 3
+#define MUXF_L2SW_LED_FLASH0 4
+#define MUXF_L2SW_LED_FLASH1 5
+#define MUXF_L2SW_LED_ON0 6
+#define MUXF_L2SW_LED_ON1 7
+#define MUXF_L2SW_MAC_SMI_MDIO 8
+#define MUXF_L2SW_P0_MAC_RMII_TXEN 9
+#define MUXF_L2SW_P0_MAC_RMII_TXD0 10
+#define MUXF_L2SW_P0_MAC_RMII_TXD1 11
+#define MUXF_L2SW_P0_MAC_RMII_CRSDV 12
+#define MUXF_L2SW_P0_MAC_RMII_RXD0 13
+#define MUXF_L2SW_P0_MAC_RMII_RXD1 14
+#define MUXF_L2SW_P0_MAC_RMII_RXER 15
+#define MUXF_L2SW_P1_MAC_RMII_TXEN 16
+#define MUXF_L2SW_P1_MAC_RMII_TXD0 17
+#define MUXF_L2SW_P1_MAC_RMII_TXD1 18
+#define MUXF_L2SW_P1_MAC_RMII_CRSDV 19
+#define MUXF_L2SW_P1_MAC_RMII_RXD0 20
+#define MUXF_L2SW_P1_MAC_RMII_RXD1 21
+#define MUXF_L2SW_P1_MAC_RMII_RXER 22
+#define MUXF_DAISY_MODE 23
+#define MUXF_SDIO_CLK 24
+#define MUXF_SDIO_CMD 25
+#define MUXF_SDIO_D0 26
+#define MUXF_SDIO_D1 27
+#define MUXF_SDIO_D2 28
+#define MUXF_SDIO_D3 29
+#define MUXF_PWM0 30
+#define MUXF_PWM1 31
+#define MUXF_PWM2 32
+#define MUXF_PWM3 33
+#define MUXF_PWM4 34
+#define MUXF_PWM5 35
+#define MUXF_PWM6 36
+#define MUXF_PWM7 37
+#define MUXF_ICM0_D 38
+#define MUXF_ICM1_D 39
+#define MUXF_ICM2_D 40
+#define MUXF_ICM3_D 41
+#define MUXF_ICM0_CLK 42
+#define MUXF_ICM1_CLK 43
+#define MUXF_ICM2_CLK 44
+#define MUXF_ICM3_CLK 45
+#define MUXF_SPIM0_INT 46
+#define MUXF_SPIM0_CLK 47
+#define MUXF_SPIM0_EN 48
+#define MUXF_SPIM0_DO 49
+#define MUXF_SPIM0_DI 50
+#define MUXF_SPIM1_INT 51
+#define MUXF_SPIM1_CLK 52
+#define MUXF_SPIM1_EN 53
+#define MUXF_SPIM1_DO 54
+#define MUXF_SPIM1_DI 55
+#define MUXF_SPIM2_INT 56
+#define MUXF_SPIM2_CLK 57
+#define MUXF_SPIM2_EN 58
+#define MUXF_SPIM2_DO 59
+#define MUXF_SPIM2_DI 60
+#define MUXF_SPIM3_INT 61
+#define MUXF_SPIM3_CLK 62
+#define MUXF_SPIM3_EN 63
+#define MUXF_SPIM3_DO 64
+#define MUXF_SPIM3_DI 65
+#define MUXF_SPI0S_INT 66
+#define MUXF_SPI0S_CLK 67
+#define MUXF_SPI0S_EN 68
+#define MUXF_SPI0S_DO 69
+#define MUXF_SPI0S_DI 70
+#define MUXF_SPI1S_INT 71
+#define MUXF_SPI1S_CLK 72
+#define MUXF_SPI1S_EN 73
+#define MUXF_SPI1S_DO 74
+#define MUXF_SPI1S_DI 75
+#define MUXF_SPI2S_INT 76
+#define MUXF_SPI2S_CLK 77
+#define MUXF_SPI2S_EN 78
+#define MUXF_SPI2S_DO 79
+#define MUXF_SPI2S_DI 80
+#define MUXF_SPI3S_INT 81
+#define MUXF_SPI3S_CLK 82
+#define MUXF_SPI3S_EN 83
+#define MUXF_SPI3S_DO 84
+#define MUXF_SPI3S_DI 85
+#define MUXF_I2CM0_CLK 86
+#define MUXF_I2CM0_DAT 87
+#define MUXF_I2CM1_CLK 88
+#define MUXF_I2CM1_DAT 89
+#define MUXF_I2CM2_CLK 90
+#define MUXF_I2CM2_DAT 91
+#define MUXF_I2CM3_CLK 92
+#define MUXF_I2CM3_DAT 93
+#define MUXF_UA1_TX 94
+#define MUXF_UA1_RX 95
+#define MUXF_UA1_CTS 96
+#define MUXF_UA1_RTS 97
+#define MUXF_UA2_TX 98
+#define MUXF_UA2_RX 99
+#define MUXF_UA2_CTS 100
+#define MUXF_UA2_RTS 101
+#define MUXF_UA3_TX 102
+#define MUXF_UA3_RX 103
+#define MUXF_UA3_CTS 104
+#define MUXF_UA3_RTS 105
+#define MUXF_UA4_TX 106
+#define MUXF_UA4_RX 107
+#define MUXF_UA4_CTS 108
+#define MUXF_UA4_RTS 109
+#define MUXF_TIMER0_INT 110
+#define MUXF_TIMER1_INT 111
+#define MUXF_TIMER2_INT 112
+#define MUXF_TIMER3_INT 113
+#define MUXF_GPIO_INT0 114
+#define MUXF_GPIO_INT1 115
+#define MUXF_GPIO_INT2 116
+#define MUXF_GPIO_INT3 117
+#define MUXF_GPIO_INT4 118
+#define MUXF_GPIO_INT5 119
+#define MUXF_GPIO_INT6 120
+#define MUXF_GPIO_INT7 121
+
+#define GROP_SPI_FLASH 122
+#define GROP_SPI_FLASH_4BIT 123
+#define GROP_SPI_NAND 124
+#define GROP_CARD0_EMMC 125
+#define GROP_SD_CARD 126
+#define GROP_UA0 127
+#define GROP_ACHIP_DEBUG 128
+#define GROP_ACHIP_UA2AXI 129
+#define GROP_FPGA_IFX 130
+#define GROP_HDMI_TX 131
+#define GROP_AUD_EXT_ADC_IFX0 132
+#define GROP_AUD_EXT_DAC_IFX0 133
+#define GROP_SPDIF_RX 134
+#define GROP_SPDIF_TX 135
+#define GROP_TDMTX_IFX0 136
+#define GROP_TDMRX_IFX0 137
+#define GROP_PDMRX_IFX0 138
+#define GROP_PCM_IEC_TX 139
+#define GROP_LCDIF 140
+#define GROP_DVD_DSP_DEBUG 141
+#define GROP_I2C_DEBUG 142
+#define GROP_I2C_SLAVE 143
+#define GROP_WAKEUP 144
+#define GROP_UART2AXI 145
+#define GROP_USB0_I2C 146
+#define GROP_USB1_I2C 147
+#define GROP_USB0_OTG 148
+#define GROP_USB1_OTG 149
+#define GROP_UPHY0_DEBUG 150
+#define GROP_UPHY1_DEBUG 151
+#define GROP_UPHY0_EXT 152
+#define GROP_PROBE_PORT 153
+
+#endif
diff --git a/include/dt-bindings/pinctrl/sppctl.h b/include/dt-bindings/pinctrl/sppctl.h
new file mode 100644
index 0000000..7e5c7a6
--- /dev/null
+++ b/include/dt-bindings/pinctrl/sppctl.h
@@ -0,0 +1,40 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/* Sunplus dt-bindings Pinctrl header file
+ * Copyright (C) Sunplus Tech/Tibbo Tech. 2020
+ * Author: Dvorkin Dmitry <[email protected]>
+ */
+
+#ifndef __DT_BINDINGS_PINCTRL_SPPCTL_H__
+#define __DT_BINDINGS_PINCTRL_SPPCTL_H__
+
+#define IOP_G_MASTE (0x01 << 0)
+#define IOP_G_FIRST (0x01 << 1)
+
+#define SPPCTL_PCTL_G_PMUX (0x00 | IOP_G_MASTE)
+#define SPPCTL_PCTL_G_GPIO (IOP_G_FIRST | IOP_G_MASTE)
+#define SPPCTL_PCTL_G_IOPP (IOP_G_FIRST | 0x00)
+
+#define SPPCTL_PCTL_L_OUT (0x01 << 0) /* Output LOW */
+#define SPPCTL_PCTL_L_OU1 (0x01 << 1) /* Output HIGH */
+#define SPPCTL_PCTL_L_INV (0x01 << 2) /* Input Invert */
+#define SPPCTL_PCTL_L_ONV (0x01 << 3) /* Output Invert */
+#define SPPCTL_PCTL_L_ODR (0x01 << 4) /* Output Open Drain */
+
+#define SPPCTL_PCTLE_P(v) ((v) << 24)
+#define SPPCTL_PCTLE_G(v) ((v) << 16)
+#define SPPCTL_PCTLE_F(v) ((v) << 8)
+#define SPPCTL_PCTLE_L(v) ((v) << 0)
+
+#define SPPCTL_PCTLD_P(v) (((v) >> 24) & 0xff)
+#define SPPCTL_PCTLD_G(v) (((v) >> 16) & 0xff)
+#define SPPCTL_PCTLD_F(v) (((v) >> 8) & 0xff)
+#define SPPCTL_PCTLD_L(v) (((v) >> 0) & 0xff)
+
+/*
+ * pack into 32-bit value:
+ * pin#(8bit), typ(8bit), function(8bit), flag(8bit)
+ */
+#define SPPCTL_IOPAD(pin, typ, fun, flg) (((pin) << 24) | ((typ) << 16) | \
+ ((fun) << 8) | (flg))
+
+#endif
--
2.7.4


2021-12-07 04:17:55

by Wells Lu

[permalink] [raw]
Subject: [PATCH v3 2/2] pinctrl: Add driver for Sunplus SP7021

Add driver for Sunplus SP7021.

Signed-off-by: Wells Lu <[email protected]>
---
Changes in V3
- Addressed comments of Mr. Linus Walleij.
- Remove sysfs functions.
- Cleaned up code.

MAINTAINERS | 1 +
drivers/pinctrl/Kconfig | 1 +
drivers/pinctrl/Makefile | 1 +
drivers/pinctrl/sunplus/Kconfig | 21 +
drivers/pinctrl/sunplus/Makefile | 5 +
drivers/pinctrl/sunplus/sppctl.c | 1074 +++++++++++++++++++++++++++++++
drivers/pinctrl/sunplus/sppctl.h | 154 +++++
drivers/pinctrl/sunplus/sppctl_sp7021.c | 536 +++++++++++++++
8 files changed, 1793 insertions(+)
create mode 100644 drivers/pinctrl/sunplus/Kconfig
create mode 100644 drivers/pinctrl/sunplus/Makefile
create mode 100644 drivers/pinctrl/sunplus/sppctl.c
create mode 100644 drivers/pinctrl/sunplus/sppctl.h
create mode 100644 drivers/pinctrl/sunplus/sppctl_sp7021.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 723bbe3..c19bbfd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15135,6 +15135,7 @@ L: [email protected] (moderated for non-subscribers)
S: Maintained
W: https://sunplus-tibbo.atlassian.net/wiki/spaces/doc/overview
F: Documentation/devicetree/bindings/pinctrl/sunplus,*
+F: drivers/pinctrl/sunplus/
F: include/dt-bindings/pinctrl/sppctl*

PKTCDVD DRIVER
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 6a961d5..5aa4e29 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -459,6 +459,7 @@ source "drivers/pinctrl/samsung/Kconfig"
source "drivers/pinctrl/spear/Kconfig"
source "drivers/pinctrl/sprd/Kconfig"
source "drivers/pinctrl/stm32/Kconfig"
+source "drivers/pinctrl/sunplus/Kconfig"
source "drivers/pinctrl/sunxi/Kconfig"
source "drivers/pinctrl/tegra/Kconfig"
source "drivers/pinctrl/ti/Kconfig"
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index 5e63de2..9f1eef3 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -67,6 +67,7 @@ obj-$(CONFIG_PINCTRL_SAMSUNG) += samsung/
obj-$(CONFIG_PINCTRL_SPEAR) += spear/
obj-y += sprd/
obj-$(CONFIG_PINCTRL_STM32) += stm32/
+obj-y += sunplus/
obj-$(CONFIG_PINCTRL_SUNXI) += sunxi/
obj-y += ti/
obj-$(CONFIG_PINCTRL_UNIPHIER) += uniphier/
diff --git a/drivers/pinctrl/sunplus/Kconfig b/drivers/pinctrl/sunplus/Kconfig
new file mode 100644
index 0000000..d9291a7
--- /dev/null
+++ b/drivers/pinctrl/sunplus/Kconfig
@@ -0,0 +1,21 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Sunplus Pin control driver configuration
+#
+
+config PINCTRL_SPPCTL
+ bool "Sunplus SP7021 PinMux and GPIO driver"
+ depends on SOC_SP7021
+ depends on OF && HAS_IOMEM
+ select GENERIC_PINCTRL_GROUPS
+ select GENERIC_PINMUX_FUNCTIONS
+ select GENERIC_PINCONF
+ select PINCONF
+ select PINMUX
+ select GPIOLIB
+ select OF_GPIO
+ help
+ Say Y here to support Sunplus SP7021 pinmux controller.
+ The driver is selected automatically by platform.
+ This driver requires the pinctrl framework.
+ GPIO is provided by the same driver.
diff --git a/drivers/pinctrl/sunplus/Makefile b/drivers/pinctrl/sunplus/Makefile
new file mode 100644
index 0000000..63482da
--- /dev/null
+++ b/drivers/pinctrl/sunplus/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for the Sunplus Pin control drivers.
+#
+obj-$(CONFIG_PINCTRL_SPPCTL) += sppctl.o sppctl_sp7021.o
diff --git a/drivers/pinctrl/sunplus/sppctl.c b/drivers/pinctrl/sunplus/sppctl.c
new file mode 100644
index 0000000..f8b382e
--- /dev/null
+++ b/drivers/pinctrl/sunplus/sppctl.c
@@ -0,0 +1,1074 @@
+// SPDX-License-Identifier: GPL-2.0
+/* SP7021 Pin Controller Driver.
+ * Copyright (C) Sunplus Tech/Tibbo Tech.
+ */
+
+#include <linux/platform_device.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/gpio/driver.h>
+#include <linux/module.h>
+
+#include <dt-bindings/pinctrl/sppctl-sp7021.h>
+#include "../pinctrl-utils.h"
+#include "../core.h"
+#include "sppctl.h"
+
+static void sppctl_func_set(struct sppctl_pdata *pctl, u8 func, u8 val)
+{
+ u32 reg, offset;
+
+ /* Note that upper 16-bit word is mask
+ * and lower 16-bit word is value.
+ * Enable mask before write.
+ */
+ reg = 0x007f0000 | val; /* Set value and enable mask. */
+
+ if (func & 1)
+ reg <<= 8;
+
+ /* Convert function # to register offset. */
+ offset = func & ~1;
+ offset <<= 1;
+
+ dev_dbg(pctl->pctl_dev->dev, "%s(0x%x, 0x%x): offset: 0x%x, reg: 0x%08x\n",
+ __func__, func, val, offset, reg);
+ writel(reg, pctl->moon2_base + offset);
+}
+
+static u8 sppctl_func_get(struct sppctl_pdata *pctl, u8 func)
+{
+ u32 reg, offset;
+ u8 val;
+
+ /* Convert function # to register offset. */
+ offset = func & ~1;
+ offset <<= 1;
+
+ reg = readl(pctl->moon2_base + offset);
+ if (func & 1)
+ val = reg >> 8;
+ else
+ val = reg;
+ val &= 0x7f;
+
+ dev_dbg(pctl->pctl_dev->dev, "%s(0x%x): offset: 0x%x, reg: 0x%08X, val: 0x%x\n",
+ __func__, func, offset, reg, val);
+
+ return val;
+}
+
+static void sppctl_gmx_set(struct sppctl_pdata *pctl, u8 reg_off, u8 bit_off, u8 bit_sz,
+ u8 val)
+{
+ u32 mask, reg;
+
+ /* Note that upper 16-bit word is mask
+ * and lower 16-bit word is value.
+ * Enable mask before write.
+ */
+ mask = ~(~0 << bit_sz);
+ reg = (mask << 16) | (val & mask);
+ reg <<= bit_off;
+ writel(reg, pctl->moon1_base + (reg_off << 2));
+
+ dev_dbg(pctl->pctl_dev->dev, "%s(0x%x, 0x%x, 0x%x, 0x%x): reg: 0x%08X\n",
+ __func__, reg_off, bit_off, bit_sz, val, reg);
+}
+
+static int sppctl_first_get(struct gpio_chip *chip, unsigned int offset)
+{
+ struct sppctl_gpio_chip *spp_gchip = gpiochip_get_data(chip);
+ u32 reg;
+
+ reg = readl(spp_gchip->first_base + SPPCTL_GPIO_OFF_FIRST + R32_ROF(offset));
+
+ dev_dbg(chip->parent, "%s(%u): addr = %p, reg = %08x, val = %d\n",
+ __func__, offset, spp_gchip->first_base + SPPCTL_GPIO_OFF_FIRST +
+ R32_ROF(offset), reg, (int)R32_VAL(reg, R32_BOF(offset)));
+
+ return R32_VAL(reg, R32_BOF(offset));
+}
+
+static int sppctl_master_get(struct gpio_chip *chip, unsigned int offset)
+{
+ struct sppctl_gpio_chip *spp_gchip = gpiochip_get_data(chip);
+ u32 reg;
+
+ reg = readl(spp_gchip->gpioxt_base + SPPCTL_GPIO_OFF_MASTER + R16_ROF(offset));
+
+ dev_dbg(chip->parent, "%s(%u): addr = %p, reg = %08x, val = %d\n",
+ __func__, offset, spp_gchip->gpioxt_base + SPPCTL_GPIO_OFF_MASTER +
+ R16_ROF(offset), reg, (int)R32_VAL(reg, R16_BOF(offset)));
+
+ return R32_VAL(reg, R16_BOF(offset));
+}
+
+static void sppctl_first_master_set(struct gpio_chip *chip, unsigned int offset,
+ enum mux_f_mg first, enum mux_m_ig master)
+{
+ struct sppctl_gpio_chip *spp_gchip = gpiochip_get_data(chip);
+ u32 reg;
+
+ /* FIRST register */
+ if (first != mux_f_keep) {
+ reg = readl(spp_gchip->first_base + SPPCTL_GPIO_OFF_FIRST + R32_ROF(offset));
+ dev_dbg(chip->parent, "First: %08x (%p)\n", reg, spp_gchip->first_base +
+ SPPCTL_GPIO_OFF_FIRST + R32_ROF(offset));
+ if (first != R32_VAL(reg, R32_BOF(offset))) {
+ if (first == mux_f_gpio)
+ reg |= BIT(R32_BOF(offset));
+ else
+ reg &= ~BIT(R32_BOF(offset));
+ dev_dbg(chip->parent, "First: %08x\n", reg);
+ writel(reg, spp_gchip->first_base + SPPCTL_GPIO_OFF_FIRST +
+ R32_ROF(offset));
+ }
+ }
+
+ /* MASTER register */
+ if (master != mux_m_keep) {
+ reg = (BIT(R16_BOF(offset)) << 16);
+ if (master == mux_m_gpio)
+ reg |= BIT(R16_BOF(offset));
+ dev_dbg(chip->parent, "Master: %08x (%p)\n", reg, spp_gchip->gpioxt_base +
+ SPPCTL_GPIO_OFF_MASTER + R16_ROF(offset));
+ writel(reg, spp_gchip->gpioxt_base + SPPCTL_GPIO_OFF_MASTER +
+ R16_ROF(offset));
+ }
+}
+
+static void sppctl_gpio_input_inv_set(struct gpio_chip *chip, unsigned int offset)
+{
+ struct sppctl_gpio_chip *spp_gchip = gpiochip_get_data(chip);
+ u32 reg;
+
+ /* Upper 16-bit word is mask. Lower 16-bit word is value. */
+ reg = (BIT(R16_BOF(offset)) << 16) | BIT(R16_BOF(offset));
+ writel(reg, spp_gchip->gpioxt2_base + SPPCTL_GPIO_OFF_IINV + R16_ROF(offset));
+}
+
+static void sppctl_gpio_output_inv_set(struct gpio_chip *chip, unsigned int offset)
+{
+ struct sppctl_gpio_chip *spp_gchip = gpiochip_get_data(chip);
+ u32 reg;
+
+ /* Upper 16-bit word is mask. Lower 16-bit word is value. */
+ reg = (BIT(R16_BOF(offset)) << 16) | BIT(R16_BOF(offset));
+ writel(reg, spp_gchip->gpioxt2_base + SPPCTL_GPIO_OFF_OINV + R16_ROF(offset));
+}
+
+static int sppctl_gpio_output_od_get(struct gpio_chip *chip, unsigned int offset)
+{
+ struct sppctl_gpio_chip *spp_gchip = gpiochip_get_data(chip);
+ u32 reg;
+
+ reg = readl(spp_gchip->gpioxt2_base + SPPCTL_GPIO_OFF_OD + R16_ROF(offset));
+
+ return R32_VAL(reg, R16_BOF(offset));
+}
+
+static void sppctl_gpio_output_od_set(struct gpio_chip *chip, unsigned int offset,
+ unsigned int val)
+{
+ struct sppctl_gpio_chip *spp_gchip = gpiochip_get_data(chip);
+ u32 reg;
+
+ /* Upper 16-bit word is mask. Lower 16-bit word is value. */
+ reg = (BIT(R16_BOF(offset)) << 16) | ((val & BIT(0)) << R16_BOF(offset));
+ writel(reg, spp_gchip->gpioxt2_base + SPPCTL_GPIO_OFF_OD + R16_ROF(offset));
+}
+
+static int sppctl_gpio_get_direction(struct gpio_chip *chip, unsigned int offset)
+{
+ struct sppctl_gpio_chip *spp_gchip = gpiochip_get_data(chip);
+ u32 reg;
+
+ reg = readl(spp_gchip->gpioxt_base + SPPCTL_GPIO_OFF_OE + R16_ROF(offset));
+
+ return R32_VAL(reg, R16_BOF(offset)) ^ BIT(0);
+}
+
+static int sppctl_gpio_inv_get(struct gpio_chip *chip, unsigned int offset)
+{
+ struct sppctl_gpio_chip *spp_gchip = gpiochip_get_data(chip);
+ u16 inv_off;
+ u32 reg;
+
+ inv_off = SPPCTL_GPIO_OFF_IINV;
+ if (sppctl_gpio_get_direction(chip, offset) == 0)
+ inv_off = SPPCTL_GPIO_OFF_OINV;
+
+ reg = readl(spp_gchip->gpioxt2_base + inv_off + R16_ROF(offset));
+
+ return R32_VAL(reg, R16_BOF(offset));
+}
+
+static int sppctl_gpio_direction_input(struct gpio_chip *chip, unsigned int offset)
+{
+ struct sppctl_gpio_chip *spp_gchip = gpiochip_get_data(chip);
+ u32 reg;
+
+ /* Upper 16-bit word is mask. Lower 16-bit word is value. */
+ reg = (BIT(R16_BOF(offset)) << 16);
+ writel(reg, spp_gchip->gpioxt_base + SPPCTL_GPIO_OFF_OE + R16_ROF(offset));
+
+ return 0;
+}
+
+static int sppctl_gpio_direction_output(struct gpio_chip *chip, unsigned int offset, int val)
+{
+ struct sppctl_gpio_chip *spp_gchip = gpiochip_get_data(chip);
+ u32 reg;
+
+ /* Upper 16-bit word is mask. Lower 16-bit word is value. */
+ reg = (BIT(R16_BOF(offset)) << 16) | BIT(R16_BOF(offset));
+ writel(reg, spp_gchip->gpioxt_base + SPPCTL_GPIO_OFF_OE + R16_ROF(offset));
+
+ if (val < 0)
+ return 0;
+
+ /* Upper 16-bit word is mask. Lower 16-bit word is value. */
+ reg = (BIT(R16_BOF(offset)) << 16) | ((val & BIT(0)) << R16_BOF(offset));
+ writel(reg, spp_gchip->gpioxt_base + SPPCTL_GPIO_OFF_OUT + R16_ROF(offset));
+
+ return 0;
+}
+
+static int sppctl_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+ struct sppctl_gpio_chip *spp_gchip = gpiochip_get_data(chip);
+ u32 reg;
+
+ reg = readl(spp_gchip->gpioxt_base + SPPCTL_GPIO_OFF_IN + R32_ROF(offset));
+
+ return R32_VAL(reg, R32_BOF(offset));
+}
+
+static void sppctl_gpio_set(struct gpio_chip *chip, unsigned int offset, int value)
+{
+ struct sppctl_gpio_chip *spp_gchip = gpiochip_get_data(chip);
+ u32 reg;
+
+ /* Upper 16-bit word is mask. Lower 16-bit word is value. */
+ reg = (BIT(R16_BOF(offset)) << 16) | (value & 0x0001) << R16_BOF(offset);
+ writel(reg, spp_gchip->gpioxt_base + SPPCTL_GPIO_OFF_OUT + R16_ROF(offset));
+}
+
+static int sppctl_gpio_set_config(struct gpio_chip *chip, unsigned int offset,
+ unsigned long config)
+{
+ enum pin_config_param param = pinconf_to_config_param(config);
+ struct sppctl_gpio_chip *spp_gchip = gpiochip_get_data(chip);
+ u16 arg = pinconf_to_config_argument(config);
+ int ret = 0;
+ u32 reg;
+
+ dev_dbg(chip->parent, "%s(%03d, %lX) param: %d, arg: %d\n", __func__,
+ offset, config, param, arg);
+
+ switch (param) {
+ case PIN_CONFIG_DRIVE_OPEN_DRAIN:
+ /* Upper 16-bit word is mask. Lower 16-bit word is value. */
+ reg = (BIT(R16_BOF(offset)) << 16) | BIT(R16_BOF(offset));
+ writel(reg, spp_gchip->gpioxt2_base + SPPCTL_GPIO_OFF_OD + R16_ROF(offset));
+ break;
+
+ case PIN_CONFIG_INPUT_ENABLE:
+ dev_dbg(chip->parent, "%s(%03d, %lX) arg: %d\n", __func__,
+ offset, config, arg);
+ break;
+
+ case PIN_CONFIG_OUTPUT:
+ ret = sppctl_gpio_direction_output(chip, offset, 0);
+ break;
+
+ case PIN_CONFIG_PERSIST_STATE:
+ dev_dbg(chip->parent, "%s(%03d, %lX) not support, param: %d\n", __func__,
+ offset, config, param);
+ ret = -EOPNOTSUPP;
+ break;
+
+ default:
+ dev_dbg(chip->parent, "%s(%03d, %lX) unknown, param: %d\n", __func__,
+ offset, config, param);
+ ret = -EINVAL;
+ break;
+ }
+
+ return ret;
+}
+
+#ifdef CONFIG_DEBUG_FS
+static void sppctl_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
+{
+ const char *label;
+ int i;
+
+ for (i = 0; i < chip->ngpio; i++) {
+ label = gpiochip_is_requested(chip, i);
+ if (!label)
+ label = "";
+
+ seq_printf(s, " gpio-%03d (%-16.16s | %-16.16s)", i + chip->base,
+ chip->names[i], label);
+ seq_printf(s, " %c", sppctl_gpio_get_direction(chip, i) == 0 ? 'O' : 'I');
+ seq_printf(s, ":%d", sppctl_gpio_get(chip, i));
+ seq_printf(s, " %s", (sppctl_first_get(chip, i) ? "gpi" : "mux"));
+ seq_printf(s, " %s", (sppctl_master_get(chip, i) ? "gpi" : "iop"));
+ seq_printf(s, " %s", (sppctl_gpio_inv_get(chip, i) ? "inv" : " "));
+ seq_printf(s, " %s", (sppctl_gpio_output_od_get(chip, i) ? "oDr" : ""));
+ seq_puts(s, "\n");
+ }
+}
+#endif
+
+static int sppctl_gpio_new(struct platform_device *pdev, struct sppctl_pdata *pctl)
+{
+ struct sppctl_gpio_chip *spp_gchip;
+ struct gpio_chip *gchip;
+ int err;
+
+ if (!of_find_property(pdev->dev.of_node, "gpio-controller", NULL)) {
+ dev_err_probe(&pdev->dev, -EINVAL, "Not a gpio-controller!\n");
+ return -EINVAL;
+ }
+
+ spp_gchip = devm_kzalloc(&pdev->dev, sizeof(*spp_gchip), GFP_KERNEL);
+ if (!spp_gchip)
+ return -ENOMEM;
+ pctl->spp_gchip = spp_gchip;
+
+ spp_gchip->gpioxt_base = pctl->gpioxt_base;
+ spp_gchip->gpioxt2_base = pctl->gpioxt2_base;
+ spp_gchip->first_base = pctl->first_base;
+
+ gchip = &spp_gchip->chip;
+ gchip->label = SPPCTL_MODULE_NAME;
+ gchip->parent = &pdev->dev;
+ gchip->owner = THIS_MODULE;
+ gchip->request = gpiochip_generic_request;
+ gchip->free = gpiochip_generic_free;
+ gchip->get_direction = sppctl_gpio_get_direction;
+ gchip->direction_input = sppctl_gpio_direction_input;
+ gchip->direction_output = sppctl_gpio_direction_output;
+ gchip->get = sppctl_gpio_get;
+ gchip->set = sppctl_gpio_set;
+ gchip->set_config = sppctl_gpio_set_config;
+#ifdef CONFIG_DEBUG_FS
+ gchip->dbg_show = sppctl_gpio_dbg_show;
+#endif
+ gchip->base = 0; /* it is main platform GPIO controller */
+ gchip->ngpio = sppctl_gpio_list_sz;
+ gchip->names = sppctl_gpio_list_s;
+ gchip->can_sleep = 0;
+ gchip->of_node = pdev->dev.of_node;
+ gchip->of_gpio_n_cells = 2;
+
+ pctl->pctl_grange.npins = gchip->ngpio;
+ pctl->pctl_grange.base = gchip->base;
+ pctl->pctl_grange.name = gchip->label;
+ pctl->pctl_grange.gc = gchip;
+
+ err = devm_gpiochip_add_data(&pdev->dev, gchip, spp_gchip);
+ if (err) {
+ dev_err_probe(&pdev->dev, err, "Failed to add gpiochip!\n");
+ return err;
+ }
+
+ return 0;
+}
+
+/* pinconf operations */
+static int stpctl_pin_config_get(struct pinctrl_dev *pctldev, unsigned int pin,
+ unsigned long *config)
+{
+ struct sppctl_pdata *pctl = pinctrl_dev_get_drvdata(pctldev);
+ unsigned int param = pinconf_to_config_param(*config);
+ unsigned int arg = 0;
+
+ dev_dbg(pctldev->dev, "%s(%d)\n", __func__, pin);
+
+ switch (param) {
+ case PIN_CONFIG_DRIVE_OPEN_DRAIN:
+ if (!sppctl_gpio_output_od_get(&pctl->spp_gchip->chip, pin))
+ return -EINVAL;
+ break;
+
+ case PIN_CONFIG_OUTPUT:
+ if (!sppctl_first_get(&pctl->spp_gchip->chip, pin))
+ return -EINVAL;
+ if (!sppctl_master_get(&pctl->spp_gchip->chip, pin))
+ return -EINVAL;
+ if (sppctl_gpio_get_direction(&pctl->spp_gchip->chip, pin) != 0)
+ return -EINVAL;
+ arg = sppctl_gpio_get(&pctl->spp_gchip->chip, pin);
+ break;
+
+ default:
+ dev_dbg(pctldev->dev, "%s(%d) skipping, param: 0x%x\n",
+ __func__, pin, param);
+ return -EOPNOTSUPP;
+ }
+ *config = pinconf_to_config_packed(param, arg);
+
+ return 0;
+}
+
+static int stpctl_pin_config_set(struct pinctrl_dev *pctldev, unsigned int pin,
+ unsigned long *configs, unsigned int num_configs)
+{
+ struct sppctl_pdata *pctl = pinctrl_dev_get_drvdata(pctldev);
+ int i = 0;
+
+ dev_dbg(pctldev->dev, "%s(%d, %ld, %d)\n", __func__, pin, *configs, num_configs);
+
+ /* Special handling for IOP */
+ if (configs[i] == 0xFF) {
+ sppctl_first_master_set(&pctl->spp_gchip->chip, pin, mux_f_gpio, mux_m_iop);
+ return 0;
+ }
+
+ for (i = 0; i < num_configs; i++) {
+ if (configs[i] & SPPCTL_PCTL_L_OUT) {
+ dev_dbg(pctldev->dev, "%d: OUT\n", i);
+ sppctl_gpio_direction_output(&pctl->spp_gchip->chip, pin, 0);
+ }
+ if (configs[i] & SPPCTL_PCTL_L_OU1) {
+ dev_dbg(pctldev->dev, "%d: OU1\n", i);
+ sppctl_gpio_direction_output(&pctl->spp_gchip->chip, pin, 1);
+ }
+ if (configs[i] & SPPCTL_PCTL_L_INV) {
+ dev_dbg(pctldev->dev, "%d: INV\n", i);
+ sppctl_gpio_input_inv_set(&pctl->spp_gchip->chip, pin);
+ }
+ if (configs[i] & SPPCTL_PCTL_L_ONV) {
+ dev_dbg(pctldev->dev, "%d: ONV\n", i);
+ sppctl_gpio_output_inv_set(&pctl->spp_gchip->chip, pin);
+ }
+ if (configs[i] & SPPCTL_PCTL_L_ODR) {
+ dev_dbg(pctldev->dev, "%d: ODR\n", i);
+ sppctl_gpio_output_od_set(&pctl->spp_gchip->chip, pin, 1);
+ }
+ }
+
+ return 0;
+}
+
+#ifdef CONFIG_DEBUG_FS
+static void stpctl_config_dbg_show(struct pinctrl_dev *pctldev, struct seq_file *s,
+ unsigned int offset)
+{
+ dev_dbg(pctldev->dev, "%s(%d)\n", __func__, offset);
+ seq_printf(s, " %s", dev_name(pctldev->dev));
+}
+
+static void stpctl_config_group_dbg_show(struct pinctrl_dev *pctldev, struct seq_file *s,
+ unsigned int selector)
+{
+ dev_dbg(pctldev->dev, "%s(%d)\n", __func__, selector);
+}
+
+static void stpctl_config_config_dbg_show(struct pinctrl_dev *pctldev, struct seq_file *s,
+ unsigned long config)
+{
+ dev_dbg(pctldev->dev, "%s(%ld)\n", __func__, config);
+}
+#endif
+
+static const struct pinconf_ops sppctl_pconf_ops = {
+ .is_generic = true,
+ .pin_config_get = stpctl_pin_config_get,
+ .pin_config_set = stpctl_pin_config_set,
+#ifdef CONFIG_DEBUG_FS
+ .pin_config_dbg_show = stpctl_config_dbg_show,
+ .pin_config_group_dbg_show = stpctl_config_group_dbg_show,
+ .pin_config_config_dbg_show = stpctl_config_config_dbg_show,
+#endif
+};
+
+/* pinmux operations */
+static int stpctl_request(struct pinctrl_dev *pctldev, unsigned int offset)
+{
+ dev_dbg(pctldev->dev, "%s(%d)\n", __func__, offset);
+ return 0;
+}
+
+static int stpctl_free(struct pinctrl_dev *pctldev, unsigned int offset)
+{
+ dev_dbg(pctldev->dev, "%s(%d)\n", __func__, offset);
+ return 0;
+}
+
+static int stpctl_get_functions_count(struct pinctrl_dev *pctldev)
+{
+ return sppctl_list_funcs_sz;
+}
+
+static const char *stpctl_get_function_name(struct pinctrl_dev *pctldev,
+ unsigned int selector)
+{
+ return sppctl_list_funcs[selector].name;
+}
+
+static int stpctl_get_function_groups(struct pinctrl_dev *pctldev, unsigned int selector,
+ const char * const **groups, unsigned int *num_groups)
+{
+ struct sppctl_pdata *pctl = pinctrl_dev_get_drvdata(pctldev);
+ const struct sppctl_func *f = &sppctl_list_funcs[selector];
+
+ *num_groups = 0;
+ switch (f->freg) {
+ case f_off_i:
+ case f_off_0: /* gen GPIO/IOP: all groups = all pins */
+ *num_groups = sppctl_gpio_list_sz;
+ *groups = sppctl_gpio_list_s;
+ break;
+
+ case f_off_m: /* pin-mux */
+ *num_groups = sppctl_pmux_list_sz;
+ *groups = sppctl_pmux_list_s;
+ break;
+
+ case f_off_g: /* pin-group */
+ if (!f->grps)
+ break;
+ *num_groups = f->gnum;
+ *groups = &pctl->groups_name[selector * SPPCTL_MAX_GROUPS];
+ break;
+
+ default:
+ dev_err(pctldev->dev, "%s(selector: %d) unknown fOFF %d\n", __func__,
+ selector, f->freg);
+ break;
+ }
+
+ dev_dbg(pctldev->dev, "%s(selector: %d) %d\n", __func__, selector, *num_groups);
+ return 0;
+}
+
+static int stpctl_set_mux(struct pinctrl_dev *pctldev, unsigned int func_selector,
+ unsigned int group_selector)
+{
+ const struct sppctl_func *f = &sppctl_list_funcs[func_selector];
+ struct sppctl_pdata *pctl = pinctrl_dev_get_drvdata(pctldev);
+ struct grp2fp_map g2fpm = pctl->g2fp_maps[group_selector];
+ int i = -1, j = -1;
+
+ dev_dbg(pctldev->dev, "%s(func: %d, grp: %d)\n", __func__,
+ func_selector, group_selector);
+
+ switch (f->freg) {
+ case f_off_0: /* GPIO. detouch from all funcs - ? */
+ for (i = 0; i < sppctl_list_funcs_sz; i++) {
+ if (sppctl_list_funcs[i].freg != f_off_m)
+ continue;
+ j++;
+ if (sppctl_func_get(pctl, j) != group_selector)
+ continue;
+ sppctl_func_set(pctl, j, 0);
+ }
+ break;
+
+ case f_off_m: /* Mux */
+ sppctl_first_master_set(&pctl->spp_gchip->chip, group_selector,
+ mux_f_mux, mux_m_keep);
+ sppctl_func_set(pctl, func_selector - 2, (group_selector == 0) ?
+ group_selector : group_selector - 7);
+ break;
+
+ case f_off_g: /* Group */
+ for (i = 0; i < f->grps[g2fpm.g_idx].pnum; i++)
+ sppctl_first_master_set(&pctl->spp_gchip->chip,
+ f->grps[g2fpm.g_idx].pins[i],
+ mux_f_mux, mux_m_keep);
+ sppctl_gmx_set(pctl, f->roff, f->boff, f->blen, f->grps[g2fpm.g_idx].gval);
+ break;
+
+ case f_off_i: /* IOP */
+ sppctl_first_master_set(&pctl->spp_gchip->chip, group_selector,
+ mux_f_gpio, mux_m_iop);
+ break;
+
+ default:
+ dev_err(pctldev->dev, "%s(func_selector: %d) unknown f_off: %d\n",
+ __func__, func_selector, f->freg);
+ break;
+ }
+
+ return 0;
+}
+
+static int stpctl_gpio_request_enable(struct pinctrl_dev *pctldev,
+ struct pinctrl_gpio_range *range, unsigned int offset)
+{
+ struct sppctl_pdata *pctl = pinctrl_dev_get_drvdata(pctldev);
+ struct pin_desc *pdesc;
+ int g_f, g_m;
+
+ dev_dbg(pctldev->dev, "%s(%d)\n", __func__, offset);
+
+ g_f = sppctl_first_get(&pctl->spp_gchip->chip, offset);
+ g_m = sppctl_master_get(&pctl->spp_gchip->chip, offset);
+ if (g_f == mux_f_gpio && g_m == mux_m_gpio)
+ return 0;
+
+ pdesc = pin_desc_get(pctldev, offset);
+ if (pdesc->mux_owner)
+ return -EACCES;
+
+ sppctl_first_master_set(&pctl->spp_gchip->chip, offset, mux_f_gpio, mux_m_gpio);
+ return 0;
+}
+
+static void stpctl_gpio_disable_free(struct pinctrl_dev *pctldev,
+ struct pinctrl_gpio_range *range, unsigned int offset)
+{
+ dev_dbg(pctldev->dev, "%s(%d)\n", __func__, offset);
+}
+
+static int stpctl_gpio_set_direction(struct pinctrl_dev *pctldev,
+ struct pinctrl_gpio_range *range, unsigned int offset,
+ bool input)
+{
+ dev_dbg(pctldev->dev, "%s(%d,%d)\n", __func__, offset, input);
+ return 0;
+}
+
+static const struct pinmux_ops sppctl_pinmux_ops = {
+ .request = stpctl_request,
+ .free = stpctl_free,
+ .get_functions_count = stpctl_get_functions_count,
+ .get_function_name = stpctl_get_function_name,
+ .get_function_groups = stpctl_get_function_groups,
+ .set_mux = stpctl_set_mux,
+ .gpio_request_enable = stpctl_gpio_request_enable,
+ .gpio_disable_free = stpctl_gpio_disable_free,
+ .gpio_set_direction = stpctl_gpio_set_direction,
+ .strict = 1
+};
+
+/* pinctrl operations */
+static int stpctl_get_groups_count(struct pinctrl_dev *pctldev)
+{
+ struct sppctl_pdata *pctl = pinctrl_dev_get_drvdata(pctldev);
+
+ return pctl->unq_grps_sz;
+}
+
+static const char *stpctl_get_group_name(struct pinctrl_dev *pctldev, unsigned int selector)
+{
+ struct sppctl_pdata *pctl = pinctrl_dev_get_drvdata(pctldev);
+
+ return pctl->unq_grps[selector];
+}
+
+static int stpctl_get_group_pins(struct pinctrl_dev *pctldev, unsigned int selector,
+ const unsigned int **pins, unsigned int *num_pins)
+{
+ struct sppctl_pdata *pctl = pinctrl_dev_get_drvdata(pctldev);
+ struct grp2fp_map g2fpm = pctl->g2fp_maps[selector];
+ const struct sppctl_func *f;
+
+ f = &sppctl_list_funcs[g2fpm.f_idx];
+ dev_dbg(pctldev->dev, "%s(%d), f_idx: %d, g_idx: %d, freg: %d\n",
+ __func__, selector, g2fpm.f_idx, g2fpm.g_idx, f->freg);
+
+ *num_pins = 0;
+
+ /* MUX | GPIO | IOP: 1 pin -> 1 group */
+ if (f->freg != f_off_g) {
+ *num_pins = 1;
+ *pins = &sppctl_pins_gpio[selector];
+ return 0;
+ }
+
+ /* IOP (several pins at once in a group) */
+ if (!f->grps)
+ return 0;
+ if (f->gnum < 1)
+ return 0;
+
+ *num_pins = f->grps[g2fpm.g_idx].pnum;
+ *pins = f->grps[g2fpm.g_idx].pins;
+
+ return 0;
+}
+
+#ifdef CONFIG_DEBUG_FS
+static void stpctl_pin_dbg_show(struct pinctrl_dev *pctldev, struct seq_file *s,
+ unsigned int offset)
+{
+ struct sppctl_pdata *pctl = pinctrl_dev_get_drvdata(pctldev);
+ const char *tmpp;
+ u8 g_f, g_m;
+
+ seq_printf(s, "%s", dev_name(pctldev->dev));
+ g_f = sppctl_first_get(&pctl->spp_gchip->chip, offset);
+ g_m = sppctl_master_get(&pctl->spp_gchip->chip, offset);
+
+ tmpp = "?";
+ if (g_f && g_m)
+ tmpp = "GPIO";
+ if (g_f && !g_m)
+ tmpp = " IOP";
+ if (!g_f)
+ tmpp = " MUX";
+ seq_printf(s, " %s", tmpp);
+}
+#endif
+
+static int stpctl_dt_node_to_map(struct pinctrl_dev *pctldev, struct device_node *np_config,
+ struct pinctrl_map **map, unsigned int *num_maps)
+{
+ struct sppctl_pdata *pctl = pinctrl_dev_get_drvdata(pctldev);
+ int nmG = of_property_count_strings(np_config, "groups");
+ const struct sppctl_func *f = NULL;
+ struct device_node *parent;
+ unsigned long *configs;
+ struct property *prop;
+ const char *s_f, *s_g;
+ u8 p_p, p_g, p_f, p_l;
+ const __be32 *list;
+ u32 dt_pin, dt_fun;
+ int i, size = 0;
+
+ list = of_get_property(np_config, "pins", &size);
+
+ if (nmG <= 0)
+ nmG = 0;
+
+ parent = of_get_parent(np_config);
+ *num_maps = size / sizeof(*list);
+
+ /* Check if out of range or invalid? */
+ for (i = 0; i < (*num_maps); i++) {
+ dt_pin = be32_to_cpu(list[i]);
+ p_p = SPPCTL_PCTLD_P(dt_pin);
+ p_g = SPPCTL_PCTLD_G(dt_pin);
+
+ if (p_p >= sppctl_pins_all_sz) {
+ dev_dbg(pctldev->dev, "Invalid pin property at index %d (0x%08x)\n",
+ i, dt_pin);
+ return -EINVAL;
+ }
+ }
+
+ *map = kcalloc(*num_maps + nmG, sizeof(**map), GFP_KERNEL);
+ for (i = 0; i < (*num_maps); i++) {
+ dt_pin = be32_to_cpu(list[i]);
+ p_p = SPPCTL_PCTLD_P(dt_pin);
+ p_g = SPPCTL_PCTLD_G(dt_pin);
+ p_f = SPPCTL_PCTLD_F(dt_pin);
+ p_l = SPPCTL_PCTLD_L(dt_pin);
+ (*map)[i].name = parent->name;
+ dev_dbg(pctldev->dev, "map [%d]=%08x, p=%d, g=%d, f=%d, l=%d\n",
+ i, dt_pin, p_p, p_g, p_f, p_l);
+
+ if (p_g == SPPCTL_PCTL_G_GPIO) {
+ (*map)[i].type = PIN_MAP_TYPE_CONFIGS_PIN;
+ (*map)[i].data.configs.num_configs = 1;
+ (*map)[i].data.configs.group_or_pin = pin_get_name(pctldev, p_p);
+ configs = kcalloc(1, sizeof(*configs), GFP_KERNEL);
+ *configs = p_l;
+ (*map)[i].data.configs.configs = configs;
+
+ dev_dbg(pctldev->dev, "%s(%d) = 0x%x\n",
+ (*map)[i].data.configs.group_or_pin, p_p, p_l);
+ } else if (p_g == SPPCTL_PCTL_G_IOPP) {
+ (*map)[i].type = PIN_MAP_TYPE_CONFIGS_PIN;
+ (*map)[i].data.configs.num_configs = 1;
+ (*map)[i].data.configs.group_or_pin = pin_get_name(pctldev, p_p);
+ configs = kcalloc(1, sizeof(*configs), GFP_KERNEL);
+ *configs = 0xFF;
+ (*map)[i].data.configs.configs = configs;
+
+ dev_dbg(pctldev->dev, "%s(%d) = 0x%x\n",
+ (*map)[i].data.configs.group_or_pin, p_p, p_l);
+ } else {
+ (*map)[i].type = PIN_MAP_TYPE_MUX_GROUP;
+ (*map)[i].data.mux.function = sppctl_list_funcs[p_f].name;
+ (*map)[i].data.mux.group = pin_get_name(pctldev, p_p);
+
+ dev_dbg(pctldev->dev, "f->p: %s(%d)->%s(%d)\n",
+ (*map)[i].data.mux.function, p_f,
+ (*map)[i].data.mux.group, p_p);
+ }
+ }
+
+ /* Handle pin-group function. */
+ if (nmG > 0 && of_property_read_string(np_config, "function", &s_f) == 0) {
+ dev_dbg(pctldev->dev, "found func: %s\n", s_f);
+ of_property_for_each_string(np_config, "groups", prop, s_g) {
+ dev_dbg(pctldev->dev, " %s: %s\n", s_f, s_g);
+ (*map)[*num_maps].type = PIN_MAP_TYPE_MUX_GROUP;
+ (*map)[*num_maps].data.mux.function = s_f;
+ (*map)[*num_maps].data.mux.group = s_g;
+ dev_dbg(pctldev->dev, "f->g: %s->%s\n",
+ (*map)[*num_maps].data.mux.function,
+ (*map)[*num_maps].data.mux.group);
+ (*num_maps)++;
+ }
+ }
+
+ /* Handle zero function. */
+ list = of_get_property(np_config, "zero_func", &size);
+ if (list) {
+ for (i = 0; i < (size / sizeof(*list)); i++) {
+ dt_fun = be32_to_cpu(list[i]);
+ if (dt_fun >= sppctl_list_funcs_sz) {
+ dev_err(pctldev->dev, "Zero-func %d out of range!\n",
+ dt_fun);
+ continue;
+ }
+
+ f = &sppctl_list_funcs[dt_fun];
+ switch (f->freg) {
+ case f_off_m:
+ dev_dbg(pctldev->dev, "Zero-func: %d (%s)\n",
+ dt_fun, f->name);
+ sppctl_func_set(pctl, dt_fun - 2, 0);
+ break;
+
+ case f_off_g:
+ dev_dbg(pctldev->dev, "zero-group: %d (%s)\n",
+ dt_fun, f->name);
+ sppctl_gmx_set(pctl, f->roff, f->boff, f->blen, 0);
+ break;
+
+ default:
+ dev_err(pctldev->dev, "Wrong zero-group: %d (%s)\n",
+ dt_fun, f->name);
+ break;
+ }
+ }
+ }
+
+ of_node_put(parent);
+ dev_dbg(pctldev->dev, "%d pins mapped\n", *num_maps);
+ return 0;
+}
+
+static void stpctl_dt_free_map(struct pinctrl_dev *pctldev, struct pinctrl_map *map,
+ unsigned int num_maps)
+{
+ dev_dbg(pctldev->dev, "%s(%d)\n", __func__, num_maps);
+ pinctrl_utils_free_map(pctldev, map, num_maps);
+}
+
+static const struct pinctrl_ops sppctl_pctl_ops = {
+ .get_groups_count = stpctl_get_groups_count,
+ .get_group_name = stpctl_get_group_name,
+ .get_group_pins = stpctl_get_group_pins,
+#ifdef CONFIG_DEBUG_FS
+ .pin_dbg_show = stpctl_pin_dbg_show,
+#endif
+ .dt_node_to_map = stpctl_dt_node_to_map,
+ .dt_free_map = stpctl_dt_free_map,
+};
+
+/* platform driver functions */
+static int sppctl_group_groups(struct platform_device *pdev)
+{
+ struct sppctl_pdata *sppctl = pdev->dev.platform_data;
+ const char *name;
+ int i, k, j;
+
+ /* Fill array of all groups. */
+ sppctl->unq_grps = NULL;
+ sppctl->unq_grps_sz = sppctl_gpio_list_sz;
+
+ /* Calculate unique group names array size. */
+ for (i = 0; i < sppctl_list_funcs_sz; i++)
+ if (sppctl_list_funcs[i].freg == f_off_g)
+ sppctl->unq_grps_sz += sppctl_list_funcs[i].gnum;
+
+ /* Fill up unique group names array. */
+ sppctl->unq_grps = devm_kzalloc(&pdev->dev, (sppctl->unq_grps_sz + 1) *
+ sizeof(char *), GFP_KERNEL);
+ if (!sppctl->unq_grps)
+ return -ENOMEM;
+
+ sppctl->g2fp_maps = devm_kzalloc(&pdev->dev, (sppctl->unq_grps_sz + 1) *
+ sizeof(struct grp2fp_map), GFP_KERNEL);
+ if (!sppctl->g2fp_maps)
+ return -ENOMEM;
+
+ sppctl->groups_name = devm_kzalloc(&pdev->dev, sppctl_list_funcs_sz *
+ SPPCTL_MAX_GROUPS * sizeof(char *), GFP_KERNEL);
+ if (!sppctl->groups_name)
+ return -ENOMEM;
+
+ /* gpio */
+ for (i = 0; i < sppctl_gpio_list_sz; i++) {
+ sppctl->unq_grps[i] = sppctl_gpio_list_s[i];
+ sppctl->g2fp_maps[i].f_idx = 0;
+ sppctl->g2fp_maps[i].g_idx = i;
+ }
+
+ /* groups */
+ j = sppctl_gpio_list_sz;
+ for (i = 0; i < sppctl_list_funcs_sz; i++) {
+ if (sppctl_list_funcs[i].freg != f_off_g)
+ continue;
+
+ for (k = 0; k < sppctl_list_funcs[i].gnum; k++) {
+ name = sppctl_list_funcs[i].grps[k].name;
+ sppctl->groups_name[i * SPPCTL_MAX_GROUPS + k] = name;
+ sppctl->unq_grps[j] = name;
+ sppctl->g2fp_maps[j].f_idx = i;
+ sppctl->g2fp_maps[j].g_idx = k;
+ j++;
+ }
+ }
+
+ dev_dbg(&pdev->dev, "funcs: %zd unq_grps: %zd\n", sppctl_list_funcs_sz,
+ sppctl->unq_grps_sz);
+ return 0;
+}
+
+static int sppctl_pinctrl_init(struct platform_device *pdev)
+{
+ struct device_node *np = of_node_get(pdev->dev.of_node);
+ struct sppctl_pdata *sppctl = pdev->dev.platform_data;
+ int err;
+
+ /* Initialize pctl_desc */
+ sppctl->pctl_desc.owner = THIS_MODULE;
+ sppctl->pctl_desc.name = dev_name(&pdev->dev);
+ sppctl->pctl_desc.pins = &sppctl_pins_all[0];
+ sppctl->pctl_desc.npins = sppctl_pins_all_sz;
+ sppctl->pctl_desc.pctlops = &sppctl_pctl_ops;
+ sppctl->pctl_desc.confops = &sppctl_pconf_ops;
+ sppctl->pctl_desc.pmxops = &sppctl_pinmux_ops;
+
+ err = sppctl_group_groups(pdev);
+ if (err) {
+ of_node_put(np);
+ return err;
+ }
+
+ err = devm_pinctrl_register_and_init(&pdev->dev, &sppctl->pctl_desc,
+ sppctl, &sppctl->pctl_dev);
+ if (err) {
+ dev_err_probe(&pdev->dev, err, "Failed to register pinctrl!\n");
+ of_node_put(np);
+ return err;
+ }
+
+ pinctrl_enable(sppctl->pctl_dev);
+ return 0;
+}
+
+static int sppctl_resource_map(struct platform_device *pdev, struct sppctl_pdata *sppctl)
+{
+ struct resource *rp;
+ int ret;
+
+ /* MOON2 registers */
+ rp = platform_get_resource_byname(pdev, IORESOURCE_MEM, "moon2");
+ sppctl->moon2_base = devm_ioremap_resource(&pdev->dev, rp);
+ if (IS_ERR(sppctl->moon2_base)) {
+ ret = PTR_ERR(sppctl->moon2_base);
+ goto ioremap_failed;
+ }
+ dev_dbg(&pdev->dev, "MOON2: %pr\n", rp);
+
+ /* GPIOXT registers */
+ rp = platform_get_resource_byname(pdev, IORESOURCE_MEM, "gpioxt");
+ sppctl->gpioxt_base = devm_ioremap_resource(&pdev->dev, rp);
+ if (IS_ERR(sppctl->gpioxt_base)) {
+ ret = PTR_ERR(sppctl->gpioxt_base);
+ goto ioremap_failed;
+ }
+ dev_dbg(&pdev->dev, "GPIOXT: %pr\n", rp);
+
+ /* GPIOXT 2 registers */
+ rp = platform_get_resource_byname(pdev, IORESOURCE_MEM, "gpioxt2");
+ sppctl->gpioxt2_base = devm_ioremap_resource(&pdev->dev, rp);
+ if (IS_ERR(sppctl->gpioxt2_base)) {
+ ret = PTR_ERR(sppctl->gpioxt2_base);
+ goto ioremap_failed;
+ }
+ dev_dbg(&pdev->dev, "GPIOXT2: %pr\n", rp);
+
+ /* FIRST registers */
+ rp = platform_get_resource_byname(pdev, IORESOURCE_MEM, "first");
+ sppctl->first_base = devm_ioremap_resource(&pdev->dev, rp);
+ if (IS_ERR(sppctl->first_base)) {
+ ret = PTR_ERR(sppctl->first_base);
+ goto ioremap_failed;
+ }
+ dev_dbg(&pdev->dev, "FIRST: %pr\n", rp);
+
+ /* MOON1 registers */
+ rp = platform_get_resource_byname(pdev, IORESOURCE_MEM, "moon1");
+ sppctl->moon1_base = devm_ioremap_resource(&pdev->dev, rp);
+ if (IS_ERR(sppctl->moon1_base)) {
+ ret = PTR_ERR(sppctl->moon1_base);
+ goto ioremap_failed;
+ }
+ dev_dbg(&pdev->dev, "MOON1: %pr\n", rp);
+
+ return 0;
+
+ioremap_failed:
+ dev_err_probe(&pdev->dev, ret, "ioremap failed!\n");
+ return ret;
+}
+
+static int sppctl_probe(struct platform_device *pdev)
+{
+ struct sppctl_pdata *sppctl;
+ int ret;
+
+ sppctl = devm_kzalloc(&pdev->dev, sizeof(*sppctl), GFP_KERNEL);
+ if (!sppctl)
+ return -ENOMEM;
+ pdev->dev.platform_data = sppctl;
+
+ ret = sppctl_resource_map(pdev, sppctl);
+ if (ret)
+ return ret;
+
+ ret = sppctl_gpio_new(pdev, sppctl);
+ if (ret)
+ return ret;
+
+ ret = sppctl_pinctrl_init(pdev);
+ if (ret)
+ return ret;
+
+ pinctrl_add_gpio_range(sppctl->pctl_dev, &sppctl->pctl_grange);
+ dev_info(&pdev->dev, "SP7021 PinCtrl by Sunplus/Tibbo Tech. (C) 2020");
+
+ return 0;
+}
+
+static int sppctl_remove(struct platform_device *pdev)
+{
+ struct sppctl_pdata *sppctl = pdev->dev.platform_data;
+
+ devm_pinctrl_unregister(&pdev->dev, sppctl->pctl_dev);
+ return 0;
+}
+
+static const struct of_device_id sppctl_match_table[] = {
+ { .compatible = "sunplus,sp7021-pctl" },
+ { /* zero */ }
+};
+
+static struct platform_driver sppctl_pinctrl_driver = {
+ .driver = {
+ .name = SPPCTL_MODULE_NAME,
+ .owner = THIS_MODULE,
+ .of_match_table = sppctl_match_table,
+ },
+ .probe = sppctl_probe,
+ .remove = sppctl_remove,
+};
+builtin_platform_driver(sppctl_pinctrl_driver)
+
+MODULE_AUTHOR("Dvorkin Dmitry <[email protected]>");
+MODULE_AUTHOR("Wells Lu <[email protected]>");
+MODULE_DESCRIPTION("Sunplus SP7021 Pin Control and GPIO driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/pinctrl/sunplus/sppctl.h b/drivers/pinctrl/sunplus/sppctl.h
new file mode 100644
index 0000000..49a59fe
--- /dev/null
+++ b/drivers/pinctrl/sunplus/sppctl.h
@@ -0,0 +1,154 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* SP7021 Pin Controller Driver.
+ * Copyright (C) Sunplus Tech/Tibbo Tech.
+ */
+
+#ifndef __SPPCTL_H__
+#define __SPPCTL_H__
+
+#define SPPCTL_MODULE_NAME "sppctl_sp7021"
+#define SPPCTL_MAX_GROUPS 5
+
+#define SPPCTL_GPIO_OFF_FIRST 0x00
+#define SPPCTL_GPIO_OFF_MASTER 0x00
+#define SPPCTL_GPIO_OFF_OE 0x20
+#define SPPCTL_GPIO_OFF_OUT 0x40
+#define SPPCTL_GPIO_OFF_IN 0x60
+#define SPPCTL_GPIO_OFF_IINV 0x00
+#define SPPCTL_GPIO_OFF_OINV 0x20
+#define SPPCTL_GPIO_OFF_OD 0x40
+
+/* (/16)*4 */
+#define R16_ROF(r) (((r) >> 4) << 2)
+#define R16_BOF(r) ((r) % 16)
+
+/* (/32)*4 */
+#define R32_ROF(r) (((r) >> 5) << 2)
+#define R32_BOF(r) ((r) % 32)
+#define R32_VAL(r, boff) (((r) >> (boff)) & BIT(0))
+
+#define FNCE(n, r, o, bo, bl, g) { \
+ .name = n, \
+ .freg = r, \
+ .roff = o, \
+ .boff = bo, \
+ .blen = bl, \
+ .grps = (g), \
+ .gnum = ARRAY_SIZE(g), \
+}
+
+#define FNCN(n, r, o, bo, bl) { \
+ .name = n, \
+ .freg = r, \
+ .roff = o, \
+ .boff = bo, \
+ .blen = bl, \
+ .grps = NULL, \
+ .gnum = 0, \
+}
+
+#define EGRP(n, v, p) { \
+ .name = n, \
+ .gval = (v), \
+ .pins = (p), \
+ .pnum = ARRAY_SIZE(p), \
+}
+
+/* FIRST register:
+ * 0: MUX
+ * 1: GPIO/IOP
+ * 2: No change
+ */
+enum mux_f_mg {
+ mux_f_mux = 0,
+ mux_f_gpio = 1,
+ mux_f_keep = 2,
+};
+
+/* MASTER register:
+ * 0: IOP
+ * 1: GPIO
+ * 2: No change
+ */
+enum mux_m_ig {
+ mux_m_iop = 0,
+ mux_m_gpio = 1,
+ mux_m_keep = 2,
+};
+
+enum f_off {
+ f_off_0, /* nowhere */
+ f_off_m, /* mux registers */
+ f_off_g, /* group registers */
+ f_off_i, /* iop registers */
+};
+
+struct grp2fp_map {
+ u16 f_idx; /* function index */
+ u16 g_idx; /* pins/group index */
+};
+
+struct sppctl_sdata {
+ u8 i;
+ u8 ridx;
+ struct sppctl_pdata *pdata;
+};
+
+struct sppctl_gpio_chip {
+ void __iomem *gpioxt_base; /* MASTER, OE, OUT, IN */
+ void __iomem *gpioxt2_base; /* I_INV, O_INV, OD */
+ void __iomem *first_base; /* GPIO_FIRST */
+
+ struct gpio_chip chip;
+};
+
+struct sppctl_pdata {
+ /* base addresses */
+ void __iomem *moon2_base; /* MOON2 */
+ void __iomem *gpioxt_base; /* MASTER, OE, OUT, IN */
+ void __iomem *gpioxt2_base; /* I_INV, O_INV, OD */
+ void __iomem *first_base; /* FIRST */
+ void __iomem *moon1_base; /* MOON1 */
+
+ /* pinctrl and gpio-chip */
+ struct pinctrl_desc pctl_desc;
+ struct pinctrl_dev *pctl_dev;
+ struct pinctrl_gpio_range pctl_grange;
+ struct sppctl_gpio_chip *spp_gchip;
+
+ /* others */
+ char const **unq_grps;
+ struct grp2fp_map *g2fp_maps;
+ size_t unq_grps_sz;
+ const char **groups_name;
+};
+
+struct sppctl_grp {
+ const char * const name;
+ const u8 gval; /* group number */
+ const unsigned * const pins; /* list of pins */
+ const unsigned int pnum; /* number of pins */
+};
+
+struct sppctl_func {
+ const char * const name;
+ const enum f_off freg; /* function register type */
+ const u8 roff; /* register offset */
+ const u8 boff; /* bit offset */
+ const u8 blen; /* bit length */
+ const struct sppctl_grp * const grps; /* list of groups */
+ const unsigned int gnum; /* number of groups */
+};
+
+extern const struct sppctl_func sppctl_list_funcs[];
+extern const char * const sppctl_pmux_list_s[];
+extern const char * const sppctl_gpio_list_s[];
+extern const struct pinctrl_pin_desc sppctl_pins_all[];
+extern const unsigned int sppctl_pins_gpio[];
+
+extern const size_t sppctl_list_funcs_sz;
+extern const size_t sppctl_pmux_list_sz;
+extern const size_t sppctl_gpio_list_sz;
+extern const size_t sppctl_pins_all_sz;
+
+#endif
diff --git a/drivers/pinctrl/sunplus/sppctl_sp7021.c b/drivers/pinctrl/sunplus/sppctl_sp7021.c
new file mode 100644
index 0000000..457342e
--- /dev/null
+++ b/drivers/pinctrl/sunplus/sppctl_sp7021.c
@@ -0,0 +1,536 @@
+// SPDX-License-Identifier: GPL-2.0
+/* SP7021 Pin Controller Driver.
+ * Copyright (C) Sunplus Tech/Tibbo Tech.
+ */
+
+#include <linux/gpio/driver.h>
+
+#include "sppctl.h"
+
+#define D_PIS(x, y) "P" __stringify(x) "_0" __stringify(y)
+#define D(x, y) ((x) * 8 + (y))
+#define P(x, y) PINCTRL_PIN(D(x, y), D_PIS(x, y))
+
+const char * const sppctl_gpio_list_s[] = {
+ D_PIS(0, 0), D_PIS(0, 1), D_PIS(0, 2), D_PIS(0, 3),
+ D_PIS(0, 4), D_PIS(0, 5), D_PIS(0, 6), D_PIS(0, 7),
+ D_PIS(1, 0), D_PIS(1, 1), D_PIS(1, 2), D_PIS(1, 3),
+ D_PIS(1, 4), D_PIS(1, 5), D_PIS(1, 6), D_PIS(1, 7),
+ D_PIS(2, 0), D_PIS(2, 1), D_PIS(2, 2), D_PIS(2, 3),
+ D_PIS(2, 4), D_PIS(2, 5), D_PIS(2, 6), D_PIS(2, 7),
+ D_PIS(3, 0), D_PIS(3, 1), D_PIS(3, 2), D_PIS(3, 3),
+ D_PIS(3, 4), D_PIS(3, 5), D_PIS(3, 6), D_PIS(3, 7),
+ D_PIS(4, 0), D_PIS(4, 1), D_PIS(4, 2), D_PIS(4, 3),
+ D_PIS(4, 4), D_PIS(4, 5), D_PIS(4, 6), D_PIS(4, 7),
+ D_PIS(5, 0), D_PIS(5, 1), D_PIS(5, 2), D_PIS(5, 3),
+ D_PIS(5, 4), D_PIS(5, 5), D_PIS(5, 6), D_PIS(5, 7),
+ D_PIS(6, 0), D_PIS(6, 1), D_PIS(6, 2), D_PIS(6, 3),
+ D_PIS(6, 4), D_PIS(6, 5), D_PIS(6, 6), D_PIS(6, 7),
+ D_PIS(7, 0), D_PIS(7, 1), D_PIS(7, 2), D_PIS(7, 3),
+ D_PIS(7, 4), D_PIS(7, 5), D_PIS(7, 6), D_PIS(7, 7),
+ D_PIS(8, 0), D_PIS(8, 1), D_PIS(8, 2), D_PIS(8, 3),
+ D_PIS(8, 4), D_PIS(8, 5), D_PIS(8, 6), D_PIS(8, 7),
+ D_PIS(9, 0), D_PIS(9, 1), D_PIS(9, 2), D_PIS(9, 3),
+ D_PIS(9, 4), D_PIS(9, 5), D_PIS(9, 6), D_PIS(9, 7),
+ D_PIS(10, 0), D_PIS(10, 1), D_PIS(10, 2), D_PIS(10, 3),
+ D_PIS(10, 4), D_PIS(10, 5), D_PIS(10, 6), D_PIS(10, 7),
+ D_PIS(11, 0), D_PIS(11, 1), D_PIS(11, 2), D_PIS(11, 3),
+ D_PIS(11, 4), D_PIS(11, 5), D_PIS(11, 6), D_PIS(11, 7),
+ D_PIS(12, 0), D_PIS(12, 1), D_PIS(12, 2)
+};
+
+const size_t sppctl_gpio_list_sz = ARRAY_SIZE(sppctl_gpio_list_s);
+
+/* function: GPIO. list of groups (pins) */
+const unsigned int sppctl_pins_gpio[] = {
+ D(0, 0), D(0, 1), D(0, 2), D(0, 3), D(0, 4), D(0, 5), D(0, 6), D(0, 7),
+ D(1, 0), D(1, 1), D(1, 2), D(1, 3), D(1, 4), D(1, 5), D(1, 6), D(1, 7),
+ D(2, 0), D(2, 1), D(2, 2), D(2, 3), D(2, 4), D(2, 5), D(2, 6), D(2, 7),
+ D(3, 0), D(3, 1), D(3, 2), D(3, 3), D(3, 4), D(3, 5), D(3, 6), D(3, 7),
+ D(4, 0), D(4, 1), D(4, 2), D(4, 3), D(4, 4), D(4, 5), D(4, 6), D(4, 7),
+ D(5, 0), D(5, 1), D(5, 2), D(5, 3), D(5, 4), D(5, 5), D(5, 6), D(5, 7),
+ D(6, 0), D(6, 1), D(6, 2), D(6, 3), D(6, 4), D(6, 5), D(6, 6), D(6, 7),
+ D(7, 0), D(7, 1), D(7, 2), D(7, 3), D(7, 4), D(7, 5), D(7, 6), D(7, 7),
+ D(8, 0), D(8, 1), D(8, 2), D(8, 3), D(8, 4), D(8, 5), D(8, 6), D(8, 7),
+ D(9, 0), D(9, 1), D(9, 2), D(9, 3), D(9, 4), D(9, 5), D(9, 6), D(9, 7),
+ D(10, 0), D(10, 1), D(10, 2), D(10, 3), D(10, 4), D(10, 5), D(10, 6), D(10, 7),
+ D(11, 0), D(11, 1), D(11, 2), D(11, 3), D(11, 4), D(11, 5), D(11, 6), D(11, 7),
+ D(12, 0), D(12, 1), D(12, 2)
+};
+
+const struct pinctrl_pin_desc sppctl_pins_all[] = {
+ /* gpio and iop only */
+ P(0, 0), P(0, 1), P(0, 2), P(0, 3), P(0, 4), P(0, 5), P(0, 6), P(0, 7),
+ /* gpio, iop, muxable */
+ P(1, 0), P(1, 1), P(1, 2), P(1, 3), P(1, 4), P(1, 5), P(1, 6), P(1, 7),
+ P(2, 0), P(2, 1), P(2, 2), P(2, 3), P(2, 4), P(2, 5), P(2, 6), P(2, 7),
+ P(3, 0), P(3, 1), P(3, 2), P(3, 3), P(3, 4), P(3, 5), P(3, 6), P(3, 7),
+ P(4, 0), P(4, 1), P(4, 2), P(4, 3), P(4, 4), P(4, 5), P(4, 6), P(4, 7),
+ P(5, 0), P(5, 1), P(5, 2), P(5, 3), P(5, 4), P(5, 5), P(5, 6), P(5, 7),
+ P(6, 0), P(6, 1), P(6, 2), P(6, 3), P(6, 4), P(6, 5), P(6, 6), P(6, 7),
+ P(7, 0), P(7, 1), P(7, 2), P(7, 3), P(7, 4), P(7, 5), P(7, 6), P(7, 7),
+ P(8, 0), P(8, 1), P(8, 2), P(8, 3), P(8, 4), P(8, 5), P(8, 6), P(8, 7),
+ /* gpio (not wired) and iop only */
+ P(9, 0), P(9, 1), P(9, 2), P(9, 3), P(9, 4), P(9, 5), P(9, 6), P(9, 7),
+ P(10, 0), P(10, 1), P(10, 2), P(10, 3), P(10, 4), P(10, 5), P(10, 6), P(10, 7),
+ P(11, 0), P(11, 1), P(11, 2), P(11, 3), P(11, 4), P(11, 5), P(11, 6), P(11, 7),
+ P(12, 0), P(12, 1), P(12, 2)
+};
+
+const size_t sppctl_pins_all_sz = ARRAY_SIZE(sppctl_pins_all);
+
+/* pmux groups: some pins are muxable. group = pin */
+const char * const sppctl_pmux_list_s[] = {
+ D_PIS(0, 0),
+ D_PIS(1, 0), D_PIS(1, 1), D_PIS(1, 2), D_PIS(1, 3),
+ D_PIS(1, 4), D_PIS(1, 5), D_PIS(1, 6), D_PIS(1, 7),
+ D_PIS(2, 0), D_PIS(2, 1), D_PIS(2, 2), D_PIS(2, 3),
+ D_PIS(2, 4), D_PIS(2, 5), D_PIS(2, 6), D_PIS(2, 7),
+ D_PIS(3, 0), D_PIS(3, 1), D_PIS(3, 2), D_PIS(3, 3),
+ D_PIS(3, 4), D_PIS(3, 5), D_PIS(3, 6), D_PIS(3, 7),
+ D_PIS(4, 0), D_PIS(4, 1), D_PIS(4, 2), D_PIS(4, 3),
+ D_PIS(4, 4), D_PIS(4, 5), D_PIS(4, 6), D_PIS(4, 7),
+ D_PIS(5, 0), D_PIS(5, 1), D_PIS(5, 2), D_PIS(5, 3),
+ D_PIS(5, 4), D_PIS(5, 5), D_PIS(5, 6), D_PIS(5, 7),
+ D_PIS(6, 0), D_PIS(6, 1), D_PIS(6, 2), D_PIS(6, 3),
+ D_PIS(6, 4), D_PIS(6, 5), D_PIS(6, 6), D_PIS(6, 7),
+ D_PIS(7, 0), D_PIS(7, 1), D_PIS(7, 2), D_PIS(7, 3),
+ D_PIS(7, 4), D_PIS(7, 5), D_PIS(7, 6), D_PIS(7, 7),
+ D_PIS(8, 0), D_PIS(8, 1), D_PIS(8, 2), D_PIS(8, 3),
+ D_PIS(8, 4), D_PIS(8, 5), D_PIS(8, 6), D_PIS(8, 7)
+};
+
+const size_t sppctl_pmux_list_sz = ARRAY_SIZE(sppctl_pmux_list_s);
+
+static const unsigned int pins_spif1[] = { D(10, 3), D(10, 4), D(10, 6), D(10, 7) };
+static const unsigned int pins_spif2[] = { D(9, 4), D(9, 6), D(9, 7), D(10, 1) };
+static const struct sppctl_grp sp7021grps_spif[] = {
+ EGRP("SPI_FLASH1", 1, pins_spif1),
+ EGRP("SPI_FLASH2", 2, pins_spif2)
+};
+
+static const unsigned int pins_spi41[] = { D(10, 2), D(10, 5) };
+static const unsigned int pins_spi42[] = { D(9, 5), D(9, 8) };
+static const struct sppctl_grp sp7021grps_spi4[] = {
+ EGRP("SPI_FLASH_4BIT1", 1, pins_spi41),
+ EGRP("SPI_FLASH_4BIT2", 2, pins_spi42)
+};
+
+static const unsigned int pins_snan[] = {
+ D(9, 4), D(9, 5), D(9, 6), D(9, 7), D(10, 0), D(10, 1)
+};
+
+static const struct sppctl_grp sp7021grps_snan[] = {
+ EGRP("SPI_NAND", 1, pins_snan)
+};
+
+static const unsigned int pins_emmc[] = {
+ D(9, 0), D(9, 1), D(9, 2), D(9, 3), D(9, 4), D(9, 5),
+ D(9, 6), D(9, 7), D(10, 0), D(10, 1) };
+
+static const struct sppctl_grp sp7021grps_emmc[] = {
+ EGRP("CARD0_EMMC", 1, pins_emmc)
+};
+
+static const unsigned int pins_sdsd[] = {
+ D(8, 1), D(8, 2), D(8, 3), D(8, 4), D(8, 5), D(8, 6)
+};
+
+static const struct sppctl_grp sp7021grps_sdsd[] = {
+ EGRP("SD_CARD", 1, pins_sdsd)
+};
+
+static const unsigned int pins_uar0[] = { D(11, 0), D(11, 1) };
+
+static const struct sppctl_grp sp7021grps_uar0[] = {
+ EGRP("UA0", 1, pins_uar0)
+};
+
+static const unsigned int pins_adbg1[] = { D(10, 2), D(10, 3) };
+static const unsigned int pins_adbg2[] = { D(7, 1), D(7, 2) };
+
+static const struct sppctl_grp sp7021grps_adbg[] = {
+ EGRP("ACHIP_DEBUG1", 1, pins_adbg1),
+ EGRP("ACHIP_DEBUG2", 2, pins_adbg2)
+};
+
+static const unsigned int pins_aua2axi1[] = { D(2, 0), D(2, 1), D(2, 2) };
+static const unsigned int pins_aua2axi2[] = { D(1, 0), D(1, 1), D(1, 2) };
+
+static const struct sppctl_grp sp7021grps_au2x[] = {
+ EGRP("ACHIP_UA2AXI1", 1, pins_aua2axi1),
+ EGRP("ACHIP_UA2AXI2", 2, pins_aua2axi2)
+};
+
+static const unsigned int pins_fpga[] = {
+ D(0, 2), D(0, 3), D(0, 4), D(0, 5), D(0, 6), D(0, 7),
+ D(1, 0), D(1, 1), D(1, 2), D(1, 3), D(1, 4), D(1, 5),
+ D(1, 6), D(1, 7), D(2, 0), D(2, 1), D(2, 2), D(2, 3),
+ D(2, 4), D(2, 5), D(2, 6), D(2, 7), D(3, 0), D(3, 1),
+ D(3, 2), D(3, 3), D(3, 4), D(3, 5), D(3, 6), D(3, 7),
+ D(4, 0), D(4, 1), D(4, 2), D(4, 3), D(4, 4), D(4, 5),
+ D(4, 6), D(4, 7), D(5, 0), D(5, 1), D(5, 2)
+};
+
+static const struct sppctl_grp sp7021grps_fpga[] = {
+ EGRP("FPGA_IFX", 1, pins_fpga)
+};
+
+static const unsigned int pins_hdmi1[] = { D(10, 6), D(12, 2), D(12, 1) };
+static const unsigned int pins_hdmi2[] = { D(8, 3), D(8, 5), D(8, 6) };
+static const unsigned int pins_hdmi3[] = { D(7, 4), D(7, 6), D(7, 7) };
+
+static const struct sppctl_grp sp7021grps_hdmi[] = {
+ EGRP("HDMI_TX1", 1, pins_hdmi1),
+ EGRP("HDMI_TX2", 2, pins_hdmi2),
+ EGRP("HDMI_TX3", 3, pins_hdmi3)
+};
+
+static const unsigned int pins_eadc[] = {
+ D(1, 0), D(1, 1), D(1, 2), D(1, 3), D(1, 4), D(1, 5), D(1, 6)
+};
+
+static const struct sppctl_grp sp7021grps_eadc[] = {
+ EGRP("AUD_EXT_ADC_IFX0", 1, pins_eadc)
+};
+
+static const unsigned int pins_edac[] = {
+ D(2, 5), D(2, 6), D(2, 7), D(3, 0), D(3, 1), D(3, 2), D(3, 4)
+};
+
+static const struct sppctl_grp sp7021grps_edac[] = {
+ EGRP("AUD_EXT_DAC_IFX0", 1, pins_edac)
+};
+
+static const unsigned int pins_spdi[] = { D(2, 4) };
+
+static const struct sppctl_grp sp7021grps_spdi[] = {
+ EGRP("AUD_IEC_RX0", 1, pins_spdi)
+};
+
+static const unsigned int pins_spdo[] = { D(3, 6) };
+
+static const struct sppctl_grp sp7021grps_spdo[] = {
+ EGRP("AUD_IEC_TX0", 1, pins_spdo)
+};
+
+static const unsigned int pins_tdmt[] = {
+ D(2, 5), D(2, 6), D(2, 7), D(3, 0), D(3, 1), D(3, 2)
+};
+
+static const struct sppctl_grp sp7021grps_tdmt[] = {
+ EGRP("TDMTX_IFX0", 1, pins_tdmt)
+};
+
+static const unsigned int pins_tdmr[] = { D(1, 7), D(2, 0), D(2, 1), D(2, 2) };
+
+static const struct sppctl_grp sp7021grps_tdmr[] = {
+ EGRP("TDMRX_IFX0", 1, pins_tdmr)
+};
+
+static const unsigned int pins_pdmr[] = {
+ D(1, 7), D(2, 0), D(2, 1), D(2, 2), D(2, 3)
+};
+
+static const struct sppctl_grp sp7021grps_pdmr[] = {
+ EGRP("PDMRX_IFX0", 1, pins_pdmr)
+};
+
+static const unsigned int pins_pcmt[] = {
+ D(3, 7), D(4, 0), D(4, 1), D(4, 2), D(4, 3), D(4, 4)
+};
+
+static const struct sppctl_grp sp7021grps_pcmt[] = {
+ EGRP("PCM_IEC_TX", 1, pins_pcmt)
+};
+
+static const unsigned int pins_lcdi[] = {
+ D(1, 4), D(1, 5),
+ D(1, 6), D(1, 7), D(2, 0), D(2, 1), D(2, 2), D(2, 3),
+ D(2, 4), D(2, 5), D(2, 6), D(2, 7), D(3, 0), D(3, 1),
+ D(3, 2), D(3, 3), D(3, 4), D(3, 5), D(3, 6), D(3, 7),
+ D(4, 0), D(4, 1), D(4, 2), D(4, 3), D(4, 4), D(4, 5),
+ D(4, 6), D(4, 7)
+};
+
+static const struct sppctl_grp sp7021grps_lcdi[] = {
+ EGRP("LCDIF", 1, pins_lcdi)
+};
+
+static const unsigned int pins_dvdd[] = {
+ D(7, 0), D(7, 1), D(7, 2), D(7, 3), D(7, 4), D(7, 5), D(7, 6), D(7, 7),
+ D(8, 0), D(8, 1), D(8, 2), D(8, 3), D(8, 4), D(8, 5)
+};
+
+static const struct sppctl_grp sp7021grps_dvdd[] = {
+ EGRP("DVD_DSP_DEBUG", 1, pins_dvdd)
+};
+
+static const unsigned int pins_i2cd[] = { D(1, 0), D(1, 1) };
+
+static const struct sppctl_grp sp7021grps_i2cd[] = {
+ EGRP("I2C_DEBUG", 1, pins_i2cd)
+};
+
+static const unsigned int pins_i2cs[] = { D(0, 0), D(0, 1) };
+
+static const struct sppctl_grp sp7021grps_i2cs[] = {
+ EGRP("I2C_SLAVE", 1, pins_i2cs)
+};
+
+static const unsigned int pins_wakp[] = { D(10, 5) };
+
+static const struct sppctl_grp sp7021grps_wakp[] = {
+ EGRP("WAKEUP", 1, pins_wakp)
+};
+
+static const unsigned int pins_u2ax[] = { D(2, 0), D(2, 1), D(3, 0), D(3, 1) };
+
+static const struct sppctl_grp sp7021grps_u2ax[] = {
+ EGRP("UART2AXI", 1, pins_u2ax)
+};
+
+static const unsigned int pins_u0ic[] = {
+ D(0, 0), D(0, 1), D(0, 4), D(0, 5), D(1, 0), D(1, 1)
+};
+
+static const struct sppctl_grp sp7021grps_u0ic[] = {
+ EGRP("USB0_I2C", 1, pins_u0ic)
+};
+
+static const unsigned int pins_u1ic[] = {
+ D(0, 2), D(0, 3), D(0, 6), D(0, 7), D(1, 2), D(1, 3)
+};
+
+static const struct sppctl_grp sp7021grps_u1ic[] = {
+ EGRP("USB1_I2C", 1, pins_u1ic)
+};
+
+static const unsigned int pins_u0ot[] = { D(11, 2) };
+
+static const struct sppctl_grp sp7021grps_u0ot[] = {
+ EGRP("USB0_OTG", 1, pins_u0ot)
+};
+
+static const unsigned int pins_u1ot[] = { D(11, 3) };
+
+static const struct sppctl_grp sp7021grps_u1ot[] = {
+ EGRP("USB1_OTG", 1, pins_u1ot)
+};
+
+static const unsigned int pins_uphd[] = {
+ D(0, 1), D(0, 2), D(0, 3), D(7, 4), D(7, 5), D(7, 6),
+ D(7, 7), D(8, 0), D(8, 1), D(8, 2), D(8, 3),
+ D(9, 7), D(10, 2), D(10, 3), D(10, 4)
+};
+
+static const struct sppctl_grp sp7021grps_up0d[] = {
+ EGRP("UPHY0_DEBUG", 1, pins_uphd)
+};
+
+static const struct sppctl_grp sp7021grps_up1d[] = {
+ EGRP("UPHY1_DEBUG", 1, pins_uphd)
+};
+
+static const unsigned int pins_upex[] = {
+ D(0, 0), D(0, 1), D(0, 2), D(0, 3), D(0, 4), D(0, 5), D(0, 6), D(0, 7),
+ D(1, 0), D(1, 1), D(1, 2), D(1, 3), D(1, 4), D(1, 5), D(1, 6), D(1, 7),
+ D(2, 0), D(2, 1), D(2, 2), D(2, 3), D(2, 4), D(2, 5), D(2, 6), D(2, 7),
+ D(3, 0), D(3, 1), D(3, 2), D(3, 3), D(3, 4), D(3, 5), D(3, 6), D(3, 7),
+ D(4, 0), D(4, 1), D(4, 2), D(4, 3), D(4, 4), D(4, 5), D(4, 6), D(4, 7),
+ D(5, 0), D(5, 1), D(5, 2), D(5, 3), D(5, 4), D(5, 5), D(5, 6), D(5, 7),
+ D(6, 0), D(6, 1), D(6, 2), D(6, 3), D(6, 4), D(6, 5), D(6, 6), D(6, 7),
+ D(7, 0), D(7, 1), D(7, 2), D(7, 3), D(7, 4), D(7, 5), D(7, 6), D(7, 7),
+ D(8, 0), D(8, 1), D(8, 2), D(8, 3), D(8, 4), D(8, 5), D(8, 6), D(8, 7),
+ D(9, 0), D(9, 1), D(9, 2), D(9, 3), D(9, 4), D(9, 5), D(9, 6), D(9, 7),
+ D(10, 0), D(10, 1), D(10, 2), D(10, 3), D(10, 4), D(10, 5), D(10, 6), D(10, 7)
+};
+
+static const struct sppctl_grp sp7021grps_upex[] = {
+ EGRP("UPHY0_EXT", 1, pins_upex)
+};
+
+static const unsigned int pins_prp1[] = {
+ D(0, 6), D(0, 7),
+ D(1, 0), D(1, 1), D(1, 2), D(1, 3), D(1, 4), D(1, 5), D(1, 6), D(1, 7),
+ D(2, 1), D(2, 2), D(2, 3), D(2, 4), D(2, 5), D(2, 6), D(2, 7),
+ D(3, 0), D(3, 1), D(3, 2)
+};
+
+static const unsigned int pins_prp2[] = {
+ D(3, 4), D(3, 6), D(3, 7),
+ D(4, 0), D(4, 1), D(4, 2), D(4, 3), D(4, 4), D(4, 5), D(4, 6), D(4, 7),
+ D(5, 0), D(5, 1), D(5, 2), D(5, 3), D(5, 4), D(5, 5), D(5, 6), D(5, 7),
+ D(6, 4)
+};
+
+static const struct sppctl_grp sp7021grps_prbp[] = {
+ EGRP("PROBE_PORT1", 1, pins_prp1),
+ EGRP("PROBE_PORT2", 2, pins_prp2)
+};
+
+const struct sppctl_func sppctl_list_funcs[] = {
+ FNCN("GPIO", f_off_0, 0x00, 0, 0),
+ FNCN("IOP", f_off_0, 0x00, 0, 0),
+
+ FNCN("L2SW_CLK_OUT", f_off_m, 0x00, 0, 7),
+ FNCN("L2SW_MAC_SMI_MDC", f_off_m, 0x00, 8, 7),
+ FNCN("L2SW_LED_FLASH0", f_off_m, 0x01, 0, 7),
+ FNCN("L2SW_LED_FLASH1", f_off_m, 0x01, 8, 7),
+ FNCN("L2SW_LED_ON0", f_off_m, 0x02, 0, 7),
+ FNCN("L2SW_LED_ON1", f_off_m, 0x02, 8, 7),
+ FNCN("L2SW_MAC_SMI_MDIO", f_off_m, 0x03, 0, 7),
+ FNCN("L2SW_P0_MAC_RMII_TXEN", f_off_m, 0x03, 8, 7),
+ FNCN("L2SW_P0_MAC_RMII_TXD0", f_off_m, 0x04, 0, 7),
+ FNCN("L2SW_P0_MAC_RMII_TXD1", f_off_m, 0x04, 8, 7),
+ FNCN("L2SW_P0_MAC_RMII_CRSDV", f_off_m, 0x05, 0, 7),
+ FNCN("L2SW_P0_MAC_RMII_RXD0", f_off_m, 0x05, 8, 7),
+ FNCN("L2SW_P0_MAC_RMII_RXD1", f_off_m, 0x06, 0, 7),
+ FNCN("L2SW_P0_MAC_RMII_RXER", f_off_m, 0x06, 8, 7),
+ FNCN("L2SW_P1_MAC_RMII_TXEN", f_off_m, 0x07, 0, 7),
+ FNCN("L2SW_P1_MAC_RMII_TXD0", f_off_m, 0x07, 8, 7),
+ FNCN("L2SW_P1_MAC_RMII_TXD1", f_off_m, 0x08, 0, 7),
+ FNCN("L2SW_P1_MAC_RMII_CRSDV", f_off_m, 0x08, 8, 7),
+ FNCN("L2SW_P1_MAC_RMII_RXD0", f_off_m, 0x09, 0, 7),
+ FNCN("L2SW_P1_MAC_RMII_RXD1", f_off_m, 0x09, 8, 7),
+ FNCN("L2SW_P1_MAC_RMII_RXER", f_off_m, 0x0A, 0, 7),
+ FNCN("DAISY_MODE", f_off_m, 0x0A, 8, 7),
+ FNCN("SDIO_CLK", f_off_m, 0x0B, 0, 7),
+ FNCN("SDIO_CMD", f_off_m, 0x0B, 8, 7),
+ FNCN("SDIO_D0", f_off_m, 0x0C, 0, 7),
+ FNCN("SDIO_D1", f_off_m, 0x0C, 8, 7),
+ FNCN("SDIO_D2", f_off_m, 0x0D, 0, 7),
+ FNCN("SDIO_D3", f_off_m, 0x0D, 8, 7),
+ FNCN("PWM0", f_off_m, 0x0E, 0, 7),
+ FNCN("PWM1", f_off_m, 0x0E, 8, 7),
+ FNCN("PWM2", f_off_m, 0x0F, 0, 7),
+ FNCN("PWM3", f_off_m, 0x0F, 8, 7),
+
+ FNCN("PWM4", f_off_m, 0x10, 0, 7),
+ FNCN("PWM5", f_off_m, 0x10, 8, 7),
+ FNCN("PWM6", f_off_m, 0x11, 0, 7),
+ FNCN("PWM7", f_off_m, 0x11, 8, 7),
+ FNCN("ICM0_D", f_off_m, 0x12, 0, 7), /* 4x Input captures */
+ FNCN("ICM1_D", f_off_m, 0x12, 8, 7),
+ FNCN("ICM2_D", f_off_m, 0x13, 0, 7),
+ FNCN("ICM3_D", f_off_m, 0x13, 8, 7),
+ FNCN("ICM0_CLK", f_off_m, 0x14, 0, 7),
+ FNCN("ICM1_CLK", f_off_m, 0x14, 8, 7),
+ FNCN("ICM2_CLK", f_off_m, 0x15, 0, 7),
+ FNCN("ICM3_CLK", f_off_m, 0x15, 8, 7),
+ FNCN("SPIM0_INT", f_off_m, 0x16, 0, 7), /* 4x SPI masters */
+ FNCN("SPIM0_CLK", f_off_m, 0x16, 8, 7),
+ FNCN("SPIM0_EN", f_off_m, 0x17, 0, 7),
+ FNCN("SPIM0_DO", f_off_m, 0x17, 8, 7),
+ FNCN("SPIM0_DI", f_off_m, 0x18, 0, 7),
+ FNCN("SPIM1_INT", f_off_m, 0x18, 8, 7),
+ FNCN("SPIM1_CLK", f_off_m, 0x19, 0, 7),
+ FNCN("SPIM1_EN", f_off_m, 0x19, 8, 7),
+ FNCN("SPIM1_DO", f_off_m, 0x1A, 0, 7),
+ FNCN("SPIM1_DI", f_off_m, 0x1A, 8, 7),
+ FNCN("SPIM2_INT", f_off_m, 0x1B, 0, 7),
+ FNCN("SPIM2_CLK", f_off_m, 0x1B, 8, 7),
+ FNCN("SPIM2_EN", f_off_m, 0x1C, 0, 7),
+ FNCN("SPIM2_DO", f_off_m, 0x1C, 8, 7),
+ FNCN("SPIM2_DI", f_off_m, 0x1D, 0, 7),
+ FNCN("SPIM3_INT", f_off_m, 0x1D, 8, 7),
+ FNCN("SPIM3_CLK", f_off_m, 0x1E, 0, 7),
+ FNCN("SPIM3_EN", f_off_m, 0x1E, 8, 7),
+ FNCN("SPIM3_DO", f_off_m, 0x1F, 0, 7),
+ FNCN("SPIM3_DI", f_off_m, 0x1F, 8, 7),
+
+ FNCN("SPI0S_INT", f_off_m, 0x20, 0, 7), /* 4x SPI slaves */
+ FNCN("SPI0S_CLK", f_off_m, 0x20, 8, 7),
+ FNCN("SPI0S_EN", f_off_m, 0x21, 0, 7),
+ FNCN("SPI0S_DO", f_off_m, 0x21, 8, 7),
+ FNCN("SPI0S_DI", f_off_m, 0x22, 0, 7),
+ FNCN("SPI1S_INT", f_off_m, 0x22, 8, 7),
+ FNCN("SPI1S_CLK", f_off_m, 0x23, 0, 7),
+ FNCN("SPI1S_EN", f_off_m, 0x23, 8, 7),
+ FNCN("SPI1S_DO", f_off_m, 0x24, 0, 7),
+ FNCN("SPI1S_DI", f_off_m, 0x24, 8, 7),
+ FNCN("SPI2S_INT", f_off_m, 0x25, 0, 7),
+ FNCN("SPI2S_CLK", f_off_m, 0x25, 8, 7),
+ FNCN("SPI2S_EN", f_off_m, 0x26, 0, 7),
+ FNCN("SPI2S_DO", f_off_m, 0x26, 8, 7),
+ FNCN("SPI2S_DI", f_off_m, 0x27, 0, 7),
+ FNCN("SPI3S_INT", f_off_m, 0x27, 8, 7),
+ FNCN("SPI3S_CLK", f_off_m, 0x28, 0, 7),
+ FNCN("SPI3S_EN", f_off_m, 0x28, 8, 7),
+ FNCN("SPI3S_DO", f_off_m, 0x29, 0, 7),
+ FNCN("SPI3S_DI", f_off_m, 0x29, 8, 7),
+ FNCN("I2CM0_CLK", f_off_m, 0x2A, 0, 7), /* 4x I2C masters */
+ FNCN("I2CM0_DAT", f_off_m, 0x2A, 8, 7),
+ FNCN("I2CM1_CLK", f_off_m, 0x2B, 0, 7),
+ FNCN("I2CM1_DAT", f_off_m, 0x2B, 8, 7),
+ FNCN("I2CM2_CLK", f_off_m, 0x2C, 0, 7),
+ FNCN("I2CM2_DAT", f_off_m, 0x2C, 8, 7),
+ FNCN("I2CM3_CLK", f_off_m, 0x2D, 0, 7),
+ FNCN("I2CM3_DAT", f_off_m, 0x2D, 8, 7),
+ FNCN("UA1_TX", f_off_m, 0x2E, 0, 7), /* 4x UARTS */
+ FNCN("UA1_RX", f_off_m, 0x2E, 8, 7),
+ FNCN("UA1_CTS", f_off_m, 0x2F, 0, 7),
+ FNCN("UA1_RTS", f_off_m, 0x2F, 8, 7),
+
+ FNCN("UA2_TX", f_off_m, 0x30, 0, 7),
+ FNCN("UA2_RX", f_off_m, 0x30, 8, 7),
+ FNCN("UA2_CTS", f_off_m, 0x31, 0, 7),
+ FNCN("UA2_RTS", f_off_m, 0x31, 8, 7),
+ FNCN("UA3_TX", f_off_m, 0x32, 0, 7),
+ FNCN("UA3_RX", f_off_m, 0x32, 8, 7),
+ FNCN("UA3_CTS", f_off_m, 0x33, 0, 7),
+ FNCN("UA3_RTS", f_off_m, 0x33, 8, 7),
+ FNCN("UA4_TX", f_off_m, 0x34, 0, 7),
+ FNCN("UA4_RX", f_off_m, 0x34, 8, 7),
+ FNCN("UA4_CTS", f_off_m, 0x35, 0, 7),
+ FNCN("UA4_RTS", f_off_m, 0x35, 8, 7),
+ FNCN("TIMER0_INT", f_off_m, 0x36, 0, 7), /* 4x timer int. */
+ FNCN("TIMER1_INT", f_off_m, 0x36, 8, 7),
+ FNCN("TIMER2_INT", f_off_m, 0x37, 0, 7),
+ FNCN("TIMER3_INT", f_off_m, 0x37, 8, 7),
+ FNCN("GPIO_INT0", f_off_m, 0x38, 0, 7), /* 8x GPIO int. */
+ FNCN("GPIO_INT1", f_off_m, 0x38, 8, 7),
+ FNCN("GPIO_INT2", f_off_m, 0x39, 0, 7),
+ FNCN("GPIO_INT3", f_off_m, 0x39, 8, 7),
+ FNCN("GPIO_INT4", f_off_m, 0x3A, 0, 7),
+ FNCN("GPIO_INT5", f_off_m, 0x3A, 8, 7),
+ FNCN("GPIO_INT6", f_off_m, 0x3B, 0, 7),
+ FNCN("GPIO_INT7", f_off_m, 0x3B, 8, 7),
+
+ /* MOON1 register */
+ FNCE("SPI_FLASH", f_off_g, 0x01, 0, 2, sp7021grps_spif),
+ FNCE("SPI_FLASH_4BIT", f_off_g, 0x01, 2, 2, sp7021grps_spi4),
+ FNCE("SPI_NAND", f_off_g, 0x01, 4, 1, sp7021grps_snan),
+ FNCE("CARD0_EMMC", f_off_g, 0x01, 5, 1, sp7021grps_emmc),
+ FNCE("SD_CARD", f_off_g, 0x01, 6, 1, sp7021grps_sdsd),
+ FNCE("UA0", f_off_g, 0x01, 7, 1, sp7021grps_uar0),
+ FNCE("ACHIP_DEBUG", f_off_g, 0x01, 8, 2, sp7021grps_adbg),
+ FNCE("ACHIP_UA2AXI", f_off_g, 0x01, 10, 2, sp7021grps_au2x),
+ FNCE("FPGA_IFX", f_off_g, 0x01, 12, 1, sp7021grps_fpga),
+ FNCE("HDMI_TX", f_off_g, 0x01, 13, 2, sp7021grps_hdmi),
+
+ FNCE("AUD_EXT_ADC_IFX0", f_off_g, 0x01, 15, 1, sp7021grps_eadc),
+ FNCE("AUD_EXT_DAC_IFX0", f_off_g, 0x02, 0, 1, sp7021grps_edac),
+ FNCE("SPDIF_RX", f_off_g, 0x02, 2, 1, sp7021grps_spdi),
+ FNCE("SPDIF_TX", f_off_g, 0x02, 3, 1, sp7021grps_spdo),
+ FNCE("TDMTX_IFX0", f_off_g, 0x02, 4, 1, sp7021grps_tdmt),
+ FNCE("TDMRX_IFX0", f_off_g, 0x02, 5, 1, sp7021grps_tdmr),
+ FNCE("PDMRX_IFX0", f_off_g, 0x02, 6, 1, sp7021grps_pdmr),
+ FNCE("PCM_IEC_TX", f_off_g, 0x02, 7, 1, sp7021grps_pcmt),
+ FNCE("LCDIF", f_off_g, 0x04, 6, 1, sp7021grps_lcdi),
+ FNCE("DVD_DSP_DEBUG", f_off_g, 0x02, 8, 1, sp7021grps_dvdd),
+ FNCE("I2C_DEBUG", f_off_g, 0x02, 9, 1, sp7021grps_i2cd),
+ FNCE("I2C_SLAVE", f_off_g, 0x02, 10, 1, sp7021grps_i2cs),
+ FNCE("WAKEUP", f_off_g, 0x02, 11, 1, sp7021grps_wakp),
+ FNCE("UART2AXI", f_off_g, 0x02, 12, 2, sp7021grps_u2ax),
+ FNCE("USB0_I2C", f_off_g, 0x02, 14, 2, sp7021grps_u0ic),
+ FNCE("USB1_I2C", f_off_g, 0x03, 0, 2, sp7021grps_u1ic),
+ FNCE("USB0_OTG", f_off_g, 0x03, 2, 1, sp7021grps_u0ot),
+ FNCE("USB1_OTG", f_off_g, 0x03, 3, 1, sp7021grps_u1ot),
+ FNCE("UPHY0_DEBUG", f_off_g, 0x03, 4, 1, sp7021grps_up0d),
+ FNCE("UPHY1_DEBUG", f_off_g, 0x03, 5, 1, sp7021grps_up1d),
+ FNCE("UPHY0_EXT", f_off_g, 0x03, 6, 1, sp7021grps_upex),
+ FNCE("PROBE_PORT", f_off_g, 0x03, 7, 2, sp7021grps_prbp),
+};
+
+const size_t sppctl_list_funcs_sz = ARRAY_SIZE(sppctl_list_funcs);
--
2.7.4


2021-12-09 23:30:29

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] dt-bindings: pinctrl: Add dt-bindings for Sunplus SP7021

On Tue, Dec 7, 2021 at 5:17 AM Wells Lu <[email protected]> wrote:

> Add dt-bindings header files and documentation for Sunplus SP7021 SoC.
>
> Signed-off-by: Wells Lu <[email protected]>

> +patternProperties:
> + '-pins$':
> + if:
> + type: object
> + then:
> + description: |
> + A pinctrl node should contain at least one subnodes representing the
> + pins or function-pins group available on the machine. Each subnode
> + will list the pins it needs, and how they should be configured.
> +
> + Pinctrl node's client devices use subnodes for desired pin
> + configuration. Client device subnodes use below standard properties.

I don't understand this if type object stuff here, Rob, help...

> + properties:
> + pins:
> + description: |
> + Define pins which are used by pinctrl node's client device.
(...)
> + $ref: /schemas/types.yaml#/definitions/uint32-array

Why can this not $ref the standard binings in
Documentation/devicetree/bindings/pinctrl/pinmux-node.yaml

See for example
Documentation/devicetree/bindings/pinctrl/actions,s500-pinctrl.yaml
for a nice example of how to use this.

Yours,
Linus Walleij

2021-12-10 01:25:12

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] pinctrl: Add driver for Sunplus SP7021

Hi Wells,

this is improving! Keep working on this driver. I now naturally have
more comments :)

On Tue, Dec 7, 2021 at 5:17 AM Wells Lu <[email protected]> wrote:

> +static void sppctl_func_set(struct sppctl_pdata *pctl, u8 func, u8 val)
> +{
> + u32 reg, offset;
> +
> + /* Note that upper 16-bit word is mask
> + * and lower 16-bit word is value.
> + * Enable mask before write.
> + */
> + reg = 0x007f0000 | val; /* Set value and enable mask. */

Define these types of masks and use them like this:

#include <linux/bits.h>

#define SPPCTL_FUNC_MASK GENMASK(22, 16)

Also switch the order with the mask to the right please:

reg = val & SPPCTL_FUNC_MASK;

> + if (func & 1)

I would write

#define SSPCTL_FUNC_FLAG BIT(0)

if (func & SSPCTL_FUNC_FLAG)

Use the name that bit has in your documentation for the
define so we know what is going on.

> + reg <<= 8;

Likewise

#define SSPCTL_FUNC_UPPER_SHIFT 8

reg <<= SSPCTL_FUNC_UPPER_SHIFT;


Can also be a comment. The general idea is to break out as many
of these magic numbers as possible to #defines and give them
some names from the reference manual, so we understand them
instead of the numbers being magic.

> + /* Convert function # to register offset. */
> + offset = func & ~1;

Step 1 write:
offset = func & GENMASK(31, 1);

> + offset <<= 1;

I would write:
offset *= 2;
because we are dealing with an offset and not an arithmetic
operation. It will be the same to the compiler.

But the best is to just merge all this and write (if I'm not wrong):

#include <linux/bitfield.h>

/*
* Bit 1 .. 31 gives the function, index this into a 32-bit offset by
* multiplying by four to find the register.
*/
offset = FIELD_GET(GENMASK(31, 1), func);
offset *= 4;

This gets pretty clear. We see that we remove BIT(0) and use the
rest as offset index and there are four bytes per register.

(Beware of bugs in my pseudocode, check it!)

> +static u8 sppctl_func_get(struct sppctl_pdata *pctl, u8 func)
> +{
> + u32 reg, offset;
> + u8 val;
> +
> + /* Convert function # to register offset. */
> + offset = func & ~1;
> + offset <<= 1;

Same comments.

> + reg = readl(pctl->moon2_base + offset);
> + if (func & 1)
> + val = reg >> 8;
> + else
> + val = reg;
> + val &= 0x7f;

#define SSPCTL_*_MASK for this 0x7f so we understand it.

> +static void sppctl_gmx_set(struct sppctl_pdata *pctl, u8 reg_off, u8 bit_off, u8 bit_sz,
> + u8 val)
> +{
> + u32 mask, reg;
> +
> + /* Note that upper 16-bit word is mask
> + * and lower 16-bit word is value.
> + * Enable mask before write.
> + */
> + mask = ~(~0 << bit_sz);
> + reg = (mask << 16) | (val & mask);
> + reg <<= bit_off;

Please familiarize yourself with <linux/bitfield.h> and use things like
FIELD_PREP() for this (I think, atleast).

> +static int sppctl_first_get(struct gpio_chip *chip, unsigned int offset)
> +{
> + struct sppctl_gpio_chip *spp_gchip = gpiochip_get_data(chip);
> + u32 reg;
> +
> + reg = readl(spp_gchip->first_base + SPPCTL_GPIO_OFF_FIRST + R32_ROF(offset));

So R32_ROF() is register offset.

> +
> + dev_dbg(chip->parent, "%s(%u): addr = %p, reg = %08x, val = %d\n",
> + __func__, offset, spp_gchip->first_base + SPPCTL_GPIO_OFF_FIRST +
> + R32_ROF(offset), reg, (int)R32_VAL(reg, R32_BOF(offset)));
> +
> + return R32_VAL(reg, R32_BOF(offset));

And R32_BOF is register bit offset.

I think these macros just make it hard to read because the reader has to
go to another file and look it up and then figure out what does ROF and
BOF actually mean (no explanation given).

I would just inline the stuff.

u32 reg = (offset / 32) * 4;
u32 bit = offset % 32;

reg = readl(spp_gchip->first_base + SPPCTL_GPIO_OFF_FIRST + reg);

// Some debug code

return !!(reg & BIT(bit));

> +static void sppctl_gpio_output_inv_set(struct gpio_chip *chip, unsigned int offset)
> +{
> + struct sppctl_gpio_chip *spp_gchip = gpiochip_get_data(chip);
> + u32 reg;
> +
> + /* Upper 16-bit word is mask. Lower 16-bit word is value. */
> + reg = (BIT(R16_BOF(offset)) << 16) | BIT(R16_BOF(offset));
> + writel(reg, spp_gchip->gpioxt2_base + SPPCTL_GPIO_OFF_OINV + R16_ROF(offset));
> +}

Same comments about the BOF and ROF.

This layout with "mask and value" in registers needs to be explained
somewhere it looks complex. I don't understand why a machine register
contains a mask for example.

> +static int stpctl_set_mux(struct pinctrl_dev *pctldev, unsigned int func_selector,
> + unsigned int group_selector)
> +{
> + const struct sppctl_func *f = &sppctl_list_funcs[func_selector];
> + struct sppctl_pdata *pctl = pinctrl_dev_get_drvdata(pctldev);
> + struct grp2fp_map g2fpm = pctl->g2fp_maps[group_selector];
> + int i = -1, j = -1;

Please do not initialize loop variable i to -1, just declare it.

> + dev_dbg(pctldev->dev, "%s(func: %d, grp: %d)\n", __func__,
> + func_selector, group_selector);
> +
> + switch (f->freg) {
> + case f_off_0: /* GPIO. detouch from all funcs - ? */
> + for (i = 0; i < sppctl_list_funcs_sz; i++) {
> + if (sppctl_list_funcs[i].freg != f_off_m)
> + continue;
> + j++;

Insert a comment that j is set to -1 so this will be zero here after the first
iteration.

> + if (sppctl_func_get(pctl, j) != group_selector)
> + continue;
> + sppctl_func_set(pctl, j, 0);
> + }
> + break;
> +
> + case f_off_m: /* Mux */
> + sppctl_first_master_set(&pctl->spp_gchip->chip, group_selector,
> + mux_f_mux, mux_m_keep);
> + sppctl_func_set(pctl, func_selector - 2, (group_selector == 0) ?
> + group_selector : group_selector - 7);

-2 and -7? Why? Add some comments or maybe #define these
constants?

> +static int stpctl_gpio_request_enable(struct pinctrl_dev *pctldev,
> + struct pinctrl_gpio_range *range, unsigned int offset)
> +{
> + struct sppctl_pdata *pctl = pinctrl_dev_get_drvdata(pctldev);
> + struct pin_desc *pdesc;
> + int g_f, g_m;
> +
> + dev_dbg(pctldev->dev, "%s(%d)\n", __func__, offset);
> +
> + g_f = sppctl_first_get(&pctl->spp_gchip->chip, offset);
> + g_m = sppctl_master_get(&pctl->spp_gchip->chip, offset);
> + if (g_f == mux_f_gpio && g_m == mux_m_gpio)
> + return 0;
> +
> + pdesc = pin_desc_get(pctldev, offset);
> + if (pdesc->mux_owner)
> + return -EACCES;

Do not reimplement the pinmux core please.

What you want to achieve here is "strict pinmux", i.e. setting the field
"strict" in struct pinmux_ops to true. Then you can just delete this
check.

> +static const struct pinmux_ops sppctl_pinmux_ops = {
> + .request = stpctl_request,
> + .free = stpctl_free,

These are just set to empty functions. Delete these entries
and the empty functions.

> + .get_functions_count = stpctl_get_functions_count,
> + .get_function_name = stpctl_get_function_name,
> + .get_function_groups = stpctl_get_function_groups,
> + .set_mux = stpctl_set_mux,
> + .gpio_request_enable = stpctl_gpio_request_enable,
> + .gpio_disable_free = stpctl_gpio_disable_free,
> + .gpio_set_direction = stpctl_gpio_set_direction,
> + .strict = 1

Use "true" rather than 1. (And do not reimplement the check.)

> +static int sppctl_remove(struct platform_device *pdev)
> +{
> + struct sppctl_pdata *sppctl = pdev->dev.platform_data;
> +
> + devm_pinctrl_unregister(&pdev->dev, sppctl->pctl_dev);

This defies the idea with devm_* calls. Drop remove() entirely because
devm_ allocated resources go away by themselves.

> +++ b/drivers/pinctrl/sunplus/sppctl.h
(...)
> +/* (/16)*4 */
> +#define R16_ROF(r) (((r) >> 4) << 2)
> +#define R16_BOF(r) ((r) % 16)
> +/* (/32)*4 */
> +#define R32_ROF(r) (((r) >> 5) << 2)
> +#define R32_BOF(r) ((r) % 32)

As mentioned I prefer explicit inlined code for these.
The bit shifting here makes it really hard to know what is going
on, the compiler will get it right if you use the right types
and just write (n / 32) * 4. Please do not try to help the compiler
optimizing it just leads to code that is hard to read.

> +#define R32_VAL(r, boff) (((r) >> (boff)) & BIT(0))

To check the value of a certain bit use this pattern:

if (val & BIT(n))

To return a boolean clamped bit (return 0/1) do this idiom:

return !!(val & BIT(n));

Other than these things I didn't notice anything more this
time, but I might find even more stuff, but hey it's getting there!

Yours,
Linus Walleij

2021-12-10 14:07:57

by Wells Lu

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] dt-bindings: pinctrl: Add dt-bindings for Sunplus SP7021

Re-send this email because it was rejected due to the wrong email format!

Hi Linus,

Thank you very much for your review!

Please see my answers below:

> > Add dt-bindings header files and documentation for Sunplus SP7021 SoC.
> >
> > Signed-off-by: Wells Lu <[email protected]>
>
> > +patternProperties:
> > + '-pins$':
> > + if:
> > + type: object
> > + then:
> > + description: |
> > + A pinctrl node should contain at least one subnodes representing the
> > + pins or function-pins group available on the machine. Each subnode
> > + will list the pins it needs, and how they should be configured.
> > +
> > + Pinctrl node's client devices use subnodes for desired pin
> > + configuration. Client device subnodes use below standard properties.
>
> I don't understand this if type object stuff here, Rob, help...

I'll remove "if type object" stuff next patch since
pinctrl node has no properties with "-pins" suffix.
except sub nodes.


> > + properties:
> > + pins:
> > + description: |
> > + Define pins which are used by pinctrl node's client device.
> (...)
> > + $ref: /schemas/types.yaml#/definitions/uint32-array
>
> Why can this not $ref the standard binings in
> Documentation/devicetree/bindings/pinctrl/pinmux-node.yaml
>
> See for example
> Documentation/devicetree/bindings/pinctrl/actions,s500-pinctrl.yaml
> for a nice example of how to use this.
> Yours,
> Linus Walleij
The pins' types and control of SP7021 are a bit complex.
Let me explain:

SP7021 has 99 digital GPIO pins which are numbered from
GPIO 0 to 98. All are multiplexed with some special function
pins. There are 3 types special function pins:

(1) function-pins:
For example, if control-field SPI_FLASH_SEL is set to 1,
GPIO 83, 84, 86 and 87 will be pins of SPI-NOR flash.
If it is set to 2, GPIO 76, 78, 79 and 81 will be pins of
SPI-NOR flash.

For example 2, if control-bit UA0_SEL is set to 1,
GPIO 88 and 89 will be TX and RX pins of UART_0
(UART channel 0).

For example 3, if control-bit EMMC_SEL is set to 1,
GPIO 72, 73, 74, 75, 76, 77, 78, 79, 80, 81 will be
pins of an eMMC device.

The driver defines properties "function" and "groups"
to select this kind of pins-function.

(2) fully pin-mux pins:
GPIO 8 to 71 are 'fully pin-mux' pins.
Pins of peripherals of SP7021 (ex: UART_1, UART_2,
UART_3, UART_4, I2C_0, I2C_1, ..., SPI_0, SPI_1, ...
GPIO_INT0, GPIO_INT1, .., RMII_of_Ethernet, and etc.)
can be set to any pins of fully pin-mux pins.

EX1 (UART channel 1):
If control-field UA1_TX_SEL is set to 3, TX pin of
UART_1 will be routed to GPIO 10 (3 - 1 + 8 = 10)
If control-field UA1_RX_SEL is set to 4, RX pin of
UART_1 will be routed to GPIO 11 (4 - 1 + 8 = 11)
If control-field UA1_RTS_SEL is set to 5, RTS pin of
UART_1 will be routed to GPIO 12 (5 - 1 + 8 = 12)
If control-field UA1_CTS_SEL is set to 6, CTS pin of
UART_1 will be routed to GPIO 13 (6 - 1 + 8 = 13)

EX2 (I2C channel 0):
If control-field I2C0_CLK_SEL is set to 20, CLK pin
of I2C_0 will be routed to GPIO 27 (20 - 1 + 8 = 27)
If control-field I2C0_DATA_SEL is set to 21, DATA pin
of I2C_0 will be routed to GPIO 28 (21 - 1 + 9 = 28)

Totally, SP7021 has 120 peripheral pins. The
peripheral pins can be routed to any of 64 'fully
pin-mux' pins. So total combinations are:
120 x 64 = 7680
This is why we cannot enumerate all combinations.

(3) I/O processor pins
SP7021 has a built-in I/O processor.
Any GPIO pins (GPIO 0 to 98) can be set to pins of
I/O processor.

Property 'pins' is defined as uint32 array. Each item
defines a pin as below:
Bit 32~24 defines GPIO number. Its range is 0 ~ 98.
Bit 23~16 defines types: digital GPIO pins (3), IO processor pins (2)
or fully pin-mux pins (1)
Bit 15~8 defines pins of peripherals which are defined in
'include/dt-binging/pinctrl/sppctl.h'.
Bit 7~0 defines types or initial-state of digital GPIO pins.


Best regards,
Wells

2021-12-10 18:33:41

by Wells Lu

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] pinctrl: Add driver for Sunplus SP7021

Resend the email because it was rejected due to wrong format!

Hi Linus,

Thank you very much for your review.

Please see my answers below:

> Hi Wells,
>
> this is improving! Keep working on this driver. I now naturally have
> more comments :)
>
> On Tue, Dec 7, 2021 at 5:17 AM Wells Lu <[email protected]> wrote:
>
> > +static void sppctl_func_set(struct sppctl_pdata *pctl, u8 func, u8 val)
> > +{
> > + u32 reg, offset;
> > +
> > + /* Note that upper 16-bit word is mask
> > + * and lower 16-bit word is value.
> > + * Enable mask before write.
> > + */
> > + reg = 0x007f0000 | val; /* Set value and enable mask. */
>
> Define these types of masks and use them like this:
>
> #include <linux/bits.h>
>
> #define SPPCTL_FUNC_MASK GENMASK(22, 16)
>
> Also switch the order with the mask to the right please:
>
> reg = val & SPPCTL_FUNC_MASK;

Yes, I'll modify code next patch.


> > + if (func & 1)
>
> I would write
>
> #define SSPCTL_FUNC_FLAG BIT(0)
>
> if (func & SSPCTL_FUNC_FLAG)
>
> Use the name that bit has in your documentation for the
> define so we know what is going on.

Actually, 'if (func & 1)' is not used to test bit 0,
but test 'func' is an odd number or not.
If 'func' is even number, control-field is at bit 6 ~ 0.
Its corresponding mask-field is at bit 22 ~ 16.
If 'func' is odd number, control-field is at bit 14 ~ 8.
Its corresponding mask-field is at bit 30 ~ 24.

Control and mask fields of 'func' are arranged as shown
below:

func # | register control-field mask-field
-------+------------------------------------
0 | reg[0] ( 6:0) (22 : 6)
1 | reg[0] (14:8) (30 : 24)
2 | reg[1] ( 6:0) (22 : 6)
3 | reg[1] (14:8) (30 : 24)

> > + reg <<= 8;
>
> Likewise
>
> #define SSPCTL_FUNC_UPPER_SHIFT 8
>
> reg <<= SSPCTL_FUNC_UPPER_SHIFT;
> Can also be a comment. The general idea is to break out as many
> of these magic numbers as possible to #defines and give them
> some names from the reference manual, so we understand them
> instead of the numbers being magic.

Yes, I'll modify codes, add defines and more comments next patch.


> > + /* Convert function # to register offset. */
> > + offset = func & ~1;
>
> Step 1 write:
> offset = func & GENMASK(31, 1);
>
> > + offset <<= 1;
>
> I would write:
> offset *= 2;
> because we are dealing with an offset and not an arithmetic
> operation. It will be the same to the compiler.
>
> But the best is to just merge all this and write (if I'm not wrong):
>
> #include <linux/bitfield.h>
>
> /*
> * Bit 1 .. 31 gives the function, index this into a 32-bit offset by
> * multiplying by four to find the register.
> */
> offset = FIELD_GET(GENMASK(31, 1), func);
> offset *= 4;
>
> This gets pretty clear. We see that we remove BIT(0) and use the
> rest as offset index and there are four bytes per register.
>
> (Beware of bugs in my pseudo code, check it!)

Thank you for example code!
Yes, I will use BIT, GENMASK, FIELD_GET, and FIELD_PREP
macros next patch.


> > +static u8 sppctl_func_get(struct sppctl_pdata *pctl, u8 func)
> > +{
> > + u32 reg, offset;
> > + u8 val;
> > +
> > + /* Convert function # to register offset. */
> > + offset = func & ~1;
> > + offset <<= 1;
>
> Same comments.

Yes, I got it. I'll modify codes next patch.


> > + reg = readl(pctl->moon2_base + offset);
> > + if (func & 1)
> > + val = reg >> 8;
> > + else
> > + val = reg;
> > + val &= 0x7f;
>
> #define SSPCTL_*_MASK for this 0x7f so we understand it.

Yes, I'll do it next patch.


> > +static void sppctl_gmx_set(struct sppctl_pdata *pctl, u8 reg_off, u8 bit_off, u8 bit_sz,
> > + u8 val)
> > +{
> > + u32 mask, reg;
> > +
> > + /* Note that upper 16-bit word is mask
> > + * and lower 16-bit word is value.
> > + * Enable mask before write.
> > + */
> > + mask = ~(~0 << bit_sz);
> > + reg = (mask << 16) | (val & mask);
> > + reg <<= bit_off;
>
> Please familiarize yourself with <linux/bitfield.h> and use things like
> FIELD_PREP() for this (I think, at least).

Yes, I will use BIT, GENMASK, FIELD_GET, and FIELD_PREP
macros next patch.

> > +static int sppctl_first_get(struct gpio_chip *chip, unsigned int offset)
> > +{
> > + struct sppctl_gpio_chip *spp_gchip = gpiochip_get_data(chip);
> > + u32 reg;
> > +
> > + reg = readl(spp_gchip->first_base + SPPCTL_GPIO_OFF_FIRST + R32_ROF(offset));
>
> So R32_ROF() is register offset.

Yes, it converts 'offset' to register offset (w.r.t. base register)

R32_ROF() is for 32-bit width registers
R16_ROF() is for 16-bit width registers (higher 16-bit of the register is mask).


> > +
> > + dev_dbg(chip->parent, "%s(%u): addr = %p, reg = %08x, val = %d\n",
> > + __func__, offset, spp_gchip->first_base + SPPCTL_GPIO_OFF_FIRST +
> > + R32_ROF(offset), reg, (int)R32_VAL(reg, R32_BOF(offset)));
> > +
> > + return R32_VAL(reg, R32_BOF(offset));
>
> And R32_BOF is register bit offset.
>
> I think these macros just make it hard to read because the reader has to
> go to another file and look it up and then figure out what does ROF and
> BOF actually mean (no explanation given).
>
> I would just inline the stuff.
>
> u32 reg = (offset / 32) * 4;
> u32 bit = offset % 32;
>
> reg = readl(spp_gchip->first_base + SPPCTL_GPIO_OFF_FIRST + reg);
>
> // Some debug code
>
> return !!(reg & BIT(bit));

Yes, I'll modify codes to follow your suggestions next patch.


> > +static void sppctl_gpio_output_inv_set(struct gpio_chip *chip, unsigned int offset)
> > +{
> > + struct sppctl_gpio_chip *spp_gchip = gpiochip_get_data(chip);
> > + u32 reg;
> > +
> > + /* Upper 16-bit word is mask. Lower 16-bit word is value. */
> > + reg = (BIT(R16_BOF(offset)) << 16) | BIT(R16_BOF(offset));
> > + writel(reg, spp_gchip->gpioxt2_base + SPPCTL_GPIO_OFF_OINV + R16_ROF(offset));
> > +}
>
> Same comments about the BOF and ROF.
>
> This layout with "mask and value" in registers needs to be explained
> somewhere it looks complex. I don't understand why a machine register
> contains a mask for example.

This is a hardware mechanism for protecting some important registers from
being overwritten accidentally. The corresponding mask bit should be set
first and then the control-bits or fields can be written. The design is
originally requested from car makers.


> > +static int stpctl_set_mux(struct pinctrl_dev *pctldev, unsigned int func_selector,
> > + unsigned int group_selector)
> > +{
> > + const struct sppctl_func *f = &sppctl_list_funcs[func_selector];
> > + struct sppctl_pdata *pctl = pinctrl_dev_get_drvdata(pctldev);
> > + struct grp2fp_map g2fpm = pctl->g2fp_maps[group_selector];
> > + int i = -1, j = -1;
>
> Please do not initialize loop variable i to -1, just declare it.

Yes, I'll remove the initialization for loop variables next patch


> > + dev_dbg(pctldev->dev, "%s(func: %d, grp: %d)\n", __func__,
> > + func_selector, group_selector);
> > +
> > + switch (f->freg) {
> > + case f_off_0: /* GPIO. detouch from all funcs - ? */
> > + for (i = 0; i < sppctl_list_funcs_sz; i++) {
> > + if (sppctl_list_funcs[i].freg != f_off_m)
> > + continue;
> > + j++;
>
> Insert a comment that j is set to -1 so this will be zero here after the first
> iteration.

Yes, I'll add comments next patch.


> > + if (sppctl_func_get(pctl, j) != group_selector)
> > + continue;
> > + sppctl_func_set(pctl, j, 0);
> > + }
> > + break;
> > +
> > + case f_off_m: /* Mux */
> > + sppctl_first_master_set(&pctl->spp_gchip->chip, group_selector,
> > + mux_f_mux, mux_m_keep);
> > + sppctl_func_set(pctl, func_selector - 2, (group_selector == 0) ?
> > + group_selector : group_selector - 7);
>
> -2 and -7? Why? Add some comments or maybe #define these
> constants?

Yes, I'll defines and add some comments for the magic numbers
next patch.


> > +static int stpctl_gpio_request_enable(struct pinctrl_dev *pctldev,
> > + struct pinctrl_gpio_range *range, unsigned int offset)
> > +{
> > + struct sppctl_pdata *pctl = pinctrl_dev_get_drvdata(pctldev);
> > + struct pin_desc *pdesc;
> > + int g_f, g_m;
> > +
> > + dev_dbg(pctldev->dev, "%s(%d)\n", __func__, offset);
> > +
> > + g_f = sppctl_first_get(&pctl->spp_gchip->chip, offset);
> > + g_m = sppctl_master_get(&pctl->spp_gchip->chip, offset);
> > + if (g_f == mux_f_gpio && g_m == mux_m_gpio)
> > + return 0;
> > +
> > + pdesc = pin_desc_get(pctldev, offset);
> > + if (pdesc->mux_owner)
> > + return -EACCES;
>
> Do not reimplement the pinmux core please.
>
> What you want to achieve here is "strict pinmux", i.e. setting the field
> "strict" in struct pinmux_ops to true. Then you can just delete this
> check.

Yes, I'll remove the if-statement next patch.


> > +static const struct pinmux_ops sppctl_pinmux_ops = {
> > + .request = stpctl_request,
> > + .free = stpctl_free,
>
> These are just set to empty functions. Delete these entries
> and the empty functions.

Yes, I'll remove all 'empty' functions next patch.


> > + .get_functions_count = stpctl_get_functions_count,
> > + .get_function_name = stpctl_get_function_name,
> > + .get_function_groups = stpctl_get_function_groups,
> > + .set_mux = stpctl_set_mux,
> > + .gpio_request_enable = stpctl_gpio_request_enable,
> > + .gpio_disable_free = stpctl_gpio_disable_free,
> > + .gpio_set_direction = stpctl_gpio_set_direction,
> > + .strict = 1
>
> Use "true" rather than 1. (And do not reimplement the check.)

Yes, I'll do it next patch.


> > +static int sppctl_remove(struct platform_device *pdev)
> > +{
> > + struct sppctl_pdata *sppctl = pdev->dev.platform_data;
> > +
> > + devm_pinctrl_unregister(&pdev->dev, sppctl->pctl_dev);
>
> This defies the idea with devm_* calls. Drop remove() entirely because
> devm_ allocated resources go away by themselves.

Yes, I'll remove devm_pinctrl_unregister() next patch.
Sorry for buggy code. Fortunately, pinctrl driver
will never be removed.


> > +++ b/drivers/pinctrl/sunplus/sppctl.h
> (...)
> > +/* (/16)*4 */
> > +#define R16_ROF(r) (((r) >> 4) << 2)
> > +#define R16_BOF(r) ((r) % 16)
> > +/* (/32)*4 */
> > +#define R32_ROF(r) (((r) >> 5) << 2)
> > +#define R32_BOF(r) ((r) % 32)
>
> As mentioned I prefer explicit inlined code for these.
> The bit shifting here makes it really hard to know what is going
> on, the compiler will get it right if you use the right types
> and just write (n / 32) * 4. Please do not try to help the compiler
> optimizing it just leads to code that is hard to read.
>
> > +#define R32_VAL(r, boff) (((r) >> (boff)) & BIT(0))
>
> To check the value of a certain bit use this pattern:
>
> if (val & BIT(n))
>
> To return a boolean clamped bit (return 0/1) do this idiom:
>
> return !!(val & BIT(n));

Yes, I'll remove the four macros.
Write explicit inline code.


> Other than these things I didn't notice anything more this
> time, but I might find even more stuff, but hey it's getting there!
> Yours,
> Linus Walleij
Thanks, I'll review the whole code again and fix
the similar improper coding you pointed out.


Best regards,
Wells Lu

2021-12-10 23:36:50

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] dt-bindings: pinctrl: Add dt-bindings for Sunplus SP7021

On Fri, Dec 10, 2021 at 1:41 PM 呂芳騰 <[email protected]> wrote:

> SP7021 has 99 digital GPIO pins which are numbered from
> GPIO 0 to 98. All are multiplexed with some special function
> pins. There are 3 types special function pins:
>
> (1) function-pins:
> For example, if control-field SPI_FLASH_SEL is set to 1,
> GPIO 83, 84, 86 and 87 will be pins of SPI-NOR flash.
> If it is set to 2, GPIO 76, 78, 79 and 81 will be pins of
> SPI-NOR flash.
>
> For example 2, if control-bit UA0_SEL is set to 1,
> GPIO 88 and 89 will be TX and RX pins of UART_0
> (UART channel 0).
>
> For example 3, if control-bit EMMC_SEL is set to 1,
> GPIO 72, 73, 74, 75, 76, 77, 78, 79, 80, 81 will be
> pins of an eMMC device.
>
> The driver defines properties "function" and "groups"
> to select this kind of pins-function.
>
> (2) fully pin-mux pins:
> GPIO 8 to 71 are 'fully pin-mux' pins.
> Pins of peripherals of SP7021 (ex: UART_1, UART_2,
> UART_3, UART_4, I2C_0, I2C_1, ..., SPI_0, SPI_1, ...
> GPIO_INT0, GPIO_INT1, .., RMII_of_Ethernet, and etc.)
> can be set to any pins of fully pin-mux pins.
>
> EX1 (UART channel 1):
> If control-field UA1_TX_SEL is set to 3, TX pin of
> UART_1 will be routed to GPIO 10 (3 - 1 + 8 = 10)
> If control-field UA1_RX_SEL is set to 4, RX pin of
> UART_1 will be routed to GPIO 11 (4 - 1 + 8 = 11)
> If control-field UA1_RTS_SEL is set to 5, RTS pin of
> UART_1 will be routed to GPIO 12 (5 - 1 + 8 = 12)
> If control-field UA1_CTS_SEL is set to 6, CTS pin of
> UART_1 will be routed to GPIO 13 (6 - 1 + 8 = 13)
>
> EX2 (I2C channel 0):
> If control-field I2C0_CLK_SEL is set to 20, CLK pin
> of I2C_0 will be routed to GPIO 27 (20 - 1 + 8 = 27)
> If control-field I2C0_DATA_SEL is set to 21, DATA pin
> of I2C_0 will be routed to GPIO 28 (21 - 1 + 9 = 28)
>
> Totally, SP7021 has 120 peripheral pins. The
> peripheral pins can be routed to any of 64 'fully
> pin-mux' pins. So total combinations are:
> 120 x 64 = 7680
> This is why we cannot enumerate all combinations.

Please copy some version of the above into the top level
description in the bindings. People writing DTS files need to
know what they are doing and why this looks so funny
and if the explanation is right there in the bindings document,
that is helpful.

This is pretty cool. It is what is called a "phone exchange mux",
(anyone can call any number) which is a type that the pin
control subsystem simply assumed did not exist because it
would take up a lot of silicon space. But here it is.

So:

>> > + properties:
>> > + pins:
>> > + description: |
>> > + Define pins which are used by pinctrl node's client device.
>> (...)
>> > + $ref: /schemas/types.yaml#/definitions/uint32-array
>>
>> Why can this not $ref the standard binings in
>> Documentation/devicetree/bindings/pinctrl/pinmux-node.yaml

If this does not conform with the standard definition of pins from
pinmux-node.yaml then you need to vendor prefix them, this will
then be something like sunplus,pins.

Yours,
Linus Walleij

2021-12-10 23:45:46

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] pinctrl: Add driver for Sunplus SP7021

Hi Wells,

I understand things better now!

On Fri, Dec 10, 2021 at 6:34 PM 呂芳騰 <[email protected]> wrote:

> > #define SSPCTL_FUNC_FLAG BIT(0)
> >
> > if (func & SSPCTL_FUNC_FLAG)
> >
> > Use the name that bit has in your documentation for the
> > define so we know what is going on.
>
> Actually, 'if (func & 1)' is not used to test bit 0,
> but test 'func' is an odd number or not.
> If 'func' is even number, control-field is at bit 6 ~ 0.
> Its corresponding mask-field is at bit 22 ~ 16.
> If 'func' is odd number, control-field is at bit 14 ~ 8.
> Its corresponding mask-field is at bit 30 ~ 24.

Aha! That makes sense. Just put in exactly that comment in
a block comment in the code so people maintaining this
can see what is going on here and why you are checking
for an odd/even number.

> Control and mask fields of 'func' are arranged as shown
> below:
>
> func # | register control-field mask-field
> -------+------------------------------------
> 0 | reg[0] ( 6:0) (22 : 6)
> 1 | reg[0] (14:8) (30 : 24)
> 2 | reg[1] ( 6:0) (22 : 6)
> 3 | reg[1] (14:8) (30 : 24)

This is also nice to actually have in the code. Be generous
with comments I like that.

> > This layout with "mask and value" in registers needs to be explained
> > somewhere it looks complex. I don't understand why a machine register
> > contains a mask for example.
>
> This is a hardware mechanism for protecting some important registers from
> being overwritten accidentally. The corresponding mask bit should be set
> first and then the control-bits or fields can be written. The design is
> originally requested from car makers.

Wow that is very very interesting! So the feature is there to stop a
program that goes astray and just write random numbers all over the
memory from doing something harmful.

Insert some comments about this at the top of the file or at the first
time you use a mask.

Thanks for keeping fixing this up despite my sometimes confused
comments. Most of my comment is about making the code easy
to read by people that will maintain it, so my rule of thumb is if I
have a hard time to understand it then others may have a hard time
with it too.

Yours,
Linus Walleij