pinctrl: enhance reporting of errors when loading from DT

There are a few places in the api where the code simply returns -EINVAL when
it finds an error. An example is pinmux_map_to_setting() which now reports an
error if we try to match a group with a function that it does not support.

The reporting of errors in pinconf_check_ops and pinmux_check_ops now has the
same style and is located inside the according functions and not the calling
code.

When the map is found in the DT but the default state can not be selected we
get an error to know that the code at least tried.

The patch also removes a stray word from one comment and a "->" from another
for the sake of consistency.

Finally we replace a few pr_err/debug() calls with dev_err/dbg().

Thanks go to Stephen Warren for reviewing the patch and enhancing the reporting
inside pinmux_map_to_setting().

Signed-off-by: John Crispin <blogic@openwrt.org>
Acked-by: Stephen Warren <swarren@wwwdotorg.org>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index 913c83e2bf1c53c9126b960ae8ee0bdcfbe56191..138e72f74377a2c5d034b22f9ebf32f9a83d7d34 100644 (file)
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -1391,37 +1391,29 @@ struct pinctrl_dev *pinctrl_register(struct pinctrl_desc *pctldesc,
        /* check core ops for sanity */
        ret = pinctrl_check_ops(pctldev);
        if (ret) {
-               pr_err("%s pinctrl ops lacks necessary functions\n",
-                       pctldesc->name);
+               dev_err(dev, "pinctrl ops lacks necessary functions\n");
                goto out_err;
        }
 
        /* If we're implementing pinmuxing, check the ops for sanity */
        if (pctldesc->pmxops) {
                ret = pinmux_check_ops(pctldev);
-               if (ret) {
-                       pr_err("%s pinmux ops lacks necessary functions\n",
-                              pctldesc->name);
+               if (ret)
                        goto out_err;
-               }
        }
 
        /* If we're implementing pinconfig, check the ops for sanity */
        if (pctldesc->confops) {
                ret = pinconf_check_ops(pctldev);
-               if (ret) {
-                       pr_err("%s pin config ops lacks necessary functions\n",
-                              pctldesc->name);
+               if (ret)
                        goto out_err;
-               }
        }
 
        /* Register all the pins */
-       pr_debug("try to register %d pins on %s...\n",
-                pctldesc->npins, pctldesc->name);
+       dev_dbg(dev, "try to register %d pins ...\n",  pctldesc->npins);
        ret = pinctrl_register_pins(pctldev, pctldesc->pins, pctldesc->npins);
        if (ret) {
-               pr_err("error during pin registration\n");
+               dev_err(dev, "error during pin registration\n");
                pinctrl_free_pindescs(pctldev, pctldesc->pins,
                                      pctldesc->npins);
                goto out_err;
@@ -1436,8 +1428,15 @@ struct pinctrl_dev *pinctrl_register(struct pinctrl_desc *pctldesc,
                struct pinctrl_state *s =
                        pinctrl_lookup_state_locked(pctldev->p,
                                                    PINCTRL_STATE_DEFAULT);
-               if (!IS_ERR(s))
-                       pinctrl_select_state_locked(pctldev->p, s);
+               if (IS_ERR(s)) {
+                       dev_dbg(dev, "failed to lookup the default state\n");
+               } else {
+                       ret = pinctrl_select_state_locked(pctldev->p, s);
+                       if (ret) {
+                               dev_err(dev,
+                                       "failed to select default state\n");
+                       }
+               }
        }
 
        mutex_unlock(&pinctrl_mutex);

diff --git a/drivers/pinctrl/pinconf.c b/drivers/pinctrl/pinconf.c
index 14f48c96b20db8bea0ec497077ba36d662d28e57..7ce139ef7e64b39d64518d164f20de80442b857c 100644 (file)
--- a/drivers/pinctrl/pinconf.c
+++ b/drivers/pinctrl/pinconf.c
@@ -28,11 +28,17 @@ int pinconf_check_ops(struct pinctrl_dev *pctldev)
        const struct pinconf_ops *ops = pctldev->desc->confops;
 
        /* We must be able to read out pin status */
-       if (!ops->pin_config_get && !ops->pin_config_group_get)
+       if (!ops->pin_config_get && !ops->pin_config_group_get) {
+               dev_err(pctldev->dev,
+                       "pinconf must be able to read out pin status\n");
                return -EINVAL;
+       }
        /* We have to be able to config the pins in SOME way */
-       if (!ops->pin_config_set && !ops->pin_config_group_set)
+       if (!ops->pin_config_set && !ops->pin_config_group_set) {
+               dev_err(pctldev->dev,
+                       "pinconf has to be able to set a pins config\n");
                return -EINVAL;
+       }
        return 0;
 }
 

diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
index 7cd0c7d0f2cf7344f7e83a53e0e2d404f4ce5969..2df753508ecabf2cb5284ec150c6a8cb07996fcb 100644 (file)
--- a/drivers/pinctrl/pinmux.c
+++ b/drivers/pinctrl/pinmux.c
@@ -42,9 +42,10 @@ int pinmux_check_ops(struct pinctrl_dev *pctldev)
            !ops->get_function_name ||
            !ops->get_function_groups ||
            !ops->enable ||
-           !ops->disable)
+           !ops->disable) {
+               dev_err(pctldev->dev, "pinmux ops lacks necessary functions\n");
                return -EINVAL;
-
+       }
        /* Check that all functions registered have names */
        nfuncs = ops->get_functions_count(pctldev);
        while (selector < nfuncs) {
@@ -143,7 +144,7 @@ static int pin_request(struct pinctrl_dev *pctldev,
                status = 0;
 
        if (status) {
-               dev_err(pctldev->dev, "->request on device %s failed for pin %d\n",
+               dev_err(pctldev->dev, "request on device %s failed for pin %d\n",
                       pctldev->desc->name, pin);
                module_put(pctldev->owner);
        }
@@ -330,17 +331,26 @@ int pinmux_map_to_setting(struct pinctrl_map const *map,
        }
 
        ret = pinmux_func_name_to_selector(pctldev, map->data.mux.function);
-       if (ret < 0)
+       if (ret < 0) {
+               dev_err(pctldev->dev, "invalid function %s in map table\n",
+                       map->data.mux.function);
                return ret;
+       }
        setting->data.mux.func = ret;
 
        ret = pmxops->get_function_groups(pctldev, setting->data.mux.func,
                                          &groups, &num_groups);
-       if (ret < 0)
+       if (ret < 0) {
+               dev_err(pctldev->dev, "can't query groups for function %s\n",
+                       map->data.mux.function);
                return ret;
-       if (!num_groups)
+       }
+       if (!num_groups) {
+               dev_err(pctldev->dev,
+                       "function %s can't be selected on any group\n",
+                       map->data.mux.function);
                return -EINVAL;
-
+       }
        if (map->data.mux.group) {
                bool found = false;
                group = map->data.mux.group;
@@ -350,15 +360,22 @@ int pinmux_map_to_setting(struct pinctrl_map const *map,
                                break;
                        }
                }
-               if (!found)
+               if (!found) {
+                       dev_err(pctldev->dev,
+                               "invalid group \"%s\" for function \"%s\"\n",
+                               group, map->data.mux.function);
                        return -EINVAL;
+               }
        } else {
                group = groups[0];
        }
 
        ret = pinctrl_get_group_selector(pctldev, group);
-       if (ret < 0)
+       if (ret < 0) {
+               dev_err(pctldev->dev, "invalid group %s in map table\n",
+                       map->data.mux.group);
                return ret;
+       }
        setting->data.mux.group = ret;
 
        ret = pctlops->get_group_pins(pctldev, setting->data.mux.group, &pins,
@@ -375,7 +392,7 @@ int pinmux_map_to_setting(struct pinctrl_map const *map,
                ret = pin_request(pctldev, pins[i], map->dev_name, NULL);
                if (ret) {
                        dev_err(pctldev->dev,
-                               "could not get request pin %d on device %s\n",
+                               "could not request pin %d on device %s\n",
                                pins[i], pinctrl_dev_get_name(pctldev));
                        /* On error release all taken pins */
                        i--; /* this pin just failed */