diff mbox series

[3/4] dt-bindings: clock: Add AST2700 clock bindings

Message ID 20240808075937.2756733-4-ryan_chen@aspeedtech.com (mailing list archive)
State New, archived
Headers show
Series Add support for AST2700 clk driver | expand

Commit Message

Ryan Chen Aug. 8, 2024, 7:59 a.m. UTC 
Add dt bindings for AST2700 clock controller

Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
 .../dt-bindings/clock/aspeed,ast2700-clk.h    | 175 ++++++++++++++++++
 1 file changed, 175 insertions(+)
 create mode 100644 include/dt-bindings/clock/aspeed,ast2700-clk.h

Comments

Christophe JAILLET Aug. 8, 2024, 8:39 a.m. UTC | #1 
Le 08/08/2024 à 09:59, Ryan Chen a écrit :
> Add dt bindings for AST2700 clock controller
> 
> Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
> ---
>   .../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*/
> +#define SCU0_CLK_GATE_NUM	(SCU0_CLK_GATE_RVAS1CLK + 1)
> +
> +/* SOC0 clk */
> +#define SCU0_CLKIN		(SCU0_CLK_GATE_NUM + 0)

So SCU0_CLKIN is 28+1+0 = 29 which is said to be reserved in the comment 
above.

> +#define SCU0_CLK_24M		(SCU0_CLK_GATE_NUM + 1)
> +#define SCU0_CLK_192M		(SCU0_CLK_GATE_NUM + 2)
> +#define SCU0_CLK_UART		(SCU0_CLK_GATE_NUM + 3)
> +#define SCU0_CLK_PSP		(SCU0_CLK_GATE_NUM + 4)
> +#define SCU0_CLK_HPLL		(SCU0_CLK_GATE_NUM + 5)

...
Krzysztof Kozlowski Aug. 8, 2024, 10:17 a.m. UTC | #2 
On 08/08/2024 09:59, Ryan Chen wrote:
> Add dt bindings for AST2700 clock controller
> 
> Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>

NAK, why did you ignore all previous comments? Where is the changelog
and proper versioning?

Is there total mess in Aspeed that you all do the same in the same time?

Best regards,
Krzysztof
Ryan Chen Aug. 9, 2024, 5:47 a.m. UTC | #3 
> 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@aspeedtech.com>
> > ---
> >   .../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*/
> > +#define SCU0_CLK_GATE_NUM  (SCU0_CLK_GATE_RVAS1CLK + 1)
> > +
> > +/* SOC0 clk */
> > +#define SCU0_CLKIN         (SCU0_CLK_GATE_NUM + 0)
>
> So SCU0_CLKIN is 28+1+0 = 29 which is said to be reserved in the comment
> above.

Acutely, I keep reserved is because data sheet has reserved bits from 29~31.
If you have concern about it, I can remove the comment.
Or are you prefer by following?
#define SCU0_CLK_GATE_RESERVED29        (29)
#define SCU0_CLK_GATE_RESERVED30        (30)
#define SCU0_CLK_GATE_RESERVED31        (31)
#define SCU0_CLK_GATE_NUM       (SCU0_CLK_GATE_RESERVED31 + 1)

>
> > +#define SCU0_CLK_24M               (SCU0_CLK_GATE_NUM + 1)
> > +#define SCU0_CLK_192M              (SCU0_CLK_GATE_NUM + 2)
> > +#define SCU0_CLK_UART              (SCU0_CLK_GATE_NUM + 3)
> > +#define SCU0_CLK_PSP               (SCU0_CLK_GATE_NUM + 4)
> > +#define SCU0_CLK_HPLL              (SCU0_CLK_GATE_NUM + 5)
>
> ...

************* Email Confidentiality Notice ********************
免責聲明:
本信件(或其附件)可能包含機密資訊，並受法律保護。如 台端非指定之收件者，請以電子郵件通知本電子郵件之發送者, 並請立即刪除本電子郵件及其附件和銷毀所有複印件。謝謝您的合作!

DISCLAIMER:
This message (and any attachments) may contain legally privileged and/or other confidential information. If you have received it in error, please notify the sender by reply e-mail and immediately delete the e-mail and any attachments without copying or disclosing the contents. Thank you.
Krzysztof Kozlowski Aug. 9, 2024, 6:06 a.m. UTC | #4 
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@aspeedtech.com>
>>> ---
>>>   .../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.

>>> +#define SCU0_CLK_GATE_NUM  (SCU0_CLK_GATE_RVAS1CLK + 1)

No, not a binding.

>>> +
>>> +/* SOC0 clk */
>>> +#define SCU0_CLKIN         (SCU0_CLK_GATE_NUM + 0)
>>
>> So SCU0_CLKIN is 28+1+0 = 29 which is said to be reserved in the comment
>> above.
> 
> Acutely, I keep reserved is because data sheet has reserved bits from 29~31.
> If you have concern about it, I can remove the comment.
> Or are you prefer by following?
> #define SCU0_CLK_GATE_RESERVED29        (29)
> #define SCU0_CLK_GATE_RESERVED30        (30)
> #define SCU0_CLK_GATE_RESERVED31        (31)
> #define SCU0_CLK_GATE_NUM       (SCU0_CLK_GATE_RESERVED31 + 1)
> 
>>
>>> +#define SCU0_CLK_24M               (SCU0_CLK_GATE_NUM + 1)
>>> +#define SCU0_CLK_192M              (SCU0_CLK_GATE_NUM + 2)
>>> +#define SCU0_CLK_UART              (SCU0_CLK_GATE_NUM + 3)
>>> +#define SCU0_CLK_PSP               (SCU0_CLK_GATE_NUM + 4)
>>> +#define SCU0_CLK_HPLL              (SCU0_CLK_GATE_NUM + 5)
>>
>> ...
> 
> ************* Email Confidentiality Notice ********************
> 免責聲明:
> 本信件(或其附件)可能包含機密資訊，並受法律保護。如 台端非指定之收件者，請以電子郵件通知本電子郵件之發送者, 並請立即刪除本電子郵件及其附件和銷毀所有複印件。謝謝您的合作!
> 
> DISCLAIMER:
> This message (and any attachments) may contain legally privileged and/or other confidential information. If you have received it in error, please notify the sender by reply e-mail and immediately delete the e-mail and any attachments without copying or disclosing the contents. Thank you.

Maybe I am the intended recipient of your message, maybe not. I don't
want to have any legal questions regarding upstream, public
collaboration, thus probably I should just remove your messages.

Please talk with your IT that such disclaimers in open-source are not
desired (and maybe even harmful).
If you do not understand why, please also see:
https://www.youtube.com/live/fMeH7wqOwXA?si=GY7igfbda6vnjXlJ&t=835

If you need to go around company SMTP server, then consider using b4
web-relay: https://b4.docs.kernel.org/en/latest/contributor/send.html

Please be informed that by responding to this email you agree that all
communications from you and/or your company is made public. In other
words, all messages originating from you and/or your company will be
made public.

Best regards,
Krzysztof
Ryan Chen Aug. 9, 2024, 6:25 a.m. UTC | #5 
> 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@aspeedtech.com>
> >>> ---
> >>>   .../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 the subject.

>
> >>> +
> >>> +/* SOC0 clk */
> >>> +#define SCU0_CLKIN         (SCU0_CLK_GATE_NUM + 0)
> >>
> >> So SCU0_CLKIN is 28+1+0 = 29 which is said to be reserved in the
> >> comment above.
> >
> > Acutely, I keep reserved is because data sheet has reserved bits from 29~31.
> > If you have concern about it, I can remove the comment.
> > Or are you prefer by following?
> > #define SCU0_CLK_GATE_RESERVED29        (29)
> > #define SCU0_CLK_GATE_RESERVED30        (30)
> > #define SCU0_CLK_GATE_RESERVED31        (31)
> > #define SCU0_CLK_GATE_NUM       (SCU0_CLK_GATE_RESERVED31 + 1)
> >
> >>
> >>> +#define SCU0_CLK_24M               (SCU0_CLK_GATE_NUM + 1)
> >>> +#define SCU0_CLK_192M              (SCU0_CLK_GATE_NUM + 2)
> >>> +#define SCU0_CLK_UART              (SCU0_CLK_GATE_NUM + 3)
> >>> +#define SCU0_CLK_PSP               (SCU0_CLK_GATE_NUM + 4)
> >>> +#define SCU0_CLK_HPLL              (SCU0_CLK_GATE_NUM + 5)
> >>
> >> ...
> >
> > ************* Email Confidentiality Notice ********************
> > 免責聲明:
> > 本信件(或其附件)可能包含機密資訊，並受法律保護。如 台端非指定
> 之收件者，請以電子郵件通知本電子郵件之發送者, 並請立即刪除本電子
> 郵件及其附件和銷毀所有複印件。謝謝您的合作!
> >
> > DISCLAIMER:
> > This message (and any attachments) may contain legally privileged and/or
> other confidential information. If you have received it in error, please notify the
> sender by reply e-mail and immediately delete the e-mail and any
> attachments without copying or disclosing the contents. Thank you.
>
> Maybe I am the intended recipient of your message, maybe not. I don't want
> to have any legal questions regarding upstream, public collaboration, thus
> probably I should just remove your messages.
>
> Please talk with your IT that such disclaimers in open-source are not desired
> (and maybe even harmful).
> If you do not understand why, please also see:
> https://www.youtube.com/live/fMeH7wqOwXA?si=GY7igfbda6vnjXlJ&t=835
>
> If you need to go around company SMTP server, then consider using b4
> web-relay: https://b4.docs.kernel.org/en/latest/contributor/send.html
>
> Please be informed that by responding to this email you agree that all
> communications from you and/or your company is made public. In other words,
> all messages originating from you and/or your company will be made public.
>
> Best regards,
> Krzysztof

************* Email Confidentiality Notice ********************
免責聲明:
本信件(或其附件)可能包含機密資訊，並受法律保護。如 台端非指定之收件者，請以電子郵件通知本電子郵件之發送者, 並請立即刪除本電子郵件及其附件和銷毀所有複印件。謝謝您的合作!

DISCLAIMER:
This message (and any attachments) may contain legally privileged and/or other confidential information. If you have received it in error, please notify the sender by reply e-mail and immediately delete the e-mail and any attachments without copying or disclosing the contents. Thank you.
Krzysztof Kozlowski Aug. 9, 2024, 7:31 a.m. UTC | #6 
On 09/08/2024 08:25, Ryan Chen wrote:
>> 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

Again you keep sending "confidential" emails. I am not going to continue
this.

Best regards,
Krzysztof
Ryan Chen Aug. 12, 2024, 7:26 a.m. UTC | #7 
> 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@aspeedtech.com>
> > >>> ---
> > >>>   .../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) 
> 
> >
> > >>> +
> > >>> +/* SOC0 clk */
> > >>> +#define SCU0_CLKIN         (SCU0_CLK_GATE_NUM + 0)
> > >>
> > >> So SCU0_CLKIN is 28+1+0 = 29 which is said to be reserved in the
> > >> comment above.
> > >
> > > Acutely, I keep reserved is because data sheet has reserved bits from
> 29~31.
> > > If you have concern about it, I can remove the comment.
> > > Or are you prefer by following?
> > > #define SCU0_CLK_GATE_RESERVED29        (29)
> > > #define SCU0_CLK_GATE_RESERVED30        (30)
> > > #define SCU0_CLK_GATE_RESERVED31        (31)
> > > #define SCU0_CLK_GATE_NUM       (SCU0_CLK_GATE_RESERVED31 +
> 1)
> > >
> > >>
> > >>> +#define SCU0_CLK_24M               (SCU0_CLK_GATE_NUM +
> 1)
> > >>> +#define SCU0_CLK_192M              (SCU0_CLK_GATE_NUM +
> 2)
> > >>> +#define SCU0_CLK_UART              (SCU0_CLK_GATE_NUM +
> 3)
> > >>> +#define SCU0_CLK_PSP               (SCU0_CLK_GATE_NUM + 4)
> > >>> +#define SCU0_CLK_HPLL              (SCU0_CLK_GATE_NUM + 5)
> > >>
> > >> ...
> > >
> > > ************* Email Confidentiality Notice ********************
> > > 免責聲明:
> > > 本信件(或其附件)可能包含機密資訊，並受法律保護。如 台端非指定
> > 之收件者，請以電子郵件通知本電子郵件之發送者, 並請立即刪除本電
> 子
> > 郵件及其附件和銷毀所有複印件。謝謝您的合作!
> > >
> > > DISCLAIMER:
> > > This message (and any attachments) may contain legally privileged
> > > and/or
> > other confidential information. If you have received it in error,
> > please notify the sender by reply e-mail and immediately delete the
> > e-mail and any attachments without copying or disclosing the contents.
> Thank you.
> >
> > Maybe I am the intended recipient of your message, maybe not. I don't
> > want to have any legal questions regarding upstream, public
> > collaboration, thus probably I should just remove your messages.
> >
> > Please talk with your IT that such disclaimers in open-source are not
> > desired (and maybe even harmful).
> > If you do not understand why, please also see:
> > https://www.youtube.com/live/fMeH7wqOwXA?si=GY7igfbda6vnjXlJ&t=835
> >
> > If you need to go around company SMTP server, then consider using b4
> > web-relay: https://b4.docs.kernel.org/en/latest/contributor/send.html
> >
> > Please be informed that by responding to this email you agree that all
> > communications from you and/or your company is made public. In other
> > words, all messages originating from you and/or your company will be made
> public.
> >
> > Best regards,
> > Krzysztof
> 
> ************* Email Confidentiality Notice ********************
> 免責聲明:
> 本信件(或其附件)可能包含機密資訊，並受法律保護。如 台端非指定之
> 收件者，請以電子郵件通知本電子郵件之發送者, 並請立即刪除本電子郵
> 件及其附件和銷毀所有複印件。謝謝您的合作!
> 
> DISCLAIMER:
> This message (and any attachments) may contain legally privileged and/or
> other confidential information. If you have received it in error, please notify the
> sender by reply e-mail and immediately delete the e-mail and any
> attachments without copying or disclosing the contents. Thank you.
Krzysztof Kozlowski Aug. 12, 2024, 8:16 a.m. UTC | #8 
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@aspeedtech.com>
>>>>>> ---
>>>>>>   .../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.

Best regards,
Krzysztof
Ryan Chen Aug. 12, 2024, 8:22 a.m. UTC | #9 
> 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@aspeedtech.com>
> >>>>>> ---
> >>>>>>   .../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?

> 
> Best regards,
> Krzysztof
Krzysztof Kozlowski Aug. 12, 2024, 8:30 a.m. UTC | #10 
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@aspeedtech.com>
>>>>>>>> ---
>>>>>>>>   .../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?

Drop from the header. I am not saying you need to define it in the
driver, because maybe it is pointless anyway.

Best regards,
Krzysztof
Ryan Chen Aug. 12, 2024, 8:54 a.m. UTC | #11 
> 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@aspeedtech.com>
> >>>>>>>> ---
> >>>>>>>>   .../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 ~



> 
> Drop from the header. I am not saying you need to define it in the driver,
> because maybe it is pointless anyway.
> 
> Best regards,
> Krzysztof
Ryan Chen Aug. 12, 2024, 9:39 a.m. UTC | #12 
> 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@aspeedtech.com>
> > >>>>>>>> ---
> > >>>>>>>>   .../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.2181563-3-kevin_chen@aspeedtech.com/

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 from the header. I am not saying you need to define it in the
> > driver, because maybe it is pointless anyway.
> >
> > Best regards,
> > Krzysztof
Krzysztof Kozlowski Aug. 12, 2024, 9:54 a.m. UTC | #13 
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@aspeedtech.com>
>>>>>>>>>>> ---
>>>>>>>>>>>   .../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.2181563-3-kevin_chen@aspeedtech.com/
> 
> 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.

Best regards,
Krzysztof
Ryan Chen Aug. 13, 2024, 1:53 a.m. UTC | #14 
> 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@aspeedtech.com>
> >>>>>>>>>>> ---
> >>>>>>>>>>>   .../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@aspeedtech.com/
> >
> > 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
Krzysztof Kozlowski Aug. 13, 2024, 5:55 a.m. UTC | #15 
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.

> ...
> }
> 
> It need for dtsi binding include for clock enable. 
> 
> If there is no binding clock include file, how device know the clock index?

Just look how ALL other bindings for new platforms are doing it. Why are
we discussing obvious kernel aspects? Spend time to read other drivers
before posting yours.

Best regards,
Krzysztof
Ryan Chen Aug. 19, 2024, 5:55 a.m. UTC | #16 
> 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

Refer:
https://github.com/torvalds/linux/blob/master/include/dt-bindings/clock/imx8-clock.h#L87

> > ...
> > }
> >
> > It need for dtsi binding include for clock enable.
> >
> > If there is no binding clock include file, how device know the clock index?
> 
> Just look how ALL other bindings for new platforms are doing it. Why are we
> discussing obvious kernel aspects? Spend time to read other drivers before
> posting yours.
> 
> Best regards,
> Krzysztof
Krzysztof Kozlowski Aug. 19, 2024, 6:01 a.m. UTC | #17 
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/clock/imx8-clock.h#L87

So you found a bug and this allows you to create the same bug?


Best regards,
Krzysztof
Ryan Chen Aug. 19, 2024, 6:42 a.m. UTC | #18 
> 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.
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

dt-binding is index table like following.
https://github.com/torvalds/linux/blob/master/drivers/clk/meson/g12a.c#L4380

> 
> Best regards,
> Krzysztof
Krzysztof Kozlowski Aug. 19, 2024, 8:45 a.m. UTC | #19 
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
Ryan Chen Aug. 19, 2024, 9:31 a.m. UTC | #20 
> Subject: Re: [PATCH 3/4] dt-bindings: clock: Add AST2700 clock bindings
> 
> 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/cl
> >>> oc
> >>> 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.
Understood, I will remove *NUMBER OF CLOCKS*
> 
> 
> Best regards,
> Krzysztof
diff mbox series

Patch

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*/
+#define SCU0_CLK_GATE_NUM	(SCU0_CLK_GATE_RVAS1CLK + 1)
+
+/* SOC0 clk */
+#define SCU0_CLKIN		(SCU0_CLK_GATE_NUM + 0)
+#define SCU0_CLK_24M		(SCU0_CLK_GATE_NUM + 1)
+#define SCU0_CLK_192M		(SCU0_CLK_GATE_NUM + 2)
+#define SCU0_CLK_UART		(SCU0_CLK_GATE_NUM + 3)
+#define SCU0_CLK_PSP		(SCU0_CLK_GATE_NUM + 4)
+#define SCU0_CLK_HPLL		(SCU0_CLK_GATE_NUM + 5)
+#define SCU0_CLK_HPLL_DIV2	(SCU0_CLK_GATE_NUM + 6)
+#define SCU0_CLK_HPLL_DIV4	(SCU0_CLK_GATE_NUM + 7)
+#define SCU0_CLK_DPLL		(SCU0_CLK_GATE_NUM + 8)
+#define SCU0_CLK_MPLL		(SCU0_CLK_GATE_NUM + 9)
+#define SCU0_CLK_MPLL_DIV2	(SCU0_CLK_GATE_NUM + 10)
+#define SCU0_CLK_MPLL_DIV4	(SCU0_CLK_GATE_NUM + 11)
+#define SCU0_CLK_MPLL_DIV8	(SCU0_CLK_GATE_NUM + 12)
+#define SCU0_CLK_VGA0		(SCU0_CLK_GATE_NUM + 13)
+#define SCU0_CLK_VGA1		(SCU0_CLK_GATE_NUM + 14)
+#define SCU0_CLK_CRT0		(SCU0_CLK_GATE_NUM + 15)
+#define SCU0_CLK_CRT1		(SCU0_CLK_GATE_NUM + 16)
+#define SCU0_CLK_MPHY		(SCU0_CLK_GATE_NUM + 17)
+#define SCU0_CLK_AXI0		(SCU0_CLK_GATE_NUM + 18)
+#define SCU0_CLK_AXI1		(SCU0_CLK_GATE_NUM + 19)
+#define SCU0_CLK_AHB		(SCU0_CLK_GATE_NUM + 20)
+#define SCU0_CLK_APB		(SCU0_CLK_GATE_NUM + 21)
+#define SCU0_CLK_MCLK		(SCU0_CLK_GATE_NUM + 22)
+#define SCU0_CLK_ECLK		(SCU0_CLK_GATE_NUM + 23)
+#define SCU0_CLK_VCLK		(SCU0_CLK_GATE_NUM + 24)
+#define SCU0_CLK_BCLK		(SCU0_CLK_GATE_NUM + 25)
+#define SCU0_CLK_REF		(SCU0_CLK_GATE_NUM + 26)
+#define SCU0_CLK_UART4		(SCU0_CLK_GATE_NUM + 27)
+#define SCU0_CLK_SLI		(SCU0_CLK_GATE_NUM + 28)
+#define SCU0_CLK_UFS		(SCU0_CLK_GATE_NUM + 29)
+#define SCU0_CLK_EMMCMUX	(SCU0_CLK_GATE_NUM + 30)
+#define SCU0_CLK_EMMC		(SCU0_CLK_GATE_NUM + 31)
+#define SCU0_CLK_U2PHY_CLK12M	(SCU0_CLK_GATE_NUM + 32)
+#define SCU0_CLK_U2PHY_REFCLK	(SCU0_CLK_GATE_NUM + 33)
+
+#define SCU0_NUM_CLKS		(SCU0_CLK_U2PHY_REFCLK + 1)
+
+/* SOC1 clk gate */
+#define SCU1_CLK_GATE_LCLK0		(0)
+#define SCU1_CLK_GATE_LCLK1		(1)
+#define SCU1_CLK_GATE_ESPI0CLK		(2)
+#define SCU1_CLK_GATE_ESPI1CLK		(3)
+#define SCU1_CLK_GATE_SDCLK		(4)
+#define SCU1_CLK_GATE_IPEREFCLK		(5)	/* io die pcie ref clk */
+#define SCU1_CLK_GATE_RSV5CLK		(6)
+#define SCU1_CLK_GATE_LPCHCLK		(7)
+#define SCU1_CLK_GATE_MAC0CLK		(8)
+#define SCU1_CLK_GATE_MAC1CLK		(9)
+#define SCU1_CLK_GATE_MAC2CLK		(10)
+#define SCU1_CLK_GATE_UART0CLK		(11)
+#define SCU1_CLK_GATE_UART1CLK		(12)
+#define SCU1_CLK_GATE_UART2CLK		(13)
+#define SCU1_CLK_GATE_UART3CLK		(14)
+#define SCU1_CLK_GATE_I2CCLK		(15)
+#define SCU1_CLK_GATE_I3C0CLK		(16)
+#define SCU1_CLK_GATE_I3C1CLK		(17)
+#define SCU1_CLK_GATE_I3C2CLK		(18)
+#define SCU1_CLK_GATE_I3C3CLK		(19)
+#define SCU1_CLK_GATE_I3C4CLK		(20)
+#define SCU1_CLK_GATE_I3C5CLK		(21)
+#define SCU1_CLK_GATE_I3C6CLK		(22)
+#define SCU1_CLK_GATE_I3C7CLK		(23)
+#define SCU1_CLK_GATE_I3C8CLK		(24)
+#define SCU1_CLK_GATE_I3C9CLK		(25)
+#define SCU1_CLK_GATE_I3C10CLK		(26)
+#define SCU1_CLK_GATE_I3C11CLK		(27)
+#define SCU1_CLK_GATE_I3C12CLK		(28)
+#define SCU1_CLK_GATE_I3C13CLK		(29)
+#define SCU1_CLK_GATE_I3C14CLK		(30)
+#define SCU1_CLK_GATE_I3C15CLK		(31)
+
+#define SCU1_CLK_GATE_UART5CLK		(32 + 0)
+#define SCU1_CLK_GATE_UART6CLK		(32 + 1)
+#define SCU1_CLK_GATE_UART7CLK		(32 + 2)
+#define SCU1_CLK_GATE_UART8CLK		(32 + 3)
+#define SCU1_CLK_GATE_UART9CLK		(32 + 4)
+#define SCU1_CLK_GATE_UART10CLK		(32 + 5)
+#define SCU1_CLK_GATE_UART11CLK		(32 + 6)
+#define SCU1_CLK_GATE_UART12CLK		(32 + 7)
+#define SCU1_CLK_GATE_FSICLK		(32 + 8)
+#define SCU1_CLK_GATE_LTPIPHYCLK	(32 + 9)
+#define SCU1_CLK_GATE_LTPICLK		(32 + 10)
+#define SCU1_CLK_GATE_VGALCLK		(32 + 11)
+#define SCU1_CLK_GATE_USBUARTCLK	(32 + 12)
+#define SCU1_CLK_GATE_CANCLK		(32 + 13)
+#define SCU1_CLK_GATE_PCICLK		(32 + 14)
+#define SCU1_CLK_GATE_SLICLK		(32 + 15)
+#define SCU1_CLK_GATE_E2MCLK		(32 + 16)
+#define SCU1_CLK_GATE_PORTCUSB2CLK	(32 + 17)
+#define SCU1_CLK_GATE_PORTDUSB2CLK	(32 + 18)
+#define SCU1_CLK_GATE_LTPI1TXCLK	(32 + 19)
+
+#define SCU1_CLK_GATE_NUM	(SCU1_CLK_GATE_LTPI1TXCLK + 1)
+
+/* SOC1 clk */
+#define SCU1_CLKIN		(SCU1_CLK_GATE_NUM + 0)
+#define SCU1_CLK_HPLL		(SCU1_CLK_GATE_NUM + 1)
+#define SCU1_CLK_APLL		(SCU1_CLK_GATE_NUM + 2)
+#define SCU1_CLK_APLL_DIV2	(SCU1_CLK_GATE_NUM + 3)
+#define SCU1_CLK_APLL_DIV4	(SCU1_CLK_GATE_NUM + 4)
+#define SCU1_CLK_DPLL		(SCU1_CLK_GATE_NUM + 5)
+#define SCU1_CLK_UXCLK		(SCU1_CLK_GATE_NUM + 6)
+#define SCU1_CLK_HUXCLK		(SCU1_CLK_GATE_NUM + 7)
+#define SCU1_CLK_UARTX		(SCU1_CLK_GATE_NUM + 8)
+#define SCU1_CLK_HUARTX		(SCU1_CLK_GATE_NUM + 9)
+#define SCU1_CLK_AHB		(SCU1_CLK_GATE_NUM + 10)
+#define SCU1_CLK_APB		(SCU1_CLK_GATE_NUM + 11)
+#define SCU1_CLK_UART0		(SCU1_CLK_GATE_NUM + 12)
+#define SCU1_CLK_UART1		(SCU1_CLK_GATE_NUM + 13)
+#define SCU1_CLK_UART2		(SCU1_CLK_GATE_NUM + 14)
+#define SCU1_CLK_UART3		(SCU1_CLK_GATE_NUM + 15)
+#define SCU1_CLK_UART5		(SCU1_CLK_GATE_NUM + 16)
+#define SCU1_CLK_UART6		(SCU1_CLK_GATE_NUM + 17)
+#define SCU1_CLK_UART7		(SCU1_CLK_GATE_NUM + 18)
+#define SCU1_CLK_UART8		(SCU1_CLK_GATE_NUM + 19)
+#define SCU1_CLK_UART9		(SCU1_CLK_GATE_NUM + 20)
+#define SCU1_CLK_UART10		(SCU1_CLK_GATE_NUM + 21)
+#define SCU1_CLK_UART11		(SCU1_CLK_GATE_NUM + 22)
+#define SCU1_CLK_UART12		(SCU1_CLK_GATE_NUM + 23)
+#define SCU1_CLK_APLL_DIVN	(SCU1_CLK_GATE_NUM + 24)
+#define SCU1_CLK_SDMUX		(SCU1_CLK_GATE_NUM + 25)
+#define SCU1_CLK_SDCLK		(SCU1_CLK_GATE_NUM + 26)
+#define SCU1_CLK_RMII		(SCU1_CLK_GATE_NUM + 27)
+#define SCU1_CLK_RGMII		(SCU1_CLK_GATE_NUM + 28)
+#define SCU1_CLK_MACHCLK	(SCU1_CLK_GATE_NUM + 29)
+#define SCU1_CLK_MAC0RCLK	(SCU1_CLK_GATE_NUM + 30)
+#define SCU1_CLK_MAC1RCLK	(SCU1_CLK_GATE_NUM + 31)
+
+#define SCU1_NUM_CLKS		(SCU1_CLK_MAC1RCLK + 1)
+
+#endif