platform/x86: intel_pmc_core: Show Latency Tolerance info

Cannonlake PCH allows us to ignore LTR from more IPs than Sunrisepoint PCH so make the LTR ignore platform specific.

Signed-off-by Rajneesh Bhardwaj <[email protected]>

---
 drivers/platform/x86/intel_pmc_core.c | 4 +++-
 drivers/platform/x86/intel_pmc_core.h | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

drivers/platform/x86/intel_pmc_core.c +3 -1

index 972735bd4c75..c1330a03523d 100644
@@ -149,6 +149,7 @@ static const struct pmc_reg_map spt_reg_map = {
        .ppfear_buckets = SPT_PPFEAR_NUM_ENTRIES,
        .pm_cfg_offset = SPT_PMC_PM_CFG_OFFSET,
        .pm_read_disable_bit = SPT_PMC_READ_DISABLE_BIT,
+       .ltr_ignore_max = SPT_NUM_IP_IGN_ALLOWED,
 };
 
 /* Cannonlake: PGD PFET Enable Ack Status Register(s) bitmap */

@@ -320,6 +321,7 @@ static const struct pmc_reg_map cnp_reg_map = {
        .ppfear_buckets = CNP_PPFEAR_NUM_ENTRIES,
        .pm_cfg_offset = CNP_PMC_PM_CFG_OFFSET,
        .pm_read_disable_bit = CNP_PMC_READ_DISABLE_BIT,
+       .ltr_ignore_max = CNP_NUM_IP_IGN_ALLOWED,
 };
 
 static inline u8 pmc_core_reg_read_byte(struct pmc_dev *pmcdev, int offset)

@@ -566,7 +568,7 @@ static ssize_t pmc_core_ltr_ignore_write(struct file *file, const char __user
                goto out_unlock;
        }
 
-       if (val > NUM_IP_IGN_ALLOWED) {
+       if (val > map->ltr_ignore_max) {
                err = -EINVAL;
                goto out_unlock;
        }

drivers/platform/x86/intel_pmc_core.h +3 -2

index b24407048fa1..12663c58f122 100644
@@ -44,7 +44,7 @@
 #define SPT_PMC_READ_DISABLE_BIT               0x16
 #define SPT_PMC_MSG_FULL_STS_BIT               0x18
 #define NUM_RETRIES                            100
-#define NUM_IP_IGN_ALLOWED                     17
+#define SPT_NUM_IP_IGN_ALLOWED                 17
 #define SPT_PMC_LTR_CUR_PLT                    0x350
 #define SPT_PMC_LTR_CUR_ASLT                   0x354
 #define SPT_PMC_LTR_SPA                                0x360

@@ -153,6 +153,7 @@ enum ppfear_regs {
 #define CNP_PPFEAR_NUM_ENTRIES                 8
 #define CNP_PMC_READ_DISABLE_BIT               22
 #define CNP_PMC_LATCH_SLPS0_EVENTS             BIT(31)
+#define CNP_NUM_IP_IGN_ALLOWED                 19
 #define CNP_PMC_LTR_CUR_PLT                    0x1B50
 #define CNP_PMC_LTR_CUR_ASLT                   0x1B54
 #define CNP_PMC_LTR_SPA                                0x1B60

@@ -215,6 +216,7 @@ struct pmc_reg_map {
        const u32 pm_cfg_offset;
        const int pm_read_disable_bit;
        const u32 slps0_dbg_offset;
+       const u32 ltr_ignore_max;
 };
 
 /**
--

2.17.1

Quoted 3 lines · Rajneesh Bhardwaj

Cannonlake PCH allows us to ignore LTR from more IPs than Sunrisepoint PCH so make the LTR ignore platform specific.

This looks fine to me.

Quoted 67 lines

Signed-off-by Rajneesh Bhardwaj <[email protected]>

---
  drivers/platform/x86/intel_pmc_core.c | 4 +++-
  drivers/platform/x86/intel_pmc_core.h | 4 +++-
  2 files changed, 6 insertions(+), 2 deletions(-)

drivers/platform/x86/intel_pmc_core.c +3 -1

index 972735bd4c75..c1330a03523d 100644
@@ -149,6 +149,7 @@ static const struct pmc_reg_map spt_reg_map = {
         .ppfear_buckets = SPT_PPFEAR_NUM_ENTRIES,
         .pm_cfg_offset = SPT_PMC_PM_CFG_OFFSET,
         .pm_read_disable_bit = SPT_PMC_READ_DISABLE_BIT,
+       .ltr_ignore_max = SPT_NUM_IP_IGN_ALLOWED,
  };
 
  /* Cannonlake: PGD PFET Enable Ack Status Register(s) bitmap */

@@ -320,6 +321,7 @@ static const struct pmc_reg_map cnp_reg_map = {
         .ppfear_buckets = CNP_PPFEAR_NUM_ENTRIES,
         .pm_cfg_offset = CNP_PMC_PM_CFG_OFFSET,
         .pm_read_disable_bit = CNP_PMC_READ_DISABLE_BIT,
+       .ltr_ignore_max = CNP_NUM_IP_IGN_ALLOWED,
  };
 
  static inline u8 pmc_core_reg_read_byte(struct pmc_dev *pmcdev, int offset)

@@ -566,7 +568,7 @@ static ssize_t pmc_core_ltr_ignore_write(struct file *file, const char __user
                 goto out_unlock;
         }
 
-       if (val > NUM_IP_IGN_ALLOWED) {
+       if (val > map->ltr_ignore_max) {
                 err = -EINVAL;
                 goto out_unlock;
         }

drivers/platform/x86/intel_pmc_core.h +3 -2

index b24407048fa1..12663c58f122 100644
@@ -44,7 +44,7 @@
  #define SPT_PMC_READ_DISABLE_BIT               0x16
  #define SPT_PMC_MSG_FULL_STS_BIT               0x18
  #define NUM_RETRIES                            100
-#define NUM_IP_IGN_ALLOWED                     17
+#define SPT_NUM_IP_IGN_ALLOWED                 17
  #define SPT_PMC_LTR_CUR_PLT                    0x350
  #define SPT_PMC_LTR_CUR_ASLT                   0x354
  #define SPT_PMC_LTR_SPA                                0x360

@@ -153,6 +153,7 @@ enum ppfear_regs {
  #define CNP_PPFEAR_NUM_ENTRIES                 8
  #define CNP_PMC_READ_DISABLE_BIT               22
  #define CNP_PMC_LATCH_SLPS0_EVENTS             BIT(31)
+#define CNP_NUM_IP_IGN_ALLOWED                 19
  #define CNP_PMC_LTR_CUR_PLT                    0x1B50
  #define CNP_PMC_LTR_CUR_ASLT                   0x1B54
  #define CNP_PMC_LTR_SPA                                0x1B60

@@ -215,6 +216,7 @@ struct pmc_reg_map {
         const u32 pm_cfg_offset;
         const int pm_read_disable_bit;
         const u32 slps0_dbg_offset;
+       const u32 ltr_ignore_max;
  };
 
  /**
--

2.17.1

The LTR values follow PCIE LTR encoding format and can be decoded as per
https://pcisig.com/sites/default/files/specification_documents/ECN_LatencyTolnReporting_14Aug08.pdf

This adds support to translate the raw LTR values as read from the PMC to meaningful values in nanosecond units of time.

+static void get_ltr_scale(u32 *val)

drivers/platform/x86/intel_pmc_core.c +29 -0

+        * ECN_LatencyTolnReporting_14Aug08.pdf the Latency
+        * Tolerance Reporting data payload is encoded in a
+        * 3 bit scale and 10 bit value fields. Values are
+        * multiplied by the indicated scale to yield an absolute time
+        * value, expressible in a range from 1 nanosecond to
+        * 2^25*(2^10-1) = 34,326,183,936 nanoseconds.
+        *
+        * scale encoding is as follows:
+        *
+        * ----------------------------------------------
+        * |scale factor        |       Multiplier (ns) |
+        * ----------------------------------------------
+        * |    0               |       1               |
+        * |    1               |       32              |
+        * |    2               |       1024            |
+        * |    3               |       32768           |
+        * |    4               |       1048576         |
+        * |    5               |       33554432        |
+        * |    6               |       Invalid         |
+        * |    7               |       Invalid         |
+        * ----------------------------------------------
+        */
+       if (*val > 5) {
+               *val = 0;
+               pr_warn("Invalid LTR scale factor.\n");
+       } else {
+               *val = 1U << (5 * (*val));
+       }
+}

        int index;

        for (index = 0; map[index].name ; index++) {

drivers/platform/x86/intel_pmc_core.c +16 -1

-               seq_printf(s, "%-32s\tRAW LTR: 0x%x\n",
+               ltr_data.raw_data = pmc_core_reg_read(pmcdev,
+                                                     map[index].bit_mask);
+
+               if (ltr_data.bits.non_snoop_req) {
+                       scale = ltr_data.bits.non_snoop_scale;
+                       get_ltr_scale(&scale);
+                       decoded_non_snoop_ltr =
+                               ltr_data.bits.non_snoop_val * scale;
+               }
+
+               if (ltr_data.bits.snoop_req) {
+                       scale = ltr_data.bits.snoop_scale;
+                       get_ltr_scale(&scale);
+                       decoded_snoop_ltr =
+                               ltr_data.bits.snoop_val * scale;
+               }

        }
        return 0;

}

drivers/platform/x86/intel_pmc_core.h +3 -0

+union ltr_payload {
+       u32 raw_data;
+       struct {

Quoted 6 lines · Rajneesh Bhardwaj

The LTR values follow PCIE LTR encoding format and can be decoded as per
https://pcisig.com/sites/default/files/specification_documents/ECN_LatencyTolnReporting_14Aug08.pdf

This adds support to translate the raw LTR values as read from the PMC to meaningful values in nanosecond units of time.
+static void get_ltr_scale(u32 *val)

What's wrong to return converted value? Actually the name should reflect what it does, ie *convert* value.

drivers/platform/x86/intel_pmc_core.c static int pmc_core_slps0_dbg_show(struct seq_file *s, void *unused) · line 657

Quoted 4 lines

drivers/platform/x86/intel_pmc_core.c +3 -0

+{
+       /*
+        * As per PCIE specification supprting document

supporting

Quoted 31 lines

drivers/platform/x86/intel_pmc_core.c +30 -0

+        * ECN_LatencyTolnReporting_14Aug08.pdf the Latency
+        * Tolerance Reporting data payload is encoded in a
+        * 3 bit scale and 10 bit value fields. Values are
+        * multiplied by the indicated scale to yield an absolute time
+        * value, expressible in a range from 1 nanosecond to
+        * 2^25*(2^10-1) = 34,326,183,936 nanoseconds.
+        *
+        * scale encoding is as follows:
+        *
+        * ----------------------------------------------
+        * |scale factor        |       Multiplier (ns) |
+        * ----------------------------------------------
+        * |    0               |       1               |
+        * |    1               |       32              |
+        * |    2               |       1024            |
+        * |    3               |       32768           |
+        * |    4               |       1048576         |
+        * |    5               |       33554432        |
+        * |    6               |       Invalid         |
+        * |    7               |       Invalid         |
+        * ----------------------------------------------
+        */
+       if (*val > 5) {
+               *val = 0;
+               pr_warn("Invalid LTR scale factor.\n");
+       } else {
+               *val = 1U << (5 * (*val));
+       }
+}
+

drivers/platform/x86/intel_pmc_core.c static int pmc_core_slps0_dbg_show(struct seq_file *s, void *unused) · line 694

Quoted 7 lines

  static int pmc_core_ltr_show(struct seq_file *s, void *unused)
  {
         struct pmc_dev *pmcdev = s->private;
         const struct pmc_bit_map *map = pmcdev->map->ltr_show_sts;

drivers/platform/x86/intel_pmc_core.c +3 -0

+       u64 decoded_snoop_ltr = 0, decoded_non_snoop_ltr = 0;
+       union ltr_payload ltr_data;
+       u32 scale = 0;

Redundant assignment.

Quoted 24 lines

         int index;

         for (index = 0; map[index].name ; index++) {

drivers/platform/x86/intel_pmc_core.c +18 -1

-               seq_printf(s, "%-32s\tRAW LTR: 0x%x\n",
+               ltr_data.raw_data = pmc_core_reg_read(pmcdev,
+                                                     map[index].bit_mask);
+
+               if (ltr_data.bits.non_snoop_req) {
+                       scale = ltr_data.bits.non_snoop_scale;
+                       get_ltr_scale(&scale);
+                       decoded_non_snoop_ltr =
+                               ltr_data.bits.non_snoop_val * scale;
+               }
+
+               if (ltr_data.bits.snoop_req) {
+                       scale = ltr_data.bits.snoop_scale;
+                       get_ltr_scale(&scale);
+                       decoded_snoop_ltr =
+                               ltr_data.bits.snoop_val * scale;
+               }
+
+               seq_printf(s, "%-24s\tRaw LTR: 0x%-16x\t Non-Snoop LTR (ns): %-16llu\t Snoop LTR (ns): %-16llu\n",
                            map[index].name,

drivers/platform/x86/intel_pmc_core.c static int pmc_core_slps0_dbg_show(struct seq_file *s, void *unused) · line 721

Quoted 6 lines

drivers/platform/x86/intel_pmc_core.c +5 -1

-                          pmc_core_reg_read(pmcdev, map[index].bit_mask));
+                          ltr_data.raw_data,
+                          decoded_non_snoop_ltr,
+                          decoded_snoop_ltr);
+
+               decoded_snoop_ltr = decoded_non_snoop_ltr = 0;

You may do this at the beginning of the loop and get rid of assignment in the definition block.

Quoted 10 lines

         }
         return 0;

}

drivers/platform/x86/intel_pmc_core.h +6 -0

+union ltr_payload {
+       u32 raw_data;
+       struct {
+       u32 snoop_val : 10;
+       u32 snoop_scale : 3;
+       u32 snoop_res : 2;

drivers/platform/x86/intel_pmc_core.h struct pmc_dev { · line 258

Quoted 7 lines

drivers/platform/x86/intel_pmc_core.h +7 -0

+       u32 snoop_req : 1;
+       u32 non_snoop_val : 10;
+       u32 non_snoop_scale : 3;
+       u32 non_snoop_res : 2;
+       u32 non_snoop_req : 1;
+       } bits;
+};

Just use normal masks and shifts.

Quoted 6 lines · Andy Shevchenko

Quoted 1 line · Rajneesh Bhardwaj

+static void get_ltr_scale(u32 *val)

What's wrong to return converted value? Actually the name should reflect what it does, ie *convert* value.

I can change it as per your suggestion.

Quoted 6 lines

Quoted 6 lines

drivers/platform/x86/intel_pmc_core.h +6 -0

+union ltr_payload {
+       u32 raw_data;
+       struct {
+       u32 snoop_val : 10;
+       u32 snoop_scale : 3;
+       u32 snoop_res : 2;

Quoted 8 lines

drivers/platform/x86/intel_pmc_core.h struct pmc_dev { · line 258

Quoted 7 lines

drivers/platform/x86/intel_pmc_core.h +7 -0

+       u32 snoop_req : 1;
+       u32 non_snoop_val : 10;
+       u32 non_snoop_scale : 3;
+       u32 non_snoop_res : 2;
+       u32 non_snoop_req : 1;
+       } bits;
+};

Just use normal masks and shifts.

I chose union over masks and shifts to reduce code size and ensured correct endian-ness.

Just for my understanding, can you please let me know why you feel masks/shift are better suited here?

On some Goldmont based systems such as ASRock J3455M the BIOS may not enable the IPC1 device that provides access to the PMC and PUNIT. In such scenarios, the ioss and pss resources from the platform device can

not be obtained and result in a invalid telemetry_plt_config.

This is also applicable to the platforms where the BIOS supports IPC1 device under debug configurations but IPC1 is disabled by user or the policy.

This change allows user to know the reason for not seeing entries under /sys/kernel/debug/telemetry/* when there is no apparent failure at boot.

Cc Matt Turner <[email protected]>

Cc Len Brown <[email protected]>

Cc Souvik Kumar Chakravarty <[email protected]>

Cc Kuppuswamy Sathyanarayanan <[email protected]>

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=198779

Quoted 3 lines · Rajneesh Bhardwaj

On some Goldmont based systems such as ASRock J3455M the BIOS may not enable the IPC1 device that provides access to the PMC and PUNIT. In such scenarios, the ioss and pss resources from the platform device can

IOSS
PSS

Quoted 2 lines

not be obtained and result in a invalid telemetry_plt_config.

What is telemetry_plt_config?

Quoted 8 lines

This is also applicable to the platforms where the BIOS supports IPC1 device under debug configurations but IPC1 is disabled by user or the policy.

This change allows user to know the reason for not seeing entries under /sys/kernel/debug/telemetry/* when there is no apparent failure at boot.

Quoted 7 lines

Cc Matt Turner <[email protected]>

Cc Len Brown <[email protected]>

Cc Souvik Kumar Chakravarty <[email protected]>

Cc Kuppuswamy Sathyanarayanan <[email protected]>

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=198779

There should be not a blank line.

drivers/platform/x86/intel_telemetry_debugfs.c static int __init telemetry_debugfs_init(void) · line 1028

Quoted 5 lines

Acked-by Matt Turner <[email protected]>

Signed-off-by Rajneesh Bhardwaj <[email protected]>

drivers/platform/x86/intel_telemetry_debugfs.c +2 -0

+exit:
+       pr_debug(pr_fmt(DRIVER_NAME) " Failed\n");

Completely useless.

Device core does it in generic way.

Quoted 5 lines · Andy Shevchenko

Quoted 1 line · Rajneesh Bhardwaj

not be obtained and result in a invalid telemetry_plt_config.

What is telemetry_plt_config?

Internal data structure that holds platform config, maintained by the telemetry platform driver.

Quoted 6 lines

Quoted 6 lines

This is also applicable to the platforms where the BIOS supports IPC1 device under debug configurations but IPC1 is disabled by user or the policy.

This change allows user to know the reason for not seeing entries under /sys/kernel/debug/telemetry/* when there is no apparent failure at boot.

Quoted 5 lines

drivers/platform/x86/intel_telemetry_debugfs.c static int __init telemetry_debugfs_init(void) · line 1028

Quoted 2 lines

drivers/platform/x86/intel_telemetry_debugfs.c +2 -0

+exit:
+       pr_debug(pr_fmt(DRIVER_NAME) " Failed\n");

Completely useless.

Device core does it in generic way.

If i remove this print then perhaps there is no need of this patch.

Reason to print this is that the platform driver / core driver does not show any error.

In-fact they are even loaded in module table. OTOH, this debugfs interface fails. This is very confusing to the users if they check the lsmod output so i feel this print might help.

Quoted 5 lines

Quoted 3 lines · Rajneesh

Quoted 2 lines · Andy Shevchenko

On Mon, Sep 3, 2018 at 9:05 PM Rajneesh Bhardwaj
<[email protected]> wrote:

Quoted 5 lines

Quoted 4 lines

Quoted 2 lines

Quoted 1 line

not be obtained and result in a invalid telemetry_plt_config.

What is telemetry_plt_config?

Internal data structure that holds platform config, maintained by the telemetry platform driver.

You need to spell if for the reader.

Sure, thanks for the suggestion. I will do it if you agree to my explanation below and require v2.

Quoted 20 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

This is also applicable to the platforms where the BIOS supports IPC1 device under debug configurations but IPC1 is disabled by user or the policy.

Quoted 8 lines

Quoted 7 lines

drivers/platform/x86/intel_telemetry_debugfs.c static int __init telemetry_debugfs_init(void) · line 1028

Quoted 4 lines

This change allows user to know the reason for not seeing entries under /sys/kernel/debug/telemetry/* when there is no apparent failure at boot.

drivers/platform/x86/intel_telemetry_debugfs.c +2 -0

+exit:
+       pr_debug(pr_fmt(DRIVER_NAME) " Failed\n");

Completely useless.

Device core does it in generic way.

If i remove this print then perhaps there is no need of this patch.

Maybe.

Quoted 2 lines

Reason to print this is that the platform driver / core driver does not show any error.

If the code fails and returns 0 — it's a bug in error reporting inside the code.

The existing Telemetry ecosystem for Atom consists of IPC driver, telem core driver and telem platform driver but there are no consumers of the APIs except the ones in telem debugfs driver. Here in this case, not all drivers that make up the telemetry infra return failure. Here is how the system looks w/wo IPC1 setting in the BIOS.

When IPC is present

root@raj-glk:~# lsmod | grep -i telem

intel_telemetry_debugfs245760

intel_telemetry_pltdrv204800

intel_punit_ipc163841 intel_telemetry_pltdrv

intel_telemetry_core163842 intel_telemetry_debugfs,intel_telemetry_pltdrv

intel_pmc_ipc204802 intel_telemetry_debugfs,intel_telemetry_pltdrv

When IPC is missing

root@raj-glk:~# lsmod | grep -i telem

intel_telemetry_pltdrv204800

intel_punit_ipc163841 intel_telemetry_pltdrv

intel_telemetry_core163841 intel_telemetry_pltdrv

intel_pmc_ipc204801 intel_telemetry_pltdrv

If we look at the dmesg log, we see  "intel_telemetry_core Init". So one might think that the driver has loaded fine since user may not know that telemetry is more than just one driver. Such things are often reported by users of this driver.
So IMHO, keeping this error helps user triage the problem easily. I can change to pr_info you'd like it.

Quoted 7 lines

Quoted 4 lines

In-fact they are even loaded in module table. OTOH, this debugfs interface fails. This is very confusing to the users if they check the lsmod output so i feel this print might help.

Again, device core *already has* this and even more (it prints also a return code!).

This adds support to show the Latency Tolerance Reporting for the IPs on the PCH as reported by the PMC. The format shown here is raw LTR data payload that can further be decoded as per the PCI specification.

This also fixes some minor alignment issues in the header file by removing spaces and converting to tabs at some places.

drivers/platform/x86/intel_pmc_core.c +13 -0

+static const struct pmc_bit_map spt_ltr_show_map[] = {
+       {"IP 0  : LTR_SOUTHPORT_A",             SPT_PMC_LTR_SPA},
+       {"IP 1  : LTR_SOUTHPORT_B",             SPT_PMC_LTR_SPB},
+       {"IP 2  : LTR_SATA",                    SPT_PMC_LTR_SATA},
+       {"IP 3  : LTR_GIGABIT_ETHERNET",        SPT_PMC_LTR_GBE},
+       {"IP 4  : LTR_XHCI",                    SPT_PMC_LTR_XHCI},
+       /* IP 5 is reserved */
+       {"IP 6  : LTR_ME",                      SPT_PMC_LTR_ME},
+       /* EVA is Enterprise Value Add, doesn't really exist on PCH */
+       {"IP 7  : LTR_EVA",                     SPT_PMC_LTR_EVA},
+       {"IP 8  : LTR_SOUTHPORT_C",             SPT_PMC_LTR_SPC},
+       {"IP 9  : LTR_HD_AUDIO",                SPT_PMC_LTR_AZ},
+       /* IP 10 is reserved */

+ {},

+

#define NUM_RETRIES 100
#define NUM_IP_IGN_ALLOWED 17

+#define SPT_PMC_LTR_CUR_PLT 0x350

Quoted 6 lines · Rajneesh Bhardwaj

This adds support to show the Latency Tolerance Reporting for the IPs on the PCH as reported by the PMC. The format shown here is raw LTR data payload that can further be decoded as per the PCI specification.

This also fixes some minor alignment issues in the header file by removing spaces and converting to tabs at some places.

Thanks for the patch, my comments below.

Quoted 14 lines

drivers/platform/x86/intel_pmc_core.c +13 -0

+static const struct pmc_bit_map spt_ltr_show_map[] = {
+       {"IP 0  : LTR_SOUTHPORT_A",             SPT_PMC_LTR_SPA},
+       {"IP 1  : LTR_SOUTHPORT_B",             SPT_PMC_LTR_SPB},
+       {"IP 2  : LTR_SATA",                    SPT_PMC_LTR_SATA},
+       {"IP 3  : LTR_GIGABIT_ETHERNET",        SPT_PMC_LTR_GBE},
+       {"IP 4  : LTR_XHCI",                    SPT_PMC_LTR_XHCI},
+       /* IP 5 is reserved */
+       {"IP 6  : LTR_ME",                      SPT_PMC_LTR_ME},
+       /* EVA is Enterprise Value Add, doesn't really exist on PCH */
+       {"IP 7  : LTR_EVA",                     SPT_PMC_LTR_EVA},
+       {"IP 8  : LTR_SOUTHPORT_C",             SPT_PMC_LTR_SPC},
+       {"IP 9  : LTR_HD_AUDIO",                SPT_PMC_LTR_AZ},
+       /* IP 10 is reserved */

drivers/platform/x86/intel_pmc_core.c static const struct pmc_bit_map spt_pfear_map[] = { · line 135

Quoted 10 lines

drivers/platform/x86/intel_pmc_core.c +10 -0

+       {"IP 11 : LTR_LPSS",                    SPT_PMC_LTR_LPSS},
+       {"IP 12 : LTR_SOUTHPORT_D",             SPT_PMC_LTR_SPD},
+       {"IP 13 : LTR_SOUTHPORT_E",             SPT_PMC_LTR_SPE},
+       {"IP 14 : LTR_CAMERA",                  SPT_PMC_LTR_CAM},
+       {"IP 15 : LTR_ESPI",                    SPT_PMC_LTR_ESPI},
+       {"IP 16 : LTR_SCC",                     SPT_PMC_LTR_SCC},
+       {"IP 17 : LTR_ISH",                     SPT_PMC_LTR_ISH},
+       /* Below two cannot be for LTR_IGNORE */
+       {"LTR_CURRENT_PLATFORM",                SPT_PMC_LTR_CUR_PLT},
+       {"LTR_AGGREGATED_SYSTEM",               SPT_PMC_LTR_CUR_ASLT},

Before no map has this fancy "IP xx :" prefixes. Please, remove.

Quoted 2 lines

+ {},

No need for comma

Quoted 1 line

+

Redundant.

Review

Quoted 3 lines

(quoted diff) +2 -0

+};
+static const struct pmc_bit_map cnp_ltr_show_map[] = {

Same comments as above.

drivers/platform/x86/intel_pmc_core.c static int pmc_core_dbgfs_register(struct pmc_dev *pmcdev) · line 692

Quoted 3 lines

drivers/platform/x86/intel_pmc_core.c +3 -0

+};
+       debugfs_create_file("ltr_show", 0644, dir, pmcdev,
+                           &pmc_core_ltr_fops);

One line?

Quoted 3 lines

  #define NUM_RETRIES                            100
  #define NUM_IP_IGN_ALLOWED                     17

+ blank line here.

Quoted 2 lines

+#define SPT_PMC_LTR_CUR_PLT 0x350

Quoted 7 lines · Andy Shevchenko

drivers/platform/x86/intel_pmc_core.c static const struct pmc_bit_map spt_pfear_map[] = { · line 132

Quoted 2 lines · Rajneesh Bhardwaj

drivers/platform/x86/intel_pmc_core.c +2 -0

+       {"IP 16 : LTR_SCC",                     SPT_PMC_LTR_SCC},
+       {"IP 17 : LTR_ISH",                     SPT_PMC_LTR_ISH},

Before no map has this fancy "IP xx :" prefixes. Please, remove.

The users of the driver often ask for IP Numbers while performing
LTR_IGNORE operation so this is deliberately added. Please consider it.