[PATCH v3 0/18] On-demand device probing

[PATCH v3 0/18] On-demand device probing

[Tomeu Vizoso]

Hello,

I have a problem with the panel on my Tegra Chromebook taking longer
than expected to be ready during boot (Stéphane Marchesin reported what
is basically the same issue in [0]), and have looked into ordered
probing as a better way of solving this than moving nodes around in the
DT or playing with initcall levels and linking order.

While reading the thread [1] that Alexander Holler started with his
series to make probing order deterministic, it occurred to me that it
should be possible to achieve the same by probing devices as they are
referenced by other devices.

This basically reuses the information that is already implicit in the
probe() implementations, saving us from refactoring existing drivers or
adding information to DTBs.

During review of v1 of this series Linus Walleij suggested that it
should be the device driver core to make sure that dependencies are
ready before probing a device. I gave this idea a try [2] but Mark Brown
pointed out to the logic duplication between the resource acquisition
and dependency discovery code paths (though I think it's fairly minor).

To address that code duplication I experimented with Arnd's devm_probe
[3] concept of having drivers declare their dependencies instead of
acquiring them during probe, and while it worked [4], I don't think we
end up winning anything when compared to just probing devices on-demand
from resource getters.

One remaining objection is to the "sprinkling" of calls to
fwnode_ensure_device() in the resource getters of each subsystem, but I
think it's the right thing to do given that the storage of resources is
currently subsystem-specific.

We could avoid the above by moving resource storage into the core, but I
don't think there's a compelling case for that.

I have tested this on boards with Tegra, iMX.6, Exynos and OMAP SoCs,
and these patches were enough to eliminate all the deferred probes
(except one in PandaBoard because omap_dma_system doesn't have a
firmware node as of yet).

Have submitted a branch [5] with these patches to kernelci.org and I'm
currently trying to fix all regressions, usually due to code assuming
that devices will be probed in a specific order. Current results [6] are
348 passes, 30 fails and 42 unknowns (linux-next [7] is currently
387/3/23).

With this series I get the kernel to output to the panel in 0.5s,
instead of 2.8s.

Regards,

Tomeu

[0] http://lists.freedesktop.org/archives/dri-devel/2014-August/066527.html

[1] https://lkml.org/lkml/2014/5/12/452

[2] https://lkml.org/lkml/2015/6/17/305

[3] http://article.gmane.org/gmane.linux.ports.arm.kernel/277689

[4] https://lkml.org/lkml/2015/7/21/441a

[5] https://git.collabora.com/cgit/user/tomeu/linux.git/log/?h=on-demand-probes-v5

[6] http://kernelci.org/boot/all/job/collabora/kernel/v4.2-rc5-6548-g632b98c83840/

[7] http://kernelci.org/boot/all/job/next/kernel/next-20150806/

Changes in v3:
- Only delay platform devices with OF nodes
- Set and use device_node.platform_dev instead of reversing the logic to
  find the platform device that encloses a device node.
- Drop the fwnode API to probe firmware nodes and add OF-only API for
  now. I think this same scheme could be used for machines with ACPI,
  but I haven't been able to find one that had to defer its probes because
  of the device probe order.
- Avoid unlocking the regulator device's mutex if we don't have a device

Changes in v2:
- Move delay to platform.c
- Acquire regulator device lock before returning from regulator_dev_lookup()

Tomeu Vizoso (18):
  platform: delay OF device-driver matches until late_initcall
  of/platform: add of_platform_probe
  gpio: Probe GPIO drivers on demand
  gpio: Probe pinctrl devices on demand
  regulator: core: Reduce critical area in _regulator_get
  regulator: core: Probe regulators on demand
  drm: Probe panels on demand
  drm/tegra: Probe dpaux devices on demand
  i2c: core: Probe i2c adapters and devices on demand
  pwm: Probe PWM chip devices on demand
  backlight: Probe backlight devices on demand
  usb: phy: Probe phy devices on demand
  clk: Probe clk providers on demand
  pinctrl: Probe pinctrl devices on demand
  phy: core: Probe phy providers on demand
  dma: of: Probe DMA controllers on demand
  power-supply: Probe power supplies on demand
  ASoC: core: Probe components on demand

 drivers/base/platform.c             |  29 ++++++++++
 drivers/clk/clk.c                   |   3 +
 drivers/dma/of-dma.c                |   3 +
 drivers/gpio/gpiolib-of.c           |   5 ++
 drivers/gpu/drm/drm_panel.c         |   3 +
 drivers/gpu/drm/tegra/dpaux.c       |   3 +
 drivers/i2c/i2c-core.c              |   5 ++
 drivers/of/platform.c               |  61 +++++++++++++++++++++
 drivers/phy/phy-core.c              |   3 +
 drivers/pinctrl/devicetree.c        |   3 +
 drivers/power/power_supply_core.c   |   3 +
 drivers/pwm/core.c                  |   3 +
 drivers/regulator/core.c            | 106 +++++++++++++++++++++---------------
 drivers/usb/phy/phy.c               |   3 +
 drivers/video/backlight/backlight.c |   3 +
 include/linux/of.h                  |   1 +
 include/linux/of_platform.h         |   2 +
 sound/soc/soc-core.c                |   7 ++-
 18 files changed, 200 insertions(+), 46 deletions(-)

-- 
2.4.3

[PATCH v3 01/18] platform: delay OF device-driver matches until late_initcall

[Tomeu Vizoso]

Delay matches of platform devices with OF nodes until late_initcall,
when we are sure that all built-in drivers have been registered already.
This is needed to prevent deferred probes because of some drivers not
having registered yet.

The reason why only platform devices are delayed is that some other
devices are expected to be probed earlier than late_initcall, for
example, the system PNP driver needs to probe its devices in
fs_initcall.

Additionally, only platform devices with OF nodes are delayed because
some machines may depend on oter platform devices being registered at
specific times.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---

Changes in v3:
- Only delay platform devices with OF nodes

Changes in v2:
- Move delay to platform.c

 drivers/base/platform.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 063f0ab15259..0848ece2bf9d 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -33,6 +33,8 @@
 /* For automatically allocated device IDs */
 static DEFINE_IDA(platform_devid_ida);
 
+static bool enable_matches;
+
 struct device platform_bus = {
 	.init_name	= "platform",
 };
@@ -839,6 +841,15 @@ static int platform_match(struct device *dev, struct device_driver *drv)
 	struct platform_device *pdev = to_platform_device(dev);
 	struct platform_driver *pdrv = to_platform_driver(drv);
 
+	/*
+	 * Delay matches of OF platform devices until late_initcall, when we
+	 * are sure that all built-in drivers have been registered already.
+	 * This is needed to prevent deferred probes because of some drivers
+	 * not having registered yet.
+	 */
+	if (dev->of_node && !enable_matches)
+		return 0;
+
 	/* When driver_override is set, only bind to the matching driver */
 	if (pdev->driver_override)
 		return !strcmp(pdev->driver_override, drv->name);
@@ -859,6 +870,24 @@ static int platform_match(struct device *dev, struct device_driver *drv)
 	return (strcmp(pdev->name, drv->name) == 0);
 }
 
+static int __probe_device(struct device *dev, void *data)
+{
+	if (dev->of_node)
+		device_initial_probe(dev);
+
+	return 0;
+}
+
+static int probe_delayed_matches_initcall(void)
+{
+	enable_matches = true;
+
+	bus_for_each_dev(&platform_bus_type, NULL, NULL, __probe_device);
+
+	return 0;
+}
+late_initcall(probe_delayed_matches_initcall);
+
 #ifdef CONFIG_PM_SLEEP
 
 static int platform_legacy_suspend(struct device *dev, pm_message_t mesg)
-- 
2.4.3

[PATCH v3 02/18] of/platform: add of_platform_probe

[Tomeu Vizoso]

Walks the OF tree up and finds the closest ancestor that has a platform
device associated with it, probing it if isn't bound to a driver yet.

The above should ensure that the dependency represented by the passed OF
node is available, because probing a platform device should cause its
descendants to be probed as well.

Subsystems can use this when looking up resources for drivers, to reduce
the chances of deferred probes because of the probing order of devices.

Also adds a platform_dev member to struct device_node, pointing to the
platform device that was registered based on that node, if any.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---

Changes in v3:
- Set and use device_node.platform_dev instead of reversing the logic to
  find the platform device that encloses a device node.
- Drop the fwnode API to probe firmware nodes and add OF-only API for
  now. I think this same scheme could be used for machines with ACPI,
  but I haven't been able to find one that had to defer its probes because
  of the device probe order.

Changes in v2: None

 drivers/of/platform.c       | 61 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/of.h          |  1 +
 include/linux/of_platform.h |  2 ++
 3 files changed, 64 insertions(+)

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 8a002d6151f2..a1930c0d1fee 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -192,6 +192,8 @@ static struct platform_device *of_platform_device_create_pdata(
 		goto err_clear_flag;
 	}
 
+	np->platform_dev = dev;
+
 	return dev;
 
 err_clear_flag:
@@ -501,6 +503,65 @@ void of_platform_depopulate(struct device *parent)
 }
 EXPORT_SYMBOL_GPL(of_platform_depopulate);
 
+/**
+ * of_platform_probe() - Probe platform device associated with OF node
+ * @np: node to probe
+ *
+ * Walks the OF tree up and finds the closest ancestor that has a platform
+ * device associated with it, probing it if isn't bound to a driver yet.
+ *
+ * The above should ensure that the dependency represented by the passed OF
+ * node is available, because probing a platform device should cause its
+ * descendants to be probed as well.
+ */
+void of_platform_probe(struct device_node *np)
+{
+	struct device_node *target;
+	struct platform_device *pdev = NULL;
+
+	if (!of_root || !of_node_check_flag(of_root, OF_POPULATED_BUS))
+		return;
+
+	if (!np)
+		return;
+
+	of_node_get(np);
+
+	for (target = np;
+	     !of_node_is_root(target);
+	     target = of_get_next_parent(target))
+		if (target->platform_dev) {
+			pdev = target->platform_dev;
+			break;
+		}
+
+	of_node_put(target);
+
+	if (!pdev) {
+		pr_warn("Couldn't find a platform device for node '%s'\n",
+			of_node_full_name(np));
+		return;
+	}
+
+	/*
+	 * Device is bound or is being probed right now. If we have bad luck
+	 * and the dependency isn't ready when it's needed, deferred probe
+	 * will save us.
+	 */
+	if (pdev->dev.driver)
+		return;
+
+	if (device_attach(&pdev->dev) != 1)
+		/*
+		 * This cannot be a warning for now because clock nodes have a
+		 * compatible string but the clock framework doesn't follow the
+		 * device/driver model.
+		 */
+		pr_debug("Probe failed for %s (%s)\n", of_node_full_name(np),
+			 dev_name(&pdev->dev));
+}
+EXPORT_SYMBOL_GPL(of_platform_probe);
+
 #ifdef CONFIG_OF_DYNAMIC
 static int of_platform_notify(struct notifier_block *nb,
 				unsigned long action, void *arg)
diff --git a/include/linux/of.h b/include/linux/of.h
index edc068d19c79..2ace86a5fa2d 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -52,6 +52,7 @@ struct device_node {
 	phandle phandle;
 	const char *full_name;
 	struct fwnode_handle fwnode;
+	struct platform_device *platform_dev;
 
 	struct	property *properties;
 	struct	property *deadprops;	/* removed properties */
diff --git a/include/linux/of_platform.h b/include/linux/of_platform.h
index 611a691145c4..91ca92c7c061 100644
--- a/include/linux/of_platform.h
+++ b/include/linux/of_platform.h
@@ -73,6 +73,7 @@ extern int of_platform_populate(struct device_node *root,
 				const struct of_dev_auxdata *lookup,
 				struct device *parent);
 extern void of_platform_depopulate(struct device *parent);
+extern void of_platform_probe(struct device_node *np);
 #else
 static inline int of_platform_populate(struct device_node *root,
 					const struct of_device_id *matches,
@@ -82,6 +83,7 @@ static inline int of_platform_populate(struct device_node *root,
 	return -ENODEV;
 }
 static inline void of_platform_depopulate(struct device *parent) { }
+static inline void of_platform_probe(struct device_node *np) { }
 #endif
 
 #if defined(CONFIG_OF_DYNAMIC) && defined(CONFIG_OF_ADDRESS)
-- 
2.4.3

[PATCH v3 03/18] gpio: Probe GPIO drivers on demand

[Tomeu Vizoso]

When looking up a gpiochip through its firmware node, probe it if it
hasn't already.

The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---

Changes in v3: None
Changes in v2: None

 drivers/gpio/gpiolib-of.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index fa6e3c8823d6..d8c617c34c85 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -20,6 +20,7 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_gpio.h>
+#include <linux/of_platform.h>
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/slab.h>
 #include <linux/gpio/machine.h>
@@ -95,6 +96,8 @@ struct gpio_desc *of_get_named_gpiod_flags(struct device_node *np,
 		return ERR_PTR(ret);
 	}
 
+	of_platform_probe(gg_data.gpiospec.np);
+
 	gpiochip_find(&gg_data, of_gpiochip_find_and_xlate);
 
 	of_node_put(gg_data.gpiospec.np);
-- 
2.4.3

[PATCH v3 04/18] gpio: Probe pinctrl devices on demand

[Tomeu Vizoso]

When looking up a pin controller through its OF node, probe it if it
hasn't already.

The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---

Changes in v3: None
Changes in v2: None

 drivers/gpio/gpiolib-of.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index d8c617c34c85..2d4a1cca995b 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -359,6 +359,8 @@ static int of_gpiochip_add_pin_range(struct gpio_chip *chip)
 		if (ret)
 			break;
 
+		of_platform_probe(pinspec.np);
+
 		pctldev = of_pinctrl_get(pinspec.np);
 		if (!pctldev)
 			return -EPROBE_DEFER;
-- 
2.4.3

[PATCH v3 05/18] regulator: core: Reduce critical area in _regulator_get

[Tomeu Vizoso]

...by moving the locking of regulator_list_mutex into
regulator_dev_lookup(), where it is iterated over.

The regulator device lock gets acquired before returning, and the caller
is responsible for releasing it after it's done with the regulator
device.

In _regulator_get() the regulator_list_mutex mutex is held for most of
the function, but is only strictly needed to protect the list lookups.

This change would be useful if for example regulator devices could be
registered on demand when a driver requests them. regulator_register()
could end up being called from within _regulator_get while the lock on
regulator_list_mutex is being held, causing a deadlock.

This backtrace illustrates the situation described above:

(regulator_register) from [<c05efe64>]
(devm_regulator_register+0x48/0x84)
(devm_regulator_register) from [<c05f0b20>]
(reg_fixed_voltage_probe+0x214/0x35c)
(reg_fixed_voltage_probe) from [<c06cc7fc>]
(platform_drv_probe+0x54/0xbc)
(platform_drv_probe) from [<c06caac8>] (driver_probe_device+0x184/0x2c4)
(driver_probe_device) from [<c06cac58>] (__device_attach+0x50/0x54)
(__device_attach) from [<c06c8eac>] (bus_for_each_drv+0x70/0xa4)
(bus_for_each_drv) from [<c06ca900>] (device_attach+0x90/0xa4)
(device_attach) from [<c06c9eb4>] (bus_probe_device+0x94/0xb8)
(bus_probe_device) from [<c06c7de8>] (device_add+0x384/0x580)
(device_add) from [<c095c104>] (of_device_add+0x44/0x4c)
(of_device_add) from [<c095c968>]
...
(regulator_dev_lookup) from [<c05ee7c0>] (_regulator_get+0x8c/0x26c)
(_regulator_get) from [<c05ee9c0>] (regulator_get+0x20/0x24)
(regulator_get) from [<c05efb1c>] (_devm_regulator_get+0xa4/0xc8)
(_devm_regulator_get) from [<c05efb5c>] (devm_regulator_get+0x1c/0x20)
(devm_regulator_get) from [<c06ba870>] (tegra_hdmi_probe+0xe0/0x278)
(tegra_hdmi_probe) from [<c06cc7fc>] (platform_drv_probe+0x54/0xbc)

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---

Changes in v3:
- Avoid unlocking the regulator device's mutex if we don't have a device

Changes in v2:
- Acquire regulator device lock before returning from regulator_dev_lookup()

 drivers/regulator/core.c | 104 +++++++++++++++++++++++++++--------------------
 1 file changed, 59 insertions(+), 45 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 613034667b93..5f6c76142b08 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -110,6 +110,7 @@ static struct regulator *create_regulator(struct regulator_dev *rdev,
 					  struct device *dev,
 					  const char *supply_name);
 static void _regulator_put(struct regulator *regulator);
+static int _regulator_enable(struct regulator *regulator, bool do_lock);
 
 static const char *rdev_get_name(struct regulator_dev *rdev)
 {
@@ -1212,6 +1213,7 @@ static void unset_regulator_supplies(struct regulator_dev *rdev)
 
 #define REG_STR_SIZE	64
 
+/* rdev->mutex held by caller */
 static struct regulator *create_regulator(struct regulator_dev *rdev,
 					  struct device *dev,
 					  const char *supply_name)
@@ -1224,7 +1226,7 @@ static struct regulator *create_regulator(struct regulator_dev *rdev,
 	if (regulator == NULL)
 		return NULL;
 
-	mutex_lock(&rdev->mutex);
+	lockdep_assert_held_once(&rdev->mutex);
 	regulator->rdev = rdev;
 	list_add(&regulator->list, &rdev->consumer_list);
 
@@ -1276,12 +1278,10 @@ static struct regulator *create_regulator(struct regulator_dev *rdev,
 	    _regulator_is_enabled(rdev))
 		regulator->always_on = true;
 
-	mutex_unlock(&rdev->mutex);
 	return regulator;
 overflow_err:
 	list_del(&regulator->list);
 	kfree(regulator);
-	mutex_unlock(&rdev->mutex);
 	return NULL;
 }
 
@@ -1320,6 +1320,7 @@ static void regulator_supply_alias(struct device **dev, const char **supply)
 	}
 }
 
+/* Caller has to release r->mutex once done with r */
 static struct regulator_dev *regulator_dev_lookup(struct device *dev,
 						  const char *supply,
 						  int *ret)
@@ -1335,10 +1336,15 @@ static struct regulator_dev *regulator_dev_lookup(struct device *dev,
 	if (dev && dev->of_node) {
 		node = of_get_regulator(dev, supply);
 		if (node) {
+			mutex_lock(&regulator_list_mutex);
 			list_for_each_entry(r, &regulator_list, list)
 				if (r->dev.parent &&
-					node == r->dev.of_node)
+					node == r->dev.of_node) {
+					mutex_lock(&r->mutex);
+					mutex_unlock(&regulator_list_mutex);
 					return r;
+				}
+			mutex_unlock(&regulator_list_mutex);
 			*ret = -EPROBE_DEFER;
 			return NULL;
 		} else {
@@ -1356,9 +1362,14 @@ static struct regulator_dev *regulator_dev_lookup(struct device *dev,
 	if (dev)
 		devname = dev_name(dev);
 
+	mutex_lock(&regulator_list_mutex);
 	list_for_each_entry(r, &regulator_list, list)
-		if (strcmp(rdev_get_name(r), supply) == 0)
+		if (strcmp(rdev_get_name(r), supply) == 0) {
+			mutex_lock(&r->mutex);
+			mutex_unlock(&regulator_list_mutex);
 			return r;
+		}
+	mutex_unlock(&regulator_list_mutex);
 
 	list_for_each_entry(map, &regulator_map_list, list) {
 		/* If the mapping has a device set up it must match */
@@ -1400,6 +1411,7 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
 	if (!r) {
 		if (have_full_constraints()) {
 			r = dummy_regulator_rdev;
+			mutex_lock(&r->mutex);
 		} else {
 			dev_err(dev, "Failed to resolve %s-supply for %s\n",
 				rdev->supply_name, rdev->desc->name);
@@ -1410,23 +1422,25 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
 	/* Recursively resolve the supply of the supply */
 	ret = regulator_resolve_supply(r);
 	if (ret < 0)
-		return ret;
+		goto out;
 
 	ret = set_supply(rdev, r);
 	if (ret < 0)
-		return ret;
+		goto out;
 
 	/* Cascade always-on state to supply */
 	if (_regulator_is_enabled(rdev)) {
-		ret = regulator_enable(rdev->supply);
+		ret = _regulator_enable(rdev->supply, false);
 		if (ret < 0) {
 			if (rdev->supply)
 				_regulator_put(rdev->supply);
-			return ret;
+			goto out;
 		}
 	}
 
-	return 0;
+out:
+	mutex_unlock(&r->mutex);
+	return ret;
 }
 
 /* Internal regulator request function */
@@ -1451,8 +1465,6 @@ static struct regulator *_regulator_get(struct device *dev, const char *id,
 	else
 		ret = -EPROBE_DEFER;
 
-	mutex_lock(&regulator_list_mutex);
-
 	rdev = regulator_dev_lookup(dev, id, &ret);
 	if (rdev)
 		goto found;
@@ -1464,7 +1476,7 @@ static struct regulator *_regulator_get(struct device *dev, const char *id,
 	 * succeed, so, quit with appropriate error value
 	 */
 	if (ret && ret != -ENODEV)
-		goto out;
+		return regulator;
 
 	if (!devname)
 		devname = "deviceless";
@@ -1478,13 +1490,13 @@ static struct regulator *_regulator_get(struct device *dev, const char *id,
 			devname, id);
 
 		rdev = dummy_regulator_rdev;
+		mutex_lock(&rdev->mutex);
 		goto found;
 	/* Don't log an error when called from regulator_get_optional() */
 	} else if (!have_full_constraints() || exclusive) {
 		dev_warn(dev, "dummy supplies not allowed\n");
 	}
 
-	mutex_unlock(&regulator_list_mutex);
 	return regulator;
 
 found:
@@ -1526,8 +1538,7 @@ found:
 	}
 
 out:
-	mutex_unlock(&regulator_list_mutex);
-
+	mutex_unlock(&rdev->mutex);
 	return regulator;
 }
 
@@ -1986,12 +1997,24 @@ static int _regulator_do_enable(struct regulator_dev *rdev)
 	return 0;
 }
 
-/* locks held by regulator_enable() */
-static int _regulator_enable(struct regulator_dev *rdev)
+static int _regulator_enable(struct regulator *regulator, bool do_lock)
 {
-	int ret;
+	struct regulator_dev *rdev = regulator->rdev;
+	int ret = 0;
 
-	lockdep_assert_held_once(&rdev->mutex);
+	if (regulator->always_on)
+		return 0;
+
+	if (rdev->supply) {
+		ret = regulator_enable(rdev->supply);
+		if (ret != 0)
+			return ret;
+	}
+
+	if (do_lock)
+		mutex_lock(&rdev->mutex);
+	else
+		lockdep_assert_held_once(&rdev->mutex);
 
 	/* check voltage and requested load before enabling */
 	if (rdev->constraints &&
@@ -2002,23 +2025,33 @@ static int _regulator_enable(struct regulator_dev *rdev)
 		/* The regulator may on if it's not switchable or left on */
 		ret = _regulator_is_enabled(rdev);
 		if (ret == -EINVAL || ret == 0) {
-			if (!_regulator_can_change_status(rdev))
-				return -EPERM;
+			if (!_regulator_can_change_status(rdev)) {
+				ret = -EPERM;
+				goto out;
+			}
 
 			ret = _regulator_do_enable(rdev);
 			if (ret < 0)
-				return ret;
+				goto out;
 
 		} else if (ret < 0) {
 			rdev_err(rdev, "is_enabled() failed: %d\n", ret);
-			return ret;
+			goto out;
 		}
 		/* Fallthrough on positive return values - already enabled */
+		ret = 0;
 	}
 
 	rdev->use_count++;
 
-	return 0;
+out:
+	if (do_lock)
+		mutex_unlock(&rdev->mutex);
+
+	if (ret != 0 && rdev->supply)
+		regulator_disable(rdev->supply);
+
+	return ret;
 }
 
 /**
@@ -2034,26 +2067,7 @@ static int _regulator_enable(struct regulator_dev *rdev)
  */
 int regulator_enable(struct regulator *regulator)
 {
-	struct regulator_dev *rdev = regulator->rdev;
-	int ret = 0;
-
-	if (regulator->always_on)
-		return 0;
-
-	if (rdev->supply) {
-		ret = regulator_enable(rdev->supply);
-		if (ret != 0)
-			return ret;
-	}
-
-	mutex_lock(&rdev->mutex);
-	ret = _regulator_enable(rdev);
-	mutex_unlock(&rdev->mutex);
-
-	if (ret != 0 && rdev->supply)
-		regulator_disable(rdev->supply);
-
-	return ret;
+	return _regulator_enable(regulator, true);
 }
 EXPORT_SYMBOL_GPL(regulator_enable);
 
-- 
2.4.3

[PATCH v3 06/18] regulator: core: Probe regulators on demand

[Tomeu Vizoso]

When looking up a regulator through its OF node, probe it if it hasn't
already.

The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---

Changes in v3: None
Changes in v2: None

 drivers/regulator/core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 5f6c76142b08..73ac9fb07f96 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -26,6 +26,7 @@
 #include <linux/gpio.h>
 #include <linux/gpio/consumer.h>
 #include <linux/of.h>
+#include <linux/of_platform.h>
 #include <linux/regmap.h>
 #include <linux/regulator/of_regulator.h>
 #include <linux/regulator/consumer.h>
@@ -1336,6 +1337,7 @@ static struct regulator_dev *regulator_dev_lookup(struct device *dev,
 	if (dev && dev->of_node) {
 		node = of_get_regulator(dev, supply);
 		if (node) {
+			of_platform_probe(node);
 			mutex_lock(&regulator_list_mutex);
 			list_for_each_entry(r, &regulator_list, list)
 				if (r->dev.parent &&
-- 
2.4.3

[PATCH v3 07/18] drm: Probe panels on demand

[Tomeu Vizoso]

When looking up a panel through its OF node, probe it if it hasn't
already.

The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---

Changes in v3: None
Changes in v2: None

 drivers/gpu/drm/drm_panel.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
index 2ef988e037b7..8f01574cabda 100644
--- a/drivers/gpu/drm/drm_panel.c
+++ b/drivers/gpu/drm/drm_panel.c
@@ -23,6 +23,7 @@
 
 #include <linux/err.h>
 #include <linux/module.h>
+#include <linux/of_platform.h>
 
 #include <drm/drm_crtc.h>
 #include <drm/drm_panel.h>
@@ -80,6 +81,8 @@ struct drm_panel *of_drm_find_panel(struct device_node *np)
 {
 	struct drm_panel *panel;
 
+	of_platform_probe(np);
+
 	mutex_lock(&panel_lock);
 
 	list_for_each_entry(panel, &panel_list, list) {
-- 
2.4.3

[PATCH v3 08/18] drm/tegra: Probe dpaux devices on demand

[Tomeu Vizoso]

When looking up a dpaux device through its OF node, probe it if it
hasn't already.

The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---

Changes in v3: None
Changes in v2: None

 drivers/gpu/drm/tegra/dpaux.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
index 07b26972f487..5a98acf07b2d 100644
--- a/drivers/gpu/drm/tegra/dpaux.c
+++ b/drivers/gpu/drm/tegra/dpaux.c
@@ -12,6 +12,7 @@
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/of_gpio.h>
+#include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/reset.h>
 #include <linux/regulator/consumer.h>
@@ -394,6 +395,8 @@ struct tegra_dpaux *tegra_dpaux_find_by_of_node(struct device_node *np)
 {
 	struct tegra_dpaux *dpaux;
 
+	of_platform_probe(np);
+
 	mutex_lock(&dpaux_lock);
 
 	list_for_each_entry(dpaux, &dpaux_list, list)
-- 
2.4.3

[PATCH v3 09/18] i2c: core: Probe i2c adapters and devices on demand

[Tomeu Vizoso]

When looking up an i2c adapter or device through its OF node, probe it
if it hasn't already.

The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---

Changes in v3: None
Changes in v2: None

 drivers/i2c/i2c-core.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index c83e4d13cfc5..9c507b558fa2 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -40,6 +40,7 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/of_irq.h>
+#include <linux/of_platform.h>
 #include <linux/clk/clk-conf.h>
 #include <linux/completion.h>
 #include <linux/hardirq.h>
@@ -1342,6 +1343,8 @@ struct i2c_client *of_find_i2c_device_by_node(struct device_node *node)
 	struct device *dev;
 	struct i2c_client *client;
 
+	of_platform_probe(node);
+
 	dev = bus_find_device(&i2c_bus_type, NULL, node, of_dev_node_match);
 	if (!dev)
 		return NULL;
@@ -1360,6 +1363,8 @@ struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node)
 	struct device *dev;
 	struct i2c_adapter *adapter;
 
+	of_platform_probe(node);
+
 	dev = bus_find_device(&i2c_bus_type, NULL, node, of_dev_node_match);
 	if (!dev)
 		return NULL;
-- 
2.4.3

[PATCH v3 10/18] pwm: Probe PWM chip devices on demand

[Tomeu Vizoso]

When looking up a PWM chip through its OF node, probe it if it hasn't
already.

The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---

Changes in v3: None
Changes in v2: None

 drivers/pwm/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 3a7769fe53de..86649ab60cad 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -29,6 +29,7 @@
 #include <linux/device.h>
 #include <linux/debugfs.h>
 #include <linux/seq_file.h>
+#include <linux/of_platform.h>
 
 #include <dt-bindings/pwm/pwm.h>
 
@@ -496,6 +497,8 @@ static struct pwm_chip *of_node_to_pwmchip(struct device_node *np)
 {
 	struct pwm_chip *chip;
 
+	of_platform_probe(np);
+
 	mutex_lock(&pwm_lock);
 
 	list_for_each_entry(chip, &pwm_chips, list)
-- 
2.4.3

[PATCH v3 11/18] backlight: Probe backlight devices on demand

[Tomeu Vizoso]

When looking up a backlight device through its OF node, probe it if it
hasn't already.

The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---

Changes in v3: None
Changes in v2: None

 drivers/video/backlight/backlight.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index bddc8b17a4d8..cad854196ee3 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -16,6 +16,7 @@
 #include <linux/err.h>
 #include <linux/fb.h>
 #include <linux/slab.h>
+#include <linux/of_platform.h>
 
 #ifdef CONFIG_PMAC_BACKLIGHT
 #include <asm/backlight.h>
@@ -559,6 +560,8 @@ struct backlight_device *of_find_backlight_by_node(struct device_node *node)
 {
 	struct device *dev;
 
+	of_platform_probe(node);
+
 	dev = class_find_device(backlight_class, NULL, node, of_parent_match);
 
 	return dev ? to_backlight_device(dev) : NULL;
-- 
2.4.3

[PATCH v3 12/18] usb: phy: Probe phy devices on demand

[Tomeu Vizoso]

When looking up a phy through its OF node, probe it if it hasn't
already.

The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---

Changes in v3: None
Changes in v2: None

 drivers/usb/phy/phy.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c
index 98f75d2842b7..a357681362f5 100644
--- a/drivers/usb/phy/phy.c
+++ b/drivers/usb/phy/phy.c
@@ -15,6 +15,7 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/of.h>
+#include <linux/of_platform.h>
 
 #include <linux/usb/phy.h>
 
@@ -196,6 +197,8 @@ struct  usb_phy *devm_usb_get_phy_by_node(struct device *dev,
 		goto err0;
 	}
 
+	of_platform_probe(node);
+
 	spin_lock_irqsave(&phy_lock, flags);
 
 	phy = __of_usb_find_phy(node);
-- 
2.4.3

[PATCH v3 13/18] clk: Probe clk providers on demand

[Tomeu Vizoso]

When looking up a clock through its OF node, probe it if it hasn't
already.

The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---

Changes in v3: None
Changes in v2: None

 drivers/clk/clk.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 898052ee0efa..065870e38881 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -19,6 +19,7 @@
 #include <linux/list.h>
 #include <linux/slab.h>
 #include <linux/of.h>
+#include <linux/of_platform.h>
 #include <linux/device.h>
 #include <linux/init.h>
 #include <linux/sched.h>
@@ -2981,6 +2982,8 @@ struct clk *__of_clk_get_from_provider(struct of_phandle_args *clkspec,
 	if (!clkspec)
 		return ERR_PTR(-EINVAL);
 
+	of_platform_probe(clkspec->np);
+
 	/* Check if we have such a provider in our array */
 	mutex_lock(&of_clk_mutex);
 	list_for_each_entry(provider, &of_clk_providers, link) {
-- 
2.4.3

[PATCH v3 14/18] pinctrl: Probe pinctrl devices on demand

[Tomeu Vizoso]

When looking up a pin controller through its OF node, probe it if it
hasn't already.

The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---

Changes in v3: None
Changes in v2: None

 drivers/pinctrl/devicetree.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/pinctrl/devicetree.c b/drivers/pinctrl/devicetree.c
index fe04e748dfe4..2326ae99e06a 100644
--- a/drivers/pinctrl/devicetree.c
+++ b/drivers/pinctrl/devicetree.c
@@ -18,6 +18,7 @@
 
 #include <linux/device.h>
 #include <linux/of.h>
+#include <linux/of_platform.h>
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/slab.h>
 
@@ -110,6 +111,8 @@ static int dt_to_map_one_config(struct pinctrl *p, const char *statename,
 	struct pinctrl_map *map;
 	unsigned num_maps;
 
+	of_platform_probe(np_config);
+
 	/* Find the pin controller containing np_config */
 	np_pctldev = of_node_get(np_config);
 	for (;;) {
-- 
2.4.3

[PATCH v3 15/18] phy: core: Probe phy providers on demand

[Tomeu Vizoso]

When looking up a phy provider through its OF node, probe it if it
hasn't already.

The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---

Changes in v3: None
Changes in v2: None

 drivers/phy/phy-core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index fc48fac003a6..1ba55fc8cce4 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -18,6 +18,7 @@
 #include <linux/device.h>
 #include <linux/slab.h>
 #include <linux/of.h>
+#include <linux/of_platform.h>
 #include <linux/phy/phy.h>
 #include <linux/idr.h>
 #include <linux/pm_runtime.h>
@@ -363,6 +364,8 @@ static struct phy *_of_phy_get(struct device_node *np, int index)
 	if (ret)
 		return ERR_PTR(-ENODEV);
 
+	of_platform_probe(args.np);
+
 	mutex_lock(&phy_provider_mutex);
 	phy_provider = of_phy_provider_lookup(args.np);
 	if (IS_ERR(phy_provider) || !try_module_get(phy_provider->owner)) {
-- 
2.4.3

[PATCH v3 16/18] dma: of: Probe DMA controllers on demand

[Tomeu Vizoso]

When looking up a DMA controller through its OF node, probe it if it
hasn't already.

The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---

Changes in v3: None
Changes in v2: None

 drivers/dma/of-dma.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c
index 1e1f2986eba8..9cfc818b9dc8 100644
--- a/drivers/dma/of-dma.c
+++ b/drivers/dma/of-dma.c
@@ -17,6 +17,7 @@
 #include <linux/slab.h>
 #include <linux/of.h>
 #include <linux/of_dma.h>
+#include <linux/of_platform.h>
 
 static LIST_HEAD(of_dma_list);
 static DEFINE_MUTEX(of_dma_lock);
@@ -263,6 +264,8 @@ struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
 		if (of_dma_match_channel(np, name, i, &dma_spec))
 			continue;
 
+		of_platform_probe(dma_spec.np);
+
 		mutex_lock(&of_dma_lock);
 		ofdma = of_dma_find_controller(&dma_spec);
 
-- 
2.4.3

[PATCH v3 17/18] power-supply: Probe power supplies on demand

[Tomeu Vizoso]

When looking up a power supply through its OF node, probe it if it
hasn't already.

The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---

Changes in v3: None
Changes in v2: None

 drivers/power/power_supply_core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index 456987c88baa..668d71656592 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -19,6 +19,7 @@
 #include <linux/err.h>
 #include <linux/power_supply.h>
 #include <linux/thermal.h>
+#include <linux/of_platform.h>
 #include "power_supply.h"
 
 /* exported for the APM Power driver, APM emulation */
@@ -206,6 +207,8 @@ static int power_supply_find_supply_from_node(struct device_node *supply_node)
 {
 	int error;
 
+	of_platform_probe(supply_node);
+
 	/*
 	 * class_for_each_device() either returns its own errors or values
 	 * returned by __power_supply_find_supply_from_node().
-- 
2.4.3

[PATCH v3 18/18] ASoC: core: Probe components on demand

[Tomeu Vizoso]

When looking up a component through its OF node, probe it if it hasn't
already.

The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---

Changes in v3: None
Changes in v2: None

 sound/soc/soc-core.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index c81aec9c872a..1157049b1a34 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -34,6 +34,7 @@
 #include <linux/ctype.h>
 #include <linux/slab.h>
 #include <linux/of.h>
+#include <linux/of_platform.h>
 #include <sound/core.h>
 #include <sound/jack.h>
 #include <sound/pcm.h>
@@ -865,10 +866,12 @@ static const struct snd_soc_dai_ops null_dai_ops = {
 };
 
 static struct snd_soc_component *soc_find_component(
-	const struct device_node *of_node, const char *name)
+	struct device_node *of_node, const char *name)
 {
 	struct snd_soc_component *component;
 
+	of_platform_probe(of_node);
+
 	lockdep_assert_held(&client_mutex);
 
 	list_for_each_entry(component, &component_list, list) {
@@ -890,6 +893,8 @@ static struct snd_soc_dai *snd_soc_find_dai(
 	struct snd_soc_dai *dai;
 	struct device_node *component_of_node;
 
+	of_platform_probe(dlc->of_node);
+
 	lockdep_assert_held(&client_mutex);
 
 	/* Find CPU DAI from registered DAIs*/
-- 
2.4.3

Re: [PATCH v3 0/18] On-demand device probing

[Rob Herring]

On Thu, Aug 6, 2015 at 9:11 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
> Hello,
>
> I have a problem with the panel on my Tegra Chromebook taking longer
> than expected to be ready during boot (Stéphane Marchesin reported what
> is basically the same issue in [0]), and have looked into ordered
> probing as a better way of solving this than moving nodes around in the
> DT or playing with initcall levels and linking order.
>
> While reading the thread [1] that Alexander Holler started with his
> series to make probing order deterministic, it occurred to me that it
> should be possible to achieve the same by probing devices as they are
> referenced by other devices.
>
> This basically reuses the information that is already implicit in the
> probe() implementations, saving us from refactoring existing drivers or
> adding information to DTBs.
>
> During review of v1 of this series Linus Walleij suggested that it
> should be the device driver core to make sure that dependencies are
> ready before probing a device. I gave this idea a try [2] but Mark Brown
> pointed out to the logic duplication between the resource acquisition
> and dependency discovery code paths (though I think it's fairly minor).
>
> To address that code duplication I experimented with Arnd's devm_probe
> [3] concept of having drivers declare their dependencies instead of
> acquiring them during probe, and while it worked [4], I don't think we
> end up winning anything when compared to just probing devices on-demand
> from resource getters.
>
> One remaining objection is to the "sprinkling" of calls to
> fwnode_ensure_device() in the resource getters of each subsystem, but I
> think it's the right thing to do given that the storage of resources is
> currently subsystem-specific.
>
> We could avoid the above by moving resource storage into the core, but I
> don't think there's a compelling case for that.
>
> I have tested this on boards with Tegra, iMX.6, Exynos and OMAP SoCs,
> and these patches were enough to eliminate all the deferred probes
> (except one in PandaBoard because omap_dma_system doesn't have a
> firmware node as of yet).
>
> Have submitted a branch [5] with these patches to kernelci.org and I'm
> currently trying to fix all regressions, usually due to code assuming
> that devices will be probed in a specific order. Current results [6] are
> 348 passes, 30 fails and 42 unknowns (linux-next [7] is currently
> 387/3/23).

This is a bit worrying. If this causes a high number of boot failures,
fixing the errors you can find is not the path forward as we can't
test a lot of platforms (and many people don't look at -next). We may
want to put this behind a kconfig option so that we can easily restore
old behavior it needed. Otherwise, we could have to revert the series.

Are all the commits before this series fixing boot failures? You can't
do dts updates as the fix or backwards compatibility will be broken.

>
> With this series I get the kernel to output to the panel in 0.5s,
> instead of 2.8s.
>
> Regards,
>
> Tomeu
>
> [0] http://lists.freedesktop.org/archives/dri-devel/2014-August/066527.html
>
> [1] https://lkml.org/lkml/2014/5/12/452
>
> [2] https://lkml.org/lkml/2015/6/17/305
>
> [3] http://article.gmane.org/gmane.linux.ports.arm.kernel/277689
>
> [4] https://lkml.org/lkml/2015/7/21/441a
>
> [5] https://git.collabora.com/cgit/user/tomeu/linux.git/log/?h=on-demand-probes-v5
>
> [6] http://kernelci.org/boot/all/job/collabora/kernel/v4.2-rc5-6548-g632b98c83840/
>
> [7] http://kernelci.org/boot/all/job/next/kernel/next-20150806/
>
> Changes in v3:
> - Only delay platform devices with OF nodes
> - Set and use device_node.platform_dev instead of reversing the logic to
>   find the platform device that encloses a device node.

I still want this to be a struct device and not a struct
platform_device and am not convinced it can't be. It can simply be an
optimization of the existing function:

struct platform_device *of_find_device_by_node(struct device_node *np)
{
  if (node->device && node->device->bus == &platform_bus_type)
    return to_platform_device(node->device);
  return NULL;
}

Rob

Re: [PATCH v3 01/18] platform: delay OF device-driver matches until late_initcall

[Rob Herring]

On Thu, Aug 6, 2015 at 9:11 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
> Delay matches of platform devices with OF nodes until late_initcall,
> when we are sure that all built-in drivers have been registered already.
> This is needed to prevent deferred probes because of some drivers not
> having registered yet.
>
> The reason why only platform devices are delayed is that some other
> devices are expected to be probed earlier than late_initcall, for
> example, the system PNP driver needs to probe its devices in
> fs_initcall.
>
> Additionally, only platform devices with OF nodes are delayed because
> some machines may depend on oter platform devices being registered at
> specific times.

How do we know that these probes occur before the unused clocks and
regulators are turned off? Just getting lucky (as is deferred probe)?
Can we do this one level earlier so we have a level left to do things
after probe.

Rob

Re: [PATCH v3 0/18] On-demand device probing

[Tomeu Vizoso]

On 6 August 2015 at 22:14, Rob Herring <robherring2@gmail.com> wrote:
> On Thu, Aug 6, 2015 at 9:11 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
>> Hello,
>>
>> I have a problem with the panel on my Tegra Chromebook taking longer
>> than expected to be ready during boot (Stéphane Marchesin reported what
>> is basically the same issue in [0]), and have looked into ordered
>> probing as a better way of solving this than moving nodes around in the
>> DT or playing with initcall levels and linking order.
>>
>> While reading the thread [1] that Alexander Holler started with his
>> series to make probing order deterministic, it occurred to me that it
>> should be possible to achieve the same by probing devices as they are
>> referenced by other devices.
>>
>> This basically reuses the information that is already implicit in the
>> probe() implementations, saving us from refactoring existing drivers or
>> adding information to DTBs.
>>
>> During review of v1 of this series Linus Walleij suggested that it
>> should be the device driver core to make sure that dependencies are
>> ready before probing a device. I gave this idea a try [2] but Mark Brown
>> pointed out to the logic duplication between the resource acquisition
>> and dependency discovery code paths (though I think it's fairly minor).
>>
>> To address that code duplication I experimented with Arnd's devm_probe
>> [3] concept of having drivers declare their dependencies instead of
>> acquiring them during probe, and while it worked [4], I don't think we
>> end up winning anything when compared to just probing devices on-demand
>> from resource getters.
>>
>> One remaining objection is to the "sprinkling" of calls to
>> fwnode_ensure_device() in the resource getters of each subsystem, but I
>> think it's the right thing to do given that the storage of resources is
>> currently subsystem-specific.
>>
>> We could avoid the above by moving resource storage into the core, but I
>> don't think there's a compelling case for that.
>>
>> I have tested this on boards with Tegra, iMX.6, Exynos and OMAP SoCs,
>> and these patches were enough to eliminate all the deferred probes
>> (except one in PandaBoard because omap_dma_system doesn't have a
>> firmware node as of yet).
>>
>> Have submitted a branch [5] with these patches to kernelci.org and I'm
>> currently trying to fix all regressions, usually due to code assuming
>> that devices will be probed in a specific order. Current results [6] are
>> 348 passes, 30 fails and 42 unknowns (linux-next [7] is currently
>> 387/3/23).
>
> This is a bit worrying. If this causes a high number of boot failures,
> fixing the errors you can find is not the path forward as we can't
> test a lot of platforms (and many people don't look at -next). We may
> want to put this behind a kconfig option so that we can easily restore
> old behavior it needed. Otherwise, we could have to revert the series.

A Kconfig sounds fine to me. Altogether, I don't think it's that bad
because only these boards are known to have broken because of this
series:

at91-sama5d3_xplained
sama5d35ek

ste-snowball

vexpress-v2p-ca15
vexpress-v2p-ca15
vexpress-v2p-ca15_a7
vexpress-v2p-ca15-tc1
vexpress-v2p-ca9

I assume there's only 3 different bugs to fix there, plus a race in
imx boards that I have only papered over with a delay so far.

The failure rate seems to be so high because each boot is a
combination of board+defconfig and there are duplicated boards in
several labs and many were just offline at that moment.

But I agree that there's no way I can test it on all supported hw, so
a Kconfig that people can quickly switch on to disable the feature
sounds good to me.

> Are all the commits before this series fixing boot failures? You can't
> do dts updates as the fix or backwards compatibility will be broken.

The gpio-ranges fix for Tegra has a commit that safeguards backwards
compatibility, and the typo in regulator names for ux500 doesn't
really break anything that I can see, I just stumped into it when
trying to blindly fix the boot for ste-snowball (I don't have access
to that hw).

>> With this series I get the kernel to output to the panel in 0.5s,
>> instead of 2.8s.
>>
>> Regards,
>>
>> Tomeu
>>
>> [0] http://lists.freedesktop.org/archives/dri-devel/2014-August/066527.html
>>
>> [1] https://lkml.org/lkml/2014/5/12/452
>>
>> [2] https://lkml.org/lkml/2015/6/17/305
>>
>> [3] http://article.gmane.org/gmane.linux.ports.arm.kernel/277689
>>
>> [4] https://lkml.org/lkml/2015/7/21/441a
>>
>> [5] https://git.collabora.com/cgit/user/tomeu/linux.git/log/?h=on-demand-probes-v5
>>
>> [6] http://kernelci.org/boot/all/job/collabora/kernel/v4.2-rc5-6548-g632b98c83840/
>>
>> [7] http://kernelci.org/boot/all/job/next/kernel/next-20150806/
>>
>> Changes in v3:
>> - Only delay platform devices with OF nodes
>> - Set and use device_node.platform_dev instead of reversing the logic to
>>   find the platform device that encloses a device node.
>
> I still want this to be a struct device and not a struct
> platform_device and am not convinced it can't be. It can simply be an
> optimization of the existing function:

Now I realize what you meant, that makes sense to me.

Thanks,

Tomeu

> struct platform_device *of_find_device_by_node(struct device_node *np)
> {
>   if (node->device && node->device->bus == &platform_bus_type)
>     return to_platform_device(node->device);
>   return NULL;
> }
>
> Rob
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 01/18] platform: delay OF device-driver matches until late_initcall

[Tomeu Vizoso]

On 6 August 2015 at 22:19, Rob Herring <robherring2@gmail.com> wrote:
> On Thu, Aug 6, 2015 at 9:11 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
>> Delay matches of platform devices with OF nodes until late_initcall,
>> when we are sure that all built-in drivers have been registered already.
>> This is needed to prevent deferred probes because of some drivers not
>> having registered yet.
>>
>> The reason why only platform devices are delayed is that some other
>> devices are expected to be probed earlier than late_initcall, for
>> example, the system PNP driver needs to probe its devices in
>> fs_initcall.
>>
>> Additionally, only platform devices with OF nodes are delayed because
>> some machines may depend on oter platform devices being registered at
>> specific times.
>
> How do we know that these probes occur before the unused clocks and
> regulators are turned off? Just getting lucky (as is deferred probe)?
> Can we do this one level earlier so we have a level left to do things
> after probe.

Those are already late_initcall_sync so I guess we're fine.

Thanks,

Tomeu

> Rob
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 05/18] regulator: core: Reduce critical area in _regulator_get

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1096 bytes --]

On Thu, Aug 06, 2015 at 04:11:42PM +0200, Tomeu Vizoso wrote:

> This backtrace illustrates the situation described above:
> 
> (regulator_register) from [<c05efe64>]
> (devm_regulator_register+0x48/0x84)

Please don't paste entire backtraces into commit messages, they are
enormous and contain very little useful content - they just obscure
actual information in the commit message.  If you feel there's useful
information in there just include edited highlights with only that.

> +static int _regulator_enable(struct regulator *regulator, bool do_lock)
>  {
> -	int ret;
> +	struct regulator_dev *rdev = regulator->rdev;
> +	int ret = 0;
>  
> -	lockdep_assert_held_once(&rdev->mutex);
> +	if (regulator->always_on)
> +		return 0;
> +
> +	if (rdev->supply) {
> +		ret = regulator_enable(rdev->supply);
> +		if (ret != 0)
> +			return ret;
> +	}
> +
> +	if (do_lock)
> +		mutex_lock(&rdev->mutex);
> +	else
> +		lockdep_assert_held_once(&rdev->mutex);

Eew.  This do_lock stuff is *not* nice and going to be fragile.  I'm not
a fan, we need something better.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH v3 06/18] regulator: core: Probe regulators on demand

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 765 bytes --]

On Thu, Aug 06, 2015 at 04:11:43PM +0200, Tomeu Vizoso wrote:

> When looking up a regulator through its OF node, probe it if it hasn't
> already.
> 
> The goal is to reduce deferred probes to a minimum, as it makes it very
> cumbersome to find out why a device failed to probe, and can introduce
> very big delays in when a critical device is probed.

Still the same problem as we had before with this stuff, why is this DT
only?

>  #include <linux/regulator/consumer.h>
> @@ -1336,6 +1337,7 @@ static struct regulator_dev *regulator_dev_lookup(struct device *dev,
>  	if (dev && dev->of_node) {
>  		node = of_get_regulator(dev, supply);
>  		if (node) {
> +			of_platform_probe(node);

And why the assumption that this is a platform device?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH v3 02/18] of/platform: add of_platform_probe

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1167 bytes --]

On Thu, Aug 06, 2015 at 04:11:39PM +0200, Tomeu Vizoso wrote:

> Walks the OF tree up and finds the closest ancestor that has a platform
> device associated with it, probing it if isn't bound to a driver yet.

> The above should ensure that the dependency represented by the passed OF
> node is available, because probing a platform device should cause its
> descendants to be probed as well.

This sounds like it's going to break in the case where we have MFDs that
represent their functions in DT (not a pattern I'm a fan of but it's a
thing people do).  We'll walk back to the platform device for the MFD
function, try to probe it and then give up.  Perhaps that's good enough
anyway but it's not clear to me why we don't just try every parent we
find?

I'm also not a fan of the fact that the interface here is explicitly
saying that we want to probe a platform device, that's an implementation
detail that callers shouldn't need to know about.  From the point of
view of the callers what they're trying to do is kick any dependencies
into being instantiated, the fact that we currently try to accomplish it
with platform devices isn't something they care about.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH v3 06/18] regulator: core: Probe regulators on demand

[Rob Herring]

On Fri, Aug 7, 2015 at 7:09 AM, Mark Brown <broonie@kernel.org> wrote:
> On Thu, Aug 06, 2015 at 04:11:43PM +0200, Tomeu Vizoso wrote:
>
>> When looking up a regulator through its OF node, probe it if it hasn't
>> already.
>>
>> The goal is to reduce deferred probes to a minimum, as it makes it very
>> cumbersome to find out why a device failed to probe, and can introduce
>> very big delays in when a critical device is probed.
>
> Still the same problem as we had before with this stuff, why is this DT
> only?

The last version was more generic, but it was obvious that doing that
was pointless with current code structure. You have a call sequence
like either:

generic get -> DT specific get -> generic poke  -> DT specific poke

or

generic get -> DT specific get -> DT specific poke

v2 did the former and this version does the latter. However, for some
reason with regulators this is not all contained within
of_get_regulator.

>
>>  #include <linux/regulator/consumer.h>
>> @@ -1336,6 +1337,7 @@ static struct regulator_dev *regulator_dev_lookup(struct device *dev,
>>       if (dev && dev->of_node) {
>>               node = of_get_regulator(dev, supply);
>>               if (node) {
>> +                     of_platform_probe(node);
>
> And why the assumption that this is a platform device?

Agreed on this point.

Rob

Re: [PATCH v3 01/18] platform: delay OF device-driver matches until late_initcall

[Grygorii Strashko]

On 08/07/2015 10:11 AM, Tomeu Vizoso wrote:
> On 6 August 2015 at 22:19, Rob Herring <robherring2@gmail.com> wrote:
>> On Thu, Aug 6, 2015 at 9:11 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
>>> Delay matches of platform devices with OF nodes until late_initcall,
>>> when we are sure that all built-in drivers have been registered already.
>>> This is needed to prevent deferred probes because of some drivers not
>>> having registered yet.
>>>
>>> The reason why only platform devices are delayed is that some other
>>> devices are expected to be probed earlier than late_initcall, for
>>> example, the system PNP driver needs to probe its devices in
>>> fs_initcall.
>>>
>>> Additionally, only platform devices with OF nodes are delayed because
>>> some machines may depend on oter platform devices being registered at
>>> specific times.
>>
>> How do we know that these probes occur before the unused clocks and
>> regulators are turned off? Just getting lucky (as is deferred probe)?
>> Can we do this one level earlier so we have a level left to do things
>> after probe.
> 
> Those are already late_initcall_sync so I guess we're fine.

I wouldn't be so sure :(
FYI:
http://git.ti.com/ti-linux-kernel/ti-linux-kernel/commit/763d643bbfc0f445c6685c541fcae3c370e4314a


-- 
regards,
-grygorii

Re: [PATCH v3 01/18] platform: delay OF device-driver matches until late_initcall

[Tomeu Vizoso]

On 7 August 2015 at 19:06, Grygorii Strashko <grygorii.strashko@ti.com> wrote:
> On 08/07/2015 10:11 AM, Tomeu Vizoso wrote:
>> On 6 August 2015 at 22:19, Rob Herring <robherring2@gmail.com> wrote:
>>> On Thu, Aug 6, 2015 at 9:11 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
>>>> Delay matches of platform devices with OF nodes until late_initcall,
>>>> when we are sure that all built-in drivers have been registered already.
>>>> This is needed to prevent deferred probes because of some drivers not
>>>> having registered yet.
>>>>
>>>> The reason why only platform devices are delayed is that some other
>>>> devices are expected to be probed earlier than late_initcall, for
>>>> example, the system PNP driver needs to probe its devices in
>>>> fs_initcall.
>>>>
>>>> Additionally, only platform devices with OF nodes are delayed because
>>>> some machines may depend on oter platform devices being registered at
>>>> specific times.
>>>
>>> How do we know that these probes occur before the unused clocks and
>>> regulators are turned off? Just getting lucky (as is deferred probe)?
>>> Can we do this one level earlier so we have a level left to do things
>>> after probe.
>>
>> Those are already late_initcall_sync so I guess we're fine.
>
> I wouldn't be so sure :(
> FYI:
> http://git.ti.com/ti-linux-kernel/ti-linux-kernel/commit/763d643bbfc0f445c6685c541fcae3c370e4314a

If I understand the situation correctly, this is one more instance of
starting to do some work at some point and hoping that something else
that started before has already finished happening. If that's the
case, how does this series make that worst?

During this development I have found many hacks intended to put some
order, even if not enough care was taken to make sure that the order
was guaranteed. In general I would recommend for moving code into
proper drivers and have them to defer the probe of their devices if
some dependency isn't fulfilled at that moment.

Once that's done and we have a safe and reliable boot, we can avoid
those deferred probes by fulfilling the dependency on-demand as this
series shows.

There was some recent thread about how the disabling of unused clocks
and regulators isn't really safe because after late_initcall_sync more
drivers can be registered from modules. Furthermore, there's async
device probes.

Regards,

Tomeu

>
> --
> regards,
> -grygorii
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 01/18] platform: delay OF device-driver matches until late_initcall

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 590 bytes --]

On Sun, Aug 09, 2015 at 03:03:14PM +0200, Tomeu Vizoso wrote:

> There was some recent thread about how the disabling of unused clocks
> and regulators isn't really safe because after late_initcall_sync more
> drivers can be registered from modules. Furthermore, there's async
> device probes.

What recent thread and why would that be unsafe?  Any driver using a
clock or regulator ought to be making sure that the clock or regulator
is enabled before it tries to use it.  The worst that should happen is
that something gets the power bounced during boot which isn't the end of
the world.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH v3 02/18] of/platform: add of_platform_probe

[Tomeu Vizoso]

On 7 August 2015 at 14:19, Mark Brown <broonie@kernel.org> wrote:
> On Thu, Aug 06, 2015 at 04:11:39PM +0200, Tomeu Vizoso wrote:
>
>> Walks the OF tree up and finds the closest ancestor that has a platform
>> device associated with it, probing it if isn't bound to a driver yet.
>
>> The above should ensure that the dependency represented by the passed OF
>> node is available, because probing a platform device should cause its
>> descendants to be probed as well.
>
> This sounds like it's going to break in the case where we have MFDs that
> represent their functions in DT (not a pattern I'm a fan of but it's a
> thing people do).  We'll walk back to the platform device for the MFD
> function, try to probe it and then give up.  Perhaps that's good enough
> anyway but it's not clear to me why we don't just try every parent we
> find?

Agreed. In the attempt at probing dependencies before a device is
probed, I considered that a device's parent is also a dependency and
that worked well. From what I saw, few devices will defer their probe
if their parent hasn't been probed yet, assuming that it will have
probed already. But with simple-mfd and simple-bus that shouldn't be
relied upon as things will break if their parents defer their probe.
With async probing enabled this failure scenario becomes more
probable.

> I'm also not a fan of the fact that the interface here is explicitly
> saying that we want to probe a platform device, that's an implementation
> detail that callers shouldn't need to know about.  From the point of
> view of the callers what they're trying to do is kick any dependencies
> into being instantiated, the fact that we currently try to accomplish it
> with platform devices isn't something they care about.

Agreed.

Thanks,

Tomeu

Re: [PATCH v3 01/18] platform: delay OF device-driver matches until late_initcall

[Grygorii Strashko]

Hi Tomeu,

On 08/09/2015 04:03 PM, Tomeu Vizoso wrote:
> On 7 August 2015 at 19:06, Grygorii Strashko <grygorii.strashko@ti.com> wrote:
>> On 08/07/2015 10:11 AM, Tomeu Vizoso wrote:
>>> On 6 August 2015 at 22:19, Rob Herring <robherring2@gmail.com> wrote:
>>>> On Thu, Aug 6, 2015 at 9:11 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
>>>>> Delay matches of platform devices with OF nodes until late_initcall,
>>>>> when we are sure that all built-in drivers have been registered already.
>>>>> This is needed to prevent deferred probes because of some drivers not
>>>>> having registered yet.
>>>>>
>>>>> The reason why only platform devices are delayed is that some other
>>>>> devices are expected to be probed earlier than late_initcall, for
>>>>> example, the system PNP driver needs to probe its devices in
>>>>> fs_initcall.
>>>>>
>>>>> Additionally, only platform devices with OF nodes are delayed because
>>>>> some machines may depend on oter platform devices being registered at
>>>>> specific times.
>>>>
>>>> How do we know that these probes occur before the unused clocks and
>>>> regulators are turned off? Just getting lucky (as is deferred probe)?
>>>> Can we do this one level earlier so we have a level left to do things
>>>> after probe.
>>>
>>> Those are already late_initcall_sync so I guess we're fine.
>>
>> I wouldn't be so sure :(
>> FYI:
>> http://git.ti.com/ti-linux-kernel/ti-linux-kernel/commit/763d643bbfc0f445c6685c541fcae3c370e4314a

I'm very sorry for delayed reply.

First of all, I'd like to say that proposition to move drivers probing/matching 
one layer early (device_initcall_sync) is reasonable 
(and I've already mentioned the same here https://lkml.org/lkml/2015/6/3/979).

> 
> If I understand the situation correctly, this is one more instance of
> starting to do some work at some point and hoping that something else
> that started before has already finished happening. If that's the
> case, how does this series make that worst?

You going to increase pressure on the late boot stages without taking
into account that there could be platforms which perfectly works
just using init_calls. And the fact that drivers will be not probed at
expected time could be very surprising for them.

Or, there should be an option to disable this functionality.

> 
> During this development I have found many hacks intended to put some
> order, even if not enough care was taken to make sure that the order
> was guaranteed. In general I would recommend for moving code into
> proper drivers and have them to defer the probe of their devices if
> some dependency isn't fulfilled at that moment.

Unfortunately, not everything can be moved to drivers or it could be
just unreasonable. 

> 
> Once that's done and we have a safe and reliable boot, we can avoid
> those deferred probes by fulfilling the dependency on-demand as this
> series shows.
> 
> There was some recent thread about how the disabling of unused clocks
> and regulators isn't really safe because after late_initcall_sync more
> drivers can be registered from modules. Furthermore, there's async
> device probes.

Clocks and regulators are safe, problems may happen first of all with 
platform's specific initcalls.


Just take a look how many late_initcalls defined now in Kernel
(grep -w -RI  "late_initcall" ./*) and everything which belongs
to  /arch/* will be called before your probe_delayed_matches_initcall().
The sequence of late_initcalls in /drivers/* is defined by
/drivers/Makefile. 

#
./arch/um/kernel/process.c:late_initcall(make_proc_sysemu);
./arch/um/drivers/umcast_kern.c:late_initcall(register_umcast);
./arch/um/drivers/stdio_console.c:late_initcall(stdio_init);
./arch/um/drivers/pcap_kern.c:late_initcall(register_pcap);
./arch/um/drivers/ssl.c:late_initcall(ssl_init);
./arch/um/drivers/slip_kern.c:late_initcall(register_slip);
./arch/um/drivers/vde_kern.c:late_initcall(register_vde);
./arch/um/drivers/mconsole_kern.c:late_initcall(mc_add_console);
./arch/um/drivers/ubd_kern.c:late_initcall(ubd_init);
./arch/um/drivers/slirp_kern.c:late_initcall(register_slirp);
./arch/um/drivers/daemon_kern.c:late_initcall(register_daemon);
./arch/um/os-Linux/drivers/tuntap_kern.c:late_initcall(register_tuntap);
./arch/um/os-Linux/drivers/ethertap_kern.c:late_initcall(register_ethertap);
./arch/arm/kernel/pj4-cp0.c:late_initcall(pj4_cp0_init);
./arch/arm/kernel/setup.c:late_initcall(init_machine_late);
./arch/arm/kernel/xscale-cp0.c:late_initcall(xscale_cp0_init);
./arch/arm/kernel/swp_emulate.c:late_initcall(swp_emulation_init);
./arch/arm/kernel/thumbee.c:late_initcall(thumbee_init);
./arch/arm/kernel/traps.c:late_initcall(arm_mrc_hook_init);
./arch/arm/mach-mmp/pm-pxa910.c:late_initcall(pxa910_pm_init);
./arch/arm/mach-mmp/pm-mmp2.c:late_initcall(mmp2_pm_init);
./arch/arm/common/bL_switcher.c:late_initcall(bL_switcher_init);
./arch/arm/mach-pxa/sharpsl_pm.c:late_initcall(sharpsl_pm_init);
./arch/arm/kvm/coproc_a15.c:late_initcall(coproc_a15_init);
./arch/arm/kvm/coproc_a7.c:late_initcall(coproc_a7_init);
./arch/arm/mach-omap1/clock.c:late_initcall(clk_disable_unused);
./arch/arm/mach-omap1/clock.c:late_initcall(omap_clk_enable_autoidle_all);
./arch/arm/mach-omap1/clock.c:late_initcall(clk_debugfs_init);
./arch/arm/xen/enlighten.c:late_initcall(xen_pm_init);
./arch/arm/probes/kprobes/test-core.c:late_initcall(run_all_tests);
./arch/arm/mach-mvebu/pm-board.c:late_initcall(mvebu_armada_xp_gp_pm_init);
./arch/unicore32/kernel/fpu-ucf64.c:late_initcall(ucf64_init);
./arch/avr32/mm/tlb.c:late_initcall(proctlb_init);
./arch/sh/kernel/cpu/sh4a/smp-shx3.c:late_initcall(register_shx3_cpu_notifier);
./arch/sh/kernel/cpu/shmobile/pm.c:late_initcall(sh_pm_init);
./arch/sh/boards/mach-hp6xx/pm.c:late_initcall(hp6x0_pm_init);
./arch/xtensa/platforms/iss/console.c:late_initcall(rs_init);
./arch/frv/kernel/setup.c:late_initcall(setup_arch_serial);
./arch/powerpc/kernel/machine_kexec.c:late_initcall(kexec_setup);
./arch/powerpc/kernel/machine_kexec_64.c:late_initcall(export_htab_values);
./arch/powerpc/kernel/setup-common.c:late_initcall(check_cache_coherency);
./arch/powerpc/kernel/iommu.c:late_initcall(fail_iommu_debugfs);
./arch/powerpc/lib/code-patching.c:late_initcall(test_code_patching);
./arch/powerpc/lib/feature-fixups.c:late_initcall(test_feature_fixups);
./arch/powerpc/sysdev/ppc4xx_cpm.c:late_initcall(cpm_init);
./arch/powerpc/sysdev/dart_iommu.c:late_initcall(iommu_init_late_dart);
./arch/powerpc/sysdev/msi_bitmap.c:late_initcall(msi_bitmap_selftest);
./arch/tile/kernel/hardwall.c:late_initcall(dev_hardwall_init);
./arch/tile/kernel/smpboot.c:late_initcall(reset_init_affinity);
./arch/arm64/kernel/fpsimd.c:late_initcall(fpsimd_init);
./arch/arm64/kernel/armv8_deprecated.c:late_initcall(armv8_deprecated_init);
./arch/arm64/kvm/sys_regs_generic_v8.c:late_initcall(sys_reg_genericv8_init);
./arch/m68k/kernel/early_printk.c:late_initcall(unregister_early_console);
./arch/sparc/kernel/leon_pmc.c:late_initcall(leon_pmc_install);
./arch/sparc/kernel/sstate.c:late_initcall(sstate_running);
./arch/metag/kernel/setup.c:late_initcall(init_machine_late);
./arch/x86/kernel/tboot.c:late_initcall(tboot_late_init);
./arch/x86/kernel/cpu/mcheck/mce-severity.c:late_initcall(severities_debugfs_init);
./arch/x86/kernel/cpu/mcheck/mce_amd.c:late_initcall(threshold_init_device);
./arch/x86/kernel/cpu/mcheck/mce.c:late_initcall(mcheck_debugfs_init);
./arch/x86/kernel/mpparse.c:late_initcall(update_mp_table);
./arch/x86/kernel/apic/x2apic_uv_x.c:late_initcall(uv_init_heartbeat);
./arch/x86/kernel/apic/apic.c:late_initcall(lapic_insert_resource);
./arch/x86/kernel/apic/vector.c:late_initcall(print_ICs);
./arch/x86/kernel/apic/probe_32.c:late_initcall(print_ipi_mode);
./arch/x86/kernel/acpi/boot.c:late_initcall(hpet_insert_resource);
./arch/x86/crypto/aesni-intel_glue.c:late_initcall(aesni_init);
./arch/x86/mm/tlb.c:late_initcall(create_tlb_single_page_flush_ceiling);
./arch/x86/mm/pat.c:late_initcall(pat_memtype_list_init);
./arch/x86/platform/intel-mid/device_libs/platform_gpio_keys.c:late_initcall(pb_keys_init);
./arch/x86/pci/mmconfig-shared.c:late_initcall(pci_mmcfg_late_insert_resources);
./arch/arc/kernel/setup.c:late_initcall(init_late_machine);
./arch/mips/alchemy/devboards/pm.c:late_initcall(pm_init);
./arch/mips/loongson64/common/mem.c:late_initcall(find_vga_mem_init);
./arch/mips/txx9/generic/setup_tx4927.c:late_initcall(tx4927_late_init);
./arch/mips/txx9/generic/setup_tx4939.c:late_initcall(tx4939_late_init);
./arch/mips/txx9/generic/setup_tx4938.c:late_initcall(tx4938_late_init);
./arch/mips/cavium-octeon/setup.c:late_initcall(octeon_no_pci_release);
./arch/mips/cavium-octeon/flash_setup.c:late_initcall(octeon_flash_init);
./arch/mips/cavium-octeon/smp.c:late_initcall(register_cavium_notifier);
./arch/mips/mti-malta/malta-pm.c:late_initcall(malta_pm_setup);
./arch/mips/jz4740/pm.c:late_initcall(jz4740_pm_init);
./arch/mips/sgi-ip27/ip27-timer.c:late_initcall(sgi_ip27_rtc_devinit);
./arch/blackfin/kernel/cplbinfo.c:late_initcall(cplbinfo_init);
./arch/blackfin/kernel/bfin_dma.c:late_initcall(proc_dma_init);
./arch/blackfin/kernel/nmi.c:late_initcall(init_nmi_wdt_syscore);
./arch/blackfin/mach-bf609/pm.c:late_initcall(bf609_init_pm);
./arch/blackfin/mm/sram-alloc.c:late_initcall(sram_proc_init);
./arch/blackfin/mm/isram-driver.c:late_initcall(isram_test_init);
./block/blk-core.c:late_initcall(fail_make_request_debugfs);
./block/blk-timeout.c:late_initcall(fail_io_timeout_debugfs);
./crypto/async_tx/raid6test.c:late_initcall(raid6_test);
./drivers/clk/clk.c:late_initcall(clk_debug_init);
./drivers/mtd/ubi/build.c:late_initcall(ubi_init);
./drivers/mtd/devices/block2mtd.c:late_initcall(block2mtd_init);
./drivers/devfreq/exynos/exynos5_bus.c:late_initcall(exynos5_busfreq_int_init);
./drivers/devfreq/exynos/exynos4_bus.c:late_initcall(exynos4_busfreq_init);
./drivers/gpu/drm/omapdrm/omap_drv.c:late_initcall(omap_drm_init);
./drivers/gpu/drm/gma500/psb_drv.c:late_initcall(psb_init);
./drivers/video/logo/logo.c:late_initcall(fb_logo_late_init);
./drivers/of/fdt.c:late_initcall(of_fdt_raw_init);
./drivers/base/dd.c:late_initcall(deferred_probe_initcall);
./drivers/base/power/trace.c:late_initcall(late_resume_init);
./drivers/base/power/domain.c:late_initcall(genpd_poweroff_unused);
./drivers/base/power/domain.c:late_initcall(pm_genpd_debug_init);
./drivers/mfd/wl1273-core.c:late_initcall(wl1273_core_init);
./drivers/cpufreq/speedstep-centrino.c:late_initcall(centrino_init);
./drivers/cpufreq/amd_freq_sensitivity.c:late_initcall(amd_freq_sensitivity_init);
./drivers/cpufreq/sfi-cpufreq.c:late_initcall(sfi_cpufreq_init);
./drivers/cpufreq/powernow-k8.c:late_initcall(powernowk8_init);
./drivers/cpufreq/acpi-cpufreq.c:late_initcall(acpi_cpufreq_init);
./drivers/cpufreq/ia64-acpi-cpufreq.c:late_initcall(acpi_cpufreq_init);
./drivers/cpufreq/p4-clockmod.c:late_initcall(cpufreq_p4_init);
./drivers/cpufreq/pcc-cpufreq.c:late_initcall(pcc_cpufreq_init);
./drivers/cpufreq/powernow-k7.c:late_initcall(powernow_init);
./drivers/cpufreq/longhaul.c:late_initcall(longhaul_init);
./drivers/cpufreq/s3c24xx-cpufreq-debugfs.c:late_initcall(s3c_freq_debugfs_init);
./drivers/cpufreq/at32ap-cpufreq.c:late_initcall(at32_cpufreq_init);
./drivers/cpufreq/s3c24xx-cpufreq.c:late_initcall(s3c_cpufreq_initcall);
./drivers/sh/clk/core.c:late_initcall(clk_late_init);
./drivers/sh/intc/userimask.c:late_initcall(userimask_sysdev_init);
./drivers/net/netconsole.c:late_initcall(init_netconsole);
./drivers/net/vxlan.c:late_initcall(vxlan_init_module);
./drivers/net/geneve.c:late_initcall(geneve_init_module);
./drivers/net/ethernet/ti/davinci_emac.c:late_initcall(davinci_emac_init);
./drivers/net/ethernet/ti/cpsw.c:late_initcall(cpsw_init);
./drivers/net/rionet.c:late_initcall(rionet_init);
./drivers/gpio/gpio-tegra.c:late_initcall(tegra_gpio_debuginit);
./drivers/block/hd.c:late_initcall(hd_init);
./drivers/thermal/db8500_cpufreq_cooling.c:late_initcall(db8500_cpufreq_cooling_init);
./drivers/staging/android/sync_debug.c:late_initcall(sync_debugfs_init);
./drivers/staging/wilc1000/linux_wlan.c:late_initcall(init_wilc_driver);
./drivers/iommu/dmar.c:late_initcall(dmar_free_unused_resources);
./drivers/watchdog/ie6xx_wdt.c:late_initcall(ie6xx_wdt_init);
./drivers/watchdog/intel_scu_watchdog.c:late_initcall(intel_scu_watchdog_init);
./drivers/tty/serial/mpsc.c:late_initcall(mpsc_late_console_init);
./drivers/media/platform/omap/omap_vout.c:late_initcall(omap_vout_init);
./drivers/media/pci/cx25821/cx25821-alsa.c:late_initcall(cx25821_alsa_init);
./drivers/media/pci/saa7134/saa7134-alsa.c:late_initcall(saa7134_alsa_init);
./drivers/dma/dmatest.c:late_initcall(dmatest_init);
./drivers/rapidio/rio-scan.c:late_initcall(rio_basic_attach);
./drivers/xen/xenbus/xenbus_probe_frontend.c:late_initcall(boot_wait_for_devices);
./drivers/platform/x86/dell-laptop.c:late_initcall(dell_init);
./drivers/firmware/memmap.c:late_initcall(firmware_memmap_init);
./drivers/firmware/edd.c:late_initcall(edd_init);
./drivers/firmware/efi/reboot.c:late_initcall(efi_shutdown_init);
./drivers/input/keyboard/gpio_keys.c:late_initcall(gpio_keys_init);
./drivers/pci/pci.c:late_initcall(pci_resource_alignment_sysfs_init);
./drivers/pci/pci-sysfs.c:late_initcall(pci_sysfs_init);
./drivers/macintosh/via-pmu-led.c:late_initcall(via_pmu_led_init);
./drivers/macintosh/via-pmu-event.c:late_initcall(via_pmu_event_init);
./drivers/ide/sgiioc4.c:late_initcall(ioc4_ide_init); /* Call only after IDE init is done */
./drivers/ide/ide-cs.c:late_initcall(init_ide_cs);
./drivers/rtc/Kconfig:	  The driver for this RTC device must be loaded before late_initcall
./drivers/rtc/hctosys.c:late_initcall(rtc_hctosys);
./drivers/irqchip/irq-vic.c:late_initcall(vic_pm_init);
./drivers/power/avs/smartreflex.c:late_initcall(sr_init);
./drivers/power/charger-manager.c:late_initcall(charger_manager_init);
./fs/btrfs/super.c:late_initcall(init_btrfs_fs);
./fs/afs/main.c:late_initcall(afs_init);	/* must be called after net/ to create socket */
./fs/ubifs/super.c:late_initcall(ubifs_init);
./kernel/sched/core.c:late_initcall(sched_init_debug);
./kernel/time/timekeeping_debug.c:late_initcall(tk_debug_sleep_time_init);
./kernel/taskstats.c:late_initcall(taskstats_init);
./kernel/printk/printk.c:late_initcall(printk_late_init);
./kernel/bpf/arraymap.c:late_initcall(register_array_map);
./kernel/bpf/arraymap.c:late_initcall(register_prog_array_map);
./kernel/bpf/hashtab.c:late_initcall(register_htab_map);
./kernel/trace/trace_events.c:late_initcall(event_trace_self_tests_init);
./kernel/trace/bpf_trace.c:late_initcall(register_kprobe_prog_ops);
./kernel/trace/trace.c:late_initcall(clear_boot_tracer);
./kernel/trace/trace_kprobe.c:late_initcall(kprobe_trace_self_tests_init);
./kernel/trace/trace_events_filter.c:late_initcall(ftrace_test_event_filter);
./kernel/trace/trace_kdb.c:late_initcall(kdb_ftrace_register);
./kernel/trace/ring_buffer.c:late_initcall(test_ringbuffer);
./kernel/kprobes.c:late_initcall(debugfs_kprobe_init);
./kernel/rcu/update.c:late_initcall(rcu_verify_early_boot_tests);
./kernel/panic.c:late_initcall(init_oops_id);
./kernel/system_keyring.c:late_initcall(load_system_certificate_list);
./kernel/power/qos.c:late_initcall(pm_qos_power_init);
./kernel/power/Kconfig:	  late_initcall.
./kernel/power/main.c:late_initcall(pm_debugfs_init);
./kernel/power/suspend_test.c:late_initcall(test_suspend);
./lib/list_sort.c:late_initcall(list_sort_test);
./lib/random32.c:late_initcall(prandom_reseed);
./mm/kmemleak.c:late_initcall(kmemleak_late_init);
./mm/failslab.c:late_initcall(failslab_debugfs_init);
./mm/memory.c:late_initcall(fault_around_debugfs);
./mm/early_ioremap.c:late_initcall(check_early_ioremap_leak);
./mm/cma_debug.c:late_initcall(cma_debugfs_init);
./mm/page_owner.c:late_initcall(pageowner_init)
./mm/page_alloc.c:late_initcall(fail_page_alloc_debugfs);
./mm/swapfile.c:late_initcall(max_swapfiles_check);
./mm/zswap.c:late_initcall(init_zswap);
./net/ipv4/ipconfig.c:late_initcall(ip_auto_config);
./net/ipv4/tcp_cong.c:late_initcall(tcp_congestion_default);
./net/core/filter.c:late_initcall(register_sk_filter_ops);
./net/bluetooth/selftest.c:late_initcall(bt_selftest_init);
./security/integrity/evm/evm_main.c:late_initcall(init_evm);
./security/integrity/ima/ima_main.c:late_initcall(init_ima);	/* Start IMA after the TPM is available */
./security/apparmor/crypto.c:late_initcall(init_profile_hash);
./security/keys/trusted.c:late_initcall(init_trusted);
./security/keys/encrypted-keys/encrypted.c:late_initcall(init_encrypted);
./security/keys/process_keys.c:late_initcall(init_root_keyring);
./sound/soc/fsl/p1022_rdk.c:late_initcall(p1022_rdk_init);
./sound/soc/fsl/phycore-ac97.c:late_initcall(imx_phycore_init);

-- 
regards,
-grygorii

Re: [PATCH v3 01/18] platform: delay OF device-driver matches until late_initcall

[Tomeu Vizoso]

On 10 August 2015 at 12:25, Mark Brown <broonie@kernel.org> wrote:
> On Sun, Aug 09, 2015 at 03:03:14PM +0200, Tomeu Vizoso wrote:
>
>> There was some recent thread about how the disabling of unused clocks
>> and regulators isn't really safe because after late_initcall_sync more
>> drivers can be registered from modules. Furthermore, there's async
>> device probes.
>
> What recent thread and why would that be unsafe?

Sorry, I should have said instead that it's already unpredictable and
that from what I have seen during testing these series doesn't make it
any worst.

Regards,

Tomeu

> Any driver using a
> clock or regulator ought to be making sure that the clock or regulator
> is enabled before it tries to use it.  The worst that should happen is
> that something gets the power bounced during boot which isn't the end of
> the world.

Re: [PATCH v3 01/18] platform: delay OF device-driver matches until late_initcall

[Tomeu Vizoso]

On 14 August 2015 at 21:09, Grygorii Strashko <grygorii.strashko@ti.com> wrote:
> Hi Tomeu,
>
> On 08/09/2015 04:03 PM, Tomeu Vizoso wrote:
>> On 7 August 2015 at 19:06, Grygorii Strashko <grygorii.strashko@ti.com> wrote:
>>> On 08/07/2015 10:11 AM, Tomeu Vizoso wrote:
>>>> On 6 August 2015 at 22:19, Rob Herring <robherring2@gmail.com> wrote:
>>>>> On Thu, Aug 6, 2015 at 9:11 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
>>>>>> Delay matches of platform devices with OF nodes until late_initcall,
>>>>>> when we are sure that all built-in drivers have been registered already.
>>>>>> This is needed to prevent deferred probes because of some drivers not
>>>>>> having registered yet.
>>>>>>
>>>>>> The reason why only platform devices are delayed is that some other
>>>>>> devices are expected to be probed earlier than late_initcall, for
>>>>>> example, the system PNP driver needs to probe its devices in
>>>>>> fs_initcall.
>>>>>>
>>>>>> Additionally, only platform devices with OF nodes are delayed because
>>>>>> some machines may depend on oter platform devices being registered at
>>>>>> specific times.
>>>>>
>>>>> How do we know that these probes occur before the unused clocks and
>>>>> regulators are turned off? Just getting lucky (as is deferred probe)?
>>>>> Can we do this one level earlier so we have a level left to do things
>>>>> after probe.
>>>>
>>>> Those are already late_initcall_sync so I guess we're fine.
>>>
>>> I wouldn't be so sure :(
>>> FYI:
>>> http://git.ti.com/ti-linux-kernel/ti-linux-kernel/commit/763d643bbfc0f445c6685c541fcae3c370e4314a
>
> I'm very sorry for delayed reply.
>
> First of all, I'd like to say that proposition to move drivers probing/matching
> one layer early (device_initcall_sync) is reasonable
> (and I've already mentioned the same here https://lkml.org/lkml/2015/6/3/979).

Yeah, I have submitted this change to kernelci and I'm awaiting the
results. Note though that none of the boards in there (currently 91)
had any problem when probes were deferred until late_initcall.

>> If I understand the situation correctly, this is one more instance of
>> starting to do some work at some point and hoping that something else
>> that started before has already finished happening. If that's the
>> case, how does this series make that worst?
>
> You going to increase pressure on the late boot stages without taking
> into account that there could be platforms which perfectly works
> just using init_calls. And the fact that drivers will be not probed at
> expected time could be very surprising for them.

Well, they work perfectly until a driver in the dependency chain
starts deferring its probe, or gets renamed/moved and thus probed
later, or async probing gets enabled, etc. Then the boot breaks and
someone has to spend an evening adding printks all along the place.

I know that things have worked to date, but it's so fragile to have to
impose order by manually ordering things in the makefiles and fiddling
with initcall levels that we had to introduce the very big hammer that
deferred probing is, to assure that we get a working system at the
end.

But probing devices in a random order and relying on the return value
of the probe callback to retry them later means that we can no longer
warn about failed probes because the norm now is for them to fail a
few times before they succeed. And that also means that if you want
for, say, the panel to be up and running as soon as possible during
boot, you have to do an absurd amount of fiddling in the DT to have
the nodes in dependency order.

But if drivers and initcalls explicitly try to acquire the resources
they depend on instead of just hoping for them to be there, then
of_device_probe should end up being called and will ensure the
dependencies are there at the point when they are needed.

> Or, there should be an option to disable this functionality.

Yeah, Rob suggested it already. I will be adding such an option just
in case it's useful to someone.

>> During this development I have found many hacks intended to put some
>> order, even if not enough care was taken to make sure that the order
>> was guaranteed. In general I would recommend for moving code into
>> proper drivers and have them to defer the probe of their devices if
>> some dependency isn't fulfilled at that moment.
>
> Unfortunately, not everything can be moved to drivers or it could be
> just unreasonable.

Hopefully those initcalls can state their dependencies explicitly
(which should end up causing the required devices to be probed).

>> Once that's done and we have a safe and reliable boot, we can avoid
>> those deferred probes by fulfilling the dependency on-demand as this
>> series shows.
>>
>> There was some recent thread about how the disabling of unused clocks
>> and regulators isn't really safe because after late_initcall_sync more
>> drivers can be registered from modules. Furthermore, there's async
>> device probes.
>
> Clocks and regulators are safe, problems may happen first of all with
> platform's specific initcalls.

Guess that starting to process the deferred queue in
device_initcall_sync will help with these (in the next revision).

Thanks,

Tomeu

> Just take a look how many late_initcalls defined now in Kernel
> (grep -w -RI  "late_initcall" ./*) and everything which belongs
> to  /arch/* will be called before your probe_delayed_matches_initcall().
> The sequence of late_initcalls in /drivers/* is defined by
> /drivers/Makefile.
>
> #
> ./arch/um/kernel/process.c:late_initcall(make_proc_sysemu);
> ./arch/um/drivers/umcast_kern.c:late_initcall(register_umcast);
> ./arch/um/drivers/stdio_console.c:late_initcall(stdio_init);
> ./arch/um/drivers/pcap_kern.c:late_initcall(register_pcap);
> ./arch/um/drivers/ssl.c:late_initcall(ssl_init);
> ./arch/um/drivers/slip_kern.c:late_initcall(register_slip);
> ./arch/um/drivers/vde_kern.c:late_initcall(register_vde);
> ./arch/um/drivers/mconsole_kern.c:late_initcall(mc_add_console);
> ./arch/um/drivers/ubd_kern.c:late_initcall(ubd_init);
> ./arch/um/drivers/slirp_kern.c:late_initcall(register_slirp);
> ./arch/um/drivers/daemon_kern.c:late_initcall(register_daemon);
> ./arch/um/os-Linux/drivers/tuntap_kern.c:late_initcall(register_tuntap);
> ./arch/um/os-Linux/drivers/ethertap_kern.c:late_initcall(register_ethertap);
> ./arch/arm/kernel/pj4-cp0.c:late_initcall(pj4_cp0_init);
> ./arch/arm/kernel/setup.c:late_initcall(init_machine_late);
> ./arch/arm/kernel/xscale-cp0.c:late_initcall(xscale_cp0_init);
> ./arch/arm/kernel/swp_emulate.c:late_initcall(swp_emulation_init);
> ./arch/arm/kernel/thumbee.c:late_initcall(thumbee_init);
> ./arch/arm/kernel/traps.c:late_initcall(arm_mrc_hook_init);
> ./arch/arm/mach-mmp/pm-pxa910.c:late_initcall(pxa910_pm_init);
> ./arch/arm/mach-mmp/pm-mmp2.c:late_initcall(mmp2_pm_init);
> ./arch/arm/common/bL_switcher.c:late_initcall(bL_switcher_init);
> ./arch/arm/mach-pxa/sharpsl_pm.c:late_initcall(sharpsl_pm_init);
> ./arch/arm/kvm/coproc_a15.c:late_initcall(coproc_a15_init);
> ./arch/arm/kvm/coproc_a7.c:late_initcall(coproc_a7_init);
> ./arch/arm/mach-omap1/clock.c:late_initcall(clk_disable_unused);
> ./arch/arm/mach-omap1/clock.c:late_initcall(omap_clk_enable_autoidle_all);
> ./arch/arm/mach-omap1/clock.c:late_initcall(clk_debugfs_init);
> ./arch/arm/xen/enlighten.c:late_initcall(xen_pm_init);
> ./arch/arm/probes/kprobes/test-core.c:late_initcall(run_all_tests);
> ./arch/arm/mach-mvebu/pm-board.c:late_initcall(mvebu_armada_xp_gp_pm_init);
> ./arch/unicore32/kernel/fpu-ucf64.c:late_initcall(ucf64_init);
> ./arch/avr32/mm/tlb.c:late_initcall(proctlb_init);
> ./arch/sh/kernel/cpu/sh4a/smp-shx3.c:late_initcall(register_shx3_cpu_notifier);
> ./arch/sh/kernel/cpu/shmobile/pm.c:late_initcall(sh_pm_init);
> ./arch/sh/boards/mach-hp6xx/pm.c:late_initcall(hp6x0_pm_init);
> ./arch/xtensa/platforms/iss/console.c:late_initcall(rs_init);
> ./arch/frv/kernel/setup.c:late_initcall(setup_arch_serial);
> ./arch/powerpc/kernel/machine_kexec.c:late_initcall(kexec_setup);
> ./arch/powerpc/kernel/machine_kexec_64.c:late_initcall(export_htab_values);
> ./arch/powerpc/kernel/setup-common.c:late_initcall(check_cache_coherency);
> ./arch/powerpc/kernel/iommu.c:late_initcall(fail_iommu_debugfs);
> ./arch/powerpc/lib/code-patching.c:late_initcall(test_code_patching);
> ./arch/powerpc/lib/feature-fixups.c:late_initcall(test_feature_fixups);
> ./arch/powerpc/sysdev/ppc4xx_cpm.c:late_initcall(cpm_init);
> ./arch/powerpc/sysdev/dart_iommu.c:late_initcall(iommu_init_late_dart);
> ./arch/powerpc/sysdev/msi_bitmap.c:late_initcall(msi_bitmap_selftest);
> ./arch/tile/kernel/hardwall.c:late_initcall(dev_hardwall_init);
> ./arch/tile/kernel/smpboot.c:late_initcall(reset_init_affinity);
> ./arch/arm64/kernel/fpsimd.c:late_initcall(fpsimd_init);
> ./arch/arm64/kernel/armv8_deprecated.c:late_initcall(armv8_deprecated_init);
> ./arch/arm64/kvm/sys_regs_generic_v8.c:late_initcall(sys_reg_genericv8_init);
> ./arch/m68k/kernel/early_printk.c:late_initcall(unregister_early_console);
> ./arch/sparc/kernel/leon_pmc.c:late_initcall(leon_pmc_install);
> ./arch/sparc/kernel/sstate.c:late_initcall(sstate_running);
> ./arch/metag/kernel/setup.c:late_initcall(init_machine_late);
> ./arch/x86/kernel/tboot.c:late_initcall(tboot_late_init);
> ./arch/x86/kernel/cpu/mcheck/mce-severity.c:late_initcall(severities_debugfs_init);
> ./arch/x86/kernel/cpu/mcheck/mce_amd.c:late_initcall(threshold_init_device);
> ./arch/x86/kernel/cpu/mcheck/mce.c:late_initcall(mcheck_debugfs_init);
> ./arch/x86/kernel/mpparse.c:late_initcall(update_mp_table);
> ./arch/x86/kernel/apic/x2apic_uv_x.c:late_initcall(uv_init_heartbeat);
> ./arch/x86/kernel/apic/apic.c:late_initcall(lapic_insert_resource);
> ./arch/x86/kernel/apic/vector.c:late_initcall(print_ICs);
> ./arch/x86/kernel/apic/probe_32.c:late_initcall(print_ipi_mode);
> ./arch/x86/kernel/acpi/boot.c:late_initcall(hpet_insert_resource);
> ./arch/x86/crypto/aesni-intel_glue.c:late_initcall(aesni_init);
> ./arch/x86/mm/tlb.c:late_initcall(create_tlb_single_page_flush_ceiling);
> ./arch/x86/mm/pat.c:late_initcall(pat_memtype_list_init);
> ./arch/x86/platform/intel-mid/device_libs/platform_gpio_keys.c:late_initcall(pb_keys_init);
> ./arch/x86/pci/mmconfig-shared.c:late_initcall(pci_mmcfg_late_insert_resources);
> ./arch/arc/kernel/setup.c:late_initcall(init_late_machine);
> ./arch/mips/alchemy/devboards/pm.c:late_initcall(pm_init);
> ./arch/mips/loongson64/common/mem.c:late_initcall(find_vga_mem_init);
> ./arch/mips/txx9/generic/setup_tx4927.c:late_initcall(tx4927_late_init);
> ./arch/mips/txx9/generic/setup_tx4939.c:late_initcall(tx4939_late_init);
> ./arch/mips/txx9/generic/setup_tx4938.c:late_initcall(tx4938_late_init);
> ./arch/mips/cavium-octeon/setup.c:late_initcall(octeon_no_pci_release);
> ./arch/mips/cavium-octeon/flash_setup.c:late_initcall(octeon_flash_init);
> ./arch/mips/cavium-octeon/smp.c:late_initcall(register_cavium_notifier);
> ./arch/mips/mti-malta/malta-pm.c:late_initcall(malta_pm_setup);
> ./arch/mips/jz4740/pm.c:late_initcall(jz4740_pm_init);
> ./arch/mips/sgi-ip27/ip27-timer.c:late_initcall(sgi_ip27_rtc_devinit);
> ./arch/blackfin/kernel/cplbinfo.c:late_initcall(cplbinfo_init);
> ./arch/blackfin/kernel/bfin_dma.c:late_initcall(proc_dma_init);
> ./arch/blackfin/kernel/nmi.c:late_initcall(init_nmi_wdt_syscore);
> ./arch/blackfin/mach-bf609/pm.c:late_initcall(bf609_init_pm);
> ./arch/blackfin/mm/sram-alloc.c:late_initcall(sram_proc_init);
> ./arch/blackfin/mm/isram-driver.c:late_initcall(isram_test_init);
> ./block/blk-core.c:late_initcall(fail_make_request_debugfs);
> ./block/blk-timeout.c:late_initcall(fail_io_timeout_debugfs);
> ./crypto/async_tx/raid6test.c:late_initcall(raid6_test);
> ./drivers/clk/clk.c:late_initcall(clk_debug_init);
> ./drivers/mtd/ubi/build.c:late_initcall(ubi_init);
> ./drivers/mtd/devices/block2mtd.c:late_initcall(block2mtd_init);
> ./drivers/devfreq/exynos/exynos5_bus.c:late_initcall(exynos5_busfreq_int_init);
> ./drivers/devfreq/exynos/exynos4_bus.c:late_initcall(exynos4_busfreq_init);
> ./drivers/gpu/drm/omapdrm/omap_drv.c:late_initcall(omap_drm_init);
> ./drivers/gpu/drm/gma500/psb_drv.c:late_initcall(psb_init);
> ./drivers/video/logo/logo.c:late_initcall(fb_logo_late_init);
> ./drivers/of/fdt.c:late_initcall(of_fdt_raw_init);
> ./drivers/base/dd.c:late_initcall(deferred_probe_initcall);
> ./drivers/base/power/trace.c:late_initcall(late_resume_init);
> ./drivers/base/power/domain.c:late_initcall(genpd_poweroff_unused);
> ./drivers/base/power/domain.c:late_initcall(pm_genpd_debug_init);
> ./drivers/mfd/wl1273-core.c:late_initcall(wl1273_core_init);
> ./drivers/cpufreq/speedstep-centrino.c:late_initcall(centrino_init);
> ./drivers/cpufreq/amd_freq_sensitivity.c:late_initcall(amd_freq_sensitivity_init);
> ./drivers/cpufreq/sfi-cpufreq.c:late_initcall(sfi_cpufreq_init);
> ./drivers/cpufreq/powernow-k8.c:late_initcall(powernowk8_init);
> ./drivers/cpufreq/acpi-cpufreq.c:late_initcall(acpi_cpufreq_init);
> ./drivers/cpufreq/ia64-acpi-cpufreq.c:late_initcall(acpi_cpufreq_init);
> ./drivers/cpufreq/p4-clockmod.c:late_initcall(cpufreq_p4_init);
> ./drivers/cpufreq/pcc-cpufreq.c:late_initcall(pcc_cpufreq_init);
> ./drivers/cpufreq/powernow-k7.c:late_initcall(powernow_init);
> ./drivers/cpufreq/longhaul.c:late_initcall(longhaul_init);
> ./drivers/cpufreq/s3c24xx-cpufreq-debugfs.c:late_initcall(s3c_freq_debugfs_init);
> ./drivers/cpufreq/at32ap-cpufreq.c:late_initcall(at32_cpufreq_init);
> ./drivers/cpufreq/s3c24xx-cpufreq.c:late_initcall(s3c_cpufreq_initcall);
> ./drivers/sh/clk/core.c:late_initcall(clk_late_init);
> ./drivers/sh/intc/userimask.c:late_initcall(userimask_sysdev_init);
> ./drivers/net/netconsole.c:late_initcall(init_netconsole);
> ./drivers/net/vxlan.c:late_initcall(vxlan_init_module);
> ./drivers/net/geneve.c:late_initcall(geneve_init_module);
> ./drivers/net/ethernet/ti/davinci_emac.c:late_initcall(davinci_emac_init);
> ./drivers/net/ethernet/ti/cpsw.c:late_initcall(cpsw_init);
> ./drivers/net/rionet.c:late_initcall(rionet_init);
> ./drivers/gpio/gpio-tegra.c:late_initcall(tegra_gpio_debuginit);
> ./drivers/block/hd.c:late_initcall(hd_init);
> ./drivers/thermal/db8500_cpufreq_cooling.c:late_initcall(db8500_cpufreq_cooling_init);
> ./drivers/staging/android/sync_debug.c:late_initcall(sync_debugfs_init);
> ./drivers/staging/wilc1000/linux_wlan.c:late_initcall(init_wilc_driver);
> ./drivers/iommu/dmar.c:late_initcall(dmar_free_unused_resources);
> ./drivers/watchdog/ie6xx_wdt.c:late_initcall(ie6xx_wdt_init);
> ./drivers/watchdog/intel_scu_watchdog.c:late_initcall(intel_scu_watchdog_init);
> ./drivers/tty/serial/mpsc.c:late_initcall(mpsc_late_console_init);
> ./drivers/media/platform/omap/omap_vout.c:late_initcall(omap_vout_init);
> ./drivers/media/pci/cx25821/cx25821-alsa.c:late_initcall(cx25821_alsa_init);
> ./drivers/media/pci/saa7134/saa7134-alsa.c:late_initcall(saa7134_alsa_init);
> ./drivers/dma/dmatest.c:late_initcall(dmatest_init);
> ./drivers/rapidio/rio-scan.c:late_initcall(rio_basic_attach);
> ./drivers/xen/xenbus/xenbus_probe_frontend.c:late_initcall(boot_wait_for_devices);
> ./drivers/platform/x86/dell-laptop.c:late_initcall(dell_init);
> ./drivers/firmware/memmap.c:late_initcall(firmware_memmap_init);
> ./drivers/firmware/edd.c:late_initcall(edd_init);
> ./drivers/firmware/efi/reboot.c:late_initcall(efi_shutdown_init);
> ./drivers/input/keyboard/gpio_keys.c:late_initcall(gpio_keys_init);
> ./drivers/pci/pci.c:late_initcall(pci_resource_alignment_sysfs_init);
> ./drivers/pci/pci-sysfs.c:late_initcall(pci_sysfs_init);
> ./drivers/macintosh/via-pmu-led.c:late_initcall(via_pmu_led_init);
> ./drivers/macintosh/via-pmu-event.c:late_initcall(via_pmu_event_init);
> ./drivers/ide/sgiioc4.c:late_initcall(ioc4_ide_init); /* Call only after IDE init is done */
> ./drivers/ide/ide-cs.c:late_initcall(init_ide_cs);
> ./drivers/rtc/Kconfig:    The driver for this RTC device must be loaded before late_initcall
> ./drivers/rtc/hctosys.c:late_initcall(rtc_hctosys);
> ./drivers/irqchip/irq-vic.c:late_initcall(vic_pm_init);
> ./drivers/power/avs/smartreflex.c:late_initcall(sr_init);
> ./drivers/power/charger-manager.c:late_initcall(charger_manager_init);
> ./fs/btrfs/super.c:late_initcall(init_btrfs_fs);
> ./fs/afs/main.c:late_initcall(afs_init);        /* must be called after net/ to create socket */
> ./fs/ubifs/super.c:late_initcall(ubifs_init);
> ./kernel/sched/core.c:late_initcall(sched_init_debug);
> ./kernel/time/timekeeping_debug.c:late_initcall(tk_debug_sleep_time_init);
> ./kernel/taskstats.c:late_initcall(taskstats_init);
> ./kernel/printk/printk.c:late_initcall(printk_late_init);
> ./kernel/bpf/arraymap.c:late_initcall(register_array_map);
> ./kernel/bpf/arraymap.c:late_initcall(register_prog_array_map);
> ./kernel/bpf/hashtab.c:late_initcall(register_htab_map);
> ./kernel/trace/trace_events.c:late_initcall(event_trace_self_tests_init);
> ./kernel/trace/bpf_trace.c:late_initcall(register_kprobe_prog_ops);
> ./kernel/trace/trace.c:late_initcall(clear_boot_tracer);
> ./kernel/trace/trace_kprobe.c:late_initcall(kprobe_trace_self_tests_init);
> ./kernel/trace/trace_events_filter.c:late_initcall(ftrace_test_event_filter);
> ./kernel/trace/trace_kdb.c:late_initcall(kdb_ftrace_register);
> ./kernel/trace/ring_buffer.c:late_initcall(test_ringbuffer);
> ./kernel/kprobes.c:late_initcall(debugfs_kprobe_init);
> ./kernel/rcu/update.c:late_initcall(rcu_verify_early_boot_tests);
> ./kernel/panic.c:late_initcall(init_oops_id);
> ./kernel/system_keyring.c:late_initcall(load_system_certificate_list);
> ./kernel/power/qos.c:late_initcall(pm_qos_power_init);
> ./kernel/power/Kconfig:   late_initcall.
> ./kernel/power/main.c:late_initcall(pm_debugfs_init);
> ./kernel/power/suspend_test.c:late_initcall(test_suspend);
> ./lib/list_sort.c:late_initcall(list_sort_test);
> ./lib/random32.c:late_initcall(prandom_reseed);
> ./mm/kmemleak.c:late_initcall(kmemleak_late_init);
> ./mm/failslab.c:late_initcall(failslab_debugfs_init);
> ./mm/memory.c:late_initcall(fault_around_debugfs);
> ./mm/early_ioremap.c:late_initcall(check_early_ioremap_leak);
> ./mm/cma_debug.c:late_initcall(cma_debugfs_init);
> ./mm/page_owner.c:late_initcall(pageowner_init)
> ./mm/page_alloc.c:late_initcall(fail_page_alloc_debugfs);
> ./mm/swapfile.c:late_initcall(max_swapfiles_check);
> ./mm/zswap.c:late_initcall(init_zswap);
> ./net/ipv4/ipconfig.c:late_initcall(ip_auto_config);
> ./net/ipv4/tcp_cong.c:late_initcall(tcp_congestion_default);
> ./net/core/filter.c:late_initcall(register_sk_filter_ops);
> ./net/bluetooth/selftest.c:late_initcall(bt_selftest_init);
> ./security/integrity/evm/evm_main.c:late_initcall(init_evm);
> ./security/integrity/ima/ima_main.c:late_initcall(init_ima);    /* Start IMA after the TPM is available */
> ./security/apparmor/crypto.c:late_initcall(init_profile_hash);
> ./security/keys/trusted.c:late_initcall(init_trusted);
> ./security/keys/encrypted-keys/encrypted.c:late_initcall(init_encrypted);
> ./security/keys/process_keys.c:late_initcall(init_root_keyring);
> ./sound/soc/fsl/p1022_rdk.c:late_initcall(p1022_rdk_init);
> ./sound/soc/fsl/phycore-ac97.c:late_initcall(imx_phycore_init);
>
> --
> regards,
> -grygorii
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 02/18] of/platform: add of_platform_probe

[Tomeu Vizoso]

On 11 August 2015 at 11:37, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
> On 7 August 2015 at 14:19, Mark Brown <broonie@kernel.org> wrote:
>> On Thu, Aug 06, 2015 at 04:11:39PM +0200, Tomeu Vizoso wrote:
>>
>>> Walks the OF tree up and finds the closest ancestor that has a platform
>>> device associated with it, probing it if isn't bound to a driver yet.
>>
>>> The above should ensure that the dependency represented by the passed OF
>>> node is available, because probing a platform device should cause its
>>> descendants to be probed as well.
>>
>> This sounds like it's going to break in the case where we have MFDs that
>> represent their functions in DT (not a pattern I'm a fan of but it's a
>> thing people do).  We'll walk back to the platform device for the MFD
>> function, try to probe it and then give up.  Perhaps that's good enough
>> anyway but it's not clear to me why we don't just try every parent we
>> find?
>
> Agreed. In the attempt at probing dependencies before a device is
> probed, I considered that a device's parent is also a dependency and
> that worked well. From what I saw, few devices will defer their probe
> if their parent hasn't been probed yet, assuming that it will have
> probed already. But with simple-mfd and simple-bus that shouldn't be
> relied upon as things will break if their parents defer their probe.
> With async probing enabled this failure scenario becomes more
> probable.

Actually I'm not sure how we could probe the ascendants on demand, as
currently the parent's device lock is taken when probing so trying to
probe a sibling from within a probe callback will cause a deadlock.

AFAICS this is only needed for USB interface devices and this
behaviour could be limited to them, but I don't like much assuming
that no USB device will ever have a dependency on a sibling (though
that probably won't happen ever).

Regards,

Tomeu

>> I'm also not a fan of the fact that the interface here is explicitly
>> saying that we want to probe a platform device, that's an implementation
>> detail that callers shouldn't need to know about.  From the point of
>> view of the callers what they're trying to do is kick any dependencies
>> into being instantiated, the fact that we currently try to accomplish it
>> with platform devices isn't something they care about.
>
> Agreed.
>
> Thanks,
>
> Tomeu

Re: [PATCH v3 02/18] of/platform: add of_platform_probe

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1382 bytes --]

On Mon, Sep 07, 2015 at 02:31:06PM +0200, Tomeu Vizoso wrote:
> On 11 August 2015 at 11:37, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
> > On 7 August 2015 at 14:19, Mark Brown <broonie@kernel.org> wrote:

> >> This sounds like it's going to break in the case where we have MFDs that
> >> represent their functions in DT (not a pattern I'm a fan of but it's a
> >> thing people do).  We'll walk back to the platform device for the MFD
> >> function, try to probe it and then give up.  Perhaps that's good enough
> >> anyway but it's not clear to me why we don't just try every parent we
> >> find?

> > Agreed. In the attempt at probing dependencies before a device is
> > probed, I considered that a device's parent is also a dependency and

> Actually I'm not sure how we could probe the ascendants on demand, as
> currently the parent's device lock is taken when probing so trying to
> probe a sibling from within a probe callback will cause a deadlock.

How do silbilings come into this?  There is an issue there but it's
going to happen anyway.

> AFAICS this is only needed for USB interface devices and this
> behaviour could be limited to them, but I don't like much assuming
> that no USB device will ever have a dependency on a sibling (though
> that probably won't happen ever).

I don't see the connection with USB here, sorry - my initial thought was
about MFDs?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH v3 02/18] of/platform: add of_platform_probe

[Tomeu Vizoso]

On 11 September 2015 at 11:57, Mark Brown <broonie@kernel.org> wrote:
> On Mon, Sep 07, 2015 at 02:31:06PM +0200, Tomeu Vizoso wrote:
>> On 11 August 2015 at 11:37, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
>> > On 7 August 2015 at 14:19, Mark Brown <broonie@kernel.org> wrote:
>
>> >> This sounds like it's going to break in the case where we have MFDs that
>> >> represent their functions in DT (not a pattern I'm a fan of but it's a
>> >> thing people do).  We'll walk back to the platform device for the MFD
>> >> function, try to probe it and then give up.  Perhaps that's good enough
>> >> anyway but it's not clear to me why we don't just try every parent we
>> >> find?
>
>> > Agreed. In the attempt at probing dependencies before a device is
>> > probed, I considered that a device's parent is also a dependency and
>
>> Actually I'm not sure how we could probe the ascendants on demand, as
>> currently the parent's device lock is taken when probing so trying to
>> probe a sibling from within a probe callback will cause a deadlock.
>
> How do silbilings come into this?  There is an issue there but it's
> going to happen anyway.

Once a platform device (with the platform bus as its parent) is
retrieved from the deferred queue, both the parent and the device in
question are locked (because of the USB stuff mentioned below). If
that device depends on another device whose parent is the platform bus
and we try to probe it (useless, but I don't see a good way of
avoiding it) then we'll deadlock when device_attach locks that device.

>> AFAICS this is only needed for USB interface devices and this
>> behaviour could be limited to them, but I don't like much assuming
>> that no USB device will ever have a dependency on a sibling (though
>> that probably won't happen ever).
>
> I don't see the connection with USB here, sorry - my initial thought was
> about MFDs?

This commit introduced locking of a device's parent before it's
probed, mainly for USB interfaces:

bf74ad5bc417 ("[PATCH] Hold the device's parent's lock during probe and remove")

Regards,

Tomeu

Re: [PATCH v3 02/18] of/platform: add of_platform_probe

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 679 bytes --]

On Fri, Sep 11, 2015 at 04:06:07PM +0200, Tomeu Vizoso wrote:

> Once a platform device (with the platform bus as its parent) is
> retrieved from the deferred queue, both the parent and the device in
> question are locked (because of the USB stuff mentioned below). If
> that device depends on another device whose parent is the platform bus
> and we try to probe it (useless, but I don't see a good way of
> avoiding it) then we'll deadlock when device_attach locks that device.

Ugh, that's nasty.  Trying to fix this would most likely devolve into
trying to shove things onto the deferred list in the right order but
that's definitely a very different solution with problems.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH v3 02/18] of/platform: add of_platform_probe

[Tomeu Vizoso]

On 11 September 2015 at 17:35, Mark Brown <broonie@kernel.org> wrote:
> On Fri, Sep 11, 2015 at 04:06:07PM +0200, Tomeu Vizoso wrote:
>
>> Once a platform device (with the platform bus as its parent) is
>> retrieved from the deferred queue, both the parent and the device in
>> question are locked (because of the USB stuff mentioned below). If
>> that device depends on another device whose parent is the platform bus
>> and we try to probe it (useless, but I don't see a good way of
>> avoiding it) then we'll deadlock when device_attach locks that device.
>
> Ugh, that's nasty.  Trying to fix this would most likely devolve into
> trying to shove things onto the deferred list in the right order but
> that's definitely a very different solution with problems.

Well, that's effectively what my "ordered probing" series did (and
indeed didn't have this problem and parents were made sure to have
probed before their children), but if we want to have dependency
information before the probe starts, then we cannot reuse the
implementation of the DT bindings that we already have in the resource
getters as this series do.

Regards,

Tomeu