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

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

[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 <d.schultz@phytec.de>
---
Changes:
	v2: Re-submit with recipients from Rockchip.

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

diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
index 216fbf6..d138721 100644
--- a/drivers/mfd/rk808.c
+++ b/drivers/mfd/rk808.c
@@ -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");
 }
-- 
2.7.4

[PATCH v2 2/2] mfd: rk808: Add restart functionality

[Daniel Schultz]

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 <d.schultz@phytec.de>
---
Changes:
	v2: Re-submit with recipients from Rockchip.

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

diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
index d138721..2c68f8c 100644
--- a/drivers/mfd/rk808.c
+++ b/drivers/mfd/rk808.c
@@ -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;
 }
diff --git a/include/linux/mfd/rk808.h b/include/linux/mfd/rk808.h
index 338e0f6..9c26ca2 100644
--- a/include/linux/mfd/rk808.h
+++ b/include/linux/mfd/rk808.h
@@ -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 */
-- 
2.7.4

Re: [PATCH v2 2/2] mfd: rk808: Add restart functionality

[Lee Jones]

Rockchip guys,

I'd really appreciate your input on these two patches please.

Please provide Reviewed-by/Tested-by tags.

On Wed, 07 Mar 2018, Daniel Schultz wrote:
> 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 <d.schultz@phytec.de>
> ---
> Changes:
> 	v2: Re-submit with recipients from Rockchip.
> 
>  drivers/mfd/rk808.c       | 97 ++++++++++++++++++++++++++++++-----------------
>  include/linux/mfd/rk808.h |  1 +
>  2 files changed, 63 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
> index d138721..2c68f8c 100644
> --- a/drivers/mfd/rk808.c
> +++ b/drivers/mfd/rk808.c
> @@ -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;
>  }
> diff --git a/include/linux/mfd/rk808.h b/include/linux/mfd/rk808.h
> index 338e0f6..9c26ca2 100644
> --- a/include/linux/mfd/rk808.h
> +++ b/include/linux/mfd/rk808.h
> @@ -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 */

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

Re: [PATCH v2 2/2] mfd: rk808: Add restart functionality

[Joseph Chen]

Hi, Daniel：

         On Rockchip platforms, we always use CRU to restart system. 
 From your patch, I think you would like to support restart by CRU or 
PMIC, right ?

         I think restart by PMIC is not accepted, because PMIC restart 
means all regualtors are reset (voltage drops to 0mv and then power on 
again), including vdd_arm, vdd_logic, vcc_ddr, etc. This is like a cold 
boot for machine.

        We use CRU reset, because we want to keep status of last log, 
GRF, CRU or other registers, but PMIC restart can make them lost.


在 2018/3/12 22:10, Lee Jones 写道:
> Rockchip guys,
>
> I'd really appreciate your input on these two patches please.
>
> Please provide Reviewed-by/Tested-by tags.
>
> On Wed, 07 Mar 2018, Daniel Schultz wrote:
>> 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 <d.schultz@phytec.de>
>> ---
>> Changes:
>> 	v2: Re-submit with recipients from Rockchip.
>>
>>   drivers/mfd/rk808.c       | 97 ++++++++++++++++++++++++++++++-----------------
>>   include/linux/mfd/rk808.h |  1 +
>>   2 files changed, 63 insertions(+), 35 deletions(-)
>>
>> diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
>> index d138721..2c68f8c 100644
>> --- a/drivers/mfd/rk808.c
>> +++ b/drivers/mfd/rk808.c
>> @@ -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;
>>   }
>> diff --git a/include/linux/mfd/rk808.h b/include/linux/mfd/rk808.h
>> index 338e0f6..9c26ca2 100644
>> --- a/include/linux/mfd/rk808.h
>> +++ b/include/linux/mfd/rk808.h
>> @@ -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 */

-- 
Best Regards
----------------------------------------------------------------------------
陈健洪 (Joseph Chen)
E-mail：chenjh@rock-chips.com
福州瑞芯微电子股份有限公司
Fuzhou Rockchip Electronics Co.Ltd
福建省福州市铜盘路软件大道89号软件园A区21号楼 (350003)
No. 21 Building, A District, No.89,software Boulevard Fuzhou,Fujian,PRC
TEL：0591-83991906/07-8573

Re: [PATCH v2 2/2] mfd: rk808: Add restart functionality

[Daniel Schultz]

Hi,

I can add a DTS property to enable the PMIC restart functions? So this 
feature can be used if its desired.

My problem with the CRU reset is a strange behaviour in the ROM code. 
During a reset from MMC1 (eMMC), the ROM tries to load the SPL from MMC0 
(SD).

Daniel


On 03/13/2018 04:32 AM, Joseph Chen wrote:
> Hi, Daniel：
>
>         On Rockchip platforms, we always use CRU to restart system. 
> From your patch, I think you would like to support restart by CRU or 
> PMIC, right ?
>
>         I think restart by PMIC is not accepted, because PMIC restart 
> means all regualtors are reset (voltage drops to 0mv and then power on 
> again), including vdd_arm, vdd_logic, vcc_ddr, etc. This is like a 
> cold boot for machine.
>
>        We use CRU reset, because we want to keep status of last log, 
> GRF, CRU or other registers, but PMIC restart can make them lost.
>
>
> 在 2018/3/12 22:10, Lee Jones 写道:
>> Rockchip guys,
>>
>> I'd really appreciate your input on these two patches please.
>>
>> Please provide Reviewed-by/Tested-by tags.
>>
>> On Wed, 07 Mar 2018, Daniel Schultz wrote:
>>> 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 <d.schultz@phytec.de>
>>> ---
>>> Changes:
>>>     v2: Re-submit with recipients from Rockchip.
>>>
>>>   drivers/mfd/rk808.c       | 97 
>>> ++++++++++++++++++++++++++++++-----------------
>>>   include/linux/mfd/rk808.h |  1 +
>>>   2 files changed, 63 insertions(+), 35 deletions(-)
>>>
>>> diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
>>> index d138721..2c68f8c 100644
>>> --- a/drivers/mfd/rk808.c
>>> +++ b/drivers/mfd/rk808.c
>>> @@ -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;
>>>   }
>>> diff --git a/include/linux/mfd/rk808.h b/include/linux/mfd/rk808.h
>>> index 338e0f6..9c26ca2 100644
>>> --- a/include/linux/mfd/rk808.h
>>> +++ b/include/linux/mfd/rk808.h
>>> @@ -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 */
>

Re: [PATCH v2 2/2] mfd: rk808: Add restart functionality

[Joseph Chen]

Hi, Daniel：

     I discussed with my colleagues, we agree with your idear to make 
PMIC restart as a option and controlled by a new DTS property.


在 2018/3/13 17:48, Daniel Schultz 写道:
> Hi,
>
> I can add a DTS property to enable the PMIC restart functions? So this 
> feature can be used if its desired.
>
> My problem with the CRU reset is a strange behaviour in the ROM code. 
> During a reset from MMC1 (eMMC), the ROM tries to load the SPL from 
> MMC0 (SD).
>
> Daniel
>
>
> On 03/13/2018 04:32 AM, Joseph Chen wrote:
>> Hi, Daniel：
>>
>>         On Rockchip platforms, we always use CRU to restart system. 
>> From your patch, I think you would like to support restart by CRU or 
>> PMIC, right ?
>>
>>         I think restart by PMIC is not accepted, because PMIC restart 
>> means all regualtors are reset (voltage drops to 0mv and then power 
>> on again), including vdd_arm, vdd_logic, vcc_ddr, etc. This is like a 
>> cold boot for machine.
>>
>>        We use CRU reset, because we want to keep status of last log, 
>> GRF, CRU or other registers, but PMIC restart can make them lost.
>>
>>
>> 在 2018/3/12 22:10, Lee Jones 写道:
>>> Rockchip guys,
>>>
>>> I'd really appreciate your input on these two patches please.
>>>
>>> Please provide Reviewed-by/Tested-by tags.
>>>
>>> On Wed, 07 Mar 2018, Daniel Schultz wrote:
>>>> 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 <d.schultz@phytec.de>
>>>> ---
>>>> Changes:
>>>>     v2: Re-submit with recipients from Rockchip.
>>>>
>>>>   drivers/mfd/rk808.c       | 97 
>>>> ++++++++++++++++++++++++++++++-----------------
>>>>   include/linux/mfd/rk808.h |  1 +
>>>>   2 files changed, 63 insertions(+), 35 deletions(-)
>>>>
>>>> diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
>>>> index d138721..2c68f8c 100644
>>>> --- a/drivers/mfd/rk808.c
>>>> +++ b/drivers/mfd/rk808.c
>>>> @@ -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;
>>>>   }
>>>> diff --git a/include/linux/mfd/rk808.h b/include/linux/mfd/rk808.h
>>>> index 338e0f6..9c26ca2 100644
>>>> --- a/include/linux/mfd/rk808.h
>>>> +++ b/include/linux/mfd/rk808.h
>>>> @@ -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 */
>>
>
>
>

-- 
Best Regards
----------------------------------------------------------------------------
陈健洪 (Joseph Chen)
E-mail：chenjh@rock-chips.com
福州瑞芯微电子股份有限公司
Fuzhou Rockchip Electronics Co.Ltd
福建省福州市铜盘路软件大道89号软件园A区21号楼 (350003)
No. 21 Building, A District, No.89,software Boulevard Fuzhou,Fujian,PRC
TEL：0591-83991906/07-8573

Re: [PATCH v2 1/2] mfd: rk808: Change DEV_OFF to DEV_OFF_RST for rk818 shutdown

[Lee Jones]

Rockchip guys,

I'd really appreciate your input on these two patches please.

Please provide Reviewed-by/Tested-by tags.

On Wed, 07 Mar 2018, Daniel Schultz wrote:
> 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 <d.schultz@phytec.de>
> ---
> Changes:
> 	v2: Re-submit with recipients from Rockchip.
> 
>  drivers/mfd/rk808.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
> index 216fbf6..d138721 100644
> --- a/drivers/mfd/rk808.c
> +++ b/drivers/mfd/rk808.c
> @@ -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");
>  }

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

Re: [PATCH v2 1/2] mfd: rk808: Change DEV_OFF to DEV_OFF_RST for rk818 shutdown

[Daniel Schultz]

Hi,

On 03/13/2018 03:44 AM, Joseph Chen wrote:
> Hi, Lee and Daniel：
> 
>      I am really sorry for reply so late,  I don't realize that Daniel's 
> emails ended up in my junk mail file.
> 
> 
> 在 2018/3/12 22:09, Lee Jones 写道:
>> Rockchip guys,
>>
>> I'd really appreciate your input on these two patches please.
>>
>> Please provide Reviewed-by/Tested-by tags.
>>
>> On Wed, 07 Mar 2018, Daniel Schultz wrote:
>>> 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<d.schultz@phytec.de>
>>> ---
>>> Changes:
>>> 	v2: Re-submit with recipients from Rockchip.
>>>
>>>   drivers/mfd/rk808.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
>>> index 216fbf6..d138721 100644
>>> --- a/drivers/mfd/rk808.c
>>> +++ b/drivers/mfd/rk808.c
>>> @@ -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");
>>>   }
> 
> I think there maybe something wrong makes your machine restart after 
> "poweroff".
>      (1) Is your machine attaching charger when you run "poweroff"? 
> RK818 is hardware designed to auto power on when it detects voltage on 
> input pins(8,9);

That's the point! Both pins are driven high... thank you!

We will check how we can handle this on our hardware. For now, I will 
recall this patch.

-- 
Mit freundlichen Grüßen,
With best regards,
   Daniel Schultz

>      (2) If restart, you can dump ON_SOURCE(address: 0xAE) and 
> OFF_SOURCE(address: 0xAF) in rk808.c probe() to know  last power off 
> reason and currently power on reason;
> 
> -- 
> Best Regards
> ----------------------------------------------------------------------------
> 陈健洪 (Joseph Chen)
> E-mail：chenjh@rock-chips.com
> 福州瑞芯微电子股份有限公司
> Fuzhou Rockchip Electronics Co.Ltd
> 福建省福州市铜盘路软件大道89号软件园A区21号楼 (350003)
> No. 21 Building, A District, No.89,software Boulevard Fuzhou,Fujian,PRC
> TEL：0591-83991906/07-8573
>