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@xxxxxxxxx)
> 
> --- 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/