* Re: Bug report - gpio: improve error path in gpiolib [not found] <F7C544E1-F25D-4E83-9896-A761ADBEBA93@goldelico.com> @ 2013-10-04 1:27 ` Alex Courbot 2013-10-04 6:21 ` Dr. H. Nikolaus Schaller 0 siblings, 1 reply; 6+ messages in thread From: Alex Courbot @ 2013-10-04 1:27 UTC (permalink / raw) To: Dr. H. Nikolaus Schaller, Linus Walleij Cc: Frank Rowand, NeilBrown Brown, Belisko Marek, linux-gpio@vger.kernel.org Hi Nikolaus, On 10/03/2013 01:22 AM, Dr. H. Nikolaus Schaller wrote: > Hi Alexandre, > i think have traced down one problem we have to the latest patch in 3.12-rc: > > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/gpio/gpiolib.c?id=be1a4b13089b1e18da83a549d49163ccad3c19ba > > The problem is that by this patch a desc == NULL now returns -EINVAL while earlier it did return an -EPROBE_DEFER, which I consider the better option. > > By this -EINVAL, now some dependent drivers are no longer initialized. > > So I't suggest to revert that. Adding Linus since he authored the patch. I suppose the behavior should not change indeed. But could you specify which use-case now returns -EINVAL instead of -EPROBE_DEFER? It is not obvious by just looking at the patch alone. Thanks, Alex. ----------------------------------------------------------------------------------- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. ----------------------------------------------------------------------------------- ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bug report - gpio: improve error path in gpiolib 2013-10-04 1:27 ` Bug report - gpio: improve error path in gpiolib Alex Courbot @ 2013-10-04 6:21 ` Dr. H. Nikolaus Schaller 2013-10-04 18:02 ` Alexandre Courbot 0 siblings, 1 reply; 6+ messages in thread From: Dr. H. Nikolaus Schaller @ 2013-10-04 6:21 UTC (permalink / raw) To: Alex Courbot Cc: Linus Walleij, Frank Rowand, NeilBrown Brown, Belisko Marek, linux-gpio@vger.kernel.org Hi all, Am 04.10.2013 um 03:27 schrieb Alex Courbot: > Hi Nikolaus, > > On 10/03/2013 01:22 AM, Dr. H. Nikolaus Schaller wrote: >> Hi Alexandre, >> i think have traced down one problem we have to the latest patch in 3.12-rc: >> >> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/gpio/gpiolib.c?id=be1a4b13089b1e18da83a549d49163ccad3c19ba >> >> The problem is that by this patch a desc == NULL now returns -EINVAL while earlier it did return an -EPROBE_DEFER, which I consider the better option. >> >> By this -EINVAL, now some dependent drivers are no longer initialized. >> >> So I't suggest to revert that. > > Adding Linus since he authored the patch. > > I suppose the behavior should not change indeed. But could you specify which use-case now returns -EINVAL instead of -EPROBE_DEFER? It is not obvious by just looking at the patch alone. To give some more background information and how we use the gpio system to run into this issue: We are currently in the middle of preparing patches for upstreaming for the ARM based GTA04 board. One of these additions needs a "reverse gpio-regulator" driver. It presents itself as a virtual GPIO to other drivers that expect to control a GPIO but internally controls a regulator. We need this to enable some external power e.g. if the DTR line of an UART is asserted by the UART driver so that an external GPS chip&antenna gets powered on. The code of that driver is here: https://github.com/goldelico/gta04-kernel/blob/master/drivers/gpio/gpio-reg.c It was developed from 3.4 to 3.7 and I have started to forward-port it to 3.12-rc to make it ready for future submission. There I found that this GPIO is now being treated as invalid instead of -EPROBE_DEFER and is therefore no longer deferring the probing of the UART. I.e. the whole UART probing process fails. This appears to have its root in the function gpiod_request() that the descriptor exists but not the chip pointer. If I understand the old code correctly it did return -EINVAL only for a NULL descriptor but did return the default value of the variable 'status' if desc != NULL but desc->chip == NULL. This default is -EPROBE_DEFER. I don't know the exact interworking of gpiolib and gpiod_request() with our driver and it's probe function, but adding this line at the beginning of gpiod_request did make the whole deferring chain work again (after IMHO restoring the "older" logic of gpiod_request up to 3.11): if (desc && !desc->chip) return -EPROBE_DEFER; BR, Nikolaus Schaller ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bug report - gpio: improve error path in gpiolib 2013-10-04 6:21 ` Dr. H. Nikolaus Schaller @ 2013-10-04 18:02 ` Alexandre Courbot 2013-10-05 20:50 ` Dr. H. Nikolaus Schaller 0 siblings, 1 reply; 6+ messages in thread From: Alexandre Courbot @ 2013-10-04 18:02 UTC (permalink / raw) To: Dr. H. Nikolaus Schaller Cc: Alex Courbot, Linus Walleij, Frank Rowand, NeilBrown Brown, Belisko Marek, linux-gpio@vger.kernel.org On Fri, Oct 4, 2013 at 3:21 PM, Dr. H. Nikolaus Schaller <hns@goldelico.com> wrote: > Hi all, > > Am 04.10.2013 um 03:27 schrieb Alex Courbot: > >> Hi Nikolaus, >> >> On 10/03/2013 01:22 AM, Dr. H. Nikolaus Schaller wrote: >>> Hi Alexandre, >>> i think have traced down one problem we have to the latest patch in 3.12-rc: >>> >>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/gpio/gpiolib.c?id=be1a4b13089b1e18da83a549d49163ccad3c19ba >>> >>> The problem is that by this patch a desc == NULL now returns -EINVAL while earlier it did return an -EPROBE_DEFER, which I consider the better option. >>> >>> By this -EINVAL, now some dependent drivers are no longer initialized. >>> >>> So I't suggest to revert that. >> >> Adding Linus since he authored the patch. >> >> I suppose the behavior should not change indeed. But could you specify which use-case now returns -EINVAL instead of -EPROBE_DEFER? It is not obvious by just looking at the patch alone. > > To give some more background information and how we use the gpio system to run > into this issue: > > We are currently in the middle of preparing patches for upstreaming for the ARM > based GTA04 board. > > One of these additions needs a "reverse gpio-regulator" driver. It presents itself > as a virtual GPIO to other drivers that expect to control a GPIO but internally controls > a regulator. We need this to enable some external power e.g. if the DTR line of > an UART is asserted by the UART driver so that an external GPS chip&antenna > gets powered on. > > The code of that driver is here: > > https://github.com/goldelico/gta04-kernel/blob/master/drivers/gpio/gpio-reg.c > > It was developed from 3.4 to 3.7 and I have started to forward-port it to 3.12-rc > to make it ready for future submission. > > There I found that this GPIO is now being treated as invalid instead of > -EPROBE_DEFER and is therefore no longer deferring the probing of the > UART. I.e. the whole UART probing process fails. > > This appears to have its root in the function gpiod_request() that the descriptor > exists but not the chip pointer. > > If I understand the old code correctly it did return -EINVAL only for a NULL > descriptor but did return the default value of the variable 'status' if desc != NULL > but desc->chip == NULL. This default is -EPROBE_DEFER. > > I don't know the exact interworking of gpiolib and gpiod_request() with our > driver and it's probe function, but adding this line at the beginning of gpiod_request > did make the whole deferring chain work again (after IMHO restoring the "older" > logic of gpiod_request up to 3.11): > > if (desc && !desc->chip) > return -EPROBE_DEFER; I see - that's fair enough, the return value of gpiod_request() should not change, especially when it bears such useful information. I have just sent 2 patches that should fix this issue. Could you test them and give your Tested-by after confirming they work? Then I think Linus can safely merge them (disclaimer: I have not tested them myself yet). Thanks, Alex. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bug report - gpio: improve error path in gpiolib 2013-10-04 18:02 ` Alexandre Courbot @ 2013-10-05 20:50 ` Dr. H. Nikolaus Schaller 2013-10-06 18:55 ` Alexandre Courbot 0 siblings, 1 reply; 6+ messages in thread From: Dr. H. Nikolaus Schaller @ 2013-10-05 20:50 UTC (permalink / raw) To: Alexandre Courbot Cc: Alex Courbot, Linus Walleij, Frank Rowand, NeilBrown Brown, Belisko Marek, linux-gpio@vger.kernel.org Hi Alexandre, Am 04.10.2013 um 20:02 schrieb Alexandre Courbot: > On Fri, Oct 4, 2013 at 3:21 PM, Dr. H. Nikolaus Schaller > <hns@goldelico.com> wrote: >> Hi all, >> >> Am 04.10.2013 um 03:27 schrieb Alex Courbot: >> >>> Hi Nikolaus, >>> >>> On 10/03/2013 01:22 AM, Dr. H. Nikolaus Schaller wrote: >>>> Hi Alexandre, >>>> i think have traced down one problem we have to the latest patch in 3.12-rc: >>>> >>>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/gpio/gpiolib.c?id=be1a4b13089b1e18da83a549d49163ccad3c19ba >>>> >>>> The problem is that by this patch a desc == NULL now returns -EINVAL while earlier it did return an -EPROBE_DEFER, which I consider the better option. >>>> >>>> By this -EINVAL, now some dependent drivers are no longer initialized. >>>> >>>> So I't suggest to revert that. >>> >>> Adding Linus since he authored the patch. >>> >>> I suppose the behavior should not change indeed. But could you specify which use-case now returns -EINVAL instead of -EPROBE_DEFER? It is not obvious by just looking at the patch alone. >> >> To give some more background information and how we use the gpio system to run >> into this issue: >> >> We are currently in the middle of preparing patches for upstreaming for the ARM >> based GTA04 board. >> >> One of these additions needs a "reverse gpio-regulator" driver. It presents itself >> as a virtual GPIO to other drivers that expect to control a GPIO but internally controls >> a regulator. We need this to enable some external power e.g. if the DTR line of >> an UART is asserted by the UART driver so that an external GPS chip&antenna >> gets powered on. >> >> The code of that driver is here: >> >> https://github.com/goldelico/gta04-kernel/blob/master/drivers/gpio/gpio-reg.c >> >> It was developed from 3.4 to 3.7 and I have started to forward-port it to 3.12-rc >> to make it ready for future submission. >> >> There I found that this GPIO is now being treated as invalid instead of >> -EPROBE_DEFER and is therefore no longer deferring the probing of the >> UART. I.e. the whole UART probing process fails. >> >> This appears to have its root in the function gpiod_request() that the descriptor >> exists but not the chip pointer. >> >> If I understand the old code correctly it did return -EINVAL only for a NULL >> descriptor but did return the default value of the variable 'status' if desc != NULL >> but desc->chip == NULL. This default is -EPROBE_DEFER. >> >> I don't know the exact interworking of gpiolib and gpiod_request() with our >> driver and it's probe function, but adding this line at the beginning of gpiod_request >> did make the whole deferring chain work again (after IMHO restoring the "older" >> logic of gpiod_request up to 3.11): >> >> if (desc && !desc->chip) >> return -EPROBE_DEFER; > > I see - that's fair enough, the return value of gpiod_request() should > not change, especially when it bears such useful information. > > I have just sent 2 patches that should fix this issue. Great. > Could you test > them and give your Tested-by after confirming they work? Yes, these patches work for us as well and have the intended effect (like my quick hack). Tested-by: Dr. H. Nikolaus Schaller <hns@goldelico.com> > > Then I think > Linus can safely merge them (disclaimer: I have not tested them myself > yet). > > Thanks, > Alex. Thanks to you, Nikolaus ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bug report - gpio: improve error path in gpiolib 2013-10-05 20:50 ` Dr. H. Nikolaus Schaller @ 2013-10-06 18:55 ` Alexandre Courbot 2013-10-11 14:29 ` Linus Walleij 0 siblings, 1 reply; 6+ messages in thread From: Alexandre Courbot @ 2013-10-06 18:55 UTC (permalink / raw) To: Dr. H. Nikolaus Schaller Cc: Alex Courbot, Linus Walleij, Frank Rowand, NeilBrown Brown, Belisko Marek, linux-gpio@vger.kernel.org On Sat, Oct 5, 2013 at 1:50 PM, Dr. H. Nikolaus Schaller <hns@goldelico.com> wrote: >> I have just sent 2 patches that should fix this issue. > > Great. > >> Could you test >> them and give your Tested-by after confirming they work? > > Yes, these patches work for us as well and have the intended > effect (like my quick hack). > > Tested-by: Dr. H. Nikolaus Schaller <hns@goldelico.com> Great! Linus, are you ok to take these into your tree? They should probably go into 3.12 as the patch that causes the difference in behavior is already merged. Thanks, Alex. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bug report - gpio: improve error path in gpiolib 2013-10-06 18:55 ` Alexandre Courbot @ 2013-10-11 14:29 ` Linus Walleij 0 siblings, 0 replies; 6+ messages in thread From: Linus Walleij @ 2013-10-11 14:29 UTC (permalink / raw) To: Alexandre Courbot Cc: Dr. H. Nikolaus Schaller, Alex Courbot, Frank Rowand, NeilBrown Brown, Belisko Marek, linux-gpio@vger.kernel.org On Sun, Oct 6, 2013 at 8:55 PM, Alexandre Courbot <gnurou@gmail.com> wrote: > On Sat, Oct 5, 2013 at 1:50 PM, Dr. H. Nikolaus Schaller > <hns@goldelico.com> wrote: >>> I have just sent 2 patches that should fix this issue. >> >> Great. >> >>> Could you test >>> them and give your Tested-by after confirming they work? >> >> Yes, these patches work for us as well and have the intended >> effect (like my quick hack). >> >> Tested-by: Dr. H. Nikolaus Schaller <hns@goldelico.com> > > Great! Linus, are you ok to take these into your tree? They should > probably go into 3.12 as the patch that causes the difference in > behavior is already merged. OK saw this now, move these two for fixes. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-10-11 14:29 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <F7C544E1-F25D-4E83-9896-A761ADBEBA93@goldelico.com>
2013-10-04 1:27 ` Bug report - gpio: improve error path in gpiolib Alex Courbot
2013-10-04 6:21 ` Dr. H. Nikolaus Schaller
2013-10-04 18:02 ` Alexandre Courbot
2013-10-05 20:50 ` Dr. H. Nikolaus Schaller
2013-10-06 18:55 ` Alexandre Courbot
2013-10-11 14:29 ` Linus Walleij
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.