diff mbox series

[2/5] dt-bindings: soc: loongson,ls2k-pmc: Add missing compatible for Loongson-2K2000

Message ID 54ee114c08f35ab8b5dc584fd76135ac9076f5a7.1693218539.git.zhoubinbin@loongson.cn (mailing list archive)
State Handled Elsewhere, archived
Headers show
Series soc: loongson: Fix some issues about loongson_pm2 | expand

Commit Message

Binbin Zhou Aug. 28, 2023, 12:38 p.m. UTC
Document the Power Management Unit system controller compatible for
Loongson-2K2000.

This is a missing compatible, now we add it.

Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 .../devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml      | 1 +
 1 file changed, 1 insertion(+)

Comments

Conor Dooley Aug. 28, 2023, 3:49 p.m. UTC | #1
On Mon, Aug 28, 2023 at 08:38:32PM +0800, Binbin Zhou wrote:
> Document the Power Management Unit system controller compatible for
> Loongson-2K2000.
> 
> This is a missing compatible, now we add it.
> 
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> ---
>  .../devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml      | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
> index da2dcfeebf12..7473c5659929 100644
> --- a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
> +++ b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
> @@ -15,6 +15,7 @@ properties:
>        - enum:
>            - loongson,ls2k0500-pmc
>            - loongson,ls2k1000-pmc
> +          - loongson,ls2k2000-pmc

Same thing here as the driver patch, the pmc on this newly added SoC
appears to use the same codepaths as the existing ones. Does it share a
programming model & should there be a fallback compatible here?

>        - const: syscon
>  
>    reg:
> -- 
> 2.39.3
>
Binbin Zhou Aug. 29, 2023, 3:21 a.m. UTC | #2
HI Conor:

Thanks for your reply.

On Mon, Aug 28, 2023 at 11:49 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Mon, Aug 28, 2023 at 08:38:32PM +0800, Binbin Zhou wrote:
> > Document the Power Management Unit system controller compatible for
> > Loongson-2K2000.
> >
> > This is a missing compatible, now we add it.
> >
> > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > ---
> >  .../devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml      | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
> > index da2dcfeebf12..7473c5659929 100644
> > --- a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
> > +++ b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
> > @@ -15,6 +15,7 @@ properties:
> >        - enum:
> >            - loongson,ls2k0500-pmc
> >            - loongson,ls2k1000-pmc
> > +          - loongson,ls2k2000-pmc
>
> Same thing here as the driver patch, the pmc on this newly added SoC
> appears to use the same codepaths as the existing ones. Does it share a
> programming model & should there be a fallback compatible here?

I noticed the guideline about fallback compatible:

"DO use fallback compatibles when devices are the same as or a subset
of prior implementations."

But in fact, ls2k0500/ls2k1000/ls2k2000 are independent, there is no subset.
Can we define a "loongson,ls2k-pmc" superset for each ls2k SoC
compatible fallback.

Such as:

  compatible:
    oneOf:
      - enum:
          - loongson,ls2k0500-pmc
          - loongson,ls2k1000-pmc
          - loongson,ls2k2000-pmc
      - const: loongson,ls2k-pmc
      - const: syscon

Thanks.
Binbin

>
> >        - const: syscon
> >
> >    reg:
> > --
> > 2.39.3
> >
Krzysztof Kozlowski Aug. 29, 2023, 6:29 a.m. UTC | #3
On 29/08/2023 05:21, Binbin Zhou wrote:
> HI Conor:
> 
> Thanks for your reply.
> 
> On Mon, Aug 28, 2023 at 11:49 PM Conor Dooley <conor@kernel.org> wrote:
>>
>> On Mon, Aug 28, 2023 at 08:38:32PM +0800, Binbin Zhou wrote:
>>> Document the Power Management Unit system controller compatible for
>>> Loongson-2K2000.
>>>
>>> This is a missing compatible, now we add it.
>>>
>>> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
>>> ---
>>>  .../devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml      | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
>>> index da2dcfeebf12..7473c5659929 100644
>>> --- a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
>>> +++ b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
>>> @@ -15,6 +15,7 @@ properties:
>>>        - enum:
>>>            - loongson,ls2k0500-pmc
>>>            - loongson,ls2k1000-pmc
>>> +          - loongson,ls2k2000-pmc
>>
>> Same thing here as the driver patch, the pmc on this newly added SoC
>> appears to use the same codepaths as the existing ones. Does it share a
>> programming model & should there be a fallback compatible here?
> 
> I noticed the guideline about fallback compatible:
> 
> "DO use fallback compatibles when devices are the same as or a subset
> of prior implementations."
> 
> But in fact, ls2k0500/ls2k1000/ls2k2000 are independent, there is no subset.

We do not consider here ls2k0500/ls2k1000/ls2k2000, but PMC in each of
them. If they are independent, why would they use the same interface?

> Can we define a "loongson,ls2k-pmc" superset for each ls2k SoC
> compatible fallback.
> 
> Such as:
> 
>   compatible:
>     oneOf:
>       - enum:
>           - loongson,ls2k0500-pmc
>           - loongson,ls2k1000-pmc
>           - loongson,ls2k2000-pmc
>       - const: loongson,ls2k-pmc

This is discouraged. Use 0500 as fallback.



Best regards,
Krzysztof
Conor Dooley Aug. 29, 2023, 6:42 a.m. UTC | #4
On Tue, Aug 29, 2023 at 08:29:43AM +0200, Krzysztof Kozlowski wrote:
> On 29/08/2023 05:21, Binbin Zhou wrote:
> > HI Conor:
> > 
> > Thanks for your reply.
> > 
> > On Mon, Aug 28, 2023 at 11:49 PM Conor Dooley <conor@kernel.org> wrote:
> >>
> >> On Mon, Aug 28, 2023 at 08:38:32PM +0800, Binbin Zhou wrote:
> >>> Document the Power Management Unit system controller compatible for
> >>> Loongson-2K2000.
> >>>
> >>> This is a missing compatible, now we add it.
> >>>
> >>> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> >>> ---
> >>>  .../devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml      | 1 +
> >>>  1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
> >>> index da2dcfeebf12..7473c5659929 100644
> >>> --- a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
> >>> +++ b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
> >>> @@ -15,6 +15,7 @@ properties:
> >>>        - enum:
> >>>            - loongson,ls2k0500-pmc
> >>>            - loongson,ls2k1000-pmc
> >>> +          - loongson,ls2k2000-pmc
> >>
> >> Same thing here as the driver patch, the pmc on this newly added SoC
> >> appears to use the same codepaths as the existing ones. Does it share a
> >> programming model & should there be a fallback compatible here?
> > 
> > I noticed the guideline about fallback compatible:
> > 
> > "DO use fallback compatibles when devices are the same as or a subset
> > of prior implementations."
> > 
> > But in fact, ls2k0500/ls2k1000/ls2k2000 are independent, there is no subset.
> 
> We do not consider here ls2k0500/ls2k1000/ls2k2000, but PMC in each of
> them. If they are independent, why would they use the same interface?
> 
> > Can we define a "loongson,ls2k-pmc" superset for each ls2k SoC
> > compatible fallback.
> > 
> > Such as:
> > 
> >   compatible:
> >     oneOf:
> >       - enum:
> >           - loongson,ls2k0500-pmc
> >           - loongson,ls2k1000-pmc
> >           - loongson,ls2k2000-pmc
> >       - const: loongson,ls2k-pmc
> 
> This is discouraged. Use 0500 as fallback.

The "code" here is also invalid, the oneOf would been to be items.
Binbin Zhou Aug. 29, 2023, 6:52 a.m. UTC | #5
On Tue, Aug 29, 2023 at 2:29 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 29/08/2023 05:21, Binbin Zhou wrote:
> > HI Conor:
> >
> > Thanks for your reply.
> >
> > On Mon, Aug 28, 2023 at 11:49 PM Conor Dooley <conor@kernel.org> wrote:
> >>
> >> On Mon, Aug 28, 2023 at 08:38:32PM +0800, Binbin Zhou wrote:
> >>> Document the Power Management Unit system controller compatible for
> >>> Loongson-2K2000.
> >>>
> >>> This is a missing compatible, now we add it.
> >>>
> >>> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> >>> ---
> >>>  .../devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml      | 1 +
> >>>  1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
> >>> index da2dcfeebf12..7473c5659929 100644
> >>> --- a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
> >>> +++ b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
> >>> @@ -15,6 +15,7 @@ properties:
> >>>        - enum:
> >>>            - loongson,ls2k0500-pmc
> >>>            - loongson,ls2k1000-pmc
> >>> +          - loongson,ls2k2000-pmc
> >>
> >> Same thing here as the driver patch, the pmc on this newly added SoC
> >> appears to use the same codepaths as the existing ones. Does it share a
> >> programming model & should there be a fallback compatible here?
> >
> > I noticed the guideline about fallback compatible:
> >
> > "DO use fallback compatibles when devices are the same as or a subset
> > of prior implementations."
> >
> > But in fact, ls2k0500/ls2k1000/ls2k2000 are independent, there is no subset.
>
> We do not consider here ls2k0500/ls2k1000/ls2k2000, but PMC in each of
> them. If they are independent, why would they use the same interface?

Sorry. I may have misunderstood.
The "subset" in the above guideline, here we should be talking about
PMC, not SoC.
For PMC, ls2k0500/ls2k1000/ls2k2000 are the same.

Am I understanding correctly now?

Also, when I said "independent" above, I meant they are three different SoCs.

Thanks.
Binbin
>
> > Can we define a "loongson,ls2k-pmc" superset for each ls2k SoC
> > compatible fallback.
> >
> > Such as:
> >
> >   compatible:
> >     oneOf:
> >       - enum:
> >           - loongson,ls2k0500-pmc
> >           - loongson,ls2k1000-pmc
> >           - loongson,ls2k2000-pmc
> >       - const: loongson,ls2k-pmc
>
> This is discouraged. Use 0500 as fallback.
>
>
>
> Best regards,
> Krzysztof
>
Rob Herring (Arm) Aug. 31, 2023, 4:39 p.m. UTC | #6
On Tue, Aug 29, 2023 at 02:52:48PM +0800, Binbin Zhou wrote:
> On Tue, Aug 29, 2023 at 2:29 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
> >
> > On 29/08/2023 05:21, Binbin Zhou wrote:
> > > HI Conor:
> > >
> > > Thanks for your reply.
> > >
> > > On Mon, Aug 28, 2023 at 11:49 PM Conor Dooley <conor@kernel.org> wrote:
> > >>
> > >> On Mon, Aug 28, 2023 at 08:38:32PM +0800, Binbin Zhou wrote:
> > >>> Document the Power Management Unit system controller compatible for
> > >>> Loongson-2K2000.
> > >>>
> > >>> This is a missing compatible, now we add it.
> > >>>
> > >>> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > >>> ---
> > >>>  .../devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml      | 1 +
> > >>>  1 file changed, 1 insertion(+)
> > >>>
> > >>> diff --git a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
> > >>> index da2dcfeebf12..7473c5659929 100644
> > >>> --- a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
> > >>> +++ b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
> > >>> @@ -15,6 +15,7 @@ properties:
> > >>>        - enum:
> > >>>            - loongson,ls2k0500-pmc
> > >>>            - loongson,ls2k1000-pmc
> > >>> +          - loongson,ls2k2000-pmc
> > >>
> > >> Same thing here as the driver patch, the pmc on this newly added SoC
> > >> appears to use the same codepaths as the existing ones. Does it share a
> > >> programming model & should there be a fallback compatible here?
> > >
> > > I noticed the guideline about fallback compatible:
> > >
> > > "DO use fallback compatibles when devices are the same as or a subset
> > > of prior implementations."
> > >
> > > But in fact, ls2k0500/ls2k1000/ls2k2000 are independent, there is no subset.
> >
> > We do not consider here ls2k0500/ls2k1000/ls2k2000, but PMC in each of
> > them. If they are independent, why would they use the same interface?
> 
> Sorry. I may have misunderstood.
> The "subset" in the above guideline, here we should be talking about
> PMC, not SoC.
> For PMC, ls2k0500/ls2k1000/ls2k2000 are the same.
> 
> Am I understanding correctly now?

The test is can an OS/client which only understands the fallback 
compatible (loongson,ls2k-pmc) use that h/w block (or some subset of 
it)? If so, then a fallback is appropriate. If not, then don't use a 
fallback. If the block in one SoC is a proper subset of another SoC, 
then make the compatible that's a subset the fallback. However, that's 
really only helpful if an OS/client already understands the fallback. 


> Also, when I said "independent" above, I meant they are three different SoCs.

But those SoCs are probably not designed completely independently. An 
existing SoC is the basis for the next SoC design.

OTOH, things like pinmux, clocks, power mgt, etc. tend to change in 
every chip.

Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
index da2dcfeebf12..7473c5659929 100644
--- a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
+++ b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
@@ -15,6 +15,7 @@  properties:
       - enum:
           - loongson,ls2k0500-pmc
           - loongson,ls2k1000-pmc
+          - loongson,ls2k2000-pmc
       - const: syscon
 
   reg: