mfd: rk808: Change DEV_OFF to DEV_OFF_RST for rk818 shutdown

PATCH Superseded Daniel Schultz 2 participants 3 messages Mar 5, 2018 – Mar 7, 2018 Multifunction devices (MFD)

lore

Share & tools

lore

Daniel Schultz

author

2018-03-05 13:16 UTC #0 lore raw

PATCH 1/2 mfd: rk808: Change DEV_OFF to DEV_OFF_RST for rk818 shutdown

After running "poweroff", the PMIC restarts the SoC instead of shutting it down.

It seems like the description for those two register bits is swapped. After changing from DEV_OFF to DEV_OFF_RST, the system can poweroff correctly.

Signed-off-by Daniel Schultz <[email protected]>

---
 drivers/mfd/rk808.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/mfd/rk808.c +1 -1

index 216fbf6..d138721 100644
@@ -418,7 +418,7 @@ static void rk818_device_shutdown(void)
 
 	ret = regmap_update_bits(rk808->regmap,
 				 RK818_DEVCTRL_REG,
-				 DEV_OFF, DEV_OFF);
+				 DEV_OFF_RST, DEV_OFF_RST);
 	if (ret)
 		dev_err(&rk808_i2c_client->dev, "power off error!\n");
 }

Signature · 1 line

2.7.4

Daniel Schultz

author

2018-03-05 13:17 UTC #1 lore raw

PATCH 2/2 mfd: rk808: Add restart functionality

When using Rockchip SoCs with rk805/808/818 PMICs, restarts are realized by setting the reset registers in the "Clock and Reset Unit".

Since all three 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.

After that, notifier blocks and the restart functions were added to the driver. Like the shutdown function, the restart is bound to the "rockchip,system-power-controller" device tree property and can easily revoked to the software restart by removing the property.

Signed-off-by Daniel Schultz <[email protected]>

---
 drivers/mfd/rk808.c       | 97 ++++++++++++++++++++++++++++++-----------------
 include/linux/mfd/rk808.h |  1 +
 2 files changed, 63 insertions(+), 35 deletions(-)

drivers/mfd/rk808.c +62 ~~-35~~

index d138721..2c68f8c 100644
@@ -27,6 +27,7 @@
 #include <linux/module.h>
 #include <linux/of_device.h>
 #include <linux/regmap.h>
+#include <linux/reboot.h>
 
 struct rk808_reg_data {
 	int addr;

@@ -369,59 +370,73 @@ 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 bit_mask)
 {
 	int ret;
 	struct rk808 *rk808 = i2c_get_clientdata(rk808_i2c_client);
 
 	if (!rk808) {
 		dev_warn(&rk808_i2c_client->dev,
-			 "have no rk805, so do nothing here\n");
+			 "have no %s, so do nothing here\n",
+			 rk808->regmap_irq_chip->name);
 		return;
 	}
 
 	ret = regmap_update_bits(rk808->regmap,
-				 RK805_DEV_CTRL_REG,
-				 DEV_OFF, DEV_OFF);
+				 reg,
+				 bit_mask, bit_mask);
 	if (ret)
-		dev_err(&rk808_i2c_client->dev, "power off error!\n");
+		dev_err(&rk808_i2c_client->dev, "can't write to DEVCTRL: %x!\n",
+			ret);
 }
 
-static void rk808_device_shutdown(void)
+static void rk805_device_shutdown(void)
 {
-	int ret;
-	struct rk808 *rk808 = i2c_get_clientdata(rk808_i2c_client);
-
-	if (!rk808) {
-		dev_warn(&rk808_i2c_client->dev,
-			 "have no rk808, so do nothing here\n");
-		return;
-	}
+	rk808_update_bits(RK805_DEV_CTRL_REG, DEV_OFF);
+}
+static int rk805_restart_notify(struct notifier_block *this,
+				   unsigned long mode, void *cmd)
+{
+	rk808_update_bits(RK805_DEV_CTRL_REG, DEV_OFF_RST);
+	return NOTIFY_DONE;
+}
 
-	ret = regmap_update_bits(rk808->regmap,
-				 RK808_DEVCTRL_REG,
-				 DEV_OFF_RST, DEV_OFF_RST);
-	if (ret)
-		dev_err(&rk808_i2c_client->dev, "power off error!\n");
+static void rk808_device_shutdown(void)
+{
+	rk808_update_bits(RK808_DEVCTRL_REG, DEV_OFF_RST);
+}
+static int rk808_restart_notify(struct notifier_block *this,
+				   unsigned long mode, void *cmd)
+{
+	rk808_update_bits(RK808_DEVCTRL_REG, DEV_OFF);
+	return NOTIFY_DONE;
 }
 
 static void rk818_device_shutdown(void)
 {
-	int ret;
-	struct rk808 *rk808 = i2c_get_clientdata(rk808_i2c_client);
+	rk808_update_bits(RK818_DEVCTRL_REG, DEV_OFF_RST);
+}
+static int rk818_restart_notify(struct notifier_block *this,
+				   unsigned long mode, void *cmd)
+{
+	rk808_update_bits(RK818_DEVCTRL_REG, DEV_OFF);
+	return NOTIFY_DONE;
+}
 
-	if (!rk808) {
-		dev_warn(&rk808_i2c_client->dev,
-			 "have no rk818, so do nothing here\n");
-		return;
-	}
+static struct notifier_block rk805_restart_handler = {
+	.notifier_call = rk805_restart_notify,
+	.priority = 196,
+};
 
-	ret = regmap_update_bits(rk808->regmap,
-				 RK818_DEVCTRL_REG,
-				 DEV_OFF_RST, DEV_OFF_RST);
-	if (ret)
-		dev_err(&rk808_i2c_client->dev, "power off error!\n");
-}
+static struct notifier_block rk808_restart_handler = {
+	.notifier_call = rk808_restart_notify,
+	.priority = 196,
+};
+
+static struct notifier_block rk818_restart_handler = {
+	.notifier_call = rk818_restart_notify,
+	.priority = 196,
+};
 
 static const struct of_device_id rk808_of_match[] = {
 	{ .compatible = "rockchip,rk805" },

@@ -476,6 +491,7 @@ static int rk808_probe(struct i2c_client *client,
 		cells = rk805s;
 		nr_cells = ARRAY_SIZE(rk805s);
 		pm_pwroff_fn = rk805_device_shutdown;
+		rk808->nb = &rk805_restart_handler;
 		break;
 	case RK808_ID:
 		rk808->regmap_cfg = &rk808_regmap_config;

@@ -485,6 +501,7 @@ static int rk808_probe(struct i2c_client *client,
 		cells = rk808s;
 		nr_cells = ARRAY_SIZE(rk808s);
 		pm_pwroff_fn = rk808_device_shutdown;
+		rk808->nb = &rk808_restart_handler;
 		break;
 	case RK818_ID:
 		rk808->regmap_cfg = &rk818_regmap_config;

@@ -494,6 +511,7 @@ static int rk808_probe(struct i2c_client *client,
 		cells = rk818s;
 		nr_cells = ARRAY_SIZE(rk818s);
 		pm_pwroff_fn = rk818_device_shutdown;
+		rk808->nb = &rk818_restart_handler;
 		break;
 	default:
 		dev_err(&client->dev, "Unsupported RK8XX ID %lu\n",

@@ -546,11 +564,18 @@ static int rk808_probe(struct i2c_client *client,
 
 	pm_off = of_property_read_bool(np,
 				"rockchip,system-power-controller");
-	if (pm_off && !pm_power_off) {
+	if (pm_off) {
 		rk808_i2c_client = client;
-		pm_power_off = pm_pwroff_fn;
+		if (!pm_power_off)
+			pm_power_off = pm_pwroff_fn;
+		if (rk808->nb) {
+			ret = register_restart_handler(rk808->nb);
+			if (ret)
+				dev_err(&client->dev,
+					"cannot register restart handler, %d\n",
+					ret);
+		}
 	}
-
 	return 0;
 
 err_irq:

@@ -564,6 +589,8 @@ static int rk808_remove(struct i2c_client *client)
 
 	regmap_del_irq_chip(client->irq, rk808->irq_data);
 	pm_power_off = NULL;
+	if (rk808->nb)
+		unregister_restart_handler(rk808->nb);
 
 	return 0;
 }

include/linux/mfd/rk808.h +1 -0

index 338e0f6..9c26ca2 100644
@@ -453,5 +453,6 @@ struct rk808 {
 	long				variant;
 	const struct regmap_config	*regmap_cfg;
 	const struct regmap_irq_chip	*regmap_irq_chip;
+	struct notifier_block		*nb;
 };
 #endif /* __LINUX_REGULATOR_RK808_H */

Signature · 1 line

2.7.4

Lee Jones

2018-03-07 16:40 UTC #2 lore raw

In reply to Daniel Schultz

Quoted 11 lines · Daniel Schultz

After running "poweroff", the PMIC restarts the SoC instead of shutting it down.
It seems like the description for those two register bits is swapped. After changing from DEV_OFF to DEV_OFF_RST, the system can poweroff correctly.
Signed-off-by Daniel Schultz <[email protected]>
---
 drivers/mfd/rk808.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Please could you re-submit these 2 patches with the Rock Chips guys in Cc. It would be easier to review if we had some expert eyes-on and a Tested-by or 2.

Quoted 13 lines

drivers/mfd/rk808.c +1 -1

index 216fbf6..d138721 100644
@@ -418,7 +418,7 @@ static void rk818_device_shutdown(void)
 
 	ret = regmap_update_bits(rk808->regmap,
 				 RK818_DEVCTRL_REG,
-				 DEV_OFF, DEV_OFF);
+				 DEV_OFF_RST, DEV_OFF_RST);
 	if (ret)
 		dev_err(&rk808_i2c_client->dev, "power off error!\n");
 }

Signature · 4 lines

Lee Jones
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog