Re: [PATCH] gpio: Add support for GPIOF_ACTIVE_LOW to gpio_request_one
From: Guenter Roeck
Date: Tue Jul 22 2014 - 01:16:53 EST
On 07/21/2014 09:30 PM, Alexandre Courbot wrote:
On Thu, Jul 17, 2014 at 11:34 PM, Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
On 07/17/2014 12:26 AM, Alexandre Courbot wrote:
On Thu, Jul 17, 2014 at 3:37 PM, Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
On 07/16/2014 11:09 PM, Alexandre Courbot wrote:
On Thu, Jul 17, 2014 at 8:11 AM, Guenter Roeck <linux@xxxxxxxxxxxx>
wrote:
The gpio include file and the gpio documentation declare and document
GPIOF_ACTIVE_LOW as one of the flags to be passed to gpio_request_one
and related functions. However, the flag is not evaluated or used.
Check the flag in gpio_request_one and set the gpio internal flag
FLAG_ACTIVE_LOW if it is set.
What is the point since the integer GPIO API has no clue of the
active-low status of a GPIO? It is only used by the gpiod and sysfs
interfaces.
One can use gpio_request_one() to export a gpio pin to user space from
the kernel. That code path does use the flag, as you point out yourself
above.
Ok, in that case I suppose it makes sense.
Reviewed-by: Alexandre Courbot <acourbot@xxxxxxxxxx>
One could also argue that the integer gpio API _should_ support this as
well,
but that is a different question.
Probably not going to happen. The integer GPIO interface is deprecated
and users who need new features should seriously consider switching to
gpiod.
The new API is unfortunately not equivalent to the old one. For example,
if I understand correctly, gpiod_get is expected to be used instead of
gpio_request_one.
That is correct. The reason is to have a separate authority to assigns
GPIOs and prevent drivers from arbitrarily requesting any GPIO they
want, as long as everybody sticks to the gpiod interface.
That may work nicely in a world with full DT or ACPI
support, but doesn't work as well otherwise unless one drops the notion
of using platform specific drivers built as modules (gpiod_add_lookup_table
is not exported, and there is no remove function).
Specifically, I don't see an easy way to convert mdio-gpio to use the new
model, and that driver could really use support for an API which supports
active-low pins.
If you want to benefit from the active-low property but cannot use
gpiod_get() for some reason, you can still request a GPIO by
gpio_request_one() and then convert it to a descriptor with
gpio_to_desc(), which is what I suspect your patch will allow you to
do. But the real fix would be to work any limitations that gpiod has
that prevent you from doing what you need.
I am not familiar with mdio-gpio - could you explain what makes it
impossible to convert this driver to the new model in your view?
gpiod_get has the notion of 'con_id'. That is all fine if you have
a system with acpi or devicetree to set this up, but the platform
data fallback is pretty clumsy (at least in my opinion). To convert
mdio-gpio such that it can provide the required platform data to gpio
would complicate the code so much that the conversion would not be worth
it. It would also require to convert all drivers _using_ mdio-gpio to the
new platform data format required by gpiod_get, or the mdio-gpio driver
would have to do some kind of internal conversion, both of which would
make the code even more complicated (and less likely to get it accepted).
My current solution is exactly what you suggested - to use gpio_request_one()
followed by gpio_to_desc(). I plan to submit the necessary patches
once this patch has been accepted.
Thanks,
Guenter
--
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/