| Message ID | 20211123111134.26409-1-jonathanh@nvidia.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [V2] soc/tegra: pmc: Add reboot notifier | expand |
23.11.2021 14:11, Jon Hunter пишет: > The Tegra PMC driver implements a restart handler that supports Tegra > specific reboot commands such as placing the device into 'recovery' mode > in order to reprogram the platform. This is accomplished by setting the > appropriate bit in the PMC scratch0 register prior to rebooting the > platform. > > For Tegra platforms that support PSCI or EFI, the default Tegra restart > handler is not called and the PSCI or EFI restart handler is called > instead. Hence, for Tegra platforms that support PSCI or EFI, the Tegra > specific reboot commands do not currently work. Fix this by moving the > code that programs the PMC scratch0 register into a separate reboot > notifier that will always be called on reboot. > > Signed-off-by: Jon Hunter <jonathanh@nvidia.com> > --- > Changes since V1: > - Don't change the behaviour for writing scratch0 register when the > notifier is called. > > drivers/soc/tegra/pmc.c | 33 ++++++++++++++++++++++++++++++--- > 1 file changed, 30 insertions(+), 3 deletions(-) > > diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c > index 575d6d5b4294..bb2f39597823 100644 > --- a/drivers/soc/tegra/pmc.c > +++ b/drivers/soc/tegra/pmc.c > @@ -1064,10 +1064,8 @@ int tegra_pmc_cpu_remove_clamping(unsigned int cpuid) > return tegra_powergate_remove_clamping(id); > } > > -static int tegra_pmc_restart_notify(struct notifier_block *this, > - unsigned long action, void *data) > +static void tegra_pmc_program_reboot_reason(const char *cmd) > { > - const char *cmd = data; > u32 value; > > value = tegra_pmc_scratch_readl(pmc, pmc->soc->regs->scratch0); > @@ -1085,6 +1083,27 @@ static int tegra_pmc_restart_notify(struct notifier_block *this, > } > > tegra_pmc_scratch_writel(pmc, value, pmc->soc->regs->scratch0); > +} > + > +static int tegra_pmc_reboot_notify(struct notifier_block *this, > + unsigned long action, void *data) > +{ > + if (action == SYS_RESTART) > + tegra_pmc_program_reboot_reason(data); > + > + return NOTIFY_DONE; > +} > + > +static struct notifier_block tegra_pmc_reboot_notifier = { > + .notifier_call = tegra_pmc_reboot_notify, > +}; > + > +static int tegra_pmc_restart_notify(struct notifier_block *this, > + unsigned long action, void *data) > +{ > + u32 value; > + > + tegra_pmc_program_reboot_reason(data); So the PMC reason is programmed twice now? First time by the reboot handler and second by the restart? Why? BTW, could you please always CC LKML or request to include linux-tegra ML onto lore? Tegra ML uses Gmane and it's unusable for development since all email addresses are mangled, the Gmane support told me that only Tegra ML admin can disable mangling, but I'm not sure who is it, maybe Stephen Warren?
On 25/11/2021 04:33, Dmitry Osipenko wrote: > 23.11.2021 14:11, Jon Hunter пишет: >> The Tegra PMC driver implements a restart handler that supports Tegra >> specific reboot commands such as placing the device into 'recovery' mode >> in order to reprogram the platform. This is accomplished by setting the >> appropriate bit in the PMC scratch0 register prior to rebooting the >> platform. >> >> For Tegra platforms that support PSCI or EFI, the default Tegra restart >> handler is not called and the PSCI or EFI restart handler is called >> instead. Hence, for Tegra platforms that support PSCI or EFI, the Tegra >> specific reboot commands do not currently work. Fix this by moving the >> code that programs the PMC scratch0 register into a separate reboot >> notifier that will always be called on reboot. >> >> Signed-off-by: Jon Hunter <jonathanh@nvidia.com> >> --- >> Changes since V1: >> - Don't change the behaviour for writing scratch0 register when the >> notifier is called. >> >> drivers/soc/tegra/pmc.c | 33 ++++++++++++++++++++++++++++++--- >> 1 file changed, 30 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c >> index 575d6d5b4294..bb2f39597823 100644 >> --- a/drivers/soc/tegra/pmc.c >> +++ b/drivers/soc/tegra/pmc.c >> @@ -1064,10 +1064,8 @@ int tegra_pmc_cpu_remove_clamping(unsigned int cpuid) >> return tegra_powergate_remove_clamping(id); >> } >> >> -static int tegra_pmc_restart_notify(struct notifier_block *this, >> - unsigned long action, void *data) >> +static void tegra_pmc_program_reboot_reason(const char *cmd) >> { >> - const char *cmd = data; >> u32 value; >> >> value = tegra_pmc_scratch_readl(pmc, pmc->soc->regs->scratch0); >> @@ -1085,6 +1083,27 @@ static int tegra_pmc_restart_notify(struct notifier_block *this, >> } >> >> tegra_pmc_scratch_writel(pmc, value, pmc->soc->regs->scratch0); >> +} >> + >> +static int tegra_pmc_reboot_notify(struct notifier_block *this, >> + unsigned long action, void *data) >> +{ >> + if (action == SYS_RESTART) >> + tegra_pmc_program_reboot_reason(data); >> + >> + return NOTIFY_DONE; >> +} >> + >> +static struct notifier_block tegra_pmc_reboot_notifier = { >> + .notifier_call = tegra_pmc_reboot_notify, >> +}; >> + >> +static int tegra_pmc_restart_notify(struct notifier_block *this, >> + unsigned long action, void *data) >> +{ >> + u32 value; >> + >> + tegra_pmc_program_reboot_reason(data); > > So the PMC reason is programmed twice now? First time by the reboot > handler and second by the restart? Why? That's an oversight. OK, thanks I will fix that in a V3. > BTW, could you please always CC LKML or request to include linux-tegra > ML onto lore? Tegra ML uses Gmane and it's unusable for development > since all email addresses are mangled, the Gmane support told me that > only Tegra ML admin can disable mangling, but I'm not sure who is it, > maybe Stephen Warren? I see linux-tegra here: https://lore.kernel.org/linux-tegra/ Jon
30.11.2021 12:08, Jon Hunter пишет: ... >>> +static int tegra_pmc_restart_notify(struct notifier_block *this, >>> + unsigned long action, void *data) >>> +{ >>> + u32 value; >>> + >>> + tegra_pmc_program_reboot_reason(data); >> >> So the PMC reason is programmed twice now? First time by the reboot >> handler and second by the restart? Why? > > That's an oversight. OK, thanks I will fix that in a V3. > >> BTW, could you please always CC LKML or request to include linux-tegra >> ML onto lore? Tegra ML uses Gmane and it's unusable for development >> since all email addresses are mangled, the Gmane support told me that >> only Tegra ML admin can disable mangling, but I'm not sure who is it, >> maybe Stephen Warren? > > I see linux-tegra here: https://lore.kernel.org/linux-tegra/ Nice, I refreshed the list of MLs in Thunderbird and see that it's indeed there now, thank you.
diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c index 575d6d5b4294..bb2f39597823 100644 --- a/drivers/soc/tegra/pmc.c +++ b/drivers/soc/tegra/pmc.c @@ -1064,10 +1064,8 @@ int tegra_pmc_cpu_remove_clamping(unsigned int cpuid) return tegra_powergate_remove_clamping(id); } -static int tegra_pmc_restart_notify(struct notifier_block *this, - unsigned long action, void *data) +static void tegra_pmc_program_reboot_reason(const char *cmd) { - const char *cmd = data; u32 value; value = tegra_pmc_scratch_readl(pmc, pmc->soc->regs->scratch0); @@ -1085,6 +1083,27 @@ static int tegra_pmc_restart_notify(struct notifier_block *this, } tegra_pmc_scratch_writel(pmc, value, pmc->soc->regs->scratch0); +} + +static int tegra_pmc_reboot_notify(struct notifier_block *this, + unsigned long action, void *data) +{ + if (action == SYS_RESTART) + tegra_pmc_program_reboot_reason(data); + + return NOTIFY_DONE; +} + +static struct notifier_block tegra_pmc_reboot_notifier = { + .notifier_call = tegra_pmc_reboot_notify, +}; + +static int tegra_pmc_restart_notify(struct notifier_block *this, + unsigned long action, void *data) +{ + u32 value; + + tegra_pmc_program_reboot_reason(data); /* reset everything but PMC_SCRATCH0 and PMC_RST_STATUS */ value = tegra_pmc_readl(pmc, PMC_CNTRL); @@ -2890,6 +2909,14 @@ static int tegra_pmc_probe(struct platform_device *pdev) goto cleanup_sysfs; } + err = devm_register_reboot_notifier(&pdev->dev, + &tegra_pmc_reboot_notifier); + if (err) { + dev_err(&pdev->dev, "unable to register reboot notifier, %d\n", + err); + goto cleanup_debugfs; + } + err = register_restart_handler(&tegra_pmc_restart_handler); if (err) { dev_err(&pdev->dev, "unable to register restart handler, %d\n",
The Tegra PMC driver implements a restart handler that supports Tegra specific reboot commands such as placing the device into 'recovery' mode in order to reprogram the platform. This is accomplished by setting the appropriate bit in the PMC scratch0 register prior to rebooting the platform. For Tegra platforms that support PSCI or EFI, the default Tegra restart handler is not called and the PSCI or EFI restart handler is called instead. Hence, for Tegra platforms that support PSCI or EFI, the Tegra specific reboot commands do not currently work. Fix this by moving the code that programs the PMC scratch0 register into a separate reboot notifier that will always be called on reboot. Signed-off-by: Jon Hunter <jonathanh@nvidia.com> --- Changes since V1: - Don't change the behaviour for writing scratch0 register when the notifier is called. drivers/soc/tegra/pmc.c | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-)