| Message ID | 20180117133423.4482-4-peda@axentia.se |
|---|---|
| State | New |
| Headers | show |
| Series | pinctrl: sx150x: fixes for the probe | expand |
On Wed, 2018-01-17 at 14:34 +0100, Peter Rosin wrote: > Without such a range, gpiolib fails with -EPROBE_DEFER, pending the > addition of the range. So, without a range, gpiolib will keep > deferring indefinitely. > > Fixes: e10f72bf4b3e ("gpio: gpiolib: Generalise state persistence beyond sleep") This is a bit of a nit-pick, but I'd argue the hazard always existed in the pinctrl-sx150x's probe however hadn't been triggered until now. My instinct is Fixes should instead point to 9e80f9064e73 ("pinctrl: Add SX150X GPIO Extender Pinctrl Driver"). Having said that I still think it's worth pointing out in the commit message that e10f72bf4b3e ("gpio: gpiolib: Generalise state persistence beyond sleep") is the motivation for your patch (i.e. mention both commits). Regardless, thanks to you both for getting to the bottom of this. Andrew > Suggested-by: Linus Walleij <linus.walleij@linaro.org> > Signed-off-by: Peter Rosin <peda@axentia.se> > --- > drivers/pinctrl/pinctrl-sx150x.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/pinctrl/pinctrl-sx150x.c b/drivers/pinctrl/pinctrl-sx150x.c > index 049dd15e04ef..cbf58a10113d 100644 > --- a/drivers/pinctrl/pinctrl-sx150x.c > +++ b/drivers/pinctrl/pinctrl-sx150x.c > @@ -1193,6 +1193,11 @@ static int sx150x_probe(struct i2c_client *client, > if (ret) > return ret; > > + ret = gpiochip_add_pin_range(&pctl->gpio, dev_name(dev), > + 0, 0, pctl->data->npins); > + if (ret) > + return ret; > + > /* Add Interrupt support if an irq is specified */ > if (client->irq > 0) { > pctl->irq_chip.name = devm_kstrdup(dev, client->name,
On 2018-01-17 16:05, Andrew Jeffery wrote: > On Wed, 2018-01-17 at 14:34 +0100, Peter Rosin wrote: >> Without such a range, gpiolib fails with -EPROBE_DEFER, pending the >> addition of the range. So, without a range, gpiolib will keep >> deferring indefinitely. >> >> Fixes: e10f72bf4b3e ("gpio: gpiolib: Generalise state persistence beyond sleep") > > This is a bit of a nit-pick, but I'd argue the hazard always existed in > the pinctrl-sx150x's probe however hadn't been triggered until now. My > instinct is Fixes should instead point to 9e80f9064e73 ("pinctrl: Add > SX150X GPIO Extender Pinctrl Driver"). Having said that I still think > it's worth pointing out in the commit message that e10f72bf4b3e ("gpio: > gpiolib: Generalise state persistence beyond sleep") is the motivation > for your patch (i.e. mention both commits). Linus, if you agree, just change the commit message as you see fit. Can't say I'm proud of any of it... Cheers, Peter > Regardless, thanks to you both for getting to the bottom of this. > > Andrew > >> Suggested-by: Linus Walleij <linus.walleij@linaro.org> >> Signed-off-by: Peter Rosin <peda@axentia.se> >> --- >> drivers/pinctrl/pinctrl-sx150x.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/drivers/pinctrl/pinctrl-sx150x.c b/drivers/pinctrl/pinctrl-sx150x.c >> index 049dd15e04ef..cbf58a10113d 100644 >> --- a/drivers/pinctrl/pinctrl-sx150x.c >> +++ b/drivers/pinctrl/pinctrl-sx150x.c >> @@ -1193,6 +1193,11 @@ static int sx150x_probe(struct i2c_client *client, >> if (ret) >> return ret; >> >> + ret = gpiochip_add_pin_range(&pctl->gpio, dev_name(dev), >> + 0, 0, pctl->data->npins); >> + if (ret) >> + return ret; >> + >> /* Add Interrupt support if an irq is specified */ >> if (client->irq > 0) { >> pctl->irq_chip.name = devm_kstrdup(dev, client->name, -- 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
On Wed, Jan 17, 2018 at 2:34 PM, Peter Rosin <peda@axentia.se> wrote: > Without such a range, gpiolib fails with -EPROBE_DEFER, pending the > addition of the range. So, without a range, gpiolib will keep > deferring indefinitely. > > Fixes: e10f72bf4b3e ("gpio: gpiolib: Generalise state persistence beyond sleep") > Suggested-by: Linus Walleij <linus.walleij@linaro.org> > Signed-off-by: Peter Rosin <peda@axentia.se> Added the other Fixes: tag as well, add Cc: stable and applied. It will go in for the merge window as it is now, unless Torvalds decide to do an -rc9 in which case I will consider sending it as a separate fix next week. Yours, Linus Walleij -- 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
On 2018-01-18 08:58, Linus Walleij wrote: > On Wed, Jan 17, 2018 at 2:34 PM, Peter Rosin <peda@axentia.se> wrote: > >> Without such a range, gpiolib fails with -EPROBE_DEFER, pending the >> addition of the range. So, without a range, gpiolib will keep >> deferring indefinitely. >> >> Fixes: e10f72bf4b3e ("gpio: gpiolib: Generalise state persistence beyond sleep") >> Suggested-by: Linus Walleij <linus.walleij@linaro.org> >> Signed-off-by: Peter Rosin <peda@axentia.se> > > Added the other Fixes: tag as well, add Cc: stable and applied. > > It will go in for the merge window as it is now, unless Torvalds > decide to do an -rc9 in which case I will consider sending it as > a separate fix next week. I didn't (yet?) receive any notification about patch 2/3 and thought I'd test what would happen without it, and results are not good. I get: gpio gpiochip5: (sx1502q): could not create pin range So, I guess if 3/3 is going to stable, all three patches should go there. Cheers, Peter -- 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
On Thu, Jan 18, 2018 at 9:19 AM, Peter Rosin <peda@axentia.se> wrote: > On 2018-01-18 08:58, Linus Walleij wrote: >> On Wed, Jan 17, 2018 at 2:34 PM, Peter Rosin <peda@axentia.se> wrote: >> >>> Without such a range, gpiolib fails with -EPROBE_DEFER, pending the >>> addition of the range. So, without a range, gpiolib will keep >>> deferring indefinitely. >>> >>> Fixes: e10f72bf4b3e ("gpio: gpiolib: Generalise state persistence beyond sleep") >>> Suggested-by: Linus Walleij <linus.walleij@linaro.org> >>> Signed-off-by: Peter Rosin <peda@axentia.se> >> >> Added the other Fixes: tag as well, add Cc: stable and applied. >> >> It will go in for the merge window as it is now, unless Torvalds >> decide to do an -rc9 in which case I will consider sending it as >> a separate fix next week. > > I didn't (yet?) receive any notification about patch 2/3 and thought > I'd test what would happen without it, and results are not good. > I get: > > gpio gpiochip5: (sx1502q): could not create pin range > > So, I guess if 3/3 is going to stable, all three patches should go > there. OK then I mark them all for stable. Yours, Linus Walleij -- 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
diff --git a/drivers/pinctrl/pinctrl-sx150x.c b/drivers/pinctrl/pinctrl-sx150x.c index 049dd15e04ef..cbf58a10113d 100644 --- a/drivers/pinctrl/pinctrl-sx150x.c +++ b/drivers/pinctrl/pinctrl-sx150x.c @@ -1193,6 +1193,11 @@ static int sx150x_probe(struct i2c_client *client, if (ret) return ret; + ret = gpiochip_add_pin_range(&pctl->gpio, dev_name(dev), + 0, 0, pctl->data->npins); + if (ret) + return ret; + /* Add Interrupt support if an irq is specified */ if (client->irq > 0) { pctl->irq_chip.name = devm_kstrdup(dev, client->name,
Without such a range, gpiolib fails with -EPROBE_DEFER, pending the addition of the range. So, without a range, gpiolib will keep deferring indefinitely. Fixes: e10f72bf4b3e ("gpio: gpiolib: Generalise state persistence beyond sleep") Suggested-by: Linus Walleij <linus.walleij@linaro.org> Signed-off-by: Peter Rosin <peda@axentia.se> --- drivers/pinctrl/pinctrl-sx150x.c | 5 +++++ 1 file changed, 5 insertions(+)