[PATCH v2 0/4] power: axp: Move poweroff to DM_PMIC/SYSRESET driver

Samuel Holland

25 Oct 2021 25 Oct '21

4 a.m.

This series replaces the do_poweroff() implementation from the non-DM AXP PMIC drivers with a sysreset uclass device, if sysreset is providing its own do_poweroff() definition.

After applying this series, plus the following additional series: https://patchwork.ozlabs.org/project/uboot/list/?series=259143 https://patchwork.ozlabs.org/project/uboot/list/?series=259151 https://patchwork.ozlabs.org/project/uboot/list/?series=259152 and further enabling three Kconfig options: SYSRESET SYSRESET_WATCHDOG SYSRESET_WATCHDOG_AUTO you will have removed exactly one (1) call to pmic_bus_write() from U-Boot, hopefully without breaking anything else. Once all of these series land, I will send a patch enabling those options and removing the old do_poweroff() function implementations. At that point we will be one quarter of the way to removing pmic_bus from U-Boot proper.

Note that because SYSRESET is disabled by default, each of those other series can be applied independently.

Changes in v2: - Rebased on top of u-boot/master (axp_pmic_bind)

Samuel Holland (4): include: axp_pmic: Add missing header guard definition include: axp_pmic: Include headers for all variants power: axp: Avoid do_poweroff conflict with sysreset power: pmic: axp: Implement poweroff via sysreset

-- 2.32.0

Show replies by date

Samuel Holland

25 Oct 25 Oct

4 a.m.

New subject: [PATCH v2 1/4] include: axp_pmic: Add missing header guard definition

This header attempted to avoid multiple inclusion using a header guard. But the preprocessor symbol was never defined, so the guard had no effect. Fix this by defining the symbol.

Signed-off-by: Samuel Holland samuel@sholland.org Reviewed-by: Andre Przywara andre.przywara@arm.com ---

(no changes since v1)

include/axp_pmic.h | 1 + 1 file changed, 1 insertion(+)

diff --git a/include/axp_pmic.h b/include/axp_pmic.h index 46a017d2ef..2eab18937b 100644 --- a/include/axp_pmic.h +++ b/include/axp_pmic.h @@ -5,6 +5,7 @@ * X-Powers AX Power Management IC support header */ #ifndef _AXP_PMIC_H_ +#define _AXP_PMIC_H_

#include <stdbool.h>

-- 2.32.0

Samuel Holland

4 a.m.

New subject: [PATCH v2 2/4] include: axp_pmic: Include headers for all variants

A single DM-based driver should be able to support some feature for several PMIC variants where the interface is the same. For example, all PMIC variants use the same register bit to trigger poweroff.

However, currently only definitions for a single PMIC are available at a time. This requires drivers to use #ifdefs and different indentifiers for each variant they support.

Let's simplify this by making register definitions for all variants available from the header. Then no preprocessor conditions are needed; the driver can use the register definition from any variant that supports the relevant feature.

An exception is the GPIO-related definitions, which do not use unique identifiers. So for now, keep them like before. They will be cleaned up along with the GPIO driver.

Signed-off-by: Samuel Holland samuel@sholland.org Reviewed-by: Andre Przywara andre.przywara@arm.com ---

(no changes since v1)

diff --git a/include/axp152.h b/include/axp152.h index c81f172502..10d845fec4 100644 --- a/include/axp152.h +++ b/include/axp152.h @@ -15,6 +15,7 @@ enum axp152_reg { #define AXP152_POWEROFF (1 << 7)

/* For axp_gpio.c */ +#ifdef CONFIG_AXP152_POWER #define AXP_GPIO0_CTRL 0x90 #define AXP_GPIO1_CTRL 0x91 #define AXP_GPIO2_CTRL 0x92 @@ -24,3 +25,4 @@ enum axp152_reg { #define AXP_GPIO_CTRL_INPUT 0x02 /* Input */ #define AXP_GPIO_STATE 0x97 #define AXP_GPIO_STATE_OFFSET 0 +#endif diff --git a/include/axp209.h b/include/axp209.h index f4f1b2fe56..30399a8d62 100644 --- a/include/axp209.h +++ b/include/axp209.h @@ -74,6 +74,7 @@ enum axp209_reg { #define AXP209_POWEROFF BIT(7)

/* For axp_gpio.c */ +#ifdef CONFIG_AXP209_POWER #define AXP_POWER_STATUS 0x00 #define AXP_POWER_STATUS_VBUS_PRESENT BIT(5) #define AXP_GPIO0_CTRL 0x90 @@ -84,3 +85,4 @@ enum axp209_reg { #define AXP_GPIO_CTRL_INPUT 0x02 /* Input */ #define AXP_GPIO_STATE 0x94 #define AXP_GPIO_STATE_OFFSET 4 +#endif diff --git a/include/axp221.h b/include/axp221.h index caffb910f4..a02e9b4f64 100644 --- a/include/axp221.h +++ b/include/axp221.h @@ -50,6 +50,7 @@ #define AXP221_SID 0x20

/* For axp_gpio.c */ +#ifdef CONFIG_AXP221_POWER #define AXP_POWER_STATUS 0x00 #define AXP_POWER_STATUS_VBUS_PRESENT (1 << 5) #define AXP_VBUS_IPSOUT 0x30 @@ -63,3 +64,4 @@ #define AXP_GPIO_CTRL_INPUT 0x02 /* Input */ #define AXP_GPIO_STATE 0x94 #define AXP_GPIO_STATE_OFFSET 0 +#endif diff --git a/include/axp809.h b/include/axp809.h index 86b2925330..430dbef622 100644 --- a/include/axp809.h +++ b/include/axp809.h @@ -44,6 +44,7 @@ #define AXP809_SHUTDOWN_POWEROFF (1 << 7)

/* For axp_gpio.c */ +#ifdef CONFIG_AXP809_POWER #define AXP_POWER_STATUS 0x00 #define AXP_POWER_STATUS_VBUS_PRESENT (1 << 5) #define AXP_VBUS_IPSOUT 0x30 @@ -57,3 +58,4 @@ #define AXP_GPIO_CTRL_INPUT 0x02 /* Input */ #define AXP_GPIO_STATE 0x94 #define AXP_GPIO_STATE_OFFSET 0 +#endif diff --git a/include/axp818.h b/include/axp818.h index b16fe0b152..8bac6b67ca 100644 --- a/include/axp818.h +++ b/include/axp818.h @@ -58,6 +58,7 @@ #define AXP818_SHUTDOWN_POWEROFF (1 << 7)

/* For axp_gpio.c */ +#ifdef CONFIG_AXP818_POWER #define AXP_POWER_STATUS 0x00 #define AXP_POWER_STATUS_VBUS_PRESENT (1 << 5) #define AXP_VBUS_IPSOUT 0x30 @@ -71,3 +72,4 @@ #define AXP_GPIO_CTRL_INPUT 0x02 /* Input */ #define AXP_GPIO_STATE 0x94 #define AXP_GPIO_STATE_OFFSET 0 +#endif diff --git a/include/axp_pmic.h b/include/axp_pmic.h index 2eab18937b..01ebba6347 100644 --- a/include/axp_pmic.h +++ b/include/axp_pmic.h @@ -9,24 +9,12 @@

#include <stdbool.h>

-#ifdef CONFIG_AXP152_POWER #include <axp152.h> -#endif -#ifdef CONFIG_AXP209_POWER #include <axp209.h> -#endif -#ifdef CONFIG_AXP221_POWER #include <axp221.h> -#endif -#ifdef CONFIG_AXP305_POWER #include <axp305.h> -#endif -#ifdef CONFIG_AXP809_POWER #include <axp809.h> -#endif -#ifdef CONFIG_AXP818_POWER #include <axp818.h> -#endif

#define AXP_PMIC_MODE_REG 0x3e #define AXP_PMIC_MODE_I2C 0x00

-- 2.32.0

Samuel Holland

4 a.m.

New subject: [PATCH v2 3/4] power: axp: Avoid do_poweroff conflict with sysreset

The sysreset uclass has an option to provide the do_poweroff() function. When that option is enabled, the AXP power drivers should not provide their own definition.

For the AXP305, which is paired with 64-bit systems where TF-A provides PSCI, there is another possible conflict with the PSCI firmware driver. This driver can be enabled even if CONFIG_PSCI_RESET is disabled, so make sure to use the right symbol in the condition.

Signed-off-by: Samuel Holland samuel@sholland.org Reviewed-by: Andre Przywara andre.przywara@arm.com ---

(no changes since v1)

diff --git a/drivers/power/axp152.c b/drivers/power/axp152.c index d6e36125c1..a93987c153 100644 --- a/drivers/power/axp152.c +++ b/drivers/power/axp152.c @@ -79,6 +79,7 @@ int axp_init(void) return 0; }

+#if !IS_ENABLED(CONFIG_SYSRESET_CMD_POWEROFF) int do_poweroff(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { pmic_bus_write(AXP152_SHUTDOWN, AXP152_POWEROFF); @@ -89,3 +90,4 @@ int do_poweroff(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) /* not reached */ return 0; } +#endif diff --git a/drivers/power/axp209.c b/drivers/power/axp209.c index ade531940b..3447b9f011 100644 --- a/drivers/power/axp209.c +++ b/drivers/power/axp209.c @@ -230,6 +230,7 @@ int axp_init(void) return 0; }

+#if !IS_ENABLED(CONFIG_SYSRESET_CMD_POWEROFF) int do_poweroff(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { pmic_bus_write(AXP209_SHUTDOWN, AXP209_POWEROFF); @@ -240,3 +241,4 @@ int do_poweroff(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) /* not reached */ return 0; } +#endif diff --git a/drivers/power/axp221.c b/drivers/power/axp221.c index 3446fe7365..d251c314b9 100644 --- a/drivers/power/axp221.c +++ b/drivers/power/axp221.c @@ -264,6 +264,7 @@ int axp_get_sid(unsigned int *sid) return 0; }

+#if !IS_ENABLED(CONFIG_SYSRESET_CMD_POWEROFF) int do_poweroff(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { pmic_bus_write(AXP221_SHUTDOWN, AXP221_SHUTDOWN_POWEROFF); @@ -274,3 +275,4 @@ int do_poweroff(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) /* not reached */ return 0; } +#endif diff --git a/drivers/power/axp305.c b/drivers/power/axp305.c index 0191e4d427..049ef07f74 100644 --- a/drivers/power/axp305.c +++ b/drivers/power/axp305.c @@ -69,7 +69,7 @@ int axp_init(void) return ret; }

-#ifndef CONFIG_PSCI_RESET +#if !CONFIG_IS_ENABLED(ARM_PSCI_FW) && !IS_ENABLED(CONFIG_SYSRESET_CMD_POWEROFF) int do_poweroff(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { pmic_bus_write(AXP305_SHUTDOWN, AXP305_POWEROFF); diff --git a/drivers/power/axp809.c b/drivers/power/axp809.c index 0396502cdb..d327a584de 100644 --- a/drivers/power/axp809.c +++ b/drivers/power/axp809.c @@ -219,6 +219,7 @@ int axp_init(void) return pmic_bus_init(); }

+#if !IS_ENABLED(CONFIG_SYSRESET_CMD_POWEROFF) int do_poweroff(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { pmic_bus_write(AXP809_SHUTDOWN, AXP809_SHUTDOWN_POWEROFF); @@ -229,3 +230,4 @@ int do_poweroff(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) /* not reached */ return 0; } +#endif diff --git a/drivers/power/axp818.c b/drivers/power/axp818.c index 2dc736467b..08286ea3b5 100644 --- a/drivers/power/axp818.c +++ b/drivers/power/axp818.c @@ -255,6 +255,7 @@ int axp_init(void) return 0; }

+#if !IS_ENABLED(CONFIG_SYSRESET_CMD_POWEROFF) int do_poweroff(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { pmic_bus_write(AXP818_SHUTDOWN, AXP818_SHUTDOWN_POWEROFF); @@ -265,3 +266,4 @@ int do_poweroff(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) /* not reached */ return 0; } +#endif

-- 2.32.0

Samuel Holland

4 a.m.

New subject: [PATCH v2 4/4] power: pmic: axp: Implement poweroff via sysreset

The AXP PMICs have the ability to power off the system. The existing code for this is duplicated for each PMIC variant, and uses the legacy non-DM "pmic_bus" interface. When SYSRESET is enabled, this can all be replaced with a sysreset device using the DM_PMIC interface.

Since the trigger bit is the same on all PMIC variants, use the register definitions from the oldest supported PMIC.

Signed-off-by: Samuel Holland samuel@sholland.org ---

Changes in v2: - Rebased on top of u-boot/master (axp_pmic_bind)

drivers/power/pmic/Kconfig | 2 ++ drivers/power/pmic/axp.c | 49 +++++++++++++++++++++++++++++++++++++- 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/drivers/power/pmic/Kconfig b/drivers/power/pmic/Kconfig index 92e2ace279..b9fda428df 100644 --- a/drivers/power/pmic/Kconfig +++ b/drivers/power/pmic/Kconfig @@ -66,6 +66,8 @@ config PMIC_ACT8846 config PMIC_AXP bool "Enable Driver Model for X-Powers AXP PMICs" depends on DM_I2C + select SYSRESET_CMD_POWEROFF if SYSRESET && CMD_POWEROFF + imply CMD_POWEROFF if SYSRESET help This config enables driver-model PMIC uclass features for X-Powers AXP152, AXP2xx, and AXP8xx PMICs. diff --git a/drivers/power/pmic/axp.c b/drivers/power/pmic/axp.c index 74c94bdb47..0f2b24a8b5 100644 --- a/drivers/power/pmic/axp.c +++ b/drivers/power/pmic/axp.c @@ -1,8 +1,37 @@ // SPDX-License-Identifier: GPL-2.0+

+#include <axp_pmic.h> #include <dm.h> +#include <dm/lists.h> #include <i2c.h> #include <power/pmic.h> +#include <sysreset.h> + +#if CONFIG_IS_ENABLED(SYSRESET) +static int axp_sysreset_request(struct udevice *dev, enum sysreset_t type) +{ + int ret; + + if (type != SYSRESET_POWER_OFF) + return -EPROTONOSUPPORT; + + ret = pmic_clrsetbits(dev->parent, AXP152_SHUTDOWN, 0, AXP152_POWEROFF); + if (ret < 0) + return ret; + + return -EINPROGRESS; +} + +static struct sysreset_ops axp_sysreset_ops = { + .request = axp_sysreset_request, +}; + +U_BOOT_DRIVER(axp_sysreset) = { + .name = "axp_sysreset", + .id = UCLASS_SYSRESET, + .ops = &axp_sysreset_ops, +}; +#endif

static int axp_pmic_reg_count(struct udevice *dev) { @@ -16,6 +45,24 @@ static struct dm_pmic_ops axp_pmic_ops = { .write = dm_i2c_write, };

+static int axp_pmic_bind(struct udevice *dev) +{ + int ret; + + ret = dm_scan_fdt_dev(dev); + if (ret) + return ret; + + if (CONFIG_IS_ENABLED(SYSRESET)) { + ret = device_bind_driver_to_node(dev, "axp_sysreset", "axp_sysreset", + dev_ofnode(dev), NULL); + if (ret) + return ret; + } + + return 0; +} + static const struct udevice_id axp_pmic_ids[] = { { .compatible = "x-powers,axp152" }, { .compatible = "x-powers,axp202" }, @@ -33,6 +80,6 @@ U_BOOT_DRIVER(axp_pmic) = { .name = "axp_pmic", .id = UCLASS_PMIC, .of_match = axp_pmic_ids, - .bind = dm_scan_fdt_dev, + .bind = axp_pmic_bind, .ops = &axp_pmic_ops, };

-- 2.32.0

Andre Przywara

4:09 p.m.

New subject: [PATCH v2 4/4] power: pmic: axp: Implement poweroff via sysreset

On Sun, 24 Oct 2021 21:00:10 -0500 Samuel Holland samuel@sholland.org wrote:

...

The AXP PMICs have the ability to power off the system. The existing code for this is duplicated for each PMIC variant, and uses the legacy non-DM "pmic_bus" interface. When SYSRESET is enabled, this can all be replaced with a sysreset device using the DM_PMIC interface.

Since the trigger bit is the same on all PMIC variants, use the register definitions from the oldest supported PMIC.

Signed-off-by: Samuel Holland samuel@sholland.org

Thanks for the quick turnaround!

Reviewed-by: Andre Przywara andre.przywara@arm.com

Cheers, Andre

...

Changes in v2:

Rebased on top of u-boot/master (axp_pmic_bind)

drivers/power/pmic/Kconfig | 2 ++ drivers/power/pmic/axp.c | 49 +++++++++++++++++++++++++++++++++++++- 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/drivers/power/pmic/Kconfig b/drivers/power/pmic/Kconfig index 92e2ace279..b9fda428df 100644 --- a/drivers/power/pmic/Kconfig +++ b/drivers/power/pmic/Kconfig @@ -66,6 +66,8 @@ config PMIC_ACT8846 config PMIC_AXP bool "Enable Driver Model for X-Powers AXP PMICs" depends on DM_I2C

select SYSRESET_CMD_POWEROFF if SYSRESET && CMD_POWEROFF

imply CMD_POWEROFF if SYSRESET help This config enables driver-model PMIC uclass features for X-Powers AXP152, AXP2xx, and AXP8xx PMICs.

diff --git a/drivers/power/pmic/axp.c b/drivers/power/pmic/axp.c index 74c94bdb47..0f2b24a8b5 100644 --- a/drivers/power/pmic/axp.c +++ b/drivers/power/pmic/axp.c @@ -1,8 +1,37 @@ // SPDX-License-Identifier: GPL-2.0+

+#include <axp_pmic.h> #include <dm.h> +#include <dm/lists.h> #include <i2c.h> #include <power/pmic.h> +#include <sysreset.h>

+#if CONFIG_IS_ENABLED(SYSRESET) +static int axp_sysreset_request(struct udevice *dev, enum sysreset_t type) +{
int ret;

if (type != SYSRESET_POWER_OFF)
return -EPROTONOSUPPORT;
ret = pmic_clrsetbits(dev->parent, AXP152_SHUTDOWN, 0, AXP152_POWEROFF);

if (ret < 0)
return ret;
return -EINPROGRESS;
+}

+static struct sysreset_ops axp_sysreset_ops = {

.request = axp_sysreset_request,

+};

+U_BOOT_DRIVER(axp_sysreset) = {

.name = "axp_sysreset",

.id = UCLASS_SYSRESET,

.ops = &axp_sysreset_ops,

+}; +#endif

static int axp_pmic_reg_count(struct udevice *dev) { @@ -16,6 +45,24 @@ static struct dm_pmic_ops axp_pmic_ops = { .write = dm_i2c_write, };

+static int axp_pmic_bind(struct udevice *dev) +{
int ret;

ret = dm_scan_fdt_dev(dev);

if (ret)
return ret;
if (CONFIG_IS_ENABLED(SYSRESET)) {
ret = device_bind_driver_to_node(dev, "axp_sysreset", "axp_sysreset",
				 dev_ofnode(dev), NULL);
if (ret)
	return ret;
}

return 0;
+}

static const struct udevice_id axp_pmic_ids[] = { { .compatible = "x-powers,axp152" }, { .compatible = "x-powers,axp202" }, @@ -33,6 +80,6 @@ U_BOOT_DRIVER(axp_pmic) = { .name = "axp_pmic", .id = UCLASS_PMIC, .of_match = axp_pmic_ids,

.bind = dm_scan_fdt_dev,

.bind = axp_pmic_bind, .ops = &axp_pmic_ops,

};

1294

Age (days ago)

1294

Last active (days ago)

List overview

Download

5 comments

2 participants

tags (0)

participants (2)

Andre Przywara
Samuel Holland