| Message ID | d6b5ce85a17164970d454583560e07f7aed7b8ca.1435777856.git.rcochran@linutronix.de |
|---|---|
| State | Needs Review / ACK, archived |
| Headers | show |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 1 errors, 0 warnings, 0 lines checked |
| robh/patch-applied | success |
I have a very similar patch in my tree, and I’ve substituted this patch for mine, tested it and it works correctly. However, there are administrative issues with this change. > On 01 Jul 2015, at 8:34 PM, Richard Cochran <richardcochran@gmail.com> wrote: > > The system-on-chips using this IP core have well defined gpio numbers. > Instead of using random numbers, this patch lets the device tree specify > the correct gpio numbering. The subject should not end with a full-stop. Also, the commit message should explain why the change is needed, not what it does. While the message is accurate, the problem is not that humans can’t really make sense of the GPIO numbering, its that the GPIO numbering exposed to userland is often important for backward compatibility with a vendor BSP and it can be important (conceptually) to tie this numbering to the vendor documentation. Ultimately, the numbering is inconsequential to users in kernel-space when DT is in use. It’s the userland users we’re trying to help. In my case, I have quite a lot of vendor-supplied code that needs the numbers to be stable and backward compatible. This quickly devolves into a discussion of GPIOs in sysfs as an ABI, which is not a discussion I want to have :) > > Signed-off-by: Richard Cochran <rcochran@linutronix.de> > --- > Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt | 2 ++ > drivers/gpio/gpio-dwapb.c | 8 +++++++- > 2 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt b/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt > index dd5d2c0..5c9effd 100644 > --- a/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt > +++ b/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt > @@ -28,6 +28,7 @@ controller. > - interrupt-parent : The parent interrupt controller. > - interrupts : The interrupt to the parent controller raised when GPIOs > generate the interrupts. > +- snps,base : The base gpio number. > - snps,nr-gpios : The number of pins in the port, a single cell. > > Example: > @@ -42,6 +43,7 @@ gpio: gpio@20000 { > compatible = "snps,dw-apb-gpio-port"; > gpio-controller; > #gpio-cells = <2>; > + snps,base = <8>; > snps,nr-gpios = <8>; > reg = <0>; > interrupt-controller; This bindings change need to be in a separate patch for review by the devicetree maintainers. > diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c > index 58faf04..b7e7977 100644 > --- a/drivers/gpio/gpio-dwapb.c > +++ b/drivers/gpio/gpio-dwapb.c > @@ -491,6 +491,13 @@ dwapb_gpio_get_pdata_of(struct device *dev) > return ERR_PTR(-EINVAL); > } > > + if (of_property_read_u32(port_np, "snps,base", > + &pp->gpio_base)) { > + dev_info(dev, "no base gpio specified for %s\n", > + port_np->full_name); This should be dev_dbg, or should just be removed. This is not a problem, as it is simply maintaining existing behaviour. Michael -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/01/2015 09:34 PM, Richard Cochran wrote: > diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c > index 58faf04..b7e7977 100644 > --- a/drivers/gpio/gpio-dwapb.c > +++ b/drivers/gpio/gpio-dwapb.c > @@ -491,6 +491,13 @@ dwapb_gpio_get_pdata_of(struct device *dev) > return ERR_PTR(-EINVAL); > } > > + if (of_property_read_u32(port_np, "snps,base", > + &pp->gpio_base)) { > + dev_info(dev, "no base gpio specified for %s\n", > + port_np->full_name); > + pp->gpio_base = -1; > + } > + If you are in a specific SoC you could do base = of_alias_get_id(np, "gpio") * num_of_gpio_per_chip and get consistent numbers / sane. I think this is the one reason why there is no generic binding for the starting address. The other reason might be that this is simply a user space problem. To get consistent numbers all you need to do to lookup each gpio's memory address and decide if this is the one you look for. Sebastian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jul 02, 2015 at 08:05:26AM +0100, Michael van der Westhuizen wrote: > Ultimately, the numbering is inconsequential to users in kernel-space when DT is > in use. It’s the userland users we’re trying to help. Yes. Lets help the users! > In my case, I have quite a lot of vendor-supplied code that needs the numbers to > be stable and backward compatible. This quickly devolves into a discussion of > GPIOs in sysfs as an ABI, which is not a discussion I want to have :) In the present state, the gpio numbers on the Altera socfpga change with changing kernel releases. Not only does that suck, but it also proves that the gpio numbering is not any kind of ABI. Rather, it is some kind of oversight. Anyhow, the whole point is, when the data sheet says that the SoC has GPIO0 to GPION, then those numbers should be exposed to user space. Thanks, Richard -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jul 02, 2015 at 09:36:22AM +0200, Sebastian Andrzej Siewior wrote: > If you are in a specific SoC you could do > base = of_alias_get_id(np, "gpio") * num_of_gpio_per_chip > and get consistent numbers / sane. And what about /sys/class/gpio ? > I think this is the one reason why there is no generic binding for the > starting address. The other reason might be that this is simply a user > space problem. To get consistent numbers all you need to do to lookup > each gpio's memory address and decide if this is the one you look for. The user should be able to simply look up a GPIO in the data sheet, and then use it from a shell script. Why not make that easy to do? (Other gpio controllers are doing that, too, BTW.) Thanks, Richard -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/02/2015 04:26 PM, Richard Cochran wrote: > On Thu, Jul 02, 2015 at 09:36:22AM +0200, Sebastian Andrzej Siewior wrote: >> If you are in a specific SoC you could do >> base = of_alias_get_id(np, "gpio") * num_of_gpio_per_chip >> and get consistent numbers / sane. > > And what about /sys/class/gpio ? What about it? > >> I think this is the one reason why there is no generic binding for the >> starting address. The other reason might be that this is simply a user >> space problem. To get consistent numbers all you need to do to lookup >> each gpio's memory address and decide if this is the one you look for. > > The user should be able to simply look up a GPIO in the data sheet, > and then use it from a shell script. Why not make that easy to do? > > (Other gpio controllers are doing that, too, BTW.) I'm not saying that you should not do so. There is _no_ generic binding for this and this is what I suggest. > > Thanks, > Richard > Sebastian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jul 02, 2015 at 04:30:47PM +0200, Sebastian Andrzej Siewior wrote: > On 07/02/2015 04:26 PM, Richard Cochran wrote: > > On Thu, Jul 02, 2015 at 09:36:22AM +0200, Sebastian Andrzej Siewior wrote: > >> If you are in a specific SoC you could do > >> base = of_alias_get_id(np, "gpio") * num_of_gpio_per_chip > >> and get consistent numbers / sane. > > > > And what about /sys/class/gpio ? > > What about it? The poor users of that interface cannot use "of_alias_get_id" as you suggest. > > (Other gpio controllers are doing that, too, BTW.) > > I'm not saying that you should not do so. There is _no_ generic binding > for this and this is what I suggest. I can surely change "snps,base" to something like "gpio-base" as a generic binding. Would that be better? I expected to find the base number as a standard property, but I'm no DT expert, and I have learned not to question it. Thanks, Richard -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/02/2015 05:21 PM, Richard Cochran wrote: > On Thu, Jul 02, 2015 at 04:30:47PM +0200, Sebastian Andrzej Siewior wrote: >> On 07/02/2015 04:26 PM, Richard Cochran wrote: >>> On Thu, Jul 02, 2015 at 09:36:22AM +0200, Sebastian Andrzej Siewior wrote: >>>> If you are in a specific SoC you could do >>>> base = of_alias_get_id(np, "gpio") * num_of_gpio_per_chip >>>> and get consistent numbers / sane. >>> >>> And what about /sys/class/gpio ? >> >> What about it? > > The poor users of that interface cannot use "of_alias_get_id" as you suggest. You do that in the driver. The only problem with that is that the synopsys controller can have between one and four banks and a bank can have 1-32 GPIOs if I remember correctly. That means you can't have a static number of GPIOs like others do. Therefore I think a starting property is the only way and I would prefer a generic one. What confuses me a little: Why is there a snps,nr-gpios property? Doesn't the snps' IP-Core expose this information? > Thanks, > Richard > Sebastian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> On 02 Jul 2015, at 4:54 PM, Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote: > > On 07/02/2015 05:21 PM, Richard Cochran wrote: >> On Thu, Jul 02, 2015 at 04:30:47PM +0200, Sebastian Andrzej Siewior wrote: >>> On 07/02/2015 04:26 PM, Richard Cochran wrote: >>>> On Thu, Jul 02, 2015 at 09:36:22AM +0200, Sebastian Andrzej Siewior wrote: >>>>> If you are in a specific SoC you could do >>>>> base = of_alias_get_id(np, "gpio") * num_of_gpio_per_chip >>>>> and get consistent numbers / sane. >>>> >>>> And what about /sys/class/gpio ? >>> >>> What about it? >> >> The poor users of that interface cannot use "of_alias_get_id" as you suggest. > > You do that in the driver. The only problem with that is that the > synopsys controller can have between one and four banks and a bank can > have 1-32 GPIOs if I remember correctly. That means you can't have a > static number of GPIOs like others do. This is correct. > Therefore I think a starting property is the only way and I would > prefer a generic one. I would prefer a generic property too. I was rather surprised that one did not exist. > > What confuses me a little: Why is there a snps,nr-gpios property? > Doesn't the snps' IP-Core expose this information? Not in the documentation I have (but there’s an awful lot of reserved space in the register map). Michael -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jul 02, 2015 at 08:05:26AM +0100, Michael van der Westhuizen wrote: > > On 01 Jul 2015, at 8:34 PM, Richard Cochran <richardcochran@gmail.com> wrote: > > > > The system-on-chips using this IP core have well defined gpio numbers. > > Instead of using random numbers, this patch lets the device tree specify > > the correct gpio numbering. > > The subject should not end with a full-stop. Sorry, I really have no idea what you mean. Thanks, Richard -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> On 03 Jul 2015, at 10:13 AM, Richard Cochran <richardcochran@gmail.com> wrote: > > On Thu, Jul 02, 2015 at 08:05:26AM +0100, Michael van der Westhuizen wrote: >>> On 01 Jul 2015, at 8:34 PM, Richard Cochran <richardcochran@gmail.com> wrote: >>> >>> The system-on-chips using this IP core have well defined gpio numbers. >>> Instead of using random numbers, this patch lets the device tree specify >>> the correct gpio numbering. >> >> The subject should not end with a full-stop. > > Sorry, I really have no idea what you mean. The subject should be: gpio: dwapb: Use human understandable gpio numbering Not: gpio: dwapb: Use human understandable gpio numbering. Michael-- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jul 03, 2015 at 10:18:28AM +0100, Michael van der Westhuizen wrote: > The subject should be: > gpio: dwapb: Use human understandable gpio numbering > Not: > gpio: dwapb: Use human understandable gpio numbering. Ok, I see. I would call that dot a "period", not a full-stop. Must be a British thing... Thanks, Richard -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jul 03, 2015 at 12:36:26PM +0200, Richard Cochran wrote: > On Fri, Jul 03, 2015 at 10:18:28AM +0100, Michael van der Westhuizen wrote: > > The subject should be: > > gpio: dwapb: Use human understandable gpio numbering > > Not: > > gpio: dwapb: Use human understandable gpio numbering. > > Ok, I see. I would call that dot a "period", not a full-stop. > > Must be a British thing... https://en.wikipedia.org/wiki/Full_stop says that '.' is a period in both American and British English, but when used at the end of a sentence, the period is called a "full stop" in both. (A full stop is a period used to end a sentence.) It also says that in British English, '.' used elsewhere has also become known as a "full stop" (I regard that as incorrect, despite myself being British - it's a dilution of terms, and we'd now be in need of a new term to describe the '.' at the end of a sentence because of this.)
> On 02 Jul 2015, at 5:02 PM, Michael van der Westhuizen <michael@smart-africa.com> wrote: > > >> On 02 Jul 2015, at 4:54 PM, Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote: >> >> On 07/02/2015 05:21 PM, Richard Cochran wrote: >>> On Thu, Jul 02, 2015 at 04:30:47PM +0200, Sebastian Andrzej Siewior wrote: >>>> On 07/02/2015 04:26 PM, Richard Cochran wrote: >>>>> On Thu, Jul 02, 2015 at 09:36:22AM +0200, Sebastian Andrzej Siewior wrote: >>>>>> If you are in a specific SoC you could do >>>>>> base = of_alias_get_id(np, "gpio") * num_of_gpio_per_chip >>>>>> and get consistent numbers / sane. >>>>> >>>>> And what about /sys/class/gpio ? >>>> >>>> What about it? >>> >>> The poor users of that interface cannot use "of_alias_get_id" as you suggest. >> >> You do that in the driver. The only problem with that is that the >> synopsys controller can have between one and four banks and a bank can >> have 1-32 GPIOs if I remember correctly. That means you can't have a >> static number of GPIOs like others do. > > This is correct. > >> Therefore I think a starting property is the only way and I would >> prefer a generic one. > > I would prefer a generic property too. I was rather surprised that one did not exist. … looking into this more closely, by the time we get into gpiochip_add there’s no notion of the DT node that is associated to the gpiochip being added (and the gpiochip->dev points to the parent device, which is the generally the parent DT node), so this can’t be done generically without at least some churn in the individual drivers. Michael-- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jul 1, 2015 at 9:34 PM, Richard Cochran <richardcochran@gmail.com> wrote: > The system-on-chips using this IP core have well defined gpio numbers. > Instead of using random numbers, this patch lets the device tree specify > the correct gpio numbering. > > Signed-off-by: Richard Cochran <rcochran@linutronix.de> (...) > +- snps,base : The base gpio number. NACK. And it has been NACKed before again and again, search the mailinglist for repetitive answers. The GPIO numbers inside the Linux kernel are Linux specific and have nothing to do with the hardware numbers. If they sometimes match it is a lucky coincidence. The same goes for IRQ numbers in the kernel FWIW. So this "binding" has nothing to do with describing the hardware, which device tree is for. If it ever comes to exist it needs to be a "linux-*" property. But I doubt it will. We are working to remove dependency on them by introducing GPIO descriptors and thinking heavily about how to handle the userspace sysfs ABI moving forward. But for the ABI the numbers just need to be stable, not intuitive. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jul 2, 2015 at 9:05 AM, Michael van der Westhuizen <michael@smart-africa.com> wrote: > In my case, I have quite a lot of vendor-supplied code that needs the numbers to > be stable and backward compatible. This quickly devolves into a discussion of > GPIOs in sysfs as an ABI, which is not a discussion I want to have :) That is impossible to avoid. What you're saying is you just want to drill a hole in the boat and not have a discussion whether the boat is going to float after this or not. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jul 2, 2015 at 4:20 PM, Richard Cochran <richardcochran@gmail.com> wrote: > On Thu, Jul 02, 2015 at 08:05:26AM +0100, Michael van der Westhuizen wrote: > Anyhow, the whole point is, when the data sheet says that the SoC has > GPIO0 to GPION, then those numbers should be exposed to user space. The gpio_chip has this: struct gpio_chip { const char *label; struct device *dev; (...) const char *const *names; (...) If you want to help the users, provide string names for all the GPIO lines and suggest them to do code in userspace that use these names instead of GPIO numbers. Also the string names "gpio0", "gpio1" etc are much better than just using the Linux-specific numbers. So what about making a patch to drivers/gpio/gpio-dwapb.c that adds the .names property to its GPIO chip? Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> On 16 Jul 2015, at 9:52 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > > On Thu, Jul 2, 2015 at 9:05 AM, Michael van der Westhuizen > <michael@smart-africa.com> wrote: > >> In my case, I have quite a lot of vendor-supplied code that needs the numbers to >> be stable and backward compatible. This quickly devolves into a discussion of >> GPIOs in sysfs as an ABI, which is not a discussion I want to have :) > > That is impossible to avoid. > > What you're saying is you just want to drill a hole in the boat and not > have a discussion whether the boat is going to float after this or > not. Not at all. I was just noting that this patch will touch on this topic, and it’s not a topic that I would weigh in on as my opinions are irrelevant in this discussion. No incendiary topic intended :-) Michael -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> On 16 Jul 2015, at 9:57 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > > On Thu, Jul 2, 2015 at 4:20 PM, Richard Cochran > <richardcochran@gmail.com> wrote: >> On Thu, Jul 02, 2015 at 08:05:26AM +0100, Michael van der Westhuizen wrote: > >> Anyhow, the whole point is, when the data sheet says that the SoC has >> GPIO0 to GPION, then those numbers should be exposed to user space. > > The gpio_chip has this: > > struct gpio_chip { > const char *label; > struct device *dev; > (...) > const char *const *names; > (...) > > If you want to help the users, provide string names for all the GPIO lines > and suggest them to do code in userspace that use these names instead > of GPIO numbers. > > Also the string names "gpio0", "gpio1" etc are much better than just using > the Linux-specific numbers. > > So what about making a patch to drivers/gpio/gpio-dwapb.c that adds the > .names property to its GPIO chip? That would meet my needs. I’ll start putting something together. Michael -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jul 16, 2015 at 09:50:44AM +0200, Linus Walleij wrote: > We are working to remove dependency on them by introducing GPIO > descriptors and thinking heavily about how to handle the userspace > sysfs ABI moving forward. But for the ABI the numbers just need to > be stable, not intuitive. The numbers are *not* stable. That is a problem for users like me. Thanks, Richard -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jul 16, 2015 at 09:50:44AM +0200, Linus Walleij wrote: > NACK. And it has been NACKed before again and again, search > the mailinglist for repetitive answers. Yeah, ok, I give up. This happens with every DT discussion that tries to make things logical for the end user. > The GPIO numbers inside the Linux kernel are Linux specific and > have nothing to do with the hardware numbers. If they sometimes > match it is a lucky coincidence. Someone should tell the users that. > The same goes for IRQ numbers > in the kernel FWIW. Isn't there a mapping interface for irq numbers? > So this "binding" has nothing to do with describing the hardware, > which device tree is for. If it ever comes to exist it needs to be > a "linux-*" property. But I doubt it will. How can you say that the GPIO numbers associated with the pins and appearing in published data sheets (and used in every schematic) are not part of the hardware? Thanks, Richard -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jul 16, 2015 at 09:57:23AM +0200, Linus Walleij wrote: > So what about making a patch to drivers/gpio/gpio-dwapb.c that adds the > .names property to its GPIO chip? OK, so the user knows that he wants the instance with .name = "gpio0". What should he write into /sys/class/gpio/export? Thanks, Richard -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jul 16, 2015 at 7:19 PM, Richard Cochran <richardcochran@gmail.com> wrote: > On Thu, Jul 16, 2015 at 09:50:44AM +0200, Linus Walleij wrote: >> NACK. And it has been NACKed before again and again, search >> the mailinglist for repetitive answers. > > Yeah, ok, I give up. > > This happens with every DT discussion that tries to make things > logical for the end user. Are you assuming bad faith? I'm trying to maintain the GPIO subsystem, the problem with DT is that there are no proper DT reviewers for stuff like this, instead the task of reviewing DT stuff is pushed down to Linux subsystem maintainers like me. I think Michael made a very very nice patch series trying to make things logical for end users, please participate in that discussion. >> The GPIO numbers inside the Linux kernel are Linux specific and >> have nothing to do with the hardware numbers. If they sometimes >> match it is a lucky coincidence. > > Someone should tell the users that. > >> The same goes for IRQ numbers >> in the kernel FWIW. > > Isn't there a mapping interface for irq numbers? I don't see what you mean here. The numbers that appear in say /proc/interrupts may seem stable but they are not. They depend on things like probe order between irqchips and can change between two boots. The reason users don't have an issue with it is that there is (thank god) no userspace ABI for interrupts. >> So this "binding" has nothing to do with describing the hardware, >> which device tree is for. If it ever comes to exist it needs to be >> a "linux-*" property. But I doubt it will. > > How can you say that the GPIO numbers associated with the pins and > appearing in published data sheets (and used in every schematic) are > not part of the hardware? What the Linux kernel call GPIO numbers (or IRQ numbers for that matter) is something different from the hardware numbers in the datasheets. To the kernel it is just some cookie number. In the irqchip subsystem we have ->hwirq that corresponds to whatever the datasheet says, and which appears in the second column i /proc/interrupts, and in the GPIO subdrivers I tend to try to use "offset" the same way, giving a local number for that GPIO controller. Things were simple in the past because the GPIO numberspace was designed under the assumption that there would be only one GPIO controller on a system, but the exploding complexity of systems made it unmanageable. The problem is for example if I have a SoC with a GPIO expander: both have a GPIO line named "0". Which one wins? The SoC because it is "more important"? This issue is all over the place in any modern chipset. In Ux500 I have sometimes three GPIO expanders, all three with GPIO line 0, and also a GPIO 0 on the SoC of course. So we need to express this complexity to userspace, not try to simplify the world. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jul 27, 2015 at 12:19:08PM +0200, Linus Walleij wrote: > On Thu, Jul 16, 2015 at 7:19 PM, Richard Cochran > > This happens with every DT discussion that tries to make things > > logical for the end user. > > Are you assuming bad faith? No, I was reacting to the NAK (which I can accept) that does not address or even acknowledge the problem (which I cannot accept so easily). But now that a solution has appeared, I am happy again. > I think Michael made a very very nice patch series trying to make > things logical for end users, please participate in that discussion. Yes, I read it, and it sounds like it will solve the problem. I will give the series a try... > > Isn't there a mapping interface for irq numbers? > > I don't see what you mean here. The numbers that appear in > say /proc/interrupts may seem stable but they are not. > They depend on things like probe order between irqchips > and can change between two boots. > > The reason users don't have an issue with it is that there is > (thank god) no userspace ABI for interrupts. I was talking about CONFIG_IRQ_DOMAIN_DEBUG. > The problem is for example if I have a SoC with a GPIO expander: > both have a GPIO line named "0". Which one wins? The SoC > because it is "more important"? This issue is all over the place > in any modern chipset. In Ux500 I have sometimes three GPIO > expanders, all three with GPIO line 0, and also a GPIO 0 on > the SoC of course. So we need to express this complexity to > userspace, not try to simplify the world. Thanks for the explanation. Of course I don't want an arbitrarily simplified interface, but I can't understand why the simple case of built-in SoC GPIOs must have such a weird and variable numbering patterns. But once user space can call GPIOs by name, then it doesn't matter anymore what the kernel numbers are. Thanks, Richard -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/27/2015 12:19 PM, Linus Walleij wrote: > I don't see what you mean here. The numbers that appear in > say /proc/interrupts may seem stable but they are not. > They depend on things like probe order between irqchips > and can change between two boots. Not only on DT platforms but also on X86 where one has MSI interrupts which are allocated on probe time of the driver for a PCI device. > Yours, > Linus Walleij Sebastian -- To unsubscribe from this list: send the line "unsubscribe devicetree" 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/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt b/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt index dd5d2c0..5c9effd 100644 --- a/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt +++ b/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt @@ -28,6 +28,7 @@ controller. - interrupt-parent : The parent interrupt controller. - interrupts : The interrupt to the parent controller raised when GPIOs generate the interrupts. +- snps,base : The base gpio number. - snps,nr-gpios : The number of pins in the port, a single cell. Example: @@ -42,6 +43,7 @@ gpio: gpio@20000 { compatible = "snps,dw-apb-gpio-port"; gpio-controller; #gpio-cells = <2>; + snps,base = <8>; snps,nr-gpios = <8>; reg = <0>; interrupt-controller; diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c index 58faf04..b7e7977 100644 --- a/drivers/gpio/gpio-dwapb.c +++ b/drivers/gpio/gpio-dwapb.c @@ -491,6 +491,13 @@ dwapb_gpio_get_pdata_of(struct device *dev) return ERR_PTR(-EINVAL); } + if (of_property_read_u32(port_np, "snps,base", + &pp->gpio_base)) { + dev_info(dev, "no base gpio specified for %s\n", + port_np->full_name); + pp->gpio_base = -1; + } + if (of_property_read_u32(port_np, "snps,nr-gpios", &pp->ngpio)) { dev_info(dev, "failed to get number of gpios for %s\n", @@ -512,7 +519,6 @@ dwapb_gpio_get_pdata_of(struct device *dev) } pp->irq_shared = false; - pp->gpio_base = -1; pp->name = port_np->full_name; }
The system-on-chips using this IP core have well defined gpio numbers. Instead of using random numbers, this patch lets the device tree specify the correct gpio numbering. Signed-off-by: Richard Cochran <rcochran@linutronix.de> --- Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt | 2 ++ drivers/gpio/gpio-dwapb.c | 8 +++++++- 2 files changed, 9 insertions(+), 1 deletion(-)