On Thu, Apr 24, 2014 at 09:06:24AM -0500, Rob Herring wrote: > On Thu, Apr 24, 2014 at 7:47 AM, Linus Walleij wrote: > > On Wed, Apr 23, 2014 at 5:28 PM, Thierry Reding > > wrote: > > > >> From: Thierry Reding > >> > >> Many bindings use the -gpio suffix in property names. Support this in > >> addition to the -gpios suffix when requesting GPIOs using the new > >> descriptor-based API. > >> > >> Signed-off-by: Thierry Reding > > > > Are the DT bindings really full of such ambiguity between > > singular and plural? Examples? > > > > What happens in affected drivers today? It just doesn't work? > > They mostly seem to use of_get_named_gpio. Indeed. That has the downside of requiring manual parsing and handling of the GPIO polarity, though. > > Does that mean these bindings are not actively used by any > > drivers yet so we could augment the bindings instead, or are > > they already deployed so we must implement this? > > > > Would like a word from the DT people here... > > Interestingly, there is not a single occurrence of '-gpio ' in > powerpc, but a bunch in ARM. In hindsight, we should have perhaps > enforced using -gpios only, but that doesn't really matter now. We > have -gpio in use, so we need to support it. I think I also saw a proposal only recently to add support for a gpios/gpio-names type of binding > That doesn't necessarily mean this function has to support it. For > example, this function could a legacy method and some other function > should be used instead (of_get_named_gpio perhaps). The reason why I posted this is precisely because I wanted to convert over some drivers to use the new helpers (because they make things like polarity handling much easier). My first attempt was to fix the binding because I was under the impression that -gpio usage was discouraged, but people didn't like that because, you know, DT bindings being a stable ABI and all that. The downside of not allowing the gpiod API to support the -gpio suffix is that we'll never be able to convert drivers that use such a binding and will forever have a hodgepodge of GPIO APIs that we need to support. > >> drivers/gpio/gpiolib.c | 18 ++++++++++++------ > >> 1 file changed, 12 insertions(+), 6 deletions(-) > >> > >> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c > >> index 7a0b97076374..b991462c22fb 100644 > >> --- a/drivers/gpio/gpiolib.c > >> +++ b/drivers/gpio/gpiolib.c > >> @@ -2594,17 +2594,23 @@ static struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id, > >> unsigned int idx, > >> enum gpio_lookup_flags *flags) > >> { > >> + static const char *suffixes[] = { "gpios", "gpio" }; > >> char prop_name[32]; /* 32 is max size of property name */ > >> enum of_gpio_flags of_flags; > >> struct gpio_desc *desc; > >> + unsigned int i; > >> > >> - if (con_id) > >> - snprintf(prop_name, 32, "%s-gpios", con_id); > >> - else > >> - snprintf(prop_name, 32, "gpios"); > >> + for (i = 0; i < ARRAY_SIZE(suffixes); i++) { > >> + if (con_id) > >> + snprintf(prop_name, 32, "%s-%s", con_id, suffixes[i]); > >> + else > >> + snprintf(prop_name, 32, "%s", suffixes[i]); > > This has the side effect of searching for "gpio" as property name > which I don't think we want to allow. Why don't we want to allow a "gpio" property when we already allow "gpios"? > It also allows a DT with either suffix to work. While I don't > necessarily think the kernel's job should be DT validation, we don't > have any other validation today and I don't see how this change > simplifies the code. Between stricter DT checking (in the kernel) and > simpler code, I'd pick the latter. I had briefly considered adding more validation here as well, such as refusing to hand out any GPIO with idx > 0 for the -gpio suffix, but then opted not to do that in favour of code simplicity. Thierry