| Message ID | 20240107140310.46512-5-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | x86: atom-punit/-pmc s2idle device state checks | expand |
On Sun, 7 Jan 2024, Hans de Goede wrote: > Extend the s2idle check with checking that none of the PMC clocks > is in the forced-on state. If one of the clocks is in forced on > state then S0i3 cannot be reached. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > Changes in v2: > - Drop the PMC_CLK_* defines these are defined in > include/linux/platform_data/x86/pmc_atom.h now > - Drop duplicated "pmc_atom: " prefix from pr_err() message > --- > drivers/platform/x86/pmc_atom.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/platform/x86/pmc_atom.c b/drivers/platform/x86/pmc_atom.c > index 81ad66117365..d04f635c4075 100644 > --- a/drivers/platform/x86/pmc_atom.c > +++ b/drivers/platform/x86/pmc_atom.c > @@ -477,6 +477,7 @@ static void pmc_s2idle_check(void) > u32 func_dis, func_dis_2; > u32 d3_sts_0, d3_sts_1; > u32 false_pos_sts_0, false_pos_sts_1; > + int i; > > func_dis = pmc_reg_read(pmc, PMC_FUNC_DIS); > func_dis_2 = pmc_reg_read(pmc, PMC_FUNC_DIS_2); > @@ -504,6 +505,16 @@ static void pmc_s2idle_check(void) > > /* High part */ > pmc_dev_state_check(d3_sts_1, m->d3_sts_1, func_dis_2, m->func_dis_2, false_pos_sts_1); > + > + /* Check PMC clocks */ Kind of obvious comment, how about: /* Check PMC clocks don't prevent S0i3 */ Or /* Forced-on PMC clock prevents S0i3? */ ? > + for (i = 0; i < PMC_CLK_NUM; i++) { > + u32 ctl = pmc_reg_read(pmc, PMC_CLK_CTL_OFFSET + 4 * i); > + > + if ((ctl & PMC_MASK_CLK_CTL) != PMC_CLK_CTL_FORCE_ON) > + continue; > + > + pr_err("clock %d is ON prior to freeze (ctl 0x%08x)\n", i, ctl); > + } > } > > static struct acpi_s2idle_dev_ops pmc_s2idle_ops = { > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Hi, On 1/8/24 12:27, Ilpo Järvinen wrote: > On Sun, 7 Jan 2024, Hans de Goede wrote: > >> Extend the s2idle check with checking that none of the PMC clocks >> is in the forced-on state. If one of the clocks is in forced on >> state then S0i3 cannot be reached. >> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> Changes in v2: >> - Drop the PMC_CLK_* defines these are defined in >> include/linux/platform_data/x86/pmc_atom.h now >> - Drop duplicated "pmc_atom: " prefix from pr_err() message >> --- >> drivers/platform/x86/pmc_atom.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/drivers/platform/x86/pmc_atom.c b/drivers/platform/x86/pmc_atom.c >> index 81ad66117365..d04f635c4075 100644 >> --- a/drivers/platform/x86/pmc_atom.c >> +++ b/drivers/platform/x86/pmc_atom.c >> @@ -477,6 +477,7 @@ static void pmc_s2idle_check(void) >> u32 func_dis, func_dis_2; >> u32 d3_sts_0, d3_sts_1; >> u32 false_pos_sts_0, false_pos_sts_1; >> + int i; >> >> func_dis = pmc_reg_read(pmc, PMC_FUNC_DIS); >> func_dis_2 = pmc_reg_read(pmc, PMC_FUNC_DIS_2); >> @@ -504,6 +505,16 @@ static void pmc_s2idle_check(void) >> >> /* High part */ >> pmc_dev_state_check(d3_sts_1, m->d3_sts_1, func_dis_2, m->func_dis_2, false_pos_sts_1); >> + >> + /* Check PMC clocks */ > > Kind of obvious comment, how about: > > /* Check PMC clocks don't prevent S0i3 */ > > Or > > /* Forced-on PMC clock prevents S0i3? */ > > ? Good point. I have gone with the "Forced-on PMC clock prevents S0i3" comment. Regards, Hans > >> + for (i = 0; i < PMC_CLK_NUM; i++) { >> + u32 ctl = pmc_reg_read(pmc, PMC_CLK_CTL_OFFSET + 4 * i); >> + >> + if ((ctl & PMC_MASK_CLK_CTL) != PMC_CLK_CTL_FORCE_ON) >> + continue; >> + >> + pr_err("clock %d is ON prior to freeze (ctl 0x%08x)\n", i, ctl); >> + } >> } >> >> static struct acpi_s2idle_dev_ops pmc_s2idle_ops = { >> > > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > >
diff --git a/drivers/platform/x86/pmc_atom.c b/drivers/platform/x86/pmc_atom.c index 81ad66117365..d04f635c4075 100644 --- a/drivers/platform/x86/pmc_atom.c +++ b/drivers/platform/x86/pmc_atom.c @@ -477,6 +477,7 @@ static void pmc_s2idle_check(void) u32 func_dis, func_dis_2; u32 d3_sts_0, d3_sts_1; u32 false_pos_sts_0, false_pos_sts_1; + int i; func_dis = pmc_reg_read(pmc, PMC_FUNC_DIS); func_dis_2 = pmc_reg_read(pmc, PMC_FUNC_DIS_2); @@ -504,6 +505,16 @@ static void pmc_s2idle_check(void) /* High part */ pmc_dev_state_check(d3_sts_1, m->d3_sts_1, func_dis_2, m->func_dis_2, false_pos_sts_1); + + /* Check PMC clocks */ + for (i = 0; i < PMC_CLK_NUM; i++) { + u32 ctl = pmc_reg_read(pmc, PMC_CLK_CTL_OFFSET + 4 * i); + + if ((ctl & PMC_MASK_CLK_CTL) != PMC_CLK_CTL_FORCE_ON) + continue; + + pr_err("clock %d is ON prior to freeze (ctl 0x%08x)\n", i, ctl); + } } static struct acpi_s2idle_dev_ops pmc_s2idle_ops = {
Extend the s2idle check with checking that none of the PMC clocks is in the forced-on state. If one of the clocks is in forced on state then S0i3 cannot be reached. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- Changes in v2: - Drop the PMC_CLK_* defines these are defined in include/linux/platform_data/x86/pmc_atom.h now - Drop duplicated "pmc_atom: " prefix from pr_err() message --- drivers/platform/x86/pmc_atom.c | 11 +++++++++++ 1 file changed, 11 insertions(+)