[PATCH 0/4] Add support for AST2700 clk driver

[PATCH 0/4] Add support for AST2700 clk driver

[Ryan Chen]

This patch series is add clk driver for AST2700.

AST2700 is the 8th generation of Integrated Remote Management Processor
introduced by ASPEED Technology Inc. Which is Board Management controller
(BMC) SoC family.

Ryan Chen (4):
  dt-bindings: mfd: aspeed: support for AST2700
  dt-bindings: reset Add AST2700 reset bindings
  dt-bindings: clock: Add AST2700 clock bindings
  dt-bindings: clock: Add AST2700 clock bindings

 .../bindings/mfd/aspeed,ast2x00-scu.yaml      |   31 +-
 drivers/clk/Makefile                          |    1 +
 drivers/clk/clk-ast2700.c                     | 1091 +++++++++++++++++
 .../dt-bindings/clock/aspeed,ast2700-clk.h    |  175 +++
 .../dt-bindings/reset/aspeed,ast2700-reset.h  |  132 ++
 5 files changed, 1428 insertions(+), 2 deletions(-)
 create mode 100644 drivers/clk/clk-ast2700.c
 create mode 100644 include/dt-bindings/clock/aspeed,ast2700-clk.h
 create mode 100644 include/dt-bindings/reset/aspeed,ast2700-reset.h

-- 
2.34.1

[PATCH 1/4] dt-bindings: mfd: aspeed: support for AST2700

[Ryan Chen]

Add compatible support for AST2700 clk, reset, pinctrl, silicon-id
and example for AST2700 scu.

Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
 .../bindings/mfd/aspeed,ast2x00-scu.yaml      | 31 +++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml b/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
index 86ee69c0f45b..c0965f08ae8c 100644
--- a/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
+++ b/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
@@ -21,6 +21,8 @@ properties:
           - aspeed,ast2400-scu
           - aspeed,ast2500-scu
           - aspeed,ast2600-scu
+          - aspeed,ast2700-scu0
+          - aspeed,ast2700-scu1
       - const: syscon
       - const: simple-mfd
 
@@ -30,10 +32,12 @@ properties:
   ranges: true
 
   '#address-cells':
-    const: 1
+    minimum: 1
+    maximum: 2
 
   '#size-cells':
-    const: 1
+    minimum: 1
+    maximum: 2
 
   '#clock-cells':
     const: 1
@@ -56,6 +60,8 @@ patternProperties:
             - aspeed,ast2400-pinctrl
             - aspeed,ast2500-pinctrl
             - aspeed,ast2600-pinctrl
+            - aspeed,ast2700-soc0-pinctrl
+            - aspeed,ast2700-soc1-pinctrl
 
     required:
       - compatible
@@ -76,6 +82,7 @@ patternProperties:
               - aspeed,ast2400-silicon-id
               - aspeed,ast2500-silicon-id
               - aspeed,ast2600-silicon-id
+              - aspeed,ast2700-silicon-id
           - const: aspeed,silicon-id
 
       reg:
@@ -115,4 +122,24 @@ examples:
             reg = <0x7c 0x4>, <0x150 0x8>;
         };
     };
+  - |
+    soc0 {
+        #address-cells = <2>;
+        #size-cells = <2>;
+        syscon@12c02000 {
+            compatible = "aspeed,ast2700-scu0", "syscon", "simple-mfd";
+            reg = <0x0 0x12c02000 0x0 0x1000>;
+            #clock-cells = <1>;
+            #reset-cells = <1>;
+
+            #address-cells = <2>;
+            #size-cells = <2>;
+            ranges = <0x0 0x0 0 0x12c02000 0 0x1000>;
+
+            silicon-id@0 {
+                compatible = "aspeed,ast2700-silicon-id", "aspeed,silicon-id";
+                reg = <0x0 0x0 0x0 0x4>;
+            };
+        };
+    };
 ...
-- 
2.34.1

Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for AST2700

[Krzysztof Kozlowski]

On 08/08/2024 09:59, Ryan Chen wrote:
> Add compatible support for AST2700 clk, reset, pinctrl, silicon-id
> and example for AST2700 scu.
> 
> Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
> ---
>  .../bindings/mfd/aspeed,ast2x00-scu.yaml      | 31 +++++++++++++++++--
>  1 file changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml b/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
> index 86ee69c0f45b..c0965f08ae8c 100644
> --- a/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
> +++ b/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
> @@ -21,6 +21,8 @@ properties:
>            - aspeed,ast2400-scu
>            - aspeed,ast2500-scu
>            - aspeed,ast2600-scu
> +          - aspeed,ast2700-scu0
> +          - aspeed,ast2700-scu1

What are the differences between these two?

>        - const: syscon
>        - const: simple-mfd
>  
> @@ -30,10 +32,12 @@ properties:
>    ranges: true
>  
>    '#address-cells':
> -    const: 1
> +    minimum: 1
> +    maximum: 2
>  
>    '#size-cells':
> -    const: 1
> +    minimum: 1
> +    maximum: 2
>  
>    '#clock-cells':
>      const: 1
> @@ -56,6 +60,8 @@ patternProperties:
>              - aspeed,ast2400-pinctrl
>              - aspeed,ast2500-pinctrl
>              - aspeed,ast2600-pinctrl
> +            - aspeed,ast2700-soc0-pinctrl
> +            - aspeed,ast2700-soc1-pinctrl
>  
>      required:
>        - compatible
> @@ -76,6 +82,7 @@ patternProperties:
>                - aspeed,ast2400-silicon-id
>                - aspeed,ast2500-silicon-id
>                - aspeed,ast2600-silicon-id
> +              - aspeed,ast2700-silicon-id
>            - const: aspeed,silicon-id
>  
>        reg:
> @@ -115,4 +122,24 @@ examples:
>              reg = <0x7c 0x4>, <0x150 0x8>;
>          };
>      };
> +  - |
> +    soc0 {
> +        #address-cells = <2>;
> +        #size-cells = <2>;

That's the same example as previous, right? The drop, no need.

Best regards,
Krzysztof

RE: [PATCH 1/4] dt-bindings: mfd: aspeed: support for AST2700

[Ryan Chen]

> Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for AST2700
>
> On 08/08/2024 09:59, Ryan Chen wrote:
> > Add compatible support for AST2700 clk, reset, pinctrl, silicon-id and
> > example for AST2700 scu.
> >
> > Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
> > ---
> >  .../bindings/mfd/aspeed,ast2x00-scu.yaml      | 31
> +++++++++++++++++--
> >  1 file changed, 29 insertions(+), 2 deletions(-)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
> > b/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
> > index 86ee69c0f45b..c0965f08ae8c 100644
> > --- a/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
> > +++ b/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
> > @@ -21,6 +21,8 @@ properties:
> >            - aspeed,ast2400-scu
> >            - aspeed,ast2500-scu
> >            - aspeed,ast2600-scu
> > +          - aspeed,ast2700-scu0
> > +          - aspeed,ast2700-scu1
>
> What are the differences between these two?

The next [PATCH 4/4] is scu driver that include ast2700-scu0 and ast2700-scu1
CLK_OF_DECLARE_DRIVER(ast2700_soc0, "aspeed,ast2700-scu0", ast2700_soc0_clk_init);
CLK_OF_DECLARE_DRIVER(ast2700_soc1, "aspeed,ast2700-scu1", ast2700_soc1_clk_init);
So I add these two.

>
> >        - const: syscon
> >        - const: simple-mfd
> >
> > @@ -30,10 +32,12 @@ properties:
> >    ranges: true
> >
> >    '#address-cells':
> > -    const: 1
> > +    minimum: 1
> > +    maximum: 2
> >
> >    '#size-cells':
> > -    const: 1
> > +    minimum: 1
> > +    maximum: 2
> >
> >    '#clock-cells':
> >      const: 1
> > @@ -56,6 +60,8 @@ patternProperties:
> >              - aspeed,ast2400-pinctrl
> >              - aspeed,ast2500-pinctrl
> >              - aspeed,ast2600-pinctrl
> > +            - aspeed,ast2700-soc0-pinctrl
> > +            - aspeed,ast2700-soc1-pinctrl
> >
> >      required:
> >        - compatible
> > @@ -76,6 +82,7 @@ patternProperties:
> >                - aspeed,ast2400-silicon-id
> >                - aspeed,ast2500-silicon-id
> >                - aspeed,ast2600-silicon-id
> > +              - aspeed,ast2700-silicon-id
> >            - const: aspeed,silicon-id
> >
> >        reg:
> > @@ -115,4 +122,24 @@ examples:
> >              reg = <0x7c 0x4>, <0x150 0x8>;
> >          };
> >      };
> > +  - |
> > +    soc0 {
> > +        #address-cells = <2>;
> > +        #size-cells = <2>;
>
> That's the same example as previous, right? The drop, no need.

AST2700 is 64bits address mode platform, that the reason.
So I add example for 64bits platform descript in dtsi
I have to add soc0 to be address-cells and size-cells to be <2>
Then I can define the register to be 64bits address and size.
>
> 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.

Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for AST2700

[Krzysztof Kozlowski]

On 09/08/2024 07:55, Ryan Chen wrote:
>> Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for AST2700
>>
>> On 08/08/2024 09:59, Ryan Chen wrote:
>>> Add compatible support for AST2700 clk, reset, pinctrl, silicon-id and
>>> example for AST2700 scu.
>>>
>>> Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
>>> ---
>>>  .../bindings/mfd/aspeed,ast2x00-scu.yaml      | 31
>> +++++++++++++++++--
>>>  1 file changed, 29 insertions(+), 2 deletions(-)
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
>>> b/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
>>> index 86ee69c0f45b..c0965f08ae8c 100644
>>> --- a/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
>>> +++ b/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
>>> @@ -21,6 +21,8 @@ properties:
>>>            - aspeed,ast2400-scu
>>>            - aspeed,ast2500-scu
>>>            - aspeed,ast2600-scu
>>> +          - aspeed,ast2700-scu0
>>> +          - aspeed,ast2700-scu1
>>
>> What are the differences between these two?
> 
> The next [PATCH 4/4] is scu driver that include ast2700-scu0 and ast2700-scu1
> CLK_OF_DECLARE_DRIVER(ast2700_soc0, "aspeed,ast2700-scu0", ast2700_soc0_clk_init);
> CLK_OF_DECLARE_DRIVER(ast2700_soc1, "aspeed,ast2700-scu1", ast2700_soc1_clk_init);

What are hardware differences? Entirely different devices?

> So I add these two.
> 
>>
>>>        - const: syscon
>>>        - const: simple-mfd
>>>
>>> @@ -30,10 +32,12 @@ properties:
>>>    ranges: true
>>>
>>>    '#address-cells':
>>> -    const: 1
>>> +    minimum: 1
>>> +    maximum: 2
>>>
>>>    '#size-cells':
>>> -    const: 1
>>> +    minimum: 1
>>> +    maximum: 2
>>>
>>>    '#clock-cells':
>>>      const: 1
>>> @@ -56,6 +60,8 @@ patternProperties:
>>>              - aspeed,ast2400-pinctrl
>>>              - aspeed,ast2500-pinctrl
>>>              - aspeed,ast2600-pinctrl
>>> +            - aspeed,ast2700-soc0-pinctrl
>>> +            - aspeed,ast2700-soc1-pinctrl
>>>
>>>      required:
>>>        - compatible
>>> @@ -76,6 +82,7 @@ patternProperties:
>>>                - aspeed,ast2400-silicon-id
>>>                - aspeed,ast2500-silicon-id
>>>                - aspeed,ast2600-silicon-id
>>> +              - aspeed,ast2700-silicon-id
>>>            - const: aspeed,silicon-id
>>>
>>>        reg:
>>> @@ -115,4 +122,24 @@ examples:
>>>              reg = <0x7c 0x4>, <0x150 0x8>;
>>>          };
>>>      };
>>> +  - |
>>> +    soc0 {
>>> +        #address-cells = <2>;
>>> +        #size-cells = <2>;
>>
>> That's the same example as previous, right? The drop, no need.
> 
> AST2700 is 64bits address mode platform, that the reason.
> So I add example for 64bits platform descript in dtsi
> I have to add soc0 to be address-cells and size-cells to be <2>
> Then I can define the register to be 64bits address and size.

That's trivial. Drop.

>>
>> 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.

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

I will not respond to any other confidential emails. That's the last one
you got.

To be clear: all messages from your company will be made published. By
responding to this email you agree that all communications from you
and/or your company is made public.

Best regards,
Krzysztof

RE: [PATCH 1/4] dt-bindings: mfd: aspeed: support for AST2700

[Ryan Chen]

> Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for AST2700
>
> On 09/08/2024 07:55, Ryan Chen wrote:
> >> Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for
> >> AST2700
> >>
> >> On 08/08/2024 09:59, Ryan Chen wrote:
> >>> Add compatible support for AST2700 clk, reset, pinctrl, silicon-id
> >>> and example for AST2700 scu.
> >>>
> >>> Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
> >>> ---
> >>>  .../bindings/mfd/aspeed,ast2x00-scu.yaml      | 31
> >> +++++++++++++++++--
> >>>  1 file changed, 29 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git
> >>> a/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
> >>> b/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
> >>> index 86ee69c0f45b..c0965f08ae8c 100644
> >>> --- a/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
> >>> +++ b/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
> >>> @@ -21,6 +21,8 @@ properties:
> >>>            - aspeed,ast2400-scu
> >>>            - aspeed,ast2500-scu
> >>>            - aspeed,ast2600-scu
> >>> +          - aspeed,ast2700-scu0
> >>> +          - aspeed,ast2700-scu1
> >>
> >> What are the differences between these two?
> >
> > The next [PATCH 4/4] is scu driver that include ast2700-scu0 and
> > ast2700-scu1 CLK_OF_DECLARE_DRIVER(ast2700_soc0,
> > "aspeed,ast2700-scu0", ast2700_soc0_clk_init);
> > CLK_OF_DECLARE_DRIVER(ast2700_soc1, "aspeed,ast2700-scu1",
> > ast2700_soc1_clk_init);
>
> What are hardware differences? Entirely different devices?

AST2700 have two soc die connected each other.
Each soc die have it own scu, so the naming is ast2700-scu0 for soc0, another is ast2700-scu1 for soc1.

>
> > So I add these two.
> >
> >>
> >>>        - const: syscon
> >>>        - const: simple-mfd
> >>>
> >>> @@ -30,10 +32,12 @@ properties:
> >>>    ranges: true
> >>>
> >>>    '#address-cells':
> >>> -    const: 1
> >>> +    minimum: 1
> >>> +    maximum: 2
> >>>
> >>>    '#size-cells':
> >>> -    const: 1
> >>> +    minimum: 1
> >>> +    maximum: 2
> >>>
> >>>    '#clock-cells':
> >>>      const: 1
> >>> @@ -56,6 +60,8 @@ patternProperties:
> >>>              - aspeed,ast2400-pinctrl
> >>>              - aspeed,ast2500-pinctrl
> >>>              - aspeed,ast2600-pinctrl
> >>> +            - aspeed,ast2700-soc0-pinctrl
> >>> +            - aspeed,ast2700-soc1-pinctrl
> >>>
> >>>      required:
> >>>        - compatible
> >>> @@ -76,6 +82,7 @@ patternProperties:
> >>>                - aspeed,ast2400-silicon-id
> >>>                - aspeed,ast2500-silicon-id
> >>>                - aspeed,ast2600-silicon-id
> >>> +              - aspeed,ast2700-silicon-id
> >>>            - const: aspeed,silicon-id
> >>>
> >>>        reg:
> >>> @@ -115,4 +122,24 @@ examples:
> >>>              reg = <0x7c 0x4>, <0x150 0x8>;
> >>>          };
> >>>      };
> >>> +  - |
> >>> +    soc0 {
> >>> +        #address-cells = <2>;
> >>> +        #size-cells = <2>;
> >>
> >> That's the same example as previous, right? The drop, no need.
> >
> > AST2700 is 64bits address mode platform, that the reason.
> > So I add example for 64bits platform descript in dtsi I have to add
> > soc0 to be address-cells and size-cells to be <2> Then I can define
> > the register to be 64bits address and size.
>
> That's trivial. Drop.
Do you mean, I don’t need add example for ast2700-scu0?

Or delete #address-cells = <2>;  #size-cells = <2>;
If I remove it will make dt_binding_check fail.
>
> >>
> >> 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.
>
> 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
>
> I will not respond to any other confidential emails. That's the last one you got.
>
> To be clear: all messages from your company will be made published. By
> responding to this email you agree that all communications from you and/or
> your company is 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.

RE: [PATCH 1/4] dt-bindings: mfd: aspeed: support for AST2700

[Ryan Chen]

> Subject: RE: [PATCH 1/4] dt-bindings: mfd: aspeed: support for AST2700
> 
> > Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for AST2700
> >
> > On 09/08/2024 07:55, Ryan Chen wrote:
> > >> Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for
> > >> AST2700
> > >>
> > >> On 08/08/2024 09:59, Ryan Chen wrote:
> > >>> Add compatible support for AST2700 clk, reset, pinctrl, silicon-id
> > >>> and example for AST2700 scu.
> > >>>
> > >>> Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
> > >>> ---
> > >>>  .../bindings/mfd/aspeed,ast2x00-scu.yaml      | 31
> > >> +++++++++++++++++--
> > >>>  1 file changed, 29 insertions(+), 2 deletions(-)
> > >>>
> > >>> diff --git
> > >>> a/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
> > >>> b/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
> > >>> index 86ee69c0f45b..c0965f08ae8c 100644
> > >>> ---
> > >>> a/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
> > >>> +++
> b/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yam
> > >>> +++ l
> > >>> @@ -21,6 +21,8 @@ properties:
> > >>>            - aspeed,ast2400-scu
> > >>>            - aspeed,ast2500-scu
> > >>>            - aspeed,ast2600-scu
> > >>> +          - aspeed,ast2700-scu0
> > >>> +          - aspeed,ast2700-scu1
> > >>
> > >> What are the differences between these two?
> > >
> > > The next [PATCH 4/4] is scu driver that include ast2700-scu0 and
> > > ast2700-scu1 CLK_OF_DECLARE_DRIVER(ast2700_soc0,
> > > "aspeed,ast2700-scu0", ast2700_soc0_clk_init);
> > > CLK_OF_DECLARE_DRIVER(ast2700_soc1, "aspeed,ast2700-scu1",
> > > ast2700_soc1_clk_init);
> >
> > What are hardware differences? Entirely different devices?
> 
> AST2700 have two soc die connected each other.
> Each soc die have it own scu, so the naming is ast2700-scu0 for soc0, another
> is ast2700-scu1 for soc1.
> 
> >
> > > So I add these two.
> > >
> > >>
> > >>>        - const: syscon
> > >>>        - const: simple-mfd
> > >>>
> > >>> @@ -30,10 +32,12 @@ properties:
> > >>>    ranges: true
> > >>>
> > >>>    '#address-cells':
> > >>> -    const: 1
> > >>> +    minimum: 1
> > >>> +    maximum: 2
> > >>>
> > >>>    '#size-cells':
> > >>> -    const: 1
> > >>> +    minimum: 1
> > >>> +    maximum: 2
> > >>>
> > >>>    '#clock-cells':
> > >>>      const: 1
> > >>> @@ -56,6 +60,8 @@ patternProperties:
> > >>>              - aspeed,ast2400-pinctrl
> > >>>              - aspeed,ast2500-pinctrl
> > >>>              - aspeed,ast2600-pinctrl
> > >>> +            - aspeed,ast2700-soc0-pinctrl
> > >>> +            - aspeed,ast2700-soc1-pinctrl
> > >>>
> > >>>      required:
> > >>>        - compatible
> > >>> @@ -76,6 +82,7 @@ patternProperties:
> > >>>                - aspeed,ast2400-silicon-id
> > >>>                - aspeed,ast2500-silicon-id
> > >>>                - aspeed,ast2600-silicon-id
> > >>> +              - aspeed,ast2700-silicon-id
> > >>>            - const: aspeed,silicon-id
> > >>>
> > >>>        reg:
> > >>> @@ -115,4 +122,24 @@ examples:
> > >>>              reg = <0x7c 0x4>, <0x150 0x8>;
> > >>>          };
> > >>>      };
> > >>> +  - |
> > >>> +    soc0 {
> > >>> +        #address-cells = <2>;
> > >>> +        #size-cells = <2>;
> > >>
> > >> That's the same example as previous, right? The drop, no need.
> > >
> > > AST2700 is 64bits address mode platform, that the reason.
> > > So I add example for 64bits platform descript in dtsi I have to add
> > > soc0 to be address-cells and size-cells to be <2> Then I can define
> > > the register to be 64bits address and size.
> >
> > That's trivial. Drop.
> Do you mean, I don’t need add example for ast2700-scu0?
> 
> Or delete #address-cells = <2>;  #size-cells = <2>; If I remove it will make
> dt_binding_check fail.
> >
Hello Krzysztof

Use dt_binding_check, it need #address-cells = <2>;  #size-cells = <2> for 64 bit address description.
Or I don't need example?

> > >>
> > >> 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.
> >
> > 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
> >
> > I will not respond to any other confidential emails. That's the last one you
> got.
> >
> > To be clear: all messages from your company will be made published. By
> > responding to this email you agree that all communications from you
> > and/or your company is made public.

Sorry, I have request IT remove it.
> >
> > Best regards,
> > Krzysztof
> 

Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for AST2700

[Krzysztof Kozlowski]

On 12/08/2024 08:26, Ryan Chen wrote:
>> Subject: RE: [PATCH 1/4] dt-bindings: mfd: aspeed: support for AST2700
>>
>>> Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for AST2700
>>>
>>> On 09/08/2024 07:55, Ryan Chen wrote:
>>>>> Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for
>>>>> AST2700
>>>>>
>>>>> On 08/08/2024 09:59, Ryan Chen wrote:
>>>>>> Add compatible support for AST2700 clk, reset, pinctrl, silicon-id
>>>>>> and example for AST2700 scu.
>>>>>>
>>>>>> Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
>>>>>> ---
>>>>>>  .../bindings/mfd/aspeed,ast2x00-scu.yaml      | 31
>>>>> +++++++++++++++++--
>>>>>>  1 file changed, 29 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git
>>>>>> a/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
>>>>>> b/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
>>>>>> index 86ee69c0f45b..c0965f08ae8c 100644
>>>>>> ---
>>>>>> a/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
>>>>>> +++
>> b/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yam
>>>>>> +++ l
>>>>>> @@ -21,6 +21,8 @@ properties:
>>>>>>            - aspeed,ast2400-scu
>>>>>>            - aspeed,ast2500-scu
>>>>>>            - aspeed,ast2600-scu
>>>>>> +          - aspeed,ast2700-scu0
>>>>>> +          - aspeed,ast2700-scu1
>>>>>
>>>>> What are the differences between these two?
>>>>
>>>> The next [PATCH 4/4] is scu driver that include ast2700-scu0 and
>>>> ast2700-scu1 CLK_OF_DECLARE_DRIVER(ast2700_soc0,
>>>> "aspeed,ast2700-scu0", ast2700_soc0_clk_init);
>>>> CLK_OF_DECLARE_DRIVER(ast2700_soc1, "aspeed,ast2700-scu1",
>>>> ast2700_soc1_clk_init);
>>>
>>> What are hardware differences? Entirely different devices?
>>
>> AST2700 have two soc die connected each other.
>> Each soc die have it own scu, so the naming is ast2700-scu0 for soc0, another
>> is ast2700-scu1 for soc1.
>>
>>>
>>>> So I add these two.
>>>>
>>>>>
>>>>>>        - const: syscon
>>>>>>        - const: simple-mfd
>>>>>>
>>>>>> @@ -30,10 +32,12 @@ properties:
>>>>>>    ranges: true
>>>>>>
>>>>>>    '#address-cells':
>>>>>> -    const: 1
>>>>>> +    minimum: 1
>>>>>> +    maximum: 2
>>>>>>
>>>>>>    '#size-cells':
>>>>>> -    const: 1
>>>>>> +    minimum: 1
>>>>>> +    maximum: 2
>>>>>>
>>>>>>    '#clock-cells':
>>>>>>      const: 1
>>>>>> @@ -56,6 +60,8 @@ patternProperties:
>>>>>>              - aspeed,ast2400-pinctrl
>>>>>>              - aspeed,ast2500-pinctrl
>>>>>>              - aspeed,ast2600-pinctrl
>>>>>> +            - aspeed,ast2700-soc0-pinctrl
>>>>>> +            - aspeed,ast2700-soc1-pinctrl
>>>>>>
>>>>>>      required:
>>>>>>        - compatible
>>>>>> @@ -76,6 +82,7 @@ patternProperties:
>>>>>>                - aspeed,ast2400-silicon-id
>>>>>>                - aspeed,ast2500-silicon-id
>>>>>>                - aspeed,ast2600-silicon-id
>>>>>> +              - aspeed,ast2700-silicon-id
>>>>>>            - const: aspeed,silicon-id
>>>>>>
>>>>>>        reg:
>>>>>> @@ -115,4 +122,24 @@ examples:
>>>>>>              reg = <0x7c 0x4>, <0x150 0x8>;
>>>>>>          };
>>>>>>      };
>>>>>> +  - |
>>>>>> +    soc0 {
>>>>>> +        #address-cells = <2>;
>>>>>> +        #size-cells = <2>;
>>>>>
>>>>> That's the same example as previous, right? The drop, no need.
>>>>
>>>> AST2700 is 64bits address mode platform, that the reason.
>>>> So I add example for 64bits platform descript in dtsi I have to add
>>>> soc0 to be address-cells and size-cells to be <2> Then I can define
>>>> the register to be 64bits address and size.
>>>
>>> That's trivial. Drop.
>> Do you mean, I don’t need add example for ast2700-scu0?
>>
>> Or delete #address-cells = <2>;  #size-cells = <2>; If I remove it will make
>> dt_binding_check fail.
>>>
> Hello Krzysztof
> 
> Use dt_binding_check, it need #address-cells = <2>;  #size-cells = <2> for 64 bit address description.
> Or I don't need example?

Drop example. It's basically the same as existing one.

Best regards,
Krzysztof

Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for AST2700

[Rob Herring]

On Fri, Aug 09, 2024 at 06:10:22AM +0000, Ryan Chen wrote:
> > Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for AST2700
> >
> > On 09/08/2024 07:55, Ryan Chen wrote:
> > >> Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for
> > >> AST2700
> > >>
> > >> On 08/08/2024 09:59, Ryan Chen wrote:
> > >>> Add compatible support for AST2700 clk, reset, pinctrl, silicon-id
> > >>> and example for AST2700 scu.
> > >>>
> > >>> Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
> > >>> ---
> > >>>  .../bindings/mfd/aspeed,ast2x00-scu.yaml      | 31
> > >> +++++++++++++++++--
> > >>>  1 file changed, 29 insertions(+), 2 deletions(-)
> > >>>
> > >>> diff --git
> > >>> a/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
> > >>> b/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
> > >>> index 86ee69c0f45b..c0965f08ae8c 100644
> > >>> --- a/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
> > >>> +++ b/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
> > >>> @@ -21,6 +21,8 @@ properties:
> > >>>            - aspeed,ast2400-scu
> > >>>            - aspeed,ast2500-scu
> > >>>            - aspeed,ast2600-scu
> > >>> +          - aspeed,ast2700-scu0
> > >>> +          - aspeed,ast2700-scu1
> > >>
> > >> What are the differences between these two?
> > >
> > > The next [PATCH 4/4] is scu driver that include ast2700-scu0 and
> > > ast2700-scu1 CLK_OF_DECLARE_DRIVER(ast2700_soc0,
> > > "aspeed,ast2700-scu0", ast2700_soc0_clk_init);
> > > CLK_OF_DECLARE_DRIVER(ast2700_soc1, "aspeed,ast2700-scu1",
> > > ast2700_soc1_clk_init);
> >
> > What are hardware differences? Entirely different devices?
> 
> AST2700 have two soc die connected each other.
> Each soc die have it own scu, so the naming is ast2700-scu0 for soc0, another is ast2700-scu1 for soc1.

Didn't I see in another patch one die is cpu and one is io? Use those in 
the compatible rather than 0 and 1 if so.

Rob

RE: [PATCH 1/4] dt-bindings: mfd: aspeed: support for AST2700

[Ryan Chen]

> Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for AST2700
> 
> On Fri, Aug 09, 2024 at 06:10:22AM +0000, Ryan Chen wrote:
> > > Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for
> > > AST2700
> > >
> > > On 09/08/2024 07:55, Ryan Chen wrote:
> > > >> Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for
> > > >> AST2700
> > > >>
> > > >> On 08/08/2024 09:59, Ryan Chen wrote:
> > > >>> Add compatible support for AST2700 clk, reset, pinctrl,
> > > >>> silicon-id and example for AST2700 scu.
> > > >>>
> > > >>> Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
> > > >>> ---
> > > >>>  .../bindings/mfd/aspeed,ast2x00-scu.yaml      | 31
> > > >> +++++++++++++++++--
> > > >>>  1 file changed, 29 insertions(+), 2 deletions(-)
> > > >>>
> > > >>> diff --git
> > > >>> a/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
> > > >>> b/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
> > > >>> index 86ee69c0f45b..c0965f08ae8c 100644
> > > >>> ---
> > > >>> a/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
> > > >>> +++ b/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.y
> > > >>> +++ aml
> > > >>> @@ -21,6 +21,8 @@ properties:
> > > >>>            - aspeed,ast2400-scu
> > > >>>            - aspeed,ast2500-scu
> > > >>>            - aspeed,ast2600-scu
> > > >>> +          - aspeed,ast2700-scu0
> > > >>> +          - aspeed,ast2700-scu1
> > > >>
> > > >> What are the differences between these two?
> > > >
> > > > The next [PATCH 4/4] is scu driver that include ast2700-scu0 and
> > > > ast2700-scu1 CLK_OF_DECLARE_DRIVER(ast2700_soc0,
> > > > "aspeed,ast2700-scu0", ast2700_soc0_clk_init);
> > > > CLK_OF_DECLARE_DRIVER(ast2700_soc1, "aspeed,ast2700-scu1",
> > > > ast2700_soc1_clk_init);
> > >
> > > What are hardware differences? Entirely different devices?
> >
> > AST2700 have two soc die connected each other.
> > Each soc die have it own scu, so the naming is ast2700-scu0 for soc0,
> another is ast2700-scu1 for soc1.
> 
> Didn't I see in another patch one die is cpu and one is io? Use those in the
> compatible rather than 0 and 1 if so.
> 
Sorry, I want to align with our datasheet description. 
It will but scu0 and scu1 register setting. 

> Rob

Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for AST2700

[Andrew Jeffery]

On Wed, 2024-08-14 at 06:35 +0000, Ryan Chen wrote:
> > Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for
> > AST2700
> > 
> > On Fri, Aug 09, 2024 at 06:10:22AM +0000, Ryan Chen wrote:
> > > > Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for
> > > > AST2700
> > > > 
> > > > On 09/08/2024 07:55, Ryan Chen wrote:
> > > > > > Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support
> > > > > > for
> > > > > > AST2700
> > > > > > 
> > > > > > On 08/08/2024 09:59, Ryan Chen wrote:
> > > > > > > Add compatible support for AST2700 clk, reset, pinctrl,
> > > > > > > silicon-id and example for AST2700 scu.
> > > > > > > 
> > > > > > > Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
> > > > > > > ---
> > > > > > >  .../bindings/mfd/aspeed,ast2x00-scu.yaml      | 31
> > > > > > +++++++++++++++++--
> > > > > > >  1 file changed, 29 insertions(+), 2 deletions(-)
> > > > > > > 
> > > > > > > diff --git
> > > > > > > a/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-
> > > > > > > scu.yaml
> > > > > > > b/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-
> > > > > > > scu.yaml
> > > > > > > index 86ee69c0f45b..c0965f08ae8c 100644
> > > > > > > ---
> > > > > > > a/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-
> > > > > > > scu.yaml
> > > > > > > +++
> > > > > > > b/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-
> > > > > > > scu.y
> > > > > > > +++ aml
> > > > > > > @@ -21,6 +21,8 @@ properties:
> > > > > > >            - aspeed,ast2400-scu
> > > > > > >            - aspeed,ast2500-scu
> > > > > > >            - aspeed,ast2600-scu
> > > > > > > +          - aspeed,ast2700-scu0
> > > > > > > +          - aspeed,ast2700-scu1
> > > > > > 
> > > > > > What are the differences between these two?
> > > > > 
> > > > > The next [PATCH 4/4] is scu driver that include ast2700-scu0
> > > > > and
> > > > > ast2700-scu1 CLK_OF_DECLARE_DRIVER(ast2700_soc0,
> > > > > "aspeed,ast2700-scu0", ast2700_soc0_clk_init);
> > > > > CLK_OF_DECLARE_DRIVER(ast2700_soc1, "aspeed,ast2700-scu1",
> > > > > ast2700_soc1_clk_init);
> > > > 
> > > > What are hardware differences? Entirely different devices?
> > > 
> > > AST2700 have two soc die connected each other.
> > > Each soc die have it own scu, so the naming is ast2700-scu0 for
> > > soc0,
> > another is ast2700-scu1 for soc1.
> > 
> > Didn't I see in another patch one die is cpu and one is io? Use
> > those in the
> > compatible rather than 0 and 1 if so.
> > 
> Sorry, I want to align with our datasheet description. 
> It will but scu0 and scu1 register setting. 

Can we document that relationship in the binding? Rob's suggestion
seems more descriptive.

Andrew

RE: [PATCH 1/4] dt-bindings: mfd: aspeed: support for AST2700

[Ryan Chen]

> Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for AST2700
> 
> On Wed, 2024-08-14 at 06:35 +0000, Ryan Chen wrote:
> > > Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for
> > > AST2700
> > >
> > > On Fri, Aug 09, 2024 at 06:10:22AM +0000, Ryan Chen wrote:
> > > > > Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for
> > > > > AST2700
> > > > >
> > > > > On 09/08/2024 07:55, Ryan Chen wrote:
> > > > > > > Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support
> > > > > > > for
> > > > > > > AST2700
> > > > > > >
> > > > > > > On 08/08/2024 09:59, Ryan Chen wrote:
> > > > > > > > Add compatible support for AST2700 clk, reset, pinctrl,
> > > > > > > > silicon-id and example for AST2700 scu.
> > > > > > > >
> > > > > > > > Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
> > > > > > > > ---
> > > > > > > >  .../bindings/mfd/aspeed,ast2x00-scu.yaml      | 31
> > > > > > > +++++++++++++++++--
> > > > > > > >  1 file changed, 29 insertions(+), 2 deletions(-)
> > > > > > > >
> > > > > > > > diff --git
> > > > > > > > a/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-
> > > > > > > > scu.yaml
> > > > > > > > b/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-
> > > > > > > > scu.yaml
> > > > > > > > index 86ee69c0f45b..c0965f08ae8c 100644
> > > > > > > > ---
> > > > > > > > a/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-
> > > > > > > > scu.yaml
> > > > > > > > +++
> > > > > > > > b/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-
> > > > > > > > scu.y
> > > > > > > > +++ aml
> > > > > > > > @@ -21,6 +21,8 @@ properties:
> > > > > > > >            - aspeed,ast2400-scu
> > > > > > > >            - aspeed,ast2500-scu
> > > > > > > >            - aspeed,ast2600-scu
> > > > > > > > +          - aspeed,ast2700-scu0
> > > > > > > > +          - aspeed,ast2700-scu1
> > > > > > >
> > > > > > > What are the differences between these two?
> > > > > >
> > > > > > The next [PATCH 4/4] is scu driver that include ast2700-scu0
> > > > > > and
> > > > > > ast2700-scu1 CLK_OF_DECLARE_DRIVER(ast2700_soc0,
> > > > > > "aspeed,ast2700-scu0", ast2700_soc0_clk_init);
> > > > > > CLK_OF_DECLARE_DRIVER(ast2700_soc1, "aspeed,ast2700-scu1",
> > > > > > ast2700_soc1_clk_init);
> > > > >
> > > > > What are hardware differences? Entirely different devices?
> > > >
> > > > AST2700 have two soc die connected each other.
> > > > Each soc die have it own scu, so the naming is ast2700-scu0 for
> > > > soc0,
> > > another is ast2700-scu1 for soc1.
> > >
> > > Didn't I see in another patch one die is cpu and one is io? Use
> > > those in the compatible rather than 0 and 1 if so.
> > >
> > Sorry, I want to align with our datasheet description.
> > It will but scu0 and scu1 register setting.
> 
> Can we document that relationship in the binding? Rob's suggestion seems
> more descriptive.
Hello,
	Do you want me document it in yaml file or just in commit message?

Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for AST2700

[Andrew Jeffery]

On Thu, 2024-08-15 at 01:43 +0000, Ryan Chen wrote:
> > Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for
> > AST2700
> > 
> > On Wed, 2024-08-14 at 06:35 +0000, Ryan Chen wrote:
> > > > Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for
> > > > AST2700
> > > > 
> > > > On Fri, Aug 09, 2024 at 06:10:22AM +0000, Ryan Chen wrote:
> > > > > > Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support
> > > > > > for
> > > > > > AST2700
> > > > > > 
> > > > > > On 09/08/2024 07:55, Ryan Chen wrote:
> > > > > > > > Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed:
> > > > > > > > support
> > > > > > > > for
> > > > > > > > AST2700
> > > > > > > > 
> > > > > > > > On 08/08/2024 09:59, Ryan Chen wrote:
> > > > > > > > > Add compatible support for AST2700 clk, reset,
> > > > > > > > > pinctrl,
> > > > > > > > > silicon-id and example for AST2700 scu.
> > > > > > > > > 
> > > > > > > > > Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
> > > > > > > > > ---
> > > > > > > > >  .../bindings/mfd/aspeed,ast2x00-scu.yaml      | 31
> > > > > > > > +++++++++++++++++--
> > > > > > > > >  1 file changed, 29 insertions(+), 2 deletions(-)
> > > > > > > > > 
> > > > > > > > > diff --git
> > > > > > > > > a/Documentation/devicetree/bindings/mfd/aspeed,ast2x0
> > > > > > > > > 0-
> > > > > > > > > scu.yaml
> > > > > > > > > b/Documentation/devicetree/bindings/mfd/aspeed,ast2x0
> > > > > > > > > 0-
> > > > > > > > > scu.yaml
> > > > > > > > > index 86ee69c0f45b..c0965f08ae8c 100644
> > > > > > > > > ---
> > > > > > > > > a/Documentation/devicetree/bindings/mfd/aspeed,ast2x0
> > > > > > > > > 0-
> > > > > > > > > scu.yaml
> > > > > > > > > +++
> > > > > > > > > b/Documentation/devicetree/bindings/mfd/aspeed,ast2x0
> > > > > > > > > 0-
> > > > > > > > > scu.y
> > > > > > > > > +++ aml
> > > > > > > > > @@ -21,6 +21,8 @@ properties:
> > > > > > > > >            - aspeed,ast2400-scu
> > > > > > > > >            - aspeed,ast2500-scu
> > > > > > > > >            - aspeed,ast2600-scu
> > > > > > > > > +          - aspeed,ast2700-scu0
> > > > > > > > > +          - aspeed,ast2700-scu1
> > > > > > > > 
> > > > > > > > What are the differences between these two?
> > > > > > > 
> > > > > > > The next [PATCH 4/4] is scu driver that include ast2700-
> > > > > > > scu0
> > > > > > > and
> > > > > > > ast2700-scu1 CLK_OF_DECLARE_DRIVER(ast2700_soc0,
> > > > > > > "aspeed,ast2700-scu0", ast2700_soc0_clk_init);
> > > > > > > CLK_OF_DECLARE_DRIVER(ast2700_soc1, "aspeed,ast2700-
> > > > > > > scu1",
> > > > > > > ast2700_soc1_clk_init);
> > > > > > 
> > > > > > What are hardware differences? Entirely different devices?
> > > > > 
> > > > > AST2700 have two soc die connected each other.
> > > > > Each soc die have it own scu, so the naming is ast2700-scu0
> > > > > for
> > > > > soc0,
> > > > another is ast2700-scu1 for soc1.
> > > > 
> > > > Didn't I see in another patch one die is cpu and one is io? Use
> > > > those in the compatible rather than 0 and 1 if so.
> > > > 
> > > Sorry, I want to align with our datasheet description.
> > > It will but scu0 and scu1 register setting.
> > 
> > Can we document that relationship in the binding? Rob's suggestion
> > seems
> > more descriptive.
> Hello,
> 	Do you want me document it in yaml file or just in commit
> message?

In the binding document please!

Andrew

RE: [PATCH 1/4] dt-bindings: mfd: aspeed: support for AST2700

[Ryan Chen]

> > Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for AST2700
> >
> > On Wed, 2024-08-14 at 06:35 +0000, Ryan Chen wrote:
> > > > Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for
> > > > AST2700
> > > >
> > > > On Fri, Aug 09, 2024 at 06:10:22AM +0000, Ryan Chen wrote:
> > > > > > Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for
> > > > > > AST2700
> > > > > >
> > > > > > On 09/08/2024 07:55, Ryan Chen wrote:
> > > > > > > > Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support
> > > > > > > > for
> > > > > > > > AST2700
> > > > > > > >
> > > > > > > > On 08/08/2024 09:59, Ryan Chen wrote:
> > > > > > > > > Add compatible support for AST2700 clk, reset, pinctrl,
> > > > > > > > > silicon-id and example for AST2700 scu.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
> > > > > > > > > ---
> > > > > > > > >  .../bindings/mfd/aspeed,ast2x00-scu.yaml      | 31
> > > > > > > > +++++++++++++++++--
> > > > > > > > >  1 file changed, 29 insertions(+), 2 deletions(-)
> > > > > > > > >
> > > > > > > > > diff --git
> > > > > > > > > a/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-
> > > > > > > > > scu.yaml
> > > > > > > > > b/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-
> > > > > > > > > scu.yaml
> > > > > > > > > index 86ee69c0f45b..c0965f08ae8c 100644
> > > > > > > > > ---
> > > > > > > > > a/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-
> > > > > > > > > scu.yaml
> > > > > > > > > +++
> > > > > > > > > b/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-
> > > > > > > > > scu.y
> > > > > > > > > +++ aml
> > > > > > > > > @@ -21,6 +21,8 @@ properties:
> > > > > > > > >            - aspeed,ast2400-scu
> > > > > > > > >            - aspeed,ast2500-scu
> > > > > > > > >            - aspeed,ast2600-scu
> > > > > > > > > +          - aspeed,ast2700-scu0
> > > > > > > > > +          - aspeed,ast2700-scu1
> > > > > > > >
> > > > > > > > What are the differences between these two?
> > > > > > >
> > > > > > > The next [PATCH 4/4] is scu driver that include ast2700-scu0
> > > > > > > and
> > > > > > > ast2700-scu1 CLK_OF_DECLARE_DRIVER(ast2700_soc0,
> > > > > > > "aspeed,ast2700-scu0", ast2700_soc0_clk_init);
> > > > > > > CLK_OF_DECLARE_DRIVER(ast2700_soc1, "aspeed,ast2700-scu1",
> > > > > > > ast2700_soc1_clk_init);
> > > > > >
> > > > > > What are hardware differences? Entirely different devices?
> > > > >
> > > > > AST2700 have two soc die connected each other.
> > > > > Each soc die have it own scu, so the naming is ast2700-scu0 for
> > > > > soc0,
> > > > another is ast2700-scu1 for soc1.
> > > >
> > > > Didn't I see in another patch one die is cpu and one is io? Use
> > > > those in the compatible rather than 0 and 1 if so.
> > > >
> > > Sorry, I want to align with our datasheet description.
> > > It will but scu0 and scu1 register setting.
> >
> > Can we document that relationship in the binding? Rob's suggestion
> > seems more descriptive.
> Hello,
> 	Do you want me document it in yaml file or just in commit message?

Hello Rob, Andrew,
	I will add in yaml file in description. Like following.

description:
  The Aspeed System Control Unit manages the global behaviour of the SoC,
  configuring elements such as clocks, pinmux, and reset.
+  In AST2700, it has two soc combination. Each soc include its own scu register control.
+  ast2700-scu0 for soc0, ast2700-scu1 for soc1.

Is that will be better way ?

Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for AST2700

[Andrew Jeffery]

On Mon, 2024-08-19 at 03:05 +0000, Ryan Chen wrote:
> > > Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for
> > > AST2700
> > > 
> > > On Wed, 2024-08-14 at 06:35 +0000, Ryan Chen wrote:
> > > > > Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support
> > > > > for
> > > > > AST2700
> > > > > 
> > > > > On Fri, Aug 09, 2024 at 06:10:22AM +0000, Ryan Chen wrote:
> > > > > > > Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed:
> > > > > > > support for
> > > > > > > AST2700
> > > > > > > 
> > > > > > > On 09/08/2024 07:55, Ryan Chen wrote:
> > > > > > > > > Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed:
> > > > > > > > > support
> > > > > > > > > for
> > > > > > > > > AST2700
> > > > > > > > > 
> > > > > > > > > On 08/08/2024 09:59, Ryan Chen wrote:
> > > > > > > > > > Add compatible support for AST2700 clk, reset,
> > > > > > > > > > pinctrl,
> > > > > > > > > > silicon-id and example for AST2700 scu.
> > > > > > > > > > 
> > > > > > > > > > Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
> > > > > > > > > > ---
> > > > > > > > > >  .../bindings/mfd/aspeed,ast2x00-scu.yaml      | 31
> > > > > > > > > +++++++++++++++++--
> > > > > > > > > >  1 file changed, 29 insertions(+), 2 deletions(-)
> > > > > > > > > > 
> > > > > > > > > > diff --git
> > > > > > > > > > a/Documentation/devicetree/bindings/mfd/aspeed,ast2
> > > > > > > > > > x00-
> > > > > > > > > > scu.yaml
> > > > > > > > > > b/Documentation/devicetree/bindings/mfd/aspeed,ast2
> > > > > > > > > > x00-
> > > > > > > > > > scu.yaml
> > > > > > > > > > index 86ee69c0f45b..c0965f08ae8c 100644
> > > > > > > > > > ---
> > > > > > > > > > a/Documentation/devicetree/bindings/mfd/aspeed,ast2
> > > > > > > > > > x00-
> > > > > > > > > > scu.yaml
> > > > > > > > > > +++
> > > > > > > > > > b/Documentation/devicetree/bindings/mfd/aspeed,ast2
> > > > > > > > > > x00-
> > > > > > > > > > scu.y
> > > > > > > > > > +++ aml
> > > > > > > > > > @@ -21,6 +21,8 @@ properties:
> > > > > > > > > >            - aspeed,ast2400-scu
> > > > > > > > > >            - aspeed,ast2500-scu
> > > > > > > > > >            - aspeed,ast2600-scu
> > > > > > > > > > +          - aspeed,ast2700-scu0
> > > > > > > > > > +          - aspeed,ast2700-scu1
> > > > > > > > > 
> > > > > > > > > What are the differences between these two?
> > > > > > > > 
> > > > > > > > The next [PATCH 4/4] is scu driver that include
> > > > > > > > ast2700-scu0
> > > > > > > > and
> > > > > > > > ast2700-scu1 CLK_OF_DECLARE_DRIVER(ast2700_soc0,
> > > > > > > > "aspeed,ast2700-scu0", ast2700_soc0_clk_init);
> > > > > > > > CLK_OF_DECLARE_DRIVER(ast2700_soc1, "aspeed,ast2700-
> > > > > > > > scu1",
> > > > > > > > ast2700_soc1_clk_init);
> > > > > > > 
> > > > > > > What are hardware differences? Entirely different
> > > > > > > devices?
> > > > > > 
> > > > > > AST2700 have two soc die connected each other.
> > > > > > Each soc die have it own scu, so the naming is ast2700-scu0
> > > > > > for
> > > > > > soc0,
> > > > > another is ast2700-scu1 for soc1.
> > > > > 
> > > > > Didn't I see in another patch one die is cpu and one is io?
> > > > > Use
> > > > > those in the compatible rather than 0 and 1 if so.
> > > > > 
> > > > Sorry, I want to align with our datasheet description.
> > > > It will but scu0 and scu1 register setting.
> > > 
> > > Can we document that relationship in the binding? Rob's
> > > suggestion
> > > seems more descriptive.
> > Hello,
> > 	Do you want me document it in yaml file or just in commit
> > message?
> 
> Hello Rob, Andrew,
> 	I will add in yaml file in description. Like following.
> 
> description:
>   The Aspeed System Control Unit manages the global behaviour of the
> SoC,
>   configuring elements such as clocks, pinmux, and reset.
> +  In AST2700, it has two soc combination. Each soc include its own
> scu register control.
> +  ast2700-scu0 for soc0, ast2700-scu1 for soc1.
> 
> Is that will be better way ?

What Rob is suggesting is to add the compatibles "aspeed,ast2700-scu-
cpu" and "aspeed,ast2700-scu-io", and then in the description say
something like:

   The AST2700 integrates both a CPU and an IO die, each with their own
   SCU. The "aspeed,ast2700-scu-cpu" and "aspeed,ast2700-scu-io"
   compatibles correspond to SCU0 and SCU1 respectively.

Andrew

RE: [PATCH 1/4] dt-bindings: mfd: aspeed: support for AST2700

[Ryan Chen]

> Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for AST2700
> 
> On Mon, 2024-08-19 at 03:05 +0000, Ryan Chen wrote:
> > > > Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for
> > > > AST2700
> > > >
> > > > On Wed, 2024-08-14 at 06:35 +0000, Ryan Chen wrote:
> > > > > > Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for
> > > > > > AST2700
> > > > > >
> > > > > > On Fri, Aug 09, 2024 at 06:10:22AM +0000, Ryan Chen wrote:
> > > > > > > > Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed:
> > > > > > > > support for
> > > > > > > > AST2700
> > > > > > > >
> > > > > > > > On 09/08/2024 07:55, Ryan Chen wrote:
> > > > > > > > > > Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed:
> > > > > > > > > > support
> > > > > > > > > > for
> > > > > > > > > > AST2700
> > > > > > > > > >
> > > > > > > > > > On 08/08/2024 09:59, Ryan Chen wrote:
> > > > > > > > > > > Add compatible support for AST2700 clk, reset,
> > > > > > > > > > > pinctrl, silicon-id and example for AST2700 scu.
> > > > > > > > > > >
> > > > > > > > > > > Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
> > > > > > > > > > > ---
> > > > > > > > > > >  .../bindings/mfd/aspeed,ast2x00-scu.yaml      | 31
> > > > > > > > > > +++++++++++++++++--
> > > > > > > > > > >  1 file changed, 29 insertions(+), 2 deletions(-)
> > > > > > > > > > >
> > > > > > > > > > > diff --git
> > > > > > > > > > > a/Documentation/devicetree/bindings/mfd/aspeed,ast2
> > > > > > > > > > > x00-
> > > > > > > > > > > scu.yaml
> > > > > > > > > > > b/Documentation/devicetree/bindings/mfd/aspeed,ast2
> > > > > > > > > > > x00-
> > > > > > > > > > > scu.yaml
> > > > > > > > > > > index 86ee69c0f45b..c0965f08ae8c 100644
> > > > > > > > > > > ---
> > > > > > > > > > > a/Documentation/devicetree/bindings/mfd/aspeed,ast2
> > > > > > > > > > > x00-
> > > > > > > > > > > scu.yaml
> > > > > > > > > > > +++
> > > > > > > > > > > b/Documentation/devicetree/bindings/mfd/aspeed,ast2
> > > > > > > > > > > x00-
> > > > > > > > > > > scu.y
> > > > > > > > > > > +++ aml
> > > > > > > > > > > @@ -21,6 +21,8 @@ properties:
> > > > > > > > > > >            - aspeed,ast2400-scu
> > > > > > > > > > >            - aspeed,ast2500-scu
> > > > > > > > > > >            - aspeed,ast2600-scu
> > > > > > > > > > > +          - aspeed,ast2700-scu0
> > > > > > > > > > > +          - aspeed,ast2700-scu1
> > > > > > > > > >
> > > > > > > > > > What are the differences between these two?
> > > > > > > > >
> > > > > > > > > The next [PATCH 4/4] is scu driver that include
> > > > > > > > > ast2700-scu0
> > > > > > > > > and
> > > > > > > > > ast2700-scu1 CLK_OF_DECLARE_DRIVER(ast2700_soc0,
> > > > > > > > > "aspeed,ast2700-scu0", ast2700_soc0_clk_init);
> > > > > > > > > CLK_OF_DECLARE_DRIVER(ast2700_soc1, "aspeed,ast2700-
> > > > > > > > > scu1", ast2700_soc1_clk_init);
> > > > > > > >
> > > > > > > > What are hardware differences? Entirely different devices?
> > > > > > >
> > > > > > > AST2700 have two soc die connected each other.
> > > > > > > Each soc die have it own scu, so the naming is ast2700-scu0
> > > > > > > for soc0,
> > > > > > another is ast2700-scu1 for soc1.
> > > > > >
> > > > > > Didn't I see in another patch one die is cpu and one is io?
> > > > > > Use
> > > > > > those in the compatible rather than 0 and 1 if so.
> > > > > >
> > > > > Sorry, I want to align with our datasheet description.
> > > > > It will but scu0 and scu1 register setting.
> > > >
> > > > Can we document that relationship in the binding? Rob's
> > > > suggestion
> > > > seems more descriptive.
> > > Hello,
> > > 	Do you want me document it in yaml file or just in commit
> > > message?
> >
> > Hello Rob, Andrew,
> > 	I will add in yaml file in description. Like following.
> >
> > description:
> >   The Aspeed System Control Unit manages the global behaviour of the
> > SoC,
> >   configuring elements such as clocks, pinmux, and reset.
> > +  In AST2700, it has two soc combination. Each soc include its own
> > scu register control.
> > +  ast2700-scu0 for soc0, ast2700-scu1 for soc1.
> >
> > Is that will be better way ?
> 
> What Rob is suggesting is to add the compatibles "aspeed,ast2700-scu-
> cpu" and "aspeed,ast2700-scu-io", and then in the description say
> something like:
> 
>    The AST2700 integrates both a CPU and an IO die, each with their own
>    SCU. The "aspeed,ast2700-scu-cpu" and "aspeed,ast2700-scu-io"
>    compatibles correspond to SCU0 and SCU1 respectively.
> 
Hello Andrew,
	Sorry, for correspond for ast2700 datasheet, the description is scu0/scu1.
	System Control Unit #0 (SCU0)/ System Control Unit #1 (SCU1) why not we
	Keep align with datasheet statement?

Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for AST2700

[Andrew Jeffery]

On Tue, 2024-08-20 at 01:52 +0000, Ryan Chen wrote:
> > Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for
> > AST2700
> > 
> > On Mon, 2024-08-19 at 03:05 +0000, Ryan Chen wrote:
> > > > > Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support
> > > > > for
> > > > > AST2700
> > > > > 
> > > > > On Wed, 2024-08-14 at 06:35 +0000, Ryan Chen wrote:
> > > > > > > 
> > > > > > > Didn't I see in another patch one die is cpu and one is
> > > > > > > io?
> > > > > > > Use
> > > > > > > those in the compatible rather than 0 and 1 if so.
> > > > > > > 
> > > > > > Sorry, I want to align with our datasheet description.
> > > > > > It will but scu0 and scu1 register setting.
> > > > > 
> > > > > Can we document that relationship in the binding? Rob's
> > > > > suggestion
> > > > > seems more descriptive.
> > > > Hello,
> > > > 	Do you want me document it in yaml file or just in
> > > > commit
> > > > message?
> > > 
> > > Hello Rob, Andrew,
> > > 	I will add in yaml file in description. Like following.
> > > 
> > > description:
> > >   The Aspeed System Control Unit manages the global behaviour of
> > > the
> > > SoC,
> > >   configuring elements such as clocks, pinmux, and reset.
> > > +  In AST2700, it has two soc combination. Each soc include its
> > > own
> > > scu register control.
> > > +  ast2700-scu0 for soc0, ast2700-scu1 for soc1.
> > > 
> > > Is that will be better way ?
> > 
> > What Rob is suggesting is to add the compatibles "aspeed,ast2700-
> > scu-
> > cpu" and "aspeed,ast2700-scu-io", and then in the description say
> > something like:
> > 
> >    The AST2700 integrates both a CPU and an IO die, each with their
> > own
> >    SCU. The "aspeed,ast2700-scu-cpu" and "aspeed,ast2700-scu-io"
> >    compatibles correspond to SCU0 and SCU1 respectively.
> > 
> Hello Andrew,
> 	Sorry, for correspond for ast2700 datasheet, the description
> is scu0/scu1.
> 	System Control Unit #0 (SCU0)/ System Control Unit #1 (SCU1)
> why not we
> 	Keep align with datasheet statement?

Well, IMO we have an opportunity do better in the compatibles. I expect
we should take advantage of it. As some support for Rob's suggestion,
the datasheet chapter for SCU1 calls it "SCUIO" in the first sentence
of the description. Further, there are only two SCUs, and I don't think
the mapping of "cpu" to 0 and "io" to 1 is too difficult to keep track
of, certainly not if it's written in the binding documentation (as long
as these names are accurate!). The argument works both ways but I don't
think it's contentious that using the indexes is _less_ descriptive.

That said, this is just my semi-informed opinion. It's up to you to
decide what names you're going to push for. Rob's suggestion seems
reasonable to me though.

Andrew

RE: [PATCH 1/4] dt-bindings: mfd: aspeed: support for AST2700

[Ryan Chen]

> Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for AST2700
> 
> On Tue, 2024-08-20 at 01:52 +0000, Ryan Chen wrote:
> > > Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for
> > > AST2700
> > >
> > > On Mon, 2024-08-19 at 03:05 +0000, Ryan Chen wrote:
> > > > > > Subject: Re: [PATCH 1/4] dt-bindings: mfd: aspeed: support for
> > > > > > AST2700
> > > > > >
> > > > > > On Wed, 2024-08-14 at 06:35 +0000, Ryan Chen wrote:
> > > > > > > >
> > > > > > > > Didn't I see in another patch one die is cpu and one is
> > > > > > > > io?
> > > > > > > > Use
> > > > > > > > those in the compatible rather than 0 and 1 if so.
> > > > > > > >
> > > > > > > Sorry, I want to align with our datasheet description.
> > > > > > > It will but scu0 and scu1 register setting.
> > > > > >
> > > > > > Can we document that relationship in the binding? Rob's
> > > > > > suggestion seems more descriptive.
> > > > > Hello,
> > > > > 	Do you want me document it in yaml file or just in commit
> > > > > message?
> > > >
> > > > Hello Rob, Andrew,
> > > > 	I will add in yaml file in description. Like following.
> > > >
> > > > description:
> > > >   The Aspeed System Control Unit manages the global behaviour of
> > > > the SoC,
> > > >   configuring elements such as clocks, pinmux, and reset.
> > > > +  In AST2700, it has two soc combination. Each soc include its
> > > > own
> > > > scu register control.
> > > > +  ast2700-scu0 for soc0, ast2700-scu1 for soc1.
> > > >
> > > > Is that will be better way ?
> > >
> > > What Rob is suggesting is to add the compatibles "aspeed,ast2700-
> > > scu-
> > > cpu" and "aspeed,ast2700-scu-io", and then in the description say
> > > something like:
> > >
> > >    The AST2700 integrates both a CPU and an IO die, each with their
> > > own
> > >    SCU. The "aspeed,ast2700-scu-cpu" and "aspeed,ast2700-scu-io"
> > >    compatibles correspond to SCU0 and SCU1 respectively.
> > >
> > Hello Andrew,
> > 	Sorry, for correspond for ast2700 datasheet, the description is
> > scu0/scu1.
> > 	System Control Unit #0 (SCU0)/ System Control Unit #1 (SCU1) why not
> > we
> > 	Keep align with datasheet statement?
> 
> Well, IMO we have an opportunity do better in the compatibles. I expect we
> should take advantage of it. As some support for Rob's suggestion, the
> datasheet chapter for SCU1 calls it "SCUIO" in the first sentence of the
> description. Further, there are only two SCUs, and I don't think the mapping of
> "cpu" to 0 and "io" to 1 is too difficult to keep track of, certainly not if it's
> written in the binding documentation (as long as these names are accurate!).
> The argument works both ways but I don't think it's contentious that using the
> indexes is _less_ descriptive.
> 
> That said, this is just my semi-informed opinion. It's up to you to decide what
> names you're going to push for. Rob's suggestion seems reasonable to me
> though.
> 
Understood, I think I will keep ast2700-scu0,ast2700-scu1, and I will also align with
Our datasheet generates to be consistence. scu0 and scu1.
> Andrew

[PATCH 2/4] dt-bindings: reset Add AST2700 reset bindings

[Ryan Chen]

Add dt bindings for AST2700 reset driver.

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

diff --git a/include/dt-bindings/reset/aspeed,ast2700-reset.h b/include/dt-bindings/reset/aspeed,ast2700-reset.h
new file mode 100644
index 000000000000..ea261108abfb
--- /dev/null
+++ b/include/dt-bindings/reset/aspeed,ast2700-reset.h
@@ -0,0 +1,132 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Device Tree binding constants for AST2700 reset controller.
+ *
+ * Copyright (c) 2024 Aspeed Technology Inc.
+ */
+
+#ifndef _MACH_ASPEED_AST2700_RESET_H_
+#define _MACH_ASPEED_AST2700_RESET_H_
+
+/* SOC0 */
+#define SCU0_RESET_SDRAM		(0)
+#define SCU0_RESET_DDRPHY		(1)
+#define SCU0_RESET_RSA			(2)
+#define SCU0_RESET_SHA3			(3)
+#define SCU0_RESET_HACE			(4)
+#define SCU0_RESET_SOC			(5)
+#define SCU0_RESET_VIDEO		(6)
+#define SCU0_RESET_2D			(7)
+#define SCU0_RESET_PCIS			(8)
+#define SCU0_RESET_RVAS0		(9)
+#define SCU0_RESET_RVAS1		(10)
+#define SCU0_RESET_SM3			(11)
+#define SCU0_RESET_SM4			(12)
+#define SCU0_RESET_CRT0			(13)
+#define SCU0_RESET_ECC			(14)
+#define SCU0_RESET_DP_PCI		(15)
+#define SCU0_RESET_UFS			(16)
+#define SCU0_RESET_EMMC			(17)
+#define SCU0_RESET_PCIE1RST		(18)
+#define SCU0_RESET_PCIE1RSTOE		(19)
+#define SCU0_RESET_PCIE0RST		(20)
+#define SCU0_RESET_PCIE0RSTOE		(21)
+#define SCU0_RESET_JTAG			(22)
+#define SCU0_RESET_MCTP0		(23)
+#define SCU0_RESET_MCTP1		(24)
+#define SCU0_RESET_XDMA0		(25)
+#define SCU0_RESET_XDMA1		(26)
+#define SCU0_RESET_H2X1			(27)
+#define SCU0_RESET_DP			(28)
+#define SCU0_RESET_DP_MCU		(29)
+#define SCU0_RESET_SSP			(30)
+#define SCU0_RESET_H2X0			(31)
+#define SCU0_RESET_PORTA_VHUB1		(32)
+#define SCU0_RESET_PORTA_PHY3		(33)
+#define SCU0_RESET_PORTA_XHCI		(34)
+#define SCU0_RESET_PORTB_VHUB1		(35)
+#define SCU0_RESET_PORTB_PHY3		(36)
+#define SCU0_RESET_PORTB_XHCI		(37)
+#define SCU0_RESET_PORTA_EHCI		(38)
+#define SCU0_RESET_PORTA_VHUB0		(38)
+#define SCU0_RESET_PORTB_EHCI		(39)
+#define SCU0_RESET_PORTB_VHUB0		(39)
+#define SCU0_RESET_UHCI			(40)
+#define SCU0_RESET_TSP			(41)
+#define SCU0_RESET_E2M0			(42)
+#define SCU0_RESET_E2M1			(43)
+#define SCU0_RESET_VLINK		(44)
+
+#define SCU0_RESET_NUMS		(SCU0_RESET_VLINK + 1)
+
+/* SOC1 */
+#define SCU1_RESET_LPC0			(0)
+#define SCU1_RESET_LPC1			(1)
+#define SCU1_RESET_MII			(2)
+#define SCU1_RESET_PECI			(3)
+#define SCU1_RESET_PWM			(4)
+#define SCU1_RESET_MAC0			(5)
+#define SCU1_RESET_MAC1			(6)
+#define SCU1_RESET_MAC2			(7)
+#define SCU1_RESET_ADC			(8)
+#define SCU1_RESET_SD			(9)
+#define SCU1_RESET_ESPI0		(10)
+#define SCU1_RESET_ESPI1		(11)
+#define SCU1_RESET_JTAG1		(12)
+#define SCU1_RESET_SPI0			(13)
+#define SCU1_RESET_SPI1			(14)
+#define SCU1_RESET_SPI2			(15)
+#define SCU1_RESET_I3C0			(16)
+#define SCU1_RESET_I3C1			(17)
+#define SCU1_RESET_I3C2			(18)
+#define SCU1_RESET_I3C3			(19)
+#define SCU1_RESET_I3C4			(20)
+#define SCU1_RESET_I3C5			(21)
+#define SCU1_RESET_I3C6			(22)
+#define SCU1_RESET_I3C7			(23)
+#define SCU1_RESET_I3C8			(24)
+#define SCU1_RESET_I3C9			(25)
+#define SCU1_RESET_I3C10		(26)
+#define SCU1_RESET_I3C11		(27)
+#define SCU1_RESET_I3C12		(28)
+#define SCU1_RESET_I3C13		(29)
+#define SCU1_RESET_I3C14		(30)
+#define SCU1_RESET_I3C15		(31)
+#define SCU1_RESET_I3C15		(31)
+#define SCU1_RESET_MCU0			(32)
+#define SCU1_RESET_MCU1			(33)
+#define SCU1_RESET_H2A_SPI1		(34)
+#define SCU1_RESET_H2A_SPI2		(35)
+#define SCU1_RESET_UART0		(36)
+#define SCU1_RESET_UART1		(37)
+#define SCU1_RESET_UART2		(38)
+#define SCU1_RESET_UART3		(39)
+#define SCU1_RESET_I2C_FILTER		(40)
+#define SCU1_RESET_CALIPTRA		(41)
+#define SCU1_RESET_XDMA			(42)
+/* reserved 43 */
+#define SCU1_RESET_FSI			(44)
+#define SCU1_RESET_CAN			(45)
+#define SCU1_RESET_MCTP			(46)
+#define SCU1_RESET_I2C			(47)
+#define SCU1_RESET_UART6		(48)
+#define SCU1_RESET_UART7		(49)
+#define SCU1_RESET_UART8		(50)
+#define SCU1_RESET_UART9		(51)
+#define SCU1_RESET_LTPI			(52)
+#define SCU1_RESET_VGAL			(53)
+#define SCU1_RESET_LTPI1		(54)
+#define SCU1_RESET_ACE			(55)
+#define SCU1_RESET_E2M			(56)
+#define SCU1_RESET_UHCI			(57)
+#define SCU1_RESET_PORTC_EHCI		(58)
+#define SCU1_RESET_PORTC_VHUB		(59)
+#define SCU1_RESET_PORTD_EHCI		(60)
+#define SCU1_RESET_PORTD_VHUB		(61)
+#define SCU1_RESET_H2X			(62)
+#define SCU1_RESET_I3CDMA		(63)
+#define SCU1_RESET_PCIE2RST		(64)
+
+#define SCU1_RESET_NUMS		(SCU1_RESET_PCIE2RST + 1)
+
+#endif  /* _MACH_ASPEED_AST2700_RESET_H_ */
-- 
2.34.1

Re: [PATCH 2/4] dt-bindings: reset Add AST2700 reset bindings

[Christophe JAILLET]

Le 08/08/2024 à 09:59, Ryan Chen a écrit :
> Add dt bindings for AST2700 reset driver.
> 
> Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
> ---
>   .../dt-bindings/reset/aspeed,ast2700-reset.h  | 132 ++++++++++++++++++
>   1 file changed, 132 insertions(+)
>   create mode 100644 include/dt-bindings/reset/aspeed,ast2700-reset.h
> 
> diff --git a/include/dt-bindings/reset/aspeed,ast2700-reset.h b/include/dt-bindings/reset/aspeed,ast2700-reset.h
> new file mode 100644
> index 000000000000..ea261108abfb
> --- /dev/null
> +++ b/include/dt-bindings/reset/aspeed,ast2700-reset.h
> @@ -0,0 +1,132 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +/*
> + * Device Tree binding constants for AST2700 reset controller.
> + *
> + * Copyright (c) 2024 Aspeed Technology Inc.
> + */
> +
> +#ifndef _MACH_ASPEED_AST2700_RESET_H_
> +#define _MACH_ASPEED_AST2700_RESET_H_
> +
> +/* SOC0 */
> +#define SCU0_RESET_SDRAM		(0)
> +#define SCU0_RESET_DDRPHY		(1)
> +#define SCU0_RESET_RSA			(2)
> +#define SCU0_RESET_SHA3			(3)
> +#define SCU0_RESET_HACE			(4)
> +#define SCU0_RESET_SOC			(5)
> +#define SCU0_RESET_VIDEO		(6)
> +#define SCU0_RESET_2D			(7)
> +#define SCU0_RESET_PCIS			(8)
> +#define SCU0_RESET_RVAS0		(9)
> +#define SCU0_RESET_RVAS1		(10)
> +#define SCU0_RESET_SM3			(11)
> +#define SCU0_RESET_SM4			(12)
> +#define SCU0_RESET_CRT0			(13)
> +#define SCU0_RESET_ECC			(14)
> +#define SCU0_RESET_DP_PCI		(15)
> +#define SCU0_RESET_UFS			(16)
> +#define SCU0_RESET_EMMC			(17)
> +#define SCU0_RESET_PCIE1RST		(18)
> +#define SCU0_RESET_PCIE1RSTOE		(19)
> +#define SCU0_RESET_PCIE0RST		(20)
> +#define SCU0_RESET_PCIE0RSTOE		(21)
> +#define SCU0_RESET_JTAG			(22)
> +#define SCU0_RESET_MCTP0		(23)
> +#define SCU0_RESET_MCTP1		(24)
> +#define SCU0_RESET_XDMA0		(25)
> +#define SCU0_RESET_XDMA1		(26)
> +#define SCU0_RESET_H2X1			(27)
> +#define SCU0_RESET_DP			(28)
> +#define SCU0_RESET_DP_MCU		(29)
> +#define SCU0_RESET_SSP			(30)
> +#define SCU0_RESET_H2X0			(31)
> +#define SCU0_RESET_PORTA_VHUB1		(32)
> +#define SCU0_RESET_PORTA_PHY3		(33)
> +#define SCU0_RESET_PORTA_XHCI		(34)
> +#define SCU0_RESET_PORTB_VHUB1		(35)
> +#define SCU0_RESET_PORTB_PHY3		(36)
> +#define SCU0_RESET_PORTB_XHCI		(37)
> +#define SCU0_RESET_PORTA_EHCI		(38)
> +#define SCU0_RESET_PORTA_VHUB0		(38)

Is having 38 twice expected?
If not, why not use an enum, BTW?

> +#define SCU0_RESET_PORTB_EHCI		(39)
> +#define SCU0_RESET_PORTB_VHUB0		(39)
> +#define SCU0_RESET_UHCI			(40)
> +#define SCU0_RESET_TSP			(41)
> +#define SCU0_RESET_E2M0			(42)
> +#define SCU0_RESET_E2M1			(43)
> +#define SCU0_RESET_VLINK		(44)
> +
> +#define SCU0_RESET_NUMS		(SCU0_RESET_VLINK + 1)
> +
> +/* SOC1 */
> +#define SCU1_RESET_LPC0			(0)
> +#define SCU1_RESET_LPC1			(1)
> +#define SCU1_RESET_MII			(2)
> +#define SCU1_RESET_PECI			(3)
> +#define SCU1_RESET_PWM			(4)
> +#define SCU1_RESET_MAC0			(5)
> +#define SCU1_RESET_MAC1			(6)
> +#define SCU1_RESET_MAC2			(7)
> +#define SCU1_RESET_ADC			(8)
> +#define SCU1_RESET_SD			(9)
> +#define SCU1_RESET_ESPI0		(10)
> +#define SCU1_RESET_ESPI1		(11)
> +#define SCU1_RESET_JTAG1		(12)
> +#define SCU1_RESET_SPI0			(13)
> +#define SCU1_RESET_SPI1			(14)
> +#define SCU1_RESET_SPI2			(15)
> +#define SCU1_RESET_I3C0			(16)
> +#define SCU1_RESET_I3C1			(17)
> +#define SCU1_RESET_I3C2			(18)
> +#define SCU1_RESET_I3C3			(19)
> +#define SCU1_RESET_I3C4			(20)
> +#define SCU1_RESET_I3C5			(21)
> +#define SCU1_RESET_I3C6			(22)
> +#define SCU1_RESET_I3C7			(23)
> +#define SCU1_RESET_I3C8			(24)
> +#define SCU1_RESET_I3C9			(25)
> +#define SCU1_RESET_I3C10		(26)
> +#define SCU1_RESET_I3C11		(27)
> +#define SCU1_RESET_I3C12		(28)
> +#define SCU1_RESET_I3C13		(29)
> +#define SCU1_RESET_I3C14		(30)
> +#define SCU1_RESET_I3C15		(31)
> +#define SCU1_RESET_I3C15		(31)

SCU1_RESET_I3C15 is defined twice.

> +#define SCU1_RESET_MCU0			(32)
> +#define SCU1_RESET_MCU1			(33)
> +#define SCU1_RESET_H2A_SPI1		(34)
> +#define SCU1_RESET_H2A_SPI2		(35)
> +#define SCU1_RESET_UART0		(36)
> +#define SCU1_RESET_UART1		(37)
> +#define SCU1_RESET_UART2		(38)
> +#define SCU1_RESET_UART3		(39)
> +#define SCU1_RESET_I2C_FILTER		(40)
> +#define SCU1_RESET_CALIPTRA		(41)
> +#define SCU1_RESET_XDMA			(42)
> +/* reserved 43 */
> +#define SCU1_RESET_FSI			(44)
> +#define SCU1_RESET_CAN			(45)
> +#define SCU1_RESET_MCTP			(46)
> +#define SCU1_RESET_I2C			(47)
> +#define SCU1_RESET_UART6		(48)
> +#define SCU1_RESET_UART7		(49)
> +#define SCU1_RESET_UART8		(50)
> +#define SCU1_RESET_UART9		(51)
> +#define SCU1_RESET_LTPI			(52)
> +#define SCU1_RESET_VGAL			(53)
> +#define SCU1_RESET_LTPI1		(54)
> +#define SCU1_RESET_ACE			(55)
> +#define SCU1_RESET_E2M			(56)
> +#define SCU1_RESET_UHCI			(57)
> +#define SCU1_RESET_PORTC_EHCI		(58)
> +#define SCU1_RESET_PORTC_VHUB		(59)
> +#define SCU1_RESET_PORTD_EHCI		(60)
> +#define SCU1_RESET_PORTD_VHUB		(61)
> +#define SCU1_RESET_H2X			(62)
> +#define SCU1_RESET_I3CDMA		(63)
> +#define SCU1_RESET_PCIE2RST		(64)
> +
> +#define SCU1_RESET_NUMS		(SCU1_RESET_PCIE2RST + 1)
> +
> +#endif  /* _MACH_ASPEED_AST2700_RESET_H_ */

RE: [PATCH 2/4] dt-bindings: reset Add AST2700 reset bindings

[Ryan Chen]

> Subject: Re: [PATCH 2/4] dt-bindings: reset Add AST2700 reset bindings
>
> Le 08/08/2024 à 09:59, Ryan Chen a écrit :
> > Add dt bindings for AST2700 reset driver.
> >
> > Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
> > ---
> >   .../dt-bindings/reset/aspeed,ast2700-reset.h  | 132
> ++++++++++++++++++
> >   1 file changed, 132 insertions(+)
> >   create mode 100644 include/dt-bindings/reset/aspeed,ast2700-reset.h
> >
> > diff --git a/include/dt-bindings/reset/aspeed,ast2700-reset.h
> > b/include/dt-bindings/reset/aspeed,ast2700-reset.h
> > new file mode 100644
> > index 000000000000..ea261108abfb
> > --- /dev/null
> > +++ b/include/dt-bindings/reset/aspeed,ast2700-reset.h
> > @@ -0,0 +1,132 @@
> > +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> > +/*
> > + * Device Tree binding constants for AST2700 reset controller.
> > + *
> > + * Copyright (c) 2024 Aspeed Technology Inc.
> > + */
> > +
> > +#ifndef _MACH_ASPEED_AST2700_RESET_H_ #define
> > +_MACH_ASPEED_AST2700_RESET_H_
> > +
> > +/* SOC0 */
> > +#define SCU0_RESET_SDRAM           (0)
> > +#define SCU0_RESET_DDRPHY          (1)
> > +#define SCU0_RESET_RSA                     (2)
> > +#define SCU0_RESET_SHA3                    (3)
> > +#define SCU0_RESET_HACE                    (4)
> > +#define SCU0_RESET_SOC                     (5)
> > +#define SCU0_RESET_VIDEO           (6)
> > +#define SCU0_RESET_2D                      (7)
> > +#define SCU0_RESET_PCIS                    (8)
> > +#define SCU0_RESET_RVAS0           (9)
> > +#define SCU0_RESET_RVAS1           (10)
> > +#define SCU0_RESET_SM3                     (11)
> > +#define SCU0_RESET_SM4                     (12)
> > +#define SCU0_RESET_CRT0                    (13)
> > +#define SCU0_RESET_ECC                     (14)
> > +#define SCU0_RESET_DP_PCI          (15)
> > +#define SCU0_RESET_UFS                     (16)
> > +#define SCU0_RESET_EMMC                    (17)
> > +#define SCU0_RESET_PCIE1RST                (18)
> > +#define SCU0_RESET_PCIE1RSTOE              (19)
> > +#define SCU0_RESET_PCIE0RST                (20)
> > +#define SCU0_RESET_PCIE0RSTOE              (21)
> > +#define SCU0_RESET_JTAG                    (22)
> > +#define SCU0_RESET_MCTP0           (23)
> > +#define SCU0_RESET_MCTP1           (24)
> > +#define SCU0_RESET_XDMA0           (25)
> > +#define SCU0_RESET_XDMA1           (26)
> > +#define SCU0_RESET_H2X1                    (27)
> > +#define SCU0_RESET_DP                      (28)
> > +#define SCU0_RESET_DP_MCU          (29)
> > +#define SCU0_RESET_SSP                     (30)
> > +#define SCU0_RESET_H2X0                    (31)
> > +#define SCU0_RESET_PORTA_VHUB1             (32)
> > +#define SCU0_RESET_PORTA_PHY3              (33)
> > +#define SCU0_RESET_PORTA_XHCI              (34)
> > +#define SCU0_RESET_PORTB_VHUB1             (35)
> > +#define SCU0_RESET_PORTB_PHY3              (36)
> > +#define SCU0_RESET_PORTB_XHCI              (37)
> > +#define SCU0_RESET_PORTA_EHCI              (38)
> > +#define SCU0_RESET_PORTA_VHUB0             (38)
>
> Is having 38 twice expected?
> If not, why not use an enum, BTW?
>
Yes, it is expected. Due to 38 is shared reset for 2 usb controller.
One for EHCI, another is for vhub0. So I do define the same value.
That I can do more clear in dtsi description.
The following will be expected dtsi file descript.

ehci0: usb@12061000 {
        .....
        resets = <&syscon0 SCU0_RESET_PORTA_EHCI>;
};

vhuba0: usb-vhub@12060000 {
        .......
        resets = <&syscon0 SCU0_RESET_PORTA_VHUB0>;
};
> > +#define SCU0_RESET_PORTB_EHCI              (39)
> > +#define SCU0_RESET_PORTB_VHUB0             (39)
> > +#define SCU0_RESET_UHCI                    (40)
> > +#define SCU0_RESET_TSP                     (41)
> > +#define SCU0_RESET_E2M0                    (42)
> > +#define SCU0_RESET_E2M1                    (43)
> > +#define SCU0_RESET_VLINK           (44)
> > +
> > +#define SCU0_RESET_NUMS            (SCU0_RESET_VLINK + 1)
> > +
> > +/* SOC1 */
> > +#define SCU1_RESET_LPC0                    (0)
> > +#define SCU1_RESET_LPC1                    (1)
> > +#define SCU1_RESET_MII                     (2)
> > +#define SCU1_RESET_PECI                    (3)
> > +#define SCU1_RESET_PWM                     (4)
> > +#define SCU1_RESET_MAC0                    (5)
> > +#define SCU1_RESET_MAC1                    (6)
> > +#define SCU1_RESET_MAC2                    (7)
> > +#define SCU1_RESET_ADC                     (8)
> > +#define SCU1_RESET_SD                      (9)
> > +#define SCU1_RESET_ESPI0           (10)
> > +#define SCU1_RESET_ESPI1           (11)
> > +#define SCU1_RESET_JTAG1           (12)
> > +#define SCU1_RESET_SPI0                    (13)
> > +#define SCU1_RESET_SPI1                    (14)
> > +#define SCU1_RESET_SPI2                    (15)
> > +#define SCU1_RESET_I3C0                    (16)
> > +#define SCU1_RESET_I3C1                    (17)
> > +#define SCU1_RESET_I3C2                    (18)
> > +#define SCU1_RESET_I3C3                    (19)
> > +#define SCU1_RESET_I3C4                    (20)
> > +#define SCU1_RESET_I3C5                    (21)
> > +#define SCU1_RESET_I3C6                    (22)
> > +#define SCU1_RESET_I3C7                    (23)
> > +#define SCU1_RESET_I3C8                    (24)
> > +#define SCU1_RESET_I3C9                    (25)
> > +#define SCU1_RESET_I3C10           (26)
> > +#define SCU1_RESET_I3C11           (27)
> > +#define SCU1_RESET_I3C12           (28)
> > +#define SCU1_RESET_I3C13           (29)
> > +#define SCU1_RESET_I3C14           (30)
> > +#define SCU1_RESET_I3C15           (31)
> > +#define SCU1_RESET_I3C15           (31)
>
> SCU1_RESET_I3C15 is defined twice.
>
> > +#define SCU1_RESET_MCU0                    (32)
> > +#define SCU1_RESET_MCU1                    (33)
> > +#define SCU1_RESET_H2A_SPI1                (34)
> > +#define SCU1_RESET_H2A_SPI2                (35)
> > +#define SCU1_RESET_UART0           (36)
> > +#define SCU1_RESET_UART1           (37)
> > +#define SCU1_RESET_UART2           (38)
> > +#define SCU1_RESET_UART3           (39)
> > +#define SCU1_RESET_I2C_FILTER              (40)
> > +#define SCU1_RESET_CALIPTRA                (41)
> > +#define SCU1_RESET_XDMA                    (42)
> > +/* reserved 43 */
> > +#define SCU1_RESET_FSI                     (44)
> > +#define SCU1_RESET_CAN                     (45)
> > +#define SCU1_RESET_MCTP                    (46)
> > +#define SCU1_RESET_I2C                     (47)
> > +#define SCU1_RESET_UART6           (48)
> > +#define SCU1_RESET_UART7           (49)
> > +#define SCU1_RESET_UART8           (50)
> > +#define SCU1_RESET_UART9           (51)
> > +#define SCU1_RESET_LTPI                    (52)
> > +#define SCU1_RESET_VGAL                    (53)
> > +#define SCU1_RESET_LTPI1           (54)
> > +#define SCU1_RESET_ACE                     (55)
> > +#define SCU1_RESET_E2M                     (56)
> > +#define SCU1_RESET_UHCI                    (57)
> > +#define SCU1_RESET_PORTC_EHCI              (58)
> > +#define SCU1_RESET_PORTC_VHUB              (59)
> > +#define SCU1_RESET_PORTD_EHCI              (60)
> > +#define SCU1_RESET_PORTD_VHUB              (61)
> > +#define SCU1_RESET_H2X                     (62)
> > +#define SCU1_RESET_I3CDMA          (63)
> > +#define SCU1_RESET_PCIE2RST                (64)
> > +
> > +#define SCU1_RESET_NUMS            (SCU1_RESET_PCIE2RST + 1)
> > +
> > +#endif  /* _MACH_ASPEED_AST2700_RESET_H_ */

************* 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.

Re: [PATCH 2/4] dt-bindings: reset Add AST2700 reset bindings

[Krzysztof Kozlowski]

On 09/08/2024 07:42, Ryan Chen wrote:
>>> +#define SCU0_RESET_PORTA_EHCI              (38)
>>> +#define SCU0_RESET_PORTA_VHUB0             (38)
>>
>> Is having 38 twice expected?
>> If not, why not use an enum, BTW?
>>
> Yes, it is expected. Due to 38 is shared reset for 2 usb controller.
> One for EHCI, another is for vhub0. So I do define the same value.
> That I can do more clear in dtsi description.
> The following will be expected dtsi file descript.

No, that's confusing. Don't do this. You are hiding information that one
reset is shared. Terrible idea.

Best regards,
Krzysztof

RE: [PATCH 2/4] dt-bindings: reset Add AST2700 reset bindings

[Ryan Chen]

> Subject: Re: [PATCH 2/4] dt-bindings: reset Add AST2700 reset bindings
>
> On 09/08/2024 07:42, Ryan Chen wrote:
> >>> +#define SCU0_RESET_PORTA_EHCI              (38)
> >>> +#define SCU0_RESET_PORTA_VHUB0             (38)
> >>
> >> Is having 38 twice expected?
> >> If not, why not use an enum, BTW?
> >>
> > Yes, it is expected. Due to 38 is shared reset for 2 usb controller.
> > One for EHCI, another is for vhub0. So I do define the same value.
> > That I can do more clear in dtsi description.
> > The following will be expected dtsi file descript.
>
> No, that's confusing. Don't do this. You are hiding information that one reset is
> shared. Terrible idea.

OK I can remove this.
Use #define SCU0_RESET_PORTA_USB              (38)
>
> 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.

Re: [PATCH 2/4] dt-bindings: reset Add AST2700 reset bindings

[Krzysztof Kozlowski]

On 08/08/2024 09:59, Ryan Chen wrote:
> Add dt bindings for AST2700 reset driver.
> 
> Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>


No, that's not how it works. Aspeed already sent it and recieved
feedback. Do not send duplicated patches, without history/changelog. You
keep avoiding discussion, do not reply and then send something again
without changes.

Respond to feedback you got and implement it.

NAK

Best regards,
Krzysztof

RE: [PATCH 2/4] dt-bindings: reset Add AST2700 reset bindings

[Ryan Chen]

> Subject: Re: [PATCH 2/4] dt-bindings: reset Add AST2700 reset bindings
>
> On 08/08/2024 09:59, Ryan Chen wrote:
> > Add dt bindings for AST2700 reset driver.
> >
> > Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
>
>
> No, that's not how it works. Aspeed already sent it and recieved feedback. Do
> not send duplicated patches, without history/changelog. You keep avoiding
> discussion, do not reply and then send something again without changes.
>
> Respond to feedback you got and implement it.
>
> NAK
Apologize for your mis-lead, I do the internal discussion, it should not send "Introduce ASPEED AST27XX BMC SoC" series patch. it should be separate series patch.
Not mess together like previous "Introduce ASPEED AST27XX BMC SoC".
It should be bite by bite, example clk driver patches, platform patches, interrupt patches.
That the reason I send clk driver patch series in separate.
Apologize again.

>
> 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.

Re: [PATCH 2/4] dt-bindings: reset Add AST2700 reset bindings

[Krzysztof Kozlowski]

On 09/08/2024 08:06, Ryan Chen wrote:
>> Subject: Re: [PATCH 2/4] dt-bindings: reset Add AST2700 reset bindings
>>
>> On 08/08/2024 09:59, Ryan Chen wrote:
>>> Add dt bindings for AST2700 reset driver.
>>>
>>> Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
>>
>>
>> No, that's not how it works. Aspeed already sent it and recieved feedback. Do
>> not send duplicated patches, without history/changelog. You keep avoiding
>> discussion, do not reply and then send something again without changes.
>>
>> Respond to feedback you got and implement it.
>>
>> NAK
> Apologize for your mis-lead, I do the internal discussion, it should not send "Introduce ASPEED AST27XX BMC SoC" series patch. it should be separate series patch.
> Not mess together like previous "Introduce ASPEED AST27XX BMC SoC".
> It should be bite by bite, example clk driver patches, platform patches, interrupt patches.
> That the reason I send clk driver patch series in separate.

Best regards,
Krzysztof

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

[Ryan Chen]

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)
+#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
-- 
2.34.1

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

[Christophe JAILLET]

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)

...

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

[Ryan Chen]

> 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.

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

[Krzysztof Kozlowski]

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

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

[Ryan Chen]

> 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.

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

[Krzysztof Kozlowski]

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

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

[Ryan Chen]

> 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.

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

[Krzysztof Kozlowski]

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

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

[Ryan Chen]

> 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

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

[Krzysztof Kozlowski]

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

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

[Ryan Chen]

> 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

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

[Ryan Chen]

> 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

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

[Krzysztof Kozlowski]

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

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

[Ryan Chen]

> 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

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

[Krzysztof Kozlowski]

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

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

[Ryan Chen]

> 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

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

[Krzysztof Kozlowski]

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

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

[Ryan Chen]

> 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

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

[Krzysztof Kozlowski]

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

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

[Ryan Chen]

> 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

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

[Krzysztof Kozlowski]

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

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

[Ryan Chen]

Add dt bindings for AST2700 clock controller

Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
 drivers/clk/Makefile      |    1 +
 drivers/clk/clk-ast2700.c | 1091 +++++++++++++++++++++++++++++++++++++
 2 files changed, 1092 insertions(+)
 create mode 100644 drivers/clk/clk-ast2700.c

diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index f793a16cad40..0d5992ea0fa4 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -38,6 +38,7 @@ obj-$(CONFIG_COMMON_CLK_FSL_SAI)	+= clk-fsl-sai.o
 obj-$(CONFIG_COMMON_CLK_GEMINI)		+= clk-gemini.o
 obj-$(CONFIG_COMMON_CLK_ASPEED)		+= clk-aspeed.o
 obj-$(CONFIG_MACH_ASPEED_G6)		+= clk-ast2600.o
+obj-$(CONFIG_MACH_ASPEED_G7)		+= clk-ast2700.o
 obj-$(CONFIG_ARCH_HIGHBANK)		+= clk-highbank.o
 obj-$(CONFIG_CLK_HSDK)			+= clk-hsdk-pll.o
 obj-$(CONFIG_COMMON_CLK_K210)		+= clk-k210.o
diff --git a/drivers/clk/clk-ast2700.c b/drivers/clk/clk-ast2700.c
new file mode 100644
index 000000000000..59c5df822de3
--- /dev/null
+++ b/drivers/clk/clk-ast2700.c
@@ -0,0 +1,1091 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2024 ASPEED Technology Inc.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/reset-controller.h>
+#include <linux/slab.h>
+
+#include <dt-bindings/clock/aspeed,ast2700-clk.h>
+#include <dt-bindings/reset/aspeed,ast2700-reset.h>
+
+#define SCU_CLK_24MHZ 24000000
+#define SCU_CLK_25MHZ 25000000
+#define SCU_CLK_192MHZ 192000000
+/* SOC0 USB2 PHY CLK*/
+#define SCU_CLK_12MHZ 12000000
+/* SOC0 */
+#define SCU0_HWSTRAP1 0x010
+#define SCU0_CLK_STOP 0x240
+#define SCU0_CLK_SEL1 0x280
+#define SCU0_CLK_SEL2 0x284
+#define GET_USB_REFCLK_DIV(x) ((GENMASK(23, 20) & (x)) >> 20)
+#define UART_DIV13_EN BIT(30)
+#define SCU0_HPLL_PARAM 0x300
+#define SCU0_DPLL_PARAM 0x308
+#define SCU0_MPLL_PARAM 0x310
+#define SCU0_D1CLK_PARAM 0x320
+#define SCU0_D2CLK_PARAM 0x330
+#define SCU0_CRT1CLK_PARAM 0x340
+#define SCU0_CRT2CLK_PARAM 0x350
+#define SCU0_MPHYCLK_PARAM 0x360
+
+/* SOC1 */
+#define SCU1_CLK_STOP 0x240
+#define SCU1_CLK_STOP2 0x260
+#define SCU1_CLK_SEL1 0x280
+#define SCU1_CLK_SEL2 0x284
+#define UXCLK_MASK GENMASK(1, 0)
+#define HUXCLK_MASK GENMASK(4, 3)
+#define SCU1_HPLL_PARAM 0x300
+#define SCU1_APLL_PARAM 0x310
+#define SCU1_DPLL_PARAM 0x320
+#define SCU1_UXCLK_CTRL 0x330
+#define SCU1_HUXCLK_CTRL 0x334
+#define SCU1_MAC12_CLK_DLY 0x390
+#define SCU1_MAC12_CLK_DLY_100M 0x394
+#define SCU1_MAC12_CLK_DLY_10M 0x398
+
+/* Globally visible clocks */
+static DEFINE_SPINLOCK(ast2700_clk_lock);
+
+/* Division of RGMII Clock */
+static const struct clk_div_table ast2700_rgmii_div_table[] = {
+	{ 0x0, 4 },
+	{ 0x1, 4 },
+	{ 0x2, 6 },
+	{ 0x3, 8 },
+	{ 0x4, 10 },
+	{ 0x5, 12 },
+	{ 0x6, 14 },
+	{ 0x7, 16 },
+	{ 0 }
+};
+
+/* Division of RMII Clock */
+static const struct clk_div_table ast2700_rmii_div_table[] = {
+	{ 0x0, 8 },
+	{ 0x1, 8 },
+	{ 0x2, 12 },
+	{ 0x3, 16 },
+	{ 0x4, 20 },
+	{ 0x5, 24 },
+	{ 0x6, 28 },
+	{ 0x7, 32 },
+	{ 0 }
+};
+
+/* Division of HCLK/SDIO/MAC/apll_divn CLK */
+static const struct clk_div_table ast2700_clk_div_table[] = {
+	{ 0x0, 2 },
+	{ 0x1, 2 },
+	{ 0x2, 3 },
+	{ 0x3, 4 },
+	{ 0x4, 5 },
+	{ 0x5, 6 },
+	{ 0x6, 7 },
+	{ 0x7, 8 },
+	{ 0 }
+};
+
+/* Division of PCLK/EMMC CLK */
+static const struct clk_div_table ast2700_clk_div_table2[] = {
+	{ 0x0, 2 },
+	{ 0x1, 4 },
+	{ 0x2, 6 },
+	{ 0x3, 8 },
+	{ 0x4, 10 },
+	{ 0x5, 12 },
+	{ 0x6, 14 },
+	{ 0x7, 16 },
+	{ 0 }
+};
+
+/* HPLL/DPLL: 2000Mhz(default) */
+static struct clk_hw *ast2700_soc0_hw_pll(const char *name, const char *parent_name, u32 val)
+{
+	unsigned int mult, div;
+
+	if (val & BIT(24)) {
+		/* Pass through mode */
+		mult = 1;
+		div = 1;
+	} else {
+		/* F = CLKIN(25MHz) * [(M+1) / 2(N+1)] / (P+1) */
+		u32 m = val & 0x1fff;
+		u32 n = (val >> 13) & 0x3f;
+		u32 p = (val >> 19) & 0xf;
+
+		mult = (m + 1) / (2 * (n + 1));
+		div = (p + 1);
+	}
+
+	return clk_hw_register_fixed_factor(NULL, name, parent_name, 0, mult, div);
+};
+
+/* MPLL 1600Mhz(default) */
+static struct clk_hw *ast2700_calc_mpll(const char *name, const char *parent_name, u32 val)
+{
+	unsigned int mult, div;
+
+	if (val & BIT(24)) {
+		/* Pass through mode */
+		mult = 1;
+		div = 1;
+	} else {
+		/* F = CLKIN(25MHz) * [CLKF/(CLKR+1)] /(CLKOD+1) */
+		u32 m = val & 0x1fff;
+		u32 n = (val >> 13) & 0x3f;
+		u32 p = (val >> 19) & 0xf;
+
+		mult = m / (n + 1);
+		div = (p + 1);
+	}
+	return clk_hw_register_fixed_factor(NULL, name, parent_name, 0, mult, div);
+};
+
+static struct clk_hw *ast2700_calc_uclk(const char *name, u32 val)
+{
+	unsigned int mult, div;
+
+	/* UARTCLK = UXCLK * R / (N * 2) */
+	u32 r = val & 0xff;
+	u32 n = (val >> 8) & 0x3ff;
+
+	mult = r;
+	div = n * 2;
+
+	return clk_hw_register_fixed_factor(NULL, name, "uxclk", 0, mult, div);
+};
+
+static struct clk_hw *ast2700_calc_huclk(const char *name, u32 val)
+{
+	unsigned int mult, div;
+
+	/* UARTCLK = UXCLK * R / (N * 2) */
+	u32 r = val & 0xff;
+	u32 n = (val >> 8) & 0x3ff;
+
+	mult = r;
+	div = n * 2;
+
+	return clk_hw_register_fixed_factor(NULL, name, "huxclk", 0, mult, div);
+};
+
+static struct clk_hw *ast2700_calc_soc1_pll(const char *name, const char *parent_name, u32 val)
+{
+	unsigned int mult, div;
+
+	if (val & BIT(24)) {
+		/* Pass through mode */
+		mult = 1;
+		div = 1;
+	} else {
+		/* F = 25Mhz * [(M + 1) / (n + 1)] / (p + 1) */
+		u32 m = val & 0x1fff;
+		u32 n = (val >> 13) & 0x3f;
+		u32 p = (val >> 19) & 0xf;
+
+		mult = (m + 1) / (n + 1);
+		div = (p + 1);
+	}
+	return clk_hw_register_fixed_factor(NULL, name, parent_name, 0, mult, div);
+};
+
+static int ast2700_clk_is_enabled(struct clk_hw *hw)
+{
+	struct clk_gate *gate = to_clk_gate(hw);
+	u32 clk = BIT(gate->bit_idx);
+	u32 reg;
+
+	reg = readl(gate->reg);
+
+	return !(reg & clk);
+}
+
+static int ast2700_clk_enable(struct clk_hw *hw)
+{
+	struct clk_gate *gate = to_clk_gate(hw);
+	u32 clk = BIT(gate->bit_idx);
+
+	if (readl(gate->reg) & clk)
+		writel(clk, gate->reg + 0x04);
+
+	return 0;
+}
+
+static void ast2700_clk_disable(struct clk_hw *hw)
+{
+	struct clk_gate *gate = to_clk_gate(hw);
+	u32 clk = BIT(gate->bit_idx);
+
+	/* Clock is set to enable, so use write to set register */
+	writel(clk, gate->reg);
+}
+
+static const struct clk_ops ast2700_clk_gate_ops = {
+	.enable = ast2700_clk_enable,
+	.disable = ast2700_clk_disable,
+	.is_enabled = ast2700_clk_is_enabled,
+};
+
+static struct clk_hw *ast2700_clk_hw_register_gate(struct device *dev, const char *name,
+						   const char *parent_name, unsigned long flags,
+						   void __iomem *reg, u8 clock_idx,
+						   u8 clk_gate_flags, spinlock_t *lock)
+{
+	struct clk_gate *gate;
+	struct clk_hw *hw;
+	struct clk_init_data init;
+	int ret = -EINVAL;
+
+	gate = kzalloc(sizeof(*gate), GFP_KERNEL);
+	if (!gate)
+		return ERR_PTR(-ENOMEM);
+
+	init.name = name;
+	init.ops = &ast2700_clk_gate_ops;
+	init.flags = flags;
+	init.parent_names = parent_name ? &parent_name : NULL;
+	init.num_parents = parent_name ? 1 : 0;
+
+	gate->reg = reg;
+	gate->bit_idx = clock_idx;
+	gate->flags = clk_gate_flags;
+	gate->lock = lock;
+	gate->hw.init = &init;
+
+	hw = &gate->hw;
+	ret = clk_hw_register(dev, hw);
+	if (ret) {
+		kfree(gate);
+		hw = ERR_PTR(ret);
+	}
+
+	return hw;
+}
+
+struct ast2700_reset {
+	void __iomem *base;
+	struct reset_controller_dev rcdev;
+};
+
+#define to_rc_data(p) container_of(p, struct ast2700_reset, rcdev)
+
+static int ast2700_reset_assert(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	struct ast2700_reset *rc = to_rc_data(rcdev);
+	u32 rst = BIT(id % 32);
+	u32 reg = id >= 32 ? 0x220 : 0x200;
+
+	if (id == SCU1_RESET_PCIE2RST)
+		writel(readl(rc->base + 0x908) & ~BIT(0), rc->base + 0x908);
+	else
+		writel(rst, rc->base + reg);
+	return 0;
+}
+
+static int ast2700_reset_deassert(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	struct ast2700_reset *rc = to_rc_data(rcdev);
+	u32 rst = BIT(id % 32);
+	u32 reg = id >= 32 ? 0x220 : 0x200;
+
+	if (id == SCU1_RESET_PCIE2RST)
+		writel(readl(rc->base + 0x908) | BIT(0), rc->base + 0x908);
+	else
+		/* Use set to clear register */
+		writel(rst, rc->base + reg + 0x04);
+	return 0;
+}
+
+static int ast2700_reset_status(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	struct ast2700_reset *rc = to_rc_data(rcdev);
+	u32 rst = BIT(id % 32);
+	u32 reg = id >= 32 ? 0x220 : 0x200;
+
+	return (readl(rc->base + reg) & rst);
+}
+
+static const struct reset_control_ops ast2700_reset_ops = {
+	.assert = ast2700_reset_assert,
+	.deassert = ast2700_reset_deassert,
+	.status = ast2700_reset_status,
+};
+
+static const char *const sdclk_sel[] = {
+	"soc1-hpll",
+	"soc1-apll",
+};
+
+static const char *const uartclk_sel[] = {
+	"uartxclk",
+	"huartxclk",
+};
+
+static const char *const uxclk_sel[] = {
+	"soc1-apll_div4",
+	"soc1-apll_div2",
+	"soc1-apll",
+	"soc1-hpll",
+};
+
+static int ast2700_soc1_clk_init(struct device_node *soc1_node)
+{
+	struct clk_hw_onecell_data *clk_data;
+	struct ast2700_reset *reset;
+	u32 uart_clk_source = 0;
+	void __iomem *clk_base;
+	struct clk_hw **clks;
+	u32 val, id;
+	int ret;
+
+	clk_base = of_iomap(soc1_node, 0);
+	WARN_ON(!clk_base);
+
+	clk_data = kzalloc(struct_size(clk_data, hws, SCU1_NUM_CLKS), GFP_KERNEL);
+	if (!clk_data)
+		return -ENOMEM;
+
+	clk_data->num = SCU1_NUM_CLKS;
+	clks = clk_data->hws;
+
+	reset = kzalloc(sizeof(*reset), GFP_KERNEL);
+	if (!reset)
+		return -ENOMEM;
+
+	reset->base = clk_base;
+
+	reset->rcdev.owner = THIS_MODULE;
+	reset->rcdev.nr_resets = SCU1_RESET_NUMS;
+	reset->rcdev.ops = &ast2700_reset_ops;
+	reset->rcdev.of_node = soc1_node;
+
+	ret = reset_controller_register(&reset->rcdev);
+	if (ret) {
+		pr_err("soc1 failed to register reset controller\n");
+		return ret;
+	}
+	/*
+	 * Ast2700 A0 workaround:
+	 * I3C reset should assert all of the I3C controllers simultaneously.
+	 * Otherwise, it may lead to failure in accessing I3C registers.
+	 */
+	for (id = SCU1_RESET_I3C0; id <= SCU1_RESET_I3C15; id++)
+		ast2700_reset_assert(&reset->rcdev, id);
+
+	clks[SCU1_CLKIN] =
+		clk_hw_register_fixed_rate(NULL, "soc1-clkin", NULL, 0, SCU_CLK_25MHZ);
+
+	/* HPLL 1000Mhz */
+	val = readl(clk_base + SCU1_HPLL_PARAM);
+	clks[SCU1_CLK_HPLL] = ast2700_calc_soc1_pll("soc1-hpll", "soc1-clkin", val);
+
+	/* HPLL 800Mhz */
+	val = readl(clk_base + SCU1_APLL_PARAM);
+	clks[SCU1_CLK_APLL] = ast2700_calc_soc1_pll("soc1-apll", "soc1-clkin", val);
+
+	clks[SCU1_CLK_APLL_DIV2] =
+		clk_hw_register_fixed_factor(NULL, "soc1-apll_div2", "soc1-apll", 0, 1, 2);
+
+	clks[SCU1_CLK_APLL_DIV4] =
+		clk_hw_register_fixed_factor(NULL, "soc1-apll_div4", "soc1-apll", 0, 1, 4);
+
+	val = readl(clk_base + SCU1_DPLL_PARAM);
+	clks[SCU1_CLK_DPLL] = ast2700_calc_soc1_pll("dpll", "soc1-clkin", val);
+
+	/* uxclk mux selection */
+	clks[SCU1_CLK_UXCLK] =
+		clk_hw_register_mux(NULL, "uxclk", uxclk_sel, ARRAY_SIZE(uxclk_sel),
+				    0, clk_base + SCU1_CLK_SEL2,
+				    0, 2, 0, &ast2700_clk_lock);
+
+	val = readl(clk_base + SCU1_UXCLK_CTRL);
+	clks[SCU1_CLK_UARTX] = ast2700_calc_uclk("uartxclk", val);
+
+	/* huxclk mux selection */
+	clks[SCU1_CLK_HUXCLK] =
+		clk_hw_register_mux(NULL, "huxclk", uxclk_sel, ARRAY_SIZE(uxclk_sel),
+				    0, clk_base + SCU1_CLK_SEL2,
+				    3, 2, 0, &ast2700_clk_lock);
+
+	val = readl(clk_base + SCU1_HUXCLK_CTRL);
+	clks[SCU1_CLK_HUARTX] = ast2700_calc_huclk("huartxclk", val);
+
+	/* AHB CLK = 200Mhz */
+	clks[SCU1_CLK_AHB] =
+		clk_hw_register_divider_table(NULL, "soc1-ahb", "soc1-hpll",
+					      0, clk_base + SCU1_CLK_SEL2,
+					      20, 3, 0, ast2700_clk_div_table, &ast2700_clk_lock);
+
+	/* APB CLK = 100Mhz */
+	clks[SCU1_CLK_APB] =
+		clk_hw_register_divider_table(NULL, "soc1-apb", "soc1-hpll",
+					      0, clk_base + SCU1_CLK_SEL1,
+					      18, 3, 0, ast2700_clk_div_table2, &ast2700_clk_lock);
+
+	/* RMII */
+	clks[SCU1_CLK_RMII] =
+		clk_hw_register_divider_table(NULL, "rmii", "soc1-hpll",
+					      0, clk_base + SCU1_CLK_SEL1,
+					      21, 3, 0, ast2700_rmii_div_table, &ast2700_clk_lock);
+
+	/* RMII0 50MHz (RCLK) output enable */
+	clks[SCU1_CLK_MAC0RCLK] =
+		clk_hw_register_gate(NULL, "mac0rclk", "rmii", 0,
+				     clk_base + SCU1_MAC12_CLK_DLY, 29,
+				     0, &ast2700_clk_lock);
+
+	/* RMII1 50MHz (RCLK) output enable */
+	clks[SCU1_CLK_MAC1RCLK] =
+		clk_hw_register_gate(NULL, "mac1rclk", "rmii", 0,
+				     clk_base + SCU1_MAC12_CLK_DLY, 30,
+				     0, &ast2700_clk_lock);
+
+	/* RGMII */
+	clks[SCU1_CLK_RGMII] =
+		clk_hw_register_divider_table(NULL, "rgmii", "soc1-hpll",
+					      0, clk_base + SCU1_CLK_SEL1,
+					      25, 3, 0, ast2700_rgmii_div_table, &ast2700_clk_lock);
+
+	/* MAC HCLK */
+	clks[SCU1_CLK_MACHCLK] =
+		clk_hw_register_divider_table(NULL, "machclk", "soc1-hpll",
+					      0, clk_base + SCU1_CLK_SEL1,
+					      29, 3, 0, ast2700_clk_div_table, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_LCLK0] =
+		ast2700_clk_hw_register_gate(NULL, "lclk0-gate", NULL,
+					     CLK_IS_CRITICAL, clk_base + SCU1_CLK_STOP,
+					     0, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_LCLK0] =
+		ast2700_clk_hw_register_gate(NULL, "lclk1-gate", NULL,
+					     CLK_IS_CRITICAL, clk_base + SCU1_CLK_STOP,
+					     1, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_ESPI0CLK] =
+		ast2700_clk_hw_register_gate(NULL, "espi0clk-gate", NULL,
+					     CLK_IS_CRITICAL, clk_base + SCU1_CLK_STOP,
+					     2, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_ESPI1CLK] =
+		ast2700_clk_hw_register_gate(NULL, "espi1clk-gate", NULL,
+					     CLK_IS_CRITICAL, clk_base + SCU1_CLK_STOP,
+					     3, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_APLL_DIVN] =
+		clk_hw_register_divider_table(NULL, "soc1-apll_divn", "soc1-apll",
+					      0, clk_base + SCU1_CLK_SEL2,
+					      8, 3, 0, ast2700_clk_div_table, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_SDMUX] =
+		clk_hw_register_mux(NULL, "sdclk-mux", sdclk_sel, ARRAY_SIZE(sdclk_sel),
+				    0, clk_base + SCU1_CLK_SEL1,
+				    13, 1, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_SDCLK] =
+		clk_hw_register_divider_table(NULL, "sdclk", "sdclk-mux",
+					      0, clk_base + SCU1_CLK_SEL1,
+					      14, 3, 0, ast2700_clk_div_table, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_SDCLK] =
+		ast2700_clk_hw_register_gate(NULL, "sdclk-gate", "sdclk",
+					     0, clk_base + SCU1_CLK_STOP,
+					     4, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_IPEREFCLK] =
+		ast2700_clk_hw_register_gate(NULL, "soc1-refclk-gate", NULL,
+					     CLK_IS_CRITICAL, clk_base + SCU1_CLK_STOP,
+					     6, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_LPCHCLK] =
+		ast2700_clk_hw_register_gate(NULL, "lpchclk-gate", NULL,
+					     CLK_IS_CRITICAL, clk_base + SCU1_CLK_STOP,
+					     7, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_MAC0CLK] =
+		ast2700_clk_hw_register_gate(NULL, "mac0clk-gate", NULL,
+					     0, clk_base + SCU1_CLK_STOP,
+					     8, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_MAC1CLK] =
+		ast2700_clk_hw_register_gate(NULL, "mac1clk-gate", NULL,
+					     0, clk_base + SCU1_CLK_STOP,
+					     9, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_MAC2CLK] =
+		ast2700_clk_hw_register_gate(NULL, "mac2clk-gate", NULL,
+					     0, clk_base + SCU1_CLK_STOP,
+					     10, 0, &ast2700_clk_lock);
+
+	of_property_read_u32(soc1_node, "uart-clk-source", &uart_clk_source);
+	if (uart_clk_source) {
+		val = readl(clk_base + SCU1_CLK_SEL1) & ~GENMASK(12, 0);
+		uart_clk_source &= GENMASK(12, 0);
+		writel(val | uart_clk_source, clk_base + SCU1_CLK_SEL1);
+	}
+
+	//UART0
+	clks[SCU1_CLK_UART0] =
+		clk_hw_register_mux(NULL, "uart0clk", uartclk_sel, ARRAY_SIZE(uartclk_sel),
+				    0, clk_base + SCU1_CLK_SEL1,
+				    0, 1, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_UART0CLK] =
+		ast2700_clk_hw_register_gate(NULL, "uart0clk-gate", "uart0clk",
+					     CLK_IS_CRITICAL, clk_base + SCU1_CLK_STOP,
+					     11, 0, &ast2700_clk_lock);
+
+	//UART1
+	clks[SCU1_CLK_UART1] =
+		clk_hw_register_mux(NULL, "uart1clk", uartclk_sel, ARRAY_SIZE(uartclk_sel),
+				    0, clk_base + SCU1_CLK_SEL1,
+				    1, 1, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_UART1CLK] =
+		ast2700_clk_hw_register_gate(NULL, "uart1clk-gate", "uart1clk",
+					     CLK_IS_CRITICAL, clk_base + SCU1_CLK_STOP,
+					     12, 0, &ast2700_clk_lock);
+
+	//UART2
+	clks[SCU1_CLK_UART2] =
+		clk_hw_register_mux(NULL, "uart2clk", uartclk_sel, ARRAY_SIZE(uartclk_sel),
+				    0, clk_base + SCU1_CLK_SEL1,
+				    2, 1, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_UART2CLK] =
+		ast2700_clk_hw_register_gate(NULL, "uart2clk-gate", "uart2clk",
+					     CLK_IS_CRITICAL, clk_base + SCU1_CLK_STOP,
+					     13, 0, &ast2700_clk_lock);
+
+	//UART3
+	clks[SCU1_CLK_UART3] =
+		clk_hw_register_mux(NULL, "uart3clk", uartclk_sel, ARRAY_SIZE(uartclk_sel),
+				    0, clk_base + SCU1_CLK_SEL1,
+				    3, 1, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_UART3CLK] =
+		ast2700_clk_hw_register_gate(NULL, "uart3clk-gate", "uart3clk",
+					     CLK_IS_CRITICAL, clk_base + SCU1_CLK_STOP,
+					     14, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_I3C0CLK] =
+		ast2700_clk_hw_register_gate(NULL, "i3c0clk-gate", "soc1-ahb",
+					     0, clk_base + SCU1_CLK_STOP,
+					     16, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_I3C1CLK] =
+		ast2700_clk_hw_register_gate(NULL, "i3c1clk-gate", "soc1-ahb",
+					     0, clk_base + SCU1_CLK_STOP,
+					     17, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_I3C2CLK] =
+		ast2700_clk_hw_register_gate(NULL, "i3c2clk-gate", "soc1-ahb",
+					     0, clk_base + SCU1_CLK_STOP,
+					     18, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_I3C3CLK] =
+		ast2700_clk_hw_register_gate(NULL, "i3c3clk-gate", "soc1-ahb",
+					     0, clk_base + SCU1_CLK_STOP,
+					     19, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_I3C4CLK] =
+		ast2700_clk_hw_register_gate(NULL, "i3c4clk-gate", "soc1-ahb",
+					     0, clk_base + SCU1_CLK_STOP,
+					     20, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_I3C5CLK] =
+		ast2700_clk_hw_register_gate(NULL, "i3c5clk-gate", "soc1-ahb",
+					     0, clk_base + SCU1_CLK_STOP,
+					     21, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_I3C6CLK] =
+		ast2700_clk_hw_register_gate(NULL, "i3c6clk-gate", "soc1-ahb",
+					     0, clk_base + SCU1_CLK_STOP,
+					     22, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_I3C7CLK] =
+		ast2700_clk_hw_register_gate(NULL, "i3c7clk-gate", "soc1-ahb",
+					     0, clk_base + SCU1_CLK_STOP,
+					     23, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_I3C8CLK] =
+		ast2700_clk_hw_register_gate(NULL, "i3c8clk-gate", "soc1-ahb",
+					     0, clk_base + SCU1_CLK_STOP,
+					     24, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_I3C9CLK] =
+		ast2700_clk_hw_register_gate(NULL, "i3c9clk-gate", "soc1-ahb",
+					     0, clk_base + SCU1_CLK_STOP,
+					     25, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_I3C10CLK] =
+		ast2700_clk_hw_register_gate(NULL, "i3c10clk-gate", "soc1-ahb",
+					     0, clk_base + SCU1_CLK_STOP,
+					     26, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_I3C11CLK] =
+		ast2700_clk_hw_register_gate(NULL, "i3c11clk-gate", "soc1-ahb",
+					     0, clk_base + SCU1_CLK_STOP,
+					     27, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_I3C12CLK] =
+		ast2700_clk_hw_register_gate(NULL, "i3c12clk-gate", "soc1-ahb",
+					     0, clk_base + SCU1_CLK_STOP,
+					     28, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_I3C13CLK] =
+		ast2700_clk_hw_register_gate(NULL, "i3c13clk-gate", "soc1-ahb",
+					     0, clk_base + SCU1_CLK_STOP,
+					     29, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_I3C14CLK] =
+		ast2700_clk_hw_register_gate(NULL, "i3c14clk-gate", "soc1-ahb",
+					     0, clk_base + SCU1_CLK_STOP,
+					     30, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_I3C15CLK] =
+		ast2700_clk_hw_register_gate(NULL, "i3c15clk-gate", "soc1-ahb",
+					     0, clk_base + SCU1_CLK_STOP,
+					     31, 0, &ast2700_clk_lock);
+
+	/*clk stop 2 */
+	//UART5
+	clks[SCU1_CLK_UART5] =
+		clk_hw_register_mux(NULL, "uart5clk", uartclk_sel, ARRAY_SIZE(uartclk_sel),
+				    0, clk_base + SCU1_CLK_SEL1,
+				    5, 1, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_UART5CLK] =
+		ast2700_clk_hw_register_gate(NULL, "uart5clk-gate", "uart5clk",
+					     CLK_IS_CRITICAL, clk_base + SCU1_CLK_STOP2,
+					     0, 0, &ast2700_clk_lock);
+
+	//UART6
+	clks[SCU1_CLK_UART6] =
+		clk_hw_register_mux(NULL, "uart6clk", uartclk_sel, ARRAY_SIZE(uartclk_sel),
+				    0, clk_base + SCU1_CLK_SEL1,
+				    6, 1, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_UART6CLK] =
+		ast2700_clk_hw_register_gate(NULL, "uart6clk-gate", "uart6clk",
+					     CLK_IS_CRITICAL, clk_base + SCU1_CLK_STOP2,
+					     1, 0, &ast2700_clk_lock);
+
+	//UART7
+	clks[SCU1_CLK_UART7] =
+		clk_hw_register_mux(NULL, "uart7clk", uartclk_sel, ARRAY_SIZE(uartclk_sel),
+				    0, clk_base + SCU1_CLK_SEL1,
+				    7, 1, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_UART7CLK] =
+		ast2700_clk_hw_register_gate(NULL, "uart7clk-gate", "uart7clk",
+					     CLK_IS_CRITICAL, clk_base + SCU1_CLK_STOP2,
+					     2, 0, &ast2700_clk_lock);
+
+	//UART8
+	clks[SCU1_CLK_UART8] =
+		clk_hw_register_mux(NULL, "uart8clk", uartclk_sel, ARRAY_SIZE(uartclk_sel),
+				    0, clk_base + SCU1_CLK_SEL1,
+				    8, 1, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_UART8CLK] =
+		ast2700_clk_hw_register_gate(NULL, "uart8clk-gate", "uart8clk",
+					     CLK_IS_CRITICAL, clk_base + SCU1_CLK_STOP2,
+					     3, 0, &ast2700_clk_lock);
+
+	//UART9
+	clks[SCU1_CLK_UART9] =
+		clk_hw_register_mux(NULL, "uart9clk", uartclk_sel, ARRAY_SIZE(uartclk_sel),
+				    0, clk_base + SCU1_CLK_SEL1,
+				    9, 1, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_UART9CLK] =
+		ast2700_clk_hw_register_gate(NULL, "uart9clk-gate", "uart9clk",
+					     0, clk_base + SCU1_CLK_STOP2,
+					     4, 0, &ast2700_clk_lock);
+
+	//UART10
+	clks[SCU1_CLK_UART10] =
+		clk_hw_register_mux(NULL, "uart10clk", uartclk_sel, ARRAY_SIZE(uartclk_sel),
+				    0, clk_base + SCU1_CLK_SEL1,
+				    10, 1, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_UART10CLK] =
+		ast2700_clk_hw_register_gate(NULL, "uart10clk-gate", "uart10clk",
+					     0, clk_base + SCU1_CLK_STOP2,
+					     5, 0, &ast2700_clk_lock);
+
+	//UART11
+	clks[SCU1_CLK_UART11] =
+		clk_hw_register_mux(NULL, "uart11clk", uartclk_sel, ARRAY_SIZE(uartclk_sel),
+				    0, clk_base + SCU1_CLK_SEL1,
+				    11, 1, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_UART11CLK] =
+		ast2700_clk_hw_register_gate(NULL, "uart11clk-gate", "uart11clk",
+					     CLK_IS_CRITICAL, clk_base + SCU1_CLK_STOP2,
+					     6, 0, &ast2700_clk_lock);
+
+	//uart12: call bmc uart
+	clks[SCU1_CLK_UART12] =
+		clk_hw_register_mux(NULL, "uart12clk", uartclk_sel, ARRAY_SIZE(uartclk_sel),
+				    0, clk_base + SCU1_CLK_SEL1,
+				    12, 1, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_UART12CLK] =
+		ast2700_clk_hw_register_gate(NULL, "uart12clk-gate", "uart12clk",
+					     CLK_IS_CRITICAL, clk_base + SCU1_CLK_STOP2,
+					     7, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_FSICLK] =
+		ast2700_clk_hw_register_gate(NULL, "fsiclk-gate", NULL,
+					     0, clk_base + SCU1_CLK_STOP2,
+					     8, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_LTPIPHYCLK] =
+		ast2700_clk_hw_register_gate(NULL, "ltpiphyclk-gate", NULL,
+					     0, clk_base + SCU1_CLK_STOP2,
+					     9, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_LTPICLK] =
+		ast2700_clk_hw_register_gate(NULL, "ltpiclk-gate", NULL,
+					     0, clk_base + SCU1_CLK_STOP2,
+					     10, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_VGALCLK] =
+		ast2700_clk_hw_register_gate(NULL, "vgalclk-gate", NULL,
+					     CLK_IS_CRITICAL, clk_base + SCU1_CLK_STOP2,
+					     11, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_USBUARTCLK] =
+		ast2700_clk_hw_register_gate(NULL, "usbuartclk-gate", NULL,
+					     0, clk_base + SCU1_CLK_STOP2,
+					     12, 0, &ast2700_clk_lock);
+
+	clk_hw_register_fixed_factor(NULL, "canclk", "soc1-apll", 0, 1, 10);
+
+	clks[SCU1_CLK_GATE_CANCLK] =
+		ast2700_clk_hw_register_gate(NULL, "canclk-gate", "canclk",
+					     0, clk_base + SCU1_CLK_STOP2,
+					     13, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_PCICLK] =
+		ast2700_clk_hw_register_gate(NULL, "pciclk-gate", NULL,
+					     0, clk_base + SCU1_CLK_STOP2,
+					     14, 0, &ast2700_clk_lock);
+
+	clks[SCU1_CLK_GATE_SLICLK] =
+		ast2700_clk_hw_register_gate(NULL, "sliclk-gate", NULL,
+					     CLK_IS_CRITICAL, clk_base + SCU1_CLK_STOP2,
+					     15, 0, &ast2700_clk_lock);
+
+	of_clk_add_hw_provider(soc1_node, of_clk_hw_onecell_get, clk_data);
+
+	return 0;
+};
+
+static const char *const pspclk_sel[] = {
+	"soc0-mpll",
+	"soc0-hpll",
+};
+
+static const char *const soc0_uartclk_sel[] = {
+	"soc0-clk24Mhz",
+	"soc0-clk192Mhz",
+};
+
+static const char *const emmcclk_sel[] = {
+	"soc0-mpll_div4",
+	"soc0-hpll_div4",
+};
+
+static int ast2700_soc0_clk_init(struct device_node *soc0_node)
+{
+	struct clk_hw_onecell_data *clk_data;
+	void __iomem *clk_base;
+	struct ast2700_reset *reset;
+	struct clk_hw **clks;
+	int div;
+	u32 val;
+	int ret;
+
+	clk_data = kzalloc(struct_size(clk_data, hws, SCU0_NUM_CLKS), GFP_KERNEL);
+	if (!clk_data)
+		return -ENOMEM;
+
+	clk_data->num = SCU0_NUM_CLKS;
+	clks = clk_data->hws;
+
+	clk_base = of_iomap(soc0_node, 0);
+	if (WARN_ON(IS_ERR(clk_base)))
+		return PTR_ERR(clk_base);
+
+	reset = kzalloc(sizeof(*reset), GFP_KERNEL);
+	if (!reset)
+		return -ENOMEM;
+
+	reset->base = clk_base;
+
+	reset->rcdev.owner = THIS_MODULE;
+	reset->rcdev.nr_resets = SCU0_RESET_NUMS;
+	reset->rcdev.ops = &ast2700_reset_ops;
+	reset->rcdev.of_node = soc0_node;
+
+	ret = reset_controller_register(&reset->rcdev);
+	if (ret) {
+		pr_err("soc0 failed to register reset controller\n");
+		return ret;
+	}
+
+	//refclk
+	clks[SCU0_CLKIN] =
+		clk_hw_register_fixed_rate(NULL, "soc0-clkin", NULL, 0, SCU_CLK_25MHZ);
+
+	clks[SCU0_CLK_24M] =
+		clk_hw_register_fixed_rate(NULL, "soc0-clk24Mhz", NULL, 0, SCU_CLK_24MHZ);
+
+	clks[SCU0_CLK_192M] =
+		clk_hw_register_fixed_rate(NULL, "soc0-clk192Mhz", NULL, 0, SCU_CLK_192MHZ);
+
+	//hpll
+	val = readl(clk_base + SCU0_HWSTRAP1);
+	if ((val & GENMASK(3, 2)) != 0) {
+		switch ((val & GENMASK(3, 2)) >> 2) {
+		case 1:
+			clks[SCU0_CLK_HPLL] =
+				clk_hw_register_fixed_rate(NULL, "soc0-hpll", NULL, 0, 1900000000);
+			break;
+		case 2:
+			clks[SCU0_CLK_HPLL] =
+				clk_hw_register_fixed_rate(NULL, "soc0-hpll", NULL, 0, 1800000000);
+			break;
+		case 3:
+			clks[SCU0_CLK_HPLL] =
+				clk_hw_register_fixed_rate(NULL, "soc0-hpll", NULL, 0, 1700000000);
+			break;
+		}
+	} else {
+		val = readl(clk_base + SCU0_HPLL_PARAM);
+		clks[SCU0_CLK_HPLL] = ast2700_soc0_hw_pll("soc0-hpll", "soc0-clkin", val);
+	}
+	clks[SCU0_CLK_HPLL_DIV2] =
+			clk_hw_register_fixed_factor(NULL, "soc0-hpll_div2", "soc0-hpll", 0, 1, 2);
+	clks[SCU0_CLK_HPLL_DIV4] =
+			clk_hw_register_fixed_factor(NULL, "soc0-hpll_div4", "soc0-hpll", 0, 1, 4);
+
+	//dpll
+	val = readl(clk_base + SCU0_DPLL_PARAM);
+	clks[SCU0_CLK_DPLL] = ast2700_soc0_hw_pll("dpll", "soc0-clkin", val);
+
+	//mpll
+	val = readl(clk_base + SCU0_MPLL_PARAM);
+	clks[SCU0_CLK_MPLL] = ast2700_calc_mpll("soc0-mpll", "soc0-clkin", val);
+	clks[SCU0_CLK_MPLL_DIV2] =
+			clk_hw_register_fixed_factor(NULL, "soc0-mpll_div2", "soc0-mpll", 0, 1, 2);
+	clks[SCU0_CLK_MPLL_DIV4] =
+			clk_hw_register_fixed_factor(NULL, "soc0-mpll_div4", "soc0-mpll", 0, 1, 4);
+	clks[SCU0_CLK_MPLL_DIV8] =
+			clk_hw_register_fixed_factor(NULL, "soc0-mpll_div8", "soc0-mpll", 0, 1, 8);
+
+	val = readl(clk_base + SCU0_D1CLK_PARAM);
+	clks[SCU0_CLK_VGA0] = ast2700_soc0_hw_pll("d1clk", "soc0-clkin", val);
+
+	val = readl(clk_base + SCU0_D2CLK_PARAM);
+	clks[SCU0_CLK_VGA1] = ast2700_soc0_hw_pll("d2clk", "soc0-clkin", val);
+
+	val = readl(clk_base + SCU0_CRT1CLK_PARAM);
+	clks[SCU0_CLK_CRT0] = ast2700_soc0_hw_pll("crt0clk", "soc0-clkin", val);
+
+	val = readl(clk_base + SCU0_CRT2CLK_PARAM);
+	clks[SCU0_CLK_CRT1] = ast2700_soc0_hw_pll("crt1clk", "soc0-clkin", val);
+
+	val = readl(clk_base + SCU0_MPHYCLK_PARAM);
+	clks[SCU0_CLK_MPHY] =
+		clk_hw_register_fixed_factor(NULL, "mphyclk", "soc0-hpll", 0, 1, val + 1);
+
+	clks[SCU0_CLK_PSP] =
+		clk_hw_register_mux(NULL, "pspclk", pspclk_sel, ARRAY_SIZE(pspclk_sel),
+				    0, clk_base + SCU0_HWSTRAP1,
+				    4, 1, 0, &ast2700_clk_lock);
+
+	clks[SCU0_CLK_AXI0] =
+		clk_hw_register_fixed_factor(NULL, "axi0clk", "pspclk", 0, 1, 2);
+
+	val = readl(clk_base + SCU0_HWSTRAP1);
+	if (val & BIT(7)) {
+		clks[SCU0_CLK_AHB] =
+			clk_hw_register_divider_table(NULL, "soc0-ahb", "soc0-hpll",
+						      0, clk_base + SCU0_HWSTRAP1,
+						      5, 2, 0, ast2700_clk_div_table,
+						      &ast2700_clk_lock);
+	} else {
+		clks[SCU0_CLK_AHB] =
+			clk_hw_register_divider_table(NULL, "soc0-ahb", "soc0-mpll",
+						      0, clk_base + SCU0_HWSTRAP1,
+						      5, 2, 0, ast2700_clk_div_table,
+						      &ast2700_clk_lock);
+	}
+
+	clks[SCU0_CLK_AXI1] =
+		clk_hw_register_fixed_factor(NULL, "axi1clk", "soc0-ahb", 0, 1, 2);
+
+	clks[SCU0_CLK_APB] =
+		clk_hw_register_divider_table(NULL, "soc0-apb", "axi0clk",
+					      0, clk_base + SCU0_CLK_SEL1,
+					      23, 3, 0, ast2700_clk_div_table2, &ast2700_clk_lock);
+
+	clks[SCU0_CLK_GATE_MCLK] =
+		ast2700_clk_hw_register_gate(NULL, "mclk", "soc0-mpll",
+					     CLK_IS_CRITICAL, clk_base + SCU0_CLK_STOP,
+					     0, 0, &ast2700_clk_lock);
+
+	clks[SCU0_CLK_GATE_ECLK] =
+		ast2700_clk_hw_register_gate(NULL, "eclk", NULL,
+					     0, clk_base + SCU0_CLK_STOP,
+					     1, 0, &ast2700_clk_lock);
+
+	clks[SCU0_CLK_GATE_2DCLK] =
+		ast2700_clk_hw_register_gate(NULL, "gclk", NULL,
+					     CLK_IS_CRITICAL, clk_base + SCU0_CLK_STOP,
+					     2, 0, &ast2700_clk_lock);
+
+	clks[SCU0_CLK_GATE_VCLK] =
+		ast2700_clk_hw_register_gate(NULL, "vclk", NULL,
+					     0, clk_base + SCU0_CLK_STOP,
+					     3, 0, &ast2700_clk_lock);
+
+	clks[SCU0_CLK_GATE_BCLK] =
+		ast2700_clk_hw_register_gate(NULL, "bclk", NULL,
+					     CLK_IS_CRITICAL, clk_base + SCU0_CLK_STOP,
+					     4, 0, &ast2700_clk_lock);
+
+	clks[SCU0_CLK_GATE_VGA0CLK] =
+		ast2700_clk_hw_register_gate(NULL, "d1clk-gate", NULL,
+					     CLK_IS_CRITICAL, clk_base + SCU0_CLK_STOP,
+					     5, 0, &ast2700_clk_lock);
+
+	clks[SCU0_CLK_GATE_REFCLK] =
+		ast2700_clk_hw_register_gate(NULL, "soc0-refclk-gate", "soc0-clkin",
+					     CLK_IS_CRITICAL, clk_base + SCU0_CLK_STOP,
+					     6, 0, &ast2700_clk_lock);
+
+	div = (GET_USB_REFCLK_DIV(readl(clk_base + SCU0_CLK_SEL2)) + 1) * 2;
+	clks[SCU0_CLK_U2PHY_REFCLK] =
+		clk_hw_register_fixed_factor(NULL, "xhci_ref_clk", "soc0-mpll_div8", 0, 1, div);
+
+	clks[SCU0_CLK_U2PHY_CLK12M] =
+		clk_hw_register_fixed_rate(NULL, "xhci_suspend_clk", NULL, 0, SCU_CLK_12MHZ);
+
+	clks[SCU0_CLK_GATE_PORTBUSB2CLK] =
+		ast2700_clk_hw_register_gate(NULL, "portb-usb2clk", NULL,
+					     0, clk_base + SCU0_CLK_STOP,
+					     7, 0, &ast2700_clk_lock);
+
+	clks[SCU0_CLK_GATE_UHCICLK] =
+		ast2700_clk_hw_register_gate(NULL, "uhciclk", NULL,
+					     0, clk_base + SCU0_CLK_STOP,
+					     9, 0, &ast2700_clk_lock);
+
+	clks[SCU0_CLK_GATE_VGA1CLK] =
+		ast2700_clk_hw_register_gate(NULL, "d2clk-gate", NULL,
+					     CLK_IS_CRITICAL, clk_base + SCU0_CLK_STOP,
+					     10, 0, &ast2700_clk_lock);
+
+	clks[SCU0_CLK_GATE_HACCLK] =
+		ast2700_clk_hw_register_gate(NULL, "hac-clk", NULL,
+					     0, clk_base + SCU0_CLK_STOP,
+					     13, 0, &ast2700_clk_lock);
+
+	clks[SCU0_CLK_GATE_PORTAUSB2CLK] =
+		ast2700_clk_hw_register_gate(NULL, "porta-usb2clk", NULL,
+					     0, clk_base + SCU0_CLK_STOP,
+					     14, 0, &ast2700_clk_lock);
+
+	clks[SCU0_CLK_UART] =
+		clk_hw_register_mux(NULL, "soc0-uartclk", soc0_uartclk_sel,
+				    ARRAY_SIZE(soc0_uartclk_sel),
+				    0, clk_base + SCU0_CLK_SEL2,
+				    14, 1, 0, &ast2700_clk_lock);
+
+	if (readl(clk_base + SCU0_CLK_SEL2) & UART_DIV13_EN)
+		div = 13;
+	else
+		div = 1;
+
+	clks[SCU0_CLK_UART4] =
+		clk_hw_register_fixed_factor(NULL, "uart4clk", "soc0-uartclk", 0, 1, div);
+
+	clks[SCU0_CLK_GATE_UART4CLK] =
+		ast2700_clk_hw_register_gate(NULL, "uart4clk-gate", "uart4clk",
+					     CLK_IS_CRITICAL, clk_base + SCU0_CLK_STOP,
+					     15, 0, &ast2700_clk_lock);
+
+	clks[SCU0_CLK_GATE_DACCLK] =
+		ast2700_clk_hw_register_gate(NULL, "dacclk", NULL,
+					     CLK_IS_CRITICAL, clk_base + SCU0_CLK_STOP,
+					     17, 0, &ast2700_clk_lock);
+
+	clks[SCU0_CLK_GATE_CRT0CLK] =
+		ast2700_clk_hw_register_gate(NULL, "crt0clk-gate", NULL,
+					     0, clk_base + SCU0_CLK_STOP,
+					     20, 0, &ast2700_clk_lock);
+
+	clks[SCU0_CLK_GATE_CRT1CLK] =
+		ast2700_clk_hw_register_gate(NULL, "crt1clk-gate", NULL,
+					     0, clk_base + SCU0_CLK_STOP,
+					     21, 0, &ast2700_clk_lock);
+
+	clks[SCU0_CLK_GATE_ECDSACLK] =
+		ast2700_clk_hw_register_gate(NULL, "eccclk", NULL,
+					     0, clk_base + SCU0_CLK_STOP,
+					     23, 0, &ast2700_clk_lock);
+
+	clks[SCU0_CLK_GATE_RSACLK] =
+		ast2700_clk_hw_register_gate(NULL, "rsaclk", NULL,
+					     0, clk_base + SCU0_CLK_STOP,
+					     24, 0, &ast2700_clk_lock);
+
+	clks[SCU0_CLK_GATE_RVAS0CLK] =
+		ast2700_clk_hw_register_gate(NULL, "rvasclk", NULL,
+					     0, clk_base + SCU0_CLK_STOP,
+					     25, 0, &ast2700_clk_lock);
+
+	clks[SCU0_CLK_GATE_UFSCLK] =
+		ast2700_clk_hw_register_gate(NULL, "ufsclk", NULL,
+					     0, clk_base + SCU0_CLK_STOP,
+					     26, 0, &ast2700_clk_lock);
+
+	clks[SCU0_CLK_EMMCMUX] =
+		clk_hw_register_mux(NULL, "emmcsrc-mux", emmcclk_sel, ARRAY_SIZE(emmcclk_sel),
+				    0, clk_base + SCU0_CLK_SEL1,
+				    11, 1, 0, &ast2700_clk_lock);
+
+	clks[SCU0_CLK_EMMC] =
+		clk_hw_register_divider_table(NULL, "emmcclk", "emmcsrc-mux",
+					      0, clk_base + SCU0_CLK_SEL1,
+					      12, 3, 0, ast2700_clk_div_table2,
+					      &ast2700_clk_lock);
+
+	clks[SCU0_CLK_GATE_EMMCCLK] =
+		ast2700_clk_hw_register_gate(NULL, "emmcclk-gate", "emmcclk",
+					     0, clk_base + SCU0_CLK_STOP,
+					     27, 0, &ast2700_clk_lock);
+
+	clks[SCU0_CLK_GATE_RVAS1CLK] =
+		ast2700_clk_hw_register_gate(NULL, "rvas2clk", NULL,
+					     0, clk_base + SCU0_CLK_STOP,
+					     28, 0, &ast2700_clk_lock);
+
+	of_clk_add_hw_provider(soc0_node, of_clk_hw_onecell_get, clk_data);
+
+	return 0;
+};
+
+CLK_OF_DECLARE_DRIVER(ast2700_soc0, "aspeed,ast2700-scu0", ast2700_soc0_clk_init);
+CLK_OF_DECLARE_DRIVER(ast2700_soc1, "aspeed,ast2700-scu1", ast2700_soc1_clk_init);
-- 
2.34.1

Re: [PATCH 4/4] dt-bindings: clock: Add AST2700 clock bindings

[Krzysztof Kozlowski]

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, that's wrong on so many levels. There are no bindings here!

You ignored previous feedback given to Assped.

Best regards,
Krzysztof

RE: [PATCH 4/4] dt-bindings: clock: Add AST2700 clock bindings

[Ryan Chen]

> Subject: Re: [PATCH 4/4] dt-bindings: clock: Add AST2700 clock bindings
> 
> 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, that's wrong on so many levels. There are no bindings here!
> 
> You ignored previous feedback given to Assped.

Will modify subject to "clk: aspeed: add ast2700 clk driver"
> 
> Best regards,
> Krzysztof

Re: [PATCH 4/4] dt-bindings: clock: Add AST2700 clock bindings

[Krzysztof Kozlowski]

On 19/08/2024 07:57, Ryan Chen wrote:
>> Subject: Re: [PATCH 4/4] dt-bindings: clock: Add AST2700 clock bindings
>>
>> 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, that's wrong on so many levels. There are no bindings here!
>>
>> You ignored previous feedback given to Assped.
> 
> Will modify subject to "clk: aspeed: add ast2700 clk driver"

Respond and implement first feedback which was given to Aspeed.

Best regards,
Krzysztof

RE: [PATCH 4/4] dt-bindings: clock: Add AST2700 clock bindings

[Ryan Chen]

> Subject: Re: [PATCH 4/4] dt-bindings: clock: Add AST2700 clock bindings
> 
> On 19/08/2024 07:57, Ryan Chen wrote:
> >> Subject: Re: [PATCH 4/4] dt-bindings: clock: Add AST2700 clock bindings
> >>
> >> 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, that's wrong on so many levels. There are no bindings here!
> >>
> >> You ignored previous feedback given to Assped.
> >
> > Will modify subject to "clk: aspeed: add ast2700 clk driver"
> 
> Respond and implement first feedback which was given to Aspeed.
> 
Sorry, I check your first feedback, I will remove "Drop WARN_ON", "Weird comment"
https://patchwork.kernel.org/project/linux-clk/patch/20240726110355.2181563-4-kevin_chen@aspeedtech.com/
What else I miss with your response?

> Best regards,
> Krzysztof