Skip to contentSkip to conversation

x86: clean ups and feature enhancement in pmc_atom

PATCH New Andy Shevchenko 2 participants 11 messages Jan 14, 2015 – Jan 20, 2015

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

v5 cover x86: clean ups and feature enhancement in pmc_atom

Couple of clean ups and one feature enhancement (expose PSS register in the debug fs).

Changes since v4:
- address one Thomas' comment (patch 1/5 was added)
- rebased on recent linux-next

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 (5):
x86: pmc-atom: assign debugfs node as soon as possible
x86: pmc_atom: don't check for NULL twice
x86: pmc-atom: remove unused macro
x86: pmc_atom: clean up init function
x86: pmc_atom: expose contents of PSS

 arch/x86/include/asm/pmc_atom.h | 22 +++++++++++
 arch/x86/kernel/pmc_atom.c      | 81 ++++++++++++++++++++++++++++++++---------
 2 files changed, 86 insertions(+), 17 deletions(-)
Signature · 1 line
2.1.4
#1 lore raw

PATCH v5 1/5 x86: pmc-atom: assign debugfs node as soon as possible

pmc_dbgfs_unregister() will be called when pmc->dbgfs_dir is unconditionally NULL on error path in pmc_dbgfs_register(). To prevent this we move an assignment to where is should be.

Fixes f855911c1f48 (x86/pmc_atom: Expose PMC device state and platform sleep state)
---
 arch/x86/kernel/pmc_atom.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
arch/x86/kernel/pmc_atom.c +3 -1
index 0ee5025e..8bb9a61 100644
@@ -217,6 +217,8 @@ static int pmc_dbgfs_register(struct pmc_dev *pmc, struct pci_dev *pdev)
 	if (!dir)
 		return -ENOMEM;
 
+	pmc->dbgfs_dir = dir;
+
 	f = debugfs_create_file("dev_state", S_IFREG | S_IRUGO,
 				dir, pmc, &pmc_dev_state_ops);
 	if (!f) {
@@ -229,7 +231,7 @@ static int pmc_dbgfs_register(struct pmc_dev *pmc, struct pci_dev *pdev)
 		dev_err(&pdev->dev, "sleep_state register failed\n");
 		goto err;
 	}
-	pmc->dbgfs_dir = dir;
+
 	return 0;
 err:
 	pmc_dbgfs_unregister(pmc);
Signature · 1 line
2.1.4
#2 lore raw

[tip:x86/platform] x86: pmc-atom: Assign debugfs node as soon as possible

x86: pmc-atom: Assign debugfs node as soon as possible
Commit-ID:  1b43d7125f3b6f7d46e72da64f65f3187a83b66b
Gitweb:     http://git.kernel.org/tip/1b43d7125f3b6f7d46e72da64f65f3187a83b66b
Author:     Andy Shevchenko <[email protected]>
AuthorDate: Wed, 14 Jan 2015 18:39:31 +0200
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Tue, 20 Jan 2015 12:50:14 +0100

x86: pmc-atom: Assign debugfs node as soon as possible

pmc_dbgfs_unregister() will be called when pmc->dbgfs_dir is unconditionally NULL on error path in pmc_dbgfs_register(). To prevent this we move the assignment to where is should be.

Fixes f855911c1f48 (x86/pmc_atom: Expose PMC device state and platform sleep state)
Cc Aubrey Li <[email protected]>
Cc Rafael J. Wysocki <[email protected]>
Cc Kumar P. Mahesh <[email protected]>
---
 arch/x86/kernel/pmc_atom.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
arch/x86/kernel/pmc_atom.c +3 -1
index 0ee5025e..8bb9a61 100644
@@ -217,6 +217,8 @@ static int pmc_dbgfs_register(struct pmc_dev *pmc, struct pci_dev *pdev)
 	if (!dir)
 		return -ENOMEM;
 
+	pmc->dbgfs_dir = dir;
+
 	f = debugfs_create_file("dev_state", S_IFREG | S_IRUGO,
 				dir, pmc, &pmc_dev_state_ops);
 	if (!f) {
@@ -229,7 +231,7 @@ static int pmc_dbgfs_register(struct pmc_dev *pmc, struct pci_dev *pdev)
 		dev_err(&pdev->dev, "sleep_state register failed\n");
 		goto err;
 	}
-	pmc->dbgfs_dir = dir;
+
 	return 0;
 err:
 	pmc_dbgfs_unregister(pmc);
#3 lore raw

PATCH v5 2/5 x86: pmc_atom: don't check for NULL twice

In reply to 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.

---
 arch/x86/kernel/pmc_atom.c | 4 ----
 1 file changed, 4 deletions(-)
arch/x86/kernel/pmc_atom.c +0 -4
index 8bb9a61..1dd6c62 100644
@@ -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)
Signature · 1 line
2.1.4
#4 lore raw

[tip:x86/platform] x86: pmc_atom: don%27t check for NULL twice

x86: pmc_atom: don%27t check for NULL twice
Commit-ID:  d5df8fe34bec4539e259525feabd16efccf16750
Gitweb:     http://git.kernel.org/tip/d5df8fe34bec4539e259525feabd16efccf16750
Author:     Andy Shevchenko <[email protected]>
AuthorDate: Wed, 14 Jan 2015 18:39:32 +0200
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Tue, 20 Jan 2015 12:50:14 +0100

x86: pmc_atom: don%27t check for NULL twice

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.

---
 arch/x86/kernel/pmc_atom.c | 4 ----
 1 file changed, 4 deletions(-)
arch/x86/kernel/pmc_atom.c +0 -4
index 8bb9a61..1dd6c62 100644
@@ -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)
#5 lore raw

PATCH v5 3/5 x86: pmc-atom: remove unused macro

In reply to Andy Shevchenko

DRIVER_NAME seems unused. This patch just removes it. There is no functional change.

---
 arch/x86/kernel/pmc_atom.c | 2 --
 1 file changed, 2 deletions(-)
arch/x86/kernel/pmc_atom.c +0 -2
index 1dd6c62..6b62f55 100644
@@ -25,8 +25,6 @@
 
 #include <asm/pmc_atom.h>
 
-#define	DRIVER_NAME	KBUILD_MODNAME
-
 struct pmc_dev {
 	u32 base_addr;
 	void __iomem *regmap;
Signature · 1 line
2.1.4
#6 lore raw

[tip:x86/platform] x86: pmc-atom: Remove unused macro

x86: pmc-atom: Remove unused macro
Commit-ID:  4922b9ce89ccdeb99364666f1fcc297c4af31bf7
Gitweb:     http://git.kernel.org/tip/4922b9ce89ccdeb99364666f1fcc297c4af31bf7
Author:     Andy Shevchenko <[email protected]>
AuthorDate: Wed, 14 Jan 2015 18:39:33 +0200
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Tue, 20 Jan 2015 12:50:14 +0100

x86: pmc-atom: Remove unused macro

DRIVER_NAME seems unused. This patch just removes it. There is no functional change.

---
 arch/x86/kernel/pmc_atom.c | 2 --
 1 file changed, 2 deletions(-)
arch/x86/kernel/pmc_atom.c +0 -2
index 1dd6c62..6b62f55 100644
@@ -25,8 +25,6 @@
 
 #include <asm/pmc_atom.h>
 
-#define	DRIVER_NAME	KBUILD_MODNAME
-
 struct pmc_dev {
 	u32 base_addr;
 	void __iomem *regmap;
#7 lore raw

PATCH v5 4/5 x86: pmc_atom: clean up init function

In reply to Andy Shevchenko

There is no need to use err variable.

---
 arch/x86/kernel/pmc_atom.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)
arch/x86/kernel/pmc_atom.c +3 -7
index 6b62f55..f8becc4 100644
@@ -288,7 +288,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;
 
@@ -302,14 +301,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);
Signature · 1 line
2.1.4
#8 lore raw

[tip:x86/platform] x86: pmc_atom: Clean up init function

x86: pmc_atom: Clean up init function
Commit-ID:  4b25f42a371b16807f0966490f8faad9abc712d9
Gitweb:     http://git.kernel.org/tip/4b25f42a371b16807f0966490f8faad9abc712d9
Author:     Andy Shevchenko <[email protected]>
AuthorDate: Wed, 14 Jan 2015 18:39:34 +0200
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Tue, 20 Jan 2015 12:50:14 +0100

x86: pmc_atom: Clean up init function

There is no need to use err variable.

---
 arch/x86/kernel/pmc_atom.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)
arch/x86/kernel/pmc_atom.c +3 -7
index 6b62f55..f8becc4 100644
@@ -288,7 +288,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;
 
@@ -302,14 +301,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);
#9 lore raw

PATCH v5 5/5 x86: pmc_atom: expose contents of PSS

In reply to 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.

---
 arch/x86/include/asm/pmc_atom.h | 22 +++++++++++++++
 arch/x86/kernel/pmc_atom.c      | 61 +++++++++++++++++++++++++++++++++++++++--
 2 files changed, 80 insertions(+), 3 deletions(-)
arch/x86/include/asm/pmc_atom.h +22 -0
index fc7a17c..bc0fc08 100644
@@ -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
 
arch/x86/kernel/pmc_atom.c +58 -3

91 lines not on this page · raw message

Signature · 1 line
2.1.4
#10 lore raw

[tip:x86/platform] x86: pmc_atom: Expose contents of PSS

x86: pmc_atom: Expose contents of PSS
Commit-ID:  0e1540208ef34a2246822fa56f751efe23748e7a
Gitweb:     http://git.kernel.org/tip/0e1540208ef34a2246822fa56f751efe23748e7a
Author:     Andy Shevchenko <[email protected]>
AuthorDate: Wed, 14 Jan 2015 18:39:35 +0200
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Tue, 20 Jan 2015 12:50:14 +0100

x86: pmc_atom: Expose contents of PSS

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.

---
 arch/x86/include/asm/pmc_atom.h | 22 +++++++++++++++
 arch/x86/kernel/pmc_atom.c      | 61 +++++++++++++++++++++++++++++++++++++++--
 2 files changed, 80 insertions(+), 3 deletions(-)
arch/x86/include/asm/pmc_atom.h +22 -0
index fc7a17c..bc0fc08 100644
@@ -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
 
arch/x86/kernel/pmc_atom.c +58 -3

91 lines not on this page · raw message