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.
> Adds debugfs access to registers in the Cannon Point PCH PMC that are
Cannonlake PCH
> useful for debugging #SLP_S0 signal assertion and other low power relate
Typo, 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.
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.
>
> Signed-off-by: Box, David E <david.e.box@xxxxxxxxx>
> ---
> 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(+)
>
> diff --git a/drivers/platform/x86/intel_pmc_core.c b/drivers/platform/x86/intel_pmc_core.c
> index 43bbe74743d9..2d272a3e0176 100644
> --- a/drivers/platform/x86/intel_pmc_core.c
> +++ b/drivers/platform/x86/intel_pmc_core.c
> @@ -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
> diff --git a/drivers/platform/x86/intel_pmc_core.h b/drivers/platform/x86/intel_pmc_core.h
> index 5fa5f97870aa..93a7e99e1f8b 100644
> --- a/drivers/platform/x86/intel_pmc_core.h
> +++ b/drivers/platform/x86/intel_pmc_core.h
> @@ -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;
> };
>
> /**
> --
> 2.14.4
>
--
Best Regards,
Rajneesh
