On 19/08/2024 08:42, Ryan Chen wrote: >> Subject: Re: [PATCH 3/4] dt-bindings: clock: Add AST2700 clock bindings >> >> On 19/08/2024 07:55, Ryan Chen wrote: >>>> Subject: Re: [PATCH 3/4] dt-bindings: clock: Add AST2700 clock >>>> bindings >>>> >>>> On 13/08/2024 03:53, Ryan Chen wrote: >>>>>> Drop the define for number of clocks from the header, because it is >>>>>> not a >>>> >>>> *NUMBER OF CLOCKS* >>>> >>>>>> binding. You can put it in the driver or not, I don't care and do >>>>>> not provide guidance on this because I don't know if it makes sense at all. >>>>>> What I know is that number of clocks is not related to binding. It >>>>>> is not needed >>>> >>>> *NUMBER OF CLOCKS* >>>> >>>>>> in the binding, either. >>>>> >>>>> Sorry, I am confused. >>>>> if you think that number of clocks is not related to binding. >>>> >>>> *NUMBER OF CLOCKS* >>>> >>>>> How dtsi claim for clk? >>>>> For example in dtsi. >>>>> include <dt-bindings/clock/aspeed,ast2700-clk.h> >>>>> usb3bhp: usb3bhp { >>>>> .... >>>>> clocks = <&syscon0 SCU0_CLK_GATE_PORTAUSB>; >>>> >>>> And where is *NUMBER OF CLOCKS* here? I don't see any problem. No >>>> useless SCU0_CLK_GATE_NUM define here. >>>> >>> Understood now, I will remove those *NUMBER OF CLOCKS*. >>> And will replace to >>> #define SCU0_CLK_END 34 >> >> NAK, it's like you keep ignoring my comments entirely. Even if you call it >> "SCU0_CLK_NOT_END" it does not change. Do you understand that it is not >> about name? Read my first comment. >> >>> >>> Refer: >>> https://github.com/torvalds/linux/blob/master/include/dt-bindings/cloc >>> k/imx8-clock.h#L87 >> >> So you found a bug and this allows you to create the same bug? >> > Sorry, I don't see this is a bug. No, it's not a bug, but I do not agree for using arguments like "someone did it, so I can do the same". Why did you pick up exactly this example instead of others who removed the clock number? > But I try to understand your point, you prefer following for clock nums, am I correct? > https://github.com/torvalds/linux/blob/master/drivers/clk/meson/g12a.c#L5558-L5559 I said that this is not a binding. Don't add to the binding things which are not a binding. I don't care how do you implement in drivers - there are several ways how to achieve it. Best regards, Krzysztof