extcon: Fix extcon_get_extcon_dev() error handling

[ Upstream commit 58e4a2d27d3255e4e8c507fdc13734dccc9fc4c7 ]

The extcon_get_extcon_dev() function returns error pointers on error,
NULL when it's a -EPROBE_DEFER defer situation, and ERR_PTR(-ENODEV)
when the CONFIG_EXTCON option is disabled.  This is very complicated for
the callers to handle and a number of them had bugs that would lead to
an Oops.

In real life, there are two things which prevented crashes.  First,
error pointers would only be returned if there was bug in the caller
where they passed a NULL "extcon_name" and none of them do that.
Second, only two out of the eight drivers will build when CONFIG_EXTCON
is disabled.

The normal way to write this would be to return -EPROBE_DEFER directly
when appropriate and return NULL when CONFIG_EXTCON is disabled.  Then
the error handling is simple and just looks like:

	dev->edev = extcon_get_extcon_dev(acpi_dev_name(adev));
	if (IS_ERR(dev->edev))
		return PTR_ERR(dev->edev);

For the two drivers which can build with CONFIG_EXTCON disabled, then
extcon_get_extcon_dev() will now return NULL which is not treated as an
error and the probe will continue successfully.  Those two drivers are
"typec_fusb302" and "max8997-battery".  In the original code, the
typec_fusb302 driver had an 800ms hang in tcpm_get_current_limit() but
now that function is a no-op.  For the max8997-battery driver everything
should continue working as is.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
index fdb31954cf2b6f7346fda83fcf06e525cc381653..8073bc7d3e615273db4722fa666a85c34182f9d0 100644 (file)
--- a/drivers/extcon/extcon-axp288.c
+++ b/drivers/extcon/extcon-axp288.c
@@ -375,8 +375,8 @@ static int axp288_extcon_probe(struct platform_device *pdev)
                if (adev) {
                        info->id_extcon = extcon_get_extcon_dev(acpi_dev_name(adev));
                        put_device(&adev->dev);
-                       if (!info->id_extcon)
-                               return -EPROBE_DEFER;
+                       if (IS_ERR(info->id_extcon))
+                               return PTR_ERR(info->id_extcon);
 
                        dev_info(dev, "controlling USB role\n");
                } else {

diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index e7a9561a826d3908a8c0ffb3c7e61f8c9ac856d4..9eb92997f3aed0cc27c5a1c476a0d230b8119d65 100644 (file)
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -863,6 +863,8 @@ EXPORT_SYMBOL_GPL(extcon_set_property_capability);
  * @extcon_name:       the extcon name provided with extcon_dev_register()
  *
  * Return the pointer of extcon device if success or ERR_PTR(err) if fail.
+ * NOTE: This function returns -EPROBE_DEFER so it may only be called from
+ * probe() functions.
  */
 struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name)
 {
@@ -876,7 +878,7 @@ struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name)
                if (!strcmp(sd->name, extcon_name))
                        goto out;
        }
-       sd = NULL;
+       sd = ERR_PTR(-EPROBE_DEFER);
 out:
        mutex_unlock(&extcon_dev_list_lock);
        return sd;

diff --git a/drivers/power/supply/axp288_charger.c b/drivers/power/supply/axp288_charger.c
index fb9db7f4389518c43586436f194e1de0acf60cb2..22378dad4d9fc5832a5b05db08817138d51f0cb9 100644 (file)
--- a/drivers/power/supply/axp288_charger.c
+++ b/drivers/power/supply/axp288_charger.c
@@ -832,17 +832,20 @@ static int axp288_charger_probe(struct platform_device *pdev)
        info->regmap_irqc = axp20x->regmap_irqc;
 
        info->cable.edev = extcon_get_extcon_dev(AXP288_EXTCON_DEV_NAME);
-       if (info->cable.edev == NULL) {
-               dev_dbg(dev, "%s is not ready, probe deferred\n",
-                       AXP288_EXTCON_DEV_NAME);
-               return -EPROBE_DEFER;
+       if (IS_ERR(info->cable.edev)) {
+               dev_err_probe(dev, PTR_ERR(info->cable.edev),
+                             "extcon_get_extcon_dev(%s) failed\n",
+                             AXP288_EXTCON_DEV_NAME);
+               return PTR_ERR(info->cable.edev);
        }
 
        if (acpi_dev_present(USB_HOST_EXTCON_HID, NULL, -1)) {
                info->otg.cable = extcon_get_extcon_dev(USB_HOST_EXTCON_NAME);
-               if (info->otg.cable == NULL) {
-                       dev_dbg(dev, "EXTCON_USB_HOST is not ready, probe deferred\n");
-                       return -EPROBE_DEFER;
+               if (IS_ERR(info->otg.cable)) {
+                       dev_err_probe(dev, PTR_ERR(info->otg.cable),
+                                     "extcon_get_extcon_dev(%s) failed\n",
+                                     USB_HOST_EXTCON_NAME);
+                       return PTR_ERR(info->otg.cable);
                }
                dev_info(dev, "Using " USB_HOST_EXTCON_HID " extcon for usb-id\n");
        }

diff --git a/drivers/power/supply/charger-manager.c b/drivers/power/supply/charger-manager.c
index d67edb760c948dbb9c0e665ecfae5c9ae5a8a7af..92db79400a6adee2701325a57df51f0cbb332292 100644 (file)
--- a/drivers/power/supply/charger-manager.c
+++ b/drivers/power/supply/charger-manager.c
@@ -985,13 +985,10 @@ static int charger_extcon_init(struct charger_manager *cm,
        cable->nb.notifier_call = charger_extcon_notifier;
 
        cable->extcon_dev = extcon_get_extcon_dev(cable->extcon_name);
-       if (IS_ERR_OR_NULL(cable->extcon_dev)) {
+       if (IS_ERR(cable->extcon_dev)) {
                pr_err("Cannot find extcon_dev for %s (cable: %s)\n",
                        cable->extcon_name, cable->name);
-               if (cable->extcon_dev == NULL)
-                       return -EPROBE_DEFER;
-               else
-                       return PTR_ERR(cable->extcon_dev);
+               return PTR_ERR(cable->extcon_dev);
        }
 
        for (i = 0; i < ARRAY_SIZE(extcon_mapping); i++) {

diff --git a/drivers/power/supply/max8997_charger.c b/drivers/power/supply/max8997_charger.c
index 25207fe2aa68ef4a28c1191e36a607a45de9b4fd..bfa7a576523df2615d414dd44a5aeab37aaa1528 100644 (file)
--- a/drivers/power/supply/max8997_charger.c
+++ b/drivers/power/supply/max8997_charger.c
@@ -248,10 +248,10 @@ static int max8997_battery_probe(struct platform_device *pdev)
                dev_info(&pdev->dev, "couldn't get charger regulator\n");
        }
        charger->edev = extcon_get_extcon_dev("max8997-muic");
-       if (IS_ERR_OR_NULL(charger->edev)) {
-               if (!charger->edev)
-                       return -EPROBE_DEFER;
-               dev_info(charger->dev, "couldn't get extcon device\n");
+       if (IS_ERR(charger->edev)) {
+               dev_err_probe(charger->dev, PTR_ERR(charger->edev),
+                             "couldn't get extcon device: max8997-muic\n");
+               return PTR_ERR(charger->edev);
        }
 
        if (!IS_ERR(charger->reg) && !IS_ERR_OR_NULL(charger->edev)) {

diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
index f148b0370f829d452f23c865acceda4ad058bb88..81ff21bd405a869dc39e9dd212ae1a70cb1e8bc8 100644 (file)
--- a/drivers/usb/dwc3/drd.c
+++ b/drivers/usb/dwc3/drd.c
@@ -454,13 +454,8 @@ static struct extcon_dev *dwc3_get_extcon(struct dwc3 *dwc)
         * This device property is for kernel internal use only and
         * is expected to be set by the glue code.
         */
-       if (device_property_read_string(dev, "linux,extcon-name", &name) == 0) {
-               edev = extcon_get_extcon_dev(name);
-               if (!edev)
-                       return ERR_PTR(-EPROBE_DEFER);
-
-               return edev;
-       }
+       if (device_property_read_string(dev, "linux,extcon-name", &name) == 0)
+               return extcon_get_extcon_dev(name);
 
        /*
         * Try to get an extcon device from the USB PHY controller's "port"

diff --git a/drivers/usb/phy/phy-omap-otg.c b/drivers/usb/phy/phy-omap-otg.c
index ee0863c6553edf320428cda85b4974207aecae65..6e6ef8c0bc7ed7e4da07af64008a2ed7ce138a6f 100644 (file)
--- a/drivers/usb/phy/phy-omap-otg.c
+++ b/drivers/usb/phy/phy-omap-otg.c
@@ -95,8 +95,8 @@ static int omap_otg_probe(struct platform_device *pdev)
                return -ENODEV;
 
        extcon = extcon_get_extcon_dev(config->extcon);
-       if (!extcon)
-               return -EPROBE_DEFER;
+       if (IS_ERR(extcon))
+               return PTR_ERR(extcon);
 
        otg_dev = devm_kzalloc(&pdev->dev, sizeof(*otg_dev), GFP_KERNEL);
        if (!otg_dev)

diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
index 72f9001b07921c96c4f3a91937d1e622fb0edafa..96c55eaf3f808de4ccac0e84192a426ec2273d33 100644 (file)
--- a/drivers/usb/typec/tcpm/fusb302.c
+++ b/drivers/usb/typec/tcpm/fusb302.c
@@ -1708,8 +1708,8 @@ static int fusb302_probe(struct i2c_client *client,
         */
        if (device_property_read_string(dev, "linux,extcon-name", &name) == 0) {
                chip->extcon = extcon_get_extcon_dev(name);
-               if (!chip->extcon)
-                       return -EPROBE_DEFER;
+               if (IS_ERR(chip->extcon))
+                       return PTR_ERR(chip->extcon);
        }
 
        chip->vbus = devm_regulator_get(chip->dev, "vbus");

diff --git a/include/linux/extcon.h b/include/linux/extcon.h
index 0c19010da77fa1633d59a6e379d4d436bd0b0525..685401d94d39819474861b574326209086a2b6d3 100644 (file)
--- a/include/linux/extcon.h
+++ b/include/linux/extcon.h
@@ -296,7 +296,7 @@ static inline void devm_extcon_unregister_notifier_all(struct device *dev,
 
 static inline struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name)
 {
-       return ERR_PTR(-ENODEV);
+       return NULL;
 }
 
 static inline struct extcon_dev *extcon_find_edev_by_node(struct device_node *node)