lkml.org 
[lkml]   [2018]   [May]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 4.16 269/272] pinctrl: msm: Use dynamic GPIO numbering
    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@linaro.org>
    > > >
    > > > [ 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@codeaurora.org>
    > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
    > > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
    > > > Signed-off-by: Sasha Levin <alexander.levin@microsoft.com>
    > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    > > > ---
    > > > 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

    \
     
     \ /
      Last update: 2018-05-31 18:59    [W:4.155 / U:0.188 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site