devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFCv1 0/8] RK3399 clean shutdown issue
@ 2019-12-06 18:45 Anand Moon
  2019-12-06 18:45 ` [RFCv1 1/8] mfd: rk808: Refactor shutdown functions Anand Moon
                   ` (9 more replies)
  0 siblings, 10 replies; 21+ messages in thread
From: Anand Moon @ 2019-12-06 18:45 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Heiko Stuebner, Jagan Teki,
	Manivannan Sadhasivam, Robin Murphy, Daniel Schultz
  Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel

Most of the RK3399 SBC boards do not perform clean
shutdown and clean reboot.

These patches try to help resolve the issue with proper
shutdown by turning off the PMIC.

For reference 
RK805 PMCI data sheet:
[0] http://rockchip.fr/RK805%20datasheet%20V1.3.pdf
RK808 PMIC data sheet:
[1] http://rockchip.fr/RK808%20datasheet%20V1.4.pdf
RK817 PMIC data sheet:
[2] http://rockchip.fr/RK817%20datasheet%20V1.01.pdf 
RK818 PMIC data sheet:
[3] http://rockchip.fr/RK818%20datasheet%20V1.0.pdf

Reboot issue:
My guess is that we need to some proper sequence of
setting to PMCI to perform clean.

If you have any input please share them.

Tested on SBC
Rock960 Model A
Odroid N1
Rock64

-Anand Moon

Anand Moon (8):
  mfd: rk808: Refactor shutdown functions
  mfd: rk808: use syscore for RK805 PMIC shutdown
  mfd: rk808: use syscore for RK808 PMIC shutdown
  mfd: rk808: use syscore for RK818 PMIC shutdown
  mfd: rk808: cleanup unused function pointer
  mfd: rk808: use common syscore for all PMCI for clean shutdown
  arm64: rockchip: drop unused field from rk8xx i2c node
  arm: rockchip: drop unused field from rk8xx i2c node

 arch/arm/boot/dts/rk3036-kylin.dts            |   1 -
 arch/arm/boot/dts/rk3188-px3-evb.dts          |   1 -
 arch/arm/boot/dts/rk3288-evb-rk808.dts        |   1 -
 arch/arm/boot/dts/rk3288-phycore-som.dtsi     |   1 -
 arch/arm/boot/dts/rk3288-popmetal.dts         |   1 -
 arch/arm/boot/dts/rk3288-tinker.dtsi          |   1 -
 arch/arm/boot/dts/rk3288-veyron.dtsi          |   1 -
 arch/arm/boot/dts/rk3288-vyasa.dts            |   1 -
 arch/arm/boot/dts/rv1108-elgin-r1.dts         |   1 -
 arch/arm/boot/dts/rv1108-evb.dts              |   1 -
 arch/arm64/boot/dts/rockchip/px30-evb.dts     |   1 -
 arch/arm64/boot/dts/rockchip/rk3328-a1.dts    |   1 -
 arch/arm64/boot/dts/rockchip/rk3328-evb.dts   |   1 -
 .../arm64/boot/dts/rockchip/rk3328-roc-cc.dts |   1 -
 .../arm64/boot/dts/rockchip/rk3328-rock64.dts |   1 -
 .../boot/dts/rockchip/rk3368-geekbox.dts      |   1 -
 arch/arm64/boot/dts/rockchip/rk3368-lion.dtsi |   1 -
 .../boot/dts/rockchip/rk3368-px5-evb.dts      |   1 -
 .../boot/dts/rockchip/rk3399-firefly.dts      |   1 -
 .../boot/dts/rockchip/rk3399-hugsun-x99.dts   |   1 -
 .../boot/dts/rockchip/rk3399-khadas-edge.dtsi |   1 -
 .../boot/dts/rockchip/rk3399-leez-p710.dts    |   1 -
 .../boot/dts/rockchip/rk3399-nanopi4.dtsi     |   1 -
 .../boot/dts/rockchip/rk3399-orangepi.dts     |   1 -
 arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi |   1 -
 .../boot/dts/rockchip/rk3399-roc-pc.dtsi      |   1 -
 .../boot/dts/rockchip/rk3399-rock-pi-4.dts    |   1 -
 .../boot/dts/rockchip/rk3399-rock960.dtsi     |   1 -
 .../boot/dts/rockchip/rk3399-rockpro64.dts    |   1 -
 .../boot/dts/rockchip/rk3399-sapphire.dtsi    |   1 -
 drivers/mfd/rk808.c                           | 144 +++++-------------
 include/linux/mfd/rk808.h                     |   2 -
 32 files changed, 42 insertions(+), 134 deletions(-)

-- 
2.24.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [RFCv1 1/8] mfd: rk808: Refactor shutdown functions
  2019-12-06 18:45 [RFCv1 0/8] RK3399 clean shutdown issue Anand Moon
@ 2019-12-06 18:45 ` Anand Moon
  2019-12-16 11:11   ` Lee Jones
  2019-12-06 18:45 ` [RFCv1 2/8] mfd: rk808: use syscore for RK805 PMIC shutdown Anand Moon
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Anand Moon @ 2019-12-06 18:45 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Heiko Stuebner, Jagan Teki,
	Manivannan Sadhasivam, Robin Murphy, Daniel Schultz
  Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel

From: Daniel Schultz <d.schultz@phytec.de>

Since all shutdown functions have almost the same code, all logic
from the shutdown functions can be refactored to a new function
"rk808_update_bits", which can update a register by a given address
and bitmask and value.

link: https://lore.kernel.org/patchwork/patch/937404/
Cc: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Daniel Schultz <d.schultz@phytec.de>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
[rebased on latest kernel]
Modified the API to set the value.
This changes were submited with below patch.
[0] https://lore.kernel.org/patchwork/patch/937404/
---
 drivers/mfd/rk808.c | 87 ++++++++++++++-------------------------------
 1 file changed, 26 insertions(+), 61 deletions(-)

diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
index a69a6742ecdc..e637f5bcc8bb 100644
--- a/drivers/mfd/rk808.c
+++ b/drivers/mfd/rk808.c
@@ -449,81 +449,52 @@ static const struct regmap_irq_chip rk818_irq_chip = {
 
 static struct i2c_client *rk808_i2c_client;
 
-static void rk805_device_shutdown(void)
+static void rk808_update_bits(unsigned int reg, unsigned int mask,
+		unsigned int value)
 {
-	int ret;
 	struct rk808 *rk808 = i2c_get_clientdata(rk808_i2c_client);
+	int ret;
 
-	if (!rk808)
+	if (!rk808) {
+		dev_warn(&rk808_i2c_client->dev,
+			"have no rk805/rk808/rk817/rk818, so do nothing here\n");
 		return;
+	}
 
-	ret = regmap_update_bits(rk808->regmap,
-				 RK805_DEV_CTRL_REG,
-				 DEV_OFF, DEV_OFF);
+	ret = regmap_update_bits(rk808->regmap,	reg, mask, value);
 	if (ret)
-		dev_err(&rk808_i2c_client->dev, "Failed to shutdown device!\n");
+		dev_err(&rk808_i2c_client->dev,
+			"can't write to register 0x%x: %x!\n", reg, ret);
 }
 
-static void rk805_device_shutdown_prepare(void)
+static void rk805_device_shutdown(void)
 {
-	int ret;
-	struct rk808 *rk808 = i2c_get_clientdata(rk808_i2c_client);
-
-	if (!rk808)
-		return;
+	rk808_update_bits(RK805_DEV_CTRL_REG, DEV_OFF, DEV_OFF);
+}
 
-	ret = regmap_update_bits(rk808->regmap,
-				 RK805_GPIO_IO_POL_REG,
-				 SLP_SD_MSK, SHUTDOWN_FUN);
-	if (ret)
-		dev_err(&rk808_i2c_client->dev, "Failed to shutdown device!\n");
+static void rk805_device_shutdown_prepare(void)
+{
+	rk808_update_bits(RK805_GPIO_IO_POL_REG, SLP_SD_MSK, SHUTDOWN_FUN);
 }
 
 static void rk808_device_shutdown(void)
 {
-	int ret;
-	struct rk808 *rk808 = i2c_get_clientdata(rk808_i2c_client);
-
-	if (!rk808)
-		return;
-
-	ret = regmap_update_bits(rk808->regmap,
-				 RK808_DEVCTRL_REG,
-				 DEV_OFF_RST, DEV_OFF_RST);
-	if (ret)
-		dev_err(&rk808_i2c_client->dev, "Failed to shutdown device!\n");
+	rk808_update_bits(RK808_DEVCTRL_REG, DEV_OFF_RST, DEV_OFF_RST);
 }
 
 static void rk818_device_shutdown(void)
 {
-	int ret;
-	struct rk808 *rk808 = i2c_get_clientdata(rk808_i2c_client);
-
-	if (!rk808)
-		return;
-
-	ret = regmap_update_bits(rk808->regmap,
-				 RK818_DEVCTRL_REG,
-				 DEV_OFF, DEV_OFF);
-	if (ret)
-		dev_err(&rk808_i2c_client->dev, "Failed to shutdown device!\n");
+	rk808_update_bits(RK818_DEVCTRL_REG, DEV_OFF, DEV_OFF);
 }
 
 static void rk8xx_syscore_shutdown(void)
 {
 	struct rk808 *rk808 = i2c_get_clientdata(rk808_i2c_client);
-	int ret;
 
 	if (system_state == SYSTEM_POWER_OFF &&
 	    (rk808->variant == RK809_ID || rk808->variant == RK817_ID)) {
-		ret = regmap_update_bits(rk808->regmap,
-					 RK817_SYS_CFG(3),
-					 RK817_SLPPIN_FUNC_MSK,
-					 SLPPIN_DN_FUN);
-		if (ret) {
-			dev_warn(&rk808_i2c_client->dev,
-				 "Cannot switch to power down function\n");
-		}
+		rk808_update_bits(RK817_SYS_CFG(3), RK817_SLPPIN_FUNC_MSK,
+				SLPPIN_DN_FUN);
 	}
 }
 
@@ -720,41 +691,35 @@ static int rk808_remove(struct i2c_client *client)
 static int __maybe_unused rk8xx_suspend(struct device *dev)
 {
 	struct rk808 *rk808 = i2c_get_clientdata(rk808_i2c_client);
-	int ret = 0;
 
 	switch (rk808->variant) {
 	case RK809_ID:
 	case RK817_ID:
-		ret = regmap_update_bits(rk808->regmap,
-					 RK817_SYS_CFG(3),
-					 RK817_SLPPIN_FUNC_MSK,
-					 SLPPIN_SLP_FUN);
+		rk808_update_bits(RK817_SYS_CFG(3), RK817_SLPPIN_FUNC_MSK,
+				SLPPIN_SLP_FUN);
 		break;
 	default:
 		break;
 	}
 
-	return ret;
+	return 0;
 }
 
 static int __maybe_unused rk8xx_resume(struct device *dev)
 {
 	struct rk808 *rk808 = i2c_get_clientdata(rk808_i2c_client);
-	int ret = 0;
 
 	switch (rk808->variant) {
 	case RK809_ID:
 	case RK817_ID:
-		ret = regmap_update_bits(rk808->regmap,
-					 RK817_SYS_CFG(3),
-					 RK817_SLPPIN_FUNC_MSK,
-					 SLPPIN_NULL_FUN);
+		rk808_update_bits(RK817_SYS_CFG(3), RK817_SLPPIN_FUNC_MSK,
+				SLPPIN_NULL_FUN);
 		break;
 	default:
 		break;
 	}
 
-	return ret;
+	return 0;
 }
 static SIMPLE_DEV_PM_OPS(rk8xx_pm_ops, rk8xx_suspend, rk8xx_resume);
 
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [RFCv1 2/8] mfd: rk808: use syscore for RK805 PMIC shutdown
  2019-12-06 18:45 [RFCv1 0/8] RK3399 clean shutdown issue Anand Moon
  2019-12-06 18:45 ` [RFCv1 1/8] mfd: rk808: Refactor shutdown functions Anand Moon
@ 2019-12-06 18:45 ` Anand Moon
  2019-12-09 13:34   ` Robin Murphy
  2019-12-06 18:45 ` [RFCv1 3/8] mfd: rk808: use syscore for RK808 " Anand Moon
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Anand Moon @ 2019-12-06 18:45 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Heiko Stuebner, Jagan Teki,
	Manivannan Sadhasivam, Robin Murphy, Daniel Schultz
  Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel

Use common syscore_shutdown for RK805 PMIC to do
clean I2C shutdown, drop the unused pm_pwroff_prep_fn
and pm_pwroff_fn function pointers.

Cc: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 drivers/mfd/rk808.c | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
index e637f5bcc8bb..713d989064ba 100644
--- a/drivers/mfd/rk808.c
+++ b/drivers/mfd/rk808.c
@@ -467,16 +467,6 @@ static void rk808_update_bits(unsigned int reg, unsigned int mask,
 			"can't write to register 0x%x: %x!\n", reg, ret);
 }
 
-static void rk805_device_shutdown(void)
-{
-	rk808_update_bits(RK805_DEV_CTRL_REG, DEV_OFF, DEV_OFF);
-}
-
-static void rk805_device_shutdown_prepare(void)
-{
-	rk808_update_bits(RK805_GPIO_IO_POL_REG, SLP_SD_MSK, SHUTDOWN_FUN);
-}
-
 static void rk808_device_shutdown(void)
 {
 	rk808_update_bits(RK808_DEVCTRL_REG, DEV_OFF_RST, DEV_OFF_RST);
@@ -491,10 +481,23 @@ static void rk8xx_syscore_shutdown(void)
 {
 	struct rk808 *rk808 = i2c_get_clientdata(rk808_i2c_client);
 
-	if (system_state == SYSTEM_POWER_OFF &&
-	    (rk808->variant == RK809_ID || rk808->variant == RK817_ID)) {
-		rk808_update_bits(RK817_SYS_CFG(3), RK817_SLPPIN_FUNC_MSK,
-				SLPPIN_DN_FUN);
+	if (system_state == SYSTEM_POWER_OFF) {
+		dev_info(&rk808_i2c_client->dev, "System Shutdown Event\n");
+
+		switch (rk808->variant) {
+		case RK805_ID:
+			rk808_update_bits(RK805_GPIO_IO_POL_REG,
+					SLP_SD_MSK, SHUTDOWN_FUN);
+			rk808_update_bits(RK805_DEV_CTRL_REG, DEV_OFF, DEV_OFF);
+			break;
+		case RK809_ID:
+		case RK817_ID:
+			rk808_update_bits(RK817_SYS_CFG(3),
+					RK817_SLPPIN_FUNC_MSK, SLPPIN_DN_FUN);
+			break;
+		default:
+			break;
+		}
 	}
 }
 
@@ -565,8 +568,6 @@ static int rk808_probe(struct i2c_client *client,
 		nr_pre_init_regs = ARRAY_SIZE(rk805_pre_init_reg);
 		cells = rk805s;
 		nr_cells = ARRAY_SIZE(rk805s);
-		rk808->pm_pwroff_fn = rk805_device_shutdown;
-		rk808->pm_pwroff_prep_fn = rk805_device_shutdown_prepare;
 		break;
 	case RK808_ID:
 		rk808->regmap_cfg = &rk808_regmap_config;
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [RFCv1 3/8] mfd: rk808: use syscore for RK808 PMIC shutdown
  2019-12-06 18:45 [RFCv1 0/8] RK3399 clean shutdown issue Anand Moon
  2019-12-06 18:45 ` [RFCv1 1/8] mfd: rk808: Refactor shutdown functions Anand Moon
  2019-12-06 18:45 ` [RFCv1 2/8] mfd: rk808: use syscore for RK805 PMIC shutdown Anand Moon
@ 2019-12-06 18:45 ` Anand Moon
  2019-12-06 18:45 ` [RFCv1 4/8] mfd: rk808: use syscore for RK818 " Anand Moon
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Anand Moon @ 2019-12-06 18:45 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Heiko Stuebner, Jagan Teki,
	Manivannan Sadhasivam, Robin Murphy, Daniel Schultz
  Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel

Use common syscore_shutdown for RK808 PMIC to do
clean I2C shutdown, drop the unused pm_pwroff_fn
function pointers.

Cc: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 drivers/mfd/rk808.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
index 713d989064ba..0a098fbdf112 100644
--- a/drivers/mfd/rk808.c
+++ b/drivers/mfd/rk808.c
@@ -467,11 +467,6 @@ static void rk808_update_bits(unsigned int reg, unsigned int mask,
 			"can't write to register 0x%x: %x!\n", reg, ret);
 }
 
-static void rk808_device_shutdown(void)
-{
-	rk808_update_bits(RK808_DEVCTRL_REG, DEV_OFF_RST, DEV_OFF_RST);
-}
-
 static void rk818_device_shutdown(void)
 {
 	rk808_update_bits(RK818_DEVCTRL_REG, DEV_OFF, DEV_OFF);
@@ -490,6 +485,10 @@ static void rk8xx_syscore_shutdown(void)
 					SLP_SD_MSK, SHUTDOWN_FUN);
 			rk808_update_bits(RK805_DEV_CTRL_REG, DEV_OFF, DEV_OFF);
 			break;
+		case RK808_ID:
+			rk808_update_bits(RK808_DEVCTRL_REG,
+					DEV_OFF_RST, DEV_OFF_RST);
+			break;
 		case RK809_ID:
 		case RK817_ID:
 			rk808_update_bits(RK817_SYS_CFG(3),
@@ -576,7 +575,6 @@ static int rk808_probe(struct i2c_client *client,
 		nr_pre_init_regs = ARRAY_SIZE(rk808_pre_init_reg);
 		cells = rk808s;
 		nr_cells = ARRAY_SIZE(rk808s);
-		rk808->pm_pwroff_fn = rk808_device_shutdown;
 		break;
 	case RK818_ID:
 		rk808->regmap_cfg = &rk818_regmap_config;
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [RFCv1 4/8] mfd: rk808: use syscore for RK818 PMIC shutdown
  2019-12-06 18:45 [RFCv1 0/8] RK3399 clean shutdown issue Anand Moon
                   ` (2 preceding siblings ...)
  2019-12-06 18:45 ` [RFCv1 3/8] mfd: rk808: use syscore for RK808 " Anand Moon
@ 2019-12-06 18:45 ` Anand Moon
  2019-12-06 18:45 ` [RFCv1 5/8] mfd: rk808: cleanup unused function pointer Anand Moon
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Anand Moon @ 2019-12-06 18:45 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Heiko Stuebner, Jagan Teki,
	Manivannan Sadhasivam, Robin Murphy, Daniel Schultz
  Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel

Use common syscore_shutdown for RK818 PMIC to do
clean I2C shutdown, drop the unused pm_pwroff_fn
function pointers.

Cc: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 drivers/mfd/rk808.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
index 0a098fbdf112..4b3b90dad4f8 100644
--- a/drivers/mfd/rk808.c
+++ b/drivers/mfd/rk808.c
@@ -467,11 +467,6 @@ static void rk808_update_bits(unsigned int reg, unsigned int mask,
 			"can't write to register 0x%x: %x!\n", reg, ret);
 }
 
-static void rk818_device_shutdown(void)
-{
-	rk808_update_bits(RK818_DEVCTRL_REG, DEV_OFF, DEV_OFF);
-}
-
 static void rk8xx_syscore_shutdown(void)
 {
 	struct rk808 *rk808 = i2c_get_clientdata(rk808_i2c_client);
@@ -494,6 +489,9 @@ static void rk8xx_syscore_shutdown(void)
 			rk808_update_bits(RK817_SYS_CFG(3),
 					RK817_SLPPIN_FUNC_MSK, SLPPIN_DN_FUN);
 			break;
+		case RK818_ID:
+			rk808_update_bits(RK818_DEVCTRL_REG, DEV_OFF, DEV_OFF);
+			break;
 		default:
 			break;
 		}
@@ -583,7 +581,6 @@ static int rk808_probe(struct i2c_client *client,
 		nr_pre_init_regs = ARRAY_SIZE(rk818_pre_init_reg);
 		cells = rk818s;
 		nr_cells = ARRAY_SIZE(rk818s);
-		rk808->pm_pwroff_fn = rk818_device_shutdown;
 		break;
 	case RK809_ID:
 	case RK817_ID:
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [RFCv1 5/8] mfd: rk808: cleanup unused function pointer
  2019-12-06 18:45 [RFCv1 0/8] RK3399 clean shutdown issue Anand Moon
                   ` (3 preceding siblings ...)
  2019-12-06 18:45 ` [RFCv1 4/8] mfd: rk808: use syscore for RK818 " Anand Moon
@ 2019-12-06 18:45 ` Anand Moon
  2019-12-06 18:45 ` [RFCv1 6/8] mfd: rk808: use common syscore for all PMCI for clean shutdown Anand Moon
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Anand Moon @ 2019-12-06 18:45 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Heiko Stuebner, Jagan Teki,
	Manivannan Sadhasivam, Robin Murphy, Daniel Schultz
  Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel

Cleanup unused pm_poweroff_fn and pm_pwroff_prep_fn function,
drop the unused rockchip,system-power-controller dts binding.

Cc: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 drivers/mfd/rk808.c       | 28 ++--------------------------
 include/linux/mfd/rk808.h |  2 --
 2 files changed, 2 insertions(+), 28 deletions(-)

diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
index 4b3b90dad4f8..9a7be379946a 100644
--- a/drivers/mfd/rk808.c
+++ b/drivers/mfd/rk808.c
@@ -521,7 +521,7 @@ static int rk808_probe(struct i2c_client *client,
 	const struct mfd_cell *cells;
 	int nr_pre_init_regs;
 	int nr_cells;
-	int pm_off = 0, msb, lsb;
+	int msb, lsb;
 	unsigned char pmic_id_msb, pmic_id_lsb;
 	int ret;
 	int i;
@@ -641,18 +641,8 @@ static int rk808_probe(struct i2c_client *client,
 		goto err_irq;
 	}
 
-	pm_off = of_property_read_bool(np,
-				"rockchip,system-power-controller");
-	if (pm_off && !pm_power_off) {
+	if (!rk808_i2c_client)
 		rk808_i2c_client = client;
-		pm_power_off = rk808->pm_pwroff_fn;
-	}
-
-	if (pm_off && !pm_power_off_prepare) {
-		if (!rk808_i2c_client)
-			rk808_i2c_client = client;
-		pm_power_off_prepare = rk808->pm_pwroff_prep_fn;
-	}
 
 	return 0;
 
@@ -667,20 +657,6 @@ static int rk808_remove(struct i2c_client *client)
 
 	regmap_del_irq_chip(client->irq, rk808->irq_data);
 
-	/**
-	 * pm_power_off may points to a function from another module.
-	 * Check if the pointer is set by us and only then overwrite it.
-	 */
-	if (rk808->pm_pwroff_fn && pm_power_off == rk808->pm_pwroff_fn)
-		pm_power_off = NULL;
-
-	/**
-	 * As above, check if the pointer is set by us before overwrite.
-	 */
-	if (rk808->pm_pwroff_prep_fn &&
-	    pm_power_off_prepare == rk808->pm_pwroff_prep_fn)
-		pm_power_off_prepare = NULL;
-
 	return 0;
 }
 
diff --git a/include/linux/mfd/rk808.h b/include/linux/mfd/rk808.h
index a59bf323f713..e07f6e61cd38 100644
--- a/include/linux/mfd/rk808.h
+++ b/include/linux/mfd/rk808.h
@@ -620,7 +620,5 @@ struct rk808 {
 	long				variant;
 	const struct regmap_config	*regmap_cfg;
 	const struct regmap_irq_chip	*regmap_irq_chip;
-	void				(*pm_pwroff_fn)(void);
-	void				(*pm_pwroff_prep_fn)(void);
 };
 #endif /* __LINUX_REGULATOR_RK808_H */
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [RFCv1 6/8] mfd: rk808: use common syscore for all PMCI for clean shutdown
  2019-12-06 18:45 [RFCv1 0/8] RK3399 clean shutdown issue Anand Moon
                   ` (4 preceding siblings ...)
  2019-12-06 18:45 ` [RFCv1 5/8] mfd: rk808: cleanup unused function pointer Anand Moon
@ 2019-12-06 18:45 ` Anand Moon
  2019-12-06 18:45 ` [RFCv1 7/8] arm64: rockchip: drop unused field from rk8xx i2c node Anand Moon
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Anand Moon @ 2019-12-06 18:45 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Heiko Stuebner, Jagan Teki,
	Manivannan Sadhasivam, Robin Murphy, Daniel Schultz
  Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel

Move the register_syscore_ops to common location for all PMIC.
Add missing unregister_syscore_ops in rk808_remove function.

Cc: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 drivers/mfd/rk808.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
index 9a7be379946a..54f3999e4948 100644
--- a/drivers/mfd/rk808.c
+++ b/drivers/mfd/rk808.c
@@ -590,7 +590,6 @@ static int rk808_probe(struct i2c_client *client,
 		nr_pre_init_regs = ARRAY_SIZE(rk817_pre_init_reg);
 		cells = rk817s;
 		nr_cells = ARRAY_SIZE(rk817s);
-		register_syscore_ops(&rk808_syscore_ops);
 		break;
 	default:
 		dev_err(&client->dev, "Unsupported RK8XX ID %lu\n",
@@ -644,6 +643,8 @@ static int rk808_probe(struct i2c_client *client,
 	if (!rk808_i2c_client)
 		rk808_i2c_client = client;
 
+	register_syscore_ops(&rk808_syscore_ops);
+
 	return 0;
 
 err_irq:
@@ -657,6 +658,8 @@ static int rk808_remove(struct i2c_client *client)
 
 	regmap_del_irq_chip(client->irq, rk808->irq_data);
 
+	unregister_syscore_ops(&rk808_syscore_ops);
+
 	return 0;
 }
 
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [RFCv1 7/8] arm64: rockchip: drop unused field from rk8xx i2c node
  2019-12-06 18:45 [RFCv1 0/8] RK3399 clean shutdown issue Anand Moon
                   ` (5 preceding siblings ...)
  2019-12-06 18:45 ` [RFCv1 6/8] mfd: rk808: use common syscore for all PMCI for clean shutdown Anand Moon
@ 2019-12-06 18:45 ` Anand Moon
  2019-12-06 18:45 ` [RFCv1 8/8] arm: " Anand Moon
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Anand Moon @ 2019-12-06 18:45 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Heiko Stuebner, Jagan Teki,
	Manivannan Sadhasivam, Robin Murphy, Daniel Schultz
  Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel

Drop unused rockchip,system-power-controller from rk8xx
i2c0 node.

Cc: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 arch/arm64/boot/dts/rockchip/px30-evb.dts            | 1 -
 arch/arm64/boot/dts/rockchip/rk3328-a1.dts           | 1 -
 arch/arm64/boot/dts/rockchip/rk3328-evb.dts          | 1 -
 arch/arm64/boot/dts/rockchip/rk3328-roc-cc.dts       | 1 -
 arch/arm64/boot/dts/rockchip/rk3328-rock64.dts       | 1 -
 arch/arm64/boot/dts/rockchip/rk3368-geekbox.dts      | 1 -
 arch/arm64/boot/dts/rockchip/rk3368-lion.dtsi        | 1 -
 arch/arm64/boot/dts/rockchip/rk3368-px5-evb.dts      | 1 -
 arch/arm64/boot/dts/rockchip/rk3399-firefly.dts      | 1 -
 arch/arm64/boot/dts/rockchip/rk3399-hugsun-x99.dts   | 1 -
 arch/arm64/boot/dts/rockchip/rk3399-khadas-edge.dtsi | 1 -
 arch/arm64/boot/dts/rockchip/rk3399-leez-p710.dts    | 1 -
 arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi     | 1 -
 arch/arm64/boot/dts/rockchip/rk3399-orangepi.dts     | 1 -
 arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi        | 1 -
 arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi      | 1 -
 arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dts    | 1 -
 arch/arm64/boot/dts/rockchip/rk3399-rock960.dtsi     | 1 -
 arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts    | 1 -
 arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi    | 1 -
 20 files changed, 20 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/px30-evb.dts b/arch/arm64/boot/dts/rockchip/px30-evb.dts
index 936ed7d71ffc..6f51b5f1b17a 100644
--- a/arch/arm64/boot/dts/rockchip/px30-evb.dts
+++ b/arch/arm64/boot/dts/rockchip/px30-evb.dts
@@ -142,7 +142,6 @@ rk809: pmic@20 {
 		interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&pmic_int>;
-		rockchip,system-power-controller;
 		wakeup-source;
 		#clock-cells = <0>;
 		clock-output-names = "xin32k";
diff --git a/arch/arm64/boot/dts/rockchip/rk3328-a1.dts b/arch/arm64/boot/dts/rockchip/rk3328-a1.dts
index 76b49f573101..97fdf654fe7e 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328-a1.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3328-a1.dts
@@ -151,7 +151,6 @@ pmic@18 {
 		interrupts = <RK_PA6 IRQ_TYPE_LEVEL_LOW>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&pmic_int_l>;
-		rockchip,system-power-controller;
 		wakeup-source;
 
 		vcc1-supply = <&vcc_sys>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3328-evb.dts b/arch/arm64/boot/dts/rockchip/rk3328-evb.dts
index 49c4b96da3d4..f1db204e4e0c 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328-evb.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3328-evb.dts
@@ -103,7 +103,6 @@ rk805: rk805@18 {
 		#gpio-cells = <2>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&pmic_int_l>;
-		rockchip,system-power-controller;
 		wakeup-source;
 
 		vcc1-supply = <&vcc_sys>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3328-roc-cc.dts b/arch/arm64/boot/dts/rockchip/rk3328-roc-cc.dts
index 8d553c92182a..27318c1a57be 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328-roc-cc.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3328-roc-cc.dts
@@ -175,7 +175,6 @@ rk805: pmic@18 {
 		#gpio-cells = <2>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&pmic_int_l>;
-		rockchip,system-power-controller;
 		wakeup-source;
 
 		vcc1-supply = <&vcc_sys>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
index 62936b432f9a..ccb0baa2eae9 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
@@ -180,7 +180,6 @@ rk805: rk805@18 {
 		#gpio-cells = <2>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&pmic_int_l>;
-		rockchip,system-power-controller;
 		wakeup-source;
 
 		vcc1-supply = <&vcc_sys>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3368-geekbox.dts b/arch/arm64/boot/dts/rockchip/rk3368-geekbox.dts
index 1d0778ff217c..af3ac89156fd 100644
--- a/arch/arm64/boot/dts/rockchip/rk3368-geekbox.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3368-geekbox.dts
@@ -108,7 +108,6 @@ rk808: pmic@1b {
 		pinctrl-0 = <&pmic_int>, <&pmic_sleep>;
 		interrupt-parent = <&gpio0>;
 		interrupts = <RK_PA5 IRQ_TYPE_LEVEL_LOW>;
-		rockchip,system-power-controller;
 		vcc1-supply = <&vcc_sys>;
 		vcc2-supply = <&vcc_sys>;
 		vcc3-supply = <&vcc_sys>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3368-lion.dtsi b/arch/arm64/boot/dts/rockchip/rk3368-lion.dtsi
index e17311e09082..9d5d674735ea 100644
--- a/arch/arm64/boot/dts/rockchip/rk3368-lion.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3368-lion.dtsi
@@ -174,7 +174,6 @@ rk808: pmic@1b {
 		#clock-cells = <1>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&pmic_int_l>, <&pmic_sleep>;
-		rockchip,system-power-controller;
 		vcc1-supply = <&vcc_sys>;
 		vcc2-supply = <&vcc_sys>;
 		vcc3-supply = <&vcc_sys>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3368-px5-evb.dts b/arch/arm64/boot/dts/rockchip/rk3368-px5-evb.dts
index 231db0305a03..5c154b20d6d4 100644
--- a/arch/arm64/boot/dts/rockchip/rk3368-px5-evb.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3368-px5-evb.dts
@@ -68,7 +68,6 @@ rk808: pmic@1b {
 		interrupts = <RK_PA5 IRQ_TYPE_LEVEL_LOW>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&pmic_int>, <&pmic_sleep>;
-		rockchip,system-power-controller;
 		vcc1-supply = <&vcc_sys>;
 		vcc2-supply = <&vcc_sys>;
 		vcc3-supply = <&vcc_sys>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-firefly.dts b/arch/arm64/boot/dts/rockchip/rk3399-firefly.dts
index c706db0ee9ec..3df29a7c5091 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-firefly.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-firefly.dts
@@ -278,7 +278,6 @@ rk808: pmic@1b {
 		clock-output-names = "xin32k", "rk808-clkout2";
 		pinctrl-names = "default";
 		pinctrl-0 = <&pmic_int_l>;
-		rockchip,system-power-controller;
 		wakeup-source;
 
 		vcc1-supply = <&vcc_sys>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-hugsun-x99.dts b/arch/arm64/boot/dts/rockchip/rk3399-hugsun-x99.dts
index c133e8d64b2a..174b4e34f23e 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-hugsun-x99.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-hugsun-x99.dts
@@ -225,7 +225,6 @@ rk808: pmic@1b {
 		interrupts = <21 IRQ_TYPE_LEVEL_LOW>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&pmic_int_l>;
-		rockchip,system-power-controller;
 		wakeup-source;
 		#clock-cells = <1>;
 		clock-output-names = "xin32k", "rtc_clko_wifi";
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-khadas-edge.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-khadas-edge.dtsi
index 4944d78a0a1c..f638d00adfd9 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-khadas-edge.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-khadas-edge.dtsi
@@ -301,7 +301,6 @@ rk808: pmic@1b {
 		clock-output-names = "xin32k", "rk808-clkout2";
 		pinctrl-names = "default";
 		pinctrl-0 = <&pmic_int_l>;
-		rockchip,system-power-controller;
 		wakeup-source;
 
 		vcc1-supply = <&vsys_3v3>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-leez-p710.dts b/arch/arm64/boot/dts/rockchip/rk3399-leez-p710.dts
index 73be38a53796..5e5ed71d29cd 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-leez-p710.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-leez-p710.dts
@@ -180,7 +180,6 @@ rk808: pmic@1b {
 		clock-output-names = "xin32k", "rk808-clkout2";
 		pinctrl-names = "default";
 		pinctrl-0 = <&pmic_int_l>;
-		rockchip,system-power-controller;
 		wakeup-source;
 
 		vcc1-supply = <&vcc5v0_sys>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
index b788ae4f47f0..f9f25a663d98 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
@@ -241,7 +241,6 @@ rk808: pmic@1b {
 		interrupts = <21 IRQ_TYPE_LEVEL_LOW>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&pmic_int_l>;
-		rockchip,system-power-controller;
 		wakeup-source;
 
 		vcc1-supply = <&vcc3v3_sys>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-orangepi.dts b/arch/arm64/boot/dts/rockchip/rk3399-orangepi.dts
index 0541dfce924d..f04a755416dd 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-orangepi.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-orangepi.dts
@@ -241,7 +241,6 @@ rk808: pmic@1b {
 		clock-output-names = "rtc_clko_soc", "rtc_clko_wifi";
 		pinctrl-names = "default";
 		pinctrl-0 = <&pmic_int_l>;
-		rockchip,system-power-controller;
 		wakeup-source;
 
 		vcc1-supply = <&vcc3v3_sys>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi
index c1edca3872c7..1f1998b80e7f 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi
@@ -185,7 +185,6 @@ rk808: pmic@1b {
 		clock-output-names = "xin32k", "rk808-clkout2";
 		pinctrl-names = "default";
 		pinctrl-0 = <&pmic_int_l>;
-		rockchip,system-power-controller;
 		wakeup-source;
 
 		vcc1-supply = <&vcc5v0_sys>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
index 7e07dae33d0f..0fabebd7244c 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
@@ -260,7 +260,6 @@ rk808: pmic@1b {
 		clock-output-names = "xin32k", "rk808-clkout2";
 		pinctrl-names = "default";
 		pinctrl-0 = <&pmic_int_l>;
-		rockchip,system-power-controller;
 		wakeup-source;
 
 		vcc1-supply = <&vcc3v3_sys>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dts b/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dts
index 188d9dfc297b..3dbbb19b29bc 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dts
@@ -197,7 +197,6 @@ rk808: pmic@1b {
 		clock-output-names = "xin32k", "rk808-clkout2";
 		pinctrl-names = "default";
 		pinctrl-0 = <&pmic_int_l>;
-		rockchip,system-power-controller;
 		wakeup-source;
 
 		vcc1-supply = <&vcc5v0_sys>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rock960.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-rock960.dtsi
index c7d48d41e184..a6592b22bbb4 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rock960.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rock960.dtsi
@@ -169,7 +169,6 @@ rk808: pmic@1b {
 		interrupts = <21 IRQ_TYPE_LEVEL_LOW>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&pmic_int_l>;
-		rockchip,system-power-controller;
 		wakeup-source;
 		#clock-cells = <1>;
 		clock-output-names = "xin32k", "rk808-clkout2";
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
index 7f4b2eba31d4..2b6379c18153 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
@@ -259,7 +259,6 @@ rk808: pmic@1b {
 		clock-output-names = "xin32k", "rk808-clkout2";
 		pinctrl-names = "default";
 		pinctrl-0 = <&pmic_int_l>;
-		rockchip,system-power-controller;
 		wakeup-source;
 
 		vcc1-supply = <&vcc5v0_sys>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi
index 1bc1579674e5..66aa905766e2 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi
@@ -228,7 +228,6 @@ rk808: pmic@1b {
 		clock-output-names = "xin32k", "rk808-clkout2";
 		pinctrl-names = "default";
 		pinctrl-0 = <&pmic_int_l>;
-		rockchip,system-power-controller;
 		wakeup-source;
 
 		vcc1-supply = <&vcc_sys>;
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [RFCv1 8/8] arm: rockchip: drop unused field from rk8xx i2c node
  2019-12-06 18:45 [RFCv1 0/8] RK3399 clean shutdown issue Anand Moon
                   ` (6 preceding siblings ...)
  2019-12-06 18:45 ` [RFCv1 7/8] arm64: rockchip: drop unused field from rk8xx i2c node Anand Moon
@ 2019-12-06 18:45 ` Anand Moon
  2019-12-06 22:32 ` [RFCv1 0/8] RK3399 clean shutdown issue Heiko Stuebner
  2019-12-09 13:29 ` Robin Murphy
  9 siblings, 0 replies; 21+ messages in thread
From: Anand Moon @ 2019-12-06 18:45 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Heiko Stuebner, Jagan Teki,
	Manivannan Sadhasivam, Robin Murphy, Daniel Schultz
  Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel

Drop unused rockchip,system-power-controller from rk8xx
i2c node.

Cc: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 arch/arm/boot/dts/rk3036-kylin.dts        | 1 -
 arch/arm/boot/dts/rk3188-px3-evb.dts      | 1 -
 arch/arm/boot/dts/rk3288-evb-rk808.dts    | 1 -
 arch/arm/boot/dts/rk3288-phycore-som.dtsi | 1 -
 arch/arm/boot/dts/rk3288-popmetal.dts     | 1 -
 arch/arm/boot/dts/rk3288-tinker.dtsi      | 1 -
 arch/arm/boot/dts/rk3288-veyron.dtsi      | 1 -
 arch/arm/boot/dts/rk3288-vyasa.dts        | 1 -
 arch/arm/boot/dts/rv1108-elgin-r1.dts     | 1 -
 arch/arm/boot/dts/rv1108-evb.dts          | 1 -
 10 files changed, 10 deletions(-)

diff --git a/arch/arm/boot/dts/rk3036-kylin.dts b/arch/arm/boot/dts/rk3036-kylin.dts
index fb3cf005cc90..0a7290ab718d 100644
--- a/arch/arm/boot/dts/rk3036-kylin.dts
+++ b/arch/arm/boot/dts/rk3036-kylin.dts
@@ -118,7 +118,6 @@ rk808: pmic@1b {
 		interrupts = <RK_PA2 IRQ_TYPE_LEVEL_LOW>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&pmic_int &global_pwroff>;
-		rockchip,system-power-controller;
 		wakeup-source;
 		#clock-cells = <1>;
 		clock-output-names = "xin32k", "rk808-clkout2";
diff --git a/arch/arm/boot/dts/rk3188-px3-evb.dts b/arch/arm/boot/dts/rk3188-px3-evb.dts
index c32e1d441cf7..334fa510995c 100644
--- a/arch/arm/boot/dts/rk3188-px3-evb.dts
+++ b/arch/arm/boot/dts/rk3188-px3-evb.dts
@@ -88,7 +88,6 @@ rk808: pmic@1c {
 		reg = <0x1c>;
 		interrupt-parent = <&gpio0>;
 		interrupts = <RK_PB3 IRQ_TYPE_LEVEL_LOW>;
-		rockchip,system-power-controller;
 		wakeup-source;
 		#clock-cells = <1>;
 		clock-output-names = "xin32k", "rk808-clkout2";
diff --git a/arch/arm/boot/dts/rk3288-evb-rk808.dts b/arch/arm/boot/dts/rk3288-evb-rk808.dts
index 16788209625b..4b280c4b4850 100644
--- a/arch/arm/boot/dts/rk3288-evb-rk808.dts
+++ b/arch/arm/boot/dts/rk3288-evb-rk808.dts
@@ -17,7 +17,6 @@ rk808: pmic@1b {
 		interrupts = <RK_PA4 IRQ_TYPE_LEVEL_LOW>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&pmic_int &global_pwroff>;
-		rockchip,system-power-controller;
 		wakeup-source;
 		#clock-cells = <1>;
 		clock-output-names = "xin32k", "rk808-clkout2";
diff --git a/arch/arm/boot/dts/rk3288-phycore-som.dtsi b/arch/arm/boot/dts/rk3288-phycore-som.dtsi
index 77a47b9b756d..2076566a2c97 100644
--- a/arch/arm/boot/dts/rk3288-phycore-som.dtsi
+++ b/arch/arm/boot/dts/rk3288-phycore-som.dtsi
@@ -148,7 +148,6 @@ rk818: pmic@1c {
 		interrupts = <4 IRQ_TYPE_LEVEL_LOW>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&pmic_int>;
-		rockchip,system-power-controller;
 		wakeup-source;
 		#clock-cells = <1>;
 
diff --git a/arch/arm/boot/dts/rk3288-popmetal.dts b/arch/arm/boot/dts/rk3288-popmetal.dts
index 6a51940398b5..251668fee14d 100644
--- a/arch/arm/boot/dts/rk3288-popmetal.dts
+++ b/arch/arm/boot/dts/rk3288-popmetal.dts
@@ -168,7 +168,6 @@ rk808: pmic@1b {
 		interrupts = <RK_PA4 IRQ_TYPE_LEVEL_LOW>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&pmic_int &global_pwroff>;
-		rockchip,system-power-controller;
 		wakeup-source;
 		#clock-cells = <1>;
 		clock-output-names = "xin32k", "rk808-clkout2";
diff --git a/arch/arm/boot/dts/rk3288-tinker.dtsi b/arch/arm/boot/dts/rk3288-tinker.dtsi
index 0aeef23ca3db..15fc1caca852 100644
--- a/arch/arm/boot/dts/rk3288-tinker.dtsi
+++ b/arch/arm/boot/dts/rk3288-tinker.dtsi
@@ -154,7 +154,6 @@ rk808: pmic@1b {
 				<&gpio0 12 GPIO_ACTIVE_HIGH>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&pmic_int &global_pwroff &dvs_1 &dvs_2>;
-		rockchip,system-power-controller;
 		wakeup-source;
 
 		vcc1-supply = <&vcc_sys>;
diff --git a/arch/arm/boot/dts/rk3288-veyron.dtsi b/arch/arm/boot/dts/rk3288-veyron.dtsi
index 7525e3dd1fc1..d7663ebc798f 100644
--- a/arch/arm/boot/dts/rk3288-veyron.dtsi
+++ b/arch/arm/boot/dts/rk3288-veyron.dtsi
@@ -221,7 +221,6 @@ rk808: pmic@1b {
 		interrupts = <RK_PA4 IRQ_TYPE_LEVEL_LOW>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&pmic_int_l>;
-		rockchip,system-power-controller;
 		wakeup-source;
 		#clock-cells = <1>;
 
diff --git a/arch/arm/boot/dts/rk3288-vyasa.dts b/arch/arm/boot/dts/rk3288-vyasa.dts
index ba06e9f97ddc..98e48ecb473e 100644
--- a/arch/arm/boot/dts/rk3288-vyasa.dts
+++ b/arch/arm/boot/dts/rk3288-vyasa.dts
@@ -167,7 +167,6 @@ rk808: pmic@1b {
 		clock-output-names = "xin32k", "rk808-clkout2";
 		pinctrl-names = "default";
 		pinctrl-0 = <&pmic_int &global_pwroff>;
-		rockchip,system-power-controller;
 		wakeup-source;
 
 		vcc1-supply = <&vcc_sys>;
diff --git a/arch/arm/boot/dts/rv1108-elgin-r1.dts b/arch/arm/boot/dts/rv1108-elgin-r1.dts
index b1db924710c8..39acc472774d 100644
--- a/arch/arm/boot/dts/rv1108-elgin-r1.dts
+++ b/arch/arm/boot/dts/rv1108-elgin-r1.dts
@@ -67,7 +67,6 @@ rk805: pmic@18 {
 		reg = <0x18>;
 		interrupt-parent = <&gpio0>;
 		interrupts = <RK_PB4 IRQ_TYPE_LEVEL_LOW>;
-		rockchip,system-power-controller;
 
 		vcc1-supply = <&vcc_sys>;
 		vcc2-supply = <&vcc_sys>;
diff --git a/arch/arm/boot/dts/rv1108-evb.dts b/arch/arm/boot/dts/rv1108-evb.dts
index 30f3d0470ad9..e21817237792 100644
--- a/arch/arm/boot/dts/rv1108-evb.dts
+++ b/arch/arm/boot/dts/rv1108-evb.dts
@@ -80,7 +80,6 @@ rk805: pmic@18 {
 		reg = <0x18>;
 		interrupt-parent = <&gpio0>;
 		interrupts = <RK_PB4 IRQ_TYPE_LEVEL_LOW>;
-		rockchip,system-power-controller;
 
 		vcc1-supply = <&vcc_sys>;
 		vcc2-supply = <&vcc_sys>;
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [RFCv1 0/8] RK3399 clean shutdown issue
  2019-12-06 18:45 [RFCv1 0/8] RK3399 clean shutdown issue Anand Moon
                   ` (7 preceding siblings ...)
  2019-12-06 18:45 ` [RFCv1 8/8] arm: " Anand Moon
@ 2019-12-06 22:32 ` Heiko Stuebner
  2019-12-07  5:07   ` Anand Moon
  2019-12-09 13:29 ` Robin Murphy
  9 siblings, 1 reply; 21+ messages in thread
From: Heiko Stuebner @ 2019-12-06 22:32 UTC (permalink / raw)
  To: Anand Moon
  Cc: Rob Herring, Mark Rutland, Jagan Teki, Manivannan Sadhasivam,
	Robin Murphy, Daniel Schultz, devicetree, linux-arm-kernel,
	linux-rockchip, linux-kernel

Hi Anand,

Am Freitag, 6. Dezember 2019, 19:45:28 CET schrieb Anand Moon:
> Most of the RK3399 SBC boards do not perform clean
> shutdown and clean reboot.
> 
> These patches try to help resolve the issue with proper
> shutdown by turning off the PMIC.
> 
> For reference 
> RK805 PMCI data sheet:
> [0] http://rockchip.fr/RK805%20datasheet%20V1.3.pdf
> RK808 PMIC data sheet:
> [1] http://rockchip.fr/RK808%20datasheet%20V1.4.pdf
> RK817 PMIC data sheet:
> [2] http://rockchip.fr/RK817%20datasheet%20V1.01.pdf 
> RK818 PMIC data sheet:
> [3] http://rockchip.fr/RK818%20datasheet%20V1.0.pdf
> 
> Reboot issue:
> My guess is that we need to some proper sequence of
> setting to PMCI to perform clean.
> 
> If you have any input please share them.

The rk8xx pmics may not on all devices be responsible for powering down
the device. That is what the system-power-controller dt-property is for.

So that property is there for a reason - to indicate that the pmic is
responsible for power-off-handling.

Heiko

> Tested on SBC
> Rock960 Model A
> Odroid N1
> Rock64
> 
> -Anand Moon
> 
> Anand Moon (8):
>   mfd: rk808: Refactor shutdown functions
>   mfd: rk808: use syscore for RK805 PMIC shutdown
>   mfd: rk808: use syscore for RK808 PMIC shutdown
>   mfd: rk808: use syscore for RK818 PMIC shutdown
>   mfd: rk808: cleanup unused function pointer
>   mfd: rk808: use common syscore for all PMCI for clean shutdown
>   arm64: rockchip: drop unused field from rk8xx i2c node
>   arm: rockchip: drop unused field from rk8xx i2c node
> 
>  arch/arm/boot/dts/rk3036-kylin.dts            |   1 -
>  arch/arm/boot/dts/rk3188-px3-evb.dts          |   1 -
>  arch/arm/boot/dts/rk3288-evb-rk808.dts        |   1 -
>  arch/arm/boot/dts/rk3288-phycore-som.dtsi     |   1 -
>  arch/arm/boot/dts/rk3288-popmetal.dts         |   1 -
>  arch/arm/boot/dts/rk3288-tinker.dtsi          |   1 -
>  arch/arm/boot/dts/rk3288-veyron.dtsi          |   1 -
>  arch/arm/boot/dts/rk3288-vyasa.dts            |   1 -
>  arch/arm/boot/dts/rv1108-elgin-r1.dts         |   1 -
>  arch/arm/boot/dts/rv1108-evb.dts              |   1 -
>  arch/arm64/boot/dts/rockchip/px30-evb.dts     |   1 -
>  arch/arm64/boot/dts/rockchip/rk3328-a1.dts    |   1 -
>  arch/arm64/boot/dts/rockchip/rk3328-evb.dts   |   1 -
>  .../arm64/boot/dts/rockchip/rk3328-roc-cc.dts |   1 -
>  .../arm64/boot/dts/rockchip/rk3328-rock64.dts |   1 -
>  .../boot/dts/rockchip/rk3368-geekbox.dts      |   1 -
>  arch/arm64/boot/dts/rockchip/rk3368-lion.dtsi |   1 -
>  .../boot/dts/rockchip/rk3368-px5-evb.dts      |   1 -
>  .../boot/dts/rockchip/rk3399-firefly.dts      |   1 -
>  .../boot/dts/rockchip/rk3399-hugsun-x99.dts   |   1 -
>  .../boot/dts/rockchip/rk3399-khadas-edge.dtsi |   1 -
>  .../boot/dts/rockchip/rk3399-leez-p710.dts    |   1 -
>  .../boot/dts/rockchip/rk3399-nanopi4.dtsi     |   1 -
>  .../boot/dts/rockchip/rk3399-orangepi.dts     |   1 -
>  arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi |   1 -
>  .../boot/dts/rockchip/rk3399-roc-pc.dtsi      |   1 -
>  .../boot/dts/rockchip/rk3399-rock-pi-4.dts    |   1 -
>  .../boot/dts/rockchip/rk3399-rock960.dtsi     |   1 -
>  .../boot/dts/rockchip/rk3399-rockpro64.dts    |   1 -
>  .../boot/dts/rockchip/rk3399-sapphire.dtsi    |   1 -
>  drivers/mfd/rk808.c                           | 144 +++++-------------
>  include/linux/mfd/rk808.h                     |   2 -
>  32 files changed, 42 insertions(+), 134 deletions(-)
> 
> 





^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFCv1 0/8] RK3399 clean shutdown issue
  2019-12-06 22:32 ` [RFCv1 0/8] RK3399 clean shutdown issue Heiko Stuebner
@ 2019-12-07  5:07   ` Anand Moon
  2019-12-07 11:45     ` Heiko Stuebner
  0 siblings, 1 reply; 21+ messages in thread
From: Anand Moon @ 2019-12-07  5:07 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: Rob Herring, Mark Rutland, Jagan Teki, Manivannan Sadhasivam,
	Robin Murphy, Daniel Schultz, devicetree, linux-arm-kernel,
	linux-rockchip, Linux Kernel

Hi Heiko,

On Sat, 7 Dec 2019 at 04:02, Heiko Stuebner <heiko@sntech.de> wrote:
>
> Hi Anand,
>
> Am Freitag, 6. Dezember 2019, 19:45:28 CET schrieb Anand Moon:
> > Most of the RK3399 SBC boards do not perform clean
> > shutdown and clean reboot.
> >
> > These patches try to help resolve the issue with proper
> > shutdown by turning off the PMIC.
> >
> > For reference
> > RK805 PMCI data sheet:
> > [0] http://rockchip.fr/RK805%20datasheet%20V1.3.pdf
> > RK808 PMIC data sheet:
> > [1] http://rockchip.fr/RK808%20datasheet%20V1.4.pdf
> > RK817 PMIC data sheet:
> > [2] http://rockchip.fr/RK817%20datasheet%20V1.01.pdf
> > RK818 PMIC data sheet:
> > [3] http://rockchip.fr/RK818%20datasheet%20V1.0.pdf
> >
> > Reboot issue:
> > My guess is that we need to some proper sequence of
> > setting to PMCI to perform clean.
> >
> > If you have any input please share them.
>
> The rk8xx pmics may not on all devices be responsible for powering down
> the device. That is what the system-power-controller dt-property is for.
>
> So that property is there for a reason - to indicate that the pmic is
> responsible for power-off-handling.
>
> Heiko
>

Ok, my intent was to have common framework for
shutdown, restart, suspend, resume routines.

-Anand

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFCv1 0/8] RK3399 clean shutdown issue
  2019-12-07  5:07   ` Anand Moon
@ 2019-12-07 11:45     ` Heiko Stuebner
  0 siblings, 0 replies; 21+ messages in thread
From: Heiko Stuebner @ 2019-12-07 11:45 UTC (permalink / raw)
  To: Anand Moon
  Cc: Rob Herring, Mark Rutland, Jagan Teki, Manivannan Sadhasivam,
	Robin Murphy, Daniel Schultz, devicetree, linux-arm-kernel,
	linux-rockchip, Linux Kernel

Am Samstag, 7. Dezember 2019, 06:07:49 CET schrieb Anand Moon:
> Hi Heiko,
> 
> On Sat, 7 Dec 2019 at 04:02, Heiko Stuebner <heiko@sntech.de> wrote:
> >
> > Hi Anand,
> >
> > Am Freitag, 6. Dezember 2019, 19:45:28 CET schrieb Anand Moon:
> > > Most of the RK3399 SBC boards do not perform clean
> > > shutdown and clean reboot.
> > >
> > > These patches try to help resolve the issue with proper
> > > shutdown by turning off the PMIC.
> > >
> > > For reference
> > > RK805 PMCI data sheet:
> > > [0] http://rockchip.fr/RK805%20datasheet%20V1.3.pdf
> > > RK808 PMIC data sheet:
> > > [1] http://rockchip.fr/RK808%20datasheet%20V1.4.pdf
> > > RK817 PMIC data sheet:
> > > [2] http://rockchip.fr/RK817%20datasheet%20V1.01.pdf
> > > RK818 PMIC data sheet:
> > > [3] http://rockchip.fr/RK818%20datasheet%20V1.0.pdf
> > >
> > > Reboot issue:
> > > My guess is that we need to some proper sequence of
> > > setting to PMCI to perform clean.
> > >
> > > If you have any input please share them.
> >
> > The rk8xx pmics may not on all devices be responsible for powering down
> > the device. That is what the system-power-controller dt-property is for.
> >
> > So that property is there for a reason - to indicate that the pmic is
> > responsible for power-off-handling.
> >
> > Heiko
> >
> 
> Ok, my intent was to have common framework for
> shutdown, restart, suspend, resume routines.

That is a great goal actually :-)

I guess just keep in mind that it should only handle power-off
if instructed by the devicetree property.



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFCv1 0/8] RK3399 clean shutdown issue
  2019-12-06 18:45 [RFCv1 0/8] RK3399 clean shutdown issue Anand Moon
                   ` (8 preceding siblings ...)
  2019-12-06 22:32 ` [RFCv1 0/8] RK3399 clean shutdown issue Heiko Stuebner
@ 2019-12-09 13:29 ` Robin Murphy
  2019-12-09 13:37   ` Peter Geis
  2019-12-09 14:56   ` Anand Moon
  9 siblings, 2 replies; 21+ messages in thread
From: Robin Murphy @ 2019-12-09 13:29 UTC (permalink / raw)
  To: Anand Moon, Rob Herring, Mark Rutland, Heiko Stuebner,
	Jagan Teki, Manivannan Sadhasivam, Daniel Schultz
  Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel

On 06/12/2019 6:45 pm, Anand Moon wrote:
> Most of the RK3399 SBC boards do not perform clean
> shutdown and clean reboot.

FWIW reboot problems on RK3399 have been tracked down to issues in 
upstream ATF, and are unrelated to the PMIC.

> These patches try to help resolve the issue with proper
> shutdown by turning off the PMIC.

As mentioned elsewhere[1], although this is what the BSP kernel seems to 
do, and in practice it's unlikely to matter for the majority of devboard 
users like you and me, I still feel a bit uncomfortable with this 
solution for systems using ATF as in principle the secure world might 
want to know about orderly shutdowns, and this effectively makes every 
shutdown an unexpected power loss from secure software's point of view.

Robin.

[1] 
http://lists.infradead.org/pipermail/linux-rockchip/2019-December/028183.html

> For reference
> RK805 PMCI data sheet:
> [0] http://rockchip.fr/RK805%20datasheet%20V1.3.pdf
> RK808 PMIC data sheet:
> [1] http://rockchip.fr/RK808%20datasheet%20V1.4.pdf
> RK817 PMIC data sheet:
> [2] http://rockchip.fr/RK817%20datasheet%20V1.01.pdf
> RK818 PMIC data sheet:
> [3] http://rockchip.fr/RK818%20datasheet%20V1.0.pdf
> 
> Reboot issue:
> My guess is that we need to some proper sequence of
> setting to PMCI to perform clean.
> 
> If you have any input please share them.
> 
> Tested on SBC
> Rock960 Model A
> Odroid N1
> Rock64
> 
> -Anand Moon
> 
> Anand Moon (8):
>    mfd: rk808: Refactor shutdown functions
>    mfd: rk808: use syscore for RK805 PMIC shutdown
>    mfd: rk808: use syscore for RK808 PMIC shutdown
>    mfd: rk808: use syscore for RK818 PMIC shutdown
>    mfd: rk808: cleanup unused function pointer
>    mfd: rk808: use common syscore for all PMCI for clean shutdown
>    arm64: rockchip: drop unused field from rk8xx i2c node
>    arm: rockchip: drop unused field from rk8xx i2c node
> 
>   arch/arm/boot/dts/rk3036-kylin.dts            |   1 -
>   arch/arm/boot/dts/rk3188-px3-evb.dts          |   1 -
>   arch/arm/boot/dts/rk3288-evb-rk808.dts        |   1 -
>   arch/arm/boot/dts/rk3288-phycore-som.dtsi     |   1 -
>   arch/arm/boot/dts/rk3288-popmetal.dts         |   1 -
>   arch/arm/boot/dts/rk3288-tinker.dtsi          |   1 -
>   arch/arm/boot/dts/rk3288-veyron.dtsi          |   1 -
>   arch/arm/boot/dts/rk3288-vyasa.dts            |   1 -
>   arch/arm/boot/dts/rv1108-elgin-r1.dts         |   1 -
>   arch/arm/boot/dts/rv1108-evb.dts              |   1 -
>   arch/arm64/boot/dts/rockchip/px30-evb.dts     |   1 -
>   arch/arm64/boot/dts/rockchip/rk3328-a1.dts    |   1 -
>   arch/arm64/boot/dts/rockchip/rk3328-evb.dts   |   1 -
>   .../arm64/boot/dts/rockchip/rk3328-roc-cc.dts |   1 -
>   .../arm64/boot/dts/rockchip/rk3328-rock64.dts |   1 -
>   .../boot/dts/rockchip/rk3368-geekbox.dts      |   1 -
>   arch/arm64/boot/dts/rockchip/rk3368-lion.dtsi |   1 -
>   .../boot/dts/rockchip/rk3368-px5-evb.dts      |   1 -
>   .../boot/dts/rockchip/rk3399-firefly.dts      |   1 -
>   .../boot/dts/rockchip/rk3399-hugsun-x99.dts   |   1 -
>   .../boot/dts/rockchip/rk3399-khadas-edge.dtsi |   1 -
>   .../boot/dts/rockchip/rk3399-leez-p710.dts    |   1 -
>   .../boot/dts/rockchip/rk3399-nanopi4.dtsi     |   1 -
>   .../boot/dts/rockchip/rk3399-orangepi.dts     |   1 -
>   arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi |   1 -
>   .../boot/dts/rockchip/rk3399-roc-pc.dtsi      |   1 -
>   .../boot/dts/rockchip/rk3399-rock-pi-4.dts    |   1 -
>   .../boot/dts/rockchip/rk3399-rock960.dtsi     |   1 -
>   .../boot/dts/rockchip/rk3399-rockpro64.dts    |   1 -
>   .../boot/dts/rockchip/rk3399-sapphire.dtsi    |   1 -
>   drivers/mfd/rk808.c                           | 144 +++++-------------
>   include/linux/mfd/rk808.h                     |   2 -
>   32 files changed, 42 insertions(+), 134 deletions(-)
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFCv1 2/8] mfd: rk808: use syscore for RK805 PMIC shutdown
  2019-12-06 18:45 ` [RFCv1 2/8] mfd: rk808: use syscore for RK805 PMIC shutdown Anand Moon
@ 2019-12-09 13:34   ` Robin Murphy
  2019-12-09 15:38     ` Anand Moon
  0 siblings, 1 reply; 21+ messages in thread
From: Robin Murphy @ 2019-12-09 13:34 UTC (permalink / raw)
  To: Anand Moon, Rob Herring, Mark Rutland, Heiko Stuebner,
	Jagan Teki, Manivannan Sadhasivam, Daniel Schultz
  Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel

On 06/12/2019 6:45 pm, Anand Moon wrote:
> Use common syscore_shutdown for RK805 PMIC to do
> clean I2C shutdown, drop the unused pm_pwroff_prep_fn
> and pm_pwroff_fn function pointers.

Coincidentally, I've also been looking at RK805 for the sake of trying 
to get suspend to behave on my RK3328 box, and I've ended up with some 
slightly different cleanup patches - I'll tidy them up and post them for 
comparison as soon as I can.

> Cc: Heiko Stuebner <heiko@sntech.de>
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
>   drivers/mfd/rk808.c | 33 +++++++++++++++++----------------
>   1 file changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
> index e637f5bcc8bb..713d989064ba 100644
> --- a/drivers/mfd/rk808.c
> +++ b/drivers/mfd/rk808.c
> @@ -467,16 +467,6 @@ static void rk808_update_bits(unsigned int reg, unsigned int mask,
>   			"can't write to register 0x%x: %x!\n", reg, ret);
>   }
>   
> -static void rk805_device_shutdown(void)
> -{
> -	rk808_update_bits(RK805_DEV_CTRL_REG, DEV_OFF, DEV_OFF);
> -}
> -
> -static void rk805_device_shutdown_prepare(void)
> -{
> -	rk808_update_bits(RK805_GPIO_IO_POL_REG, SLP_SD_MSK, SHUTDOWN_FUN);
> -}
> -
>   static void rk808_device_shutdown(void)
>   {
>   	rk808_update_bits(RK808_DEVCTRL_REG, DEV_OFF_RST, DEV_OFF_RST);
> @@ -491,10 +481,23 @@ static void rk8xx_syscore_shutdown(void)
>   {
>   	struct rk808 *rk808 = i2c_get_clientdata(rk808_i2c_client);
>   
> -	if (system_state == SYSTEM_POWER_OFF &&
> -	    (rk808->variant == RK809_ID || rk808->variant == RK817_ID)) {
> -		rk808_update_bits(RK817_SYS_CFG(3), RK817_SLPPIN_FUNC_MSK,
> -				SLPPIN_DN_FUN);
> +	if (system_state == SYSTEM_POWER_OFF) {
> +		dev_info(&rk808_i2c_client->dev, "System Shutdown Event\n");
> +
> +		switch (rk808->variant) {
> +		case RK805_ID:
> +			rk808_update_bits(RK805_GPIO_IO_POL_REG,
> +					SLP_SD_MSK, SHUTDOWN_FUN);
> +			rk808_update_bits(RK805_DEV_CTRL_REG, DEV_OFF, DEV_OFF);

Why this change? Shutdown via the SLEEP pin is working just fine on my 
box :/

Robin.

> +			break;
> +		case RK809_ID:
> +		case RK817_ID:
> +			rk808_update_bits(RK817_SYS_CFG(3),
> +					RK817_SLPPIN_FUNC_MSK, SLPPIN_DN_FUN);
> +			break;
> +		default:
> +			break;
> +		}
>   	}
>   }
>   
> @@ -565,8 +568,6 @@ static int rk808_probe(struct i2c_client *client,
>   		nr_pre_init_regs = ARRAY_SIZE(rk805_pre_init_reg);
>   		cells = rk805s;
>   		nr_cells = ARRAY_SIZE(rk805s);
> -		rk808->pm_pwroff_fn = rk805_device_shutdown;
> -		rk808->pm_pwroff_prep_fn = rk805_device_shutdown_prepare;
>   		break;
>   	case RK808_ID:
>   		rk808->regmap_cfg = &rk808_regmap_config;
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFCv1 0/8] RK3399 clean shutdown issue
  2019-12-09 13:29 ` Robin Murphy
@ 2019-12-09 13:37   ` Peter Geis
  2019-12-09 13:53     ` Heiko Stübner
                       ` (2 more replies)
  2019-12-09 14:56   ` Anand Moon
  1 sibling, 3 replies; 21+ messages in thread
From: Peter Geis @ 2019-12-09 13:37 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Anand Moon, Rob Herring, Mark Rutland, Heiko Stuebner,
	Jagan Teki, Manivannan Sadhasivam, Daniel Schultz, devicetree,
	linux-kernel, linux-arm-kernel, open list:ARM/Rockchip SoC...

On Mon, Dec 9, 2019 at 8:29 AM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 06/12/2019 6:45 pm, Anand Moon wrote:
> > Most of the RK3399 SBC boards do not perform clean
> > shutdown and clean reboot.
>
> FWIW reboot problems on RK3399 have been tracked down to issues in
> upstream ATF, and are unrelated to the PMIC.
>
> > These patches try to help resolve the issue with proper
> > shutdown by turning off the PMIC.
>
> As mentioned elsewhere[1], although this is what the BSP kernel seems to
> do, and in practice it's unlikely to matter for the majority of devboard
> users like you and me, I still feel a bit uncomfortable with this
> solution for systems using ATF as in principle the secure world might
> want to know about orderly shutdowns, and this effectively makes every
> shutdown an unexpected power loss from secure software's point of view.
>
> Robin.

Since ATF is operating completely in volatile memory, and shouldn't be
touching hardware once it passes off control to the kernel anyways,
what is the harm of pulling the rug out from under it?
If this idea is to prevent issues in the future, such as if ATF does
gain the ability to preempt hardware control, then at that time ATF
will need to be able to handle actually powering off devices using the
same functionality.

But as we discussed previously, ATF doesn't have this capability, so
in this case any board without a dedicated power-off gpio will be
unable to power off at all.
Also it seems that giving ATF this functionality, with the current
state of ATF, would be cost prohibitive.

I personally feel that allowing the kernel to do this is a solution to
the problem we have now.


>
> [1]
> http://lists.infradead.org/pipermail/linux-rockchip/2019-December/028183.html
>
> > For reference
> > RK805 PMCI data sheet:
> > [0] http://rockchip.fr/RK805%20datasheet%20V1.3.pdf
> > RK808 PMIC data sheet:
> > [1] http://rockchip.fr/RK808%20datasheet%20V1.4.pdf
> > RK817 PMIC data sheet:
> > [2] http://rockchip.fr/RK817%20datasheet%20V1.01.pdf
> > RK818 PMIC data sheet:
> > [3] http://rockchip.fr/RK818%20datasheet%20V1.0.pdf
> >
> > Reboot issue:
> > My guess is that we need to some proper sequence of
> > setting to PMCI to perform clean.
> >
> > If you have any input please share them.
> >
> > Tested on SBC
> > Rock960 Model A
> > Odroid N1
> > Rock64
> >
> > -Anand Moon
> >
> > Anand Moon (8):
> >    mfd: rk808: Refactor shutdown functions
> >    mfd: rk808: use syscore for RK805 PMIC shutdown
> >    mfd: rk808: use syscore for RK808 PMIC shutdown
> >    mfd: rk808: use syscore for RK818 PMIC shutdown
> >    mfd: rk808: cleanup unused function pointer
> >    mfd: rk808: use common syscore for all PMCI for clean shutdown
> >    arm64: rockchip: drop unused field from rk8xx i2c node
> >    arm: rockchip: drop unused field from rk8xx i2c node
> >
> >   arch/arm/boot/dts/rk3036-kylin.dts            |   1 -
> >   arch/arm/boot/dts/rk3188-px3-evb.dts          |   1 -
> >   arch/arm/boot/dts/rk3288-evb-rk808.dts        |   1 -
> >   arch/arm/boot/dts/rk3288-phycore-som.dtsi     |   1 -
> >   arch/arm/boot/dts/rk3288-popmetal.dts         |   1 -
> >   arch/arm/boot/dts/rk3288-tinker.dtsi          |   1 -
> >   arch/arm/boot/dts/rk3288-veyron.dtsi          |   1 -
> >   arch/arm/boot/dts/rk3288-vyasa.dts            |   1 -
> >   arch/arm/boot/dts/rv1108-elgin-r1.dts         |   1 -
> >   arch/arm/boot/dts/rv1108-evb.dts              |   1 -
> >   arch/arm64/boot/dts/rockchip/px30-evb.dts     |   1 -
> >   arch/arm64/boot/dts/rockchip/rk3328-a1.dts    |   1 -
> >   arch/arm64/boot/dts/rockchip/rk3328-evb.dts   |   1 -
> >   .../arm64/boot/dts/rockchip/rk3328-roc-cc.dts |   1 -
> >   .../arm64/boot/dts/rockchip/rk3328-rock64.dts |   1 -
> >   .../boot/dts/rockchip/rk3368-geekbox.dts      |   1 -
> >   arch/arm64/boot/dts/rockchip/rk3368-lion.dtsi |   1 -
> >   .../boot/dts/rockchip/rk3368-px5-evb.dts      |   1 -
> >   .../boot/dts/rockchip/rk3399-firefly.dts      |   1 -
> >   .../boot/dts/rockchip/rk3399-hugsun-x99.dts   |   1 -
> >   .../boot/dts/rockchip/rk3399-khadas-edge.dtsi |   1 -
> >   .../boot/dts/rockchip/rk3399-leez-p710.dts    |   1 -
> >   .../boot/dts/rockchip/rk3399-nanopi4.dtsi     |   1 -
> >   .../boot/dts/rockchip/rk3399-orangepi.dts     |   1 -
> >   arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi |   1 -
> >   .../boot/dts/rockchip/rk3399-roc-pc.dtsi      |   1 -
> >   .../boot/dts/rockchip/rk3399-rock-pi-4.dts    |   1 -
> >   .../boot/dts/rockchip/rk3399-rock960.dtsi     |   1 -
> >   .../boot/dts/rockchip/rk3399-rockpro64.dts    |   1 -
> >   .../boot/dts/rockchip/rk3399-sapphire.dtsi    |   1 -
> >   drivers/mfd/rk808.c                           | 144 +++++-------------
> >   include/linux/mfd/rk808.h                     |   2 -
> >   32 files changed, 42 insertions(+), 134 deletions(-)
> >
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFCv1 0/8] RK3399 clean shutdown issue
  2019-12-09 13:37   ` Peter Geis
@ 2019-12-09 13:53     ` Heiko Stübner
  2019-12-09 13:58     ` Robin Murphy
  2019-12-09 14:51     ` Tobias Schramm
  2 siblings, 0 replies; 21+ messages in thread
From: Heiko Stübner @ 2019-12-09 13:53 UTC (permalink / raw)
  To: Peter Geis
  Cc: Robin Murphy, Anand Moon, Rob Herring, Mark Rutland, Jagan Teki,
	Manivannan Sadhasivam, Daniel Schultz, devicetree, linux-kernel,
	linux-arm-kernel, open list:ARM/Rockchip SoC...

Hi,

Am Montag, 9. Dezember 2019, 14:37:28 CET schrieb Peter Geis:
> On Mon, Dec 9, 2019 at 8:29 AM Robin Murphy <robin.murphy@arm.com> wrote:
> >
> > On 06/12/2019 6:45 pm, Anand Moon wrote:
> > > Most of the RK3399 SBC boards do not perform clean
> > > shutdown and clean reboot.
> >
> > FWIW reboot problems on RK3399 have been tracked down to issues in
> > upstream ATF, and are unrelated to the PMIC.
> >
> > > These patches try to help resolve the issue with proper
> > > shutdown by turning off the PMIC.
> >
> > As mentioned elsewhere[1], although this is what the BSP kernel seems to
> > do, and in practice it's unlikely to matter for the majority of devboard
> > users like you and me, I still feel a bit uncomfortable with this
> > solution for systems using ATF as in principle the secure world might
> > want to know about orderly shutdowns, and this effectively makes every
> > shutdown an unexpected power loss from secure software's point of view.
> >
> > Robin.
> 
> Since ATF is operating completely in volatile memory, and shouldn't be
> touching hardware once it passes off control to the kernel anyways,
> what is the harm of pulling the rug out from under it?

Secure-world doesn't end with ATF :-)

There can also be an instance of OP-TEE or another TEE on top and they
often actually do secure storage encrypted onto the hosts mass storage
(see kernel's optee module).


> If this idea is to prevent issues in the future, such as if ATF does
> gain the ability to preempt hardware control, then at that time ATF
> will need to be able to handle actually powering off devices using the
> same functionality.
> 
> But as we discussed previously, ATF doesn't have this capability, so
> in this case any board without a dedicated power-off gpio will be
> unable to power off at all.
> Also it seems that giving ATF this functionality, with the current
> state of ATF, would be cost prohibitive.
> 
> I personally feel that allowing the kernel to do this is a solution to
> the problem we have now.

For the rest I guess I'll just point to Robin's text and Ack under your
system-power-controller patch ;-)

Heiko


> > [1]
> > http://lists.infradead.org/pipermail/linux-rockchip/2019-December/028183.html
> >
> > > For reference
> > > RK805 PMCI data sheet:
> > > [0] http://rockchip.fr/RK805%20datasheet%20V1.3.pdf
> > > RK808 PMIC data sheet:
> > > [1] http://rockchip.fr/RK808%20datasheet%20V1.4.pdf
> > > RK817 PMIC data sheet:
> > > [2] http://rockchip.fr/RK817%20datasheet%20V1.01.pdf
> > > RK818 PMIC data sheet:
> > > [3] http://rockchip.fr/RK818%20datasheet%20V1.0.pdf
> > >
> > > Reboot issue:
> > > My guess is that we need to some proper sequence of
> > > setting to PMCI to perform clean.
> > >
> > > If you have any input please share them.
> > >
> > > Tested on SBC
> > > Rock960 Model A
> > > Odroid N1
> > > Rock64
> > >
> > > -Anand Moon
> > >
> > > Anand Moon (8):
> > >    mfd: rk808: Refactor shutdown functions
> > >    mfd: rk808: use syscore for RK805 PMIC shutdown
> > >    mfd: rk808: use syscore for RK808 PMIC shutdown
> > >    mfd: rk808: use syscore for RK818 PMIC shutdown
> > >    mfd: rk808: cleanup unused function pointer
> > >    mfd: rk808: use common syscore for all PMCI for clean shutdown
> > >    arm64: rockchip: drop unused field from rk8xx i2c node
> > >    arm: rockchip: drop unused field from rk8xx i2c node
> > >
> > >   arch/arm/boot/dts/rk3036-kylin.dts            |   1 -
> > >   arch/arm/boot/dts/rk3188-px3-evb.dts          |   1 -
> > >   arch/arm/boot/dts/rk3288-evb-rk808.dts        |   1 -
> > >   arch/arm/boot/dts/rk3288-phycore-som.dtsi     |   1 -
> > >   arch/arm/boot/dts/rk3288-popmetal.dts         |   1 -
> > >   arch/arm/boot/dts/rk3288-tinker.dtsi          |   1 -
> > >   arch/arm/boot/dts/rk3288-veyron.dtsi          |   1 -
> > >   arch/arm/boot/dts/rk3288-vyasa.dts            |   1 -
> > >   arch/arm/boot/dts/rv1108-elgin-r1.dts         |   1 -
> > >   arch/arm/boot/dts/rv1108-evb.dts              |   1 -
> > >   arch/arm64/boot/dts/rockchip/px30-evb.dts     |   1 -
> > >   arch/arm64/boot/dts/rockchip/rk3328-a1.dts    |   1 -
> > >   arch/arm64/boot/dts/rockchip/rk3328-evb.dts   |   1 -
> > >   .../arm64/boot/dts/rockchip/rk3328-roc-cc.dts |   1 -
> > >   .../arm64/boot/dts/rockchip/rk3328-rock64.dts |   1 -
> > >   .../boot/dts/rockchip/rk3368-geekbox.dts      |   1 -
> > >   arch/arm64/boot/dts/rockchip/rk3368-lion.dtsi |   1 -
> > >   .../boot/dts/rockchip/rk3368-px5-evb.dts      |   1 -
> > >   .../boot/dts/rockchip/rk3399-firefly.dts      |   1 -
> > >   .../boot/dts/rockchip/rk3399-hugsun-x99.dts   |   1 -
> > >   .../boot/dts/rockchip/rk3399-khadas-edge.dtsi |   1 -
> > >   .../boot/dts/rockchip/rk3399-leez-p710.dts    |   1 -
> > >   .../boot/dts/rockchip/rk3399-nanopi4.dtsi     |   1 -
> > >   .../boot/dts/rockchip/rk3399-orangepi.dts     |   1 -
> > >   arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi |   1 -
> > >   .../boot/dts/rockchip/rk3399-roc-pc.dtsi      |   1 -
> > >   .../boot/dts/rockchip/rk3399-rock-pi-4.dts    |   1 -
> > >   .../boot/dts/rockchip/rk3399-rock960.dtsi     |   1 -
> > >   .../boot/dts/rockchip/rk3399-rockpro64.dts    |   1 -
> > >   .../boot/dts/rockchip/rk3399-sapphire.dtsi    |   1 -
> > >   drivers/mfd/rk808.c                           | 144 +++++-------------
> > >   include/linux/mfd/rk808.h                     |   2 -
> > >   32 files changed, 42 insertions(+), 134 deletions(-)
> > >
> >
> > _______________________________________________
> > Linux-rockchip mailing list
> > Linux-rockchip@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-rockchip
> 





^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFCv1 0/8] RK3399 clean shutdown issue
  2019-12-09 13:37   ` Peter Geis
  2019-12-09 13:53     ` Heiko Stübner
@ 2019-12-09 13:58     ` Robin Murphy
  2019-12-09 14:51     ` Tobias Schramm
  2 siblings, 0 replies; 21+ messages in thread
From: Robin Murphy @ 2019-12-09 13:58 UTC (permalink / raw)
  To: Peter Geis
  Cc: Anand Moon, Rob Herring, Mark Rutland, Heiko Stuebner,
	Jagan Teki, Manivannan Sadhasivam, Daniel Schultz, devicetree,
	linux-kernel, linux-arm-kernel, open list:ARM/Rockchip SoC...

On 09/12/2019 1:37 pm, Peter Geis wrote:
> On Mon, Dec 9, 2019 at 8:29 AM Robin Murphy <robin.murphy@arm.com> wrote:
>>
>> On 06/12/2019 6:45 pm, Anand Moon wrote:
>>> Most of the RK3399 SBC boards do not perform clean
>>> shutdown and clean reboot.
>>
>> FWIW reboot problems on RK3399 have been tracked down to issues in
>> upstream ATF, and are unrelated to the PMIC.
>>
>>> These patches try to help resolve the issue with proper
>>> shutdown by turning off the PMIC.
>>
>> As mentioned elsewhere[1], although this is what the BSP kernel seems to
>> do, and in practice it's unlikely to matter for the majority of devboard
>> users like you and me, I still feel a bit uncomfortable with this
>> solution for systems using ATF as in principle the secure world might
>> want to know about orderly shutdowns, and this effectively makes every
>> shutdown an unexpected power loss from secure software's point of view.
>>
>> Robin.
> 
> Since ATF is operating completely in volatile memory, and shouldn't be
> touching hardware once it passes off control to the kernel anyways,
> what is the harm of pulling the rug out from under it?
> If this idea is to prevent issues in the future, such as if ATF does
> gain the ability to preempt hardware control, then at that time ATF
> will need to be able to handle actually powering off devices using the
> same functionality.

It's not ATF itself I'm concerned about, but arbitrary Secure EL1 
payloads. In theory, a TEE might want to do something like cycle keys or 
log events in a TPM (or just encrypted in an external SPI flash), or 
scrub secure DRAM for cold boot protection. I'm pretty sure none of us 
are running such a thing on our boards today, but an upstream solution 
really wants to be robust for the general case.

> But as we discussed previously, ATF doesn't have this capability, so
> in this case any board without a dedicated power-off gpio will be
> unable to power off at all.
> Also it seems that giving ATF this functionality, with the current
> state of ATF, would be cost prohibitive.
> 
> I personally feel that allowing the kernel to do this is a solution to
> the problem we have now.

Sure - I do like your cool idea of using a custom reboot reason to 
handle a 'trusted' poweroff in the U-Boot SPL - but in the meantime 
anything would be a practical improvement over the current situation. I 
reckon this other patch[1] (this seems to be a popular issue all of a 
sudden!) looks like the neatest interim step.

Robin.

[1] 
http://lists.infradead.org/pipermail/linux-rockchip/2019-December/028245.html

>>> For reference
>>> RK805 PMCI data sheet:
>>> [0] http://rockchip.fr/RK805%20datasheet%20V1.3.pdf
>>> RK808 PMIC data sheet:
>>> [1] http://rockchip.fr/RK808%20datasheet%20V1.4.pdf
>>> RK817 PMIC data sheet:
>>> [2] http://rockchip.fr/RK817%20datasheet%20V1.01.pdf
>>> RK818 PMIC data sheet:
>>> [3] http://rockchip.fr/RK818%20datasheet%20V1.0.pdf
>>>
>>> Reboot issue:
>>> My guess is that we need to some proper sequence of
>>> setting to PMCI to perform clean.
>>>
>>> If you have any input please share them.
>>>
>>> Tested on SBC
>>> Rock960 Model A
>>> Odroid N1
>>> Rock64
>>>
>>> -Anand Moon
>>>
>>> Anand Moon (8):
>>>     mfd: rk808: Refactor shutdown functions
>>>     mfd: rk808: use syscore for RK805 PMIC shutdown
>>>     mfd: rk808: use syscore for RK808 PMIC shutdown
>>>     mfd: rk808: use syscore for RK818 PMIC shutdown
>>>     mfd: rk808: cleanup unused function pointer
>>>     mfd: rk808: use common syscore for all PMCI for clean shutdown
>>>     arm64: rockchip: drop unused field from rk8xx i2c node
>>>     arm: rockchip: drop unused field from rk8xx i2c node
>>>
>>>    arch/arm/boot/dts/rk3036-kylin.dts            |   1 -
>>>    arch/arm/boot/dts/rk3188-px3-evb.dts          |   1 -
>>>    arch/arm/boot/dts/rk3288-evb-rk808.dts        |   1 -
>>>    arch/arm/boot/dts/rk3288-phycore-som.dtsi     |   1 -
>>>    arch/arm/boot/dts/rk3288-popmetal.dts         |   1 -
>>>    arch/arm/boot/dts/rk3288-tinker.dtsi          |   1 -
>>>    arch/arm/boot/dts/rk3288-veyron.dtsi          |   1 -
>>>    arch/arm/boot/dts/rk3288-vyasa.dts            |   1 -
>>>    arch/arm/boot/dts/rv1108-elgin-r1.dts         |   1 -
>>>    arch/arm/boot/dts/rv1108-evb.dts              |   1 -
>>>    arch/arm64/boot/dts/rockchip/px30-evb.dts     |   1 -
>>>    arch/arm64/boot/dts/rockchip/rk3328-a1.dts    |   1 -
>>>    arch/arm64/boot/dts/rockchip/rk3328-evb.dts   |   1 -
>>>    .../arm64/boot/dts/rockchip/rk3328-roc-cc.dts |   1 -
>>>    .../arm64/boot/dts/rockchip/rk3328-rock64.dts |   1 -
>>>    .../boot/dts/rockchip/rk3368-geekbox.dts      |   1 -
>>>    arch/arm64/boot/dts/rockchip/rk3368-lion.dtsi |   1 -
>>>    .../boot/dts/rockchip/rk3368-px5-evb.dts      |   1 -
>>>    .../boot/dts/rockchip/rk3399-firefly.dts      |   1 -
>>>    .../boot/dts/rockchip/rk3399-hugsun-x99.dts   |   1 -
>>>    .../boot/dts/rockchip/rk3399-khadas-edge.dtsi |   1 -
>>>    .../boot/dts/rockchip/rk3399-leez-p710.dts    |   1 -
>>>    .../boot/dts/rockchip/rk3399-nanopi4.dtsi     |   1 -
>>>    .../boot/dts/rockchip/rk3399-orangepi.dts     |   1 -
>>>    arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi |   1 -
>>>    .../boot/dts/rockchip/rk3399-roc-pc.dtsi      |   1 -
>>>    .../boot/dts/rockchip/rk3399-rock-pi-4.dts    |   1 -
>>>    .../boot/dts/rockchip/rk3399-rock960.dtsi     |   1 -
>>>    .../boot/dts/rockchip/rk3399-rockpro64.dts    |   1 -
>>>    .../boot/dts/rockchip/rk3399-sapphire.dtsi    |   1 -
>>>    drivers/mfd/rk808.c                           | 144 +++++-------------
>>>    include/linux/mfd/rk808.h                     |   2 -
>>>    32 files changed, 42 insertions(+), 134 deletions(-)
>>>
>>
>> _______________________________________________
>> Linux-rockchip mailing list
>> Linux-rockchip@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFCv1 0/8] RK3399 clean shutdown issue
  2019-12-09 13:37   ` Peter Geis
  2019-12-09 13:53     ` Heiko Stübner
  2019-12-09 13:58     ` Robin Murphy
@ 2019-12-09 14:51     ` Tobias Schramm
  2 siblings, 0 replies; 21+ messages in thread
From: Tobias Schramm @ 2019-12-09 14:51 UTC (permalink / raw)
  To: Peter Geis, Robin Murphy
  Cc: Anand Moon, Rob Herring, Mark Rutland, Heiko Stuebner,
	Jagan Teki, Manivannan Sadhasivam, Daniel Schultz, devicetree,
	linux-kernel, linux-arm-kernel, open list:ARM/Rockchip SoC...

Hi,

> On Mon, Dec 9, 2019 at 8:29 AM Robin Murphy <robin.murphy@arm.com> wrote:
>> On 06/12/2019 6:45 pm, Anand Moon wrote:
>>> Most of the RK3399 SBC boards do not perform clean
>>> shutdown and clean reboot.
>> FWIW reboot problems on RK3399 have been tracked down to issues in
>> upstream ATF, and are unrelated to the PMIC.
>>
>>> These patches try to help resolve the issue with proper
>>> shutdown by turning off the PMIC.
>> As mentioned elsewhere[1], although this is what the BSP kernel seems to
>> do, and in practice it's unlikely to matter for the majority of devboard
>> users like you and me, I still feel a bit uncomfortable with this
>> solution for systems using ATF as in principle the secure world might
>> want to know about orderly shutdowns, and this effectively makes every
>> shutdown an unexpected power loss from secure software's point of view.
>>
>> Robin.
> Since ATF is operating completely in volatile memory, and shouldn't be
> touching hardware once it passes off control to the kernel anyways,
> what is the harm of pulling the rug out from under it?
> If this idea is to prevent issues in the future, such as if ATF does
> gain the ability to preempt hardware control, then at that time ATF
> will need to be able to handle actually powering off devices using the
> same functionality.

As far as I know ATF implements PSCI, doesn't it? Thus I would assume 
that it should most definitely handle power off for all platforms as 
indicated by the presence of platform handlers in [1].

> But as we discussed previously, ATF doesn't have this capability, so
> in this case any board without a dedicated power-off gpio will be
> unable to power off at all.
> Also it seems that giving ATF this functionality, with the current
> state of ATF, would be cost prohibitive.
>
> I personally feel that allowing the kernel to do this is a solution to
> the problem we have now.

Maybe I'm missing something here but I'd suggest that implementing an 
i2c driver in the rockchip platform part of ATF using libfdt to find the 
PMIC from the devicetree would be the way to go.

[1] 
https://github.com/ARM-software/arm-trusted-firmware/blob/master/lib/psci/psci_system_off.c#L31


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFCv1 0/8] RK3399 clean shutdown issue
  2019-12-09 13:29 ` Robin Murphy
  2019-12-09 13:37   ` Peter Geis
@ 2019-12-09 14:56   ` Anand Moon
  1 sibling, 0 replies; 21+ messages in thread
From: Anand Moon @ 2019-12-09 14:56 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Rob Herring, Mark Rutland, Heiko Stuebner, Jagan Teki,
	Manivannan Sadhasivam, Daniel Schultz, devicetree,
	linux-arm-kernel, linux-rockchip, Linux Kernel

Hi Robin,

On Mon, 9 Dec 2019 at 18:59, Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 06/12/2019 6:45 pm, Anand Moon wrote:
> > Most of the RK3399 SBC boards do not perform clean
> > shutdown and clean reboot.
>
> FWIW reboot problems on RK3399 have been tracked down to issues in
> upstream ATF, and are unrelated to the PMIC.

Yes I am aware of this changes.
But, I have tired to study *RK808 datasheet V1.4* [0] below section
*5.2.3 Power Channel Control/Monitor Registers*
for clean reboot I was going to try disable some bit in below
into reboot handle in the future patch.

DCDC_EN_REG
SLEEP_SET_OFF_REG1
SLEEP_SET_OFF_REG2
DCDC_UV_STS_REG

I was going see if this helps to do clean reboot.
further more use this in suspend/resume operation.

[0] http://rockchip.fr/RK808%20datasheet%20V1.4.pdf

But I feed that their is some more issue with related to mmc or PCIe
not able to cleanly release the resources while reboot which caused
then to disable after reboot.

>
> > These patches try to help resolve the issue with proper
> > shutdown by turning off the PMIC.
>
> As mentioned elsewhere[1], although this is what the BSP kernel seems to
> do, and in practice it's unlikely to matter for the majority of devboard
> users like you and me, I still feel a bit uncomfortable with this
> solution for systems using ATF as in principle the secure world might
> want to know about orderly shutdowns, and this effectively makes every
> shutdown an unexpected power loss from secure software's point of view.
>
> Robin.
>
> [1]
> http://lists.infradead.org/pipermail/linux-rockchip/2019-December/028183.html
>

Yes I have follow the mailing list and I read this thread.
I am not aware of ATF complete architecture.

-Anand

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFCv1 2/8] mfd: rk808: use syscore for RK805 PMIC shutdown
  2019-12-09 13:34   ` Robin Murphy
@ 2019-12-09 15:38     ` Anand Moon
  0 siblings, 0 replies; 21+ messages in thread
From: Anand Moon @ 2019-12-09 15:38 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Rob Herring, Mark Rutland, Heiko Stuebner, Jagan Teki,
	Manivannan Sadhasivam, Daniel Schultz, devicetree,
	linux-arm-kernel, linux-rockchip, Linux Kernel

Hi Robin,

On Mon, 9 Dec 2019 at 19:04, Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 06/12/2019 6:45 pm, Anand Moon wrote:
> > Use common syscore_shutdown for RK805 PMIC to do
> > clean I2C shutdown, drop the unused pm_pwroff_prep_fn
> > and pm_pwroff_fn function pointers.
>
> Coincidentally, I've also been looking at RK805 for the sake of trying
> to get suspend to behave on my RK3328 box, and I've ended up with some
> slightly different cleanup patches - I'll tidy them up and post them for
> comparison as soon as I can.

No issue if their is better clean approach, I will definitely test that series.
>
> > Cc: Heiko Stuebner <heiko@sntech.de>
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > ---
> >   drivers/mfd/rk808.c | 33 +++++++++++++++++----------------
> >   1 file changed, 17 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
> > index e637f5bcc8bb..713d989064ba 100644
> > --- a/drivers/mfd/rk808.c
> > +++ b/drivers/mfd/rk808.c
> > @@ -467,16 +467,6 @@ static void rk808_update_bits(unsigned int reg, unsigned int mask,
> >                       "can't write to register 0x%x: %x!\n", reg, ret);
> >   }
> >
> > -static void rk805_device_shutdown(void)
> > -{
> > -     rk808_update_bits(RK805_DEV_CTRL_REG, DEV_OFF, DEV_OFF);
> > -}
> > -
> > -static void rk805_device_shutdown_prepare(void)
> > -{
> > -     rk808_update_bits(RK805_GPIO_IO_POL_REG, SLP_SD_MSK, SHUTDOWN_FUN);
> > -}
> > -
> >   static void rk808_device_shutdown(void)
> >   {
> >       rk808_update_bits(RK808_DEVCTRL_REG, DEV_OFF_RST, DEV_OFF_RST);
> > @@ -491,10 +481,23 @@ static void rk8xx_syscore_shutdown(void)
> >   {
> >       struct rk808 *rk808 = i2c_get_clientdata(rk808_i2c_client);
> >
> > -     if (system_state == SYSTEM_POWER_OFF &&
> > -         (rk808->variant == RK809_ID || rk808->variant == RK817_ID)) {
> > -             rk808_update_bits(RK817_SYS_CFG(3), RK817_SLPPIN_FUNC_MSK,
> > -                             SLPPIN_DN_FUN);
> > +     if (system_state == SYSTEM_POWER_OFF) {
> > +             dev_info(&rk808_i2c_client->dev, "System Shutdown Event\n");
> > +
> > +             switch (rk808->variant) {
> > +             case RK805_ID:
> > +                     rk808_update_bits(RK805_GPIO_IO_POL_REG,
> > +                                     SLP_SD_MSK, SHUTDOWN_FUN);
> > +                     rk808_update_bits(RK805_DEV_CTRL_REG, DEV_OFF, DEV_OFF);
>
> Why this change? Shutdown via the SLEEP pin is working just fine on my
> box :/
>
> Robin.

As per RK-805 datasheet [0] below.
For clean poweroff we need to set in DEV_CTRL_REG reg
Bit 0 DEV_OFF: write “1” to turn down the PMU.

[0] http://files.pine64.org/doc/rock64/Rockchip_RK805_Datasheet_V1.1%C2%A020160921.pdf

-Anand

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFCv1 1/8] mfd: rk808: Refactor shutdown functions
  2019-12-06 18:45 ` [RFCv1 1/8] mfd: rk808: Refactor shutdown functions Anand Moon
@ 2019-12-16 11:11   ` Lee Jones
  0 siblings, 0 replies; 21+ messages in thread
From: Lee Jones @ 2019-12-16 11:11 UTC (permalink / raw)
  To: Anand Moon
  Cc: Rob Herring, Mark Rutland, Heiko Stuebner, Jagan Teki,
	Manivannan Sadhasivam, Robin Murphy, Daniel Schultz, devicetree,
	linux-arm-kernel, linux-rockchip, linux-kernel

On Fri, 06 Dec 2019, Anand Moon wrote:

> From: Daniel Schultz <d.schultz@phytec.de>
> 
> Since all shutdown functions have almost the same code, all logic
> from the shutdown functions can be refactored to a new function
> "rk808_update_bits", which can update a register by a given address
> and bitmask and value.
> 
> link: https://lore.kernel.org/patchwork/patch/937404/
> Cc: Heiko Stuebner <heiko@sntech.de>
> Signed-off-by: Daniel Schultz <d.schultz@phytec.de>
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
> [rebased on latest kernel]
> Modified the API to set the value.
> This changes were submited with below patch.
> [0] https://lore.kernel.org/patchwork/patch/937404/
> ---
>  drivers/mfd/rk808.c | 87 ++++++++++++++-------------------------------
>  1 file changed, 26 insertions(+), 61 deletions(-)

Not sure what's happening with these (competing?) patch-sets.  I'm not
planning on getting involved until you guys have arrived at and agreed
upon a single solution.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2019-12-16 11:11 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-06 18:45 [RFCv1 0/8] RK3399 clean shutdown issue Anand Moon
2019-12-06 18:45 ` [RFCv1 1/8] mfd: rk808: Refactor shutdown functions Anand Moon
2019-12-16 11:11   ` Lee Jones
2019-12-06 18:45 ` [RFCv1 2/8] mfd: rk808: use syscore for RK805 PMIC shutdown Anand Moon
2019-12-09 13:34   ` Robin Murphy
2019-12-09 15:38     ` Anand Moon
2019-12-06 18:45 ` [RFCv1 3/8] mfd: rk808: use syscore for RK808 " Anand Moon
2019-12-06 18:45 ` [RFCv1 4/8] mfd: rk808: use syscore for RK818 " Anand Moon
2019-12-06 18:45 ` [RFCv1 5/8] mfd: rk808: cleanup unused function pointer Anand Moon
2019-12-06 18:45 ` [RFCv1 6/8] mfd: rk808: use common syscore for all PMCI for clean shutdown Anand Moon
2019-12-06 18:45 ` [RFCv1 7/8] arm64: rockchip: drop unused field from rk8xx i2c node Anand Moon
2019-12-06 18:45 ` [RFCv1 8/8] arm: " Anand Moon
2019-12-06 22:32 ` [RFCv1 0/8] RK3399 clean shutdown issue Heiko Stuebner
2019-12-07  5:07   ` Anand Moon
2019-12-07 11:45     ` Heiko Stuebner
2019-12-09 13:29 ` Robin Murphy
2019-12-09 13:37   ` Peter Geis
2019-12-09 13:53     ` Heiko Stübner
2019-12-09 13:58     ` Robin Murphy
2019-12-09 14:51     ` Tobias Schramm
2019-12-09 14:56   ` Anand Moon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).