| Message ID | 1397544101-18135-2-git-send-email-wens@csie.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
Hi Chen-Yu, On Tue, Apr 15, 2014 at 02:41:35PM +0800, Chen-Yu Tsai wrote: > This patch provides of_get_gpiod_flags_by_name(), which looks up GPIO > phandles by name only, through gpios/gpio-names, and not by index. IIRC, gpios only uses the *-gpios properties, and not gpios/gpio-names pattern seen on various other things. Is it some new property you introduce? If so, please add it to the documentation. Now, I'm not sure that having two distinct representations of GPIOs in the DT is a good thing. Yes, it's looking odd compared to other similar bindings, but it's what we have to deal with. Maxime
On Tue, Apr 15, 2014 at 11:20 PM, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > Hi Chen-Yu, > > On Tue, Apr 15, 2014 at 02:41:35PM +0800, Chen-Yu Tsai wrote: >> This patch provides of_get_gpiod_flags_by_name(), which looks up GPIO >> phandles by name only, through gpios/gpio-names, and not by index. > > IIRC, gpios only uses the *-gpios properties, and not gpios/gpio-names > pattern seen on various other things. > > Is it some new property you introduce? If so, please add it to the > documentation. > > Now, I'm not sure that having two distinct representations of GPIOs in > the DT is a good thing. Yes, it's looking odd compared to other > similar bindings, but it's what we have to deal with. Mmmm I *think* I somehow remember a discussion about this topic recently, but I cannot find it. Maybe Chen-yu could point us to the conclusion of this discussion and the rationale for (re)implementing named GPIOs this way? -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Apr 16, 2014 at 3:12 PM, Alexandre Courbot <gnurou@gmail.com> wrote: > On Tue, Apr 15, 2014 at 11:20 PM, Maxime Ripard > <maxime.ripard@free-electrons.com> wrote: >> Hi Chen-Yu, >> >> On Tue, Apr 15, 2014 at 02:41:35PM +0800, Chen-Yu Tsai wrote: >>> This patch provides of_get_gpiod_flags_by_name(), which looks up GPIO >>> phandles by name only, through gpios/gpio-names, and not by index. >> >> IIRC, gpios only uses the *-gpios properties, and not gpios/gpio-names >> pattern seen on various other things. >> >> Is it some new property you introduce? If so, please add it to the >> documentation. >> >> Now, I'm not sure that having two distinct representations of GPIOs in >> the DT is a good thing. Yes, it's looking odd compared to other >> similar bindings, but it's what we have to deal with. > > Mmmm I *think* I somehow remember a discussion about this topic > recently, but I cannot find it. Maybe Chen-yu could point us to the > conclusion of this discussion and the rationale for (re)implementing > named GPIOs this way? Aha, here maybe: https://lkml.org/lkml/2014/1/21/164 However I don't see a clear conclusion that we should implement that scheme. Not that I am strongly against it, but I'd like to see a practical purpose for it. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Wed, Apr 16, 2014 at 3:06 PM, Alexandre Courbot <gnurou@gmail.com> wrote: > On Wed, Apr 16, 2014 at 3:12 PM, Alexandre Courbot <gnurou@gmail.com> wrote: >> On Tue, Apr 15, 2014 at 11:20 PM, Maxime Ripard >> <maxime.ripard@free-electrons.com> wrote: >>> Hi Chen-Yu, >>> >>> On Tue, Apr 15, 2014 at 02:41:35PM +0800, Chen-Yu Tsai wrote: >>>> This patch provides of_get_gpiod_flags_by_name(), which looks up GPIO >>>> phandles by name only, through gpios/gpio-names, and not by index. >>> >>> IIRC, gpios only uses the *-gpios properties, and not gpios/gpio-names >>> pattern seen on various other things. >>> >>> Is it some new property you introduce? If so, please add it to the >>> documentation. >>> >>> Now, I'm not sure that having two distinct representations of GPIOs in >>> the DT is a good thing. Yes, it's looking odd compared to other >>> similar bindings, but it's what we have to deal with. >> >> Mmmm I *think* I somehow remember a discussion about this topic >> recently, but I cannot find it. Maybe Chen-yu could point us to the >> conclusion of this discussion and the rationale for (re)implementing >> named GPIOs this way? > > Aha, here maybe: > > https://lkml.org/lkml/2014/1/21/164 They're also mentioned in: https://lkml.org/lkml/2014/2/25/581 > However I don't see a clear conclusion that we should implement that > scheme. Not that I am strongly against it, but I'd like to see a > practical purpose for it. Again no clear conclusion on this. I wrote this as it was one possible way out of the index-based GPIO stuff. Hopefully others will chime in and we can decide whether this is what we want or not. Cheers ChenYu -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 15, 2014 at 8:41 AM, Chen-Yu Tsai <wens@csie.org> wrote: > This patch provides of_get_gpiod_flags_by_name(), which looks up GPIO > phandles by name only, through gpios/gpio-names, and not by index. > > Signed-off-by: Chen-Yu Tsai <wens@csie.org> Like Alexandre I have no strong opinion on this alternative scheme. However if I shall apply this patch I want ACKs from the DT maintainers with them expressing that they want things to look like this going forward. Otherwise the set is stalled right here. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Apr 23, 2014 at 12:02 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > On Tue, Apr 15, 2014 at 8:41 AM, Chen-Yu Tsai <wens@csie.org> wrote: > >> This patch provides of_get_gpiod_flags_by_name(), which looks up GPIO >> phandles by name only, through gpios/gpio-names, and not by index. >> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org> > > Like Alexandre I have no strong opinion on this alternative scheme. Yeah this new lookup scheme probably does no harm, but I think it should be a little bit more motivated as it is, after all, introducing more potential confusion for DT users. It does not look like this new lookup scheme is necessary to Chen-Yu's patchset and that he could as well have used the current one. Right now there is only one way to define GPIOs - if we introduce a second one, then which one should new DT users choose? Which one looks better? I can already endless fights taking place over this. Does this new lookup help with some of the existing problems we have like ACPI/DT lookup compatibility? I just need to be given one practical reason to give my ack. Alex. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Apr 23, 2014 at 9:49 AM, Alexandre Courbot <gnurou@gmail.com> wrote: > On Wed, Apr 23, 2014 at 12:02 AM, Linus Walleij > <linus.walleij@linaro.org> wrote: >> On Tue, Apr 15, 2014 at 8:41 AM, Chen-Yu Tsai <wens@csie.org> wrote: >> >>> This patch provides of_get_gpiod_flags_by_name(), which looks up GPIO >>> phandles by name only, through gpios/gpio-names, and not by index. >>> >>> Signed-off-by: Chen-Yu Tsai <wens@csie.org> >> >> Like Alexandre I have no strong opinion on this alternative scheme. > > Yeah this new lookup scheme probably does no harm, but I think it > should be a little bit more motivated as it is, after all, introducing > more potential confusion for DT users. > > It does not look like this new lookup scheme is necessary to Chen-Yu's > patchset and that he could as well have used the current one. Right > now there is only one way to define GPIOs - if we introduce a second > one, then which one should new DT users choose? Which one looks > better? I can already endless fights taking place over this. I will pull out the two patches. > Does this new lookup help with some of the existing problems we have > like ACPI/DT lookup compatibility? I hope they will be compatible with ACPI named gpios, whenever support for ACPI named properties extension lands. But that really depends on the ACPI implementation. For now I think it's best that I just hold on to these. We can revisit the discussion later if needed. > I just need to be given one practical reason to give my ack. Thanks ChenYu -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c index 2024d45..5c586fa 100644 --- a/drivers/gpio/gpiolib-of.c +++ b/drivers/gpio/gpiolib-of.c @@ -97,6 +97,54 @@ struct gpio_desc *of_get_named_gpiod_flags(struct device_node *np, EXPORT_SYMBOL(of_get_named_gpiod_flags); /** + * of_get_gpiod_flags_by_name() - Get a GPIO descriptor and flags by name + * @np: device node to get GPIO from + * @name: matching name of gpio phandle + * @flags: a flags pointer to fill in + * + * Returns GPIO descriptor to use with Linux GPIO API, or one of the errno + * value on the error condition. If @flags is not NULL the function also fills + * in flags for the GPIO. + */ +struct gpio_desc *of_get_gpiod_flags_by_name(struct device_node *np, + const char *name, enum of_gpio_flags *flags) +{ + /* Return -EPROBE_DEFER to support probe() functions to be called + * later when the GPIO actually becomes available + */ + struct gg_data gg_data = { + .flags = flags, + .out_gpio = ERR_PTR(-EPROBE_DEFER) + }; + int index = 0; + int ret; + + /* exit if no name given */ + if (!name) + return ERR_PTR(-EINVAL); + + /* .of_xlate might decide to not fill in the flags, so clear it. */ + if (flags) + *flags = 0; + + if (name) + index = of_property_match_string(np, "gpio-names", name); + + ret = of_parse_phandle_with_args(np, "gpios", "#gpio-cells", index, + &gg_data.gpiospec); + + if (ret) + return ERR_PTR(ret); + + gpiochip_find(&gg_data, of_gpiochip_find_and_xlate); + + of_node_put(gg_data.gpiospec.np); + + return gg_data.out_gpio; +} +EXPORT_SYMBOL(of_get_gpiod_flags_by_names); + +/** * of_gpio_simple_xlate - translate gpio_spec to the GPIO number and flags * @gc: pointer to the gpio_chip structure * @np: device node of the GPIO chip diff --git a/include/linux/of_gpio.h b/include/linux/of_gpio.h index f14123a..134331f 100644 --- a/include/linux/of_gpio.h +++ b/include/linux/of_gpio.h @@ -51,6 +51,9 @@ static inline struct of_mm_gpio_chip *to_of_mm_gpio_chip(struct gpio_chip *gc) extern struct gpio_desc *of_get_named_gpiod_flags(struct device_node *np, const char *list_name, int index, enum of_gpio_flags *flags); +extern struct gpio_desc *of_get_gpiod_flags_by_name(struct device_node *np, + const char *name, enum of_gpio_flags *flags); + extern int of_mm_gpiochip_add(struct device_node *np, struct of_mm_gpio_chip *mm_gc);
This patch provides of_get_gpiod_flags_by_name(), which looks up GPIO phandles by name only, through gpios/gpio-names, and not by index. Signed-off-by: Chen-Yu Tsai <wens@csie.org> --- drivers/gpio/gpiolib-of.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++ include/linux/of_gpio.h | 3 +++ 2 files changed, 51 insertions(+)