On Mon, 28 Nov 2022 09:32:45 +0100, Krzysztof Kozlowski wrote: > On 28/11/2022 01:48, Jianlong Huang wrote: > >>>>> +/* 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). >> >> Thanks your comments. >> These macros are used to configure pinctrl in dts, so the file should stay, > > Why they should stay? What's the reason? If it is not a constant used by > driver, then register values should not be placed in the bindings, so > drop it. > Thanks. These macros in binding header(example, DOUT, DOEN etc) will be used in DTS, and driver will parse the DT for pinctrl configuration. Example in dts: uart0_pins: uart0-0 { tx-pins { pinmux = <GPIOMUX(5, GPOUT_SYS_UART0_TX, GPOEN_ENABLE, GPI_NONE)>; bias-disable; drive-strength = <12>; input-disable; input-schmitt-disable; slew-rate = <0>; }; rx-pins { pinmux = <GPIOMUX(6, GPOUT_LOW, GPOEN_DISABLE, GPI_SYS_UART0_RX)>; bias-pull-up; drive-strength = <2>; input-enable; input-schmitt-enable; slew-rate = <0>; }; }; Best regards, Jianlong Huang