Re: [PATCH v5 3/4] platform/x86/intel/pmc: core: Always capture counters on suspend

["Box, David E" <david.e.box@intel.com>]

On Thu, 2023-03-30 at 14:44 -0500, Mario Limonciello wrote:
> Currently counters are only captured during suspend when the
> warn_on_s0ix_failures module parameter is set.
> 
> In order to relay this counter information to the kernel reporting
> infrastructure adjust it so that the counters are always captured.
> 
> warn_on_s0ix_failures will be utilized solely for messaging by
> the driver instead.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Reviewed-by: David E. Box <david.e.box@linux.intel.com>

> ---
> v4->v5:
>  * Squash patches together
>  * Add extra pm_suspend_via_firmware() check for resume routine too
> ---
>  drivers/platform/x86/intel/pmc/core.c | 13 +++++--------
>  drivers/platform/x86/intel/pmc/core.h |  2 --
>  2 files changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/pmc/core.c
> b/drivers/platform/x86/intel/pmc/core.c
> index 3a15d32d7644..e2f171fac094 100644
> --- a/drivers/platform/x86/intel/pmc/core.c
> +++ b/drivers/platform/x86/intel/pmc/core.c
> @@ -1168,12 +1168,6 @@ static __maybe_unused int pmc_core_suspend(struct
> device *dev)
>  {
>         struct pmc_dev *pmcdev = dev_get_drvdata(dev);
>  
> -       pmcdev->check_counters = false;
> -
> -       /* No warnings on S0ix failures */
> -       if (!warn_on_s0ix_failures)
> -               return 0;
> -
>         /* Check if the syspend will actually use S0ix */
>         if (pm_suspend_via_firmware())
>                 return 0;
> @@ -1186,7 +1180,6 @@ static __maybe_unused int pmc_core_suspend(struct device
> *dev)
>         if (pmc_core_dev_state_get(pmcdev, &pmcdev->s0ix_counter))
>                 return -EIO;
>  
> -       pmcdev->check_counters = true;
>         return 0;
>  }
>  
> @@ -1222,12 +1215,16 @@ static __maybe_unused int pmc_core_resume(struct
> device *dev)
>         const struct pmc_bit_map **maps = pmcdev->map->lpm_sts;
>         int offset = pmcdev->map->lpm_status_offset;
>  
> -       if (!pmcdev->check_counters)
> +       /* Check if the syspend used S0ix */
> +       if (pm_suspend_via_firmware())
>                 return 0;
>  
>         if (!pmc_core_is_s0ix_failed(pmcdev))
>                 return 0;
>  
> +       if (!warn_on_s0ix_failures)
> +               return 0;
> +
>         if (pmc_core_is_pc10_failed(pmcdev)) {
>                 /* S0ix failed because of PC10 entry failure */
>                 dev_info(dev, "CPU did not enter PC10!!! (PC10 cnt=0x%llx)\n",
> diff --git a/drivers/platform/x86/intel/pmc/core.h
> b/drivers/platform/x86/intel/pmc/core.h
> index 810204d758ab..51d73efceaf3 100644
> --- a/drivers/platform/x86/intel/pmc/core.h
> +++ b/drivers/platform/x86/intel/pmc/core.h
> @@ -319,7 +319,6 @@ struct pmc_reg_map {
>   * @pmc_xram_read_bit: flag to indicate whether PMC XRAM shadow registers
>   *                     used to read MPHY PG and PLL status are available
>   * @mutex_lock:                mutex to complete one transcation
> - * @check_counters:    On resume, check if counters are getting incremented
>   * @pc10_counter:      PC10 residency counter
>   * @s0ix_counter:      S0ix residency (step adjusted)
>   * @num_lpm_modes:     Count of enabled modes
> @@ -338,7 +337,6 @@ struct pmc_dev {
>         int pmc_xram_read_bit;
>         struct mutex lock; /* generic mutex lock for PMC Core */
>  
> -       bool check_counters; /* Check for counter increments on resume */
>         u64 pc10_counter;
>         u64 s0ix_counter;
>         int num_lpm_modes;