On Tuesday, February 23, 2016 02:36:42 PM Linus Walleij wrote:

On Tue, Feb 23, 2016 at 8:54 AM, Markus Pargmann [off-list ref] wrote:

quoted

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 <redacted>

NICE! And this is what I want too.

We need to remove some rough edges:

quoted

+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?

Yes, that sounds good.

Thanks,

Markus

quoted

+/**
+ * gpid_set_name() - sets the name of a gpio descriptor

Missing "o" in gpid_

quoted

+ * @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

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |