diff mbox series

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

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

Commit Message

Ryan Chen Aug. 8, 2024, 7:59 a.m. UTC
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(-)

Comments

Krzysztof Kozlowski Aug. 8, 2024, 10:14 a.m. UTC | #1
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
Ryan Chen Aug. 9, 2024, 5:55 a.m. UTC | #2
> 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.
Krzysztof Kozlowski Aug. 9, 2024, 6:02 a.m. UTC | #3
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
Ryan Chen Aug. 9, 2024, 6:10 a.m. UTC | #4
> 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.
Ryan Chen Aug. 12, 2024, 6:26 a.m. UTC | #5
> 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
>
Krzysztof Kozlowski Aug. 12, 2024, 6:34 a.m. UTC | #6
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
Rob Herring (Arm) Aug. 13, 2024, 7:14 p.m. UTC | #7
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
Ryan Chen Aug. 14, 2024, 6:35 a.m. UTC | #8
> 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
Andrew Jeffery Aug. 15, 2024, 12:26 a.m. UTC | #9
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
Ryan Chen Aug. 15, 2024, 1:43 a.m. UTC | #10
> 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?
Andrew Jeffery Aug. 15, 2024, 1:56 a.m. UTC | #11
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
Ryan Chen Aug. 19, 2024, 3:05 a.m. UTC | #12
> > 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 ?
Andrew Jeffery Aug. 20, 2024, 12:46 a.m. UTC | #13
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
Ryan Chen Aug. 20, 2024, 1:52 a.m. UTC | #14
> 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?
Andrew Jeffery Aug. 20, 2024, 5:01 a.m. UTC | #15
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
Ryan Chen Aug. 20, 2024, 6:51 a.m. UTC | #16
> 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
diff mbox series

Patch

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>;
+            };
+        };
+    };
 ...