Messages in this thread | | | Subject | Re: [PATCH v4 02/17] clk: at91: pmc: execute suspend/resume only for backup mode | From | Stephen Boyd <> | Date | Fri, 08 Oct 2021 15:05:17 -0700 |
| |
Quoting Claudiu.Beznea@microchip.com (2021-10-07 23:47:14) > On 08.10.2021 06:51, Stephen Boyd wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > > Quoting Claudiu Beznea (2021-09-23 06:20:31) > >> Before going to backup mode architecture specific PM code sets the first > >> word in securam (file arch/arm/mach-at91/pm.c, function at91_pm_begin()). > >> Thus take this into account when suspending/resuming clocks. This will > >> avoid executing unnecessary instructions when suspending to non backup > >> modes. Also this commit changed the postcore_initcall() with > >> subsys_initcall() to be able to execute of_find_compatible_node() since > >> this was not available at the moment of postcore_initcall(). This should > >> not alter the tcb_clksrc since the changes are related to clocks > >> suspend/resume procedure that will be executed at the user space request, > >> thus long ago after subsys_initcall(). > > > > Is the comment still relevant though? > > For architecture PM code yes, the securam is set in [1]. > > Related to replacing postcore_init() with subsys_initcall() to be able to > have the proper result of of_find_compatible_node() I have to re-check > (don't know if something has been changed in this area since January). If > you know something please let me know.
I mostly don't want to lose the comment if it is still useful.
> > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/mach-at91/pm.c#n290 > > > > >> > >> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> > >> --- > >> diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c > >> index b2806946a77a..58e9c088cb22 100644 > >> --- a/drivers/clk/at91/pmc.c > >> +++ b/drivers/clk/at91/pmc.c > >> @@ -110,13 +112,35 @@ struct pmc_data *pmc_data_allocate(unsigned int ncore, unsigned int nsystem, > >> } > >> > >> #ifdef CONFIG_PM > >> + > >> +/* Address in SECURAM that say if we suspend to backup mode. */ > >> +static void __iomem *at91_pmc_backup_suspend; > >> + > >> static int at91_pmc_suspend(void) > >> { > >> + unsigned int backup; > >> + > >> + if (!at91_pmc_backup_suspend) > >> + return 0; > >> + > >> + backup = *(unsigned int *)at91_pmc_backup_suspend; > > > > This will fail sparse. Why are we reading iomem without using iomem > > reading wrapper? > > By mistake. I'll switch to iomem reading wrapper. > > Is it OK to send soon a new version with these adjustments or do you have > other patches in this series to review? >
Feel free to resend.
| |