| Message ID | 1465898604-16294-1-git-send-email-u.kleine-koenig@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Uwe, On 06/14/2016 01:03 PM, Uwe Kleine-König wrote: > Traditionally the n-th gpio device probed by the omap gpio driver got > the gpio number range [n*32 .. n*32+31]. > When order of the devices probed by the driver changes (which can happen > already now when some devices have a pinctrl and so the first probe > attempt returns -ENODEV) the numbering changes. > > To ensure a deterministical numbering use of_alias_get_id to determine > the number base for a given device. If no respective alias exists fall > back to the traditional numbering. > > For the unusual case where only a part of the gpio devices have a > matching alias some of them might fail to probe. But if none of them has > an alias or all, there is no conflict which should be good enough to > maintain backward compatibility. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > Hello, > > if you're happy with this patch I can follow up and add the respective > aliases to the device trees. > Thanks for the patch. I have no objection to the idea of the patch, but there are some comments. > > drivers/gpio/gpio-omap.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c > index b98ede78c9d8..6814245a54aa 100644 > --- a/drivers/gpio/gpio-omap.c > +++ b/drivers/gpio/gpio-omap.c > @@ -1034,6 +1034,7 @@ static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc) > static int gpio; > int irq_base = 0; > int ret; > + int gpio_alias_id; > > /* > * REVISIT eventually switch from OMAP-specific gpio structs > @@ -1056,6 +1057,17 @@ static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc) > bank->chip.label = "gpio"; > bank->chip.base = gpio; I think, the gpio base correction should be done here > } > + > + /* > + * Traditionally the base is given out in first-come-first-serve order. > + * This might shuffle the numbering of gpios if the probe order changes. > + * So make the base deterministical if the device tree specifies alias > + * ids. > + */ > + gpio_alias_id = of_alias_get_id(bank->chip.of_node, "gpio"); > + if (gpio_alias_id >= 0) > + bank->chip.base = bank->width * gpio_alias_id; > + Unfortunately, this driver is still used by non-DT platforms, so above code will break build if !OF && !OF_GPIO I think, right way would be to get alias_id in .probe(), then calc base and save it in struct gpio_bank. Then use it here to fixup GPIO base if positive. > bank->chip.ngpio = bank->width; > > ret = gpiochip_add_data(&bank->chip, bank); >
Hello Grygorii, On Tue, Jun 14, 2016 at 02:18:08PM +0300, Grygorii Strashko wrote: > On 06/14/2016 01:03 PM, Uwe Kleine-König wrote: > > diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c > > index b98ede78c9d8..6814245a54aa 100644 > > --- a/drivers/gpio/gpio-omap.c > > +++ b/drivers/gpio/gpio-omap.c > > @@ -1034,6 +1034,7 @@ static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc) > > static int gpio; > > int irq_base = 0; > > int ret; > > + int gpio_alias_id; > > > > /* > > * REVISIT eventually switch from OMAP-specific gpio structs > > @@ -1056,6 +1057,17 @@ static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc) > > bank->chip.label = "gpio"; > > bank->chip.base = gpio; > > I think, the gpio base correction should be done here What is the upside of doing it here? It has the downside that it needs one more indention level. The only difference I see is that it doesn't apply for mpuio devices then and wonder if that is relevant. > > } > > + > > + /* > > + * Traditionally the base is given out in first-come-first-serve order. > > + * This might shuffle the numbering of gpios if the probe order changes. > > + * So make the base deterministical if the device tree specifies alias > > + * ids. > > + */ > > + gpio_alias_id = of_alias_get_id(bank->chip.of_node, "gpio"); > > + if (gpio_alias_id >= 0) > > + bank->chip.base = bank->width * gpio_alias_id; > > + > > Unfortunately, this driver is still used by non-DT platforms, so above code will > break build if !OF && !OF_GPIO Without CONFIG_OF of_alias_get_id still exists and returns -ENOSYS which should make the added code a nop. Does your compiler agree with you that my patch breaks building the driver? Best regards Uwe
On 06/14/2016 03:01 PM, Uwe Kleine-König wrote: > Hello Grygorii, > > On Tue, Jun 14, 2016 at 02:18:08PM +0300, Grygorii Strashko wrote: >> On 06/14/2016 01:03 PM, Uwe Kleine-König wrote: >>> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c >>> index b98ede78c9d8..6814245a54aa 100644 >>> --- a/drivers/gpio/gpio-omap.c >>> +++ b/drivers/gpio/gpio-omap.c >>> @@ -1034,6 +1034,7 @@ static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc) >>> static int gpio; >>> int irq_base = 0; >>> int ret; >>> + int gpio_alias_id; >>> >>> /* >>> * REVISIT eventually switch from OMAP-specific gpio structs >>> @@ -1056,6 +1057,17 @@ static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc) >>> bank->chip.label = "gpio"; >>> bank->chip.base = gpio; >> >> I think, the gpio base correction should be done here > > What is the upside of doing it here? It has the downside that it needs > one more indention level. The only difference I see is that it doesn't > apply for mpuio devices then and wonder if that is relevant. Personally, I prefer not ti touch mpuio code unless really required. > >>> } >>> + >>> + /* >>> + * Traditionally the base is given out in first-come-first-serve order. >>> + * This might shuffle the numbering of gpios if the probe order changes. >>> + * So make the base deterministical if the device tree specifies alias >>> + * ids. >>> + */ >>> + gpio_alias_id = of_alias_get_id(bank->chip.of_node, "gpio"); >>> + if (gpio_alias_id >= 0) >>> + bank->chip.base = bank->width * gpio_alias_id; >>> + >> >> Unfortunately, this driver is still used by non-DT platforms, so above code will >> break build if !OF && !OF_GPIO > > Without CONFIG_OF of_alias_get_id still exists and returns -ENOSYS which > should make the added code a nop. Does your compiler agree with you that > my patch breaks building the driver? make ARCH=arm omap1_defconfig make ARCH=arm ./drivers/gpio/gpio-omap.o drivers/gpio/gpio-omap.c: In function 'omap_gpio_chip_init': drivers/gpio/gpio-omap.c:1066:44: error: 'struct gpio_chip' has no member named 'of_node' gpio_alias_id = of_alias_get_id(bank->chip.of_node, "gpio"); ^ make[1]: *** [drivers/gpio/gpio-omap.o] Error 1 make: *** [drivers/gpio/gpio-omap.o] Error 2
Hi,
[auto build test ERROR on gpio/for-next]
[also build test ERROR on v4.7-rc3 next-20160614]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Uwe-Kleine-K-nig/gpio-omap-make-gpio-numbering-deterministical-by-using-of-aliases/20160614-180703
base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git for-next
config: arm-omap1_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 5.3.1-8) 5.3.1 20160205
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm
All errors (new ones prefixed by >>):
drivers/gpio/gpio-omap.c: In function 'omap_gpio_chip_init':
>> drivers/gpio/gpio-omap.c:1067:44: error: 'struct gpio_chip' has no member named 'of_node'
gpio_alias_id = of_alias_get_id(bank->chip.of_node, "gpio");
^
vim +1067 drivers/gpio/gpio-omap.c
1061 /*
1062 * Traditionally the base is given out in first-come-first-serve order.
1063 * This might shuffle the numbering of gpios if the probe order changes.
1064 * So make the base deterministical if the device tree specifies alias
1065 * ids.
1066 */
> 1067 gpio_alias_id = of_alias_get_id(bank->chip.of_node, "gpio");
1068 if (gpio_alias_id >= 0)
1069 bank->chip.base = bank->width * gpio_alias_id;
1070
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Tue, Jun 14, 2016 at 12:03 PM, Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > Traditionally the n-th gpio device probed by the omap gpio driver got > the gpio number range [n*32 .. n*32+31]. > When order of the devices probed by the driver changes (which can happen > already now when some devices have a pinctrl and so the first probe > attempt returns -ENODEV) the numbering changes. > > To ensure a deterministical numbering use of_alias_get_id to determine > the number base for a given device. If no respective alias exists fall > back to the traditional numbering. I'm not too happy about this approach. The patch doesn't mention what practical problems it is trying to solve. I am very much suspecting the sysfs ABI to be what you're trying to get consistent, and that is OBSOLETED, see fe95046e960b4b76e73dc1486955d93f47276134 "gpio: ABI: mark the sysfs ABI as obsolete" The GPIO numbering scheme is a matter of Linux internals and not about hardware description IMO. The way forward is to use the character device and use gpiochip devices with offset indexes and look up GPIOs by name from the character devices. If nothing substantial happens I am merging the final pieces of the GPIO chardev ABI for v4.8 and that is doing all that sysfs was doing and then some. I just need to change a small thing before sending the final version for review. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Linus, On Wed, Jun 15, 2016 at 08:56:58AM +0200, Linus Walleij wrote: > On Tue, Jun 14, 2016 at 12:03 PM, Uwe Kleine-König > <u.kleine-koenig@pengutronix.de> wrote: > > > Traditionally the n-th gpio device probed by the omap gpio driver got > > the gpio number range [n*32 .. n*32+31]. > > When order of the devices probed by the driver changes (which can happen > > already now when some devices have a pinctrl and so the first probe > > attempt returns -ENODEV) the numbering changes. > > > > To ensure a deterministical numbering use of_alias_get_id to determine > > the number base for a given device. If no respective alias exists fall > > back to the traditional numbering. > > I'm not too happy about this approach. > > The patch doesn't mention what practical problems it is trying to > solve. the practical problem is that a (binary-only and hardware specific) application uses GPIO47 via /sys/class/gpio and this number refers to the wrong GPIO since I introduced gpio-hogs in the device tree together with pinctrl stuff to make the gpio-hogs work. I agree that this doesn't need to be a reason to care for it from a mainline POV. > The GPIO numbering scheme is a matter of Linux internals and > not about hardware description IMO. Not sure if I should agree here or not. It's very usual that the "internal" gpio numbers match the hardware reference manual. I know this from imx, at91, all pre-dt platforms, I'm sure there are more, and I bet I'm not the only one relying on this for omap. And this is very usual in the dt world, too: $ git grep -El 'gpio. = \&gpio' arch/arm/boot/dts | wc -l 37 > The way forward is to use the character device and use gpiochip > devices with offset indexes and look up GPIOs by name from the > character devices. If nothing substantial happens I am merging the > final pieces of the GPIO chardev ABI for v4.8 and that is doing all that > sysfs was doing and then some. I just need to change a small thing > before sending the final version for review. Hmm, so /sys/class/gpio was obsoleted before the substitution was ready? I'd say an overlapping of several kernel versions would be good as we cannot expect that userspace changes as fast as the kernel. Independent of the inclusion of my patch series in the mainline kernel I'll have to use it on my custom kernel to keep the hardware do what it should. Best regards Uwe
* Uwe Kleine-König <u.kleine-koenig@pengutronix.de> [160615 00:27]: > On Wed, Jun 15, 2016 at 08:56:58AM +0200, Linus Walleij wrote: > > The way forward is to use the character device and use gpiochip > > devices with offset indexes and look up GPIOs by name from the > > character devices. If nothing substantial happens I am merging the > > final pieces of the GPIO chardev ABI for v4.8 and that is doing all that > > sysfs was doing and then some. I just need to change a small thing > > before sending the final version for review. > > Hmm, so /sys/class/gpio was obsoleted before the substitution was ready? > I'd say an overlapping of several kernel versions would be good as we > cannot expect that userspace changes as fast as the kernel. Well the /sys/class/gpio is an interface, and we'll have to maintain it basically forever as we all know. It also works just fine for simple things, so let's make sure it is usable. Having it inconsistent can cause nasty side effects upgrading kernels. Naturally I don't have any objections for adding a character device :) Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/15/2016 11:27 AM, Tony Lindgren wrote: > * Uwe Kleine-König <u.kleine-koenig@pengutronix.de> [160615 00:27]: >> On Wed, Jun 15, 2016 at 08:56:58AM +0200, Linus Walleij wrote: >>> The way forward is to use the character device and use gpiochip >>> devices with offset indexes and look up GPIOs by name from the >>> character devices. If nothing substantial happens I am merging the >>> final pieces of the GPIO chardev ABI for v4.8 and that is doing all that >>> sysfs was doing and then some. I just need to change a small thing >>> before sending the final version for review. >> >> Hmm, so /sys/class/gpio was obsoleted before the substitution was ready? >> I'd say an overlapping of several kernel versions would be good as we >> cannot expect that userspace changes as fast as the kernel. > > Well the /sys/class/gpio is an interface, and we'll have to maintain > it basically forever as we all know. It also works just fine for simple > things, so let's make sure it is usable. Having it inconsistent can > cause nasty side effects upgrading kernels. > Wouldn't gpio-line-names be helpful for such cases as mentioned in [1],[2] But I don't know it these properties were exposed in legacy GPIO sysfs interface also. [1] http://www.spinics.net/lists/linux-gpio/msg15040.html [2] https://patchwork.ozlabs.org/patch/617713/
On Wed, Jun 15, 2016 at 9:24 AM, Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > On Wed, Jun 15, 2016 at 08:56:58AM +0200, Linus Walleij wrote: >> The GPIO numbering scheme is a matter of Linux internals and >> not about hardware description IMO. > > Not sure if I should agree here or not. It's very usual that the > "internal" gpio numbers match the hardware reference manual. I know this > from imx, at91, all pre-dt platforms, I'm sure there are more, and I bet > I'm not the only one relying on this for omap. I think it will still match nicely against the chip-local offsets of the primary gpiochip so it'll be fine with a chardev too. The same was/is the case of the first interrupts on x86 I think, but with the plethora of irqchips and dependency on probe order etc the assumption is nowadays to dangerous. > > And this is very usual in the dt world, too: > > $ git grep -El 'gpio. = \&gpio' arch/arm/boot/dts | wc -l > 37 Aha I didn't even know. Well I guess I could allow it for OMAP too then, but I want an ACK from one of the DT binding maintainers. >> The way forward is to use the character device and use gpiochip >> devices with offset indexes and look up GPIOs by name from the >> character devices. If nothing substantial happens I am merging the >> final pieces of the GPIO chardev ABI for v4.8 and that is doing all that >> sysfs was doing and then some. I just need to change a small thing >> before sending the final version for review. > > Hmm, so /sys/class/gpio was obsoleted before the substitution was ready? Yeah, the sysfs was so broken that it could not be maintained. > I'd say an overlapping of several kernel versions would be good as we > cannot expect that userspace changes as fast as the kernel. This is why it is scheduled for removal in 2020 and not tomorrow. > Independent of the inclusion of my patch series in the mainline kernel > I'll have to use it on my custom kernel to keep the hardware do what it > should. Nah well, let's discuss it. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 15, 2016 at 10:27 AM, Tony Lindgren <tony@atomide.com> wrote: > * Uwe Kleine-König <u.kleine-koenig@pengutronix.de> [160615 00:27]: >> On Wed, Jun 15, 2016 at 08:56:58AM +0200, Linus Walleij wrote: >> > The way forward is to use the character device and use gpiochip >> > devices with offset indexes and look up GPIOs by name from the >> > character devices. If nothing substantial happens I am merging the >> > final pieces of the GPIO chardev ABI for v4.8 and that is doing all that >> > sysfs was doing and then some. I just need to change a small thing >> > before sending the final version for review. >> >> Hmm, so /sys/class/gpio was obsoleted before the substitution was ready? >> I'd say an overlapping of several kernel versions would be good as we >> cannot expect that userspace changes as fast as the kernel. > > Well the /sys/class/gpio is an interface, and we'll have to maintain > it basically forever as we all know. It also works just fine for simple > things, so let's make sure it is usable. Having it inconsistent can > cause nasty side effects upgrading kernels. The clever approach is that while sysfs GPIO has to be explicitly turned on with a Kconfig, the character device is mandatory, and people can rely on it always being present. This will in my dreams encourage people to move toward using the chardev. Any removal of the sysfs depend on real-world users migrating to the chardev though. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 15, 2016 at 11:56 AM, Grygorii Strashko <grygorii.strashko@ti.com> wrote: > Wouldn't gpio-line-names be helpful for such cases as mentioned in [1],[2] > But I don't know it these properties were exposed in legacy GPIO sysfs interface also. The line names only expose through the chardev interface because the sysfs ABI is frozen and should not be extended. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Linus, On Sat, Jun 18, 2016 at 10:25:45AM +0200, Linus Walleij wrote: > On Wed, Jun 15, 2016 at 9:24 AM, Uwe Kleine-König > <u.kleine-koenig@pengutronix.de> wrote: > > On Wed, Jun 15, 2016 at 08:56:58AM +0200, Linus Walleij wrote: > > >> The GPIO numbering scheme is a matter of Linux internals and > >> not about hardware description IMO. > > > > Not sure if I should agree here or not. It's very usual that the > > "internal" gpio numbers match the hardware reference manual. I know this > > from imx, at91, all pre-dt platforms, I'm sure there are more, and I bet > > I'm not the only one relying on this for omap. > > I think it will still match nicely against the chip-local offsets of the > primary gpiochip so it'll be fine with a chardev too. The same was/is I cannot follow. What is the primary gpiochip? The first one? What is a "chip-local offset". Just 3 for the fourth gpio of a given gpio bank? I guess the problem is that I didn't follow development of the gpio chardev. > the case of the first interrupts on x86 I think, but with the plethora of > irqchips and dependency on probe order etc the assumption is > nowadays to dangerous. > > > > > And this is very usual in the dt world, too: > > > > $ git grep -El 'gpio. = \&gpio' arch/arm/boot/dts | wc -l > > 37 > > Aha I didn't even know. Well I guess I could allow it for OMAP too > then, but I want an ACK from one of the DT binding maintainers. I added Rob, Frank, Mark and the device tree list to the recipients of this mail. Can you please comment? There is already a v2 that you can find at http://thread.gmane.org/gmane.linux.kernel.gpio/17399/ in case it didn't hit your mailbox. If you tell me that you want it, I can also bounce you the series. Best regards Uwe
On Sun, Jun 19, 2016 at 03:08:23AM +0200, Uwe Kleine-König wrote: > Hello Linus, > > On Sat, Jun 18, 2016 at 10:25:45AM +0200, Linus Walleij wrote: > > On Wed, Jun 15, 2016 at 9:24 AM, Uwe Kleine-König > > <u.kleine-koenig@pengutronix.de> wrote: > > > On Wed, Jun 15, 2016 at 08:56:58AM +0200, Linus Walleij wrote: > > > > >> The GPIO numbering scheme is a matter of Linux internals and > > >> not about hardware description IMO. > > > > > > Not sure if I should agree here or not. It's very usual that the > > > "internal" gpio numbers match the hardware reference manual. I know this > > > from imx, at91, all pre-dt platforms, I'm sure there are more, and I bet > > > I'm not the only one relying on this for omap. > > > > I think it will still match nicely against the chip-local offsets of the > > primary gpiochip so it'll be fine with a chardev too. The same was/is > > I cannot follow. What is the primary gpiochip? The first one? What is a > "chip-local offset". Just 3 for the fourth gpio of a given gpio bank? > I guess the problem is that I didn't follow development of the gpio > chardev. If I've understood correctly, Linus was on about the id space for GPIOs under a particular gpiochip. If I've understand correctly, you're trying to ensure consistent numbering the the *global* ID space shared by all GPIO chips present in a system? > > the case of the first interrupts on x86 I think, but with the plethora of > > irqchips and dependency on probe order etc the assumption is > > nowadays to dangerous. > > > > > And this is very usual in the dt world, too: > > > > > > $ git grep -El 'gpio. = \&gpio' arch/arm/boot/dts | wc -l > > > 37 > > > > Aha I didn't even know. Well I guess I could allow it for OMAP too > > then, but I want an ACK from one of the DT binding maintainers. In general, our use of aliases is rather ill-defined. It would be nicer if we could address devices in a similar manner to disks or partitions, e.g. by path or uuid, but I don't think we have anything sensible we can use there. Given that, I can see the use of an alias to provide a consistent way of referring to a particular gpiochip (and maybe we need to expose the alises information somehow to userspace), but IMO that's independent of any global ID space, probe ordering, etc. Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 22, 2016 at 6:16 PM, Mark Rutland <mark.rutland@arm.com> wrote: > In general, our use of aliases is rather ill-defined. It would be nicer > if we could address devices in a similar manner to disks or partitions, > e.g. by path or uuid, but I don't think we have anything sensible we can > use there. > > Given that, I can see the use of an alias to provide a consistent way of > referring to a particular gpiochip (and maybe we need to expose the > alises information somehow to userspace), but IMO that's independent of > any global ID space, probe ordering, etc. From the kernel point of view the way forward to identify and refer to a particular gpiochip is using /dev/gpiochipN the character device. If complete topology of the bus placement etc is needed, userspace can traverse /sys/bus/gpio/* This solves the big problem with the current global numbering system in /sys/class/gpio/* So what this alias should address would be two things: - Solve the immediate issue of the global number space for the legacy sysfs ABI, but also: - Determine which chip is gpiochip0, gpiochip1, .. etc in the new ABI, so the devices get consistent numbering. The latter is lightly frowned upon by the udev people: they think it is more proper to traverse /sys to get topological information about the devices. I would appreciate if a patch to add alias handling would take care of both these things if we apply it. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/23/2016 12:04 PM, Linus Walleij wrote: > On Wed, Jun 22, 2016 at 6:16 PM, Mark Rutland <mark.rutland@arm.com> wrote: > >> In general, our use of aliases is rather ill-defined. It would be nicer >> if we could address devices in a similar manner to disks or partitions, >> e.g. by path or uuid, but I don't think we have anything sensible we can >> use there. >> >> Given that, I can see the use of an alias to provide a consistent way of >> referring to a particular gpiochip (and maybe we need to expose the >> alises information somehow to userspace), but IMO that's independent of >> any global ID space, probe ordering, etc. > > From the kernel point of view the way forward to identify and refer to > a particular gpiochip is using /dev/gpiochipN the character device. > > If complete topology of the bus placement etc is needed, userspace > can traverse /sys/bus/gpio/* > > This solves the big problem with the current global numbering system > in /sys/class/gpio/* Hm. May be i misunderstood samthing (and sry, if my following question is dummy as I've not followed closely new GPIO ABI development), but.. - from above description it seems that global numbering system is not really solved :( instead it's been moved one level up and now the same happens with gpiocipX devices :( Wouldn't it be reasonable to add possibility to create named/labeled gpiocipX devices from the very beginning, like: "/dev/gpiocipX[_name|label], or "/dev/gpiocip[_name|label]", or .. Actually, struct gpio_chip has label field already. PS. From my experience, the worst case with dev numbering usually happens after few insmod/rmmod (or sometimes after suspend/resume) iterations - ttyX ;..(. > > So what this alias should address would be two things: > > - Solve the immediate issue of the global number space for the > legacy sysfs ABI, but also: > > - Determine which chip is gpiochip0, gpiochip1, .. etc in the > new ABI, so the devices get consistent numbering. > > The latter is lightly frowned upon by the udev people: they think it > is more proper to traverse /sys to get topological information about > the devices. > > I would appreciate if a patch to add alias handling would take care > of both these things if we apply it. >
On Thu, Jun 23, 2016 at 11:38 AM, Grygorii Strashko <grygorii.strashko@ti.com> wrote: >> This solves the big problem with the current global numbering system >> in /sys/class/gpio/* > > Hm. May be i misunderstood samthing (and sry, if my following question > is dummy as I've not followed closely new GPIO ABI development), but.. > > - from above description it seems that global numbering system is not really > solved :( instead it's been moved one level up and now the same happens with > gpiocipX devices :( Yes that is true, but also the discovery mechanism is changed so it should not be a problem: by traversing /sys/bus/gpio/* you know from the topology information which GPIO chip is which. This is the same method that e.g. USB, IIO or disks use: you have to complement the device name with bus information from sysfs. /sys/class/gpio/* on the other hand has no such information: it is flat. But as I wrote in the other mail in response to Mark Rutland: if it is desired to have predictable numbering of the GPIO chips the exact same alias method can be used for that, and a patch utilizing aliases should fix also this for the character device usecase. My own position on these aliases is "eating popcorn", I'm happy with it, happy without it, the people who care need to argue for it with the DT maintainers. > Wouldn't it be reasonable to add possibility to create named/labeled > gpiocipX devices from the very beginning, like: "/dev/gpiocipX[_name|label], or "/dev/gpiocip[_name|label]", or .. > > Actually, struct gpio_chip has label field already. I don't think that would be nice. There is an ioctl() to get that information (amongst other stuff) from the device. Check tools/gpio/lsgpio.c > PS. From my experience, the worst case with dev numbering usually happens after > few insmod/rmmod (or sometimes after suspend/resume) iterations - ttyX ;..(. In the GPIO case I think the numbers will be reused as we're using <linux/idr.h> infrastructure (same method as IIO). Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-omap" 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/gpio-omap.c b/drivers/gpio/gpio-omap.c index b98ede78c9d8..6814245a54aa 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -1034,6 +1034,7 @@ static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc) static int gpio; int irq_base = 0; int ret; + int gpio_alias_id; /* * REVISIT eventually switch from OMAP-specific gpio structs @@ -1056,6 +1057,17 @@ static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc) bank->chip.label = "gpio"; bank->chip.base = gpio; } + + /* + * Traditionally the base is given out in first-come-first-serve order. + * This might shuffle the numbering of gpios if the probe order changes. + * So make the base deterministical if the device tree specifies alias + * ids. + */ + gpio_alias_id = of_alias_get_id(bank->chip.of_node, "gpio"); + if (gpio_alias_id >= 0) + bank->chip.base = bank->width * gpio_alias_id; + bank->chip.ngpio = bank->width; ret = gpiochip_add_data(&bank->chip, bank);
Traditionally the n-th gpio device probed by the omap gpio driver got the gpio number range [n*32 .. n*32+31]. When order of the devices probed by the driver changes (which can happen already now when some devices have a pinctrl and so the first probe attempt returns -ENODEV) the numbering changes. To ensure a deterministical numbering use of_alias_get_id to determine the number base for a given device. If no respective alias exists fall back to the traditional numbering. For the unusual case where only a part of the gpio devices have a matching alias some of them might fail to probe. But if none of them has an alias or all, there is no conflict which should be good enough to maintain backward compatibility. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- Hello, if you're happy with this patch I can follow up and add the respective aliases to the device trees. Best regards Uwe drivers/gpio/gpio-omap.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)