mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] platform/x86: intel_pmc_core: Make the driver PCH family agnostic
@ 2017-08-12 16:50 Srinivas Pandruvada
  2017-08-13 13:49 ` Andy Shevchenko
  0 siblings, 1 reply; 2+ messages in thread
From: Srinivas Pandruvada @ 2017-08-12 16:50 UTC (permalink / raw)
  To: rajneesh.bhardwaj, vishwanath.somayaji, dvhart, andy
  Cc: platform-driver-x86, linux-kernel, Srinivas Pandruvada

Although this driver did pretty good job in abstracting PCH specific
interfaces, but still there are some loose ends. For example
SLP_S0 counter (for reading SLP_S0 residency), PM config offset (for
checking permissions to read XRAM) and PPFEAR offset (for reading IP
status) is still hardcoded for a specific family of PCH.

This change extended the struct pmc_reg_map to allow per family
configuration of offsets and bits.

No functional change is expected with this change. This change allows
seamless additions to new PCH and create a baseline for other platform
specific extensions.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/platform/x86/intel_pmc_core.c | 31 +++++++++++++++++++++----------
 drivers/platform/x86/intel_pmc_core.h | 30 +++++++++++++++++++++++++++++-
 2 files changed, 50 insertions(+), 11 deletions(-)

diff --git a/drivers/platform/x86/intel_pmc_core.c b/drivers/platform/x86/intel_pmc_core.c
index 914bcd2..17e08b4 100644
--- a/drivers/platform/x86/intel_pmc_core.c
+++ b/drivers/platform/x86/intel_pmc_core.c
@@ -110,6 +110,13 @@ static const struct pmc_reg_map spt_reg_map = {
 	.pfear_sts = spt_pfear_map,
 	.mphy_sts = spt_mphy_map,
 	.pll_sts = spt_pll_map,
+	.slp_s0_offset = SPT_PMC_SLP_S0_RES_COUNTER_OFFSET,
+	.ltr_ignore_offset = SPT_PMC_LTR_IGNORE_OFFSET,
+	.regmap_length = SPT_PMC_MMIO_REG_LEN,
+	.ppfear0_offset = SPT_PMC_XRAM_PPFEAR0A,
+	.ppfear_buckets = SPT_PPFEAR_NUM_ENTRIES,
+	.pm_cfg_offset = SPT_PMC_PM_CFG_OFFSET,
+	.pm_read_disable_bit = SPT_PMC_READ_DISABLE_BIT,
 };
 
 static const struct pci_device_id pmc_pci_ids[] = {
@@ -157,12 +164,13 @@ static inline u32 pmc_core_adjust_slp_s0_step(u32 value)
 int intel_pmc_slp_s0_counter_read(u32 *data)
 {
 	struct pmc_dev *pmcdev = &pmc;
+	const struct pmc_reg_map *map = pmcdev->map;
 	u32 value;
 
 	if (!pmcdev->has_slp_s0_res)
 		return -EACCES;
 
-	value = pmc_core_reg_read(pmcdev, SPT_PMC_SLP_S0_RES_COUNTER_OFFSET);
+	value = pmc_core_reg_read(pmcdev, map->slp_s0_offset);
 	*data = pmc_core_adjust_slp_s0_step(value);
 
 	return 0;
@@ -172,9 +180,10 @@ EXPORT_SYMBOL_GPL(intel_pmc_slp_s0_counter_read);
 static int pmc_core_dev_state_get(void *data, u64 *val)
 {
 	struct pmc_dev *pmcdev = data;
+	const struct pmc_reg_map *map = pmcdev->map;
 	u32 value;
 
-	value = pmc_core_reg_read(pmcdev, SPT_PMC_SLP_S0_RES_COUNTER_OFFSET);
+	value = pmc_core_reg_read(pmcdev, map->slp_s0_offset);
 	*val = pmc_core_adjust_slp_s0_step(value);
 
 	return 0;
@@ -187,8 +196,8 @@ static int pmc_core_check_read_lock_bit(void)
 	struct pmc_dev *pmcdev = &pmc;
 	u32 value;
 
-	value = pmc_core_reg_read(pmcdev, SPT_PMC_PM_CFG_OFFSET);
-	return value & BIT(SPT_PMC_READ_DISABLE_BIT);
+	value = pmc_core_reg_read(pmcdev, pmcdev->map->pm_cfg_offset);
+	return value & BIT(pmcdev->map->pm_read_disable_bit);
 }
 
 #if IS_ENABLED(CONFIG_DEBUG_FS)
@@ -204,12 +213,13 @@ static int pmc_core_ppfear_sts_show(struct seq_file *s, void *unused)
 {
 	struct pmc_dev *pmcdev = s->private;
 	const struct pmc_bit_map *map = pmcdev->map->pfear_sts;
-	u8 pf_regs[NUM_ENTRIES];
+	u8 pf_regs[PPFEAR_MAX_NUM_ENTRIES];
 	int index, iter;
 
-	iter = SPT_PMC_XRAM_PPFEAR0A;
+	iter = pmcdev->map->ppfear0_offset;
 
-	for (index = 0; index < NUM_ENTRIES; index++, iter++)
+	for (index = 0; index < pmcdev->map->ppfear_buckets &&
+	     index < PPFEAR_MAX_NUM_ENTRIES; index++, iter++)
 		pf_regs[index] = pmc_core_reg_read_byte(pmcdev, iter);
 
 	for (index = 0; map[index].name; index++)
@@ -376,6 +386,7 @@ static ssize_t pmc_core_ltr_ignore_write(struct file *file, const char __user
 *userbuf, size_t count, loff_t *ppos)
 {
 	struct pmc_dev *pmcdev = &pmc;
+	const struct pmc_reg_map *map = pmcdev->map;
 	u32 val, buf_size, fd;
 	int err = 0;
 
@@ -392,9 +403,9 @@ static ssize_t pmc_core_ltr_ignore_write(struct file *file, const char __user
 		goto out_unlock;
 	}
 
-	fd = pmc_core_reg_read(pmcdev, SPT_PMC_LTR_IGNORE_OFFSET);
+	fd = pmc_core_reg_read(pmcdev, map->ltr_ignore_offset);
 	fd |= (1U << val);
-	pmc_core_reg_write(pmcdev, SPT_PMC_LTR_IGNORE_OFFSET, fd);
+	pmc_core_reg_write(pmcdev, map->ltr_ignore_offset, fd);
 
 out_unlock:
 	mutex_unlock(&pmcdev->lock);
@@ -530,8 +541,8 @@ static int pmc_core_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	}
 
 	mutex_init(&pmcdev->lock);
-	pmcdev->pmc_xram_read_bit = pmc_core_check_read_lock_bit();
 	pmcdev->map = map;
+	pmcdev->pmc_xram_read_bit = pmc_core_check_read_lock_bit();
 
 	err = pmc_core_dbgfs_register(pmcdev);
 	if (err < 0)
diff --git a/drivers/platform/x86/intel_pmc_core.h b/drivers/platform/x86/intel_pmc_core.h
index 5a48e77..3d225a9 100644
--- a/drivers/platform/x86/intel_pmc_core.h
+++ b/drivers/platform/x86/intel_pmc_core.h
@@ -38,7 +38,8 @@
 #define SPT_PMC_SLP_S0_RES_COUNTER_STEP		0x64
 #define PMC_BASE_ADDR_MASK			~(SPT_PMC_MMIO_REG_LEN - 1)
 #define MTPMC_MASK				0xffff0000
-#define NUM_ENTRIES				5
+#define PPFEAR_MAX_NUM_ENTRIES			5
+#define SPT_PPFEAR_NUM_ENTRIES			5
 #define SPT_PMC_READ_DISABLE_BIT		0x16
 #define SPT_PMC_MSG_FULL_STS_BIT		0x18
 #define NUM_RETRIES				100
@@ -126,10 +127,37 @@ struct pmc_bit_map {
 	u32 bit_mask;
 };
 
+/**
+ * struct pmc_reg_map - Structure used to define parameter unique to a
+			PCH family
+ * @pfear_sts:		Maps name of IP block to PPFEAR* bit
+ * @mphy_sts:		Maps name of MPHY lane to MPHY status lane status bit
+ * @pll_sts:		Maps name of PLL to corresponding bit status
+ * @slp_s0_offset:	PWRMBASE offset to read SLP_S0 residency
+ * @ltr_ignore_offset:	PWRMBASE offset to read/write LTR ignore bit
+ * @base_address:	Base address of PWRMBASE defined in BIOS writer guide
+ * @regmap_length:	Length of memory to map from PWRMBASE address to access
+ * @ppfear0_offset:	PWRMBASE offset to to read PPFEAR*
+ * @ppfear_buckets:	Number of 8 bits blocks to read all IP blocks from
+ *			PPFEAR
+ * @pm_cfg_offset:	PWRMBASE offset to PM_CFG register
+ * @pm_read_disable_bit: Bit index to read PMC_READ_DISABLE
+ *
+ * Each PCH has unique set of register offsets and bit indexes. This structure
+ * captures them to have a common implementation.
+ */
 struct pmc_reg_map {
 	const struct pmc_bit_map *pfear_sts;
 	const struct pmc_bit_map *mphy_sts;
 	const struct pmc_bit_map *pll_sts;
+	const u32 slp_s0_offset;
+	const u32 ltr_ignore_offset;
+	const u32 base_address;
+	const int regmap_length;
+	const u32 ppfear0_offset;
+	const int ppfear_buckets;
+	const u32 pm_cfg_offset;
+	const int pm_read_disable_bit;
 };
 
 /**
-- 
2.7.4

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] platform/x86: intel_pmc_core: Make the driver PCH family agnostic
  2017-08-12 16:50 [PATCH] platform/x86: intel_pmc_core: Make the driver PCH family agnostic Srinivas Pandruvada
@ 2017-08-13 13:49 ` Andy Shevchenko
  0 siblings, 0 replies; 2+ messages in thread
From: Andy Shevchenko @ 2017-08-13 13:49 UTC (permalink / raw)
  To: Srinivas Pandruvada
  Cc: Rajneesh Bhardwaj, Vishwanath Somayaji, dvhart, Andy Shevchenko,
	Platform Driver, linux-kernel

On Sat, Aug 12, 2017 at 7:50 PM, Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
> Although this driver did pretty good job in abstracting PCH specific
> interfaces, but still there are some loose ends. For example
> SLP_S0 counter (for reading SLP_S0 residency), PM config offset (for
> checking permissions to read XRAM) and PPFEAR offset (for reading IP
> status) is still hardcoded for a specific family of PCH.
>
> This change extended the struct pmc_reg_map to allow per family
> configuration of offsets and bits.
>
> No functional change is expected with this change. This change allows
> seamless additions to new PCH and create a baseline for other platform
> specific extensions.
>

Pushed to testing, thanks!

> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
>  drivers/platform/x86/intel_pmc_core.c | 31 +++++++++++++++++++++----------
>  drivers/platform/x86/intel_pmc_core.h | 30 +++++++++++++++++++++++++++++-
>  2 files changed, 50 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/platform/x86/intel_pmc_core.c b/drivers/platform/x86/intel_pmc_core.c
> index 914bcd2..17e08b4 100644
> --- a/drivers/platform/x86/intel_pmc_core.c
> +++ b/drivers/platform/x86/intel_pmc_core.c
> @@ -110,6 +110,13 @@ static const struct pmc_reg_map spt_reg_map = {
>         .pfear_sts = spt_pfear_map,
>         .mphy_sts = spt_mphy_map,
>         .pll_sts = spt_pll_map,
> +       .slp_s0_offset = SPT_PMC_SLP_S0_RES_COUNTER_OFFSET,
> +       .ltr_ignore_offset = SPT_PMC_LTR_IGNORE_OFFSET,
> +       .regmap_length = SPT_PMC_MMIO_REG_LEN,
> +       .ppfear0_offset = SPT_PMC_XRAM_PPFEAR0A,
> +       .ppfear_buckets = SPT_PPFEAR_NUM_ENTRIES,
> +       .pm_cfg_offset = SPT_PMC_PM_CFG_OFFSET,
> +       .pm_read_disable_bit = SPT_PMC_READ_DISABLE_BIT,
>  };
>
>  static const struct pci_device_id pmc_pci_ids[] = {
> @@ -157,12 +164,13 @@ static inline u32 pmc_core_adjust_slp_s0_step(u32 value)
>  int intel_pmc_slp_s0_counter_read(u32 *data)
>  {
>         struct pmc_dev *pmcdev = &pmc;
> +       const struct pmc_reg_map *map = pmcdev->map;
>         u32 value;
>
>         if (!pmcdev->has_slp_s0_res)
>                 return -EACCES;
>
> -       value = pmc_core_reg_read(pmcdev, SPT_PMC_SLP_S0_RES_COUNTER_OFFSET);
> +       value = pmc_core_reg_read(pmcdev, map->slp_s0_offset);
>         *data = pmc_core_adjust_slp_s0_step(value);
>
>         return 0;
> @@ -172,9 +180,10 @@ EXPORT_SYMBOL_GPL(intel_pmc_slp_s0_counter_read);
>  static int pmc_core_dev_state_get(void *data, u64 *val)
>  {
>         struct pmc_dev *pmcdev = data;
> +       const struct pmc_reg_map *map = pmcdev->map;
>         u32 value;
>
> -       value = pmc_core_reg_read(pmcdev, SPT_PMC_SLP_S0_RES_COUNTER_OFFSET);
> +       value = pmc_core_reg_read(pmcdev, map->slp_s0_offset);
>         *val = pmc_core_adjust_slp_s0_step(value);
>
>         return 0;
> @@ -187,8 +196,8 @@ static int pmc_core_check_read_lock_bit(void)
>         struct pmc_dev *pmcdev = &pmc;
>         u32 value;
>
> -       value = pmc_core_reg_read(pmcdev, SPT_PMC_PM_CFG_OFFSET);
> -       return value & BIT(SPT_PMC_READ_DISABLE_BIT);
> +       value = pmc_core_reg_read(pmcdev, pmcdev->map->pm_cfg_offset);
> +       return value & BIT(pmcdev->map->pm_read_disable_bit);
>  }
>
>  #if IS_ENABLED(CONFIG_DEBUG_FS)
> @@ -204,12 +213,13 @@ static int pmc_core_ppfear_sts_show(struct seq_file *s, void *unused)
>  {
>         struct pmc_dev *pmcdev = s->private;
>         const struct pmc_bit_map *map = pmcdev->map->pfear_sts;
> -       u8 pf_regs[NUM_ENTRIES];
> +       u8 pf_regs[PPFEAR_MAX_NUM_ENTRIES];
>         int index, iter;
>
> -       iter = SPT_PMC_XRAM_PPFEAR0A;
> +       iter = pmcdev->map->ppfear0_offset;
>
> -       for (index = 0; index < NUM_ENTRIES; index++, iter++)
> +       for (index = 0; index < pmcdev->map->ppfear_buckets &&
> +            index < PPFEAR_MAX_NUM_ENTRIES; index++, iter++)
>                 pf_regs[index] = pmc_core_reg_read_byte(pmcdev, iter);
>
>         for (index = 0; map[index].name; index++)
> @@ -376,6 +386,7 @@ static ssize_t pmc_core_ltr_ignore_write(struct file *file, const char __user
>  *userbuf, size_t count, loff_t *ppos)
>  {
>         struct pmc_dev *pmcdev = &pmc;
> +       const struct pmc_reg_map *map = pmcdev->map;
>         u32 val, buf_size, fd;
>         int err = 0;
>
> @@ -392,9 +403,9 @@ static ssize_t pmc_core_ltr_ignore_write(struct file *file, const char __user
>                 goto out_unlock;
>         }
>
> -       fd = pmc_core_reg_read(pmcdev, SPT_PMC_LTR_IGNORE_OFFSET);
> +       fd = pmc_core_reg_read(pmcdev, map->ltr_ignore_offset);
>         fd |= (1U << val);
> -       pmc_core_reg_write(pmcdev, SPT_PMC_LTR_IGNORE_OFFSET, fd);
> +       pmc_core_reg_write(pmcdev, map->ltr_ignore_offset, fd);
>
>  out_unlock:
>         mutex_unlock(&pmcdev->lock);
> @@ -530,8 +541,8 @@ static int pmc_core_probe(struct pci_dev *dev, const struct pci_device_id *id)
>         }
>
>         mutex_init(&pmcdev->lock);
> -       pmcdev->pmc_xram_read_bit = pmc_core_check_read_lock_bit();
>         pmcdev->map = map;
> +       pmcdev->pmc_xram_read_bit = pmc_core_check_read_lock_bit();
>
>         err = pmc_core_dbgfs_register(pmcdev);
>         if (err < 0)
> diff --git a/drivers/platform/x86/intel_pmc_core.h b/drivers/platform/x86/intel_pmc_core.h
> index 5a48e77..3d225a9 100644
> --- a/drivers/platform/x86/intel_pmc_core.h
> +++ b/drivers/platform/x86/intel_pmc_core.h
> @@ -38,7 +38,8 @@
>  #define SPT_PMC_SLP_S0_RES_COUNTER_STEP                0x64
>  #define PMC_BASE_ADDR_MASK                     ~(SPT_PMC_MMIO_REG_LEN - 1)
>  #define MTPMC_MASK                             0xffff0000
> -#define NUM_ENTRIES                            5
> +#define PPFEAR_MAX_NUM_ENTRIES                 5
> +#define SPT_PPFEAR_NUM_ENTRIES                 5
>  #define SPT_PMC_READ_DISABLE_BIT               0x16
>  #define SPT_PMC_MSG_FULL_STS_BIT               0x18
>  #define NUM_RETRIES                            100
> @@ -126,10 +127,37 @@ struct pmc_bit_map {
>         u32 bit_mask;
>  };
>
> +/**
> + * struct pmc_reg_map - Structure used to define parameter unique to a
> +                       PCH family
> + * @pfear_sts:         Maps name of IP block to PPFEAR* bit
> + * @mphy_sts:          Maps name of MPHY lane to MPHY status lane status bit
> + * @pll_sts:           Maps name of PLL to corresponding bit status
> + * @slp_s0_offset:     PWRMBASE offset to read SLP_S0 residency
> + * @ltr_ignore_offset: PWRMBASE offset to read/write LTR ignore bit
> + * @base_address:      Base address of PWRMBASE defined in BIOS writer guide
> + * @regmap_length:     Length of memory to map from PWRMBASE address to access
> + * @ppfear0_offset:    PWRMBASE offset to to read PPFEAR*
> + * @ppfear_buckets:    Number of 8 bits blocks to read all IP blocks from
> + *                     PPFEAR
> + * @pm_cfg_offset:     PWRMBASE offset to PM_CFG register
> + * @pm_read_disable_bit: Bit index to read PMC_READ_DISABLE
> + *
> + * Each PCH has unique set of register offsets and bit indexes. This structure
> + * captures them to have a common implementation.
> + */
>  struct pmc_reg_map {
>         const struct pmc_bit_map *pfear_sts;
>         const struct pmc_bit_map *mphy_sts;
>         const struct pmc_bit_map *pll_sts;
> +       const u32 slp_s0_offset;
> +       const u32 ltr_ignore_offset;
> +       const u32 base_address;
> +       const int regmap_length;
> +       const u32 ppfear0_offset;
> +       const int ppfear_buckets;
> +       const u32 pm_cfg_offset;
> +       const int pm_read_disable_bit;
>  };
>
>  /**
> --
> 2.7.4
>



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2017-08-13 13:49 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-12 16:50 [PATCH] platform/x86: intel_pmc_core: Make the driver PCH family agnostic Srinivas Pandruvada
2017-08-13 13:49 ` Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®