gpio: dwapb: allow sane gpio numbering

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 <[email protected]>

---
Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt | 2 ++
drivers/gpio/gpio-dwapb.c | 8 +++++++-

2 files changed, 9 insertions(+), 1 deletion(-)

Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt +1 -3

index dd5d2c0..5c9effd 100644
@@ -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:

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 :)

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.

Quoted 4 lines · Richard Cochran

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?

So what about making a patch to drivers/gpio/gpio-dwapb.c that adds the .names property to its GPIO chip?

Quoted 5 lines · Richard Cochran

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.

Quoted 2 lines · Michael van der Westhuizen

Quoted 2 lines · Richard Cochran

Quoted 5 lines

Quoted 3 lines

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.

Quoted 4 lines · Michael van der Westhuizen

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

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 :)

Quoted 3 lines · Michael van der Westhuizen

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.

drivers/gpio/gpio-dwapb.c +0 -0

index 58faf04..b7e7977 100644

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.

Quoted 3 lines · Sebastian Andrzej Siewior

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 ?

Quoted 5 lines

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

Quoted 6 lines · Richard Cochran

Quoted 3 lines · Sebastian Andrzej Siewior

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?

Quoted 1 line

(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.

Quoted 6 lines · Sebastian Andrzej Siewior

Quoted 5 lines · Richard Cochran

Quoted 3 lines · Sebastian Andrzej Siewior

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.

Quoted 3 lines

Quoted 1 line

And what about /sys/class/gpio ?

What about it?

The poor users of that interface cannot use "of_alias_get_id" as you suggest.

Thanks,
Richard

Quoted 9 lines · Richard Cochran

Quoted 8 lines · Sebastian Andrzej Siewior

Quoted 6 lines · Richard Cochran

Quoted 3 lines · Sebastian Andrzej Siewior

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 ?

Quoted 3 lines

Quoted 1 line

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?

Quoted 14 lines · Sebastian Andrzej Siewior

Quoted 9 lines · Richard Cochran

Quoted 8 lines · Sebastian Andrzej Siewior

Quoted 6 lines · Richard Cochran

Quoted 3 lines · Sebastian Andrzej Siewior

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 ?

Quoted 8 lines

Quoted 3 lines

Quoted 1 line

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.

Quoted 2 lines

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.

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 <[email protected]>

+- snps,base : The base gpio number.

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.

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.

Quoted 2 lines · Linus Walleij

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.

Quoted 3 lines

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.

Quoted 2 lines

The same goes for IRQ numbers
in the kernel FWIW.

Isn't there a mapping interface for irq numbers?

Quoted 3 lines

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?

On Thu, Jul 16, 2015 at 7:19 PM, Richard Cochran

Quoted 2 lines

This happens with every DT discussion that tries to make things logical for the end user.

Are you assuming bad faith?

I think Michael made a very very nice patch series trying to make things logical for end users, please participate in that discussion.

Quoted 1 line

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.

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.

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.

Yours,
Linus Walleij