Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

extcon: Fix extcon_get_extcon_dev() error handling

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>

authored by

Dan Carpenter and committed by
Chanwoo Choi
58e4a2d2 672c0c51

+28 -31
+2 -2
drivers/extcon/extcon-axp288.c
··· 394 394 if (adev) { 395 395 info->id_extcon = extcon_get_extcon_dev(acpi_dev_name(adev)); 396 396 put_device(&adev->dev); 397 - if (!info->id_extcon) 398 - return -EPROBE_DEFER; 397 + if (IS_ERR(info->id_extcon)) 398 + return PTR_ERR(info->id_extcon); 399 399 400 400 dev_info(dev, "controlling USB role\n"); 401 401 } else {
+3 -1
drivers/extcon/extcon.c
··· 851 851 * @extcon_name: the extcon name provided with extcon_dev_register() 852 852 * 853 853 * Return the pointer of extcon device if success or ERR_PTR(err) if fail. 854 + * NOTE: This function returns -EPROBE_DEFER so it may only be called from 855 + * probe() functions. 854 856 */ 855 857 struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name) 856 858 { ··· 866 864 if (!strcmp(sd->name, extcon_name)) 867 865 goto out; 868 866 } 869 - sd = NULL; 867 + sd = ERR_PTR(-EPROBE_DEFER); 870 868 out: 871 869 mutex_unlock(&extcon_dev_list_lock); 872 870 return sd;
+10 -7
drivers/power/supply/axp288_charger.c
··· 865 865 info->regmap_irqc = axp20x->regmap_irqc; 866 866 867 867 info->cable.edev = extcon_get_extcon_dev(AXP288_EXTCON_DEV_NAME); 868 - if (info->cable.edev == NULL) { 869 - dev_dbg(dev, "%s is not ready, probe deferred\n", 870 - AXP288_EXTCON_DEV_NAME); 871 - return -EPROBE_DEFER; 868 + if (IS_ERR(info->cable.edev)) { 869 + dev_err_probe(dev, PTR_ERR(info->cable.edev), 870 + "extcon_get_extcon_dev(%s) failed\n", 871 + AXP288_EXTCON_DEV_NAME); 872 + return PTR_ERR(info->cable.edev); 872 873 } 873 874 874 875 if (acpi_dev_present(USB_HOST_EXTCON_HID, NULL, -1)) { 875 876 info->otg.cable = extcon_get_extcon_dev(USB_HOST_EXTCON_NAME); 876 - if (info->otg.cable == NULL) { 877 - dev_dbg(dev, "EXTCON_USB_HOST is not ready, probe deferred\n"); 878 - return -EPROBE_DEFER; 877 + if (IS_ERR(info->otg.cable)) { 878 + dev_err_probe(dev, PTR_ERR(info->otg.cable), 879 + "extcon_get_extcon_dev(%s) failed\n", 880 + USB_HOST_EXTCON_NAME); 881 + return PTR_ERR(info->otg.cable); 879 882 } 880 883 dev_info(dev, "Using " USB_HOST_EXTCON_HID " extcon for usb-id\n"); 881 884 }
+2 -5
drivers/power/supply/charger-manager.c
··· 985 985 cable->nb.notifier_call = charger_extcon_notifier; 986 986 987 987 cable->extcon_dev = extcon_get_extcon_dev(cable->extcon_name); 988 - if (IS_ERR_OR_NULL(cable->extcon_dev)) { 988 + if (IS_ERR(cable->extcon_dev)) { 989 989 pr_err("Cannot find extcon_dev for %s (cable: %s)\n", 990 990 cable->extcon_name, cable->name); 991 - if (cable->extcon_dev == NULL) 992 - return -EPROBE_DEFER; 993 - else 994 - return PTR_ERR(cable->extcon_dev); 991 + return PTR_ERR(cable->extcon_dev); 995 992 } 996 993 997 994 for (i = 0; i < ARRAY_SIZE(extcon_mapping); i++) {
+4 -4
drivers/power/supply/max8997_charger.c
··· 242 242 dev_info(&pdev->dev, "couldn't get charger regulator\n"); 243 243 } 244 244 charger->edev = extcon_get_extcon_dev("max8997-muic"); 245 - if (IS_ERR_OR_NULL(charger->edev)) { 246 - if (!charger->edev) 247 - return -EPROBE_DEFER; 248 - dev_info(charger->dev, "couldn't get extcon device\n"); 245 + if (IS_ERR(charger->edev)) { 246 + dev_err_probe(charger->dev, PTR_ERR(charger->edev), 247 + "couldn't get extcon device: max8997-muic\n"); 248 + return PTR_ERR(charger->edev); 249 249 } 250 250 251 251 if (!IS_ERR(charger->reg) && !IS_ERR_OR_NULL(charger->edev)) {
+2 -7
drivers/usb/dwc3/drd.c
··· 455 455 * This device property is for kernel internal use only and 456 456 * is expected to be set by the glue code. 457 457 */ 458 - if (device_property_read_string(dev, "linux,extcon-name", &name) == 0) { 459 - edev = extcon_get_extcon_dev(name); 460 - if (!edev) 461 - return ERR_PTR(-EPROBE_DEFER); 462 - 463 - return edev; 464 - } 458 + if (device_property_read_string(dev, "linux,extcon-name", &name) == 0) 459 + return extcon_get_extcon_dev(name); 465 460 466 461 /* 467 462 * Try to get an extcon device from the USB PHY controller's "port"
+2 -2
drivers/usb/phy/phy-omap-otg.c
··· 95 95 return -ENODEV; 96 96 97 97 extcon = extcon_get_extcon_dev(config->extcon); 98 - if (!extcon) 99 - return -EPROBE_DEFER; 98 + if (IS_ERR(extcon)) 99 + return PTR_ERR(extcon); 100 100 101 101 otg_dev = devm_kzalloc(&pdev->dev, sizeof(*otg_dev), GFP_KERNEL); 102 102 if (!otg_dev)
+2 -2
drivers/usb/typec/tcpm/fusb302.c
··· 1708 1708 */ 1709 1709 if (device_property_read_string(dev, "linux,extcon-name", &name) == 0) { 1710 1710 chip->extcon = extcon_get_extcon_dev(name); 1711 - if (!chip->extcon) 1712 - return -EPROBE_DEFER; 1711 + if (IS_ERR(chip->extcon)) 1712 + return PTR_ERR(chip->extcon); 1713 1713 } 1714 1714 1715 1715 chip->vbus = devm_regulator_get(chip->dev, "vbus");
+1 -1
include/linux/extcon.h
··· 296 296 297 297 static inline struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name) 298 298 { 299 - return ERR_PTR(-ENODEV); 299 + return NULL; 300 300 } 301 301 302 302 static inline struct extcon_dev *extcon_find_edev_by_node(struct device_node *node)