Re: [PATCH 4/4] gpio: Add parsing of DT GPIO line-names
From: Linus Walleij <hidden>
Date: 2016-02-23 13:36:42
Also in:
linux-gpio
On Tue, Feb 23, 2016 at 8:54 AM, Markus Pargmann [off-list ref] 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 <redacted>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