Skip to content

Commit

Permalink
extcon: Fix extcon_get_extcon_dev() error handling
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Dan Carpenter authored and Chanwoo Choi committed Apr 4, 2022
1 parent 3123109 commit 88490c7
Show file tree
Hide file tree
Showing 9 changed files with 28 additions and 31 deletions.
4 changes: 2 additions & 2 deletions drivers/extcon/extcon-axp288.c
Original file line number Diff line number Diff line change
Expand Up @@ -394,8 +394,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 {
Expand Down
4 changes: 3 additions & 1 deletion drivers/extcon/extcon.c
Original file line number Diff line number Diff line change
Expand Up @@ -851,6 +851,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)
{
Expand All @@ -864,7 +866,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;
Expand Down
17 changes: 10 additions & 7 deletions drivers/power/supply/axp288_charger.c
Original file line number Diff line number Diff line change
Expand Up @@ -865,17 +865,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");
}
Expand Down
7 changes: 2 additions & 5 deletions drivers/power/supply/charger-manager.c
Original file line number Diff line number Diff line change
Expand Up @@ -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++) {
Expand Down
8 changes: 4 additions & 4 deletions drivers/power/supply/max8997_charger.c
Original file line number Diff line number Diff line change
Expand Up @@ -242,10 +242,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)) {
Expand Down
9 changes: 2 additions & 7 deletions drivers/usb/dwc3/drd.c
Original file line number Diff line number Diff line change
Expand Up @@ -455,13 +455,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"
Expand Down
4 changes: 2 additions & 2 deletions drivers/usb/phy/phy-omap-otg.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions drivers/usb/typec/tcpm/fusb302.c
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
2 changes: 1 addition & 1 deletion include/linux/extcon.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 88490c7

Please sign in to comment.