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

[PATCH 0/4] rockchip: rk8xx: allow to customize PMIC reset method 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.

Although RK809 and RK817 also have this feature, this isn't implementing
the feature for them.

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>
---
Quentin Schulz (4):
      dt-bindings: mfd: rk806: allow to customize PMIC reset method
      mfd: rk8xx-core: allow to customize RK806 reset method
      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    | 24 ++++++++++++++++++++++
 arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts     |  1 +
 arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi     |  1 +
 drivers/mfd/rk8xx-core.c                           | 15 ++++++++++++++
 include/linux/mfd/rk808.h                          |  2 ++
 5 files changed, 43 insertions(+)
---
base-commit: 0ff41df1cb268fc69e703a08a57ee14ae967d0ca
change-id: 20250526-rk8xx-rst-fun-f261c40b6d0c

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

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

[Quentin Schulz]

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

The RK806 PMIC (and RK809, RK817; but those aren't handled here) has a
bitfield for configuring the restart/reset behavior (which I assume
Rockchip calls "function") whenever the PMIC is reset (at least by
software; c.f. DEV_RST in the datasheet).

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.

I don't believe this is suitable for a subsystem-generic property hence
let's make it a vendor property called rockchip,rst-fun.

The first few sentences in the description of the property are
voluntarily generic so they could be copied to the DT binding for
RK809/RK817 whenever someone wants to implement that for those PMIC.

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
 .../devicetree/bindings/mfd/rockchip,rk806.yaml    | 24 ++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml b/Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml
index 3c2b06629b75ea94f90712470bf14ed7fc16d68d..0f931a6da93f7596eac89c5f0deb8ee3bd934c31 100644
--- a/Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml
+++ b/Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml
@@ -31,6 +31,30 @@ properties:
 
   system-power-controller: true
 
+  rockchip,rst-fun:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 1, 2, 3]
+    description:
+      RST_FUN value to set for the PMIC.
+
+      This is the value in the RST_FUN bitfield according to the
+      datasheet. I.e. if RST_FUN is bits 6 and 7 and the desired value
+      of RST_FUN is 1, this property needs to be set to 1 (and not 64,
+      0x40, or BIT(6)).
+
+      The meaning of this value is specific to the PMIC and is
+      explained in the datasheet.
+
+      For RK806, the following applies
+
+      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.
+
   vcc1-supply:
     description:
       The input supply for dcdc-reg1.

-- 
2.49.0

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

[Quentin Schulz]

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

The RK806 PMIC (and RK809, RK817; but those aren't handled here) has a
bitfield for configuring the restart/reset behavior (which I assume
Rockchip calls "function") whenever the PMIC is reset (at least by
software; c.f. DEV_RST in the datasheet).

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,rst-fun DT property to pass this information.

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  | 15 +++++++++++++++
 include/linux/mfd/rk808.h |  2 ++
 2 files changed, 17 insertions(+)

diff --git a/drivers/mfd/rk8xx-core.c b/drivers/mfd/rk8xx-core.c
index 71c2b80a4678d627e86cfbec8135f08e262559d3..c59cda7709c01d938870795c55bd1ea2b541b006 100644
--- a/drivers/mfd/rk8xx-core.c
+++ b/drivers/mfd/rk8xx-core.c
@@ -720,12 +720,27 @@ int rk8xx_probe(struct device *dev, int variant, unsigned int irq, struct regmap
 		nr_cells = ARRAY_SIZE(rk805s);
 		break;
 	case RK806_ID:
+		u32 rst_fun;
+
 		rk808->regmap_irq_chip = &rk806_irq_chip;
 		pre_init_reg = rk806_pre_init_reg;
 		nr_pre_init_regs = ARRAY_SIZE(rk806_pre_init_reg);
 		cells = rk806s;
 		nr_cells = ARRAY_SIZE(rk806s);
 		dual_support = IRQF_SHARED;
+
+		ret = device_property_read_u32(dev, "rockchip,rst-fun", &rst_fun);
+		if (ret) {
+			dev_dbg(dev,
+				"rockchip,rst-fun 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

[PATCH 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,rst-fun 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 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts b/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
index 9fceea6c1398e92114dcb735cf2babb7d05d67a5..0f431a9a01c4b93ce7ffcb409dbe06de25cc7edd 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
@@ -685,6 +685,7 @@ pmic@0 {
 		vcc13-supply = <&vcc_1v1_nldo_s3>;
 		vcc14-supply = <&vcc_1v1_nldo_s3>;
 		vcca-supply = <&vcc5v0_sys>;
+		rockchip,rst-fun = <0>;
 
 		rk806_dvs1_null: dvs1-null-pins {
 			pins = "gpio_pwrctrl1";

-- 
2.49.0

[PATCH 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,rst-fun 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 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi
index c4933a08dd1e3c92f3e0747135faf97c5eeca906..1bb6a3ee7579e30721bd5e36554b409c7a88dd04 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi
@@ -440,6 +440,7 @@ pmic@0 {
 		vcc13-supply = <&vcc_1v1_nldo_s3>;
 		vcc14-supply = <&vcc_1v1_nldo_s3>;
 		vcca-supply = <&vcc5v0_sys>;
+		rockchip,rst-fun = <0>;
 
 		rk806_dvs1_null: dvs1-null-pins {
 			pins = "gpio_pwrctrl1";

-- 
2.49.0

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

[kernel test robot]

Hi Quentin,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 0ff41df1cb268fc69e703a08a57ee14ae967d0ca]

url:    https://github.com/intel-lab-lkp/linux/commits/Quentin-Schulz/dt-bindings-mfd-rk806-allow-to-customize-PMIC-reset-method/20250527-011711
base:   0ff41df1cb268fc69e703a08a57ee14ae967d0ca
patch link:    https://lore.kernel.org/r/20250526-rk8xx-rst-fun-v1-2-ea894d9474e0%40cherry.de
patch subject: [PATCH 2/4] mfd: rk8xx-core: allow to customize RK806 reset method
config: arm-randconfig-001-20250527 (https://download.01.org/0day-ci/archive/20250527/202505270807.tc7zzrMf-lkp@intel.com/config)
compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project f819f46284f2a79790038e1f6649172789734ae8)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250527/202505270807.tc7zzrMf-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/202505270807.tc7zzrMf-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/mfd/rk8xx-core.c:723:3: warning: label followed by a declaration is a C23 extension [-Wc23-extensions]
     723 |                 u32 rst_fun;
         |                 ^
   1 warning generated.


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

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

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

[kernel test robot]

Hi Quentin,

kernel test robot noticed the following build errors:

[auto build test ERROR on 0ff41df1cb268fc69e703a08a57ee14ae967d0ca]

url:    https://github.com/intel-lab-lkp/linux/commits/Quentin-Schulz/dt-bindings-mfd-rk806-allow-to-customize-PMIC-reset-method/20250527-011711
base:   0ff41df1cb268fc69e703a08a57ee14ae967d0ca
patch link:    https://lore.kernel.org/r/20250526-rk8xx-rst-fun-v1-2-ea894d9474e0%40cherry.de
patch subject: [PATCH 2/4] mfd: rk8xx-core: allow to customize RK806 reset method
config: arm64-randconfig-003-20250527 (https://download.01.org/0day-ci/archive/20250527/202505271032.U9jloPe6-lkp@intel.com/config)
compiler: clang version 16.0.6 (https://github.com/llvm/llvm-project 7cbf1a2591520c2491aa35339f227775f4d3adf6)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250527/202505271032.U9jloPe6-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/202505271032.U9jloPe6-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/mfd/rk8xx-core.c:723:3: error: expected expression
                   u32 rst_fun;
                   ^
>> drivers/mfd/rk8xx-core.c:732:60: error: use of undeclared identifier 'rst_fun'
                   ret = device_property_read_u32(dev, "rockchip,rst-fun", &rst_fun);
                                                                            ^
   drivers/mfd/rk8xx-core.c:741:37: error: use of undeclared identifier 'rst_fun'
                                            FIELD_PREP(RK806_RST_FUN_MSK, rst_fun));
                                                                          ^
   drivers/mfd/rk8xx-core.c:741:37: error: use of undeclared identifier 'rst_fun'
   drivers/mfd/rk8xx-core.c:741:37: error: use of undeclared identifier 'rst_fun'
>> drivers/mfd/rk8xx-core.c:741:7: error: passing 'void' to parameter of incompatible type 'unsigned int'
                                            FIELD_PREP(RK806_RST_FUN_MSK, rst_fun));
                                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/bitfield.h:114:2: note: expanded from macro 'FIELD_PREP'
           ({                                                              \
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/regmap.h:1312:42: note: passing argument to parameter 'val' here
                                        unsigned int mask, unsigned int val)
                                                                        ^
   6 errors generated.


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

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

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

[Krzysztof Kozlowski]

On 26/05/2025 19:05, Quentin Schulz wrote:
> From: Quentin Schulz <quentin.schulz@cherry.de>
> 
> The RK806 PMIC (and RK809, RK817; but those aren't handled here) has a
> bitfield for configuring the restart/reset behavior (which I assume
> Rockchip calls "function") whenever the PMIC is reset (at least by
> software; c.f. DEV_RST in the datasheet).
> 
> 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.
> 
> I don't believe this is suitable for a subsystem-generic property hence
> let's make it a vendor property called rockchip,rst-fun.
> 
> The first few sentences in the description of the property are
> voluntarily generic so they could be copied to the DT binding for
> RK809/RK817 whenever someone wants to implement that for those PMIC.
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
> ---
>  .../devicetree/bindings/mfd/rockchip,rk806.yaml    | 24 ++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml b/Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml
> index 3c2b06629b75ea94f90712470bf14ed7fc16d68d..0f931a6da93f7596eac89c5f0deb8ee3bd934c31 100644
> --- a/Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml
> +++ b/Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml
> @@ -31,6 +31,30 @@ properties:
>  
>    system-power-controller: true
>  
> +  rockchip,rst-fun:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 1, 2, 3]
> +    description:
> +      RST_FUN value to set for the PMIC.
> +
> +      This is the value in the RST_FUN bitfield according to the
> +      datasheet. I.e. if RST_FUN is bits 6 and 7 and the desired value
> +      of RST_FUN is 1, this property needs to be set to 1 (and not 64,
> +      0x40, or BIT(6)).
> +
> +      The meaning of this value is specific to the PMIC and is
> +      explained in the datasheet.

And why would that be exactly board-level configuration? IOW, I expect
all boards to be reset in the same - correct and optimal - way. Looks
close to SW policy.

> +
> +      For RK806, the following applies
> +
> +      0b00 means "restart PMU"

Use decimal numbers.

> +      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.
> +
>    vcc1-supply:
>      description:
>        The input supply for dcdc-reg1.
> 


Best regards,
Krzysztof

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

[Quentin Schulz]

Hi Krzysztof,

On 5/27/25 10:25 AM, Krzysztof Kozlowski wrote:
> On 26/05/2025 19:05, Quentin Schulz wrote:
>> From: Quentin Schulz <quentin.schulz@cherry.de>
>>
>> The RK806 PMIC (and RK809, RK817; but those aren't handled here) has a
>> bitfield for configuring the restart/reset behavior (which I assume
>> Rockchip calls "function") whenever the PMIC is reset (at least by
>> software; c.f. DEV_RST in the datasheet).
>>
>> 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.
>>
>> I don't believe this is suitable for a subsystem-generic property hence
>> let's make it a vendor property called rockchip,rst-fun.
>>
>> The first few sentences in the description of the property are
>> voluntarily generic so they could be copied to the DT binding for
>> RK809/RK817 whenever someone wants to implement that for those PMIC.
>>
>> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
>> ---
>>   .../devicetree/bindings/mfd/rockchip,rk806.yaml    | 24 ++++++++++++++++++++++
>>   1 file changed, 24 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml b/Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml
>> index 3c2b06629b75ea94f90712470bf14ed7fc16d68d..0f931a6da93f7596eac89c5f0deb8ee3bd934c31 100644
>> --- a/Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml
>> +++ b/Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml
>> @@ -31,6 +31,30 @@ properties:
>>   
>>     system-power-controller: true
>>   
>> +  rockchip,rst-fun:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    enum: [0, 1, 2, 3]
>> +    description:
>> +      RST_FUN value to set for the PMIC.
>> +
>> +      This is the value in the RST_FUN bitfield according to the
>> +      datasheet. I.e. if RST_FUN is bits 6 and 7 and the desired value
>> +      of RST_FUN is 1, this property needs to be set to 1 (and not 64,
>> +      0x40, or BIT(6)).
>> +
>> +      The meaning of this value is specific to the PMIC and is
>> +      explained in the datasheet.
> 
> And why would that be exactly board-level configuration? IOW, I expect
> all boards to be reset in the same - correct and optimal - way. Looks
> close to SW policy.
> 

All RK3588 boards except ours in downstream kernel have their RST_FUN 
set to 1, we need 0 and I cannot talk for what's the actual expected 
behavior for other vendors' boards. I do not feel confident 
indiscriminately changing the PMIC reset behavior for all boards using 
RK806 (which also includes RK3576 boards). Hence why I made that a property.

Additionally, if all boards were "to be reset in the same - correct and 
optimal - way", why would Rockchip even have a register for that in the 
PMIC? It's not an IP they bought (as far as I could tell), so there's 
likely a purpose to it. Especially if they also change the 
silicon-default in their own downstream fork AND provide you with a way 
to change their new default from Device Tree.

We can hardcode the change in the driver without using DT, but I wager 
we're going to see a revert in a few releases because it broke some 
devices. It may break in subtle ways as well, for example our boards 
seem to be working just fine except that because the PMIC doesn't 
entirely reset the power rails, our companion microcontroller doesn't 
detect the reboot.

If it's deemed a SW policy by the DT binding people, is there a way to 
customize this without having it hardcoded to the same value for all 
users of RK806 and without relying on module params?

Cheers,
Quentin

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

[Krzysztof Kozlowski]

On 27/05/2025 10:48, Quentin Schulz wrote:
> Hi Krzysztof,
> 
> On 5/27/25 10:25 AM, Krzysztof Kozlowski wrote:
>> On 26/05/2025 19:05, Quentin Schulz wrote:
>>> From: Quentin Schulz <quentin.schulz@cherry.de>
>>>
>>> The RK806 PMIC (and RK809, RK817; but those aren't handled here) has a
>>> bitfield for configuring the restart/reset behavior (which I assume
>>> Rockchip calls "function") whenever the PMIC is reset (at least by
>>> software; c.f. DEV_RST in the datasheet).
>>>
>>> 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.
>>>
>>> I don't believe this is suitable for a subsystem-generic property hence
>>> let's make it a vendor property called rockchip,rst-fun.
>>>
>>> The first few sentences in the description of the property are
>>> voluntarily generic so they could be copied to the DT binding for
>>> RK809/RK817 whenever someone wants to implement that for those PMIC.
>>>
>>> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
>>> ---
>>>   .../devicetree/bindings/mfd/rockchip,rk806.yaml    | 24 ++++++++++++++++++++++
>>>   1 file changed, 24 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml b/Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml
>>> index 3c2b06629b75ea94f90712470bf14ed7fc16d68d..0f931a6da93f7596eac89c5f0deb8ee3bd934c31 100644
>>> --- a/Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml
>>> +++ b/Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml
>>> @@ -31,6 +31,30 @@ properties:
>>>   
>>>     system-power-controller: true
>>>   
>>> +  rockchip,rst-fun:
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    enum: [0, 1, 2, 3]
>>> +    description:
>>> +      RST_FUN value to set for the PMIC.
>>> +
>>> +      This is the value in the RST_FUN bitfield according to the
>>> +      datasheet. I.e. if RST_FUN is bits 6 and 7 and the desired value
>>> +      of RST_FUN is 1, this property needs to be set to 1 (and not 64,
>>> +      0x40, or BIT(6)).
>>> +
>>> +      The meaning of this value is specific to the PMIC and is
>>> +      explained in the datasheet.
>>
>> And why would that be exactly board-level configuration? IOW, I expect
>> all boards to be reset in the same - correct and optimal - way. Looks
>> close to SW policy.
>>
> 
> All RK3588 boards except ours in downstream kernel have their RST_FUN 
> set to 1, we need 0 and I cannot talk for what's the actual expected 
> behavior for other vendors' boards. I do not feel confident 
> indiscriminately changing the PMIC reset behavior for all boards using 
> RK806 (which also includes RK3576 boards). Hence why I made that a property.
> 
> Additionally, if all boards were "to be reset in the same - correct and 

I don't know if they have to, but that's what I would assume in general.
Unless you say there is some specific hardware aspect of your boards,
but so far you just described the register.

> optimal - way", why would Rockchip even have a register for that in the 
> PMIC? It's not an IP they bought (as far as I could tell), so there's 

To allow SW to make a choice. Just like 1000 other registers for every
other device which we do not add to DT.

> likely a purpose to it. Especially if they also change the 
> silicon-default in their own downstream fork AND provide you with a way 
> to change their new default from Device Tree.
> 
> We can hardcode the change in the driver without using DT, but I wager 
> we're going to see a revert in a few releases because it broke some 
> devices. It may break in subtle ways as well, for example our boards 
> seem to be working just fine except that because the PMIC doesn't 
> entirely reset the power rails, our companion microcontroller doesn't 
> detect the reboot.
> 
> If it's deemed a SW policy by the DT binding people, is there a way to 
> customize this without having it hardcoded to the same value for all 
> users of RK806 and without relying on module params?

sysfs, reboot mode etc. I don't know what is the right here, because you
did not explain the actual hardware issue being fixed here. You only
described that bootloader does something, so you want to write something
else there.


Best regards,
Krzysztof

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

[Quentin Schulz]

Hi Krzysztof,

On 5/27/25 11:08 AM, Krzysztof Kozlowski wrote:
> On 27/05/2025 10:48, Quentin Schulz wrote:
>> Hi Krzysztof,
>>
>> On 5/27/25 10:25 AM, Krzysztof Kozlowski wrote:
>>> On 26/05/2025 19:05, Quentin Schulz wrote:
>>>> From: Quentin Schulz <quentin.schulz@cherry.de>
>>>>
>>>> The RK806 PMIC (and RK809, RK817; but those aren't handled here) has a
>>>> bitfield for configuring the restart/reset behavior (which I assume
>>>> Rockchip calls "function") whenever the PMIC is reset (at least by
>>>> software; c.f. DEV_RST in the datasheet).
>>>>
>>>> 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.
>>>>
>>>> I don't believe this is suitable for a subsystem-generic property hence
>>>> let's make it a vendor property called rockchip,rst-fun.
>>>>
>>>> The first few sentences in the description of the property are
>>>> voluntarily generic so they could be copied to the DT binding for
>>>> RK809/RK817 whenever someone wants to implement that for those PMIC.
>>>>
>>>> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
>>>> ---
>>>>    .../devicetree/bindings/mfd/rockchip,rk806.yaml    | 24 ++++++++++++++++++++++
>>>>    1 file changed, 24 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml b/Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml
>>>> index 3c2b06629b75ea94f90712470bf14ed7fc16d68d..0f931a6da93f7596eac89c5f0deb8ee3bd934c31 100644
>>>> --- a/Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml
>>>> +++ b/Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml
>>>> @@ -31,6 +31,30 @@ properties:
>>>>    
>>>>      system-power-controller: true
>>>>    
>>>> +  rockchip,rst-fun:
>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>>> +    enum: [0, 1, 2, 3]
>>>> +    description:
>>>> +      RST_FUN value to set for the PMIC.
>>>> +
>>>> +      This is the value in the RST_FUN bitfield according to the
>>>> +      datasheet. I.e. if RST_FUN is bits 6 and 7 and the desired value
>>>> +      of RST_FUN is 1, this property needs to be set to 1 (and not 64,
>>>> +      0x40, or BIT(6)).
>>>> +
>>>> +      The meaning of this value is specific to the PMIC and is
>>>> +      explained in the datasheet.
>>>
>>> And why would that be exactly board-level configuration? IOW, I expect
>>> all boards to be reset in the same - correct and optimal - way. Looks
>>> close to SW policy.
>>>
>>
>> All RK3588 boards except ours in downstream kernel have their RST_FUN
>> set to 1, we need 0 and I cannot talk for what's the actual expected
>> behavior for other vendors' boards. I do not feel confident
>> indiscriminately changing the PMIC reset behavior for all boards using
>> RK806 (which also includes RK3576 boards). Hence why I made that a property.
>>
>> Additionally, if all boards were "to be reset in the same - correct and
> 
> I don't know if they have to, but that's what I would assume in general.
> Unless you say there is some specific hardware aspect of your boards,
> but so far you just described the register.
> 

The cover letter

>> optimal - way", why would Rockchip even have a register for that in the
>> PMIC? It's not an IP they bought (as far as I could tell), so there's
> 
> To allow SW to make a choice. Just like 1000 other registers for every
> other device which we do not add to DT.
> 
>> likely a purpose to it. Especially if they also change the
>> silicon-default in their own downstream fork AND provide you with a way
>> to change their new default from Device Tree.
>>
>> We can hardcode the change in the driver without using DT, but I wager
>> we're going to see a revert in a few releases because it broke some
>> devices. It may break in subtle ways as well, for example our boards
>> seem to be working just fine except that because the PMIC doesn't
>> entirely reset the power rails, our companion microcontroller doesn't
>> detect the reboot.
>>
>> If it's deemed a SW policy by the DT binding people, is there a way to
>> customize this without having it hardcoded to the same value for all
>> users of RK806 and without relying on module params?
> 
> sysfs, reboot mode etc. I don't know what is the right here, because you
> did not explain the actual hardware issue being fixed here. You only
> described that bootloader does something, so you want to write something
> else there.
> 

We have a companion microcontroller on the PCB of both products which 
needs to detect if the board was reset. When the board is reset, the MCU 
FW does a few things, like essentially resetting its internal logic such 
as the PWM controller (so the beeper stops beeping), watchdogs and 
reinit most user-exposed registers so that it's like "fresh" out of 
reset (even though it actually wasn't reset since it's continuously 
powered, not from the PMIC).

To detect a reboot, it senses one of the power rails (DCDC8; vcc_3v3_s3 
on our boards) from the PMIC. This power rail is only "restarted" when 
RST_FUN is set to 0 ("restart PMU" mode) according to our experiments.

I assume it is possible other boards do not want this (all?) power rail 
to be quickly interrupted when rebooting? But that I do not know.

Cheers,
Quentin

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

[Krzysztof Kozlowski]

On 27/05/2025 11:26, Quentin Schulz wrote:
> Hi Krzysztof,
> 
> On 5/27/25 11:08 AM, Krzysztof Kozlowski wrote:
>> On 27/05/2025 10:48, Quentin Schulz wrote:
>>> Hi Krzysztof,
>>>
>>> On 5/27/25 10:25 AM, Krzysztof Kozlowski wrote:
>>>> On 26/05/2025 19:05, Quentin Schulz wrote:
>>>>> From: Quentin Schulz <quentin.schulz@cherry.de>
>>>>>
>>>>> The RK806 PMIC (and RK809, RK817; but those aren't handled here) has a
>>>>> bitfield for configuring the restart/reset behavior (which I assume
>>>>> Rockchip calls "function") whenever the PMIC is reset (at least by
>>>>> software; c.f. DEV_RST in the datasheet).
>>>>>
>>>>> 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.
>>>>>
>>>>> I don't believe this is suitable for a subsystem-generic property hence
>>>>> let's make it a vendor property called rockchip,rst-fun.
>>>>>
>>>>> The first few sentences in the description of the property are
>>>>> voluntarily generic so they could be copied to the DT binding for
>>>>> RK809/RK817 whenever someone wants to implement that for those PMIC.
>>>>>
>>>>> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
>>>>> ---
>>>>>    .../devicetree/bindings/mfd/rockchip,rk806.yaml    | 24 ++++++++++++++++++++++
>>>>>    1 file changed, 24 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml b/Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml
>>>>> index 3c2b06629b75ea94f90712470bf14ed7fc16d68d..0f931a6da93f7596eac89c5f0deb8ee3bd934c31 100644
>>>>> --- a/Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml
>>>>> +++ b/Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml
>>>>> @@ -31,6 +31,30 @@ properties:
>>>>>    
>>>>>      system-power-controller: true
>>>>>    
>>>>> +  rockchip,rst-fun:
>>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>>>> +    enum: [0, 1, 2, 3]
>>>>> +    description:
>>>>> +      RST_FUN value to set for the PMIC.
>>>>> +
>>>>> +      This is the value in the RST_FUN bitfield according to the
>>>>> +      datasheet. I.e. if RST_FUN is bits 6 and 7 and the desired value
>>>>> +      of RST_FUN is 1, this property needs to be set to 1 (and not 64,
>>>>> +      0x40, or BIT(6)).
>>>>> +
>>>>> +      The meaning of this value is specific to the PMIC and is
>>>>> +      explained in the datasheet.
>>>>
>>>> And why would that be exactly board-level configuration? IOW, I expect
>>>> all boards to be reset in the same - correct and optimal - way. Looks
>>>> close to SW policy.
>>>>
>>>
>>> All RK3588 boards except ours in downstream kernel have their RST_FUN
>>> set to 1, we need 0 and I cannot talk for what's the actual expected
>>> behavior for other vendors' boards. I do not feel confident
>>> indiscriminately changing the PMIC reset behavior for all boards using
>>> RK806 (which also includes RK3576 boards). Hence why I made that a property.
>>>
>>> Additionally, if all boards were "to be reset in the same - correct and
>>
>> I don't know if they have to, but that's what I would assume in general.
>> Unless you say there is some specific hardware aspect of your boards,
>> but so far you just described the register.
>>
> 
> The cover letter

We do not read cover letters, except when looking for changelogs.

This patch must stand on its own.

> 
>>> optimal - way", why would Rockchip even have a register for that in the
>>> PMIC? It's not an IP they bought (as far as I could tell), so there's
>>
>> To allow SW to make a choice. Just like 1000 other registers for every
>> other device which we do not add to DT.
>>
>>> likely a purpose to it. Especially if they also change the
>>> silicon-default in their own downstream fork AND provide you with a way
>>> to change their new default from Device Tree.
>>>
>>> We can hardcode the change in the driver without using DT, but I wager
>>> we're going to see a revert in a few releases because it broke some
>>> devices. It may break in subtle ways as well, for example our boards
>>> seem to be working just fine except that because the PMIC doesn't
>>> entirely reset the power rails, our companion microcontroller doesn't
>>> detect the reboot.
>>>
>>> If it's deemed a SW policy by the DT binding people, is there a way to
>>> customize this without having it hardcoded to the same value for all
>>> users of RK806 and without relying on module params?
>>
>> sysfs, reboot mode etc. I don't know what is the right here, because you
>> did not explain the actual hardware issue being fixed here. You only
>> described that bootloader does something, so you want to write something
>> else there.
>>
> 
> We have a companion microcontroller on the PCB of both products which 
> needs to detect if the board was reset. When the board is reset, the MCU 
> FW does a few things, like essentially resetting its internal logic such 
> as the PWM controller (so the beeper stops beeping), watchdogs and 
> reinit most user-exposed registers so that it's like "fresh" out of 
> reset (even though it actually wasn't reset since it's continuously 
> powered, not from the PMIC).

So you miss some wiring to the MCU?

> 
> To detect a reboot, it senses one of the power rails (DCDC8; vcc_3v3_s3 
> on our boards) from the PMIC. This power rail is only "restarted" when 
> RST_FUN is set to 0 ("restart PMU" mode) according to our experiments.
> 
> I assume it is possible other boards do not want this (all?) power rail 
> to be quickly interrupted when rebooting? But that I do not know.

Maybe that's your hardware characteristic which you want to encode in
DT. Don't focus on registers, focus on how the hardware or entire system
is done or different.

Best regards,
Krzysztof

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

[Nicolas Frattaroli]

Hi Quentin,

On Tuesday, 27 May 2025 11:26:49 Central European Summer Time Quentin Schulz wrote:
> Hi Krzysztof,
> 
> On 5/27/25 11:08 AM, Krzysztof Kozlowski wrote:
> > On 27/05/2025 10:48, Quentin Schulz wrote:
> [...]
> >>
> >> likely a purpose to it. Especially if they also change the
> >> silicon-default in their own downstream fork AND provide you with a way
> >> to change their new default from Device Tree.
> >>
> >> We can hardcode the change in the driver without using DT, but I wager
> >> we're going to see a revert in a few releases because it broke some
> >> devices. It may break in subtle ways as well, for example our boards
> >> seem to be working just fine except that because the PMIC doesn't
> >> entirely reset the power rails, our companion microcontroller doesn't
> >> detect the reboot.
> >>
> >> If it's deemed a SW policy by the DT binding people, is there a way to
> >> customize this without having it hardcoded to the same value for all
> >> users of RK806 and without relying on module params?
> > 
> > sysfs, reboot mode etc. I don't know what is the right here, because you
> > did not explain the actual hardware issue being fixed here. You only
> > described that bootloader does something, so you want to write something
> > else there.
> > 
> 
> We have a companion microcontroller on the PCB of both products which 
> needs to detect if the board was reset. When the board is reset, the MCU 
> FW does a few things, like essentially resetting its internal logic such 
> as the PWM controller (so the beeper stops beeping), watchdogs and 
> reinit most user-exposed registers so that it's like "fresh" out of 
> reset (even though it actually wasn't reset since it's continuously 
> powered, not from the PMIC).
> 
> To detect a reboot, it senses one of the power rails (DCDC8; vcc_3v3_s3 
> on our boards) from the PMIC. This power rail is only "restarted" when 
> RST_FUN is set to 0 ("restart PMU" mode) according to our experiments.
> 
> I assume it is possible other boards do not want this (all?) power rail 
> to be quickly interrupted when rebooting? But that I do not know.

I agree that this sounds like a pretty big change in behavior, yes. I am
somewhat suspicious of any silent mainline difference from silicon defaults
as being the result of cargo-culting from downstream hacks to make things
work, and are unresolved technical debt in need of cleanup.

On the RK3576 board I'm currently working on, where an RK806 is used as
well, then DCDC8 cutting out would wreak havoc on warm reboots I'd wager
as it's used for a lot of 1.8V IO voltage supply things, including one
instance where the DCDC8 rail going low would feed into a downstream
regulator that's being kept enabled as long as a different supply is on.

If you don't want to deal with DT bindings people (sysfs for reset
behaviour? What?) a workaround for this could be to add the necessary
register write to your bootloader's (probably u-boot?) board init code.

I do think however that "what does this board hardware expect to happen to
power rails on reset" is a pretty strongly board specific non-enumerable
hardware difference that belongs in DT as a declarative property, but
perhaps in a different form than the bare register contents for this, so
that it can hopefully be used as a more generic (even if vendor) property
for future PMICs going forward. Think regulator-always-on but for this
specific case.

> 
> Cheers,
> Quentin
> 

Kind regards,
Nicolas Frattaroli

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

[Rob Herring]

On Tue, May 27, 2025 at 01:18:20PM +0200, Nicolas Frattaroli wrote:
> Hi Quentin,
> 
> On Tuesday, 27 May 2025 11:26:49 Central European Summer Time Quentin Schulz wrote:
> > Hi Krzysztof,
> > 
> > On 5/27/25 11:08 AM, Krzysztof Kozlowski wrote:
> > > On 27/05/2025 10:48, Quentin Schulz wrote:
> > [...]
> > >>
> > >> likely a purpose to it. Especially if they also change the
> > >> silicon-default in their own downstream fork AND provide you with a way
> > >> to change their new default from Device Tree.
> > >>
> > >> We can hardcode the change in the driver without using DT, but I wager
> > >> we're going to see a revert in a few releases because it broke some
> > >> devices. It may break in subtle ways as well, for example our boards
> > >> seem to be working just fine except that because the PMIC doesn't
> > >> entirely reset the power rails, our companion microcontroller doesn't
> > >> detect the reboot.
> > >>
> > >> If it's deemed a SW policy by the DT binding people, is there a way to
> > >> customize this without having it hardcoded to the same value for all
> > >> users of RK806 and without relying on module params?
> > > 
> > > sysfs, reboot mode etc. I don't know what is the right here, because you
> > > did not explain the actual hardware issue being fixed here. You only
> > > described that bootloader does something, so you want to write something
> > > else there.
> > > 
> > 
> > We have a companion microcontroller on the PCB of both products which 
> > needs to detect if the board was reset. When the board is reset, the MCU 
> > FW does a few things, like essentially resetting its internal logic such 
> > as the PWM controller (so the beeper stops beeping), watchdogs and 
> > reinit most user-exposed registers so that it's like "fresh" out of 
> > reset (even though it actually wasn't reset since it's continuously 
> > powered, not from the PMIC).
> > 
> > To detect a reboot, it senses one of the power rails (DCDC8; vcc_3v3_s3 
> > on our boards) from the PMIC. This power rail is only "restarted" when 
> > RST_FUN is set to 0 ("restart PMU" mode) according to our experiments.
> > 
> > I assume it is possible other boards do not want this (all?) power rail 
> > to be quickly interrupted when rebooting? But that I do not know.
> 
> I agree that this sounds like a pretty big change in behavior, yes. I am
> somewhat suspicious of any silent mainline difference from silicon defaults
> as being the result of cargo-culting from downstream hacks to make things
> work, and are unresolved technical debt in need of cleanup.
> 
> On the RK3576 board I'm currently working on, where an RK806 is used as
> well, then DCDC8 cutting out would wreak havoc on warm reboots I'd wager
> as it's used for a lot of 1.8V IO voltage supply things, including one
> instance where the DCDC8 rail going low would feed into a downstream
> regulator that's being kept enabled as long as a different supply is on.
> 
> If you don't want to deal with DT bindings people (sysfs for reset
> behaviour? What?) a workaround for this could be to add the necessary
> register write to your bootloader's (probably u-boot?) board init code.
> 
> I do think however that "what does this board hardware expect to happen to
> power rails on reset" is a pretty strongly board specific non-enumerable
> hardware difference that belongs in DT as a declarative property, but
> perhaps in a different form than the bare register contents for this, so
> that it can hopefully be used as a more generic (even if vendor) property
> for future PMICs going forward. Think regulator-always-on but for this
> specific case.

I don't have an issue with this being in DT as it would seem to me to 
be based on how the board is wired. However, how does reset work before 
you run something that parses the DT? Seems to me it needs to be 
initialized early (or power on in the right state).

Rob

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

[Quentin Schulz]

Hi Rob,

On 6/5/25 9:41 PM, Rob Herring wrote:
> On Tue, May 27, 2025 at 01:18:20PM +0200, Nicolas Frattaroli wrote:
>> Hi Quentin,
>>
>> On Tuesday, 27 May 2025 11:26:49 Central European Summer Time Quentin Schulz wrote:
>>> Hi Krzysztof,
>>>
>>> On 5/27/25 11:08 AM, Krzysztof Kozlowski wrote:
>>>> On 27/05/2025 10:48, Quentin Schulz wrote:
>>> [...]
>>>>>
>>>>> likely a purpose to it. Especially if they also change the
>>>>> silicon-default in their own downstream fork AND provide you with a way
>>>>> to change their new default from Device Tree.
>>>>>
>>>>> We can hardcode the change in the driver without using DT, but I wager
>>>>> we're going to see a revert in a few releases because it broke some
>>>>> devices. It may break in subtle ways as well, for example our boards
>>>>> seem to be working just fine except that because the PMIC doesn't
>>>>> entirely reset the power rails, our companion microcontroller doesn't
>>>>> detect the reboot.
>>>>>
>>>>> If it's deemed a SW policy by the DT binding people, is there a way to
>>>>> customize this without having it hardcoded to the same value for all
>>>>> users of RK806 and without relying on module params?
>>>>
>>>> sysfs, reboot mode etc. I don't know what is the right here, because you
>>>> did not explain the actual hardware issue being fixed here. You only
>>>> described that bootloader does something, so you want to write something
>>>> else there.
>>>>
>>>
>>> We have a companion microcontroller on the PCB of both products which
>>> needs to detect if the board was reset. When the board is reset, the MCU
>>> FW does a few things, like essentially resetting its internal logic such
>>> as the PWM controller (so the beeper stops beeping), watchdogs and
>>> reinit most user-exposed registers so that it's like "fresh" out of
>>> reset (even though it actually wasn't reset since it's continuously
>>> powered, not from the PMIC).
>>>
>>> To detect a reboot, it senses one of the power rails (DCDC8; vcc_3v3_s3
>>> on our boards) from the PMIC. This power rail is only "restarted" when
>>> RST_FUN is set to 0 ("restart PMU" mode) according to our experiments.
>>>
>>> I assume it is possible other boards do not want this (all?) power rail
>>> to be quickly interrupted when rebooting? But that I do not know.
>>
>> I agree that this sounds like a pretty big change in behavior, yes. I am
>> somewhat suspicious of any silent mainline difference from silicon defaults
>> as being the result of cargo-culting from downstream hacks to make things
>> work, and are unresolved technical debt in need of cleanup.
>>
>> On the RK3576 board I'm currently working on, where an RK806 is used as
>> well, then DCDC8 cutting out would wreak havoc on warm reboots I'd wager
>> as it's used for a lot of 1.8V IO voltage supply things, including one
>> instance where the DCDC8 rail going low would feed into a downstream
>> regulator that's being kept enabled as long as a different supply is on.
>>
>> If you don't want to deal with DT bindings people (sysfs for reset
>> behaviour? What?) a workaround for this could be to add the necessary
>> register write to your bootloader's (probably u-boot?) board init code.
>>
>> I do think however that "what does this board hardware expect to happen to
>> power rails on reset" is a pretty strongly board specific non-enumerable
>> hardware difference that belongs in DT as a declarative property, but
>> perhaps in a different form than the bare register contents for this, so
>> that it can hopefully be used as a more generic (even if vendor) property
>> for future PMICs going forward. Think regulator-always-on but for this
>> specific case.
> 
> I don't have an issue with this being in DT as it would seem to me to
> be based on how the board is wired. However, how does reset work before
> you run something that parses the DT? Seems to me it needs to be
> initialized early (or power on in the right state).
> 

FYI, v2 sent yesterday: 
https://lore.kernel.org/linux-rockchip/20250605-rk8xx-rst-fun-v2-0-143d190596dd@cherry.de/T/#t

I don't mind the discussion continuing on v1 though.

The silicon default according to the datasheet is 0b00 (restart PMU). I 
know that Allwinner/X-Powers PMIC/regulators sometimes have different 
silicon versions or EEPROMs or bootstrapping pins that can modify the 
default but I am not aware of such a thing on the RK806.
So silicon default (0b00) is what's used until something changes it 
programmatically....

which, unfortunately due to some U-Boot contributor's (me) mistake, is 
set to 0b11 (soon 0b10 but that won't change anything), aka reset + 
notify on RESETB line. I cannot change the default as boards now 
technically rely on this (either by mistake (they could then now fix 
this by using the DT property) or intently (in which case they wouldn't 
change anything)).

The plan is to have this in the official DT so that U-Boot can read it 
and pick that mode instead of the current default of 0b10/0b11.

If one wants to do that without DT support in some U-Boot phase they 
would have to change it by hand in their board file or some other way 
(e.g. via a custom TF-A/OP-TEE if they want though that doesn't make a 
lot of sense :) ).

Why support this also in the kernel?
One, the DT is not kernel specific so I would like it there even if used 
only in U-Boot since it now is using DT based on upstream Linux kernel's 
(via devicetree-rebasing git repo) for most Rockchip boards/SoCs.
Two, because I cannot force users to patch their U-Boot that contains my 
mistake, so at least the behavior would be fine after booting the kernel.

Cheers,
Quentin

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

[Lee Jones]

On Mon, 26 May 2025, Quentin Schulz wrote:

> From: Quentin Schulz <quentin.schulz@cherry.de>
> 
> The RK806 PMIC (and RK809, RK817; but those aren't handled here) has a
> bitfield for configuring the restart/reset behavior (which I assume
> Rockchip calls "function") whenever the PMIC is reset (at least by
> software; c.f. DEV_RST in the datasheet).
> 
> 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,rst-fun DT property to pass this information.
> 
> 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  | 15 +++++++++++++++
>  include/linux/mfd/rk808.h |  2 ++
>  2 files changed, 17 insertions(+)

The test robots seem unhappy with this.  Please fix and resubmit.

-- 
Lee Jones [李琼斯]