[PATCH v2 0/4] rockchip: rk8xx: allow to customize PMIC reset mode on RK806

[PATCH v2 0/4] rockchip: rk8xx: allow to customize PMIC reset mode on RK806

[Quentin Schulz]

This allows to customize the PMIC reset method (also called RST_FUN) on
RK806 PMIC from Rockchip, mainly found on RK3588 devices but also on
RK3576.

Finally, this is required on the two RK3588 devices from Theobroma as
U-Boot changes the silicon-default (which is suitable for us) to
something that breaks our companion microcontroller's reboot detection
which breaks a bunch of assumptions in the MCU FW code.

To validate this works on those devices do the following:

On Tiger:
i2cset -y -f 6 0x6f 0x9 0x62
On Jaguar:
i2cset -y -f 0 0x6f 0x9 0x62

You hear a nice (loud :) ) beep, then reboot and it should stop right
before entering U-Boot TPL again.

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
Changes in v2:
- moved rst_fun variable declaration out of the switch-case,
- initialized rst_fun variable to make kernel test robot happy even
  though the variable wouldn't be used uninitialized due to breaking
  before using it,
- renamed rockchip,rst-fun to rockchip,reset-mode
- rewrote rockchip,reset-mode binding description to not mention the
  relation to registers or register values,
- added binding header file to make it easier to understand what the
  mode is when reading a Device Tree without having to read the binding,
- Link to v1: https://lore.kernel.org/r/20250526-rk8xx-rst-fun-v1-0-ea894d9474e0@cherry.de

---
Quentin Schulz (4):
      dt-bindings: mfd: rk806: allow to customize PMIC reset mode
      mfd: rk8xx-core: allow to customize RK806 reset mode
      arm64: dts: rockchip: force PMIC reset behavior to restart PMU on RK3588 Jaguar
      arm64: dts: rockchip: force PMIC reset behavior to restart PMU on RK3588 Tiger

 .../devicetree/bindings/mfd/rockchip,rk806.yaml    | 23 ++++++++++++++++++++++
 arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts     |  2 ++
 arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi     |  2 ++
 drivers/mfd/rk8xx-core.c                           | 14 +++++++++++++
 include/dt-bindings/mfd/rockchip,rk8xx.h           | 17 ++++++++++++++++
 include/linux/mfd/rk808.h                          |  2 ++
 6 files changed, 60 insertions(+)
---
base-commit: ec7714e4947909190ffb3041a03311a975350fe0
change-id: 20250526-rk8xx-rst-fun-f261c40b6d0c

Best regards,
-- 
Quentin Schulz <quentin.schulz@cherry.de>

[PATCH v2 1/4] dt-bindings: mfd: rk806: allow to customize PMIC reset mode

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@cherry.de>

The RK806 PMIC allows to configure its reset/restart behavior whenever
the PMIC is reset either programmatically or via some external pins
(e.g. PWRCTRL or RESETB).

The following modes exist:
 - 0 (RK806_RESTART) restart PMU,
 - 1 (RK806_RESET) reset all power off reset registers and force
   state to switch to ACTIVE mode,
 - 2 (RK806_RESET_NOTIFY) same as RK806_RESET and also pull
   RESETB pin down for 5ms,

For example, some hardware may require a full restart
(RK806_RESTART mode) in order to function properly as regulators
are shortly interrupted in this mode.

This is the case for RK3588 Jaguar and RK3588 Tiger which have a
companion microcontroller running on an independent power supply and
monitoring the PMIC power rail to know the state of the main system.
When it detects a restart, it resets its own IPs exposed to the main
system as if to simulate its own reset. Failing to perform this fake
reset of the microcontroller may break things (e.g. watchdog not
automatically disabled, buzzer still running until manually disabled,
leftover configuration from previous main system state, etc...).

Some other systems may be depending on the power rails to not be
interrupted even for a small amount of time[1].

This allows to specify how the PMIC should perform on the hardware level
and may differ between harwdare designs, so a DT property seems
warranted. I unfortunately do not see how this could be made generic
enough to make it a non-vendor property.

[1] https://lore.kernel.org/linux-rockchip/2577051.irdbgypaU6@workhorse/
Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
 .../devicetree/bindings/mfd/rockchip,rk806.yaml    | 23 ++++++++++++++++++++++
 include/dt-bindings/mfd/rockchip,rk8xx.h           | 17 ++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml b/Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml
index 3c2b06629b75ea94f90712470bf14ed7fc16d68d..c555b5956cea9f594d80ebd3b27e8489e520d97d 100644
--- a/Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml
+++ b/Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml
@@ -31,6 +31,29 @@ properties:
 
   system-power-controller: true
 
+  rockchip,reset-mode:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 1, 2]
+    description:
+      Mode to use when a reset of the PMIC is triggered.
+
+      The reset can be triggered either programmatically, via one of
+      the PWRCTRL pins (provided additional configuration) or
+      asserting RESETB pin low.
+
+      The following modes are supported (see also
+      include/dt-bindings/mfd/rockchip,rk8xx.h)
+
+      - 0 (RK806_RESTART) restart PMU,
+      - 1 (RK806_RESET) reset all power off reset registers and force
+        state to switch to ACTIVE mode,
+      - 2 (RK806_RESET_NOTIFY) same as RK806_RESET and also pull
+        RESETB pin down for 5ms,
+
+      For example, some hardware may require a full restart
+      (RK806_RESTART mode) in order to function properly as regulators
+      are shortly interrupted in this mode.
+
   vcc1-supply:
     description:
       The input supply for dcdc-reg1.
diff --git a/include/dt-bindings/mfd/rockchip,rk8xx.h b/include/dt-bindings/mfd/rockchip,rk8xx.h
new file mode 100644
index 0000000000000000000000000000000000000000..f058ed1ca661185f79738a358aa2d4f04539c590
--- /dev/null
+++ b/include/dt-bindings/mfd/rockchip,rk8xx.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
+/*
+ * Device Tree defines for Rockchip RK8xx PMICs
+ *
+ * Copyright 2025 Cherry Embedded Solutions GmbH
+ *
+ * Author: Quentin Schulz <quentin.schulz@cherry.de>
+ */
+
+#ifndef _DT_BINDINGS_MFD_ROCKCHIP_RK8XX_H
+#define _DT_BINDINGS_MFD_ROCKCHIP_RK8XX_H
+
+#define RK806_RESTART		0
+#define RK806_RESET		1
+#define RK806_RESET_NOTIFY	2
+
+#endif

-- 
2.49.0

Re: [PATCH v2 1/4] dt-bindings: mfd: rk806: allow to customize PMIC reset mode

[Krzysztof Kozlowski]

On Thu, Jun 05, 2025 at 05:41:06PM GMT, Quentin Schulz wrote:
> +  rockchip,reset-mode:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 1, 2]
> +    description:
> +      Mode to use when a reset of the PMIC is triggered.
> +
> +      The reset can be triggered either programmatically, via one of
> +      the PWRCTRL pins (provided additional configuration) or
> +      asserting RESETB pin low.
> +
> +      The following modes are supported (see also
> +      include/dt-bindings/mfd/rockchip,rk8xx.h)
> +
> +      - 0 (RK806_RESTART) restart PMU,
> +      - 1 (RK806_RESET) reset all power off reset registers and force
> +        state to switch to ACTIVE mode,
> +      - 2 (RK806_RESET_NOTIFY) same as RK806_RESET and also pull
> +        RESETB pin down for 5ms,
> +
> +      For example, some hardware may require a full restart
> +      (RK806_RESTART mode) in order to function properly as regulators
> +      are shortly interrupted in this mode.
> +

This is fine, although now points to missing restart-handler schema and
maybe this should be once made common property. But that's just
digression, nothing needed here.

>    vcc1-supply:
>      description:
>        The input supply for dcdc-reg1.
> diff --git a/include/dt-bindings/mfd/rockchip,rk8xx.h b/include/dt-bindings/mfd/rockchip,rk8xx.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..f058ed1ca661185f79738a358aa2d4f04539c590
> --- /dev/null
> +++ b/include/dt-bindings/mfd/rockchip,rk8xx.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
> +/*
> + * Device Tree defines for Rockchip RK8xx PMICs
> + *
> + * Copyright 2025 Cherry Embedded Solutions GmbH
> + *
> + * Author: Quentin Schulz <quentin.schulz@cherry.de>
> + */
> +
> +#ifndef _DT_BINDINGS_MFD_ROCKCHIP_RK8XX_H
> +#define _DT_BINDINGS_MFD_ROCKCHIP_RK8XX_H
> +
> +#define RK806_RESTART		0
> +#define RK806_RESET		1
> +#define RK806_RESET_NOTIFY	2

I do not see how this is a binding. Where do you use this in the driver
(to be a binding because otherwise you just add unused ABI)?

Best regards,
Krzysztof

Re: [PATCH v2 1/4] dt-bindings: mfd: rk806: allow to customize PMIC reset mode

[Quentin Schulz]

Hi Krzysztof,

On 6/17/25 10:08 AM, Krzysztof Kozlowski wrote:
> On Thu, Jun 05, 2025 at 05:41:06PM GMT, Quentin Schulz wrote:
>> +  rockchip,reset-mode:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    enum: [0, 1, 2]
>> +    description:
>> +      Mode to use when a reset of the PMIC is triggered.
>> +
>> +      The reset can be triggered either programmatically, via one of
>> +      the PWRCTRL pins (provided additional configuration) or
>> +      asserting RESETB pin low.
>> +
>> +      The following modes are supported (see also
>> +      include/dt-bindings/mfd/rockchip,rk8xx.h)
>> +
>> +      - 0 (RK806_RESTART) restart PMU,
>> +      - 1 (RK806_RESET) reset all power off reset registers and force
>> +        state to switch to ACTIVE mode,
>> +      - 2 (RK806_RESET_NOTIFY) same as RK806_RESET and also pull
>> +        RESETB pin down for 5ms,
>> +
>> +      For example, some hardware may require a full restart
>> +      (RK806_RESTART mode) in order to function properly as regulators
>> +      are shortly interrupted in this mode.
>> +
> 
> This is fine, although now points to missing restart-handler schema and
> maybe this should be once made common property. But that's just
> digression, nothing needed here.
> 
>>     vcc1-supply:
>>       description:
>>         The input supply for dcdc-reg1.
>> diff --git a/include/dt-bindings/mfd/rockchip,rk8xx.h b/include/dt-bindings/mfd/rockchip,rk8xx.h
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..f058ed1ca661185f79738a358aa2d4f04539c590
>> --- /dev/null
>> +++ b/include/dt-bindings/mfd/rockchip,rk8xx.h
>> @@ -0,0 +1,17 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
>> +/*
>> + * Device Tree defines for Rockchip RK8xx PMICs
>> + *
>> + * Copyright 2025 Cherry Embedded Solutions GmbH
>> + *
>> + * Author: Quentin Schulz <quentin.schulz@cherry.de>
>> + */
>> +
>> +#ifndef _DT_BINDINGS_MFD_ROCKCHIP_RK8XX_H
>> +#define _DT_BINDINGS_MFD_ROCKCHIP_RK8XX_H
>> +
>> +#define RK806_RESTART		0
>> +#define RK806_RESET		1
>> +#define RK806_RESET_NOTIFY	2
> 
> I do not see how this is a binding. Where do you use this in the driver
> (to be a binding because otherwise you just add unused ABI)?
> 

Explained in the commit log of the driver patch:

"""
This adds the appropriate logic in the driver to parse the new
rockchip,reset-mode DT property to pass this information. It just
happens that the values in the binding match the values to write in the
bitfield so no mapping is necessary.
"""

I can add useless mapping in the driver if it's preferred. I had the 
impression that simply using a hardcoded value in the DT binding and 
then writing it to the register was not desired, so the constant is now 
here to make this less obscure from DT perspective though I'm still 
writing the value directly in the register. If hardcoded values are ok 
in the binding, then I can remove that header file.

Cheers,
Quentin

Re: [PATCH v2 1/4] dt-bindings: mfd: rk806: allow to customize PMIC reset mode

[Krzysztof Kozlowski]

On 17/06/2025 11:38, Quentin Schulz wrote:
> Hi Krzysztof,
> 
> On 6/17/25 10:08 AM, Krzysztof Kozlowski wrote:
>> On Thu, Jun 05, 2025 at 05:41:06PM GMT, Quentin Schulz wrote:
>>> +  rockchip,reset-mode:
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    enum: [0, 1, 2]
>>> +    description:
>>> +      Mode to use when a reset of the PMIC is triggered.
>>> +
>>> +      The reset can be triggered either programmatically, via one of
>>> +      the PWRCTRL pins (provided additional configuration) or
>>> +      asserting RESETB pin low.
>>> +
>>> +      The following modes are supported (see also
>>> +      include/dt-bindings/mfd/rockchip,rk8xx.h)
>>> +
>>> +      - 0 (RK806_RESTART) restart PMU,
>>> +      - 1 (RK806_RESET) reset all power off reset registers and force
>>> +        state to switch to ACTIVE mode,
>>> +      - 2 (RK806_RESET_NOTIFY) same as RK806_RESET and also pull
>>> +        RESETB pin down for 5ms,
>>> +
>>> +      For example, some hardware may require a full restart
>>> +      (RK806_RESTART mode) in order to function properly as regulators
>>> +      are shortly interrupted in this mode.
>>> +
>>
>> This is fine, although now points to missing restart-handler schema and
>> maybe this should be once made common property. But that's just
>> digression, nothing needed here.
>>
>>>     vcc1-supply:
>>>       description:
>>>         The input supply for dcdc-reg1.
>>> diff --git a/include/dt-bindings/mfd/rockchip,rk8xx.h b/include/dt-bindings/mfd/rockchip,rk8xx.h
>>> new file mode 100644
>>> index 0000000000000000000000000000000000000000..f058ed1ca661185f79738a358aa2d4f04539c590
>>> --- /dev/null
>>> +++ b/include/dt-bindings/mfd/rockchip,rk8xx.h
>>> @@ -0,0 +1,17 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
>>> +/*
>>> + * Device Tree defines for Rockchip RK8xx PMICs
>>> + *
>>> + * Copyright 2025 Cherry Embedded Solutions GmbH
>>> + *
>>> + * Author: Quentin Schulz <quentin.schulz@cherry.de>
>>> + */
>>> +
>>> +#ifndef _DT_BINDINGS_MFD_ROCKCHIP_RK8XX_H
>>> +#define _DT_BINDINGS_MFD_ROCKCHIP_RK8XX_H
>>> +
>>> +#define RK806_RESTART		0
>>> +#define RK806_RESET		1
>>> +#define RK806_RESET_NOTIFY	2
>>
>> I do not see how this is a binding. Where do you use this in the driver
>> (to be a binding because otherwise you just add unused ABI)?
>>
> 
> Explained in the commit log of the driver patch:
> 
> """
> This adds the appropriate logic in the driver to parse the new
> rockchip,reset-mode DT property to pass this information. It just
> happens that the values in the binding match the values to write in the
> bitfield so no mapping is necessary.
> """
> 
> I can add useless mapping in the driver if it's preferred. I had the 

No, I comment and raise questions when you add ABI which is neither ABI
or should not be ABI.

> impression that simply using a hardcoded value in the DT binding and 
> then writing it to the register was not desired, so the constant is now 
> here to make this less obscure from DT perspective though I'm still 
> writing the value directly in the register. If hardcoded values are ok 
> in the binding, then I can remove that header file.

If you want something user readable, make it an enum string or keep the
header within DTS.

If I review it like that, it will be brought to me next time for some
other patch saying that commit was reviewed so I can do the same. [1]
Therefore since I object against unused binding headers in general
(there is no user here technically), I need to object here as well. :(


https://lore.kernel.org/all/0d381ad0-85d4-43de-a050-3b9ed03bf5d8@kernel.org/

Best regards,
Krzysztof

Re: [PATCH v2 1/4] dt-bindings: mfd: rk806: allow to customize PMIC reset mode

[Quentin Schulz]

On 6/17/25 12:21 PM, Krzysztof Kozlowski wrote:
> On 17/06/2025 11:38, Quentin Schulz wrote:
>> Hi Krzysztof,
>>
>> On 6/17/25 10:08 AM, Krzysztof Kozlowski wrote:
>>> On Thu, Jun 05, 2025 at 05:41:06PM GMT, Quentin Schulz wrote:
>>>> +  rockchip,reset-mode:
>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>>> +    enum: [0, 1, 2]
>>>> +    description:
>>>> +      Mode to use when a reset of the PMIC is triggered.
>>>> +
>>>> +      The reset can be triggered either programmatically, via one of
>>>> +      the PWRCTRL pins (provided additional configuration) or
>>>> +      asserting RESETB pin low.
>>>> +
>>>> +      The following modes are supported (see also
>>>> +      include/dt-bindings/mfd/rockchip,rk8xx.h)
>>>> +
>>>> +      - 0 (RK806_RESTART) restart PMU,
>>>> +      - 1 (RK806_RESET) reset all power off reset registers and force
>>>> +        state to switch to ACTIVE mode,
>>>> +      - 2 (RK806_RESET_NOTIFY) same as RK806_RESET and also pull
>>>> +        RESETB pin down for 5ms,
>>>> +
>>>> +      For example, some hardware may require a full restart
>>>> +      (RK806_RESTART mode) in order to function properly as regulators
>>>> +      are shortly interrupted in this mode.
>>>> +
>>>
>>> This is fine, although now points to missing restart-handler schema and
>>> maybe this should be once made common property. But that's just
>>> digression, nothing needed here.
>>>
>>>>      vcc1-supply:
>>>>        description:
>>>>          The input supply for dcdc-reg1.
>>>> diff --git a/include/dt-bindings/mfd/rockchip,rk8xx.h b/include/dt-bindings/mfd/rockchip,rk8xx.h
>>>> new file mode 100644
>>>> index 0000000000000000000000000000000000000000..f058ed1ca661185f79738a358aa2d4f04539c590
>>>> --- /dev/null
>>>> +++ b/include/dt-bindings/mfd/rockchip,rk8xx.h
>>>> @@ -0,0 +1,17 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
>>>> +/*
>>>> + * Device Tree defines for Rockchip RK8xx PMICs
>>>> + *
>>>> + * Copyright 2025 Cherry Embedded Solutions GmbH
>>>> + *
>>>> + * Author: Quentin Schulz <quentin.schulz@cherry.de>
>>>> + */
>>>> +
>>>> +#ifndef _DT_BINDINGS_MFD_ROCKCHIP_RK8XX_H
>>>> +#define _DT_BINDINGS_MFD_ROCKCHIP_RK8XX_H
>>>> +
>>>> +#define RK806_RESTART		0
>>>> +#define RK806_RESET		1
>>>> +#define RK806_RESET_NOTIFY	2
>>>
>>> I do not see how this is a binding. Where do you use this in the driver
>>> (to be a binding because otherwise you just add unused ABI)?
>>>
>>
>> Explained in the commit log of the driver patch:
>>
>> """
>> This adds the appropriate logic in the driver to parse the new
>> rockchip,reset-mode DT property to pass this information. It just
>> happens that the values in the binding match the values to write in the
>> bitfield so no mapping is necessary.
>> """
>>
>> I can add useless mapping in the driver if it's preferred. I had the
> 
> No, I comment and raise questions when you add ABI which is neither ABI
> or should not be ABI.
> 

Not sure what would make something part of the ABI or not. I would 
assume the value in the DT property to be ABI anyway so this is just 
another name for the same value no? Trying to understand this from your 
perspective.

>> impression that simply using a hardcoded value in the DT binding and
>> then writing it to the register was not desired, so the constant is now
>> here to make this less obscure from DT perspective though I'm still
>> writing the value directly in the register. If hardcoded values are ok
>> in the binding, then I can remove that header file.
> 
> If you want something user readable, make it an enum string or keep the
> header within DTS.
> 

Just to be sure I understood correctly, moving that file to e.g. 
arch/arm64/boot/dts/rockchip/rk806.h (or rk8xx.h or whatever) with the 
file content unchanged from this v2 would be fine with you? Would I be 
able to point at this file from the DT binding (I assume not)?
Of course, Heiko may have a different opinion on the location of this 
file as I don't see header files in arch/arm64/boot/dts/rockchip yet :)

> If I review it like that, it will be brought to me next time for some
> other patch saying that commit was reviewed so I can do the same. [1]

Fair :)

> Therefore since I object against unused binding headers in general
> (there is no user here technically), I need to object here as well. :(
> 

Laws do evolve too over time, same as how society view things. Something 
done decades ago could simply not be acceptable today, and vice-versa. 
It can be good, it can be bad :) Not here to judge, if there are new 
rules to contribute, there are new rules to follow :) (or discussed so 
they evolve to better work for maintainers, the community or project :) ).

It's easier to follow rules if they are made explicit somewhere. Is 
there a public documentation on those "new" rules maybe we can read 
ahead of time to make this easier on you? For example I really like 
https://www.kernel.org/doc/html/latest/devicetree/bindings/dts-coding-style.html 
and point to it often (internally, sometimes on the ML too). Maybe 
something to add to 
https://www.kernel.org/doc/html/latest/devicetree/bindings/writing-bindings.html 
or 
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html?

Cheers,
Quentin

Re: [PATCH v2 1/4] dt-bindings: mfd: rk806: allow to customize PMIC reset mode

[Krzysztof Kozlowski]

On 17/06/2025 12:45, Quentin Schulz wrote:
> On 6/17/25 12:21 PM, Krzysztof Kozlowski wrote:
>> On 17/06/2025 11:38, Quentin Schulz wrote:
>>> Hi Krzysztof,
>>>
>>> On 6/17/25 10:08 AM, Krzysztof Kozlowski wrote:
>>>> On Thu, Jun 05, 2025 at 05:41:06PM GMT, Quentin Schulz wrote:
>>>>> +  rockchip,reset-mode:
>>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>>>> +    enum: [0, 1, 2]
>>>>> +    description:
>>>>> +      Mode to use when a reset of the PMIC is triggered.
>>>>> +
>>>>> +      The reset can be triggered either programmatically, via one of
>>>>> +      the PWRCTRL pins (provided additional configuration) or
>>>>> +      asserting RESETB pin low.
>>>>> +
>>>>> +      The following modes are supported (see also
>>>>> +      include/dt-bindings/mfd/rockchip,rk8xx.h)
>>>>> +
>>>>> +      - 0 (RK806_RESTART) restart PMU,
>>>>> +      - 1 (RK806_RESET) reset all power off reset registers and force
>>>>> +        state to switch to ACTIVE mode,
>>>>> +      - 2 (RK806_RESET_NOTIFY) same as RK806_RESET and also pull
>>>>> +        RESETB pin down for 5ms,
>>>>> +
>>>>> +      For example, some hardware may require a full restart
>>>>> +      (RK806_RESTART mode) in order to function properly as regulators
>>>>> +      are shortly interrupted in this mode.
>>>>> +
>>>>
>>>> This is fine, although now points to missing restart-handler schema and
>>>> maybe this should be once made common property. But that's just
>>>> digression, nothing needed here.
>>>>
>>>>>      vcc1-supply:
>>>>>        description:
>>>>>          The input supply for dcdc-reg1.
>>>>> diff --git a/include/dt-bindings/mfd/rockchip,rk8xx.h b/include/dt-bindings/mfd/rockchip,rk8xx.h
>>>>> new file mode 100644
>>>>> index 0000000000000000000000000000000000000000..f058ed1ca661185f79738a358aa2d4f04539c590
>>>>> --- /dev/null
>>>>> +++ b/include/dt-bindings/mfd/rockchip,rk8xx.h
>>>>> @@ -0,0 +1,17 @@
>>>>> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
>>>>> +/*
>>>>> + * Device Tree defines for Rockchip RK8xx PMICs
>>>>> + *
>>>>> + * Copyright 2025 Cherry Embedded Solutions GmbH
>>>>> + *
>>>>> + * Author: Quentin Schulz <quentin.schulz@cherry.de>
>>>>> + */
>>>>> +
>>>>> +#ifndef _DT_BINDINGS_MFD_ROCKCHIP_RK8XX_H
>>>>> +#define _DT_BINDINGS_MFD_ROCKCHIP_RK8XX_H
>>>>> +
>>>>> +#define RK806_RESTART		0
>>>>> +#define RK806_RESET		1
>>>>> +#define RK806_RESET_NOTIFY	2
>>>>
>>>> I do not see how this is a binding. Where do you use this in the driver
>>>> (to be a binding because otherwise you just add unused ABI)?
>>>>
>>>
>>> Explained in the commit log of the driver patch:
>>>
>>> """
>>> This adds the appropriate logic in the driver to parse the new
>>> rockchip,reset-mode DT property to pass this information. It just
>>> happens that the values in the binding match the values to write in the
>>> bitfield so no mapping is necessary.
>>> """
>>>
>>> I can add useless mapping in the driver if it's preferred. I had the
>>
>> No, I comment and raise questions when you add ABI which is neither ABI
>> or should not be ABI.
>>
> 
> Not sure what would make something part of the ABI or not. I would 
> assume the value in the DT property to be ABI anyway so this is just 
> another name for the same value no? Trying to understand this from your 
> perspective.

Drop the header, it's not an ABI. You just use register values. This is
not a Linux ABI. The values are coming from the hardware.

> 
>>> impression that simply using a hardcoded value in the DT binding and
>>> then writing it to the register was not desired, so the constant is now
>>> here to make this less obscure from DT perspective though I'm still
>>> writing the value directly in the register. If hardcoded values are ok
>>> in the binding, then I can remove that header file.
>>
>> If you want something user readable, make it an enum string or keep the
>> header within DTS.
>>
> 
> Just to be sure I understood correctly, moving that file to e.g. 
> arch/arm64/boot/dts/rockchip/rk806.h (or rk8xx.h or whatever) with the 

Yes

> file content unchanged from this v2 would be fine with you? Would I be 
> able to point at this file from the DT binding (I assume not)?

No, because it is not a binding.

Best regards,
Krzysztof

[PATCH v2 2/4] mfd: rk8xx-core: allow to customize RK806 reset mode

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@cherry.de>

The RK806 PMIC has a bitfield for configuring the restart/reset behavior
(which I assume Rockchip calls "function") whenever the PMIC is reset
either programmatically (c.f. DEV_RST in the datasheet) or via PWRCTRL
or RESETB pins.

For RK806, the following values are possible for RST_FUN:

0b00 means "restart PMU"
0b01 means "Reset all the power off reset registers, forcing
	the state to switch to ACTIVE mode"
0b10 means "Reset all the power off reset registers, forcing
	the state to switch to ACTIVE mode, and simultaneously
	pull down the RESETB PIN for 5mS before releasing"
0b11 means the same as for 0b10 just above.

This adds the appropriate logic in the driver to parse the new
rockchip,reset-mode DT property to pass this information. It just
happens that the values in the binding match the values to write in the
bitfield so no mapping is necessary.

If it is missing, the register is left untouched and relies either on
the silicon default or on whatever was set earlier in the boot stages
(e.g. the bootloader).

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
 drivers/mfd/rk8xx-core.c  | 14 ++++++++++++++
 include/linux/mfd/rk808.h |  2 ++
 2 files changed, 16 insertions(+)

diff --git a/drivers/mfd/rk8xx-core.c b/drivers/mfd/rk8xx-core.c
index 71c2b80a4678d627e86cfbec8135f08e262559d3..32294af0b843fa20677513b1e1a5a6c8e76be4b6 100644
--- a/drivers/mfd/rk8xx-core.c
+++ b/drivers/mfd/rk8xx-core.c
@@ -699,6 +699,7 @@ int rk8xx_probe(struct device *dev, int variant, unsigned int irq, struct regmap
 	const struct mfd_cell *cells;
 	int dual_support = 0;
 	int nr_pre_init_regs;
+	u32 rst_fun = 0;
 	int nr_cells;
 	int ret;
 	int i;
@@ -726,6 +727,19 @@ int rk8xx_probe(struct device *dev, int variant, unsigned int irq, struct regmap
 		cells = rk806s;
 		nr_cells = ARRAY_SIZE(rk806s);
 		dual_support = IRQF_SHARED;
+
+		ret = device_property_read_u32(dev, "rockchip,reset-mode", &rst_fun);
+		if (ret) {
+			dev_dbg(dev,
+				"rockchip,reset-mode property missing, not setting RST_FUN\n");
+			break;
+		}
+
+		ret = regmap_update_bits(rk808->regmap, RK806_SYS_CFG3,
+					 RK806_RST_FUN_MSK,
+					 FIELD_PREP(RK806_RST_FUN_MSK, rst_fun));
+		if (ret)
+			return dev_err_probe(dev, ret, "RST_FUN write err\n");
 		break;
 	case RK808_ID:
 		rk808->regmap_irq_chip = &rk808_irq_chip;
diff --git a/include/linux/mfd/rk808.h b/include/linux/mfd/rk808.h
index 69cbea78b430b562a23d995263369d475daa6287..28170ee08898ca59c76a741a1d42763a42b72380 100644
--- a/include/linux/mfd/rk808.h
+++ b/include/linux/mfd/rk808.h
@@ -812,6 +812,8 @@ enum rk806_pin_dr_sel {
 #define RK806_INT_POL_H			BIT(1)
 #define RK806_INT_POL_L			0
 
+/* SYS_CFG3 */
+#define RK806_RST_FUN_MSK		GENMASK(7, 6)
 #define RK806_SLAVE_RESTART_FUN_MSK	BIT(1)
 #define RK806_SLAVE_RESTART_FUN_EN	BIT(1)
 #define RK806_SLAVE_RESTART_FUN_OFF	0

-- 
2.49.0

Re: [PATCH v2 2/4] mfd: rk8xx-core: allow to customize RK806 reset mode

[kernel test robot]

Hi Quentin,

kernel test robot noticed the following build errors:

[auto build test ERROR on ec7714e4947909190ffb3041a03311a975350fe0]

url:    https://github.com/intel-lab-lkp/linux/commits/Quentin-Schulz/dt-bindings-mfd-rk806-allow-to-customize-PMIC-reset-mode/20250605-234243
base:   ec7714e4947909190ffb3041a03311a975350fe0
patch link:    https://lore.kernel.org/r/20250605-rk8xx-rst-fun-v2-2-143d190596dd%40cherry.de
patch subject: [PATCH v2 2/4] mfd: rk8xx-core: allow to customize RK806 reset mode
config: arc-randconfig-001-20250607 (https://download.01.org/0day-ci/archive/20250607/202506071321.Ze0gsxC0-lkp@intel.com/config)
compiler: arc-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250607/202506071321.Ze0gsxC0-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506071321.Ze0gsxC0-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/mfd/rk8xx-core.c: In function 'rk8xx_probe':
>> drivers/mfd/rk8xx-core.c:740:42: error: implicit declaration of function 'FIELD_PREP' [-Wimplicit-function-declaration]
     740 |                                          FIELD_PREP(RK806_RST_FUN_MSK, rst_fun));
         |                                          ^~~~~~~~~~


vim +/FIELD_PREP +740 drivers/mfd/rk8xx-core.c

   694	
   695	int rk8xx_probe(struct device *dev, int variant, unsigned int irq, struct regmap *regmap)
   696	{
   697		struct rk808 *rk808;
   698		const struct rk808_reg_data *pre_init_reg;
   699		const struct mfd_cell *cells;
   700		int dual_support = 0;
   701		int nr_pre_init_regs;
   702		u32 rst_fun = 0;
   703		int nr_cells;
   704		int ret;
   705		int i;
   706	
   707		rk808 = devm_kzalloc(dev, sizeof(*rk808), GFP_KERNEL);
   708		if (!rk808)
   709			return -ENOMEM;
   710		rk808->dev = dev;
   711		rk808->variant = variant;
   712		rk808->regmap = regmap;
   713		dev_set_drvdata(dev, rk808);
   714	
   715		switch (rk808->variant) {
   716		case RK805_ID:
   717			rk808->regmap_irq_chip = &rk805_irq_chip;
   718			pre_init_reg = rk805_pre_init_reg;
   719			nr_pre_init_regs = ARRAY_SIZE(rk805_pre_init_reg);
   720			cells = rk805s;
   721			nr_cells = ARRAY_SIZE(rk805s);
   722			break;
   723		case RK806_ID:
   724			rk808->regmap_irq_chip = &rk806_irq_chip;
   725			pre_init_reg = rk806_pre_init_reg;
   726			nr_pre_init_regs = ARRAY_SIZE(rk806_pre_init_reg);
   727			cells = rk806s;
   728			nr_cells = ARRAY_SIZE(rk806s);
   729			dual_support = IRQF_SHARED;
   730	
   731			ret = device_property_read_u32(dev, "rockchip,reset-mode", &rst_fun);
   732			if (ret) {
   733				dev_dbg(dev,
   734					"rockchip,reset-mode property missing, not setting RST_FUN\n");
   735				break;
   736			}
   737	
   738			ret = regmap_update_bits(rk808->regmap, RK806_SYS_CFG3,
   739						 RK806_RST_FUN_MSK,
 > 740						 FIELD_PREP(RK806_RST_FUN_MSK, rst_fun));
   741			if (ret)
   742				return dev_err_probe(dev, ret, "RST_FUN write err\n");
   743			break;
   744		case RK808_ID:
   745			rk808->regmap_irq_chip = &rk808_irq_chip;
   746			pre_init_reg = rk808_pre_init_reg;
   747			nr_pre_init_regs = ARRAY_SIZE(rk808_pre_init_reg);
   748			cells = rk808s;
   749			nr_cells = ARRAY_SIZE(rk808s);
   750			break;
   751		case RK816_ID:
   752			rk808->regmap_irq_chip = &rk816_irq_chip;
   753			pre_init_reg = rk816_pre_init_reg;
   754			nr_pre_init_regs = ARRAY_SIZE(rk816_pre_init_reg);
   755			cells = rk816s;
   756			nr_cells = ARRAY_SIZE(rk816s);
   757			break;
   758		case RK818_ID:
   759			rk808->regmap_irq_chip = &rk818_irq_chip;
   760			pre_init_reg = rk818_pre_init_reg;
   761			nr_pre_init_regs = ARRAY_SIZE(rk818_pre_init_reg);
   762			cells = rk818s;
   763			nr_cells = ARRAY_SIZE(rk818s);
   764			break;
   765		case RK809_ID:
   766		case RK817_ID:
   767			rk808->regmap_irq_chip = &rk817_irq_chip;
   768			pre_init_reg = rk817_pre_init_reg;
   769			nr_pre_init_regs = ARRAY_SIZE(rk817_pre_init_reg);
   770			cells = rk817s;
   771			nr_cells = ARRAY_SIZE(rk817s);
   772			break;
   773		default:
   774			dev_err(dev, "Unsupported RK8XX ID %lu\n", rk808->variant);
   775			return -EINVAL;
   776		}
   777	
   778		if (!irq)
   779			return dev_err_probe(dev, -EINVAL, "No interrupt support, no core IRQ\n");
   780	
   781		ret = devm_regmap_add_irq_chip(dev, rk808->regmap, irq,
   782					       IRQF_ONESHOT | dual_support, -1,
   783					       rk808->regmap_irq_chip, &rk808->irq_data);
   784		if (ret)
   785			return dev_err_probe(dev, ret, "Failed to add irq_chip\n");
   786	
   787		for (i = 0; i < nr_pre_init_regs; i++) {
   788			ret = regmap_update_bits(rk808->regmap,
   789						pre_init_reg[i].addr,
   790						pre_init_reg[i].mask,
   791						pre_init_reg[i].value);
   792			if (ret)
   793				return dev_err_probe(dev, ret, "0x%x write err\n",
   794						     pre_init_reg[i].addr);
   795		}
   796	
   797		ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, cells, nr_cells, NULL, 0,
   798				      regmap_irq_get_domain(rk808->irq_data));
   799		if (ret)
   800			return dev_err_probe(dev, ret, "failed to add MFD devices\n");
   801	
   802		if (device_property_read_bool(dev, "system-power-controller") ||
   803		    device_property_read_bool(dev, "rockchip,system-power-controller")) {
   804			ret = devm_register_sys_off_handler(dev,
   805					    SYS_OFF_MODE_POWER_OFF_PREPARE, SYS_OFF_PRIO_HIGH,
   806					    &rk808_power_off, rk808);
   807			if (ret)
   808				return dev_err_probe(dev, ret,
   809						     "failed to register poweroff handler\n");
   810	
   811			switch (rk808->variant) {
   812			case RK809_ID:
   813			case RK817_ID:
   814				ret = devm_register_sys_off_handler(dev,
   815								    SYS_OFF_MODE_RESTART, SYS_OFF_PRIO_HIGH,
   816								    &rk808_restart, rk808);
   817				if (ret)
   818					dev_warn(dev, "failed to register rst handler, %d\n", ret);
   819				break;
   820			default:
   821				dev_dbg(dev, "pmic controlled board reset not supported\n");
   822				break;
   823			}
   824		}
   825	
   826		return 0;
   827	}
   828	EXPORT_SYMBOL_GPL(rk8xx_probe);
   829	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 2/4] mfd: rk8xx-core: allow to customize RK806 reset mode

[Quentin Schulz]

Hi all,

On 6/7/25 7:46 AM, kernel test robot wrote:
> Hi Quentin,
> 
> kernel test robot noticed the following build errors:
> 
> [auto build test ERROR on ec7714e4947909190ffb3041a03311a975350fe0]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Quentin-Schulz/dt-bindings-mfd-rk806-allow-to-customize-PMIC-reset-mode/20250605-234243
> base:   ec7714e4947909190ffb3041a03311a975350fe0
> patch link:    https://lore.kernel.org/r/20250605-rk8xx-rst-fun-v2-2-143d190596dd%40cherry.de
> patch subject: [PATCH v2 2/4] mfd: rk8xx-core: allow to customize RK806 reset mode
> config: arc-randconfig-001-20250607 (https://download.01.org/0day-ci/archive/20250607/202506071321.Ze0gsxC0-lkp@intel.com/config)
> compiler: arc-linux-gcc (GCC) 15.1.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250607/202506071321.Ze0gsxC0-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202506071321.Ze0gsxC0-lkp@intel.com/
> 
> All errors (new ones prefixed by >>):
> 
>     drivers/mfd/rk8xx-core.c: In function 'rk8xx_probe':
>>> drivers/mfd/rk8xx-core.c:740:42: error: implicit declaration of function 'FIELD_PREP' [-Wimplicit-function-declaration]

This should be simply fixed with the addition of

#include <linux/bitfield.h>

Though somehow I cannot reproduce the error locally as I get:

COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-15.1.0 
~/work/upstream/lkp-tests/kbuild/make.cross W=1 O=build/0day ARCH=arc 
olddefconfig
Compiler will be installed in /home/qschulz/0day
lftpget -c 
https://cdn.kernel.org/pub/tools/crosstool/files/bin/x86_64/15.1.0/x86_64-gcc-15.1.0-nolibc-sparc-linux.tar.xz
/home/qschulz/work/upstream/linux
tar Jxf 
/home/qschulz/0day/15.1.0/x86_64-gcc-15.1.0-nolibc-sparc-linux.tar.xz -C 
/home/qschulz/0day
No gcc cross compiler for arc
setup_crosstool failed

But I'm pretty sure that should be enough to fix it :)

Since it's a minor change, I'll give this v2 some time on the ML to 
hopefully gather some feedback before I send a v3.

Cheers,
Quentin

[PATCH v2 3/4] arm64: dts: rockchip: force PMIC reset behavior to restart PMU on RK3588 Jaguar

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@cherry.de>

The bootloader for RK3588 Jaguar currently forces the PMIC reset
behavior (stored in RST_FUN bitfield in register SYS_CFG3 of the PMIC)
to 0b1X which is incorrect for our devices.

It is required to restart the PMU as otherwise the companion
microcontroller cannot detect the PMIC (and by extension the full
product and main SoC) being rebooted which is an issue as that is used
to reset a few things like the PWM beeper and watchdogs.

Let's add the new rockchip,reset-mode property to make sure the PMIC
reset behavior is the expected one.

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
 arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts b/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
index ebe77cdd24e803b00fb848dc81258909472290f1..def6af77efaccabe0dd08aaa7959602bfb143607 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
@@ -7,6 +7,7 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/leds/common.h>
+#include <dt-bindings/mfd/rockchip,rk8xx.h>
 #include <dt-bindings/pinctrl/rockchip.h>
 #include <dt-bindings/soc/rockchip,vop2.h>
 #include <dt-bindings/usb/pd.h>
@@ -693,6 +694,7 @@ pmic@0 {
 		vcc13-supply = <&vcc_1v1_nldo_s3>;
 		vcc14-supply = <&vcc_1v1_nldo_s3>;
 		vcca-supply = <&vcc5v0_sys>;
+		rockchip,reset-mode = <RK806_RESTART>;
 
 		rk806_dvs1_null: dvs1-null-pins {
 			pins = "gpio_pwrctrl1";

-- 
2.49.0

[PATCH v2 4/4] arm64: dts: rockchip: force PMIC reset behavior to restart PMU on RK3588 Tiger

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@cherry.de>

The bootloader for RK3588 Tiger currently forces the PMIC reset behavior
(stored in RST_FUN bitfield in register SYS_CFG3 of the PMIC) to 0b1X
which is incorrect for our devices.

It is required to restart the PMU as otherwise the companion
microcontroller cannot detect the PMIC (and by extension the full
product and main SoC) being rebooted which is an issue as that is used
to reset a few things like the PWM beeper and watchdogs.

Let's add the new rockchip,reset-mode property to make sure the PMIC
reset behavior is the expected one.

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
 arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi
index c4933a08dd1e3c92f3e0747135faf97c5eeca906..4c05603abed2e458e406ed45f1bc7cdb57b42478 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi
@@ -5,6 +5,7 @@
 
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/leds/common.h>
+#include <dt-bindings/mfd/rockchip,rk8xx.h>
 #include <dt-bindings/pinctrl/rockchip.h>
 #include "rk3588.dtsi"
 
@@ -440,6 +441,7 @@ pmic@0 {
 		vcc13-supply = <&vcc_1v1_nldo_s3>;
 		vcc14-supply = <&vcc_1v1_nldo_s3>;
 		vcca-supply = <&vcc5v0_sys>;
+		rockchip,reset-mode = <RK806_RESTART>;
 
 		rk806_dvs1_null: dvs1-null-pins {
 			pins = "gpio_pwrctrl1";

-- 
2.49.0