| Message ID | 20230809-pca9450-reboot-v2-3-b98b4f8139d5@skidata.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | regulator: pca9450: register restart handlers | expand |
On 8/9/23 22:24, Benjamin Bara wrote: > From: Benjamin Bara <benjamin.bara@skidata.com> > > The current restart handler registration does not specify whether the > restart is a cold or a warm one. Now, as do_kernel_restart() knows about > the type, the priorization is implicitly done (cold restarts are > executed first) and the reboot_mode kernel parameter (which is currently > mostly ignored) can be respected. > > Acked-by: Thierry Reding <treding@nvidia.com> > Signed-off-by: Benjamin Bara <benjamin.bara@skidata.com> > --- > v2: > - improve commit message > --- > drivers/soc/tegra/pmc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c > index 162f52456f65..4f42febb9b0f 100644 > --- a/drivers/soc/tegra/pmc.c > +++ b/drivers/soc/tegra/pmc.c > @@ -2962,7 +2962,7 @@ static int tegra_pmc_probe(struct platform_device *pdev) > } > > err = devm_register_sys_off_handler(&pdev->dev, > - SYS_OFF_MODE_RESTART, > + SYS_OFF_MODE_RESTART_WARM, > SYS_OFF_PRIO_LOW, > tegra_pmc_restart_handler, NULL); > if (err) { > You have tegra-pmc restart handler that uses low priority. And then you're adding cold/warm handlers to tps65219 and pca9450 drivers with a default priorities. Hence this cold/warm separation of handlers doesn't do any practical difference in yours case because tegra-pmc will never be used as it did before your changes? Previously you wanted to make tps6586x driver to skip the warm reboot, but you're not touching tps6586x in this patchset. There is no real problem that is solved by these patches?
Hi Dmitry, thanks for your feedback! On Tue, 15 Aug 2023 at 00:38, Dmitry Osipenko <dmitry.osipenko@collabora.com> wrote: > On 8/9/23 22:24, Benjamin Bara wrote: > > From: Benjamin Bara <benjamin.bara@skidata.com> > > > > The current restart handler registration does not specify whether the > > restart is a cold or a warm one. Now, as do_kernel_restart() knows about > > the type, the priorization is implicitly done (cold restarts are > > executed first) and the reboot_mode kernel parameter (which is currently > > mostly ignored) can be respected. > > > > Acked-by: Thierry Reding <treding@nvidia.com> > > Signed-off-by: Benjamin Bara <benjamin.bara@skidata.com> > > --- > > v2: > > - improve commit message > > --- > > drivers/soc/tegra/pmc.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c > > index 162f52456f65..4f42febb9b0f 100644 > > --- a/drivers/soc/tegra/pmc.c > > +++ b/drivers/soc/tegra/pmc.c > > @@ -2962,7 +2962,7 @@ static int tegra_pmc_probe(struct platform_device *pdev) > > } > > > > err = devm_register_sys_off_handler(&pdev->dev, > > - SYS_OFF_MODE_RESTART, > > + SYS_OFF_MODE_RESTART_WARM, > > SYS_OFF_PRIO_LOW, > > tegra_pmc_restart_handler, NULL); > > if (err) { > > > > You have tegra-pmc restart handler that uses low priority. And then > you're adding cold/warm handlers to tps65219 and pca9450 drivers with a > default priorities. Exactly, but I guess it makes sense to also use the handler with default priority for the pmc reboot. The reason I kept it low prio was because there is a comment that PMC should be last resort, but the reason applies to any other soft restart handler too. I will adapt in the next version. > Hence this cold/warm separation of handlers doesn't do any practical > difference in yours case because tegra-pmc will never be used as it > did before your changes? The change is e.g. relevant for platforms without PMIC-based soft reset, e.g. when the tps6586x is in use. AFAIK, there is no possibility to actually do a soft reboot, as the hard reboot will always be executed first. This also happens if I set the kernel parameter "reboot_mode" (also available via SysFS) to "soft" and a soft restart handler is registered. > Previously you wanted to make tps6586x driver to skip the warm reboot, > but you're not touching tps6586x in this patchset. True, there might also be other affected patches which are currently not in linux-next yet. Will adapt the tps6586x too and depend on the whole series in the next version. > There is no real problem that is solved by these patches? I think another problem is if the user sets the "reboot_mode" to "cold", but there is no cold handler registered (as it was the case for me with the pca9450), a warning should indicate that. AFAIK, there is no possibility in user-space to find out if a cold restart handler is registered, there is just this knob which can be switched to "cold". I can also add a SysFS entry with "supported_modes" and check if the new "mode" is supported on a store. My other idea was to add a flags field to the notifier_block which indicates (in case of a reboot notifier) the supported reboot_modes of the registered handler, but I guess other notifier_block users won't really benefit from an additional field, therefore I decided to add a second list instead. Best regards Benjamin
diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c index 162f52456f65..4f42febb9b0f 100644 --- a/drivers/soc/tegra/pmc.c +++ b/drivers/soc/tegra/pmc.c @@ -2962,7 +2962,7 @@ static int tegra_pmc_probe(struct platform_device *pdev) } err = devm_register_sys_off_handler(&pdev->dev, - SYS_OFF_MODE_RESTART, + SYS_OFF_MODE_RESTART_WARM, SYS_OFF_PRIO_LOW, tegra_pmc_restart_handler, NULL); if (err) {