[PATCH v2] pinctrl: bcm2835: Start GPIO numeration at zero

[PATCH v2] pinctrl: bcm2835: Start GPIO numeration at zero

[Matthias Klein]

Numerate the GPIOs from 0..53 instead of 202..255.

Signed-off-by: Matthias Klein <matthias.klein@linux.com>
---
Changes in v2:
 - fix wrong gpio range 0..54 in comment
---
 drivers/pinctrl/pinctrl-bcm2835.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/pinctrl-bcm2835.c b/drivers/pinctrl/pinctrl-bcm2835.c
index 5bcfd7a..c183d0f 100644
--- a/drivers/pinctrl/pinctrl-bcm2835.c
+++ b/drivers/pinctrl/pinctrl-bcm2835.c
@@ -382,7 +382,7 @@ static struct gpio_chip bcm2835_gpio_chip = {
 	.get = bcm2835_gpio_get,
 	.set = bcm2835_gpio_set,
 	.to_irq = bcm2835_gpio_to_irq,
-	.base = -1,
+	.base = 0,
 	.ngpio = BCM2835_NUM_GPIOS,
 	.can_sleep = false,
 };
-- 
2.1.2

Re: [PATCH v2] pinctrl: bcm2835: Start GPIO numeration at zero

[Stephen Warren]

On 10/27/2014 04:09 PM, Matthias Klein wrote:
> Numerate the GPIOs from 0..53 instead of 202..255.

What's the motivation for this? The GPIO IDs should all come from DT, 
which encodes everything as an ID relative to a particular controllers, 
and hence the actual value of the base address should be irrelevant.

> diff --git a/drivers/pinctrl/pinctrl-bcm2835.c b/drivers/pinctrl/pinctrl-bcm2835.c

> @@ -382,7 +382,7 @@ static struct gpio_chip bcm2835_gpio_chip = {

> -	.base = -1,
> +	.base = 0,

Re: [PATCH v2] pinctrl: bcm2835: Start GPIO numeration at zero

[Matthias Klein]

Am 27.10.2014 um 23:44 schrieb Stephen Warren:
> On 10/27/2014 04:09 PM, Matthias Klein wrote:
>> Numerate the GPIOs from 0..53 instead of 202..255.
>
> What's the motivation for this? The GPIO IDs should all come from DT, 
> which encodes everything as an ID relative to a particular 
> controllers, and hence the actual value of the base address should be 
> irrelevant.
- To be in sync with the GPIO numbers in the datasheet / documentation
- For userland applications which rely on these GPIO numbers

Re: [PATCH v2] pinctrl: bcm2835: Start GPIO numeration at zero

[Stephen Warren]

On 10/27/2014 05:05 PM, Matthias Klein wrote:
>
> Am 27.10.2014 um 23:44 schrieb Stephen Warren:
>> On 10/27/2014 04:09 PM, Matthias Klein wrote:
>>> Numerate the GPIOs from 0..53 instead of 202..255.
>>
>> What's the motivation for this? The GPIO IDs should all come from DT,
>> which encodes everything as an ID relative to a particular
>> controllers, and hence the actual value of the base address should be
>> irrelevant.
 >
> - To be in sync with the GPIO numbers in the datasheet / documentation

I assume that's only relevant because of the second point; the GPIO IDs 
in DT files already match the datasheet.

> - For userland applications which rely on these GPIO numbers

This isn't a scalable solution for that; this "fix" can only work for a 
single GPIO controller in any one system. It'd be better for all usage 
to search for the correct GPIO controller in sysfs, find the base 
address of that, and then add on the controller-relative GPIO ID. That 
way, the same approach is taken irrespective of which GPIO controller is 
in use, and there are no special cases.

Perhaps this could be simplified (removing the need to adding 
base+offset to get the Linux ID) if the GPIO core exported a 
per-controller directory in sysfs for GPIO manipulation (the files in 
which used controller-relative numbering), rather than having a single 
directory using Linux-internal global GPIO numbering; something like 
/sys/class/gpio/gpio@7e200000/export which uses ID 0..N vs. 
/sys/class/gpio/export which uses IDs X..X+N where X is arbitary.

Re: [PATCH v2] pinctrl: bcm2835: Start GPIO numeration at zero

[Matthias Klein]

Am 28.10.2014 um 01:00 schrieb Stephen Warren:
> On 10/27/2014 05:05 PM, Matthias Klein wrote:
>
>> - To be in sync with the GPIO numbers in the datasheet / documentation
>
> I assume that's only relevant because of the second point; the GPIO 
> IDs in DT files already match the datasheet.
>
>> - For userland applications which rely on these GPIO numbers
>
> This isn't a scalable solution for that; this "fix" can only work for 
> a single GPIO controller in any one system. It'd be better for all 
> usage to search for the correct GPIO controller in sysfs, find the 
> base address of that, and then add on the controller-relative GPIO ID. 
> That way, the same approach is taken irrespective of which GPIO 
> controller is in use, and there are no special cases.
>
> Perhaps this could be simplified (removing the need to adding 
> base+offset to get the Linux ID) if the GPIO core exported a 
> per-controller directory in sysfs for GPIO manipulation (the files in 
> which used controller-relative numbering), rather than having a single 
> directory using Linux-internal global GPIO numbering; something like 
> /sys/class/gpio/gpio@7e200000/export which uses ID 0..N vs. 
> /sys/class/gpio/export which uses IDs X..X+N where X is arbitary.
Would you accept a patch which implements the "gpio-base" property into 
the device tree for pinctrl-bcm2835?

Re: [PATCH v2] pinctrl: bcm2835: Start GPIO numeration at zero

[Stephen Warren]

On 10/28/2014 01:03 AM, Matthias Klein wrote:
> 
> Am 28.10.2014 um 01:00 schrieb Stephen Warren:
>> On 10/27/2014 05:05 PM, Matthias Klein wrote:
>>
>>> - To be in sync with the GPIO numbers in the datasheet / documentation
>>
>> I assume that's only relevant because of the second point; the GPIO
>> IDs in DT files already match the datasheet.
>>
>>> - For userland applications which rely on these GPIO numbers
>>
>> This isn't a scalable solution for that; this "fix" can only work for
>> a single GPIO controller in any one system. It'd be better for all
>> usage to search for the correct GPIO controller in sysfs, find the
>> base address of that, and then add on the controller-relative GPIO ID.
>> That way, the same approach is taken irrespective of which GPIO
>> controller is in use, and there are no special cases.
>>
>> Perhaps this could be simplified (removing the need to adding
>> base+offset to get the Linux ID) if the GPIO core exported a
>> per-controller directory in sysfs for GPIO manipulation (the files in
>> which used controller-relative numbering), rather than having a single
>> directory using Linux-internal global GPIO numbering; something like
>> /sys/class/gpio/gpio@7e200000/export which uses ID 0..N vs.
>> /sys/class/gpio/export which uses IDs X..X+N where X is arbitary.
> 
> Would you accept a patch which implements the "gpio-base" property into
> the device tree for pinctrl-bcm2835?

DT is supposed to represent HW features, in an OS-agnostic way, and a
"GPIO base" is a Linux-specific SW concept.

Re: [PATCH v2] pinctrl: bcm2835: Start GPIO numeration at zero

[Linus Walleij]

On Tue, Oct 28, 2014 at 12:05 AM, Matthias Klein
<matthias.klein@linux.com> wrote:
> Am 27.10.2014 um 23:44 schrieb Stephen Warren:

>> What's the motivation for this? The GPIO IDs should all come from DT,
>> which encodes everything as an ID relative to a particular controllers, and
>> hence the actual value of the base address should be irrelevant.
>
> - To be in sync with the GPIO numbers in the datasheet / documentation

We are moving away from global GPIO numbers because they are
an eternal pain. We would replace them with a local offset number
just as the second column in /proc/interrupts

Hm we should patch <debugfs/gpio> to show local offsets...

> - For userland applications which rely on these GPIO numbers

ARGH!!!

A much better idea is to add labels to your GPIOs so you can
identify them by name. Augment the driver with labeling capability
if need be, see e.g. commit 781f6d710d4482eab05cfaad50060a0ea8c0e4e0
"gpio: generic: Add label to platform data"

You just need to figure out how to do the same naming from the
device tree. Maybe some new binding is needed, others are
discussing how to do exporting of GPIOs from devicetree
anyway, see the mailing list.

Yours,
Linus Walleij

Re: [PATCH v2] pinctrl: bcm2835: Start GPIO numeration at zero

[Alexandre Courbot]

On Fri, Oct 31, 2014 at 6:49 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Tue, Oct 28, 2014 at 12:05 AM, Matthias Klein
> <matthias.klein@linux.com> wrote:
>> Am 27.10.2014 um 23:44 schrieb Stephen Warren:
>
>>> What's the motivation for this? The GPIO IDs should all come from DT,
>>> which encodes everything as an ID relative to a particular controllers, and
>>> hence the actual value of the base address should be irrelevant.
>>
>> - To be in sync with the GPIO numbers in the datasheet / documentation
>
> We are moving away from global GPIO numbers because they are
> an eternal pain. We would replace them with a local offset number
> just as the second column in /proc/interrupts
>
> Hm we should patch <debugfs/gpio> to show local offsets...
>
>> - For userland applications which rely on these GPIO numbers
>
> ARGH!!!
>
> A much better idea is to add labels to your GPIOs so you can
> identify them by name. Augment the driver with labeling capability
> if need be, see e.g. commit 781f6d710d4482eab05cfaad50060a0ea8c0e4e0
> "gpio: generic: Add label to platform data"
>
> You just need to figure out how to do the same naming from the
> device tree. Maybe some new binding is needed, others are
> discussing how to do exporting of GPIOs from devicetree
> anyway, see the mailing list.

On a related note we are feeling more and more pressure for a way to
unambiguously access GPIOs from user-space, and we should probably try
and come with a solution for this. Ideas so far:

1) Export GPIOs with a name - it seems like this one will happen soon.

2) Export GPIOs using their controller-relative number (instead of
their global number) under the controller's sysfs node. But since
controllers are also named after the first global number they manage,
we would still have a problem here.

In any case this reliance of user-space on particular GPIO numbers is
becoming a burden. We will soon get rid of the global
ARCH_NR_GPIOS-sized array of gpio_descs, and the next step would be to
get rid of the ARCH_NR_GPIOS itself. However, this macro is used to
decide the starting number for GPIO chips without an explicit "base"
attribute, so I am not sure whether we can remove it at all, unless we
can agree that GPIO numbers are variable and not part of the ABI.