On Tue, 29 Nov 2022 15:58:12 +0100, Krzysztof Kozlowski wrote: > On 29/11/2022 15:46, Jianlong Huang wrote: >> On Tue, 29 Nov 2022 08:49:49 +0100, Krzysztof Kozlowski wrote: >>> On 29/11/2022 02:47, Jianlong Huang wrote: >>>> 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)>; >>> >>> This is usage in DTS and is not an argument to store register >>> addresses/offsets as bindings. What is the usage (of define, not value) >>> in the driver? >>> >> >> The existing implementation reuse the macros for DTS and driver. > > Where in the driver? Grep gives zero results. > >> Do you mean we need to separate the macros, one for DTS and one for driver usage? > > No, if driver uses them it is fine. The problem is I cannot find it > anywhere. > >> Or you have any better suggestion? >> >> These macros are the value of register, not register addresses/offsets, >> except for with prefix of GPI. > > Still, values are not usually part of bindings. > >> >> Drivers rarely reference macros directly, mostly parsing dts and writing them to registers. > > So drivers do not use macros? Then there is no reason to store them in > bindings? What do you "bind" if there is no usage (and we do not talk > about DTS...)? > Where do you suggest to store these macros used in DTS? Best regards, Jianlong Huang