* [U-Boot] [PATCH v2 2/3] sysreset: move stm32mp sysreset poweroff implementation to sysreset uclass
2019-05-16 21:48 [U-Boot] [PATCH v2 1/3] sysreset: switch to using SYSRESET_POWER_OFF for poweroff Urja Rannikko
@ 2019-05-16 21:48 ` Urja Rannikko
2019-05-17 6:51 ` Patrice CHOTARD
2019-05-20 13:27 ` Patrick DELAUNAY
2019-05-16 21:48 ` [U-Boot] [PATCH v2 3/3] rk8xx: add a sysreset driver for poweroff Urja Rannikko
` (2 subsequent siblings)
3 siblings, 2 replies; 10+ messages in thread
From: Urja Rannikko @ 2019-05-16 21:48 UTC (permalink / raw)
To: u-boot
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>
---
v2: Fixup STPMIC1 config selection of CONFIG_SYSRESET_CMD_POWEROFF
---
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..52edb29b48 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 && !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
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [U-Boot] [PATCH v2 2/3] sysreset: move stm32mp sysreset poweroff implementation to sysreset uclass
2019-05-16 21:48 ` [U-Boot] [PATCH v2 2/3] sysreset: move stm32mp sysreset poweroff implementation to sysreset uclass Urja Rannikko
@ 2019-05-17 6:51 ` Patrice CHOTARD
2019-05-20 13:27 ` Patrick DELAUNAY
1 sibling, 0 replies; 10+ messages in thread
From: Patrice CHOTARD @ 2019-05-17 6:51 UTC (permalink / raw)
To: u-boot
Hi Urja
On 5/16/19 11:48 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>
> ---
> v2: Fixup STPMIC1 config selection of CONFIG_SYSRESET_CMD_POWEROFF
> ---
> 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..52edb29b48 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 && !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
> 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)
>
Reviewed-by: Patrice Chotard <patrice.chotard@st.com>
Thanks
^ permalink raw reply [flat|nested] 10+ messages in thread* [U-Boot] [PATCH v2 2/3] sysreset: move stm32mp sysreset poweroff implementation to sysreset uclass
2019-05-16 21:48 ` [U-Boot] [PATCH v2 2/3] sysreset: move stm32mp sysreset poweroff implementation to sysreset uclass Urja Rannikko
2019-05-17 6:51 ` Patrice CHOTARD
@ 2019-05-20 13:27 ` Patrick DELAUNAY
1 sibling, 0 replies; 10+ messages in thread
From: Patrick DELAUNAY @ 2019-05-20 13:27 UTC (permalink / raw)
To: u-boot
Hi Urja,
>
> 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>
Reviewed-by: Patrick Delaunay <patrick.delaunay@st.com>
Tested on stm32mp1-ev1 with 2 other corrections:
[U-Boot] pmic: stpmic1: add support for SYSRESET_POWER_OFF
http://patchwork.ozlabs.org/patch/1101856/
[U-Boot] sysreset: syscon: add support for power off
http://patchwork.ozlabs.org/patch/1101905/
Tested-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
> v2: Fixup STPMIC1 config selection of CONFIG_SYSRESET_CMD_POWEROFF
> ---
> 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..52edb29b48 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 &&
> !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
> 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
Regards
Patrick Delaunay
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH v2 3/3] rk8xx: add a sysreset driver for poweroff
2019-05-16 21:48 [U-Boot] [PATCH v2 1/3] sysreset: switch to using SYSRESET_POWER_OFF for poweroff Urja Rannikko
2019-05-16 21:48 ` [U-Boot] [PATCH v2 2/3] sysreset: move stm32mp sysreset poweroff implementation to sysreset uclass Urja Rannikko
@ 2019-05-16 21:48 ` Urja Rannikko
2019-08-14 2:46 ` Kever Yang
2019-05-18 16:08 ` [U-Boot] [PATCH v2 1/3] sysreset: switch to using SYSRESET_POWER_OFF for poweroff Simon Glass
2019-05-20 13:16 ` Patrick DELAUNAY
3 siblings, 1 reply; 10+ messages in thread
From: Urja Rannikko @ 2019-05-16 21:48 UTC (permalink / raw)
To: u-boot
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 52edb29b48..3f6e30d503 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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [U-Boot] [PATCH v2 3/3] rk8xx: add a sysreset driver for poweroff
2019-05-16 21:48 ` [U-Boot] [PATCH v2 3/3] rk8xx: add a sysreset driver for poweroff Urja Rannikko
@ 2019-08-14 2:46 ` Kever Yang
2019-08-14 19:35 ` Simon Glass
0 siblings, 1 reply; 10+ messages in thread
From: Kever Yang @ 2019-08-14 2:46 UTC (permalink / raw)
To: u-boot
Hi Urja, Simon,
This patch is not able to pass the sandbox_spl test, it reports:
[1] 26463 segmentation fault (core dumped) ./u-boot
The driver looks good to me, no idea what cause the issue.
Thanks,
- Kever
Urja Rannikko <urjaman@gmail.com> 于2019年5月17日周五 上午5:49写道：
> 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 52edb29b48..3f6e30d503 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 mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
>
^ permalink raw reply [flat|nested] 10+ messages in thread* [U-Boot] [PATCH v2 3/3] rk8xx: add a sysreset driver for poweroff
2019-08-14 2:46 ` Kever Yang
@ 2019-08-14 19:35 ` Simon Glass
2019-08-15 10:38 ` [U-Boot] [PATCH v2 3/3] rk8xx: add a sysreset driver for poweroff【请注意，邮件由sjg@google.com代发】 Kever Yang
0 siblings, 1 reply; 10+ messages in thread
From: Simon Glass @ 2019-08-14 19:35 UTC (permalink / raw)
To: u-boot
Hi Kever,
On Tue, 13 Aug 2019 at 20:46, Kever Yang <kever.yang@rock-chips.com> wrote:
>
> Hi Urja, Simon,
>
> This patch is not able to pass the sandbox_spl test, it reports:
> [1] 26463 segmentation fault (core dumped) ./u-boot
>
> The driver looks good to me, no idea what cause the issue.
>
> Thanks,
> - Kever
>
> Urja Rannikko <urjaman@gmail.com> 于2019年5月17日周五 上午5:49写道：
>>
>> 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(-)
>>
This driver is enabled for sandbox, although I doubt it is in the
device tree, so I'm not sure why it would be called. But if it is, and
it directly accesses memory, then it might be the reason.
You should run the test under gdb to see where it is crashing.
Regards,
Simon
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH v2 3/3] rk8xx: add a sysreset driver for poweroff【请注意，邮件由sjg@google.com代发】
2019-08-14 19:35 ` Simon Glass
@ 2019-08-15 10:38 ` Kever Yang
0 siblings, 0 replies; 10+ messages in thread
From: Kever Yang @ 2019-08-15 10:38 UTC (permalink / raw)
To: u-boot
Simon, Stephen,
On 2019/8/15 上午3:35, Simon Glass wrote:
> Hi Kever,
>
> On Tue, 13 Aug 2019 at 20:46, Kever Yang <kever.yang@rock-chips.com> wrote:
>> Hi Urja, Simon,
>>
>> This patch is not able to pass the sandbox_spl test, it reports:
>> [1] 26463 segmentation fault (core dumped) ./u-boot
>>
>> The driver looks good to me, no idea what cause the issue.
>>
>> Thanks,
>> - Kever
>>
>> Urja Rannikko <urjaman@gmail.com> 于2019年5月17日周五 上午5:49写道：
>>> 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(-)
>>>
> This driver is enabled for sandbox, although I doubt it is in the
> device tree, so I'm not sure why it would be called. But if it is, and
> it directly accesses memory, then it might be the reason.
>
> You should run the test under gdb to see where it is crashing.
gdb output is:
Program received signal SIGSEGV, Segmentation fault.
0x00005555556182c6 in strcmp (cs=cs at entry=0x55555566023b "root_driver",
ct=0x1 <error: Cannot access memory@address 0x1>)
at lib/string.c:190
190 if ((__res = *cs - *ct++) != 0 || !*cs++)
This does not help much for crashing reason, and I have narrow down the
cause, I believe the
crash related to "DM_GET_DRIVER(pmic_rk8xx)",
- if I replace the 'pmic_rk8xx' in DM_GET_DRIVER() to any of other
available driver, u-boot does not crash;
- if I move the new 'rk8xx_sysreset' driver to other files, eg.
pmic/sandbox.c, u-boot does not crash;
Any more suggestion, or could you help to cherry pick this patch, and
you should reproduce
this issue:
make sandbox_spl_defconfig all
./u-boot
Thanks,
- Kever
>
> Regards,
> Simon
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH v2 1/3] sysreset: switch to using SYSRESET_POWER_OFF for poweroff
2019-05-16 21:48 [U-Boot] [PATCH v2 1/3] sysreset: switch to using SYSRESET_POWER_OFF for poweroff Urja Rannikko
2019-05-16 21:48 ` [U-Boot] [PATCH v2 2/3] sysreset: move stm32mp sysreset poweroff implementation to sysreset uclass Urja Rannikko
2019-05-16 21:48 ` [U-Boot] [PATCH v2 3/3] rk8xx: add a sysreset driver for poweroff Urja Rannikko
@ 2019-05-18 16:08 ` Simon Glass
2019-05-20 13:16 ` Patrick DELAUNAY
3 siblings, 0 replies; 10+ messages in thread
From: Simon Glass @ 2019-05-18 16:08 UTC (permalink / raw)
To: u-boot
On Thu, 16 May 2019 at 15:48, Urja Rannikko <urjaman@gmail.com> wrote:
>
> 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.
SYSRESET_POWER means to do a power reset (removing and reinstating all power)
SYSRESET_POWER_OFF means to turn the device off and leave it off
>
> Signed-off-by: Urja Rannikko <urjaman@gmail.com>
> ---
> v2: Do not change STPMIC1 driver (thats actually SYSRESET_POWER)
>
> 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/sysreset/sysreset_psci.c | 2 +-
> drivers/sysreset/sysreset_sandbox.c | 4 ++--
> 4 files changed, 5 insertions(+), 5 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/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
>
I actually sent a patch to the sandbox driver...er no I didn't, oops.
I'll dig up the series and send it.
Regards,
Simopn
^ permalink raw reply [flat|nested] 10+ messages in thread* [U-Boot] [PATCH v2 1/3] sysreset: switch to using SYSRESET_POWER_OFF for poweroff
2019-05-16 21:48 [U-Boot] [PATCH v2 1/3] sysreset: switch to using SYSRESET_POWER_OFF for poweroff Urja Rannikko
` (2 preceding siblings ...)
2019-05-18 16:08 ` [U-Boot] [PATCH v2 1/3] sysreset: switch to using SYSRESET_POWER_OFF for poweroff Simon Glass
@ 2019-05-20 13:16 ` Patrick DELAUNAY
3 siblings, 0 replies; 10+ messages in thread
From: Patrick DELAUNAY @ 2019-05-20 13:16 UTC (permalink / raw)
To: u-boot
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>
Reviewed-by: Patrick Delaunay <patrick.delaunay@st.com>
For information: complete support of booth POWER sysreset case for stmpic1 is added by
http://patchwork.ozlabs.org/project/uboot/list/?series=108685
> ---
> v2: Do not change STPMIC1 driver (thats actually SYSRESET_POWER)
>
> 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/sysreset/sysreset_psci.c | 2 +-
> drivers/sysreset/sysreset_sandbox.c | 4 ++--
> 4 files changed, 5 insertions(+), 5 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/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
^ permalink raw reply [flat|nested] 10+ messages in thread