Hello all,
Explanation is in patch #1.
The rest of this series is a set of representative examples of converting
away from the old API. They should be valid and can be taken by their
respective maintainers even if patch #1 doesn't find acceptance.
Thanks,
Andrew
Changes from RFC:
- Add Reviewed-by tag
- Rebase on v6.8-rc1 and drop taken patch
Andrew Davis (4):
kernel/reboot: Deprecate register_restart_handler()
drivers/soc/litex: Use devm_register_restart_handler()
firmware: psci: Use register_sys_off_handler(SYS_OFF_MODE_RESTART)
firmware: ti_sci: Use devm_register_restart_handler()
drivers/firmware/psci/psci.c | 10 ++--------
drivers/firmware/ti_sci.c | 14 +++++---------
drivers/soc/litex/litex_soc_ctrl.c | 23 +++++------------------
include/linux/reboot.h | 8 ++++++--
kernel/reboot.c | 3 +++
5 files changed, 21 insertions(+), 37 deletions(-)
--
2.39.2
Function register_restart_handler() is deprecated. Using this new API
removes our need to keep and manage a struct notifier_block.
Signed-off-by: Andrew Davis <[email protected]>
---
drivers/firmware/psci/psci.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
index d9629ff878619..767a5af5384b4 100644
--- a/drivers/firmware/psci/psci.c
+++ b/drivers/firmware/psci/psci.c
@@ -305,8 +305,7 @@ static int get_set_conduit_method(const struct device_node *np)
return 0;
}
-static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
- void *data)
+static int psci_sys_reset(struct sys_off_data *data)
{
if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
psci_system_reset2_supported) {
@@ -323,11 +322,6 @@ static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
return NOTIFY_DONE;
}
-static struct notifier_block psci_sys_reset_nb = {
- .notifier_call = psci_sys_reset,
- .priority = 129,
-};
-
static void psci_sys_poweroff(void)
{
invoke_psci_fn(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0);
@@ -623,7 +617,7 @@ static void __init psci_0_2_set_functions(void)
.migrate_info_type = psci_migrate_info_type,
};
- register_restart_handler(&psci_sys_reset_nb);
+ register_sys_off_handler(SYS_OFF_MODE_RESTART, 129, psci_sys_reset, NULL);
pm_power_off = psci_sys_poweroff;
}
--
2.39.2
There are now two ways to add a handler to the restart_handler_list.
Two ways to do the same thing is bad design, so let's unify on using
the new method register_sys_off_handler() everywhere.
Reasons:
* The other register_*_handler functions take a callback, this old
API takes a notifier_block, which makes it confusing with the
register_*_notifier class of functions.
* register_sys_off_handler (new API) is a more unified API allowing for
registering several system off types.
* The new API has more helpers built around it now, including devm and
platform helpers.
* The new API manages the struct notifier_block for us, simplifying using
code.
Mark register_restart_handler() as deprecated to try to warn off new users
while we finish converting the remaining existing users.
Signed-off-by: Andrew Davis <[email protected]>
---
include/linux/reboot.h | 8 ++++++--
kernel/reboot.c | 3 +++
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/include/linux/reboot.h b/include/linux/reboot.h
index abcdde4df6979..186291e9bf3bb 100644
--- a/include/linux/reboot.h
+++ b/include/linux/reboot.h
@@ -46,8 +46,12 @@ extern int unregister_reboot_notifier(struct notifier_block *);
extern int devm_register_reboot_notifier(struct device *, struct notifier_block *);
-extern int register_restart_handler(struct notifier_block *);
-extern int unregister_restart_handler(struct notifier_block *);
+/*
+ * This function is deprecated, use register_sys_off_handler(SYS_OFF_MODE_RESTART)
+ * or devm_register_restart_handler() instead.
+ */
+extern int __deprecated register_restart_handler(struct notifier_block *);
+extern int __deprecated unregister_restart_handler(struct notifier_block *);
extern void do_kernel_restart(char *cmd);
/*
diff --git a/kernel/reboot.c b/kernel/reboot.c
index 22c16e2564cca..38d6933fe892e 100644
--- a/kernel/reboot.c
+++ b/kernel/reboot.c
@@ -197,6 +197,9 @@ static ATOMIC_NOTIFIER_HEAD(restart_handler_list);
*
* Currently always returns zero, as atomic_notifier_chain_register()
* always returns zero.
+ *
+ * This function is deprecated, use register_sys_off_handler(SYS_OFF_MODE_RESTART)
+ * or devm_register_restart_handler() instead.
*/
int register_restart_handler(struct notifier_block *nb)
{
--
2.39.2
Use device life-cycle managed register function to simplify probe error
path and eliminate need for explicit remove function.
Signed-off-by: Andrew Davis <[email protected]>
Reviewed-by: Gabriel Somlo <[email protected]>
---
drivers/soc/litex/litex_soc_ctrl.c | 23 +++++------------------
1 file changed, 5 insertions(+), 18 deletions(-)
diff --git a/drivers/soc/litex/litex_soc_ctrl.c b/drivers/soc/litex/litex_soc_ctrl.c
index 10813299aa106..7a0096d93c73d 100644
--- a/drivers/soc/litex/litex_soc_ctrl.c
+++ b/drivers/soc/litex/litex_soc_ctrl.c
@@ -69,14 +69,11 @@ static int litex_check_csr_access(void __iomem *reg_addr)
struct litex_soc_ctrl_device {
void __iomem *base;
- struct notifier_block reset_nb;
};
-static int litex_reset_handler(struct notifier_block *this, unsigned long mode,
- void *cmd)
+static int litex_reset_handler(struct sys_off_data *data)
{
- struct litex_soc_ctrl_device *soc_ctrl_dev =
- container_of(this, struct litex_soc_ctrl_device, reset_nb);
+ struct litex_soc_ctrl_device *soc_ctrl_dev = data->cb_data;
litex_write32(soc_ctrl_dev->base + RESET_REG_OFF, RESET_REG_VALUE);
return NOTIFY_DONE;
@@ -107,11 +104,9 @@ static int litex_soc_ctrl_probe(struct platform_device *pdev)
if (error)
return error;
- platform_set_drvdata(pdev, soc_ctrl_dev);
-
- soc_ctrl_dev->reset_nb.notifier_call = litex_reset_handler;
- soc_ctrl_dev->reset_nb.priority = 128;
- error = register_restart_handler(&soc_ctrl_dev->reset_nb);
+ error = devm_register_restart_handler(&pdev->dev,
+ litex_reset_handler,
+ soc_ctrl_dev);
if (error) {
dev_warn(&pdev->dev, "cannot register restart handler: %d\n",
error);
@@ -120,20 +115,12 @@ static int litex_soc_ctrl_probe(struct platform_device *pdev)
return 0;
}
-static void litex_soc_ctrl_remove(struct platform_device *pdev)
-{
- struct litex_soc_ctrl_device *soc_ctrl_dev = platform_get_drvdata(pdev);
-
- unregister_restart_handler(&soc_ctrl_dev->reset_nb);
-}
-
static struct platform_driver litex_soc_ctrl_driver = {
.driver = {
.name = "litex-soc-controller",
.of_match_table = of_match_ptr(litex_soc_ctrl_of_match)
},
.probe = litex_soc_ctrl_probe,
- .remove_new = litex_soc_ctrl_remove,
};
module_platform_driver(litex_soc_ctrl_driver);
--
2.39.2
Use device life-cycle managed register function to simplify probe.
Signed-off-by: Andrew Davis <[email protected]>
---
drivers/firmware/ti_sci.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 8b9a2556de16d..16501aa0b84cf 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -103,7 +103,6 @@ struct ti_sci_desc {
*/
struct ti_sci_info {
struct device *dev;
- struct notifier_block nb;
const struct ti_sci_desc *desc;
struct dentry *d;
void __iomem *debug_region;
@@ -122,7 +121,6 @@ struct ti_sci_info {
#define cl_to_ti_sci_info(c) container_of(c, struct ti_sci_info, cl)
#define handle_to_ti_sci_info(h) container_of(h, struct ti_sci_info, handle)
-#define reboot_to_ti_sci_info(n) container_of(n, struct ti_sci_info, nb)
#ifdef CONFIG_DEBUG_FS
@@ -3254,10 +3252,9 @@ devm_ti_sci_get_resource(const struct ti_sci_handle *handle, struct device *dev,
}
EXPORT_SYMBOL_GPL(devm_ti_sci_get_resource);
-static int tisci_reboot_handler(struct notifier_block *nb, unsigned long mode,
- void *cmd)
+static int tisci_reboot_handler(struct sys_off_data *data)
{
- struct ti_sci_info *info = reboot_to_ti_sci_info(nb);
+ struct ti_sci_info *info = data->cb_data;
const struct ti_sci_handle *handle = &info->handle;
ti_sci_cmd_core_reboot(handle);
@@ -3400,10 +3397,9 @@ static int ti_sci_probe(struct platform_device *pdev)
ti_sci_setup_ops(info);
if (reboot) {
- info->nb.notifier_call = tisci_reboot_handler;
- info->nb.priority = 128;
-
- ret = register_restart_handler(&info->nb);
+ ret = devm_register_restart_handler(dev,
+ tisci_reboot_handler,
+ info);
if (ret) {
dev_err(dev, "reboot registration fail(%d)\n", ret);
goto out;
--
2.39.2
On Tue, Jan 23, 2024 at 10:44:43AM -0600, Andrew Davis wrote:
> Use device life-cycle managed register function to simplify probe.
>
> Signed-off-by: Andrew Davis <[email protected]>
Reviewed-by: Gabriel Somlo <[email protected]>
> ---
> drivers/firmware/ti_sci.c | 14 +++++---------
> 1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
> index 8b9a2556de16d..16501aa0b84cf 100644
> --- a/drivers/firmware/ti_sci.c
> +++ b/drivers/firmware/ti_sci.c
> @@ -103,7 +103,6 @@ struct ti_sci_desc {
> */
> struct ti_sci_info {
> struct device *dev;
> - struct notifier_block nb;
> const struct ti_sci_desc *desc;
> struct dentry *d;
> void __iomem *debug_region;
> @@ -122,7 +121,6 @@ struct ti_sci_info {
>
> #define cl_to_ti_sci_info(c) container_of(c, struct ti_sci_info, cl)
> #define handle_to_ti_sci_info(h) container_of(h, struct ti_sci_info, handle)
> -#define reboot_to_ti_sci_info(n) container_of(n, struct ti_sci_info, nb)
>
> #ifdef CONFIG_DEBUG_FS
>
> @@ -3254,10 +3252,9 @@ devm_ti_sci_get_resource(const struct ti_sci_handle *handle, struct device *dev,
> }
> EXPORT_SYMBOL_GPL(devm_ti_sci_get_resource);
>
> -static int tisci_reboot_handler(struct notifier_block *nb, unsigned long mode,
> - void *cmd)
> +static int tisci_reboot_handler(struct sys_off_data *data)
> {
> - struct ti_sci_info *info = reboot_to_ti_sci_info(nb);
> + struct ti_sci_info *info = data->cb_data;
> const struct ti_sci_handle *handle = &info->handle;
>
> ti_sci_cmd_core_reboot(handle);
> @@ -3400,10 +3397,9 @@ static int ti_sci_probe(struct platform_device *pdev)
> ti_sci_setup_ops(info);
>
> if (reboot) {
> - info->nb.notifier_call = tisci_reboot_handler;
> - info->nb.priority = 128;
> -
> - ret = register_restart_handler(&info->nb);
> + ret = devm_register_restart_handler(dev,
> + tisci_reboot_handler,
> + info);
> if (ret) {
> dev_err(dev, "reboot registration fail(%d)\n", ret);
> goto out;
> --
> 2.39.2
>
Hi Andrew,
kernel test robot noticed the following build warnings:
[auto build test WARNING on lee-leds/for-leds-next]
[also build test WARNING on linus/master v6.8-rc1 next-20240125]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Andrew-Davis/kernel-reboot-Deprecate-register_restart_handler/20240124-005424
base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/leds.git for-leds-next
patch link: https://lore.kernel.org/r/20240123164443.394642-5-afd%40ti.com
patch subject: [PATCH 4/4] firmware: ti_sci: Use devm_register_restart_handler()
config: arm64-defconfig (https://download.01.org/0day-ci/archive/20240129/[email protected]/config)
compiler: aarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240129/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
>> drivers/firmware/ti_sci.c:120: warning: Excess struct member 'nb' description in 'ti_sci_info'
vim +120 drivers/firmware/ti_sci.c
aa276781a64a5f Nishanth Menon 2016-10-18 85
aa276781a64a5f Nishanth Menon 2016-10-18 86 /**
aa276781a64a5f Nishanth Menon 2016-10-18 87 * struct ti_sci_info - Structure representing a TI SCI instance
aa276781a64a5f Nishanth Menon 2016-10-18 88 * @dev: Device pointer
aa276781a64a5f Nishanth Menon 2016-10-18 89 * @desc: SoC description for this instance
912cffb4ed8612 Nishanth Menon 2016-10-18 90 * @nb: Reboot Notifier block
aa276781a64a5f Nishanth Menon 2016-10-18 91 * @d: Debugfs file entry
aa276781a64a5f Nishanth Menon 2016-10-18 92 * @debug_region: Memory region where the debug message are available
aa276781a64a5f Nishanth Menon 2016-10-18 93 * @debug_region_size: Debug region size
aa276781a64a5f Nishanth Menon 2016-10-18 94 * @debug_buffer: Buffer allocated to copy debug messages.
aa276781a64a5f Nishanth Menon 2016-10-18 95 * @handle: Instance of TI SCI handle to send to clients.
aa276781a64a5f Nishanth Menon 2016-10-18 96 * @cl: Mailbox Client
aa276781a64a5f Nishanth Menon 2016-10-18 97 * @chan_tx: Transmit mailbox channel
aa276781a64a5f Nishanth Menon 2016-10-18 98 * @chan_rx: Receive mailbox channel
aa276781a64a5f Nishanth Menon 2016-10-18 99 * @minfo: Message info
aa276781a64a5f Nishanth Menon 2016-10-18 100 * @node: list head
e69a35531589a2 Nishanth Menon 2018-08-28 101 * @host_id: Host ID
aa276781a64a5f Nishanth Menon 2016-10-18 102 * @users: Number of users of this instance
aa276781a64a5f Nishanth Menon 2016-10-18 103 */
aa276781a64a5f Nishanth Menon 2016-10-18 104 struct ti_sci_info {
aa276781a64a5f Nishanth Menon 2016-10-18 105 struct device *dev;
aa276781a64a5f Nishanth Menon 2016-10-18 106 const struct ti_sci_desc *desc;
aa276781a64a5f Nishanth Menon 2016-10-18 107 struct dentry *d;
aa276781a64a5f Nishanth Menon 2016-10-18 108 void __iomem *debug_region;
aa276781a64a5f Nishanth Menon 2016-10-18 109 char *debug_buffer;
aa276781a64a5f Nishanth Menon 2016-10-18 110 size_t debug_region_size;
aa276781a64a5f Nishanth Menon 2016-10-18 111 struct ti_sci_handle handle;
aa276781a64a5f Nishanth Menon 2016-10-18 112 struct mbox_client cl;
aa276781a64a5f Nishanth Menon 2016-10-18 113 struct mbox_chan *chan_tx;
aa276781a64a5f Nishanth Menon 2016-10-18 114 struct mbox_chan *chan_rx;
aa276781a64a5f Nishanth Menon 2016-10-18 115 struct ti_sci_xfers_info minfo;
aa276781a64a5f Nishanth Menon 2016-10-18 116 struct list_head node;
e69a35531589a2 Nishanth Menon 2018-08-28 117 u8 host_id;
aa276781a64a5f Nishanth Menon 2016-10-18 118 /* protected by ti_sci_list_mutex */
aa276781a64a5f Nishanth Menon 2016-10-18 119 int users;
aa276781a64a5f Nishanth Menon 2016-10-18 @120 };
aa276781a64a5f Nishanth Menon 2016-10-18 121
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki