2023-01-13 14:44:28

by Rajat Khandelwal

[permalink] [raw]
Subject: [PATCH] platform/x86/intel/pmc: core: Add support to show LTR-ignored components

Currently, 'ltr_ignore' sysfs attribute, when read, returns nothing, even
if there are components whose LTR values have been ignored.

This patch adds the feature to print out such components, if they exist.

Signed-off-by: Rajat Khandelwal <[email protected]>
---
drivers/platform/x86/intel/pmc/core.c | 47 ++++++++++++++++++++-------
1 file changed, 35 insertions(+), 12 deletions(-)

diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
index a1fe1e0dcf4a..30fff4461807 100644
--- a/drivers/platform/x86/intel/pmc/core.c
+++ b/drivers/platform/x86/intel/pmc/core.c
@@ -129,6 +129,14 @@ static const struct pmc_bit_map *ext_spt_pfear_map[] = {
NULL
};

+struct ltr_entry {
+ u32 comp_index;
+ const char *comp_name;
+ struct list_head node;
+};
+
+static LIST_HEAD(ltr_ignore_list);
+
static const struct pmc_bit_map spt_ltr_show_map[] = {
{"SOUTHPORT_A", SPT_PMC_LTR_SPA},
{"SOUTHPORT_B", SPT_PMC_LTR_SPB},
@@ -1327,27 +1335,18 @@ static int pmc_core_pll_show(struct seq_file *s, void *unused)
}
DEFINE_SHOW_ATTRIBUTE(pmc_core_pll);

-static int pmc_core_send_ltr_ignore(struct pmc_dev *pmcdev, u32 value)
+static void pmc_core_send_ltr_ignore(struct pmc_dev *pmcdev, u32 value)
{
const struct pmc_reg_map *map = pmcdev->map;
u32 reg;
- int err = 0;

mutex_lock(&pmcdev->lock);

- if (value > map->ltr_ignore_max) {
- err = -EINVAL;
- goto out_unlock;
- }
-
reg = pmc_core_reg_read(pmcdev, map->ltr_ignore_offset);
reg |= BIT(value);
pmc_core_reg_write(pmcdev, map->ltr_ignore_offset, reg);

-out_unlock:
mutex_unlock(&pmcdev->lock);
-
- return err;
}

static ssize_t pmc_core_ltr_ignore_write(struct file *file,
@@ -1356,6 +1355,8 @@ static ssize_t pmc_core_ltr_ignore_write(struct file *file,
{
struct seq_file *s = file->private_data;
struct pmc_dev *pmcdev = s->private;
+ const struct pmc_reg_map *map = pmcdev->map;
+ struct ltr_entry *entry;
u32 buf_size, value;
int err;

@@ -1365,13 +1366,35 @@ static ssize_t pmc_core_ltr_ignore_write(struct file *file,
if (err)
return err;

- err = pmc_core_send_ltr_ignore(pmcdev, value);
+ if (value > map->ltr_ignore_max)
+ return -EINVAL;

- return err == 0 ? count : err;
+ list_for_each_entry(entry, &ltr_ignore_list, node) {
+ if (entry->comp_index == value)
+ return -EEXIST;
+ }
+
+ entry = kmalloc(sizeof(*entry), GFP_KERNEL);
+ if (!entry)
+ return -ENOMEM;
+
+ entry->comp_name = map->ltr_show_sts[value].name;
+ entry->comp_index = value;
+ list_add_tail(&entry->node, &ltr_ignore_list);
+
+ pmc_core_send_ltr_ignore(pmcdev, value);
+
+ return count;
}

static int pmc_core_ltr_ignore_show(struct seq_file *s, void *unused)
{
+ struct ltr_entry *entry;
+
+ list_for_each_entry(entry, &ltr_ignore_list, node) {
+ seq_printf(s, "%s\n", entry->comp_name);
+ }
+
return 0;
}

--
2.34.1


2023-01-18 22:05:55

by Box, David E

[permalink] [raw]
Subject: Re: [PATCH] platform/x86/intel/pmc: core: Add support to show LTR-ignored components

Hi,

On Fri, 2023-01-13 at 19:32 +0530, Rajat Khandelwal wrote:
> Currently, 'ltr_ignore' sysfs attribute, when read, returns nothing, even
> if there are components whose LTR values have been ignored.
>
> This patch adds the feature to print out such components, if they exist.
>
> Signed-off-by: Rajat Khandelwal <[email protected]>
> ---
>  drivers/platform/x86/intel/pmc/core.c | 47 ++++++++++++++++++++-------
>  1 file changed, 35 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/pmc/core.c
> b/drivers/platform/x86/intel/pmc/core.c
> index a1fe1e0dcf4a..30fff4461807 100644
> --- a/drivers/platform/x86/intel/pmc/core.c
> +++ b/drivers/platform/x86/intel/pmc/core.c
> @@ -129,6 +129,14 @@ static const struct pmc_bit_map *ext_spt_pfear_map[] = {
>         NULL
>  };
>  
> +struct ltr_entry {
> +       u32 comp_index;
> +       const char *comp_name;
> +       struct list_head node;
> +};
> +
> +static LIST_HEAD(ltr_ignore_list);
> +
>  static const struct pmc_bit_map spt_ltr_show_map[] = {
>         {"SOUTHPORT_A",         SPT_PMC_LTR_SPA},
>         {"SOUTHPORT_B",         SPT_PMC_LTR_SPB},
> @@ -1327,27 +1335,18 @@ static int pmc_core_pll_show(struct seq_file *s, void
> *unused)
>  }
>  DEFINE_SHOW_ATTRIBUTE(pmc_core_pll);
>  
> -static int pmc_core_send_ltr_ignore(struct pmc_dev *pmcdev, u32 value)
> +static void pmc_core_send_ltr_ignore(struct pmc_dev *pmcdev, u32 value)
>  {
>         const struct pmc_reg_map *map = pmcdev->map;
>         u32 reg;
> -       int err = 0;
>  
>         mutex_lock(&pmcdev->lock);
>  
> -       if (value > map->ltr_ignore_max) {
> -               err = -EINVAL;
> -               goto out_unlock;
> -       }
> -
>         reg = pmc_core_reg_read(pmcdev, map->ltr_ignore_offset);
>         reg |= BIT(value);
>         pmc_core_reg_write(pmcdev, map->ltr_ignore_offset, reg);
>  
> -out_unlock:
>         mutex_unlock(&pmcdev->lock);
> -
> -       return err;
>  }
>  
>  static ssize_t pmc_core_ltr_ignore_write(struct file *file,
> @@ -1356,6 +1355,8 @@ static ssize_t pmc_core_ltr_ignore_write(struct file
> *file,
>  {
>         struct seq_file *s = file->private_data;
>         struct pmc_dev *pmcdev = s->private;
> +       const struct pmc_reg_map *map = pmcdev->map;
> +       struct ltr_entry *entry;
>         u32 buf_size, value;
>         int err;
>  
> @@ -1365,13 +1366,35 @@ static ssize_t pmc_core_ltr_ignore_write(struct file
> *file,
>         if (err)
>                 return err;
>  
> -       err = pmc_core_send_ltr_ignore(pmcdev, value);
> +       if (value > map->ltr_ignore_max)
> +               return -EINVAL;
>  
> -       return err == 0 ? count : err;
> +       list_for_each_entry(entry, &ltr_ignore_list, node) {
> +               if (entry->comp_index == value)
> +                       return -EEXIST;
> +       }
> +
> +       entry = kmalloc(sizeof(*entry), GFP_KERNEL);
> +       if (!entry)
> +               return -ENOMEM;

Currently will leak memory when the driver is removed. struct pmc_dev has a
pointer to the platform device so you can use devm_kmalloc.

> +
> +       entry->comp_name = map->ltr_show_sts[value].name;
> +       entry->comp_index = value;
> +       list_add_tail(&entry->node, &ltr_ignore_list);
> +
> +       pmc_core_send_ltr_ignore(pmcdev, value);
> +
> +       return count;
>  }
>  
>  static int pmc_core_ltr_ignore_show(struct seq_file *s, void *unused)
>  {
> +       struct ltr_entry *entry;
> +
> +       list_for_each_entry(entry, &ltr_ignore_list, node) {
> +               seq_printf(s, "%s\n", entry->comp_name);
> +       }

It would be helpful to add a first column that shows the index. But I guess we
don't show this in ltr_show either.

David

> +
>         return 0;
>  }
>