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