[PATCH v2 0/2] Rockchip: Add option to prevent boot on plug-in

[PATCH v2 0/2] Rockchip: Add option to prevent boot on plug-in

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

Sometimes it is desirable to prevent a board from automatically booting
as soon as the power cable is plugged in. For boards with an rk8xx
PMIC, (excluding the rk808) we can actually query the power up source.

Changes from V1:
 - Moved a comment as requested to be above a function.
 - Removed unneeded variable of "plug_in".

Chris Morgan (2):
  power: pmic: rk8xx: Support sysreset shutdown method
  rockchip: Add option to prevent booting on power plug-in

 arch/arm/mach-rockchip/Kconfig | 10 ++++
 drivers/power/pmic/rk8xx.c     | 84 +++++++++++++++++++++++++++++++++-
 include/power/rk8xx_pmic.h     |  3 ++
 3 files changed, 96 insertions(+), 1 deletion(-)

-- 
2.25.1

[PATCH v2 1/2] power: pmic: rk8xx: Support sysreset shutdown method

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

Add support for sysreset shutdown for this PMIC. The values were pulled
from the various datasheets, but for now it has only been tested on
the rk817 (for an Odroid Go Advance).

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
---
 drivers/power/pmic/rk8xx.c | 50 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 49 insertions(+), 1 deletion(-)

diff --git a/drivers/power/pmic/rk8xx.c b/drivers/power/pmic/rk8xx.c
index 5f442fea68..1ffbecc02a 100644
--- a/drivers/power/pmic/rk8xx.c
+++ b/drivers/power/pmic/rk8xx.c
@@ -6,10 +6,50 @@
 
 #include <common.h>
 #include <dm.h>
+#include <dm/lists.h>
 #include <errno.h>
 #include <log.h>
 #include <power/rk8xx_pmic.h>
 #include <power/pmic.h>
+#include <sysreset.h>
+
+static int rk8xx_sysreset_request(struct udevice *dev, enum sysreset_t type)
+{
+	struct rk8xx_priv *priv = dev_get_priv(dev->parent);
+
+	if (type != SYSRESET_POWER_OFF)
+		return -EPROTONOSUPPORT;
+
+	switch (priv->variant) {
+	case RK805_ID:
+	case RK808_ID:
+	case RK816_ID:
+	case RK818_ID:
+		pmic_clrsetbits(dev->parent, REG_DEVCTRL, 0, BIT(0));
+		break;
+	case RK809_ID:
+	case RK817_ID:
+		pmic_clrsetbits(dev->parent, RK817_REG_SYS_CFG3, 0,
+				BIT(0));
+		break;
+	default:
+		printf("Unknown PMIC RK%x: Cannot shutdown\n",
+		       priv->variant);
+		return -EPROTONOSUPPORT;
+	};
+
+	return -EINPROGRESS;
+}
+
+static struct sysreset_ops rk8xx_sysreset_ops = {
+	.request	= rk8xx_sysreset_request,
+};
+
+U_BOOT_DRIVER(rk8xx_sysreset) = {
+	.name		= "rk8xx_sysreset",
+	.id		= UCLASS_SYSRESET,
+	.ops		= &rk8xx_sysreset_ops,
+};
 
 static struct reg_data rk817_init_reg[] = {
 /* enable the under-voltage protection,
@@ -61,7 +101,7 @@ static int rk8xx_read(struct udevice *dev, uint reg, uint8_t *buff, int len)
 static int rk8xx_bind(struct udevice *dev)
 {
 	ofnode regulators_node;
-	int children;
+	int children, ret;
 
 	regulators_node = dev_read_subnode(dev, "regulators");
 	if (!ofnode_valid(regulators_node)) {
@@ -72,6 +112,14 @@ static int rk8xx_bind(struct udevice *dev)
 
 	debug("%s: '%s' - found regulators subnode\n", __func__, dev->name);
 
+	if (CONFIG_IS_ENABLED(SYSRESET)) {
+		ret = device_bind_driver_to_node(dev, "rk8xx_sysreset",
+						 "rk8xx_sysreset",
+						 dev_ofnode(dev), NULL);
+		if (ret)
+			return ret;
+	}
+
 	children = pmic_bind_children(dev, regulators_node, pmic_children_info);
 	if (!children)
 		debug("%s: %s - no child found\n", __func__, dev->name);
-- 
2.25.1

[PATCH v2 2/2] rockchip: Add option to prevent booting on power plug-in

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

For Rockchip boards with the all rk8xx series PMICs (excluding the
rk808), it is sometimes desirable to not boot whenever the device is
plugged in. An example would be for the Odroid Go Advance.

This provides a configurable option to check the PMIC says it was
powered because of a plug-in event. If the value is 1 and this option
is selected, the device shuts down shortly after printing a message
to console stating the reason why it's shutting down. Powering up the
board with the power button is not affected.

This patch parallels the work done in the following patch series:
https://lore.kernel.org/u-boot/20220121133732.2397273-1-andre.przywara@arm.com/

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 arch/arm/mach-rockchip/Kconfig | 10 ++++++++++
 drivers/power/pmic/rk8xx.c     | 34 ++++++++++++++++++++++++++++++++++
 include/power/rk8xx_pmic.h     |  3 +++
 3 files changed, 47 insertions(+)

diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
index 18aff5480b..c561a77e6a 100644
--- a/arch/arm/mach-rockchip/Kconfig
+++ b/arch/arm/mach-rockchip/Kconfig
@@ -361,6 +361,16 @@ config ROCKCHIP_BOOT_MODE_REG
 	  The Soc will enter to different boot mode(defined in asm/arch-rockchip/boot_mode.h)
 	  according to the value from this register.
 
+config ROCKCHIP_RK8XX_DISABLE_BOOT_ON_POWERON
+	bool "Disable device boot on power plug-in"
+	depends on PMIC_RK8XX
+	default n
+	---help---
+	  Say Y here to prevent the device from booting up because of a plug-in
+	  event. When set, the device will boot briefly to determine why it was
+	  powered on, and if it was determined because of a plug-in event
+	  instead of a button press event it will shut back off.
+
 config ROCKCHIP_STIMER
 	bool "Rockchip STIMER support"
 	default y
diff --git a/drivers/power/pmic/rk8xx.c b/drivers/power/pmic/rk8xx.c
index 1ffbecc02a..25ef621f8d 100644
--- a/drivers/power/pmic/rk8xx.c
+++ b/drivers/power/pmic/rk8xx.c
@@ -51,6 +51,38 @@ U_BOOT_DRIVER(rk8xx_sysreset) = {
 	.ops		= &rk8xx_sysreset_ops,
 };
 
+/* In the event of a plug-in and the appropriate option has been
+ * selected, we simply shutdown instead of continue the normal boot
+ * process. Please note the rk808 is not supported as it doesn't
+ * have the appropriate register.
+ */
+void rk8xx_off_for_plugin(struct udevice *dev)
+{
+	struct rk8xx_priv *priv = dev_get_priv(dev);
+
+	switch (priv->variant) {
+	case RK805_ID:
+	case RK816_ID:
+	case RK818_ID:
+		if (pmic_reg_read(dev, RK8XX_ON_SOURCE) & RK8XX_ON_PLUG_IN) {
+			printf("Power Off due to plug-in event\n");
+			pmic_clrsetbits(dev, REG_DEVCTRL, 0, BIT(0));
+		}
+		break;
+	case RK809_ID:
+	case RK817_ID:
+		if (pmic_reg_read(dev, RK817_ON_SOURCE) & RK8XX_ON_PLUG_IN) {
+			printf("Power Off due to plug-in event\n");
+			pmic_clrsetbits(dev, RK817_REG_SYS_CFG3, 0,
+					BIT(0));
+		}
+		break;
+	default:
+		printf("PMIC RK%x: Cannot read boot reason.\n",
+		       priv->variant);
+	}
+}
+
 static struct reg_data rk817_init_reg[] = {
 /* enable the under-voltage protection,
  * the under-voltage protection will shutdown the LDO3 and reset the PMIC
@@ -211,6 +243,8 @@ static int rk8xx_probe(struct udevice *dev)
 		       pmic_reg_read(dev, on_source),
 		       pmic_reg_read(dev, off_source));
 	printf("\n");
+	if (CONFIG_IS_ENABLED(ROCKCHIP_RK8XX_DISABLE_BOOT_ON_POWERON))
+		rk8xx_off_for_plugin(dev);
 
 	return 0;
 }
diff --git a/include/power/rk8xx_pmic.h b/include/power/rk8xx_pmic.h
index 8ff0af35c5..3cbfc02195 100644
--- a/include/power/rk8xx_pmic.h
+++ b/include/power/rk8xx_pmic.h
@@ -214,6 +214,9 @@ enum {
 #define RK817_ON_SOURCE		0xf5
 #define RK817_OFF_SOURCE	0xf6
 
+#define RK8XX_ON_PWRON		BIT(7)
+#define RK8XX_ON_PLUG_IN	BIT(6)
+
 struct reg_data {
 	u8 reg;
 	u8 val;
-- 
2.25.1

Re: [PATCH v2 2/2] rockchip: Add option to prevent booting on power plug-in

[Jaehoon Chung]

On 5/28/22 03:18, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> For Rockchip boards with the all rk8xx series PMICs (excluding the
> rk808), it is sometimes desirable to not boot whenever the device is
> plugged in. An example would be for the Odroid Go Advance.
> 
> This provides a configurable option to check the PMIC says it was
> powered because of a plug-in event. If the value is 1 and this option
> is selected, the device shuts down shortly after printing a message
> to console stating the reason why it's shutting down. Powering up the
> board with the power button is not affected.
> 
> This patch parallels the work done in the following patch series:
> https://lore.kernel.org/u-boot/20220121133732.2397273-1-andre.przywara@arm.com/
> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> ---
>  arch/arm/mach-rockchip/Kconfig | 10 ++++++++++
>  drivers/power/pmic/rk8xx.c     | 34 ++++++++++++++++++++++++++++++++++
>  include/power/rk8xx_pmic.h     |  3 +++
>  3 files changed, 47 insertions(+)
> 
> diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
> index 18aff5480b..c561a77e6a 100644
> --- a/arch/arm/mach-rockchip/Kconfig
> +++ b/arch/arm/mach-rockchip/Kconfig
> @@ -361,6 +361,16 @@ config ROCKCHIP_BOOT_MODE_REG
>  	  The Soc will enter to different boot mode(defined in asm/arch-rockchip/boot_mode.h)
>  	  according to the value from this register.
>  
> +config ROCKCHIP_RK8XX_DISABLE_BOOT_ON_POWERON
> +	bool "Disable device boot on power plug-in"
> +	depends on PMIC_RK8XX
> +	default n
> +	---help---
> +	  Say Y here to prevent the device from booting up because of a plug-in
> +	  event. When set, the device will boot briefly to determine why it was
> +	  powered on, and if it was determined because of a plug-in event
> +	  instead of a button press event it will shut back off.
> +
>  config ROCKCHIP_STIMER
>  	bool "Rockchip STIMER support"
>  	default y
> diff --git a/drivers/power/pmic/rk8xx.c b/drivers/power/pmic/rk8xx.c
> index 1ffbecc02a..25ef621f8d 100644
> --- a/drivers/power/pmic/rk8xx.c
> +++ b/drivers/power/pmic/rk8xx.c
> @@ -51,6 +51,38 @@ U_BOOT_DRIVER(rk8xx_sysreset) = {
>  	.ops		= &rk8xx_sysreset_ops,
>  };
>  
> +/* In the event of a plug-in and the appropriate option has been
> + * selected, we simply shutdown instead of continue the normal boot
> + * process. Please note the rk808 is not supported as it doesn't
> + * have the appropriate register.
> + */
> +void rk8xx_off_for_plugin(struct udevice *dev)
> +{
> +	struct rk8xx_priv *priv = dev_get_priv(dev);
> +
> +	switch (priv->variant) {
> +	case RK805_ID:
> +	case RK816_ID:
> +	case RK818_ID:
> +		if (pmic_reg_read(dev, RK8XX_ON_SOURCE) & RK8XX_ON_PLUG_IN) {
> +			printf("Power Off due to plug-in event\n");
> +			pmic_clrsetbits(dev, REG_DEVCTRL, 0, BIT(0));
> +		}
> +		break;
> +	case RK809_ID:
> +	case RK817_ID:
> +		if (pmic_reg_read(dev, RK817_ON_SOURCE) & RK8XX_ON_PLUG_IN) {
> +			printf("Power Off due to plug-in event\n");
> +			pmic_clrsetbits(dev, RK817_REG_SYS_CFG3, 0,
> +					BIT(0));
> +		}
> +		break;
> +	default:
> +		printf("PMIC RK%x: Cannot read boot reason.\n",
> +		       priv->variant);
> +	}
> +}
> +
>  static struct reg_data rk817_init_reg[] = {
>  /* enable the under-voltage protection,
>   * the under-voltage protection will shutdown the LDO3 and reset the PMIC
> @@ -211,6 +243,8 @@ static int rk8xx_probe(struct udevice *dev)
>  		       pmic_reg_read(dev, on_source),
>  		       pmic_reg_read(dev, off_source));
>  	printf("\n");
> +	if (CONFIG_IS_ENABLED(ROCKCHIP_RK8XX_DISABLE_BOOT_ON_POWERON))
> +		rk8xx_off_for_plugin(dev);
>  
>  	return 0;
>  }
> diff --git a/include/power/rk8xx_pmic.h b/include/power/rk8xx_pmic.h
> index 8ff0af35c5..3cbfc02195 100644
> --- a/include/power/rk8xx_pmic.h
> +++ b/include/power/rk8xx_pmic.h
> @@ -214,6 +214,9 @@ enum {
>  #define RK817_ON_SOURCE		0xf5
>  #define RK817_OFF_SOURCE	0xf6
>  
> +#define RK8XX_ON_PWRON		BIT(7)
> +#define RK8XX_ON_PLUG_IN	BIT(6)
> +
>  struct reg_data {
>  	u8 reg;
>  	u8 val;

Re: [PATCH v2 1/2] power: pmic: rk8xx: Support sysreset shutdown method

[Michal Suchánek]

Hello,

this causes regression on pinebook pro:

resetting ...
System reset not supported on this platform
### ERROR ### Please RESET the board ###

Is there something missing in the DT for this board?

Or perhaps a fallback should be provided in absence of the PMIC?

Thanks

Michal

On Fri, May 27, 2022 at 01:18:19PM -0500, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Add support for sysreset shutdown for this PMIC. The values were pulled
> from the various datasheets, but for now it has only been tested on
> the rk817 (for an Odroid Go Advance).
> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
> Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
> ---
>  drivers/power/pmic/rk8xx.c | 50 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 49 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/power/pmic/rk8xx.c b/drivers/power/pmic/rk8xx.c
> index 5f442fea68..1ffbecc02a 100644
> --- a/drivers/power/pmic/rk8xx.c
> +++ b/drivers/power/pmic/rk8xx.c
> @@ -6,10 +6,50 @@
>  
>  #include <common.h>
>  #include <dm.h>
> +#include <dm/lists.h>
>  #include <errno.h>
>  #include <log.h>
>  #include <power/rk8xx_pmic.h>
>  #include <power/pmic.h>
> +#include <sysreset.h>
> +
> +static int rk8xx_sysreset_request(struct udevice *dev, enum sysreset_t type)
> +{
> +	struct rk8xx_priv *priv = dev_get_priv(dev->parent);
> +
> +	if (type != SYSRESET_POWER_OFF)
> +		return -EPROTONOSUPPORT;
> +
> +	switch (priv->variant) {
> +	case RK805_ID:
> +	case RK808_ID:
> +	case RK816_ID:
> +	case RK818_ID:
> +		pmic_clrsetbits(dev->parent, REG_DEVCTRL, 0, BIT(0));
> +		break;
> +	case RK809_ID:
> +	case RK817_ID:
> +		pmic_clrsetbits(dev->parent, RK817_REG_SYS_CFG3, 0,
> +				BIT(0));
> +		break;
> +	default:
> +		printf("Unknown PMIC RK%x: Cannot shutdown\n",
> +		       priv->variant);
> +		return -EPROTONOSUPPORT;
> +	};
> +
> +	return -EINPROGRESS;
> +}
> +
> +static struct sysreset_ops rk8xx_sysreset_ops = {
> +	.request	= rk8xx_sysreset_request,
> +};
> +
> +U_BOOT_DRIVER(rk8xx_sysreset) = {
> +	.name		= "rk8xx_sysreset",
> +	.id		= UCLASS_SYSRESET,
> +	.ops		= &rk8xx_sysreset_ops,
> +};
>  
>  static struct reg_data rk817_init_reg[] = {
>  /* enable the under-voltage protection,
> @@ -61,7 +101,7 @@ static int rk8xx_read(struct udevice *dev, uint reg, uint8_t *buff, int len)
>  static int rk8xx_bind(struct udevice *dev)
>  {
>  	ofnode regulators_node;
> -	int children;
> +	int children, ret;
>  
>  	regulators_node = dev_read_subnode(dev, "regulators");
>  	if (!ofnode_valid(regulators_node)) {
> @@ -72,6 +112,14 @@ static int rk8xx_bind(struct udevice *dev)
>  
>  	debug("%s: '%s' - found regulators subnode\n", __func__, dev->name);
>  
> +	if (CONFIG_IS_ENABLED(SYSRESET)) {
> +		ret = device_bind_driver_to_node(dev, "rk8xx_sysreset",
> +						 "rk8xx_sysreset",
> +						 dev_ofnode(dev), NULL);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	children = pmic_bind_children(dev, regulators_node, pmic_children_info);
>  	if (!children)
>  		debug("%s: %s - no child found\n", __func__, dev->name);
> -- 
> 2.25.1
> 

Re: [PATCH v2 1/2] power: pmic: rk8xx: Support sysreset shutdown method

[Chris Morgan]

On Thu, Jul 07, 2022 at 10:41:34AM +0200, Michal Suchánek wrote:
> Hello,
> 
> this causes regression on pinebook pro:
> 
> resetting ...
> System reset not supported on this platform
> ### ERROR ### Please RESET the board ###
> 
> Is there something missing in the DT for this board?
> 
> Or perhaps a fallback should be provided in absence of the PMIC?
> 
> Thanks
> 
> Michal

Sorry Michal, I was busy traveling and then came down with a pretty bad
cold, so I've been a bit away from things. Are you getting this error
when you attempt to reset or to poweroff the Pinebook Pro from U-Boot?

I tested the code on my RK817, but a cursory reading of the datasheet
suggests every rk8xx known so far should be able to shutdown using the
two methods added here. I'm wondering if there is perhaps a configuration
issue we need to account for, as this routine is only shutting down the
PMIC and not restarting it.

Thank you.

> 
> On Fri, May 27, 2022 at 01:18:19PM -0500, Chris Morgan wrote:
> > From: Chris Morgan <macromorgan@hotmail.com>
> > 
> > Add support for sysreset shutdown for this PMIC. The values were pulled
> > from the various datasheets, but for now it has only been tested on
> > the rk817 (for an Odroid Go Advance).
> > 
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
> > Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
> > ---
> >  drivers/power/pmic/rk8xx.c | 50 +++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 49 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/power/pmic/rk8xx.c b/drivers/power/pmic/rk8xx.c
> > index 5f442fea68..1ffbecc02a 100644
> > --- a/drivers/power/pmic/rk8xx.c
> > +++ b/drivers/power/pmic/rk8xx.c
> > @@ -6,10 +6,50 @@
> >  
> >  #include <common.h>
> >  #include <dm.h>
> > +#include <dm/lists.h>
> >  #include <errno.h>
> >  #include <log.h>
> >  #include <power/rk8xx_pmic.h>
> >  #include <power/pmic.h>
> > +#include <sysreset.h>
> > +
> > +static int rk8xx_sysreset_request(struct udevice *dev, enum sysreset_t type)
> > +{
> > +	struct rk8xx_priv *priv = dev_get_priv(dev->parent);
> > +
> > +	if (type != SYSRESET_POWER_OFF)
> > +		return -EPROTONOSUPPORT;
> > +
> > +	switch (priv->variant) {
> > +	case RK805_ID:
> > +	case RK808_ID:
> > +	case RK816_ID:
> > +	case RK818_ID:
> > +		pmic_clrsetbits(dev->parent, REG_DEVCTRL, 0, BIT(0));
> > +		break;
> > +	case RK809_ID:
> > +	case RK817_ID:
> > +		pmic_clrsetbits(dev->parent, RK817_REG_SYS_CFG3, 0,
> > +				BIT(0));
> > +		break;
> > +	default:
> > +		printf("Unknown PMIC RK%x: Cannot shutdown\n",
> > +		       priv->variant);
> > +		return -EPROTONOSUPPORT;
> > +	};
> > +
> > +	return -EINPROGRESS;
> > +}
> > +
> > +static struct sysreset_ops rk8xx_sysreset_ops = {
> > +	.request	= rk8xx_sysreset_request,
> > +};
> > +
> > +U_BOOT_DRIVER(rk8xx_sysreset) = {
> > +	.name		= "rk8xx_sysreset",
> > +	.id		= UCLASS_SYSRESET,
> > +	.ops		= &rk8xx_sysreset_ops,
> > +};
> >  
> >  static struct reg_data rk817_init_reg[] = {
> >  /* enable the under-voltage protection,
> > @@ -61,7 +101,7 @@ static int rk8xx_read(struct udevice *dev, uint reg, uint8_t *buff, int len)
> >  static int rk8xx_bind(struct udevice *dev)
> >  {
> >  	ofnode regulators_node;
> > -	int children;
> > +	int children, ret;
> >  
> >  	regulators_node = dev_read_subnode(dev, "regulators");
> >  	if (!ofnode_valid(regulators_node)) {
> > @@ -72,6 +112,14 @@ static int rk8xx_bind(struct udevice *dev)
> >  
> >  	debug("%s: '%s' - found regulators subnode\n", __func__, dev->name);
> >  
> > +	if (CONFIG_IS_ENABLED(SYSRESET)) {
> > +		ret = device_bind_driver_to_node(dev, "rk8xx_sysreset",
> > +						 "rk8xx_sysreset",
> > +						 dev_ofnode(dev), NULL);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> >  	children = pmic_bind_children(dev, regulators_node, pmic_children_info);
> >  	if (!children)
> >  		debug("%s: %s - no child found\n", __func__, dev->name);
> > -- 
> > 2.25.1
> > 

Re: [PATCH v2 1/2] power: pmic: rk8xx: Support sysreset shutdown method

[Michal Suchánek]

On Fri, Jul 15, 2022 at 12:10:47PM -0500, Chris Morgan wrote:
> On Thu, Jul 07, 2022 at 10:41:34AM +0200, Michal Suchánek wrote:
> > Hello,
> > 
> > this causes regression on pinebook pro:
> > 
> > resetting ...
> > System reset not supported on this platform
> > ### ERROR ### Please RESET the board ###
> > 
> > Is there something missing in the DT for this board?
> > 
> > Or perhaps a fallback should be provided in absence of the PMIC?
> > 
> > Thanks
> > 
> > Michal
> 
> Sorry Michal, I was busy traveling and then came down with a pretty bad
> cold, so I've been a bit away from things. Are you getting this error
> when you attempt to reset or to poweroff the Pinebook Pro from U-Boot?
> 
> I tested the code on my RK817, but a cursory reading of the datasheet
> suggests every rk8xx known so far should be able to shutdown using the
> two methods added here. I'm wondering if there is perhaps a configuration
> issue we need to account for, as this routine is only shutting down the
> PMIC and not restarting it.

This is trying to restart the system from u-boot.

The walk function *seems* to try all methods of all reset providers but
maybe there is some catch there.

Thanks

Michal

> 
> Thank you.
> 
> > 
> > On Fri, May 27, 2022 at 01:18:19PM -0500, Chris Morgan wrote:
> > > From: Chris Morgan <macromorgan@hotmail.com>
> > > 
> > > Add support for sysreset shutdown for this PMIC. The values were pulled
> > > from the various datasheets, but for now it has only been tested on
> > > the rk817 (for an Odroid Go Advance).
> > > 
> > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > > Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
> > > Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
> > > ---
> > >  drivers/power/pmic/rk8xx.c | 50 +++++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 49 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/power/pmic/rk8xx.c b/drivers/power/pmic/rk8xx.c
> > > index 5f442fea68..1ffbecc02a 100644
> > > --- a/drivers/power/pmic/rk8xx.c
> > > +++ b/drivers/power/pmic/rk8xx.c
> > > @@ -6,10 +6,50 @@
> > >  
> > >  #include <common.h>
> > >  #include <dm.h>
> > > +#include <dm/lists.h>
> > >  #include <errno.h>
> > >  #include <log.h>
> > >  #include <power/rk8xx_pmic.h>
> > >  #include <power/pmic.h>
> > > +#include <sysreset.h>
> > > +
> > > +static int rk8xx_sysreset_request(struct udevice *dev, enum sysreset_t type)
> > > +{
> > > +	struct rk8xx_priv *priv = dev_get_priv(dev->parent);
> > > +
> > > +	if (type != SYSRESET_POWER_OFF)
> > > +		return -EPROTONOSUPPORT;
> > > +
> > > +	switch (priv->variant) {
> > > +	case RK805_ID:
> > > +	case RK808_ID:
> > > +	case RK816_ID:
> > > +	case RK818_ID:
> > > +		pmic_clrsetbits(dev->parent, REG_DEVCTRL, 0, BIT(0));
> > > +		break;
> > > +	case RK809_ID:
> > > +	case RK817_ID:
> > > +		pmic_clrsetbits(dev->parent, RK817_REG_SYS_CFG3, 0,
> > > +				BIT(0));
> > > +		break;
> > > +	default:
> > > +		printf("Unknown PMIC RK%x: Cannot shutdown\n",
> > > +		       priv->variant);
> > > +		return -EPROTONOSUPPORT;
> > > +	};
> > > +
> > > +	return -EINPROGRESS;
> > > +}
> > > +
> > > +static struct sysreset_ops rk8xx_sysreset_ops = {
> > > +	.request	= rk8xx_sysreset_request,
> > > +};
> > > +
> > > +U_BOOT_DRIVER(rk8xx_sysreset) = {
> > > +	.name		= "rk8xx_sysreset",
> > > +	.id		= UCLASS_SYSRESET,
> > > +	.ops		= &rk8xx_sysreset_ops,
> > > +};
> > >  
> > >  static struct reg_data rk817_init_reg[] = {
> > >  /* enable the under-voltage protection,
> > > @@ -61,7 +101,7 @@ static int rk8xx_read(struct udevice *dev, uint reg, uint8_t *buff, int len)
> > >  static int rk8xx_bind(struct udevice *dev)
> > >  {
> > >  	ofnode regulators_node;
> > > -	int children;
> > > +	int children, ret;
> > >  
> > >  	regulators_node = dev_read_subnode(dev, "regulators");
> > >  	if (!ofnode_valid(regulators_node)) {
> > > @@ -72,6 +112,14 @@ static int rk8xx_bind(struct udevice *dev)
> > >  
> > >  	debug("%s: '%s' - found regulators subnode\n", __func__, dev->name);
> > >  
> > > +	if (CONFIG_IS_ENABLED(SYSRESET)) {
> > > +		ret = device_bind_driver_to_node(dev, "rk8xx_sysreset",
> > > +						 "rk8xx_sysreset",
> > > +						 dev_ofnode(dev), NULL);
> > > +		if (ret)
> > > +			return ret;
> > > +	}
> > > +
> > >  	children = pmic_bind_children(dev, regulators_node, pmic_children_info);
> > >  	if (!children)
> > >  		debug("%s: %s - no child found\n", __func__, dev->name);
> > > -- 
> > > 2.25.1
> > > 

Re: [PATCH v2 1/2] power: pmic: rk8xx: Support sysreset shutdown method

[Michal Suchánek]

On Fri, Jul 15, 2022 at 07:18:07PM +0200, Michal Suchánek wrote:
> On Fri, Jul 15, 2022 at 12:10:47PM -0500, Chris Morgan wrote:
> > On Thu, Jul 07, 2022 at 10:41:34AM +0200, Michal Suchánek wrote:
> > > Hello,
> > > 
> > > this causes regression on pinebook pro:
> > > 
> > > resetting ...
> > > System reset not supported on this platform
> > > ### ERROR ### Please RESET the board ###
> > > 
> > > Is there something missing in the DT for this board?
> > > 
> > > Or perhaps a fallback should be provided in absence of the PMIC?
> > > 
> > > Thanks
> > > 
> > > Michal
> > 
> > Sorry Michal, I was busy traveling and then came down with a pretty bad
> > cold, so I've been a bit away from things. Are you getting this error
> > when you attempt to reset or to poweroff the Pinebook Pro from U-Boot?
> > 
> > I tested the code on my RK817, but a cursory reading of the datasheet
> > suggests every rk8xx known so far should be able to shutdown using the
> > two methods added here. I'm wondering if there is perhaps a configuration
> > issue we need to account for, as this routine is only shutting down the
> > PMIC and not restarting it.
> 
> This is trying to restart the system from u-boot.
> 
> The walk function *seems* to try all methods of all reset providers but
> maybe there is some catch there.

After adding debug prints it looks like no sysreset uclass is found
anymore after the patch:

diff --git a/drivers/sysreset/sysreset-uclass.c b/drivers/sysreset/sysreset-uclass.c
index 279b087d16..4a144e517f 100644
--- a/drivers/sysreset/sysreset-uclass.c
+++ b/drivers/sysreset/sysreset-uclass.c
@@ -5,6 +5,7 @@
  */
 
 #define LOG_CATEGORY UCLASS_SYSRESET
+#define DEBUG
 
 #include <common.h>
 #include <command.h>
@@ -53,6 +54,18 @@ int sysreset_get_last(struct udevice *dev)
 	return ops->get_last(dev);
 }
 
+static inline const char * sysreset_to_str(enum sysreset_t reset)
+{
+	switch (reset) {
+		case SYSRESET_WARM: return "warm restart";
+		case SYSRESET_COLD: return "cold restart";
+		case SYSRESET_POWER: return "power cycle";
+		case SYSRESET_POWER_OFF: return "power off";
+		case SYSRESET_COUNT: break;
+	}
+	return "unknown";
+}
+
 int sysreset_walk(enum sysreset_t type)
 {
 	struct udevice *dev;
@@ -62,9 +75,14 @@ int sysreset_walk(enum sysreset_t type)
 		for (uclass_first_device(UCLASS_SYSRESET, &dev);
 		     dev;
 		     uclass_next_device(&dev)) {
+			debug("trying reset %s on %s...", dev->name,
+			      sysreset_to_str(type));
 			ret = sysreset_request(dev, type);
-			if (ret == -EINPROGRESS)
+			if (ret == -EINPROGRESS) {
+				debug("done\n");
 				break;
+			}
+			debug("\n");
 		}
 		type++;
 	}
-- 
2.36.1

With sysreset patch reverted

Hit any key to stop autoboot:  0 
=> reset 
resetting ...
trying reset sysreset on cold restart...
U-Boot TPL 2022.07-00027-gd9cc607b5e-dirty (Jul 16 2022 - 16:15:27)

With sysreset support included

Hit any key to stop autoboot:  0 
=> reset 
resetting ...
System reset not supported on this platform
### ERROR ### Please RESET the board ###

The error is printed here:

void sysreset_walk_halt(enum sysreset_t type)
{
        int ret;

        ret = sysreset_walk(type);

        /* Wait for the reset to take effect */
        if (ret == -EINPROGRESS)
                mdelay(100);

        /* Still no reset? Give up */
        if (spl_phase() <= PHASE_SPL)
                log_err("no sysreset\n");
        else
                log_err("System reset not supported on this platform\n");
        hang();
}