> Subject: Re: [PATCH 3/4] dt-bindings: clock: Add AST2700 clock bindings > > On 12/08/2024 11:39, Ryan Chen wrote: > >> Subject: RE: [PATCH 3/4] dt-bindings: clock: Add AST2700 clock > >> bindings > >> > >>> Subject: Re: [PATCH 3/4] dt-bindings: clock: Add AST2700 clock > >>> bindings > >>> > >>> On 12/08/2024 10:22, Ryan Chen wrote: > >>>>> Subject: Re: [PATCH 3/4] dt-bindings: clock: Add AST2700 clock > >>>>> bindings > >>>>> > >>>>> On 12/08/2024 09:26, Ryan Chen wrote: > >>>>>>> Subject: RE: [PATCH 3/4] dt-bindings: clock: Add AST2700 clock > >>>>>>> bindings > >>>>>>> > >>>>>>>> Subject: Re: [PATCH 3/4] dt-bindings: clock: Add AST2700 clock > >>>>>>>> bindings > >>>>>>>> > >>>>>>>> On 09/08/2024 07:47, Ryan Chen wrote: > >>>>>>>>>> Subject: Re: [PATCH 3/4] dt-bindings: clock: Add AST2700 > >>>>>>>>>> clock bindings > >>>>>>>>>> > >>>>>>>>>> Le 08/08/2024 à 09:59, Ryan Chen a écrit : > >>>>>>>>>>> Add dt bindings for AST2700 clock controller > >>>>>>>>>>> > >>>>>>>>>>> Signed-off-by: Ryan Chen <ryan_chen@xxxxxxxxxxxxxx> > >>>>>>>>>>> --- > >>>>>>>>>>> .../dt-bindings/clock/aspeed,ast2700-clk.h | 175 > >>>>>>>>>> ++++++++++++++++++ > >>>>>>>>>>> 1 file changed, 175 insertions(+) > >>>>>>>>>>> create mode 100644 > >>>>>>>>>>> include/dt-bindings/clock/aspeed,ast2700-clk.h > >>>>>>>>>>> > >>>>>>>>>>> diff --git a/include/dt-bindings/clock/aspeed,ast2700-clk.h > >>>>>>>>>>> b/include/dt-bindings/clock/aspeed,ast2700-clk.h > >>>>>>>>>>> new file mode 100644 > >>>>>>>>>>> index 000000000000..facf72352c3e > >>>>>>>>>>> --- /dev/null > >>>>>>>>>>> +++ b/include/dt-bindings/clock/aspeed,ast2700-clk.h > >>>>>>>>>>> @@ -0,0 +1,175 @@ > >>>>>>>>>>> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > >>>>>>>>>>> +*/ > >>>>>>>>>>> +/* > >>>>>>>>>>> + * Device Tree binding constants for AST2700 clock controller. > >>>>>>>>>>> + * > >>>>>>>>>>> + * Copyright (c) 2024 Aspeed Technology Inc. > >>>>>>>>>>> + */ > >>>>>>>>>>> + > >>>>>>>>>>> +#ifndef __DT_BINDINGS_CLOCK_AST2700_H #define > >>>>>>>>>>> +__DT_BINDINGS_CLOCK_AST2700_H > >>>>>>>>>>> + > >>>>>>>>>>> +/* SOC0 clk-gate */ > >>>>>>>>>>> +#define SCU0_CLK_GATE_MCLK (0) #define > >>> SCU0_CLK_GATE_ECLK (1) > >>>>>>>>>>> +#define SCU0_CLK_GATE_2DCLK (2) > >>>>>>>>>>> +#define SCU0_CLK_GATE_VCLK (3) #define > >> SCU0_CLK_GATE_BCLK > >>> (4) > >>>>>>>>>>> +#define SCU0_CLK_GATE_VGA0CLK (5) > >>>>>>>>>>> +#define SCU0_CLK_GATE_REFCLK (6) > >>>>>>>>>>> +#define SCU0_CLK_GATE_PORTBUSB2CLK (7) #define > >>>>>>>> SCU0_CLK_GATE_RSV8 > >>>>>>>>>>> +(8) > >>>>>>>>>>> +#define SCU0_CLK_GATE_UHCICLK (9) > >>>>>>>>>>> +#define SCU0_CLK_GATE_VGA1CLK (10) > >>>>>>>>>>> +#define SCU0_CLK_GATE_DDRPHYCLK (11) > >>>>>>>>>>> +#define SCU0_CLK_GATE_E2M0CLK (12) > >>>>>>>>>>> +#define SCU0_CLK_GATE_HACCLK (13) > >>>>>>>>>>> +#define SCU0_CLK_GATE_PORTAUSB2CLK (14) > >>>>>>>>>>> +#define SCU0_CLK_GATE_UART4CLK (15) > >>>>>>>>>>> +#define SCU0_CLK_GATE_SLICLK (16) > >>>>>>>>>>> +#define SCU0_CLK_GATE_DACCLK (17) > >>>>>>>>>>> +#define SCU0_CLK_GATE_DP (18) > >>>>>>>>>>> +#define SCU0_CLK_GATE_E2M1CLK (19) > >>>>>>>>>>> +#define SCU0_CLK_GATE_CRT0CLK (20) > >>>>>>>>>>> +#define SCU0_CLK_GATE_CRT1CLK (21) > >>>>>>>>>>> +#define SCU0_CLK_GATE_VLCLK (22) > >>>>>>>>>>> +#define SCU0_CLK_GATE_ECDSACLK (23) > >>>>>>>>>>> +#define SCU0_CLK_GATE_RSACLK (24) > >>>>>>>>>>> +#define SCU0_CLK_GATE_RVAS0CLK (25) > >>>>>>>>>>> +#define SCU0_CLK_GATE_UFSCLK (26) > >>>>>>>>>>> +#define SCU0_CLK_GATE_EMMCCLK (27) > >>>>>>>>>>> +#define SCU0_CLK_GATE_RVAS1CLK (28) > >>>>>>>>>>> +/* reserved 29 ~ 31*/ > >>>>>>>> > >>>>>>>> No, you cannot reserve IDs. They are always continous. > >>>>>>> I think for mis-understood. > >>>>>>> I will remove the comment. > >>>>>>> And keep it is continuous. Thanks. > >>>>>>>> > >>>>>>>>>>> +#define SCU0_CLK_GATE_NUM > (SCU0_CLK_GATE_RVAS1CLK + > >>> 1) > >>>>>>>> > >>>>>>>> No, not a binding. > >>>>>>> > >>>>>> I will modify by following. > >>>>>> > >>>>>> #define SCU0_CLK_GATE_RVAS1CLK (28) > >>>>>> #define SCU0_CLK_GATE_NUM (SCU0_CLK_GATE_RVAS1CLK > + > >> 1) > >>>>> > >>>>> Nothing changed. Still not a binding. Why do you send the same and > >>>>> expect different result? Drop. > >>>>> > >>>>> Address feedback sent to you from previous versions of the patchset. > >>>>> There was never a reply. > >>>> Sorry, mis-understood. > >>>> Since you think "#define SCU0_CLK_GATE_NUM" not a binding. > >>>> Do you mean I should #define SCU0_CLK_GATE_NUM in clk driver, not > >>>> in > >>> binding header, am I right? > >>> > >>> What did I write in the first Aspeed 2700 patch? So you are not > >>> going to respond there? Are you going to implement entire feedback > >>> received in the first version of the patchset? > >> > >> Apologize again, I do the internal discussion, it should not send > >> "Introduce ASPEED AST27XX BMC SoC" series patch. it should be separate > series patch. > >> It should be bite by bite, example clk driver patches, platform > >> patches, interrupt patches. > >> So I am not going to response there, prefer here. > >> > >> So I still not understood your point "not a binding" is ~ > >> > >> > > I review your point on > > https://patchwork.kernel.org/project/linux-clk/patch/20240726110355.21 > > 81563-3-kevin_chen@xxxxxxxxxxxxxx/ > > > > Do you mean I should not be gate naming here, all should be clk. > > Example +#define SCU0_CLK_GATE_RVAS1CLK -> +#define > SCU0_CLK_RVAS1 am I right? > > Drop the define for number of clocks from the header, because it is not a > 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 > in the binding, either. Sorry, I am confused. if you think that number of clocks is not related to binding. 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>; ... } It need for dtsi binding include for clock enable. If there is no binding clock include file, how device know the clock index? > > Best regards, > Krzysztof
