platform/x86: intel_pmc_core: Add CNP SLPS0 debug registers

PATCH Superseded David E. Box 4 participants 15 messages May 25, 2018 – Jul 2, 2018 1 message not in the archive x86 platform drivers

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): CAHp75VfLqvjVPwhcsK4enhsvXSLggX9_pO9AozZT56DWDrkjPg@mail.gmail.com

David E. Box

author

2018-05-25 01:10 UTC #1 lore raw

PATCH v2 1/1 platform/x86: intel_pmc_core: Add CNP SLPS0 debug registers

Adds debugfs access to registers in the Cannon Point PCH PMC that are useful for debugging #SLP_S0 signal assertion and other low power related activities. Device pm states are latched in these registers whenever the package enters C10 and can be read from slp_s0_debug_status. The pm states may also be latched by writing 1 to slp_s0_dbg_latch which will immediately capture the current state on the next read of slp_s0_debug_status. Also while in intel_pmc_core.h clean up spacing.

Signed-off-by David E. Box <[email protected]>

---

V2:
Per Andy Shevchenko:
- Clear latch bit after use
- Pass pmc_dev as parameter
- Use DEFINE_SHOW_ATTRIBUTE macro

 drivers/platform/x86/intel_pmc_core.c | 112 ++++++++++++++++++++++++++++++++++
 drivers/platform/x86/intel_pmc_core.h |  27 +++++---
 2 files changed, 132 insertions(+), 7 deletions(-)

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

index 43bbe74..ed40999 100644
@@ -196,9 +196,64 @@ static const struct pmc_bit_map cnp_pfear_map[] = {
 	{}
 };
 
+static const struct pmc_bit_map cnp_slps0_dbg0_map[] = {
+	{"AUDIO_D3",		BIT(0)},
+	{"OTG_D3",		BIT(1)},
+	{"XHCI_D3",		BIT(2)},
+	{"LPIO_D3",		BIT(3)},
+	{"SDX_D3",		BIT(4)},
+	{"SATA_D3",		BIT(5)},
+	{"UFS0_D3",		BIT(6)},
+	{"UFS1_D3",		BIT(7)},
+	{"EMMC_D3",		BIT(8)},
+};
+
+static const struct pmc_bit_map cnp_slps0_dbg1_map[] = {
+	{"SDIO_PLL_OFF",	BIT(0)},
+	{"USB2_PLL_OFF",	BIT(1)},
+	{"AUDIO_PLL_OFF",	BIT(2)},
+	{"OC_PLL_OFF",		BIT(3)},
+	{"MAIN_PLL_OFF",	BIT(4)},
+	{"XOSC_OFF",		BIT(5)},
+	{"LPC_CLKS_GATED",	BIT(6)},
+	{"PCIE_CLKREQS_IDLE",	BIT(7)},
+	{"AUDIO_ROSC_OFF",	BIT(8)},
+	{"HPET_XOSC_CLK_REQ",	BIT(9)},
+	{"PMC_ROSC_SLOW_CLK",	BIT(10)},
+	{"AON2_ROSC_GATED",	BIT(11)},
+	{"CLKACKS_DEASSERTED",	BIT(12)},
+};
+
+static const struct pmc_bit_map cnp_slps0_dbg2_map[] = {
+	{"MPHY_CORE_GATED",	BIT(0)},
+	{"CSME_GATED",		BIT(1)},
+	{"USB2_SUS_GATED",	BIT(2)},
+	{"DYN_FLEX_IO_IDLE",	BIT(3)},
+	{"GBE_NO_LINK",		BIT(4)},
+	{"THERM_SEN_DISABLED",	BIT(5)},
+	{"PCIE_LOW_POWER",	BIT(6)},
+	{"ISH_VNNAON_REQ_ACT",	BIT(7)},
+	{"ISH_VNN_REQ_ACT",	BIT(8)},
+	{"CNV_VNNAON_REQ_ACT",	BIT(9)},
+	{"CNV_VNN_REQ_ACT",	BIT(10)},
+	{"NPK_VNNON_REQ_ACT",	BIT(11)},
+	{"PMSYNC_STATE_IDLE",	BIT(12)},
+	{"ALST_GT_THRES",	BIT(13)},
+	{"PMC_ARC_PG_READY",	BIT(14)},
+};
+
+static const struct slps0_dbg_map cnp_slps0_dbg_maps[] = {
+	{cnp_slps0_dbg0_map, ARRAY_SIZE(cnp_slps0_dbg0_map)},
+	{cnp_slps0_dbg1_map, ARRAY_SIZE(cnp_slps0_dbg1_map)},
+	{cnp_slps0_dbg2_map, ARRAY_SIZE(cnp_slps0_dbg2_map)},
+};
+
 static const struct pmc_reg_map cnp_reg_map = {
 	.pfear_sts = cnp_pfear_map,
 	.slp_s0_offset = CNP_PMC_SLP_S0_RES_COUNTER_OFFSET,
+	.slps0_dbg_maps = cnp_slps0_dbg_maps,
+	.slps0_dbg_offset = CNP_PMC_SLPS0_DBG_OFFSET,
+	.slps0_dbg_num = ARRAY_SIZE(cnp_slps0_dbg_maps),
 	.ltr_ignore_offset = CNP_PMC_LTR_IGNORE_OFFSET,
 	.regmap_length = CNP_PMC_MMIO_REG_LEN,
 	.ppfear0_offset = CNP_PMC_HOST_PPFEAR0A,

@@ -252,6 +307,8 @@ static int pmc_core_check_read_lock_bit(void)
 }
 
 #if IS_ENABLED(CONFIG_DEBUG_FS)
+static bool slps0_dbg_latch;
+
 static void pmc_core_display_map(struct seq_file *s, int index,
 				 u8 pf_reg, const struct pmc_bit_map *pf_map)
 {

@@ -481,6 +538,52 @@ static const struct file_operations pmc_core_ltr_ignore_ops = {
 	.release        = single_release,
 };
 
+static void pmc_core_slps0_dbg_latch(struct pmc_dev *pmcdev, bool reset)
+{
+	const struct pmc_reg_map *map = pmcdev->map;
+	u32 fd;
+
+	mutex_lock(&pmcdev->lock);
+
+	if (!reset && !slps0_dbg_latch)
+		goto out_unlock;
+
+	fd = pmc_core_reg_read(pmcdev, map->slps0_dbg_offset);
+	reset ? (fd &= ~CNP_PMC_LATCH_SLPS0_EVENTS) :
+		(fd |= CNP_PMC_LATCH_SLPS0_EVENTS);
+	pmc_core_reg_write(pmcdev, map->slps0_dbg_offset, fd);
+
+	slps0_dbg_latch = 0;
+
+out_unlock:
+	mutex_unlock(&pmcdev->lock);
+}
+
+static int pmc_core_slps0_dbg_show(struct seq_file *s, void *unused)
+{
+	struct pmc_dev *pmcdev = s ? s->private : &pmc;
+	const struct slps0_dbg_map *maps = pmcdev->map->slps0_dbg_maps;
+	int num_slps0_dbg_regs = pmcdev->map->slps0_dbg_num;
+	int i, offset;
+	u32 data;
+
+	pmc_core_slps0_dbg_latch(pmcdev, false);
+	offset = pmcdev->map->slps0_dbg_offset;
+	while (num_slps0_dbg_regs--) {
+		data = pmc_core_reg_read(pmcdev, offset);
+		offset += 4;
+		for (i = 0; i < maps->size; i++)
+			seq_printf(s, "SLP_S0_DBG: %-32s\tState: %s\n",
+				   maps->slps0_dbg_sts[i].name,
+				   data & maps->slps0_dbg_sts[i].bit_mask ?
+				   "Yes" : "No");
+		maps++;
+	}
+	pmc_core_slps0_dbg_latch(pmcdev, true);
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(pmc_core_slps0_dbg);
+
 static void pmc_core_dbgfs_unregister(struct pmc_dev *pmcdev)
 {
 	debugfs_remove_recursive(pmcdev->dbgfs_dir);

@@ -514,6 +617,15 @@ static int pmc_core_dbgfs_register(struct pmc_dev *pmcdev)
 				    0444, dir, pmcdev,
 				    &pmc_core_mphy_pg_ops);
 
+	if (pmcdev->map->slps0_dbg_maps) {
+		debugfs_create_file("slp_s0_debug_status", 0444,
+				    dir, pmcdev,
+				    &pmc_core_slps0_dbg_fops);
+
+		debugfs_create_bool("slp_s0_dbg_latch", 0644,
+				    dir, &slps0_dbg_latch);
+	}
+
 	return 0;
 }
 #else

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

index 5fa5f97..97e4b5a 100644
@@ -124,27 +124,35 @@ enum ppfear_regs {
 #define SPT_PMC_BIT_MPHY_CMN_LANE3		BIT(3)
 
 /* Cannonlake Power Management Controller register offsets */
-#define CNP_PMC_SLP_S0_RES_COUNTER_OFFSET      0x193C
-#define CNP_PMC_LTR_IGNORE_OFFSET              0x1B0C
-#define CNP_PMC_PM_CFG_OFFSET                  0x1818
+#define CNP_PMC_SLP_S0_RES_COUNTER_OFFSET	0x193C
+#define CNP_PMC_LTR_IGNORE_OFFSET		0x1B0C
+#define CNP_PMC_PM_CFG_OFFSET			0x1818
+#define CNP_PMC_SLPS0_DBG_OFFSET		0x10B4
 /* Cannonlake: PGD PFET Enable Ack Status Register(s) start */
-#define CNP_PMC_HOST_PPFEAR0A                  0x1D90
+#define CNP_PMC_HOST_PPFEAR0A			0x1D90
 
-#define CNP_PMC_MMIO_REG_LEN                   0x2000
-#define CNP_PPFEAR_NUM_ENTRIES                 8
-#define CNP_PMC_READ_DISABLE_BIT               22
+#define CNP_PMC_MMIO_REG_LEN			0x2000
+#define CNP_PPFEAR_NUM_ENTRIES			8
+#define CNP_PMC_READ_DISABLE_BIT		22
+#define CNP_PMC_LATCH_SLPS0_EVENTS		BIT(31)
 
 struct pmc_bit_map {
 	const char *name;
 	u32 bit_mask;
 };
 
+struct slps0_dbg_map {
+	const struct pmc_bit_map *slps0_dbg_sts;
+	int size;
+};
+
 /**
  * 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
+ * @slps0_dbg_maps:	Array of SLP_S0_DBG* registers containing debug info
  * @slp_s0_offset:	PWRMBASE offset to read SLP_S0 residency
  * @ltr_ignore_offset:	PWRMBASE offset to read/write LTR ignore bit
  * @regmap_length:	Length of memory to map from PWRMBASE address to access

@@ -153,6 +161,8 @@ struct pmc_bit_map {
  *			PPFEAR
  * @pm_cfg_offset:	PWRMBASE offset to PM_CFG register
  * @pm_read_disable_bit: Bit index to read PMC_READ_DISABLE
+ * @slps0_dbg_offset:	PWRMBASE offset to SLP_S0_DEBUG_REG*
+ * @slps0_dbg_num:	Number of SLP_S0_DEBUG_REG registers
  *
  * Each PCH has unique set of register offsets and bit indexes. This structure
  * captures them to have a common implementation.

@@ -161,6 +171,7 @@ 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 struct slps0_dbg_map *slps0_dbg_maps;
 	const u32 slp_s0_offset;
 	const u32 ltr_ignore_offset;
 	const int regmap_length;

@@ -168,6 +179,8 @@ struct pmc_reg_map {
 	const int ppfear_buckets;
 	const u32 pm_cfg_offset;
 	const int pm_read_disable_bit;
+	const u32 slps0_dbg_offset;
+	const int slps0_dbg_num;
 };
 
 /**

Signature · 1 line

2.7.4

Rajneesh Bhardwaj

2018-05-28 07:00 UTC #2 lore raw

On Thu, May 24, 2018 at 06:10:56PM -0700, David E. Box wrote:

Thanks for sending this, Dave. Few comments below.

Quoted 1 line

Adds debugfs access to registers in the Cannon Point PCH PMC that are

Please use Cannonlake PCH.

Quoted 1 line

useful for debugging #SLP_S0 signal assertion and other low power related

assertion failures and other low power debug.

Quoted 6 lines

activities. Device pm states are latched in these registers whenever the package enters C10 and can be read from slp_s0_debug_status. The pm states may also be latched by writing 1 to slp_s0_dbg_latch which will immediately capture the current state on the next read of slp_s0_debug_status. Also while in intel_pmc_core.h clean up spacing.

Initially, unless the latch bit is not set the debugfs file does not show any information as expected but once you write Y to latch file, the debugfs file continues to show blockers even though you write N back there or unload
- reload the driver. Please fix this.

Quoted 28 lines

Signed-off-by David E. Box <[email protected]>

---

V2:
Per Andy Shevchenko:
- Clear latch bit after use
- Pass pmc_dev as parameter
- Use DEFINE_SHOW_ATTRIBUTE macro

 drivers/platform/x86/intel_pmc_core.c | 112 ++++++++++++++++++++++++++++++++++
 drivers/platform/x86/intel_pmc_core.h |  27 +++++---
 2 files changed, 132 insertions(+), 7 deletions(-)

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

index 43bbe74..ed40999 100644
@@ -196,9 +196,64 @@ static const struct pmc_bit_map cnp_pfear_map[] = {
 	{}
 };
 
+static const struct pmc_bit_map cnp_slps0_dbg0_map[] = {
+	{"AUDIO_D3",		BIT(0)},
+	{"OTG_D3",		BIT(1)},
+	{"XHCI_D3",		BIT(2)},
+	{"LPIO_D3",		BIT(3)},
+	{"SDX_D3",		BIT(4)},
+	{"SATA_D3",		BIT(5)},

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

Quoted 10 lines

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

+	{"UFS0_D3",		BIT(6)},
+	{"UFS1_D3",		BIT(7)},
+	{"EMMC_D3",		BIT(8)},
+};
+
+static const struct pmc_bit_map cnp_slps0_dbg1_map[] = {
+	{"SDIO_PLL_OFF",	BIT(0)},
+	{"USB2_PLL_OFF",	BIT(1)},
+	{"AUDIO_PLL_OFF",	BIT(2)},
+	{"OC_PLL_OFF",		BIT(3)},

Please rename to ISCLK_OC as it is the preferred nomenclature for debug.

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

Quoted 1 line

drivers/platform/x86/intel_pmc_core.c +1 -0
+	{"MAIN_PLL_OFF",	BIT(4)},

Ditto, please use ISCLK_MAIN.

Quoted 85 lines

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

+	{"XOSC_OFF",		BIT(5)},
+	{"LPC_CLKS_GATED",	BIT(6)},
+	{"PCIE_CLKREQS_IDLE",	BIT(7)},
+	{"AUDIO_ROSC_OFF",	BIT(8)},
+	{"HPET_XOSC_CLK_REQ",	BIT(9)},
+	{"PMC_ROSC_SLOW_CLK",	BIT(10)},
+	{"AON2_ROSC_GATED",	BIT(11)},
+	{"CLKACKS_DEASSERTED",	BIT(12)},
+};
+
+static const struct pmc_bit_map cnp_slps0_dbg2_map[] = {
+	{"MPHY_CORE_GATED",	BIT(0)},
+	{"CSME_GATED",		BIT(1)},
+	{"USB2_SUS_GATED",	BIT(2)},
+	{"DYN_FLEX_IO_IDLE",	BIT(3)},
+	{"GBE_NO_LINK",		BIT(4)},
+	{"THERM_SEN_DISABLED",	BIT(5)},
+	{"PCIE_LOW_POWER",	BIT(6)},
+	{"ISH_VNNAON_REQ_ACT",	BIT(7)},
+	{"ISH_VNN_REQ_ACT",	BIT(8)},
+	{"CNV_VNNAON_REQ_ACT",	BIT(9)},
+	{"CNV_VNN_REQ_ACT",	BIT(10)},
+	{"NPK_VNNON_REQ_ACT",	BIT(11)},
+	{"PMSYNC_STATE_IDLE",	BIT(12)},
+	{"ALST_GT_THRES",	BIT(13)},
+	{"PMC_ARC_PG_READY",	BIT(14)},
+};
+
+static const struct slps0_dbg_map cnp_slps0_dbg_maps[] = {
+	{cnp_slps0_dbg0_map, ARRAY_SIZE(cnp_slps0_dbg0_map)},
+	{cnp_slps0_dbg1_map, ARRAY_SIZE(cnp_slps0_dbg1_map)},
+	{cnp_slps0_dbg2_map, ARRAY_SIZE(cnp_slps0_dbg2_map)},
+};
+
 static const struct pmc_reg_map cnp_reg_map = {
 	.pfear_sts = cnp_pfear_map,
 	.slp_s0_offset = CNP_PMC_SLP_S0_RES_COUNTER_OFFSET,
+	.slps0_dbg_maps = cnp_slps0_dbg_maps,
+	.slps0_dbg_offset = CNP_PMC_SLPS0_DBG_OFFSET,
+	.slps0_dbg_num = ARRAY_SIZE(cnp_slps0_dbg_maps),
 	.ltr_ignore_offset = CNP_PMC_LTR_IGNORE_OFFSET,
 	.regmap_length = CNP_PMC_MMIO_REG_LEN,
 	.ppfear0_offset = CNP_PMC_HOST_PPFEAR0A,

@@ -252,6 +307,8 @@ static int pmc_core_check_read_lock_bit(void)
 }
 
 #if IS_ENABLED(CONFIG_DEBUG_FS)
+static bool slps0_dbg_latch;
+
 static void pmc_core_display_map(struct seq_file *s, int index,
 				 u8 pf_reg, const struct pmc_bit_map *pf_map)
 {

@@ -481,6 +538,52 @@ static const struct file_operations pmc_core_ltr_ignore_ops = {
 	.release        = single_release,
 };
 
+static void pmc_core_slps0_dbg_latch(struct pmc_dev *pmcdev, bool reset)
+{
+	const struct pmc_reg_map *map = pmcdev->map;
+	u32 fd;
+
+	mutex_lock(&pmcdev->lock);
+
+	if (!reset && !slps0_dbg_latch)
+		goto out_unlock;
+
+	fd = pmc_core_reg_read(pmcdev, map->slps0_dbg_offset);
+	reset ? (fd &= ~CNP_PMC_LATCH_SLPS0_EVENTS) :
+		(fd |= CNP_PMC_LATCH_SLPS0_EVENTS);
+	pmc_core_reg_write(pmcdev, map->slps0_dbg_offset, fd);
+
+	slps0_dbg_latch = 0;
+
+out_unlock:
+	mutex_unlock(&pmcdev->lock);
+}
+
+static int pmc_core_slps0_dbg_show(struct seq_file *s, void *unused)
+{
+	struct pmc_dev *pmcdev = s ? s->private : &pmc;
+	const struct slps0_dbg_map *maps = pmcdev->map->slps0_dbg_maps;
+	int num_slps0_dbg_regs = pmcdev->map->slps0_dbg_num;
+	int i, offset;
+	u32 data;
+

drivers/platform/x86/intel_pmc_core.c static const struct file_operations pmc_core_ltr_ignore_ops = { · line 579

Quoted 10 lines

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

+	pmc_core_slps0_dbg_latch(pmcdev, false);
+	offset = pmcdev->map->slps0_dbg_offset;
+	while (num_slps0_dbg_regs--) {
+		data = pmc_core_reg_read(pmcdev, offset);
+		offset += 4;
+		for (i = 0; i < maps->size; i++)
+			seq_printf(s, "SLP_S0_DBG: %-32s\tState: %s\n",
+				   maps->slps0_dbg_sts[i].name,
+				   data & maps->slps0_dbg_sts[i].bit_mask ?
+				   "Yes" : "No");

In current form, it looks pretty similar to pch_ip_power_gating_status output. Since it helps in debug, please use Blocks_slp_s0: instead of State:

drivers/platform/x86/intel_pmc_core.c static const struct file_operations pmc_core_ltr_ignore_ops = { · line 585

Quoted 6 lines

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

+		maps++;
+	}
+	pmc_core_slps0_dbg_latch(pmcdev, true);
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(pmc_core_slps0_dbg);

When the latch bit is not set, can we format the output of this attribute stating clearly whether the current blockers are "live" or the "last captured at PC10 entry" ?

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

Quoted 10 lines

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

+
 static void pmc_core_dbgfs_unregister(struct pmc_dev *pmcdev)
 {
 	debugfs_remove_recursive(pmcdev->dbgfs_dir);

@@ -514,6 +617,15 @@ static int pmc_core_dbgfs_register(struct pmc_dev *pmcdev)
 				    0444, dir, pmcdev,
 				    &pmc_core_mphy_pg_ops);
 
+	if (pmcdev->map->slps0_dbg_maps) {
+		debugfs_create_file("slp_s0_debug_status", 0444,

slp_s0_dbg_latch

Quoted 86 lines

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

+				    dir, pmcdev,
+				    &pmc_core_slps0_dbg_fops);
+
+		debugfs_create_bool("slp_s0_dbg_latch", 0644,
+				    dir, &slps0_dbg_latch);
+	}
+
 	return 0;
 }
 #else

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

index 5fa5f97..97e4b5a 100644
@@ -124,27 +124,35 @@ enum ppfear_regs {
 #define SPT_PMC_BIT_MPHY_CMN_LANE3		BIT(3)
 
 /* Cannonlake Power Management Controller register offsets */
-#define CNP_PMC_SLP_S0_RES_COUNTER_OFFSET      0x193C
-#define CNP_PMC_LTR_IGNORE_OFFSET              0x1B0C
-#define CNP_PMC_PM_CFG_OFFSET                  0x1818
+#define CNP_PMC_SLP_S0_RES_COUNTER_OFFSET	0x193C
+#define CNP_PMC_LTR_IGNORE_OFFSET		0x1B0C
+#define CNP_PMC_PM_CFG_OFFSET			0x1818
+#define CNP_PMC_SLPS0_DBG_OFFSET		0x10B4
 /* Cannonlake: PGD PFET Enable Ack Status Register(s) start */
-#define CNP_PMC_HOST_PPFEAR0A                  0x1D90
+#define CNP_PMC_HOST_PPFEAR0A			0x1D90
 
-#define CNP_PMC_MMIO_REG_LEN                   0x2000
-#define CNP_PPFEAR_NUM_ENTRIES                 8
-#define CNP_PMC_READ_DISABLE_BIT               22
+#define CNP_PMC_MMIO_REG_LEN			0x2000
+#define CNP_PPFEAR_NUM_ENTRIES			8
+#define CNP_PMC_READ_DISABLE_BIT		22
+#define CNP_PMC_LATCH_SLPS0_EVENTS		BIT(31)
 
 struct pmc_bit_map {
 	const char *name;
 	u32 bit_mask;
 };
 
+struct slps0_dbg_map {
+	const struct pmc_bit_map *slps0_dbg_sts;
+	int size;
+};
+
 /**
  * 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
+ * @slps0_dbg_maps:	Array of SLP_S0_DBG* registers containing debug info
  * @slp_s0_offset:	PWRMBASE offset to read SLP_S0 residency
  * @ltr_ignore_offset:	PWRMBASE offset to read/write LTR ignore bit
  * @regmap_length:	Length of memory to map from PWRMBASE address to access

@@ -153,6 +161,8 @@ struct pmc_bit_map {
  *			PPFEAR
  * @pm_cfg_offset:	PWRMBASE offset to PM_CFG register
  * @pm_read_disable_bit: Bit index to read PMC_READ_DISABLE
+ * @slps0_dbg_offset:	PWRMBASE offset to SLP_S0_DEBUG_REG*
+ * @slps0_dbg_num:	Number of SLP_S0_DEBUG_REG registers
  *
  * Each PCH has unique set of register offsets and bit indexes. This structure
  * captures them to have a common implementation.

@@ -161,6 +171,7 @@ 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 struct slps0_dbg_map *slps0_dbg_maps;
 	const u32 slp_s0_offset;
 	const u32 ltr_ignore_offset;
 	const int regmap_length;

@@ -168,6 +179,8 @@ struct pmc_reg_map {
 	const int ppfear_buckets;
 	const u32 pm_cfg_offset;
 	const int pm_read_disable_bit;
+	const u32 slps0_dbg_offset;
+	const int slps0_dbg_num;
 };
 
 /**

Signature · 1 line

2.7.4

Signature · 2 lines

Best Regards,
Rajneesh

David E. Box

author

2018-05-30 10:53 UTC #3 lore raw

Hi Rajneesh,

Quoted 24 lines · Rajneesh Bhardwaj

On Thu, May 24, 2018 at 06:10:56PM -0700, David E. Box wrote:
Thanks for sending this, Dave. Few comments below.
Quoted 2 lines
Adds debugfs access to registers in the Cannon Point PCH PMC that are
Please use Cannonlake PCH.
Quoted 2 lines
useful for debugging #SLP_S0 signal assertion and other low power related
assertion failures and other low power debug.
Quoted 9 lines
activities. Device pm states are latched in these registers whenever the
package enters C10 and can be read from slp_s0_debug_status. The pm states may also be latched by writing 1 to slp_s0_dbg_latch which will
immediately capture the current state on the next read of slp_s0_debug_status. Also while in intel_pmc_core.h clean up spacing.

Quoted 7 lines

Initially, unless the latch bit is not set the debugfs file does not show
any information as expected but once you write Y to latch file, the debugfs
file continues to show blockers even though you write N back there or unload
- reload the driver. Please fix this.

See comments below

Quoted 37 lines

Signed-off-by David E. Box <[email protected]>

---

V2:
Per Andy Shevchenko:
- Clear latch bit after use
- Pass pmc_dev as parameter
- Use DEFINE_SHOW_ATTRIBUTE macro

 drivers/platform/x86/intel_pmc_core.c | 112
++++++++++++++++++++++++++++++++++
 drivers/platform/x86/intel_pmc_core.h |  27 +++++---
 2 files changed, 132 insertions(+), 7 deletions(-)

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

index 43bbe74..ed40999 100644
@@ -196,9 +196,64 @@ static const struct pmc_bit_map
 cnp_pfear_map[] = {
 	{}
 };
 
+static const struct pmc_bit_map cnp_slps0_dbg0_map[] = {
+	{"AUDIO_D3",		BIT(0)},
+	{"OTG_D3",		BIT(1)},
+	{"XHCI_D3",		BIT(2)},
+	{"LPIO_D3",		BIT(3)},
+	{"SDX_D3",		BIT(4)},
+	{"SATA_D3",		BIT(5)},
+	{"UFS0_D3",		BIT(6)},
+	{"UFS1_D3",		BIT(7)},
+	{"EMMC_D3",		BIT(8)},
+};
+

Quoted 8 lines

drivers/platform/x86/intel_pmc_core.c static const struct pmc_bit_map cnp_pfear_map[] = { · line 215
Quoted 5 lines
drivers/platform/x86/intel_pmc_core.c +5 -0
+static const struct pmc_bit_map cnp_slps0_dbg1_map[] = {
+	{"SDIO_PLL_OFF",	BIT(0)},
+	{"USB2_PLL_OFF",	BIT(1)},
+	{"AUDIO_PLL_OFF",	BIT(2)},
+	{"OC_PLL_OFF",		BIT(3)},
Please rename to ISCLK_OC as it is the preferred nomenclature for debug.

Okay

Quoted 105 lines

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

Quoted 2 lines

drivers/platform/x86/intel_pmc_core.c +1 -0
+	{"MAIN_PLL_OFF",	BIT(4)},

Ditto, please use ISCLK_MAIN.

Quoted 100 lines

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

+	{"XOSC_OFF",		BIT(5)},
+	{"LPC_CLKS_GATED",	BIT(6)},
+	{"PCIE_CLKREQS_IDLE",	BIT(7)},
+	{"AUDIO_ROSC_OFF",	BIT(8)},
+	{"HPET_XOSC_CLK_REQ",	BIT(9)},
+	{"PMC_ROSC_SLOW_CLK",	BIT(10)},
+	{"AON2_ROSC_GATED",	BIT(11)},
+	{"CLKACKS_DEASSERTED",	BIT(12)},
+};
+
+static const struct pmc_bit_map cnp_slps0_dbg2_map[] = {
+	{"MPHY_CORE_GATED",	BIT(0)},
+	{"CSME_GATED",		BIT(1)},
+	{"USB2_SUS_GATED",	BIT(2)},
+	{"DYN_FLEX_IO_IDLE",	BIT(3)},
+	{"GBE_NO_LINK",		BIT(4)},
+	{"THERM_SEN_DISABLED",	BIT(5)},
+	{"PCIE_LOW_POWER",	BIT(6)},
+	{"ISH_VNNAON_REQ_ACT",	BIT(7)},
+	{"ISH_VNN_REQ_ACT",	BIT(8)},
+	{"CNV_VNNAON_REQ_ACT",	BIT(9)},
+	{"CNV_VNN_REQ_ACT",	BIT(10)},
+	{"NPK_VNNON_REQ_ACT",	BIT(11)},
+	{"PMSYNC_STATE_IDLE",	BIT(12)},
+	{"ALST_GT_THRES",	BIT(13)},
+	{"PMC_ARC_PG_READY",	BIT(14)},
+};
+
+static const struct slps0_dbg_map cnp_slps0_dbg_maps[] = {
+	{cnp_slps0_dbg0_map, ARRAY_SIZE(cnp_slps0_dbg0_map)},
+	{cnp_slps0_dbg1_map, ARRAY_SIZE(cnp_slps0_dbg1_map)},
+	{cnp_slps0_dbg2_map, ARRAY_SIZE(cnp_slps0_dbg2_map)},
+};
+
 static const struct pmc_reg_map cnp_reg_map = {
 	.pfear_sts = cnp_pfear_map,
 	.slp_s0_offset = CNP_PMC_SLP_S0_RES_COUNTER_OFFSET,
+	.slps0_dbg_maps = cnp_slps0_dbg_maps,
+	.slps0_dbg_offset = CNP_PMC_SLPS0_DBG_OFFSET,
+	.slps0_dbg_num = ARRAY_SIZE(cnp_slps0_dbg_maps),
 	.ltr_ignore_offset = CNP_PMC_LTR_IGNORE_OFFSET,
 	.regmap_length = CNP_PMC_MMIO_REG_LEN,

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

 	.ppfear0_offset = CNP_PMC_HOST_PPFEAR0A,

@@ -252,6 +307,8 @@ static int pmc_core_check_read_lock_bit(void)
 }
 
 #if IS_ENABLED(CONFIG_DEBUG_FS)
+static bool slps0_dbg_latch;
+
 static void pmc_core_display_map(struct seq_file *s, int index,
 				 u8 pf_reg, const struct
 pmc_bit_map *pf_map)

{

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

@@ -481,6 +538,52 @@ static const struct file_operations
 pmc_core_ltr_ignore_ops = {
 	.release        = single_release,
 };
 
+static void pmc_core_slps0_dbg_latch(struct pmc_dev *pmcdev, bool
 reset)
+{
+	const struct pmc_reg_map *map = pmcdev->map;
+	u32 fd;
+
+	mutex_lock(&pmcdev->lock);
+
+	if (!reset && !slps0_dbg_latch)
+		goto out_unlock;
+
+	fd = pmc_core_reg_read(pmcdev, map->slps0_dbg_offset);
+	reset ? (fd &= ~CNP_PMC_LATCH_SLPS0_EVENTS) :
+		(fd |= CNP_PMC_LATCH_SLPS0_EVENTS);
+	pmc_core_reg_write(pmcdev, map->slps0_dbg_offset, fd);
+
+	slps0_dbg_latch = 0;
+
+out_unlock:
+	mutex_unlock(&pmcdev->lock);
+}
+
+static int pmc_core_slps0_dbg_show(struct seq_file *s, void
 *unused)
+{
+	struct pmc_dev *pmcdev = s ? s->private : &pmc;
+	const struct slps0_dbg_map *maps = pmcdev->map-

Quoted 1 line

slps0_dbg_maps;

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

+	int num_slps0_dbg_regs = pmcdev->map->slps0_dbg_num;
+	int i, offset;
+	u32 data;
+
+	pmc_core_slps0_dbg_latch(pmcdev, false);
+	offset = pmcdev->map->slps0_dbg_offset;
+	while (num_slps0_dbg_regs--) {
+		data = pmc_core_reg_read(pmcdev, offset);
+		offset += 4;
+		for (i = 0; i < maps->size; i++)
+			seq_printf(s, "SLP_S0_DBG: %-32s\tState:

%s\n",

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

+				   maps->slps0_dbg_sts[i].name,
+				   data & maps-

Quoted 8 lines

drivers/platform/x86/intel_pmc_core.c static const struct file_operations pmc_core_ltr_ignore_ops = { · line 579
Quoted 2 lines
Quoted 1 line
slps0_dbg_sts[i].bit_mask ?
drivers/platform/x86/intel_pmc_core.c +1 -0
+				   "Yes" : "No");
In current form, it looks pretty similar to pch_ip_power_gating_status
output. Since it helps in debug, please use Blocks_slp_s0: instead of State:

That may not be true. What blocks or doesn't can be configured by the OEM.

Quoted 11 lines

drivers/platform/x86/intel_pmc_core.c static const struct file_operations pmc_core_ltr_ignore_ops = { · line 585
Quoted 6 lines
drivers/platform/x86/intel_pmc_core.c +6 -0
+		maps++;
+	}
+	pmc_core_slps0_dbg_latch(pmcdev, true);
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(pmc_core_slps0_dbg);
When the latch bit is not set, can we format the output of this attribute
stating clearly whether the current blockers are "live" or the "last captured at PC10 entry" ?

There is no "live". It's always the last captured state which is why the status doesn't update the way you expect above. With V2 the latch is automatically reset after every read.

When the latch isn't set, any updates would indeed be from PC10 transitions, but you can't be sure this isn't the last forced latch state unless you monitor PC10 residency and assume that the values were updated when the count changes. Even with the latch, if there's a PC10 transition that occurs after the bit is written but before the registers are read, the values may be from that transition, not the latch.

Quoted 110 lines

drivers/platform/x86/intel_pmc_core.c static int pmc_core_dbgfs_register(struct · line 622

Quoted 11 lines

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

+
 static void pmc_core_dbgfs_unregister(struct pmc_dev *pmcdev)
 {
 	debugfs_remove_recursive(pmcdev->dbgfs_dir);

@@ -514,6 +617,15 @@ static int pmc_core_dbgfs_register(struct
 pmc_dev *pmcdev)
 				    0444, dir, pmcdev,
 				    &pmc_core_mphy_pg_ops);
 
+	if (pmcdev->map->slps0_dbg_maps) {
+		debugfs_create_file("slp_s0_debug_status", 0444,

slp_s0_dbg_latch

Quoted 94 lines

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

+				    dir, pmcdev,
+				    &pmc_core_slps0_dbg_fops);
+
+		debugfs_create_bool("slp_s0_dbg_latch", 0644,
+				    dir, &slps0_dbg_latch);
+	}
+
 	return 0;
 }

#else

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

index 5fa5f97..97e4b5a 100644
@@ -124,27 +124,35 @@ enum ppfear_regs {
 #define SPT_PMC_BIT_MPHY_CMN_LANE3		BIT(3)
 
 /* Cannonlake Power Management Controller register offsets */
-#define CNP_PMC_SLP_S0_RES_COUNTER_OFFSET      0x193C
-#define CNP_PMC_LTR_IGNORE_OFFSET              0x1B0C
-#define CNP_PMC_PM_CFG_OFFSET                  0x1818
+#define CNP_PMC_SLP_S0_RES_COUNTER_OFFSET	0x193C
+#define CNP_PMC_LTR_IGNORE_OFFSET		0x1B0C
+#define CNP_PMC_PM_CFG_OFFSET			0x1818
+#define CNP_PMC_SLPS0_DBG_OFFSET		0x10B4
 /* Cannonlake: PGD PFET Enable Ack Status Register(s) start */
-#define CNP_PMC_HOST_PPFEAR0A                  0x1D90
+#define CNP_PMC_HOST_PPFEAR0A			0x1D90
 
-#define CNP_PMC_MMIO_REG_LEN                   0x2000
-#define CNP_PPFEAR_NUM_ENTRIES                 8
-#define CNP_PMC_READ_DISABLE_BIT               22
+#define CNP_PMC_MMIO_REG_LEN			0x2000
+#define CNP_PPFEAR_NUM_ENTRIES			8
+#define CNP_PMC_READ_DISABLE_BIT		22
+#define CNP_PMC_LATCH_SLPS0_EVENTS		BIT(31)
 
 struct pmc_bit_map {
 	const char *name;
 	u32 bit_mask;
 };
 
+struct slps0_dbg_map {
+	const struct pmc_bit_map *slps0_dbg_sts;
+	int size;
+};
+
 /**
  * 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
+ * @slps0_dbg_maps: Array of SLP_S0_DBG* registers
containing debug info
* @slp_s0_offset: PWRMBASE offset to read SLP_S0 residency
* @ltr_ignore_offset: PWRMBASE offset to read/write LTR
ignore bit
* @regmap_length: Length of memory to map from PWRMBASE
address to access

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

@@ -153,6 +161,8 @@ struct pmc_bit_map {
  *			PPFEAR
  * @pm_cfg_offset:	PWRMBASE offset to PM_CFG register
  * @pm_read_disable_bit: Bit index to read PMC_READ_DISABLE
+ * @slps0_dbg_offset:	PWRMBASE offset to SLP_S0_DEBUG_REG*
+ * @slps0_dbg_num:	Number of SLP_S0_DEBUG_REG registers
  *
  * Each PCH has unique set of register offsets and bit indexes.

This structure
* captures them to have a common implementation.

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

@@ -161,6 +171,7 @@ 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 struct slps0_dbg_map *slps0_dbg_maps;
 	const u32 slp_s0_offset;
 	const u32 ltr_ignore_offset;
 	const int regmap_length;

@@ -168,6 +179,8 @@ struct pmc_reg_map {
 	const int ppfear_buckets;
 	const u32 pm_cfg_offset;
 	const int pm_read_disable_bit;
+	const u32 slps0_dbg_offset;
+	const int slps0_dbg_num;
 };
 
 /**

Signature · 1 line

2.7.4

Rajneesh Bhardwaj

2018-05-30 11:33 UTC #4 lore raw

On Wed, May 30, 2018 at 03:53:12AM -0700, David E. Box wrote:

Hi Dave,

Quoted 199 lines

Hi Rajneesh,
Quoted 24 lines · Rajneesh Bhardwaj
On Thu, May 24, 2018 at 06:10:56PM -0700, David E. Box wrote:
Thanks for sending this, Dave. Few comments below.
Quoted 2 lines
Adds debugfs access to registers in the Cannon Point PCH PMC that are
Please use Cannonlake PCH.
Quoted 2 lines
useful for debugging #SLP_S0 signal assertion and other low power related
assertion failures and other low power debug.
Quoted 9 lines
activities. Device pm states are latched in these registers whenever the
package enters C10 and can be read from slp_s0_debug_status. The pm states may also be latched by writing 1 to slp_s0_dbg_latch which will
immediately capture the current state on the next read of slp_s0_debug_status. Also while in intel_pmc_core.h clean up spacing.
Quoted 7 lines
Initially, unless the latch bit is not set the debugfs file does not show
any information as expected but once you write Y to latch file, the debugfs
file continues to show blockers even though you write N back there or unload
- reload the driver. Please fix this.
See comments below
Quoted 37 lines
Quoted 37 lines
Signed-off-by David E. Box <[email protected]>
---
V2:
Per Andy Shevchenko:
- Clear latch bit after use
- Pass pmc_dev as parameter
- Use DEFINE_SHOW_ATTRIBUTE macro
 drivers/platform/x86/intel_pmc_core.c | 112
++++++++++++++++++++++++++++++++++
 drivers/platform/x86/intel_pmc_core.h |  27 +++++---
 2 files changed, 132 insertions(+), 7 deletions(-)
drivers/platform/x86/intel_pmc_core.c +12 -0
index 43bbe74..ed40999 100644
@@ -196,9 +196,64 @@ static const struct pmc_bit_map
 cnp_pfear_map[] = {
 	{}
 };
 
+static const struct pmc_bit_map cnp_slps0_dbg0_map[] = {
+	{"AUDIO_D3",		BIT(0)},
+	{"OTG_D3",		BIT(1)},
+	{"XHCI_D3",		BIT(2)},
+	{"LPIO_D3",		BIT(3)},
+	{"SDX_D3",		BIT(4)},
+	{"SATA_D3",		BIT(5)},
+	{"UFS0_D3",		BIT(6)},
+	{"UFS1_D3",		BIT(7)},
+	{"EMMC_D3",		BIT(8)},
+};
+
Quoted 8 lines
drivers/platform/x86/intel_pmc_core.c static const struct pmc_bit_map cnp_pfear_map[] = { · line 215
Quoted 5 lines
drivers/platform/x86/intel_pmc_core.c +5 -0
+static const struct pmc_bit_map cnp_slps0_dbg1_map[] = {
+	{"SDIO_PLL_OFF",	BIT(0)},
+	{"USB2_PLL_OFF",	BIT(1)},
+	{"AUDIO_PLL_OFF",	BIT(2)},
+	{"OC_PLL_OFF",		BIT(3)},
Please rename to ISCLK_OC as it is the preferred nomenclature for debug.
Okay
Quoted 105 lines
drivers/platform/x86/intel_pmc_core.c static const struct pmc_bit_map cnp_pfear_map[] = { · line 216
Quoted 2 lines
drivers/platform/x86/intel_pmc_core.c +1 -0
+	{"MAIN_PLL_OFF",	BIT(4)},
Ditto, please use ISCLK_MAIN.
Quoted 100 lines
drivers/platform/x86/intel_pmc_core.c +37 -0
+	{"XOSC_OFF",		BIT(5)},
+	{"LPC_CLKS_GATED",	BIT(6)},
+	{"PCIE_CLKREQS_IDLE",	BIT(7)},
+	{"AUDIO_ROSC_OFF",	BIT(8)},
+	{"HPET_XOSC_CLK_REQ",	BIT(9)},
+	{"PMC_ROSC_SLOW_CLK",	BIT(10)},
+	{"AON2_ROSC_GATED",	BIT(11)},
+	{"CLKACKS_DEASSERTED",	BIT(12)},
+};
+
+static const struct pmc_bit_map cnp_slps0_dbg2_map[] = {
+	{"MPHY_CORE_GATED",	BIT(0)},
+	{"CSME_GATED",		BIT(1)},
+	{"USB2_SUS_GATED",	BIT(2)},
+	{"DYN_FLEX_IO_IDLE",	BIT(3)},
+	{"GBE_NO_LINK",		BIT(4)},
+	{"THERM_SEN_DISABLED",	BIT(5)},
+	{"PCIE_LOW_POWER",	BIT(6)},
+	{"ISH_VNNAON_REQ_ACT",	BIT(7)},
+	{"ISH_VNN_REQ_ACT",	BIT(8)},
+	{"CNV_VNNAON_REQ_ACT",	BIT(9)},
+	{"CNV_VNN_REQ_ACT",	BIT(10)},
+	{"NPK_VNNON_REQ_ACT",	BIT(11)},
+	{"PMSYNC_STATE_IDLE",	BIT(12)},
+	{"ALST_GT_THRES",	BIT(13)},
+	{"PMC_ARC_PG_READY",	BIT(14)},
+};
+
+static const struct slps0_dbg_map cnp_slps0_dbg_maps[] = {
+	{cnp_slps0_dbg0_map, ARRAY_SIZE(cnp_slps0_dbg0_map)},
+	{cnp_slps0_dbg1_map, ARRAY_SIZE(cnp_slps0_dbg1_map)},
+	{cnp_slps0_dbg2_map, ARRAY_SIZE(cnp_slps0_dbg2_map)},
+};
+
 static const struct pmc_reg_map cnp_reg_map = {
 	.pfear_sts = cnp_pfear_map,
 	.slp_s0_offset = CNP_PMC_SLP_S0_RES_COUNTER_OFFSET,
+	.slps0_dbg_maps = cnp_slps0_dbg_maps,
+	.slps0_dbg_offset = CNP_PMC_SLPS0_DBG_OFFSET,
+	.slps0_dbg_num = ARRAY_SIZE(cnp_slps0_dbg_maps),
 	.ltr_ignore_offset = CNP_PMC_LTR_IGNORE_OFFSET,
 	.regmap_length = CNP_PMC_MMIO_REG_LEN,
drivers/platform/x86/intel_pmc_core.c +2 -0
 	.ppfear0_offset = CNP_PMC_HOST_PPFEAR0A,
@@ -252,6 +307,8 @@ static int pmc_core_check_read_lock_bit(void)
 }
 
 #if IS_ENABLED(CONFIG_DEBUG_FS)
+static bool slps0_dbg_latch;
+
 static void pmc_core_display_map(struct seq_file *s, int index,
 				 u8 pf_reg, const struct
 pmc_bit_map *pf_map)
{
drivers/platform/x86/intel_pmc_core.c +25 -0
@@ -481,6 +538,52 @@ static const struct file_operations
 pmc_core_ltr_ignore_ops = {
 	.release        = single_release,
 };
 
+static void pmc_core_slps0_dbg_latch(struct pmc_dev *pmcdev, bool
 reset)
+{
+	const struct pmc_reg_map *map = pmcdev->map;
+	u32 fd;
+
+	mutex_lock(&pmcdev->lock);
+
+	if (!reset && !slps0_dbg_latch)
+		goto out_unlock;
+
+	fd = pmc_core_reg_read(pmcdev, map->slps0_dbg_offset);
+	reset ? (fd &= ~CNP_PMC_LATCH_SLPS0_EVENTS) :
+		(fd |= CNP_PMC_LATCH_SLPS0_EVENTS);
+	pmc_core_reg_write(pmcdev, map->slps0_dbg_offset, fd);
+
+	slps0_dbg_latch = 0;
+
+out_unlock:
+	mutex_unlock(&pmcdev->lock);
+}
+
+static int pmc_core_slps0_dbg_show(struct seq_file *s, void
 *unused)
+{
+	struct pmc_dev *pmcdev = s ? s->private : &pmc;
+	const struct slps0_dbg_map *maps = pmcdev->map-
Quoted 1 line
slps0_dbg_maps;
drivers/platform/x86/intel_pmc_core.c +11 -0
+	int num_slps0_dbg_regs = pmcdev->map->slps0_dbg_num;
+	int i, offset;
+	u32 data;
+
+	pmc_core_slps0_dbg_latch(pmcdev, false);
+	offset = pmcdev->map->slps0_dbg_offset;
+	while (num_slps0_dbg_regs--) {
+		data = pmc_core_reg_read(pmcdev, offset);
+		offset += 4;
+		for (i = 0; i < maps->size; i++)
+			seq_printf(s, "SLP_S0_DBG: %-32s\tState:
%s\n",
drivers/platform/x86/intel_pmc_core.c +2 -0
+				   maps->slps0_dbg_sts[i].name,
+				   data & maps-
Quoted 8 lines
drivers/platform/x86/intel_pmc_core.c static const struct file_operations pmc_core_ltr_ignore_ops = { · line 579
Quoted 2 lines
Quoted 1 line
slps0_dbg_sts[i].bit_mask ?
drivers/platform/x86/intel_pmc_core.c +1 -0
+				   "Yes" : "No");
In current form, it looks pretty similar to pch_ip_power_gating_status
output. Since it helps in debug, please use Blocks_slp_s0: instead of State:

Quoted 2 lines

That may not be true. What blocks or doesn't can be configured by the OEM.

IIUC, The IPs exposed by these registers are the ones that the PMC always looks at before making a decision to assert SLP_S0#. I dont think they can be changed by OEMs in determining the S0ix flows.

Quoted 17 lines

Quoted 12 lines
drivers/platform/x86/intel_pmc_core.c static const struct file_operations pmc_core_ltr_ignore_ops = { · line 585
Quoted 7 lines
drivers/platform/x86/intel_pmc_core.c +6 -0
+		maps++;
+	}
+	pmc_core_slps0_dbg_latch(pmcdev, true);
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(pmc_core_slps0_dbg);
When the latch bit is not set, can we format the output of this attribute
stating clearly whether the current blockers are "live" or the "last captured at PC10 entry" ?
There is no "live". It's always the last captured state which is why the status doesn't update the way you expect above. With V2 the latch is automatically reset after every read.

Quoted 7 lines

When the latch isn't set, any updates would indeed be from PC10 transitions, but you can't be sure this isn't the last forced latch state unless you monitor PC10 residency and assume that the values were updated when the count changes. Even with the latch, if there's a PC10 transition that occurs after the bit is written but before the registers are read, the values may be from that transition, not the latch.

All default values are printed as "No". Here, we can rather print a message that unless latch is set or Package enters c10 (which is unlikely unless the display is off) the debugfs would not show any blockers.

In the current form this patch prints the output when we read the first time before setting the latch.

SLP_S0_DBG: AUDIO_D3 State: No
SLP_S0_DBG: OTG_D3 State: No
---- xxx ---- snip ---- xxx ----

Once the latch bit is set the SLP_S0_DBG_X registers show proper values but after explicitely setting latch as "N", the stale values are printed. The system on which i tested never enters PC10 so once the latch is unset, the values should be shown as default i.e. NO but that is not always correct as you mentioned above.

IMO, a better approch would be to set/unset the latch internally when the user does a read on SLP_S0_DEBUG_X registers rather than exposing a seperate debugfs file because a write of 0 to the latch bit has no effect but it is a requirement for the next read.

Quoted 111 lines

drivers/platform/x86/intel_pmc_core.c static int pmc_core_dbgfs_register(struct · line 622

Quoted 12 lines

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

+
 static void pmc_core_dbgfs_unregister(struct pmc_dev *pmcdev)
 {
 	debugfs_remove_recursive(pmcdev->dbgfs_dir);

@@ -514,6 +617,15 @@ static int pmc_core_dbgfs_register(struct
 pmc_dev *pmcdev)
 				    0444, dir, pmcdev,
 				    &pmc_core_mphy_pg_ops);
 
+	if (pmcdev->map->slps0_dbg_maps) {
+		debugfs_create_file("slp_s0_debug_status", 0444,

slp_s0_dbg_latch

Quoted 94 lines

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

+				    dir, pmcdev,
+				    &pmc_core_slps0_dbg_fops);
+
+		debugfs_create_bool("slp_s0_dbg_latch", 0644,
+				    dir, &slps0_dbg_latch);
+	}
+
 	return 0;
 }

#else

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

index 5fa5f97..97e4b5a 100644
@@ -124,27 +124,35 @@ enum ppfear_regs {
 #define SPT_PMC_BIT_MPHY_CMN_LANE3		BIT(3)
 
 /* Cannonlake Power Management Controller register offsets */
-#define CNP_PMC_SLP_S0_RES_COUNTER_OFFSET      0x193C
-#define CNP_PMC_LTR_IGNORE_OFFSET              0x1B0C
-#define CNP_PMC_PM_CFG_OFFSET                  0x1818
+#define CNP_PMC_SLP_S0_RES_COUNTER_OFFSET	0x193C
+#define CNP_PMC_LTR_IGNORE_OFFSET		0x1B0C
+#define CNP_PMC_PM_CFG_OFFSET			0x1818
+#define CNP_PMC_SLPS0_DBG_OFFSET		0x10B4
 /* Cannonlake: PGD PFET Enable Ack Status Register(s) start */
-#define CNP_PMC_HOST_PPFEAR0A                  0x1D90
+#define CNP_PMC_HOST_PPFEAR0A			0x1D90
 
-#define CNP_PMC_MMIO_REG_LEN                   0x2000
-#define CNP_PPFEAR_NUM_ENTRIES                 8
-#define CNP_PMC_READ_DISABLE_BIT               22
+#define CNP_PMC_MMIO_REG_LEN			0x2000
+#define CNP_PPFEAR_NUM_ENTRIES			8
+#define CNP_PMC_READ_DISABLE_BIT		22
+#define CNP_PMC_LATCH_SLPS0_EVENTS		BIT(31)
 
 struct pmc_bit_map {
 	const char *name;
 	u32 bit_mask;
 };
 
+struct slps0_dbg_map {
+	const struct pmc_bit_map *slps0_dbg_sts;
+	int size;
+};
+
 /**
  * struct pmc_reg_map - Structure used to define parameter unique

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

@@ -153,6 +161,8 @@ struct pmc_bit_map {
  *			PPFEAR
  * @pm_cfg_offset:	PWRMBASE offset to PM_CFG register
  * @pm_read_disable_bit: Bit index to read PMC_READ_DISABLE
+ * @slps0_dbg_offset:	PWRMBASE offset to SLP_S0_DEBUG_REG*
+ * @slps0_dbg_num:	Number of SLP_S0_DEBUG_REG registers
  *
  * Each PCH has unique set of register offsets and bit indexes.

This structure
* captures them to have a common implementation.

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

@@ -161,6 +171,7 @@ 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 struct slps0_dbg_map *slps0_dbg_maps;
 	const u32 slp_s0_offset;
 	const u32 ltr_ignore_offset;
 	const int regmap_length;

@@ -168,6 +179,8 @@ struct pmc_reg_map {
 	const int ppfear_buckets;
 	const u32 pm_cfg_offset;
 	const int pm_read_disable_bit;
+	const u32 slps0_dbg_offset;
+	const int slps0_dbg_num;
 };
 
 /**

Signature · 1 line

2.7.4

Signature · 2 lines

Best Regards,
Rajneesh

David E. Box

author

2018-06-02 01:47 UTC #5 lore raw

Quoted 222 lines · Rajneesh Bhardwaj

On Wed, May 30, 2018 at 03:53:12AM -0700, David E. Box wrote:
Hi Dave,
Quoted 214 lines
Hi Rajneesh,
Quoted 28 lines · Rajneesh Bhardwaj
On Thu, May 24, 2018 at 06:10:56PM -0700, David E. Box wrote:
Thanks for sending this, Dave. Few comments below.
Quoted 3 lines
Adds debugfs access to registers in the Cannon Point PCH PMC that
are
Please use Cannonlake PCH.
Quoted 3 lines
useful for debugging #SLP_S0 signal assertion and other low power
related
assertion failures and other low power debug.
Quoted 11 lines
activities. Device pm states are latched in these registers whenever the
package enters C10 and can be read from slp_s0_debug_status. The pm
states may also be latched by writing 1 to slp_s0_dbg_latch which
will
immediately capture the current state on the next read of slp_s0_debug_status. Also while in intel_pmc_core.h clean up spacing.
Quoted 10 lines
Initially, unless the latch bit is not set the debugfs file does not
show
any information as expected but once you write Y to latch file, the
debugfs
file continues to show blockers even though you write N back there or
unload
- reload the driver. Please fix this.
See comments below
Quoted 37 lines
Quoted 37 lines
Signed-off-by David E. Box <[email protected]>
---
V2:
Per Andy Shevchenko:
- Clear latch bit after use
- Pass pmc_dev as parameter
- Use DEFINE_SHOW_ATTRIBUTE macro
 drivers/platform/x86/intel_pmc_core.c | 112
++++++++++++++++++++++++++++++++++
 drivers/platform/x86/intel_pmc_core.h |  27 +++++---
 2 files changed, 132 insertions(+), 7 deletions(-)
drivers/platform/x86/intel_pmc_core.c +12 -0
index 43bbe74..ed40999 100644
@@ -196,9 +196,64 @@ static const struct pmc_bit_map
 cnp_pfear_map[] = {
 	{}
 };
 
+static const struct pmc_bit_map cnp_slps0_dbg0_map[] = {
+	{"AUDIO_D3",		BIT(0)},
+	{"OTG_D3",		BIT(1)},
+	{"XHCI_D3",		BIT(2)},
+	{"LPIO_D3",		BIT(3)},
+	{"SDX_D3",		BIT(4)},
+	{"SATA_D3",		BIT(5)},
+	{"UFS0_D3",		BIT(6)},
+	{"UFS1_D3",		BIT(7)},
+	{"EMMC_D3",		BIT(8)},
+};
+
Quoted 8 lines
drivers/platform/x86/intel_pmc_core.c static const struct pmc_bit_map cnp_pfear_map[] = { · line 215
Quoted 5 lines
drivers/platform/x86/intel_pmc_core.c +5 -0
+static const struct pmc_bit_map cnp_slps0_dbg1_map[] = {
+	{"SDIO_PLL_OFF",	BIT(0)},
+	{"USB2_PLL_OFF",	BIT(1)},
+	{"AUDIO_PLL_OFF",	BIT(2)},
+	{"OC_PLL_OFF",		BIT(3)},
Please rename to ISCLK_OC as it is the preferred nomenclature for debug.
Okay
Quoted 113 lines
drivers/platform/x86/intel_pmc_core.c static const struct pmc_bit_map cnp_pfear_map[] = { · line 216
Quoted 2 lines
drivers/platform/x86/intel_pmc_core.c +1 -0
+	{"MAIN_PLL_OFF",	BIT(4)},
Ditto, please use ISCLK_MAIN.
Quoted 108 lines
drivers/platform/x86/intel_pmc_core.c +37 -0
+	{"XOSC_OFF",		BIT(5)},
+	{"LPC_CLKS_GATED",	BIT(6)},
+	{"PCIE_CLKREQS_IDLE",	BIT(7)},
+	{"AUDIO_ROSC_OFF",	BIT(8)},
+	{"HPET_XOSC_CLK_REQ",	BIT(9)},
+	{"PMC_ROSC_SLOW_CLK",	BIT(10)},
+	{"AON2_ROSC_GATED",	BIT(11)},
+	{"CLKACKS_DEASSERTED",	BIT(12)},
+};
+
+static const struct pmc_bit_map cnp_slps0_dbg2_map[] = {
+	{"MPHY_CORE_GATED",	BIT(0)},
+	{"CSME_GATED",		BIT(1)},
+	{"USB2_SUS_GATED",	BIT(2)},
+	{"DYN_FLEX_IO_IDLE",	BIT(3)},
+	{"GBE_NO_LINK",		BIT(4)},
+	{"THERM_SEN_DISABLED",	BIT(5)},
+	{"PCIE_LOW_POWER",	BIT(6)},
+	{"ISH_VNNAON_REQ_ACT",	BIT(7)},
+	{"ISH_VNN_REQ_ACT",	BIT(8)},
+	{"CNV_VNNAON_REQ_ACT",	BIT(9)},
+	{"CNV_VNN_REQ_ACT",	BIT(10)},
+	{"NPK_VNNON_REQ_ACT",	BIT(11)},
+	{"PMSYNC_STATE_IDLE",	BIT(12)},
+	{"ALST_GT_THRES",	BIT(13)},
+	{"PMC_ARC_PG_READY",	BIT(14)},
+};
+
+static const struct slps0_dbg_map cnp_slps0_dbg_maps[] = {
+	{cnp_slps0_dbg0_map, ARRAY_SIZE(cnp_slps0_dbg0_map)},
+	{cnp_slps0_dbg1_map, ARRAY_SIZE(cnp_slps0_dbg1_map)},
+	{cnp_slps0_dbg2_map, ARRAY_SIZE(cnp_slps0_dbg2_map)},
+};
+
 static const struct pmc_reg_map cnp_reg_map = {
 	.pfear_sts = cnp_pfear_map,
 	.slp_s0_offset = CNP_PMC_SLP_S0_RES_COUNTER_OFFSET,
+	.slps0_dbg_maps = cnp_slps0_dbg_maps,
+	.slps0_dbg_offset = CNP_PMC_SLPS0_DBG_OFFSET,
+	.slps0_dbg_num = ARRAY_SIZE(cnp_slps0_dbg_maps),
 	.ltr_ignore_offset = CNP_PMC_LTR_IGNORE_OFFSET,
 	.regmap_length = CNP_PMC_MMIO_REG_LEN,
drivers/platform/x86/intel_pmc_core.c +2 -0
 	.ppfear0_offset = CNP_PMC_HOST_PPFEAR0A,
@@ -252,6 +307,8 @@ static int
 pmc_core_check_read_lock_bit(void)
 }
 
 #if IS_ENABLED(CONFIG_DEBUG_FS)
+static bool slps0_dbg_latch;
+
 static void pmc_core_display_map(struct seq_file *s, int
 index,
u8 pf_reg, const struct
pmc_bit_map *pf_map)
{
drivers/platform/x86/intel_pmc_core.c +1 -0
@@ -481,6 +538,52 @@ static const struct file_operations
 pmc_core_ltr_ignore_ops = {
 	.release        = single_release,
 };
 
+static void pmc_core_slps0_dbg_latch(struct pmc_dev *pmcdev,
bool
reset)
drivers/platform/x86/intel_pmc_core.c +21 -0
+{
+	const struct pmc_reg_map *map = pmcdev->map;
+	u32 fd;
+
+	mutex_lock(&pmcdev->lock);
+
+	if (!reset && !slps0_dbg_latch)
+		goto out_unlock;
+
+	fd = pmc_core_reg_read(pmcdev, map->slps0_dbg_offset);
+	reset ? (fd &= ~CNP_PMC_LATCH_SLPS0_EVENTS) :
+		(fd |= CNP_PMC_LATCH_SLPS0_EVENTS);
+	pmc_core_reg_write(pmcdev, map->slps0_dbg_offset, fd);
+
+	slps0_dbg_latch = 0;
+
+out_unlock:
+	mutex_unlock(&pmcdev->lock);
+}
+
+static int pmc_core_slps0_dbg_show(struct seq_file *s, void
*unused)
drivers/platform/x86/intel_pmc_core.c +3 -0
+{
+	struct pmc_dev *pmcdev = s ? s->private : &pmc;
+	const struct slps0_dbg_map *maps = pmcdev->map-
Quoted 1 line
slps0_dbg_maps;
drivers/platform/x86/intel_pmc_core.c +11 -0
+	int num_slps0_dbg_regs = pmcdev->map->slps0_dbg_num;
+	int i, offset;
+	u32 data;
+
+	pmc_core_slps0_dbg_latch(pmcdev, false);
+	offset = pmcdev->map->slps0_dbg_offset;
+	while (num_slps0_dbg_regs--) {
+		data = pmc_core_reg_read(pmcdev, offset);
+		offset += 4;
+		for (i = 0; i < maps->size; i++)
+			seq_printf(s, "SLP_S0_DBG: %-
32s\tState:
%s\n",
drivers/platform/x86/intel_pmc_core.c +1 -0
+				   maps-
Quoted 1 line
slps0_dbg_sts[i].name,
drivers/platform/x86/intel_pmc_core.c +1 -0
+				   data & maps-
Quoted 1 line
slps0_dbg_sts[i].bit_mask ?
Quoted 8 lines
drivers/platform/x86/intel_pmc_core.c static const struct file_operations pmc_core_ltr_ignore_ops = { · line 579
Quoted 1 line
drivers/platform/x86/intel_pmc_core.c +1 -0
+				   "Yes" : "No");
In current form, it looks pretty similar to pch_ip_power_gating_status
output. Since it helps in debug, please use Blocks_slp_s0:
instead of
State:
Quoted 3 lines
That may not be true. What blocks or doesn't can be configured by the
OEM.

Quoted 5 lines

IIUC, The IPs exposed by these registers are the ones that the PMC always looks at
before making a decision to assert SLP_S0#. I dont think they can be changed
by OEMs in determining the S0ix flows.

I confirmed with the architect that the logic operates independently of other BIOS settings OEMs can make to ignore these same IPs. Reporting an IP as blocking on a system where the IP is being ignored would be confusing. The reported state the IP is in remains correct and that is what this shows.

Quoted 58 lines

Quoted 21 lines
Quoted 14 lines
drivers/platform/x86/intel_pmc_core.c static const struct file_operations pmc_core_ltr_ignore_ops = { · line 585
Quoted 8 lines
drivers/platform/x86/intel_pmc_core.c +6 -0
+		maps++;
+	}
+	pmc_core_slps0_dbg_latch(pmcdev, true);
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(pmc_core_slps0_dbg);
When the latch bit is not set, can we format the output of this attribute
stating clearly whether the current blockers are "live" or the "last
captured at PC10 entry" ?
There is no "live". It's always the last captured state which is why
the status doesn't update the way you expect above. With V2 the latch
is automatically reset after every read.
Quoted 9 lines
When the latch isn't set, any updates would indeed be from PC10 transitions, but you can't be sure this isn't the last forced latch state unless you monitor PC10 residency and assume that the values were
updated when the count changes. Even with the latch, if there's a PC10
transition that occurs after the bit is written but before the registers are read, the values may be from that transition, not the latch.
All default values are printed as "No". Here, we can rather print a message
that unless latch is set or Package enters c10 (which is unlikely unless
the display is off) the debugfs would not show any blockers.
In the current form this patch prints the output when we read the first time
before setting the latch.
SLP_S0_DBG: AUDIO_D3 State: No
SLP_S0_DBG: OTG_D3 State: No
---- xxx ---- snip ---- xxx ----
Once the latch bit is set the SLP_S0_DBG_X registers show proper values but
after explicitely setting latch as "N", the stale values are printed.
The
system on which i tested never enters PC10 so once the latch is unset, the
values should be shown as default i.e. NO but that is not always correct as
you mentioned above.

Quoted 7 lines

IMO, a better approch would be to set/unset the latch internally when the
user does a read on SLP_S0_DEBUG_X registers rather than exposing a seperate
debugfs file because a write of 0 to the latch bit has no effect but it is a
requirement for the next read.

If the latch file is used, the code will already reset it automatically by writing 0 after each read. Writing "N" is not necessary unless you wrote "Y" and changed your mind. The reset does not set the values to all "No". The registers, once updated the first time, always hold the last saved state.

If the latch file is not used the registers will only update whenever the system enters PC10. This works on my system that does enter PC10.

What you suggest would limit the read to forcibly latched values only. The greatest value of this file IMHO is in being able to see system state after it enters PC10, a state you cannot guarantee the system will be in if you only forcibly latch the state with the latch file.

David

Andy Shevchenko

2018-05-31 18:38 UTC #6 lore raw

In reply to David E. Box

Quoted 8 lines · David E. Box

Adds debugfs access to registers in the Cannon Point PCH PMC that are useful for debugging #SLP_S0 signal assertion and other low power related activities. Device pm states are latched in these registers whenever the package enters C10 and can be read from slp_s0_debug_status. The pm states may also be latched by writing 1 to slp_s0_dbg_latch which will immediately capture the current state on the next read of slp_s0_debug_status. Also while in intel_pmc_core.h clean up spacing.

Thanks for an update. My comments below.

As far as I understand there is still ongoing discussion about the approach when and how to show data. So I'll wait a bit for a settlement between you, guys.

Quoted 4 lines

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

+static void pmc_core_slps0_dbg_latch(struct pmc_dev *pmcdev, bool reset)
+{
+       const struct pmc_reg_map *map = pmcdev->map;
+       u32 fd;

drivers/platform/x86/intel_pmc_core.c static const struct file_operations pmc_core_ltr_ignore_ops = { · line 553

Quoted 10 lines

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

+
+       mutex_lock(&pmcdev->lock);
+
+       if (!reset && !slps0_dbg_latch)
+               goto out_unlock;
+
+       fd = pmc_core_reg_read(pmcdev, map->slps0_dbg_offset);
 
+       reset ? (fd &= ~CNP_PMC_LATCH_SLPS0_EVENTS) :
+               (fd |= CNP_PMC_LATCH_SLPS0_EVENTS);

I would rather use classical pattern here, i.e.

if (reset)
fd ...
else
fd ...

drivers/platform/x86/intel_pmc_core.c static const struct file_operations pmc_core_ltr_ignore_ops = { · line 564

Quoted 9 lines

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

+       pmc_core_reg_write(pmcdev, map->slps0_dbg_offset, fd);
+
+       slps0_dbg_latch = 0;
+
+out_unlock:
+       mutex_unlock(&pmcdev->lock);
+}
 
+       struct pmc_dev *pmcdev = s ? s->private : &pmc;

I'm not sure why do we need such condition.

Simple

... pmcdev = s->private;

is enough.

drivers/platform/x86/intel_pmc_core.h enum ppfear_regs { · line 129

Quoted 7 lines

/* Cannonlake Power Management Controller register offsets */

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

-#define CNP_PMC_SLP_S0_RES_COUNTER_OFFSET      0x193C
-#define CNP_PMC_LTR_IGNORE_OFFSET              0x1B0C
-#define CNP_PMC_PM_CFG_OFFSET                  0x1818
+#define CNP_PMC_SLP_S0_RES_COUNTER_OFFSET      0x193C
+#define CNP_PMC_LTR_IGNORE_OFFSET              0x1B0C
+#define CNP_PMC_PM_CFG_OFFSET                  0x1818

I have hard time to understand what is the difference here.
Either do another clean up patch (white spaces vs. tabs?) or leave it untouched.

drivers/platform/x86/intel_pmc_core.h enum ppfear_regs { · line 136

Quoted 10 lines

/* Cannonlake: PGD PFET Enable Ack Status Register(s) start */

drivers/platform/x86/intel_pmc_core.h +4 -4

-#define CNP_PMC_HOST_PPFEAR0A                  0x1D90
+#define CNP_PMC_HOST_PPFEAR0A                  0x1D90
 
-#define CNP_PMC_MMIO_REG_LEN                   0x2000
-#define CNP_PPFEAR_NUM_ENTRIES                 8
-#define CNP_PMC_READ_DISABLE_BIT               22
+#define CNP_PMC_MMIO_REG_LEN                   0x2000
+#define CNP_PPFEAR_NUM_ENTRIES                 8
+#define CNP_PMC_READ_DISABLE_BIT               22

Ditto.

drivers/platform/x86/intel_pmc_core.h enum ppfear_regs { · line 147

Quoted 4 lines

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

+struct slps0_dbg_map {
+       const struct pmc_bit_map *slps0_dbg_sts;
+       int size;
+};

Didn't pay attention to this earlier. Why do we have a separate size member? What does it define?

Signature · 2 lines

With Best Regards,
Andy Shevchenko

David E. Box

author

2018-05-31 23:04 UTC #7 lore raw

Quoted 86 lines · Andy Shevchenko

Quoted 4 lines · David E. Box
Adds debugfs access to registers in the Cannon Point PCH PMC that are
useful for debugging #SLP_S0 signal assertion and other low power related
Quoted 9 lines
activities. Device pm states are latched in these registers whenever the
package enters C10 and can be read from slp_s0_debug_status. The pm states may also be latched by writing 1 to slp_s0_dbg_latch which will
immediately capture the current state on the next read of slp_s0_debug_status. Also while in intel_pmc_core.h clean up spacing.
Thanks for an update. My comments below.
As far as I understand there is still ongoing discussion about the approach when and how to show data. So I'll wait a bit for a settlement between you, guys.
Quoted 4 lines
+static void pmc_core_slps0_dbg_latch(struct pmc_dev *pmcdev, bool reset)
drivers/platform/x86/intel_pmc_core.c +2 -0
+{
+       const struct pmc_reg_map *map = pmcdev->map;
drivers/platform/x86/intel_pmc_core.c static const struct file_operations pmc_core_ltr_ignore_ops = { · line 553
Quoted 10 lines
drivers/platform/x86/intel_pmc_core.c +10 -0
+       u32 fd;
+
+       mutex_lock(&pmcdev->lock);
+
+       if (!reset && !slps0_dbg_latch)
+               goto out_unlock;
+
+       fd = pmc_core_reg_read(pmcdev, map->slps0_dbg_offset);
+       reset ? (fd &= ~CNP_PMC_LATCH_SLPS0_EVENTS) :
+               (fd |= CNP_PMC_LATCH_SLPS0_EVENTS);
I would rather use classical pattern here, i.e.
if (reset)
fd ...
else
fd ...
drivers/platform/x86/intel_pmc_core.c static const struct file_operations pmc_core_ltr_ignore_ops = { · line 564
Quoted 8 lines
drivers/platform/x86/intel_pmc_core.c +8 -0
+       pmc_core_reg_write(pmcdev, map->slps0_dbg_offset, fd);
+
+       slps0_dbg_latch = 0;
+
+out_unlock:
+       mutex_unlock(&pmcdev->lock);
+}
+       struct pmc_dev *pmcdev = s ? s->private : &pmc;
I'm not sure why do we need such condition.
Simple
... pmcdev = s->private;
is enough.
drivers/platform/x86/intel_pmc_core.h enum ppfear_regs { · line 129
Quoted 7 lines
/* Cannonlake Power Management Controller register offsets */
drivers/platform/x86/intel_pmc_core.h +3 -3
-#define CNP_PMC_SLP_S0_RES_COUNTER_OFFSET      0x193C
-#define CNP_PMC_LTR_IGNORE_OFFSET              0x1B0C
-#define CNP_PMC_PM_CFG_OFFSET                  0x1818
+#define CNP_PMC_SLP_S0_RES_COUNTER_OFFSET      0x193C
+#define CNP_PMC_LTR_IGNORE_OFFSET              0x1B0C
+#define CNP_PMC_PM_CFG_OFFSET                  0x1818
I have hard time to understand what is the difference here. Either do another clean up patch (white spaces vs. tabs?) or leave it untouched.
drivers/platform/x86/intel_pmc_core.h enum ppfear_regs { · line 136
Quoted 10 lines
/* Cannonlake: PGD PFET Enable Ack Status Register(s) start */
drivers/platform/x86/intel_pmc_core.h +4 -4
-#define CNP_PMC_HOST_PPFEAR0A                  0x1D90
+#define CNP_PMC_HOST_PPFEAR0A                  0x1D90
 
-#define CNP_PMC_MMIO_REG_LEN                   0x2000
-#define CNP_PPFEAR_NUM_ENTRIES                 8
-#define CNP_PMC_READ_DISABLE_BIT               22
+#define CNP_PMC_MMIO_REG_LEN                   0x2000
+#define CNP_PPFEAR_NUM_ENTRIES                 8
+#define CNP_PMC_READ_DISABLE_BIT               22
Ditto.

Quoted 7 lines

drivers/platform/x86/intel_pmc_core.h enum ppfear_regs { · line 147
Quoted 4 lines
drivers/platform/x86/intel_pmc_core.h +4 -0
+struct slps0_dbg_map {
+       const struct pmc_bit_map *slps0_dbg_sts;
+       int size;
+};
Didn't pay attention to this earlier. Why do we have a separate size member? What does it define?

It holds the size of the pmc_bit_map array, assigned as shown here:

+static const struct slps0_dbg_map cnp_slps0_dbg_maps[] = {
+ {cnp_slps0_dbg0_map, ARRAY_SIZE(cnp_slps0_dbg0_map)},
+ {cnp_slps0_dbg1_map, ARRAY_SIZE(cnp_slps0_dbg1_map)},
+ {cnp_slps0_dbg2_map, ARRAY_SIZE(cnp_slps0_dbg2_map)}, +};

Okay on all other comments

Dave

Andy Shevchenko

2018-06-01 08:25 UTC #8 lore raw

Quoted 14 lines · David E. Box

Quoted 10 lines · Andy Shevchenko
drivers/platform/x86/intel_pmc_core.h enum ppfear_regs { · line 147
Quoted 4 lines · David E. Box
drivers/platform/x86/intel_pmc_core.h +4 -0
+struct slps0_dbg_map {
+       const struct pmc_bit_map *slps0_dbg_sts;
+       int size;
+};
Didn't pay attention to this earlier. Why do we have a separate size member? What does it define?
It holds the size of the pmc_bit_map array, assigned as shown here:

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

Quoted 5 lines

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

+static const struct slps0_dbg_map cnp_slps0_dbg_maps[] = {
+ {cnp_slps0_dbg0_map, ARRAY_SIZE(cnp_slps0_dbg0_map)},
+ {cnp_slps0_dbg1_map, ARRAY_SIZE(cnp_slps0_dbg1_map)},
+ {cnp_slps0_dbg2_map, ARRAY_SIZE(cnp_slps0_dbg2_map)},
+};

I see.
Please drop this and use a terminator instead like it's done for the rest map tables.

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Box, David E

2018-06-09 00:02 UTC #9 lore raw

PATCH v3 1/1 platform/x86: intel_pmc_core: Add CNP SLPS0 debug registers

Adds debugfs access to registers in the Cannon Point PCH PMC that are useful for debugging #SLP_S0 signal assertion and other low power relate activities. Device pm states are latched in these registers whenever the package enters C10 and can be read from slp_s0_debug_status. The pm states may also be latched by writing 1 to slp_s0_dbg_latch which will immediately capture the current state on the next read of slp_s0_debug_status.

Signed-off-by Box, David E <[email protected]>

---
V3:
- use null terminator in bit_map array
- replaced ternary operator with if/else
- Removed space fixes on old code
V2:
- Clear latch bit after use
- Pass pmc_dev as parameter
- Use DEFINE_SHOW_ATTRIBUTE macro

 drivers/platform/x86/intel_pmc_core.c | 120 ++++++++++++++++++++++++++++++++++
 drivers/platform/x86/intel_pmc_core.h |   6 ++
 2 files changed, 126 insertions(+)

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

index 43bbe74743d9..2d272a3e0176 100644
@@ -196,9 +196,67 @@ static const struct pmc_bit_map cnp_pfear_map[] = {
 	{}
 };
 
+static const struct pmc_bit_map cnp_slps0_dbg0_map[] = {
+	{"AUDIO_D3",		BIT(0)},
+	{"OTG_D3",		BIT(1)},
+	{"XHCI_D3",		BIT(2)},
+	{"LPIO_D3",		BIT(3)},
+	{"SDX_D3",		BIT(4)},
+	{"SATA_D3",		BIT(5)},
+	{"UFS0_D3",		BIT(6)},
+	{"UFS1_D3",		BIT(7)},
+	{"EMMC_D3",		BIT(8)},
+	{}
+};
+
+static const struct pmc_bit_map cnp_slps0_dbg1_map[] = {
+	{"SDIO_PLL_OFF",	BIT(0)},
+	{"USB2_PLL_OFF",	BIT(1)},
+	{"AUDIO_PLL_OFF",	BIT(2)},
+	{"OC_PLL_OFF",		BIT(3)},
+	{"MAIN_PLL_OFF",	BIT(4)},
+	{"XOSC_OFF",		BIT(5)},
+	{"LPC_CLKS_GATED",	BIT(6)},
+	{"PCIE_CLKREQS_IDLE",	BIT(7)},
+	{"AUDIO_ROSC_OFF",	BIT(8)},
+	{"HPET_XOSC_CLK_REQ",	BIT(9)},
+	{"PMC_ROSC_SLOW_CLK",	BIT(10)},
+	{"AON2_ROSC_GATED",	BIT(11)},
+	{"CLKACKS_DEASSERTED",	BIT(12)},
+	{}
+};
+
+static const struct pmc_bit_map cnp_slps0_dbg2_map[] = {
+	{"MPHY_CORE_GATED",	BIT(0)},
+	{"CSME_GATED",		BIT(1)},
+	{"USB2_SUS_GATED",	BIT(2)},
+	{"DYN_FLEX_IO_IDLE",	BIT(3)},
+	{"GBE_NO_LINK",		BIT(4)},
+	{"THERM_SEN_DISABLED",	BIT(5)},
+	{"PCIE_LOW_POWER",	BIT(6)},
+	{"ISH_VNNAON_REQ_ACT",	BIT(7)},
+	{"ISH_VNN_REQ_ACT",	BIT(8)},
+	{"CNV_VNNAON_REQ_ACT",	BIT(9)},
+	{"CNV_VNN_REQ_ACT",	BIT(10)},
+	{"NPK_VNNON_REQ_ACT",	BIT(11)},
+	{"PMSYNC_STATE_IDLE",	BIT(12)},
+	{"ALST_GT_THRES",	BIT(13)},
+	{"PMC_ARC_PG_READY",	BIT(14)},
+	{}
+};
+
+static const struct pmc_bit_map *cnp_slps0_dbg_maps[] = {
+	cnp_slps0_dbg0_map,
+	cnp_slps0_dbg1_map,
+	cnp_slps0_dbg2_map,
+	NULL,
+};
+
 static const struct pmc_reg_map cnp_reg_map = {
 	.pfear_sts = cnp_pfear_map,
 	.slp_s0_offset = CNP_PMC_SLP_S0_RES_COUNTER_OFFSET,
+	.slps0_dbg_maps = cnp_slps0_dbg_maps,
+	.slps0_dbg_offset = CNP_PMC_SLPS0_DBG_OFFSET,
 	.ltr_ignore_offset = CNP_PMC_LTR_IGNORE_OFFSET,
 	.regmap_length = CNP_PMC_MMIO_REG_LEN,
 	.ppfear0_offset = CNP_PMC_HOST_PPFEAR0A,

@@ -252,6 +310,8 @@ static int pmc_core_check_read_lock_bit(void)
 }
 
 #if IS_ENABLED(CONFIG_DEBUG_FS)
+static bool slps0_dbg_latch;
+
 static void pmc_core_display_map(struct seq_file *s, int index,
 				 u8 pf_reg, const struct pmc_bit_map *pf_map)
 {

@@ -481,6 +541,57 @@ static const struct file_operations pmc_core_ltr_ignore_ops = {
 	.release        = single_release,
 };
 
+static void pmc_core_slps0_dbg_latch(struct pmc_dev *pmcdev, bool reset)
+{
+	const struct pmc_reg_map *map = pmcdev->map;
+	u32 fd;
+
+	mutex_lock(&pmcdev->lock);
+
+	if (!reset && !slps0_dbg_latch)
+		goto out_unlock;
+
+	fd = pmc_core_reg_read(pmcdev, map->slps0_dbg_offset);
+	if (reset)
+		fd &= ~CNP_PMC_LATCH_SLPS0_EVENTS;
+	else
+		fd |= CNP_PMC_LATCH_SLPS0_EVENTS;
+	pmc_core_reg_write(pmcdev, map->slps0_dbg_offset, fd);
+
+	slps0_dbg_latch = 0;
+
+out_unlock:
+	mutex_unlock(&pmcdev->lock);
+}
+
+static int pmc_core_slps0_dbg_show(struct seq_file *s, void *unused)
+{
+	struct pmc_dev *pmcdev = s->private;
+	const struct pmc_bit_map **maps = pmcdev->map->slps0_dbg_maps;
+	const struct pmc_bit_map *map;
+	int offset;
+	u32 data;
+
+	pmc_core_slps0_dbg_latch(pmcdev, false);
+	offset = pmcdev->map->slps0_dbg_offset;
+	while (*maps) {
+		map = *maps;
+		data = pmc_core_reg_read(pmcdev, offset);
+		offset += 4;
+		while (map->name) {
+			seq_printf(s, "SLP_S0_DBG: %-32s\tState: %s\n",
+				   map->name,
+				   data & map->bit_mask ?
+				   "Yes" : "No");
+			++map;
+		}
+		++maps;
+	}
+	pmc_core_slps0_dbg_latch(pmcdev, true);
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(pmc_core_slps0_dbg);
+
 static void pmc_core_dbgfs_unregister(struct pmc_dev *pmcdev)
 {
 	debugfs_remove_recursive(pmcdev->dbgfs_dir);

@@ -514,6 +625,15 @@ static int pmc_core_dbgfs_register(struct pmc_dev *pmcdev)
 				    0444, dir, pmcdev,
 				    &pmc_core_mphy_pg_ops);
 
+	if (pmcdev->map->slps0_dbg_maps) {
+		debugfs_create_file("slp_s0_debug_status", 0444,
+				    dir, pmcdev,
+				    &pmc_core_slps0_dbg_fops);
+
+		debugfs_create_bool("slp_s0_dbg_latch", 0644,
+				    dir, &slps0_dbg_latch);
+	}
+
 	return 0;
 }
 #else

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

index 5fa5f97870aa..93a7e99e1f8b 100644
@@ -127,12 +127,14 @@ enum ppfear_regs {
 #define CNP_PMC_SLP_S0_RES_COUNTER_OFFSET      0x193C
 #define CNP_PMC_LTR_IGNORE_OFFSET              0x1B0C
 #define CNP_PMC_PM_CFG_OFFSET                  0x1818
+#define CNP_PMC_SLPS0_DBG_OFFSET		0x10B4
 /* Cannonlake: PGD PFET Enable Ack Status Register(s) start */
 #define CNP_PMC_HOST_PPFEAR0A                  0x1D90
 
 #define CNP_PMC_MMIO_REG_LEN                   0x2000
 #define CNP_PPFEAR_NUM_ENTRIES                 8
 #define CNP_PMC_READ_DISABLE_BIT               22
+#define CNP_PMC_LATCH_SLPS0_EVENTS		BIT(31)
 
 struct pmc_bit_map {
 	const char *name;

@@ -145,6 +147,7 @@ struct pmc_bit_map {
  * @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
+ * @slps0_dbg_maps:	Array of SLP_S0_DBG* registers containing debug info
  * @slp_s0_offset:	PWRMBASE offset to read SLP_S0 residency
  * @ltr_ignore_offset:	PWRMBASE offset to read/write LTR ignore bit
  * @regmap_length:	Length of memory to map from PWRMBASE address to access

@@ -153,6 +156,7 @@ struct pmc_bit_map {
  *			PPFEAR
  * @pm_cfg_offset:	PWRMBASE offset to PM_CFG register
  * @pm_read_disable_bit: Bit index to read PMC_READ_DISABLE
+ * @slps0_dbg_offset:	PWRMBASE offset to SLP_S0_DEBUG_REG*
  *
  * Each PCH has unique set of register offsets and bit indexes. This structure
  * captures them to have a common implementation.

@@ -161,6 +165,7 @@ 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 struct pmc_bit_map **slps0_dbg_maps;
 	const u32 slp_s0_offset;
 	const u32 ltr_ignore_offset;
 	const int regmap_length;

@@ -168,6 +173,7 @@ struct pmc_reg_map {
 	const int ppfear_buckets;
 	const u32 pm_cfg_offset;
 	const int pm_read_disable_bit;
+	const u32 slps0_dbg_offset;
 };
 
 /**

Signature · 1 line

2.14.4

Rajneesh Bhardwaj

2018-06-13 14:07 UTC #10 lore raw

On Fri, Jun 08, 2018 at 05:02:37PM -0700, Box, David E wrote:

I am ok with the design and approach and also verified it on a Cannonlake system. I wont insist for a V4 unless Andy feels a need for respin but there are minor things that were missed.

Quoted 1 line

Adds debugfs access to registers in the Cannon Point PCH PMC that are

Cannonlake PCH

Quoted 1 line

useful for debugging #SLP_S0 signal assertion and other low power relate

Typo, related.

Quoted 5 lines

activities. Device pm states are latched in these registers whenever the package enters C10 and can be read from slp_s0_debug_status. The pm states may also be latched by writing 1 to slp_s0_dbg_latch which will immediately capture the current state on the next read of slp_s0_debug_status.

I had requested for consistency w.r.t. slp_s0_debug vs slp_s0_dbg but its fine.

Also ISCLK_OC / ISCLK_MAIN suggestion is missing.

Quoted 225 lines

Signed-off-by Box, David E <[email protected]>

 drivers/platform/x86/intel_pmc_core.c | 120 ++++++++++++++++++++++++++++++++++
 drivers/platform/x86/intel_pmc_core.h |   6 ++
 2 files changed, 126 insertions(+)

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

index 43bbe74743d9..2d272a3e0176 100644
@@ -196,9 +196,67 @@ static const struct pmc_bit_map cnp_pfear_map[] = {
 	{}
 };
 
+static const struct pmc_bit_map cnp_slps0_dbg0_map[] = {
+	{"AUDIO_D3",		BIT(0)},
+	{"OTG_D3",		BIT(1)},
+	{"XHCI_D3",		BIT(2)},
+	{"LPIO_D3",		BIT(3)},
+	{"SDX_D3",		BIT(4)},
+	{"SATA_D3",		BIT(5)},
+	{"UFS0_D3",		BIT(6)},
+	{"UFS1_D3",		BIT(7)},
+	{"EMMC_D3",		BIT(8)},
+	{}
+};
+
+static const struct pmc_bit_map cnp_slps0_dbg1_map[] = {
+	{"SDIO_PLL_OFF",	BIT(0)},
+	{"USB2_PLL_OFF",	BIT(1)},
+	{"AUDIO_PLL_OFF",	BIT(2)},
+	{"OC_PLL_OFF",		BIT(3)},
+	{"MAIN_PLL_OFF",	BIT(4)},
+	{"XOSC_OFF",		BIT(5)},
+	{"LPC_CLKS_GATED",	BIT(6)},
+	{"PCIE_CLKREQS_IDLE",	BIT(7)},
+	{"AUDIO_ROSC_OFF",	BIT(8)},
+	{"HPET_XOSC_CLK_REQ",	BIT(9)},
+	{"PMC_ROSC_SLOW_CLK",	BIT(10)},
+	{"AON2_ROSC_GATED",	BIT(11)},
+	{"CLKACKS_DEASSERTED",	BIT(12)},
+	{}
+};
+
+static const struct pmc_bit_map cnp_slps0_dbg2_map[] = {
+	{"MPHY_CORE_GATED",	BIT(0)},
+	{"CSME_GATED",		BIT(1)},
+	{"USB2_SUS_GATED",	BIT(2)},
+	{"DYN_FLEX_IO_IDLE",	BIT(3)},
+	{"GBE_NO_LINK",		BIT(4)},
+	{"THERM_SEN_DISABLED",	BIT(5)},
+	{"PCIE_LOW_POWER",	BIT(6)},
+	{"ISH_VNNAON_REQ_ACT",	BIT(7)},
+	{"ISH_VNN_REQ_ACT",	BIT(8)},
+	{"CNV_VNNAON_REQ_ACT",	BIT(9)},
+	{"CNV_VNN_REQ_ACT",	BIT(10)},
+	{"NPK_VNNON_REQ_ACT",	BIT(11)},
+	{"PMSYNC_STATE_IDLE",	BIT(12)},
+	{"ALST_GT_THRES",	BIT(13)},
+	{"PMC_ARC_PG_READY",	BIT(14)},
+	{}
+};
+
+static const struct pmc_bit_map *cnp_slps0_dbg_maps[] = {
+	cnp_slps0_dbg0_map,
+	cnp_slps0_dbg1_map,
+	cnp_slps0_dbg2_map,
+	NULL,
+};
+
 static const struct pmc_reg_map cnp_reg_map = {
 	.pfear_sts = cnp_pfear_map,
 	.slp_s0_offset = CNP_PMC_SLP_S0_RES_COUNTER_OFFSET,
+	.slps0_dbg_maps = cnp_slps0_dbg_maps,
+	.slps0_dbg_offset = CNP_PMC_SLPS0_DBG_OFFSET,
 	.ltr_ignore_offset = CNP_PMC_LTR_IGNORE_OFFSET,
 	.regmap_length = CNP_PMC_MMIO_REG_LEN,
 	.ppfear0_offset = CNP_PMC_HOST_PPFEAR0A,

@@ -252,6 +310,8 @@ static int pmc_core_check_read_lock_bit(void)
 }
 
 #if IS_ENABLED(CONFIG_DEBUG_FS)
+static bool slps0_dbg_latch;
+
 static void pmc_core_display_map(struct seq_file *s, int index,
 				 u8 pf_reg, const struct pmc_bit_map *pf_map)
 {

@@ -481,6 +541,57 @@ static const struct file_operations pmc_core_ltr_ignore_ops = {
 	.release        = single_release,
 };
 
+static void pmc_core_slps0_dbg_latch(struct pmc_dev *pmcdev, bool reset)
+{
+	const struct pmc_reg_map *map = pmcdev->map;
+	u32 fd;
+
+	mutex_lock(&pmcdev->lock);
+
+	if (!reset && !slps0_dbg_latch)
+		goto out_unlock;
+
+	fd = pmc_core_reg_read(pmcdev, map->slps0_dbg_offset);
+	if (reset)
+		fd &= ~CNP_PMC_LATCH_SLPS0_EVENTS;
+	else
+		fd |= CNP_PMC_LATCH_SLPS0_EVENTS;
+	pmc_core_reg_write(pmcdev, map->slps0_dbg_offset, fd);
+
+	slps0_dbg_latch = 0;
+
+out_unlock:
+	mutex_unlock(&pmcdev->lock);
+}
+
+static int pmc_core_slps0_dbg_show(struct seq_file *s, void *unused)
+{
+	struct pmc_dev *pmcdev = s->private;
+	const struct pmc_bit_map **maps = pmcdev->map->slps0_dbg_maps;
+	const struct pmc_bit_map *map;
+	int offset;
+	u32 data;
+
+	pmc_core_slps0_dbg_latch(pmcdev, false);
+	offset = pmcdev->map->slps0_dbg_offset;
+	while (*maps) {
+		map = *maps;
+		data = pmc_core_reg_read(pmcdev, offset);
+		offset += 4;
+		while (map->name) {
+			seq_printf(s, "SLP_S0_DBG: %-32s\tState: %s\n",
+				   map->name,
+				   data & map->bit_mask ?
+				   "Yes" : "No");
+			++map;
+		}
+		++maps;
+	}
+	pmc_core_slps0_dbg_latch(pmcdev, true);
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(pmc_core_slps0_dbg);
+
 static void pmc_core_dbgfs_unregister(struct pmc_dev *pmcdev)
 {
 	debugfs_remove_recursive(pmcdev->dbgfs_dir);

@@ -514,6 +625,15 @@ static int pmc_core_dbgfs_register(struct pmc_dev *pmcdev)
 				    0444, dir, pmcdev,
 				    &pmc_core_mphy_pg_ops);
 
+	if (pmcdev->map->slps0_dbg_maps) {
+		debugfs_create_file("slp_s0_debug_status", 0444,
+				    dir, pmcdev,
+				    &pmc_core_slps0_dbg_fops);
+
+		debugfs_create_bool("slp_s0_dbg_latch", 0644,
+				    dir, &slps0_dbg_latch);
+	}
+
 	return 0;
 }
 #else

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

index 5fa5f97870aa..93a7e99e1f8b 100644
@@ -127,12 +127,14 @@ enum ppfear_regs {
 #define CNP_PMC_SLP_S0_RES_COUNTER_OFFSET      0x193C
 #define CNP_PMC_LTR_IGNORE_OFFSET              0x1B0C
 #define CNP_PMC_PM_CFG_OFFSET                  0x1818
+#define CNP_PMC_SLPS0_DBG_OFFSET		0x10B4
 /* Cannonlake: PGD PFET Enable Ack Status Register(s) start */
 #define CNP_PMC_HOST_PPFEAR0A                  0x1D90
 
 #define CNP_PMC_MMIO_REG_LEN                   0x2000
 #define CNP_PPFEAR_NUM_ENTRIES                 8
 #define CNP_PMC_READ_DISABLE_BIT               22
+#define CNP_PMC_LATCH_SLPS0_EVENTS		BIT(31)
 
 struct pmc_bit_map {
 	const char *name;

@@ -145,6 +147,7 @@ struct pmc_bit_map {
  * @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
+ * @slps0_dbg_maps:	Array of SLP_S0_DBG* registers containing debug info
  * @slp_s0_offset:	PWRMBASE offset to read SLP_S0 residency
  * @ltr_ignore_offset:	PWRMBASE offset to read/write LTR ignore bit
  * @regmap_length:	Length of memory to map from PWRMBASE address to access

@@ -153,6 +156,7 @@ struct pmc_bit_map {
  *			PPFEAR
  * @pm_cfg_offset:	PWRMBASE offset to PM_CFG register
  * @pm_read_disable_bit: Bit index to read PMC_READ_DISABLE
+ * @slps0_dbg_offset:	PWRMBASE offset to SLP_S0_DEBUG_REG*
  *
  * Each PCH has unique set of register offsets and bit indexes. This structure
  * captures them to have a common implementation.

@@ -161,6 +165,7 @@ 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 struct pmc_bit_map **slps0_dbg_maps;
 	const u32 slp_s0_offset;
 	const u32 ltr_ignore_offset;
 	const int regmap_length;

@@ -168,6 +173,7 @@ struct pmc_reg_map {
 	const int ppfear_buckets;
 	const u32 pm_cfg_offset;
 	const int pm_read_disable_bit;
+	const u32 slps0_dbg_offset;
 };
 
 /**

Signature · 1 line

2.14.4

Signature · 2 lines

Best Regards,
Rajneesh

Andy Shevchenko

2018-06-13 16:40 UTC #11 lore raw

Quoted 7 lines · Rajneesh Bhardwaj

On Fri, Jun 08, 2018 at 05:02:37PM -0700, Box, David E wrote:
I am ok with the design and approach and also verified it on a Cannonlake system. I wont insist for a V4 unless Andy feels a need for respin but there are minor things that were missed.
Also ISCLK_OC / ISCLK_MAIN suggestion is missing.

I missed this in discussion, so, please send v4 with settled fixes.

Meanwhile I keep v3 in my review and testing queue, thanks!

Signature · 2 lines

With Best Regards,
Andy Shevchenko

David E. Box

author

2018-06-14 22:13 UTC #12 lore raw

PATCH v4 1/1 platform/x86: intel_pmc_core: Add CNP SLPS0 debug registers

In reply to Andy Shevchenko

From: <[email protected]>

Adds debugfs access to registers in the Cannonlake PCH PMC that are useful for debugging #SLP_S0 signal assertion and other low power related activities. Device pm states are latched in these registers whenever the package enters C10 and can be read from slp_s0_debug_status. The pm states may also be latched by writing 1 to slp_s0_debug_latch which will immediately capture the current state on the next read of slp_s0_debug_status.

Signed-off-by Box, David E <[email protected]>

---
V4:
- rename slp_s0_dbg string to slp_s0_debug for consistency
- ADD ISCLK prefix to MAIN_PLL and OC_PLL
V3:
- use null terminator in bit_map array
- replaced ternary operator with if/else
- Removed space fixes on old code
V2:
- Clear latch bit after use
- Pass pmc_dev as parameter
- Use DEFINE_SHOW_ATTRIBUTE macro

 drivers/platform/x86/intel_pmc_core.c | 120 ++++++++++++++++++++++++++++++++++
 drivers/platform/x86/intel_pmc_core.h |   6 ++
 2 files changed, 126 insertions(+)

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

index 43bbe74..d00fee2 100644
@@ -196,9 +196,67 @@ static const struct pmc_bit_map cnp_pfear_map[] = {
 	{}
 };
 
+static const struct pmc_bit_map cnp_slps0_dbg0_map[] = {
+	{"AUDIO_D3",		BIT(0)},
+	{"OTG_D3",		BIT(1)},
+	{"XHCI_D3",		BIT(2)},
+	{"LPIO_D3",		BIT(3)},
+	{"SDX_D3",		BIT(4)},
+	{"SATA_D3",		BIT(5)},
+	{"UFS0_D3",		BIT(6)},
+	{"UFS1_D3",		BIT(7)},
+	{"EMMC_D3",		BIT(8)},
+	{}
+};
+
+static const struct pmc_bit_map cnp_slps0_dbg1_map[] = {
+	{"SDIO_PLL_OFF",	BIT(0)},
+	{"USB2_PLL_OFF",	BIT(1)},
+	{"AUDIO_PLL_OFF",	BIT(2)},
+	{"ISCLK_OC_PLL_OFF",	BIT(3)},
+	{"ISCLK_MAIN_PLL_OFF",	BIT(4)},
+	{"XOSC_OFF",		BIT(5)},
+	{"LPC_CLKS_GATED",	BIT(6)},
+	{"PCIE_CLKREQS_IDLE",	BIT(7)},
+	{"AUDIO_ROSC_OFF",	BIT(8)},
+	{"HPET_XOSC_CLK_REQ",	BIT(9)},
+	{"PMC_ROSC_SLOW_CLK",	BIT(10)},
+	{"AON2_ROSC_GATED",	BIT(11)},
+	{"CLKACKS_DEASSERTED",	BIT(12)},
+	{}
+};
+
+static const struct pmc_bit_map cnp_slps0_dbg2_map[] = {
+	{"MPHY_CORE_GATED",	BIT(0)},
+	{"CSME_GATED",		BIT(1)},
+	{"USB2_SUS_GATED",	BIT(2)},
+	{"DYN_FLEX_IO_IDLE",	BIT(3)},
+	{"GBE_NO_LINK",		BIT(4)},
+	{"THERM_SEN_DISABLED",	BIT(5)},
+	{"PCIE_LOW_POWER",	BIT(6)},
+	{"ISH_VNNAON_REQ_ACT",	BIT(7)},
+	{"ISH_VNN_REQ_ACT",	BIT(8)},
+	{"CNV_VNNAON_REQ_ACT",	BIT(9)},
+	{"CNV_VNN_REQ_ACT",	BIT(10)},
+	{"NPK_VNNON_REQ_ACT",	BIT(11)},
+	{"PMSYNC_STATE_IDLE",	BIT(12)},
+	{"ALST_GT_THRES",	BIT(13)},
+	{"PMC_ARC_PG_READY",	BIT(14)},
+	{}
+};
+
+static const struct pmc_bit_map *cnp_slps0_dbg_maps[] = {
+	cnp_slps0_dbg0_map,
+	cnp_slps0_dbg1_map,
+	cnp_slps0_dbg2_map,
+	NULL,
+};
+
 static const struct pmc_reg_map cnp_reg_map = {
 	.pfear_sts = cnp_pfear_map,
 	.slp_s0_offset = CNP_PMC_SLP_S0_RES_COUNTER_OFFSET,
+	.slps0_dbg_maps = cnp_slps0_dbg_maps,
+	.slps0_dbg_offset = CNP_PMC_SLPS0_DBG_OFFSET,
 	.ltr_ignore_offset = CNP_PMC_LTR_IGNORE_OFFSET,
 	.regmap_length = CNP_PMC_MMIO_REG_LEN,
 	.ppfear0_offset = CNP_PMC_HOST_PPFEAR0A,

@@ -252,6 +310,8 @@ static int pmc_core_check_read_lock_bit(void)
 }
 
 #if IS_ENABLED(CONFIG_DEBUG_FS)
+static bool slps0_dbg_latch;
+
 static void pmc_core_display_map(struct seq_file *s, int index,
 				 u8 pf_reg, const struct pmc_bit_map *pf_map)
 {

@@ -481,6 +541,57 @@ static const struct file_operations pmc_core_ltr_ignore_ops = {
 	.release        = single_release,
 };
 
+static void pmc_core_slps0_dbg_latch(struct pmc_dev *pmcdev, bool reset)
+{
+	const struct pmc_reg_map *map = pmcdev->map;
+	u32 fd;
+
+	mutex_lock(&pmcdev->lock);
+
+	if (!reset && !slps0_dbg_latch)
+		goto out_unlock;
+
+	fd = pmc_core_reg_read(pmcdev, map->slps0_dbg_offset);
+	if (reset)
+		fd &= ~CNP_PMC_LATCH_SLPS0_EVENTS;
+	else
+		fd |= CNP_PMC_LATCH_SLPS0_EVENTS;
+	pmc_core_reg_write(pmcdev, map->slps0_dbg_offset, fd);
+
+	slps0_dbg_latch = 0;
+
+out_unlock:
+	mutex_unlock(&pmcdev->lock);
+}
+
+static int pmc_core_slps0_dbg_show(struct seq_file *s, void *unused)
+{
+	struct pmc_dev *pmcdev = s->private;
+	const struct pmc_bit_map **maps = pmcdev->map->slps0_dbg_maps;
+	const struct pmc_bit_map *map;
+	int offset;
+	u32 data;
+
+	pmc_core_slps0_dbg_latch(pmcdev, false);
+	offset = pmcdev->map->slps0_dbg_offset;
+	while (*maps) {
+		map = *maps;
+		data = pmc_core_reg_read(pmcdev, offset);
+		offset += 4;
+		while (map->name) {
+			seq_printf(s, "SLP_S0_DBG: %-32s\tState: %s\n",
+				   map->name,
+				   data & map->bit_mask ?
+				   "Yes" : "No");
+			++map;
+		}
+		++maps;
+	}
+	pmc_core_slps0_dbg_latch(pmcdev, true);
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(pmc_core_slps0_dbg);
+
 static void pmc_core_dbgfs_unregister(struct pmc_dev *pmcdev)
 {
 	debugfs_remove_recursive(pmcdev->dbgfs_dir);

@@ -514,6 +625,15 @@ static int pmc_core_dbgfs_register(struct pmc_dev *pmcdev)
 				    0444, dir, pmcdev,
 				    &pmc_core_mphy_pg_ops);
 
+	if (pmcdev->map->slps0_dbg_maps) {
+		debugfs_create_file("slp_s0_debug_status", 0444,
+				    dir, pmcdev,
+				    &pmc_core_slps0_dbg_fops);
+
+		debugfs_create_bool("slp_s0_debug_latch", 0644,
+				    dir, &slps0_dbg_latch);
+	}
+
 	return 0;
 }
 #else

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

index 5fa5f97..93a7e99 100644
@@ -127,12 +127,14 @@ enum ppfear_regs {
 #define CNP_PMC_SLP_S0_RES_COUNTER_OFFSET      0x193C
 #define CNP_PMC_LTR_IGNORE_OFFSET              0x1B0C
 #define CNP_PMC_PM_CFG_OFFSET                  0x1818
+#define CNP_PMC_SLPS0_DBG_OFFSET		0x10B4
 /* Cannonlake: PGD PFET Enable Ack Status Register(s) start */
 #define CNP_PMC_HOST_PPFEAR0A                  0x1D90
 
 #define CNP_PMC_MMIO_REG_LEN                   0x2000
 #define CNP_PPFEAR_NUM_ENTRIES                 8
 #define CNP_PMC_READ_DISABLE_BIT               22
+#define CNP_PMC_LATCH_SLPS0_EVENTS		BIT(31)
 
 struct pmc_bit_map {
 	const char *name;

@@ -145,6 +147,7 @@ struct pmc_bit_map {
  * @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
+ * @slps0_dbg_maps:	Array of SLP_S0_DBG* registers containing debug info
  * @slp_s0_offset:	PWRMBASE offset to read SLP_S0 residency
  * @ltr_ignore_offset:	PWRMBASE offset to read/write LTR ignore bit
  * @regmap_length:	Length of memory to map from PWRMBASE address to access

@@ -153,6 +156,7 @@ struct pmc_bit_map {
  *			PPFEAR
  * @pm_cfg_offset:	PWRMBASE offset to PM_CFG register
  * @pm_read_disable_bit: Bit index to read PMC_READ_DISABLE
+ * @slps0_dbg_offset:	PWRMBASE offset to SLP_S0_DEBUG_REG*
  *
  * Each PCH has unique set of register offsets and bit indexes. This structure
  * captures them to have a common implementation.

@@ -161,6 +165,7 @@ 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 struct pmc_bit_map **slps0_dbg_maps;
 	const u32 slp_s0_offset;
 	const u32 ltr_ignore_offset;
 	const int regmap_length;

@@ -168,6 +173,7 @@ struct pmc_reg_map {
 	const int ppfear_buckets;
 	const u32 pm_cfg_offset;
 	const int pm_read_disable_bit;
+	const u32 slps0_dbg_offset;
 };
 
 /**

Signature · 1 line

2.7.4

Rajneesh Bhardwaj

2018-06-15 11:27 UTC #13 lore raw

Quoted 9 lines · David E. Box

From: <[email protected]>
Adds debugfs access to registers in the Cannonlake PCH PMC that are useful for debugging #SLP_S0 signal assertion and other low power related activities. Device pm states are latched in these registers whenever the package enters C10 and can be read from slp_s0_debug_status. The pm states may also be latched by writing 1 to slp_s0_debug_latch which will immediately capture the current state on the next read of slp_s0_debug_status.

Reviewed-and-tested-by Rajneesh Bhardwaj <[email protected]>

Quoted 227 lines

Signed-off-by Box, David E <[email protected]>

 drivers/platform/x86/intel_pmc_core.c | 120 ++++++++++++++++++++++++++++++++++
 drivers/platform/x86/intel_pmc_core.h |   6 ++
 2 files changed, 126 insertions(+)

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

index 43bbe74..d00fee2 100644
@@ -196,9 +196,67 @@ static const struct pmc_bit_map cnp_pfear_map[] = {
 	{}
 };
 
+static const struct pmc_bit_map cnp_slps0_dbg0_map[] = {
+	{"AUDIO_D3",		BIT(0)},
+	{"OTG_D3",		BIT(1)},
+	{"XHCI_D3",		BIT(2)},
+	{"LPIO_D3",		BIT(3)},
+	{"SDX_D3",		BIT(4)},
+	{"SATA_D3",		BIT(5)},
+	{"UFS0_D3",		BIT(6)},
+	{"UFS1_D3",		BIT(7)},
+	{"EMMC_D3",		BIT(8)},
+	{}
+};
+
+static const struct pmc_bit_map cnp_slps0_dbg1_map[] = {
+	{"SDIO_PLL_OFF",	BIT(0)},
+	{"USB2_PLL_OFF",	BIT(1)},
+	{"AUDIO_PLL_OFF",	BIT(2)},
+	{"ISCLK_OC_PLL_OFF",	BIT(3)},
+	{"ISCLK_MAIN_PLL_OFF",	BIT(4)},
+	{"XOSC_OFF",		BIT(5)},
+	{"LPC_CLKS_GATED",	BIT(6)},
+	{"PCIE_CLKREQS_IDLE",	BIT(7)},
+	{"AUDIO_ROSC_OFF",	BIT(8)},
+	{"HPET_XOSC_CLK_REQ",	BIT(9)},
+	{"PMC_ROSC_SLOW_CLK",	BIT(10)},
+	{"AON2_ROSC_GATED",	BIT(11)},
+	{"CLKACKS_DEASSERTED",	BIT(12)},
+	{}
+};
+
+static const struct pmc_bit_map cnp_slps0_dbg2_map[] = {
+	{"MPHY_CORE_GATED",	BIT(0)},
+	{"CSME_GATED",		BIT(1)},
+	{"USB2_SUS_GATED",	BIT(2)},
+	{"DYN_FLEX_IO_IDLE",	BIT(3)},
+	{"GBE_NO_LINK",		BIT(4)},
+	{"THERM_SEN_DISABLED",	BIT(5)},
+	{"PCIE_LOW_POWER",	BIT(6)},
+	{"ISH_VNNAON_REQ_ACT",	BIT(7)},
+	{"ISH_VNN_REQ_ACT",	BIT(8)},
+	{"CNV_VNNAON_REQ_ACT",	BIT(9)},
+	{"CNV_VNN_REQ_ACT",	BIT(10)},
+	{"NPK_VNNON_REQ_ACT",	BIT(11)},
+	{"PMSYNC_STATE_IDLE",	BIT(12)},
+	{"ALST_GT_THRES",	BIT(13)},
+	{"PMC_ARC_PG_READY",	BIT(14)},
+	{}
+};
+
+static const struct pmc_bit_map *cnp_slps0_dbg_maps[] = {
+	cnp_slps0_dbg0_map,
+	cnp_slps0_dbg1_map,
+	cnp_slps0_dbg2_map,
+	NULL,
+};
+
 static const struct pmc_reg_map cnp_reg_map = {
 	.pfear_sts = cnp_pfear_map,
 	.slp_s0_offset = CNP_PMC_SLP_S0_RES_COUNTER_OFFSET,
+	.slps0_dbg_maps = cnp_slps0_dbg_maps,
+	.slps0_dbg_offset = CNP_PMC_SLPS0_DBG_OFFSET,
 	.ltr_ignore_offset = CNP_PMC_LTR_IGNORE_OFFSET,
 	.regmap_length = CNP_PMC_MMIO_REG_LEN,
 	.ppfear0_offset = CNP_PMC_HOST_PPFEAR0A,

@@ -252,6 +310,8 @@ static int pmc_core_check_read_lock_bit(void)
 }
 
 #if IS_ENABLED(CONFIG_DEBUG_FS)
+static bool slps0_dbg_latch;
+
 static void pmc_core_display_map(struct seq_file *s, int index,
 				 u8 pf_reg, const struct pmc_bit_map *pf_map)
 {

@@ -481,6 +541,57 @@ static const struct file_operations pmc_core_ltr_ignore_ops = {
 	.release        = single_release,
 };
 
+static void pmc_core_slps0_dbg_latch(struct pmc_dev *pmcdev, bool reset)
+{
+	const struct pmc_reg_map *map = pmcdev->map;
+	u32 fd;
+
+	mutex_lock(&pmcdev->lock);
+
+	if (!reset && !slps0_dbg_latch)
+		goto out_unlock;
+
+	fd = pmc_core_reg_read(pmcdev, map->slps0_dbg_offset);
+	if (reset)
+		fd &= ~CNP_PMC_LATCH_SLPS0_EVENTS;
+	else
+		fd |= CNP_PMC_LATCH_SLPS0_EVENTS;
+	pmc_core_reg_write(pmcdev, map->slps0_dbg_offset, fd);
+
+	slps0_dbg_latch = 0;
+
+out_unlock:
+	mutex_unlock(&pmcdev->lock);
+}
+
+static int pmc_core_slps0_dbg_show(struct seq_file *s, void *unused)
+{
+	struct pmc_dev *pmcdev = s->private;
+	const struct pmc_bit_map **maps = pmcdev->map->slps0_dbg_maps;
+	const struct pmc_bit_map *map;
+	int offset;
+	u32 data;
+
+	pmc_core_slps0_dbg_latch(pmcdev, false);
+	offset = pmcdev->map->slps0_dbg_offset;
+	while (*maps) {
+		map = *maps;
+		data = pmc_core_reg_read(pmcdev, offset);
+		offset += 4;
+		while (map->name) {
+			seq_printf(s, "SLP_S0_DBG: %-32s\tState: %s\n",
+				   map->name,
+				   data & map->bit_mask ?
+				   "Yes" : "No");
+			++map;
+		}
+		++maps;
+	}
+	pmc_core_slps0_dbg_latch(pmcdev, true);
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(pmc_core_slps0_dbg);
+
 static void pmc_core_dbgfs_unregister(struct pmc_dev *pmcdev)
 {
 	debugfs_remove_recursive(pmcdev->dbgfs_dir);

@@ -514,6 +625,15 @@ static int pmc_core_dbgfs_register(struct pmc_dev *pmcdev)
 				    0444, dir, pmcdev,
 				    &pmc_core_mphy_pg_ops);
 
+	if (pmcdev->map->slps0_dbg_maps) {
+		debugfs_create_file("slp_s0_debug_status", 0444,
+				    dir, pmcdev,
+				    &pmc_core_slps0_dbg_fops);
+
+		debugfs_create_bool("slp_s0_debug_latch", 0644,
+				    dir, &slps0_dbg_latch);
+	}
+
 	return 0;
 }
 #else

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

index 5fa5f97..93a7e99 100644
@@ -127,12 +127,14 @@ enum ppfear_regs {
 #define CNP_PMC_SLP_S0_RES_COUNTER_OFFSET      0x193C
 #define CNP_PMC_LTR_IGNORE_OFFSET              0x1B0C
 #define CNP_PMC_PM_CFG_OFFSET                  0x1818
+#define CNP_PMC_SLPS0_DBG_OFFSET		0x10B4
 /* Cannonlake: PGD PFET Enable Ack Status Register(s) start */
 #define CNP_PMC_HOST_PPFEAR0A                  0x1D90
 
 #define CNP_PMC_MMIO_REG_LEN                   0x2000
 #define CNP_PPFEAR_NUM_ENTRIES                 8
 #define CNP_PMC_READ_DISABLE_BIT               22
+#define CNP_PMC_LATCH_SLPS0_EVENTS		BIT(31)
 
 struct pmc_bit_map {
 	const char *name;

@@ -145,6 +147,7 @@ struct pmc_bit_map {
  * @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
+ * @slps0_dbg_maps:	Array of SLP_S0_DBG* registers containing debug info
  * @slp_s0_offset:	PWRMBASE offset to read SLP_S0 residency
  * @ltr_ignore_offset:	PWRMBASE offset to read/write LTR ignore bit
  * @regmap_length:	Length of memory to map from PWRMBASE address to access

@@ -153,6 +156,7 @@ struct pmc_bit_map {
  *			PPFEAR
  * @pm_cfg_offset:	PWRMBASE offset to PM_CFG register
  * @pm_read_disable_bit: Bit index to read PMC_READ_DISABLE
+ * @slps0_dbg_offset:	PWRMBASE offset to SLP_S0_DEBUG_REG*
  *
  * Each PCH has unique set of register offsets and bit indexes. This structure
  * captures them to have a common implementation.

@@ -161,6 +165,7 @@ 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 struct pmc_bit_map **slps0_dbg_maps;
 	const u32 slp_s0_offset;
 	const u32 ltr_ignore_offset;
 	const int regmap_length;

@@ -168,6 +173,7 @@ struct pmc_reg_map {
 	const int ppfear_buckets;
 	const u32 pm_cfg_offset;
 	const int pm_read_disable_bit;
+	const u32 slps0_dbg_offset;
 };
 
 /**

Signature · 1 line

2.7.4

Signature · 2 lines

Best Regards,
Rajneesh

Andy Shevchenko

2018-07-02 12:19 UTC #14 lore raw

Quoted 11 lines · Rajneesh Bhardwaj

Quoted 9 lines · David E. Box
From: <[email protected]>
Adds debugfs access to registers in the Cannonlake PCH PMC that are useful for debugging #SLP_S0 signal assertion and other low power related activities. Device pm states are latched in these registers whenever the package enters C10 and can be read from slp_s0_debug_status. The pm states may also be latched by writing 1 to slp_s0_debug_latch which will immediately capture the current state on the next read of slp_s0_debug_status.

Quoted 1 line

Reviewed-and-tested-by Rajneesh Bhardwaj <[email protected]>

Oops, sorry, it went without your tag.
patchwork seems didn't recognize this kind of combined tags.

Quoted 232 lines

Quoted 226 lines

Signed-off-by Box, David E <[email protected]>

 drivers/platform/x86/intel_pmc_core.c | 120 ++++++++++++++++++++++++++++++++++
 drivers/platform/x86/intel_pmc_core.h |   6 ++
 2 files changed, 126 insertions(+)

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

index 43bbe74..d00fee2 100644
@@ -196,9 +196,67 @@ static const struct pmc_bit_map cnp_pfear_map[] = {
      {}
 };
 
+static const struct pmc_bit_map cnp_slps0_dbg0_map[] = {
+     {"AUDIO_D3",            BIT(0)},
+     {"OTG_D3",              BIT(1)},
+     {"XHCI_D3",             BIT(2)},
+     {"LPIO_D3",             BIT(3)},
+     {"SDX_D3",              BIT(4)},
+     {"SATA_D3",             BIT(5)},
+     {"UFS0_D3",             BIT(6)},
+     {"UFS1_D3",             BIT(7)},
+     {"EMMC_D3",             BIT(8)},
+     {}
+};
+
+static const struct pmc_bit_map cnp_slps0_dbg1_map[] = {
+     {"SDIO_PLL_OFF",        BIT(0)},
+     {"USB2_PLL_OFF",        BIT(1)},
+     {"AUDIO_PLL_OFF",       BIT(2)},
+     {"ISCLK_OC_PLL_OFF",    BIT(3)},
+     {"ISCLK_MAIN_PLL_OFF",  BIT(4)},
+     {"XOSC_OFF",            BIT(5)},
+     {"LPC_CLKS_GATED",      BIT(6)},
+     {"PCIE_CLKREQS_IDLE",   BIT(7)},
+     {"AUDIO_ROSC_OFF",      BIT(8)},
+     {"HPET_XOSC_CLK_REQ",   BIT(9)},
+     {"PMC_ROSC_SLOW_CLK",   BIT(10)},
+     {"AON2_ROSC_GATED",     BIT(11)},
+     {"CLKACKS_DEASSERTED",  BIT(12)},
+     {}
+};
+
+static const struct pmc_bit_map cnp_slps0_dbg2_map[] = {
+     {"MPHY_CORE_GATED",     BIT(0)},
+     {"CSME_GATED",          BIT(1)},
+     {"USB2_SUS_GATED",      BIT(2)},
+     {"DYN_FLEX_IO_IDLE",    BIT(3)},
+     {"GBE_NO_LINK",         BIT(4)},
+     {"THERM_SEN_DISABLED",  BIT(5)},
+     {"PCIE_LOW_POWER",      BIT(6)},
+     {"ISH_VNNAON_REQ_ACT",  BIT(7)},
+     {"ISH_VNN_REQ_ACT",     BIT(8)},
+     {"CNV_VNNAON_REQ_ACT",  BIT(9)},
+     {"CNV_VNN_REQ_ACT",     BIT(10)},
+     {"NPK_VNNON_REQ_ACT",   BIT(11)},
+     {"PMSYNC_STATE_IDLE",   BIT(12)},
+     {"ALST_GT_THRES",       BIT(13)},
+     {"PMC_ARC_PG_READY",    BIT(14)},
+     {}
+};
+
+static const struct pmc_bit_map *cnp_slps0_dbg_maps[] = {
+     cnp_slps0_dbg0_map,
+     cnp_slps0_dbg1_map,
+     cnp_slps0_dbg2_map,
+     NULL,
+};
+
 static const struct pmc_reg_map cnp_reg_map = {
      .pfear_sts = cnp_pfear_map,
      .slp_s0_offset = CNP_PMC_SLP_S0_RES_COUNTER_OFFSET,
+     .slps0_dbg_maps = cnp_slps0_dbg_maps,
+     .slps0_dbg_offset = CNP_PMC_SLPS0_DBG_OFFSET,
      .ltr_ignore_offset = CNP_PMC_LTR_IGNORE_OFFSET,
      .regmap_length = CNP_PMC_MMIO_REG_LEN,
      .ppfear0_offset = CNP_PMC_HOST_PPFEAR0A,

@@ -252,6 +310,8 @@ static int pmc_core_check_read_lock_bit(void)
 }
 
 #if IS_ENABLED(CONFIG_DEBUG_FS)
+static bool slps0_dbg_latch;
+
 static void pmc_core_display_map(struct seq_file *s, int index,
                               u8 pf_reg, const struct pmc_bit_map *pf_map)
 {

@@ -481,6 +541,57 @@ static const struct file_operations pmc_core_ltr_ignore_ops = {
      .release        = single_release,
 };
 
+static void pmc_core_slps0_dbg_latch(struct pmc_dev *pmcdev, bool reset)
+{
+     const struct pmc_reg_map *map = pmcdev->map;
+     u32 fd;
+
+     mutex_lock(&pmcdev->lock);
+
+     if (!reset && !slps0_dbg_latch)
+             goto out_unlock;
+
+     fd = pmc_core_reg_read(pmcdev, map->slps0_dbg_offset);
+     if (reset)
+             fd &= ~CNP_PMC_LATCH_SLPS0_EVENTS;
+     else
+             fd |= CNP_PMC_LATCH_SLPS0_EVENTS;
+     pmc_core_reg_write(pmcdev, map->slps0_dbg_offset, fd);
+
+     slps0_dbg_latch = 0;
+
+out_unlock:
+     mutex_unlock(&pmcdev->lock);
+}
+
+static int pmc_core_slps0_dbg_show(struct seq_file *s, void *unused)
+{
+     struct pmc_dev *pmcdev = s->private;
+     const struct pmc_bit_map **maps = pmcdev->map->slps0_dbg_maps;
+     const struct pmc_bit_map *map;
+     int offset;
+     u32 data;
+
+     pmc_core_slps0_dbg_latch(pmcdev, false);
+     offset = pmcdev->map->slps0_dbg_offset;
+     while (*maps) {
+             map = *maps;
+             data = pmc_core_reg_read(pmcdev, offset);
+             offset += 4;
+             while (map->name) {
+                     seq_printf(s, "SLP_S0_DBG: %-32s\tState: %s\n",
+                                map->name,
+                                data & map->bit_mask ?
+                                "Yes" : "No");
+                     ++map;
+             }
+             ++maps;
+     }
+     pmc_core_slps0_dbg_latch(pmcdev, true);
+     return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(pmc_core_slps0_dbg);
+
 static void pmc_core_dbgfs_unregister(struct pmc_dev *pmcdev)
 {
      debugfs_remove_recursive(pmcdev->dbgfs_dir);

@@ -514,6 +625,15 @@ static int pmc_core_dbgfs_register(struct pmc_dev *pmcdev)
                                  0444, dir, pmcdev,
                                  &pmc_core_mphy_pg_ops);
 
+     if (pmcdev->map->slps0_dbg_maps) {
+             debugfs_create_file("slp_s0_debug_status", 0444,
+                                 dir, pmcdev,
+                                 &pmc_core_slps0_dbg_fops);
+
+             debugfs_create_bool("slp_s0_debug_latch", 0644,
+                                 dir, &slps0_dbg_latch);
+     }
+
      return 0;
 }
 #else

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

index 5fa5f97..93a7e99 100644
@@ -127,12 +127,14 @@ enum ppfear_regs {
 #define CNP_PMC_SLP_S0_RES_COUNTER_OFFSET      0x193C
 #define CNP_PMC_LTR_IGNORE_OFFSET              0x1B0C
 #define CNP_PMC_PM_CFG_OFFSET                  0x1818
+#define CNP_PMC_SLPS0_DBG_OFFSET             0x10B4
 /* Cannonlake: PGD PFET Enable Ack Status Register(s) start */
 #define CNP_PMC_HOST_PPFEAR0A                  0x1D90
 
 #define CNP_PMC_MMIO_REG_LEN                   0x2000
 #define CNP_PPFEAR_NUM_ENTRIES                 8
 #define CNP_PMC_READ_DISABLE_BIT               22
+#define CNP_PMC_LATCH_SLPS0_EVENTS           BIT(31)
 
 struct pmc_bit_map {
      const char *name;

@@ -145,6 +147,7 @@ struct pmc_bit_map {
  * @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
+ * @slps0_dbg_maps:  Array of SLP_S0_DBG* registers containing debug info
  * @slp_s0_offset:   PWRMBASE offset to read SLP_S0 residency
  * @ltr_ignore_offset:       PWRMBASE offset to read/write LTR ignore bit
  * @regmap_length:   Length of memory to map from PWRMBASE address to access

@@ -153,6 +156,7 @@ struct pmc_bit_map {
  *                   PPFEAR
  * @pm_cfg_offset:   PWRMBASE offset to PM_CFG register
  * @pm_read_disable_bit: Bit index to read PMC_READ_DISABLE
+ * @slps0_dbg_offset:        PWRMBASE offset to SLP_S0_DEBUG_REG*
  *
  * Each PCH has unique set of register offsets and bit indexes. This structure
  * captures them to have a common implementation.

@@ -161,6 +165,7 @@ 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 struct pmc_bit_map **slps0_dbg_maps;
      const u32 slp_s0_offset;
      const u32 ltr_ignore_offset;
      const int regmap_length;

@@ -168,6 +173,7 @@ struct pmc_reg_map {
      const int ppfear_buckets;
      const u32 pm_cfg_offset;
      const int pm_read_disable_bit;
+     const u32 slps0_dbg_offset;
 };
 
 /**
--

Quoted 2 lines

2.7.4

--
Best Regards,
Rajneesh

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Rajneesh Bhardwaj

2018-07-02 18:21 UTC #15 lore raw

Quoted 13 lines · Andy Shevchenko

Quoted 11 lines · Rajneesh Bhardwaj
Quoted 9 lines · David E. Box
From: <[email protected]>
Adds debugfs access to registers in the Cannonlake PCH PMC that are useful for debugging #SLP_S0 signal assertion and other low power related activities. Device pm states are latched in these registers whenever the package enters C10 and can be read from slp_s0_debug_status. The pm states may also be latched by writing 1 to slp_s0_debug_latch which will immediately capture the current state on the next read of slp_s0_debug_status.

Quoted 4 lines

Quoted 1 line
Reviewed-and-tested-by Rajneesh Bhardwaj <[email protected]>
Oops, sorry, it went without your tag.
patchwork seems didn't recognize this kind of combined tags.

NP, thats fine.

Quoted 239 lines

Quoted 233 lines

Quoted 227 lines

Signed-off-by Box, David E <[email protected]>

 drivers/platform/x86/intel_pmc_core.c | 120 ++++++++++++++++++++++++++++++++++
 drivers/platform/x86/intel_pmc_core.h |   6 ++
 2 files changed, 126 insertions(+)

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

index 43bbe74..d00fee2 100644
@@ -196,9 +196,67 @@ static const struct pmc_bit_map cnp_pfear_map[] = {
      {}
 };
 
+static const struct pmc_bit_map cnp_slps0_dbg0_map[] = {
+     {"AUDIO_D3",            BIT(0)},
+     {"OTG_D3",              BIT(1)},
+     {"XHCI_D3",             BIT(2)},
+     {"LPIO_D3",             BIT(3)},
+     {"SDX_D3",              BIT(4)},
+     {"SATA_D3",             BIT(5)},
+     {"UFS0_D3",             BIT(6)},
+     {"UFS1_D3",             BIT(7)},
+     {"EMMC_D3",             BIT(8)},
+     {}
+};
+
+static const struct pmc_bit_map cnp_slps0_dbg1_map[] = {
+     {"SDIO_PLL_OFF",        BIT(0)},
+     {"USB2_PLL_OFF",        BIT(1)},
+     {"AUDIO_PLL_OFF",       BIT(2)},
+     {"ISCLK_OC_PLL_OFF",    BIT(3)},
+     {"ISCLK_MAIN_PLL_OFF",  BIT(4)},
+     {"XOSC_OFF",            BIT(5)},
+     {"LPC_CLKS_GATED",      BIT(6)},
+     {"PCIE_CLKREQS_IDLE",   BIT(7)},
+     {"AUDIO_ROSC_OFF",      BIT(8)},
+     {"HPET_XOSC_CLK_REQ",   BIT(9)},
+     {"PMC_ROSC_SLOW_CLK",   BIT(10)},
+     {"AON2_ROSC_GATED",     BIT(11)},
+     {"CLKACKS_DEASSERTED",  BIT(12)},
+     {}
+};
+
+static const struct pmc_bit_map cnp_slps0_dbg2_map[] = {
+     {"MPHY_CORE_GATED",     BIT(0)},
+     {"CSME_GATED",          BIT(1)},
+     {"USB2_SUS_GATED",      BIT(2)},
+     {"DYN_FLEX_IO_IDLE",    BIT(3)},
+     {"GBE_NO_LINK",         BIT(4)},
+     {"THERM_SEN_DISABLED",  BIT(5)},
+     {"PCIE_LOW_POWER",      BIT(6)},
+     {"ISH_VNNAON_REQ_ACT",  BIT(7)},
+     {"ISH_VNN_REQ_ACT",     BIT(8)},
+     {"CNV_VNNAON_REQ_ACT",  BIT(9)},
+     {"CNV_VNN_REQ_ACT",     BIT(10)},
+     {"NPK_VNNON_REQ_ACT",   BIT(11)},
+     {"PMSYNC_STATE_IDLE",   BIT(12)},
+     {"ALST_GT_THRES",       BIT(13)},
+     {"PMC_ARC_PG_READY",    BIT(14)},
+     {}
+};
+
+static const struct pmc_bit_map *cnp_slps0_dbg_maps[] = {
+     cnp_slps0_dbg0_map,
+     cnp_slps0_dbg1_map,
+     cnp_slps0_dbg2_map,
+     NULL,
+};
+
 static const struct pmc_reg_map cnp_reg_map = {
      .pfear_sts = cnp_pfear_map,
      .slp_s0_offset = CNP_PMC_SLP_S0_RES_COUNTER_OFFSET,
+     .slps0_dbg_maps = cnp_slps0_dbg_maps,
+     .slps0_dbg_offset = CNP_PMC_SLPS0_DBG_OFFSET,
      .ltr_ignore_offset = CNP_PMC_LTR_IGNORE_OFFSET,
      .regmap_length = CNP_PMC_MMIO_REG_LEN,
      .ppfear0_offset = CNP_PMC_HOST_PPFEAR0A,

@@ -252,6 +310,8 @@ static int pmc_core_check_read_lock_bit(void)
 }
 
 #if IS_ENABLED(CONFIG_DEBUG_FS)
+static bool slps0_dbg_latch;
+
 static void pmc_core_display_map(struct seq_file *s, int index,
                               u8 pf_reg, const struct pmc_bit_map *pf_map)
 {

@@ -481,6 +541,57 @@ static const struct file_operations pmc_core_ltr_ignore_ops = {
      .release        = single_release,
 };
 
+static void pmc_core_slps0_dbg_latch(struct pmc_dev *pmcdev, bool reset)
+{
+     const struct pmc_reg_map *map = pmcdev->map;
+     u32 fd;
+
+     mutex_lock(&pmcdev->lock);
+
+     if (!reset && !slps0_dbg_latch)
+             goto out_unlock;
+
+     fd = pmc_core_reg_read(pmcdev, map->slps0_dbg_offset);
+     if (reset)
+             fd &= ~CNP_PMC_LATCH_SLPS0_EVENTS;
+     else
+             fd |= CNP_PMC_LATCH_SLPS0_EVENTS;
+     pmc_core_reg_write(pmcdev, map->slps0_dbg_offset, fd);
+
+     slps0_dbg_latch = 0;
+
+out_unlock:
+     mutex_unlock(&pmcdev->lock);
+}
+
+static int pmc_core_slps0_dbg_show(struct seq_file *s, void *unused)
+{
+     struct pmc_dev *pmcdev = s->private;
+     const struct pmc_bit_map **maps = pmcdev->map->slps0_dbg_maps;
+     const struct pmc_bit_map *map;
+     int offset;
+     u32 data;
+
+     pmc_core_slps0_dbg_latch(pmcdev, false);
+     offset = pmcdev->map->slps0_dbg_offset;
+     while (*maps) {
+             map = *maps;
+             data = pmc_core_reg_read(pmcdev, offset);
+             offset += 4;
+             while (map->name) {
+                     seq_printf(s, "SLP_S0_DBG: %-32s\tState: %s\n",
+                                map->name,
+                                data & map->bit_mask ?
+                                "Yes" : "No");
+                     ++map;
+             }
+             ++maps;
+     }
+     pmc_core_slps0_dbg_latch(pmcdev, true);
+     return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(pmc_core_slps0_dbg);
+
 static void pmc_core_dbgfs_unregister(struct pmc_dev *pmcdev)
 {
      debugfs_remove_recursive(pmcdev->dbgfs_dir);

@@ -514,6 +625,15 @@ static int pmc_core_dbgfs_register(struct pmc_dev *pmcdev)
                                  0444, dir, pmcdev,
                                  &pmc_core_mphy_pg_ops);
 
+     if (pmcdev->map->slps0_dbg_maps) {
+             debugfs_create_file("slp_s0_debug_status", 0444,
+                                 dir, pmcdev,
+                                 &pmc_core_slps0_dbg_fops);
+
+             debugfs_create_bool("slp_s0_debug_latch", 0644,
+                                 dir, &slps0_dbg_latch);
+     }
+
      return 0;
 }
 #else

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

index 5fa5f97..93a7e99 100644
@@ -127,12 +127,14 @@ enum ppfear_regs {
 #define CNP_PMC_SLP_S0_RES_COUNTER_OFFSET      0x193C
 #define CNP_PMC_LTR_IGNORE_OFFSET              0x1B0C
 #define CNP_PMC_PM_CFG_OFFSET                  0x1818
+#define CNP_PMC_SLPS0_DBG_OFFSET             0x10B4
 /* Cannonlake: PGD PFET Enable Ack Status Register(s) start */
 #define CNP_PMC_HOST_PPFEAR0A                  0x1D90
 
 #define CNP_PMC_MMIO_REG_LEN                   0x2000
 #define CNP_PPFEAR_NUM_ENTRIES                 8
 #define CNP_PMC_READ_DISABLE_BIT               22
+#define CNP_PMC_LATCH_SLPS0_EVENTS           BIT(31)
 
 struct pmc_bit_map {
      const char *name;

@@ -145,6 +147,7 @@ struct pmc_bit_map {
  * @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
+ * @slps0_dbg_maps:  Array of SLP_S0_DBG* registers containing debug info
  * @slp_s0_offset:   PWRMBASE offset to read SLP_S0 residency
  * @ltr_ignore_offset:       PWRMBASE offset to read/write LTR ignore bit
  * @regmap_length:   Length of memory to map from PWRMBASE address to access

@@ -153,6 +156,7 @@ struct pmc_bit_map {
  *                   PPFEAR
  * @pm_cfg_offset:   PWRMBASE offset to PM_CFG register
  * @pm_read_disable_bit: Bit index to read PMC_READ_DISABLE
+ * @slps0_dbg_offset:        PWRMBASE offset to SLP_S0_DEBUG_REG*
  *
  * Each PCH has unique set of register offsets and bit indexes. This structure
  * captures them to have a common implementation.

@@ -161,6 +165,7 @@ 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 struct pmc_bit_map **slps0_dbg_maps;
      const u32 slp_s0_offset;
      const u32 ltr_ignore_offset;
      const int regmap_length;

@@ -168,6 +173,7 @@ struct pmc_reg_map {
      const int ppfear_buckets;
      const u32 pm_cfg_offset;
      const int pm_read_disable_bit;
+     const u32 slps0_dbg_offset;
 };
 
 /**
--

Quoted 2 lines

2.7.4

--
Best Regards,
Rajneesh

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Signature · 2 lines

Best Regards,
Rajneesh