[PATCH 0/5] soc: loongson: Fix some issues about loongson_pm2

[PATCH 0/5] soc: loongson: Fix some issues about loongson_pm2

[Binbin Zhou]

Hi all:

Since commit 67694c076bd7 ("soc: loongson2_pm: add power management support"),
the Loongson-2K PM driver was added, some issues have been found and
this patch set is planned to fix these issues.

Specific:
Patch 1: Compilation error found by Randy;
Patch 2/3: Add Loongson-2K2000 support;
Patch 4/5: Add Loongson-2K SoC reboot/shutdown support as part of power
management.

Thanks.

Binbin Zhou (5):
  soc: loongson: loongson_pm2: add dependency for INPUT
  dt-bindings: soc: loongson,ls2k-pmc: Add missing compatible for
    Loongson-2K2000
  soc: loongson: loongson_pm2: Add missing compatible for
    Loongson-2K2000
  dt-bindings: soc: loongson,ls2k-pmc: Allow
    syscon-reboot/syscon-poweroff as child
  soc: loongson: loongson_pm2: Populate children syscon nodes

 .../soc/loongson/loongson,ls2k-pmc.yaml       | 30 ++++++++++++++++++-
 drivers/soc/loongson/Kconfig                  |  1 +
 drivers/soc/loongson/loongson2_pm.c           |  7 +++++
 3 files changed, 37 insertions(+), 1 deletion(-)

-- 
2.39.3

[PATCH 1/5] soc: loongson: loongson_pm2: add dependency for INPUT

[Binbin Zhou]

Since commit 67694c076bd7 ("soc: loongson2_pm: add power management
support"), the Loongson-2K PM driver was added, but it didn't update the
Kconfig entry for the INPUT dependency, leading to build errors, so
update the Kconfig entry to depend on INPUT.

/opt/crosstool/gcc-13.2.0-nolibc/loongarch64-linux/bin/loongarch64-linux-ld: drivers/soc/loongson/loongson2_pm.o: in function `loongson2_power_button_init':
/work/lnx/next/linux-next-20230825/LOONG64/../drivers/soc/loongson/loongson2_pm.c:101:(.text+0x350): undefined reference to `input_allocate_device'
/opt/crosstool/gcc-13.2.0-nolibc/loongarch64-linux/bin/loongarch64-linux-ld: /work/lnx/next/linux-next-20230825/LOONG64/../drivers/soc/loongson/loongson2_pm.c:109:(.text+0x3dc): undefined reference to `input_set_capability'
/opt/crosstool/gcc-13.2.0-nolibc/loongarch64-linux/bin/loongarch64-linux-ld: /work/lnx/next/linux-next-20230825/LOONG64/../drivers/soc/loongson/loongson2_pm.c:111:(.text+0x3e4): undefined reference to `input_register_device'
/opt/crosstool/gcc-13.2.0-nolibc/loongarch64-linux/bin/loongarch64-linux-ld: /work/lnx/next/linux-next-20230825/LOONG64/../drivers/soc/loongson/loongson2_pm.c:125:(.text+0x3fc): undefined reference to `input_free_device'
/opt/crosstool/gcc-13.2.0-nolibc/loongarch64-linux/bin/loongarch64-linux-ld: drivers/soc/loongson/loongson2_pm.o: in function `input_report_key':
/work/lnx/next/linux-next-20230825/LOONG64/../include/linux/input.h:425:(.text+0x58c): undefined reference to `input_event'
/opt/crosstool/gcc-13.2.0-nolibc/loongarch64-linux/bin/loongarch64-linux-ld: drivers/soc/loongson/loongson2_pm.o: in function `input_sync':
/work/lnx/next/linux-next-20230825/LOONG64/../include/linux/input.h:450:(.text+0x5a0): undefined reference to `input_event'
/opt/crosstool/gcc-13.2.0-nolibc/loongarch64-linux/bin/loongarch64-linux-ld: drivers/soc/loongson/loongson2_pm.o: in function `input_report_key':
/work/lnx/next/linux-next-20230825/LOONG64/../include/linux/input.h:425:(.text+0x5b4): undefined reference to `input_event'
/opt/crosstool/gcc-13.2.0-nolibc/loongarch64-linux/bin/loongarch64-linux-ld: drivers/soc/loongson/loongson2_pm.o: in function `input_sync':
/work/lnx/next/linux-next-20230825/LOONG64/../include/linux/input.h:450:(.text+0x5c8): undefined reference to `input_event'

Reported-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 drivers/soc/loongson/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/soc/loongson/Kconfig b/drivers/soc/loongson/Kconfig
index 314e13bb3e01..1b57af1e5529 100644
--- a/drivers/soc/loongson/Kconfig
+++ b/drivers/soc/loongson/Kconfig
@@ -20,6 +20,7 @@ config LOONGSON2_GUTS
 config LOONGSON2_PM
 	bool "Loongson-2 SoC Power Management Controller Driver"
 	depends on LOONGARCH && OF
+	depends on INPUT
 	help
 	  The Loongson-2's power management controller was ACPI, supports ACPI
 	  S2Idle (Suspend To Idle), ACPI S3 (Suspend To RAM), ACPI S4 (Suspend To
-- 
2.39.3

Re: [PATCH 1/5] soc: loongson: loongson_pm2: add dependency for INPUT

[Krzysztof Kozlowski]

On 28/08/2023 14:38, Binbin Zhou wrote:
> Since commit 67694c076bd7 ("soc: loongson2_pm: add power management
> support"), the Loongson-2K PM driver was added, but it didn't update the
> Kconfig entry for the INPUT dependency, leading to build errors, so
> update the Kconfig entry to depend on INPUT.
> 
> /opt/crosstool/gcc-13.2.0-nolibc/loongarch64-linux/bin/loongarch64-linux-ld: drivers/soc/loongson/loongson2_pm.o: in function `loongson2_power_button_init':
> /work/lnx/next/linux-next-20230825/LOONG64/../drivers/soc/loongson/loongson2_pm.c:101:(.text+0x350): undefined reference to `input_allocate_device'
> /opt/crosstool/gcc-13.2.0-nolibc/loongarch64-linux/bin/loongarch64-linux-ld: /work/lnx/next/linux-next-20230825/LOONG64/../drivers/soc/loongson/loongson2_pm.c:109:(.text+0x3dc): undefined reference to `input_set_capability'
> /opt/crosstool/gcc-13.2.0-nolibc/loongarch64-linux/bin/loongarch64-linux-ld: /work/lnx/next/linux-next-20230825/LOONG64/../drivers/soc/loongson/loongson2_pm.c:111:(.text+0x3e4): undefined reference to `input_register_device'
> /opt/crosstool/gcc-13.2.0-nolibc/loongarch64-linux/bin/loongarch64-linux-ld: /work/lnx/next/linux-next-20230825/LOONG64/../drivers/soc/loongson/loongson2_pm.c:125:(.text+0x3fc): undefined reference to `input_free_device'
> /opt/crosstool/gcc-13.2.0-nolibc/loongarch64-linux/bin/loongarch64-linux-ld: drivers/soc/loongson/loongson2_pm.o: in function `input_report_key':
> /work/lnx/next/linux-next-20230825/LOONG64/../include/linux/input.h:425:(.text+0x58c): undefined reference to `input_event'
> /opt/crosstool/gcc-13.2.0-nolibc/loongarch64-linux/bin/loongarch64-linux-ld: drivers/soc/loongson/loongson2_pm.o: in function `input_sync':
> /work/lnx/next/linux-next-20230825/LOONG64/../include/linux/input.h:450:(.text+0x5a0): undefined reference to `input_event'
> /opt/crosstool/gcc-13.2.0-nolibc/loongarch64-linux/bin/loongarch64-linux-ld: drivers/soc/loongson/loongson2_pm.o: in function `input_report_key':
> /work/lnx/next/linux-next-20230825/LOONG64/../include/linux/input.h:425:(.text+0x5b4): undefined reference to `input_event'
> /opt/crosstool/gcc-13.2.0-nolibc/loongarch64-linux/bin/loongarch64-linux-ld: drivers/soc/loongson/loongson2_pm.o: in function `input_sync':
> /work/lnx/next/linux-next-20230825/LOONG64/../include/linux/input.h:450:(.text+0x5c8): undefined reference to `input_event'

Trim unrelated parts of path and trim logs to show only one warning, not
everything.

Best regards,
Krzysztof

Re: [PATCH 1/5] soc: loongson: loongson_pm2: add dependency for INPUT

[Randy Dunlap]

Hi--

On 8/28/23 05:38, Binbin Zhou wrote:
> Since commit 67694c076bd7 ("soc: loongson2_pm: add power management
> support"), the Loongson-2K PM driver was added, but it didn't update the
> Kconfig entry for the INPUT dependency, leading to build errors, so
> update the Kconfig entry to depend on INPUT.
> 
> /opt/crosstool/gcc-13.2.0-nolibc/loongarch64-linux/bin/loongarch64-linux-ld: drivers/soc/loongson/loongson2_pm.o: in function `loongson2_power_button_init':
> /work/lnx/next/linux-next-20230825/LOONG64/../drivers/soc/loongson/loongson2_pm.c:101:(.text+0x350): undefined reference to `input_allocate_device'
> /opt/crosstool/gcc-13.2.0-nolibc/loongarch64-linux/bin/loongarch64-linux-ld: /work/lnx/next/linux-next-20230825/LOONG64/../drivers/soc/loongson/loongson2_pm.c:109:(.text+0x3dc): undefined reference to `input_set_capability'
> /opt/crosstool/gcc-13.2.0-nolibc/loongarch64-linux/bin/loongarch64-linux-ld: /work/lnx/next/linux-next-20230825/LOONG64/../drivers/soc/loongson/loongson2_pm.c:111:(.text+0x3e4): undefined reference to `input_register_device'
> /opt/crosstool/gcc-13.2.0-nolibc/loongarch64-linux/bin/loongarch64-linux-ld: /work/lnx/next/linux-next-20230825/LOONG64/../drivers/soc/loongson/loongson2_pm.c:125:(.text+0x3fc): undefined reference to `input_free_device'
> /opt/crosstool/gcc-13.2.0-nolibc/loongarch64-linux/bin/loongarch64-linux-ld: drivers/soc/loongson/loongson2_pm.o: in function `input_report_key':
> /work/lnx/next/linux-next-20230825/LOONG64/../include/linux/input.h:425:(.text+0x58c): undefined reference to `input_event'
> /opt/crosstool/gcc-13.2.0-nolibc/loongarch64-linux/bin/loongarch64-linux-ld: drivers/soc/loongson/loongson2_pm.o: in function `input_sync':
> /work/lnx/next/linux-next-20230825/LOONG64/../include/linux/input.h:450:(.text+0x5a0): undefined reference to `input_event'
> /opt/crosstool/gcc-13.2.0-nolibc/loongarch64-linux/bin/loongarch64-linux-ld: drivers/soc/loongson/loongson2_pm.o: in function `input_report_key':
> /work/lnx/next/linux-next-20230825/LOONG64/../include/linux/input.h:425:(.text+0x5b4): undefined reference to `input_event'
> /opt/crosstool/gcc-13.2.0-nolibc/loongarch64-linux/bin/loongarch64-linux-ld: drivers/soc/loongson/loongson2_pm.o: in function `input_sync':
> /work/lnx/next/linux-next-20230825/LOONG64/../include/linux/input.h:450:(.text+0x5c8): undefined reference to `input_event'
> 
> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> ---
>  drivers/soc/loongson/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/soc/loongson/Kconfig b/drivers/soc/loongson/Kconfig
> index 314e13bb3e01..1b57af1e5529 100644
> --- a/drivers/soc/loongson/Kconfig
> +++ b/drivers/soc/loongson/Kconfig
> @@ -20,6 +20,7 @@ config LOONGSON2_GUTS
>  config LOONGSON2_PM
>  	bool "Loongson-2 SoC Power Management Controller Driver"
>  	depends on LOONGARCH && OF
> +	depends on INPUT

In the failing .config file, CONFIG_INPUT=m.
This bool kconfig item is still set/enabled after this patch and the build still fails.

You could use
	depends on INPUT=y

if that is appropriate. I dunno.

>  	help
>  	  The Loongson-2's power management controller was ACPI, supports ACPI
>  	  S2Idle (Suspend To Idle), ACPI S3 (Suspend To RAM), ACPI S4 (Suspend To

-- 
~Randy

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

[Binbin Zhou]

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
       - const: syscon
 
   reg:
-- 
2.39.3

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1184 bytes --]

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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Binbin Zhou]

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

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

[Krzysztof Kozlowski]

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

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2336 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Binbin Zhou]

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
>

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

[Rob Herring]

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

[PATCH 3/5] soc: loongson: loongson_pm2: Add missing compatible for Loongson-2K2000

[Binbin Zhou]

Add compatible for Loongson-2K2000, which also has the Loongson2
Power Management Unit system controller.

This is a missing compatible, now we add it.

Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 drivers/soc/loongson/loongson2_pm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/soc/loongson/loongson2_pm.c b/drivers/soc/loongson/loongson2_pm.c
index 796add6e8b63..6d2b3708e947 100644
--- a/drivers/soc/loongson/loongson2_pm.c
+++ b/drivers/soc/loongson/loongson2_pm.c
@@ -198,6 +198,7 @@ static int loongson2_pm_probe(struct platform_device *pdev)
 static const struct of_device_id loongson2_pm_match[] = {
 	{ .compatible = "loongson,ls2k0500-pmc", },
 	{ .compatible = "loongson,ls2k1000-pmc", },
+	{ .compatible = "loongson,ls2k2000-pmc", },
 	{},
 };
 
-- 
2.39.3

Re: [PATCH 3/5] soc: loongson: loongson_pm2: Add missing compatible for Loongson-2K2000

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1043 bytes --]

On Mon, Aug 28, 2023 at 08:38:33PM +0800, Binbin Zhou wrote:
> Add compatible for Loongson-2K2000, which also has the Loongson2
> Power Management Unit system controller.
> 
> This is a missing compatible, now we add it.
> 
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> ---
>  drivers/soc/loongson/loongson2_pm.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/soc/loongson/loongson2_pm.c b/drivers/soc/loongson/loongson2_pm.c
> index 796add6e8b63..6d2b3708e947 100644
> --- a/drivers/soc/loongson/loongson2_pm.c
> +++ b/drivers/soc/loongson/loongson2_pm.c
> @@ -198,6 +198,7 @@ static int loongson2_pm_probe(struct platform_device *pdev)
>  static const struct of_device_id loongson2_pm_match[] = {
>  	{ .compatible = "loongson,ls2k0500-pmc", },
>  	{ .compatible = "loongson,ls2k1000-pmc", },
> +	{ .compatible = "loongson,ls2k2000-pmc", },

You've not added any new codepaths for this soc, so it is compatible
with the existing ones. Why are you not using fallback compatibles for
this?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 3/5] soc: loongson: loongson_pm2: Add missing compatible for Loongson-2K2000

[Krzysztof Kozlowski]

On 28/08/2023 14:38, Binbin Zhou wrote:
> Add compatible for Loongson-2K2000, which also has the Loongson2
> Power Management Unit system controller.
> 
> This is a missing compatible, now we add it.

How is it missing? Was support for it added before but not complete?



Best regards,
Krzysztof

[PATCH 4/5] dt-bindings: soc: loongson,ls2k-pmc: Allow syscon-reboot/syscon-poweroff as child

[Binbin Zhou]

The reboot and poweroff features are actually part of the Power
Management Unit system controller, thus allow them as its children,
instead of specifying as separate device nodes with syscon phandle.

Without it, the reboot/poweroff feature becomes unavailable.

Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 .../soc/loongson/loongson,ls2k-pmc.yaml       | 29 ++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
index 7473c5659929..f9e72b43d10c 100644
--- a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
+++ b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
@@ -33,6 +33,18 @@ properties:
       addition, the PM need according to it to indicate that current
       SoC whether support Suspend To RAM.
 
+  syscon-poweroff:
+    $ref: /schemas/power/reset/syscon-poweroff.yaml#
+    type: object
+    description:
+      Node for power off method
+
+  syscon-reboot:
+    $ref: /schemas/power/reset/syscon-reboot.yaml#
+    type: object
+    description:
+      Node for reboot method
+
 required:
   - compatible
   - reg
@@ -44,10 +56,25 @@ examples:
   - |
     #include <dt-bindings/interrupt-controller/irq.h>
 
-    power-management@1fe27000 {
+    pmc: power-management@1fe27000 {
         compatible = "loongson,ls2k1000-pmc", "syscon";
         reg = <0x1fe27000 0x58>;
         interrupt-parent = <&liointc1>;
         interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
         loongson,suspend-address = <0x0 0x1c000500>;
+
+        syscon-reboot {
+            compatible ="syscon-reboot";
+            regmap = <&pmc>;
+            offset = <0x30>;
+            mask = <0x1>;
+        };
+
+        syscon-poweroff {
+            compatible ="syscon-poweroff";
+            regmap = <&pmc>;
+            offset = <0x14>;
+            mask = <0x3c00>;
+            value = <0x3c00>;
+        };
     };
-- 
2.39.3

Re: [PATCH 4/5] dt-bindings: soc: loongson,ls2k-pmc: Allow syscon-reboot/syscon-poweroff as child

[Krzysztof Kozlowski]

On 28/08/2023 14:38, Binbin Zhou wrote:
> The reboot and poweroff features are actually part of the Power
> Management Unit system controller, thus allow them as its children,
> instead of specifying as separate device nodes with syscon phandle.
> 

>  required:
>    - compatible
>    - reg
> @@ -44,10 +56,25 @@ examples:
>    - |
>      #include <dt-bindings/interrupt-controller/irq.h>
>  
> -    power-management@1fe27000 {
> +    pmc: power-management@1fe27000 {

Drop the label.

>          compatible = "loongson,ls2k1000-pmc", "syscon";
>          reg = <0x1fe27000 0x58>;
>          interrupt-parent = <&liointc1>;
>          interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
>          loongson,suspend-address = <0x0 0x1c000500>;
> +
> +        syscon-reboot {
> +            compatible ="syscon-reboot";
> +            regmap = <&pmc>;

No, why? It does not make much sense and is deprecated.

> +            offset = <0x30>;
> +            mask = <0x1>;
> +        };
> +
> +        syscon-poweroff {
> +            compatible ="syscon-poweroff";

Missing space.

Best regards,
Krzysztof

Re: [PATCH 4/5] dt-bindings: soc: loongson,ls2k-pmc: Allow syscon-reboot/syscon-poweroff as child

[Binbin Zhou]

Hi Krzysztof:

Thanks for your reply.

On Tue, Aug 29, 2023 at 1:17 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 28/08/2023 14:38, Binbin Zhou wrote:
> > The reboot and poweroff features are actually part of the Power
> > Management Unit system controller, thus allow them as its children,
> > instead of specifying as separate device nodes with syscon phandle.
> >
>
> >  required:
> >    - compatible
> >    - reg
> > @@ -44,10 +56,25 @@ examples:
> >    - |
> >      #include <dt-bindings/interrupt-controller/irq.h>
> >
> > -    power-management@1fe27000 {
> > +    pmc: power-management@1fe27000 {
>
> Drop the label.
OK...
>
> >          compatible = "loongson,ls2k1000-pmc", "syscon";
> >          reg = <0x1fe27000 0x58>;
> >          interrupt-parent = <&liointc1>;
> >          interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
> >          loongson,suspend-address = <0x0 0x1c000500>;
> > +
> > +        syscon-reboot {
> > +            compatible ="syscon-reboot";
> > +            regmap = <&pmc>;
>
> No, why? It does not make much sense and is deprecated.

Oh, sorry, I should have been more careful, I'll delete it.
>
> > +            offset = <0x30>;
> > +            mask = <0x1>;
> > +        };
> > +
> > +        syscon-poweroff {
> > +            compatible ="syscon-poweroff";
>
> Missing space.
I will fix it.

Thanks.
Binbin
>
> Best regards,
> Krzysztof
>

[PATCH 5/5] soc: loongson: loongson_pm2: Populate children syscon nodes

[Binbin Zhou]

The syscon poweroff and reboot nodes logically belong to the Power
Management Unit so populate possible children.

Without it, the reboot/poweroff feature becomes unavailable.

Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 drivers/soc/loongson/loongson2_pm.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/soc/loongson/loongson2_pm.c b/drivers/soc/loongson/loongson2_pm.c
index 6d2b3708e947..6e8fb170d454 100644
--- a/drivers/soc/loongson/loongson2_pm.c
+++ b/drivers/soc/loongson/loongson2_pm.c
@@ -11,6 +11,7 @@
 #include <linux/input.h>
 #include <linux/suspend.h>
 #include <linux/interrupt.h>
+#include <linux/of_platform.h>
 #include <linux/pm_wakeirq.h>
 #include <linux/platform_device.h>
 #include <asm/bootinfo.h>
@@ -192,6 +193,11 @@ static int loongson2_pm_probe(struct platform_device *pdev)
 	if (loongson_sysconf.suspend_addr)
 		suspend_set_ops(&loongson2_suspend_ops);
 
+	/* Populate children */
+	retval = devm_of_platform_populate(dev);
+	if (retval)
+		dev_err(dev, "Error populating children, reboot and poweroff might not work properly\n");
+
 	return 0;
 }
 
-- 
2.39.3