[U-Boot] [PATCH 1/3] sysreset: switch to using SYSRESET_POWER_OFF for poweroff

[U-Boot] [PATCH 1/3] sysreset: switch to using SYSRESET_POWER_OFF for poweroff

[Urja Rannikko]

It seems that SYSRESET_POWER_OFF was added recently, and all previous code
used SYSRESET_POWER for poweroff. SYSRESET_POWER is supposed to be a
PMIC-level power cycle, not a poweroff.

Signed-off-by: Urja Rannikko <urjaman@gmail.com>
---
Note: I didnt touch the test/dm/sysreset.c code yet, mostly because
I wanted to get feedback on this first and that i'd need to understand
the tests properly to do that (and i havent used them before at all).
---
 arch/arm/mach-stm32mp/cmd_poweroff.c | 2 +-
 arch/sandbox/cpu/state.c             | 2 +-
 drivers/power/pmic/stpmic1.c         | 2 +-
 drivers/sysreset/sysreset_psci.c     | 2 +-
 drivers/sysreset/sysreset_sandbox.c  | 4 ++--
 5 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-stm32mp/cmd_poweroff.c b/arch/arm/mach-stm32mp/cmd_poweroff.c
index f54dd1daf2..62347425a0 100644
--- a/arch/arm/mach-stm32mp/cmd_poweroff.c
+++ b/arch/arm/mach-stm32mp/cmd_poweroff.c
@@ -14,7 +14,7 @@ int do_poweroff(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	puts("poweroff ...\n");
 	mdelay(100);
 
-	ret = sysreset_walk(SYSRESET_POWER);
+	ret = sysreset_walk(SYSRESET_POWER_OFF);
 
 	if (ret == -EINPROGRESS)
 		mdelay(1000);
diff --git a/arch/sandbox/cpu/state.c b/arch/sandbox/cpu/state.c
index d3b9c05985..dee5fde4f7 100644
--- a/arch/sandbox/cpu/state.c
+++ b/arch/sandbox/cpu/state.c
@@ -355,7 +355,7 @@ void state_reset_for_test(struct sandbox_state *state)
 {
 	/* No reset yet, so mark it as such. Always allow power reset */
 	state->last_sysreset = SYSRESET_COUNT;
-	state->sysreset_allowed[SYSRESET_POWER] = true;
+	state->sysreset_allowed[SYSRESET_POWER_OFF] = true;
 
 	memset(&state->wdt, '\0', sizeof(state->wdt));
 	memset(state->spi, '\0', sizeof(state->spi));
diff --git a/drivers/power/pmic/stpmic1.c b/drivers/power/pmic/stpmic1.c
index 65296c5fc3..eb735f4fe3 100644
--- a/drivers/power/pmic/stpmic1.c
+++ b/drivers/power/pmic/stpmic1.c
@@ -221,7 +221,7 @@ static int stpmic1_sysreset_request(struct udevice *dev, enum sysreset_t type)
 	struct udevice *pmic_dev;
 	int ret;
 
-	if (type != SYSRESET_POWER)
+	if (type != SYSRESET_POWER_OFF)
 		return -EPROTONOSUPPORT;
 
 	ret = uclass_get_device_by_driver(UCLASS_PMIC,
diff --git a/drivers/sysreset/sysreset_psci.c b/drivers/sysreset/sysreset_psci.c
index de2ec8aeb1..c7907b3226 100644
--- a/drivers/sysreset/sysreset_psci.c
+++ b/drivers/sysreset/sysreset_psci.c
@@ -18,7 +18,7 @@ static int psci_sysreset_request(struct udevice *dev, enum sysreset_t type)
 	case SYSRESET_COLD:
 		function_id = PSCI_0_2_FN_SYSTEM_RESET;
 		break;
-	case SYSRESET_POWER:
+	case SYSRESET_POWER_OFF:
 		function_id = PSCI_0_2_FN_SYSTEM_OFF;
 		break;
 	default:
diff --git a/drivers/sysreset/sysreset_sandbox.c b/drivers/sysreset/sysreset_sandbox.c
index 38e2a7e241..8bc9f4b4cc 100644
--- a/drivers/sysreset/sysreset_sandbox.c
+++ b/drivers/sysreset/sysreset_sandbox.c
@@ -57,13 +57,13 @@ static int sandbox_sysreset_request(struct udevice *dev, enum sysreset_t type)
 	case SYSRESET_COLD:
 		state->last_sysreset = type;
 		break;
-	case SYSRESET_POWER:
+	case SYSRESET_POWER_OFF:
 		state->last_sysreset = type;
 		if (!state->sysreset_allowed[type])
 			return -EACCES;
 		sandbox_exit();
 		break;
-	case SYSRESET_POWER_OFF:
+	case SYSRESET_POWER:
 		if (!state->sysreset_allowed[type])
 			return -EACCES;
 	default:
-- 
2.21.0

[U-Boot] [PATCH 2/3] sysreset: move stm32mp sysreset poweroff implementation to sysreset uclass

[Urja Rannikko]

This is a generic implementation. Add CONFIG_SYSRESET_CMD_POWEROFF
to signal when we need it. Enable it from the STPMIC1 config and in
sandbox.

The config flag is transitionary, that is it can be removed after all
poweroff implementations use sysreset, and just have CMD_POWEROFF depend
on sysreset.

Signed-off-by: Urja Rannikko <urjaman@gmail.com>
---
Note: I cant test STM32MP, so I would really appreciate if someone could
test this series on that.
---
 arch/Kconfig                         |  1 +
 arch/arm/mach-stm32mp/Makefile       |  3 ---
 arch/arm/mach-stm32mp/cmd_poweroff.c | 24 ------------------------
 drivers/power/pmic/Kconfig           |  1 +
 drivers/sysreset/Kconfig             | 10 ++++++++++
 drivers/sysreset/sysreset-uclass.c   | 18 ++++++++++++++++++
 6 files changed, 30 insertions(+), 27 deletions(-)
 delete mode 100644 arch/arm/mach-stm32mp/cmd_poweroff.c

diff --git a/arch/Kconfig b/arch/Kconfig
index 239289b885..83ff21dfd7 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -91,6 +91,7 @@ config SANDBOX
 	select LZO
 	select SPI
 	select SUPPORT_OF_CONTROL
+	select SYSRESET_CMD_POWEROFF if CMD_POWEROFF
 	imply BITREVERSE
 	select BLOBLIST
 	imply CMD_DM
diff --git a/arch/arm/mach-stm32mp/Makefile b/arch/arm/mach-stm32mp/Makefile
index 1493914a11..f59ced5ee1 100644
--- a/arch/arm/mach-stm32mp/Makefile
+++ b/arch/arm/mach-stm32mp/Makefile
@@ -11,9 +11,6 @@ ifdef CONFIG_SPL_BUILD
 obj-y += spl.o
 else
 obj-y += bsec.o
-ifndef CONFIG_STM32MP1_TRUSTED
-obj-$(CONFIG_SYSRESET) += cmd_poweroff.o
-endif
 endif
 obj-$(CONFIG_ARMV7_PSCI) += psci.o
 obj-$(CONFIG_$(SPL_)DM_REGULATOR) += pwr_regulator.o
diff --git a/arch/arm/mach-stm32mp/cmd_poweroff.c b/arch/arm/mach-stm32mp/cmd_poweroff.c
deleted file mode 100644
index 62347425a0..0000000000
--- a/arch/arm/mach-stm32mp/cmd_poweroff.c
+++ /dev/null
@@ -1,24 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+ OR BSD-3-Clause
-/*
- * Copyright (C) 2019, STMicroelectronics - All Rights Reserved
- */
-
-#include <common.h>
-#include <command.h>
-#include <sysreset.h>
-
-int do_poweroff(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
-{
-	int ret;
-
-	puts("poweroff ...\n");
-	mdelay(100);
-
-	ret = sysreset_walk(SYSRESET_POWER_OFF);
-
-	if (ret == -EINPROGRESS)
-		mdelay(1000);
-
-	/*NOTREACHED when power off*/
-	return CMD_RET_FAILURE;
-}
diff --git a/drivers/power/pmic/Kconfig b/drivers/power/pmic/Kconfig
index b0cd260354..5c6c045fad 100644
--- a/drivers/power/pmic/Kconfig
+++ b/drivers/power/pmic/Kconfig
@@ -234,6 +234,7 @@ config DM_PMIC_TPS65910
 config PMIC_STPMIC1
 	bool "Enable support for STMicroelectronics STPMIC1 PMIC"
 	depends on DM_PMIC && DM_I2C
+	select SYSRESET_CMD_POWEROFF if CMD_POWEROFF
 	---help---
 	The STPMIC1 PMIC provides 4 BUCKs, 6 LDOs, 1 VREF and 2 power switches.
 	It is accessed via an I2C interface. The device is used with STM32MP1
diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig
index 30aed2c4c1..4c883923bf 100644
--- a/drivers/sysreset/Kconfig
+++ b/drivers/sysreset/Kconfig
@@ -33,6 +33,16 @@ config TPL_SYSRESET
 
 if SYSRESET
 
+if CMD_POWEROFF
+
+config SYSRESET_CMD_POWEROFF
+	bool "sysreset implementation of the poweroff command"
+	help
+	  This should be selected by the appropriate PMIC driver if
+	  the poweroff command is enabled.
+
+endif
+
 config SYSRESET_GPIO
 	bool "Enable support for GPIO reset driver"
 	select DM_GPIO
diff --git a/drivers/sysreset/sysreset-uclass.c b/drivers/sysreset/sysreset-uclass.c
index ad831c703a..39202588ae 100644
--- a/drivers/sysreset/sysreset-uclass.c
+++ b/drivers/sysreset/sysreset-uclass.c
@@ -118,6 +118,24 @@ int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	return 0;
 }
 
+#if IS_ENABLED(CONFIG_SYSRESET_CMD_POWEROFF)
+int do_poweroff(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	int ret;
+
+	puts("poweroff ...\n");
+	mdelay(100);
+
+	ret = sysreset_walk(SYSRESET_POWER_OFF);
+
+	if (ret == -EINPROGRESS)
+		mdelay(1000);
+
+	/*NOTREACHED when power off*/
+	return CMD_RET_FAILURE;
+}
+#endif
+
 static int sysreset_post_bind(struct udevice *dev)
 {
 #if defined(CONFIG_NEEDS_MANUAL_RELOC)
-- 
2.21.0

[U-Boot] [PATCH 3/3] rk8xx: add a sysreset driver for poweroff

[Urja Rannikko]

Based on snooping around the linux kernel rk8xx driver.
Tested on an ASUS C201.

Signed-off-by: Urja Rannikko <urjaman@gmail.com>
---
 drivers/power/pmic/Kconfig |  1 +
 drivers/power/pmic/rk8xx.c | 62 +++++++++++++++++++++++++++++++++++---
 include/power/rk8xx_pmic.h |  4 +++
 3 files changed, 63 insertions(+), 4 deletions(-)

diff --git a/drivers/power/pmic/Kconfig b/drivers/power/pmic/Kconfig
index 5c6c045fad..d95f9aeafb 100644
--- a/drivers/power/pmic/Kconfig
+++ b/drivers/power/pmic/Kconfig
@@ -124,6 +124,7 @@ config PMIC_PM8916
 config PMIC_RK8XX
 	bool "Enable support for Rockchip PMIC RK8XX"
 	depends on DM_PMIC
+	select SYSRESET_CMD_POWEROFF if CMD_POWEROFF
 	---help---
 	The Rockchip RK808 PMIC provides four buck DC-DC convertors, 8 LDOs,
 	an RTC and two low Rds (resistance (drain to source)) switches. It is
diff --git a/drivers/power/pmic/rk8xx.c b/drivers/power/pmic/rk8xx.c
index 25c339ab12..52e41051ae 100644
--- a/drivers/power/pmic/rk8xx.c
+++ b/drivers/power/pmic/rk8xx.c
@@ -6,6 +6,9 @@
 
 #include <common.h>
 #include <dm.h>
+#include <sysreset.h>
+#include <dm/device.h>
+#include <dm/lists.h>
 #include <errno.h>
 #include <power/rk8xx_pmic.h>
 #include <power/pmic.h>
@@ -49,9 +52,9 @@ static int rk8xx_read(struct udevice *dev, uint reg, uint8_t *buff, int len)
 	return 0;
 }
 
-#if CONFIG_IS_ENABLED(PMIC_CHILDREN)
 static int rk8xx_bind(struct udevice *dev)
 {
+#if CONFIG_IS_ENABLED(PMIC_CHILDREN)
 	ofnode regulators_node;
 	int children;
 
@@ -68,10 +71,15 @@ static int rk8xx_bind(struct udevice *dev)
 	if (!children)
 		debug("%s: %s - no child found\n", __func__, dev->name);
 
+#endif
+
+	if (CONFIG_IS_ENABLED(SYSRESET))
+		return device_bind_driver(dev, "rk8xx-sysreset",
+					  "rk8xx-sysreset", NULL);
+
 	/* Always return success for this device */
 	return 0;
 }
-#endif
 
 static int rk8xx_probe(struct udevice *dev)
 {
@@ -103,10 +111,56 @@ U_BOOT_DRIVER(pmic_rk8xx) = {
 	.name = "rk8xx pmic",
 	.id = UCLASS_PMIC,
 	.of_match = rk8xx_ids,
-#if CONFIG_IS_ENABLED(PMIC_CHILDREN)
 	.bind = rk8xx_bind,
-#endif
 	.priv_auto_alloc_size   = sizeof(struct rk8xx_priv),
 	.probe = rk8xx_probe,
 	.ops = &rk8xx_ops,
 };
+
+#if IS_ENABLED(CONFIG_SYSRESET)
+/* NOTE: Should only enable this function if the rockchip,system-power-manager
+ * property is in the device tree node, but it is there in every board that has
+ * an rk8xx in u-boot currently, so this is left as an excercise for later.
+ */
+static int rk8xx_sysreset_request(struct udevice *dev, enum sysreset_t type)
+{
+	struct udevice *pmic_dev;
+	struct rk8xx_priv *priv;
+	int ret;
+	u8 bits;
+
+	if (type != SYSRESET_POWER_OFF)
+		return -EPROTONOSUPPORT;
+
+	ret = uclass_get_device_by_driver(UCLASS_PMIC,
+					  DM_GET_DRIVER(pmic_rk8xx),
+					  &pmic_dev);
+
+	if (ret)
+		return -EOPNOTSUPP;
+
+	priv = dev_get_priv(pmic_dev);
+
+	if (priv->variant == RK818_ID)
+		bits = DEV_OFF;
+	else
+		bits = DEV_OFF_RST;
+
+	ret = pmic_clrsetbits(pmic_dev, REG_DEVCTRL, 0, bits);
+
+	if (ret < 0)
+		return ret;
+
+	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,
+};
+#endif
diff --git a/include/power/rk8xx_pmic.h b/include/power/rk8xx_pmic.h
index c06248f751..565b35985e 100644
--- a/include/power/rk8xx_pmic.h
+++ b/include/power/rk8xx_pmic.h
@@ -177,6 +177,10 @@ enum {
 
 #define RK8XX_ID_MSK	0xfff0
 
+/* DEVCTRL bits for poweroff */
+#define DEV_OFF_RST	BIT(3)
+#define DEV_OFF		BIT(0)
+
 struct rk8xx_reg_table {
 	char *name;
 	u8 reg_ctl;
-- 
2.21.0

[U-Boot] [PATCH 2/3] sysreset: move stm32mp sysreset poweroff implementation to sysreset uclass

[Patrice CHOTARD]

Hi Urja

This patch doesn't compile using the stm32mp15_trusted_defconfig
configuration:

...
  LD      drivers/pinctrl/built-in.o
  LD      drivers/core/built-in.o
  LD      drivers/mmc/built-in.o
  LD      drivers/built-in.o
drivers/sysreset/built-in.o: In function `do_poweroff':
/local/home/nxp11987/projects/community/u-boot.denx/drivers/sysreset/sysreset-uclass.c:123:
multiple definition of `do_poweroff'
drivers/firmware/built-in.o:/local/home/nxp11987/projects/community/u-boot.denx/drivers/firmware/psci.c:134:
first defined here
make[1]: *** [drivers/built-in.o] Error 1
make: *** [drivers] Error 2
build error !!


I suggest you to update your patch with :

config PMIC_STPMIC1
 	bool "Enable support for STMicroelectronics STPMIC1 PMIC"
 	depends on DM_PMIC && DM_I2C
-	select SYSRESET_CMD_POWEROFF if CMD_POWEROFF
+	select SYSRESET_CMD_POWEROFF if CMD_POWEROFF && !ARM_PSCI_FW
 	---help---
 	The STPMIC1 PMIC provides 4 BUCKs, 6 LDOs, 1 VREF and 2 power switches.
 	It is accessed via an I2C interface. The device is used with STM32MP1
 	SoCs. This driver implements register read/write operations.


On 5/13/19 2:17 PM, Urja Rannikko wrote:
> This is a generic implementation. Add CONFIG_SYSRESET_CMD_POWEROFF
> to signal when we need it. Enable it from the STPMIC1 config and in
> sandbox.
> 
> The config flag is transitionary, that is it can be removed after all
> poweroff implementations use sysreset, and just have CMD_POWEROFF depend
> on sysreset.
> 
> Signed-off-by: Urja Rannikko <urjaman@gmail.com>
> ---
> Note: I cant test STM32MP, so I would really appreciate if someone could
> test this series on that.
> ---
>  arch/Kconfig                         |  1 +
>  arch/arm/mach-stm32mp/Makefile       |  3 ---
>  arch/arm/mach-stm32mp/cmd_poweroff.c | 24 ------------------------
>  drivers/power/pmic/Kconfig           |  1 +
>  drivers/sysreset/Kconfig             | 10 ++++++++++
>  drivers/sysreset/sysreset-uclass.c   | 18 ++++++++++++++++++
>  6 files changed, 30 insertions(+), 27 deletions(-)
>  delete mode 100644 arch/arm/mach-stm32mp/cmd_poweroff.c
> 
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 239289b885..83ff21dfd7 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -91,6 +91,7 @@ config SANDBOX
>  	select LZO
>  	select SPI
>  	select SUPPORT_OF_CONTROL
> +	select SYSRESET_CMD_POWEROFF if CMD_POWEROFF
>  	imply BITREVERSE
>  	select BLOBLIST
>  	imply CMD_DM
> diff --git a/arch/arm/mach-stm32mp/Makefile b/arch/arm/mach-stm32mp/Makefile
> index 1493914a11..f59ced5ee1 100644
> --- a/arch/arm/mach-stm32mp/Makefile
> +++ b/arch/arm/mach-stm32mp/Makefile
> @@ -11,9 +11,6 @@ ifdef CONFIG_SPL_BUILD
>  obj-y += spl.o
>  else
>  obj-y += bsec.o
> -ifndef CONFIG_STM32MP1_TRUSTED
> -obj-$(CONFIG_SYSRESET) += cmd_poweroff.o
> -endif
>  endif
>  obj-$(CONFIG_ARMV7_PSCI) += psci.o
>  obj-$(CONFIG_$(SPL_)DM_REGULATOR) += pwr_regulator.o
> diff --git a/arch/arm/mach-stm32mp/cmd_poweroff.c b/arch/arm/mach-stm32mp/cmd_poweroff.c
> deleted file mode 100644
> index 62347425a0..0000000000
> --- a/arch/arm/mach-stm32mp/cmd_poweroff.c
> +++ /dev/null
> @@ -1,24 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0+ OR BSD-3-Clause
> -/*
> - * Copyright (C) 2019, STMicroelectronics - All Rights Reserved
> - */
> -
> -#include <common.h>
> -#include <command.h>
> -#include <sysreset.h>
> -
> -int do_poweroff(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> -{
> -	int ret;
> -
> -	puts("poweroff ...\n");
> -	mdelay(100);
> -
> -	ret = sysreset_walk(SYSRESET_POWER_OFF);
> -
> -	if (ret == -EINPROGRESS)
> -		mdelay(1000);
> -
> -	/*NOTREACHED when power off*/
> -	return CMD_RET_FAILURE;
> -}
> diff --git a/drivers/power/pmic/Kconfig b/drivers/power/pmic/Kconfig
> index b0cd260354..5c6c045fad 100644
> --- a/drivers/power/pmic/Kconfig
> +++ b/drivers/power/pmic/Kconfig
> @@ -234,6 +234,7 @@ config DM_PMIC_TPS65910
>  config PMIC_STPMIC1
>  	bool "Enable support for STMicroelectronics STPMIC1 PMIC"
>  	depends on DM_PMIC && DM_I2C
> +	select SYSRESET_CMD_POWEROFF if CMD_POWEROFF
>  	---help---
>  	The STPMIC1 PMIC provides 4 BUCKs, 6 LDOs, 1 VREF and 2 power switches.
>  	It is accessed via an I2C interface. The device is used with STM32MP1
> diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig
> index 30aed2c4c1..4c883923bf 100644
> --- a/drivers/sysreset/Kconfig
> +++ b/drivers/sysreset/Kconfig
> @@ -33,6 +33,16 @@ config TPL_SYSRESET
>  
>  if SYSRESET
>  
> +if CMD_POWEROFF
> +
> +config SYSRESET_CMD_POWEROFF
> +	bool "sysreset implementation of the poweroff command"
> +	help
> +	  This should be selected by the appropriate PMIC driver if
> +	  the poweroff command is enabled.
> +
> +endif
> +
>  config SYSRESET_GPIO
>  	bool "Enable support for GPIO reset driver"
>  	select DM_GPIO
> diff --git a/drivers/sysreset/sysreset-uclass.c b/drivers/sysreset/sysreset-uclass.c
> index ad831c703a..39202588ae 100644
> --- a/drivers/sysreset/sysreset-uclass.c
> +++ b/drivers/sysreset/sysreset-uclass.c
> @@ -118,6 +118,24 @@ int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  	return 0;
>  }
>  
> +#if IS_ENABLED(CONFIG_SYSRESET_CMD_POWEROFF)
> +int do_poweroff(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> +	int ret;
> +
> +	puts("poweroff ...\n");
> +	mdelay(100);
> +
> +	ret = sysreset_walk(SYSRESET_POWER_OFF);
> +
> +	if (ret == -EINPROGRESS)
> +		mdelay(1000);
> +
> +	/*NOTREACHED when power off*/
> +	return CMD_RET_FAILURE;
> +}
> +#endif
> +
>  static int sysreset_post_bind(struct udevice *dev)
>  {
>  #if defined(CONFIG_NEEDS_MANUAL_RELOC)
> 

[U-Boot] [PATCH 1/3] sysreset: switch to using SYSRESET_POWER_OFF for poweroff

[Patrick DELAUNAY]

Hi Urja,

> 
> It seems that SYSRESET_POWER_OFF was added recently, and all previous
> code used SYSRESET_POWER for poweroff. SYSRESET_POWER is supposed
> to be a PMIC-level power cycle, not a poweroff.
> 
> Signed-off-by: Urja Rannikko <urjaman@gmail.com>
> ---
> Note: I didnt touch the test/dm/sysreset.c code yet, mostly because I wanted to
> get feedback on this first and that i'd need to understand the tests properly to do
> that (and i havent used them before at all).
> ---
>  arch/arm/mach-stm32mp/cmd_poweroff.c | 2 +-
>  arch/sandbox/cpu/state.c             | 2 +-
>  drivers/power/pmic/stpmic1.c         | 2 +-
>  drivers/sysreset/sysreset_psci.c     | 2 +-
>  drivers/sysreset/sysreset_sandbox.c  | 4 ++--
>  5 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/mach-stm32mp/cmd_poweroff.c b/arch/arm/mach-
> stm32mp/cmd_poweroff.c
> index f54dd1daf2..62347425a0 100644
> --- a/arch/arm/mach-stm32mp/cmd_poweroff.c
> +++ b/arch/arm/mach-stm32mp/cmd_poweroff.c
> @@ -14,7 +14,7 @@ int do_poweroff(cmd_tbl_t *cmdtp, int flag, int argc, char *
> const argv[])
>  	puts("poweroff ...\n");
>  	mdelay(100);
> 
> -	ret = sysreset_walk(SYSRESET_POWER);
> +	ret = sysreset_walk(SYSRESET_POWER_OFF);
> 
>  	if (ret == -EINPROGRESS)
>  		mdelay(1000);
> diff --git a/arch/sandbox/cpu/state.c b/arch/sandbox/cpu/state.c index
> d3b9c05985..dee5fde4f7 100644
> --- a/arch/sandbox/cpu/state.c
> +++ b/arch/sandbox/cpu/state.c
> @@ -355,7 +355,7 @@ void state_reset_for_test(struct sandbox_state *state)  {
>  	/* No reset yet, so mark it as such. Always allow power reset */
>  	state->last_sysreset = SYSRESET_COUNT;
> -	state->sysreset_allowed[SYSRESET_POWER] = true;
> +	state->sysreset_allowed[SYSRESET_POWER_OFF] = true;
> 
>  	memset(&state->wdt, '\0', sizeof(state->wdt));
>  	memset(state->spi, '\0', sizeof(state->spi)); diff --git
> a/drivers/power/pmic/stpmic1.c b/drivers/power/pmic/stpmic1.c index
> 65296c5fc3..eb735f4fe3 100644
> --- a/drivers/power/pmic/stpmic1.c
> +++ b/drivers/power/pmic/stpmic1.c
> @@ -221,7 +221,7 @@ static int stpmic1_sysreset_request(struct udevice *dev,
> enum sysreset_t type)
>  	struct udevice *pmic_dev;
>  	int ret;
> 
> -	if (type != SYSRESET_POWER)
> +	if (type != SYSRESET_POWER_OFF)
>  		return -EPROTONOSUPPORT;

In fact in the next part of the code, we are supporting 
only SYSRESET_POWER (reset with PMIC1 switch OFF and restart) 
and not SYSRESET_POWER_OFF....

because Power Cycle if RREQ_EN=1

I think you need to remove the update on this file for your patch and
I will modified this function is to support both mode....

>  	ret = uclass_get_device_by_driver(UCLASS_PMIC,
> diff --git a/drivers/sysreset/sysreset_psci.c b/drivers/sysreset/sysreset_psci.c
> index de2ec8aeb1..c7907b3226 100644
> --- a/drivers/sysreset/sysreset_psci.c
> +++ b/drivers/sysreset/sysreset_psci.c
> @@ -18,7 +18,7 @@ static int psci_sysreset_request(struct udevice *dev, enum
> sysreset_t type)
>  	case SYSRESET_COLD:
>  		function_id = PSCI_0_2_FN_SYSTEM_RESET;
>  		break;
> -	case SYSRESET_POWER:
> +	case SYSRESET_POWER_OFF:
>  		function_id = PSCI_0_2_FN_SYSTEM_OFF;
>  		break;
>  	default:
> diff --git a/drivers/sysreset/sysreset_sandbox.c
> b/drivers/sysreset/sysreset_sandbox.c
> index 38e2a7e241..8bc9f4b4cc 100644
> --- a/drivers/sysreset/sysreset_sandbox.c
> +++ b/drivers/sysreset/sysreset_sandbox.c
> @@ -57,13 +57,13 @@ static int sandbox_sysreset_request(struct udevice *dev,
> enum sysreset_t type)
>  	case SYSRESET_COLD:
>  		state->last_sysreset = type;
>  		break;
> -	case SYSRESET_POWER:
> +	case SYSRESET_POWER_OFF:
>  		state->last_sysreset = type;
>  		if (!state->sysreset_allowed[type])
>  			return -EACCES;
>  		sandbox_exit();
>  		break;
> -	case SYSRESET_POWER_OFF:
> +	case SYSRESET_POWER:
>  		if (!state->sysreset_allowed[type])
>  			return -EACCES;
>  	default:
> --
> 2.21.0

Regards.

Patrick

[U-Boot] [PATCH 1/3] sysreset: switch to using SYSRESET_POWER_OFF for poweroff

[Urja Rannikko]

Hi,

On Thu, May 16, 2019 at 7:44 AM Patrick DELAUNAY
<patrick.delaunay@st.com> wrote:
>
> Hi Urja,
>
> > -     if (type != SYSRESET_POWER)
> > +     if (type != SYSRESET_POWER_OFF)
> >               return -EPROTONOSUPPORT;
>
> In fact in the next part of the code, we are supporting
> only SYSRESET_POWER (reset with PMIC1 switch OFF and restart)
> and not SYSRESET_POWER_OFF....
>
> because Power Cycle if RREQ_EN=1
>
> I think you need to remove the update on this file for your patch and
> I will modified this function is to support both mode....
Okay I will drop this part, but just to confirm... this means that
currently you have a configuration where using the poweroff command
causes a power-cycle? Neat.

-- 
Urja Rannikko

[U-Boot] [PATCH 2/3] sysreset: move stm32mp sysreset poweroff implementation to sysreset uclass

[Urja Rannikko]

Hi,

On Tue, May 14, 2019 at 4:07 PM Patrice CHOTARD <patrice.chotard@st.com> wrote:
>
> Hi Urja
>
> This patch doesn't compile using the stm32mp15_trusted_defconfig
> configuration:
>
> ...
>   LD      drivers/pinctrl/built-in.o
>   LD      drivers/core/built-in.o
>   LD      drivers/mmc/built-in.o
>   LD      drivers/built-in.o
> drivers/sysreset/built-in.o: In function `do_poweroff':
> /local/home/nxp11987/projects/community/u-boot.denx/drivers/sysreset/sysreset-uclass.c:123:
> multiple definition of `do_poweroff'
> drivers/firmware/built-in.o:/local/home/nxp11987/projects/community/u-boot.denx/drivers/firmware/psci.c:134:
> first defined here
> make[1]: *** [drivers/built-in.o] Error 1
> make: *** [drivers] Error 2
> build error !!
>
>
> I suggest you to update your patch with :
>
> config PMIC_STPMIC1
>         bool "Enable support for STMicroelectronics STPMIC1 PMIC"
>         depends on DM_PMIC && DM_I2C
> -       select SYSRESET_CMD_POWEROFF if CMD_POWEROFF
> +       select SYSRESET_CMD_POWEROFF if CMD_POWEROFF && !ARM_PSCI_FW
>         ---help---
>         The STPMIC1 PMIC provides 4 BUCKs, 6 LDOs, 1 VREF and 2 power switches.
>         It is accessed via an I2C interface. The device is used with STM32MP1
>         SoCs. This driver implements register read/write operations.

I'll do that in v2, thanks for the test.

-- 
Urja Rannikko

[U-Boot] [PATCH 3/3] rk8xx: add a sysreset driver for poweroff

[Simon Glass]

On Mon, 13 May 2019 at 06:17, Urja Rannikko <urjaman@gmail.com> wrote:
>
> Based on snooping around the linux kernel rk8xx driver.
> Tested on an ASUS C201.
>
> Signed-off-by: Urja Rannikko <urjaman@gmail.com>
> ---
>  drivers/power/pmic/Kconfig |  1 +
>  drivers/power/pmic/rk8xx.c | 62 +++++++++++++++++++++++++++++++++++---
>  include/power/rk8xx_pmic.h |  4 +++
>  3 files changed, 63 insertions(+), 4 deletions(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 1/3] sysreset: switch to using SYSRESET_POWER_OFF for poweroff

[Patrick DELAUNAY]

Hi Urja,

> 
> Hi,
> 
> On Thu, May 16, 2019 at 7:44 AM Patrick DELAUNAY
> <patrick.delaunay@st.com> wrote:
> >
> > Hi Urja,
> >
> > > -     if (type != SYSRESET_POWER)
> > > +     if (type != SYSRESET_POWER_OFF)
> > >               return -EPROTONOSUPPORT;
> >
> > In fact in the next part of the code, we are supporting only
> > SYSRESET_POWER (reset with PMIC1 switch OFF and restart) and not
> > SYSRESET_POWER_OFF....
> >
> > because Power Cycle if RREQ_EN=1
> >
> > I think you need to remove the update on this file for your patch and
> > I will modified this function is to support both mode....
> Okay I will drop this part, but just to confirm... this means that currently you have
> a configuration where using the poweroff command causes a power-cycle? Neat.

Yes and it was stange.

I choose it at the beginning of the project because for some hardware
configuration with STPMIC1, the VDD continue to be provided by the cell even 
if the power supply is still present in power off mode; HW team ask me to avoid this case,
to avoid the cell usage.

But it is more a workaround for the bad HW configuration, and I forget this point when I upstream the driver...

Your patch allow me to correct this point now (with http://patchwork.ozlabs.org/patch/1101856/) and
I come back to a normal behavior in the driver : driver execute the requested command.

Regards
Patrick

> --
> Urja Rannikko

[U-Boot] [PATCH 1/3] sysreset: switch to using SYSRESET_POWER_OFF for poweroff

[Kever Yang]

Hi Urja and Simon,

Does this patch still needed? Since this patch delegate to u-boot-rockchip
maintainer Philipp now,
I would like to know what should I do for this patch.

Thanks,
- Kever

Patrick DELAUNAY <patrick.delaunay@st.com> 于2019年5月20日周一 下午9:38写道：

> Hi Urja,
>
> >
> > Hi,
> >
> > On Thu, May 16, 2019 at 7:44 AM Patrick DELAUNAY
> > <patrick.delaunay@st.com> wrote:
> > >
> > > Hi Urja,
> > >
> > > > -     if (type != SYSRESET_POWER)
> > > > +     if (type != SYSRESET_POWER_OFF)
> > > >               return -EPROTONOSUPPORT;
> > >
> > > In fact in the next part of the code, we are supporting only
> > > SYSRESET_POWER (reset with PMIC1 switch OFF and restart) and not
> > > SYSRESET_POWER_OFF....
> > >
> > > because Power Cycle if RREQ_EN=1
> > >
> > > I think you need to remove the update on this file for your patch and
> > > I will modified this function is to support both mode....
> > Okay I will drop this part, but just to confirm... this means that
> currently you have
> > a configuration where using the poweroff command causes a power-cycle?
> Neat.
>
> Yes and it was stange.
>
> I choose it at the beginning of the project because for some hardware
> configuration with STPMIC1, the VDD continue to be provided by the cell
> even
> if the power supply is still present in power off mode; HW team ask me to
> avoid this case,
> to avoid the cell usage.
>
> But it is more a workaround for the bad HW configuration, and I forget
> this point when I upstream the driver...
>
> Your patch allow me to correct this point now (with
> http://patchwork.ozlabs.org/patch/1101856/) and
> I come back to a normal behavior in the driver : driver execute the
> requested command.
>
> Regards
> Patrick
>
> > --
> > Urja Rannikko
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
>