Re: [PATCH v2 4/5] platform/x86: pmc_atom: Check state of PMC clocks on s2idle

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 1765 bytes --]

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>


-- 
 i.