All of lore.kernel.org
 help / color / mirror / Atom feed
From: Octavian Purdila <octavian.purdila-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
To: Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Greg Kroah-Hartman
	<gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>,
	Linus Walleij
	<linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	Alexandre Courbot
	<gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org,
	Samuel Ortiz <sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>,
	Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>,
	Daniel Baluta
	<daniel.baluta-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
	Laurentiu Palcu
	<laurentiu.palcu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
	linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	lkml <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH v3 3/3] gpio: add support for the Diolan DLN-2 USB GPIO driver
Date: Fri, 5 Sep 2014 19:04:51 +0300	[thread overview]
Message-ID: <CAE1zotK4QkA9PWW=uOmM-j=N=MNuBt1v3CgdA+GXctdFUZ3QKA@mail.gmail.com> (raw)
In-Reply-To: <20140905153835.GC13242@localhost>

On Fri, Sep 5, 2014 at 6:38 PM, Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Fri, Sep 05, 2014 at 06:17:59PM +0300, Octavian Purdila wrote:
>
>> +static int dln2_do_remove(struct dln2_gpio *dln2)
>> +{
>> +     /* When removing the DLN2 USB device, gpiochip_remove may fail
>> +      * due to i2c drivers holding a GPIO pin. However, the i2c bus
>> +      * will eventually be removed triggering an i2c driver remove
>> +      * which will release the GPIO pin. So retrying the operation
>> +      * later should succeed. */
>> +     int ret = gpiochip_remove(&dln2->gpio);
>> +     struct device *dev = dln2->gpio.dev;
>> +
>> +     if (ret < 0) {
>> +             if (ret == -EBUSY)
>> +                     schedule_delayed_work(&dln2->remove_work.work, HZ/10);
>> +             else
>> +                     dev_warn(dev, "error removing gpio chip: %d\n", ret);
>> +     } else {
>> +             kfree(dln2);
>> +     }
>> +
>> +     return ret;
>> +}
>
> Apparently, the return value from gpiochip_remove is going away:
>
>         "Start to kill off the return value from gpiochip_remove() by
>         removing the __must_check attribute and removing all checks
>         inside the drivers/gpio directory. The rationale is: well what
>         were we supposed to do if there is an error code? Not much:
>         print an error message. And gpiolib already does that. So make
>         this function return void eventually."
>
>         https://www.mail-archive.com/linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg03468.html
>

Oh, I missed this, thanks for pointing it out.

> Also, have you considered what happens if there are gpios exported
> through sysfs? These may never be released.
>
> In general, how well have these patches been tested with disconnect
> events? At least gpiolib is known to blow up (sooner or later) when a
> gpiochip is removed when having requested gpios.
>

I do disconnect tests regularly. Since switching to the new irq
interface the following patch is needed:

https://lkml.org/lkml/2014/9/5/408

With it and the current patch sets things seems to work well.

WARNING: multiple messages have this Message-ID (diff)
From: Octavian Purdila <octavian.purdila@intel.com>
To: Johan Hovold <johan@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	Alexandre Courbot <gnurou@gmail.com>,
	wsa@the-dreams.de, Samuel Ortiz <sameo@linux.intel.com>,
	Lee Jones <lee.jones@linaro.org>, Arnd Bergmann <arnd@arndb.de>,
	Daniel Baluta <daniel.baluta@intel.com>,
	Laurentiu Palcu <laurentiu.palcu@intel.com>,
	linux-usb@vger.kernel.org, lkml <linux-kernel@vger.kernel.org>,
	linux-gpio@vger.kernel.org, linux-i2c@vger.kernel.org
Subject: Re: [PATCH v3 3/3] gpio: add support for the Diolan DLN-2 USB GPIO driver
Date: Fri, 5 Sep 2014 19:04:51 +0300	[thread overview]
Message-ID: <CAE1zotK4QkA9PWW=uOmM-j=N=MNuBt1v3CgdA+GXctdFUZ3QKA@mail.gmail.com> (raw)
In-Reply-To: <20140905153835.GC13242@localhost>

On Fri, Sep 5, 2014 at 6:38 PM, Johan Hovold <johan@kernel.org> wrote:
> On Fri, Sep 05, 2014 at 06:17:59PM +0300, Octavian Purdila wrote:
>
>> +static int dln2_do_remove(struct dln2_gpio *dln2)
>> +{
>> +     /* When removing the DLN2 USB device, gpiochip_remove may fail
>> +      * due to i2c drivers holding a GPIO pin. However, the i2c bus
>> +      * will eventually be removed triggering an i2c driver remove
>> +      * which will release the GPIO pin. So retrying the operation
>> +      * later should succeed. */
>> +     int ret = gpiochip_remove(&dln2->gpio);
>> +     struct device *dev = dln2->gpio.dev;
>> +
>> +     if (ret < 0) {
>> +             if (ret == -EBUSY)
>> +                     schedule_delayed_work(&dln2->remove_work.work, HZ/10);
>> +             else
>> +                     dev_warn(dev, "error removing gpio chip: %d\n", ret);
>> +     } else {
>> +             kfree(dln2);
>> +     }
>> +
>> +     return ret;
>> +}
>
> Apparently, the return value from gpiochip_remove is going away:
>
>         "Start to kill off the return value from gpiochip_remove() by
>         removing the __must_check attribute and removing all checks
>         inside the drivers/gpio directory. The rationale is: well what
>         were we supposed to do if there is an error code? Not much:
>         print an error message. And gpiolib already does that. So make
>         this function return void eventually."
>
>         https://www.mail-archive.com/linux-gpio@vger.kernel.org/msg03468.html
>

Oh, I missed this, thanks for pointing it out.

> Also, have you considered what happens if there are gpios exported
> through sysfs? These may never be released.
>
> In general, how well have these patches been tested with disconnect
> events? At least gpiolib is known to blow up (sooner or later) when a
> gpiochip is removed when having requested gpios.
>

I do disconnect tests regularly. Since switching to the new irq
interface the following patch is needed:

https://lkml.org/lkml/2014/9/5/408

With it and the current patch sets things seems to work well.

  reply	other threads:[~2014-09-05 16:04 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-05 15:17 [PATCH v3 0/3] mfd: add support for Diolan DLN-2 Octavian Purdila
     [not found] ` <1409930279-1382-1-git-send-email-octavian.purdila-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2014-09-05 15:17   ` [PATCH v3 1/3] mfd: add support for Diolan DLN-2 devices Octavian Purdila
2014-09-05 15:17     ` Octavian Purdila
2014-09-08 11:32     ` Johan Hovold
2014-09-08 12:08       ` Johan Hovold
2014-09-08 13:20       ` Octavian Purdila
2014-09-08 13:20         ` Octavian Purdila
2014-09-08 13:24         ` Lee Jones
2014-09-08 13:45         ` Johan Hovold
2014-09-05 15:17   ` [PATCH v3 2/3] i2c: add support for Diolan DLN-2 USB-I2C adapter Octavian Purdila
2014-09-05 15:17     ` Octavian Purdila
2014-09-08 14:44     ` Johan Hovold
2014-09-08 15:57       ` Octavian Purdila
2014-09-08 16:30         ` Johan Hovold
2014-09-08 17:15           ` Octavian Purdila
2014-09-08 17:15             ` Octavian Purdila
     [not found]             ` <CAE1zotJomxwAJA3aXm9MHnHd5Pg9V=K7XaptOPWkArV0jio4DQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-09-08 17:28               ` Johan Hovold
2014-09-08 17:28                 ` Johan Hovold
2014-09-09  8:20             ` Lee Jones
2014-09-05 15:17   ` [PATCH v3 3/3] gpio: add support for the Diolan DLN-2 USB GPIO driver Octavian Purdila
2014-09-05 15:17     ` Octavian Purdila
2014-09-05 15:38     ` Johan Hovold
2014-09-05 16:04       ` Octavian Purdila [this message]
2014-09-05 16:04         ` Octavian Purdila
     [not found]         ` <CAE1zotK4QkA9PWW=uOmM-j=N=MNuBt1v3CgdA+GXctdFUZ3QKA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-09-09  9:36           ` Johan Hovold
2014-09-09  9:36             ` Johan Hovold
2014-09-09 10:27             ` Octavian Purdila
2014-09-08 16:22     ` Johan Hovold
2014-09-08 16:44     ` Johan Hovold

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAE1zotK4QkA9PWW=uOmM-j=N=MNuBt1v3CgdA+GXctdFUZ3QKA@mail.gmail.com' \
    --to=octavian.purdila-ral2jqcrhueavxtiumwx3w@public.gmane.org \
    --cc=arnd-r2nGTMty4D4@public.gmane.org \
    --cc=daniel.baluta-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \
    --cc=gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org \
    --cc=johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=laurentiu.palcu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \
    --cc=lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org \
    --cc=wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.