* [PATCH v4 0/3] x86: clean ups and feature enhancement in pmc_atom @ 2014-11-12 17:18 Andy Shevchenko 2014-11-12 17:18 ` [PATCH v4 1/3] x86: pmc_atom: clean up init function Andy Shevchenko ` (2 more replies) 0 siblings, 3 replies; 6+ messages in thread From: Andy Shevchenko @ 2014-11-12 17:18 UTC (permalink / raw) To: H . Peter Anvin, x86, Aubrey Li, Rafael J . Wysocki, Kumar P, Mahesh, linux-kernel Cc: Andy Shevchenko Couple of clean ups and one feature enhancement (expose PSS register in the debug fs). Changes since v3: - rebase on top of recent linux-next - resend to better place for review and push Changes since v2: - rebase on top of recent linux-next - applied Acks - introduce patch 3/3 Andy Shevchenko (3): x86: pmc_atom: clean up init function x86: pmc_atom: don't check for NULL twice x86: pmc_atom: expose contents of PSS arch/x86/include/asm/pmc_atom.h | 22 ++++++++++++ arch/x86/kernel/pmc_atom.c | 75 +++++++++++++++++++++++++++++++++-------- 2 files changed, 83 insertions(+), 14 deletions(-) -- 2.1.3 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v4 1/3] x86: pmc_atom: clean up init function 2014-11-12 17:18 [PATCH v4 0/3] x86: clean ups and feature enhancement in pmc_atom Andy Shevchenko @ 2014-11-12 17:18 ` Andy Shevchenko 2014-11-12 17:18 ` [PATCH v4 2/3] x86: pmc_atom: don't check for NULL twice Andy Shevchenko 2014-11-12 17:18 ` [PATCH v4 3/3] x86: pmc_atom: expose contents of PSS Andy Shevchenko 2 siblings, 0 replies; 6+ messages in thread From: Andy Shevchenko @ 2014-11-12 17:18 UTC (permalink / raw) To: H . Peter Anvin, x86, Aubrey Li, Rafael J . Wysocki, Kumar P, Mahesh, linux-kernel Cc: Andy Shevchenko There is no need to use err variable. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Acked-by: Aubrey Li <aubrey.li@linux.intel.com> --- arch/x86/kernel/pmc_atom.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/arch/x86/kernel/pmc_atom.c b/arch/x86/kernel/pmc_atom.c index 0ee5025e..19b8efa 100644 --- a/arch/x86/kernel/pmc_atom.c +++ b/arch/x86/kernel/pmc_atom.c @@ -292,7 +292,6 @@ MODULE_DEVICE_TABLE(pci, pmc_pci_ids); static int __init pmc_atom_init(void) { - int err = -ENODEV; struct pci_dev *pdev = NULL; const struct pci_device_id *ent; @@ -306,14 +305,11 @@ static int __init pmc_atom_init(void) */ for_each_pci_dev(pdev) { ent = pci_match_id(pmc_pci_ids, pdev); - if (ent) { - err = pmc_setup_dev(pdev); - goto out; - } + if (ent) + return pmc_setup_dev(pdev); } /* Device not found. */ -out: - return err; + return -ENODEV; } module_init(pmc_atom_init); -- 2.1.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v4 2/3] x86: pmc_atom: don't check for NULL twice 2014-11-12 17:18 [PATCH v4 0/3] x86: clean ups and feature enhancement in pmc_atom Andy Shevchenko 2014-11-12 17:18 ` [PATCH v4 1/3] x86: pmc_atom: clean up init function Andy Shevchenko @ 2014-11-12 17:18 ` Andy Shevchenko 2014-11-19 11:36 ` Thomas Gleixner 2014-11-12 17:18 ` [PATCH v4 3/3] x86: pmc_atom: expose contents of PSS Andy Shevchenko 2 siblings, 1 reply; 6+ messages in thread From: Andy Shevchenko @ 2014-11-12 17:18 UTC (permalink / raw) To: H . Peter Anvin, x86, Aubrey Li, Rafael J . Wysocki, Kumar P, Mahesh, linux-kernel Cc: Andy Shevchenko debugfs_remove_recursive() is NULL-aware, thus, we may safely remove the check here. There is no need to assing NULL to variable since it will be not used anywhere. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Acked-by: Aubrey Li <aubrey.li@linux.intel.com> --- arch/x86/kernel/pmc_atom.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/arch/x86/kernel/pmc_atom.c b/arch/x86/kernel/pmc_atom.c index 19b8efa..bcc91ea 100644 --- a/arch/x86/kernel/pmc_atom.c +++ b/arch/x86/kernel/pmc_atom.c @@ -202,11 +202,7 @@ static const struct file_operations pmc_sleep_tmr_ops = { static void pmc_dbgfs_unregister(struct pmc_dev *pmc) { - if (!pmc->dbgfs_dir) - return; - debugfs_remove_recursive(pmc->dbgfs_dir); - pmc->dbgfs_dir = NULL; } static int pmc_dbgfs_register(struct pmc_dev *pmc, struct pci_dev *pdev) -- 2.1.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v4 2/3] x86: pmc_atom: don't check for NULL twice 2014-11-12 17:18 ` [PATCH v4 2/3] x86: pmc_atom: don't check for NULL twice Andy Shevchenko @ 2014-11-19 11:36 ` Thomas Gleixner 2014-11-28 9:49 ` Andy Shevchenko 0 siblings, 1 reply; 6+ messages in thread From: Thomas Gleixner @ 2014-11-19 11:36 UTC (permalink / raw) To: Andy Shevchenko Cc: H . Peter Anvin, x86, Aubrey Li, Rafael J . Wysocki, Kumar P, Mahesh, linux-kernel On Wed, 12 Nov 2014, Andy Shevchenko wrote: > debugfs_remove_recursive() is NULL-aware, thus, we may safely remove the check > here. There is no need to assing NULL to variable since it will be not used > anywhere. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Acked-by: Aubrey Li <aubrey.li@linux.intel.com> > --- > arch/x86/kernel/pmc_atom.c | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/arch/x86/kernel/pmc_atom.c b/arch/x86/kernel/pmc_atom.c > index 19b8efa..bcc91ea 100644 > --- a/arch/x86/kernel/pmc_atom.c > +++ b/arch/x86/kernel/pmc_atom.c > @@ -202,11 +202,7 @@ static const struct file_operations pmc_sleep_tmr_ops = { > > static void pmc_dbgfs_unregister(struct pmc_dev *pmc) > { > - if (!pmc->dbgfs_dir) > - return; > - > debugfs_remove_recursive(pmc->dbgfs_dir); > - pmc->dbgfs_dir = NULL; > } So while the patch is correct per se, the whole function is useless. pmc_dbgfs_register() dir = debugfs_create_dir("pmc_atom", NULL); .... if (!f) goto err; pmc->dbgfs_dir = dir; return 0; err: pmc_dbgfs_unregister(pmc); So pmc->dbgfs_dir is always NULL when this is called.... Aside of that, if DEBUGFS=n the code keeps pmc->regmap ioremapped for no reason. Looking deeper: The pmc_power_off function is installed _BEFORE_ pmc_hw_reg_setup() is called, which might not be called if the ioremap fails .... Thanks, tglx ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 2/3] x86: pmc_atom: don't check for NULL twice 2014-11-19 11:36 ` Thomas Gleixner @ 2014-11-28 9:49 ` Andy Shevchenko 0 siblings, 0 replies; 6+ messages in thread From: Andy Shevchenko @ 2014-11-28 9:49 UTC (permalink / raw) To: Thomas Gleixner Cc: H . Peter Anvin, x86, Aubrey Li, Rafael J . Wysocki, Kumar P, Mahesh, linux-kernel On Wed, 2014-11-19 at 12:36 +0100, Thomas Gleixner wrote: > On Wed, 12 Nov 2014, Andy Shevchenko wrote: > > > debugfs_remove_recursive() is NULL-aware, thus, we may safely remove the check > > here. There is no need to assing NULL to variable since it will be not used > > anywhere. Thanks for review, my comments below. > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Acked-by: Aubrey Li <aubrey.li@linux.intel.com> > > --- > > arch/x86/kernel/pmc_atom.c | 4 ---- > > 1 file changed, 4 deletions(-) > > > > diff --git a/arch/x86/kernel/pmc_atom.c b/arch/x86/kernel/pmc_atom.c > > index 19b8efa..bcc91ea 100644 > > --- a/arch/x86/kernel/pmc_atom.c > > +++ b/arch/x86/kernel/pmc_atom.c > > @@ -202,11 +202,7 @@ static const struct file_operations pmc_sleep_tmr_ops = { > > > > static void pmc_dbgfs_unregister(struct pmc_dev *pmc) > > { > > - if (!pmc->dbgfs_dir) > > - return; > > - > > debugfs_remove_recursive(pmc->dbgfs_dir); > > - pmc->dbgfs_dir = NULL; > > } > > So while the patch is correct per se, the whole function is useless. > > pmc_dbgfs_register() > > dir = debugfs_create_dir("pmc_atom", NULL); > > .... > if (!f) > goto err; > > pmc->dbgfs_dir = dir; > return 0; > err: > pmc_dbgfs_unregister(pmc); > > So pmc->dbgfs_dir is always NULL when this is called.... Good catch, I add a new patch to address this. > Aside of that, if DEBUGFS=n the code keeps pmc->regmap ioremapped for > no reason. pmc_hw_reg_setup() uses it. Aubrey, is it intended behaviour? > Looking deeper: > > The pmc_power_off function is installed _BEFORE_ pmc_hw_reg_setup() is > called, which might not be called if the ioremap fails .... I don't see any problem here (pmc_power_off uses acpi_base_addr which is independent to IO mapped space), though Aubrey may shed a light on this. -- Andy Shevchenko <andriy.shevchenko@intel.com> Intel Finland Oy ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v4 3/3] x86: pmc_atom: expose contents of PSS 2014-11-12 17:18 [PATCH v4 0/3] x86: clean ups and feature enhancement in pmc_atom Andy Shevchenko 2014-11-12 17:18 ` [PATCH v4 1/3] x86: pmc_atom: clean up init function Andy Shevchenko 2014-11-12 17:18 ` [PATCH v4 2/3] x86: pmc_atom: don't check for NULL twice Andy Shevchenko @ 2014-11-12 17:18 ` Andy Shevchenko 2 siblings, 0 replies; 6+ messages in thread From: Andy Shevchenko @ 2014-11-12 17:18 UTC (permalink / raw) To: H . Peter Anvin, x86, Aubrey Li, Rafael J . Wysocki, Kumar P, Mahesh, linux-kernel Cc: Andy Shevchenko The PSS register reflects the power state of each island on SoC. It would be useful to know which of the islands is on or off at the momemnt. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Acked-by: Aubrey Li <aubrey.li@linux.intel.com> --- arch/x86/include/asm/pmc_atom.h | 22 +++++++++++++++ arch/x86/kernel/pmc_atom.c | 61 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 80 insertions(+), 3 deletions(-) diff --git a/arch/x86/include/asm/pmc_atom.h b/arch/x86/include/asm/pmc_atom.h index fc7a17c..bc0fc08 100644 --- a/arch/x86/include/asm/pmc_atom.h +++ b/arch/x86/include/asm/pmc_atom.h @@ -53,6 +53,28 @@ /* Sleep state counter is in units of of 32us */ #define PMC_TMR_SHIFT 5 +/* Power status of power islands */ +#define PMC_PSS 0x98 + +#define PMC_PSS_BIT_GBE BIT(0) +#define PMC_PSS_BIT_SATA BIT(1) +#define PMC_PSS_BIT_HDA BIT(2) +#define PMC_PSS_BIT_SEC BIT(3) +#define PMC_PSS_BIT_PCIE BIT(4) +#define PMC_PSS_BIT_LPSS BIT(5) +#define PMC_PSS_BIT_LPE BIT(6) +#define PMC_PSS_BIT_DFX BIT(7) +#define PMC_PSS_BIT_USH_CTRL BIT(8) +#define PMC_PSS_BIT_USH_SUS BIT(9) +#define PMC_PSS_BIT_USH_VCCS BIT(10) +#define PMC_PSS_BIT_USH_VCCA BIT(11) +#define PMC_PSS_BIT_OTG_CTRL BIT(12) +#define PMC_PSS_BIT_OTG_VCCS BIT(13) +#define PMC_PSS_BIT_OTG_VCCA_CLK BIT(14) +#define PMC_PSS_BIT_OTG_VCCA BIT(15) +#define PMC_PSS_BIT_USB BIT(16) +#define PMC_PSS_BIT_USB_SUS BIT(17) + /* These registers reflect D3 status of functions */ #define PMC_D3_STS_0 0xA0 diff --git a/arch/x86/kernel/pmc_atom.c b/arch/x86/kernel/pmc_atom.c index bcc91ea..0eb469b 100644 --- a/arch/x86/kernel/pmc_atom.c +++ b/arch/x86/kernel/pmc_atom.c @@ -38,12 +38,12 @@ struct pmc_dev { static struct pmc_dev pmc_device; static u32 acpi_base_addr; -struct pmc_dev_map { +struct pmc_bit_map { const char *name; u32 bit_mask; }; -static const struct pmc_dev_map dev_map[] = { +static const struct pmc_bit_map dev_map[] = { {"0 - LPSS1_F0_DMA", BIT_LPSS1_F0_DMA}, {"1 - LPSS1_F1_PWM1", BIT_LPSS1_F1_PWM1}, {"2 - LPSS1_F2_PWM2", BIT_LPSS1_F2_PWM2}, @@ -82,6 +82,27 @@ static const struct pmc_dev_map dev_map[] = { {"35 - DFX", BIT_DFX}, }; +static const struct pmc_bit_map pss_map[] = { + {"0 - GBE", PMC_PSS_BIT_GBE}, + {"1 - SATA", PMC_PSS_BIT_SATA}, + {"2 - HDA", PMC_PSS_BIT_HDA}, + {"3 - SEC", PMC_PSS_BIT_SEC}, + {"4 - PCIE", PMC_PSS_BIT_PCIE}, + {"5 - LPSS", PMC_PSS_BIT_LPSS}, + {"6 - LPE", PMC_PSS_BIT_LPE}, + {"7 - DFX", PMC_PSS_BIT_DFX}, + {"8 - USH_CTRL", PMC_PSS_BIT_USH_CTRL}, + {"9 - USH_SUS", PMC_PSS_BIT_USH_SUS}, + {"10 - USH_VCCS", PMC_PSS_BIT_USH_VCCS}, + {"11 - USH_VCCA", PMC_PSS_BIT_USH_VCCA}, + {"12 - OTG_CTRL", PMC_PSS_BIT_OTG_CTRL}, + {"13 - OTG_VCCS", PMC_PSS_BIT_OTG_VCCS}, + {"14 - OTG_VCCA_CLK", PMC_PSS_BIT_OTG_VCCA_CLK}, + {"15 - OTG_VCCA", PMC_PSS_BIT_OTG_VCCA}, + {"16 - USB", PMC_PSS_BIT_USB}, + {"17 - USB_SUS", PMC_PSS_BIT_USB_SUS}, +}; + static inline u32 pmc_reg_read(struct pmc_dev *pmc, int reg_offset) { return readl(pmc->regmap + reg_offset); @@ -169,6 +190,32 @@ static const struct file_operations pmc_dev_state_ops = { .release = single_release, }; +static int pmc_pss_state_show(struct seq_file *s, void *unused) +{ + struct pmc_dev *pmc = s->private; + u32 pss = pmc_reg_read(pmc, PMC_PSS); + int pss_index; + + for (pss_index = 0; pss_index < ARRAY_SIZE(pss_map); pss_index++) { + seq_printf(s, "Island: %-32s\tState: %s\n", + pss_map[pss_index].name, + pss_map[pss_index].bit_mask & pss ? "Off" : "On"); + } + return 0; +} + +static int pmc_pss_state_open(struct inode *inode, struct file *file) +{ + return single_open(file, pmc_pss_state_show, inode->i_private); +} + +static const struct file_operations pmc_pss_state_ops = { + .open = pmc_pss_state_open, + .read = seq_read, + .llseek = seq_lseek, + .release = single_release, +}; + static int pmc_sleep_tmr_show(struct seq_file *s, void *unused) { struct pmc_dev *pmc = s->private; @@ -216,9 +263,17 @@ static int pmc_dbgfs_register(struct pmc_dev *pmc, struct pci_dev *pdev) f = debugfs_create_file("dev_state", S_IFREG | S_IRUGO, dir, pmc, &pmc_dev_state_ops); if (!f) { - dev_err(&pdev->dev, "dev_states register failed\n"); + dev_err(&pdev->dev, "dev_state register failed\n"); goto err; } + + f = debugfs_create_file("pss_state", S_IFREG | S_IRUGO, + dir, pmc, &pmc_pss_state_ops); + if (!f) { + dev_err(&pdev->dev, "pss_state register failed\n"); + goto err; + } + f = debugfs_create_file("sleep_state", S_IFREG | S_IRUGO, dir, pmc, &pmc_sleep_tmr_ops); if (!f) { -- 2.1.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-11-28 9:49 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-11-12 17:18 [PATCH v4 0/3] x86: clean ups and feature enhancement in pmc_atom Andy Shevchenko 2014-11-12 17:18 ` [PATCH v4 1/3] x86: pmc_atom: clean up init function Andy Shevchenko 2014-11-12 17:18 ` [PATCH v4 2/3] x86: pmc_atom: don't check for NULL twice Andy Shevchenko 2014-11-19 11:36 ` Thomas Gleixner 2014-11-28 9:49 ` Andy Shevchenko 2014-11-12 17:18 ` [PATCH v4 3/3] x86: pmc_atom: expose contents of PSS Andy Shevchenko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).