Re: [PATCH 3/3] pinctrl: sx150x: add a static gpio/pinctrl pin range mapping
On Thu, Jan 18, 2018 at 9:19 AM, Peter Rosin wrote: > On 2018-01-18 08:58, Linus Walleij wrote: >> On Wed, Jan 17, 2018 at 2:34 PM, 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") >>> Suggested-by: Linus Walleij >>> Signed-off-by: Peter Rosin >> >> 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
Re: [PATCH 3/3] pinctrl: sx150x: add a static gpio/pinctrl pin range mapping
On 2018-01-18 08:58, Linus Walleij wrote: > On Wed, Jan 17, 2018 at 2:34 PM, 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") >> Suggested-by: Linus Walleij >> Signed-off-by: Peter Rosin > > 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
Re: [PATCH 3/3] pinctrl: sx150x: add a static gpio/pinctrl pin range mapping
On Wed, Jan 17, 2018 at 2:34 PM, 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") > Suggested-by: Linus Walleij > Signed-off-by: Peter Rosin 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
Re: [PATCH 3/3] pinctrl: sx150x: add a static gpio/pinctrl pin range mapping
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 >> Signed-off-by: Peter Rosin >> --- >> 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,
Re: [PATCH 3/3] pinctrl: sx150x: add a static gpio/pinctrl pin range mapping
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 > Signed-off-by: Peter Rosin > --- > 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, signature.asc Description: This is a digitally signed message part