Re: [PATCH v2] gpio: make of_get_named_gpiod_flags() private

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2682 bytes --]

On Tue, May 20, 2014 at 12:07:40AM +0900, Alexandre Courbot wrote:
> On Mon, May 19, 2014 at 10:28 PM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
> > On Sat, May 17, 2014 at 02:54:50PM +0900, Alexandre Courbot wrote:
> >> of_get_named_gpiod_flags() is visible and directly usable by GPIO
> >> consumers, but it really should not as the gpiod interface relies
> >> on the simpler gpiod_get() to provide properly-configured GPIOs.
> >>
> >> of_get_named_gpiod_flags() is just used internally by gpiolib to
> >> implement gpiod_get(), and by the old of_get_named_gpio_flags()
> >> function, therefore it makes sense to make it gpiolib-private.
> >>
> >> As a side-effect, the unused (and unneeded) of_get_gpiod_flags()
> >> inline function is also removed, and of_get_named_gpio_flags() is moved
> >> from a static inline function to a regular one in gpiolib-of.c
> >>
> >> This results in all references to gpiod_* functions in of_gpio.h being
> >> gone, which is the way it should be since this file is part of the old
> >> integer GPIO interface.
> >>
> >> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> >> ---
> >> Changes since v1:
> >> - Fixed compilation error when CONFIG_OF_GPIO is not defined
> >> - Fixed warning due to of_gpio_flags enum not being declared
> >>   in private gpiolib.h header
> >>
> >>  drivers/gpio/gpiolib-of.c | 14 ++++++++++++++
> >>  drivers/gpio/gpiolib.h    |  5 +++++
> >>  include/linux/of_gpio.h   | 35 +++--------------------------------
> >>  3 files changed, 22 insertions(+), 32 deletions(-)
> >
> > There are cases where GPIOs need to be obtained from device tree nodes
> > without a corresponding struct device. Do you have any ideas on how to
> > support such use-cases?
> >
> > There's currently no API to do that except of_get_named_gpio_flags().
> > But that doesn't handled flags automatically in a way that gpiod_get()
> > does.
> 
> Yeah, there is probably some legacy stuff that will need a few more
> functions to be handled using gpiod.
> 
> As much as possible I'd like to avoid provider-specific functions, but
> in the worst case we can imagine making another function available for
> that:
> 
> struct gpio_desc *of_gpiod_get_index(struct device_node *np, const
> char *con_id, unsigned int idx);
> 
> that will handle flags properly and everything. Would that do the trick?

Yes, I think it would. Perhaps with a of_gpiod_get() wrapper that sets
idx = 0. I'm aware of patches in the works that will require this, but
until they're posted I don't think we'll need this. We can revisit at
that time. Just wanted to mention this as a heads-up.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]