| Message ID | 57D7A700.8080402@mentor.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Sep 13, 2016 at 9:13 AM, Deepak <deepak_das@mentor.com> wrote: > strict pin controller returns -EINVAL in case of pin request which > is already claimed by somebody else. > Following is the sequence of calling pin_request() from > pinctrl_bind_pins():- > pinctrl_bind_pins()->pinctrl_select_state()->pinmux_enable_setting()-> > pin_request() > > But pinctrl_bind_pins() only returns -EPROBE_DEFER which makes device > driver probe successful even if the pin request is rejected by the pin > controller subsystem. > > This commit modifies pinctrl_bind_pins() to return error if the pin is > rejected by pin control subsystem. > > Signed-off-by: Deepak Das <deepak_das@mentor.com> Aha > /* Only return deferrals */ > - if (ret != -EPROBE_DEFER) > + if ((ret != -EPROBE_DEFER) && (ret != -EINVAL)) > ret = 0; I rewrote this when applying, like this: - /* Only return deferrals */ - if (ret != -EPROBE_DEFER) - ret = 0; + /* Return deferrals */ + if (ret == -EPROBE_DEFER) + return ret; + if (ret == -EINVAL) { + dev_err(dev, "could not initialize pin control state\n"); + return ret; + } + /* We ignore errors like -ENOENT meaning no pinctrl state */ - return ret; + return 0; Can you confim that this works for you too? 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 Tuesday 13 September 2016 05:29 PM, Linus Walleij wrote: > On Tue, Sep 13, 2016 at 9:13 AM, Deepak <deepak_das@mentor.com> wrote: > >> strict pin controller returns -EINVAL in case of pin request which >> is already claimed by somebody else. >> Following is the sequence of calling pin_request() from >> pinctrl_bind_pins():- >> pinctrl_bind_pins()->pinctrl_select_state()->pinmux_enable_setting()-> >> pin_request() >> >> But pinctrl_bind_pins() only returns -EPROBE_DEFER which makes device >> driver probe successful even if the pin request is rejected by the pin >> controller subsystem. >> >> This commit modifies pinctrl_bind_pins() to return error if the pin is >> rejected by pin control subsystem. >> >> Signed-off-by: Deepak Das <deepak_das@mentor.com> > > Aha > >> /* Only return deferrals */ >> - if (ret != -EPROBE_DEFER) >> + if ((ret != -EPROBE_DEFER) && (ret != -EINVAL)) >> ret = 0; > > I rewrote this when applying, like this: > > - /* Only return deferrals */ > - if (ret != -EPROBE_DEFER) > - ret = 0; > + /* Return deferrals */ > + if (ret == -EPROBE_DEFER) > + return ret; > + if (ret == -EINVAL) { > + dev_err(dev, "could not initialize pin control state\n"); > + return ret; > + } > + /* We ignore errors like -ENOENT meaning no pinctrl state */ > > - return ret; > + return 0; > > Can you confim that this works for you too? Yes, This works for me as well but do we really need this extra error message ? error message is printed before returning -EINVAL from most places, Although I did not checked all places. For example, error message in pin_request():- dev_err(pctldev->dev, "pin %s already requested by %s; cannot claim for %s\n", desc->name, desc->mux_owner, owner); Thanks, Deepak Das > > 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 Tue, Sep 13, 2016 at 3:41 PM, Deepak Das <deepak_das@mentor.com> wrote: >> Can you confim that this works for you too? > > Yes, This works for me as well but do we really need this extra error > message ? Nah, good point. I'll go in and drop it then. 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 Wednesday 14 September 2016 02:31 AM, Linus Walleij wrote: > On Tue, Sep 13, 2016 at 3:41 PM, Deepak Das <deepak_das@mentor.com> wrote: > >>> Can you confim that this works for you too? >> >> Yes, This works for me as well but do we really need this extra error >> message ? > > Nah, good point. I'll go in and drop it then. Hi Linus, I will release V2 version of this patch with following change :- - /* Only return deferrals */ + /* Return deferrals & invalid pin requests */ if ((ret != -EPROBE_DEFER) && (ret != -EINVAL)) ret = 0; Thanks & regards, Deepak Das > > 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/base/pinctrl.c b/drivers/base/pinctrl.c index 0762975..e65c1af 100644 --- a/drivers/base/pinctrl.c +++ b/drivers/base/pinctrl.c @@ -92,7 +92,7 @@ cleanup_alloc: dev->pins = NULL; /* Only return deferrals */ - if (ret != -EPROBE_DEFER) + if ((ret != -EPROBE_DEFER) && (ret != -EINVAL)) ret = 0; return ret;
strict pin controller returns -EINVAL in case of pin request which is already claimed by somebody else. Following is the sequence of calling pin_request() from pinctrl_bind_pins():- pinctrl_bind_pins()->pinctrl_select_state()->pinmux_enable_setting()-> pin_request() But pinctrl_bind_pins() only returns -EPROBE_DEFER which makes device driver probe successful even if the pin request is rejected by the pin controller subsystem. This commit modifies pinctrl_bind_pins() to return error if the pin is rejected by pin control subsystem. Signed-off-by: Deepak Das <deepak_das@mentor.com> --- drivers/base/pinctrl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)