2008-10-02 06:59:22

by Rakib Mullick

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

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 ([email protected])

--- 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);

2008-10-02 08:23:06

by Michael Büsch

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

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 ([email protected])
>
> --- 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.

2008-10-02 13:39:20

by Rakib Mullick

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

On 10/2/08, Michael Buesch <[email protected]> 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 ([email protected])
> >
> > --- 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.
>