Re: [PATCH] platform/x86/intel/pmc: Disable ACPI PM timer during suspend on Panther Lake

[Tzung-Bi Shih]

On Thu, Oct 15, 2026 at 11:52:05AM +0300, Ilpo Järvinen wrote:
> > diff --git a/drivers/platform/x86/intel/pmc/ptl.c b/drivers/platform/x86/intel/pmc/ptl.c
> > index a7d5d4f77934..8c32f9be9302 100644
> > --- a/drivers/platform/x86/intel/pmc/ptl.c
> > +++ b/drivers/platform/x86/intel/pmc/ptl.c
> > @@ -6,6 +6,7 @@
> >   * Copyright (c) 2025, Intel Corporation.
> >   */
> >  
> > +#include <linux/io.h>
> >  #include <linux/pci.h>
> >  
> >  #include "core.h"
> > @@ -548,6 +549,9 @@ static const u8 ptl_pmc_list[] = {PMC_IDX_MAIN};
> >  #define PTL_NPU_PCI_DEV                0xb03e
> >  #define PTL_IPU_PCI_DEV                0xb05d
> >  
> > +#define ACPI_TMR_CTL_OFFSET            0x18fc
> > +#define ACPI_TIM_DIS                   BIT(1)
> 
> Please add include for this as well.

Ack, fixed in v2.

> > +static void ptl_set_acpi_pm_timer_on_suspend(struct pmc_dev *pmcdev, bool suspend)
> > +{
> > +	struct pmc *pmc = pmcdev->pmcs[PMC_IDX_MAIN];
> > +	bool enabled;
> > +	u8 val;
> > +
> > +	if (!pmc || !pmc->regbase)
> > +		return;
> > +
> > +	if (!suspend && !pmcdev->pm_timer_enabled)
> > +		return;
> > +
> > +	val = readb(pmc->regbase + ACPI_TMR_CTL_OFFSET);
> > +	enabled = !(val & ACPI_TIM_DIS);
> > +	if (suspend && enabled)
> > +		writeb(val | ACPI_TIM_DIS, pmc->regbase + ACPI_TMR_CTL_OFFSET);
> > +	else if (!suspend)
> > +		writeb(val & ~ACPI_TIM_DIS, pmc->regbase + ACPI_TMR_CTL_OFFSET);
> 
> Change val on a separate line for clarity.
> 
> How about using this alternative:
> 
> 	old_val = val;
> 	if (...)
> 		val |= ACPI_TIM_DIS;
> 	else if (...)
> 		val &= ~ACPI_TIM_DIS;
> 	if (val != old_val)
> 		writeb(...);

Ack, tried something similar in v2.