On 21/12/2023 09:36, Alex Soo wrote: > Add pinctrl_east/pinctrl_west/pinctrl_gmac/pinctrl_aon device > tree nodes for JH8100 SoC. > > Signed-off-by: Alex Soo <yuklin.soo@xxxxxxxxxxxxxxxx> > Reviewed-by: Ley Foon Tan <leyfoon.tan@xxxxxxxxxxxxxxxx> I have some doubts about it... > --- > arch/riscv/boot/dts/starfive/jh8100-evb.dts | 5 + > arch/riscv/boot/dts/starfive/jh8100-pinfunc.h | 418 ++++++++++++++++++ > arch/riscv/boot/dts/starfive/jh8100.dtsi | 44 ++ > 3 files changed, 467 insertions(+) > create mode 100644 arch/riscv/boot/dts/starfive/jh8100-pinfunc.h > > diff --git a/arch/riscv/boot/dts/starfive/jh8100-evb.dts b/arch/riscv/boot/dts/starfive/jh8100-evb.dts > index c16bc25d8988..8634e41984f8 100644 > --- a/arch/riscv/boot/dts/starfive/jh8100-evb.dts > +++ b/arch/riscv/boot/dts/starfive/jh8100-evb.dts > @@ -26,3 +26,8 @@ memory@40000000 { > &uart0 { > status = "okay"; > }; > + > +&pinctrl_aon { Wrong order. Nodes do not go to the end. > + wakeup-gpios = <&pinctrl_aon PAD_RGPIO2 GPIO_ACTIVE_HIGH>; > + wakeup-source; None of these were tested. It does not look like you tested the DTS against bindings. Please run `make dtbs_check W=1` (see Documentation/devicetree/bindings/writing-schema.rst or https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ for instructions). > +}; > diff --git a/arch/riscv/boot/dts/starfive/jh8100-pinfunc.h b/arch/riscv/boot/dts/starfive/jh8100-pinfunc.h > new file mode 100644 > index 000000000000..3fb16ef62d90 > --- /dev/null > +++ b/arch/riscv/boot/dts/starfive/jh8100-pinfunc.h > @@ -0,0 +1,418 @@ > +/* SPDX-License-Identifier: GPL-2.0 OR MIT */ > +/* > + * Copyright (C) 2023 StarFive Technology Co., Ltd. > + * Author: Alex Soo <yuklin.soo@xxxxxxxxxxxxxxxx> > + * > + */ > + > +#ifndef __JH8100_PINFUNC_H__ > +#define __JH8100_PINFUNC_H__ > + > +/* > + * mux bits: > + * | 31 - 24 | 23 - 16 | 15 - 10 | 9 - 8 | 7 - 0 | > + * | din | dout | doen | function | gpio nr | > + * > + * dout: output signal > + * doen: output enable signal > + * din: optional input signal, 0xff = none > + * function: > + * gpio nr: gpio number, 0 - 63 > + */ > +#define GPIOMUX(n, dout, doen, din) ( \ > + (((din) & 0xff) << 24) | \ > + (((dout) & 0xff) << 16) | \ > + (((doen) & 0x3f) << 10) | \ > + ((n) & 0x3f)) > + > +#define PINMUX(n, func) ((1 << 10) | (((func) & 0x3) << 8) | ((n) & 0xff)) > + > +/* sys_iomux_east dout */ > +#define GPOUT_LOW 0 > +#define GPOUT_HIGH 1 Where are these used? > +#define GPOUT_SYS_CAN0_STBY 2 > +#define GPOUT_SYS_CAN0_TST_NEXT_BIT 3 > +#define GPOUT_SYS_CAN0_TST_SAMPLE_POINT 4 > +#define GPOUT_SYS_CAN0_TXD 5 > +#define GPOUT_SYS_I2C0_CLK 6 > +#define GPOUT_SYS_I2C0_DATA 7 > +#define GPOUT_SYS_I2S0_STEREO_RSCKO 8 You add here bunch of constants not used anywhere. No single example of their usage, not a one. Best regards, Krzysztof