pmdomain: core: switch to dynamic root device
PATCH 1/1 pmdomain: core: switch to dynamic root device
Driver core expects devices to be dynamically allocated and will, for example, complain loudly if a device that lacks a release function is ever freed.
Use root_device_register() to allocate and register the root device instead of open coding using a static device.
--- drivers/pmdomain/core.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-)
drivers/pmdomain/core.c +6 -10
@@ -33,9 +33,7 @@ static const struct bus_type genpd_provider_bus_type = { };/* The parent for genpd_provider devices. */ -static struct device genpd_provider_bus = { - .init_name ="genpd_provider" , -}; +static struct device *genpd_provider_bus;#define GENPD_RETRY_MAX_MS 250/* Approximate */
@@ -2325,7 +2323,7 @@ static int genpd_alloc_data(struct generic_pm_domain *genpd)device_initialize (&genpd->dev); genpd->dev.release = genpd_provider_release; genpd->dev.bus = &genpd_provider_bus_type; - genpd->dev.parent = &genpd_provider_bus; + genpd->dev.parent = genpd_provider_bus;if (!genpd_is_dev_name_fw (genpd)) {dev_set_name (&genpd->dev,"%s" , genpd->name);
@@ -3567,11 +3565,9 @@ static int __init genpd_bus_init(void) {int ret; - ret =device_register (&genpd_provider_bus); -if (ret) { -put_device (&genpd_provider_bus); -return ret; - } + genpd_provider_bus =root_device_register ("genpd_provider" ); +if (IS_ERR (genpd_provider_bus)) +return PTR_ERR (genpd_provider_bus); ret =bus_register (&genpd_provider_bus_type);if (ret)
@@ -3593,7 +3589,7 @@ static int __init genpd_bus_init(void) err_prov_bus:bus_unregister (&genpd_provider_bus_type); err_dev: -device_unregister (&genpd_provider_bus); +root_device_unregister (genpd_provider_bus);return ret; }core_initcall (genpd_bus_init);
Signature · 1 line
2.53.0
Quoted 9 lines · Johan Hovold
Driver core expects devices to be dynamically allocated and will, for example, complain loudly if a device that lacks a release function is ever freed.
Use
root_device_register()to allocate and register the root device instead of open coding using a static device.Signed-off-by Johan Hovold <[email protected]>
Applied for next, thanks!
Kind regards
Uffe
Quoted 54 lines
--- drivers/pmdomain/core.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-)drivers/pmdomain/core.c +6
-11@@ -33,9 +33,7 @@ static const struct bus_type genpd_provider_bus_type = { };/* The parent for genpd_provider devices. */ -static struct device genpd_provider_bus = { - .init_name ="genpd_provider" , -}; +static struct device *genpd_provider_bus;#define GENPD_RETRY_MAX_MS 250/* Approximate */ @@ -2325,7 +2323,7 @@ static int genpd_alloc_data(struct generic_pm_domain *genpd)device_initialize (&genpd->dev); genpd->dev.release = genpd_provider_release; genpd->dev.bus = &genpd_provider_bus_type; - genpd->dev.parent = &genpd_provider_bus; + genpd->dev.parent = genpd_provider_bus;if (!genpd_is_dev_name_fw (genpd)) {dev_set_name (&genpd->dev,"%s" , genpd->name);@@ -3567,11 +3565,9 @@ static int __init genpd_bus_init(void) {int ret; - ret =device_register (&genpd_provider_bus); -if (ret) { -put_device (&genpd_provider_bus); -return ret; - } + genpd_provider_bus =root_device_register ("genpd_provider" ); +if (IS_ERR (genpd_provider_bus)) +return PTR_ERR (genpd_provider_bus); ret =bus_register (&genpd_provider_bus_type);if (ret)@@ -3593,7 +3589,7 @@ static int __init genpd_bus_init(void) err_prov_bus:bus_unregister (&genpd_provider_bus_type); err_dev: -device_unregister (&genpd_provider_bus); +root_device_unregister (genpd_provider_bus);return ret; }core_initcall (genpd_bus_init); --2.53.0
Hi Johan,
Quoted 8 lines · Johan Hovold
Driver core expects devices to be dynamically allocated and will, for example, complain loudly if a device that lacks a release function is ever freed.
Use
root_device_register()to allocate and register the root device instead of open coding using a static device.Signed-off-by Johan Hovold <[email protected]>
Thanks for your patch, which is now commit a96e40f4afdcb52a ("pmdomain: core: switch to dynamic root device") in pmdomain/next. On e.g. R-Car H1, R-Car M2-W, and R-Car H3, this causes scary messages when systemd-journald.service is started:
synth uevent: /always-on: failed to send uevent genpd_provider always-on: uevent: failed to send synthetic uevent: -22 synth uevent: /ca15-cpu0: failed to send uevent genpd_provider ca15-cpu0: uevent: failed to send synthetic uevent: -22 [...]
Reverting the commit fixes the issue.
Quoted 38 lines
drivers/pmdomain/core.c +5
-9@@ -33,9 +33,7 @@ static const struct bus_type genpd_provider_bus_type = { };/* The parent for genpd_provider devices. */ -static struct device genpd_provider_bus = { - .init_name ="genpd_provider" , -}; +static struct device *genpd_provider_bus;#define GENPD_RETRY_MAX_MS 250/* Approximate */ @@ -2325,7 +2323,7 @@ static int genpd_alloc_data(struct generic_pm_domain *genpd)device_initialize (&genpd->dev); genpd->dev.release = genpd_provider_release; genpd->dev.bus = &genpd_provider_bus_type; - genpd->dev.parent = &genpd_provider_bus; + genpd->dev.parent = genpd_provider_bus;if (!genpd_is_dev_name_fw (genpd)) {dev_set_name (&genpd->dev,"%s" , genpd->name);@@ -3567,11 +3565,9 @@ static int __init genpd_bus_init(void) {int ret; - ret =device_register (&genpd_provider_bus); -if (ret) { -put_device (&genpd_provider_bus); -return ret; - } + genpd_provider_bus =root_device_register ("genpd_provider" ); +if (IS_ERR (genpd_provider_bus)) +return PTR_ERR (genpd_provider_bus); ret =bus_register (&genpd_provider_bus_type);if (ret)
drivers/pmdomain/core.c static int __init genpd_bus_init(void) · line 3595
Quoted 8 lines
drivers/pmdomain/core.c +1
-1@@ -3593,7 +3589,7 @@ static int __init genpd_bus_init(void) err_prov_bus:bus_unregister (&genpd_provider_bus_type); err_dev: -device_unregister (&genpd_provider_bus); +root_device_unregister (genpd_provider_bus);return ret; }core_initcall (genpd_bus_init);
Apparently this is done too late for drivers/pmdomain/renesas/rcar-sysc.c, which registers PM domains from an early_initcall(). Doing all work in rcar-sysc.c from the postcore_initcall() is not an option, as the CPU power domains are needed for secondary CPU startup on R-Car H1.
Note that R-Car Gen4 is not affected, as it uses rcar-gen4-sysc.c instead.
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Quoted 15 lines · Geert Uytterhoeven
Quoted 8 lines · Johan Hovold
Driver core expects devices to be dynamically allocated and will, for example, complain loudly if a device that lacks a release function is ever freed.
Use
root_device_register()to allocate and register the root device instead of open coding using a static device.Signed-off-by Johan Hovold <[email protected]>Thanks for your patch, which is now commit
a96e40f4afdcb52a("pmdomain: core: switch to dynamic root device") in pmdomain/next. On e.g. R-Car H1, R-Car M2-W, and R-Car H3, this causes scary messages when systemd-journald.service is started:
Quoted 5 lines
synth uevent: /always-on: failed to send uevent
genpd_provideralways-on: uevent: failed to send synthetic uevent: -22 synth uevent: /ca15-cpu0: failed to send ueventgenpd_providerca15-cpu0: uevent: failed to send synthetic uevent: -22 [...]
I missed that there is another set on e.g. R-Car H1 for the clock domain:
synth uevent: /clocks: failed to send uevent genpd_provider clocks: uevent: failed to send synthetic uevent: -22
Quoted 38 lines
Reverting the commit fixes the issue.
Quoted 35 lines
drivers/pmdomain/core.c +5
-9@@ -33,9 +33,7 @@ static const struct bus_type genpd_provider_bus_type = { };/* The parent for genpd_provider devices. */ -static struct device genpd_provider_bus = { - .init_name ="genpd_provider" , -}; +static struct device *genpd_provider_bus;#define GENPD_RETRY_MAX_MS 250/* Approximate */ @@ -2325,7 +2323,7 @@ static int genpd_alloc_data(struct generic_pm_domain *genpd)device_initialize (&genpd->dev); genpd->dev.release = genpd_provider_release; genpd->dev.bus = &genpd_provider_bus_type; - genpd->dev.parent = &genpd_provider_bus; + genpd->dev.parent = genpd_provider_bus;if (!genpd_is_dev_name_fw (genpd)) {dev_set_name (&genpd->dev,"%s" , genpd->name);@@ -3567,11 +3565,9 @@ static int __init genpd_bus_init(void) {int ret; - ret =device_register (&genpd_provider_bus); -if (ret) { -put_device (&genpd_provider_bus); -return ret; - } + genpd_provider_bus =root_device_register ("genpd_provider" ); +if (IS_ERR (genpd_provider_bus)) +return PTR_ERR (genpd_provider_bus);
Quoted 16 lines
drivers/pmdomain/core.c static int __init genpd_bus_init(void) · line 3595
Quoted 11 lines
drivers/pmdomain/core.c +1
-1ret =bus_register (&genpd_provider_bus_type);if (ret)@@ -3593,7 +3589,7 @@ static int __init genpd_bus_init(void) err_prov_bus:bus_unregister (&genpd_provider_bus_type); err_dev: -device_unregister (&genpd_provider_bus); +root_device_unregister (genpd_provider_bus);return ret; }core_initcall (genpd_bus_init);Apparently this is done too late for
drivers/pmdomain/renesas/rcar-sysc.c, which registers PM domains from anearly_initcall(). Doing all work inrcar-sysc.cfrom thepostcore_initcall()is not an option, as the CPU power domains are needed for secondary CPU startup on R-Car H1.
Same story for cpg_mstp_add_clk_domain(), which is called from
CLK_OF_DECLARE(r8a7778_cpg_clks, "renesas,r8a7778-cpg-clocks",
r8a7778_cpg_clocks_init);
CLK_OF_DECLARE(r8a7779_cpg_clks, "renesas,r8a7779-cpg-clocks",
r8a7779_cpg_clocks_init);
CLK_OF_DECLARE(rz_cpg_clks, "renesas,rz-cpg-clocks", rz_cpg_clocks_init);Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert,
Quoted 15 lines · Geert Uytterhoeven
Quoted 8 lines · Johan Hovold
Driver core expects devices to be dynamically allocated and will, for example, complain loudly if a device that lacks a release function is ever freed.
Use
root_device_register()to allocate and register the root device instead of open coding using a static device.Signed-off-by Johan Hovold <[email protected]>Thanks for your patch, which is now commit
a96e40f4afdcb52a("pmdomain: core: switch to dynamic root device") in pmdomain/next. On e.g. R-Car H1, R-Car M2-W, and R-Car H3, this causes scary messages when systemd-journald.service is started:
Quoted 7 lines
synth uevent: /always-on: failed to send uevent
genpd_provideralways-on: uevent: failed to send synthetic uevent: -22 synth uevent: /ca15-cpu0: failed to send ueventgenpd_providerca15-cpu0: uevent: failed to send synthetic uevent: -22 [...]Reverting the commit fixes the issue.
Thanks for the report and sorry about the breakage.
I mistakenly convinced myself that this would not be an issue due to the pmdomain code making sure that the bus has been registered but missed the two-step initialisation.
I just sent a fix here if you want to give it a spin:
https://lore.kernel.org/lkml/[email protected]/
Johan