[PATCH] drivers/gpio/bt8xxgpio.c Fix compilation warning.

[PATCH] drivers/gpio/bt8xxgpio.c Fix compilation warning.

[Rakib Mullick]

drivers/gpio/bt8xxgpio.c: In function `bt8xxgpio_remove':
drivers/gpio/bt8xxgpio.c:247: warning: ignoring return value of
`gpiochip_remove', declared with attribute warn_unused_result

Following patch removes the above warning.
Thanks.

Signed-off-by:	Md.Rakib H. Mullick (rakib.mullick@gmail.com)

--- linux-2.6.27-rc8.orig/drivers/gpio/bt8xxgpio.c	2008-10-01
19:20:59.000000000 +0600
+++ linux-2.6.27-rc8/drivers/gpio/bt8xxgpio.c	2008-10-02
12:30:26.374544032 +0600
@@ -242,9 +242,14 @@ err_freebg:

 static void bt8xxgpio_remove(struct pci_dev *pdev)
 {
+	int err;
 	struct bt8xxgpio *bg = pci_get_drvdata(pdev);

-	gpiochip_remove(&bg->gpio);
+	err = gpiochip_remove(&bg->gpio);
+	if (err) {
+		printk(KERN_ERR "bt8xxgpio: Resource busy. Failed to remove.\n");
+		return ;
+	}

 	bgwrite(0, BT848_INT_MASK);
 	bgwrite(~0x0, BT848_INT_STAT);

Re: [PATCH] drivers/gpio/bt8xxgpio.c Fix compilation warning.

[Michael Buesch]

On Thursday 02 October 2008 08:59:11 Rakib Mullick wrote:
> drivers/gpio/bt8xxgpio.c: In function `bt8xxgpio_remove':
> drivers/gpio/bt8xxgpio.c:247: warning: ignoring return value of
> `gpiochip_remove', declared with attribute warn_unused_result
> 
> Following patch removes the above warning.
> Thanks.
> 
> Signed-off-by:	Md.Rakib H. Mullick (rakib.mullick@gmail.com)
> 
> --- linux-2.6.27-rc8.orig/drivers/gpio/bt8xxgpio.c	2008-10-01
> 19:20:59.000000000 +0600
> +++ linux-2.6.27-rc8/drivers/gpio/bt8xxgpio.c	2008-10-02
> 12:30:26.374544032 +0600
> @@ -242,9 +242,14 @@ err_freebg:
> 
>  static void bt8xxgpio_remove(struct pci_dev *pdev)
>  {
> +	int err;
>  	struct bt8xxgpio *bg = pci_get_drvdata(pdev);
> 
> -	gpiochip_remove(&bg->gpio);
> +	err = gpiochip_remove(&bg->gpio);
> +	if (err) {
> +		printk(KERN_ERR "bt8xxgpio: Resource busy. Failed to remove.\n");
> +		return ;
> +	}
> 
>  	bgwrite(0, BT848_INT_MASK);
>  	bgwrite(~0x0, BT848_INT_STAT);
> 
> 

NACK.
gpiolib should be fixed.
It is impossible to handle this case. What your patch does is to leave the device
in an inconsistent state, if the gpio remove fails.

gpiolib is broken. It should not allow failure for the remove function, as most
callers cannot handle it. gpiolib should force-remove any users of the gpio line,
if the chip driver removes the chip. That's really the only way we can fix this.
Breaking the driver (to leak resources or leave the device in an inconsistent state)
just to get rid of a bogus warning is not the right fix.
Please ignore this warning for now.

-- 
Greetings Michael.

Re: [PATCH] drivers/gpio/bt8xxgpio.c Fix compilation warning.

[Rakib Mullick]

On 10/2/08, Michael Buesch <mb@bu3sch.de> wrote:
> On Thursday 02 October 2008 08:59:11 Rakib Mullick wrote:
>  > drivers/gpio/bt8xxgpio.c: In function `bt8xxgpio_remove':
>  > drivers/gpio/bt8xxgpio.c:247: warning: ignoring return value of
>  > `gpiochip_remove', declared with attribute warn_unused_result
>  >
>  > Following patch removes the above warning.
>  > Thanks.
>  >
>  > Signed-off-by:        Md.Rakib H. Mullick (rakib.mullick@gmail.com)
>  >
>  > --- linux-2.6.27-rc8.orig/drivers/gpio/bt8xxgpio.c    2008-10-01
>  > 19:20:59.000000000 +0600
>  > +++ linux-2.6.27-rc8/drivers/gpio/bt8xxgpio.c 2008-10-02
>  > 12:30:26.374544032 +0600
>  > @@ -242,9 +242,14 @@ err_freebg:
>  >
>  >  static void bt8xxgpio_remove(struct pci_dev *pdev)
>  >  {
>  > +     int err;
>  >       struct bt8xxgpio *bg = pci_get_drvdata(pdev);
>  >
>  > -     gpiochip_remove(&bg->gpio);
>  > +     err = gpiochip_remove(&bg->gpio);
>  > +     if (err) {
>  > +             printk(KERN_ERR "bt8xxgpio: Resource busy. Failed to remove.\n");
>  > +             return ;
>  > +     }
>  >
>  >       bgwrite(0, BT848_INT_MASK);
>  >       bgwrite(~0x0, BT848_INT_STAT);
>  >
>  >
>
>
> NACK.
>  gpiolib should be fixed.
>  It is impossible to handle this case. What your patch does is to leave the device
>  in an inconsistent state, if the gpio remove fails.
Yes, but it will notice through KERN_ERR that something bad has
happened. And if we don't get out of it then it will continue its
execution and will finish with a kfree ( I think which
is bad ). Am I missing anything ?
>
>  gpiolib is broken. It should not allow failure for the remove function, as most
>  callers cannot handle it. gpiolib should force-remove any users of the gpio line,
>  if the chip driver removes the chip. That's really the only way we can fix this.
>  Breaking the driver (to leak resources or leave the device in an inconsistent state)
>  just to get rid of a bogus warning is not the right fix.
>  Please ignore this warning for now.
>
>
>  --
>  Greetings Michael.
>