Hi, On Thu, Jan 26, 2023 at 06:11:21PM +0100, Neil Armstrong wrote: > The Hardkernel Odroid Go Ultra poweroff scheme requires requesting a poweroff > to its two PMICs in order, this represents the poweroff scheme needed to complete > a clean poweroff of the system. > > This implement this scheme by implementing a self registering driver to permit > using probe defer until both pmics are finally probed. > > Signed-off-by: Neil Armstrong > --- > Previous submission was at [1], but I converted it to an independent > platform device with device auto registration to permit waiting for > both the PMICs drivers to probe. > > [1] https://lore.kernel.org/all/20221031-b4-odroid-go-ultra-initial-v1-2-42e3dbea86d5@linaro.org/ > --- > Changes in v2: > - Switched to devm_register_sys_off_handler() > - Link to v1: https://lore.kernel.org/r/20221031-b4-odroid-go-ultra-initial-v1-2-42e3dbea86d5@linaro.org > --- > drivers/power/reset/Kconfig | 7 ++ > drivers/power/reset/Makefile | 1 + > drivers/power/reset/odroid-go-ultra-poweroff.c | 151 +++++++++++++++++++++++++ > 3 files changed, 159 insertions(+) > > diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig > index a8c46ba5878f..26860c2e05a9 100644 > --- a/drivers/power/reset/Kconfig > +++ b/drivers/power/reset/Kconfig > @@ -141,6 +141,13 @@ config POWER_RESET_OCELOT_RESET > help > This driver supports restart for Microsemi Ocelot SoC and similar. > > +config POWER_RESET_ODROID_GO_ULTRA_POWEROFF > + bool "Odroid Go Ultra power-off driver" > + depends on ARCH_MESON || COMPILE_TEST > + depends on MFD_RK808 You are not accessing any symbols from the MFD driver, so you can append "|| COMPILE_TEST". It will be fun to coordinate the merging, because of my series renaming it to MFD_RK8XX :/ https://lore.kernel.org/all/20230127181244.160887-1-sebastian.reichel@collabora.com/ > + help > + This driver supports Power off for Odroid Go Ultra device. > + > config POWER_RESET_OXNAS > bool "OXNAS SoC restart driver" > depends on ARCH_OXNAS > diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile > index 0a39424fc558..d763e6735ee3 100644 > --- a/drivers/power/reset/Makefile > +++ b/drivers/power/reset/Makefile > @@ -17,6 +17,7 @@ obj-$(CONFIG_POWER_RESET_MT6323) += mt6323-poweroff.o > obj-$(CONFIG_POWER_RESET_OXNAS) += oxnas-restart.o > obj-$(CONFIG_POWER_RESET_QCOM_PON) += qcom-pon.o > obj-$(CONFIG_POWER_RESET_OCELOT_RESET) += ocelot-reset.o > +obj-$(CONFIG_POWER_RESET_ODROID_GO_ULTRA_POWEROFF) += odroid-go-ultra-poweroff.o > obj-$(CONFIG_POWER_RESET_PIIX4_POWEROFF) += piix4-poweroff.o > obj-$(CONFIG_POWER_RESET_LTC2952) += ltc2952-poweroff.o > obj-$(CONFIG_POWER_RESET_QNAP) += qnap-poweroff.o > diff --git a/drivers/power/reset/odroid-go-ultra-poweroff.c b/drivers/power/reset/odroid-go-ultra-poweroff.c > new file mode 100644 > index 000000000000..51f54e65c927 > --- /dev/null > +++ b/drivers/power/reset/odroid-go-ultra-poweroff.c > @@ -0,0 +1,151 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (c) 2023 Neil Armstrong > + */ > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +/* > + * The Odroid Go Ultra has 2 PMICs: > + * - RK818 (manages the battery and USB-C power supply) > + * - RK817 > + * Both PMICs feeds power to the S922X SoC, so they must be powered-off in sequence. > + * Vendor does power-off the RK817 first, then the RK818 so here we follow this sequence. > + */ > + > +struct odroid_go_ultra_poweroff_data { > + struct device *dev; > + struct rk808 *rk817; > + struct rk808 *rk818; > +}; > + > +static int odroid_go_ultra_poweroff_prepare(struct sys_off_data *data) > +{ > + struct odroid_go_ultra_poweroff_data *poweroff_data = data->cb_data; > + int ret; > + > + dev_info(poweroff_data->dev, "Setting PMICs for power off"); > + > + /* RK817 */ > + ret = regmap_update_bits(poweroff_data->rk817->regmap, RK817_SYS_CFG(3), DEV_OFF, DEV_OFF); > + if (ret) { > + dev_err(poweroff_data->dev, "failed to poweroff rk817\n"); > + return notifier_from_errno(ret); > + } > + > + /* RK818 */ > + ret = regmap_update_bits(poweroff_data->rk818->regmap, RK818_DEVCTRL_REG, DEV_OFF, DEV_OFF); > + if (ret) { > + dev_err(poweroff_data->dev, "failed to poweroff rk818\n"); > + return notifier_from_errno(ret); > + } > + > + return NOTIFY_OK; > +} > + > +static int odroid_go_ultra_poweroff_get_pmic_drvdata(const char *compatible, struct rk808 **pmic) > +{ > + struct device_node *pmic_node; > + struct i2c_client *pmic_client; > + > + pmic_node = of_find_compatible_node(NULL, NULL, compatible); > + if (!pmic_node) > + return -ENODEV; > + > + pmic_client = of_find_i2c_device_by_node(pmic_node); > + of_node_put(pmic_node); > + if (!pmic_client) > + return -EPROBE_DEFER; > + > + *pmic = i2c_get_clientdata(pmic_client); You are only using the regmap, so you can skip the rk808 struct step and directly get and store the regmap: *pmic = dev_get_regmap(&pmic_client->dev, NULL); > + put_device(&pmic_client->dev); Technically this allows releasing the PMIC device with the poweroff driver still probed. I guess it's mostly a theoretical thing in this case, but people tend to cargo copy. I suggest to store the struct device handle in odroid_go_ultra_poweroff_data and then use dev_get_regmap() in the poweroff handler. Finally add a put_device() via devm_add_action_or_reset(). > + if (!*pmic) > + return -EPROBE_DEFER; > + > + return 0; > +} > + > +static int odroid_go_ultra_poweroff_probe(struct platform_device *pdev) > +{ > + struct odroid_go_ultra_poweroff_data *poweroff_data; > + int ret; > + > + poweroff_data = devm_kzalloc(&pdev->dev, sizeof(*poweroff_data), GFP_KERNEL); > + if (!poweroff_data) > + return -ENOMEM; > + > + dev_set_drvdata(&pdev->dev, poweroff_data); > + > + /* RK818 */ > + ret = odroid_go_ultra_poweroff_get_pmic_drvdata("rockchip,rk818", > + &poweroff_data->rk818); > + if (ret) > + return dev_err_probe(&pdev->dev, ret, "failed to get rk818 mfd data\n"); > + > + /* RK817 */ > + ret = odroid_go_ultra_poweroff_get_pmic_drvdata("rockchip,rk817", > + &poweroff_data->rk817); > + if (ret) > + return dev_err_probe(&pdev->dev, ret, "failed to get rk817 mfd data\n"); > + > + /* Register as SYS_OFF_MODE_POWER_OFF_PREPARE because regmap_update_bits may sleep */ > + ret = devm_register_sys_off_handler(&pdev->dev, > + SYS_OFF_MODE_POWER_OFF_PREPARE, > + SYS_OFF_PRIO_DEFAULT, > + odroid_go_ultra_poweroff_prepare, > + poweroff_data); > + if (ret) > + return dev_err_probe(&pdev->dev, ret, "failed to register sys-off handler\n"); > + > + dev_info(&pdev->dev, "Registered Power-Off handler\n"); > + > + return 0; > +} > + > +static struct platform_device *pdev; > + > +static struct platform_driver odroid_go_ultra_poweroff_driver = { > + .driver = { > + .name = "odroid-go-ultra-poweroff", > + }, > + .probe = odroid_go_ultra_poweroff_probe, > +}; > + > +static int __init odroid_go_ultra_poweroff_init(void) > +{ > + int ret; > + > + /* Only create when running on the Odroid Go Ultra device */ > + if (!of_device_is_compatible(of_root, "hardkernel,odroid-go-ultra")) > + return -ENODEV; > + > + ret = platform_driver_register(&odroid_go_ultra_poweroff_driver); > + if (ret) > + return ret; > + > + pdev = platform_device_register_resndata(NULL, "odroid-go-ultra-poweroff", -1, > + NULL, 0, NULL, 0); > + > + return PTR_ERR_OR_ZERO(pdev); This is missing platform_driver_unregister() in the error path. > +} > + > +static void __exit odroid_go_ultra_poweroff_exit(void) > +{ if (!of_device_is_compatible(of_root, "hardkernel,odroid-go-ultra")) return; > + platform_device_unregister(pdev); > + platform_driver_unregister(&odroid_go_ultra_poweroff_driver); > +} > + > +module_init(odroid_go_ultra_poweroff_init); > +module_exit(odroid_go_ultra_poweroff_exit); > + > +MODULE_AUTHOR("Neil Armstrong "); > +MODULE_DESCRIPTION("Odroid Go Ultra poweroff driver"); > +MODULE_LICENSE("GPL"); Thanks, -- Sebastian