pinctrl: core: Fix regression caused by delayed work for hogs

Commit df61b366af26 ("pinctrl: core: Use delayed work for hogs") caused a
regression at least with sh-pfc that is also a GPIO controller as
noted by Geert Uytterhoeven <geert@linux-m68k.org>.

As the original pinctrl_register() has issues calling pin controller
driver functions early before the controller has finished registering,
we can't just revert commit df61b366af26. That would break the drivers
using GENERIC_PINCTRL_GROUPS or GENERIC_PINMUX_FUNCTIONS.

So let's fix the issue with the following steps as a single patch:

1. Revert the late_init parts of commit df61b366af26.

   The late_init clearly won't work and we have to just give up
   on fixing pinctrl_register() for GENERIC_PINCTRL_GROUPS and
   GENERIC_PINMUX_FUNCTIONS.

2. Split pinctrl_register() into two parts

   By splitting pinctrl_register() into pinctrl_init_controller()
   and pinctrl_create_and_start() we have better control over when
   it's safe to call pinctrl_create().

3. Introduce a new pinctrl_register_and_init() function

   As suggested by Linus Walleij <linus.walleij@linaro.org>, we
   can just introduce a new function for the controllers that need
   pinctrl_create() called later.

4. Convert the four known problem cases to use new function

   Let's convert pinctrl-imx, pinctrl-single, sh-pfc and ti-iodelay
   to use the new function to fix the issues. The rest of the drivers
   can be converted later. Let's also update Documentation/pinctrl.txt
   accordingly because of the known issues with pinctrl_register().

Fixes: df61b366af26 ("pinctrl: core: Use delayed work for hogs")
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Gary Bisson <gary.bisson@boundarydevices.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

8 files changed, 165 insertions, 79 deletions

diff --git a/Documentation/pinctrl.txt b/Documentation/pinctrl.txt
index 0d3b9ce0a0b9..54bd5faa8782 100644
--- a/Documentation/pinctrl.txt
+++ b/Documentation/pinctrl.txt
@@ -79,9 +79,7 @@ int __init foo_probe(void)
 {
         struct pinctrl_dev *pctl;
 
-        pctl = pinctrl_register(&foo_desc, <PARENT>, NULL);
-        if (!pctl)
-                pr_err("could not register foo pin driver\n");
+        return pinctrl_register_and_init(&foo_desc, <PARENT>, NULL, &pctl);
 }
 
 To enable the pinctrl subsystem and the subgroups for PINMUX and PINCONF and
diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index 65c0ae0969dc..bc73a63163ee 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -1905,59 +1905,14 @@ static int pinctrl_check_ops(struct pinctrl_dev *pctldev)
 }
 
 /**
- * pinctrl_late_init() - finish pin controller device registration
- * @work: work struct
- */
-static void pinctrl_late_init(struct work_struct *work)
-{
-        struct pinctrl_dev *pctldev;
-
-        pctldev = container_of(work, struct pinctrl_dev, late_init.work);
-
-        /*
-         * If the pin controller does NOT have hogs, this will report an
-         * error and we skip over this entire branch. This is why we can
-         * call this function directly when we do not have hogs on the
-         * device.
-         */
-        pctldev->p = create_pinctrl(pctldev->dev, pctldev);
-        if (!IS_ERR(pctldev->p)) {
-                kref_get(&pctldev->p->users);
-                pctldev->hog_default =
-                        pinctrl_lookup_state(pctldev->p, PINCTRL_STATE_DEFAULT);
-                if (IS_ERR(pctldev->hog_default)) {
-                        dev_dbg(pctldev->dev,
-                                "failed to lookup the default state\n");
-                } else {
-                        if (pinctrl_select_state(pctldev->p,
-                                                 pctldev->hog_default))
-                                dev_err(pctldev->dev,
-                                        "failed to select default state\n");
-                }
-
-                pctldev->hog_sleep =
-                        pinctrl_lookup_state(pctldev->p,
-                                             PINCTRL_STATE_SLEEP);
-                if (IS_ERR(pctldev->hog_sleep))
-                        dev_dbg(pctldev->dev,
-                                "failed to lookup the sleep state\n");
-        }
-
-        mutex_lock(&pinctrldev_list_mutex);
-        list_add_tail(&pctldev->node, &pinctrldev_list);
-        mutex_unlock(&pinctrldev_list_mutex);
-
-        pinctrl_init_device_debugfs(pctldev);
-}
-
-/**
- * pinctrl_register() - register a pin controller device
+ * pinctrl_init_controller() - init a pin controller device
  * @pctldesc: descriptor for this pin controller
  * @dev: parent device for this pin controller
  * @driver_data: private pin controller data for this pin controller
  */
-struct pinctrl_dev *pinctrl_register(struct pinctrl_desc *pctldesc,
-                                    struct device *dev, void *driver_data)
+struct pinctrl_dev *pinctrl_init_controller(struct pinctrl_desc *pctldesc,
+                                            struct device *dev,
+                                            void *driver_data)
 {
         struct pinctrl_dev *pctldev;
         int ret;
@@ -1983,7 +1938,6 @@ struct pinctrl_dev *pinctrl_register(struct pinctrl_desc *pctldesc,
         INIT_RADIX_TREE(&pctldev->pin_function_tree, GFP_KERNEL);
 #endif
         INIT_LIST_HEAD(&pctldev->gpio_ranges);
-        INIT_DELAYED_WORK(&pctldev->late_init, pinctrl_late_init);
         pctldev->dev = dev;
         mutex_init(&pctldev->mutex);
 
@@ -2018,17 +1972,6 @@ struct pinctrl_dev *pinctrl_register(struct pinctrl_desc *pctldesc,
                 goto out_err;
         }
 
-        /*
-         * If the device has hogs we want the probe() function of the driver
-         * to complete before we go in and hog them and add the pin controller
-         * to the list of controllers. If it has no hogs, we can just complete
-         * the registration immediately.
-         */
-        if (pinctrl_dt_has_hogs(pctldev))
-                schedule_delayed_work(&pctldev->late_init, 0);
-        else
-                pinctrl_late_init(&pctldev->late_init.work);
-
         return pctldev;
 
 out_err:
@@ -2036,8 +1979,107 @@ out_err:
         kfree(pctldev);
         return ERR_PTR(ret);
 }
+
+static int pinctrl_create_and_start(struct pinctrl_dev *pctldev)
+{
+        pctldev->p = create_pinctrl(pctldev->dev, pctldev);
+        if (!IS_ERR(pctldev->p)) {
+                kref_get(&pctldev->p->users);
+                pctldev->hog_default =
+                        pinctrl_lookup_state(pctldev->p, PINCTRL_STATE_DEFAULT);
+                if (IS_ERR(pctldev->hog_default)) {
+                        dev_dbg(pctldev->dev,
+                                "failed to lookup the default state\n");
+                } else {
+                        if (pinctrl_select_state(pctldev->p,
+                                                pctldev->hog_default))
+                                dev_err(pctldev->dev,
+                                        "failed to select default state\n");
+                }
+
+                pctldev->hog_sleep =
+                        pinctrl_lookup_state(pctldev->p,
+                                                    PINCTRL_STATE_SLEEP);
+                if (IS_ERR(pctldev->hog_sleep))
+                        dev_dbg(pctldev->dev,
+                                "failed to lookup the sleep state\n");
+        }
+
+        mutex_lock(&pinctrldev_list_mutex);
+        list_add_tail(&pctldev->node, &pinctrldev_list);
+        mutex_unlock(&pinctrldev_list_mutex);
+
+        pinctrl_init_device_debugfs(pctldev);
+
+        return 0;
+}
+
+/**
+ * pinctrl_register() - register a pin controller device
+ * @pctldesc: descriptor for this pin controller
+ * @dev: parent device for this pin controller
+ * @driver_data: private pin controller data for this pin controller
+ *
+ * Note that pinctrl_register() is known to have problems as the pin
+ * controller driver functions are called before the driver has a
+ * struct pinctrl_dev handle. To avoid issues later on, please use the
+ * new pinctrl_register_and_init() below instead.
+ */
+struct pinctrl_dev *pinctrl_register(struct pinctrl_desc *pctldesc,
+                                    struct device *dev, void *driver_data)
+{
+        struct pinctrl_dev *pctldev;
+        int error;
+
+        pctldev = pinctrl_init_controller(pctldesc, dev, driver_data);
+        if (IS_ERR(pctldev))
+                return pctldev;
+
+        error = pinctrl_create_and_start(pctldev);
+        if (error) {
+                mutex_destroy(&pctldev->mutex);
+                kfree(pctldev);
+
+                return ERR_PTR(error);
+        }
+
+        return pctldev;
+
+}
 EXPORT_SYMBOL_GPL(pinctrl_register);
 
+int pinctrl_register_and_init(struct pinctrl_desc *pctldesc,
+                              struct device *dev, void *driver_data,
+                              struct pinctrl_dev **pctldev)
+{
+        struct pinctrl_dev *p;
+        int error;
+
+        p = pinctrl_init_controller(pctldesc, dev, driver_data);
+        if (IS_ERR(p))
+                return PTR_ERR(p);
+
+        /*
+         * We have pinctrl_start() call functions in the pin controller
+         * driver with create_pinctrl() for at least dt_node_to_map(). So
+         * let's make sure pctldev is properly initialized for the
+         * pin controller driver before we do anything.
+         */
+        *pctldev = p;
+
+        error = pinctrl_create_and_start(p);
+        if (error) {
+                mutex_destroy(&p->mutex);
+                kfree(p);
+                *pctldev = NULL;
+
+                return error;
+        }
+
+        return 0;
+}
+EXPORT_SYMBOL_GPL(pinctrl_register_and_init);
+
 /**
  * pinctrl_unregister() - unregister pinmux
  * @pctldev: pin controller to unregister
@@ -2052,7 +2094,6 @@ void pinctrl_unregister(struct pinctrl_dev *pctldev)
         if (pctldev == NULL)
                 return;
 
-        cancel_delayed_work_sync(&pctldev->late_init);
         mutex_lock(&pctldev->mutex);
         pinctrl_remove_device_debugfs(pctldev);
         mutex_unlock(&pctldev->mutex);
@@ -2134,6 +2175,42 @@ struct pinctrl_dev *devm_pinctrl_register(struct device *dev,
 EXPORT_SYMBOL_GPL(devm_pinctrl_register);
 
 /**
+ * devm_pinctrl_register_and_init() - Resource managed pinctrl register and init
+ * @dev: parent device for this pin controller
+ * @pctldesc: descriptor for this pin controller
+ * @driver_data: private pin controller data for this pin controller
+ *
+ * Returns an error pointer if pincontrol register failed. Otherwise
+ * it returns valid pinctrl handle.
+ *
+ * The pinctrl device will be automatically released when the device is unbound.
+ */
+int devm_pinctrl_register_and_init(struct device *dev,
+                                   struct pinctrl_desc *pctldesc,
+                                   void *driver_data,
+                                   struct pinctrl_dev **pctldev)
+{
+        struct pinctrl_dev **ptr;
+        int error;
+
+        ptr = devres_alloc(devm_pinctrl_dev_release, sizeof(*ptr), GFP_KERNEL);
+        if (!ptr)
+                return -ENOMEM;
+
+        error = pinctrl_register_and_init(pctldesc, dev, driver_data, pctldev);
+        if (error) {
+                devres_free(ptr);
+                return error;
+        }
+
+        *ptr = *pctldev;
+        devres_add(dev, ptr);
+
+        return 0;
+}
+EXPORT_SYMBOL_GPL(devm_pinctrl_register_and_init);
+
+/**
  * devm_pinctrl_unregister() - Resource managed version of pinctrl_unregister().
  * @dev: device for which which resource was allocated
  * @pctldev: the pinctrl device to unregister.
diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h
index ad812a2d7248..1c35de59a658 100644
--- a/drivers/pinctrl/core.h
+++ b/drivers/pinctrl/core.h
@@ -37,7 +37,6 @@ struct pinctrl_gpio_range;
  * @p: result of pinctrl_get() for this device
  * @hog_default: default state for pins hogged by this device
  * @hog_sleep: sleep state for pins hogged by this device
- * @late_init: delayed work for pin controller to finish registration
  * @mutex: mutex taken on each pin controller specific action
  * @device_root: debugfs root for this device
  */
@@ -60,7 +59,6 @@ struct pinctrl_dev {
         struct pinctrl *p;
         struct pinctrl_state *hog_default;
         struct pinctrl_state *hog_sleep;
-        struct delayed_work late_init;
         struct mutex mutex;
 #ifdef CONFIG_DEBUG_FS
         struct dentry *device_root;
diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
index bccd9416d44f..a7ace9e1ad81 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx.c
@@ -774,11 +774,11 @@ int imx_pinctrl_probe(struct platform_device *pdev,
         ipctl->info = info;
         ipctl->dev = info->dev;
         platform_set_drvdata(pdev, ipctl);
-        ipctl->pctl = devm_pinctrl_register(&pdev->dev,
-                                            imx_pinctrl_desc, ipctl);
-        if (IS_ERR(ipctl->pctl)) {
+        ret = devm_pinctrl_register_and_init(&pdev->dev,
+                                             imx_pinctrl_desc, ipctl,
+                                             &ipctl->pctl);
+        if (ret) {
                 dev_err(&pdev->dev, "could not register IMX pinctrl driver\n");
-                ret = PTR_ERR(ipctl->pctl);
                 goto free;
         }
 
diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index 8408263de466..d23e0e0aeb22 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -1747,10 +1747,9 @@ static int pcs_probe(struct platform_device *pdev)
         if (ret < 0)
                 goto free;
 
-        pcs->pctl = pinctrl_register(&pcs->desc, pcs->dev, pcs);
-        if (IS_ERR(pcs->pctl)) {
+        ret = pinctrl_register_and_init(&pcs->desc, pcs->dev, pcs, &pcs->pctl);
+        if (ret) {
                 dev_err(pcs->dev, "could not register single pinctrl driver\n");
-                ret = PTR_ERR(pcs->pctl);
                 goto free;
         }
 
diff --git a/drivers/pinctrl/sh-pfc/pinctrl.c b/drivers/pinctrl/sh-pfc/pinctrl.c
index fcacfa73ef6e..08150a321be6 100644
--- a/drivers/pinctrl/sh-pfc/pinctrl.c
+++ b/drivers/pinctrl/sh-pfc/pinctrl.c
@@ -816,6 +816,6 @@ int sh_pfc_register_pinctrl(struct sh_pfc *pfc)
         pmx->pctl_desc.pins = pmx->pins;
         pmx->pctl_desc.npins = pfc->info->nr_pins;
 
-        pmx->pctl = devm_pinctrl_register(pfc->dev, &pmx->pctl_desc, pmx);
-        return PTR_ERR_OR_ZERO(pmx->pctl);
+        return devm_pinctrl_register_and_init(pfc->dev, &pmx->pctl_desc, pmx,
+                                              &pmx->pctl);
 }
diff --git a/drivers/pinctrl/ti/pinctrl-ti-iodelay.c b/drivers/pinctrl/ti/pinctrl-ti-iodelay.c
index 3b86d3db7358..7f472f123515 100644
--- a/drivers/pinctrl/ti/pinctrl-ti-iodelay.c
+++ b/drivers/pinctrl/ti/pinctrl-ti-iodelay.c
@@ -891,10 +891,9 @@ static int ti_iodelay_probe(struct platform_device *pdev)
         iod->desc.name = dev_name(dev);
         iod->desc.owner = THIS_MODULE;
 
-        iod->pctl = pinctrl_register(&iod->desc, dev, iod);
-        if (!iod->pctl) {
+        ret = pinctrl_register_and_init(&iod->desc, dev, iod, &iod->pctl);
+        if (ret) {
                 dev_err(dev, "Failed to register pinctrl\n");
-                ret = -ENODEV;
                 goto exit_out;
         }
 
diff --git a/include/linux/pinctrl/pinctrl.h b/include/linux/pinctrl/pinctrl.h
index a42e57da270d..8ce2d87a238b 100644
--- a/include/linux/pinctrl/pinctrl.h
+++ b/include/linux/pinctrl/pinctrl.h
@@ -141,12 +141,27 @@ struct pinctrl_desc {
 };
 
 /* External interface to pin controller */
+
+extern int pinctrl_register_and_init(struct pinctrl_desc *pctldesc,
+                                     struct device *dev, void *driver_data,
+                                     struct pinctrl_dev **pctldev);
+
+/* Please use pinctrl_register_and_init() instead */
 extern struct pinctrl_dev *pinctrl_register(struct pinctrl_desc *pctldesc,
                                 struct device *dev, void *driver_data);
+
 extern void pinctrl_unregister(struct pinctrl_dev *pctldev);
+
+extern int devm_pinctrl_register_and_init(struct device *dev,
+                                struct pinctrl_desc *pctldesc,
+                                void *driver_data,
+                                struct pinctrl_dev **pctldev);
+
+/* Please use devm_pinctrl_register_and_init() instead */
 extern struct pinctrl_dev *devm_pinctrl_register(struct device *dev,
                                 struct pinctrl_desc *pctldesc,
                                 void *driver_data);
+
 extern void devm_pinctrl_unregister(struct device *dev,
                                 struct pinctrl_dev *pctldev);