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

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

[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.

Signed-off-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/i2c/busses/i2c-i801.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 8fafb254e42a..e31aa987b281 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -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);
-- 
2.1.4

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

[Jarkko Nikula]

This simplifies the error and remove paths.

Signed-off-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/i2c/busses/i2c-i801.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index e31aa987b281..b4369d1835ca 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -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)
-- 
2.1.4

Re: [PATCH 1/2] i2c: i801: Remove i801_driver forward declaration

[Jean Delvare]

Hi Jarkko,

On Fri,  6 Feb 2015 15:56:52 +0200, Jarkko Nikula wrote:
> 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.

Reviewed-by: Jean Delvare <jdelvare-l3A5Bk7waGM@public.gmane.org>

> 
> Signed-off-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> ---
>  drivers/i2c/busses/i2c-i801.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
> index 8fafb254e42a..e31aa987b281 100644
> --- a/drivers/i2c/busses/i2c-i801.c
> +++ b/drivers/i2c/busses/i2c-i801.c
> @@ -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);


-- 
Jean Delvare
SUSE L3 Support

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

[Jean Delvare]

Hi Jarkko,

On Fri,  6 Feb 2015 15:56:53 +0200, Jarkko Nikula wrote:
> This simplifies the error and remove paths.
> 
> Signed-off-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> ---
>  drivers/i2c/busses/i2c-i801.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
> index e31aa987b281..b4369d1835ca 100644
> --- a/drivers/i2c/busses/i2c-i801.c
> +++ b/drivers/i2c/busses/i2c-i801.c
> @@ -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)

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

-- 
Jean Delvare
SUSE L3 Support

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

[Jarkko Nikula]

On 02/06/2015 04:51 PM, Jean Delvare wrote:
> Hi Jarkko,
>
> On Fri,  6 Feb 2015 15:56:53 +0200, Jarkko Nikula wrote:
>> This simplifies the error and remove paths.
>>
>> Signed-off-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>> ---
> 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 :-)

-- 
Jarkko