[PATCH 0/9] LTR ignore feature improvement

[PATCH 0/9] LTR ignore feature improvement

[Xi Pardee]

This patch series contains six patches to improve LTR ignore
feature, one patch to modify the usage of pmc_core_send_msg and
two patches to improve PMC Core driver in general.

Improve LTR ignore feature:
platform/x86:intel/pmc: Add support to undo ltr_ignore
platform/x86:intel/pmc: Use the Elvis operator
platform/x86:intel/pmc: Use DEFINE_SHOW_STORE_ATTRIBUTE macro
platform/x86:intel/pmc: Remove unneeded min_t check
platform/x86:intel/pmc: Add support to show ltr_ignore value
platform/x86:intel/pmc: Move pmc assignment closer to first usage

Modify the usage of pmc_core_send_msg:
platform/x86:intel/pmc: Use the return value of pmc_core_send_msg

Improve PMC Core driver:
platform/x86:intel/pmc: Convert index variables to be unsigned
platform/x86:intel/pmc: Simplify mutex usage with cleanup helpers

Xi Pardee (9):
  platform/x86:intel/pmc: Use the return value of pmc_core_send_msg
  platform/x86:intel/pmc: Simplify mutex usage with cleanup helpers
  platform/x86:intel/pmc: Convert index variables to be unsigned
  platform/x86:intel/pmc: Move pmc assignment closer to first usage
  platform/x86:intel/pmc: Add support to show ltr_ignore value
  platform/x86:intel/pmc: Remove unneeded min_t check
  platform/x86:intel/pmc: Use DEFINE_SHOW_STORE_ATTRIBUTE macro
  platform/x86:intel/pmc: Use the Elvis operator
  platform/x86:intel/pmc: Add support to undo ltr_ignore

 drivers/platform/x86/intel/pmc/core.c | 214 +++++++++++++-------------
 1 file changed, 109 insertions(+), 105 deletions(-)

-- 
2.34.1

[PATCH 1/9] platform/x86:intel/pmc: Use the return value of pmc_core_send_msg

[Xi Pardee]

From: Xi Pardee <xi.pardee@intel.com>

Use the return value of pmc_core_send_msg() directly instead of
assigning another code when an error occurs.

Signed-off-by: Xi Pardee <xi.pardee@intel.com>
---
 drivers/platform/x86/intel/pmc/core.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
index 65eb09c7af6a..e656d51e7ad5 100644
--- a/drivers/platform/x86/intel/pmc/core.c
+++ b/drivers/platform/x86/intel/pmc/core.c
@@ -403,18 +403,16 @@ static int pmc_core_mphy_pg_show(struct seq_file *s, void *unused)
 
 	mutex_lock(&pmcdev->lock);
 
-	if (pmc_core_send_msg(pmc, &mphy_core_reg_low) != 0) {
-		err = -EBUSY;
+	err = pmc_core_send_msg(pmc, &mphy_core_reg_low);
+	if (err)
 		goto out_unlock;
-	}
 
 	msleep(10);
 	val_low = pmc_core_reg_read(pmc, SPT_PMC_MFPMC_OFFSET);
 
-	if (pmc_core_send_msg(pmc, &mphy_core_reg_high) != 0) {
-		err = -EBUSY;
+	err = pmc_core_send_msg(pmc, &mphy_core_reg_high);
+	if (err)
 		goto out_unlock;
-	}
 
 	msleep(10);
 	val_high = pmc_core_reg_read(pmc, SPT_PMC_MFPMC_OFFSET);
@@ -455,10 +453,9 @@ static int pmc_core_pll_show(struct seq_file *s, void *unused)
 	mphy_common_reg  = (SPT_PMC_MPHY_COM_STS_0 << 16);
 	mutex_lock(&pmcdev->lock);
 
-	if (pmc_core_send_msg(pmc, &mphy_common_reg) != 0) {
-		err = -EBUSY;
+	err = pmc_core_send_msg(pmc, &mphy_common_reg);
+	if (err)
 		goto out_unlock;
-	}
 
 	/* Observed PMC HW response latency for MTPMC-MFPMC is ~10 ms */
 	msleep(10);
-- 
2.34.1

[PATCH 2/9] platform/x86:intel/pmc: Simplify mutex usage with cleanup helpers

[Xi Pardee]

From: Xi Pardee <xi.pardee@intel.com>

Use macros defined in cleanup.h to automate the mutex lock/unlock flow.

Signed-off-by: Xi Pardee <xi.pardee@intel.com>
---
 drivers/platform/x86/intel/pmc/core.c | 76 +++++++++------------------
 1 file changed, 25 insertions(+), 51 deletions(-)

diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
index e656d51e7ad5..b524b74293ca 100644
--- a/drivers/platform/x86/intel/pmc/core.c
+++ b/drivers/platform/x86/intel/pmc/core.c
@@ -87,35 +87,26 @@ static int set_etr3(struct pmc_dev *pmcdev)
 	struct pmc *pmc = pmcdev->pmcs[PMC_IDX_MAIN];
 	const struct pmc_reg_map *map = pmc->map;
 	u32 reg;
-	int err;
 
 	if (!map->etr3_offset)
 		return -EOPNOTSUPP;
 
-	mutex_lock(&pmcdev->lock);
+	guard(mutex)(&pmcdev->lock);
 
 	/* check if CF9 is locked */
 	reg = pmc_core_reg_read(pmc, map->etr3_offset);
-	if (reg & ETR3_CF9LOCK) {
-		err = -EACCES;
-		goto out_unlock;
-	}
+	if (reg & ETR3_CF9LOCK)
+		return -EACCES;
 
 	/* write CF9 global reset bit */
 	reg |= ETR3_CF9GR;
 	pmc_core_reg_write(pmc, map->etr3_offset, reg);
 
 	reg = pmc_core_reg_read(pmc, map->etr3_offset);
-	if (!(reg & ETR3_CF9GR)) {
-		err = -EIO;
-		goto out_unlock;
-	}
-
-	err = 0;
+	if (!(reg & ETR3_CF9GR))
+		return -EIO;
 
-out_unlock:
-	mutex_unlock(&pmcdev->lock);
-	return err;
+	return 0;
 }
 static umode_t etr3_is_visible(struct kobject *kobj,
 				struct attribute *attr,
@@ -127,9 +118,8 @@ static umode_t etr3_is_visible(struct kobject *kobj,
 	const struct pmc_reg_map *map = pmc->map;
 	u32 reg;
 
-	mutex_lock(&pmcdev->lock);
-	reg = pmc_core_reg_read(pmc, map->etr3_offset);
-	mutex_unlock(&pmcdev->lock);
+	scoped_guard(mutex, &pmcdev->lock)
+		reg = pmc_core_reg_read(pmc, map->etr3_offset);
 
 	return reg & ETR3_CF9LOCK ? attr->mode & (SYSFS_PREALLOC | 0444) : attr->mode;
 }
@@ -145,12 +135,10 @@ static ssize_t etr3_show(struct device *dev,
 	if (!map->etr3_offset)
 		return -EOPNOTSUPP;
 
-	mutex_lock(&pmcdev->lock);
-
-	reg = pmc_core_reg_read(pmc, map->etr3_offset);
-	reg &= ETR3_CF9GR | ETR3_CF9LOCK;
-
-	mutex_unlock(&pmcdev->lock);
+	scoped_guard(mutex, &pmcdev->lock) {
+		reg = pmc_core_reg_read(pmc, map->etr3_offset);
+		reg &= ETR3_CF9GR | ETR3_CF9LOCK;
+	}
 
 	return sysfs_emit(buf, "0x%08x", reg);
 }
@@ -401,18 +389,18 @@ static int pmc_core_mphy_pg_show(struct seq_file *s, void *unused)
 	mphy_core_reg_low  = (SPT_PMC_MPHY_CORE_STS_0 << 16);
 	mphy_core_reg_high = (SPT_PMC_MPHY_CORE_STS_1 << 16);
 
-	mutex_lock(&pmcdev->lock);
+	guard(mutex)(&pmcdev->lock);
 
 	err = pmc_core_send_msg(pmc, &mphy_core_reg_low);
 	if (err)
-		goto out_unlock;
+		return err;
 
 	msleep(10);
 	val_low = pmc_core_reg_read(pmc, SPT_PMC_MFPMC_OFFSET);
 
 	err = pmc_core_send_msg(pmc, &mphy_core_reg_high);
 	if (err)
-		goto out_unlock;
+		return err;
 
 	msleep(10);
 	val_high = pmc_core_reg_read(pmc, SPT_PMC_MFPMC_OFFSET);
@@ -431,9 +419,7 @@ static int pmc_core_mphy_pg_show(struct seq_file *s, void *unused)
 			   "Power gated");
 	}
 
-out_unlock:
-	mutex_unlock(&pmcdev->lock);
-	return err;
+	return 0;
 }
 DEFINE_SHOW_ATTRIBUTE(pmc_core_mphy_pg);
 
@@ -451,11 +437,11 @@ static int pmc_core_pll_show(struct seq_file *s, void *unused)
 	}
 
 	mphy_common_reg  = (SPT_PMC_MPHY_COM_STS_0 << 16);
-	mutex_lock(&pmcdev->lock);
+	guard(mutex)(&pmcdev->lock);
 
 	err = pmc_core_send_msg(pmc, &mphy_common_reg);
 	if (err)
-		goto out_unlock;
+		return err;
 
 	/* Observed PMC HW response latency for MTPMC-MFPMC is ~10 ms */
 	msleep(10);
@@ -467,9 +453,7 @@ static int pmc_core_pll_show(struct seq_file *s, void *unused)
 			   map[index].bit_mask & val ? "Active" : "Idle");
 	}
 
-out_unlock:
-	mutex_unlock(&pmcdev->lock);
-	return err;
+	return 0;
 }
 DEFINE_SHOW_ATTRIBUTE(pmc_core_pll);
 
@@ -508,7 +492,7 @@ int pmc_core_send_ltr_ignore(struct pmc_dev *pmcdev, u32 value, int ignore)
 
 	pr_debug("ltr_ignore for pmc%d: ltr_index:%d\n", pmc_index, ltr_index);
 
-	mutex_lock(&pmcdev->lock);
+	guard(mutex)(&pmcdev->lock);
 
 	reg = pmc_core_reg_read(pmc, map->ltr_ignore_offset);
 	if (ignore)
@@ -517,8 +501,6 @@ int pmc_core_send_ltr_ignore(struct pmc_dev *pmcdev, u32 value, int ignore)
 		reg &= ~BIT(ltr_index);
 	pmc_core_reg_write(pmc, map->ltr_ignore_offset, reg);
 
-	mutex_unlock(&pmcdev->lock);
-
 	return 0;
 }
 
@@ -566,10 +548,10 @@ static void pmc_core_slps0_dbg_latch(struct pmc_dev *pmcdev, bool reset)
 	const struct pmc_reg_map *map = pmc->map;
 	u32 fd;
 
-	mutex_lock(&pmcdev->lock);
+	guard(mutex)(&pmcdev->lock);
 
 	if (!reset && !slps0_dbg_latch)
-		goto out_unlock;
+		return;
 
 	fd = pmc_core_reg_read(pmc, map->slps0_dbg_offset);
 	if (reset)
@@ -579,9 +561,6 @@ static void pmc_core_slps0_dbg_latch(struct pmc_dev *pmcdev, bool reset)
 	pmc_core_reg_write(pmc, map->slps0_dbg_offset, fd);
 
 	slps0_dbg_latch = false;
-
-out_unlock:
-	mutex_unlock(&pmcdev->lock);
 }
 
 static int pmc_core_slps0_dbg_show(struct seq_file *s, void *unused)
@@ -984,26 +963,22 @@ static ssize_t pmc_core_lpm_latch_mode_write(struct file *file,
 	}
 
 	if (clear) {
-		mutex_lock(&pmcdev->lock);
+		guard(mutex)(&pmcdev->lock);
 
 		reg = pmc_core_reg_read(pmc, pmc->map->etr3_offset);
 		reg |= ETR3_CLEAR_LPM_EVENTS;
 		pmc_core_reg_write(pmc, pmc->map->etr3_offset, reg);
 
-		mutex_unlock(&pmcdev->lock);
-
 		return count;
 	}
 
 	if (c10) {
-		mutex_lock(&pmcdev->lock);
+		guard(mutex)(&pmcdev->lock);
 
 		reg = pmc_core_reg_read(pmc, pmc->map->lpm_sts_latch_en_offset);
 		reg &= ~LPM_STS_LATCH_MODE;
 		pmc_core_reg_write(pmc, pmc->map->lpm_sts_latch_en_offset, reg);
 
-		mutex_unlock(&pmcdev->lock);
-
 		return count;
 	}
 
@@ -1012,9 +987,8 @@ static ssize_t pmc_core_lpm_latch_mode_write(struct file *file,
 	 * and clear everything else.
 	 */
 	reg = LPM_STS_LATCH_MODE | BIT(mode);
-	mutex_lock(&pmcdev->lock);
+	guard(mutex)(&pmcdev->lock);
 	pmc_core_reg_write(pmc, pmc->map->lpm_sts_latch_en_offset, reg);
-	mutex_unlock(&pmcdev->lock);
 
 	return count;
 }
-- 
2.34.1

[PATCH 3/9] platform/x86:intel/pmc: Convert index variables to be unsigned

[Xi Pardee]

From: Xi Pardee <xi.pardee@intel.com>

Convert the index variables type to unsigned to avoid confusion and
errors.

Signed-off-by: Xi Pardee <xi.pardee@intel.com>
---
 drivers/platform/x86/intel/pmc/core.c | 51 ++++++++++++++++-----------
 1 file changed, 30 insertions(+), 21 deletions(-)

diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
index b524b74293ca..cbdcbf288f67 100644
--- a/drivers/platform/x86/intel/pmc/core.c
+++ b/drivers/platform/x86/intel/pmc/core.c
@@ -247,7 +247,7 @@ static void pmc_core_slps0_display(struct pmc *pmc, struct device *dev,
 
 static int pmc_core_lpm_get_arr_size(const struct pmc_bit_map **maps)
 {
-	int idx;
+	unsigned int idx;
 
 	for (idx = 0; maps[idx]; idx++)
 		;/* Nothing */
@@ -260,8 +260,8 @@ static void pmc_core_lpm_display(struct pmc *pmc, struct device *dev,
 				 const char *str,
 				 const struct pmc_bit_map **maps)
 {
-	int index, idx, len = 32, bit_mask, arr_size;
-	u32 *lpm_regs;
+	unsigned int index, idx, len = 32, arr_size;
+	u32 bit_mask, *lpm_regs;
 
 	arr_size = pmc_core_lpm_get_arr_size(maps);
 	lpm_regs = kmalloc_array(arr_size, sizeof(*lpm_regs), GFP_KERNEL);
@@ -314,13 +314,13 @@ static void pmc_core_display_map(struct seq_file *s, int index, int idx, int ip,
 static int pmc_core_ppfear_show(struct seq_file *s, void *unused)
 {
 	struct pmc_dev *pmcdev = s->private;
-	int i;
+	unsigned int i;
 
 	for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); ++i) {
 		struct pmc *pmc = pmcdev->pmcs[i];
 		const struct pmc_bit_map **maps;
 		u8 pf_regs[PPFEAR_MAX_NUM_ENTRIES];
-		int index, iter, idx, ip = 0;
+		unsigned int index, iter, idx, ip = 0;
 
 		if (!pmc)
 			continue;
@@ -379,7 +379,8 @@ static int pmc_core_mphy_pg_show(struct seq_file *s, void *unused)
 	const struct pmc_bit_map *map = pmc->map->mphy_sts;
 	u32 mphy_core_reg_low, mphy_core_reg_high;
 	u32 val_low, val_high;
-	int index, err = 0;
+	unsigned int index;
+	int err = 0;
 
 	if (pmcdev->pmc_xram_read_bit) {
 		seq_puts(s, "Access denied: please disable PMC_READ_DISABLE setting in BIOS.");
@@ -429,7 +430,8 @@ static int pmc_core_pll_show(struct seq_file *s, void *unused)
 	struct pmc *pmc = pmcdev->pmcs[PMC_IDX_MAIN];
 	const struct pmc_bit_map *map = pmc->map->pll_sts;
 	u32 mphy_common_reg, val;
-	int index, err = 0;
+	unsigned int index;
+	int err = 0;
 
 	if (pmcdev->pmc_xram_read_bit) {
 		seq_puts(s, "Access denied: please disable PMC_READ_DISABLE setting in BIOS.");
@@ -462,7 +464,8 @@ int pmc_core_send_ltr_ignore(struct pmc_dev *pmcdev, u32 value, int ignore)
 	struct pmc *pmc;
 	const struct pmc_reg_map *map;
 	u32 reg;
-	int pmc_index, ltr_index;
+	unsigned int pmc_index;
+	int ltr_index;
 
 	ltr_index = value;
 	/* For platforms with multiple pmcs, ltr index value given by user
@@ -615,7 +618,7 @@ static int pmc_core_ltr_show(struct seq_file *s, void *unused)
 	u64 decoded_snoop_ltr, decoded_non_snoop_ltr;
 	u32 ltr_raw_data, scale, val;
 	u16 snoop_ltr, nonsnoop_ltr;
-	int i, index, ltr_index = 0;
+	unsigned int i, index, ltr_index = 0;
 
 	for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); ++i) {
 		struct pmc *pmc = pmcdev->pmcs[i];
@@ -703,7 +706,8 @@ static int pmc_core_substate_res_show(struct seq_file *s, void *unused)
 	struct pmc *pmc = pmcdev->pmcs[PMC_IDX_MAIN];
 	const int lpm_adj_x2 = pmc->map->lpm_res_counter_step_x2;
 	u32 offset = pmc->map->lpm_residency_offset;
-	int i, mode;
+	unsigned int i;
+	int mode;
 
 	seq_printf(s, "%-10s %-15s\n", "Substate", "Residency");
 
@@ -719,7 +723,7 @@ DEFINE_SHOW_ATTRIBUTE(pmc_core_substate_res);
 static int pmc_core_substate_sts_regs_show(struct seq_file *s, void *unused)
 {
 	struct pmc_dev *pmcdev = s->private;
-	int i;
+	unsigned int i;
 
 	for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); ++i) {
 		struct pmc *pmc = pmcdev->pmcs[i];
@@ -740,7 +744,7 @@ DEFINE_SHOW_ATTRIBUTE(pmc_core_substate_sts_regs);
 static int pmc_core_substate_l_sts_regs_show(struct seq_file *s, void *unused)
 {
 	struct pmc_dev *pmcdev = s->private;
-	int i;
+	unsigned int i;
 
 	for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); ++i) {
 		struct pmc *pmc = pmcdev->pmcs[i];
@@ -761,7 +765,8 @@ DEFINE_SHOW_ATTRIBUTE(pmc_core_substate_l_sts_regs);
 static void pmc_core_substate_req_header_show(struct seq_file *s, int pmc_index)
 {
 	struct pmc_dev *pmcdev = s->private;
-	int i, mode;
+	unsigned int i;
+	int mode;
 
 	seq_printf(s, "%30s |", "Element");
 	pmc_for_each_mode(i, mode, pmcdev)
@@ -775,7 +780,8 @@ static int pmc_core_substate_req_regs_show(struct seq_file *s, void *unused)
 	struct pmc_dev *pmcdev = s->private;
 	u32 sts_offset;
 	u32 *lpm_req_regs;
-	int num_maps, mp, pmc_index;
+	unsigned int mp, pmc_index;
+	int num_maps;
 
 	for (pmc_index = 0; pmc_index < ARRAY_SIZE(pmcdev->pmcs); ++pmc_index) {
 		struct pmc *pmc = pmcdev->pmcs[pmc_index];
@@ -897,9 +903,10 @@ static int pmc_core_lpm_latch_mode_show(struct seq_file *s, void *unused)
 {
 	struct pmc_dev *pmcdev = s->private;
 	struct pmc *pmc = pmcdev->pmcs[PMC_IDX_MAIN];
+	unsigned int idx;
 	bool c10;
 	u32 reg;
-	int idx, mode;
+	int mode;
 
 	reg = pmc_core_reg_read(pmc, pmc->map->lpm_sts_latch_en_offset);
 	if (reg & LPM_STS_LATCH_MODE) {
@@ -931,7 +938,8 @@ static ssize_t pmc_core_lpm_latch_mode_write(struct file *file,
 	struct pmc *pmc = pmcdev->pmcs[PMC_IDX_MAIN];
 	bool clear = false, c10 = false;
 	unsigned char buf[8];
-	int idx, m, mode;
+	unsigned int idx;
+	int m, mode;
 	u32 reg;
 
 	if (count > sizeof(buf) - 1)
@@ -999,7 +1007,7 @@ static int pmc_core_pkgc_show(struct seq_file *s, void *unused)
 	struct pmc *pmc = s->private;
 	const struct pmc_bit_map *map = pmc->map->msr_sts;
 	u64 pcstate_count;
-	int index;
+	unsigned int index;
 
 	for (index = 0; map[index].name ; index++) {
 		if (rdmsrl_safe(map[index].bit_mask, &pcstate_count))
@@ -1017,7 +1025,7 @@ DEFINE_SHOW_ATTRIBUTE(pmc_core_pkgc);
 
 static bool pmc_core_pri_verify(u32 lpm_pri, u8 *mode_order)
 {
-	int i, j;
+	unsigned int i, j;
 
 	if (!lpm_pri)
 		return false;
@@ -1052,7 +1060,8 @@ void pmc_core_get_low_power_modes(struct pmc_dev *pmcdev)
 	u8 mode_order[LPM_MAX_NUM_MODES];
 	u32 lpm_pri;
 	u32 lpm_en;
-	int mode, i, p;
+	unsigned int i;
+	int mode, p;
 
 	/* Use LPM Maps to indicate support for substates */
 	if (!pmc->map->lpm_num_maps)
@@ -1344,7 +1353,7 @@ static void pmc_core_do_dmi_quirks(struct pmc *pmc)
 static void pmc_core_clean_structure(struct platform_device *pdev)
 {
 	struct pmc_dev *pmcdev = platform_get_drvdata(pdev);
-	int i;
+	unsigned int i;
 
 	for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); ++i) {
 		struct pmc *pmc = pmcdev->pmcs[i];
@@ -1507,7 +1516,7 @@ int pmc_core_resume_common(struct pmc_dev *pmcdev)
 	struct pmc *pmc = pmcdev->pmcs[PMC_IDX_MAIN];
 	const struct pmc_bit_map **maps = pmc->map->lpm_sts;
 	int offset = pmc->map->lpm_status_offset;
-	int i;
+	unsigned int i;
 
 	/* Check if the syspend used S0ix */
 	if (pm_suspend_via_firmware())
-- 
2.34.1

Re: [PATCH 3/9] platform/x86:intel/pmc: Convert index variables to be unsigned

[Ilpo Järvinen]

On Mon, 24 Jun 2024, Xi Pardee wrote:

> From: Xi Pardee <xi.pardee@intel.com>
> 
> Convert the index variables type to unsigned to avoid confusion and
> errors.
> 
> Signed-off-by: Xi Pardee <xi.pardee@intel.com>
> ---
>  drivers/platform/x86/intel/pmc/core.c | 51 ++++++++++++++++-----------
>  1 file changed, 30 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
> index b524b74293ca..cbdcbf288f67 100644
> --- a/drivers/platform/x86/intel/pmc/core.c
> +++ b/drivers/platform/x86/intel/pmc/core.c
> @@ -247,7 +247,7 @@ static void pmc_core_slps0_display(struct pmc *pmc, struct device *dev,
>  
>  static int pmc_core_lpm_get_arr_size(const struct pmc_bit_map **maps)
>  {
> -	int idx;
> +	unsigned int idx;
>  
>  	for (idx = 0; maps[idx]; idx++)
>  		;/* Nothing */
> @@ -260,8 +260,8 @@ static void pmc_core_lpm_display(struct pmc *pmc, struct device *dev,
>  				 const char *str,
>  				 const struct pmc_bit_map **maps)
>  {
> -	int index, idx, len = 32, bit_mask, arr_size;
> -	u32 *lpm_regs;
> +	unsigned int index, idx, len = 32, arr_size;
> +	u32 bit_mask, *lpm_regs;
>  
>  	arr_size = pmc_core_lpm_get_arr_size(maps);

Hi,

Thanks for these patches. I've applied them into review-ilpo branch.

While applying, I altered the return type of pmc_core_lpm_get_arr_size() 
to unsigned int as it seemed the right thing to do after everything 
around it is already unsigned.

-- 
 i.

[PATCH 4/9] platform/x86:intel/pmc: Move pmc assignment closer to first usage

[Xi Pardee]

Move pmc variable assignment closer to the conditional statement of
its first use for better readability.

Signed-off-by: Xi Pardee <xi.pardee@linux.intel.com>
---
 drivers/platform/x86/intel/pmc/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
index cbdcbf288f67..cb7dd22e7406 100644
--- a/drivers/platform/x86/intel/pmc/core.c
+++ b/drivers/platform/x86/intel/pmc/core.c
@@ -621,9 +621,10 @@ static int pmc_core_ltr_show(struct seq_file *s, void *unused)
 	unsigned int i, index, ltr_index = 0;
 
 	for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); ++i) {
-		struct pmc *pmc = pmcdev->pmcs[i];
+		struct pmc *pmc;
 		const struct pmc_bit_map *map;
 
+		pmc = pmcdev->pmcs[i];
 		if (!pmc)
 			continue;
 
-- 
2.34.1

[PATCH 5/9] platform/x86:intel/pmc: Add support to show ltr_ignore value

[Xi Pardee]

From: Xi Pardee <xi.pardee@intel.com>

Add a column in ltr_show output to show if the IP has been ignored.
A mutex lock is used to protect the critical section as other processes
might try to write to the LTR ignore register at the same time.

Signed-off-by: Xi Pardee <xi.pardee@intel.com>
---
 drivers/platform/x86/intel/pmc/core.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
index cb7dd22e7406..3f271ca48164 100644
--- a/drivers/platform/x86/intel/pmc/core.c
+++ b/drivers/platform/x86/intel/pmc/core.c
@@ -623,13 +623,24 @@ static int pmc_core_ltr_show(struct seq_file *s, void *unused)
 	for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); ++i) {
 		struct pmc *pmc;
 		const struct pmc_bit_map *map;
+		u32 ltr_ign_reg;
 
 		pmc = pmcdev->pmcs[i];
 		if (!pmc)
 			continue;
 
+		scoped_guard(mutex, &pmcdev->lock)
+			ltr_ign_reg = pmc_core_reg_read(pmc, pmc->map->ltr_ignore_offset);
+
 		map = pmc->map->ltr_show_sts;
 		for (index = 0; map[index].name; index++) {
+			bool ltr_ign_data;
+
+			if (index > pmc->map->ltr_ignore_max)
+				ltr_ign_data = false;
+			else
+				ltr_ign_data = ltr_ign_reg & BIT(index);
+
 			decoded_snoop_ltr = decoded_non_snoop_ltr = 0;
 			ltr_raw_data = pmc_core_reg_read(pmc,
 							 map[index].bit_mask);
@@ -647,10 +658,10 @@ static int pmc_core_ltr_show(struct seq_file *s, void *unused)
 				decoded_snoop_ltr = val * convert_ltr_scale(scale);
 			}
 
-			seq_printf(s, "%d\tPMC%d:%-32s\tLTR: RAW: 0x%-16x\tNon-Snoop(ns): %-16llu\tSnoop(ns): %-16llu\n",
+			seq_printf(s, "%d\tPMC%d:%-32s\tLTR: RAW: 0x%-16x\tNon-Snoop(ns): %-16llu\tSnoop(ns): %-16llu\tLTR_IGNORE: %d\n",
 				   ltr_index, i, map[index].name, ltr_raw_data,
 				   decoded_non_snoop_ltr,
-				   decoded_snoop_ltr);
+				   decoded_snoop_ltr, ltr_ign_data);
 			ltr_index++;
 		}
 	}
-- 
2.34.1

[PATCH 6/9] platform/x86:intel/pmc: Remove unneeded min_t check

[Xi Pardee]

min_t() check is not needed in pmc_core_ltr_ignore_write().
kstrtox() has a built-in overflow check.

Signed-off-by: Xi Pardee <xi.pardee@linux.intel.com>
---
 drivers/platform/x86/intel/pmc/core.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
index 3f271ca48164..99adef28b6d0 100644
--- a/drivers/platform/x86/intel/pmc/core.c
+++ b/drivers/platform/x86/intel/pmc/core.c
@@ -513,12 +513,10 @@ static ssize_t pmc_core_ltr_ignore_write(struct file *file,
 {
 	struct seq_file *s = file->private_data;
 	struct pmc_dev *pmcdev = s->private;
-	u32 buf_size, value;
+	u32 value;
 	int err;
 
-	buf_size = min_t(u32, count, 64);
-
-	err = kstrtou32_from_user(userbuf, buf_size, 10, &value);
+	err = kstrtou32_from_user(userbuf, count, 10, &value);
 	if (err)
 		return err;
 
-- 
2.34.1

[PATCH 7/9] platform/x86:intel/pmc: Use DEFINE_SHOW_STORE_ATTRIBUTE macro

[Xi Pardee]

DEFINE_SHOW_STORE_ATTRIBUTE() macro can be used for the ltr_ignore
attribute for better readability.

Signed-off-by: Xi Pardee <xi.pardee@linux.intel.com>
---
 drivers/platform/x86/intel/pmc/core.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
index 99adef28b6d0..e75c56ee54e6 100644
--- a/drivers/platform/x86/intel/pmc/core.c
+++ b/drivers/platform/x86/intel/pmc/core.c
@@ -529,19 +529,7 @@ static int pmc_core_ltr_ignore_show(struct seq_file *s, void *unused)
 {
 	return 0;
 }
-
-static int pmc_core_ltr_ignore_open(struct inode *inode, struct file *file)
-{
-	return single_open(file, pmc_core_ltr_ignore_show, inode->i_private);
-}
-
-static const struct file_operations pmc_core_ltr_ignore_ops = {
-	.open           = pmc_core_ltr_ignore_open,
-	.read           = seq_read,
-	.write          = pmc_core_ltr_ignore_write,
-	.llseek         = seq_lseek,
-	.release        = single_release,
-};
+DEFINE_SHOW_STORE_ATTRIBUTE(pmc_core_ltr_ignore);
 
 static void pmc_core_slps0_dbg_latch(struct pmc_dev *pmcdev, bool reset)
 {
@@ -1218,7 +1206,7 @@ static void pmc_core_dbgfs_register(struct pmc_dev *pmcdev)
 				    pmcdev, &pmc_core_ppfear_fops);
 
 	debugfs_create_file("ltr_ignore", 0644, dir, pmcdev,
-			    &pmc_core_ltr_ignore_ops);
+			    &pmc_core_ltr_ignore_fops);
 
 	debugfs_create_file("ltr_show", 0444, dir, pmcdev, &pmc_core_ltr_fops);
 
-- 
2.34.1

[PATCH 8/9] platform/x86:intel/pmc: Use the Elvis operator

[Xi Pardee]

Replace ternary operator with Elvis operator in
pmc_core_ltr_ignore_write() for better readability
of the code.

Signed-off-by: Xi Pardee <xi.pardee@linux.intel.com>
---
 drivers/platform/x86/intel/pmc/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
index e75c56ee54e6..cfdacb04ff8d 100644
--- a/drivers/platform/x86/intel/pmc/core.c
+++ b/drivers/platform/x86/intel/pmc/core.c
@@ -522,7 +522,7 @@ static ssize_t pmc_core_ltr_ignore_write(struct file *file,
 
 	err = pmc_core_send_ltr_ignore(pmcdev, value, 1);
 
-	return err == 0 ? count : err;
+	return err ?: count;
 }
 
 static int pmc_core_ltr_ignore_show(struct seq_file *s, void *unused)
-- 
2.34.1

[PATCH 9/9] platform/x86:intel/pmc: Add support to undo ltr_ignore

[Xi Pardee]

Add ltr_restore support to undo the ltr_ignore action. It sets the
ltr_ignore bit of the corresponding IP to 0. Ltr_restore reuses some
functionality of pmc_core_ltr_ignore_write() so moved the common
functionality into a helper function.

Signed-off-by: Xi Pardee <xi.pardee@linux.intel.com>
---
 drivers/platform/x86/intel/pmc/core.c | 38 ++++++++++++++++++++++-----
 1 file changed, 32 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
index cfdacb04ff8d..9dbd3a43ddad 100644
--- a/drivers/platform/x86/intel/pmc/core.c
+++ b/drivers/platform/x86/intel/pmc/core.c
@@ -507,12 +507,10 @@ int pmc_core_send_ltr_ignore(struct pmc_dev *pmcdev, u32 value, int ignore)
 	return 0;
 }
 
-static ssize_t pmc_core_ltr_ignore_write(struct file *file,
-					 const char __user *userbuf,
-					 size_t count, loff_t *ppos)
+static ssize_t pmc_core_ltr_write(struct pmc_dev *pmcdev,
+				  const char __user *userbuf,
+				  size_t count, int ignore)
 {
-	struct seq_file *s = file->private_data;
-	struct pmc_dev *pmcdev = s->private;
 	u32 value;
 	int err;
 
@@ -520,17 +518,43 @@ static ssize_t pmc_core_ltr_ignore_write(struct file *file,
 	if (err)
 		return err;
 
-	err = pmc_core_send_ltr_ignore(pmcdev, value, 1);
+	err = pmc_core_send_ltr_ignore(pmcdev, value, ignore);
 
 	return err ?: count;
 }
 
+static ssize_t pmc_core_ltr_ignore_write(struct file *file,
+					 const char __user *userbuf,
+					 size_t count, loff_t *ppos)
+{
+	struct seq_file *s = file->private_data;
+	struct pmc_dev *pmcdev = s->private;
+
+	return pmc_core_ltr_write(pmcdev, userbuf, count, 1);
+}
+
 static int pmc_core_ltr_ignore_show(struct seq_file *s, void *unused)
 {
 	return 0;
 }
 DEFINE_SHOW_STORE_ATTRIBUTE(pmc_core_ltr_ignore);
 
+static ssize_t pmc_core_ltr_restore_write(struct file *file,
+					  const char __user *userbuf,
+					  size_t count, loff_t *ppos)
+{
+	struct seq_file *s = file->private_data;
+	struct pmc_dev *pmcdev = s->private;
+
+	return pmc_core_ltr_write(pmcdev, userbuf, count, 0);
+}
+
+static int pmc_core_ltr_restore_show(struct seq_file *s, void *unused)
+{
+	return 0;
+}
+DEFINE_SHOW_STORE_ATTRIBUTE(pmc_core_ltr_restore);
+
 static void pmc_core_slps0_dbg_latch(struct pmc_dev *pmcdev, bool reset)
 {
 	struct pmc *pmc = pmcdev->pmcs[PMC_IDX_MAIN];
@@ -1208,6 +1232,8 @@ static void pmc_core_dbgfs_register(struct pmc_dev *pmcdev)
 	debugfs_create_file("ltr_ignore", 0644, dir, pmcdev,
 			    &pmc_core_ltr_ignore_fops);
 
+	debugfs_create_file("ltr_restore", 0200, dir, pmcdev, &pmc_core_ltr_restore_fops);
+
 	debugfs_create_file("ltr_show", 0444, dir, pmcdev, &pmc_core_ltr_fops);
 
 	if (primary_pmc->map->s0ix_blocker_maps)
-- 
2.34.1