Re: [PATCH 4.16 269/272] pinctrl: msm: Use dynamic GPIO numbering

From: Sebastian Gottschall
Date: Fri Jun 01 2018 - 04:46:25 EST




Am 31.05.2018 um 18:57 schrieb Bjorn Andersson:
On Thu 31 May 04:45 PDT 2018, Greg Kroah-Hartman wrote:

On Thu, May 31, 2018 at 01:21:56PM +0200, Sebastian Gottschall wrote:
Am 28.05.2018 um 12:05 schrieb Greg Kroah-Hartman:
4.16-stable review patch. If anyone has any objections, please let me know.

------------------

From: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>

[ Upstream commit a7aa75a2a7dba32594291a71c3704000a2fd7089 ]

The base of the TLMM gpiochip should not be statically defined as 0, fix
this to not artificially restrict the existence of multiple pinctrl-msm
devices.

Fixes: f365be092572 ("pinctrl: Add Qualcomm TLMM driver")
Reported-by: Timur Tabi <timur@xxxxxxxxxxxxxx>
Signed-off-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>
Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
Signed-off-by: Sasha Levin <alexander.levin@xxxxxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
drivers/pinctrl/qcom/pinctrl-msm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -818,7 +818,7 @@ static int msm_gpio_init(struct msm_pinc
return -EINVAL;
chip = &pctrl->chip;
- chip->base = 0;
+ chip->base = -1;
chip->ngpio = ngpio;
chip->label = dev_name(pctrl->dev);
chip->parent = pctrl->dev;
this patch creates a regression for me. on ipq8064 the systems gpios now
start somewhere in the sky

new layout

lrwxrwxrwx 1 root root 0 Jan 1 00:00 gpiochip373 -> ../../devices/platform/soc/1b700000.pci/pci0001:00/0001:00:00.0/0001:01:00.0/gpio/gpiochip373
lrwxrwxrwx 1 root root 0 Jan 1 00:00 gpiochip408 -> ../../devices/platform/soc/1b500000.pci/pci0000:00/0000:00:00.0/0000:01:00.0/gpio/gpiochip408
lrwxrwxrwx 1 root root 0 Jan 1 00:00 gpiochip443 ->
../../devices/platform/soc/800000.pinmux/gpio/gpiochip443


before the patch

lrwxrwxrwx 1 root root 0 Jan 1 1970 gpiochip0 ->
../../devices/platform/soc/800000.pinmux/gpio/gpiochip0
lrwxrwxrwxÂÂÂ 1 rootÂÂÂÂ rootÂÂÂÂÂÂÂÂÂÂÂÂ 0 May 31 13:13 gpiochip442 -> ../../devices/platform/soc/1b700000.pci/pci0001:00/0001:00:00.0/0001:01:00.0/gpio/gpiochip442
lrwxrwxrwxÂÂÂ 1 rootÂÂÂÂ rootÂÂÂÂÂÂÂÂÂÂÂÂ 0 May 31 13:13 gpiochip477 -> ../../devices/platform/soc/1b500000.pci/pci0000:00/0000:00:00.0/0000:01:00.0/gpio/gpiochip477


this broke my userspace gpio handling. i can override this, but still it
doesnt look correct since there is a hole at the beginng likelly reserved by
unused arm gpios
So does this mean that 4.17-rc is also broken for you? If so, this
needs to be reverted there as well.

Bjorn, any ideas?

The Qualcomm TLMM (pinctrl) driver was introduced long after it was
decided that the kernel should not rely on global/static gpio numbering.

But due to a mistake on my part the dynamic gpio base for this gpiochip
was always 0, which the offending patch corrects.


So writing user space code relying on the gpio numbering for this chip
has always been incorrect, just as it is for the other two gpiochips
listed above.


The original objection for backporting this patch was a touchscreen
driver stopped working on 4.4, but the driver clearly doesn't depend on
global or static numbering of gpios. I don't think this was ever root
caused.


I guess the pragmatic approach would be to come up with something like
Timur's suggested fix that makes the first instance of this driver use
base = 0 and then use dynamic numbering for the rest, to maintain
support for this incorrect usage of the gpio sysfs interface. (Although
that would probably break if Timur's customers move their user space to
the new platform as "the first instance" isn't deterministic).

Regards,
Bjorn
that would at least guarantee backward compatiblity. i'm fine with that

Sebastian