[PATCH 00/21] On-demand device registration

[PATCH 00/21] On-demand device registration

[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.

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 registering 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.

Something I'm not completely happy with is that I have had to move the call to
of_platform_populate after all platform drivers have been registered.
Otherwise I don't see how I could register drivers on demand as we don't have
yet each driver's compatible strings.

For machs that don't move of_platform_populate() to a later point, these
patches shouldn't cause any problems but it's not guaranteed that we'll avoid
all the deferred probes as some drivers may not be registered yet.

I have tested this on boards with Tegra, iMX.6 and Exynos SoCs, and these
patches were enough to eliminate all the deferred probes.

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

Tomeu Vizoso (21):
  regulator: core: Reduce critical area in _regulator_get
  ARM: tegra: Add gpio-ranges property
  ARM: tegra: Register drivers before devices
  ARM: EXYNOS: Register drivers before devices
  ARM i.MX6q: Register drivers before devices
  of/platform: Add of_platform_device_ensure()
  of/platform: Ensure device registration on lookup
  gpio: Probe GPIO drivers on demand
  gpio: Probe pinctrl devices on demand
  regulator: core: Probe regulators on demand
  drm: Probe panels on demand
  drm/tegra: Probe dpaux devices on demand
  i2c: core: Probe i2c master 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

 arch/arm/boot/dts/tegra124.dtsi     |  1 +
 arch/arm/mach-exynos/exynos.c       |  4 +--
 arch/arm/mach-imx/mach-imx6q.c      | 12 ++++-----
 arch/arm/mach-tegra/tegra.c         | 21 ++++++---------
 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              |  3 +++
 drivers/of/platform.c               | 53 +++++++++++++++++++++++++++++++++++++
 drivers/phy/phy-core.c              |  3 +++
 drivers/pinctrl/devicetree.c        |  2 ++
 drivers/power/power_supply_core.c   |  3 +++
 drivers/pwm/core.c                  |  3 +++
 drivers/regulator/core.c            | 45 +++++++++++++++----------------
 drivers/usb/phy/phy.c               |  3 +++
 drivers/video/backlight/backlight.c |  3 +++
 include/linux/of_platform.h         |  2 ++
 19 files changed, 130 insertions(+), 45 deletions(-)

-- 
2.4.1

[PATCH 01/21] regulator: core: Reduce critical area in _regulator_get

[Tomeu Vizoso]

...by moving the locking to regulator_dev_lookup.

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>]
(of_platform_device_create_pdata+0x88/0xbc)
(of_platform_device_create_pdata) from [<c095caac>]
(of_platform_bus_create+0xec/0x30c)
(of_platform_bus_create) from [<c095cb08>]
(of_platform_bus_create+0x148/0x30c)
(of_platform_bus_create) from [<c095cf28>]
(of_platform_device_ensure+0xbc/0xe0)
(of_platform_device_ensure) from [<c05ec33c>]
(regulator_dev_lookup+0x80/0x1e8)
(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>
---
 drivers/regulator/core.c | 43 +++++++++++++++++++------------------------
 1 file changed, 19 insertions(+), 24 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 443eaab..50f4404 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1286,10 +1286,14 @@ 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_unlock(&regulator_list_mutex);
 					return r;
+				}
+			mutex_unlock(&regulator_list_mutex);
 			*ret = -EPROBE_DEFER;
 			return NULL;
 		} else {
@@ -1307,9 +1311,13 @@ 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_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 */
@@ -1395,8 +1403,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;
@@ -1408,7 +1414,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";
@@ -1428,34 +1434,26 @@ static struct regulator *_regulator_get(struct device *dev, const char *id,
 		dev_warn(dev, "dummy supplies not allowed\n");
 	}
 
-	mutex_unlock(&regulator_list_mutex);
 	return regulator;
 
 found:
-	if (rdev->exclusive) {
-		regulator = ERR_PTR(-EPERM);
-		goto out;
-	}
+	if (rdev->exclusive)
+		return ERR_PTR(-EPERM);
 
-	if (exclusive && rdev->open_count) {
-		regulator = ERR_PTR(-EBUSY);
-		goto out;
-	}
+	if (exclusive && rdev->open_count)
+		return ERR_PTR(-EBUSY);
 
 	ret = regulator_resolve_supply(rdev);
-	if (ret < 0) {
-		regulator = ERR_PTR(ret);
-		goto out;
-	}
+	if (ret < 0)
+		return ERR_PTR(ret);
 
 	if (!try_module_get(rdev->owner))
-		goto out;
+		return regulator;
 
 	regulator = create_regulator(rdev, dev, id);
 	if (regulator == NULL) {
-		regulator = ERR_PTR(-ENOMEM);
 		module_put(rdev->owner);
-		goto out;
+		return ERR_PTR(-ENOMEM);
 	}
 
 	rdev->open_count++;
@@ -1469,9 +1467,6 @@ found:
 			rdev->use_count = 0;
 	}
 
-out:
-	mutex_unlock(&regulator_list_mutex);
-
 	return regulator;
 }
 
-- 
2.4.1

[PATCH 02/21] ARM: tegra: Add gpio-ranges property

[Tomeu Vizoso]

Specify how the GPIOs map to the pins in T124, so the dependency is
explicit.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 arch/arm/boot/dts/tegra124.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/tegra124.dtsi b/arch/arm/boot/dts/tegra124.dtsi
index 13cc7ca..5d1d35f 100644
--- a/arch/arm/boot/dts/tegra124.dtsi
+++ b/arch/arm/boot/dts/tegra124.dtsi
@@ -254,6 +254,7 @@
 		gpio-controller;
 		#interrupt-cells = <2>;
 		interrupt-controller;
+		gpio-ranges = <&pinmux 0 0 250>;
 	};
 
 	apbdma: dma@0,60020000 {
-- 
2.4.1

[PATCH 03/21] ARM: tegra: Register drivers before devices

[Tomeu Vizoso]

So devices can be probed on demand, we need to have the drivers already
registered as we don't have enough information to register a driver on
demand.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 arch/arm/mach-tegra/tegra.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c
index 861d884..2874425 100644
--- a/arch/arm/mach-tegra/tegra.c
+++ b/arch/arm/mach-tegra/tegra.c
@@ -55,6 +55,8 @@
 #include "reset.h"
 #include "sleep.h"
 
+static struct soc_device *soc_dev;
+
 /*
  * Storage for debug-macro.S's state.
  *
@@ -87,12 +89,10 @@ static void __init tegra_dt_init_irq(void)
 static void __init tegra_dt_init(void)
 {
 	struct soc_device_attribute *soc_dev_attr;
-	struct soc_device *soc_dev;
-	struct device *parent = NULL;
 
 	soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
 	if (!soc_dev_attr)
-		goto out;
+		return;
 
 	soc_dev_attr->family = kasprintf(GFP_KERNEL, "Tegra");
 	soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%d",
@@ -105,17 +105,7 @@ static void __init tegra_dt_init(void)
 		kfree(soc_dev_attr->revision);
 		kfree(soc_dev_attr->soc_id);
 		kfree(soc_dev_attr);
-		goto out;
 	}
-
-	parent = soc_device_to_device(soc_dev);
-
-	/*
-	 * Finished with the static registrations now; fill in the missing
-	 * devices
-	 */
-out:
-	of_platform_populate(NULL, of_default_bus_match_table, NULL, parent);
 }
 
 static void __init paz00_init(void)
@@ -133,8 +123,13 @@ static struct {
 
 static void __init tegra_dt_init_late(void)
 {
+	struct device *parent = NULL;
 	int i;
 
+	parent = soc_device_to_device(soc_dev);
+
+	of_platform_populate(NULL, of_default_bus_match_table, NULL, parent);
+
 	tegra_init_suspend();
 	tegra_cpuidle_init();
 
-- 
2.4.1

[PATCH 04/21] ARM: EXYNOS: Register drivers before devices

[Tomeu Vizoso]

So devices can be probed on demand, we need to have the drivers already
registered as we don't have enough information to register a driver on
demand.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 arch/arm/mach-exynos/exynos.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
index 5917a30..3c98c92 100644
--- a/arch/arm/mach-exynos/exynos.c
+++ b/arch/arm/mach-exynos/exynos.c
@@ -110,6 +110,8 @@ void __init exynos_sysram_init(void)
 
 static void __init exynos_init_late(void)
 {
+	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
+
 	if (of_machine_is_compatible("samsung,exynos5440"))
 		/* to be supported later */
 		return;
@@ -246,8 +248,6 @@ static void __init exynos_dt_machine_init(void)
 		platform_device_register(&exynos_cpuidle);
 
 	platform_device_register_simple("exynos-cpufreq", -1, NULL, 0);
-
-	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
 }
 
 static char const *const exynos_dt_compat[] __initconst = {
-- 
2.4.1

[PATCH 05/21] ARM i.MX6q: Register drivers before devices

[Tomeu Vizoso]

So devices can be probed on demand, we need to have the drivers already
registered as we don't have enough information to register a driver on
demand.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 arch/arm/mach-imx/mach-imx6q.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
index 3ab6154..88de92c 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -262,21 +262,19 @@ static void __init imx6q_axi_init(void)
 	}
 }
 
+static struct device *soc_dev;
+
 static void __init imx6q_init_machine(void)
 {
-	struct device *parent;
-
 	imx_print_silicon_rev(cpu_is_imx6dl() ? "i.MX6DL" : "i.MX6Q",
 			      imx_get_soc_revision());
 
-	parent = imx_soc_device_init();
-	if (parent == NULL)
+	soc_dev = imx_soc_device_init();
+	if (soc_dev == NULL)
 		pr_warn("failed to initialize soc device\n");
 
 	imx6q_enet_phy_init();
 
-	of_platform_populate(NULL, of_default_bus_match_table, NULL, parent);
-
 	imx_anatop_init();
 	cpu_is_imx6q() ?  imx6q_pm_init() : imx6dl_pm_init();
 	imx6q_1588_init();
@@ -367,6 +365,8 @@ static struct platform_device imx6q_cpufreq_pdev = {
 
 static void __init imx6q_init_late(void)
 {
+	of_platform_populate(NULL, of_default_bus_match_table, NULL, soc_dev);
+
 	/*
 	 * WAIT mode is broken on TO 1.0 and 1.1, so there is no point
 	 * to run cpuidle on them.
-- 
2.4.1

[PATCH 06/21] of/platform: Add of_platform_device_ensure()

[Tomeu Vizoso]

This function ensures that the device that encloses the passed device
node is registered, and thus probed if the corresponding driver has been
registered already.

This function can be used by drivers to ensure that a dependency is
fulfilled.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 drivers/of/platform.c       | 51 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/of_platform.h |  2 ++
 2 files changed, 53 insertions(+)

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index a01f57c..cc5d808 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -499,6 +499,57 @@ void of_platform_depopulate(struct device *parent)
 }
 EXPORT_SYMBOL_GPL(of_platform_depopulate);
 
+/**
+ * of_platform_device_ensure - Ensure that device has been registered
+ * @np: pointer to node to create device for
+ *
+ * Ensures that a device has been registered for the given node.
+ */
+void of_platform_device_ensure(struct device_node *np)
+{
+	struct device_node *iter, *node = NULL;
+	int rc;
+
+	/* Nothing to be done here */
+	if (of_node_check_flag(np, OF_POPULATED))
+		return;
+
+	/* Too early for us to do anything useful */
+	if (!platform_bus.kobj.state_initialized)
+		return;
+
+	/* Find the node of the platform device enclosing this node */
+	for (iter = np;
+	     iter && iter->parent;
+	     iter = iter->parent) {
+		if (of_get_property(iter, "compatible", NULL))
+			node = iter;
+
+		/*
+		 * If the requested and requester nodes have a common ancestor
+		 * that is being populated at this moment, register the device
+		 * associated with the child of this ancestor that includes
+		 * the requested node.
+		 */
+		if (of_node_check_flag(iter->parent, OF_POPULATED) &&
+		    of_match_node(of_default_bus_match_table, iter->parent))
+			break;
+
+		if (of_node_is_root(iter->parent))
+			break;
+	}
+
+	if (!node)
+		return;
+
+	rc = of_platform_bus_create(node, of_default_bus_match_table, NULL,
+				    NULL, true);
+	if (rc)
+		pr_debug("%s: creation of platform device failed\n",
+			 np->full_name);
+}
+EXPORT_SYMBOL_GPL(of_platform_device_ensure);
+
 #ifdef CONFIG_OF_DYNAMIC
 static int of_platform_notify(struct notifier_block *nb,
 				unsigned long action, void *arg)
diff --git a/include/linux/of_platform.h b/include/linux/of_platform.h
index 611a691..df17890 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_device_ensure(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_device_ensure(struct device_node *np) { }
 #endif
 
 #if defined(CONFIG_OF_DYNAMIC) && defined(CONFIG_OF_ADDRESS)
-- 
2.4.1

[PATCH 07/21] of/platform: Ensure device registration on lookup

[Tomeu Vizoso]

When looking up a platform device from its device node, ensure that the
device has been registered before doing the actual search.

This increases the chances of the device having been probed by that
time, reducing deferred probes.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 drivers/of/platform.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index cc5d808..af2bc75 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -46,6 +46,8 @@ struct platform_device *of_find_device_by_node(struct device_node *np)
 {
 	struct device *dev;
 
+	of_platform_device_ensure(np);
+
 	dev = bus_find_device(&platform_bus_type, NULL, np, of_dev_node_match);
 	return dev ? to_platform_device(dev) : NULL;
 }
-- 
2.4.1

[PATCH 08/21] gpio: Probe GPIO drivers on demand

[Tomeu Vizoso]

Ensure that the device corresponding to the DT node of the gpiochip has
been registered.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 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 a6c67c6..190183d 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_device_ensure(gg_data.gpiospec.np);
+
 	gpiochip_find(&gg_data, of_gpiochip_find_and_xlate);
 
 	of_node_put(gg_data.gpiospec.np);
-- 
2.4.1

[PATCH 09/21] gpio: Probe pinctrl devices on demand

[Tomeu Vizoso]

Ensure that the device corresponding to the DT node of the pin
controller has been registered.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 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 190183d..e26e105 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -362,6 +362,8 @@ static void of_gpiochip_add_pin_range(struct gpio_chip *chip)
 		if (ret)
 			break;
 
+		of_platform_device_ensure(pinspec.np);
+
 		pctldev = of_pinctrl_get(pinspec.np);
 		if (!pctldev)
 			break;
-- 
2.4.1

[PATCH 10/21] regulator: core: Probe regulators on demand

[Tomeu Vizoso]

When looking up a regulator through its DT node, ensure that the
corresponding device has been registered.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 drivers/regulator/core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 50f4404..3ac44fe 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>
@@ -1286,6 +1287,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_device_ensure(node);
 			mutex_lock(&regulator_list_mutex);
 			list_for_each_entry(r, &regulator_list, list)
 				if (r->dev.parent &&
-- 
2.4.1

[PATCH 11/21] drm: Probe panels on demand

[Tomeu Vizoso]

When looking up a panel through its DT node, ensure that the
corresponding device has been registered.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 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 2ef988e..041211e 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_device_ensure(np);
+
 	mutex_lock(&panel_lock);
 
 	list_for_each_entry(panel, &panel_list, list) {
-- 
2.4.1

[PATCH 12/21] drm/tegra: Probe dpaux devices on demand

[Tomeu Vizoso]

When looking up a dpaux device through its DT node, ensure that the
corresponding device has been registered.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 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 d6b55e3..10bba41 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>
@@ -397,6 +398,8 @@ struct tegra_dpaux *tegra_dpaux_find_by_of_node(struct device_node *np)
 {
 	struct tegra_dpaux *dpaux;
 
+	of_platform_device_ensure(np);
+
 	mutex_lock(&dpaux_lock);
 
 	list_for_each_entry(dpaux, &dpaux_list, list)
-- 
2.4.1

[PATCH 13/21] i2c: core: Probe i2c master devices on demand

[Tomeu Vizoso]

When looking up an i2c master through its DT node, ensure that the
corresponding device has been registered.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 drivers/i2c/i2c-core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 987c124..8c9352e 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>
@@ -1339,6 +1340,8 @@ struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node)
 {
 	struct device *dev;
 
+	of_platform_device_ensure(node);
+
 	dev = bus_find_device(&i2c_bus_type, NULL, node,
 					 of_dev_node_match);
 	if (!dev)
-- 
2.4.1

[PATCH 14/21] pwm: Probe PWM chip devices on demand

[Tomeu Vizoso]

When looking up a PWM chip through its DT node, ensure that the
corresponding device has been registered.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 drivers/pwm/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index ba34c7d..31f486c 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -24,6 +24,7 @@
 #include <linux/radix-tree.h>
 #include <linux/list.h>
 #include <linux/mutex.h>
+#include <linux/of_platform.h>
 #include <linux/err.h>
 #include <linux/slab.h>
 #include <linux/device.h>
@@ -479,6 +480,8 @@ static struct pwm_chip *of_node_to_pwmchip(struct device_node *np)
 {
 	struct pwm_chip *chip;
 
+	of_platform_device_ensure(np);
+
 	mutex_lock(&pwm_lock);
 
 	list_for_each_entry(chip, &pwm_chips, list)
-- 
2.4.1

[PATCH 15/21] backlight: Probe backlight devices on demand

[Tomeu Vizoso]

When looking up a backlight device through its DT node, ensure that the
corresponding device has been registered.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 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 bddc8b1..fe7833b 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -12,6 +12,7 @@
 #include <linux/device.h>
 #include <linux/backlight.h>
 #include <linux/notifier.h>
+#include <linux/of_platform.h>
 #include <linux/ctype.h>
 #include <linux/err.h>
 #include <linux/fb.h>
@@ -559,6 +560,8 @@ struct backlight_device *of_find_backlight_by_node(struct device_node *node)
 {
 	struct device *dev;
 
+	of_platform_device_ensure(node);
+
 	dev = class_find_device(backlight_class, NULL, node, of_parent_match);
 
 	return dev ? to_backlight_device(dev) : NULL;
-- 
2.4.1

[PATCH 16/21] usb: phy: Probe phy devices on demand

[Tomeu Vizoso]

When looking up a phy through its DT node, ensure that the corresponding
device has been registered.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 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 d1cd6b5..7084f21 100644
--- a/drivers/usb/phy/phy.c
+++ b/drivers/usb/phy/phy.c
@@ -13,6 +13,7 @@
 #include <linux/err.h>
 #include <linux/device.h>
 #include <linux/module.h>
+#include <linux/of_platform.h>
 #include <linux/slab.h>
 #include <linux/of.h>
 
@@ -192,6 +193,8 @@ struct usb_phy *devm_usb_get_phy_by_phandle(struct device *dev,
 		goto err0;
 	}
 
+	of_platform_device_ensure(node);
+
 	spin_lock_irqsave(&phy_lock, flags);
 
 	phy = __of_usb_find_phy(node);
-- 
2.4.1

[PATCH 17/21] clk: Probe clk providers on demand

[Tomeu Vizoso]

When looking up a clk through its DT node, ensure that the corresponding
device has been registered.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 drivers/clk/clk.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 5b0f418..f9e8a8d 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -18,6 +18,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>
@@ -2985,6 +2986,8 @@ struct clk *__of_clk_get_from_provider(struct of_phandle_args *clkspec,
 	if (!clkspec)
 		return ERR_PTR(-EINVAL);
 
+	of_platform_device_ensure(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.1

[PATCH 18/21] pinctrl: Probe pinctrl devices on demand

[Tomeu Vizoso]

When looking up a pin controller through its DT node, ensure that the
corresponding device has been registered.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 drivers/pinctrl/devicetree.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/pinctrl/devicetree.c b/drivers/pinctrl/devicetree.c
index 0bbf7d7..17231ea 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>
 
@@ -118,6 +119,7 @@ static int dt_to_map_one_config(struct pinctrl *p, const char *statename,
 
 	/* Find the pin controller containing np_config */
 	np_pctldev = of_node_get(np_config);
+	of_platform_device_ensure(np_pctldev);
 	for (;;) {
 		np_pctldev = of_get_next_parent(np_pctldev);
 		if (!np_pctldev || of_node_is_root(np_pctldev)) {
-- 
2.4.1

[PATCH 19/21] phy: core: Probe phy providers on demand

[Tomeu Vizoso]

When looking up a phy through its DT node, ensure that the corresponding
device has been registered.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 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 3791838..cdb31bd 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_device_ensure(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.1

[PATCH 20/21] dma: of: Probe DMA controllers on demand

[Tomeu Vizoso]

When looking up a DMA controller through its DT node, ensure that the
corresponding device has been registered.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 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 cbd4a8a..2bdb3da 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);
@@ -174,6 +175,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_device_ensure(dma_spec.np);
+
 		mutex_lock(&of_dma_lock);
 		ofdma = of_dma_find_controller(&dma_spec);
 
-- 
2.4.1

[PATCH 21/21] power-supply: Probe power supplies on demand

[Tomeu Vizoso]

When looking up a power supply through its DT node, ensure that the
corresponding device has been registered.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 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 2ed4a4a..3dbfce8 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -17,6 +17,7 @@
 #include <linux/device.h>
 #include <linux/notifier.h>
 #include <linux/err.h>
+#include <linux/of_platform.h>
 #include <linux/power_supply.h>
 #include <linux/thermal.h>
 #include "power_supply.h"
@@ -180,6 +181,8 @@ static int power_supply_find_supply_from_node(struct device_node *supply_node)
 {
 	int error;
 
+	of_platform_device_ensure(supply_node);
+
 	/*
 	 * class_for_each_device() either returns its own errors or values
 	 * returned by __power_supply_find_supply_from_node().
-- 
2.4.1

Re: [PATCH 01/21] regulator: core: Reduce critical area in _regulator_get

[Mark Brown]

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

On Mon, May 25, 2015 at 04:53:05PM +0200, Tomeu Vizoso wrote:
> ...by moving the locking to regulator_dev_lookup.

I appear to have only patches 1 and 10 of a 21 patch series.  It is
difficult to tell what the series might be about or how any of the
patches in it might be related to each other, it doesn't look like the
sort of obviously repetitive series which is easily understandable.
Please don't do things like this, if you're sending a series people need
to be able to tell what it is.

What's going on here?

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

Re: [PATCH 10/21] regulator: core: Probe regulators on demand

[Mark Brown]

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

On Mon, May 25, 2015 at 04:53:14PM +0200, Tomeu Vizoso wrote:

> When looking up a regulator through its DT node, ensure that the
> corresponding device has been registered.

I'm sorry but I'm going to need a much better changelog to review this
patch.  I can't entirely tell what the intended effect is supposed to
be, the subject line says we're probing things and the body says we're
registering things which aren't the same thing at all.

>  		if (node) {
> +			of_platform_device_ensure(node);

This function doesn't appear in git so I'm guessing that some other
patch in the series adds it.  See my reply to the first patch...

The obvious questions here based on the name are why we're doing
something specific to platform devices and why this isn't something
we're abstracting in the driver core (or at least generic firmware code)
- we're going to have the same thing with ACPI.

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

Re: [PATCH 01/21] regulator: core: Reduce critical area in _regulator_get

[Mark Brown]

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

On Mon, May 25, 2015 at 04:53:05PM +0200, Tomeu Vizoso wrote:

> ...by moving the locking to regulator_dev_lookup.

This breaks the locking.

> +	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_unlock(&regulator_list_mutex);
>  			return r;
> +		}
> +	mutex_unlock(&regulator_list_mutex);

This means that we hold the lock while we walk the list but then return
a pointer to a regulator device that we don't have a reference to.  This
then means that something else can unregister the regulator we're
holding a pointer to which means that it is unsafe to use that pointer.
We need to ensure that any objects we look up via the list remain valid
while we're working with them.

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

Re: [PATCH 04/21] ARM: EXYNOS: Register drivers before devices

[Krzysztof Kozlowski]

2015-05-25 23:53 GMT+09:00 Tomeu Vizoso <tomeu.vizoso@collabora.com>:
> So devices can be probed on demand, we need to have the drivers already
> registered as we don't have enough information to register a driver on
> demand.
>
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>

This is 4/21 but I can't see other patches. I found the cover letter
on samsung-soc list but where is the rest of this?

Best regards,
Krzysztof

Re: [PATCH 10/21] regulator: core: Probe regulators on demand

[Tomeu Vizoso]

On 25 May 2015 at 19:32, Mark Brown <broonie@kernel.org> wrote:
> On Mon, May 25, 2015 at 04:53:14PM +0200, Tomeu Vizoso wrote:
>
>> When looking up a regulator through its DT node, ensure that the
>> corresponding device has been registered.
>
> I'm sorry but I'm going to need a much better changelog to review this
> patch.  I can't entirely tell what the intended effect is supposed to
> be, the subject line says we're probing things and the body says we're
> registering things which aren't the same thing at all.

Agreed, it would be more appropriate to say that we are registering
devices, I will make it clearer in the next revision. It ends up
probing the device at that point because we have made sure with an
earlier patch that all built-in drivers have been registered already.

>>               if (node) {
>> +                     of_platform_device_ensure(node);
>
> This function doesn't appear in git so I'm guessing that some other
> patch in the series adds it.  See my reply to the first patch...

Sorry about that, the series touches so many subsystems that I would
have reached the maximum number of recipients on the mailing lists I'm
sending it to. I will send it next with you and Krzysztof on CC on the
whole series.

> The obvious questions here based on the name are why we're doing
> something specific to platform devices and why this isn't something
> we're abstracting in the driver core (or at least generic firmware code)
> - we're going to have the same thing with ACPI.

I don't know how useful this is going to be in systems with ACPI. My
experience is limited to 32bit ARM, where the kernel has to manage
every regulator, clock, gpio, etc so the dependency tree is so big. Is
deferred probing a problem with ACPI as well?

Thanks,

Tomeu

Re: [PATCH 15/21] backlight: Probe backlight devices on demand

[Lee Jones]

On Mon, 25 May 2015, Tomeu Vizoso wrote:

> When looking up a backlight device through its DT node, ensure that the
> corresponding device has been registered.
> 
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> ---
>  drivers/video/backlight/backlight.c | 3 +++
>  1 file changed, 3 insertions(+)

Looks reasonable.

Until anyone screams at me, applied thanks.

> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
> index bddc8b1..fe7833b 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -12,6 +12,7 @@
>  #include <linux/device.h>
>  #include <linux/backlight.h>
>  #include <linux/notifier.h>
> +#include <linux/of_platform.h>
>  #include <linux/ctype.h>
>  #include <linux/err.h>
>  #include <linux/fb.h>
> @@ -559,6 +560,8 @@ struct backlight_device *of_find_backlight_by_node(struct device_node *node)
>  {
>  	struct device *dev;
>  
> +	of_platform_device_ensure(node);
> +
>  	dev = class_find_device(backlight_class, NULL, node, of_parent_match);
>  
>  	return dev ? to_backlight_device(dev) : NULL;

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 15/21] backlight: Probe backlight devices on demand

[Sascha Hauer]

On Tue, May 26, 2015 at 08:18:50AM +0100, Lee Jones wrote:
> On Mon, 25 May 2015, Tomeu Vizoso wrote:
> 
> > When looking up a backlight device through its DT node, ensure that the
> > corresponding device has been registered.
> > 
> > Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> > ---
> >  drivers/video/backlight/backlight.c | 3 +++
> >  1 file changed, 3 insertions(+)
> 
> Looks reasonable.
> 
> Until anyone screams at me, applied thanks.

The compiler will scream at you when it realizes that
of_platform_device_ensure() doesn't exist in your kernel...

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 15/21] backlight: Probe backlight devices on demand

[Lee Jones]

On Tue, 26 May 2015, Sascha Hauer wrote:
> On Tue, May 26, 2015 at 08:18:50AM +0100, Lee Jones wrote:
> > On Mon, 25 May 2015, Tomeu Vizoso wrote:
> > 
> > > When looking up a backlight device through its DT node, ensure that the
> > > corresponding device has been registered.
> > > 
> > > Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> > > ---
> > >  drivers/video/backlight/backlight.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > 
> > Looks reasonable.
> > 
> > Until anyone screams at me, applied thanks.
> 
> The compiler will scream at you when it realizes that
> of_platform_device_ensure() doesn't exist in your kernel...

Yup, indeed it did.

I assumed this was *only* enabling subsystems and that the
framework/API was already accepted.

So the advice I'd give to Tomeu when sending full enablement
patch-sets i.e. ones which provide the framework/API *and* enable
subsystems in the same set, is to send the entire set to everyone, so
we can see what the aim of the set is and how to deal with it.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 10/21] regulator: core: Probe regulators on demand

[Mark Brown]

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

On Tue, May 26, 2015 at 08:17:23AM +0200, Tomeu Vizoso wrote:
> On 25 May 2015 at 19:32, Mark Brown <broonie@kernel.org> wrote:

> > The obvious questions here based on the name are why we're doing
> > something specific to platform devices and why this isn't something
> > we're abstracting in the driver core (or at least generic firmware code)
> > - we're going to have the same thing with ACPI.

> I don't know how useful this is going to be in systems with ACPI. My
> experience is limited to 32bit ARM, where the kernel has to manage
> every regulator, clock, gpio, etc so the dependency tree is so big. Is
> deferred probing a problem with ACPI as well?

Yes, x86 based embedded systems use ACPI (and we really ought to be
trying to help systems using board files too for that matter).

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

Re: [PATCH 15/21] backlight: Probe backlight devices on demand

[Tomeu Vizoso]

On 26 May 2015 at 10:39, Lee Jones <lee.jones@linaro.org> wrote:
> On Tue, 26 May 2015, Sascha Hauer wrote:
>> On Tue, May 26, 2015 at 08:18:50AM +0100, Lee Jones wrote:
>> > On Mon, 25 May 2015, Tomeu Vizoso wrote:
>> >
>> > > When looking up a backlight device through its DT node, ensure that the
>> > > corresponding device has been registered.
>> > >
>> > > Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>> > > ---
>> > >  drivers/video/backlight/backlight.c | 3 +++
>> > >  1 file changed, 3 insertions(+)
>> >
>> > Looks reasonable.
>> >
>> > Until anyone screams at me, applied thanks.
>>
>> The compiler will scream at you when it realizes that
>> of_platform_device_ensure() doesn't exist in your kernel...
>
> Yup, indeed it did.
>
> I assumed this was *only* enabling subsystems and that the
> framework/API was already accepted.
>
> So the advice I'd give to Tomeu when sending full enablement
> patch-sets i.e. ones which provide the framework/API *and* enable
> subsystems in the same set, is to send the entire set to everyone, so
> we can see what the aim of the set is and how to deal with it.

Yeah, but get_maintainer.pl outputs 33 maintainer addresses, plus 1
reviewer plus 14 mailing lists, so to avoid rejects because of too
many recipients I went with the advice in [0] and sent each patch to
their maintainers and list(s) and the cover letter to all lists. Also
sent the whole series to lakml to make sure that it's at least indexed
there.

Any advice on what to do with series that span so many subsystems?

Thanks,

Tomeu

[0] http://lwn.net/Articles/585782/

> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 15/21] backlight: Probe backlight devices on demand

[Lee Jones]

On Tue, 26 May 2015, Tomeu Vizoso wrote:

> On 26 May 2015 at 10:39, Lee Jones <lee.jones@linaro.org> wrote:
> > On Tue, 26 May 2015, Sascha Hauer wrote:
> >> On Tue, May 26, 2015 at 08:18:50AM +0100, Lee Jones wrote:
> >> > On Mon, 25 May 2015, Tomeu Vizoso wrote:
> >> >
> >> > > When looking up a backlight device through its DT node, ensure that the
> >> > > corresponding device has been registered.
> >> > >
> >> > > Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> >> > > ---
> >> > >  drivers/video/backlight/backlight.c | 3 +++
> >> > >  1 file changed, 3 insertions(+)
> >> >
> >> > Looks reasonable.
> >> >
> >> > Until anyone screams at me, applied thanks.
> >>
> >> The compiler will scream at you when it realizes that
> >> of_platform_device_ensure() doesn't exist in your kernel...
> >
> > Yup, indeed it did.
> >
> > I assumed this was *only* enabling subsystems and that the
> > framework/API was already accepted.
> >
> > So the advice I'd give to Tomeu when sending full enablement
> > patch-sets i.e. ones which provide the framework/API *and* enable
> > subsystems in the same set, is to send the entire set to everyone, so
> > we can see what the aim of the set is and how to deal with it.
> 
> Yeah, but get_maintainer.pl outputs 33 maintainer addresses, plus 1
> reviewer plus 14 mailing lists, so to avoid rejects because of too
> many recipients I went with the advice in [0] and sent each patch to
> their maintainers and list(s) and the cover letter to all lists. Also
> sent the whole series to lakml to make sure that it's at least indexed
> there.

Mails aren't usually rejected because they have too many recipients,
rather they require approval.  This isn't a blocker, especially for
patch-sets like this.

> Any advice on what to do with series that span so many subsystems?

I would either ensure everyone is informed, split into two choices;
either CC everyone on every patch, or at least everyone on the
cover-letter and the core API changes.  The other method is to have
the API changes accepted first, then once accepted send out the
subsystem changes [FWIW: this is the method I (wrongly) assumed you
used].

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 16/21] usb: phy: Probe phy devices on demand

[Felipe Balbi]

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

On Mon, May 25, 2015 at 04:53:20PM +0200, Tomeu Vizoso wrote:
> When looking up a phy through its DT node, ensure that the corresponding
> device has been registered.
> 
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>

Acked-by: Felipe Balbi <balbi@ti.com>

> ---
>  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 d1cd6b5..7084f21 100644
> --- a/drivers/usb/phy/phy.c
> +++ b/drivers/usb/phy/phy.c
> @@ -13,6 +13,7 @@
>  #include <linux/err.h>
>  #include <linux/device.h>
>  #include <linux/module.h>
> +#include <linux/of_platform.h>
>  #include <linux/slab.h>
>  #include <linux/of.h>
>  
> @@ -192,6 +193,8 @@ struct usb_phy *devm_usb_get_phy_by_phandle(struct device *dev,
>  		goto err0;
>  	}
>  
> +	of_platform_device_ensure(node);
> +
>  	spin_lock_irqsave(&phy_lock, flags);
>  
>  	phy = __of_usb_find_phy(node);
> -- 
> 2.4.1
> 

-- 
balbi

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

Re: [PATCH 10/21] regulator: core: Probe regulators on demand

[Tomeu Vizoso]

On 26 May 2015 at 11:36, Mark Brown <broonie@kernel.org> wrote:
> On Tue, May 26, 2015 at 08:17:23AM +0200, Tomeu Vizoso wrote:
>> On 25 May 2015 at 19:32, Mark Brown <broonie@kernel.org> wrote:
>
>> > The obvious questions here based on the name are why we're doing
>> > something specific to platform devices and why this isn't something
>> > we're abstracting in the driver core (or at least generic firmware code)
>> > - we're going to have the same thing with ACPI.
>
>> I don't know how useful this is going to be in systems with ACPI. My
>> experience is limited to 32bit ARM, where the kernel has to manage
>> every regulator, clock, gpio, etc so the dependency tree is so big. Is
>> deferred probing a problem with ACPI as well?
>
> Yes, x86 based embedded systems use ACPI (and we really ought to be
> trying to help systems using board files too for that matter).

Yes, I see how registering devices on-demand could be implemented for
all those, but what I don't see is how they would benefit from it.

I can see an hypothetical maintenance benefit in sharing as much code
as possible between these different scenarios, but in this case,
because this feature is so closely tied to machine description I think
complexity would be actually bigger.

The problem I'm trying to address only manifests on systems with
dozens of devices that are currently registered in an arbitrary order
(as they are in the DT).

On machines that have ACPI, most of those devices aren't exposed to
the kernel and few or no deferred probes happen (though I have only
tested on qemu and Minnowboard MAX, both with no deferred probes).

On machines with board files, devices are registered in a
predetermined order, presumably without any deferred probes.

My understanding is that the problem I'm addressing is specific of
machines in which the kernel is in charge of pretty much everything
and that the information about what devices are present is given in an
arbitrary order.

Deferred probe gives us reliably a working system at the end for these
machines, but in the meantime some devices will have been deferred
several times and when they get to probe the user will have noticed
the delay.

Regards,

Tomeu

Re: [PATCH 10/21] regulator: core: Probe regulators on demand

[Mark Brown]

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

On Tue, May 26, 2015 at 05:08:38PM +0200, Tomeu Vizoso wrote:
> On 26 May 2015 at 11:36, Mark Brown <broonie@kernel.org> wrote:

> > Yes, x86 based embedded systems use ACPI (and we really ought to be
> > trying to help systems using board files too for that matter).

> Yes, I see how registering devices on-demand could be implemented for
> all those, but what I don't see is how they would benefit from it.

You'd need to be clearer about what problem you're trying to solve
there, which is something you left us guessing at!

> I can see an hypothetical maintenance benefit in sharing as much code
> as possible between these different scenarios, but in this case,
> because this feature is so closely tied to machine description I think
> complexity would be actually bigger.

We've now got abstractions for common firmware operations (look at the
fwnode stuff) and this isn't exactly deep introspection here.

If you're trying to solve the probe order problem you can probably get a
long way by just doing something that boils down to "try to instantiate
everything referenced from this node" which could probably even be
kicked from the driver core prior to probe and cover most cases.  Or put
this into the node lookup interface so we try to instantiate everything
we reference.

> On machines that have ACPI, most of those devices aren't exposed to
> the kernel and few or no deferred probes happen (though I have only
> tested on qemu and Minnowboard MAX, both with no deferred probes).

On the machines that you happen to have looked at; I would rather expect
that x86 based phones will be in a similar situation once they move to
ACPI which they should be doing this year if they didn't already, and
the embedded systems will doubtless run into this once they have any
meaningful hardware on them (the base Minnoboard isn't really
interesting here, it's once you build a system on top of that).

> On machines with board files, devices are registered in a
> predetermined order, presumably without any deferred probes.

No, not in the least.  Quite aside from anything else as soon as you
allow things to be built as modules userspace is free to load things in
whatever order amuses it.  Think about what's going on here - it's not
just registration of devices, it's also about the order in which
subsystems and drivers register themselves.

> My understanding is that the problem I'm addressing is specific of
> machines in which the kernel is in charge of pretty much everything
> and that the information about what devices are present is given in an
> arbitrary order.

I don't think you've fully understood the problem space here.

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

Re: [PATCH 10/21] regulator: core: Probe regulators on demand

[Tomeu Vizoso]

On 26 May 2015 at 18:54, Mark Brown <broonie@kernel.org> wrote:
> On Tue, May 26, 2015 at 05:08:38PM +0200, Tomeu Vizoso wrote:
>> On 26 May 2015 at 11:36, Mark Brown <broonie@kernel.org> wrote:
>
>> > Yes, x86 based embedded systems use ACPI (and we really ought to be
>> > trying to help systems using board files too for that matter).
>
>> Yes, I see how registering devices on-demand could be implemented for
>> all those, but what I don't see is how they would benefit from it.
>
> You'd need to be clearer about what problem you're trying to solve
> there, which is something you left us guessing at!
>
>> I can see an hypothetical maintenance benefit in sharing as much code
>> as possible between these different scenarios, but in this case,
>> because this feature is so closely tied to machine description I think
>> complexity would be actually bigger.
>
> We've now got abstractions for common firmware operations (look at the
> fwnode stuff) and this isn't exactly deep introspection here.
>
> If you're trying to solve the probe order problem you can probably get a
> long way by just doing something that boils down to "try to instantiate
> everything referenced from this node" which could probably even be
> kicked from the driver core prior to probe and cover most cases.  Or put
> this into the node lookup interface so we try to instantiate everything
> we reference.
>
>> On machines that have ACPI, most of those devices aren't exposed to
>> the kernel and few or no deferred probes happen (though I have only
>> tested on qemu and Minnowboard MAX, both with no deferred probes).
>
> On the machines that you happen to have looked at; I would rather expect
> that x86 based phones will be in a similar situation once they move to
> ACPI which they should be doing this year if they didn't already, and
> the embedded systems will doubtless run into this once they have any
> meaningful hardware on them (the base Minnoboard isn't really
> interesting here, it's once you build a system on top of that).
>
>> On machines with board files, devices are registered in a
>> predetermined order, presumably without any deferred probes.
>
> No, not in the least.  Quite aside from anything else as soon as you
> allow things to be built as modules userspace is free to load things in
> whatever order amuses it.  Think about what's going on here - it's not
> just registration of devices, it's also about the order in which
> subsystems and drivers register themselves.
>
>> My understanding is that the problem I'm addressing is specific of
>> machines in which the kernel is in charge of pretty much everything
>> and that the information about what devices are present is given in an
>> arbitrary order.
>
> I don't think you've fully understood the problem space here.

Fair enough, what's your understanding of it?

Thanks,

Tomeu

Re: [PATCH 06/21] of/platform: Add of_platform_device_ensure()

[Dmitry Torokhov]

Hi Tomeu,

On Mon, May 25, 2015 at 04:53:10PM +0200, Tomeu Vizoso wrote:
> This function ensures that the device that encloses the passed device
> node is registered, and thus probed if the corresponding driver has been
> registered already.

Even if the driver has already been registered this code will not
guarantee that the device has been probed if driver enabled asynchronous
probing (async probing should appear in 4.2 and the end goal is to have
async probing enabled by default for most drivers).

Also, why are we concentrating on platform drivers only? What about
other devices, for example a gpio expander behind i2c bus?

I am also concerned about adding OF-specific hooks into every subsystem
out there. Can we make process of instantiating OF devices iterative
instead and add them only if their parent has been already probed and
also devices corresponding to all phandles they reference have also been
probed? We could repeat scanning of the device tree every time we bind
a driver and bulk-add leftovers at late_initcall time. This would
mean that we could keep all logic in OF code (and maybe ACPI will add
their own implementation) and keep other subsystems unaware.

Thanks.

-- 
Dmitry

Re: [PATCH 02/21] ARM: tegra: Add gpio-ranges property

[Stephen Warren]

On 05/25/2015 08:53 AM, Tomeu Vizoso wrote:
> Specify how the GPIOs map to the pins in T124, so the dependency is
> explicit.
>
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> ---
>   arch/arm/boot/dts/tegra124.dtsi | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/boot/dts/tegra124.dtsi b/arch/arm/boot/dts/tegra124.dtsi
> index 13cc7ca..5d1d35f 100644
> --- a/arch/arm/boot/dts/tegra124.dtsi
> +++ b/arch/arm/boot/dts/tegra124.dtsi
> @@ -254,6 +254,7 @@
>   		gpio-controller;
>   		#interrupt-cells = <2>;
>   		interrupt-controller;
> +		gpio-ranges = <&pinmux 0 0 250>;

We should be consistent between SoCs. Why not make the same change for 
all Tegra SoCs?

I think this change will cause the GPIO subsystem to call into the 
pinctrl subsystem and create/add/register a new GPIO<->pinctrl range 
structure. The pinctrl driver already does this, so I think we'll end up 
with two duplicate entries in the pinctrl device's gpio_ranges list. 
This probably won't cause a problem, but I wanted to make sure you'd 
thought about it to make sure.

Right now, I think we get lucky and pinctrl ends up probing first (or at 
least very early) anyway. Somewhat related to this series, I wonder if 
we shouldn't add pinctrl client properties to every node in the Tegra DT 
that describes a controller that makes use of external pins that are 
affected by the pinmux. Such a change would guarantee this desired 
probing order. In order to preserve the "program the entire pinmux at 
once" semantics, these new pinctrl client properties would all need to 
reference empty states, yet would still need to exist to represent the 
dependency.

Re: [PATCH 10/21] regulator: core: Probe regulators on demand

[Mark Brown]

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

On Tue, May 26, 2015 at 07:53:33PM +0200, Tomeu Vizoso wrote:
> On 26 May 2015 at 18:54, Mark Brown <broonie@kernel.org> wrote:

> >> My understanding is that the problem I'm addressing is specific of
> >> machines in which the kernel is in charge of pretty much everything
> >> and that the information about what devices are present is given in an
> >> arbitrary order.

> > I don't think you've fully understood the problem space here.
> 
> Fair enough, what's your understanding of it?

Basically what I said in the e-mail you quoted fully in your reply,
especially this section:

> > No, not in the least.  Quite aside from anything else as soon as you
> > allow things to be built as modules userspace is free to load things in
> > whatever order amuses it.  Think about what's going on here - it's not
> > just registration of devices, it's also about the order in which
> > subsystems and drivers register themselves.

Probe ordering is fairly weakly related to the interface used to
register devices, you're going to get dependencies more often the more
detail is exposed to the kernel but it's not specific to that.  It's
definitely not something that we have a solution to for board files,
they rely on deferred probing just as much as device tree does.

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

Re: [PATCH 06/21] of/platform: Add of_platform_device_ensure()

[Tomeu Vizoso]

On 26 May 2015 at 20:56, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> Hi Tomeu,
>
> On Mon, May 25, 2015 at 04:53:10PM +0200, Tomeu Vizoso wrote:
>> This function ensures that the device that encloses the passed device
>> node is registered, and thus probed if the corresponding driver has been
>> registered already.
>
> Even if the driver has already been registered this code will not
> guarantee that the device has been probed if driver enabled asynchronous
> probing (async probing should appear in 4.2 and the end goal is to have
> async probing enabled by default for most drivers).

Ok, I will look at that. Do you know if there's any board plugged into
kernelci.org in which most drivers have async probing enabled already
in linux-next?

> Also, why are we concentrating on platform drivers only? What about
> other devices, for example a gpio expander behind i2c bus?

The current code will register the i2c master device when a device
requests one of those gpios, and the gpio master device will be
registered when the i2c master is probed.

> I am also concerned about adding OF-specific hooks into every subsystem
> out there.

Well, those hooks are in the OF-specific code that we already added in
every subsystem out there. But yeah, it would be great if we didn't
had to add them.

> Can we make process of instantiating OF devices iterative
> instead and add them only if their parent has been already probed and

This is what happens today when the mach code calls
of_platform_populate on the root node, right? Or are you thinking of
something different?

> also devices corresponding to all phandles they reference have also been
> probed? We could repeat scanning of the device tree every time we bind
> a driver and bulk-add leftovers at late_initcall time.

Yeah, this possibility was discussed in the thread linked from the
cover letter. The main problem is that the knowledge required to infer
from the phandles what devices are dependencies is in the DT bindings
documentation.

That knowledge is already codified in both each driver's probe
function (for example when regulator_get_optional(dev, "phy") is
called) and the functions that resolve dependencies when a device
requests them (of_get_regulator(dev, "phy") in this example).

That's why I thought of this approach, the main advantage of which is
that it makes use of all that existing code without having to modify
each driver and subsystem core.

There's several ways to address this problem but require substantial
refactoring. I just wanted to propose this alternative because it
hadn't been discussed before and because I think it brings an
interesting cost/benefit ratio.

> This would
> mean that we could keep all logic in OF code (and maybe ACPI will add
> their own implementation) and keep other subsystems unaware.

Yeah, that would be cool to have, but TBH I don't know if it's worth it.

Regards,

Tomeu

> Thanks.
>
> --
> Dmitry
> --
> 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 02/21] ARM: tegra: Add gpio-ranges property

[Tomeu Vizoso]

On 26 May 2015 at 21:41, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 05/25/2015 08:53 AM, Tomeu Vizoso wrote:
>>
>> Specify how the GPIOs map to the pins in T124, so the dependency is
>> explicit.
>>
>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>> ---
>>   arch/arm/boot/dts/tegra124.dtsi | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/boot/dts/tegra124.dtsi
>> b/arch/arm/boot/dts/tegra124.dtsi
>> index 13cc7ca..5d1d35f 100644
>> --- a/arch/arm/boot/dts/tegra124.dtsi
>> +++ b/arch/arm/boot/dts/tegra124.dtsi
>> @@ -254,6 +254,7 @@
>>                 gpio-controller;
>>                 #interrupt-cells = <2>;
>>                 interrupt-controller;
>> +               gpio-ranges = <&pinmux 0 0 250>;
>
>
> We should be consistent between SoCs. Why not make the same change for all
> Tegra SoCs?
>
> I think this change will cause the GPIO subsystem to call into the pinctrl
> subsystem and create/add/register a new GPIO<->pinctrl range structure. The
> pinctrl driver already does this, so I think we'll end up with two duplicate
> entries in the pinctrl device's gpio_ranges list. This probably won't cause
> a problem, but I wanted to make sure you'd thought about it to make sure.

Actually, I have checked and see that gpio-tegra.c registers 256
gpios, but pinctrl-tegra124.c adds a range of only 251. I don't really
remember where I got the 250 value from, sorry :(

I don't see how that would cause any concrete problems, but maybe we
should have a single authoritative source (not sure we can do so
without breaking DT ABI though).

> Right now, I think we get lucky and pinctrl ends up probing first (or at
> least very early) anyway. Somewhat related to this series, I wonder if we
> shouldn't add pinctrl client properties to every node in the Tegra DT that
> describes a controller that makes use of external pins that are affected by
> the pinmux. Such a change would guarantee this desired probing order. In
> order to preserve the "program the entire pinmux at once" semantics, these
> new pinctrl client properties would all need to reference empty states, yet
> would still need to exist to represent the dependency.

I think so, but aren't almost all those pins used as gpios? If so,
then such a controller's driver will request the gpio it wants which
will cause the gpio driver to be registered (and hopefully probed) if
needed, which in turn will check that the corresponding pinctrl device
has been registered. Or am I missing something?

Regards,

Tomeu

> --
> 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 02/21] ARM: tegra: Add gpio-ranges property

[Stephen Warren]

On 05/27/2015 08:18 AM, Tomeu Vizoso wrote:
> On 26 May 2015 at 21:41, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 05/25/2015 08:53 AM, Tomeu Vizoso wrote:
>>>
>>> Specify how the GPIOs map to the pins in T124, so the dependency is
>>> explicit.
>>>
>>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>>> ---
>>>    arch/arm/boot/dts/tegra124.dtsi | 1 +
>>>    1 file changed, 1 insertion(+)
>>>
>>> diff --git a/arch/arm/boot/dts/tegra124.dtsi
>>> b/arch/arm/boot/dts/tegra124.dtsi
>>> index 13cc7ca..5d1d35f 100644
>>> --- a/arch/arm/boot/dts/tegra124.dtsi
>>> +++ b/arch/arm/boot/dts/tegra124.dtsi
>>> @@ -254,6 +254,7 @@
>>>                  gpio-controller;
>>>                  #interrupt-cells = <2>;
>>>                  interrupt-controller;
>>> +               gpio-ranges = <&pinmux 0 0 250>;
>>
>>
>> We should be consistent between SoCs. Why not make the same change for all
>> Tegra SoCs?
>>
>> I think this change will cause the GPIO subsystem to call into the pinctrl
>> subsystem and create/add/register a new GPIO<->pinctrl range structure. The
>> pinctrl driver already does this, so I think we'll end up with two duplicate
>> entries in the pinctrl device's gpio_ranges list. This probably won't cause
>> a problem, but I wanted to make sure you'd thought about it to make sure.
>
> Actually, I have checked and see that gpio-tegra.c registers 256
> gpios, but pinctrl-tegra124.c adds a range of only 251. I don't really
> remember where I got the 250 value from, sorry :(
>
> I don't see how that would cause any concrete problems, but maybe we
> should have a single authoritative source (not sure we can do so
> without breaking DT ABI though).
>
>> Right now, I think we get lucky and pinctrl ends up probing first (or at
>> least very early) anyway. Somewhat related to this series, I wonder if we
>> shouldn't add pinctrl client properties to every node in the Tegra DT that
>> describes a controller that makes use of external pins that are affected by
>> the pinmux. Such a change would guarantee this desired probing order. In
>> order to preserve the "program the entire pinmux at once" semantics, these
>> new pinctrl client properties would all need to reference empty states, yet
>> would still need to exist to represent the dependency.
>
> I think so, but aren't almost all those pins used as gpios? If so,
> then such a controller's driver will request the gpio it wants which
> will cause the gpio driver to be registered (and hopefully probed) if
> needed, which in turn will check that the corresponding pinctrl device
> has been registered. Or am I missing something?

As you say this probably works out fine for pins that are used as GPIOs. 
I was thinking more about SFIOs. Take an I2C controller, which doesn't 
use any GPIOs itself. The pinctrl device should be probed before the I2C 
device, so that the I2C driver can initiate transactions on the I2C bus 
during its probe if it wanted to (or at least, clients could initiate 
transactions at any completely arbitrary time as soon as probe was 
complete).

Re: [PATCH 00/21] On-demand device registration

[Rob Herring]

On Mon, May 25, 2015 at 9:53 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.
>
> 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 registering devices as they are referenced by other
> devices.

I like the concept and novel approach.

> This basically reuses the information that is already implicit in the probe()
> implementations, saving us from refactoring existing drivers or adding
> information to DTBs.
>
> Something I'm not completely happy with is that I have had to move the call to
> of_platform_populate after all platform drivers have been registered.
> Otherwise I don't see how I could register drivers on demand as we don't have
> yet each driver's compatible strings.

Yeah, this is the opposite of what we'd really like. Ideally, we would
have a solution that works for modules too. However, we're no worse
off. We pretty much build-in dependencies to avoid module ordering
problems.

Perhaps we need to make the probing on-demand rather than simply on
device<->driver match occurring.

> For machs that don't move of_platform_populate() to a later point, these
> patches shouldn't cause any problems but it's not guaranteed that we'll avoid
> all the deferred probes as some drivers may not be registered yet.

Ideally, of_platform_populate is not explicitly called by each
platform. So I think we need to make this work for the default case.

> I have tested this on boards with Tegra, iMX.6 and Exynos SoCs, and these
> patches were enough to eliminate all the deferred probes.
>
> With this series I get the kernel to output to the panel in 0.5s, instead of 2.8s.

That's certainly compelling.

Rob

>
> Regards,
>
> Tomeu
>
> [0] http://lists.freedesktop.org/archives/dri-devel/2014-August/066527.html
>
> [1] https://lkml.org/lkml/2014/5/12/452
>
> Tomeu Vizoso (21):
>   regulator: core: Reduce critical area in _regulator_get
>   ARM: tegra: Add gpio-ranges property
>   ARM: tegra: Register drivers before devices
>   ARM: EXYNOS: Register drivers before devices
>   ARM i.MX6q: Register drivers before devices
>   of/platform: Add of_platform_device_ensure()
>   of/platform: Ensure device registration on lookup
>   gpio: Probe GPIO drivers on demand
>   gpio: Probe pinctrl devices on demand
>   regulator: core: Probe regulators on demand
>   drm: Probe panels on demand
>   drm/tegra: Probe dpaux devices on demand
>   i2c: core: Probe i2c master 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
>
>  arch/arm/boot/dts/tegra124.dtsi     |  1 +
>  arch/arm/mach-exynos/exynos.c       |  4 +--
>  arch/arm/mach-imx/mach-imx6q.c      | 12 ++++-----
>  arch/arm/mach-tegra/tegra.c         | 21 ++++++---------
>  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              |  3 +++
>  drivers/of/platform.c               | 53 +++++++++++++++++++++++++++++++++++++
>  drivers/phy/phy-core.c              |  3 +++
>  drivers/pinctrl/devicetree.c        |  2 ++
>  drivers/power/power_supply_core.c   |  3 +++
>  drivers/pwm/core.c                  |  3 +++
>  drivers/regulator/core.c            | 45 +++++++++++++++----------------
>  drivers/usb/phy/phy.c               |  3 +++
>  drivers/video/backlight/backlight.c |  3 +++
>  include/linux/of_platform.h         |  2 ++
>  19 files changed, 130 insertions(+), 45 deletions(-)
>
> --
> 2.4.1
>

Re: [PATCH 17/21] clk: Probe clk providers on demand

[Michael Turquette]

Quoting Tomeu Vizoso (2015-05-25 07:53:21)
> When looking up a clk through its DT node, ensure that the corresponding
> device has been registered.
> 
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>

Ack.

Regards,
Mike

> ---
>  drivers/clk/clk.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 5b0f418..f9e8a8d 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -18,6 +18,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>
> @@ -2985,6 +2986,8 @@ struct clk *__of_clk_get_from_provider(struct of_phandle_args *clkspec,
>         if (!clkspec)
>                 return ERR_PTR(-EINVAL);
>  
> +       of_platform_device_ensure(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.1
> 

Re: [PATCH 02/21] ARM: tegra: Add gpio-ranges property

[Tomeu Vizoso]

On 27 May 2015 at 16:49, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 05/27/2015 08:18 AM, Tomeu Vizoso wrote:
>>
>> On 26 May 2015 at 21:41, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>
>>> On 05/25/2015 08:53 AM, Tomeu Vizoso wrote:
>>>>
>>>>
>>>> Specify how the GPIOs map to the pins in T124, so the dependency is
>>>> explicit.
>>>>
>>>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>>>> ---
>>>>    arch/arm/boot/dts/tegra124.dtsi | 1 +
>>>>    1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/tegra124.dtsi
>>>> b/arch/arm/boot/dts/tegra124.dtsi
>>>> index 13cc7ca..5d1d35f 100644
>>>> --- a/arch/arm/boot/dts/tegra124.dtsi
>>>> +++ b/arch/arm/boot/dts/tegra124.dtsi
>>>> @@ -254,6 +254,7 @@
>>>>                  gpio-controller;
>>>>                  #interrupt-cells = <2>;
>>>>                  interrupt-controller;
>>>> +               gpio-ranges = <&pinmux 0 0 250>;
>>>
>>>
>>>
>>> We should be consistent between SoCs. Why not make the same change for
>>> all
>>> Tegra SoCs?
>>>
>>> I think this change will cause the GPIO subsystem to call into the
>>> pinctrl
>>> subsystem and create/add/register a new GPIO<->pinctrl range structure.
>>> The
>>> pinctrl driver already does this, so I think we'll end up with two
>>> duplicate
>>> entries in the pinctrl device's gpio_ranges list. This probably won't
>>> cause
>>> a problem, but I wanted to make sure you'd thought about it to make sure.
>>
>>
>> Actually, I have checked and see that gpio-tegra.c registers 256
>> gpios, but pinctrl-tegra124.c adds a range of only 251. I don't really
>> remember where I got the 250 value from, sorry :(
>>
>> I don't see how that would cause any concrete problems, but maybe we
>> should have a single authoritative source (not sure we can do so
>> without breaking DT ABI though).
>>
>>> Right now, I think we get lucky and pinctrl ends up probing first (or at
>>> least very early) anyway. Somewhat related to this series, I wonder if we
>>> shouldn't add pinctrl client properties to every node in the Tegra DT
>>> that
>>> describes a controller that makes use of external pins that are affected
>>> by
>>> the pinmux. Such a change would guarantee this desired probing order. In
>>> order to preserve the "program the entire pinmux at once" semantics,
>>> these
>>> new pinctrl client properties would all need to reference empty states,
>>> yet
>>> would still need to exist to represent the dependency.
>>
>>
>> I think so, but aren't almost all those pins used as gpios? If so,
>> then such a controller's driver will request the gpio it wants which
>> will cause the gpio driver to be registered (and hopefully probed) if
>> needed, which in turn will check that the corresponding pinctrl device
>> has been registered. Or am I missing something?
>
>
> As you say this probably works out fine for pins that are used as GPIOs. I
> was thinking more about SFIOs. Take an I2C controller, which doesn't use any
> GPIOs itself. The pinctrl device should be probed before the I2C device, so
> that the I2C driver can initiate transactions on the I2C bus during its
> probe if it wanted to (or at least, clients could initiate transactions at
> any completely arbitrary time as soon as probe was complete).

What is using the SFIO in this case, the I2C master or the I2C client?

In any case, is the problem you are referring to that ICs may rely on
a specific pinmux configuration but that's not currently reflected on
the DT because pinmux configuration happened so early that things just
worked?

Thanks,

Tomeu

Re: [PATCH 02/21] ARM: tegra: Add gpio-ranges property

[Stephen Warren]

On 05/28/2015 02:26 AM, Tomeu Vizoso wrote:
> On 27 May 2015 at 16:49, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 05/27/2015 08:18 AM, Tomeu Vizoso wrote:
>>>
>>> On 26 May 2015 at 21:41, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>>
>>>> On 05/25/2015 08:53 AM, Tomeu Vizoso wrote:
>>>>>
>>>>>
>>>>> Specify how the GPIOs map to the pins in T124, so the dependency is
>>>>> explicit.
>>>>>
>>>>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>>>>> ---
>>>>>     arch/arm/boot/dts/tegra124.dtsi | 1 +
>>>>>     1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/tegra124.dtsi
>>>>> b/arch/arm/boot/dts/tegra124.dtsi
>>>>> index 13cc7ca..5d1d35f 100644
>>>>> --- a/arch/arm/boot/dts/tegra124.dtsi
>>>>> +++ b/arch/arm/boot/dts/tegra124.dtsi
>>>>> @@ -254,6 +254,7 @@
>>>>>                   gpio-controller;
>>>>>                   #interrupt-cells = <2>;
>>>>>                   interrupt-controller;
>>>>> +               gpio-ranges = <&pinmux 0 0 250>;
>>>>
>>>>
>>>>
>>>> We should be consistent between SoCs. Why not make the same change for
>>>> all
>>>> Tegra SoCs?
>>>>
>>>> I think this change will cause the GPIO subsystem to call into the
>>>> pinctrl
>>>> subsystem and create/add/register a new GPIO<->pinctrl range structure.
>>>> The
>>>> pinctrl driver already does this, so I think we'll end up with two
>>>> duplicate
>>>> entries in the pinctrl device's gpio_ranges list. This probably won't
>>>> cause
>>>> a problem, but I wanted to make sure you'd thought about it to make sure.
>>>
>>>
>>> Actually, I have checked and see that gpio-tegra.c registers 256
>>> gpios, but pinctrl-tegra124.c adds a range of only 251. I don't really
>>> remember where I got the 250 value from, sorry :(
>>>
>>> I don't see how that would cause any concrete problems, but maybe we
>>> should have a single authoritative source (not sure we can do so
>>> without breaking DT ABI though).
>>>
>>>> Right now, I think we get lucky and pinctrl ends up probing first (or at
>>>> least very early) anyway. Somewhat related to this series, I wonder if we
>>>> shouldn't add pinctrl client properties to every node in the Tegra DT
>>>> that
>>>> describes a controller that makes use of external pins that are affected
>>>> by
>>>> the pinmux. Such a change would guarantee this desired probing order. In
>>>> order to preserve the "program the entire pinmux at once" semantics,
>>>> these
>>>> new pinctrl client properties would all need to reference empty states,
>>>> yet
>>>> would still need to exist to represent the dependency.
>>>
>>>
>>> I think so, but aren't almost all those pins used as gpios? If so,
>>> then such a controller's driver will request the gpio it wants which
>>> will cause the gpio driver to be registered (and hopefully probed) if
>>> needed, which in turn will check that the corresponding pinctrl device
>>> has been registered. Or am I missing something?
>>
>>
>> As you say this probably works out fine for pins that are used as GPIOs. I
>> was thinking more about SFIOs. Take an I2C controller, which doesn't use any
>> GPIOs itself. The pinctrl device should be probed before the I2C device, so
>> that the I2C driver can initiate transactions on the I2C bus during its
>> probe if it wanted to (or at least, clients could initiate transactions at
>> any completely arbitrary time as soon as probe was complete).
>
> What is using the SFIO in this case, the I2C master or the I2C client?

The I2C controller a/k/a the I2C master.

> In any case, is the problem you are referring to that ICs may rely on

s/ICs/drivers/ I think.

> a specific pinmux configuration but that's not currently reflected on
> the DT because pinmux configuration happened so early that things just
> worked?

Yes; the dependency of some nodes on pinctrl isn't explicitly called out 
in the DT. It's probably not a good idea to have such implicit 
dependencies, although I suppose for something so central as pinmux, 
maybe it's not terrible, since almost everything depends on it and it's 
pretty obvious.

Re: [PATCH 00/21] On-demand device registration

[Linus Walleij]

On Mon, May 25, 2015 at 4:53 PM, Tomeu Vizoso
<tomeu.vizoso@collabora.com> wrote:

> have looked into ordered probing as a
> better way of solving this than moving nodes around in the DT or playing with
> initcall levels.
>
> 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 registering devices as they are referenced by other
> devices.

This is pretty cool, but a too local solution to a global problem.

Deferred probe and initcall reordering, silly as they may seem,
does not require you to use device tree.

The real solution, which I think I pointed out already when we
added deferred probe, is to put dependency graphs in the drivers
and have the kernel device driver core percolate dependecies by
walking the graph on probing driver, removing driver (usually the
inverse use case), [runtime] suspend and [runtime] resumeing
a driver. Possibly the dependencies will even be different
depending on use case.

This is what systemd is doing in userspace for starting services:
ask for your dependencies and wait for them if they are not
there. So drivers ask for resources and wait for them. It also
needs to be abstract, so for example we need to be able to
hang on regulator_get() until the driver is up and providing that
regulator, and as long as everything is in slowpath it should
be OK. (And vice versa mutatis mutandis for clk, gpio, pin
control, interrupts (!) and DMA channels for example.)


So if this should be solved it should be solved in an abstract way
in the device driver core available for all, then have calls calling
out to DT, ACPI, possibly even PCI or USB (as these
enumerate devices themselves) to obtain a certain
dependency.

Yours,
Linus Walleij

Re: [PATCH 00/21] On-demand device registration

[Tomeu Vizoso]

On 2 June 2015 at 10:48, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Mon, May 25, 2015 at 4:53 PM, Tomeu Vizoso
> <tomeu.vizoso@collabora.com> wrote:
>
>> have looked into ordered probing as a
>> better way of solving this than moving nodes around in the DT or playing with
>> initcall levels.
>>
>> 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 registering devices as they are referenced by other
>> devices.
>
> This is pretty cool, but a too local solution to a global problem.
>
> Deferred probe and initcall reordering, silly as they may seem,
> does not require you to use device tree.
>
> The real solution, which I think I pointed out already when we
> added deferred probe, is to put dependency graphs in the drivers

By this you mean something like what Thierry suggested here?

http://article.gmane.org/gmane.linux.kernel/1774623

> and have the kernel device driver core percolate dependecies by
> walking the graph on probing driver, removing driver (usually the
> inverse use case), [runtime] suspend and [runtime] resumeing
> a driver. Possibly the dependencies will even be different
> depending on use case.
>
> This is what systemd is doing in userspace for starting services:
> ask for your dependencies and wait for them if they are not
> there. So drivers ask for resources and wait for them. It also
> needs to be abstract, so for example we need to be able to
> hang on regulator_get() until the driver is up and providing that
> regulator, and as long as everything is in slowpath it should
> be OK. (And vice versa mutatis mutandis for clk, gpio, pin
> control, interrupts (!) and DMA channels for example.)

I understood above that you propose probing devices in order, but now
you mention that resource getters would block until the dependency is
fulfilled which confuses me because if we are probing in order then
all dependencies would be fulfilled before the device in question gets
probed.

> So if this should be solved it should be solved in an abstract way
> in the device driver core available for all, then have calls calling
> out to DT, ACPI, possibly even PCI or USB (as these
> enumerate devices themselves) to obtain a certain
> dependency.

Yeah, I was planning looking into this now that I got it working with
async probing.

Thanks,

Tomeu

> Yours,
> Linus Walleij
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/21] ARM: tegra: Add gpio-ranges property

[Linus Walleij]

On Tue, May 26, 2015 at 9:41 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 05/25/2015 08:53 AM, Tomeu Vizoso wrote:
>>
>> Specify how the GPIOs map to the pins in T124, so the dependency is
>> explicit.
>>
>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>> ---
>>   arch/arm/boot/dts/tegra124.dtsi | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/boot/dts/tegra124.dtsi
>> b/arch/arm/boot/dts/tegra124.dtsi
>> index 13cc7ca..5d1d35f 100644
>> --- a/arch/arm/boot/dts/tegra124.dtsi
>> +++ b/arch/arm/boot/dts/tegra124.dtsi
>> @@ -254,6 +254,7 @@
>>                 gpio-controller;
>>                 #interrupt-cells = <2>;
>>                 interrupt-controller;
>> +               gpio-ranges = <&pinmux 0 0 250>;
>
>
> We should be consistent between SoCs. Why not make the same change for all
> Tegra SoCs?

Agreed.

> I think this change will cause the GPIO subsystem to call into the pinctrl
> subsystem and create/add/register a new GPIO<->pinctrl range structure. The
> pinctrl driver already does this, so I think we'll end up with two duplicate
> entries in the pinctrl device's gpio_ranges list. This probably won't cause
> a problem, but I wanted to make sure you'd thought about it to make sure.

That sounds like duplication indeed, I would expect that first a patch
adds the ranges to the dts[i] files and then a second patch delete the
same ranges from the pinctrl driver then, if these shall come in from
the device tree.

With GPIO ranges being possible to register from the pin controller,
gpio chip and also the device tree, things get a bit complex
admittedly :/ sorry for this, just choose one.

Yours,
Linus Walleij

Re: [PATCH 02/21] ARM: tegra: Add gpio-ranges property

[Stephen Warren]

On 06/02/2015 05:28 AM, Linus Walleij wrote:
> On Tue, May 26, 2015 at 9:41 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 05/25/2015 08:53 AM, Tomeu Vizoso wrote:
>>>
>>> Specify how the GPIOs map to the pins in T124, so the dependency is
>>> explicit.
>>>
>>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>>> ---
>>>    arch/arm/boot/dts/tegra124.dtsi | 1 +
>>>    1 file changed, 1 insertion(+)
>>>
>>> diff --git a/arch/arm/boot/dts/tegra124.dtsi
>>> b/arch/arm/boot/dts/tegra124.dtsi
>>> index 13cc7ca..5d1d35f 100644
>>> --- a/arch/arm/boot/dts/tegra124.dtsi
>>> +++ b/arch/arm/boot/dts/tegra124.dtsi
>>> @@ -254,6 +254,7 @@
>>>                  gpio-controller;
>>>                  #interrupt-cells = <2>;
>>>                  interrupt-controller;
>>> +               gpio-ranges = <&pinmux 0 0 250>;
>>
>>
>> We should be consistent between SoCs. Why not make the same change for all
>> Tegra SoCs?
>
> Agreed.
>
>> I think this change will cause the GPIO subsystem to call into the pinctrl
>> subsystem and create/add/register a new GPIO<->pinctrl range structure. The
>> pinctrl driver already does this, so I think we'll end up with two duplicate
>> entries in the pinctrl device's gpio_ranges list. This probably won't cause
>> a problem, but I wanted to make sure you'd thought about it to make sure.
>
> That sounds like duplication indeed, I would expect that first a patch
> adds the ranges to the dts[i] files and then a second patch delete the
> same ranges from the pinctrl driver then, if these shall come in from
> the device tree.

We can't delete the gpio-range-registration code from the Tegra pinmux 
driver, or old DTs won't work correctly. We could make it conditional 
based upon whether the DT contains the property or not.

Re: [PATCH 00/21] On-demand device registration

[Grygorii.Strashko@linaro.org]

Hi Tomeu,

On 05/28/2015 07:33 AM, Rob Herring wrote:
> On Mon, May 25, 2015 at 9:53 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
>> 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.
>>
>> 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 registering devices as they are referenced by other
>> devices.
> 
> I like the concept and novel approach.
> 
>> This basically reuses the information that is already implicit in the probe()
>> implementations, saving us from refactoring existing drivers or adding
>> information to DTBs.
>>
>> Something I'm not completely happy with is that I have had to move the call to
>> of_platform_populate after all platform drivers have been registered.
>> Otherwise I don't see how I could register drivers on demand as we don't have
>> yet each driver's compatible strings.
> 
> Yeah, this is the opposite of what we'd really like. Ideally, we would
> have a solution that works for modules too. However, we're no worse
> off. We pretty much build-in dependencies to avoid module ordering
> problems.
> 
> Perhaps we need to make the probing on-demand rather than simply on
> device<->driver match occurring.
> 
>> For machs that don't move of_platform_populate() to a later point, these
>> patches shouldn't cause any problems but it's not guaranteed that we'll avoid
>> all the deferred probes as some drivers may not be registered yet.
> 
> Ideally, of_platform_populate is not explicitly called by each
> platform. So I think we need to make this work for the default case.
> 
>> I have tested this on boards with Tegra, iMX.6 and Exynos SoCs, and these
>> patches were enough to eliminate all the deferred probes.
>>
>> With this series I get the kernel to output to the panel in 0.5s, instead of 2.8s.
> 
> That's certainly compelling.

I've found your idea about moving device registration later during System boot
very interesting so I've decided to try it on dra7-evem (TI) :).
It's good to know time during Kernel boot when we can assume that all drivers are
ready for probing, so there are more ways to control probing order.

Pls, Note here that TI OMAP2+ mach is not pure DT mach - it's combination of
DT and not DT devices/drivers.

Ok. So What was done...

LKML Linux 4.1-rc3 (a simple case)
1) use your patches 3/4 as reference (only these two patches :)
2) move of_platform_populate later at device_initcall_sync time
Boot time reduction ~0.4 sec


TI Android Kernel 3.14 (NOT a simple case)
1) use your patches 3/4 as reference (only these two patches :)
2) move of_platform_populate later at device_initcall_sync time
3) make it to boot (not sure I've fixed all issues, but those which
   break the System boot):
 - split non-DT and DT devices registration in platform code;
 - keep non-DT devices registration from .init_machine() [arch_initcall]
 - move DT-devices registration at device_initcall_sync time
 - fix drivers which use platform_driver_probe().
   Note. Now there are at about ~190 occurrences of this macro in Kernel.
 - re-order few devices in DT (4 devices)
 - fix one driver which uses of_find_device_by_node() wrongly
   Note. This API is used some times with assumption that
   requested dev has been probed already.
Boot time reduction ~0.3 sec. Probing of some devices are still deferred.

TI Android Kernel 3.14 + of_platform_device_ensure
1) backport of_platform_device_ensure() (also need patches 
   "of: Introduce device tree node flag helpers" and 
   "of: Keep track of populated platform devices")
2) back-port all your patches which uses of_platform_device_ensure()
3) make it to boot:
   - drop patch dma: of: Probe DMA controllers on demand
   - fix deadlock in regulator core:
   regulator_dev_lookup() called from regulator_register() in K3.14
4) get rid of deferred probes - add of_platform_device_ensure() calls in:
   - drivers/video/fbdev/omap2/dss/output.c
   - drivers/extcon/extcon-class.c

 Boot time reduction: NONE !?


So few comments from above:
- registering devices later during the System boot may improve boot time.
  But resolving of all deferred probes may NOT improve boot time ;) 
  Have you seen smth like this?

- usage of of_platform_device_ensure() will require continuous fixing of Kernel :(

- late_initcall is not (as for me not safe) a good time to register devices. A lot
  of platforms/subsystems/frameworks perform their final initialization or clean-up
  steps, with assumption that System mostly ready to work. For example, CPUIdle/CPUFreq
  are allowed and other PM staff. CPUIdle and driver's probing are not friends.

What would be nice to have for now in my opinion:
- some useful place to move device population code.
  May be machine_desc->init_device_sync() callback.
- some optional feature in DTC which will allow to re-arrange DT nodes for levels 0(root)
  and 1(simple-bus) using standard or widely used bindings (gpio, regulators, clocks, dma,
  pinctrl, irqs).

Additional note:
- Changing device's registration order will change devices's order in
 dpm_list (drivers/base/power/main.c) and devices_kset list
 (drivers/base/core.c). This might potentially affect on suspend [1] and shutdown. 
[1] https://lkml.org/lkml/2014/12/12/324


-- 
regards,
-grygorii

Re: [PATCH 00/21] On-demand device registration

[Rob Clark]

On Mon, May 25, 2015 at 10:53 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.
>
> 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 registering 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.
>
> Something I'm not completely happy with is that I have had to move the call to
> of_platform_populate after all platform drivers have been registered.
> Otherwise I don't see how I could register drivers on demand as we don't have
> yet each driver's compatible strings.
>
> For machs that don't move of_platform_populate() to a later point, these
> patches shouldn't cause any problems but it's not guaranteed that we'll avoid
> all the deferred probes as some drivers may not be registered yet.
>
> I have tested this on boards with Tegra, iMX.6 and Exynos SoCs, and these
> patches were enough to eliminate all the deferred probes.
>
> With this series I get the kernel to output to the panel in 0.5s, instead of 2.8s.

So, complete drive-by comment (and I won't claim to be a DT expert,
etc, etc, so take this with a few grains of salt), but why not push
the problem to the DT compiler (or a pre-process step that could be
run on existing DT blobs), which generates an optional DT node that is
the recommended probe order?  That seems like it avoids adding
complexity into the early boot code (which seems like a good thing)..

As a bonus, a bootarg (or something like that) which runs through the
recommended probe order in reverse (to continue our current state of
ensuring that EPROBE_DEFER error paths are well tested)

At any rate, for a device like a drm driver that has multiple
sub-components, and depends on various other clk/gpio/regulator/etc
drivers, the current EPROBE_DEFER situation is pretty comical, so any
solution that improves on things is very much welcome :-)

BR,
-R

> Regards,
>
> Tomeu
>
> [0] http://lists.freedesktop.org/archives/dri-devel/2014-August/066527.html
>
> [1] https://lkml.org/lkml/2014/5/12/452
>
> Tomeu Vizoso (21):
>   regulator: core: Reduce critical area in _regulator_get
>   ARM: tegra: Add gpio-ranges property
>   ARM: tegra: Register drivers before devices
>   ARM: EXYNOS: Register drivers before devices
>   ARM i.MX6q: Register drivers before devices
>   of/platform: Add of_platform_device_ensure()
>   of/platform: Ensure device registration on lookup
>   gpio: Probe GPIO drivers on demand
>   gpio: Probe pinctrl devices on demand
>   regulator: core: Probe regulators on demand
>   drm: Probe panels on demand
>   drm/tegra: Probe dpaux devices on demand
>   i2c: core: Probe i2c master 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
>
>  arch/arm/boot/dts/tegra124.dtsi     |  1 +
>  arch/arm/mach-exynos/exynos.c       |  4 +--
>  arch/arm/mach-imx/mach-imx6q.c      | 12 ++++-----
>  arch/arm/mach-tegra/tegra.c         | 21 ++++++---------
>  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              |  3 +++
>  drivers/of/platform.c               | 53 +++++++++++++++++++++++++++++++++++++
>  drivers/phy/phy-core.c              |  3 +++
>  drivers/pinctrl/devicetree.c        |  2 ++
>  drivers/power/power_supply_core.c   |  3 +++
>  drivers/pwm/core.c                  |  3 +++
>  drivers/regulator/core.c            | 45 +++++++++++++++----------------
>  drivers/usb/phy/phy.c               |  3 +++
>  drivers/video/backlight/backlight.c |  3 +++
>  include/linux/of_platform.h         |  2 ++
>  19 files changed, 130 insertions(+), 45 deletions(-)
>
> --
> 2.4.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 00/21] On-demand device registration

[Tomeu Vizoso]

On 3 June 2015 at 21:57, Grygorii.Strashko@linaro.org
<grygorii.strashko@linaro.org> wrote:
> Hi Tomeu,
>
> On 05/28/2015 07:33 AM, Rob Herring wrote:
>> On Mon, May 25, 2015 at 9:53 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
>>> 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.
>>>
>>> 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 registering devices as they are referenced by other
>>> devices.
>>
>> I like the concept and novel approach.
>>
>>> This basically reuses the information that is already implicit in the probe()
>>> implementations, saving us from refactoring existing drivers or adding
>>> information to DTBs.
>>>
>>> Something I'm not completely happy with is that I have had to move the call to
>>> of_platform_populate after all platform drivers have been registered.
>>> Otherwise I don't see how I could register drivers on demand as we don't have
>>> yet each driver's compatible strings.
>>
>> Yeah, this is the opposite of what we'd really like. Ideally, we would
>> have a solution that works for modules too. However, we're no worse
>> off. We pretty much build-in dependencies to avoid module ordering
>> problems.
>>
>> Perhaps we need to make the probing on-demand rather than simply on
>> device<->driver match occurring.
>>
>>> For machs that don't move of_platform_populate() to a later point, these
>>> patches shouldn't cause any problems but it's not guaranteed that we'll avoid
>>> all the deferred probes as some drivers may not be registered yet.
>>
>> Ideally, of_platform_populate is not explicitly called by each
>> platform. So I think we need to make this work for the default case.
>>
>>> I have tested this on boards with Tegra, iMX.6 and Exynos SoCs, and these
>>> patches were enough to eliminate all the deferred probes.
>>>
>>> With this series I get the kernel to output to the panel in 0.5s, instead of 2.8s.
>>
>> That's certainly compelling.
>
> I've found your idea about moving device registration later during System boot
> very interesting so I've decided to try it on dra7-evem (TI) :).
> It's good to know time during Kernel boot when we can assume that all drivers are
> ready for probing, so there are more ways to control probing order.

Thanks, though right now I'm following Rob's suggestion and only delay
probing, not registration. The patch is really simple (applies on
linux-next, with async probing):

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 8da8e07..7e6b1e1 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -407,6 +407,11 @@ int driver_probe_device(struct device_driver
*drv, struct device *dev)
        if (!device_is_registered(dev))
                return -ENODEV;

+       if (!driver_deferred_probe_enable) {
+               driver_deferred_probe_add(dev);
+               return 0;
+       }
+
        pr_debug("bus: '%s': %s: matched device %s with driver %s\n",
                 drv->bus->name, __func__, dev_name(dev), drv->name);

@@ -585,7 +590,7 @@ EXPORT_SYMBOL_GPL(device_attach);

 void device_initial_probe(struct device *dev)
 {
-       __device_attach(dev, true);
+       __device_attach(dev, driver_deferred_probe_enable);
 }

 static int __driver_attach(struct device *dev, void *data)

> Pls, Note here that TI OMAP2+ mach is not pure DT mach - it's combination of
> DT and not DT devices/drivers.
>
> Ok. So What was done...
>
> LKML Linux 4.1-rc3 (a simple case)
> 1) use your patches 3/4 as reference (only these two patches :)
> 2) move of_platform_populate later at device_initcall_sync time
> Boot time reduction ~0.4 sec

I'm a bit surprised at such a big improvement. May I ask how you are
measuring it?

> TI Android Kernel 3.14 (NOT a simple case)
> 1) use your patches 3/4 as reference (only these two patches :)
> 2) move of_platform_populate later at device_initcall_sync time
> 3) make it to boot (not sure I've fixed all issues, but those which
>    break the System boot):
>  - split non-DT and DT devices registration in platform code;
>  - keep non-DT devices registration from .init_machine() [arch_initcall]
>  - move DT-devices registration at device_initcall_sync time
>  - fix drivers which use platform_driver_probe().
>    Note. Now there are at about ~190 occurrences of this macro in Kernel.
>  - re-order few devices in DT (4 devices)
>  - fix one driver which uses of_find_device_by_node() wrongly
>    Note. This API is used some times with assumption that
>    requested dev has been probed already.
> Boot time reduction ~0.3 sec. Probing of some devices are still deferred.

I got no deferred probes on a pandaboard with just these changes:

https://git.collabora.com/cgit/user/tomeu/linux.git/commit/?id=1586c6f50b3d3c65dc219a3cdc3327d798cabca6

> TI Android Kernel 3.14 + of_platform_device_ensure
> 1) backport of_platform_device_ensure() (also need patches
>    "of: Introduce device tree node flag helpers" and
>    "of: Keep track of populated platform devices")
> 2) back-port all your patches which uses of_platform_device_ensure()
> 3) make it to boot:
>    - drop patch dma: of: Probe DMA controllers on demand
>    - fix deadlock in regulator core:
>    regulator_dev_lookup() called from regulator_register() in K3.14
> 4) get rid of deferred probes - add of_platform_device_ensure() calls in:
>    - drivers/video/fbdev/omap2/dss/output.c
>    - drivers/extcon/extcon-class.c
>
>  Boot time reduction: NONE !?
>
> So few comments from above:
> - registering devices later during the System boot may improve boot time.
>   But resolving of all deferred probes may NOT improve boot time ;)
>   Have you seen smth like this?

Yeah, I don't really expect for on-demand probing to improve boot time
much in the general case, as drivers tend to first acquire resources
(and defer probe if needed) and only then access hardware and wait for
stuff to happen.

The main advantage of ordered/ondemand probing is that it is much
easier to see the order in which devices will be bound. In my case
this is useful because one could get one device (eg. the drm panel) to
probe very early by just moving that node to the beginning of the DT.
With deferred probe, one would have to figure out all the dependencies
and shuffle them around in the DT.

Another advantage (but not the one why I'm doing this work) is that in
general a driver's probe will be called only once per device, and if
it fails then we can be almost certain that something is wrong. This
should aid in debugging as right now one has to take into account the
several reasons why a device might defer its probe.

Depending on what work your drivers do in your platform, enabling
async probing for all of them may have a noticeable impact though.

> - usage of of_platform_device_ensure() will require continuous fixing of Kernel :(

You mean adding those calls to more subsystems?

> - late_initcall is not (as for me not safe) a good time to register devices. A lot
>   of platforms/subsystems/frameworks perform their final initialization or clean-up
>   steps, with assumption that System mostly ready to work. For example, CPUIdle/CPUFreq
>   are allowed and other PM staff. CPUIdle and driver's probing are not friends.

Yeah, I was expecting to find more problems due to this, but the
platforms I tested on didn't show any. Do you have pointers to
concrete issues?

> What would be nice to have for now in my opinion:
> - some useful place to move device population code.
>   May be machine_desc->init_device_sync() callback.

I'm looking at leave device registration where it currently is and
just add devices to the deferred list until we get to late_initcall,
where we would start to actually probe them. Seems promising for now.

> - some optional feature in DTC which will allow to re-arrange DT nodes for levels 0(root)
>   and 1(simple-bus) using standard or widely used bindings (gpio, regulators, clocks, dma,
>   pinctrl, irqs).

Could you extend on this, please?

> Additional note:
> - Changing device's registration order will change devices's order in
>  dpm_list (drivers/base/power/main.c) and devices_kset list
>  (drivers/base/core.c). This might potentially affect on suspend [1] and shutdown.

Yeah, I don't plan to change registration order, but probing certainly will.

> [1] https://lkml.org/lkml/2014/12/12/324

Thanks,

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 00/21] On-demand device registration

[Grygorii.Strashko@linaro.org]

On 06/04/2015 11:39 AM, Tomeu Vizoso wrote:
> On 3 June 2015 at 21:57, Grygorii.Strashko@linaro.org
> <grygorii.strashko@linaro.org> wrote:
>> On 05/28/2015 07:33 AM, Rob Herring wrote:
>>> On Mon, May 25, 2015 at 9:53 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
>>>> 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.
>>>>
>>>> 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 registering devices as they are referenced by other
>>>> devices.
>>>
>>> I like the concept and novel approach.
>>>
>>>> This basically reuses the information that is already implicit in the probe()
>>>> implementations, saving us from refactoring existing drivers or adding
>>>> information to DTBs.
>>>>
>>>> Something I'm not completely happy with is that I have had to move the call to
>>>> of_platform_populate after all platform drivers have been registered.
>>>> Otherwise I don't see how I could register drivers on demand as we don't have
>>>> yet each driver's compatible strings.
>>>
>>> Yeah, this is the opposite of what we'd really like. Ideally, we would
>>> have a solution that works for modules too. However, we're no worse
>>> off. We pretty much build-in dependencies to avoid module ordering
>>> problems.
>>>
>>> Perhaps we need to make the probing on-demand rather than simply on
>>> device<->driver match occurring.
>>>
>>>> For machs that don't move of_platform_populate() to a later point, these
>>>> patches shouldn't cause any problems but it's not guaranteed that we'll avoid
>>>> all the deferred probes as some drivers may not be registered yet.
>>>
>>> Ideally, of_platform_populate is not explicitly called by each
>>> platform. So I think we need to make this work for the default case.
>>>
>>>> I have tested this on boards with Tegra, iMX.6 and Exynos SoCs, and these
>>>> patches were enough to eliminate all the deferred probes.
>>>>
>>>> With this series I get the kernel to output to the panel in 0.5s, instead of 2.8s.
>>>
>>> That's certainly compelling.
>>
>> I've found your idea about moving device registration later during System boot
>> very interesting so I've decided to try it on dra7-evem (TI) :).
>> It's good to know time during Kernel boot when we can assume that all drivers are
>> ready for probing, so there are more ways to control probing order.
> 
> Thanks, though right now I'm following Rob's suggestion and only delay
> probing, not registration. The patch is really simple (applies on
> linux-next, with async probing):
> 
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 8da8e07..7e6b1e1 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -407,6 +407,11 @@ int driver_probe_device(struct device_driver
> *drv, struct device *dev)
>          if (!device_is_registered(dev))
>                  return -ENODEV;
> 
> +       if (!driver_deferred_probe_enable) {
> +               driver_deferred_probe_add(dev);
> +               return 0;
> +       }
> +
>          pr_debug("bus: '%s': %s: matched device %s with driver %s\n",
>                   drv->bus->name, __func__, dev_name(dev), drv->name);
> 
> @@ -585,7 +590,7 @@ EXPORT_SYMBOL_GPL(device_attach);
> 
>   void device_initial_probe(struct device *dev)
>   {
> -       __device_attach(dev, true);
> +       __device_attach(dev, driver_deferred_probe_enable);
>   }
> 
>   static int __driver_attach(struct device *dev, void *data)

Can't boot my 3.14 kernel with this change :( Most probably,
the problem is related to platform_driver_probe() usage :(
Have no time to play with it now :(, but recommend you to check also
earlyprintk, last log message I can see:
[    1.435522] bootconsole [earlycon0] disabled

But, nice try ;) Seems -EPROBE_DEFER is reality of life which
has to be accepted as is.

> 
>> Pls, Note here that TI OMAP2+ mach is not pure DT mach - it's combination of
>> DT and not DT devices/drivers.
>>
>> Ok. So What was done...
>>
>> LKML Linux 4.1-rc3 (a simple case)
>> 1) use your patches 3/4 as reference (only these two patches :)
>> 2) move of_platform_populate later at device_initcall_sync time
>> Boot time reduction ~0.4 sec
> 
> I'm a bit surprised at such a big improvement. May I ask how you are
> measuring it?

Ah. My measurements are not precise. I've just tracking time of message
"[    4.110756] Freeing unused kernel memory: 344K (c0994000 - c09ea000)"

> 
>> TI Android Kernel 3.14 (NOT a simple case)
>> 1) use your patches 3/4 as reference (only these two patches :)
>> 2) move of_platform_populate later at device_initcall_sync time
>> 3) make it to boot (not sure I've fixed all issues, but those which
>>     break the System boot):
>>   - split non-DT and DT devices registration in platform code;
>>   - keep non-DT devices registration from .init_machine() [arch_initcall]
>>   - move DT-devices registration at device_initcall_sync time
>>   - fix drivers which use platform_driver_probe().
>>     Note. Now there are at about ~190 occurrences of this macro in Kernel.
>>   - re-order few devices in DT (4 devices)
>>   - fix one driver which uses of_find_device_by_node() wrongly
>>     Note. This API is used some times with assumption that
>>     requested dev has been probed already.
>> Boot time reduction ~0.3 sec. Probing of some devices are still deferred.
> 
> I got no deferred probes on a pandaboard with just these changes:
> 
> https://git.collabora.com/cgit/user/tomeu/linux.git/commit/?id=1586c6f50b3d3c65dc219a3cdc3327d798cabca6

As I've mentioned I tried TI Android Kernel 3.14 where all DRA7 SoC features are enabled.
It is very close to production SW. LKML, by default, enables
mostly nothing and not all features are supported. 
In my prev exercise I was able to boot till Android GUI and it has worked :)

Also, cool statistic - really_probe() was called 136 times for me
(successful execution of dev->bus->probe() and drv->probe()).

> 
>> TI Android Kernel 3.14 + of_platform_device_ensure
>> 1) backport of_platform_device_ensure() (also need patches
>>     "of: Introduce device tree node flag helpers" and
>>     "of: Keep track of populated platform devices")
>> 2) back-port all your patches which uses of_platform_device_ensure()
>> 3) make it to boot:
>>     - drop patch dma: of: Probe DMA controllers on demand
>>     - fix deadlock in regulator core:
>>     regulator_dev_lookup() called from regulator_register() in K3.14
>> 4) get rid of deferred probes - add of_platform_device_ensure() calls in:
>>     - drivers/video/fbdev/omap2/dss/output.c
>>     - drivers/extcon/extcon-class.c
>>
>>   Boot time reduction: NONE !?
>>
>> So few comments from above:
>> - registering devices later during the System boot may improve boot time.
>>    But resolving of all deferred probes may NOT improve boot time ;)
>>    Have you seen smth like this?
> 
> Yeah, I don't really expect for on-demand probing to improve boot time
> much in the general case, as drivers tend to first acquire resources
> (and defer probe if needed) and only then access hardware and wait for
> stuff to happen.
> 
> The main advantage of ordered/ondemand probing is that it is much
> easier to see the order in which devices will be bound. In my case
> this is useful because one could get one device (eg. the drm panel) to
> probe very early by just moving that node to the beginning of the DT.
> With deferred probe, one would have to figure out all the dependencies
> and shuffle them around in the DT.
> 
> Another advantage (but not the one why I'm doing this work) is that in
> general a driver's probe will be called only once per device, and if
> it fails then we can be almost certain that something is wrong. This
> should aid in debugging as right now one has to take into account the
> several reasons why a device might defer its probe.
> 
> Depending on what work your drivers do in your platform, enabling
> async probing for all of them may have a noticeable impact though.
> 
>> - usage of of_platform_device_ensure() will require continuous fixing of Kernel :(
> 
> You mean adding those calls to more subsystems?

Exactly. Each time new framework will be introduced or reworked (or even for some drivers),
because each of them implement its own of_get_xxx() API. 

> 
>> - late_initcall is not (as for me not safe) a good time to register devices. A lot
>>    of platforms/subsystems/frameworks perform their final initialization or clean-up
>>    steps, with assumption that System mostly ready to work. For example, CPUIdle/CPUFreq
>>    are allowed and other PM staff. CPUIdle and driver's probing are not friends.
> 
> Yeah, I was expecting to find more problems due to this, but the
> platforms I tested on didn't show any. Do you have pointers to
> concrete issues?

No. This observation comes from my working experience with OMAP4 devices where
lowest possible CPUIdle state was mostly equal to Device-OFF state. 
Fast search has allowed me to find possible source of issues in code - I'm pretty sure
smth. similar can be found in other ARM maches:
- arch/arm/mach-omap2/omap_device.c -> omap_device_late_init().

> 
>> What would be nice to have for now in my opinion:
>> - some useful place to move device population code.
>>    May be machine_desc->init_device_sync() callback.
> 
> I'm looking at leave device registration where it currently is and
> just add devices to the deferred list until we get to late_initcall,
> where we would start to actually probe them. Seems promising for now.
> 
>> - some optional feature in DTC which will allow to re-arrange DT nodes for levels 0(root)
>>    and 1(simple-bus) using standard or widely used bindings (gpio, regulators, clocks, dma,
>>    pinctrl, irqs).
> 
> Could you extend on this, please?

This is actually mostly the same idea as was mentioned by Rob Clark 
http://www.spinics.net/lists/arm-kernel/msg423485.html


For example, mmc1 probe will be deferred always for below DT,
but if we put evm_3v3_sw before ocp and exchange mmc1 and i2c1
- mmc1 will be probed without deferring.
Potentially, It can be done in DTC where we do not have so strict limits
as for Kernel code. 

Soc file:
/ { /* root - level 0 */
	ocp { /* simple-bus - level 1 */
		compatible =  "simple-bus";

		mmc1: mmc@4809c000 {
			compatible = "mmc";
		...
		}

		i2c1: i2c@48070000 {
			compatible = "i2c";
		}
	}
}

board file:
#include "SoC.dtsi"

/ {
	evm_3v3_sw: fixedregulator-evm_3v3_sw {
		compatible = "regulator-fixed";
		regulator-name = "evm_3v3_sw";
		regulator-min-microvolt = <3300000>;
		regulator-max-microvolt = <3300000>;
	};
};

&i2c1 {
	pcf_gpio_21: gpio@21 {
		compatible = "ti,pcf8575";
		reg = <0x21>;
		lines-initial-states = <0x1408>;
		gpio-controller;
		#gpio-cells = <2>;
	};
}

&mmc1 {
	status = "okay";
	vmmc-supply = <&evm_3v3_sw>;
	bus-width = <4>;
	cd-gpios = <&pcf_gpio_21 5 0>;
};
	


-- 
regards,
-grygorii

Re: [PATCH 00/21] On-demand device registration

[Alexander Holler]

Am 03.06.2015 um 21:57 schrieb Grygorii.Strashko@linaro.org:

...

> So few comments from above:
> - registering devices later during the System boot may improve boot time.
>   But resolving of all deferred probes may NOT improve boot time ;) 
>   Have you seen smth like this?

If someone is out for boot time reduction, I think one of the best ways
would by making driver initialization parallel. Keep in mind that all
linked in drivers currently are initialized in series.

As it seems to have been forgotten or overread, I've mentioned in my
series of patches last year that, with a few changes, it's possible to
let the algorithm I've used (dfs) to spit out all drivers which can be
initialized in parallel.

But as I'm not paid for the work I've done and just did it out of
curiosity, interest or how ever you want name it, I haven't spend any
more time into that topic, especially as I'm missing the necessary
connections to get patches into the kernel. ;)

But, as said, it's easy (at least if aren't getting panic when it comes
to a bit of algorithm theory) to get a list drivers you can start in
parallel if you have such a complete list of dependencies as DT already
offers. Just look at the pictures generate by dtc (using my patches),
you will see, they already show which drivers can be initialized in
parallel.

So it would be easy to use e.g. all cores already very early at boot to
initialize drivers, not just after init got started. Besides that the
würgaround of defered init (which, btw. leads devs to supress error
messages, which is especially bad if you are searching a problem) isn't
needed anymore if you have a list of dependecies (however you get it,
I've used DT because the dependencies already are all there).

Regards,

Alexander Holler

Re: [PATCH 00/21] On-demand device registration

[Alexander Holler]

Am 03.06.2015 um 23:12 schrieb Rob Clark:
> On Mon, May 25, 2015 at 10:53 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.
>>
>> 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 registering 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.
>>
>> Something I'm not completely happy with is that I have had to move the call to
>> of_platform_populate after all platform drivers have been registered.
>> Otherwise I don't see how I could register drivers on demand as we don't have
>> yet each driver's compatible strings.
>>
>> For machs that don't move of_platform_populate() to a later point, these
>> patches shouldn't cause any problems but it's not guaranteed that we'll avoid
>> all the deferred probes as some drivers may not be registered yet.
>>
>> I have tested this on boards with Tegra, iMX.6 and Exynos SoCs, and these
>> patches were enough to eliminate all the deferred probes.
>>
>> With this series I get the kernel to output to the panel in 0.5s, instead of 2.8s.
> 
> So, complete drive-by comment (and I won't claim to be a DT expert,
> etc, etc, so take this with a few grains of salt), but why not push
> the problem to the DT compiler (or a pre-process step that could be
> run on existing DT blobs), which generates an optional DT node that is
> the recommended probe order?  That seems like it avoids adding
> complexity into the early boot code (which seems like a good thing)..

I've played with that approach too (as my patches for dtc do contain the
same code I've put into the kernel, but decided that it doesn't make
much sense. The sort algorithm is really small (some dozen lines), very
fast (around 3-5ms on a omap) and might be later used to sort necessary
module loading too. So there would be no advantage to put a sorted list
into the DT. And having the sort algorithm in the kernel, would make it
possible to use it for acpi or something else too, if they manage it to
provide the necessary dependencies.

Regards,

Alexander Holler

Re: [PATCH 00/21] On-demand device registration

[Enrico Weigelt, metux IT consult]

Am 04.06.2015 um 22:39 schrieb Alexander Holler:

 > As it seems to have been forgotten or overread, I've mentioned in my
> series of patches last year that, with a few changes, it's possible to
> let the algorithm I've used (dfs) to spit out all drivers which can be
> initialized in parallel.

Unfortunately, I've missed that ... could you please resend you patches?
Boot time reduction is one of the topics on my 2do in several weeks.


cu
--
Enrico Weigelt, metux IT consult
+49-151-27565287
MELAG Medizintechnik oHG Sitz Berlin Registergericht AG Charlottenburg HRA 21333 B

Wichtiger Hinweis: Diese Nachricht kann vertrauliche oder nur für einen begrenzten Personenkreis bestimmte Informationen enthalten. Sie ist ausschließlich für denjenigen bestimmt, an den sie gerichtet worden ist. Wenn Sie nicht der Adressat dieser E-Mail sind, dürfen Sie diese nicht kopieren, weiterleiten, weitergeben oder sie ganz oder teilweise in irgendeiner Weise nutzen. Sollten Sie diese E-Mail irrtümlich erhalten haben, so benachrichtigen Sie bitte den Absender, indem Sie auf diese Nachricht antworten. Bitte löschen Sie in diesem Fall diese Nachricht und alle Anhänge, ohne eine Kopie zu behalten.
Important Notice: This message may contain confidential or privileged information. It is intended only for the person it was addressed to. If you are not the intended recipient of this email you may not copy, forward, disclose or otherwise use it or any part of it in any form whatsoever. If you received this email in error please notify the sender by replying and delete this message and any attachments without retaining a copy.

Re: [PATCH 00/21] On-demand device registration

[Alexander Holler]

Am 08.06.2015 um 14:26 schrieb Enrico Weigelt, metux IT consult:
> Am 04.06.2015 um 22:39 schrieb Alexander Holler:
>
>  > As it seems to have been forgotten or overread, I've mentioned in my
>> series of patches last year that, with a few changes, it's possible to
>> let the algorithm I've used (dfs) to spit out all drivers which can be
>> initialized in parallel.
>
> Unfortunately, I've missed that ... could you please resend you patches?
> Boot time reduction is one of the topics on my 2do in several weeks.

https://lkml.org/lkml/2014/5/12/452

Re: [PATCH 00/21] On-demand device registration

[Alexander Holler]

Am 08.06.2015 um 20:14 schrieb Alexander Holler:
> Am 08.06.2015 um 14:26 schrieb Enrico Weigelt, metux IT consult:
>> Am 04.06.2015 um 22:39 schrieb Alexander Holler:
>>
>>  > As it seems to have been forgotten or overread, I've mentioned in my
>>> series of patches last year that, with a few changes, it's possible to
>>> let the algorithm I've used (dfs) to spit out all drivers which can be
>>> initialized in parallel.
>>
>> Unfortunately, I've missed that ... could you please resend you patches?
>> Boot time reduction is one of the topics on my 2do in several weeks.
>
> https://lkml.org/lkml/2014/5/12/452
>

And don't forget patch 10/9 which fixed a bug in my previous patch 
series and which alos was the reason for the large difference in boot 
times with and without deps:

https://lkml.org/lkml/2014/5/13/567

Re: [PATCH 00/21] On-demand device registration

[Linus Walleij]

On Tue, Jun 2, 2015 at 12:14 PM, Tomeu Vizoso
<tomeu.vizoso@collabora.com> wrote:
> On 2 June 2015 at 10:48, Linus Walleij <linus.walleij@linaro.org> wrote:

>> This is what systemd is doing in userspace for starting services:
>> ask for your dependencies and wait for them if they are not
>> there. So drivers ask for resources and wait for them. It also
>> needs to be abstract, so for example we need to be able to
>> hang on regulator_get() until the driver is up and providing that
>> regulator, and as long as everything is in slowpath it should
>> be OK. (And vice versa mutatis mutandis for clk, gpio, pin
>> control, interrupts (!) and DMA channels for example.)
>
> I understood above that you propose probing devices in order, but now
> you mention that resource getters would block until the dependency is
> fulfilled which confuses me because if we are probing in order then
> all dependencies would be fulfilled before the device in question gets
> probed.

Sorry, the problem space is a bit convoluted so the answers
get a bit convoluted. Maybe I'm thinking aloud and altering the course
of my thoughts as I type...

I guess there can be explicit dependencies for resources like this
patch does, but another way would be for all resource fetch functions
to be instrumented, so that you do not block until you try to take
a resource that is not yet there, e.g.:

regulator_get(...) -> not available, so:
- identify target regulator provider - this will need instrumentation
- probe it

It then turns out the regulator driver is on the i2c bus, so we
need to probe the i2c driver:
- identify target i2c host for the regulator driver - this will need
  instrumentation
- probe the i2c host driver

i2c host comes out, probes the regulator driver, regulator driver
probes and then the regulator_get() call returns.

This requires instrumentation on anything providing a resource
to another driver like those I mentioned and a lot of overhead
infrastructure, but I think it's the right approach. However I don't
know if I would ever be able to pull that off myself, I know talk
is cheap and I should show the code instead.

Deepest respect for your efforts!

Yours,
Linus Walleij

Re: [PATCH 00/21] On-demand device registration

[Tomeu Vizoso]

On 10 June 2015 at 09:30, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Tue, Jun 2, 2015 at 12:14 PM, Tomeu Vizoso
> <tomeu.vizoso@collabora.com> wrote:
>> On 2 June 2015 at 10:48, Linus Walleij <linus.walleij@linaro.org> wrote:
>
>>> This is what systemd is doing in userspace for starting services:
>>> ask for your dependencies and wait for them if they are not
>>> there. So drivers ask for resources and wait for them. It also
>>> needs to be abstract, so for example we need to be able to
>>> hang on regulator_get() until the driver is up and providing that
>>> regulator, and as long as everything is in slowpath it should
>>> be OK. (And vice versa mutatis mutandis for clk, gpio, pin
>>> control, interrupts (!) and DMA channels for example.)
>>
>> I understood above that you propose probing devices in order, but now
>> you mention that resource getters would block until the dependency is
>> fulfilled which confuses me because if we are probing in order then
>> all dependencies would be fulfilled before the device in question gets
>> probed.
>
> Sorry, the problem space is a bit convoluted so the answers
> get a bit convoluted. Maybe I'm thinking aloud and altering the course
> of my thoughts as I type...
>
> I guess there can be explicit dependencies for resources like this
> patch does, but another way would be for all resource fetch functions
> to be instrumented, so that you do not block until you try to take
> a resource that is not yet there, e.g.:
>
> regulator_get(...) -> not available, so:
> - identify target regulator provider - this will need instrumentation
> - probe it
>
> It then turns out the regulator driver is on the i2c bus, so we
> need to probe the i2c driver:
> - identify target i2c host for the regulator driver - this will need
>   instrumentation
> - probe the i2c host driver
>
> i2c host comes out, probes the regulator driver, regulator driver
> probes and then the regulator_get() call returns.

Hmm, if I understand correctly what you say, this is exactly what this
particular series does:

regulator_get -> of_platform_device_ensure -> probe() on the platform
device that encloses the requested device node (i2c host) -> i2c slave
gets probed and the regulator registered -> regulator_get returns the
requested resource

The downside I'm currently looking at is that an explicit dependency
graph would be useful to have for other purposes. For example to print
a neat warning when a dependency cannot be fulfilled. Or to refuse to
unbind a device which other devices depend on, or to automatically
unbind the devices that depend on it, or to print a warning if a
device is hotplugged off and other devices depend on it.

> This requires instrumentation on anything providing a resource
> to another driver like those I mentioned and a lot of overhead
> infrastructure, but I think it's the right approach. However I don't
> know if I would ever be able to pull that off myself, I know talk
> is cheap and I should show the code instead.

Yeah, if you can give it a second look and say if it matches what you
wrote above, it would be very much appreciated.

> Deepest respect for your efforts!

Thanks!

Tomeu

> Yours,
> Linus Walleij
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/21] On-demand device registration

[Andrzej Hajda]

On 06/10/2015 12:19 PM, Tomeu Vizoso wrote:
> On 10 June 2015 at 09:30, Linus Walleij <linus.walleij@linaro.org> wrote:
>> On Tue, Jun 2, 2015 at 12:14 PM, Tomeu Vizoso
>> <tomeu.vizoso@collabora.com> wrote:
>>> On 2 June 2015 at 10:48, Linus Walleij <linus.walleij@linaro.org> wrote:
>>
>>>> This is what systemd is doing in userspace for starting services:
>>>> ask for your dependencies and wait for them if they are not
>>>> there. So drivers ask for resources and wait for them. It also
>>>> needs to be abstract, so for example we need to be able to
>>>> hang on regulator_get() until the driver is up and providing that
>>>> regulator, and as long as everything is in slowpath it should
>>>> be OK. (And vice versa mutatis mutandis for clk, gpio, pin
>>>> control, interrupts (!) and DMA channels for example.)
>>>
>>> I understood above that you propose probing devices in order, but now
>>> you mention that resource getters would block until the dependency is
>>> fulfilled which confuses me because if we are probing in order then
>>> all dependencies would be fulfilled before the device in question gets
>>> probed.
>>
>> Sorry, the problem space is a bit convoluted so the answers
>> get a bit convoluted. Maybe I'm thinking aloud and altering the course
>> of my thoughts as I type...
>>
>> I guess there can be explicit dependencies for resources like this
>> patch does, but another way would be for all resource fetch functions
>> to be instrumented, so that you do not block until you try to take
>> a resource that is not yet there, e.g.:
>>
>> regulator_get(...) -> not available, so:
>> - identify target regulator provider - this will need instrumentation
>> - probe it
>>
>> It then turns out the regulator driver is on the i2c bus, so we
>> need to probe the i2c driver:
>> - identify target i2c host for the regulator driver - this will need
>>   instrumentation
>> - probe the i2c host driver
>>
>> i2c host comes out, probes the regulator driver, regulator driver
>> probes and then the regulator_get() call returns.
> 
> Hmm, if I understand correctly what you say, this is exactly what this
> particular series does:
> 
> regulator_get -> of_platform_device_ensure -> probe() on the platform
> device that encloses the requested device node (i2c host) -> i2c slave
> gets probed and the regulator registered -> regulator_get returns the
> requested resource

The downside of this solution is that it will not work without device
tree or even without device dependencies not explicitly specified in
device tree.

> 
> The downside I'm currently looking at is that an explicit dependency
> graph would be useful to have for other purposes. For example to print
> a neat warning when a dependency cannot be fulfilled. Or to refuse to
> unbind a device which other devices depend on,

As I understand Greg you cannot prevent unbinding by design, see [1].

[1]: http://thread.gmane.org/gmane.linux.kernel/1154308/focus=1154648

> or to automatically
> unbind the devices that depend on it,

What about devices that have weak dependency? They should not be unbound
but they should be somehow noticed about unbinding.

In general many kernel frameworks are broken in handling hot-unbinding
of drivers, consumers are not noticed about unbinding of their resource
providers and usually they stay with broken handles or handles to dummy
resources.

I suspect the only proper solution for handling resources that can
dynamically appear/disappear is to provide notification to their
consumers about appearance change of the resource.

I have proposed some times ago solution for above problems based on the
statement above, cover letter explains it in more detail [2].
In short it solves following issues:
- consumer receives resource as soon as it becomes available,
- consumer is notified just before resource removal,
- it can properly handle provider unbind/re-bind,
- it avoids late init due to deferred probing,
- it allows to track optional resources.

[2]: http://thread.gmane.org/gmane.linux.kernel.gpio/5201

Regards
Andrzej

> or to print a warning if a
> device is hotplugged off and other devices depend on it.
> 
>> This requires instrumentation on anything providing a resource
>> to another driver like those I mentioned and a lot of overhead
>> infrastructure, but I think it's the right approach. However I don't
>> know if I would ever be able to pull that off myself, I know talk
>> is cheap and I should show the code instead.
> 
> Yeah, if you can give it a second look and say if it matches what you
> wrote above, it would be very much appreciated.
> 
>> Deepest respect for your efforts!
> 
> Thanks!
> 
> Tomeu
> 
>> Yours,
>> Linus Walleij
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> --
> To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 00/21] On-demand device registration

[Linus Walleij]

On Wed, Jun 10, 2015 at 10:28 AM, Alexander Holler <holler@ahsoftware.de> wrote:
> Am 10.06.2015 um 09:30 schrieb Linus Walleij:

>> i2c host comes out, probes the regulator driver, regulator driver
>> probes and then the regulator_get() call returns.
>>
>> This requires instrumentation on anything providing a resource
>> to another driver like those I mentioned and a lot of overhead
>> infrastructure, but I think it's the right approach. However I don't
>> know if I would ever be able to pull that off myself, I know talk
>> is cheap and I should show the code instead.
>
> You would end up with the same problem of deadlocks as currently, and you
> would still need something ugly like the defered probe brutforce to avoid
> them.

Sorry I don't get that. Care to elaborate on why?

Yours,
Linus Walleij

Re: [PATCH 00/21] On-demand device registration

[Linus Walleij]

On Wed, Jun 10, 2015 at 12:19 PM, Tomeu Vizoso
<tomeu.vizoso@collabora.com> wrote:
> On 10 June 2015 at 09:30, Linus Walleij <linus.walleij@linaro.org> wrote:

>> regulator_get(...) -> not available, so:
>> - identify target regulator provider - this will need instrumentation
>> - probe it
>>
>> It then turns out the regulator driver is on the i2c bus, so we
>> need to probe the i2c driver:
>> - identify target i2c host for the regulator driver - this will need
>>   instrumentation
>> - probe the i2c host driver
>>
>> i2c host comes out, probes the regulator driver, regulator driver
>> probes and then the regulator_get() call returns.
>
> Hmm, if I understand correctly what you say, this is exactly what this
> particular series does:
>
> regulator_get -> of_platform_device_ensure -> probe() on the platform
> device that encloses the requested device node (i2c host) -> i2c slave
> gets probed and the regulator registered -> regulator_get returns the
> requested resource

Yes. But only for device tree.

> The downside I'm currently looking at is that an explicit dependency
> graph would be useful to have for other purposes. For example to print
> a neat warning when a dependency cannot be fulfilled. Or to refuse to
> unbind a device which other devices depend on, or to automatically
> unbind the devices that depend on it, or to print a warning if a
> device is hotplugged off and other devices depend on it.

Unbind/remove() calls are the inverse usually yes.

But also the [runtime] power up/down sequences for the
devices tend to depend on a similar ordering or mostly
the same. (Mentioned this before I think.)

>> This requires instrumentation on anything providing a resource
>> to another driver like those I mentioned and a lot of overhead
>> infrastructure, but I think it's the right approach. However I don't
>> know if I would ever be able to pull that off myself, I know talk
>> is cheap and I should show the code instead.
>
> Yeah, if you can give it a second look and say if it matches what you
> wrote above, it would be very much appreciated.

Yes you are right. But what about ACPI, board files,
Simple Firmware and future hardware description languages...

Yours,
Linus Walleij

Re: [PATCH 00/21] On-demand device registration

[Alexander Holler]

Am 11.06.2015 um 10:12 schrieb Linus Walleij:
> On Wed, Jun 10, 2015 at 10:28 AM, Alexander Holler <holler@ahsoftware.de> wrote:
>> Am 10.06.2015 um 09:30 schrieb Linus Walleij:
>
>>> i2c host comes out, probes the regulator driver, regulator driver
>>> probes and then the regulator_get() call returns.
>>>
>>> This requires instrumentation on anything providing a resource
>>> to another driver like those I mentioned and a lot of overhead
>>> infrastructure, but I think it's the right approach. However I don't
>>> know if I would ever be able to pull that off myself, I know talk
>>> is cheap and I should show the code instead.
>>
>> You would end up with the same problem of deadlocks as currently, and you
>> would still need something ugly like the defered probe brutforce to avoid
>> them.
>
> Sorry I don't get that. Care to elaborate on why?

Because loading/initializing on demand doesn't give you any solved order 
of drivers to initialize. And it can't because it has no idea about the 
requirements of other drivers. The reason why it might work better in 
the case of the tegra is that it might give you another initialization 
order than the one which is currently choosen, which, by luck, might be 
a better one.

But maybe I missed something, I haven't looked at the patches at all. 
But just loading on demand, can't magically give you a working order of 
drivers to initialize. E.g. how do you choose the first driver to 
initialize?

Regards,

Alexander Holler

Re: [PATCH 00/21] On-demand device registration

[Alexander Holler]

Am 11.06.2015 um 12:17 schrieb Alexander Holler:
> Am 11.06.2015 um 10:12 schrieb Linus Walleij:
>> On Wed, Jun 10, 2015 at 10:28 AM, Alexander Holler
>> <holler@ahsoftware.de> wrote:
>>> Am 10.06.2015 um 09:30 schrieb Linus Walleij:
>>
>>>> i2c host comes out, probes the regulator driver, regulator driver
>>>> probes and then the regulator_get() call returns.
>>>>
>>>> This requires instrumentation on anything providing a resource
>>>> to another driver like those I mentioned and a lot of overhead
>>>> infrastructure, but I think it's the right approach. However I don't
>>>> know if I would ever be able to pull that off myself, I know talk
>>>> is cheap and I should show the code instead.
>>>
>>> You would end up with the same problem of deadlocks as currently, and
>>> you
>>> would still need something ugly like the defered probe brutforce to
>>> avoid
>>> them.
>>
>> Sorry I don't get that. Care to elaborate on why?
>
> Because loading/initializing on demand doesn't give you any solved order
> of drivers to initialize. And it can't because it has no idea about the
> requirements of other drivers. The reason why it might work better in
> the case of the tegra is that it might give you another initialization
> order than the one which is currently choosen, which, by luck, might be
> a better one.
>
> But maybe I missed something, I haven't looked at the patches at all.
> But just loading on demand, can't magically give you a working order of
> drivers to initialize. E.g. how do you choose the first driver to
> initialize?

Other problems you will run into are time constraints and multithreaded 
drivers.

E.g. we all should know how tricky it sometimes is to avoid deadlocks. 
And with loading on demand, you are extending this problem over the 
initialization of maybe a whole bunch of other drivers which might be 
started by calling one function of another driver. And a function call 
might need a very long time to finish during which an unpredictable 
amount of things may happen.

It would make me wonder if that will end up with a good, usable and as 
simple as possible solution.

Regards,

Alexander Holler

Re: [PATCH 00/21] On-demand device registration

[Alexander Holler]

Am 11.06.2015 um 13:24 schrieb Alexander Holler:
> Am 11.06.2015 um 12:17 schrieb Alexander Holler:
>> Am 11.06.2015 um 10:12 schrieb Linus Walleij:
>>> On Wed, Jun 10, 2015 at 10:28 AM, Alexander Holler
>>> <holler@ahsoftware.de> wrote:
>>>> Am 10.06.2015 um 09:30 schrieb Linus Walleij:
>>>
>>>>> i2c host comes out, probes the regulator driver, regulator driver
>>>>> probes and then the regulator_get() call returns.
>>>>>
>>>>> This requires instrumentation on anything providing a resource
>>>>> to another driver like those I mentioned and a lot of overhead
>>>>> infrastructure, but I think it's the right approach. However I don't
>>>>> know if I would ever be able to pull that off myself, I know talk
>>>>> is cheap and I should show the code instead.
>>>>
>>>> You would end up with the same problem of deadlocks as currently, and
>>>> you
>>>> would still need something ugly like the defered probe brutforce to
>>>> avoid
>>>> them.
>>>
>>> Sorry I don't get that. Care to elaborate on why?
>>
>> Because loading/initializing on demand doesn't give you any solved order
>> of drivers to initialize. And it can't because it has no idea about the
>> requirements of other drivers. The reason why it might work better in
>> the case of the tegra is that it might give you another initialization
>> order than the one which is currently choosen, which, by luck, might be
>> a better one.
>>
>> But maybe I missed something, I haven't looked at the patches at all.
>> But just loading on demand, can't magically give you a working order of
>> drivers to initialize. E.g. how do you choose the first driver to
>> initialize?
> 
> Other problems you will run into are time constraints and multithreaded
> drivers.
> 
> E.g. we all should know how tricky it sometimes is to avoid deadlocks.
> And with loading on demand, you are extending this problem over the
> initialization of maybe a whole bunch of other drivers which might be
> started by calling one function of another driver. And a function call
> might need a very long time to finish during which an unpredictable
> amount of things may happen.
> 
> It would make me wonder if that will end up with a good, usable and as
> simple as possible solution.

Besides that instrumenting every call to another driver in order to fix
a onetime operation (the initialization) sounds like an enormous
overhead. Initialization is done pnly once, regardless how long a system
runs, but the instrumentation to fix this onetime operation would slow
down the operation during the whole runtime of a system.

I don't think this is what should be done.

> 
> Regards,
> 
> Alexander Holler

Re: [PATCH 00/21] On-demand device registration

[Linus Walleij]

On Thu, Jun 11, 2015 at 12:17 PM, Alexander Holler <holler@ahsoftware.de> wrote:
> Am 11.06.2015 um 10:12 schrieb Linus Walleij:
>> On Wed, Jun 10, 2015 at 10:28 AM, Alexander Holler <holler@ahsoftware.de>
>> wrote:

>>> You would end up with the same problem of deadlocks as currently, and you
>>> would still need something ugly like the defered probe brutforce to avoid
>>> them.
>>
>>
>> Sorry I don't get that. Care to elaborate on why?
>
>
> Because loading/initializing on demand doesn't give you any solved order of
> drivers to initialize. And it can't because it has no idea about the
> requirements of other drivers. The reason why it might work better in the
> case of the tegra is that it might give you another initialization order
> than the one which is currently choosen, which, by luck, might be a better
> one.
>
> But maybe I missed something, I haven't looked at the patches at all. But
> just loading on demand, can't magically give you a working order of drivers
> to initialize. E.g. how do you choose the first driver to initialize?

So the current patch set introduces dependencies (just for device tree)
and Tomeu is working on a more generic dependency approach for
any HW description.

The first driver to initialize will be as usual the first one in the list for
that initlevel, then walking up the initilevels.

However if any driver runs into a resource roadblock it will postpone
and wait for dependencies to probe first.

Certainly it is possible to create deadlocks in this scenario, but the
scope is not to create an ubreakable system.

Yours,
Linus Walleij

Re: [PATCH 00/21] On-demand device registration

[Tomeu Vizoso]

On 06/11/2015 12:17 PM, Alexander Holler wrote:
> Am 11.06.2015 um 10:12 schrieb Linus Walleij:
>> On Wed, Jun 10, 2015 at 10:28 AM, Alexander Holler <holler@ahsoftware.de> wrote:
>>> Am 10.06.2015 um 09:30 schrieb Linus Walleij:
>>
>>>> i2c host comes out, probes the regulator driver, regulator driver
>>>> probes and then the regulator_get() call returns.
>>>>
>>>> This requires instrumentation on anything providing a resource
>>>> to another driver like those I mentioned and a lot of overhead
>>>> infrastructure, but I think it's the right approach. However I don't
>>>> know if I would ever be able to pull that off myself, I know talk
>>>> is cheap and I should show the code instead.
>>>
>>> You would end up with the same problem of deadlocks as currently, and you
>>> would still need something ugly like the defered probe brutforce to avoid
>>> them.
>>
>> Sorry I don't get that. Care to elaborate on why?
> 
> Because loading/initializing on demand doesn't give you any solved order 
> of drivers to initialize. And it can't because it has no idea about the 
> requirements of other drivers.

So, this is only about ordering device probing. All built-in drivers
have already registered themselves by when we start probing.

> The reason why it might work better in 
> the case of the tegra is that it might give you another initialization 
> order than the one which is currently choosen, which, by luck, might be 
> a better one.

Note that this series was also tested on iMX.6, Exynos and OMAP4.

> But maybe I missed something, I haven't looked at the patches at all. 

It's a really small patchset :)

  19 files changed, 130 insertions(+), 45 deletions(-)

Thanks,

Tomeu

> But just loading on demand, can't magically give you a working order of 
> drivers to initialize. E.g. how do you choose the first driver to 
> initialize?
> 
> Regards,
> 
> Alexander Holler
> 

Re: [PATCH 00/21] On-demand device registration

[Alexander Holler]

Am 11.06.2015 um 14:30 schrieb Linus Walleij:
> On Thu, Jun 11, 2015 at 12:17 PM, Alexander Holler <holler@ahsoftware.de> wrote:
>> Am 11.06.2015 um 10:12 schrieb Linus Walleij:
>>> On Wed, Jun 10, 2015 at 10:28 AM, Alexander Holler <holler@ahsoftware.de>
>>> wrote:
>
>>>> You would end up with the same problem of deadlocks as currently, and you
>>>> would still need something ugly like the defered probe brutforce to avoid
>>>> them.
>>>
>>>
>>> Sorry I don't get that. Care to elaborate on why?
>>
>>
>> Because loading/initializing on demand doesn't give you any solved order of
>> drivers to initialize. And it can't because it has no idea about the
>> requirements of other drivers. The reason why it might work better in the
>> case of the tegra is that it might give you another initialization order
>> than the one which is currently choosen, which, by luck, might be a better
>> one.
>>
>> But maybe I missed something, I haven't looked at the patches at all. But
>> just loading on demand, can't magically give you a working order of drivers
>> to initialize. E.g. how do you choose the first driver to initialize?
>
> So the current patch set introduces dependencies (just for device tree)
> and Tomeu is working on a more generic dependency approach for
> any HW description.
>
> The first driver to initialize will be as usual the first one in the list for
> that initlevel, then walking up the initilevels.
>
> However if any driver runs into a resource roadblock it will postpone
> and wait for dependencies to probe first.
>
> Certainly it is possible to create deadlocks in this scenario, but the
> scope is not to create an ubreakable system.

IAnd what happens if you run into a deadlock? Do you print "you've lost, 
try changing your kernel config" in some output hidden by a 
splash-screen? ;)

That sounds like the fun with duck typed languages where you have to 
test any and every possible screnario (something which is almost 
impossible) in order to not run into something unexpected.

Anyway, have fun, good luck.

Alexander Holler

Re: [PATCH 00/21] On-demand device registration

[Linus Walleij]

On Thu, Jun 11, 2015 at 6:40 PM, Alexander Holler <holler@ahsoftware.de> wrote:
> Am 11.06.2015 um 14:30 schrieb Linus Walleij:

>> Certainly it is possible to create deadlocks in this scenario, but the
>> scope is not to create an ubreakable system.
>
> IAnd what happens if you run into a deadlock? Do you print "you've lost, try
> changing your kernel config" in some output hidden by a splash-screen? ;)

Sorry it sounds like a blanket argument, the fact that there are
mutexes in the kernel makes it possible to deadlock, it doesn't
mean we don't use mutexes. Some programming problems are
just like such.

Yours,
Linus Walleij

Re: [PATCH 00/21] On-demand device registration

[Alexander Holler]

Am 12.06.2015 um 09:25 schrieb Linus Walleij:
> On Thu, Jun 11, 2015 at 6:40 PM, Alexander Holler <holler@ahsoftware.de> wrote:
>> Am 11.06.2015 um 14:30 schrieb Linus Walleij:
>
>>> Certainly it is possible to create deadlocks in this scenario, but the
>>> scope is not to create an ubreakable system.
>>
>> IAnd what happens if you run into a deadlock? Do you print "you've lost, try
>> changing your kernel config" in some output hidden by a splash-screen? ;)
>
> Sorry it sounds like a blanket argument, the fact that there are
> mutexes in the kernel makes it possible to deadlock, it doesn't
> mean we don't use mutexes. Some programming problems are
> just like such.

I'm not talking about specific deadlocks through mutexes. I'm talking 
about what happens when driver A needs driver B which needs driver A. 
How do you recognise and handle that with your instrumented on-demand 
device initialization? Such a circular dependency might happen by just 
adding a new fucntion call or by changing the kernel configuration. And 
with the on-demand stuff, the possibility that the developer introducing 
this new (maybe optional) call will never hit such a circular dependency 
is high. So you will end up with a never ending stream of problem 
reports whenever someone introduced such a circular dependecy without 
having noticed it.

And to come back to specific deadlocks, if you are extending function 
calls from something former simple to something which might initialize a 
whole bunch of drivers, needing maybe seconds, I wouldn't say this is a 
blanket argument, but a real thread.

Alexander Holler

Re: [PATCH 00/21] On-demand device registration

[Alexander Holler]

Am 12.06.2015 um 13:19 schrieb Alexander Holler:
> Am 12.06.2015 um 09:25 schrieb Linus Walleij:
>> On Thu, Jun 11, 2015 at 6:40 PM, Alexander Holler
>> <holler@ahsoftware.de> wrote:
>>> Am 11.06.2015 um 14:30 schrieb Linus Walleij:
>>
>>>> Certainly it is possible to create deadlocks in this scenario, but the
>>>> scope is not to create an ubreakable system.
>>>
>>> IAnd what happens if you run into a deadlock? Do you print "you've
>>> lost, try
>>> changing your kernel config" in some output hidden by a
>>> splash-screen? ;)
>>
>> Sorry it sounds like a blanket argument, the fact that there are
>> mutexes in the kernel makes it possible to deadlock, it doesn't
>> mean we don't use mutexes. Some programming problems are
>> just like such.
>
> I'm not talking about specific deadlocks through mutexes. I'm talking
> about what happens when driver A needs driver B which needs driver A.
> How do you recognise and handle that with your instrumented on-demand
> device initialization? Such a circular dependency might happen by just
> adding a new fucntion call or by changing the kernel configuration. And
> with the on-demand stuff, the possibility that the developer introducing
> this new (maybe optional) call will never hit such a circular dependency
> is high. So you will end up with a never ending stream of problem
> reports whenever someone introduced such a circular dependecy without
> having noticed it.
>
> And to come back to specific deadlocks, if you are extending function
> calls from something former simple to something which might initialize a
> whole bunch of drivers, needing maybe seconds, I wouldn't say this is a
> blanket argument, but a real thread.

Keep in mind, that the possibility that a function call ends up with 
initializing a whole bunch of other drivers, is not determined 
statically, but depends on the configuration and runtime behaviour of 
the actual system the on-demand stuff actually happens.

E.g. if driver A is faster one system that driver B, the whole bunch of 
drivers might become initialized by a call in driver A. But if driver B 
was faster on the developers system (or the system is configured to 
first init driver B), than the whole bunch of drivers might have become 
initialized by driver B on the developers system. Thus he never might 
have hit a possible problem when the whole bunch of drivers got 
initialized in driver A.

That means it isn't always a good idea to create dynamic systems (like 
on-demand device initialization), because it's very hard to foresee and 
correctly handle their runtime behaviour.

> Alexander Holler

Re: [PATCH 00/21] On-demand device registration

[Alexander Holler]

Am 12.06.2015 um 13:36 schrieb Alexander Holler:
> Am 12.06.2015 um 13:19 schrieb Alexander Holler:
>> Am 12.06.2015 um 09:25 schrieb Linus Walleij:
>>> On Thu, Jun 11, 2015 at 6:40 PM, Alexander Holler
>>> <holler@ahsoftware.de> wrote:
>>>> Am 11.06.2015 um 14:30 schrieb Linus Walleij:
>>>
>>>>> Certainly it is possible to create deadlocks in this scenario, but the
>>>>> scope is not to create an ubreakable system.
>>>>
>>>> IAnd what happens if you run into a deadlock? Do you print "you've
>>>> lost, try
>>>> changing your kernel config" in some output hidden by a
>>>> splash-screen? ;)
>>>
>>> Sorry it sounds like a blanket argument, the fact that there are
>>> mutexes in the kernel makes it possible to deadlock, it doesn't
>>> mean we don't use mutexes. Some programming problems are
>>> just like such.
>>
>> I'm not talking about specific deadlocks through mutexes. I'm talking
>> about what happens when driver A needs driver B which needs driver A.
>> How do you recognise and handle that with your instrumented on-demand
>> device initialization? Such a circular dependency might happen by just
>> adding a new fucntion call or by changing the kernel configuration. And
>> with the on-demand stuff, the possibility that the developer introducing
>> this new (maybe optional) call will never hit such a circular dependency
>> is high. So you will end up with a never ending stream of problem
>> reports whenever someone introduced such a circular dependecy without
>> having noticed it.
>>
>> And to come back to specific deadlocks, if you are extending function
>> calls from something former simple to something which might initialize a
>> whole bunch of drivers, needing maybe seconds, I wouldn't say this is a
>> blanket argument, but a real thread.
>
> Keep in mind, that the possibility that a function call ends up with
> initializing a whole bunch of other drivers, is not determined
> statically, but depends on the configuration and runtime behaviour of
> the actual system the on-demand stuff actually happens.
>
> E.g. if driver A is faster one system that driver B, the whole bunch of
> drivers might become initialized by a call in driver A. But if driver B
> was faster on the developers system (or the system is configured to
> first init driver B), than the whole bunch of drivers might have become
> initialized by driver B on the developers system. Thus he never might
> have hit a possible problem when the whole bunch of drivers got
> initialized in driver A.
>
> That means it isn't always a good idea to create dynamic systems (like
> on-demand device initialization), because it's very hard to foresee and
> correctly handle their runtime behaviour.

And because you've said that "problem space is a bit convoluted" and I 
disagree, here's a summary from my point of view:

1. All the necessary information (dependencies between drivers) already 
exists at compile time. The set of dependencies between drivers might 
become smaller by configuration, but will not become larger. So there 
should be NO need to collect them at runtime, e.g. by instrumenting 
function calls. I've described the problems I see with that above. I've 
choosen DT as source of dependencies because it offers an easy 
accessible and almost complete set of dependencies. I just had to add 
some type information to the dtb in order to identify the dependencies 
(phandles). But other ways to collect the dependencies would work too. 
Even the most simple way to add a static list of dependencies to each 
driver (which later on might be automated by some more clever stuff than 
adding them manually) would do the trick.

2. The problem to sort a set of nodes (drivers) with dependencies is 
solved since a long time and almost any developers uses it regularly in 
form of make. And everyone who used make -jN knows that the possible 
parallel initialization of drivers I've talked about, is already solved too.

3. In order to initialize the drivers in some specific order, their 
initcalls must be identified. I've offered a possible solution to that 
without much changes, but many other, even better ways, are possible 
too. It just depends on how much you want to change and on how much of 
these changes you will be able to feed into mainline kernel (which 
depends on your connections/relations inside the core kernel crew). E.g. 
instead of still just relying on one-dimensional arrays with (anonymous) 
pointers to initcalls, a multidimensional array of initcalls and 
drivername (and maybe more information) might be thinkable.

4. x86/amd64/ACPI-people, so most longtime and core kernel maintainers 
obviously don't have much interest until you've solved 1. in a way they 
can use too. So the necessary changes for 2. or 3. will have a big 
hurdle to take if 1. isn't solved usable for them too.

>> Alexander Holler

Re: [PATCH 00/21] On-demand device registration

[Linus Walleij]

On Sat, Jun 13, 2015 at 8:27 PM, Alexander Holler <holler@ahsoftware.de> wrote:

> And because you've said that "problem space is a bit convoluted" and I
> disagree, here's a summary from my point of view:
>
> 1. All the necessary information (dependencies between drivers) already
> exists at compile time. The set of dependencies between drivers might become
> smaller by configuration, but will not become larger. So there should be NO
> need to collect them at runtime, e.g. by instrumenting function calls.

I think you arrived at the core of the crux here.

When we look up a resource provided from another driver, e.g. from
regulator_get(), clk_get(), pinctrl_get(), gpiod_get() etc - the dependency
is resolved by looking in a cross-reference table for either a struct device*
pointer or a string, an index, or both or all three. Examples:

struct regulator *regulator_get(struct device *dev, const char *id);
struct clk *clk_get(struct device *dev, const char *id);
struct gpio_desc *__must_check __gpiod_get(struct device *dev,
                                         const char *con_id,
                                         enum gpiod_flags flags);
(...)

(*_index() variants exist on some of the resource retrieveal
functions.)

struct device * is the device requesting the resource, con_id
is the string name of the resource on the provider side. This is all
solved by looking in cross reference tables. ONE way of resolving
that cross reference is to look into the device tree or the ACPI table.
But for the board file case, this is resolved at runtime by the cross
reference table, registered with calls such as gpiod_add_lookup_table().

It is true that in the theoretical sense, all of this exist at compile time
especially if you can parse something like a device tree and
figure out what struct device * nodes will correspond to the struct
device_node:s in it. For ACPI I guess a similar procedure is viable.

Problem: this requires the kernel compile to know exactly *which* device tree
or ACPI table it is going to boot on. The expressed goal of device tree
and ACPI is to have *ONE* kernel booting several device trees.
Here your approach stops short: you are suggesting instrumenting
the kernel at compile time to one single device tree or ACPI table.
But we never know really what device tree or ACPI table will be used.
This just cannot be done at compile time for that reason alone.

Example: in boot case (A) the regulator may be provided by regulator
"foo" driver on an i2c bus. But in boot case (B) the very same regulator
may be provided by regulator "bar" on an SPI bus. These are very
real usecases, for example for drivers/net/ethernet/smsc/smsc911x.c,
will get regulators from the most diverse places depending on what
device tree is used.

For board files, it is neither possible in theory: you need to compile the
code to figure out the struct device * provider, and/or the string name
of the providing device (.name field in struct device for the provider)
to resolve dependencies at compile time.

For the board file case, resolving dependencies at compile time will
require a quite complex two-stage rocket: compile the code to
get resources out, then recompile with known resources.

I guess your suggested approach then need to introduce a special
build tool to order the initcalls accordingly.

Again this will fall short if you don't know at compile time exactly
*which* board file will be executed.

So the only practical way to solve this at compile time is to predict
an initcall ordering sequence for all possible boot paths, compile in
all of them, and choose the right one at boot. But the number of boot
paths is equal to the number of device trees / ACPI tables or
board files supported, and that space is uncontrolled and ordered
infinite.

Basically I think the root problem with your approach is that you
assume we know what hardware we will boot on at compile time. We
discarded that development path years ago. We have no clue, this
is resolved at runtime. Alas, people still create super-optimized
systems using exactly this knowledge, but it is not our main target
here, it is a special optimization case.

Yours,
Linus Walleij

Re: [PATCH 00/21] On-demand device registration

[Alexander Holler]

Am 15.06.2015 um 10:58 schrieb Linus Walleij:
> On Sat, Jun 13, 2015 at 8:27 PM, Alexander Holler <holler@ahsoftware.de> wrote:
>
>> And because you've said that "problem space is a bit convoluted" and I
>> disagree, here's a summary from my point of view:
>>
>> 1. All the necessary information (dependencies between drivers) already
>> exists at compile time. The set of dependencies between drivers might become
>> smaller by configuration, but will not become larger. So there should be NO
>> need to collect them at runtime, e.g. by instrumenting function calls.
>
> I think you arrived at the core of the crux here.

I've hoped so, that's why I've written it.

> I guess your suggested approach then need to introduce a special
> build tool to order the initcalls accordingly.
>
> Again this will fall short if you don't know at compile time exactly
> *which* board file will be executed.

I've just tried to describe the facts in order to make the problem space 
more clear, because, as said, I don't think it's convoluted.

Besides that, I didn't want to suggest anything else other than what 
I've already posted working patches for. What I've mentioned as possible 
other solutions above is stuff which might be possible too in order to 
give some starting points for people which are searching another 
solution. But I wouldn't have written my patches as they are, if I would 
think there is another more easier solution.

And of course, there is still a bit to resolve at runtime, even in the 
DT case (look at the "compatible" attribute). But there is already a 
runtime solution to find the right driver (in case of DT) and I haven't 
mentioned it in order to no confuse people again. Mentioning every 
little detail doesn't make sense if you want to describe something 
understandable (which is what I've tried).

> So the only practical way to solve this at compile time is to predict
> an initcall ordering sequence for all possible boot paths, compile in
> all of them, and choose the right one at boot. But the number of boot
> paths is equal to the number of device trees / ACPI tables or
> board files supported, and that space is uncontrolled and ordered
> infinite.

You just need one working ordered sequence which includes all options. 
This one will work for all others too.

> Basically I think the root problem with your approach is that you
> assume we know what hardware we will boot on at compile time. We

Totally wrong. If you assume that I assume this, than either I was 
totally unable to describe something clearly, or you were unable or 
unwilling to understand what I've written. And as the result is the 
same, we don't need to find out which was reason.

Anyway, have fun. I'm quitting the discussion here as I don't have any 
business with the kernel and already decided some time again to not post 
patches anymore as it seems to be a waste of my (and maybe others) time.

Regards,

Alexander Holler

Re: [PATCH 02/21] ARM: tegra: Add gpio-ranges property

[Tomeu Vizoso]

On 28 May 2015 at 17:50, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 05/28/2015 02:26 AM, Tomeu Vizoso wrote:
>>
>> On 27 May 2015 at 16:49, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>
>>> On 05/27/2015 08:18 AM, Tomeu Vizoso wrote:
>>>>
>>>>
>>>> On 26 May 2015 at 21:41, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>>>
>>>>>
>>>>> On 05/25/2015 08:53 AM, Tomeu Vizoso wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> Specify how the GPIOs map to the pins in T124, so the dependency is
>>>>>> explicit.
>>>>>>
>>>>>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>>>>>> ---
>>>>>>     arch/arm/boot/dts/tegra124.dtsi | 1 +
>>>>>>     1 file changed, 1 insertion(+)
>>>>>>
>>>>>> diff --git a/arch/arm/boot/dts/tegra124.dtsi
>>>>>> b/arch/arm/boot/dts/tegra124.dtsi
>>>>>> index 13cc7ca..5d1d35f 100644
>>>>>> --- a/arch/arm/boot/dts/tegra124.dtsi
>>>>>> +++ b/arch/arm/boot/dts/tegra124.dtsi
>>>>>> @@ -254,6 +254,7 @@
>>>>>>                   gpio-controller;
>>>>>>                   #interrupt-cells = <2>;
>>>>>>                   interrupt-controller;
>>>>>> +               gpio-ranges = <&pinmux 0 0 250>;
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> We should be consistent between SoCs. Why not make the same change for
>>>>> all
>>>>> Tegra SoCs?
>>>>>
>>>>> I think this change will cause the GPIO subsystem to call into the
>>>>> pinctrl
>>>>> subsystem and create/add/register a new GPIO<->pinctrl range structure.
>>>>> The
>>>>> pinctrl driver already does this, so I think we'll end up with two
>>>>> duplicate
>>>>> entries in the pinctrl device's gpio_ranges list. This probably won't
>>>>> cause
>>>>> a problem, but I wanted to make sure you'd thought about it to make
>>>>> sure.
>>>>
>>>>
>>>>
>>>> Actually, I have checked and see that gpio-tegra.c registers 256
>>>> gpios, but pinctrl-tegra124.c adds a range of only 251. I don't really
>>>> remember where I got the 250 value from, sorry :(
>>>>
>>>> I don't see how that would cause any concrete problems, but maybe we
>>>> should have a single authoritative source (not sure we can do so
>>>> without breaking DT ABI though).
>>>>
>>>>> Right now, I think we get lucky and pinctrl ends up probing first (or
>>>>> at
>>>>> least very early) anyway. Somewhat related to this series, I wonder if
>>>>> we
>>>>> shouldn't add pinctrl client properties to every node in the Tegra DT
>>>>> that
>>>>> describes a controller that makes use of external pins that are
>>>>> affected
>>>>> by
>>>>> the pinmux. Such a change would guarantee this desired probing order.
>>>>> In
>>>>> order to preserve the "program the entire pinmux at once" semantics,
>>>>> these
>>>>> new pinctrl client properties would all need to reference empty states,
>>>>> yet
>>>>> would still need to exist to represent the dependency.
>>>>
>>>>
>>>>
>>>> I think so, but aren't almost all those pins used as gpios? If so,
>>>> then such a controller's driver will request the gpio it wants which
>>>> will cause the gpio driver to be registered (and hopefully probed) if
>>>> needed, which in turn will check that the corresponding pinctrl device
>>>> has been registered. Or am I missing something?
>>>
>>>
>>>
>>> As you say this probably works out fine for pins that are used as GPIOs.
>>> I
>>> was thinking more about SFIOs. Take an I2C controller, which doesn't use
>>> any
>>> GPIOs itself. The pinctrl device should be probed before the I2C device,
>>> so
>>> that the I2C driver can initiate transactions on the I2C bus during its
>>> probe if it wanted to (or at least, clients could initiate transactions
>>> at
>>> any completely arbitrary time as soon as probe was complete).
>>
>>
>> What is using the SFIO in this case, the I2C master or the I2C client?
>
>
> The I2C controller a/k/a the I2C master.
>
>> In any case, is the problem you are referring to that ICs may rely on
>
>
> s/ICs/drivers/ I think.
>
>> a specific pinmux configuration but that's not currently reflected on
>> the DT because pinmux configuration happened so early that things just
>> worked?
>
>
> Yes; the dependency of some nodes on pinctrl isn't explicitly called out in
> the DT. It's probably not a good idea to have such implicit dependencies,
> although I suppose for something so central as pinmux, maybe it's not
> terrible, since almost everything depends on it and it's pretty obvious.

Yup, agreed.

Thanks,

Tomeu

Re: [PATCH 02/21] ARM: tegra: Add gpio-ranges property

[Tomeu Vizoso]

On 2 June 2015 at 17:40, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 06/02/2015 05:28 AM, Linus Walleij wrote:
>>
>> On Tue, May 26, 2015 at 9:41 PM, Stephen Warren <swarren@wwwdotorg.org>
>> wrote:
>>>
>>> On 05/25/2015 08:53 AM, Tomeu Vizoso wrote:
>>>>
>>>>
>>>> Specify how the GPIOs map to the pins in T124, so the dependency is
>>>> explicit.
>>>>
>>>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>>>> ---
>>>>    arch/arm/boot/dts/tegra124.dtsi | 1 +
>>>>    1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/tegra124.dtsi
>>>> b/arch/arm/boot/dts/tegra124.dtsi
>>>> index 13cc7ca..5d1d35f 100644
>>>> --- a/arch/arm/boot/dts/tegra124.dtsi
>>>> +++ b/arch/arm/boot/dts/tegra124.dtsi
>>>> @@ -254,6 +254,7 @@
>>>>                  gpio-controller;
>>>>                  #interrupt-cells = <2>;
>>>>                  interrupt-controller;
>>>> +               gpio-ranges = <&pinmux 0 0 250>;
>>>
>>>
>>>
>>> We should be consistent between SoCs. Why not make the same change for
>>> all
>>> Tegra SoCs?
>>
>>
>> Agreed.
>>
>>> I think this change will cause the GPIO subsystem to call into the
>>> pinctrl
>>> subsystem and create/add/register a new GPIO<->pinctrl range structure.
>>> The
>>> pinctrl driver already does this, so I think we'll end up with two
>>> duplicate
>>> entries in the pinctrl device's gpio_ranges list. This probably won't
>>> cause
>>> a problem, but I wanted to make sure you'd thought about it to make sure.
>>
>>
>> That sounds like duplication indeed, I would expect that first a patch
>> adds the ranges to the dts[i] files and then a second patch delete the
>> same ranges from the pinctrl driver then, if these shall come in from
>> the device tree.
>
>
> We can't delete the gpio-range-registration code from the Tegra pinmux
> driver, or old DTs won't work correctly. We could make it conditional based
> upon whether the DT contains the property or not.

I've been looking at this and haven't found a good solution. From what
I see, the pinctrl driver doesn't have a reference to the gpio device
node so cannot tell if it needs to add the range or not.

The gpio driver can tell whether it should add the range or not, but
if it has to because the gpio-ranges property isn't there, then it
doesn't have the reference to the pinctrl device to set the range to.

So, given that pinctrl_add_gpio_range is deprecated already, wonder if
the lesser evil isn't leaving the duplicated entries for now. On newer
SoC revisions such as T210 we can stop calling pinctrl_add_gpio_range
at all.

Or, we can accept that nobody is going to boot a newer kernel with an
older DT on the affected boards and just rely on the presence of the
gpio-ranges property :)

Regards,

Tomeu

Re: [PATCH 02/21] ARM: tegra: Add gpio-ranges property

[Stephen Warren]

On 06/16/2015 02:42 AM, Tomeu Vizoso wrote:
> On 2 June 2015 at 17:40, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 06/02/2015 05:28 AM, Linus Walleij wrote:
>>>
>>> On Tue, May 26, 2015 at 9:41 PM, Stephen Warren <swarren@wwwdotorg.org>
>>> wrote:
>>>>
>>>> On 05/25/2015 08:53 AM, Tomeu Vizoso wrote:
>>>>>
>>>>>
>>>>> Specify how the GPIOs map to the pins in T124, so the dependency is
>>>>> explicit.
>>>>>
>>>>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>>>>> ---
>>>>>     arch/arm/boot/dts/tegra124.dtsi | 1 +
>>>>>     1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/tegra124.dtsi
>>>>> b/arch/arm/boot/dts/tegra124.dtsi
>>>>> index 13cc7ca..5d1d35f 100644
>>>>> --- a/arch/arm/boot/dts/tegra124.dtsi
>>>>> +++ b/arch/arm/boot/dts/tegra124.dtsi
>>>>> @@ -254,6 +254,7 @@
>>>>>                   gpio-controller;
>>>>>                   #interrupt-cells = <2>;
>>>>>                   interrupt-controller;
>>>>> +               gpio-ranges = <&pinmux 0 0 250>;
>>>>
>>>>
>>>>
>>>> We should be consistent between SoCs. Why not make the same change for
>>>> all
>>>> Tegra SoCs?
>>>
>>>
>>> Agreed.
>>>
>>>> I think this change will cause the GPIO subsystem to call into the
>>>> pinctrl
>>>> subsystem and create/add/register a new GPIO<->pinctrl range structure.
>>>> The
>>>> pinctrl driver already does this, so I think we'll end up with two
>>>> duplicate
>>>> entries in the pinctrl device's gpio_ranges list. This probably won't
>>>> cause
>>>> a problem, but I wanted to make sure you'd thought about it to make sure.
>>>
>>>
>>> That sounds like duplication indeed, I would expect that first a patch
>>> adds the ranges to the dts[i] files and then a second patch delete the
>>> same ranges from the pinctrl driver then, if these shall come in from
>>> the device tree.
>>
>>
>> We can't delete the gpio-range-registration code from the Tegra pinmux
>> driver, or old DTs won't work correctly. We could make it conditional based
>> upon whether the DT contains the property or not.
>
> I've been looking at this and haven't found a good solution. From what
> I see, the pinctrl driver doesn't have a reference to the gpio device
> node so cannot tell if it needs to add the range or not.

Well, we know what the node must be called, so the pinctrl driver could 
search for it by name.

> The gpio driver can tell whether it should add the range or not, but
> if it has to because the gpio-ranges property isn't there, then it
> doesn't have the reference to the pinctrl device to set the range to.
>
> So, given that pinctrl_add_gpio_range is deprecated already, wonder if
> the lesser evil isn't leaving the duplicated entries for now. On newer
> SoC revisions such as T210 we can stop calling pinctrl_add_gpio_range
> at all.
>
> Or, we can accept that nobody is going to boot a newer kernel with an
> older DT on the affected boards and just rely on the presence of the
> gpio-ranges property :)

Isn't the simplest solution to just leave it as it is? Nothing's broken 
is it?

For any new SoCs we add, we can certainly switch to a new scheme if we 
want, but we need to catch/implement that early before the base .dtsi 
file is included in its first kernel release.

Re: [PATCH 02/21] ARM: tegra: Add gpio-ranges property

[Tomeu Vizoso]

On 16 June 2015 at 22:32, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 06/16/2015 02:42 AM, Tomeu Vizoso wrote:
>>
>> On 2 June 2015 at 17:40, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>
>>> On 06/02/2015 05:28 AM, Linus Walleij wrote:
>>>>
>>>>
>>>> On Tue, May 26, 2015 at 9:41 PM, Stephen Warren <swarren@wwwdotorg.org>
>>>> wrote:
>>>>>
>>>>>
>>>>> On 05/25/2015 08:53 AM, Tomeu Vizoso wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> Specify how the GPIOs map to the pins in T124, so the dependency is
>>>>>> explicit.
>>>>>>
>>>>>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>>>>>> ---
>>>>>>     arch/arm/boot/dts/tegra124.dtsi | 1 +
>>>>>>     1 file changed, 1 insertion(+)
>>>>>>
>>>>>> diff --git a/arch/arm/boot/dts/tegra124.dtsi
>>>>>> b/arch/arm/boot/dts/tegra124.dtsi
>>>>>> index 13cc7ca..5d1d35f 100644
>>>>>> --- a/arch/arm/boot/dts/tegra124.dtsi
>>>>>> +++ b/arch/arm/boot/dts/tegra124.dtsi
>>>>>> @@ -254,6 +254,7 @@
>>>>>>                   gpio-controller;
>>>>>>                   #interrupt-cells = <2>;
>>>>>>                   interrupt-controller;
>>>>>> +               gpio-ranges = <&pinmux 0 0 250>;
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> We should be consistent between SoCs. Why not make the same change for
>>>>> all
>>>>> Tegra SoCs?
>>>>
>>>>
>>>>
>>>> Agreed.
>>>>
>>>>> I think this change will cause the GPIO subsystem to call into the
>>>>> pinctrl
>>>>> subsystem and create/add/register a new GPIO<->pinctrl range structure.
>>>>> The
>>>>> pinctrl driver already does this, so I think we'll end up with two
>>>>> duplicate
>>>>> entries in the pinctrl device's gpio_ranges list. This probably won't
>>>>> cause
>>>>> a problem, but I wanted to make sure you'd thought about it to make
>>>>> sure.
>>>>
>>>>
>>>>
>>>> That sounds like duplication indeed, I would expect that first a patch
>>>> adds the ranges to the dts[i] files and then a second patch delete the
>>>> same ranges from the pinctrl driver then, if these shall come in from
>>>> the device tree.
>>>
>>>
>>>
>>> We can't delete the gpio-range-registration code from the Tegra pinmux
>>> driver, or old DTs won't work correctly. We could make it conditional
>>> based
>>> upon whether the DT contains the property or not.
>>
>>
>> I've been looking at this and haven't found a good solution. From what
>> I see, the pinctrl driver doesn't have a reference to the gpio device
>> node so cannot tell if it needs to add the range or not.
>
>
> Well, we know what the node must be called, so the pinctrl driver could
> search for it by name.

Ok, will write something in this direction.

>> The gpio driver can tell whether it should add the range or not, but
>> if it has to because the gpio-ranges property isn't there, then it
>> doesn't have the reference to the pinctrl device to set the range to.
>>
>> So, given that pinctrl_add_gpio_range is deprecated already, wonder if
>> the lesser evil isn't leaving the duplicated entries for now. On newer
>> SoC revisions such as T210 we can stop calling pinctrl_add_gpio_range
>> at all.
>>
>> Or, we can accept that nobody is going to boot a newer kernel with an
>> older DT on the affected boards and just rely on the presence of the
>> gpio-ranges property :)
>
>
> Isn't the simplest solution to just leave it as it is? Nothing's broken is
> it?

Well, without an explicit dependency from the gpio node to the pinctrl
node I cannot avoid a deferred probe (which will cause other deferred
probes in turn).

> For any new SoCs we add, we can certainly switch to a new scheme if we want,
> but we need to catch/implement that early before the base .dtsi file is
> included in its first kernel release.

Definitely.

Regards,

Tomeu

> --
> 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 00/21] On-demand device registration

[Tomeu Vizoso]

On 28 May 2015 at 06:33, Rob Herring <robherring2@gmail.com> wrote:
> On Mon, May 25, 2015 at 9:53 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.
>>
>> 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 registering devices as they are referenced by other
>> devices.
>
> I like the concept and novel approach.
>
>> This basically reuses the information that is already implicit in the probe()
>> implementations, saving us from refactoring existing drivers or adding
>> information to DTBs.
>>
>> Something I'm not completely happy with is that I have had to move the call to
>> of_platform_populate after all platform drivers have been registered.
>> Otherwise I don't see how I could register drivers on demand as we don't have
>> yet each driver's compatible strings.
>
> Yeah, this is the opposite of what we'd really like.

Can you elaborate on the reasons why we would like to have devices
registered before built-in drivers finish registering, even if we
don't probe them yet?

> Ideally, we would
> have a solution that works for modules too. However, we're no worse
> off. We pretty much build-in dependencies to avoid module ordering
> problems.

Nod, I haven't looked yet at requesting modules on-demand, but I guess
it should be doable. Modules that have dependencies described in the
firmware should get them probed automatically already though.

> Perhaps we need to make the probing on-demand rather than simply on
> device<->driver match occurring.

I'm afraid that too much old code depends on that. For example, Rafael
pointed out to the PNP subsystem, which registers a driver that will
probe devices with the EISA ID PNP0c02 to reserve memory regions for
devices that will be probed later.

http://lxr.free-electrons.com/source/drivers/pnp/system.c

My understanding is that probing of PNP0c02 devices must happen before
the actual devices that depend on those regions are probed, so if we
decoupled the probing from the driver/device registration, we would be
breaking that assumption.

>> For machs that don't move of_platform_populate() to a later point, these
>> patches shouldn't cause any problems but it's not guaranteed that we'll avoid
>> all the deferred probes as some drivers may not be registered yet.
>
> Ideally, of_platform_populate is not explicitly called by each
> platform. So I think we need to make this work for the default case.

The problem is that some platforms will need fixing because some
initcalls assume that some devices will have been registered already,
or even probed. I think removing those assumptions shouldn't be
problematic because I haven't had much trouble with this on the four
platforms I have tested with, but I cannot test every board that is
supported upstream.

I can ask though the KernelCI folks to boot my branch in all their
boards and make sure that those work when of_platform_populate is
called in late_initcall.

http://kernelci.org/boot/all/job/next/kernel/next-20150619/

>> I have tested this on boards with Tegra, iMX.6 and Exynos SoCs, and these
>> patches were enough to eliminate all the deferred probes.
>>
>> With this series I get the kernel to output to the panel in 0.5s, instead of 2.8s.
>
> That's certainly compelling.

Have to say that those numbers are with the serial console enabled
(without, it's 0.5s vs 1.5s), but on machines that take longer to boot
we should see bigger gains because we won't be sending devices to the
end of the queue when their probe is deferred.

Regards,

Tomeu

> Rob
>
>>
>> Regards,
>>
>> Tomeu
>>
>> [0] http://lists.freedesktop.org/archives/dri-devel/2014-August/066527.html
>>
>> [1] https://lkml.org/lkml/2014/5/12/452
>>
>> Tomeu Vizoso (21):
>>   regulator: core: Reduce critical area in _regulator_get
>>   ARM: tegra: Add gpio-ranges property
>>   ARM: tegra: Register drivers before devices
>>   ARM: EXYNOS: Register drivers before devices
>>   ARM i.MX6q: Register drivers before devices
>>   of/platform: Add of_platform_device_ensure()
>>   of/platform: Ensure device registration on lookup
>>   gpio: Probe GPIO drivers on demand
>>   gpio: Probe pinctrl devices on demand
>>   regulator: core: Probe regulators on demand
>>   drm: Probe panels on demand
>>   drm/tegra: Probe dpaux devices on demand
>>   i2c: core: Probe i2c master 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
>>
>>  arch/arm/boot/dts/tegra124.dtsi     |  1 +
>>  arch/arm/mach-exynos/exynos.c       |  4 +--
>>  arch/arm/mach-imx/mach-imx6q.c      | 12 ++++-----
>>  arch/arm/mach-tegra/tegra.c         | 21 ++++++---------
>>  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              |  3 +++
>>  drivers/of/platform.c               | 53 +++++++++++++++++++++++++++++++++++++
>>  drivers/phy/phy-core.c              |  3 +++
>>  drivers/pinctrl/devicetree.c        |  2 ++
>>  drivers/power/power_supply_core.c   |  3 +++
>>  drivers/pwm/core.c                  |  3 +++
>>  drivers/regulator/core.c            | 45 +++++++++++++++----------------
>>  drivers/usb/phy/phy.c               |  3 +++
>>  drivers/video/backlight/backlight.c |  3 +++
>>  include/linux/of_platform.h         |  2 ++
>>  19 files changed, 130 insertions(+), 45 deletions(-)
>>
>> --
>> 2.4.1
>>
> --
> 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/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 00/21] On-demand device registration

[Rob Herring]

On Mon, Jun 22, 2015 at 10:23 AM, Tomeu Vizoso
<tomeu.vizoso@collabora.com> wrote:
> On 28 May 2015 at 06:33, Rob Herring <robherring2@gmail.com> wrote:
>> On Mon, May 25, 2015 at 9:53 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.
>>>
>>> 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 registering devices as they are referenced by other
>>> devices.
>>
>> I like the concept and novel approach.
>>
>>> This basically reuses the information that is already implicit in the probe()
>>> implementations, saving us from refactoring existing drivers or adding
>>> information to DTBs.
>>>
>>> Something I'm not completely happy with is that I have had to move the call to
>>> of_platform_populate after all platform drivers have been registered.
>>> Otherwise I don't see how I could register drivers on demand as we don't have
>>> yet each driver's compatible strings.
>>
>> Yeah, this is the opposite of what we'd really like.
>
> Can you elaborate on the reasons why we would like to have devices
> registered before built-in drivers finish registering, even if we
> don't probe them yet?

My main thought was for modules we will almost always have devices
appearing first. More generally, we can have devices and drivers
coming or going at any point in time and should not put restrictions
on ordering.

Also, I think all the probe ordering and dependency tracking should be
done within the driver core (i.e. dependencies are a list of struct
devices). At some level it has to become firmware specific, but we
want to minimize that part. I could be convinced otherwise and you
have put more thought into this problem than I have.

>> Ideally, we would
>> have a solution that works for modules too. However, we're no worse
>> off. We pretty much build-in dependencies to avoid module ordering
>> problems.
>
> Nod, I haven't looked yet at requesting modules on-demand, but I guess
> it should be doable. Modules that have dependencies described in the
> firmware should get them probed automatically already though.
>
>> Perhaps we need to make the probing on-demand rather than simply on
>> device<->driver match occurring.
>
> I'm afraid that too much old code depends on that. For example, Rafael
> pointed out to the PNP subsystem, which registers a driver that will
> probe devices with the EISA ID PNP0c02 to reserve memory regions for
> devices that will be probed later.
>
> http://lxr.free-electrons.com/source/drivers/pnp/system.c
>
> My understanding is that probing of PNP0c02 devices must happen before
> the actual devices that depend on those regions are probed, so if we
> decoupled the probing from the driver/device registration, we would be
> breaking that assumption.

That shouldn't matter as PNP matching is PNP specific. We already have
different ways of matching with device/driver name or of_match_table
for example. Changing how and when OF matching occurs would not affect
PNP matching. We do matching on device and driver add currently. For
the "when" part, we would need to add what I'll call async matching or
deferred matching which in addition to matching on the of_match_table
also matches on the dependency list having probed. Your last series
essentially does this, but the difference is yours is not OF specific
and I think it needs to be. I mean it is OF specific only in the
aspect that matching already is. From a driver and subsystem
standpoint, it should not be OF specific much like deferred probe is
not OF specific, but in reality only occurs (currently) on OF probed
drivers.

>>> For machs that don't move of_platform_populate() to a later point, these
>>> patches shouldn't cause any problems but it's not guaranteed that we'll avoid
>>> all the deferred probes as some drivers may not be registered yet.
>>
>> Ideally, of_platform_populate is not explicitly called by each
>> platform. So I think we need to make this work for the default case.
>
> The problem is that some platforms will need fixing because some
> initcalls assume that some devices will have been registered already,
> or even probed. I think removing those assumptions shouldn't be
> problematic because I haven't had much trouble with this on the four
> platforms I have tested with, but I cannot test every board that is
> supported upstream.
>
> I can ask though the KernelCI folks to boot my branch in all their
> boards and make sure that those work when of_platform_populate is
> called in late_initcall.

I'd imagine Kevin would be happy to. That is still a subset of h/w, so
we'd need a way to disable any solution.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/