Skip to contentSkip to conversation

i2c: i801: Remove i801_driver forward declaration

PATCH In review Jarkko Nikula 2 participants 5 messages Feb 6, 2015 3 messages not in the archive

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

PATCH 1/2 i2c: i801: Remove i801_driver forward declaration

struct pci_driver i801_driver forward declaration is needed only for accessing the name field. Remove it and use dev_driver_string() instead.

---
 drivers/i2c/busses/i2c-i801.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/i2c/busses/i2c-i801.c +2 -4
index 8fafb254e42a..e31aa987b281 100644
@@ -223,8 +223,6 @@ struct i801_priv {
 #endif
 };
 
-static struct pci_driver i801_driver;
-
 #define FEATURE_SMBUS_PEC	(1 << 0)
 #define FEATURE_BLOCK_BUFFER	(1 << 1)
 #define FEATURE_BLOCK_PROC	(1 << 2)
@@ -1204,7 +1202,7 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
 		goto exit;
 	}
 
-	err = pci_request_region(dev, SMBBAR, i801_driver.name);
+	err = pci_request_region(dev, SMBBAR, dev_driver_string(&dev->dev));
 	if (err) {
 		dev_err(&dev->dev, "Failed to request SMBus region "
 			"0x%lx-0x%Lx\n", priv->smba,
@@ -1255,7 +1253,7 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
 		init_waitqueue_head(&priv->waitq);
 
 		err = request_irq(dev->irq, i801_isr, IRQF_SHARED,
-				  i801_driver.name, priv);
+				  dev_driver_string(&dev->dev), priv);
 		if (err) {
 			dev_err(&dev->dev, "Failed to allocate irq %d: %d\n",
 				dev->irq, err);
Signature · 1 line
2.1.4

A message that is not in the archive (sent off-list or not yet received): 1423231013-29514-1-git-send-email-jarkko.nikula-VuQAYsv1563Yd54FQh9/[email protected]

#2 lore raw

PATCH 2/2 i2c: i801: Use managed devm_* memory and irq allocation

This simplifies the error and remove paths.

---
 drivers/i2c/busses/i2c-i801.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)
drivers/i2c/busses/i2c-i801.c +6 -11
index e31aa987b281..b4369d1835ca 100644
@@ -1138,7 +1138,7 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	int err, i;
 	struct i801_priv *priv;
 
-	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	priv = devm_kzalloc(&dev->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
@@ -1252,8 +1252,9 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	if (priv->features & FEATURE_IRQ) {
 		init_waitqueue_head(&priv->waitq);
 
-		err = request_irq(dev->irq, i801_isr, IRQF_SHARED,
-				  dev_driver_string(&dev->dev), priv);
+		err = devm_request_irq(&dev->dev, dev->irq, i801_isr,
+				       IRQF_SHARED,
+				       dev_driver_string(&dev->dev), priv);
 		if (err) {
 			dev_err(&dev->dev, "Failed to allocate irq %d: %d\n",
 				dev->irq, err);
@@ -1274,7 +1275,7 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	err = i2c_add_adapter(&priv->adapter);
 	if (err) {
 		dev_err(&dev->dev, "Failed to add SMBus adapter\n");
-		goto exit_free_irq;
+		goto exit_release;
 	}
 
 	i801_probe_optional_slaves(priv);
@@ -1285,12 +1286,9 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
 
 	return 0;
 
-exit_free_irq:
-	if (priv->features & FEATURE_IRQ)
-		free_irq(dev->irq, priv);
+exit_release:
 	pci_release_region(dev, SMBBAR);
 exit:
-	kfree(priv);
 	return err;
 }
 
@@ -1302,11 +1300,8 @@ static void i801_remove(struct pci_dev *dev)
 	i2c_del_adapter(&priv->adapter);
 	pci_write_config_byte(dev, SMBHSTCFG, priv->original_hstcfg);
 
-	if (priv->features & FEATURE_IRQ)
-		free_irq(dev->irq, priv);
 	pci_release_region(dev, SMBBAR);
 
-	kfree(priv);
 	/*
 	 * do not call pci_disable_device(dev) since it can cause hard hangs on
 	 * some systems during power-off (eg. Fujitsu-Siemens Lifebook E8010)
Signature · 1 line
2.1.4

A message that is not in the archive (sent off-list or not yet received): 1423231013-29514-2-git-send-email-jarkko.nikula-VuQAYsv1563Yd54FQh9/[email protected]

#4 lore raw

Re: [PATCH 2/2] i2c: i801: Use managed devm_* memory and irq allocation

Hi Jarkko,

Quoted 57 lines · Jarkko Nikula

This simplifies the error and remove paths.

---
 drivers/i2c/busses/i2c-i801.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)
drivers/i2c/busses/i2c-i801.c +6 -8
index e31aa987b281..b4369d1835ca 100644
@@ -1138,7 +1138,7 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	int err, i;
 	struct i801_priv *priv;
 
-	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	priv = devm_kzalloc(&dev->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
@@ -1252,8 +1252,9 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	if (priv->features & FEATURE_IRQ) {
 		init_waitqueue_head(&priv->waitq);
 
-		err = request_irq(dev->irq, i801_isr, IRQF_SHARED,
-				  dev_driver_string(&dev->dev), priv);
+		err = devm_request_irq(&dev->dev, dev->irq, i801_isr,
+				       IRQF_SHARED,
+				       dev_driver_string(&dev->dev), priv);
 		if (err) {
 			dev_err(&dev->dev, "Failed to allocate irq %d: %d\n",
 				dev->irq, err);
@@ -1274,7 +1275,7 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	err = i2c_add_adapter(&priv->adapter);
 	if (err) {
 		dev_err(&dev->dev, "Failed to add SMBus adapter\n");
-		goto exit_free_irq;
+		goto exit_release;
 	}
 
 	i801_probe_optional_slaves(priv);
@@ -1285,12 +1286,9 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
 
 	return 0;
 
-exit_free_irq:
-	if (priv->features & FEATURE_IRQ)
-		free_irq(dev->irq, priv);
+exit_release:
 	pci_release_region(dev, SMBBAR);
 exit:
-	kfree(priv);
 	return err;
 }
 
@@ -1302,11 +1300,8 @@ static void i801_remove(struct pci_dev *dev)
 	i2c_del_adapter(&priv->adapter);

drivers/i2c/busses/i2c-i801.c static void i801_remove(struct pci_dev *dev) · line 1307

Quoted 10 lines
drivers/i2c/busses/i2c-i801.c +0 -3
 	pci_write_config_byte(dev, SMBHSTCFG, priv->original_hstcfg);
 
-	if (priv->features & FEATURE_IRQ)
-		free_irq(dev->irq, priv);
 	pci_release_region(dev, SMBBAR);
 
-	kfree(priv);
 	/*
 	 * do not call pci_disable_device(dev) since it can cause hard hangs on
 	 * some systems during power-off (eg. Fujitsu-Siemens Lifebook E8010)

Looks good, but what about also converting pci_request_region to devm_request_region?

Signature · 2 lines
Jean Delvare
SUSE L3 Support

A message that is not in the archive (sent off-list or not yet received): [email protected]

#6 lore raw

Re: [PATCH 2/2] i2c: i801: Use managed devm_* memory and irq allocation

Quoted 10 lines · Jean Delvare

Hi Jarkko,

Quoted 4 lines · Jarkko Nikula

This simplifies the error and remove paths.

---

Looks good, but what about also converting pci_request_region to devm_request_region?

Yep, in progress but won't finish today so wanted to send these two first :-)

Signature · 1 line
Jarkko
Reviewed-by
#7 lore raw
In reply to a message not in the archive

Hi Jarkko,

Quoted 2 lines · Jarkko Nikula

struct pci_driver i801_driver forward declaration is needed only for accessing the name field. Remove it and use dev_driver_string() instead.

This will make the driver binary slightly larger but I can live with that.

Quoted 37 lines
---
 drivers/i2c/busses/i2c-i801.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/i2c/busses/i2c-i801.c +2 -4
index 8fafb254e42a..e31aa987b281 100644
@@ -223,8 +223,6 @@ struct i801_priv {
 #endif
 };
 
-static struct pci_driver i801_driver;
-
 #define FEATURE_SMBUS_PEC	(1 << 0)
 #define FEATURE_BLOCK_BUFFER	(1 << 1)
 #define FEATURE_BLOCK_PROC	(1 << 2)
@@ -1204,7 +1202,7 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
 		goto exit;
 	}
 
-	err = pci_request_region(dev, SMBBAR, i801_driver.name);
+	err = pci_request_region(dev, SMBBAR, dev_driver_string(&dev->dev));
 	if (err) {
 		dev_err(&dev->dev, "Failed to request SMBus region "
 			"0x%lx-0x%Lx\n", priv->smba,
@@ -1255,7 +1253,7 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
 		init_waitqueue_head(&priv->waitq);
 
 		err = request_irq(dev->irq, i801_isr, IRQF_SHARED,
-				  i801_driver.name, priv);
+				  dev_driver_string(&dev->dev), priv);
 		if (err) {
 			dev_err(&dev->dev, "Failed to allocate irq %d: %d\n",
 				dev->irq, err);
Signature · 2 lines
Jean Delvare
SUSE L3 Support