* [PATCH v2] power: reset: add Odroid Go Ultra poweroff driver @ 2023-01-26 17:11 ` Neil Armstrong 0 siblings, 0 replies; 7+ messages in thread From: Neil Armstrong @ 2023-01-26 17:11 UTC (permalink / raw) To: Sebastian Reichel; +Cc: linux-kernel, linux-pm, linux-amlogic, Neil Armstrong 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 <neil.armstrong@linaro.org> --- 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 + 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 <neil.armstrong@linaro.org> + */ +#include <linux/kernel.h> +#include <linux/init.h> +#include <linux/of_platform.h> +#include <linux/mfd/rk808.h> +#include <linux/regmap.h> +#include <linux/module.h> +#include <linux/reboot.h> +#include <linux/i2c.h> + +/* + * 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); + + put_device(&pmic_client->dev); + + 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); +} + +static void __exit odroid_go_ultra_poweroff_exit(void) +{ + 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 <neil.armstrong@linaro.org>"); +MODULE_DESCRIPTION("Odroid Go Ultra poweroff driver"); +MODULE_LICENSE("GPL"); --- base-commit: 1b929c02afd37871d5afb9d498426f83432e71c2 change-id: 20230126-b4-odroid-go-ultra-poweroff-c8fdca93f3eb Best regards, -- Neil Armstrong <neil.armstrong@linaro.org> ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2] power: reset: add Odroid Go Ultra poweroff driver @ 2023-01-26 17:11 ` Neil Armstrong 0 siblings, 0 replies; 7+ messages in thread From: Neil Armstrong @ 2023-01-26 17:11 UTC (permalink / raw) To: Sebastian Reichel; +Cc: linux-kernel, linux-pm, linux-amlogic, Neil Armstrong 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 <neil.armstrong@linaro.org> --- 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 + 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 <neil.armstrong@linaro.org> + */ +#include <linux/kernel.h> +#include <linux/init.h> +#include <linux/of_platform.h> +#include <linux/mfd/rk808.h> +#include <linux/regmap.h> +#include <linux/module.h> +#include <linux/reboot.h> +#include <linux/i2c.h> + +/* + * 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); + + put_device(&pmic_client->dev); + + 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); +} + +static void __exit odroid_go_ultra_poweroff_exit(void) +{ + 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 <neil.armstrong@linaro.org>"); +MODULE_DESCRIPTION("Odroid Go Ultra poweroff driver"); +MODULE_LICENSE("GPL"); --- base-commit: 1b929c02afd37871d5afb9d498426f83432e71c2 change-id: 20230126-b4-odroid-go-ultra-poweroff-c8fdca93f3eb Best regards, -- Neil Armstrong <neil.armstrong@linaro.org> _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] power: reset: add Odroid Go Ultra poweroff driver 2023-01-26 17:11 ` Neil Armstrong (?) @ 2023-01-28 18:26 ` kernel test robot -1 siblings, 0 replies; 7+ messages in thread From: kernel test robot @ 2023-01-28 18:26 UTC (permalink / raw) To: Neil Armstrong; +Cc: oe-kbuild-all Hi Neil, I love your patch! Yet something to improve: [auto build test ERROR on 1b929c02afd37871d5afb9d498426f83432e71c2] url: https://github.com/intel-lab-lkp/linux/commits/Neil-Armstrong/power-reset-add-Odroid-Go-Ultra-poweroff-driver/20230128-102059 base: 1b929c02afd37871d5afb9d498426f83432e71c2 patch link: https://lore.kernel.org/r/20230126-b4-odroid-go-ultra-poweroff-v2-1-a8c50866f4ac%40linaro.org patch subject: [PATCH v2] power: reset: add Odroid Go Ultra poweroff driver config: powerpc-allmodconfig (https://download.01.org/0day-ci/archive/20230129/202301290233.RQ7ZdXIh-lkp@intel.com/config) compiler: powerpc-linux-gcc (GCC) 12.1.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/633aeb721d45b3a71cfb06cb087ac36b3dc9f41a git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Neil-Armstrong/power-reset-add-Odroid-Go-Ultra-poweroff-driver/20230128-102059 git checkout 633aeb721d45b3a71cfb06cb087ac36b3dc9f41a # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=powerpc olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=powerpc SHELL=/bin/bash If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): powerpc-linux-ld: drivers/power/reset/odroid-go-ultra-poweroff.o: in function `odroid_go_ultra_poweroff_get_pmic_drvdata': >> odroid-go-ultra-poweroff.c:(.text.odroid_go_ultra_poweroff_get_pmic_drvdata+0x4c): undefined reference to `of_find_i2c_device_by_node' -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] power: reset: add Odroid Go Ultra poweroff driver 2023-01-26 17:11 ` Neil Armstrong @ 2023-01-29 23:47 ` Sebastian Reichel -1 siblings, 0 replies; 7+ messages in thread From: Sebastian Reichel @ 2023-01-29 23:47 UTC (permalink / raw) To: Neil Armstrong; +Cc: linux-kernel, linux-pm, linux-amlogic [-- Attachment #1: Type: text/plain, Size: 8531 bytes --] 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 <neil.armstrong@linaro.org> > --- > 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 <neil.armstrong@linaro.org> > + */ > +#include <linux/kernel.h> > +#include <linux/init.h> > +#include <linux/of_platform.h> > +#include <linux/mfd/rk808.h> > +#include <linux/regmap.h> > +#include <linux/module.h> > +#include <linux/reboot.h> > +#include <linux/i2c.h> > + > +/* > + * 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 <neil.armstrong@linaro.org>"); > +MODULE_DESCRIPTION("Odroid Go Ultra poweroff driver"); > +MODULE_LICENSE("GPL"); Thanks, -- Sebastian [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] power: reset: add Odroid Go Ultra poweroff driver @ 2023-01-29 23:47 ` Sebastian Reichel 0 siblings, 0 replies; 7+ messages in thread From: Sebastian Reichel @ 2023-01-29 23:47 UTC (permalink / raw) To: Neil Armstrong; +Cc: linux-kernel, linux-pm, linux-amlogic [-- Attachment #1.1: Type: text/plain, Size: 8531 bytes --] 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 <neil.armstrong@linaro.org> > --- > 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 <neil.armstrong@linaro.org> > + */ > +#include <linux/kernel.h> > +#include <linux/init.h> > +#include <linux/of_platform.h> > +#include <linux/mfd/rk808.h> > +#include <linux/regmap.h> > +#include <linux/module.h> > +#include <linux/reboot.h> > +#include <linux/i2c.h> > + > +/* > + * 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 <neil.armstrong@linaro.org>"); > +MODULE_DESCRIPTION("Odroid Go Ultra poweroff driver"); > +MODULE_LICENSE("GPL"); Thanks, -- Sebastian [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] [-- Attachment #2: Type: text/plain, Size: 167 bytes --] _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] power: reset: add Odroid Go Ultra poweroff driver 2023-01-29 23:47 ` Sebastian Reichel @ 2023-01-30 8:31 ` Neil Armstrong -1 siblings, 0 replies; 7+ messages in thread From: Neil Armstrong @ 2023-01-30 8:31 UTC (permalink / raw) To: Sebastian Reichel; +Cc: linux-kernel, linux-pm, linux-amlogic On 30/01/2023 00:47, Sebastian Reichel wrote: > 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 <neil.armstrong@linaro.org> >> --- >> 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/ If I do not depend anymore on `struct rk808`, so I don't depend anymore on it, but if it doesn't probe I won't get the regmap from the i2c device. So it will be a runtime dependency instead, so it's ok to drop. But I'll need to add a depend to guard of_find_i2c_device_by_node() > >> + 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 <neil.armstrong@linaro.org> >> + */ >> +#include <linux/kernel.h> >> +#include <linux/init.h> >> +#include <linux/of_platform.h> >> +#include <linux/mfd/rk808.h> >> +#include <linux/regmap.h> >> +#include <linux/module.h> >> +#include <linux/reboot.h> >> +#include <linux/i2c.h> >> + >> +/* >> + * 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); Indeed ti makes things much simpler. > >> + 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(). Indeed, this makes it much cleaner. > >> + 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. Ack > >> +} >> + >> +static void __exit odroid_go_ultra_poweroff_exit(void) >> +{ > > if (!of_device_is_compatible(of_root, "hardkernel,odroid-go-ultra")) > return; Ack > >> + 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 <neil.armstrong@linaro.org>"); >> +MODULE_DESCRIPTION("Odroid Go Ultra poweroff driver"); >> +MODULE_LICENSE("GPL"); > > Thanks, > > -- Sebastian Thanks, Neil ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] power: reset: add Odroid Go Ultra poweroff driver @ 2023-01-30 8:31 ` Neil Armstrong 0 siblings, 0 replies; 7+ messages in thread From: Neil Armstrong @ 2023-01-30 8:31 UTC (permalink / raw) To: Sebastian Reichel; +Cc: linux-kernel, linux-pm, linux-amlogic On 30/01/2023 00:47, Sebastian Reichel wrote: > 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 <neil.armstrong@linaro.org> >> --- >> 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/ If I do not depend anymore on `struct rk808`, so I don't depend anymore on it, but if it doesn't probe I won't get the regmap from the i2c device. So it will be a runtime dependency instead, so it's ok to drop. But I'll need to add a depend to guard of_find_i2c_device_by_node() > >> + 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 <neil.armstrong@linaro.org> >> + */ >> +#include <linux/kernel.h> >> +#include <linux/init.h> >> +#include <linux/of_platform.h> >> +#include <linux/mfd/rk808.h> >> +#include <linux/regmap.h> >> +#include <linux/module.h> >> +#include <linux/reboot.h> >> +#include <linux/i2c.h> >> + >> +/* >> + * 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); Indeed ti makes things much simpler. > >> + 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(). Indeed, this makes it much cleaner. > >> + 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. Ack > >> +} >> + >> +static void __exit odroid_go_ultra_poweroff_exit(void) >> +{ > > if (!of_device_is_compatible(of_root, "hardkernel,odroid-go-ultra")) > return; Ack > >> + 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 <neil.armstrong@linaro.org>"); >> +MODULE_DESCRIPTION("Odroid Go Ultra poweroff driver"); >> +MODULE_LICENSE("GPL"); > > Thanks, > > -- Sebastian Thanks, Neil _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-01-30 17:26 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-01-26 17:11 [PATCH v2] power: reset: add Odroid Go Ultra poweroff driver Neil Armstrong 2023-01-26 17:11 ` Neil Armstrong 2023-01-28 18:26 ` kernel test robot 2023-01-29 23:47 ` Sebastian Reichel 2023-01-29 23:47 ` Sebastian Reichel 2023-01-30 8:31 ` Neil Armstrong 2023-01-30 8:31 ` Neil Armstrong
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.