Re: [PATCH v1 2/4] gpio: add parameter to allow the use named gpios

[Alexandre Courbot <gnurou@xxxxxxxxx>]

On Thu, Jan 22, 2015 at 6:44 AM, Olliver Schinagl <oliver@xxxxxxxxxxx> wrote:
> Hey Alexandre,
>
> On 01/19/2015 05:04 AM, Alexandre Courbot wrote:
>>
>> On Wed, Jan 7, 2015 at 6:08 PM, Olliver Schinagl
>> <oliver+list@xxxxxxxxxxx> wrote:
>>>
>>> From: Olliver Schinagl <oliver@xxxxxxxxxxx>
>>>
>>> The gpio binding document says that new code should always use named
>>> gpios. Patch 40b73183 added support to parse a list of gpios from child
>>> nodes, but does not make it possible to use named gpios. This patch adds
>>> the con_id property and implements it is done in gpiolib.c, where the
>>> old-style of using unnamed gpios still works.
>>
>> This is absolutely correct - thanks for spotting this.
>>
>> <snip>
>> ... since it looks like this part has been mostly copy/pasted from
>> of_find_gpio(), can you add another patch that fixes it there as well?
>
> Yeah, since it has the same functionality, i copy pasted it. Wasn't sure if
> it was worth to macro it or anything. I've sent a v2 with that patch added
> to the mix :)
>>
>>
>> Also in the case of ACPI this will prove to be an incomplete lookup
>> since acpi_find_gpio() has an additional fallback if the named lookup
>> fails.
>
> I'm not very familiar (or at all) how ACPI falls into all of this, I'm just
> starting to get a hang of the DT, but since this is how the dts is being
> parsed, where is the relation here? Or did I misunderstand?

GPIO mappings can be provided by DT or by ACPI. In the latter case
there is a fallback method if the name does not match (see
acpi_find_gpio()). fwnode_get_named_gpiod() does not check it though,
so maybe we can just ignore that.

>> In that respect, I wonder if it would not be better for
>> devm_get_gpiod_from_child() to call of_find_gpio() and
>> acpi_find_gpio() (after making them non-static) followed by
>> gpiod_request() instead of calling fwnode_get_named_gpiod(). But in
>> that case it will have to do the OF/ACPI handling by itself.
>>
>> I'm not really sure about which way is better. I'd appreciate if you
>> could give a thought to a possible refactoring that would improve the
>> situation ; otherwise feel free to ignore what I have written above
>> and to duplicate the property name building code.
>
> I'm afraid I'm a little too inexperienced to follow exactly what you say ;)

Don't worry about it then, this patch is already an improvement. GPIO
mappings lookup from firmware has become kind of messy, and I'm just
trying to enroll people to clean it instead of doing it myself. ;)
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html