Re: [PATCH v2 1/4] platform/x86: pmc_atom: Fix SLP_TYPx bitfield mask

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

Hi,

On 8/1/22 13:37, Andy Shevchenko wrote:
> On Intel hardware the SLP_TYPx bitfield occupies bits 10-12 as per ACPI
> specification (see Table 4.13 "PM1 Control Registers Fixed Hardware
> Feature Control Bits" for the details).
> 
> Fix the mask and other related definitions accordingly.
> 
> Fixes: 93e5eadd1f6e ("x86/platform: New Intel Atom SOC power management controller driver")
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Thank you for your patch-series, I've applied this series to my
review-hans branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.

Patches which are added to review-hans now are intended for
the next rc1. This branch will get rebased to the next rc1 when
it is out and after the rebasing the contents of review-hans
will be pushed to the platform-drivers-x86/for-next branch.

Regards,

Hans


> ---
> v2: addressed compilation error
>  drivers/platform/x86/pmc_atom.c            | 2 +-
>  include/linux/platform_data/x86/pmc_atom.h | 6 ++++--
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/platform/x86/pmc_atom.c b/drivers/platform/x86/pmc_atom.c
> index b8b1ed1406de..c220172fefbb 100644
> --- a/drivers/platform/x86/pmc_atom.c
> +++ b/drivers/platform/x86/pmc_atom.c
> @@ -232,7 +232,7 @@ static void pmc_power_off(void)
>  	pm1_cnt_port = acpi_base_addr + PM1_CNT;
>  
>  	pm1_cnt_value = inl(pm1_cnt_port);
> -	pm1_cnt_value &= SLEEP_TYPE_MASK;
> +	pm1_cnt_value &= ~SLEEP_TYPE_MASK;
>  	pm1_cnt_value |= SLEEP_TYPE_S5;
>  	pm1_cnt_value |= SLEEP_ENABLE;
>  
> diff --git a/include/linux/platform_data/x86/pmc_atom.h b/include/linux/platform_data/x86/pmc_atom.h
> index 6807839c718b..ea01dd80153b 100644
> --- a/include/linux/platform_data/x86/pmc_atom.h
> +++ b/include/linux/platform_data/x86/pmc_atom.h
> @@ -7,6 +7,8 @@
>  #ifndef PMC_ATOM_H
>  #define PMC_ATOM_H
>  
> +#include <linux/bits.h>
> +
>  /* ValleyView Power Control Unit PCI Device ID */
>  #define	PCI_DEVICE_ID_VLV_PMC	0x0F1C
>  /* CherryTrail Power Control Unit PCI Device ID */
> @@ -139,9 +141,9 @@
>  #define	ACPI_MMIO_REG_LEN	0x100
>  
>  #define	PM1_CNT			0x4
> -#define	SLEEP_TYPE_MASK		0xFFFFECFF
> +#define	SLEEP_TYPE_MASK		GENMASK(12, 10)
>  #define	SLEEP_TYPE_S5		0x1C00
> -#define	SLEEP_ENABLE		0x2000
> +#define	SLEEP_ENABLE		BIT(13)
>  
>  extern int pmc_atom_read(int offset, u32 *value);
>