Re: [PATCH] platform/x86:intel/pmc: Encapsulate callbacks registration in CONFIG_X86_PM_TIMER check

[Hans de Goede <hdegoede@redhat.com>]

Hi Marek,

On 9/8/24 10:01 PM, Marek Maslanka wrote:
> Encapsulate the code that registers and unregisters the ACPI PM Timer
> suspend/resume callback to checks that CONFIG_X86_PM_TIMER is enabled.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202409090259.6vsS5Bni-lkp@intel.com/
> Signed-off-by: Marek Maslanka <mmaslanka@google.com>

Thank you for the quick fix.

Arnd has submitted a cleaner fix for this, which looks better so
I think we should go with Arnd's fix:

https://lore.kernel.org/platform-driver-x86/20240909111644.248756-1-arnd@kernel.org/

Regards,

Hans




> ---
>  drivers/platform/x86/intel/pmc/core.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
> index 695804ca8de4..0c6392eeacee 100644
> --- a/drivers/platform/x86/intel/pmc/core.c
> +++ b/drivers/platform/x86/intel/pmc/core.c
> @@ -1215,6 +1215,7 @@ static bool pmc_core_is_pson_residency_enabled(struct pmc_dev *pmcdev)
>  	return val == 1;
>  }
>  
> +#ifdef CONFIG_X86_PM_TIMER
>  /*
>   * Enable or disable ACPI PM Timer
>   *
> @@ -1247,6 +1248,7 @@ static void pmc_core_acpi_pm_timer_suspend_resume(void *data, bool suspend)
>  
>  	pmcdev->enable_acpi_pm_timer_on_resume = suspend && enabled;
>  }
> +#endif
>  
>  static void pmc_core_dbgfs_unregister(struct pmc_dev *pmcdev)
>  {
> @@ -1443,9 +1445,11 @@ static int pmc_core_probe(struct platform_device *pdev)
>  	struct pmc_dev *pmcdev;
>  	const struct x86_cpu_id *cpu_id;
>  	int (*core_init)(struct pmc_dev *pmcdev);
> -	const struct pmc_reg_map *map;
>  	struct pmc *primary_pmc;
>  	int ret;
> +#ifdef CONFIG_X86_PM_TIMER
> +	const struct pmc_reg_map *map;
> +#endif
>  
>  	if (device_initialized)
>  		return -ENODEV;
> @@ -1502,10 +1506,12 @@ static int pmc_core_probe(struct platform_device *pdev)
>  	pm_report_max_hw_sleep(FIELD_MAX(SLP_S0_RES_COUNTER_MASK) *
>  			       pmc_core_adjust_slp_s0_step(primary_pmc, 1));
>  
> +#ifdef CONFIG_X86_PM_TIMER
>  	map = primary_pmc->map;
>  	if (map->acpi_pm_tmr_ctl_offset)
>  		acpi_pmtmr_register_suspend_resume_callback(pmc_core_acpi_pm_timer_suspend_resume,
> -							 pmcdev);
> +							    pmcdev);
> +#endif
>  
>  	device_initialized = true;
>  	dev_info(&pdev->dev, " initialized\n");
> @@ -1516,11 +1522,13 @@ static int pmc_core_probe(struct platform_device *pdev)
>  static void pmc_core_remove(struct platform_device *pdev)
>  {
>  	struct pmc_dev *pmcdev = platform_get_drvdata(pdev);
> +#ifdef CONFIG_X86_PM_TIMER
>  	const struct pmc *pmc = pmcdev->pmcs[PMC_IDX_MAIN];
>  	const struct pmc_reg_map *map = pmc->map;
>  
>  	if (map->acpi_pm_tmr_ctl_offset)
>  		acpi_pmtmr_unregister_suspend_resume_callback();
> +#endif
>  
>  	pmc_core_dbgfs_unregister(pmcdev);
>  	pmc_core_clean_structure(pdev);