driver: base: pinctrl: return error from pinctrl_bind_pins()

PATCH Superseded Deepak 2 participants 5 messages Sep 13, 2016 – Sep 14, 2016

Subsystems (2)

GPIO Driver core

lore

Share & tools

lore

Deepak

author

2016-09-13 07:13 UTC #0 lore raw

PATCH 1/1 driver: base: pinctrl: return error from pinctrl_bind_pins()

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 <[email protected]>

---
 drivers/base/pinctrl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/base/pinctrl.c +1 -1

index 0762975..e65c1af 100644
@@ -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;

Signature · 1 line

1.9.1

Linus Walleij

2016-09-13 11:59 UTC #1 lore raw

Quoted 7 lines · Deepak

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()

Quoted 8 lines

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 <[email protected]>

Aha

drivers/base/pinctrl.c cleanup_alloc: · line 96

Quoted 4 lines

     /* Only return deferrals */

drivers/base/pinctrl.c +1 -1

-    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

Deepak Das

author

2016-09-13 13:41 UTC #2 lore raw

Quoted 42 lines · Linus Walleij

Quoted 7 lines · Deepak
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()
Quoted 8 lines
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 <[email protected]>
Aha
drivers/base/pinctrl.c cleanup_alloc: · line 96
Quoted 4 lines
      /* Only return deferrals */
drivers/base/pinctrl.c +1 -1
-    if (ret != -EPROBE_DEFER)
+    if ((ret != -EPROBE_DEFER) && (ret != -EINVAL))
          ret = 0;
I rewrote this when applying, like this:
(quoted diff) +9 -4
-       /* 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;

Quoted 1 line

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

Quoted 4 lines

Yours,
Linus Walleij

Linus Walleij

2016-09-13 21:01 UTC #3 lore raw

Quoted 4 lines · Deepak Das

Quoted 1 line
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

Deepak Das

author

2016-09-14 06:22 UTC #4 lore raw

Quoted 8 lines · Linus Walleij

Quoted 4 lines · Deepak Das
Quoted 1 line
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

Quoted 4 lines

Yours,
Linus Walleij