On Tue, Feb 23, 2016 at 8:54 AM, Markus Pargmann <mpa@xxxxxxxxxxxxxx> wrote:
> This patch reuses the DT bindings that are already in place for the
> gpio-hogging mechanism. These bindings define line-name properties for
> GPIOs inside the gpio-chip device node.
>
> of_parse_own_gpio() now sets the gpio descriptor name using the newly
> introduced gpiod_set_name(). It checks for name collisions within a GPIO
> chip to avoid GPIOs with the same name that are exported over the same
> GPIO character device.
>
> The GPIO flags that describe the GPIO state are not required anymore in
> general but are checked if the gpio-hog property was found.
>
> This can be used to use the line names from the schematic. Example of lsgpio on
> a modified i.MX6s Riotboard:
>
> GPIO chip: gpiochip0, "209c000.gpio", 32 GPIO lines
> line 0: unnamed unlabeled
> line 1: unnamed unlabeled
> line 2: SD2_WP "wp" [kernel output open-drain]
> line 3: GPIO_3_CLK unlabeled
> line 4: SD2_CD "cd" [kernel output open-drain]
> ...
>
> The modified DT:
> &gpio1 {
> sd2_wp {
> gpios = <2 0>;
> line-name = "SD2_WP";
> };
>
> gpio_3_clk {
> gpios = <3 0>;
> line-name = "GPIO_3_CLK";
> };
>
> sd2_cd {
> gpios = <4 0>;
> line-name = "SD2_CD";
> };
> };
>
> Signed-off-by: Markus Pargmann <mpa@xxxxxxxxxxxxxx>
NICE! And this is what I want too.
We need to remove some rough edges:
> +static struct gpio_desc *gpiodev_find_gpiod_by_name(struct gpio_device *gdev,
> + const char *name)
> +{
> + int i;
> +
> + for (i = 0; i != gdev->ngpio; ++i) {
> + struct gpio_desc *desc = &gdev->descs[i];
> +
> + if (desc->name && !strcmp(desc->name, name))
> + return desc;
> + }
> +
> + return NULL;
> +}
We already have gpio_name_to_desc() which does something
similar but across all chips.
Can we break out one gpiodev_name_to_desc() like
yours, and refactor gpio_name_to_desc() to just call
that for each chip?
> +/**
> + * gpid_set_name() - sets the name of a gpio descriptor
Missing "o" in gpid_
> + * @desc: the gpio descriptor
> + * @name: the name pointer that is assigned. It is internally not copied.
> + *
> + * This function sets a new name for the GPIO. It checks for collisions with
> + * other GPIOs with the same name within the gpio chip. It returns 0 on success
> + * or -EEXIST if the name is already used within the GPIO chip.
> + */
> +int gpiod_set_name(struct gpio_desc *desc, const char *name)
> +{
> + struct gpio_desc *coll = gpiodev_find_gpiod_by_name(desc->gdev, name);
> +
> + if (coll)
> + return -EEXIST;
> +
> + desc->name = name;
> +
> + return 0;
> +}
Otherwise I'm OK with this.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
