[PATCH v2 1/2] driver core: bus: Fix drivers_probe_store() giving user wrong scanning result

Zijun Hu posted 2 patches 2 months, 2 weeks ago
There is a newer version of this series
[PATCH v2 1/2] driver core: bus: Fix drivers_probe_store() giving user wrong scanning result
Posted by Zijun Hu 2 months, 2 weeks ago
From: Zijun Hu <quic_zijuhu@quicinc.com>

drivers_probe_store(), as store() of bus attribute drivers_probe, may give
user wrong scanning result as explained below when user scans drivers for
a single device synchronously via the attribute:

- It wrongly collapses bus_rescan_devices_helper()'s various error
  return values as -EINVAL, that is not expected since user may want
  precise scanning result.

- It wrongly regards bus_rescan_devices_helper()'s return value 0 as
  success since the following failed cases also return 0:

  (1) the device is dead.
  (2) bus has no driver which match() the device.
  (3) bus fails to probe() the device with all its drivers.

Fixed by giving user precise scanning result or right prompt via
static bus_rescan_single_device() implemented for scanning drivers
for a single device.

Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 drivers/base/bus.c | 42 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 40 insertions(+), 2 deletions(-)

diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index 657c93c38b0d..4b5958c5ee7d 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -40,6 +40,24 @@ static struct kset *bus_kset;
 	struct driver_attribute driver_attr_##_name =		\
 		__ATTR_IGNORE_LOCKDEP(_name, _mode, _show, _store)
 
+/*
+ *  Bus rescans drivers for a single device, and derives from
+ *  bus_rescan_devices_helper(), but returns scanning result
+ *  as precise as possible.
+ */
+static int __must_check bus_rescan_single_device(struct device *dev)
+{
+	int ret;
+
+	if (dev->parent && dev->bus->need_parent_lock)
+		device_lock(dev->parent);
+	ret = device_attach(dev);
+	if (dev->parent && dev->bus->need_parent_lock)
+		device_unlock(dev->parent);
+
+	return ret;
+}
+
 static int __must_check bus_rescan_devices_helper(struct device *dev,
 						void *data);
 
@@ -310,13 +328,33 @@ static ssize_t drivers_probe_store(const struct bus_type *bus,
 				   const char *buf, size_t count)
 {
 	struct device *dev;
-	int err = -EINVAL;
+	int err;
+	int res;
 
 	dev = bus_find_device_by_name(bus, NULL, buf);
 	if (!dev)
 		return -ENODEV;
-	if (bus_rescan_devices_helper(dev, NULL) == 0)
+
+	res = bus_rescan_single_device(dev);
+	if (res < 0) {
+		/*
+		 * Give user known -EAGAIN in case of internal
+		 * -EPROBE_DEFER, and propagate error code upwards
+		 * as precise as possible.
+		 */
+		err = res == -EPROBE_DEFER ? -EAGAIN : res;
+	} else if (res > 0) {
 		err = count;
+	} else {
+		/*
+		 * Which error code to return for such synchronous
+		 * probing result ?
+		 */
+		dev_err(dev, "device '%s' fails to attach a driver\n",
+			dev_name(dev));
+		err = count;
+	}
+
 	put_device(dev);
 	return err;
 }

-- 
2.34.1