RE: [PATCH v2 2/3] dt-bindings: pinctrl: Add dt-bindings for Sunplus SP7021

Wells Lu 呂芳騰
2021-11-18T09:04:36Z

Hi,

Thank you for your review.

> On Mon, Nov 01, 2021 at 04:11:16PM +0800, Wells Lu wrote:
> > Add dt-bindings header files for Sunplus SP7021 SoC.
> >
> > Signed-off-by: Wells Lu <wells.lu@sunplus.com>
> > ---
> > Changes in v2:
> > - Added more 'defines' in dt-bindings header files (forgot to add in v1).
> >
> > MAINTAINERS | 1 +
> > include/dt-bindings/pinctrl/sppctl-sp7021.h | 171 ++++++++++++++++++++++++++++
> > include/dt-bindings/pinctrl/sppctl.h | 40 +++++++
> > 3 files changed, 212 insertions(+)
> > create mode 100644 include/dt-bindings/pinctrl/sppctl-sp7021.h
> > create mode 100644 include/dt-bindings/pinctrl/sppctl.h
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS index fd82c77..da6378f 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -14873,6 +14873,7 @@ L: linux-arm-kernel@lists.infradead.org (moderated for
> non-subscribers)
> > S: Maintained
> > W: https://sunplus-tibbo.atlassian.net/wiki/spaces/doc/overview
> > F: drivers/pinctrl/sunplus/
> > +F: include/dt-bindings/pinctrl/sppctl*
> >
> > PKTCDVD DRIVER
> > M: linux-block@vger.kernel.org
> > diff --git a/include/dt-bindings/pinctrl/sppctl-sp7021.h
> > b/include/dt-bindings/pinctrl/sppctl-sp7021.h
> > new file mode 100644
> > index 0000000..4e07d03
> > --- /dev/null
> > +++ b/include/dt-bindings/pinctrl/sppctl-sp7021.h
> > @@ -0,0 +1,171 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
>
> Care about OS other than Linux? Should be dual licensed.

Yes, I'll add dual license for dt-binding header file.

> > +/*
> > + * SP7021 pinmux pinctrl bindings.
> > + * Copyright (C) Sunplus Tech/Tibbo Tech. 2020
> > + * Author: Dvorkin Dmitry <dvorkin@tibbo.com> */
> > +
> > +#ifndef _DT_BINDINGS_PINCTRL_SPPCTL_SP7021_H
> > +#define _DT_BINDINGS_PINCTRL_SPPCTL_SP7021_H
> > +
> > +#include <dt-bindings/pinctrl/sppctl.h>
> > +
> > +#define MUXF_GPIO 0
>
> Where do these numbers come from? Hopefully they correspond to register offsets and aren't
> made up. A comment either way here would help.

The numbers are based on define of hardware.
SP7021 supports so-called 'fully pin-mux' function.
GPIO8 ~ GPIO71 are 'fully pin-mux' pins

For example,
If I set pin-mux register of GPIO8 to 1,
GPIO8 will output L2SW_CLK_OUT signal.

If I set pin-mux register of GPIO8 to 2,
GPIO8 will output L2SW_MAC_SMI_MDC signal.

If I set pin-mux register of GPIO8 to 7,
GPIO8 will output L2SW_MAC_SMI_DMIO signal.

In fact, all signals from number 2 to 121,
can be selected to output to any fully
pin-mux pins (GPIO8 ~ GPIO71).

Numbers 121 to 155 are also based on define
of hardware. It is used to turn off specific
pin-function. With the number, drivers can
calculate the control-bit easily.

> > +#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
> > +#define GROP_ANA_I2C_IF 154
> > +#define GROP_ANA_TEST_IF 155
> > +
> > +#endif
> > diff --git a/include/dt-bindings/pinctrl/sppctl.h
> > b/include/dt-bindings/pinctrl/sppctl.h
> > new file mode 100644
> > index 0000000..3e82989
> > --- /dev/null
> > +++ b/include/dt-bindings/pinctrl/sppctl.h
> > @@ -0,0 +1,40 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * SP7021 pinmux pinctrl bindings.
> > + * Copyright (C) Sunplus Tech/Tibbo Tech. 2020
> > + * Author: Dvorkin Dmitry <dvorkin@tibbo.com> */
> > +
> > +#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)
> > +#define SPPCTL_PCTL_L_OU1 (0x01<<1)
> > +#define SPPCTL_PCTL_L_INV (0x01<<2)
> > +#define SPPCTL_PCTL_L_ONV (0x01<<3)
> > +#define SPPCTL_PCTL_L_ODR (0x01<<4)
> > +
> > +#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}, flags{8bit} */ #define
> > +SPPCTL_IOPAD(pin, typ, fun, fls)
> > +(((pin)<<24)|((typ)<<16)|((fun)<<8)|(fls))
> > +
> > +#endif
> > --
> > 2.7.4
> >
> >