platform/x86: intel_ips: Simplify error handling via devres API

PATCH New Andy Shevchenko 1 participant 6 messages Oct 5, 2017 x86 platform drivers

lore

Share & tools

lore

Andy Shevchenko

author

2017-10-05 11:54 UTC #0 lore raw

PATCH 1/6 platform/x86: intel_ips: Simplify error handling via devres API

Use devm_ and pcim_ functions to make error handling simpler and code smaller and tidier.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/platform/x86/intel_ips.c | 62 ++++++++++------------------------------
 1 file changed, 15 insertions(+), 47 deletions(-)

drivers/platform/x86/intel_ips.c +15 ~~-47~~

index 58dcee562d64..063d9a1624b4 100644
@@ -296,7 +296,7 @@ static struct ips_mcp_limits ips_ulv_limits = {
 
 struct ips_driver {
 	struct pci_dev *dev;
-	void *regmap;
+	void __iomem *regmap;
 	struct task_struct *monitor;
 	struct task_struct *adjust;
 	struct dentry *debug_root;

@@ -1517,62 +1517,45 @@ static int ips_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	if (dmi_check_system(ips_blacklist))
 		return -ENODEV;
 
-	ips = kzalloc(sizeof(struct ips_driver), GFP_KERNEL);
+	ips = devm_kzalloc(&dev->dev, sizeof(*ips), GFP_KERNEL);
 	if (!ips)
 		return -ENOMEM;
 
-	pci_set_drvdata(dev, ips);
+	spin_lock_init(&ips->turbo_status_lock);
 	ips->dev = dev;
 
 	ips->limits = ips_detect_cpu(ips);
 	if (!ips->limits) {
 		dev_info(&dev->dev, "IPS not supported on this CPU\n");
-		ret = -ENXIO;
-		goto error_free;
+		return -ENXIO;
 	}
 
-	spin_lock_init(&ips->turbo_status_lock);
-
-	ret = pci_enable_device(dev);
+	ret = pcim_enable_device(dev);
 	if (ret) {
 		dev_err(&dev->dev, "can't enable PCI device, aborting\n");
-		goto error_free;
+		return ret;
 	}
 
-	if (!pci_resource_start(dev, 0)) {
-		dev_err(&dev->dev, "TBAR not assigned, aborting\n");
-		ret = -ENXIO;
-		goto error_free;
-	}
-
-	ret = pci_request_regions(dev, "ips thermal sensor");
+	ret = pcim_iomap_regions(dev, 1 << 0, pci_name(dev));
 	if (ret) {
-		dev_err(&dev->dev, "thermal resource busy, aborting\n");
-		goto error_free;
-	}
-
-
-	ips->regmap = ioremap(pci_resource_start(dev, 0),
-			      pci_resource_len(dev, 0));
-	if (!ips->regmap) {
 		dev_err(&dev->dev, "failed to map thermal regs, aborting\n");
-		ret = -EBUSY;
-		goto error_release;
+		return ret;
 	}
+	ips->regmap = pcim_iomap_table(dev)[0];
+
+	pci_set_drvdata(dev, ips);
 
 	tse = thm_readb(THM_TSE);
 	if (tse != TSE_EN) {
 		dev_err(&dev->dev, "thermal device not enabled (0x%02x), aborting\n", tse);
-		ret = -ENXIO;
-		goto error_unmap;
+		return -ENXIO;
 	}
 
 	trc = thm_readw(THM_TRC);
 	trc_required_mask = TRC_CORE1_EN | TRC_CORE_PWR | TRC_MCH_EN;
 	if ((trc & trc_required_mask) != trc_required_mask) {
 		dev_err(&dev->dev, "thermal reporting for required devices not enabled, aborting\n");
-		ret = -ENXIO;
-		goto error_unmap;
+		return -ENXIO;
 	}
 
 	if (trc & TRC_CORE2_EN)

@@ -1602,8 +1585,7 @@ static int ips_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	rdmsrl(PLATFORM_INFO, platform_info);
 	if (!(platform_info & PLATFORM_TDP)) {
 		dev_err(&dev->dev, "platform indicates TDP override unavailable, aborting\n");
-		ret = -ENODEV;
-		goto error_unmap;
+		return -ENODEV;
 	}
 
 	/*

@@ -1615,7 +1597,7 @@ static int ips_probe(struct pci_dev *dev, const struct pci_device_id *id)
 			  ips);
 	if (ret) {
 		dev_err(&dev->dev, "request irq failed, aborting\n");
-		goto error_unmap;
+		return ret;
 	}
 
 	/* Enable aux, hot & critical interrupts */

@@ -1673,12 +1655,6 @@ static int ips_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	kthread_stop(ips->adjust);
 error_free_irq:
 	free_irq(ips->dev->irq, ips);
-error_unmap:
-	iounmap(ips->regmap);
-error_release:
-	pci_release_regions(dev);
-error_free:
-	kfree(ips);
 	return ret;
 }

@@ -1714,22 +1690,14 @@ static void ips_remove(struct pci_dev *dev)
 		kthread_stop(ips->adjust);
 	if (ips->monitor)
 		kthread_stop(ips->monitor);
-	iounmap(ips->regmap);
-	pci_release_regions(dev);
-	kfree(ips);
 	dev_dbg(&dev->dev, "IPS driver removed\n");
 }
 
-static void ips_shutdown(struct pci_dev *dev)
-{
-}
-
 static struct pci_driver ips_pci_driver = {
 	.name = "intel ips",
 	.id_table = ips_id_table,
 	.probe = ips_probe,
 	.remove = ips_remove,
-	.shutdown = ips_shutdown,
 };
 
 module_pci_driver(ips_pci_driver);

Signature · 1 line

2.14.2

Andy Shevchenko

author

2017-10-05 11:54 UTC #1 lore raw

PATCH 2/6 platform/x86: intel_ips: Switch to new PCI IRQ allocation API

This makes code cleaner.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/platform/x86/intel_ips.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

drivers/platform/x86/intel_ips.c +13 -5

index 063d9a1624b4..6f6900065bc2 100644
@@ -297,6 +297,8 @@ static struct ips_mcp_limits ips_ulv_limits = {
 struct ips_driver {
 	struct pci_dev *dev;
 	void __iomem *regmap;
+	int irq;
+
 	struct task_struct *monitor;
 	struct task_struct *adjust;
 	struct dentry *debug_root;

@@ -1592,9 +1594,13 @@ static int ips_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	 * IRQ handler for ME interaction
 	 * Note: don't use MSI here as the PCH has bugs.
 	 */
-	pci_disable_msi(dev);
-	ret = request_irq(dev->irq, ips_irq_handler, IRQF_SHARED, "ips",
-			  ips);
+	ret = pci_alloc_irq_vectors(dev, 1, 1, PCI_IRQ_LEGACY);
+	if (ret < 0)
+		return ret;
+
+	ips->irq = pci_irq_vector(dev, 0);
+
+	ret = request_irq(ips->irq, ips_irq_handler, IRQF_SHARED, "ips", ips);
 	if (ret) {
 		dev_err(&dev->dev, "request irq failed, aborting\n");
 		return ret;

@@ -1654,7 +1660,8 @@ static int ips_probe(struct pci_dev *dev, const struct pci_device_id *id)
 error_thread_cleanup:
 	kthread_stop(ips->adjust);
 error_free_irq:
-	free_irq(ips->dev->irq, ips);
+	free_irq(ips->irq, ips);
+	pci_free_irq_vectors(dev);
 	return ret;
 }

@@ -1685,7 +1692,8 @@ static void ips_remove(struct pci_dev *dev)
 	wrmsrl(TURBO_POWER_CURRENT_LIMIT, turbo_override);
 	wrmsrl(TURBO_POWER_CURRENT_LIMIT, ips->orig_turbo_limit);
 
-	free_irq(ips->dev->irq, ips);
+	free_irq(ips->irq, ips);
+	pci_free_irq_vectors(dev);
 	if (ips->adjust)
 		kthread_stop(ips->adjust);
 	if (ips->monitor)

Signature · 1 line

2.14.2

Andy Shevchenko

author

2017-10-05 11:54 UTC #2 lore raw

PATCH 3/6 platform/x86: intel_ips: Use PCI_VDEVICE() macro

In reply to Andy Shevchenko

Intel vendor ID is defined globally, thus we may use PCI_VDEVICE().

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/platform/x86/intel_ips.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

drivers/platform/x86/intel_ips.c +1 -2

index 6f6900065bc2..489ff6fff9f9 100644
@@ -1482,8 +1482,7 @@ ips_link_to_i915_driver(void)
 EXPORT_SYMBOL_GPL(ips_link_to_i915_driver);
 
 static const struct pci_device_id ips_id_table[] = {
-	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL,
-		     PCI_DEVICE_ID_INTEL_THERMAL_SENSOR), },
+	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_THERMAL_SENSOR), },
 	{ 0, }
 };

Signature · 1 line

2.14.2

Andy Shevchenko

author

2017-10-05 11:54 UTC #3 lore raw

PATCH 4/6 platform/x86: intel_ips: Keep pointer to struct device

In reply to Andy Shevchenko

...instead of keeping pointer to struct pci_dev.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/platform/x86/intel_ips.c | 51 ++++++++++++++++++++--------------------
 1 file changed, 25 insertions(+), 26 deletions(-)

drivers/platform/x86/intel_ips.c +25 ~~-26~~

index 489ff6fff9f9..27e70637e40b 100644
@@ -295,7 +295,7 @@ static struct ips_mcp_limits ips_ulv_limits = {
 };
 
 struct ips_driver {
-	struct pci_dev *dev;
+	struct device *dev;
 	void __iomem *regmap;
 	int irq;

@@ -596,7 +596,7 @@ static void ips_disable_gpu_turbo(struct ips_driver *ips)
 		return;
 
 	if (!ips->gpu_turbo_disable())
-		dev_err(&ips->dev->dev, "failed to disable graphics turbo\n");
+		dev_err(ips->dev, "failed to disable graphics turbo\n");
 	else
 		ips->__gpu_turbo_on = false;
 }

@@ -651,8 +651,7 @@ static bool cpu_exceeded(struct ips_driver *ips, int cpu)
 	spin_unlock_irqrestore(&ips->turbo_status_lock, flags);
 
 	if (ret)
-		dev_info(&ips->dev->dev,
-			 "CPU power or thermal limit exceeded\n");
+		dev_info(ips->dev, "CPU power or thermal limit exceeded\n");
 
 	return ret;
 }

@@ -771,7 +770,7 @@ static int ips_adjust(void *data)
 	struct ips_driver *ips = data;
 	unsigned long flags;
 
-	dev_dbg(&ips->dev->dev, "starting ips-adjust thread\n");
+	dev_dbg(ips->dev, "starting ips-adjust thread\n");
 
 	/*
 	 * Adjust CPU and GPU clamps every 5s if needed.  Doing it more

@@ -818,7 +817,7 @@ static int ips_adjust(void *data)
 		schedule_timeout_interruptible(msecs_to_jiffies(IPS_ADJUST_PERIOD));
 	} while (!kthread_should_stop());
 
-	dev_dbg(&ips->dev->dev, "ips-adjust thread stopped\n");
+	dev_dbg(ips->dev, "ips-adjust thread stopped\n");
 
 	return 0;
 }

@@ -978,7 +977,7 @@ static int ips_monitor(void *data)
 	mchp_samples = kzalloc(sizeof(u32) * IPS_SAMPLE_COUNT, GFP_KERNEL);
 	if (!mcp_samples || !ctv1_samples || !ctv2_samples || !mch_samples ||
 			!cpu_samples || !mchp_samples) {
-		dev_err(&ips->dev->dev,
+		dev_err(ips->dev,
 			"failed to allocate sample array, ips disabled\n");
 		kfree(mcp_samples);
 		kfree(ctv1_samples);

@@ -1099,7 +1098,8 @@ static int ips_monitor(void *data)
 			ITV_ME_SEQNO_SHIFT;
 		if (cur_seqno == last_seqno &&
 		    time_after(jiffies, seqno_timestamp + HZ)) {
-			dev_warn(&ips->dev->dev, "ME failed to update for more than 1s, likely hung\n");
+			dev_warn(ips->dev,
+				 "ME failed to update for more than 1s, likely hung\n");
 		} else {
 			seqno_timestamp = get_jiffies_64();
 			last_seqno = cur_seqno;

@@ -1121,7 +1121,7 @@ static int ips_monitor(void *data)
 	del_timer_sync(&timer);
 	destroy_timer_on_stack(&timer);
 
-	dev_dbg(&ips->dev->dev, "ips-monitor thread stopped\n");
+	dev_dbg(ips->dev, "ips-monitor thread stopped\n");
 
 	return 0;
 }

@@ -1130,17 +1130,17 @@ static int ips_monitor(void *data)
 #define THM_DUMPW(reg) \
 	{ \
 	u16 val = thm_readw(reg); \
-	dev_dbg(&ips->dev->dev, #reg ": 0x%04x\n", val); \
+	dev_dbg(ips->dev, #reg ": 0x%04x\n", val); \
 	}
 #define THM_DUMPL(reg) \
 	{ \
 	u32 val = thm_readl(reg); \
-	dev_dbg(&ips->dev->dev, #reg ": 0x%08x\n", val); \
+	dev_dbg(ips->dev, #reg ": 0x%08x\n", val); \
 	}
 #define THM_DUMPQ(reg) \
 	{ \
 	u64 val = thm_readq(reg); \
-	dev_dbg(&ips->dev->dev, #reg ": 0x%016x\n", val); \
+	dev_dbg(ips->dev, #reg ": 0x%016x\n", val); \
 	}
 
 static void dump_thermal_info(struct ips_driver *ips)

@@ -1148,7 +1148,7 @@ static void dump_thermal_info(struct ips_driver *ips)
 	u16 ptl;
 
 	ptl = thm_readw(THM_PTL);
-	dev_dbg(&ips->dev->dev, "Processor temp limit: %d\n", ptl);
+	dev_dbg(ips->dev, "Processor temp limit: %d\n", ptl);
 
 	THM_DUMPW(THM_CTA);
 	THM_DUMPW(THM_TRC);

@@ -1177,8 +1177,8 @@ static irqreturn_t ips_irq_handler(int irq, void *arg)
 	if (!tses && !tes)
 		return IRQ_NONE;
 
-	dev_info(&ips->dev->dev, "TSES: 0x%02x\n", tses);
-	dev_info(&ips->dev->dev, "TES: 0x%02x\n", tes);
+	dev_info(ips->dev, "TSES: 0x%02x\n", tses);
+	dev_info(ips->dev, "TES: 0x%02x\n", tes);
 
 	/* STS update from EC? */
 	if (tes & 1) {

@@ -1216,8 +1216,8 @@ static irqreturn_t ips_irq_handler(int irq, void *arg)
 
 	/* Thermal trip */
 	if (tses) {
-		dev_warn(&ips->dev->dev,
-			 "thermal trip occurred, tses: 0x%04x\n", tses);
+		dev_warn(ips->dev, "thermal trip occurred, tses: 0x%04x\n",
+			 tses);
 		thm_writeb(THM_TSES, tses);
 	}

@@ -1332,8 +1332,7 @@ static void ips_debugfs_init(struct ips_driver *ips)
 
 	ips->debug_root = debugfs_create_dir("ips", NULL);
 	if (!ips->debug_root) {
-		dev_err(&ips->dev->dev,
-			"failed to create debugfs entries: %ld\n",
+		dev_err(ips->dev, "failed to create debugfs entries: %ld\n",
 			PTR_ERR(ips->debug_root));
 		return;
 	}

@@ -1347,8 +1346,7 @@ static void ips_debugfs_init(struct ips_driver *ips)
 					  ips->debug_root, node,
 					  &ips_debugfs_ops);
 		if (!ent) {
-			dev_err(&ips->dev->dev,
-				"failed to create debug file: %ld\n",
+			dev_err(ips->dev, "failed to create debug file: %ld\n",
 				PTR_ERR(ent));
 			goto err_cleanup;
 		}

@@ -1375,7 +1373,7 @@ static struct ips_mcp_limits *ips_detect_cpu(struct ips_driver *ips)
 	u16 tdp;
 
 	if (!(boot_cpu_data.x86 == 6 && boot_cpu_data.x86_model == 37)) {
-		dev_info(&ips->dev->dev, "Non-IPS CPU detected.\n");
+		dev_info(ips->dev, "Non-IPS CPU detected.\n");
 		goto out;
 	}

@@ -1397,7 +1395,7 @@ static struct ips_mcp_limits *ips_detect_cpu(struct ips_driver *ips)
 	else if (strstr(boot_cpu_data.x86_model_id, "CPU       U"))
 		limits = &ips_ulv_limits;
 	else {
-		dev_info(&ips->dev->dev, "No CPUID match found.\n");
+		dev_info(ips->dev, "No CPUID match found.\n");
 		goto out;
 	}

@@ -1406,7 +1404,8 @@ static struct ips_mcp_limits *ips_detect_cpu(struct ips_driver *ips)
 
 	/* Sanity check TDP against CPU */
 	if (limits->core_power_limit != (tdp / 8) * 1000) {
-		dev_info(&ips->dev->dev, "CPU TDP doesn't match expected value (found %d, expected %d)\n",
+		dev_info(ips->dev,
+			 "CPU TDP doesn't match expected value (found %d, expected %d)\n",
 			 tdp / 8, limits->core_power_limit / 1000);
 		limits->core_power_limit = (tdp / 8) * 1000;
 	}

@@ -1461,7 +1460,7 @@ ips_gpu_turbo_enabled(struct ips_driver *ips)
 {
 	if (!ips->gpu_busy && late_i915_load) {
 		if (ips_get_i915_syms(ips)) {
-			dev_info(&ips->dev->dev,
+			dev_info(ips->dev,
 				 "i915 driver attached, reenabling gpu turbo\n");
 			ips->gpu_turbo_enabled = !(thm_readl(THM_HTS) & HTS_GTD_DIS);
 		}

@@ -1523,7 +1522,7 @@ static int ips_probe(struct pci_dev *dev, const struct pci_device_id *id)
 		return -ENOMEM;
 
 	spin_lock_init(&ips->turbo_status_lock);
-	ips->dev = dev;
+	ips->dev = &dev->dev;
 
 	ips->limits = ips_detect_cpu(ips);
 	if (!ips->limits) {

Signature · 1 line

2.14.2

Andy Shevchenko

author

2017-10-05 11:54 UTC #4 lore raw

PATCH 5/6 platform/x86: intel_ips: Remove unneeded fields and label

In reply to Andy Shevchenko

There are fields in the struct ips_mcp_limits which are not used anywhere and a label which we may get rid of.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/platform/x86/intel_ips.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

drivers/platform/x86/intel_ips.c +2 -5

index 27e70637e40b..9f5afdd123bb 100644
@@ -259,8 +259,6 @@ static const int IPS_SAMPLE_WINDOW = 5000; /* 5s moving window of samples */
 
 /* Per-SKU limits */
 struct ips_mcp_limits {
-	int cpu_family;
-	int cpu_model; /* includes extended model... */
 	int mcp_power_limit; /* mW units */
 	int core_power_limit;
 	int mch_power_limit;

@@ -1374,7 +1372,7 @@ static struct ips_mcp_limits *ips_detect_cpu(struct ips_driver *ips)
 
 	if (!(boot_cpu_data.x86 == 6 && boot_cpu_data.x86_model == 37)) {
 		dev_info(ips->dev, "Non-IPS CPU detected.\n");
-		goto out;
+		return NULL;
 	}
 
 	rdmsrl(IA32_MISC_ENABLE, misc_en);

@@ -1396,7 +1394,7 @@ static struct ips_mcp_limits *ips_detect_cpu(struct ips_driver *ips)
 		limits = &ips_ulv_limits;
 	else {
 		dev_info(ips->dev, "No CPUID match found.\n");
-		goto out;
+		return NULL;
 	}
 
 	rdmsrl(TURBO_POWER_CURRENT_LIMIT, turbo_power);

@@ -1410,7 +1408,6 @@ static struct ips_mcp_limits *ips_detect_cpu(struct ips_driver *ips)
 		limits->core_power_limit = (tdp / 8) * 1000;
 	}
 
-out:
 	return limits;
 }

Signature · 1 line

2.14.2

Andy Shevchenko

author

2017-10-05 11:54 UTC #5 lore raw

PATCH 6/6 platform/x86: intel_ips: Remove FSF address from GPL notice

In reply to Andy Shevchenko

This patch removes the FSF address from the GPL notice to fix a checkpatch.pl CHECK message.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/platform/x86/intel_ips.c | 4 ----
 drivers/platform/x86/intel_ips.h | 4 ----
 2 files changed, 8 deletions(-)

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

index 9f5afdd123bb..680ab4fd7087 100644
@@ -10,10 +10,6 @@
  * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
  * more details.
  *
- * You should have received a copy of the GNU General Public License along with
- * this program; if not, write to the Free Software Foundation, Inc.,
- * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
- *
  * The full GNU General Public License is included in this distribution in
  * the file called "COPYING".
  *

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

index 73299beff5b3..60f4e3ddbe9f 100644
@@ -10,10 +10,6 @@
  * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
  * more details.
  *
- * You should have received a copy of the GNU General Public License along with
- * this program; if not, write to the Free Software Foundation, Inc.,
- * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
- *
  * The full GNU General Public License is included in this distribution in
  * the file called "COPYING".
  */

Signature · 1 line

2.14.2