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

> 

> Then I think
> Linus can safely merge them (disclaimer: I have not tested them myself
> yet).
> 
> Thanks,
> Alex.

Thanks to you,
Nikolaus

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to