[PATCH] arm64: dts: rockchip: make "poweroff" work on rock-5a

[PATCH] arm64: dts: rockchip: make "poweroff" work on rock-5a

[FUKAUMI Naoki]

to make "poweroff" command work, add "system-power-controller;" into
pmic@0. without this, "poweroff" works as same as "reboot".

Fixes: 75fdcbc8f4c1 ("arm64: dts: rockchip: add PMIC to rock-5a")

Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
---
 arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
index b070955627be..cf7b878a3eb2 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
@@ -395,6 +395,8 @@ pmic@0 {
 			    <&rk806_dvs2_null>, <&rk806_dvs3_null>;
 		spi-max-frequency = <1000000>;
 
+		system-power-controller;
+
 		vcc1-supply = <&vcc5v0_sys>;
 		vcc2-supply = <&vcc5v0_sys>;
 		vcc3-supply = <&vcc5v0_sys>;
-- 
2.43.0

Re: [PATCH] arm64: dts: rockchip: make "poweroff" work on rock-5a

[Dragan Simic]

Hello Fukami,

On 2024-06-11 14:27, FUKAUMI Naoki wrote:
> to make "poweroff" command work, add "system-power-controller;" into
> pmic@0. without this, "poweroff" works as same as "reboot".
> 
> Fixes: 75fdcbc8f4c1 ("arm64: dts: rockchip: add PMIC to rock-5a")
> 
> Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>

Looking good to me, with a small remark below.

Reviewed-by: Dragan Simic <dsimic@manjaro.org>

> ---
>  arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
> b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
> index b070955627be..cf7b878a3eb2 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
> @@ -395,6 +395,8 @@ pmic@0 {
>  			    <&rk806_dvs2_null>, <&rk806_dvs3_null>;
>  		spi-max-frequency = <1000000>;
> 

I don't think there's need for the empty line right above, i.e.
between the "spi-max-frequency" and "system-power-controller" lines.

> +		system-power-controller;
> +
>  		vcc1-supply = <&vcc5v0_sys>;
>  		vcc2-supply = <&vcc5v0_sys>;
>  		vcc3-supply = <&vcc5v0_sys>;

Re: [PATCH] arm64: dts: rockchip: make "poweroff" work on rock-5a

[Dragan Simic]

On 2024-06-11 15:37, Dragan Simic wrote:
> Hello Fukami,
> 
> On 2024-06-11 14:27, FUKAUMI Naoki wrote:
>> to make "poweroff" command work, add "system-power-controller;" into
>> pmic@0. without this, "poweroff" works as same as "reboot".

Sorry, I forgot to add that the description above could be reworded
a bit, e.g. not to repeat what's already visible in the patch itself.

For example:

     Designate the RK806 PMIC on the Radxa ROCK 5A as the system power
     controller, so the board shuts down properly on poweroff(8).

The patch subject could also be tweaked a bit, e.g. to refer to the
board as "Radxa ROCK 5A".

>> Fixes: 75fdcbc8f4c1 ("arm64: dts: rockchip: add PMIC to rock-5a")
>> Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
> 
> Looking good to me, with a small remark below.
> 
> Reviewed-by: Dragan Simic <dsimic@manjaro.org>
> 
>> ---
>>  arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts | 2 ++
>>  1 file changed, 2 insertions(+)
>> 
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
>> b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
>> index b070955627be..cf7b878a3eb2 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
>> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
>> @@ -395,6 +395,8 @@ pmic@0 {
>>  			    <&rk806_dvs2_null>, <&rk806_dvs3_null>;
>>  		spi-max-frequency = <1000000>;
>> 
> 
> I don't think there's need for the empty line right above, i.e.
> between the "spi-max-frequency" and "system-power-controller" lines.
> 
>> +		system-power-controller;
>> +
>>  		vcc1-supply = <&vcc5v0_sys>;
>>  		vcc2-supply = <&vcc5v0_sys>;
>>  		vcc3-supply = <&vcc5v0_sys>;
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH v2] arm64: dts: rockchip: make poweroff(8) work on Radxa ROCK 5A

[FUKAUMI Naoki]

Designate the RK806 PMIC on the Radxa ROCK 5A as the system power
controller, so the board shuts down properly on poweroff(8).

Fixes: 75fdcbc8f4c1 ("arm64: dts: rockchip: add PMIC to rock-5a")

Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
Reviewed-by: Dragan Simic <dsimic@manjaro.org>

Changes in v2:
- remove an empty line above "system-power-controller;"
- reword commit message
- add R-b tag
---
 arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
index b070955627be..3b9a349362db 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
@@ -394,6 +394,7 @@ pmic@0 {
 		pinctrl-0 = <&pmic_pins>, <&rk806_dvs1_null>,
 			    <&rk806_dvs2_null>, <&rk806_dvs3_null>;
 		spi-max-frequency = <1000000>;
+		system-power-controller;
 
 		vcc1-supply = <&vcc5v0_sys>;
 		vcc2-supply = <&vcc5v0_sys>;
-- 
2.43.0

Re: [PATCH] arm64: dts: rockchip: make "poweroff" work on rock-5a

[FUKAUMI Naoki]

Hello Dragan,

Thank you so much for your reply!

Best regards,

--
FUKAUMI Naoki
Radxa Computer (Shenzhen) Co., Ltd.

On 6/11/24 22:45, Dragan Simic wrote:
> On 2024-06-11 15:37, Dragan Simic wrote:
>> Hello Fukami,
>>
>> On 2024-06-11 14:27, FUKAUMI Naoki wrote:
>>> to make "poweroff" command work, add "system-power-controller;" into
>>> pmic@0. without this, "poweroff" works as same as "reboot".
> 
> Sorry, I forgot to add that the description above could be reworded
> a bit, e.g. not to repeat what's already visible in the patch itself.
> 
> For example:
> 
>      Designate the RK806 PMIC on the Radxa ROCK 5A as the system power
>      controller, so the board shuts down properly on poweroff(8).
> 
> The patch subject could also be tweaked a bit, e.g. to refer to the
> board as "Radxa ROCK 5A".
> 
>>> Fixes: 75fdcbc8f4c1 ("arm64: dts: rockchip: add PMIC to rock-5a")
>>> Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
>>
>> Looking good to me, with a small remark below.
>>
>> Reviewed-by: Dragan Simic <dsimic@manjaro.org>
>>
>>> ---
>>>  arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
>>> b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
>>> index b070955627be..cf7b878a3eb2 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
>>> @@ -395,6 +395,8 @@ pmic@0 {
>>>                  <&rk806_dvs2_null>, <&rk806_dvs3_null>;
>>>          spi-max-frequency = <1000000>;
>>>
>>
>> I don't think there's need for the empty line right above, i.e.
>> between the "spi-max-frequency" and "system-power-controller" lines.
>>
>>> +        system-power-controller;
>>> +
>>>          vcc1-supply = <&vcc5v0_sys>;
>>>          vcc2-supply = <&vcc5v0_sys>;
>>>          vcc3-supply = <&vcc5v0_sys>;
>>
>> _______________________________________________
>> Linux-rockchip mailing list
>> Linux-rockchip@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
> 

Re: [PATCH v2] arm64: dts: rockchip: make poweroff(8) work on Radxa ROCK 5A

[Dragan Simic]

Hello Fukami,

On 2024-06-12 03:12, FUKAUMI Naoki wrote:
> Designate the RK806 PMIC on the Radxa ROCK 5A as the system power
> controller, so the board shuts down properly on poweroff(8).
> 
> Fixes: 75fdcbc8f4c1 ("arm64: dts: rockchip: add PMIC to rock-5a")
> 
> Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
> Reviewed-by: Dragan Simic <dsimic@manjaro.org>

Thanks for the v2.  Just another small suggestion, the tags above
should go like this, with no empty lines between them:

   Fixes: ...
   Reviewed-by: ...
   Signed-off-by: ...

> Changes in v2:
> - remove an empty line above "system-power-controller;"
> - reword commit message
> - add R-b tag
> ---
>  arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
> b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
> index b070955627be..3b9a349362db 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
> @@ -394,6 +394,7 @@ pmic@0 {
>  		pinctrl-0 = <&pmic_pins>, <&rk806_dvs1_null>,
>  			    <&rk806_dvs2_null>, <&rk806_dvs3_null>;
>  		spi-max-frequency = <1000000>;
> +		system-power-controller;
> 
>  		vcc1-supply = <&vcc5v0_sys>;
>  		vcc2-supply = <&vcc5v0_sys>;

[PATCH v3] arm64: dts: rockchip: make poweroff(8) work on Radxa ROCK 5A

[FUKAUMI Naoki]

Designate the RK806 PMIC on the Radxa ROCK 5A as the system power
controller, so the board shuts down properly on poweroff(8).

Fixes: 75fdcbc8f4c1 ("arm64: dts: rockchip: add PMIC to rock-5a")
Reviewed-by: Dragan Simic <dsimic@manjaro.org>
Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>

Changes in v3:
- sort tags
Changes in v2:
- remove an empty line above "system-power-controller;"
- reword commit message
- add R-b tag
---
 arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
index b070955627be..3b9a349362db 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
@@ -394,6 +394,7 @@ pmic@0 {
 		pinctrl-0 = <&pmic_pins>, <&rk806_dvs1_null>,
 			    <&rk806_dvs2_null>, <&rk806_dvs3_null>;
 		spi-max-frequency = <1000000>;
+		system-power-controller;
 
 		vcc1-supply = <&vcc5v0_sys>;
 		vcc2-supply = <&vcc5v0_sys>;
-- 
2.43.0

Re: [PATCH v3] arm64: dts: rockchip: make poweroff(8) work on Radxa ROCK 5A

[Heiko Stübner]

Hi,

Am Mittwoch, 12. Juni 2024, 05:35:23 CEST schrieb FUKAUMI Naoki:
> Designate the RK806 PMIC on the Radxa ROCK 5A as the system power
> controller, so the board shuts down properly on poweroff(8).
> 
> Fixes: 75fdcbc8f4c1 ("arm64: dts: rockchip: add PMIC to rock-5a")
> Reviewed-by: Dragan Simic <dsimic@manjaro.org>
> Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>

applied as fix for 6.10

Same comment about patch-versions in mail-threads like on the
cd-gpios revert.

Thanks
Heiko