All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Dan Carpenter <dan.carpenter@oracle.com>,
	Hans de Goede <hdegoede@redhat.com>,
	Heikki Krogerus <heikki.krogerus@linux.intel.com>,
	Guenter Roeck <linux@roeck-us.net>,
	Sebastian Reichel <sebastian.reichel@collabora.com>,
	Chanwoo Choi <cw00.choi@samsung.com>,
	Sasha Levin <sashal@kernel.org>,
	myungjoo.ham@samsung.com, wens@csie.org, sre@kernel.org,
	balbi@kernel.org, gregkh@linuxfoundation.org,
	linux-pm@vger.kernel.org, linux-usb@vger.kernel.org,
	linux-omap@vger.kernel.org
Subject: [PATCH AUTOSEL 5.15 21/51] extcon: Fix extcon_get_extcon_dev() error handling
Date: Tue,  7 Jun 2022 13:55:20 -0400	[thread overview]
Message-ID: <20220607175552.479948-21-sashal@kernel.org> (raw)
In-Reply-To: <20220607175552.479948-1-sashal@kernel.org>

From: Dan Carpenter <dan.carpenter@oracle.com>

[ 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>
---
 drivers/extcon/extcon-axp288.c         |  4 ++--
 drivers/extcon/extcon.c                |  4 +++-
 drivers/power/supply/axp288_charger.c  | 17 ++++++++++-------
 drivers/power/supply/charger-manager.c |  7 ++-----
 drivers/power/supply/max8997_charger.c |  8 ++++----
 drivers/usb/dwc3/drd.c                 |  9 ++-------
 drivers/usb/phy/phy-omap-otg.c         |  4 ++--
 drivers/usb/typec/tcpm/fusb302.c       |  4 ++--
 include/linux/extcon.h                 |  2 +-
 9 files changed, 28 insertions(+), 31 deletions(-)

diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
index fdb31954cf2b..8073bc7d3e61 100644
--- 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 e7a9561a826d..9eb92997f3ae 100644
--- 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 fb9db7f43895..22378dad4d9f 100644
--- 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 d67edb760c94..92db79400a6a 100644
--- 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 25207fe2aa68..bfa7a576523d 100644
--- 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 f148b0370f82..81ff21bd405a 100644
--- 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 ee0863c6553e..6e6ef8c0bc7e 100644
--- 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 72f9001b0792..96c55eaf3f80 100644
--- 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 0c19010da77f..685401d94d39 100644
--- 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)
-- 
2.35.1


  parent reply	other threads:[~2022-06-07 19:28 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-07 17:55 [PATCH AUTOSEL 5.15 01/51] iio: dummy: iio_simple_dummy: check the return value of kstrdup() Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 02/51] staging: rtl8712: fix a potential memory leak in r871xu_drv_init() Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 03/51] iio: st_sensors: Add a local lock for protecting odr Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 04/51] lkdtm/usercopy: Expand size of "out of frame" object Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 05/51] drivers: staging: rtl8723bs: Fix deadlock in rtw_surveydone_event_callback() Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 06/51] drivers: staging: rtl8192bs: Fix deadlock in rtw_joinbss_event_prehandle() Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 07/51] tty: synclink_gt: Fix null-pointer-dereference in slgt_clean() Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 08/51] tty: Fix a possible resource leak in icom_probe Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 09/51] thunderbolt: Use different lane for second DisplayPort tunnel Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 10/51] drivers: staging: rtl8192u: Fix deadlock in ieee80211_beacons_stop() Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 11/51] drivers: staging: rtl8192e: Fix deadlock in rtllib_beacons_stop() Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 12/51] USB: host: isp116x: check return value after calling platform_get_resource() Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 13/51] drivers: tty: serial: Fix deadlock in sa1100_set_termios() Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 14/51] drivers: usb: host: Fix deadlock in oxu_bus_suspend() Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 15/51] USB: hcd-pci: Fully suspend across freeze/thaw cycle Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 16/51] char: xillybus: fix a refcount leak in cleanup_dev() Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 17/51] sysrq: do not omit current cpu when showing backtrace of all active CPUs Sasha Levin
2022-06-07 17:55   ` Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 18/51] usb: dwc2: gadget: don't reset gadget's driver->bus Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 19/51] soundwire: qcom: adjust autoenumeration timeout Sasha Levin
2022-06-07 17:55   ` Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 20/51] misc: rtsx: set NULL intfdata when probe fails Sasha Levin
2022-06-07 17:55 ` Sasha Levin [this message]
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 22/51] extcon: Modify extcon device to be created after driver data is set Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 23/51] clocksource/drivers/sp804: Avoid error on multiple instances Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 24/51] staging: rtl8723bs: Fix alignment to match open parenthesis Sasha Levin
2022-06-08  0:04   ` Joe Perches
2022-06-08  6:15     ` Greg Kroah-Hartman
2022-06-09 13:55       ` Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 25/51] staging: rtl8712: fix uninit-value in usb_read8() and friends Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 26/51] staging: rtl8712: fix uninit-value in r871xu_drv_init() Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 27/51] serial: msm_serial: disable interrupts in __msm_console_write() Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 28/51] accessiblity: speakup: Add missing misc_deregister in softsynth_probe Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 29/51] kernfs: Separate kernfs_pr_cont_buf and rename_lock Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 30/51] watchdog: wdat_wdt: Stop watchdog when rebooting the system Sasha Levin
2022-06-07 17:55 ` [dm-devel] [PATCH AUTOSEL 5.15 31/51] md: don't unregister sync_thread with reconfig_mutex held Sasha Levin
2022-06-07 17:55   ` Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 32/51] md: protect md_unregister_thread from reentrancy Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 33/51] scsi: myrb: Fix up null pointer access on myrb_cleanup() Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 34/51] Revert "net: af_key: add check for pfkey_broadcast in function pfkey_process" Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 35/51] ceph: allow ceph.dir.rctime xattr to be updatable Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 36/51] ceph: flush the mdlog for filesystem sync Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 37/51] drm/amd/display: Check if modulo is 0 before dividing Sasha Levin
2022-06-07 17:55   ` Sasha Levin
2022-06-07 17:55   ` Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 38/51] drm/radeon: fix a possible null pointer dereference Sasha Levin
2022-06-07 17:55   ` Sasha Levin
2022-06-07 17:55   ` Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 39/51] drm/amd/pm: Fix missing thermal throttler status Sasha Levin
2022-06-07 17:55   ` Sasha Levin
2022-06-07 17:55   ` Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 40/51] um: line: Use separate IRQs per line Sasha Levin
2022-06-07 17:55   ` Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 41/51] modpost: fix undefined behavior of is_arm_mapping_symbol() Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 42/51] x86/cpu: Elide KCSAN for cpu_has() and friends Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 43/51] jump_label,noinstr: Avoid instrumentation for JUMP_LABEL=n builds Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 44/51] nbd: call genl_unregister_family() first in nbd_cleanup() Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 45/51] nbd: fix race between nbd_alloc_config() and module removal Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 46/51] nbd: fix io hung while disconnecting device Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 47/51] fs/ntfs3: Fix invalid free in log_replay Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 48/51] s390/gmap: voluntarily schedule during key setting Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 49/51] cifs: version operations for smb20 unneeded when legacy support disabled Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 50/51] drm/amd/pm: use bitmap_{from, to}_arr32 where appropriate Sasha Levin
2022-06-07 17:55   ` [PATCH AUTOSEL 5.15 50/51] drm/amd/pm: use bitmap_{from,to}_arr32 " Sasha Levin
2022-06-07 17:55   ` [PATCH AUTOSEL 5.15 50/51] drm/amd/pm: use bitmap_{from, to}_arr32 " Sasha Levin
2022-06-07 17:55 ` [PATCH AUTOSEL 5.15 51/51] nodemask: Fix return values to be unsigned Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220607175552.479948-21-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=balbi@kernel.org \
    --cc=cw00.choi@samsung.com \
    --cc=dan.carpenter@oracle.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hdegoede@redhat.com \
    --cc=heikki.krogerus@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=myungjoo.ham@samsung.com \
    --cc=sebastian.reichel@collabora.com \
    --cc=sre@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=wens@csie.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.