[PATCH 0/2] gpio: dwapb: allow sane gpio numbering

[PATCH 0/2] gpio: dwapb: allow sane gpio numbering

[Richard Cochran]

The dwapb driver always uses dynamic (pseudo-random) gpio numbers
selected in blocks *descending* from the arch's maximum gpio value.
Maybe this was meant for some theoretical hot pluggable gpio
controller, but normal SoC devices have their gpio numbering set in
stone.

This series extends the driver to accept a base number from the device
tree and add the correct base numbers into the dts for the Altera
socfpga.


Richard Cochran (2):
  gpio: dwapb: Use human understandable gpio numbering.
  arm: dts: socfpga: Provide the gpio numbers in the controller nodes.

 Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt | 2 ++
 arch/arm/boot/dts/socfpga.dtsi                             | 3 +++
 drivers/gpio/gpio-dwapb.c                                  | 8 +++++++-
 3 files changed, 12 insertions(+), 1 deletion(-)

-- 
2.1.4

[PATCH 1/2] gpio: dwapb: Use human understandable gpio numbering.

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

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;
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;
 	}
 
-- 
2.1.4

Re: [PATCH 1/2] gpio: dwapb: Use human understandable gpio numbering.

[Michael van der Westhuizen]

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 linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] gpio: dwapb: Use human understandable gpio numbering.

[Richard Cochran]

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 linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] gpio: dwapb: Use human understandable gpio numbering.

[Linus Walleij]

On Thu, Jul 2, 2015 at 4:20 PM, Richard Cochran
<richardcochran-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] gpio: dwapb: Use human understandable gpio numbering.

[Michael van der Westhuizen]

> 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 linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] gpio: dwapb: Use human understandable gpio numbering.

[Richard Cochran]

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

Re: [PATCH 1/2] gpio: dwapb: Use human understandable gpio numbering.

[Richard Cochran]

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

Re: [PATCH 1/2] gpio: dwapb: Use human understandable gpio numbering.

[Michael van der Westhuizen]

> 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

Re: [PATCH 1/2] gpio: dwapb: Use human understandable gpio numbering.

[Richard Cochran]

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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] gpio: dwapb: Use human understandable gpio numbering.

[Russell King - ARM Linux]

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

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 1/2] gpio: dwapb: Use human understandable gpio numbering.

[Linus Walleij]

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

Re: [PATCH 1/2] gpio: dwapb: Use human understandable gpio numbering.

[Michael van der Westhuizen]

> 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 linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] gpio: dwapb: Use human understandable gpio numbering.

[Sebastian Andrzej Siewior]

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

Re: [PATCH 1/2] gpio: dwapb: Use human understandable gpio numbering.

[Richard Cochran]

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

Re: [PATCH 1/2] gpio: dwapb: Use human understandable gpio numbering.

[Sebastian Andrzej Siewior]

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

Re: [PATCH 1/2] gpio: dwapb: Use human understandable gpio numbering.

[Richard Cochran]

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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] gpio: dwapb: Use human understandable gpio numbering.

[Sebastian Andrzej Siewior]

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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] gpio: dwapb: Use human understandable gpio numbering.

[Michael van der Westhuizen]

> 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 linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] gpio: dwapb: Use human understandable gpio numbering.

[Michael van der Westhuizen]

> 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 linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] gpio: dwapb: Use human understandable gpio numbering.

[Linus Walleij]

On Wed, Jul 1, 2015 at 9:34 PM, Richard Cochran
<richardcochran-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
(...)
> +- 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] gpio: dwapb: Use human understandable gpio numbering.

[Richard Cochran]

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

Re: [PATCH 1/2] gpio: dwapb: Use human understandable gpio numbering.

[Richard Cochran]

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

Re: [PATCH 1/2] gpio: dwapb: Use human understandable gpio numbering.

[Linus Walleij]

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

Re: [PATCH 1/2] gpio: dwapb: Use human understandable gpio numbering.

[Richard Cochran]

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

Re: [PATCH 1/2] gpio: dwapb: Use human understandable gpio numbering.

[Sebastian Andrzej Siewior]

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

[PATCH 2/2] arm: dts: socfpga: Provide the gpio numbers in the controller nodes.

[Richard Cochran]

This patch provides the base gpio numbers in accordance with the published
data sheet.

Signed-off-by: Richard Cochran <rcochran-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
---
 arch/arm/boot/dts/socfpga.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
index d9176e6..b3eecc1 100644
--- a/arch/arm/boot/dts/socfpga.dtsi
+++ b/arch/arm/boot/dts/socfpga.dtsi
@@ -559,6 +559,7 @@
 				compatible = "snps,dw-apb-gpio-port";
 				gpio-controller;
 				#gpio-cells = <2>;
+				snps,base = <0>;
 				snps,nr-gpios = <29>;
 				reg = <0>;
 				interrupt-controller;
@@ -579,6 +580,7 @@
 				compatible = "snps,dw-apb-gpio-port";
 				gpio-controller;
 				#gpio-cells = <2>;
+				snps,base = <29>;
 				snps,nr-gpios = <29>;
 				reg = <0>;
 				interrupt-controller;
@@ -599,6 +601,7 @@
 				compatible = "snps,dw-apb-gpio-port";
 				gpio-controller;
 				#gpio-cells = <2>;
+				snps,base = <58>;
 				snps,nr-gpios = <27>;
 				reg = <0>;
 				interrupt-controller;
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html