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 <[email protected]>
---
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);
--
2.35.1
Make terminator entry uniform by dropping:
- trailing commas
- anything inside curly braces
Signed-off-by: Andy Shevchenko <[email protected]>
---
v2: no changes
drivers/platform/x86/pmc_atom.c | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/drivers/platform/x86/pmc_atom.c b/drivers/platform/x86/pmc_atom.c
index 4c3d6b48d2f0..8d91999446f5 100644
--- a/drivers/platform/x86/pmc_atom.c
+++ b/drivers/platform/x86/pmc_atom.c
@@ -60,7 +60,7 @@ static const struct pmc_clk byt_clks[] = {
.freq = 19200000,
.parent_name = "xtal",
},
- {},
+ {}
};
static const struct pmc_clk cht_clks[] = {
@@ -69,7 +69,7 @@ static const struct pmc_clk cht_clks[] = {
.freq = 19200000,
.parent_name = NULL,
},
- {},
+ {}
};
static const struct pmc_bit_map d3_sts_0_map[] = {
@@ -105,7 +105,7 @@ static const struct pmc_bit_map d3_sts_0_map[] = {
{"LPSS2_F5_I2C5", BIT_LPSS2_F5_I2C5},
{"LPSS2_F6_I2C6", BIT_LPSS2_F6_I2C6},
{"LPSS2_F7_I2C7", BIT_LPSS2_F7_I2C7},
- {},
+ {}
};
static struct pmc_bit_map byt_d3_sts_1_map[] = {
@@ -113,21 +113,21 @@ static struct pmc_bit_map byt_d3_sts_1_map[] = {
{"OTG_SS_PHY", BIT_OTG_SS_PHY},
{"USH_SS_PHY", BIT_USH_SS_PHY},
{"DFX", BIT_DFX},
- {},
+ {}
};
static struct pmc_bit_map cht_d3_sts_1_map[] = {
{"SMB", BIT_SMB},
{"GMM", BIT_STS_GMM},
{"ISH", BIT_STS_ISH},
- {},
+ {}
};
static struct pmc_bit_map cht_func_dis_2_map[] = {
{"SMB", BIT_SMB},
{"GMM", BIT_FD_GMM},
{"ISH", BIT_FD_ISH},
- {},
+ {}
};
static const struct pmc_bit_map byt_pss_map[] = {
@@ -149,7 +149,7 @@ static const struct pmc_bit_map byt_pss_map[] = {
{"OTG_VCCA", PMC_PSS_BIT_OTG_VCCA},
{"USB", PMC_PSS_BIT_USB},
{"USB_SUS", PMC_PSS_BIT_USB_SUS},
- {},
+ {}
};
static const struct pmc_bit_map cht_pss_map[] = {
@@ -172,7 +172,7 @@ static const struct pmc_bit_map cht_pss_map[] = {
{"DFX_CLUSTER3", PMC_PSS_BIT_CHT_DFX_CLUSTER3},
{"DFX_CLUSTER4", PMC_PSS_BIT_CHT_DFX_CLUSTER4},
{"DFX_CLUSTER5", PMC_PSS_BIT_CHT_DFX_CLUSTER5},
- {},
+ {}
};
static const struct pmc_reg_map byt_reg_map = {
@@ -422,8 +422,7 @@ static const struct dmi_system_id critclk_systems[] = {
DMI_MATCH(DMI_SYS_VENDOR, "SIEMENS AG"),
},
},
-
- { /*sentinel*/ }
+ {}
};
static int pmc_setup_clks(struct pci_dev *pdev, void __iomem *pmc_regmap,
@@ -503,7 +502,7 @@ static int pmc_setup_dev(struct pci_dev *pdev, const struct pci_device_id *ent)
static const struct pci_device_id pmc_pci_ids[] = {
{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_VLV_PMC), (kernel_ulong_t)&byt_data },
{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_CHT_PMC), (kernel_ulong_t)&cht_data },
- { 0, },
+ {}
};
static int __init pmc_atom_init(void)
--
2.35.1
The style of the comments is not uniform, make it so and fix
a few grammar issues. While at it, update Copyright years.
Signed-off-by: Andy Shevchenko <[email protected]>
---
v2: no changes
drivers/platform/x86/pmc_atom.c | 19 ++++++++-----------
include/linux/platform_data/x86/pmc_atom.h | 4 ++--
2 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/drivers/platform/x86/pmc_atom.c b/drivers/platform/x86/pmc_atom.c
index 8d91999446f5..3271aec23abe 100644
--- a/drivers/platform/x86/pmc_atom.c
+++ b/drivers/platform/x86/pmc_atom.c
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
- * Intel Atom SOC Power Management Controller Driver
- * Copyright (c) 2014, Intel Corporation.
+ * Intel Atom SoC Power Management Controller Driver
+ * Copyright (c) 2014-2015,2017,2022 Intel Corporation.
*/
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
@@ -494,11 +494,7 @@ static int pmc_setup_dev(struct pci_dev *pdev, const struct pci_device_id *ent)
return ret;
}
-/*
- * Data for PCI driver interface
- *
- * used by pci_match_id() call below.
- */
+/* Data for PCI driver interface used by pci_match_id() call below */
static const struct pci_device_id pmc_pci_ids[] = {
{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_VLV_PMC), (kernel_ulong_t)&byt_data },
{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_CHT_PMC), (kernel_ulong_t)&cht_data },
@@ -510,8 +506,9 @@ static int __init pmc_atom_init(void)
struct pci_dev *pdev = NULL;
const struct pci_device_id *ent;
- /* We look for our device - PCU PMC
- * we assume that there is max. one device.
+ /*
+ * We look for our device - PCU PMC.
+ * We assume that there is maximum one device.
*
* We can't use plain pci_driver mechanism,
* as the device is really a multiple function device,
@@ -523,7 +520,7 @@ static int __init pmc_atom_init(void)
if (ent)
return pmc_setup_dev(pdev, ent);
}
- /* Device not found. */
+ /* Device not found */
return -ENODEV;
}
@@ -531,6 +528,6 @@ device_initcall(pmc_atom_init);
/*
MODULE_AUTHOR("Aubrey Li <[email protected]>");
-MODULE_DESCRIPTION("Intel Atom SOC Power Management Controller Interface");
+MODULE_DESCRIPTION("Intel Atom SoC Power Management Controller Interface");
MODULE_LICENSE("GPL v2");
*/
diff --git a/include/linux/platform_data/x86/pmc_atom.h b/include/linux/platform_data/x86/pmc_atom.h
index ea01dd80153b..ae9b9f4a8697 100644
--- a/include/linux/platform_data/x86/pmc_atom.h
+++ b/include/linux/platform_data/x86/pmc_atom.h
@@ -1,7 +1,7 @@
/* SPDX-License-Identifier: GPL-2.0-only */
/*
- * Intel Atom SOC Power Management Controller Header File
- * Copyright (c) 2014, Intel Corporation.
+ * Intel Atom SoC Power Management Controller Header File
+ * Copyright (c) 2014-2015,2022 Intel Corporation.
*/
#ifndef PMC_ATOM_H
--
2.35.1
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 <[email protected]>
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);
>