Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss
[PATCH v4 04/22] of: add function to allow probing a device from a OF node
253 views
Skip to first unread message
Tomeu Vizoso
Sep 7, 2015, 8:30:07 AM9/7/15
to
Walks the OF tree up and finds the closest ancestor that has a struct
device associated with it, probing it if isn't bound to a driver yet.
The above should ensure that the dependency represented by the passed OF
node is available, because probing a device should cause its descendants
to be probed as well (when they get registered).
Subsystems can use this when looking up resources for drivers, to reduce
the chances of deferred probes because of the probing order of devices.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
---
Changes in v4:
- Rename of_platform_probe to of_device_probe
- Use device_node.device instead of device_node.platform_dev
Changes in v3:
- Set and use device_node.platform_dev instead of reversing the logic to
find the platform device that encloses a device node.
- Drop the fwnode API to probe firmware nodes and add OF-only API for
now. I think this same scheme could be used for machines with ACPI,
but I haven't been able to find one that had to defer its probes because
of the device probe order.
Changes in v2: None
drivers/of/device.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++
drivers/of/platform.c | 2 ++
include/linux/of_device.h | 3 +++
3 files changed, 63 insertions(+)
diff --git a/drivers/of/device.c b/drivers/of/device.c
index 8b91ea241b10..c32ac7b6fbe2 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -286,3 +286,61 @@ int of_device_uevent_modalias(struct device *dev, struct kobj_uevent_env *env)
return 0;
}
+
+/**
+ * of_device_probe() - Probe device associated with OF node
+ * @np: node to probe
+ *
+ * Probe the device associated with the passed device node.
+ */
+void of_device_probe(struct device_node *np)
+{
+ struct device_node *target;
+ struct device *dev = NULL;
+
+ if (!of_root || !of_node_check_flag(of_root, OF_POPULATED_BUS))
+ return;
+
+ if (!np)
+ return;
+
+ of_node_get(np);
+
+ /* Find the closest ancestor that has a device associated */
+ for (target = np;
+ !of_node_is_root(target);
+ target = of_get_next_parent(target))
+ if (target->device) {
+ dev = target->device;
+ break;
+ }
+
+ of_node_put(target);
+
+ if (!dev) {
+ pr_warn("Couldn't find a device for node '%s'\n",
+ of_node_full_name(np));
+ return;
+ }
+
+ /*
+ * Device is bound or is being probed right now. If we have bad luck
+ * and the dependency isn't ready when it's needed, deferred probe
+ * will save us.
+ */
+ if (dev->driver)
+ return;
+
+ /*
+ * Probing a device should cause its descendants to be probed as
+ * well, which includes the passed device node.
+ */
+ if (device_attach(dev) != 1)
+ /*
+ * This cannot be a warning for now because clock nodes have a
+ * compatible string but the clock framework doesn't follow
+ * the device/driver model yet.
+ */
+ dev_dbg(dev, "Probe failed for %s\n", of_node_full_name(np));
+}
+EXPORT_SYMBOL_GPL(of_device_probe);
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index baf04d7249bd..f089d95ac961 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -269,6 +269,8 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
goto err_free;
}
+ node->device = &dev->dev;
+
return dev;
err_free:
diff --git a/include/linux/of_device.h b/include/linux/of_device.h
index cc7dd687a89d..da8d489e73ad 100644
--- a/include/linux/of_device.h
+++ b/include/linux/of_device.h
@@ -40,6 +40,7 @@ extern ssize_t of_device_get_modalias(struct device *dev,
extern void of_device_uevent(struct device *dev, struct kobj_uevent_env *env);
extern int of_device_uevent_modalias(struct device *dev, struct kobj_uevent_env *env);
+extern void of_device_probe(struct device_node *np);
static inline void of_device_node_put(struct device *dev)
{
@@ -84,6 +85,8 @@ static inline int of_device_uevent_modalias(struct device *dev,
return -ENODEV;
}
+static inline void of_device_probe(struct device_node *np) { }
+
static inline void of_device_node_put(struct device *dev) { }
static inline const struct of_device_id *__of_match_device(
--
2.4.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
device associated with it, probing it if isn't bound to a driver yet.
The above should ensure that the dependency represented by the passed OF
node is available, because probing a device should cause its descendants
to be probed as well (when they get registered).
Subsystems can use this when looking up resources for drivers, to reduce
the chances of deferred probes because of the probing order of devices.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
---
Changes in v4:
- Rename of_platform_probe to of_device_probe
- Use device_node.device instead of device_node.platform_dev
Changes in v3:
- Set and use device_node.platform_dev instead of reversing the logic to
find the platform device that encloses a device node.
- Drop the fwnode API to probe firmware nodes and add OF-only API for
now. I think this same scheme could be used for machines with ACPI,
but I haven't been able to find one that had to defer its probes because
of the device probe order.
Changes in v2: None
drivers/of/device.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++
drivers/of/platform.c | 2 ++
include/linux/of_device.h | 3 +++
3 files changed, 63 insertions(+)
diff --git a/drivers/of/device.c b/drivers/of/device.c
index 8b91ea241b10..c32ac7b6fbe2 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -286,3 +286,61 @@ int of_device_uevent_modalias(struct device *dev, struct kobj_uevent_env *env)
return 0;
}
+
+/**
+ * of_device_probe() - Probe device associated with OF node
+ * @np: node to probe
+ *
+ * Probe the device associated with the passed device node.
+ */
+void of_device_probe(struct device_node *np)
+{
+ struct device_node *target;
+ struct device *dev = NULL;
+
+ if (!of_root || !of_node_check_flag(of_root, OF_POPULATED_BUS))
+ return;
+
+ if (!np)
+ return;
+
+ of_node_get(np);
+
+ /* Find the closest ancestor that has a device associated */
+ for (target = np;
+ !of_node_is_root(target);
+ target = of_get_next_parent(target))
+ if (target->device) {
+ dev = target->device;
+ break;
+ }
+
+ of_node_put(target);
+
+ if (!dev) {
+ pr_warn("Couldn't find a device for node '%s'\n",
+ of_node_full_name(np));
+ return;
+ }
+
+ /*
+ * Device is bound or is being probed right now. If we have bad luck
+ * and the dependency isn't ready when it's needed, deferred probe
+ * will save us.
+ */
+ if (dev->driver)
+ return;
+
+ /*
+ * Probing a device should cause its descendants to be probed as
+ * well, which includes the passed device node.
+ */
+ if (device_attach(dev) != 1)
+ /*
+ * This cannot be a warning for now because clock nodes have a
+ * compatible string but the clock framework doesn't follow
+ * the device/driver model yet.
+ */
+ dev_dbg(dev, "Probe failed for %s\n", of_node_full_name(np));
+}
+EXPORT_SYMBOL_GPL(of_device_probe);
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index baf04d7249bd..f089d95ac961 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -269,6 +269,8 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
goto err_free;
}
+ node->device = &dev->dev;
+
return dev;
err_free:
diff --git a/include/linux/of_device.h b/include/linux/of_device.h
index cc7dd687a89d..da8d489e73ad 100644
--- a/include/linux/of_device.h
+++ b/include/linux/of_device.h
@@ -40,6 +40,7 @@ extern ssize_t of_device_get_modalias(struct device *dev,
extern void of_device_uevent(struct device *dev, struct kobj_uevent_env *env);
extern int of_device_uevent_modalias(struct device *dev, struct kobj_uevent_env *env);
+extern void of_device_probe(struct device_node *np);
static inline void of_device_node_put(struct device *dev)
{
@@ -84,6 +85,8 @@ static inline int of_device_uevent_modalias(struct device *dev,
return -ENODEV;
}
+static inline void of_device_probe(struct device_node *np) { }
+
static inline void of_device_node_put(struct device *dev) { }
static inline const struct of_device_id *__of_match_device(
--
2.4.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Tomeu Vizoso
Sep 7, 2015, 8:30:08 AM9/7/15
to
When looking up a DMA controller through its OF node, probe it if it
hasn't already.
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Changes in v4: None
Changes in v3: None
Changes in v2: None
drivers/dma/of-dma.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c
index 1e1f2986eba8..e899832f7df3 100644
--- a/drivers/dma/of-dma.c
+++ b/drivers/dma/of-dma.c
@@ -16,6 +16,7 @@
#include <linux/mutex.h>
#include <linux/slab.h>
#include <linux/of.h>
+#include <linux/of_device.h>
#include <linux/of_dma.h>
static LIST_HEAD(of_dma_list);
@@ -263,6 +264,8 @@ struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
if (of_dma_match_channel(np, name, i, &dma_spec))
continue;
+ of_device_probe(dma_spec.np);
+
mutex_lock(&of_dma_lock);
ofdma = of_dma_find_controller(&dma_spec);
hasn't already.
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Changes in v4: None
Changes in v3: None
Changes in v2: None
drivers/dma/of-dma.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c
index 1e1f2986eba8..e899832f7df3 100644
--- a/drivers/dma/of-dma.c
+++ b/drivers/dma/of-dma.c
@@ -16,6 +16,7 @@
#include <linux/mutex.h>
#include <linux/slab.h>
#include <linux/of.h>
+#include <linux/of_device.h>
#include <linux/of_dma.h>
static LIST_HEAD(of_dma_list);
@@ -263,6 +264,8 @@ struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
if (of_dma_match_channel(np, name, i, &dma_spec))
continue;
+ of_device_probe(dma_spec.np);
+
mutex_lock(&of_dma_lock);
ofdma = of_dma_find_controller(&dma_spec);
Tomeu Vizoso
Sep 7, 2015, 8:30:08 AM9/7/15
to
When looking up a phy provider through its OF node, probe it if it
index fc48fac003a6..94e90031d7f3 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -18,6 +18,7 @@
#include <linux/device.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_device_probe(args.np);
+
mutex_lock(&phy_provider_mutex);
phy_provider = of_phy_provider_lookup(args.np);
if (IS_ERR(phy_provider) || !try_module_get(phy_provider->owner)) {
hasn't already.
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
---
Changes in v4: None
Changes in v3: None
Changes in v2: None
drivers/phy/phy-core.c | 3 +++
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
---
Changes in v4: None
Changes in v3: None
Changes in v2: None
1 file changed, 3 insertions(+)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index fc48fac003a6..94e90031d7f3 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_device.h>
#include <linux/phy/phy.h>
#include <linux/of.h>
+#include <linux/of_device.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_device_probe(args.np);
+
mutex_lock(&phy_provider_mutex);
phy_provider = of_phy_provider_lookup(args.np);
if (IS_ERR(phy_provider) || !try_module_get(phy_provider->owner)) {
Tomeu Vizoso
Sep 7, 2015, 8:30:08 AM9/7/15
to
When looking up a regulator through its OF node, probe it if it hasn't
1 file changed, 2 insertions(+)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index f4aa6cae76d5..615133f45c76 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_device.h>
#include <linux/regmap.h>
#include <linux/regulator/of_regulator.h>
#include <linux/regulator/consumer.h>
@@ -1340,6 +1341,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_device_probe(node);
mutex_lock(®ulator_list_mutex);
list_for_each_entry(r, ®ulator_list, list)
if (r->dev.parent &&
already.
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
---
Changes in v4: None
Changes in v3: None
Changes in v2: None
drivers/regulator/core.c | 2 ++
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
---
Changes in v4: None
Changes in v3: None
Changes in v2: None
1 file changed, 2 insertions(+)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index f4aa6cae76d5..615133f45c76 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_device.h>
#include <linux/regmap.h>
#include <linux/regulator/of_regulator.h>
#include <linux/regulator/consumer.h>
@@ -1340,6 +1341,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_device_probe(node);
mutex_lock(®ulator_list_mutex);
list_for_each_entry(r, ®ulator_list, list)
if (r->dev.parent &&
Tomeu Vizoso
Sep 7, 2015, 8:30:08 AM9/7/15
to
When looking up a gpiochip through its firmware node, probe it if it
index fa6e3c8823d6..9a439dab7a87 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -19,6 +19,7 @@
#include <linux/gpio/consumer.h>
#include <linux/of.h>
#include <linux/of_address.h>
+#include <linux/of_device.h>
#include <linux/of_gpio.h>
#include <linux/pinctrl/pinctrl.h>
#include <linux/slab.h>
@@ -95,6 +96,8 @@ struct gpio_desc *of_get_named_gpiod_flags(struct device_node *np,
return ERR_PTR(ret);
}
+ of_device_probe(gg_data.gpiospec.np);
+
gpiochip_find(&gg_data, of_gpiochip_find_and_xlate);
of_node_put(gg_data.gpiospec.np);
hasn't already.
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
---
Changes in v4: None
Changes in v3: None
Changes in v2: None
drivers/gpio/gpiolib-of.c | 3 +++
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
---
Changes in v4: None
Changes in v3: None
Changes in v2: None
1 file changed, 3 insertions(+)
diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index fa6e3c8823d6..9a439dab7a87 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -19,6 +19,7 @@
#include <linux/gpio/consumer.h>
#include <linux/of.h>
#include <linux/of_address.h>
+#include <linux/of_device.h>
#include <linux/of_gpio.h>
#include <linux/pinctrl/pinctrl.h>
#include <linux/slab.h>
@@ -95,6 +96,8 @@ struct gpio_desc *of_get_named_gpiod_flags(struct device_node *np,
return ERR_PTR(ret);
}
+ of_device_probe(gg_data.gpiospec.np);
+
gpiochip_find(&gg_data, of_gpiochip_find_and_xlate);
of_node_put(gg_data.gpiospec.np);
Tomeu Vizoso
Sep 7, 2015, 8:30:08 AM9/7/15
to
When looking up a power supply through its OF node, probe it if it
index 456987c88baa..80bc89f4ae89 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -19,6 +19,7 @@
#include <linux/err.h>
#include <linux/power_supply.h>
#include <linux/thermal.h>
+#include <linux/of_device.h>
#include "power_supply.h"
/* exported for the APM Power driver, APM emulation */
@@ -206,6 +207,8 @@ static int power_supply_find_supply_from_node(struct device_node *supply_node)
{
int error;
+ of_device_probe(supply_node);
+
/*
* class_for_each_device() either returns its own errors or values
* returned by __power_supply_find_supply_from_node().
hasn't already.
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
---
Changes in v4: None
Changes in v3: None
Changes in v2: None
drivers/power/power_supply_core.c | 3 +++
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
---
Changes in v4: None
Changes in v3: None
Changes in v2: None
1 file changed, 3 insertions(+)
diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index 456987c88baa..80bc89f4ae89 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -19,6 +19,7 @@
#include <linux/err.h>
#include <linux/power_supply.h>
#include <linux/thermal.h>
+#include <linux/of_device.h>
#include "power_supply.h"
/* exported for the APM Power driver, APM emulation */
@@ -206,6 +207,8 @@ static int power_supply_find_supply_from_node(struct device_node *supply_node)
{
int error;
+ of_device_probe(supply_node);
+
/*
* class_for_each_device() either returns its own errors or values
* returned by __power_supply_find_supply_from_node().
Tomeu Vizoso
Sep 7, 2015, 8:30:09 AM9/7/15
to
When looking up a clock through its OF node, probe it if it hasn't
index 43e2c3ad6c31..e5fe02a11c36 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -19,6 +19,7 @@
#include <linux/list.h>
#include <linux/slab.h>
#include <linux/of.h>
+#include <linux/of_device.h>
#include <linux/device.h>
#include <linux/init.h>
#include <linux/sched.h>
@@ -3004,6 +3005,8 @@ struct clk *__of_clk_get_from_provider(struct of_phandle_args *clkspec,
if (!clkspec)
return ERR_PTR(-EINVAL);
+ of_device_probe(clkspec->np);
+
/* Check if we have such a provider in our array */
mutex_lock(&of_clk_mutex);
list_for_each_entry(provider, &of_clk_providers, link) {
already.
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
---
Changes in v4: None
Changes in v3: None
Changes in v2: None
drivers/clk/clk.c | 3 +++
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
---
Changes in v4: None
Changes in v3: None
Changes in v2: None
1 file changed, 3 insertions(+)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 43e2c3ad6c31..e5fe02a11c36 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -19,6 +19,7 @@
#include <linux/list.h>
#include <linux/slab.h>
#include <linux/of.h>
+#include <linux/of_device.h>
#include <linux/device.h>
#include <linux/init.h>
#include <linux/sched.h>
@@ -3004,6 +3005,8 @@ struct clk *__of_clk_get_from_provider(struct of_phandle_args *clkspec,
if (!clkspec)
return ERR_PTR(-EINVAL);
+ of_device_probe(clkspec->np);
+
/* Check if we have such a provider in our array */
mutex_lock(&of_clk_mutex);
list_for_each_entry(provider, &of_clk_providers, link) {
Tomeu Vizoso
Sep 7, 2015, 8:30:09 AM9/7/15
to
Some initcalls in the late level assume that some devices will have
already probed without explicitly checking for that.
After the recent move to defer most device probes when they are
registered, pressure increased in the late initcall level.
By starting the processing of the deferred queue in device_initcall_sync
we increase the chances that the initcalls mentioned before will find
the devices they depend on to have already probed.
- Start processing deferred probes in device_initcall_sync
Changes in v3: None
Changes in v2: None
drivers/base/dd.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 0654fb771a53..b6a22cff78cf 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -176,7 +176,7 @@ static void driver_deferred_probe_trigger(void)
*
* We don't want to get in the way when the bulk of drivers are getting probed.
* Instead, this initcall makes sure that deferred probing is delayed until
- * late_initcall time.
+ * device_initcall_sync time.
*/
static int deferred_probe_initcall(void)
{
@@ -190,7 +190,7 @@ static int deferred_probe_initcall(void)
flush_workqueue(deferred_wq);
return 0;
}
-late_initcall(deferred_probe_initcall);
+device_initcall_sync(deferred_probe_initcall);
static void driver_bound(struct device *dev)
already probed without explicitly checking for that.
After the recent move to defer most device probes when they are
registered, pressure increased in the late initcall level.
By starting the processing of the deferred queue in device_initcall_sync
we increase the chances that the initcalls mentioned before will find
the devices they depend on to have already probed.
- Start processing deferred probes in device_initcall_sync
Changes in v3: None
Changes in v2: None
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 0654fb771a53..b6a22cff78cf 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -176,7 +176,7 @@ static void driver_deferred_probe_trigger(void)
*
* We don't want to get in the way when the bulk of drivers are getting probed.
* Instead, this initcall makes sure that deferred probing is delayed until
- * late_initcall time.
+ * device_initcall_sync time.
*/
static int deferred_probe_initcall(void)
{
@@ -190,7 +190,7 @@ static int deferred_probe_initcall(void)
flush_workqueue(deferred_wq);
return 0;
}
-late_initcall(deferred_probe_initcall);
+device_initcall_sync(deferred_probe_initcall);
static void driver_bound(struct device *dev)
Tomeu Vizoso
Sep 7, 2015, 8:30:09 AM9/7/15
to
When looking up a pin controller through its OF node, probe it if it
Changes in v4: None
1 file changed, 2 insertions(+)
diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 9a439dab7a87..05da9a56608d 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -359,6 +359,8 @@ static int of_gpiochip_add_pin_range(struct gpio_chip *chip)
if (ret)
break;
+ of_device_probe(pinspec.np);
+
pctldev = of_pinctrl_get(pinspec.np);
if (!pctldev)
return -EPROBE_DEFER;
hasn't already.
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
Changes in v4: None
Changes in v3: None
Changes in v2: None
drivers/gpio/gpiolib-of.c | 2 ++
Changes in v2: None
1 file changed, 2 insertions(+)
diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 9a439dab7a87..05da9a56608d 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -359,6 +359,8 @@ static int of_gpiochip_add_pin_range(struct gpio_chip *chip)
if (ret)
break;
+ of_device_probe(pinspec.np);
+
pctldev = of_pinctrl_get(pinspec.np);
if (!pctldev)
return -EPROBE_DEFER;
Tomeu Vizoso
Sep 7, 2015, 8:30:10 AM9/7/15
to
Add a field to struct device that instructs the device-driver core to
defer the probe of this device until the late_initcall level.
By letting all built-in drivers to register before starting to probe, we
can avoid any deferred probes by probing dependencies on demand.
- Add Kconfig DELAY_DEVICE_PROBES to allow disabling delayed probing in
machines with initcalls that depend on devices probing at a given time.
Changes in v3: None
Changes in v2: None
drivers/base/Kconfig | 18 ++++++++++++++++++
drivers/base/dd.c | 7 +++++++
include/linux/device.h | 2 ++
3 files changed, 27 insertions(+)
diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index 98504ec99c7d..44b5d33b1f49 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -324,4 +324,22 @@ config CMA_ALIGNMENT
endif
+config DELAY_DEVICE_PROBES
+ bool "Allow delaying the probe of some devices"
+ default y
+ help
+ Devices can be matched to a driver and probed from the moment they
+ are registered, but early during boot their probes are likely to be
+ deferred because some dependency isn't available yet because most
+ drivers haven't been registered yet.
+
+ Enabling this option allows the device registration code to delay the
+ probing of a specific device until device_initcall_sync, when all
+ built-in drivers have been registered already.
+
+ In some platforms there may be implicit assumptions about when some
+ devices are probed, so enabling this option could cause problems there.
+
+ If unsure, say Y here.
+
endmenu
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index faf60bfc46b3..0654fb771a53 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -417,6 +417,13 @@ int driver_probe_device(struct device_driver *drv, struct device *dev)
if (!device_is_registered(dev))
return -ENODEV;
+#if IS_ENABLED(CONFIG_DELAY_DEVICE_PROBES)
+ if (!driver_deferred_probe_enable && dev->probe_late) {
+ driver_deferred_probe_add(dev);
+ return 0;
+ }
+#endif
+
pr_debug("bus: '%s': %s: matched device %s with driver %s\n",
drv->bus->name, __func__, dev_name(dev), drv->name);
diff --git a/include/linux/device.h b/include/linux/device.h
index d8be07bc9c3f..fe5c50ffd7d3 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -748,6 +748,7 @@ struct device_dma_parameters {
*
* @offline_disabled: If set, the device is permanently online.
* @offline: Set after successful invocation of bus type's .offline().
+ * @probe_late: If set, device will be probed in the late initcall level.
*
* At the lowest level, every device in a Linux system is represented by an
* instance of struct device. The device structure contains the information
@@ -832,6 +833,7 @@ struct device {
bool offline_disabled:1;
bool offline:1;
+ bool probe_late:1;
};
static inline struct device *kobj_to_dev(struct kobject *kobj)
defer the probe of this device until the late_initcall level.
By letting all built-in drivers to register before starting to probe, we
can avoid any deferred probes by probing dependencies on demand.
- Add Kconfig DELAY_DEVICE_PROBES to allow disabling delayed probing in
machines with initcalls that depend on devices probing at a given time.
Changes in v3: None
Changes in v2: None
drivers/base/dd.c | 7 +++++++
include/linux/device.h | 2 ++
3 files changed, 27 insertions(+)
diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index 98504ec99c7d..44b5d33b1f49 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -324,4 +324,22 @@ config CMA_ALIGNMENT
endif
+config DELAY_DEVICE_PROBES
+ bool "Allow delaying the probe of some devices"
+ default y
+ help
+ Devices can be matched to a driver and probed from the moment they
+ are registered, but early during boot their probes are likely to be
+ deferred because some dependency isn't available yet because most
+ drivers haven't been registered yet.
+
+ Enabling this option allows the device registration code to delay the
+ probing of a specific device until device_initcall_sync, when all
+ built-in drivers have been registered already.
+
+ In some platforms there may be implicit assumptions about when some
+ devices are probed, so enabling this option could cause problems there.
+
+ If unsure, say Y here.
+
endmenu
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index faf60bfc46b3..0654fb771a53 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -417,6 +417,13 @@ int driver_probe_device(struct device_driver *drv, struct device *dev)
if (!device_is_registered(dev))
return -ENODEV;
+#if IS_ENABLED(CONFIG_DELAY_DEVICE_PROBES)
+ if (!driver_deferred_probe_enable && dev->probe_late) {
+ driver_deferred_probe_add(dev);
+ return 0;
+ }
+#endif
+
pr_debug("bus: '%s': %s: matched device %s with driver %s\n",
drv->bus->name, __func__, dev_name(dev), drv->name);
diff --git a/include/linux/device.h b/include/linux/device.h
index d8be07bc9c3f..fe5c50ffd7d3 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -748,6 +748,7 @@ struct device_dma_parameters {
*
* @offline_disabled: If set, the device is permanently online.
* @offline: Set after successful invocation of bus type's .offline().
+ * @probe_late: If set, device will be probed in the late initcall level.
*
* At the lowest level, every device in a Linux system is represented by an
* instance of struct device. The device structure contains the information
@@ -832,6 +833,7 @@ struct device {
bool offline_disabled:1;
bool offline:1;
+ bool probe_late:1;
};
static inline struct device *kobj_to_dev(struct kobject *kobj)
Tomeu Vizoso
Sep 7, 2015, 8:30:10 AM9/7/15
to
When looking up a panel through its OF node, probe it if it hasn't
index 2ef988e037b7..ad79a7b9c74d 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_device.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_device_probe(np);
+
mutex_lock(&panel_lock);
list_for_each_entry(panel, &panel_list, list) {
already.
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Changes in v4: None
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Changes in v3: None
Changes in v2: None
drivers/gpu/drm/drm_panel.c | 3 +++
Changes in v2: None
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
index 2ef988e037b7..ad79a7b9c74d 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_device.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_device_probe(np);
+
mutex_lock(&panel_lock);
list_for_each_entry(panel, &panel_list, list) {
Tomeu Vizoso
Sep 7, 2015, 8:30:10 AM9/7/15
to
When looking up a pin controller through its OF node, probe it if it
index fe04e748dfe4..f5340b8e1dbe 100644
--- a/drivers/pinctrl/devicetree.c
+++ b/drivers/pinctrl/devicetree.c
+#include <linux/of_device.h>
#include <linux/pinctrl/pinctrl.h>
#include <linux/slab.h>
@@ -110,6 +111,8 @@ static int dt_to_map_one_config(struct pinctrl *p, const char *statename,
struct pinctrl_map *map;
unsigned num_maps;
+ of_device_probe(np_config);
+
/* Find the pin controller containing np_config */
np_pctldev = of_node_get(np_config);
for (;;) {
hasn't already.
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
---
Changes in v4: None
Changes in v3: None
Changes in v2: None
drivers/pinctrl/devicetree.c | 3 +++
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
---
Changes in v4: None
Changes in v3: None
Changes in v2: None
1 file changed, 3 insertions(+)
diff --git a/drivers/pinctrl/devicetree.c b/drivers/pinctrl/devicetree.c
index fe04e748dfe4..f5340b8e1dbe 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/device.h>
+#include <linux/of_device.h>
#include <linux/pinctrl/pinctrl.h>
#include <linux/slab.h>
@@ -110,6 +111,8 @@ static int dt_to_map_one_config(struct pinctrl *p, const char *statename,
struct pinctrl_map *map;
unsigned num_maps;
+ of_device_probe(np_config);
+
/* Find the pin controller containing np_config */
np_pctldev = of_node_get(np_config);
for (;;) {
Tomeu Vizoso
Sep 7, 2015, 8:30:10 AM9/7/15
to
When looking up a backlight device through its OF node, probe it if it
index bddc8b17a4d8..9bcdc16eacdf 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -16,6 +16,7 @@
#include <linux/err.h>
#include <linux/fb.h>
#include <linux/slab.h>
+#include <linux/of_device.h>
#ifdef CONFIG_PMAC_BACKLIGHT
#include <asm/backlight.h>
@@ -559,6 +560,8 @@ struct backlight_device *of_find_backlight_by_node(struct device_node *node)
{
struct device *dev;
+ of_device_probe(node);
+
dev = class_find_device(backlight_class, NULL, node, of_parent_match);
return dev ? to_backlight_device(dev) : NULL;
hasn't already.
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
---
Changes in v4: None
Changes in v3: None
Changes in v2: None
drivers/video/backlight/backlight.c | 3 +++
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
---
Changes in v4: None
Changes in v3: None
Changes in v2: None
1 file changed, 3 insertions(+)
diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index bddc8b17a4d8..9bcdc16eacdf 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -16,6 +16,7 @@
#include <linux/err.h>
#include <linux/fb.h>
#include <linux/slab.h>
+#include <linux/of_device.h>
#ifdef CONFIG_PMAC_BACKLIGHT
#include <asm/backlight.h>
@@ -559,6 +560,8 @@ struct backlight_device *of_find_backlight_by_node(struct device_node *node)
{
struct device *dev;
+ of_device_probe(node);
+
dev = class_find_device(backlight_class, NULL, node, of_parent_match);
return dev ? to_backlight_device(dev) : NULL;
Tomeu Vizoso
Sep 7, 2015, 8:30:11 AM9/7/15
to
Instead of trying to match and probe platform and AMBA devices right
after each is registered, delay their probes until device_initcall_sync.
This means that devices will start probing once all built-in drivers
have registered, and after all platform and AMBA devices from the DT
have been registered already.
This allows us to prevent deferred probes by probing dependencies on
demand.
- Also defer probes of AMBA devices registered from the DT as they can
also request resources.
Changes in v3: None
Changes in v2: None
drivers/of/platform.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index f089d95ac961..a9397ce838ea 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -164,7 +164,8 @@ static struct platform_device *of_platform_device_create_pdata(
struct device_node *np,
const char *bus_id,
void *platform_data,
- struct device *parent)
+ struct device *parent,
+ bool probe_late)
{
struct platform_device *dev;
@@ -178,6 +179,7 @@ static struct platform_device *of_platform_device_create_pdata(
dev->dev.bus = &platform_bus_type;
dev->dev.platform_data = platform_data;
+ dev->dev.probe_late = probe_late;
of_dma_configure(&dev->dev, dev->dev.of_node);
of_msi_configure(&dev->dev, dev->dev.of_node);
@@ -209,7 +211,8 @@ struct platform_device *of_platform_device_create(struct device_node *np,
const char *bus_id,
struct device *parent)
{
- return of_platform_device_create_pdata(np, bus_id, NULL, parent);
+ return of_platform_device_create_pdata(np, bus_id, NULL, parent,
+ false);
}
EXPORT_SYMBOL(of_platform_device_create);
@@ -240,6 +243,7 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
dev->dev.of_node = of_node_get(node);
dev->dev.parent = parent ? : &platform_bus;
dev->dev.platform_data = platform_data;
+ dev->dev.probe_late = true;
if (bus_id)
dev_set_name(&dev->dev, "%s", bus_id);
else
@@ -358,7 +362,8 @@ static int of_platform_bus_create(struct device_node *bus,
return 0;
}
- dev = of_platform_device_create_pdata(bus, bus_id, platform_data, parent);
+ dev = of_platform_device_create_pdata(bus, bus_id, platform_data,
+ parent, true);
if (!dev || !of_match_node(matches, bus))
return 0;
after each is registered, delay their probes until device_initcall_sync.
This means that devices will start probing once all built-in drivers
have registered, and after all platform and AMBA devices from the DT
have been registered already.
This allows us to prevent deferred probes by probing dependencies on
demand.
- Also defer probes of AMBA devices registered from the DT as they can
also request resources.
Changes in v3: None
Changes in v2: None
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index f089d95ac961..a9397ce838ea 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -164,7 +164,8 @@ static struct platform_device *of_platform_device_create_pdata(
struct device_node *np,
const char *bus_id,
void *platform_data,
- struct device *parent)
+ struct device *parent,
+ bool probe_late)
{
struct platform_device *dev;
@@ -178,6 +179,7 @@ static struct platform_device *of_platform_device_create_pdata(
dev->dev.bus = &platform_bus_type;
dev->dev.platform_data = platform_data;
+ dev->dev.probe_late = probe_late;
of_dma_configure(&dev->dev, dev->dev.of_node);
of_msi_configure(&dev->dev, dev->dev.of_node);
@@ -209,7 +211,8 @@ struct platform_device *of_platform_device_create(struct device_node *np,
const char *bus_id,
struct device *parent)
{
- return of_platform_device_create_pdata(np, bus_id, NULL, parent);
+ return of_platform_device_create_pdata(np, bus_id, NULL, parent,
+ false);
}
EXPORT_SYMBOL(of_platform_device_create);
@@ -240,6 +243,7 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
dev->dev.of_node = of_node_get(node);
dev->dev.parent = parent ? : &platform_bus;
dev->dev.platform_data = platform_data;
+ dev->dev.probe_late = true;
if (bus_id)
dev_set_name(&dev->dev, "%s", bus_id);
else
@@ -358,7 +362,8 @@ static int of_platform_bus_create(struct device_node *bus,
return 0;
}
- dev = of_platform_device_create_pdata(bus, bus_id, platform_data, parent);
+ dev = of_platform_device_create_pdata(bus, bus_id, platform_data,
+ parent, true);
if (!dev || !of_match_node(matches, bus))
return 0;
Tomeu Vizoso
Sep 7, 2015, 8:30:13 AM9/7/15
to
When looking up a phy through its OF node, probe it if it hasn't
index 98f75d2842b7..fb0b650bb494 100644
--- a/drivers/usb/phy/phy.c
+++ b/drivers/usb/phy/phy.c
@@ -15,6 +15,7 @@
#include <linux/module.h>
#include <linux/slab.h>
#include <linux/of.h>
+#include <linux/of_device.h>
#include <linux/usb/phy.h>
@@ -196,6 +197,8 @@ struct usb_phy *devm_usb_get_phy_by_node(struct device *dev,
goto err0;
}
+ of_device_probe(node);
+
spin_lock_irqsave(&phy_lock, flags);
phy = __of_usb_find_phy(node);
already.
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Changes in v4: None
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Changes in v3: None
Changes in v2: None
drivers/usb/phy/phy.c | 3 +++
Changes in v2: None
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c
index 98f75d2842b7..fb0b650bb494 100644
--- a/drivers/usb/phy/phy.c
+++ b/drivers/usb/phy/phy.c
@@ -15,6 +15,7 @@
#include <linux/module.h>
#include <linux/slab.h>
#include <linux/of.h>
+#include <linux/of_device.h>
#include <linux/usb/phy.h>
@@ -196,6 +197,8 @@ struct usb_phy *devm_usb_get_phy_by_node(struct device *dev,
goto err0;
}
+ of_device_probe(node);
+
spin_lock_irqsave(&phy_lock, flags);
phy = __of_usb_find_phy(node);
Tomeu Vizoso
Sep 7, 2015, 8:30:13 AM9/7/15
to
When adding a platform device, set the device node's device member to
point to it.
This speeds lookups considerably and is safe because we only create one
platform device for any given device node.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
---
Changes in v4: None
Changes in v3: None
Changes in v2: None
drivers/of/platform.c | 13 +++++--------
include/linux/of.h | 1 +
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 1001efaedcb8..baf04d7249bd 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -32,11 +32,6 @@ const struct of_device_id of_default_bus_match_table[] = {
{} /* Empty terminated list */
};
-static int of_dev_node_match(struct device *dev, void *data)
-{
- return dev->of_node == data;
-}
-
/**
* of_find_device_by_node - Find the platform_device associated with a node
* @np: Pointer to device tree node
@@ -45,10 +40,10 @@ static int of_dev_node_match(struct device *dev, void *data)
*/
struct platform_device *of_find_device_by_node(struct device_node *np)
{
- struct device *dev;
+ if (np->device && np->device->bus == &platform_bus_type)
+ return to_platform_device(np->device);
- dev = bus_find_device(&platform_bus_type, NULL, np, of_dev_node_match);
- return dev ? to_platform_device(dev) : NULL;
+ return NULL;
}
EXPORT_SYMBOL(of_find_device_by_node);
@@ -192,6 +187,8 @@ static struct platform_device *of_platform_device_create_pdata(
goto err_clear_flag;
}
+ np->device = &dev->dev;
+
return dev;
err_clear_flag:
diff --git a/include/linux/of.h b/include/linux/of.h
index 2194b8ca41f9..eb091be0f8ee 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -52,6 +52,7 @@ struct device_node {
phandle phandle;
const char *full_name;
struct fwnode_handle fwnode;
+ struct device *device;
struct property *properties;
struct property *deadprops; /* removed properties */
point to it.
This speeds lookups considerably and is safe because we only create one
platform device for any given device node.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
---
Changes in v4: None
Changes in v3: None
Changes in v2: None
include/linux/of.h | 1 +
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 1001efaedcb8..baf04d7249bd 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -32,11 +32,6 @@ const struct of_device_id of_default_bus_match_table[] = {
{} /* Empty terminated list */
};
-static int of_dev_node_match(struct device *dev, void *data)
-{
- return dev->of_node == data;
-}
-
/**
* of_find_device_by_node - Find the platform_device associated with a node
* @np: Pointer to device tree node
@@ -45,10 +40,10 @@ static int of_dev_node_match(struct device *dev, void *data)
*/
struct platform_device *of_find_device_by_node(struct device_node *np)
{
- struct device *dev;
+ if (np->device && np->device->bus == &platform_bus_type)
+ return to_platform_device(np->device);
- dev = bus_find_device(&platform_bus_type, NULL, np, of_dev_node_match);
- return dev ? to_platform_device(dev) : NULL;
+ return NULL;
}
EXPORT_SYMBOL(of_find_device_by_node);
@@ -192,6 +187,8 @@ static struct platform_device *of_platform_device_create_pdata(
goto err_clear_flag;
}
+ np->device = &dev->dev;
+
return dev;
err_clear_flag:
diff --git a/include/linux/of.h b/include/linux/of.h
index 2194b8ca41f9..eb091be0f8ee 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -52,6 +52,7 @@ struct device_node {
phandle phandle;
const char *full_name;
struct fwnode_handle fwnode;
+ struct device *device;
struct property *properties;
struct property *deadprops; /* removed properties */
Tomeu Vizoso
Sep 7, 2015, 8:40:05 AM9/7/15
to
When looking up an i2c adapter or device through its OF node, probe it
1 file changed, 4 insertions(+)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 5f89f1e3c2f2..02da3acbbd35 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1413,6 +1413,8 @@ struct i2c_client *of_find_i2c_device_by_node(struct device_node *node)
struct device *dev;
struct i2c_client *client;
+ of_device_probe(node);
+
dev = bus_find_device(&i2c_bus_type, NULL, node, of_dev_node_match);
if (!dev)
return NULL;
@@ -1431,6 +1433,8 @@ struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node)
struct device *dev;
struct i2c_adapter *adapter;
+ of_device_probe(node);
+
dev = bus_find_device(&i2c_bus_type, NULL, node, of_dev_node_match);
if (!dev)
return NULL;
if it hasn't already.
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
---
Changes in v4: None
Changes in v3: None
Changes in v2: None
drivers/i2c/i2c-core.c | 4 ++++
---
Changes in v4: None
Changes in v3: None
Changes in v2: None
1 file changed, 4 insertions(+)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 5f89f1e3c2f2..02da3acbbd35 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1413,6 +1413,8 @@ struct i2c_client *of_find_i2c_device_by_node(struct device_node *node)
struct device *dev;
struct i2c_client *client;
+ of_device_probe(node);
+
dev = bus_find_device(&i2c_bus_type, NULL, node, of_dev_node_match);
if (!dev)
return NULL;
@@ -1431,6 +1433,8 @@ struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node)
struct device *dev;
struct i2c_adapter *adapter;
+ of_device_probe(node);
+
dev = bus_find_device(&i2c_bus_type, NULL, node, of_dev_node_match);
if (!dev)
return NULL;
Tomeu Vizoso
Sep 7, 2015, 8:40:06 AM9/7/15
to
When looking up a PWM chip through its OF node, probe it if it hasn't
index 3f9df3ea3350..794a923df0d8 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -29,6 +29,7 @@
#include <linux/device.h>
#include <linux/debugfs.h>
#include <linux/seq_file.h>
+#include <linux/of_device.h>
#include <dt-bindings/pwm/pwm.h>
@@ -516,6 +517,8 @@ static struct pwm_chip *of_node_to_pwmchip(struct device_node *np)
{
struct pwm_chip *chip;
+ of_device_probe(np);
+
mutex_lock(&pwm_lock);
list_for_each_entry(chip, &pwm_chips, list)
already.
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
---
Changes in v4: None
Changes in v3: None
Changes in v2: None
drivers/pwm/core.c | 3 +++
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
---
Changes in v4: None
Changes in v3: None
Changes in v2: None
1 file changed, 3 insertions(+)
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 3f9df3ea3350..794a923df0d8 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -29,6 +29,7 @@
#include <linux/device.h>
#include <linux/debugfs.h>
#include <linux/seq_file.h>
+#include <linux/of_device.h>
#include <dt-bindings/pwm/pwm.h>
@@ -516,6 +517,8 @@ static struct pwm_chip *of_node_to_pwmchip(struct device_node *np)
{
struct pwm_chip *chip;
+ of_device_probe(np);
+
mutex_lock(&pwm_lock);
list_for_each_entry(chip, &pwm_chips, list)
Tomeu Vizoso
Sep 7, 2015, 8:40:06 AM9/7/15
to
By moving the locking of regulator_list_mutex into regulator_dev_lookup,
where it is iterated over. The reference count of the regulator's device
is increased in case it's unregistered while in use.
In _regulator_get() the regulator_list_mutex mutex was held for most of
the function, but that 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 sequence illustrates the situation described above:
tegra_hdmi_probe
_regulator_get
regulator_dev_lookup
of_device_probe
reg_fixed_voltage_probe
regulator_register
- Take a reference to the regulator's device to prevent dangling
pointers
Changes in v3:
- Avoid unlocking the regulator device's mutex if we don't have a device
Changes in v2:
- Acquire regulator device lock before returning from regulator_dev_lookup()
drivers/regulator/core.c | 56 ++++++++++++++++++++++++++++++++----------------
1 file changed, 38 insertions(+), 18 deletions(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 7150ff6ef46b..f4aa6cae76d5 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1340,10 +1340,15 @@ static struct regulator_dev *regulator_dev_lookup(struct device *dev,
+ node == r->dev.of_node &&
+ get_device(&r->dev)) {
+ mutex_unlock(®ulator_list_mutex);
return r;
+ }
+ mutex_unlock(®ulator_list_mutex);
*ret = -EPROBE_DEFER;
return NULL;
} else {
@@ -1361,9 +1366,13 @@ static struct regulator_dev *regulator_dev_lookup(struct device *dev,
if (dev)
devname = dev_name(dev);
+ mutex_lock(®ulator_list_mutex);
list_for_each_entry(r, ®ulator_list, list)
- if (strcmp(rdev_get_name(r), supply) == 0)
+ if (strcmp(rdev_get_name(r), supply) == 0 &&
+ get_device(&r->dev)) {
+ mutex_unlock(®ulator_list_mutex);
return r;
+ }
list_for_each_entry(map, ®ulator_map_list, list) {
/* If the mapping has a device set up it must match */
@@ -1371,9 +1380,13 @@ static struct regulator_dev *regulator_dev_lookup(struct device *dev,
(!devname || strcmp(map->dev_name, devname)))
continue;
- if (strcmp(map->supply, supply) == 0)
+ if (strcmp(map->supply, supply) == 0 &&
+ get_device(&map->regulator->dev)) {
+ mutex_unlock(®ulator_list_mutex);
return map->regulator;
+ }
}
+ mutex_unlock(®ulator_list_mutex);
return NULL;
@@ -1405,6 +1418,7 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
if (!r) {
if (have_full_constraints()) {
r = dummy_regulator_rdev;
+ get_device(&r->dev);
} else {
dev_err(dev, "Failed to resolve %s-supply for %s\n",
rdev->supply_name, rdev->desc->name);
@@ -1414,12 +1428,16 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
/* Recursively resolve the supply of the supply */
ret = regulator_resolve_supply(r);
- if (ret < 0)
+ if (ret < 0) {
+ put_device(&r->dev);
return ret;
+ }
ret = set_supply(rdev, r);
- if (ret < 0)
+ if (ret < 0) {
+ put_device(&r->dev);
return ret;
+ }
/* Cascade always-on state to supply */
if (_regulator_is_enabled(rdev) && rdev->supply) {
@@ -1455,8 +1473,6 @@ static struct regulator *_regulator_get(struct device *dev, const char *id,
else
ret = -EPROBE_DEFER;
- mutex_lock(®ulator_list_mutex);
-
rdev = regulator_dev_lookup(dev, id, &ret);
if (rdev)
goto found;
@@ -1468,7 +1484,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";
@@ -1482,40 +1498,46 @@ static struct regulator *_regulator_get(struct device *dev, const char *id,
devname, id);
rdev = dummy_regulator_rdev;
+ get_device(&rdev->dev);
goto found;
/* Don't log an error when called from regulator_get_optional() */
} else if (!have_full_constraints() || exclusive) {
dev_warn(dev, "dummy supplies not allowed\n");
}
- mutex_unlock(®ulator_list_mutex);
return regulator;
found:
if (rdev->exclusive) {
regulator = ERR_PTR(-EPERM);
- goto out;
+ put_device(&rdev->dev);
+ return regulator;
}
if (exclusive && rdev->open_count) {
regulator = ERR_PTR(-EBUSY);
- goto out;
+ put_device(&rdev->dev);
+ return regulator;
}
ret = regulator_resolve_supply(rdev);
if (ret < 0) {
regulator = ERR_PTR(ret);
- goto out;
+ put_device(&rdev->dev);
+ return regulator;
}
- if (!try_module_get(rdev->owner))
- goto out;
+ if (!try_module_get(rdev->owner)) {
+ put_device(&rdev->dev);
+ return regulator;
+ }
regulator = create_regulator(rdev, dev, id);
if (regulator == NULL) {
regulator = ERR_PTR(-ENOMEM);
+ put_device(&rdev->dev);
module_put(rdev->owner);
- goto out;
+ return regulator;
}
rdev->open_count++;
@@ -1529,9 +1551,6 @@ found:
rdev->use_count = 0;
}
-out:
- mutex_unlock(®ulator_list_mutex);
-
return regulator;
}
@@ -1629,6 +1648,7 @@ static void _regulator_put(struct regulator *regulator)
rdev->open_count--;
rdev->exclusive = 0;
+ put_device(&rdev->dev);
mutex_unlock(&rdev->mutex);
kfree(regulator->supply_name);
where it is iterated over. The reference count of the regulator's device
is increased in case it's unregistered while in use.
In _regulator_get() the regulator_list_mutex mutex was held for most of
the function, but that 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 sequence illustrates the situation described above:
tegra_hdmi_probe
_regulator_get
regulator_dev_lookup
of_device_probe
reg_fixed_voltage_probe
regulator_register
- Take a reference to the regulator's device to prevent dangling
pointers
Changes in v3:
- Avoid unlocking the regulator device's mutex if we don't have a device
Changes in v2:
- Acquire regulator device lock before returning from regulator_dev_lookup()
drivers/regulator/core.c | 56 ++++++++++++++++++++++++++++++++----------------
1 file changed, 38 insertions(+), 18 deletions(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 7150ff6ef46b..f4aa6cae76d5 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1340,10 +1340,15 @@ static struct regulator_dev *regulator_dev_lookup(struct device *dev,
if (dev && dev->of_node) {
node = of_get_regulator(dev, supply);
if (node) {
+ mutex_lock(®ulator_list_mutex);
node = of_get_regulator(dev, supply);
if (node) {
list_for_each_entry(r, ®ulator_list, list)
if (r->dev.parent &&
- node == r->dev.of_node)
if (r->dev.parent &&
+ node == r->dev.of_node &&
+ get_device(&r->dev)) {
+ mutex_unlock(®ulator_list_mutex);
return r;
+ }
+ mutex_unlock(®ulator_list_mutex);
*ret = -EPROBE_DEFER;
return NULL;
} else {
@@ -1361,9 +1366,13 @@ static struct regulator_dev *regulator_dev_lookup(struct device *dev,
if (dev)
devname = dev_name(dev);
+ mutex_lock(®ulator_list_mutex);
list_for_each_entry(r, ®ulator_list, list)
- if (strcmp(rdev_get_name(r), supply) == 0)
+ if (strcmp(rdev_get_name(r), supply) == 0 &&
+ get_device(&r->dev)) {
+ mutex_unlock(®ulator_list_mutex);
return r;
+ }
list_for_each_entry(map, ®ulator_map_list, list) {
/* If the mapping has a device set up it must match */
@@ -1371,9 +1380,13 @@ static struct regulator_dev *regulator_dev_lookup(struct device *dev,
(!devname || strcmp(map->dev_name, devname)))
continue;
- if (strcmp(map->supply, supply) == 0)
+ if (strcmp(map->supply, supply) == 0 &&
+ get_device(&map->regulator->dev)) {
+ mutex_unlock(®ulator_list_mutex);
return map->regulator;
+ }
}
+ mutex_unlock(®ulator_list_mutex);
return NULL;
@@ -1405,6 +1418,7 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
if (!r) {
if (have_full_constraints()) {
r = dummy_regulator_rdev;
+ get_device(&r->dev);
} else {
dev_err(dev, "Failed to resolve %s-supply for %s\n",
rdev->supply_name, rdev->desc->name);
@@ -1414,12 +1428,16 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
/* Recursively resolve the supply of the supply */
ret = regulator_resolve_supply(r);
- if (ret < 0)
+ if (ret < 0) {
+ put_device(&r->dev);
return ret;
+ }
ret = set_supply(rdev, r);
- if (ret < 0)
+ if (ret < 0) {
+ put_device(&r->dev);
return ret;
+ }
/* Cascade always-on state to supply */
if (_regulator_is_enabled(rdev) && rdev->supply) {
@@ -1455,8 +1473,6 @@ static struct regulator *_regulator_get(struct device *dev, const char *id,
else
ret = -EPROBE_DEFER;
- mutex_lock(®ulator_list_mutex);
-
rdev = regulator_dev_lookup(dev, id, &ret);
if (rdev)
goto found;
@@ -1468,7 +1484,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";
@@ -1482,40 +1498,46 @@ static struct regulator *_regulator_get(struct device *dev, const char *id,
devname, id);
rdev = dummy_regulator_rdev;
+ get_device(&rdev->dev);
goto found;
/* Don't log an error when called from regulator_get_optional() */
} else if (!have_full_constraints() || exclusive) {
dev_warn(dev, "dummy supplies not allowed\n");
}
- mutex_unlock(®ulator_list_mutex);
return regulator;
found:
if (rdev->exclusive) {
regulator = ERR_PTR(-EPERM);
- goto out;
+ put_device(&rdev->dev);
+ return regulator;
}
if (exclusive && rdev->open_count) {
regulator = ERR_PTR(-EBUSY);
- goto out;
+ put_device(&rdev->dev);
+ return regulator;
}
ret = regulator_resolve_supply(rdev);
if (ret < 0) {
regulator = ERR_PTR(ret);
- goto out;
+ put_device(&rdev->dev);
+ return regulator;
}
- if (!try_module_get(rdev->owner))
- goto out;
+ if (!try_module_get(rdev->owner)) {
+ put_device(&rdev->dev);
+ return regulator;
+ }
regulator = create_regulator(rdev, dev, id);
if (regulator == NULL) {
regulator = ERR_PTR(-ENOMEM);
+ put_device(&rdev->dev);
module_put(rdev->owner);
- goto out;
+ return regulator;
}
rdev->open_count++;
@@ -1529,9 +1551,6 @@ found:
rdev->use_count = 0;
}
-out:
- mutex_unlock(®ulator_list_mutex);
-
return regulator;
}
@@ -1629,6 +1648,7 @@ static void _regulator_put(struct regulator *regulator)
rdev->open_count--;
rdev->exclusive = 0;
+ put_device(&rdev->dev);
mutex_unlock(&rdev->mutex);
kfree(regulator->supply_name);
Tomeu Vizoso
Sep 7, 2015, 8:40:08 AM9/7/15
to
Some buses (eg. AMBA) need access to some HW resources (it may need a
clock to be enabled so a device ID can be read) before a device can be
matched to a driver.
The pre_probe callback allows the device-driver core to request the bus
to perform this initialization and can defer the probe if any of the
resources needed are missing.
This gives us more flexibility when setting the order in which devices
are probed because the resources needed to get the matching information
don't need to be available by the time that the bus devices are
registered.
include/linux/device.h | 4 ++++
2 files changed, 28 insertions(+)
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index be0eb4639128..faf60bfc46b3 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -544,6 +544,17 @@ static int __device_attach(struct device *dev, bool allow_async)
int ret = 0;
device_lock(dev);
+
+ if (dev->bus && dev->bus->pre_probe) {
+ ret = dev->bus->pre_probe(dev);
+ if (ret) {
+ if (ret == -EPROBE_DEFER)
+ driver_deferred_probe_add(dev);
+ ret = 0;
+ goto out_unlock;
+ }
+ }
+
if (dev->driver) {
if (klist_node_attached(&dev->p->knode_driver)) {
ret = 1;
@@ -619,6 +630,7 @@ void device_initial_probe(struct device *dev)
static int __driver_attach(struct device *dev, void *data)
{
struct device_driver *drv = data;
+ int ret;
/*
* Lock device and try to bind to it. We drop the error
@@ -636,8 +648,20 @@ static int __driver_attach(struct device *dev, void *data)
if (dev->parent) /* Needed for USB */
device_lock(dev->parent);
device_lock(dev);
+
+ if (dev->bus && dev->bus->pre_probe) {
+ ret = dev->bus->pre_probe(dev);
+ if (ret) {
+ if (ret == -EPROBE_DEFER)
+ driver_deferred_probe_add(dev);
+ goto out;
+ }
+ }
+
if (!dev->driver)
driver_probe_device(drv, dev);
+
+out:
device_unlock(dev);
if (dev->parent)
device_unlock(dev->parent);
diff --git a/include/linux/device.h b/include/linux/device.h
index 5d7bc6349930..d8be07bc9c3f 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -74,6 +74,9 @@ extern void bus_remove_file(struct bus_type *, struct bus_attribute *);
* given device can be handled by the given driver.
* @uevent: Called when a device is added, removed, or a few other things
* that generate uevents to add the environment variables.
+ * @pre_probe: Called when a new device or driver is added to this bus, to
+ * perform any initializations that are needed so the device can
+ * be matched to a driver.
* @probe: Called when a new device or driver add to this bus, and callback
* the specific driver's probe to initial the matched device.
* @remove: Called when a device removed from this bus.
@@ -113,6 +116,7 @@ struct bus_type {
int (*match)(struct device *dev, struct device_driver *drv);
int (*uevent)(struct device *dev, struct kobj_uevent_env *env);
+ int (*pre_probe)(struct device *dev);
int (*probe)(struct device *dev);
int (*remove)(struct device *dev);
void (*shutdown)(struct device *dev);
clock to be enabled so a device ID can be read) before a device can be
matched to a driver.
The pre_probe callback allows the device-driver core to request the bus
to perform this initialization and can defer the probe if any of the
resources needed are missing.
This gives us more flexibility when setting the order in which devices
are probed because the resources needed to get the matching information
don't need to be available by the time that the bus devices are
registered.
Changes in v4: None
Changes in v3: None
Changes in v2: None
drivers/base/dd.c | 24 ++++++++++++++++++++++++
Changes in v3: None
Changes in v2: None
include/linux/device.h | 4 ++++
2 files changed, 28 insertions(+)
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index be0eb4639128..faf60bfc46b3 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -544,6 +544,17 @@ static int __device_attach(struct device *dev, bool allow_async)
int ret = 0;
device_lock(dev);
+
+ if (dev->bus && dev->bus->pre_probe) {
+ ret = dev->bus->pre_probe(dev);
+ if (ret) {
+ if (ret == -EPROBE_DEFER)
+ driver_deferred_probe_add(dev);
+ ret = 0;
+ goto out_unlock;
+ }
+ }
+
if (dev->driver) {
if (klist_node_attached(&dev->p->knode_driver)) {
ret = 1;
@@ -619,6 +630,7 @@ void device_initial_probe(struct device *dev)
static int __driver_attach(struct device *dev, void *data)
{
struct device_driver *drv = data;
+ int ret;
/*
* Lock device and try to bind to it. We drop the error
@@ -636,8 +648,20 @@ static int __driver_attach(struct device *dev, void *data)
if (dev->parent) /* Needed for USB */
device_lock(dev->parent);
device_lock(dev);
+
+ if (dev->bus && dev->bus->pre_probe) {
+ ret = dev->bus->pre_probe(dev);
+ if (ret) {
+ if (ret == -EPROBE_DEFER)
+ driver_deferred_probe_add(dev);
+ goto out;
+ }
+ }
+
if (!dev->driver)
driver_probe_device(drv, dev);
+
+out:
device_unlock(dev);
if (dev->parent)
device_unlock(dev->parent);
diff --git a/include/linux/device.h b/include/linux/device.h
index 5d7bc6349930..d8be07bc9c3f 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -74,6 +74,9 @@ extern void bus_remove_file(struct bus_type *, struct bus_attribute *);
* given device can be handled by the given driver.
* @uevent: Called when a device is added, removed, or a few other things
* that generate uevents to add the environment variables.
+ * @pre_probe: Called when a new device or driver is added to this bus, to
+ * perform any initializations that are needed so the device can
+ * be matched to a driver.
* @probe: Called when a new device or driver add to this bus, and callback
* the specific driver's probe to initial the matched device.
* @remove: Called when a device removed from this bus.
@@ -113,6 +116,7 @@ struct bus_type {
int (*match)(struct device *dev, struct device_driver *drv);
int (*uevent)(struct device *dev, struct kobj_uevent_env *env);
+ int (*pre_probe)(struct device *dev);
int (*probe)(struct device *dev);
int (*remove)(struct device *dev);
void (*shutdown)(struct device *dev);
Tomeu Vizoso
Sep 7, 2015, 8:40:13 AM9/7/15
to
Reading the periphid when the Primecell device is registered means that
the apb pclk must be available by then or the device won't be registered
at all.
By moving this code to pre_probe (to be called by the device-driver core
before the device is matched to a driver) we remove any order
requirements.
- Added bus.pre_probe callback so the probes of Primecell devices can be
deferred if their device IDs cannot be yet read because of the clock
driver not having probed when they are registered. Maybe this goes
overboard and the matching information should be in the DT if there is
one.
Changes in v3: None
Changes in v2: None
drivers/amba/bus.c | 78 ++++++++++++++++++++++++++----------------------------
1 file changed, 38 insertions(+), 40 deletions(-)
diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
index f0099360039e..1cbe43b4acd3 100644
--- a/drivers/amba/bus.c
+++ b/drivers/amba/bus.c
@@ -24,6 +24,8 @@
#define to_amba_driver(d) container_of(d, struct amba_driver, drv)
+static int amba_pre_probe(struct device *dev);
+
static const struct amba_id *
amba_lookup(const struct amba_id *table, struct amba_device *dev)
{
@@ -193,6 +195,7 @@ struct bus_type amba_bustype = {
.name = "amba",
.dev_attrs = amba_dev_attrs,
.match = amba_match,
+ .pre_probe = amba_pre_probe,
.uevent = amba_uevent,
.pm = &amba_pm,
};
@@ -336,44 +339,26 @@ static void amba_device_release(struct device *dev)
kfree(d);
}
-/**
- * amba_device_add - add a previously allocated AMBA device structure
- * @dev: AMBA device allocated by amba_device_alloc
- * @parent: resource parent for this devices resources
- *
- * Claim the resource, and read the device cell ID if not already
- * initialized. Register the AMBA device with the Linux device
- * manager.
- */
-int amba_device_add(struct amba_device *dev, struct resource *parent)
+static int amba_pre_probe(struct device *dev)
{
+ struct amba_device *d = to_amba_device(dev);
u32 size;
void __iomem *tmp;
int i, ret;
- WARN_ON(dev->irq[0] == (unsigned int)-1);
- WARN_ON(dev->irq[1] == (unsigned int)-1);
-
- ret = request_resource(parent, &dev->res);
- if (ret)
- goto err_out;
-
- /* Hard-coded primecell ID instead of plug-n-play */
- if (dev->periphid != 0)
- goto skip_probe;
+ if (d->periphid != 0)
+ return 0;
/*
* Dynamically calculate the size of the resource
* and use this for iomap
*/
- size = resource_size(&dev->res);
- tmp = ioremap(dev->res.start, size);
- if (!tmp) {
- ret = -ENOMEM;
- goto err_release;
- }
+ size = resource_size(&d->res);
+ tmp = ioremap(d->res.start, size);
+ if (!tmp)
+ return -ENOMEM;
- ret = amba_get_enable_pclk(dev);
+ ret = amba_get_enable_pclk(d);
if (ret == 0) {
u32 pid, cid;
@@ -388,37 +373,50 @@ int amba_device_add(struct amba_device *dev, struct resource *parent)
cid |= (readl(tmp + size - 0x10 + 4 * i) & 255) <<
(i * 8);
- amba_put_disable_pclk(dev);
+ amba_put_disable_pclk(d);
if (cid == AMBA_CID || cid == CORESIGHT_CID)
- dev->periphid = pid;
+ d->periphid = pid;
- if (!dev->periphid)
+ if (!d->periphid)
ret = -ENODEV;
}
iounmap(tmp);
+ return ret;
+}
+
+/**
+ * amba_device_add - add a previously allocated AMBA device structure
+ * @dev: AMBA device allocated by amba_device_alloc
+ * @parent: resource parent for this devices resources
+ *
+ * Claim the resource, and register the AMBA device with the Linux device
+ * manager.
+ */
+int amba_device_add(struct amba_device *dev, struct resource *parent)
+{
+ int ret;
+
+ WARN_ON(dev->irq[0] == (unsigned int)-1);
+ WARN_ON(dev->irq[1] == (unsigned int)-1);
+
+ ret = request_resource(parent, &dev->res);
if (ret)
- goto err_release;
+ return ret;
- skip_probe:
ret = device_add(&dev->dev);
if (ret)
- goto err_release;
+ return ret;
if (dev->irq[0])
ret = device_create_file(&dev->dev, &dev_attr_irq0);
if (ret == 0 && dev->irq[1])
ret = device_create_file(&dev->dev, &dev_attr_irq1);
- if (ret == 0)
- return ret;
-
- device_unregister(&dev->dev);
+ if (ret)
+ device_unregister(&dev->dev);
- err_release:
- release_resource(&dev->res);
- err_out:
return ret;
}
EXPORT_SYMBOL_GPL(amba_device_add);
the apb pclk must be available by then or the device won't be registered
at all.
By moving this code to pre_probe (to be called by the device-driver core
before the device is matched to a driver) we remove any order
requirements.
- Added bus.pre_probe callback so the probes of Primecell devices can be
deferred if their device IDs cannot be yet read because of the clock
driver not having probed when they are registered. Maybe this goes
overboard and the matching information should be in the DT if there is
one.
Changes in v3: None
Changes in v2: None
1 file changed, 38 insertions(+), 40 deletions(-)
diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
index f0099360039e..1cbe43b4acd3 100644
--- a/drivers/amba/bus.c
+++ b/drivers/amba/bus.c
@@ -24,6 +24,8 @@
#define to_amba_driver(d) container_of(d, struct amba_driver, drv)
+static int amba_pre_probe(struct device *dev);
+
static const struct amba_id *
amba_lookup(const struct amba_id *table, struct amba_device *dev)
{
@@ -193,6 +195,7 @@ struct bus_type amba_bustype = {
.name = "amba",
.dev_attrs = amba_dev_attrs,
.match = amba_match,
+ .pre_probe = amba_pre_probe,
.uevent = amba_uevent,
.pm = &amba_pm,
};
@@ -336,44 +339,26 @@ static void amba_device_release(struct device *dev)
kfree(d);
}
-/**
- * amba_device_add - add a previously allocated AMBA device structure
- * @dev: AMBA device allocated by amba_device_alloc
- * @parent: resource parent for this devices resources
- *
- * Claim the resource, and read the device cell ID if not already
- * initialized. Register the AMBA device with the Linux device
- * manager.
- */
-int amba_device_add(struct amba_device *dev, struct resource *parent)
+static int amba_pre_probe(struct device *dev)
{
+ struct amba_device *d = to_amba_device(dev);
u32 size;
void __iomem *tmp;
int i, ret;
- WARN_ON(dev->irq[0] == (unsigned int)-1);
- WARN_ON(dev->irq[1] == (unsigned int)-1);
-
- ret = request_resource(parent, &dev->res);
- if (ret)
- goto err_out;
-
- /* Hard-coded primecell ID instead of plug-n-play */
- if (dev->periphid != 0)
- goto skip_probe;
+ if (d->periphid != 0)
+ return 0;
/*
* Dynamically calculate the size of the resource
* and use this for iomap
*/
- size = resource_size(&dev->res);
- tmp = ioremap(dev->res.start, size);
- if (!tmp) {
- ret = -ENOMEM;
- goto err_release;
- }
+ size = resource_size(&d->res);
+ tmp = ioremap(d->res.start, size);
+ if (!tmp)
+ return -ENOMEM;
- ret = amba_get_enable_pclk(dev);
+ ret = amba_get_enable_pclk(d);
if (ret == 0) {
u32 pid, cid;
@@ -388,37 +373,50 @@ int amba_device_add(struct amba_device *dev, struct resource *parent)
cid |= (readl(tmp + size - 0x10 + 4 * i) & 255) <<
(i * 8);
- amba_put_disable_pclk(dev);
+ amba_put_disable_pclk(d);
if (cid == AMBA_CID || cid == CORESIGHT_CID)
- dev->periphid = pid;
+ d->periphid = pid;
- if (!dev->periphid)
+ if (!d->periphid)
ret = -ENODEV;
}
iounmap(tmp);
+ return ret;
+}
+
+/**
+ * amba_device_add - add a previously allocated AMBA device structure
+ * @dev: AMBA device allocated by amba_device_alloc
+ * @parent: resource parent for this devices resources
+ *
+ * Claim the resource, and register the AMBA device with the Linux device
+ * manager.
+ */
+int amba_device_add(struct amba_device *dev, struct resource *parent)
+{
+ int ret;
+
+ WARN_ON(dev->irq[0] == (unsigned int)-1);
+ WARN_ON(dev->irq[1] == (unsigned int)-1);
+
+ ret = request_resource(parent, &dev->res);
if (ret)
- goto err_release;
+ return ret;
- skip_probe:
ret = device_add(&dev->dev);
if (ret)
- goto err_release;
+ return ret;
if (dev->irq[0])
ret = device_create_file(&dev->dev, &dev_attr_irq0);
if (ret == 0 && dev->irq[1])
ret = device_create_file(&dev->dev, &dev_attr_irq1);
- if (ret == 0)
- return ret;
-
- device_unregister(&dev->dev);
+ if (ret)
+ device_unregister(&dev->dev);
- err_release:
- release_resource(&dev->res);
- err_out:
return ret;
}
EXPORT_SYMBOL_GPL(amba_device_add);
Tomeu Vizoso
Sep 7, 2015, 8:40:15 AM9/7/15
to
When looking up a dpaux device through its OF node, probe it if it
index 224a7dc8e4ed..96a2eec7e020 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_device.h>
#include <linux/platform_device.h>
#include <linux/reset.h>
#include <linux/regulator/consumer.h>
@@ -439,6 +440,8 @@ struct tegra_dpaux *tegra_dpaux_find_by_of_node(struct device_node *np)
{
struct tegra_dpaux *dpaux;
+ of_device_probe(np);
+
mutex_lock(&dpaux_lock);
list_for_each_entry(dpaux, &dpaux_list, list)
hasn't already.
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Changes in v4: None
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Changes in v3: None
Changes in v2: None
drivers/gpu/drm/tegra/dpaux.c | 3 +++
Changes in v2: None
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
index 224a7dc8e4ed..96a2eec7e020 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_device.h>
#include <linux/platform_device.h>
#include <linux/reset.h>
#include <linux/regulator/consumer.h>
@@ -439,6 +440,8 @@ struct tegra_dpaux *tegra_dpaux_find_by_of_node(struct device_node *np)
{
struct tegra_dpaux *dpaux;
+ of_device_probe(np);
+
mutex_lock(&dpaux_lock);
list_for_each_entry(dpaux, &dpaux_list, list)
Rob Herring
Sep 7, 2015, 5:00:06 PM9/7/15
to
On Mon, Sep 7, 2015 at 7:23 AM, Tomeu Vizoso <tomeu....@collabora.com> wrote:
> Hello,
>
> I have a problem with the panel on my Tegra Chromebook taking longer
> than expected to be ready during boot (Stéphane Marchesin reported what
> is basically the same issue in [0]), and have looked into ordered
> probing as a better way of solving this than moving nodes around in the
> DT or playing with initcall levels and linking order.
>
> While reading the thread [1] that Alexander Holler started with his
> series to make probing order deterministic, it occurred to me that it
> should be possible to achieve the same by probing devices as they are
> referenced by other devices.
>
> This basically reuses the information that is already implicit in the
> probe() implementations, saving us from refactoring existing drivers or
> adding information to DTBs.
>
> During review of v1 of this series Linus Walleij suggested that it
> should be the device driver core to make sure that dependencies are
> ready before probing a device. I gave this idea a try [2] but Mark Brown
> pointed out to the logic duplication between the resource acquisition
> and dependency discovery code paths (though I think it's fairly minor).
>
> To address that code duplication I experimented with Arnd's devm_probe
> [3] concept of having drivers declare their dependencies instead of
> acquiring them during probe, and while it worked [4], I don't think we
> end up winning anything when compared to just probing devices on-demand
> from resource getters.
>
> One remaining objection is to the "sprinkling" of calls to
> of_device_probe() in the resource getters of each subsystem, but I think
> it's the right thing to do given that the storage of resources is
> currently subsystem-specific.
>
> We could avoid the above by moving resource storage into the core, but I
> don't think there's a compelling case for that.
>
> I have tested this on boards with Tegra, iMX.6, Exynos, Rockchip and
> OMAP SoCs, and these patches were enough to eliminate all the deferred
> probes (except one in PandaBoard because omap_dma_system doesn't have a
> firmware node as of yet).
>
> Have submitted a branch [5] with only these patches on top of thursday's
> linux-next to kernelci.org and I don't see any issues that could be
> caused by them. For some reason it currently has more passes than the
> version of -next it's based on!
>
> With this series I get the kernel to output to the panel in 0.5s,
> instead of 2.8s.
>
> Regards,
>
> Tomeu
>
> [0] http://lists.freedesktop.org/archives/dri-devel/2014-August/066527.html
>
> [1] https://lkml.org/lkml/2014/5/12/452
>
> [2] https://lkml.org/lkml/2015/6/17/305
>
> [3] http://article.gmane.org/gmane.linux.ports.arm.kernel/277689
>
> [4] https://lkml.org/lkml/2015/7/21/441a
>
> [5] https://git.collabora.com/cgit/user/tomeu/linux.git/log/?h=on-demand-probes-v6
>
> [6] http://kernelci.org/boot/all/job/collabora/kernel/v4.2-11902-g25d80c927f8b/
>
> [7] http://kernelci.org/boot/all/job/next/kernel/next-20150903/
to be setup before the driver model simply because timers depend on
clocks usually.
Rob
> Hello,
>
> I have a problem with the panel on my Tegra Chromebook taking longer
> than expected to be ready during boot (Stéphane Marchesin reported what
> is basically the same issue in [0]), and have looked into ordered
> probing as a better way of solving this than moving nodes around in the
> DT or playing with initcall levels and linking order.
>
> While reading the thread [1] that Alexander Holler started with his
> series to make probing order deterministic, it occurred to me that it
> should be possible to achieve the same by probing devices as they are
> referenced by other devices.
>
> This basically reuses the information that is already implicit in the
> probe() implementations, saving us from refactoring existing drivers or
> adding information to DTBs.
>
> During review of v1 of this series Linus Walleij suggested that it
> should be the device driver core to make sure that dependencies are
> ready before probing a device. I gave this idea a try [2] but Mark Brown
> pointed out to the logic duplication between the resource acquisition
> and dependency discovery code paths (though I think it's fairly minor).
>
> To address that code duplication I experimented with Arnd's devm_probe
> [3] concept of having drivers declare their dependencies instead of
> acquiring them during probe, and while it worked [4], I don't think we
> end up winning anything when compared to just probing devices on-demand
> from resource getters.
>
> One remaining objection is to the "sprinkling" of calls to
> of_device_probe() in the resource getters of each subsystem, but I think
> it's the right thing to do given that the storage of resources is
> currently subsystem-specific.
>
> We could avoid the above by moving resource storage into the core, but I
> don't think there's a compelling case for that.
>
> I have tested this on boards with Tegra, iMX.6, Exynos, Rockchip and
> OMAP SoCs, and these patches were enough to eliminate all the deferred
> probes (except one in PandaBoard because omap_dma_system doesn't have a
> firmware node as of yet).
>
> Have submitted a branch [5] with only these patches on top of thursday's
> linux-next to kernelci.org and I don't see any issues that could be
> caused by them. For some reason it currently has more passes than the
> version of -next it's based on!
>
> With this series I get the kernel to output to the panel in 0.5s,
> instead of 2.8s.
>
> Regards,
>
> Tomeu
>
> [0] http://lists.freedesktop.org/archives/dri-devel/2014-August/066527.html
>
> [1] https://lkml.org/lkml/2014/5/12/452
>
> [2] https://lkml.org/lkml/2015/6/17/305
>
> [3] http://article.gmane.org/gmane.linux.ports.arm.kernel/277689
>
> [4] https://lkml.org/lkml/2015/7/21/441a
>
> [5] https://git.collabora.com/cgit/user/tomeu/linux.git/log/?h=on-demand-probes-v6
>
> [6] http://kernelci.org/boot/all/job/collabora/kernel/v4.2-11902-g25d80c927f8b/
>
> [7] http://kernelci.org/boot/all/job/next/kernel/next-20150903/
>
> Changes in v4:
> - Added bus.pre_probe callback so the probes of Primecell devices can be
> deferred if their device IDs cannot be yet read because of the clock
> driver not having probed when they are registered. Maybe this goes
> overboard and the matching information should be in the DT if there is
> one.
Seems overboard to me or at least a separate problem. Most clocks have
> Changes in v4:
> - Added bus.pre_probe callback so the probes of Primecell devices can be
> deferred if their device IDs cannot be yet read because of the clock
> driver not having probed when they are registered. Maybe this goes
> overboard and the matching information should be in the DT if there is
> one.
to be setup before the driver model simply because timers depend on
clocks usually.
Rob
Tomeu Vizoso
Sep 8, 2015, 3:40:06 AM9/8/15
to
working on a few boards.
> Most clocks have
> to be setup before the driver model simply because timers depend on
> clocks usually.
implemented in a normal platform driver (vexpress_osc_driver), instead
of using CLK_OF_DECLARE.
Regards,
Tomeu
Rob Herring
Sep 9, 2015, 1:50:07 AM9/9/15
to
be overboard.
>
>> Most clocks have
>> to be setup before the driver model simply because timers depend on
>> clocks usually.
>
> Yes, but in this case the apb clocks for the primecell devices are
> implemented in a normal platform driver (vexpress_osc_driver), instead
> of using CLK_OF_DECLARE.
Rob Herring
Sep 9, 2015, 1:50:11 AM9/9/15
to
On 09/07/2015 07:23 AM, Tomeu Vizoso wrote:
> Walks the OF tree up and finds the closest ancestor that has a struct
> device associated with it, probing it if isn't bound to a driver yet.
>
> The above should ensure that the dependency represented by the passed OF
> node is available, because probing a device should cause its descendants
> to be probed as well (when they get registered).
>
> Subsystems can use this when looking up resources for drivers, to reduce
> the chances of deferred probes because of the probing order of devices.
>
> Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
Looks pretty good to me. One comment below.
> Walks the OF tree up and finds the closest ancestor that has a struct
> device associated with it, probing it if isn't bound to a driver yet.
>
> The above should ensure that the dependency represented by the passed OF
> node is available, because probing a device should cause its descendants
> to be probed as well (when they get registered).
>
> Subsystems can use this when looking up resources for drivers, to reduce
> the chances of deferred probes because of the probing order of devices.
>
> Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
[...]
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index baf04d7249bd..f089d95ac961 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -269,6 +269,8 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
> goto err_free;
> }
>
> + node->device = &dev->dev;
> +
> return dev;
>
> err_free:
> diff --git a/include/linux/of_device.h b/include/linux/of_device.h
> index cc7dd687a89d..da8d489e73ad 100644
> --- a/include/linux/of_device.h
> +++ b/include/linux/of_device.h
> @@ -40,6 +40,7 @@ extern ssize_t of_device_get_modalias(struct device *dev,
>
> extern void of_device_uevent(struct device *dev, struct kobj_uevent_env *env);
> extern int of_device_uevent_modalias(struct device *dev, struct kobj_uevent_env *env);
> +extern void of_device_probe(struct device_node *np);
>
> static inline void of_device_node_put(struct device *dev)
> {
> @@ -84,6 +85,8 @@ static inline int of_device_uevent_modalias(struct device *dev,
> return -ENODEV;
> }
>
> +static inline void of_device_probe(struct device_node *np) { }
> +
> static inline void of_device_node_put(struct device *dev) { }
>
> static inline const struct of_device_id *__of_match_device(
>
--
Tomeu Vizoso
Sep 9, 2015, 5:50:09 AM9/9/15
to
machine booted and I was able to open a ssh session, but serial was
broken among other AMBA devices:
/memory-controller@2b0a0000
/memory-controller@7ffd0000
/dma@7ffb0000
/smb/motherboard/iofpga@3,00000000/sysctl@020000
/smb/motherboard/iofpga@3,00000000/aaci@040000
/smb/motherboard/iofpga@3,00000000/mmci@050000
/smb/motherboard/iofpga@3,00000000/kmi@060000
/smb/motherboard/iofpga@3,00000000/kmi@070000
/smb/motherboard/iofpga@3,00000000/uart@090000
/smb/motherboard/iofpga@3,00000000/uart@0a0000
/smb/motherboard/iofpga@3,00000000/uart@0b0000
/smb/motherboard/iofpga@3,00000000/uart@0c0000
/smb/motherboard/iofpga@3,00000000/wdt@0f0000
/smb/motherboard/iofpga@3,00000000/timer@110000
/smb/motherboard/iofpga@3,00000000/timer@120000
/smb/motherboard/iofpga@3,00000000/rtc@170000
/smb/motherboard/iofpga@3,00000000/clcd@1f0000
Another way of avoiding this particular problem would be not delaying
the probe of devices in the configuration bus, by doing something like
this:
diff --git a/drivers/bus/vexpress-config.c b/drivers/bus/vexpress-config.c
index 6575c0fe6a4e..eda293869cd3 100644
--- a/drivers/bus/vexpress-config.c
+++ b/drivers/bus/vexpress-config.c
@@ -181,7 +181,7 @@ static int vexpress_config_populate(struct
device_node *node)
if (WARN_ON(!parent))
return -ENODEV;
- return of_platform_populate(node, NULL, NULL, parent);
+ return of_platform_populate_early(node, NULL, NULL, parent);
}
static int __init vexpress_config_init(void)
But I think this would be papering over the underlying issue and it
would be better to have proper explicit dependencies.
Regards,
Tomeu
Tomeu Vizoso
Sep 10, 2015, 11:00:07 AM9/10/15
to
On 9 September 2015 at 03:29, Rob Herring <ro...@kernel.org> wrote:
> On 09/07/2015 07:23 AM, Tomeu Vizoso wrote:
>> Walks the OF tree up and finds the closest ancestor that has a struct
>> device associated with it, probing it if isn't bound to a driver yet.
>>
>> The above should ensure that the dependency represented by the passed OF
>> node is available, because probing a device should cause its descendants
>> to be probed as well (when they get registered).
>>
>> Subsystems can use this when looking up resources for drivers, to reduce
>> the chances of deferred probes because of the probing order of devices.
>>
>> Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
>
> Looks pretty good to me. One comment below.
>
> [...]
>
>> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
>> index baf04d7249bd..f089d95ac961 100644
>> --- a/drivers/of/platform.c
>> +++ b/drivers/of/platform.c
>> @@ -269,6 +269,8 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
>> goto err_free;
>> }
>>
>> + node->device = &dev->dev;
>> +
>
> This seems oddly placed. Can you move to patch 3?
Sure, no problem. Besides this, are you ok with the rest of the series?
> On 09/07/2015 07:23 AM, Tomeu Vizoso wrote:
>> Walks the OF tree up and finds the closest ancestor that has a struct
>> device associated with it, probing it if isn't bound to a driver yet.
>>
>> The above should ensure that the dependency represented by the passed OF
>> node is available, because probing a device should cause its descendants
>> to be probed as well (when they get registered).
>>
>> Subsystems can use this when looking up resources for drivers, to reduce
>> the chances of deferred probes because of the probing order of devices.
>>
>> Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
>
> Looks pretty good to me. One comment below.
>
> [...]
>
>> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
>> index baf04d7249bd..f089d95ac961 100644
>> --- a/drivers/of/platform.c
>> +++ b/drivers/of/platform.c
>> @@ -269,6 +269,8 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
>> goto err_free;
>> }
>>
>> + node->device = &dev->dev;
>> +
>
> This seems oddly placed. Can you move to patch 3?
Regards,
Tomeu
Mark Brown
Sep 11, 2015, 8:10:06 AM9/11/15
to
On Mon, Sep 07, 2015 at 02:23:29PM +0200, Tomeu Vizoso wrote:
> Walks the OF tree up and finds the closest ancestor that has a struct
> device associated with it, probing it if isn't bound to a driver yet.
> The above should ensure that the dependency represented by the passed OF
> node is available, because probing a device should cause its descendants
> to be probed as well (when they get registered).
I'm still not seeing how this works for MFDs where the MFD binding is
> Walks the OF tree up and finds the closest ancestor that has a struct
> device associated with it, probing it if isn't bound to a driver yet.
> The above should ensure that the dependency represented by the passed OF
> node is available, because probing a device should cause its descendants
> to be probed as well (when they get registered).
present directly in DT.
Mark Brown
Sep 11, 2015, 8:10:06 AM9/11/15
to
On Mon, Sep 07, 2015 at 02:23:26PM +0200, Tomeu Vizoso wrote:
> + if (dev->bus && dev->bus->pre_probe) {
> + ret = dev->bus->pre_probe(dev);
> + if (ret) {
> + if (ret == -EPROBE_DEFER)
> + driver_deferred_probe_add(dev);
> + ret = 0;
> + goto out_unlock;
> + }
> + }
So if we get an error other than -EPROBE_DEFER we silently ignore it?
> + if (dev->bus && dev->bus->pre_probe) {
> + ret = dev->bus->pre_probe(dev);
> + if (ret) {
> + if (ret == -EPROBE_DEFER)
> + driver_deferred_probe_add(dev);
> + ret = 0;
> + goto out_unlock;
> + }
> + }
That seems surprising and at least worth a comment.
> + if (dev->bus && dev->bus->pre_probe) {
> + ret = dev->bus->pre_probe(dev);
> + if (ret) {
> + if (ret == -EPROBE_DEFER)
> + driver_deferred_probe_add(dev);
> + goto out;
> + }
> + }
Mark Brown
Sep 11, 2015, 8:20:16 AM9/11/15
to
On Mon, Sep 07, 2015 at 02:23:32PM +0200, Tomeu Vizoso wrote:
> By moving the locking of regulator_list_mutex into regulator_dev_lookup,
> where it is iterated over. The reference count of the regulator's device
> is increased in case it's unregistered while in use.
>
> In _regulator_get() the regulator_list_mutex mutex was held for most of
> the function, but that 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.
The fix for this is to convert the code to use the list maintained by
> By moving the locking of regulator_list_mutex into regulator_dev_lookup,
> where it is iterated over. The reference count of the regulator's device
> is increased in case it's unregistered while in use.
>
> In _regulator_get() the regulator_list_mutex mutex was held for most of
> the function, but that 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.
the driver core for regulator class to iterate the regulators rather
than fiddle about with the locking. I'm pretty sure the current locking
has problems and I'm worried that this may make those problems worse.
Mark Brown
Sep 11, 2015, 8:20:17 AM9/11/15
to
On Mon, Sep 07, 2015 at 02:23:45PM +0200, Tomeu Vizoso wrote:
> Add a field to struct device that instructs the device-driver core to
> defer the probe of this device until the late_initcall level.
>
> By letting all built-in drivers to register before starting to probe, we
> can avoid any deferred probes by probing dependencies on demand.
Is this not going to resut in massive churn as we go through and set
> Add a field to struct device that instructs the device-driver core to
> defer the probe of this device until the late_initcall level.
>
> By letting all built-in drivers to register before starting to probe, we
> can avoid any deferred probes by probing dependencies on demand.
this flag for a massive proportion of drivers? Could we mitigate this
by having a first pass at setting this per subsystem or something so
that we get a good proportion of drivers with changes in core code?
Tomeu Vizoso
Sep 14, 2015, 5:10:06 AM9/14/15
to
of devices (eg. acpi_device_add, of_device_add, etc), as by delaying
the probe of those we are automatically delaying the probe of the
rest.
Regards,
Tomeu
Tomeu Vizoso
Sep 16, 2015, 4:20:06 AM9/16/15
to
nvidia,tegra124-host1x in which children devices are represented in
the DT (and registered right after their parent) and depend on their
parent for their operation.
Looked at why it wasn't being a problem in my tests and Thierry
mentioned that tegra_host1x_driver takes care of the synchronization
between the bus and their children. So children would make use of the
bus only once it has finished probing and is ready to work (the init
and exit callbacks in host1x_client_ops signal when the bus is safe to
use).
AFAICS, this is a must currently for correct operation in simple-bus
and simple-mfd situations, because probing order is currently very
unpredictable and it's totally possible for the probing of a child to
start before the probing of its parent has finished (if async probing
is enabled or if a module is loaded that registers a child's driver at
the wrong time).
I would prefer if the core would take care of making sure that parents
are always probed before their children, but the unconditional locking
of the parent device stands in the way.
Mark Brown
Sep 16, 2015, 3:40:08 PM9/16/15
to
On Wed, Sep 16, 2015 at 10:17:43AM +0200, Tomeu Vizoso wrote:
> AFAICS, this is a must currently for correct operation in simple-bus
> and simple-mfd situations, because probing order is currently very
> unpredictable and it's totally possible for the probing of a child to
> start before the probing of its parent has finished (if async probing
> is enabled or if a module is loaded that registers a child's driver at
> the wrong time).
Hrm. That's worrying for devices that rely on peering into their parent
> AFAICS, this is a must currently for correct operation in simple-bus
> and simple-mfd situations, because probing order is currently very
> unpredictable and it's totally possible for the probing of a child to
> start before the probing of its parent has finished (if async probing
> is enabled or if a module is loaded that registers a child's driver at
> the wrong time).
device for things...
> I would prefer if the core would take care of making sure that parents
> are always probed before their children, but the unconditional locking
> of the parent device stands in the way.
structure into the DT - if it isn't then the ordering is controlled by
the device being instantiated.
Tomeu Vizoso
Sep 17, 2015, 9:00:09 AM9/17/15
to
linux-next (20150917) to kernelci.org and I don't see any issues that
could be caused by them.
With this series I get the kernel to output to the panel in 0.5s,
instead of 2.8s.
Regards,
Tomeu
[0] http://lists.freedesktop.org/archives/dri-devel/2014-August/066527.html
[1] https://lkml.org/lkml/2014/5/12/452
[2] https://lkml.org/lkml/2015/6/17/305
[3] http://article.gmane.org/gmane.linux.ports.arm.kernel/277689
[4] https://lkml.org/lkml/2015/7/21/441a
[5] https://git.collabora.com/cgit/user/tomeu/linux.git/log/?h=on-demand-probes-v7
instead of 2.8s.
Regards,
Tomeu
[0] http://lists.freedesktop.org/archives/dri-devel/2014-August/066527.html
[1] https://lkml.org/lkml/2014/5/12/452
[2] https://lkml.org/lkml/2015/6/17/305
[3] http://article.gmane.org/gmane.linux.ports.arm.kernel/277689
[4] https://lkml.org/lkml/2015/7/21/441a
[6] http://kernelci.org/boot/all/job/collabora/kernel/v4.3-rc1-2248-g115f59aaec1f/
[7] http://kernelci.org/boot/all/job/next/kernel/next-20150917
Changes in v5:
- Reduce some code duplication by adding device_pre_probe()
- Print a warning if pre_probe() returns an error
- Set the pointer to struct device also for AMBA devices
- Unset the pointer to struct device when the platform device is about
to be unregistered
- Increase the reference count of the device before returning from
of_find_device_by_node()
- Move the assignment to device_node->device for AMBA devices to another
commit.
- Hold a reference to the struct device while it's in use in
of_device_probe().
- Use regulator_class' klist of devices instead of regulator_list to
store and lookup regulator devices.
Changes in v4:
- Added bus.pre_probe callback so the probes of Primecell devices can be
deferred if their device IDs cannot be yet read because of the clock
driver not having probed when they are registered. Maybe this goes
overboard and the matching information should be in the DT if there is
one.
- Use device_node.device instead of device_node.platform_dev
- Take a reference to the regulator's device to prevent dangling
pointers
- Add Kconfig DELAY_DEVICE_PROBES to allow disabling delayed probing in
pointers
machines with initcalls that depend on devices probing at a given time.
- Start processing deferred probes in device_initcall_sync
- Also defer probes of AMBA devices registered from the DT as they can
also request resources.
Changes in v3:
- Set and use device_node.platform_dev instead of reversing the logic to
also request resources.
Changes in v3:
find the platform device that encloses a device node.
- Drop the fwnode API to probe firmware nodes and add OF-only API for
now. I think this same scheme could be used for machines with ACPI,
but I haven't been able to find one that had to defer its probes because
of the device probe order.
- Avoid unlocking the regulator device's mutex if we don't have a device
Changes in v2:
- Acquire regulator device lock before returning from
regulator_dev_lookup()
Tomeu Vizoso (23):
Changes in v2:
- Acquire regulator device lock before returning from
regulator_dev_lookup()
driver core: Add pre_probe callback to bus_type
ARM: amba: Move reading of periphid to pre_probe()
of/platform: Point to struct device from device node
of: add function to allow probing a device from a OF node
gpio: Probe GPIO drivers on demand
gpio: Probe pinctrl devices on demand
regulator: core: Remove regulator_list
regulator: core: Drop redundant locking
regulator: core: Probe regulators on demand
drm: Probe panels on demand
drm/tegra: Probe dpaux devices on demand
i2c: core: Probe i2c adapters and devices on demand
pwm: Probe PWM chip devices on demand
backlight: Probe backlight devices on demand
usb: phy: Probe phy devices on demand
clk: Probe clk providers on demand
pinctrl: Probe pinctrl devices on demand
phy: core: Probe phy providers on demand
dma: of: Probe DMA controllers on demand
power-supply: Probe power supplies on demand
driver core: Allow deferring probes until late init
driver core: Start processing deferred probes earlier
of/platform: Defer probes of registered devices
drivers/amba/bus.c | 78 ++++++------
drivers/base/Kconfig | 18 +++
drivers/base/dd.c | 50 +++++++-
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 | 4 +
drivers/of/device.c | 61 +++++++++
drivers/of/platform.c | 30 +++--
drivers/phy/phy-core.c | 3 +
drivers/pinctrl/devicetree.c | 3 +
drivers/power/power_supply_core.c | 3 +
drivers/pwm/core.c | 3 +
drivers/regulator/core.c | 246 +++++++++++++++++++++++-------------
drivers/usb/phy/phy.c | 3 +
drivers/video/backlight/backlight.c | 3 +
include/linux/device.h | 6 +
include/linux/of.h | 1 +
include/linux/of_device.h | 3 +
21 files changed, 387 insertions(+), 145 deletions(-)
--
2.4.3
Tomeu Vizoso
Sep 17, 2015, 9:10:10 AM9/17/15
to
When looking up a clock through its OF node, probe it if it hasn't
index 43e2c3ad6c31..e5fe02a11c36 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -19,6 +19,7 @@
#include <linux/list.h>
#include <linux/init.h>
#include <linux/sched.h>
@@ -3004,6 +3005,8 @@ struct clk *__of_clk_get_from_provider(struct of_phandle_args *clkspec,
if (!clkspec)
return ERR_PTR(-EINVAL);
+ of_device_probe(clkspec->np);
+
/* Check if we have such a provider in our array */
mutex_lock(&of_clk_mutex);
list_for_each_entry(provider, &of_clk_providers, link) {
already.
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
---
Changes in v5: None
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
---
Changes in v4: None
Changes in v3: None
Changes in v2: None
drivers/clk/clk.c | 3 +++
Changes in v3: None
Changes in v2: None
1 file changed, 3 insertions(+)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 43e2c3ad6c31..e5fe02a11c36 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -19,6 +19,7 @@
#include <linux/list.h>
#include <linux/slab.h>
#include <linux/of.h>
+#include <linux/of_device.h>
#include <linux/device.h>
#include <linux/of.h>
+#include <linux/of_device.h>
#include <linux/init.h>
#include <linux/sched.h>
@@ -3004,6 +3005,8 @@ struct clk *__of_clk_get_from_provider(struct of_phandle_args *clkspec,
if (!clkspec)
return ERR_PTR(-EINVAL);
+ of_device_probe(clkspec->np);
+
/* Check if we have such a provider in our array */
mutex_lock(&of_clk_mutex);
list_for_each_entry(provider, &of_clk_providers, link) {
Tomeu Vizoso
Sep 21, 2015, 10:10:09 AM9/21/15
to
could be caused by them.
With this series I get the kernel to output to the panel in 0.5s,
instead of 2.8s.
Regards,
Tomeu
[0] http://lists.freedesktop.org/archives/dri-devel/2014-August/066527.html
[1] https://lkml.org/lkml/2014/5/12/452
[2] https://lkml.org/lkml/2015/6/17/305
[3] http://article.gmane.org/gmane.linux.ports.arm.kernel/277689
[4] https://lkml.org/lkml/2015/7/21/441a
[5] https://git.collabora.com/cgit/user/tomeu/linux.git/log/?h=on-demand-probes-v8
With this series I get the kernel to output to the panel in 0.5s,
instead of 2.8s.
Regards,
Tomeu
[0] http://lists.freedesktop.org/archives/dri-devel/2014-August/066527.html
[1] https://lkml.org/lkml/2014/5/12/452
[2] https://lkml.org/lkml/2015/6/17/305
[3] http://article.gmane.org/gmane.linux.ports.arm.kernel/277689
[4] https://lkml.org/lkml/2015/7/21/441a
[6] http://kernelci.org/boot/all/job/collabora/kernel/v4.3-rc2-2587-gf92b0ab33d14/
[7] http://kernelci.org/boot/all/job/next/kernel/next-20150921
Changes in v6:
- Drop bus_type.pre_probe and read the periphid in match() instead as
suggested by Alan Stern.
- Merge changes to the regulator subsystem's locking so no references
are leaked between commits.
Changes in v5:
- Set the pointer to struct device also for AMBA devices
- Unset the pointer to struct device when the platform device is about
to be unregistered
- Increase the reference count of the device before returning from
of_find_device_by_node()
- Move the assignment to device_node->device for AMBA devices to another
commit.
- Hold a reference to the struct device while it's in use in
of_device_probe().
- Use regulator_class' klist of devices instead of regulator_list to
store and lookup regulator devices.
Changes in v4:
- Added bus.pre_probe callback so the probes of Primecell devices can be
deferred if their device IDs cannot be yet read because of the clock
driver not having probed when they are registered. Maybe this goes
overboard and the matching information should be in the DT if there is
one.
- Rename of_platform_probe to of_device_probe
- Use device_node.device instead of device_node.platform_dev
- Unset the pointer to struct device when the platform device is about
to be unregistered
- Increase the reference count of the device before returning from
of_find_device_by_node()
- Move the assignment to device_node->device for AMBA devices to another
commit.
- Hold a reference to the struct device while it's in use in
of_device_probe().
- Use regulator_class' klist of devices instead of regulator_list to
store and lookup regulator devices.
Changes in v4:
- Added bus.pre_probe callback so the probes of Primecell devices can be
deferred if their device IDs cannot be yet read because of the clock
driver not having probed when they are registered. Maybe this goes
overboard and the matching information should be in the DT if there is
one.
- Rename of_platform_probe to of_device_probe
- Use device_node.device instead of device_node.platform_dev
- Add Kconfig DELAY_DEVICE_PROBES to allow disabling delayed probing in
machines with initcalls that depend on devices probing at a given time.
- Start processing deferred probes in device_initcall_sync
- Also defer probes of AMBA devices registered from the DT as they can
also request resources.
Changes in v3:
- Set and use device_node.platform_dev instead of reversing the logic to
find the platform device that encloses a device node.
- Drop the fwnode API to probe firmware nodes and add OF-only API for
now. I think this same scheme could be used for machines with ACPI,
but I haven't been able to find one that had to defer its probes because
of the device probe order.
Tomeu Vizoso (22):
machines with initcalls that depend on devices probing at a given time.
- Start processing deferred probes in device_initcall_sync
- Also defer probes of AMBA devices registered from the DT as they can
also request resources.
Changes in v3:
- Set and use device_node.platform_dev instead of reversing the logic to
find the platform device that encloses a device node.
- Drop the fwnode API to probe firmware nodes and add OF-only API for
now. I think this same scheme could be used for machines with ACPI,
but I haven't been able to find one that had to defer its probes because
of the device probe order.
driver core: handle -EPROBE_DEFER from bus_type.match()
ARM: amba: Move reading of periphid to amba_match()
of/platform: Point to struct device from device node
of: add function to allow probing a device from a OF node
gpio: Probe GPIO drivers on demand
gpio: Probe pinctrl devices on demand
regulator: core: Remove regulator_list
of: add function to allow probing a device from a OF node
gpio: Probe GPIO drivers on demand
gpio: Probe pinctrl devices on demand
regulator: core: Remove regulator_list
regulator: core: Probe regulators on demand
drm: Probe panels on demand
drm/tegra: Probe dpaux devices on demand
i2c: core: Probe i2c adapters and devices on demand
pwm: Probe PWM chip devices on demand
backlight: Probe backlight devices on demand
usb: phy: Probe phy devices on demand
clk: Probe clk providers on demand
pinctrl: Probe pinctrl devices on demand
phy: core: Probe phy providers on demand
dma: of: Probe DMA controllers on demand
power-supply: Probe power supplies on demand
driver core: Allow deferring probes until late init
driver core: Start processing deferred probes earlier
of/platform: Defer probes of registered devices
drivers/amba/bus.c | 88 ++++++------
drm: Probe panels on demand
drm/tegra: Probe dpaux devices on demand
i2c: core: Probe i2c adapters and devices on demand
pwm: Probe PWM chip devices on demand
backlight: Probe backlight devices on demand
usb: phy: Probe phy devices on demand
clk: Probe clk providers on demand
pinctrl: Probe pinctrl devices on demand
phy: core: Probe phy providers on demand
dma: of: Probe DMA controllers on demand
power-supply: Probe power supplies on demand
driver core: Allow deferring probes until late init
driver core: Start processing deferred probes earlier
of/platform: Defer probes of registered devices
drivers/base/Kconfig | 18 +++
drivers/base/dd.c | 35 ++++-
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 | 4 +
drivers/of/device.c | 61 +++++++++
drivers/of/platform.c | 30 +++--
drivers/phy/phy-core.c | 3 +
drivers/pinctrl/devicetree.c | 3 +
drivers/power/power_supply_core.c | 3 +
drivers/pwm/core.c | 3 +
drivers/regulator/core.c | 257 +++++++++++++++++++++++-------------
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 | 4 +
drivers/of/device.c | 61 +++++++++
drivers/of/platform.c | 30 +++--
drivers/phy/phy-core.c | 3 +
drivers/pinctrl/devicetree.c | 3 +
drivers/power/power_supply_core.c | 3 +
drivers/pwm/core.c | 3 +
drivers/usb/phy/phy.c | 3 +
drivers/video/backlight/backlight.c | 3 +
include/linux/device.h | 4 +-
drivers/video/backlight/backlight.c | 3 +
include/linux/of.h | 1 +
include/linux/of_device.h | 3 +
21 files changed, 386 insertions(+), 150 deletions(-)
include/linux/of_device.h | 3 +
Linus Walleij
Sep 25, 2015, 1:10:08 PM9/25/15
to
On Mon, Sep 7, 2015 at 5:23 AM, Tomeu Vizoso <tomeu....@collabora.com> wrote:
Nit: prefix patch with "pinctrl:"
> When looking up a pin controller through its OF node, probe it if it
Yours,
Linus Walleij
Nit: prefix patch with "pinctrl:"
> When looking up a pin controller through its OF node, probe it if it
> hasn't already.
>
> The goal is to reduce deferred probes to a minimum, as it makes it very
> cumbersome to find out why a device failed to probe, and can introduce
> very big delays in when a critical device is probed.
>
> Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
> ---
>
>
> The goal is to reduce deferred probes to a minimum, as it makes it very
> cumbersome to find out why a device failed to probe, and can introduce
> very big delays in when a critical device is probed.
>
> Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
> ---
>
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
Acked-by: Linus Walleij <linus....@linaro.org>
> Changes in v3: None
> Changes in v2: None
Yours,
Linus Walleij
Linus Walleij
Sep 25, 2015, 1:10:08 PM9/25/15
to
Linus Walleij
Sep 25, 2015, 1:10:08 PM9/25/15
to
> When looking up a gpiochip through its firmware node, probe it if it
> hasn't already.
>
> The goal is to reduce deferred probes to a minimum, as it makes it very
> cumbersome to find out why a device failed to probe, and can introduce
> very big delays in when a critical device is probed.
>
> Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
> ---
>
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
Acked-by: Linus Walleij <linus....@linaro.org>
That is if the whole thing goes in as-is for the GPIO parts.
>
> The goal is to reduce deferred probes to a minimum, as it makes it very
> cumbersome to find out why a device failed to probe, and can introduce
> very big delays in when a critical device is probed.
>
> Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
> ---
>
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
Acked-by: Linus Walleij <linus....@linaro.org>
Rob Herring
Sep 26, 2015, 2:20:07 PM9/26/15
to
to see ack's from Greg and some reviewed-bys from others. The subsystem
changes are minor and there has been plenty of chance to comment, so I
don't think acks from all subsystems are needed.
Your branch is based on -next. Is there any dependence on something in
-next? I want to get this into -next soon, but need a branch not based
on -next. Please send me a pull request with the collected acks and
minor comments I have addressed.
Rob
Greg Kroah-Hartman
Sep 26, 2015, 3:30:09 PM9/26/15
to
thanks,
greg k-h
Tomeu Vizoso
Sep 29, 2015, 4:10:13 AM9/29/15
to
without 21 and with the minor changes you suggested, and once Greg is
happy I can send the pull request.
Thanks,
Tomeu
Tomeu Vizoso
Sep 29, 2015, 5:20:07 AM9/29/15
to
When looking up a gpiochip through its firmware node, probe it if it
hasn't already.
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
Acked-by: Linus Walleij <linus....@linaro.org>
hasn't already.
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@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 fa6e3c8823d6..9a439dab7a87 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -19,6 +19,7 @@
#include <linux/gpio/consumer.h>
#include <linux/of.h>
#include <linux/of_address.h>
+#include <linux/of_device.h>
#include <linux/of_gpio.h>
#include <linux/pinctrl/pinctrl.h>
#include <linux/slab.h>
@@ -95,6 +96,8 @@ struct gpio_desc *of_get_named_gpiod_flags(struct device_node *np,
return ERR_PTR(ret);
}
+ of_device_probe(gg_data.gpiospec.np);
+
gpiochip_find(&gg_data, of_gpiochip_find_and_xlate);
of_node_put(gg_data.gpiospec.np);
--
2.4.3
Tomeu Vizoso
Sep 29, 2015, 5:20:08 AM9/29/15
to
When looking up a DMA controller through its OF node, probe it if it
drivers/dma/of-dma.c | 3 +++
index 1e1f2986eba8..e899832f7df3 100644
--- a/drivers/dma/of-dma.c
+++ b/drivers/dma/of-dma.c
@@ -16,6 +16,7 @@
#include <linux/mutex.h>
static LIST_HEAD(of_dma_list);
@@ -263,6 +264,8 @@ struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
if (of_dma_match_channel(np, name, i, &dma_spec))
continue;
+ of_device_probe(dma_spec.np);
+
mutex_lock(&of_dma_lock);
ofdma = of_dma_find_controller(&dma_spec);
hasn't already.
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
---
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@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 1e1f2986eba8..e899832f7df3 100644
--- a/drivers/dma/of-dma.c
+++ b/drivers/dma/of-dma.c
@@ -16,6 +16,7 @@
#include <linux/mutex.h>
#include <linux/slab.h>
#include <linux/of.h>
+#include <linux/of_device.h>
#include <linux/of_dma.h>
#include <linux/of.h>
+#include <linux/of_device.h>
static LIST_HEAD(of_dma_list);
@@ -263,6 +264,8 @@ struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
if (of_dma_match_channel(np, name, i, &dma_spec))
continue;
+ of_device_probe(dma_spec.np);
+
mutex_lock(&of_dma_lock);
ofdma = of_dma_find_controller(&dma_spec);
Tomeu Vizoso
Sep 29, 2015, 5:20:08 AM9/29/15
to
When looking up a PWM chip through its OF node, probe it if it hasn't
index 3f9df3ea3350..794a923df0d8 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -29,6 +29,7 @@
#include <linux/device.h>
#include <linux/debugfs.h>
#include <linux/seq_file.h>
+#include <linux/of_device.h>
#include <dt-bindings/pwm/pwm.h>
@@ -516,6 +517,8 @@ static struct pwm_chip *of_node_to_pwmchip(struct device_node *np)
{
struct pwm_chip *chip;
+ of_device_probe(np);
+
mutex_lock(&pwm_lock);
list_for_each_entry(chip, &pwm_chips, list)
already.
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
---
drivers/pwm/core.c | 3 +++
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
---
1 file changed, 3 insertions(+)
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 3f9df3ea3350..794a923df0d8 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -29,6 +29,7 @@
#include <linux/device.h>
#include <linux/debugfs.h>
#include <linux/seq_file.h>
+#include <linux/of_device.h>
#include <dt-bindings/pwm/pwm.h>
@@ -516,6 +517,8 @@ static struct pwm_chip *of_node_to_pwmchip(struct device_node *np)
{
struct pwm_chip *chip;
+ of_device_probe(np);
+
mutex_lock(&pwm_lock);
list_for_each_entry(chip, &pwm_chips, list)
Tomeu Vizoso
Sep 29, 2015, 5:20:08 AM9/29/15
to
When looking up a regulator through its OF node, probe it if it hasn't
---
drivers/regulator/core.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 7a85ac9e32c5..ee75199c2cfa 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_device.h>
#include <linux/regmap.h>
#include <linux/regulator/of_regulator.h>
#include <linux/regulator/consumer.h>
@@ -1340,6 +1341,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_device_probe(node);
list_for_each_entry(r, ®ulator_list, list)
if (r->dev.parent &&
node == r->dev.of_node)
already.
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
Acked-by: Mark Brown <bro...@kernel.org>
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@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 7a85ac9e32c5..ee75199c2cfa 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_device.h>
#include <linux/regmap.h>
#include <linux/regulator/of_regulator.h>
#include <linux/regulator/consumer.h>
@@ -1340,6 +1341,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_device_probe(node);
list_for_each_entry(r, ®ulator_list, list)
if (r->dev.parent &&
node == r->dev.of_node)
Tomeu Vizoso
Sep 29, 2015, 5:20:08 AM9/29/15
to
When looking up a backlight device through its OF node, probe it if it
drivers/video/backlight/backlight.c | 3 +++
index bddc8b17a4d8..9bcdc16eacdf 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -16,6 +16,7 @@
#include <linux/err.h>
#include <linux/fb.h>
#include <linux/slab.h>
+#include <linux/of_device.h>
#ifdef CONFIG_PMAC_BACKLIGHT
#include <asm/backlight.h>
@@ -559,6 +560,8 @@ struct backlight_device *of_find_backlight_by_node(struct device_node *node)
{
struct device *dev;
+ of_device_probe(node);
+
dev = class_find_device(backlight_class, NULL, node, of_parent_match);
return dev ? to_backlight_device(dev) : NULL;
hasn't already.
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
---
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@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 bddc8b17a4d8..9bcdc16eacdf 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -16,6 +16,7 @@
#include <linux/err.h>
#include <linux/fb.h>
#include <linux/slab.h>
+#include <linux/of_device.h>
#ifdef CONFIG_PMAC_BACKLIGHT
#include <asm/backlight.h>
@@ -559,6 +560,8 @@ struct backlight_device *of_find_backlight_by_node(struct device_node *node)
{
struct device *dev;
+ of_device_probe(node);
+
dev = class_find_device(backlight_class, NULL, node, of_parent_match);
return dev ? to_backlight_device(dev) : NULL;
Tomeu Vizoso
Sep 29, 2015, 5:20:08 AM9/29/15
to
Walks the OF tree up and finds the closest ancestor that has a struct
device associated with it, probing it if isn't bound to a driver yet.
The above should ensure that the dependency represented by the passed OF
node is available, because probing a device should cause its descendants
to be probed as well (when they get registered).
device associated with it, probing it if isn't bound to a driver yet.
The above should ensure that the dependency represented by the passed OF
node is available, because probing a device should cause its descendants
to be probed as well (when they get registered).
Subsystems can use this when looking up resources for drivers, to reduce
the chances of deferred probes because of the probing order of devices.
Changes in v5:
the chances of deferred probes because of the probing order of devices.
- Move the assignment to device_node->device for AMBA devices to another
commit.
- Hold a reference to the struct device while it's in use in
of_device_probe().
Changes in v4:
commit.
- Hold a reference to the struct device while it's in use in
of_device_probe().
- Rename of_platform_probe to of_device_probe
- Use device_node.device instead of device_node.platform_dev
- Use device_node.device instead of device_node.platform_dev
Changes in v3:
- Set and use device_node.platform_dev instead of reversing the logic to
find the platform device that encloses a device node.
- Drop the fwnode API to probe firmware nodes and add OF-only API for
now. I think this same scheme could be used for machines with ACPI,
but I haven't been able to find one that had to defer its probes because
of the device probe order.
drivers/of/device.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++
- Set and use device_node.platform_dev instead of reversing the logic to
find the platform device that encloses a device node.
- Drop the fwnode API to probe firmware nodes and add OF-only API for
now. I think this same scheme could be used for machines with ACPI,
but I haven't been able to find one that had to defer its probes because
of the device probe order.
include/linux/of_device.h | 3 +++
2 files changed, 64 insertions(+)
diff --git a/drivers/of/device.c b/drivers/of/device.c
index 8b91ea241b10..836be71fc90e 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -286,3 +286,64 @@ int of_device_uevent_modalias(struct device *dev, struct kobj_uevent_env *env)
return 0;
}
+
+/**
+ * of_device_probe() - Probe device associated with OF node
+ * @np: node to probe
+ *
+ * Probe the device associated with the passed device node.
+ */
+void of_device_probe(struct device_node *np)
+{
+ struct device_node *target;
+ struct device *dev = NULL;
+
+ if (!of_root || !of_node_check_flag(of_root, OF_POPULATED_BUS))
+ return;
+
+ if (!np)
+ return;
+
+ of_node_get(np);
+
+ /* Find the closest ancestor that has a device associated */
+ for (target = np;
+ !of_node_is_root(target);
+ target = of_get_next_parent(target))
+ if (get_device(target->device)) {
+ dev = target->device;
+ break;
+ }
+
+ of_node_put(target);
+
+ if (!dev) {
+ pr_warn("Couldn't find a device for node '%s'\n",
+ of_node_full_name(np));
+ return;
+ }
+
+ /*
+ * Device is bound or is being probed right now. If we have bad luck
+ * and the dependency isn't ready when it's needed, deferred probe
+ * will save us.
+ */
+ if (dev->driver)
+ goto out;
+
+ /*
+ * Probing a device should cause its descendants to be probed as
+ * well, which includes the passed device node.
+ */
+ if (device_attach(dev) != 1)
+ /*
+ * This cannot be a warning for now because clock nodes have a
+ * compatible string but the clock framework doesn't follow
+ * the device/driver model yet.
+ */
+ dev_dbg(dev, "Probe failed for %s\n", of_node_full_name(np));
+
+out:
+ put_device(dev);
+}
+EXPORT_SYMBOL_GPL(of_device_probe);
diff --git a/include/linux/of_device.h b/include/linux/of_device.h
index cc7dd687a89d..da8d489e73ad 100644
--- a/include/linux/of_device.h
+++ b/include/linux/of_device.h
@@ -40,6 +40,7 @@ extern ssize_t of_device_get_modalias(struct device *dev,
extern void of_device_uevent(struct device *dev, struct kobj_uevent_env *env);
extern int of_device_uevent_modalias(struct device *dev, struct kobj_uevent_env *env);
+extern void of_device_probe(struct device_node *np);
static inline void of_device_node_put(struct device *dev)
{
@@ -84,6 +85,8 @@ static inline int of_device_uevent_modalias(struct device *dev,
return -ENODEV;
}
+static inline void of_device_probe(struct device_node *np) { }
+
static inline void of_device_node_put(struct device *dev) { }
static inline const struct of_device_id *__of_match_device(
index cc7dd687a89d..da8d489e73ad 100644
--- a/include/linux/of_device.h
+++ b/include/linux/of_device.h
@@ -40,6 +40,7 @@ extern ssize_t of_device_get_modalias(struct device *dev,
extern void of_device_uevent(struct device *dev, struct kobj_uevent_env *env);
extern int of_device_uevent_modalias(struct device *dev, struct kobj_uevent_env *env);
+extern void of_device_probe(struct device_node *np);
static inline void of_device_node_put(struct device *dev)
{
@@ -84,6 +85,8 @@ static inline int of_device_uevent_modalias(struct device *dev,
return -ENODEV;
}
+static inline void of_device_probe(struct device_node *np) { }
+
static inline void of_device_node_put(struct device *dev) { }
static inline const struct of_device_id *__of_match_device(
Tomeu Vizoso
Sep 29, 2015, 5:20:08 AM9/29/15
to
Instead of trying to match and probe platform and AMBA devices right
after each is registered, delay their probes until device_initcall_sync.
This means that devices will start probing once all built-in drivers
have registered, and after all platform and AMBA devices from the DT
have been registered already.
This allows us to prevent deferred probes by probing dependencies on
demand.
Changes in v7:
- Removed patch that moved deferred probe processing to
device_initcall_sync, as it's not currently needed and can be applied
later if we find any boards that fail to boot without this change.
Changes in v4:
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 408d89f1d124..7b33e0369374 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -164,7 +164,8 @@ static struct platform_device *of_platform_device_create_pdata(
struct device_node *np,
const char *bus_id,
void *platform_data,
- struct device *parent)
+ struct device *parent,
+ bool probe_late)
{
struct platform_device *dev;
@@ -178,6 +179,7 @@ static struct platform_device *of_platform_device_create_pdata(
dev->dev.bus = &platform_bus_type;
dev->dev.platform_data = platform_data;
+ dev->dev.probe_late = probe_late;
of_dma_configure(&dev->dev, dev->dev.of_node);
of_msi_configure(&dev->dev, dev->dev.of_node);
@@ -209,7 +211,8 @@ struct platform_device *of_platform_device_create(struct device_node *np,
const char *bus_id,
struct device *parent)
{
- return of_platform_device_create_pdata(np, bus_id, NULL, parent);
+ return of_platform_device_create_pdata(np, bus_id, NULL, parent,
+ false);
}
EXPORT_SYMBOL(of_platform_device_create);
@@ -240,6 +243,7 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
dev->dev.of_node = of_node_get(node);
dev->dev.parent = parent ? : &platform_bus;
dev->dev.platform_data = platform_data;
+ dev->dev.probe_late = true;
if (bus_id)
dev_set_name(&dev->dev, "%s", bus_id);
else
@@ -358,7 +362,8 @@ static int of_platform_bus_create(struct device_node *bus,
return 0;
}
- dev = of_platform_device_create_pdata(bus, bus_id, platform_data, parent);
+ dev = of_platform_device_create_pdata(bus, bus_id, platform_data,
+ parent, true);
if (!dev || !of_match_node(matches, bus))
return 0;
after each is registered, delay their probes until device_initcall_sync.
This means that devices will start probing once all built-in drivers
have registered, and after all platform and AMBA devices from the DT
have been registered already.
This allows us to prevent deferred probes by probing dependencies on
demand.
Changes in v7:
- Removed patch that moved deferred probe processing to
device_initcall_sync, as it's not currently needed and can be applied
later if we find any boards that fail to boot without this change.
Changes in v4:
- Also defer probes of AMBA devices registered from the DT as they can
also request resources.
drivers/of/platform.c | 11 ++++++++---
also request resources.
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 408d89f1d124..7b33e0369374 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -164,7 +164,8 @@ static struct platform_device *of_platform_device_create_pdata(
struct device_node *np,
const char *bus_id,
void *platform_data,
- struct device *parent)
+ struct device *parent,
+ bool probe_late)
{
struct platform_device *dev;
@@ -178,6 +179,7 @@ static struct platform_device *of_platform_device_create_pdata(
dev->dev.bus = &platform_bus_type;
dev->dev.platform_data = platform_data;
+ dev->dev.probe_late = probe_late;
of_dma_configure(&dev->dev, dev->dev.of_node);
of_msi_configure(&dev->dev, dev->dev.of_node);
@@ -209,7 +211,8 @@ struct platform_device *of_platform_device_create(struct device_node *np,
const char *bus_id,
struct device *parent)
{
- return of_platform_device_create_pdata(np, bus_id, NULL, parent);
+ return of_platform_device_create_pdata(np, bus_id, NULL, parent,
+ false);
}
EXPORT_SYMBOL(of_platform_device_create);
@@ -240,6 +243,7 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
dev->dev.of_node = of_node_get(node);
dev->dev.parent = parent ? : &platform_bus;
dev->dev.platform_data = platform_data;
+ dev->dev.probe_late = true;
if (bus_id)
dev_set_name(&dev->dev, "%s", bus_id);
else
@@ -358,7 +362,8 @@ static int of_platform_bus_create(struct device_node *bus,
return 0;
}
- dev = of_platform_device_create_pdata(bus, bus_id, platform_data, parent);
+ dev = of_platform_device_create_pdata(bus, bus_id, platform_data,
+ parent, true);
if (!dev || !of_match_node(matches, bus))
return 0;
Tomeu Vizoso
Sep 29, 2015, 5:20:08 AM9/29/15
to
When looking up a pin controller through its OF node, probe it if it
index 9a439dab7a87..05da9a56608d 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -359,6 +359,8 @@ static int of_gpiochip_add_pin_range(struct gpio_chip *chip)
if (ret)
break;
+ of_device_probe(pinspec.np);
+
pctldev = of_pinctrl_get(pinspec.np);
if (!pctldev)
return -EPROBE_DEFER;
hasn't already.
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
drivers/gpio/gpiolib-of.c | 2 ++
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
1 file changed, 2 insertions(+)
diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 9a439dab7a87..05da9a56608d 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -359,6 +359,8 @@ static int of_gpiochip_add_pin_range(struct gpio_chip *chip)
if (ret)
break;
+ of_device_probe(pinspec.np);
+
pctldev = of_pinctrl_get(pinspec.np);
if (!pctldev)
return -EPROBE_DEFER;
Tomeu Vizoso
Sep 29, 2015, 5:20:08 AM9/29/15
to
linux-next (20150925) to kernelci.org and I don't see any issues that
could be caused by them.
With this series I get the kernel to output to the panel in 0.5s,
instead of 2.8s.
With this series I get the kernel to output to the panel in 0.5s,
instead of 2.8s.
Regards,
Tomeu
[0] http://lists.freedesktop.org/archives/dri-devel/2014-August/066527.html
[1] https://lkml.org/lkml/2014/5/12/452
[2] https://lkml.org/lkml/2015/6/17/305
[3] http://article.gmane.org/gmane.linux.ports.arm.kernel/277689
[4] https://lkml.org/lkml/2015/7/21/441a
[5] https://git.collabora.com/cgit/user/tomeu/linux.git/log/?h=on-demand-probes-v9
Tomeu
[0] http://lists.freedesktop.org/archives/dri-devel/2014-August/066527.html
[1] https://lkml.org/lkml/2014/5/12/452
[2] https://lkml.org/lkml/2015/6/17/305
[3] http://article.gmane.org/gmane.linux.ports.arm.kernel/277689
[4] https://lkml.org/lkml/2015/7/21/441a
[6] http://kernelci.org/boot/all/job/collabora/kernel/v4.3-rc2-3497-g96d8df1bbcc1/
[7] http://kernelci.org/boot/all/job/next/kernel/next-20150925
Changes in v7:
- Move IS_ENABLED(CONFIG_DELAY_DEVICE_PROBES) into if condition
- Hide CONFIG_DELAY_DEVICE_PROBES behind EXPERT
- Removed patch that moved deferred probe processing to
device_initcall_sync, as it's not currently needed and can be applied
later if we find any boards that fail to boot without this change.
device_initcall_sync, as it's not currently needed and can be applied
later if we find any boards that fail to boot without this change.
Changes in v6:
- Drop bus_type.pre_probe and read the periphid in match() instead as
suggested by Alan Stern.
- Drop bus_type.pre_probe and read the periphid in match() instead as
suggested by Alan Stern.
Changes in v5:
- Set the pointer to struct device also for AMBA devices
- Unset the pointer to struct device when the platform device is about
to be unregistered
- Increase the reference count of the device before returning from
of_find_device_by_node()
- Set the pointer to struct device also for AMBA devices
- Unset the pointer to struct device when the platform device is about
to be unregistered
- Increase the reference count of the device before returning from
of_find_device_by_node()
- Move the assignment to device_node->device for AMBA devices to another
commit.
- Hold a reference to the struct device while it's in use in
of_device_probe().
Changes in v4:
commit.
- Hold a reference to the struct device while it's in use in
of_device_probe().
Changes in v4:
- Added bus.pre_probe callback so the probes of Primecell devices can be
deferred if their device IDs cannot be yet read because of the clock
driver not having probed when they are registered. Maybe this goes
overboard and the matching information should be in the DT if there is
one.
deferred if their device IDs cannot be yet read because of the clock
driver not having probed when they are registered. Maybe this goes
overboard and the matching information should be in the DT if there is
one.
- Rename of_platform_probe to of_device_probe
- Use device_node.device instead of device_node.platform_dev
- Use device_node.device instead of device_node.platform_dev
- Add Kconfig DELAY_DEVICE_PROBES to allow disabling delayed probing in
machines with initcalls that depend on devices probing at a given time.
machines with initcalls that depend on devices probing at a given time.
- Also defer probes of AMBA devices registered from the DT as they can
also request resources.
also request resources.
Changes in v3:
- Set and use device_node.platform_dev instead of reversing the logic to
find the platform device that encloses a device node.
- Drop the fwnode API to probe firmware nodes and add OF-only API for
now. I think this same scheme could be used for machines with ACPI,
but I haven't been able to find one that had to defer its probes because
of the device probe order.
Tomeu Vizoso (20):
- Set and use device_node.platform_dev instead of reversing the logic to
find the platform device that encloses a device node.
- Drop the fwnode API to probe firmware nodes and add OF-only API for
now. I think this same scheme could be used for machines with ACPI,
but I haven't been able to find one that had to defer its probes because
of the device probe order.
driver core: handle -EPROBE_DEFER from bus_type.match()
ARM: amba: Move reading of periphid to amba_match()
of/platform: Point to struct device from device node
of: add function to allow probing a device from a OF node
gpio: Probe GPIO drivers on demand
ARM: amba: Move reading of periphid to amba_match()
of/platform: Point to struct device from device node
of: add function to allow probing a device from a OF node
gpio: Probe GPIO drivers on demand
pinctrl: 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 adapters and devices on demand
pwm: Probe PWM chip devices on demand
backlight: Probe backlight devices on demand
usb: phy: Probe phy devices on demand
clk: Probe clk providers on demand
pinctrl: Probe pinctrl devices on demand
phy: core: Probe phy providers on demand
dma: of: Probe DMA controllers on demand
power-supply: Probe power supplies on demand
driver core: Allow deferring probes until late init
regulator: core: Probe regulators on demand
drm: Probe panels on demand
drm/tegra: Probe dpaux devices on demand
i2c: core: Probe i2c adapters and devices on demand
pwm: Probe PWM chip devices on demand
backlight: Probe backlight devices on demand
usb: phy: Probe phy devices on demand
clk: Probe clk providers on demand
pinctrl: Probe pinctrl devices on demand
phy: core: Probe phy providers on demand
dma: of: Probe DMA controllers on demand
power-supply: Probe power supplies on demand
driver core: Allow deferring probes until late init
of/platform: Defer probes of registered devices
drivers/amba/bus.c | 88 +++++++++++++++++++------------------
drivers/base/Kconfig | 18 ++++++++
drivers/base/dd.c | 30 ++++++++++++-
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 | 4 ++
drivers/of/device.c | 61 +++++++++++++++++++++++++
drivers/of/platform.c | 30 ++++++++-----
drivers/phy/phy-core.c | 3 ++
drivers/pinctrl/devicetree.c | 3 ++
drivers/power/power_supply_core.c | 3 ++
drivers/pwm/core.c | 3 ++
drivers/regulator/core.c | 2 +
drivers/usb/phy/phy.c | 3 ++
drivers/video/backlight/backlight.c | 3 ++
include/linux/device.h | 4 +-
include/linux/of.h | 1 +
include/linux/of_device.h | 3 ++
include/linux/of.h | 1 +
21 files changed, 219 insertions(+), 57 deletions(-)
Tomeu Vizoso
Sep 29, 2015, 5:20:08 AM9/29/15
to
When looking up an i2c adapter or device through its OF node, probe it
drivers/i2c/i2c-core.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 5f89f1e3c2f2..02da3acbbd35 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1413,6 +1413,8 @@ struct i2c_client *of_find_i2c_device_by_node(struct device_node *node)
struct device *dev;
struct i2c_client *client;
+ of_device_probe(node);
+
dev = bus_find_device(&i2c_bus_type, NULL, node, of_dev_node_match);
if (!dev)
return NULL;
@@ -1431,6 +1433,8 @@ struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node)
struct device *dev;
struct i2c_adapter *adapter;
+ of_device_probe(node);
+
dev = bus_find_device(&i2c_bus_type, NULL, node, of_dev_node_match);
if (!dev)
return NULL;
if it hasn't already.
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
---
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
drivers/i2c/i2c-core.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 5f89f1e3c2f2..02da3acbbd35 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1413,6 +1413,8 @@ struct i2c_client *of_find_i2c_device_by_node(struct device_node *node)
struct device *dev;
struct i2c_client *client;
+ of_device_probe(node);
+
dev = bus_find_device(&i2c_bus_type, NULL, node, of_dev_node_match);
if (!dev)
return NULL;
@@ -1431,6 +1433,8 @@ struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node)
struct device *dev;
struct i2c_adapter *adapter;
+ of_device_probe(node);
+
dev = bus_find_device(&i2c_bus_type, NULL, node, of_dev_node_match);
if (!dev)
return NULL;
Tomeu Vizoso
Sep 29, 2015, 5:20:09 AM9/29/15
to
When looking up a dpaux device through its OF node, probe it if it
index 224a7dc8e4ed..96a2eec7e020 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_device.h>
#include <linux/platform_device.h>
#include <linux/reset.h>
#include <linux/regulator/consumer.h>
@@ -439,6 +440,8 @@ struct tegra_dpaux *tegra_dpaux_find_by_of_node(struct device_node *np)
{
struct tegra_dpaux *dpaux;
+ of_device_probe(np);
+
mutex_lock(&dpaux_lock);
list_for_each_entry(dpaux, &dpaux_list, list)
hasn't already.
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
---
drivers/gpu/drm/tegra/dpaux.c | 3 +++
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
---
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
index 224a7dc8e4ed..96a2eec7e020 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_device.h>
#include <linux/platform_device.h>
#include <linux/reset.h>
#include <linux/regulator/consumer.h>
@@ -439,6 +440,8 @@ struct tegra_dpaux *tegra_dpaux_find_by_of_node(struct device_node *np)
{
struct tegra_dpaux *dpaux;
+ of_device_probe(np);
+
mutex_lock(&dpaux_lock);
list_for_each_entry(dpaux, &dpaux_list, list)
Tomeu Vizoso
Sep 29, 2015, 5:20:09 AM9/29/15
to
When looking up a pin controller through its OF node, probe it if it
index fe04e748dfe4..f5340b8e1dbe 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_device.h>
#include <linux/pinctrl/pinctrl.h>
#include <linux/slab.h>
@@ -110,6 +111,8 @@ static int dt_to_map_one_config(struct pinctrl *p, const char *statename,
struct pinctrl_map *map;
unsigned num_maps;
+ of_device_probe(np_config);
+
/* Find the pin controller containing np_config */
np_pctldev = of_node_get(np_config);
for (;;) {
hasn't already.
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
drivers/pinctrl/devicetree.c | 3 +++
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
1 file changed, 3 insertions(+)
diff --git a/drivers/pinctrl/devicetree.c b/drivers/pinctrl/devicetree.c
index fe04e748dfe4..f5340b8e1dbe 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_device.h>
#include <linux/pinctrl/pinctrl.h>
#include <linux/slab.h>
@@ -110,6 +111,8 @@ static int dt_to_map_one_config(struct pinctrl *p, const char *statename,
struct pinctrl_map *map;
unsigned num_maps;
+ of_device_probe(np_config);
+
/* Find the pin controller containing np_config */
np_pctldev = of_node_get(np_config);
for (;;) {
Tomeu Vizoso
Sep 29, 2015, 5:20:10 AM9/29/15
to
Lets implementations of the match() callback in struct bus_type to
return errors and if it's -EPROBE_DEFER then queue the device for
deferred probing.
This is useful to buses such as AMBA in which devices are registered
before their matching information can be retrieved from the HW
(typically because a clock driver hasn't probed yet).
drivers/base/dd.c | 24 ++++++++++++++++++++++--
include/linux/device.h | 2 +-
2 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index be0eb4639128..7dc04ee81c8b 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -488,6 +488,7 @@ static int __device_attach_driver(struct device_driver *drv, void *_data)
struct device_attach_data *data = _data;
struct device *dev = data->dev;
bool async_allowed;
+ int ret;
/*
* Check if device has already been claimed. This may
@@ -498,8 +499,17 @@ static int __device_attach_driver(struct device_driver *drv, void *_data)
if (dev->driver)
return -EBUSY;
- if (!driver_match_device(drv, dev))
+ ret = driver_match_device(drv, dev);
+ if (!ret)
return 0;
+ else if (ret < 0) {
+ if (ret == -EPROBE_DEFER) {
+ dev_dbg(dev, "Device match requests probe deferral\n");
+ driver_deferred_probe_add(dev);
+ } else
+ dev_warn(dev, "Bus failed to match device: %d", ret);
+ return ret;
+ }
async_allowed = driver_allows_async_probing(drv);
@@ -619,6 +629,7 @@ void device_initial_probe(struct device *dev)
static int __driver_attach(struct device *dev, void *data)
{
struct device_driver *drv = data;
+ int ret;
/*
* Lock device and try to bind to it. We drop the error
@@ -630,8 +641,17 @@ static int __driver_attach(struct device *dev, void *data)
* is an error.
*/
- if (!driver_match_device(drv, dev))
+ ret = driver_match_device(drv, dev);
+ if (!ret)
+ return 0;
+ else if (ret < 0) {
+ if (ret == -EPROBE_DEFER) {
+ dev_dbg(dev, "Device match requests probe deferral\n");
+ driver_deferred_probe_add(dev);
+ } else
+ dev_warn(dev, "Bus failed to match device: %d", ret);
return 0;
+ }
if (dev->parent) /* Needed for USB */
device_lock(dev->parent);
diff --git a/include/linux/device.h b/include/linux/device.h
index 5d7bc6349930..8e7b806f0744 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -70,7 +70,7 @@ extern void bus_remove_file(struct bus_type *, struct bus_attribute *);
* @dev_groups: Default attributes of the devices on the bus.
* @drv_groups: Default attributes of the device drivers on the bus.
* @match: Called, perhaps multiple times, whenever a new device or driver
- * is added for this bus. It should return a nonzero value if the
+ * is added for this bus. It should return a positive value if the
* given device can be handled by the given driver.
* @uevent: Called when a device is added, removed, or a few other things
* that generate uevents to add the environment variables.
return errors and if it's -EPROBE_DEFER then queue the device for
deferred probing.
This is useful to buses such as AMBA in which devices are registered
before their matching information can be retrieved from the HW
(typically because a clock driver hasn't probed yet).
drivers/base/dd.c | 24 ++++++++++++++++++++++--
include/linux/device.h | 2 +-
2 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index be0eb4639128..7dc04ee81c8b 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -488,6 +488,7 @@ static int __device_attach_driver(struct device_driver *drv, void *_data)
struct device_attach_data *data = _data;
struct device *dev = data->dev;
bool async_allowed;
+ int ret;
/*
* Check if device has already been claimed. This may
@@ -498,8 +499,17 @@ static int __device_attach_driver(struct device_driver *drv, void *_data)
if (dev->driver)
return -EBUSY;
- if (!driver_match_device(drv, dev))
+ ret = driver_match_device(drv, dev);
+ if (!ret)
return 0;
+ else if (ret < 0) {
+ if (ret == -EPROBE_DEFER) {
+ dev_dbg(dev, "Device match requests probe deferral\n");
+ driver_deferred_probe_add(dev);
+ } else
+ dev_warn(dev, "Bus failed to match device: %d", ret);
+ return ret;
+ }
async_allowed = driver_allows_async_probing(drv);
@@ -619,6 +629,7 @@ void device_initial_probe(struct device *dev)
static int __driver_attach(struct device *dev, void *data)
{
struct device_driver *drv = data;
+ int ret;
/*
* Lock device and try to bind to it. We drop the error
@@ -630,8 +641,17 @@ static int __driver_attach(struct device *dev, void *data)
* is an error.
*/
- if (!driver_match_device(drv, dev))
+ ret = driver_match_device(drv, dev);
+ if (!ret)
+ return 0;
+ else if (ret < 0) {
+ if (ret == -EPROBE_DEFER) {
+ dev_dbg(dev, "Device match requests probe deferral\n");
+ driver_deferred_probe_add(dev);
+ } else
+ dev_warn(dev, "Bus failed to match device: %d", ret);
return 0;
+ }
if (dev->parent) /* Needed for USB */
device_lock(dev->parent);
diff --git a/include/linux/device.h b/include/linux/device.h
index 5d7bc6349930..8e7b806f0744 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -70,7 +70,7 @@ extern void bus_remove_file(struct bus_type *, struct bus_attribute *);
* @dev_groups: Default attributes of the devices on the bus.
* @drv_groups: Default attributes of the device drivers on the bus.
* @match: Called, perhaps multiple times, whenever a new device or driver
- * is added for this bus. It should return a nonzero value if the
+ * is added for this bus. It should return a positive value if the
* given device can be handled by the given driver.
* @uevent: Called when a device is added, removed, or a few other things
* that generate uevents to add the environment variables.
Tomeu Vizoso
Sep 29, 2015, 5:20:10 AM9/29/15
to
When looking up a phy provider through its OF node, probe it if it
index fc48fac003a6..94e90031d7f3 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
#include <linux/of.h>
+#include <linux/of_device.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_device_probe(args.np);
+
mutex_lock(&phy_provider_mutex);
phy_provider = of_phy_provider_lookup(args.np);
if (IS_ERR(phy_provider) || !try_module_get(phy_provider->owner)) {
hasn't already.
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
drivers/phy/phy-core.c | 3 +++
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
1 file changed, 3 insertions(+)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index fc48fac003a6..94e90031d7f3 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/device.h>
#include <linux/of.h>
+#include <linux/of_device.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_device_probe(args.np);
+
mutex_lock(&phy_provider_mutex);
phy_provider = of_phy_provider_lookup(args.np);
if (IS_ERR(phy_provider) || !try_module_get(phy_provider->owner)) {
Tomeu Vizoso
Sep 29, 2015, 5:20:11 AM9/29/15
to
Reading the periphid when the Primecell device is registered means that
the apb pclk must be available by then or the device won't be registered
at all.
By reading the periphid in amba_match() we can return -EPROBE_DEFER if
the apb pclk isn't there yet and the device will be retried later.
1 file changed, 46 insertions(+), 42 deletions(-)
diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
index f0099360039e..72ebf9b1c715 100644
--- a/drivers/amba/bus.c
+++ b/drivers/amba/bus.c
@@ -24,6 +24,8 @@
#define to_amba_driver(d) container_of(d, struct amba_driver, drv)
+static int read_periphid(struct amba_device *d, unsigned int *periphid);
+
static const struct amba_id *
amba_lookup(const struct amba_id *table, struct amba_device *dev)
{
@@ -43,11 +45,22 @@ static int amba_match(struct device *dev, struct device_driver *drv)
{
struct amba_device *pcdev = to_amba_device(dev);
struct amba_driver *pcdrv = to_amba_driver(drv);
+ int ret;
/* When driver_override is set, only bind to the matching driver */
if (pcdev->driver_override)
return !strcmp(pcdev->driver_override, drv->name);
+ if (!pcdev->periphid) {
+ ret = read_periphid(pcdev, &pcdev->periphid);
+ if (ret) {
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "Failed to read periphid: %d",
+ ret);
+ return ret;
+ }
+ }
+
return amba_lookup(pcdrv->id_table, pcdev) != NULL;
}
@@ -336,44 +349,22 @@ static void amba_device_release(struct device *dev)
kfree(d);
}
-/**
- * amba_device_add - add a previously allocated AMBA device structure
- * @dev: AMBA device allocated by amba_device_alloc
- * @parent: resource parent for this devices resources
- *
- * Claim the resource, and read the device cell ID if not already
- * initialized. Register the AMBA device with the Linux device
- * manager.
- */
-int amba_device_add(struct amba_device *dev, struct resource *parent)
+static int read_periphid(struct amba_device *d, unsigned int *periphid)
{
u32 size;
void __iomem *tmp;
- int i, ret;
-
- WARN_ON(dev->irq[0] == (unsigned int)-1);
- WARN_ON(dev->irq[1] == (unsigned int)-1);
-
- ret = request_resource(parent, &dev->res);
- if (ret)
- goto err_out;
-
- /* Hard-coded primecell ID instead of plug-n-play */
- if (dev->periphid != 0)
- goto skip_probe;
+ int i, ret = 0;
/*
* Dynamically calculate the size of the resource
* and use this for iomap
*/
- size = resource_size(&dev->res);
- tmp = ioremap(dev->res.start, size);
- if (!tmp) {
- ret = -ENOMEM;
- goto err_release;
- }
+ size = resource_size(&d->res);
+ tmp = ioremap(d->res.start, size);
+ if (!tmp)
+ return -ENOMEM;
- ret = amba_get_enable_pclk(dev);
+ ret = amba_get_enable_pclk(d);
if (ret == 0) {
u32 pid, cid;
@@ -388,37 +379,50 @@ int amba_device_add(struct amba_device *dev, struct resource *parent)
cid |= (readl(tmp + size - 0x10 + 4 * i) & 255) <<
(i * 8);
- amba_put_disable_pclk(dev);
+ amba_put_disable_pclk(d);
if (cid == AMBA_CID || cid == CORESIGHT_CID)
- dev->periphid = pid;
+ *periphid = pid;
- if (!dev->periphid)
+ if (!*periphid)
ret = -ENODEV;
}
iounmap(tmp);
+ return ret;
+}
+
+/**
+ * amba_device_add - add a previously allocated AMBA device structure
+ * @dev: AMBA device allocated by amba_device_alloc
+ * @parent: resource parent for this devices resources
+ *
+ * Claim the resource, and register the AMBA device with the Linux device
+ * manager.
+ */
+int amba_device_add(struct amba_device *dev, struct resource *parent)
+{
+ int ret;
+
+ WARN_ON(dev->irq[0] == (unsigned int)-1);
+ WARN_ON(dev->irq[1] == (unsigned int)-1);
+
+ ret = request_resource(parent, &dev->res);
if (ret)
- goto err_release;
+ return ret;
- skip_probe:
ret = device_add(&dev->dev);
if (ret)
- goto err_release;
+ return ret;
if (dev->irq[0])
ret = device_create_file(&dev->dev, &dev_attr_irq0);
if (ret == 0 && dev->irq[1])
ret = device_create_file(&dev->dev, &dev_attr_irq1);
- if (ret == 0)
- return ret;
-
- device_unregister(&dev->dev);
+ if (ret)
+ device_unregister(&dev->dev);
- err_release:
- release_resource(&dev->res);
- err_out:
return ret;
}
EXPORT_SYMBOL_GPL(amba_device_add);
the apb pclk must be available by then or the device won't be registered
at all.
By reading the periphid in amba_match() we can return -EPROBE_DEFER if
the apb pclk isn't there yet and the device will be retried later.
Changes in v6:
- Drop bus_type.pre_probe and read the periphid in match() instead as
suggested by Alan Stern.
- Drop bus_type.pre_probe and read the periphid in match() instead as
suggested by Alan Stern.
Changes in v4:
- Added bus.pre_probe callback so the probes of Primecell devices can be
deferred if their device IDs cannot be yet read because of the clock
driver not having probed when they are registered. Maybe this goes
overboard and the matching information should be in the DT if there is
one.
drivers/amba/bus.c | 88 ++++++++++++++++++++++++++++--------------------------
- Added bus.pre_probe callback so the probes of Primecell devices can be
deferred if their device IDs cannot be yet read because of the clock
driver not having probed when they are registered. Maybe this goes
overboard and the matching information should be in the DT if there is
one.
1 file changed, 46 insertions(+), 42 deletions(-)
diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
index f0099360039e..72ebf9b1c715 100644
--- a/drivers/amba/bus.c
+++ b/drivers/amba/bus.c
@@ -24,6 +24,8 @@
#define to_amba_driver(d) container_of(d, struct amba_driver, drv)
+static int read_periphid(struct amba_device *d, unsigned int *periphid);
+
static const struct amba_id *
amba_lookup(const struct amba_id *table, struct amba_device *dev)
{
@@ -43,11 +45,22 @@ static int amba_match(struct device *dev, struct device_driver *drv)
{
struct amba_device *pcdev = to_amba_device(dev);
struct amba_driver *pcdrv = to_amba_driver(drv);
+ int ret;
/* When driver_override is set, only bind to the matching driver */
if (pcdev->driver_override)
return !strcmp(pcdev->driver_override, drv->name);
+ if (!pcdev->periphid) {
+ ret = read_periphid(pcdev, &pcdev->periphid);
+ if (ret) {
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "Failed to read periphid: %d",
+ ret);
+ return ret;
+ }
+ }
+
return amba_lookup(pcdrv->id_table, pcdev) != NULL;
}
@@ -336,44 +349,22 @@ static void amba_device_release(struct device *dev)
kfree(d);
}
-/**
- * amba_device_add - add a previously allocated AMBA device structure
- * @dev: AMBA device allocated by amba_device_alloc
- * @parent: resource parent for this devices resources
- *
- * Claim the resource, and read the device cell ID if not already
- * initialized. Register the AMBA device with the Linux device
- * manager.
- */
-int amba_device_add(struct amba_device *dev, struct resource *parent)
+static int read_periphid(struct amba_device *d, unsigned int *periphid)
{
u32 size;
void __iomem *tmp;
- int i, ret;
-
- WARN_ON(dev->irq[0] == (unsigned int)-1);
- WARN_ON(dev->irq[1] == (unsigned int)-1);
-
- ret = request_resource(parent, &dev->res);
- if (ret)
- goto err_out;
-
- /* Hard-coded primecell ID instead of plug-n-play */
- if (dev->periphid != 0)
- goto skip_probe;
+ int i, ret = 0;
/*
* Dynamically calculate the size of the resource
* and use this for iomap
*/
- size = resource_size(&dev->res);
- tmp = ioremap(dev->res.start, size);
- if (!tmp) {
- ret = -ENOMEM;
- goto err_release;
- }
+ size = resource_size(&d->res);
+ tmp = ioremap(d->res.start, size);
+ if (!tmp)
+ return -ENOMEM;
- ret = amba_get_enable_pclk(dev);
+ ret = amba_get_enable_pclk(d);
if (ret == 0) {
u32 pid, cid;
@@ -388,37 +379,50 @@ int amba_device_add(struct amba_device *dev, struct resource *parent)
cid |= (readl(tmp + size - 0x10 + 4 * i) & 255) <<
(i * 8);
- amba_put_disable_pclk(dev);
+ amba_put_disable_pclk(d);
if (cid == AMBA_CID || cid == CORESIGHT_CID)
- dev->periphid = pid;
+ *periphid = pid;
- if (!dev->periphid)
+ if (!*periphid)
ret = -ENODEV;
}
iounmap(tmp);
+ return ret;
+}
+
+/**
+ * amba_device_add - add a previously allocated AMBA device structure
+ * @dev: AMBA device allocated by amba_device_alloc
+ * @parent: resource parent for this devices resources
+ *
+ * Claim the resource, and register the AMBA device with the Linux device
+ * manager.
+ */
+int amba_device_add(struct amba_device *dev, struct resource *parent)
+{
+ int ret;
+
+ WARN_ON(dev->irq[0] == (unsigned int)-1);
+ WARN_ON(dev->irq[1] == (unsigned int)-1);
+
+ ret = request_resource(parent, &dev->res);
if (ret)
- goto err_release;
+ return ret;
- skip_probe:
ret = device_add(&dev->dev);
if (ret)
- goto err_release;
+ return ret;
if (dev->irq[0])
ret = device_create_file(&dev->dev, &dev_attr_irq0);
if (ret == 0 && dev->irq[1])
ret = device_create_file(&dev->dev, &dev_attr_irq1);
- if (ret == 0)
- return ret;
-
- device_unregister(&dev->dev);
+ if (ret)
+ device_unregister(&dev->dev);
- err_release:
- release_resource(&dev->res);
- err_out:
return ret;
}
EXPORT_SYMBOL_GPL(amba_device_add);
Tomeu Vizoso
Sep 29, 2015, 5:20:11 AM9/29/15
to
When looking up a power supply through its OF node, probe it if it
index 456987c88baa..80bc89f4ae89 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -19,6 +19,7 @@
#include <linux/err.h>
#include <linux/power_supply.h>
#include <linux/thermal.h>
+#include <linux/of_device.h>
#include "power_supply.h"
/* exported for the APM Power driver, APM emulation */
@@ -206,6 +207,8 @@ static int power_supply_find_supply_from_node(struct device_node *supply_node)
{
int error;
+ of_device_probe(supply_node);
+
/*
* class_for_each_device() either returns its own errors or values
* returned by __power_supply_find_supply_from_node().
hasn't already.
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
drivers/power/power_supply_core.c | 3 +++
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
1 file changed, 3 insertions(+)
diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index 456987c88baa..80bc89f4ae89 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -19,6 +19,7 @@
#include <linux/err.h>
#include <linux/power_supply.h>
#include <linux/thermal.h>
+#include <linux/of_device.h>
#include "power_supply.h"
/* exported for the APM Power driver, APM emulation */
@@ -206,6 +207,8 @@ static int power_supply_find_supply_from_node(struct device_node *supply_node)
{
int error;
+ of_device_probe(supply_node);
+
/*
* class_for_each_device() either returns its own errors or values
* returned by __power_supply_find_supply_from_node().
Tomeu Vizoso
Sep 29, 2015, 5:20:12 AM9/29/15
to
When looking up a clock through its OF node, probe it if it hasn't
#include <linux/list.h>
already.
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
---
drivers/clk/clk.c | 3 +++
The goal is to reduce deferred probes to a minimum, as it makes it very
cumbersome to find out why a device failed to probe, and can introduce
very big delays in when a critical device is probed.
Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
---
1 file changed, 3 insertions(+)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 43e2c3ad6c31..e5fe02a11c36 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -19,6 +19,7 @@
index 43e2c3ad6c31..e5fe02a11c36 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
#include <linux/list.h>
#include <linux/slab.h>
#include <linux/of.h>
+#include <linux/of_device.h>
#include <linux/of.h>
+#include <linux/of_device.h>
#include <linux/device.h>
#include <linux/init.h>
#include <linux/sched.h>
@@ -3004,6 +3005,8 @@ struct clk *__of_clk_get_from_provider(struct of_phandle_args *clkspec,
if (!clkspec)
return ERR_PTR(-EINVAL);
+ of_device_probe(clkspec->np);
+
/* Check if we have such a provider in our array */
mutex_lock(&of_clk_mutex);
list_for_each_entry(provider, &of_clk_providers, link) {
#include <linux/init.h>
#include <linux/sched.h>
@@ -3004,6 +3005,8 @@ struct clk *__of_clk_get_from_provider(struct of_phandle_args *clkspec,
if (!clkspec)
return ERR_PTR(-EINVAL);
+ of_device_probe(clkspec->np);
+
/* Check if we have such a provider in our array */
mutex_lock(&of_clk_mutex);
list_for_each_entry(provider, &of_clk_providers, link) {
Tomeu Vizoso
Sep 30, 2015, 6:20:07 AM9/30/15
to
On 26 September 2015 at 21:22, Greg Kroah-Hartman
Hi Greg, hope you don't mind that I ping you regarding this, just in
case it fell through some crack.
Regards,
Tomeu
case it fell through some crack.
Regards,
Tomeu
Greg Kroah-Hartman
Sep 30, 2015, 6:30:08 AM9/30/15
to
didn't get to it on Monday, hopefully soon...
greg "my todo list is too long" k-h
Tomeu Vizoso
Oct 13, 2015, 4:00:14 PM10/13/15
to
On 26 September 2015 at 20:17, Rob Herring <ro...@kernel.org> wrote:
I'm not sure we are going to get much more feedback by just waiting or
resending what has been sent so many times.
Did you have in mind specific people you wanted to see reviewed-bys from?
Guess Greg will get to this sometime soon.
Regards,
Tomeu
Rob Herring
Oct 13, 2015, 5:30:08 PM10/13/15
to
> Did you have in mind specific people you wanted to see reviewed-bys from?
Please send me a pull req. I want to get this into -next and can
always drop it if there is further review.
Rob
Tomeu Vizoso
Oct 14, 2015, 4:40:06 AM10/14/15
to
Hi Rob,
here is the pull request you asked for, with no changes from the version
that I posted last to the list.
The following changes since commit 6ff33f3902c3b1c5d0db6b1e2c70b6d76fba357f:
Linux 4.3-rc1 (2015-09-12 16:35:56 -0700)
are available in the git repository at:
git+ssh://git.collabora.co.uk/git/user/tomeu/linux.git
on-demand-probes-for-next
for you to fetch changes up to 587402133fe433759d2d535e5d92ead87fd7f615:
of/platform: Defer probes of registered devices (2015-10-14 10:08:23 +0200)
----------------------------------------------------------------
Tomeu Vizoso (20):
drivers/base/Kconfig | 18 ++++++++++++++++++
drivers/base/dd.c | 30 ++++++++++++++++++++++++++++--
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 | 4 ++++
drivers/of/device.c | 61
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
drivers/of/platform.c | 30 ++++++++++++++++++------------
drivers/phy/phy-core.c | 3 +++
drivers/pinctrl/devicetree.c | 3 +++
drivers/power/power_supply_core.c | 3 +++
drivers/pwm/core.c | 3 +++
drivers/regulator/core.c | 2 ++
drivers/usb/phy/phy.c | 3 +++
drivers/video/backlight/backlight.c | 3 +++
include/linux/device.h | 4 +++-
include/linux/of.h | 1 +
include/linux/of_device.h | 3 +++
Tomeu
here is the pull request you asked for, with no changes from the version
that I posted last to the list.
The following changes since commit 6ff33f3902c3b1c5d0db6b1e2c70b6d76fba357f:
Linux 4.3-rc1 (2015-09-12 16:35:56 -0700)
are available in the git repository at:
git+ssh://git.collabora.co.uk/git/user/tomeu/linux.git
on-demand-probes-for-next
for you to fetch changes up to 587402133fe433759d2d535e5d92ead87fd7f615:
of/platform: Defer probes of registered devices (2015-10-14 10:08:23 +0200)
----------------------------------------------------------------
Tomeu Vizoso (20):
driver core: handle -EPROBE_DEFER from bus_type.match()
ARM: amba: Move reading of periphid to amba_match()
of/platform: Point to struct device from device node
of: add function to allow probing a device from a OF node
gpio: Probe GPIO drivers on demand
ARM: amba: Move reading of periphid to amba_match()
of/platform: Point to struct device from device node
of: add function to allow probing a device from a OF node
gpio: Probe GPIO drivers on demand
pinctrl: 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 adapters and devices on demand
pwm: Probe PWM chip devices on demand
backlight: Probe backlight devices on demand
usb: phy: Probe phy devices on demand
clk: Probe clk providers on demand
pinctrl: Probe pinctrl devices on demand
phy: core: Probe phy providers on demand
dma: of: Probe DMA controllers on demand
power-supply: Probe power supplies on demand
driver core: Allow deferring probes until late init
regulator: core: Probe regulators on demand
drm: Probe panels on demand
drm/tegra: Probe dpaux devices on demand
i2c: core: Probe i2c adapters and devices on demand
pwm: Probe PWM chip devices on demand
backlight: Probe backlight devices on demand
usb: phy: Probe phy devices on demand
clk: Probe clk providers on demand
pinctrl: Probe pinctrl devices on demand
phy: core: Probe phy providers on demand
dma: of: Probe DMA controllers on demand
power-supply: Probe power supplies on demand
driver core: Allow deferring probes until late init
of/platform: Defer probes of registered devices
drivers/amba/bus.c | 88
++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------
drivers/amba/bus.c | 88
drivers/base/Kconfig | 18 ++++++++++++++++++
drivers/base/dd.c | 30 ++++++++++++++++++++++++++++--
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 | 4 ++++
drivers/of/device.c | 61
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
drivers/of/platform.c | 30 ++++++++++++++++++------------
drivers/phy/phy-core.c | 3 +++
drivers/pinctrl/devicetree.c | 3 +++
drivers/power/power_supply_core.c | 3 +++
drivers/pwm/core.c | 3 +++
drivers/regulator/core.c | 2 ++
drivers/usb/phy/phy.c | 3 +++
drivers/video/backlight/backlight.c | 3 +++
include/linux/device.h | 4 +++-
include/linux/of.h | 1 +
include/linux/of_device.h | 3 +++
21 files changed, 219 insertions(+), 57 deletions(-)
Thanks,
Tomeu
Mark Brown
Oct 14, 2015, 5:30:06 AM10/14/15
to
On Wed, Oct 14, 2015 at 10:34:00AM +0200, Tomeu Vizoso wrote:
> git+ssh://git.collabora.co.uk/git/user/tomeu/linux.git
> on-demand-probes-for-next
In don't think that's the URL you intended to use (also everything looks
word wrapped here)?
> git+ssh://git.collabora.co.uk/git/user/tomeu/linux.git
> on-demand-probes-for-next
In don't think that's the URL you intended to use (also everything looks
word wrapped here)?
Frank Rowand
Oct 14, 2015, 7:20:07 PM10/14/15
to
On 9/29/2015 2:10 AM, Tomeu Vizoso wrote:
> Instead of trying to match and probe platform and AMBA devices right
> after each is registered, delay their probes until device_initcall_sync.
^^^^^^^^^^^^^^^^^^^^
> Instead of trying to match and probe platform and AMBA devices right
> after each is registered, delay their probes until device_initcall_sync.
late_initcall
>
> This means that devices will start probing once all built-in drivers
> have registered, and after all platform and AMBA devices from the DT
> have been registered already.
>
> This allows us to prevent deferred probes by probing dependencies on
> demand.
>
> Signed-off-by: Tomeu Vizoso <tomeu....@collabora.com>
> ---
Tomeu Vizoso
Oct 15, 2015, 7:50:08 AM10/15/15
to
Hi,
this second pull request replaces the last references to device_initcall_sync with late_initcall, as noticed by Frank Rowand.
Also fixes the url of the git repo and the wrapping, as suggested by Mark Brown.
Thanks,
Tomeu
The following changes since commit 6ff33f3902c3b1c5d0db6b1e2c70b6d76fba357f:
Linux 4.3-rc1 (2015-09-12 16:35:56 -0700)
are available in the git repository at:
git://git.collabora.com/git/user/tomeu/linux.git on-demand-probes-for-next
for you to fetch changes up to c074fef5d36e1c27dfdf7474e23c01a1b044ff98:
of/platform: Defer probes of registered devices (2015-10-15 13:25:47 +0200)
----------------------------------------------------------------
Tomeu Vizoso (20):
driver core: handle -EPROBE_DEFER from bus_type.match()
ARM: amba: Move reading of periphid to amba_match()
of/platform: Point to struct device from device node
of: add function to allow probing a device from a OF node
gpio: Probe GPIO drivers on demand
pinctrl: 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 adapters and devices on demand
pwm: Probe PWM chip devices on demand
backlight: Probe backlight devices on demand
usb: phy: Probe phy devices on demand
clk: Probe clk providers on demand
pinctrl: Probe pinctrl devices on demand
phy: core: Probe phy providers on demand
dma: of: Probe DMA controllers on demand
power-supply: Probe power supplies on demand
driver core: Allow deferring probes until late init
of/platform: Defer probes of registered devices
this second pull request replaces the last references to device_initcall_sync with late_initcall, as noticed by Frank Rowand.
Also fixes the url of the git repo and the wrapping, as suggested by Mark Brown.
Thanks,
Tomeu
The following changes since commit 6ff33f3902c3b1c5d0db6b1e2c70b6d76fba357f:
Linux 4.3-rc1 (2015-09-12 16:35:56 -0700)
are available in the git repository at:
for you to fetch changes up to c074fef5d36e1c27dfdf7474e23c01a1b044ff98:
of/platform: Defer probes of registered devices (2015-10-15 13:25:47 +0200)
----------------------------------------------------------------
Tomeu Vizoso (20):
driver core: handle -EPROBE_DEFER from bus_type.match()
ARM: amba: Move reading of periphid to amba_match()
of/platform: Point to struct device from device node
of: add function to allow probing a device from a OF node
gpio: Probe GPIO drivers on demand
pinctrl: 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 adapters and devices on demand
pwm: Probe PWM chip devices on demand
backlight: Probe backlight devices on demand
usb: phy: Probe phy devices on demand
clk: Probe clk providers on demand
pinctrl: Probe pinctrl devices on demand
phy: core: Probe phy providers on demand
dma: of: Probe DMA controllers on demand
power-supply: Probe power supplies on demand
driver core: Allow deferring probes until late init
of/platform: Defer probes of registered devices
drivers/amba/bus.c | 88 +++++++++++++++++++------------------
drivers/base/Kconfig | 18 ++++++++
drivers/base/dd.c | 30 ++++++++++++-
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 | 4 ++
drivers/of/device.c | 61 +++++++++++++++++++++++++
drivers/of/platform.c | 30 ++++++++-----
drivers/phy/phy-core.c | 3 ++
drivers/pinctrl/devicetree.c | 3 ++
drivers/power/power_supply_core.c | 3 ++
drivers/pwm/core.c | 3 ++
drivers/regulator/core.c | 2 +
drivers/usb/phy/phy.c | 3 ++
drivers/video/backlight/backlight.c | 3 ++
include/linux/device.h | 4 +-
drivers/base/Kconfig | 18 ++++++++
drivers/base/dd.c | 30 ++++++++++++-
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 | 4 ++
drivers/of/device.c | 61 +++++++++++++++++++++++++
drivers/of/platform.c | 30 ++++++++-----
drivers/phy/phy-core.c | 3 ++
drivers/pinctrl/devicetree.c | 3 ++
drivers/power/power_supply_core.c | 3 ++
drivers/pwm/core.c | 3 ++
drivers/regulator/core.c | 2 +
drivers/usb/phy/phy.c | 3 ++
drivers/video/backlight/backlight.c | 3 ++
include/linux/device.h | 4 +-
include/linux/of.h | 1 +
include/linux/of_device.h | 3 ++
include/linux/of_device.h | 3 ++
21 files changed, 219 insertions(+), 57 deletions(-)
Olof Johansson
Oct 16, 2015, 5:30:08 PM10/16/15
to
Hi,
I've bisected boot failures in next-20151016 down to patches in this branch:
On Thu, Oct 15, 2015 at 4:42 AM, Tomeu Vizoso
<tomeu....@collabora.com> wrote:
> Tomeu Vizoso (20):
> driver core: handle -EPROBE_DEFER from bus_type.match()
The machine it happened on was OMAP5UEVM:
http://arm-soc.lixom.net/bootlogs/next/next-20151016/omap5uevm-arm-omap2plus_defconfig.html
But I've also seen it on tegra2, that one bisected down to:
> regulator: core: Probe regulators on demand
http://arm-soc.lixom.net/bootlogs/next/next-20151016/seaboard-arm-multi_v7_defconfig.html
-Olof
I've bisected boot failures in next-20151016 down to patches in this branch:
On Thu, Oct 15, 2015 at 4:42 AM, Tomeu Vizoso
<tomeu....@collabora.com> wrote:
> Tomeu Vizoso (20):
> driver core: handle -EPROBE_DEFER from bus_type.match()
http://arm-soc.lixom.net/bootlogs/next/next-20151016/omap5uevm-arm-omap2plus_defconfig.html
But I've also seen it on tegra2, that one bisected down to:
> regulator: core: Probe regulators on demand
-Olof
Greg Kroah-Hartman
Oct 17, 2015, 3:00:07 AM10/17/15
to
On Wed, Oct 14, 2015 at 10:34:00AM +0200, Tomeu Vizoso wrote:
> Hi Rob,
>
> here is the pull request you asked for, with no changes from the version
> that I posted last to the list.
>
> The following changes since commit 6ff33f3902c3b1c5d0db6b1e2c70b6d76fba357f:
>
> Linux 4.3-rc1 (2015-09-12 16:35:56 -0700)
>
> are available in the git repository at:
>
> git+ssh://git.collabora.co.uk/git/user/tomeu/linux.git
> on-demand-probes-for-next
That's not a signed tag :(
>
> here is the pull request you asked for, with no changes from the version
> that I posted last to the list.
>
> The following changes since commit 6ff33f3902c3b1c5d0db6b1e2c70b6d76fba357f:
>
> Linux 4.3-rc1 (2015-09-12 16:35:56 -0700)
>
> are available in the git repository at:
>
> git+ssh://git.collabora.co.uk/git/user/tomeu/linux.git
> on-demand-probes-for-next
Anyway, I REALLY don't like this series (sorry for the delay in
reviewing them, normally I trust Rob's judgement...)
I can't see adding calls like this all over the tree just to solve a
bus-specific problem, you are adding of_* calls where they aren't
needed, or wanted, at all.
What is the root-problem of your delay in device probing? I read your
last patch series and I can't seem to figure out what the issue is that
this is solving in any "better" way from the existing deferred probing.
thanks,
greg k-h
Greg Kroah-Hartman
Oct 17, 2015, 3:00:07 AM10/17/15
to
none, which isn't going to be a good idea. It's completly normal for a
bus to not match a device, let's not be noisy for no good reason, unless
you are going to deal with all of the confused user emails?
You do this a bunch in this patch, please don't.
thanks,
greg k-h
Greg Kroah-Hartman
Oct 17, 2015, 3:00:07 AM10/17/15
to
On Tue, Sep 29, 2015 at 11:10:42AM +0200, Tomeu Vizoso wrote:
> Walks the OF tree up and finds the closest ancestor that has a struct
> device associated with it, probing it if isn't bound to a driver yet.
>
> The above should ensure that the dependency represented by the passed OF
> node is available, because probing a device should cause its descendants
> to be probed as well (when they get registered).
>
> Subsystems can use this when looking up resources for drivers, to reduce
> the chances of deferred probes because of the probing order of devices.
How do subsystems know to do this? Under what situation? Why is this a
> Walks the OF tree up and finds the closest ancestor that has a struct
> device associated with it, probing it if isn't bound to a driver yet.
>
> The above should ensure that the dependency represented by the passed OF
> node is available, because probing a device should cause its descendants
> to be probed as well (when they get registered).
>
> Subsystems can use this when looking up resources for drivers, to reduce
> the chances of deferred probes because of the probing order of devices.
of-only type thing? Why don't other busses need this?
I don't like to special-case a single bus like this at all if
possible...
thanks,
greg k-h
Rob Herring
Oct 17, 2015, 11:10:09 AM10/17/15
to
On Sat, Oct 17, 2015 at 1:57 AM, Greg Kroah-Hartman
<gre...@linuxfoundation.org> wrote:
> On Wed, Oct 14, 2015 at 10:34:00AM +0200, Tomeu Vizoso wrote:
>> Hi Rob,
>>
>> here is the pull request you asked for, with no changes from the version
>> that I posted last to the list.
>>
>> The following changes since commit 6ff33f3902c3b1c5d0db6b1e2c70b6d76fba357f:
>>
>> Linux 4.3-rc1 (2015-09-12 16:35:56 -0700)
>>
>> are available in the git repository at:
>>
>> git+ssh://git.collabora.co.uk/git/user/tomeu/linux.git
>> on-demand-probes-for-next
>
> That's not a signed tag :(
>
> Anyway, I REALLY don't like this series (sorry for the delay in
> reviewing them, normally I trust Rob's judgement...)
We've seen a lot of attempts here. This is really the best solution so
<gre...@linuxfoundation.org> wrote:
> On Wed, Oct 14, 2015 at 10:34:00AM +0200, Tomeu Vizoso wrote:
>> Hi Rob,
>>
>> here is the pull request you asked for, with no changes from the version
>> that I posted last to the list.
>>
>> The following changes since commit 6ff33f3902c3b1c5d0db6b1e2c70b6d76fba357f:
>>
>> Linux 4.3-rc1 (2015-09-12 16:35:56 -0700)
>>
>> are available in the git repository at:
>>
>> git+ssh://git.collabora.co.uk/git/user/tomeu/linux.git
>> on-demand-probes-for-next
>
> That's not a signed tag :(
>
> Anyway, I REALLY don't like this series (sorry for the delay in
> reviewing them, normally I trust Rob's judgement...)
far in that it is simple, uses existing data from DT, and was low risk
for breaking platforms (at least I thought it would be). Anyway,
getting more exposure is why I've put it into -next.
> I can't see adding calls like this all over the tree just to solve a
> bus-specific problem, you are adding of_* calls where they aren't
> needed, or wanted, at all.
that dependencies should be handled in the driver core. We went down
the path of making this not firmware (aka bus) specific and an earlier
version had just that (with fwnode_* calls). That turned out to be
pointless as the calling locations were almost always in DT specific
code anyway. If you notice, the calls are next to other DT specific
calls generally (usually a "get"). So yes, I'd prefer not to have to
touch every subsystem, but we had to do that anyway to add DT support.
We've generally split the DT code into the core (in drivers/of) and
the binding specific (in subsystems). Extracting dependency
information the DT is going to require binding specific knowledge, so
subsystem changes are probably unavoidable.
The alternative is we put binding specific knowledge into the core DT
code to parse dependencies.
> What is the root-problem of your delay in device probing? I read your
> last patch series and I can't seem to figure out what the issue is that
> this is solving in any "better" way from the existing deferred probing.
alone seems compelling to me.
Another downside to deferred probing is you have to touch every driver
and subsystem to support it. This contains the problem to the
subsystems.
Rob
Rob Herring
Oct 17, 2015, 11:30:06 AM10/17/15
to
On Fri, Oct 16, 2015 at 4:23 PM, Olof Johansson <ol...@lixom.net> wrote:
> Hi,
>
> I've bisected boot failures in next-20151016 down to patches in this branch:
>
> On Thu, Oct 15, 2015 at 4:42 AM, Tomeu Vizoso
> <tomeu....@collabora.com> wrote:
>> Tomeu Vizoso (20):
>> driver core: handle -EPROBE_DEFER from bus_type.match()
>
> The machine it happened on was OMAP5UEVM:
>
> http://arm-soc.lixom.net/bootlogs/next/next-20151016/omap5uevm-arm-omap2plus_defconfig.html
So this one is because the MMC node numbering changed. I don't know
> Hi,
>
> I've bisected boot failures in next-20151016 down to patches in this branch:
>
> On Thu, Oct 15, 2015 at 4:42 AM, Tomeu Vizoso
> <tomeu....@collabora.com> wrote:
>> Tomeu Vizoso (20):
>> driver core: handle -EPROBE_DEFER from bus_type.match()
>
> The machine it happened on was OMAP5UEVM:
>
> http://arm-soc.lixom.net/bootlogs/next/next-20151016/omap5uevm-arm-omap2plus_defconfig.html
how to fix that other than with aliases, but that doesn't solve
backwards compatibility.
> But I've also seen it on tegra2, that one bisected down to:
>
>> regulator: core: Probe regulators on demand
>
> http://arm-soc.lixom.net/bootlogs/next/next-20151016/seaboard-arm-multi_v7_defconfig.html
guaranteed to be done before the rootfs mounting AFAIK. There may be
other problems, but we can't see them since it panics.
Rob
Greg Kroah-Hartman
Oct 17, 2015, 11:50:06 AM10/17/15
to
On Sat, Oct 17, 2015 at 10:04:55AM -0500, Rob Herring wrote:
> On Sat, Oct 17, 2015 at 1:57 AM, Greg Kroah-Hartman
> <gre...@linuxfoundation.org> wrote:
> > On Wed, Oct 14, 2015 at 10:34:00AM +0200, Tomeu Vizoso wrote:
> >> Hi Rob,
> >>
> >> here is the pull request you asked for, with no changes from the version
> >> that I posted last to the list.
> >>
> >> The following changes since commit 6ff33f3902c3b1c5d0db6b1e2c70b6d76fba357f:
> >>
> >> Linux 4.3-rc1 (2015-09-12 16:35:56 -0700)
> >>
> >> are available in the git repository at:
> >>
> >> git+ssh://git.collabora.co.uk/git/user/tomeu/linux.git
> >> on-demand-probes-for-next
> >
> > That's not a signed tag :(
> >
> > Anyway, I REALLY don't like this series (sorry for the delay in
> > reviewing them, normally I trust Rob's judgement...)
>
> We've seen a lot of attempts here. This is really the best solution so
> far in that it is simple, uses existing data from DT, and was low risk
> for breaking platforms (at least I thought it would be). Anyway,
> getting more exposure is why I've put it into -next.
Exposure is good, now we know it breaks some builds, which was useful :)
> On Sat, Oct 17, 2015 at 1:57 AM, Greg Kroah-Hartman
> <gre...@linuxfoundation.org> wrote:
> > On Wed, Oct 14, 2015 at 10:34:00AM +0200, Tomeu Vizoso wrote:
> >> Hi Rob,
> >>
> >> here is the pull request you asked for, with no changes from the version
> >> that I posted last to the list.
> >>
> >> The following changes since commit 6ff33f3902c3b1c5d0db6b1e2c70b6d76fba357f:
> >>
> >> Linux 4.3-rc1 (2015-09-12 16:35:56 -0700)
> >>
> >> are available in the git repository at:
> >>
> >> git+ssh://git.collabora.co.uk/git/user/tomeu/linux.git
> >> on-demand-probes-for-next
> >
> > That's not a signed tag :(
> >
> > Anyway, I REALLY don't like this series (sorry for the delay in
> > reviewing them, normally I trust Rob's judgement...)
>
> We've seen a lot of attempts here. This is really the best solution so
> far in that it is simple, uses existing data from DT, and was low risk
> for breaking platforms (at least I thought it would be). Anyway,
> getting more exposure is why I've put it into -next.
> > I can't see adding calls like this all over the tree just to solve a
> > bus-specific problem, you are adding of_* calls where they aren't
> > needed, or wanted, at all.
>
> I think Linus W, Mark B, and I all said a similar thing initially in
> that dependencies should be handled in the driver core. We went down
> the path of making this not firmware (aka bus) specific and an earlier
> version had just that (with fwnode_* calls). That turned out to be
> pointless as the calling locations were almost always in DT specific
> code anyway. If you notice, the calls are next to other DT specific
> calls generally (usually a "get"). So yes, I'd prefer not to have to
> touch every subsystem, but we had to do that anyway to add DT support.
really object to having to "sprinkle" this all over the kernel, for no
obvious reason why that is happening at all (look at the USB patch for
one such example.)
> We've generally split the DT code into the core (in drivers/of) and
> the binding specific (in subsystems). Extracting dependency
> information the DT is going to require binding specific knowledge, so
> subsystem changes are probably unavoidable.
>
> The alternative is we put binding specific knowledge into the core DT
> code to parse dependencies.
>
> > What is the root-problem of your delay in device probing? I read your
> > last patch series and I can't seem to figure out what the issue is that
> > this is solving in any "better" way from the existing deferred probing.
>
> It saves 2 seconds in the boot time as re-probing takes time. That
> alone seems compelling to me.
sleeping and causing this problem. What does the bootlog time-chart say
is really causing this long delay? There's no way we are stuck in some
sort of logic loop for that long (i.e. having to walk the list of
devices somehow.) This sounds like a driver-specific problem that is
being worked around by having to touch all subsystems, which isn't nice.
Hint, we didn't have to do this type of thing to solve boot delays on
x86 when we had hardware that was slow to initialize, why should DT be
special? :)
> Another downside to deferred probing is you have to touch every driver
> and subsystem to support it. This contains the problem to the
> subsystems.
it have to do anything, why create yet-another model here?
thanks,
greg k-h
Rob Herring
Oct 17, 2015, 12:30:12 PM10/17/15
to
On Sat, Oct 17, 2015 at 10:47 AM, Greg Kroah-Hartman
They do show the fragile nature of probe ordering and the implicit
dependencies we have.
>> > I can't see adding calls like this all over the tree just to solve a
>> > bus-specific problem, you are adding of_* calls where they aren't
>> > needed, or wanted, at all.
>>
>> I think Linus W, Mark B, and I all said a similar thing initially in
>> that dependencies should be handled in the driver core. We went down
>> the path of making this not firmware (aka bus) specific and an earlier
>> version had just that (with fwnode_* calls). That turned out to be
>> pointless as the calling locations were almost always in DT specific
>> code anyway. If you notice, the calls are next to other DT specific
>> calls generally (usually a "get"). So yes, I'd prefer not to have to
>> touch every subsystem, but we had to do that anyway to add DT support.
>
> If they are "next" to a call like that, why not put it in that call? I
> really object to having to "sprinkle" this all over the kernel, for no
> obvious reason why that is happening at all (look at the USB patch for
> one such example.)
Looking at it again, they are in DT specific code already. The USB one
is in devm_usb_get_phy_by_node() which is a DT specific call.
>> We've generally split the DT code into the core (in drivers/of) and
>> the binding specific (in subsystems). Extracting dependency
>> information the DT is going to require binding specific knowledge, so
>> subsystem changes are probably unavoidable.
>>
>> The alternative is we put binding specific knowledge into the core DT
>> code to parse dependencies.
>>
>> > What is the root-problem of your delay in device probing? I read your
>> > last patch series and I can't seem to figure out what the issue is that
>> > this is solving in any "better" way from the existing deferred probing.
>>
>> It saves 2 seconds in the boot time as re-probing takes time. That
>> alone seems compelling to me.
>
> 2 seconds is _forever_, and really seems like some other driver is
> sleeping and causing this problem. What does the bootlog time-chart say
> is really causing this long delay? There's no way we are stuck in some
> sort of logic loop for that long (i.e. having to walk the list of
> devices somehow.) This sounds like a driver-specific problem that is
> being worked around by having to touch all subsystems, which isn't nice.
I don't think it is one driver as the improvement is seen on multiple
platforms. I'll let Tomeu comment further on where the time was spent.
> Hint, we didn't have to do this type of thing to solve boot delays on
> x86 when we had hardware that was slow to initialize, why should DT be
> special? :)
x86 did not need deferred probe either (though we probably can find
some initcall ordering hacks). This is an embedded problem, not a DT
problem.
I'm guessing the time is a matter of probing and undoing the probes
rather than slow h/w. We could maybe improve things by making sure
drivers move what they defer on to the beginning of probe, but that
seems like a horrible, fragile hack.
>> Another downside to deferred probing is you have to touch every driver
>> and subsystem to support it. This contains the problem to the
>> subsystems.
>
> But we have deferred probing already, only those drivers that need/want
> it have to do anything, why create yet-another model here?
Yes, the only ones needing it are drivers dependent on clocks, gpio,
regulators, pwm, pin-ctrl, dma, etc. That's not a small number. This
is a side benefit and wouldn't take this series for that reason alone.
I've used the deferred probing is good enough argument myself on
previous attempts. The boot time improvements convinced me it is not
good enough except for simple cases.
Rob
<gre...@linuxfoundation.org> wrote:
> On Sat, Oct 17, 2015 at 10:04:55AM -0500, Rob Herring wrote:
>> On Sat, Oct 17, 2015 at 1:57 AM, Greg Kroah-Hartman
>> <gre...@linuxfoundation.org> wrote:
>> > On Wed, Oct 14, 2015 at 10:34:00AM +0200, Tomeu Vizoso wrote:
>> >> Hi Rob,
>> >>
>> >> here is the pull request you asked for, with no changes from the version
>> >> that I posted last to the list.
>> >>
>> >> The following changes since commit 6ff33f3902c3b1c5d0db6b1e2c70b6d76fba357f:
>> >>
>> >> Linux 4.3-rc1 (2015-09-12 16:35:56 -0700)
>> >>
>> >> are available in the git repository at:
>> >>
>> >> git+ssh://git.collabora.co.uk/git/user/tomeu/linux.git
>> >> on-demand-probes-for-next
>> >
>> > That's not a signed tag :(
>> >
>> > Anyway, I REALLY don't like this series (sorry for the delay in
>> > reviewing them, normally I trust Rob's judgement...)
>>
>> We've seen a lot of attempts here. This is really the best solution so
>> far in that it is simple, uses existing data from DT, and was low risk
>> for breaking platforms (at least I thought it would be). Anyway,
>> getting more exposure is why I've put it into -next.
>
> Exposure is good, now we know it breaks some builds, which was useful :)
Now that I've looked at them, they are somewhat questionable failures.
> On Sat, Oct 17, 2015 at 10:04:55AM -0500, Rob Herring wrote:
>> On Sat, Oct 17, 2015 at 1:57 AM, Greg Kroah-Hartman
>> <gre...@linuxfoundation.org> wrote:
>> > On Wed, Oct 14, 2015 at 10:34:00AM +0200, Tomeu Vizoso wrote:
>> >> Hi Rob,
>> >>
>> >> here is the pull request you asked for, with no changes from the version
>> >> that I posted last to the list.
>> >>
>> >> The following changes since commit 6ff33f3902c3b1c5d0db6b1e2c70b6d76fba357f:
>> >>
>> >> Linux 4.3-rc1 (2015-09-12 16:35:56 -0700)
>> >>
>> >> are available in the git repository at:
>> >>
>> >> git+ssh://git.collabora.co.uk/git/user/tomeu/linux.git
>> >> on-demand-probes-for-next
>> >
>> > That's not a signed tag :(
>> >
>> > Anyway, I REALLY don't like this series (sorry for the delay in
>> > reviewing them, normally I trust Rob's judgement...)
>>
>> We've seen a lot of attempts here. This is really the best solution so
>> far in that it is simple, uses existing data from DT, and was low risk
>> for breaking platforms (at least I thought it would be). Anyway,
>> getting more exposure is why I've put it into -next.
>
> Exposure is good, now we know it breaks some builds, which was useful :)
They do show the fragile nature of probe ordering and the implicit
dependencies we have.
>> > I can't see adding calls like this all over the tree just to solve a
>> > bus-specific problem, you are adding of_* calls where they aren't
>> > needed, or wanted, at all.
>>
>> I think Linus W, Mark B, and I all said a similar thing initially in
>> that dependencies should be handled in the driver core. We went down
>> the path of making this not firmware (aka bus) specific and an earlier
>> version had just that (with fwnode_* calls). That turned out to be
>> pointless as the calling locations were almost always in DT specific
>> code anyway. If you notice, the calls are next to other DT specific
>> calls generally (usually a "get"). So yes, I'd prefer not to have to
>> touch every subsystem, but we had to do that anyway to add DT support.
>
> If they are "next" to a call like that, why not put it in that call? I
> really object to having to "sprinkle" this all over the kernel, for no
> obvious reason why that is happening at all (look at the USB patch for
> one such example.)
is in devm_usb_get_phy_by_node() which is a DT specific call.
>> We've generally split the DT code into the core (in drivers/of) and
>> the binding specific (in subsystems). Extracting dependency
>> information the DT is going to require binding specific knowledge, so
>> subsystem changes are probably unavoidable.
>>
>> The alternative is we put binding specific knowledge into the core DT
>> code to parse dependencies.
>>
>> > What is the root-problem of your delay in device probing? I read your
>> > last patch series and I can't seem to figure out what the issue is that
>> > this is solving in any "better" way from the existing deferred probing.
>>
>> It saves 2 seconds in the boot time as re-probing takes time. That
>> alone seems compelling to me.
>
> 2 seconds is _forever_, and really seems like some other driver is
> sleeping and causing this problem. What does the bootlog time-chart say
> is really causing this long delay? There's no way we are stuck in some
> sort of logic loop for that long (i.e. having to walk the list of
> devices somehow.) This sounds like a driver-specific problem that is
> being worked around by having to touch all subsystems, which isn't nice.
platforms. I'll let Tomeu comment further on where the time was spent.
> Hint, we didn't have to do this type of thing to solve boot delays on
> x86 when we had hardware that was slow to initialize, why should DT be
> special? :)
some initcall ordering hacks). This is an embedded problem, not a DT
problem.
I'm guessing the time is a matter of probing and undoing the probes
rather than slow h/w. We could maybe improve things by making sure
drivers move what they defer on to the beginning of probe, but that
seems like a horrible, fragile hack.
>> Another downside to deferred probing is you have to touch every driver
>> and subsystem to support it. This contains the problem to the
>> subsystems.
>
> But we have deferred probing already, only those drivers that need/want
> it have to do anything, why create yet-another model here?
regulators, pwm, pin-ctrl, dma, etc. That's not a small number. This
is a side benefit and wouldn't take this series for that reason alone.
I've used the deferred probing is good enough argument myself on
previous attempts. The boot time improvements convinced me it is not
good enough except for simple cases.
Rob
Greg Kroah-Hartman
Oct 17, 2015, 1:00:07 PM10/17/15
to
to add a new .h file, which implies that suddenly this file is now
touching a new subsystem.
> >> We've generally split the DT code into the core (in drivers/of) and
> >> the binding specific (in subsystems). Extracting dependency
> >> information the DT is going to require binding specific knowledge, so
> >> subsystem changes are probably unavoidable.
> >>
> >> The alternative is we put binding specific knowledge into the core DT
> >> code to parse dependencies.
> >>
> >> > What is the root-problem of your delay in device probing? I read your
> >> > last patch series and I can't seem to figure out what the issue is that
> >> > this is solving in any "better" way from the existing deferred probing.
> >>
> >> It saves 2 seconds in the boot time as re-probing takes time. That
> >> alone seems compelling to me.
> >
> > 2 seconds is _forever_, and really seems like some other driver is
> > sleeping and causing this problem. What does the bootlog time-chart say
> > is really causing this long delay? There's no way we are stuck in some
> > sort of logic loop for that long (i.e. having to walk the list of
> > devices somehow.) This sounds like a driver-specific problem that is
> > being worked around by having to touch all subsystems, which isn't nice.
>
> I don't think it is one driver as the improvement is seen on multiple
> platforms. I'll let Tomeu comment further on where the time was spent.
boots to a gnome login faster than that.)
> > Hint, we didn't have to do this type of thing to solve boot delays on
> > x86 when we had hardware that was slow to initialize, why should DT be
> > special? :)
>
> x86 did not need deferred probe either (though we probably can find
> some initcall ordering hacks). This is an embedded problem, not a DT
> problem.
> I'm guessing the time is a matter of probing and undoing the probes
> rather than slow h/w. We could maybe improve things by making sure
> drivers move what they defer on to the beginning of probe, but that
> seems like a horrible, fragile hack.
probe calls are failing here? Again, a boot log graph would be great to
see as it will show the root cause, not just guessing at this.
> >> Another downside to deferred probing is you have to touch every driver
> >> and subsystem to support it. This contains the problem to the
> >> subsystems.
> >
> > But we have deferred probing already, only those drivers that need/want
> > it have to do anything, why create yet-another model here?
>
> Yes, the only ones needing it are drivers dependent on clocks, gpio,
> regulators, pwm, pin-ctrl, dma, etc. That's not a small number. This
> is a side benefit and wouldn't take this series for that reason alone.
>
> I've used the deferred probing is good enough argument myself on
> previous attempts. The boot time improvements convinced me it is not
> good enough except for simple cases.
yet-another-way-to-probe instead please, as we will be forever
sprinkling these calls around subsystems in a cargo-cult-like manner for
forever.
thanks,
greg k-h
Rob Clark
Oct 17, 2015, 2:00:14 PM10/17/15
to
On Sat, Oct 17, 2015 at 12:56 PM, Greg Kroah-Hartman
<gre...@linuxfoundation.org> wrote:
>> I'm guessing the time is a matter of probing and undoing the probes
>> rather than slow h/w. We could maybe improve things by making sure
>> drivers move what they defer on to the beginning of probe, but that
>> seems like a horrible, fragile hack.
>
> How can calling probe and failing cause 2 seconds? How many different
> probe calls are failing here? Again, a boot log graph would be great to
> see as it will show the root cause, not just guessing at this.
just fwiw, but when you have a driver that depends on several other
<gre...@linuxfoundation.org> wrote:
>> I'm guessing the time is a matter of probing and undoing the probes
>> rather than slow h/w. We could maybe improve things by making sure
>> drivers move what they defer on to the beginning of probe, but that
>> seems like a horrible, fragile hack.
>
> How can calling probe and failing cause 2 seconds? How many different
> probe calls are failing here? Again, a boot log graph would be great to
> see as it will show the root cause, not just guessing at this.
drivers (which in turn depend on other drivers and so on), the amount
of probe-defer we end up seeing is pretty comical. Yeah, there
probably is some room to optimize by juggling around order drivers do
things in probe. But that doesn't solve the fundamental problem with
the current state, about probe order having no clue about
dependencies..
BR,
-R
Greg Kroah-Hartman
Oct 17, 2015, 2:30:07 PM10/17/15
to
On Sat, Oct 17, 2015 at 01:54:43PM -0400, Rob Clark wrote:
> On Sat, Oct 17, 2015 at 12:56 PM, Greg Kroah-Hartman
> <gre...@linuxfoundation.org> wrote:
> >> I'm guessing the time is a matter of probing and undoing the probes
> >> rather than slow h/w. We could maybe improve things by making sure
> >> drivers move what they defer on to the beginning of probe, but that
> >> seems like a horrible, fragile hack.
> >
> > How can calling probe and failing cause 2 seconds? How many different
> > probe calls are failing here? Again, a boot log graph would be great to
> > see as it will show the root cause, not just guessing at this.
>
>
> just fwiw, but when you have a driver that depends on several other
> drivers (which in turn depend on other drivers and so on), the amount
> of probe-defer we end up seeing is pretty comical. Yeah, there
> probably is some room to optimize by juggling around order drivers do
> things in probe. But that doesn't solve the fundamental problem with
> the current state, about probe order having no clue about
> dependencies..
I can imagine it is a lot of iterations, but how long does it really
> On Sat, Oct 17, 2015 at 12:56 PM, Greg Kroah-Hartman
> <gre...@linuxfoundation.org> wrote:
> >> I'm guessing the time is a matter of probing and undoing the probes
> >> rather than slow h/w. We could maybe improve things by making sure
> >> drivers move what they defer on to the beginning of probe, but that
> >> seems like a horrible, fragile hack.
> >
> > How can calling probe and failing cause 2 seconds? How many different
> > probe calls are failing here? Again, a boot log graph would be great to
> > see as it will show the root cause, not just guessing at this.
>
>
> just fwiw, but when you have a driver that depends on several other
> drivers (which in turn depend on other drivers and so on), the amount
> of probe-defer we end up seeing is pretty comical. Yeah, there
> probably is some room to optimize by juggling around order drivers do
> things in probe. But that doesn't solve the fundamental problem with
> the current state, about probe order having no clue about
> dependencies..
take? How many different devices are involved that it takes multiple
loops in order to finally work out the correct order? Where is the time
delays here, just calling probe() and having it instantly return
shouldn't take all that long.
thanks,
greg k-h
Rob Clark
Oct 17, 2015, 2:50:05 PM10/17/15
to
On Sat, Oct 17, 2015 at 2:27 PM, Greg Kroah-Hartman
I'd say, from memory, I see drm/msm taking at least 5 or 6 tries to
get all the way through requesting it's various different
regulators/clks/gpios. I hadn't really paid attention to how many
tries the drivers I depend on go through. (Of those, I take clks from
two different clk drivers (which have dependency on a 3rd clk driver),
and regulators and gpio's come from at least two places, which in turn
have dependencies on clks, etc.) I don't have really good hard
numbers handy (since my observations of this are w/ console over uart
which effects timings, and so I see it taking much longer than 2sec)..
but the 2sec figure that Tomeu mentioned seemed pretty plausible to
me.
I can try to get better #'s... I should have my kernel hat on at least
some of the time next week.. but the 2sec figure didn't seem
unrealistic to me.
Just as an aside, the amount of probe-defer adds quite a lot of noise
when you are trying to debug why some driver doesn't probe
successfully. Which itself would be a nice reason to do something
more clever..
BR,
-R
<gre...@linuxfoundation.org> wrote:
> On Sat, Oct 17, 2015 at 01:54:43PM -0400, Rob Clark wrote:
>> On Sat, Oct 17, 2015 at 12:56 PM, Greg Kroah-Hartman
>> <gre...@linuxfoundation.org> wrote:
>> >> I'm guessing the time is a matter of probing and undoing the probes
>> >> rather than slow h/w. We could maybe improve things by making sure
>> >> drivers move what they defer on to the beginning of probe, but that
>> >> seems like a horrible, fragile hack.
>> >
>> > How can calling probe and failing cause 2 seconds? How many different
>> > probe calls are failing here? Again, a boot log graph would be great to
>> > see as it will show the root cause, not just guessing at this.
>>
>>
>> just fwiw, but when you have a driver that depends on several other
>> drivers (which in turn depend on other drivers and so on), the amount
>> of probe-defer we end up seeing is pretty comical. Yeah, there
>> probably is some room to optimize by juggling around order drivers do
>> things in probe. But that doesn't solve the fundamental problem with
>> the current state, about probe order having no clue about
>> dependencies..
>
> I can imagine it is a lot of iterations, but how long does it really
> take? How many different devices are involved that it takes multiple
> loops in order to finally work out the correct order? Where is the time
> delays here, just calling probe() and having it instantly return
> shouldn't take all that long.
offhand, I think the dependencies go at *least* three levels deep..
> On Sat, Oct 17, 2015 at 01:54:43PM -0400, Rob Clark wrote:
>> On Sat, Oct 17, 2015 at 12:56 PM, Greg Kroah-Hartman
>> <gre...@linuxfoundation.org> wrote:
>> >> I'm guessing the time is a matter of probing and undoing the probes
>> >> rather than slow h/w. We could maybe improve things by making sure
>> >> drivers move what they defer on to the beginning of probe, but that
>> >> seems like a horrible, fragile hack.
>> >
>> > How can calling probe and failing cause 2 seconds? How many different
>> > probe calls are failing here? Again, a boot log graph would be great to
>> > see as it will show the root cause, not just guessing at this.
>>
>>
>> just fwiw, but when you have a driver that depends on several other
>> drivers (which in turn depend on other drivers and so on), the amount
>> of probe-defer we end up seeing is pretty comical. Yeah, there
>> probably is some room to optimize by juggling around order drivers do
>> things in probe. But that doesn't solve the fundamental problem with
>> the current state, about probe order having no clue about
>> dependencies..
>
> I can imagine it is a lot of iterations, but how long does it really
> take? How many different devices are involved that it takes multiple
> loops in order to finally work out the correct order? Where is the time
> delays here, just calling probe() and having it instantly return
> shouldn't take all that long.
I'd say, from memory, I see drm/msm taking at least 5 or 6 tries to
get all the way through requesting it's various different
regulators/clks/gpios. I hadn't really paid attention to how many
tries the drivers I depend on go through. (Of those, I take clks from
two different clk drivers (which have dependency on a 3rd clk driver),
and regulators and gpio's come from at least two places, which in turn
have dependencies on clks, etc.) I don't have really good hard
numbers handy (since my observations of this are w/ console over uart
which effects timings, and so I see it taking much longer than 2sec)..
but the 2sec figure that Tomeu mentioned seemed pretty plausible to
me.
I can try to get better #'s... I should have my kernel hat on at least
some of the time next week.. but the 2sec figure didn't seem
unrealistic to me.
Just as an aside, the amount of probe-defer adds quite a lot of noise
when you are trying to debug why some driver doesn't probe
successfully. Which itself would be a nice reason to do something
more clever..
BR,
-R
Greg Kroah-Hartman
Oct 17, 2015, 3:00:07 PM10/17/15
to
> I hadn't really paid attention to how many
> tries the drivers I depend on go through. (Of those, I take clks from
> two different clk drivers (which have dependency on a 3rd clk driver),
> and regulators and gpio's come from at least two places, which in turn
> have dependencies on clks, etc.) I don't have really good hard
> numbers handy (since my observations of this are w/ console over uart
> which effects timings, and so I see it taking much longer than 2sec)..
> but the 2sec figure that Tomeu mentioned seemed pretty plausible to
> me.
>
> I can try to get better #'s... I should have my kernel hat on at least
> some of the time next week.. but the 2sec figure didn't seem
> unrealistic to me.
forever, there has to be something else going on here other than just
calling probe() a bunch of times. Please use the tools we have to
determine this before trying to change the driver core.
> Just as an aside, the amount of probe-defer adds quite a lot of noise
> when you are trying to debug why some driver doesn't probe
> successfully. Which itself would be a nice reason to do something
> more clever..
that should speed things up :)
Noralf Trønnes
Oct 17, 2015, 3:10:07 PM10/17/15
to
deferred probing, or that display initialization is happening 2 sec
after it's first try?
Rob Clark
Oct 17, 2015, 3:40:07 PM10/17/15
to
On Sat, Oct 17, 2015 at 2:59 PM, Greg Kroah-Hartman
yes, I am aware of the tools.. although so far I spend most of my time
just trying to get things working in the first place ;-)
All I was trying to point out was that Tomeu's figures didn't really
seem unrealistic. I mean, given that the average SoC driver probably
depends on at least one clock and at least one regulator, having to
probe each driver at least twice seems plausible. And that having a
noticeable effect on boot time doesn't seem surprising. I'm not sure
that saying 'modern laptop can boot in 2sec' adds much to the
discussion since I don't think you have quite so much interdependency
between devices vs random probe order. I have seen arm devices boot
to UI in similar times, but that was pre-devicetree days.
I expect Tomeu has some better number.. if not I can collect some.
>> Just as an aside, the amount of probe-defer adds quite a lot of noise
>> when you are trying to debug why some driver doesn't probe
>> successfully. Which itself would be a nice reason to do something
>> more clever..
>
> People seem to not like the noise, so let's turn off those messages,
> that should speed things up :)
heh, except for when you are trying to debug what is missing
preventing the driver you depend on from probing ;-)
BR,
-R
just trying to get things working in the first place ;-)
All I was trying to point out was that Tomeu's figures didn't really
seem unrealistic. I mean, given that the average SoC driver probably
depends on at least one clock and at least one regulator, having to
probe each driver at least twice seems plausible. And that having a
noticeable effect on boot time doesn't seem surprising. I'm not sure
that saying 'modern laptop can boot in 2sec' adds much to the
discussion since I don't think you have quite so much interdependency
between devices vs random probe order. I have seen arm devices boot
to UI in similar times, but that was pre-devicetree days.
I expect Tomeu has some better number.. if not I can collect some.
>> Just as an aside, the amount of probe-defer adds quite a lot of noise
>> when you are trying to debug why some driver doesn't probe
>> successfully. Which itself would be a nice reason to do something
>> more clever..
>
> People seem to not like the noise, so let's turn off those messages,
> that should speed things up :)
preventing the driver you depend on from probing ;-)
BR,
-R
Rob Clark
Oct 17, 2015, 3:50:06 PM10/17/15
to
in first pass through list of devices (and ofc deferring the first
time), I'll say "probably both"..
BR,
-R
Greg Kroah-Hartman
Oct 17, 2015, 4:30:08 PM10/17/15
to
to put in more effort, sorry. Don't expect the driver core to work
around driver bugs for you.
> All I was trying to point out was that Tomeu's figures didn't really
> seem unrealistic. I mean, given that the average SoC driver probably
> depends on at least one clock and at least one regulator, having to
> probe each driver at least twice seems plausible. And that having a
> noticeable effect on boot time doesn't seem surprising. I'm not sure
> that saying 'modern laptop can boot in 2sec' adds much to the
> discussion since I don't think you have quite so much interdependency
> between devices vs random probe order. I have seen arm devices boot
> to UI in similar times, but that was pre-devicetree days.
broken drivers to me :)
thanks,
greg k-h
Mark Brown
Oct 18, 2015, 3:40:07 PM10/18/15
to
On Fri, Oct 16, 2015 at 11:57:50PM -0700, Greg Kroah-Hartman wrote:
> I can't see adding calls like this all over the tree just to solve a
> bus-specific problem, you are adding of_* calls where they aren't
> needed, or wanted, at all.
This isn't bus specific, I'm not sure what makes you say that?
> I can't see adding calls like this all over the tree just to solve a
> bus-specific problem, you are adding of_* calls where they aren't
> needed, or wanted, at all.
> What is the root-problem of your delay in device probing? I read your
> last patch series and I can't seem to figure out what the issue is that
> this is solving in any "better" way from the existing deferred probing.
this (at least not for my use cases) so there's a bit of educated
guessing going on here but there's two broad things I'm aware of.
One is that regardless of the actual performance of the system when
deferred probe goes off it splats errors all over the console which
makes it look like something is going wrong even if everything is fine
in the end. If lots of deferred probing happens then the volume gets
big too. People find this distracting, noisy and ugly - it obscures
actual issues and trains people to ignore errors. I do think this is a
reasonable concern and that it's worth trying to mitigate against
deferral for this reason alone. We don't want to just ignore the errors
and not print anything either since if the resource doesn't appear the
user needs to know what is preventing the driver from instantiating so
they can try to fix it.
The other is that if you're printing to a serial console then that's not
an especially fast operation so if you're getting lots of messages being
printed simply physically outputting them takes measurable time. I'm
not aware of any performance concerns outside of that, but like I say
I'm not affected by this myself in any great way. Obviously this can be
configured but not having actual errors on the console isn't super
awesome either for systems that make use of the logging there and we
don't have a good way of telling what's from deferral and what's not.
Greg Kroah-Hartman
Oct 18, 2015, 3:40:07 PM10/18/15
to
On Sun, Oct 18, 2015 at 08:29:31PM +0100, Mark Brown wrote:
> On Fri, Oct 16, 2015 at 11:57:50PM -0700, Greg Kroah-Hartman wrote:
>
> > I can't see adding calls like this all over the tree just to solve a
> > bus-specific problem, you are adding of_* calls where they aren't
> > needed, or wanted, at all.
>
> This isn't bus specific, I'm not sure what makes you say that?
You are making it bus-specific by putting these calls all over the tree
> On Fri, Oct 16, 2015 at 11:57:50PM -0700, Greg Kroah-Hartman wrote:
>
> > I can't see adding calls like this all over the tree just to solve a
> > bus-specific problem, you are adding of_* calls where they aren't
> > needed, or wanted, at all.
>
> This isn't bus specific, I'm not sure what makes you say that?
in different bus subsystems semi-randomly for all I can determine.
> > What is the root-problem of your delay in device probing? I read your
> > last patch series and I can't seem to figure out what the issue is that
> > this is solving in any "better" way from the existing deferred probing.
>
> So, I don't actually have any platforms that are especially bothered by
> this (at least not for my use cases) so there's a bit of educated
> guessing going on here but there's two broad things I'm aware of.
>
> One is that regardless of the actual performance of the system when
> deferred probe goes off it splats errors all over the console which
> makes it look like something is going wrong even if everything is fine
> in the end. If lots of deferred probing happens then the volume gets
> big too. People find this distracting, noisy and ugly - it obscures
> actual issues and trains people to ignore errors. I do think this is a
> reasonable concern and that it's worth trying to mitigate against
> deferral for this reason alone. We don't want to just ignore the errors
> and not print anything either since if the resource doesn't appear the
> user needs to know what is preventing the driver from instantiating so
> they can try to fix it.
message into a debug message that can be dynamically enabled for those
people wanting to debug their systems for boot time issues.
Please send a patch to do so.
Mark Brown
Oct 18, 2015, 3:50:07 PM10/18/15
to
On Sat, Oct 17, 2015 at 08:47:09AM -0700, Greg Kroah-Hartman wrote:
> On Sat, Oct 17, 2015 at 10:04:55AM -0500, Rob Herring wrote:
> > I think Linus W, Mark B, and I all said a similar thing initially in
> > that dependencies should be handled in the driver core. We went down
> > the path of making this not firmware (aka bus) specific and an earlier
> > version had just that (with fwnode_* calls). That turned out to be
> > pointless as the calling locations were almost always in DT specific
> > code anyway. If you notice, the calls are next to other DT specific
> > calls generally (usually a "get"). So yes, I'd prefer not to have to
> > touch every subsystem, but we had to do that anyway to add DT support.
> If they are "next" to a call like that, why not put it in that call? I
> really object to having to "sprinkle" this all over the kernel, for no
> obvious reason why that is happening at all (look at the USB patch for
> one such example.)
I did ask that question myself IIRC - we could probably get a long way
> On Sat, Oct 17, 2015 at 10:04:55AM -0500, Rob Herring wrote:
> > I think Linus W, Mark B, and I all said a similar thing initially in
> > that dependencies should be handled in the driver core. We went down
> > the path of making this not firmware (aka bus) specific and an earlier
> > version had just that (with fwnode_* calls). That turned out to be
> > pointless as the calling locations were almost always in DT specific
> > code anyway. If you notice, the calls are next to other DT specific
> > calls generally (usually a "get"). So yes, I'd prefer not to have to
> > touch every subsystem, but we had to do that anyway to add DT support.
> If they are "next" to a call like that, why not put it in that call? I
> really object to having to "sprinkle" this all over the kernel, for no
> obvious reason why that is happening at all (look at the USB patch for
> one such example.)
by trying to instantiate anything that looks probable when we do a
phandle lookup on it.
Mark Brown
Oct 18, 2015, 4:00:08 PM10/18/15
to
On Sun, Oct 18, 2015 at 12:37:57PM -0700, Greg Kroah-Hartman wrote:
> On Sun, Oct 18, 2015 at 08:29:31PM +0100, Mark Brown wrote:
> > On Fri, Oct 16, 2015 at 11:57:50PM -0700, Greg Kroah-Hartman wrote:
> > > I can't see adding calls like this all over the tree just to solve a
> > > bus-specific problem, you are adding of_* calls where they aren't
> > > needed, or wanted, at all.
> > This isn't bus specific, I'm not sure what makes you say that?
> You are making it bus-specific by putting these calls all over the tree
> in different bus subsystems semi-randomly for all I can determine.
Do you mean firmware rather than bus here? I think that's the confusion
> On Sun, Oct 18, 2015 at 08:29:31PM +0100, Mark Brown wrote:
> > On Fri, Oct 16, 2015 at 11:57:50PM -0700, Greg Kroah-Hartman wrote:
> > > I can't see adding calls like this all over the tree just to solve a
> > > bus-specific problem, you are adding of_* calls where they aren't
> > > needed, or wanted, at all.
> > This isn't bus specific, I'm not sure what makes you say that?
> You are making it bus-specific by putting these calls all over the tree
> in different bus subsystems semi-randomly for all I can determine.
I have...
> > One is that regardless of the actual performance of the system when
> > deferred probe goes off it splats errors all over the console which
> > makes it look like something is going wrong even if everything is fine
> > in the end. If lots of deferred probing happens then the volume gets
> > big too. People find this distracting, noisy and ugly - it obscures
> > actual issues and trains people to ignore errors. I do think this is a
> > reasonable concern and that it's worth trying to mitigate against
> > deferral for this reason alone. We don't want to just ignore the errors
> > and not print anything either since if the resource doesn't appear the
> > user needs to know what is preventing the driver from instantiating so
> > they can try to fix it.
> This has come up many times, I have no objection to just turning that
> message into a debug message that can be dynamically enabled for those
> people wanting to debug their systems for boot time issues.
drivers logging that they failed to get whatever resource since silently
failing is not a great user experience. Many, hopefully most, of the
drivers don't actually have special handling for probe deferral since
half the beauty of probe deferral is that the subsystem supplying the
resource can just return -EPROBE_DEFER when it notices something is
missing but might appear and then the drivers will do the right thing so
long as they have error handling code that they really should have
anyway.
We'd need to have a special dev_err() that handled probe deferral
errors for drivers to use during probe or some other smarts in the
logging infrastructure. Which isn't a totally horrible idea.
Russell King - ARM Linux
Oct 19, 2015, 6:00:08 AM10/19/15
to
On Mon, Oct 19, 2015 at 10:44:41AM +0100, David Woodhouse wrote:
> be gratuitously firmware-specific. Nothing should be using those these
> days; any new code should be using the generic device property APIs
> (except in special cases).
I asked Linus Walleij about that with the fwnode_get_named_gpiod() stuff,
and Linus didn't seem to know how this should be used.
It doesn't help that dev->fwnode is not initialised, but dev->of_node
is. Are we supposed to grope around in dev->of_node for the embedded
fwnode instead of using dev->fwnode?
At the moment, at least to me, fwnode looks like some kind of
experimental half-baked thing rather than a real usable solution.
--
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
> On Sun, 2015-10-18 at 20:53 +0100, Mark Brown wrote:
> > On Sun, Oct 18, 2015 at 12:37:57PM -0700, Greg Kroah-Hartman wrote:
> > > On Sun, Oct 18, 2015 at 08:29:31PM +0100, Mark Brown wrote:
> > > > On Fri, Oct 16, 2015 at 11:57:50PM -0700, Greg Kroah-Hartman wrote:
> >
> > > > > I can't see adding calls like this all over the tree just to solve a
> > > > > bus-specific problem, you are adding of_* calls where they aren't
> > > > > needed, or wanted, at all.
> >
> > > > This isn't bus specific, I'm not sure what makes you say that?
> >
> > > You are making it bus-specific by putting these calls all over the tree
> > > in different bus subsystems semi-randomly for all I can determine.
> >
> > Do you mean firmware rather than bus here? I think that's the confusion
> > I have...
>
> Certainly, if it literally is adding of_* calls then that would seem to
> > On Sun, Oct 18, 2015 at 12:37:57PM -0700, Greg Kroah-Hartman wrote:
> > > On Sun, Oct 18, 2015 at 08:29:31PM +0100, Mark Brown wrote:
> > > > On Fri, Oct 16, 2015 at 11:57:50PM -0700, Greg Kroah-Hartman wrote:
> >
> > > > > I can't see adding calls like this all over the tree just to solve a
> > > > > bus-specific problem, you are adding of_* calls where they aren't
> > > > > needed, or wanted, at all.
> >
> > > > This isn't bus specific, I'm not sure what makes you say that?
> >
> > > You are making it bus-specific by putting these calls all over the tree
> > > in different bus subsystems semi-randomly for all I can determine.
> >
> > Do you mean firmware rather than bus here? I think that's the confusion
> > I have...
>
> be gratuitously firmware-specific. Nothing should be using those these
> days; any new code should be using the generic device property APIs
> (except in special cases).
I asked Linus Walleij about that with the fwnode_get_named_gpiod() stuff,
and Linus didn't seem to know how this should be used.
It doesn't help that dev->fwnode is not initialised, but dev->of_node
is. Are we supposed to grope around in dev->of_node for the embedded
fwnode instead of using dev->fwnode?
At the moment, at least to me, fwnode looks like some kind of
experimental half-baked thing rather than a real usable solution.
--
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
Mark Brown
Oct 19, 2015, 7:10:08 AM10/19/15
to
On Mon, Oct 19, 2015 at 10:44:41AM +0100, David Woodhouse wrote:
> On Sun, 2015-10-18 at 20:53 +0100, Mark Brown wrote:
> On Sun, 2015-10-18 at 20:53 +0100, Mark Brown wrote:
> > Do you mean firmware rather than bus here? I think that's the confusion
> > I have...
> > I have...
> Certainly, if it literally is adding of_* calls then that would seem to
> be gratuitously firmware-specific. Nothing should be using those these
> days; any new code should be using the generic device property APIs
> (except in special cases).
It's not entirely clear to me that we should be moving to fwnode_
> be gratuitously firmware-specific. Nothing should be using those these
> days; any new code should be using the generic device property APIs
> (except in special cases).
wholesale yet - the last advice was to hold off for a little while which
makes sense given that the ACPI community still doesn't seem to have
worked out what it wants to do here and how. The x86 embedded people
are all gung ho but it's less clear that anyone else wants to use _DSD
in quite the same way (I know of some efforts to use _DSD separately to
the DT compatibility stuff) and there are some vendors who definitely do
have completely different binding schemes for ACPI and DT and therefore
specifically care which is in use.
It would really help if ACPI could get their binding review process in
place, and if we do want to actually start converting everything to
fwnode_ we need to start communicating that actively since otherwise
people can't really be expected to know.
Rob Herring
Oct 19, 2015, 8:40:06 AM10/19/15
to
On Mon, Oct 19, 2015 at 4:44 AM, David Woodhouse <dw...@infradead.org> wrote:
was pointless because the call paths ended up looking like this:
Generic subsystem code -> DT look-up code -> fwnode_probe_device ->
of_probe_device
Rob
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/361137.html
> On Sun, 2015-10-18 at 20:53 +0100, Mark Brown wrote:
>> On Sun, Oct 18, 2015 at 12:37:57PM -0700, Greg Kroah-Hartman wrote:
>> > On Sun, Oct 18, 2015 at 08:29:31PM +0100, Mark Brown wrote:
>> > > On Fri, Oct 16, 2015 at 11:57:50PM -0700, Greg Kroah-Hartman wrote:
>>
>> > > > I can't see adding calls like this all over the tree just to solve a
>> > > > bus-specific problem, you are adding of_* calls where they aren't
>> > > > needed, or wanted, at all.
>>
>> > > This isn't bus specific, I'm not sure what makes you say that?
>>
>> > You are making it bus-specific by putting these calls all over the tree
>> > in different bus subsystems semi-randomly for all I can determine.
>>
>> Do you mean firmware rather than bus here? I think that's the confusion
>> I have...
>
>> On Sun, Oct 18, 2015 at 12:37:57PM -0700, Greg Kroah-Hartman wrote:
>> > On Sun, Oct 18, 2015 at 08:29:31PM +0100, Mark Brown wrote:
>> > > On Fri, Oct 16, 2015 at 11:57:50PM -0700, Greg Kroah-Hartman wrote:
>>
>> > > > I can't see adding calls like this all over the tree just to solve a
>> > > > bus-specific problem, you are adding of_* calls where they aren't
>> > > > needed, or wanted, at all.
>>
>> > > This isn't bus specific, I'm not sure what makes you say that?
>>
>> > You are making it bus-specific by putting these calls all over the tree
>> > in different bus subsystems semi-randomly for all I can determine.
>>
>> Do you mean firmware rather than bus here? I think that's the confusion
>> I have...
>
> Certainly, if it literally is adding of_* calls then that would seem to
> be gratuitously firmware-specific. Nothing should be using those these
> days; any new code should be using the generic device property APIs
> (except in special cases).
See version 2 of the series[1] which did that. It became obvious that
> be gratuitously firmware-specific. Nothing should be using those these
> days; any new code should be using the generic device property APIs
> (except in special cases).
was pointless because the call paths ended up looking like this:
Generic subsystem code -> DT look-up code -> fwnode_probe_device ->
of_probe_device
Rob
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/361137.html
Tomeu Vizoso
Oct 19, 2015, 8:40:06 AM10/19/15
to
On 18 October 2015 at 21:53, Mark Brown <bro...@kernel.org> wrote:
> On Sun, Oct 18, 2015 at 12:37:57PM -0700, Greg Kroah-Hartman wrote:
>> On Sun, Oct 18, 2015 at 08:29:31PM +0100, Mark Brown wrote:
>> > On Fri, Oct 16, 2015 at 11:57:50PM -0700, Greg Kroah-Hartman wrote:
>
>> > > I can't see adding calls like this all over the tree just to solve a
>> > > bus-specific problem, you are adding of_* calls where they aren't
>> > > needed, or wanted, at all.
>
>> > This isn't bus specific, I'm not sure what makes you say that?
>
>> You are making it bus-specific by putting these calls all over the tree
>> in different bus subsystems semi-randomly for all I can determine.
>
> Do you mean firmware rather than bus here? I think that's the confusion
> I have...
Hi all,
> On Sun, Oct 18, 2015 at 12:37:57PM -0700, Greg Kroah-Hartman wrote:
>> On Sun, Oct 18, 2015 at 08:29:31PM +0100, Mark Brown wrote:
>> > On Fri, Oct 16, 2015 at 11:57:50PM -0700, Greg Kroah-Hartman wrote:
>
>> > > I can't see adding calls like this all over the tree just to solve a
>> > > bus-specific problem, you are adding of_* calls where they aren't
>> > > needed, or wanted, at all.
>
>> > This isn't bus specific, I'm not sure what makes you say that?
>
>> You are making it bus-specific by putting these calls all over the tree
>> in different bus subsystems semi-randomly for all I can determine.
>
> Do you mean firmware rather than bus here? I think that's the confusion
> I have...
hope you don't mind I summarize the points taken instead of replying
to the individual emails. I tried to address all the concerns that
have been raised again in the cover letter, but I guess I did a bad
job at explaining myself, so here's another (more in-depth) go at it.
1) About the sprinkling of calls, everybody agreed it's a bad smell
from the start, but the intention is to modify the behaviour of the
already-DT-specific part of each subsystem without duplicating code.
A way to avoid the sprinkling would be to move the storage and lookup
of resources to the core (using classes and their list of devices to
replace the likes of __of_usb_find_phy). I also like Mark's idea of
calling of_device_probe from of_parse_phandle, which would be much
less invasive but I'm not sure if it would be right to call that
function in all the current cases in which of_parse_phandle is called.
2) About the goal of the series, what matters to my employer is that
once a device defers its probe it's only going to be reprobed in
late_initcall, after all the devices have been tentatively probed
once. In the practice this means that devices get probed in a
dependency order in which first go devices without dependencies then
go up the tree until the leave devices (which tend to be the ones with
effects visible to the user).
This series changes to another dependency order in which when a leaf
node gets probed, it recursively "pulls" its dependencies. This way we
stop massively delaying the probing of the display devices and vendors
can stop carrying sizeable hacks in their trees which just further
reduce the incentive to upstream.
The above is what funds this work, but in my personal opinion the
biggest advantage of this work is that it makes development on
embedded platforms more efficient because right now we don't have a
way of telling if a device deferred its probe because of an ordering
issue, or because there's a problem. If a device is available and has
a compatible driver, but it cannot be probed because a dependency
isn't going to be available, that's an error and is going to cause
real-world problems unless the device is redundant. Currently we say
nothing because with deferred probe the probe callbacks are also part
of the mechanism that determines the dependency order. I have wasted
countless hours hunting for the reason why a device didn't probe and I
have heard the same several times from others.
Having a specific switch for enabling deferred probe logging sounds
good, but there's going to be hundreds of spurious messages about
deferred probes that were just deferrals and only one of them is going
to be the actual error in which a device failed to find a dependency.
3) Regarding total boot time, I don't expect this series to make much
of a difference because though we would save a lot of matching and
querying for resources, that's little time compared with how long we
wait for hardware to react during probing. Async probing is more
likely to help with drivers that take a long time to probe.
4) About the breakage we have seen, that's not caused so far by
probing devices on-demand but by delaying probes until all built-in
drivers have been registered. The latter isn't strictly needed for
on-demand probing but without it most of the benefits are lost because
probes of dependencies are going to be deferred because the drivers
aren't there yet. We could avoid that by registering drivers also
on-demand but we would need to make the matching information available
beforehand, which is a massive change in itself. This should speed up
boot some, and also cause leaf devices to be up earlier.
One more thing about the breakage we have seen so far is that it's
generally caused by implicit dependencies and hunting those is
probably the second biggest timesink of the linux embedded developer
after failed probes. We depend on hacks such as link order, node order
in the DT, initcall gerrymandering and a blind hope in things that
started some time ago to have finished by now. And those implicit
dependencies are often left completely undocumented. This is really
fragile and breaks often when changing something unrelated such as
when adding another revision of a board or soc and a dependency starts
deferring its probe or is delayed because of something else. Also
breaks with async probing.
Delayed probes can be reverted by disabling a Kconfig, so we can fix
those issues in an ordered manner as time allows (we could disable it
by default now and add CI jobs with that enabled during a transitory
period).
Back when I made the series FW-independent with fwnode additions I
felt in my interaction with the ACPI folks that there's a bit of a
chasm in this issue between embedded and non-embedded people. This
could be because with ACPI most of the low-level hw elements such as
clocks, regulators, gpios and pins are hidden from the kernel and are
already ready when we start probing devices. With DT, the kernel has
to initialize all those and only then it can initialize the higher
level devices that depend on them. This means lots more of devices and
dependencies and thus we feel more acutely the shortcomings of the
current device framework at the scale we are using it today.
I think that having all dependencies be explicit and represented in
the device-driver model, along with a more advanced method of ordering
probes is something that would be good to have at this moment, even if
it won't benefit all users of the kernel.
Thanks,
Tomeu
Russell King - ARM Linux
Oct 19, 2015, 9:20:08 AM10/19/15
to
On Mon, Oct 19, 2015 at 02:34:22PM +0200, Tomeu Vizoso wrote:
> ... If a device is available and has
device Y built-in to the kernel, but the driver for device X is a
module?
I don't see this being solvable in the way you describe above - it's
going to identify X as being unable to be satisfied, and report it as
an error - but it's not an error at all.
> Having a specific switch for enabling deferred probe logging sounds
> good, but there's going to be hundreds of spurious messages about
> deferred probes that were just deferrals and only one of them is going
> to be the actual error in which a device failed to find a dependency.
Why would there be? Sounds like something's very wrong there.
You should only get deferred probes for devices which are declared to
be present, but their resources have not yet been satisfied. It
doesn't change anything if you have a kernel with lots of device drivers
or just the device drivers you need - the device drivers you don't need
do not contribute to the deferred probing in any way.
So, really, after boot and all appropriate modules have been loaded,
you should end up with no deferred probes. Are you saying that you
still have "hundreds" at that point? If you do, that sounds like
there's something very wrong.
> 3) Regarding total boot time, I don't expect this series to make much
> of a difference because though we would save a lot of matching and
> querying for resources, that's little time compared with how long we
> wait for hardware to react during probing. Async probing is more
> likely to help with drivers that take a long time to probe.
For me, on my fastest ARM board, though running serial console:
[ 2.293468] VFS: Mounted root (ext4 filesystem) on device 179:1.
There's a couple of delays in there, but they're not down to deferred
probing. The biggest one is serial console startup (due to the time
it takes to write out the initial kernel messages):
[ 0.289962] f1012000.serial: ttyS0 at MMIO 0xf1012000 (irq = 23, base_baud = 15625000) is a 16550A
[ 0.944124] console [ttyS0] enabled
and DSA switch initialisation:
[ 1.530655] libphy: dsa slave smi: probed
[ 2.034426] dsa dsa@0 lan6 (uninitialized): attached PHY at address 0 [Generic PHY]
I'm not sure what causes that, but at a guess it's having to talk to the
DSA switch over the MDIO bus via several layers of indirect accesses.
Of course, serial console adds to the boot time significantly anyway,
especially at the "standard" kernel logging level.
> One more thing about the breakage we have seen so far is that it's
> generally caused by implicit dependencies and hunting those is
> probably the second biggest timesink of the linux embedded developer
> after failed probes.
... which is generally caused by the crappy code which the average
embedded Linux developer creates, particularly with the crappy error
messages they like creating. For the most part, they _might_ as well
just print "Error!\n" and be done with it, for all the use they are.
When creating an error print, your average embedded Linux developer
fails to print the _reason_ why something failed, which makes debugging
it much harder.
The first thing I do when I touch code that needs this kind of debugging
is to go through and add printing of the error code. That normally lets
me quickly narrow down what's failed.
If embedded Linux developers are struggling with this, they only have
themselves to blame.
In the case of deferred probing, what _may_ help is if we got rid of the
core code printing that driver X requested deferred probing, instead
moving the responsibility to report this onto the driver or subsystem.
Resource claiming generally has the struct device, and can use dev_warn()
to report which device is being probed, along with which resource is
not yet available.
This debug problem is solvable without needing to resort to complex
probing solutions.
--
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
> ... If a device is available and has
> a compatible driver, but it cannot be probed because a dependency
> isn't going to be available, that's an error and is going to cause
> real-world problems unless the device is redundant. Currently we say
> nothing because with deferred probe the probe callbacks are also part
> of the mechanism that determines the dependency order.
So what if device X depends on device Y, and we have a driver for
> isn't going to be available, that's an error and is going to cause
> real-world problems unless the device is redundant. Currently we say
> nothing because with deferred probe the probe callbacks are also part
> of the mechanism that determines the dependency order.
device Y built-in to the kernel, but the driver for device X is a
module?
I don't see this being solvable in the way you describe above - it's
going to identify X as being unable to be satisfied, and report it as
an error - but it's not an error at all.
> Having a specific switch for enabling deferred probe logging sounds
> good, but there's going to be hundreds of spurious messages about
> deferred probes that were just deferrals and only one of them is going
> to be the actual error in which a device failed to find a dependency.
You should only get deferred probes for devices which are declared to
be present, but their resources have not yet been satisfied. It
doesn't change anything if you have a kernel with lots of device drivers
or just the device drivers you need - the device drivers you don't need
do not contribute to the deferred probing in any way.
So, really, after boot and all appropriate modules have been loaded,
you should end up with no deferred probes. Are you saying that you
still have "hundreds" at that point? If you do, that sounds like
there's something very wrong.
> 3) Regarding total boot time, I don't expect this series to make much
> of a difference because though we would save a lot of matching and
> querying for resources, that's little time compared with how long we
> wait for hardware to react during probing. Async probing is more
> likely to help with drivers that take a long time to probe.
[ 2.293468] VFS: Mounted root (ext4 filesystem) on device 179:1.
There's a couple of delays in there, but they're not down to deferred
probing. The biggest one is serial console startup (due to the time
it takes to write out the initial kernel messages):
[ 0.289962] f1012000.serial: ttyS0 at MMIO 0xf1012000 (irq = 23, base_baud = 15625000) is a 16550A
[ 0.944124] console [ttyS0] enabled
and DSA switch initialisation:
[ 1.530655] libphy: dsa slave smi: probed
[ 2.034426] dsa dsa@0 lan6 (uninitialized): attached PHY at address 0 [Generic PHY]
I'm not sure what causes that, but at a guess it's having to talk to the
DSA switch over the MDIO bus via several layers of indirect accesses.
Of course, serial console adds to the boot time significantly anyway,
especially at the "standard" kernel logging level.
> One more thing about the breakage we have seen so far is that it's
> generally caused by implicit dependencies and hunting those is
> probably the second biggest timesink of the linux embedded developer
> after failed probes.
embedded Linux developer creates, particularly with the crappy error
messages they like creating. For the most part, they _might_ as well
just print "Error!\n" and be done with it, for all the use they are.
When creating an error print, your average embedded Linux developer
fails to print the _reason_ why something failed, which makes debugging
it much harder.
The first thing I do when I touch code that needs this kind of debugging
is to go through and add printing of the error code. That normally lets
me quickly narrow down what's failed.
If embedded Linux developers are struggling with this, they only have
themselves to blame.
In the case of deferred probing, what _may_ help is if we got rid of the
core code printing that driver X requested deferred probing, instead
moving the responsibility to report this onto the driver or subsystem.
Resource claiming generally has the struct device, and can use dev_warn()
to report which device is being probed, along with which resource is
not yet available.
This debug problem is solvable without needing to resort to complex
probing solutions.
--
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
Tomeu Vizoso
Oct 19, 2015, 10:20:08 AM10/19/15
to
On 19 October 2015 at 15:18, Russell King - ARM Linux
is registered. No deferred probes nor messages about it.
But if you meant to write the opposite case (X built-in and Y in a
module), then I have to ask you in what situation that would make
sense.
>> Having a specific switch for enabling deferred probe logging sounds
>> good, but there's going to be hundreds of spurious messages about
>> deferred probes that were just deferrals and only one of them is going
>> to be the actual error in which a device failed to find a dependency.
>
> Why would there be? Sounds like something's very wrong there.
Sorry about that, I have checked that only now and I "only" get 39
deferred probe messages on exynos5250-snow.
> You should only get deferred probes for devices which are declared to
> be present, but their resources have not yet been satisfied. It
> doesn't change anything if you have a kernel with lots of device drivers
> or just the device drivers you need - the device drivers you don't need
> do not contribute to the deferred probing in any way.
I don't think that the number of registered drivers affects the number
of probes that get deferred (but I'm not sure why you mention that).
> So, really, after boot and all appropriate modules have been loaded,
> you should end up with no deferred probes. Are you saying that you
> still have "hundreds" at that point? If you do, that sounds like
> there's something very wrong.
I was talking about messages if we log each -EPROBE_DEFER, not devices
that remain to be probed. The point being that right now we don't have
a way to know if we are deferring because the dependency will be
around later, or if we have a problem and the dependency isn't going
to be there at all.
If we had a way to enable printing the cause of each -EPROBE_DEFER,
right now that would print 39 messages of this board that are only due
to ordering. The actual issue would be printed in exactly the same way
somewhere in the middle.
>> 3) Regarding total boot time, I don't expect this series to make much
>> of a difference because though we would save a lot of matching and
>> querying for resources, that's little time compared with how long we
>> wait for hardware to react during probing. Async probing is more
>> likely to help with drivers that take a long time to probe.
>
> For me, on my fastest ARM board, though running serial console:
>
> [ 2.293468] VFS: Mounted root (ext4 filesystem) on device 179:1.
>
> There's a couple of delays in there, but they're not down to deferred
> probing. The biggest one is serial console startup (due to the time
> it takes to write out the initial kernel messages):
>
> [ 0.289962] f1012000.serial: ttyS0 at MMIO 0xf1012000 (irq = 23, base_baud = 15625000) is a 16550A
> [ 0.944124] console [ttyS0] enabled
>
> and DSA switch initialisation:
>
> [ 1.530655] libphy: dsa slave smi: probed
> [ 2.034426] dsa dsa@0 lan6 (uninitialized): attached PHY at address 0 [Generic PHY]
>
> I'm not sure what causes that, but at a guess it's having to talk to the
> DSA switch over the MDIO bus via several layers of indirect accesses.
> Of course, serial console adds to the boot time significantly anyway,
> especially at the "standard" kernel logging level.
Yes, I don't think it makes any sense to measure boot times with the
serial console on, because it's not comparable to production and
because printing an additional line during boot affects significantly
the times.
To be clear, I was saying that this series should NOT affect total
boot times much.
>> One more thing about the breakage we have seen so far is that it's
>> generally caused by implicit dependencies and hunting those is
>> probably the second biggest timesink of the linux embedded developer
>> after failed probes.
>
> ... which is generally caused by the crappy code which the average
> embedded Linux developer creates, particularly with the crappy error
> messages they like creating. For the most part, they _might_ as well
> just print "Error!\n" and be done with it, for all the use they are.
> When creating an error print, your average embedded Linux developer
> fails to print the _reason_ why something failed, which makes debugging
> it much harder.
>
> The first thing I do when I touch code that needs this kind of debugging
> is to go through and add printing of the error code. That normally lets
> me quickly narrow down what's failed.
>
> If embedded Linux developers are struggling with this, they only have
> themselves to blame.
>
> In the case of deferred probing, what _may_ help is if we got rid of the
> core code printing that driver X requested deferred probing, instead
> moving the responsibility to report this onto the driver or subsystem.
> Resource claiming generally has the struct device, and can use dev_warn()
> to report which device is being probed, along with which resource is
> not yet available.
Agreed, with the note from above on why it would be better to only
print such a message only when the -EPROBE_DEFER is likely to be a
problem.
> This debug problem is solvable without needing to resort to complex
> probing solutions.
If you really think anything in this series is complex, you should
look at the other ones that tried to accomplish the same!
Thanks,
Tomeu
<li...@arm.linux.org.uk> wrote:
> On Mon, Oct 19, 2015 at 02:34:22PM +0200, Tomeu Vizoso wrote:
>> ... If a device is available and has
>> a compatible driver, but it cannot be probed because a dependency
>> isn't going to be available, that's an error and is going to cause
>> real-world problems unless the device is redundant. Currently we say
>> nothing because with deferred probe the probe callbacks are also part
>> of the mechanism that determines the dependency order.
>
> So what if device X depends on device Y, and we have a driver for
> device Y built-in to the kernel, but the driver for device X is a
> module?
>
> I don't see this being solvable in the way you describe above - it's
> going to identify X as being unable to be satisfied, and report it as
> an error - but it's not an error at all.
It's going to probe Y at late_initcall, then probe X when its driver
> On Mon, Oct 19, 2015 at 02:34:22PM +0200, Tomeu Vizoso wrote:
>> ... If a device is available and has
>> a compatible driver, but it cannot be probed because a dependency
>> isn't going to be available, that's an error and is going to cause
>> real-world problems unless the device is redundant. Currently we say
>> nothing because with deferred probe the probe callbacks are also part
>> of the mechanism that determines the dependency order.
>
> So what if device X depends on device Y, and we have a driver for
> device Y built-in to the kernel, but the driver for device X is a
> module?
>
> I don't see this being solvable in the way you describe above - it's
> going to identify X as being unable to be satisfied, and report it as
> an error - but it's not an error at all.
is registered. No deferred probes nor messages about it.
But if you meant to write the opposite case (X built-in and Y in a
module), then I have to ask you in what situation that would make
sense.
>> Having a specific switch for enabling deferred probe logging sounds
>> good, but there's going to be hundreds of spurious messages about
>> deferred probes that were just deferrals and only one of them is going
>> to be the actual error in which a device failed to find a dependency.
>
> Why would there be? Sounds like something's very wrong there.
deferred probe messages on exynos5250-snow.
> You should only get deferred probes for devices which are declared to
> be present, but their resources have not yet been satisfied. It
> doesn't change anything if you have a kernel with lots of device drivers
> or just the device drivers you need - the device drivers you don't need
> do not contribute to the deferred probing in any way.
of probes that get deferred (but I'm not sure why you mention that).
> So, really, after boot and all appropriate modules have been loaded,
> you should end up with no deferred probes. Are you saying that you
> still have "hundreds" at that point? If you do, that sounds like
> there's something very wrong.
that remain to be probed. The point being that right now we don't have
a way to know if we are deferring because the dependency will be
around later, or if we have a problem and the dependency isn't going
to be there at all.
If we had a way to enable printing the cause of each -EPROBE_DEFER,
right now that would print 39 messages of this board that are only due
to ordering. The actual issue would be printed in exactly the same way
somewhere in the middle.
>> 3) Regarding total boot time, I don't expect this series to make much
>> of a difference because though we would save a lot of matching and
>> querying for resources, that's little time compared with how long we
>> wait for hardware to react during probing. Async probing is more
>> likely to help with drivers that take a long time to probe.
>
> For me, on my fastest ARM board, though running serial console:
>
> [ 2.293468] VFS: Mounted root (ext4 filesystem) on device 179:1.
>
> There's a couple of delays in there, but they're not down to deferred
> probing. The biggest one is serial console startup (due to the time
> it takes to write out the initial kernel messages):
>
> [ 0.289962] f1012000.serial: ttyS0 at MMIO 0xf1012000 (irq = 23, base_baud = 15625000) is a 16550A
> [ 0.944124] console [ttyS0] enabled
>
> and DSA switch initialisation:
>
> [ 1.530655] libphy: dsa slave smi: probed
> [ 2.034426] dsa dsa@0 lan6 (uninitialized): attached PHY at address 0 [Generic PHY]
>
> I'm not sure what causes that, but at a guess it's having to talk to the
> DSA switch over the MDIO bus via several layers of indirect accesses.
> Of course, serial console adds to the boot time significantly anyway,
> especially at the "standard" kernel logging level.
serial console on, because it's not comparable to production and
because printing an additional line during boot affects significantly
the times.
To be clear, I was saying that this series should NOT affect total
boot times much.
>> One more thing about the breakage we have seen so far is that it's
>> generally caused by implicit dependencies and hunting those is
>> probably the second biggest timesink of the linux embedded developer
>> after failed probes.
>
> ... which is generally caused by the crappy code which the average
> embedded Linux developer creates, particularly with the crappy error
> messages they like creating. For the most part, they _might_ as well
> just print "Error!\n" and be done with it, for all the use they are.
> When creating an error print, your average embedded Linux developer
> fails to print the _reason_ why something failed, which makes debugging
> it much harder.
>
> The first thing I do when I touch code that needs this kind of debugging
> is to go through and add printing of the error code. That normally lets
> me quickly narrow down what's failed.
>
> If embedded Linux developers are struggling with this, they only have
> themselves to blame.
>
> In the case of deferred probing, what _may_ help is if we got rid of the
> core code printing that driver X requested deferred probing, instead
> moving the responsibility to report this onto the driver or subsystem.
> Resource claiming generally has the struct device, and can use dev_warn()
> to report which device is being probed, along with which resource is
> not yet available.
print such a message only when the -EPROBE_DEFER is likely to be a
problem.
> This debug problem is solvable without needing to resort to complex
> probing solutions.
look at the other ones that tried to accomplish the same!
Thanks,
Tomeu
Russell King - ARM Linux
Oct 19, 2015, 10:40:09 AM10/19/15
to
On Mon, Oct 19, 2015 at 04:10:56PM +0200, Tomeu Vizoso wrote:
> On 19 October 2015 at 15:18, Russell King - ARM Linux
> <li...@arm.linux.org.uk> wrote:
> > On Mon, Oct 19, 2015 at 02:34:22PM +0200, Tomeu Vizoso wrote:
> >> ... If a device is available and has
> >> a compatible driver, but it cannot be probed because a dependency
> >> isn't going to be available, that's an error and is going to cause
> >> real-world problems unless the device is redundant. Currently we say
> >> nothing because with deferred probe the probe callbacks are also part
> >> of the mechanism that determines the dependency order.
> >
> > So what if device X depends on device Y, and we have a driver for
> > device Y built-in to the kernel, but the driver for device X is a
> > module?
> >
> > I don't see this being solvable in the way you describe above - it's
> > going to identify X as being unable to be satisfied, and report it as
> > an error - but it's not an error at all.
>
> It's going to probe Y at late_initcall, then probe X when its driver
> is registered. No deferred probes nor messages about it.
>
> But if you meant to write the opposite case (X built-in and Y in a
> module), then I have to ask you in what situation that would make
> sense.
I did mean the opposite way around. It may not make sense if you're
> On 19 October 2015 at 15:18, Russell King - ARM Linux
> <li...@arm.linux.org.uk> wrote:
> > On Mon, Oct 19, 2015 at 02:34:22PM +0200, Tomeu Vizoso wrote:
> >> ... If a device is available and has
> >> a compatible driver, but it cannot be probed because a dependency
> >> isn't going to be available, that's an error and is going to cause
> >> real-world problems unless the device is redundant. Currently we say
> >> nothing because with deferred probe the probe callbacks are also part
> >> of the mechanism that determines the dependency order.
> >
> > So what if device X depends on device Y, and we have a driver for
> > device Y built-in to the kernel, but the driver for device X is a
> > module?
> >
> > I don't see this being solvable in the way you describe above - it's
> > going to identify X as being unable to be satisfied, and report it as
> > an error - but it's not an error at all.
>
> It's going to probe Y at late_initcall, then probe X when its driver
> is registered. No deferred probes nor messages about it.
>
> But if you meant to write the opposite case (X built-in and Y in a
> module), then I have to ask you in what situation that would make
> sense.
targetting a single platform, but it may make sense in a single zImage
kernel.
Consider something like a single zImage kernel that is built with
everything built-in to be able to boot and mount rootfs without
initramfs support on both platform A and platform B. Both platforms
share some hardware (eg, an I2C GPIO expander) which is built as a
module. It is a resource provider. Platform B contains a driver
which is required to boot on platform A, but not platform B (so the
kernel has to have that driver built-in.) On platform B, there is
a dependency to the I2C GPIO expander device.
> >> Having a specific switch for enabling deferred probe logging sounds
> >> good, but there's going to be hundreds of spurious messages about
> >> deferred probes that were just deferrals and only one of them is going
> >> to be the actual error in which a device failed to find a dependency.
> >
> > Why would there be? Sounds like something's very wrong there.
>
> Sorry about that, I have checked that only now and I "only" get 39
> deferred probe messages on exynos5250-snow.
here, but normally zero.
> > So, really, after boot and all appropriate modules have been loaded,
> > you should end up with no deferred probes. Are you saying that you
> > still have "hundreds" at that point? If you do, that sounds like
> > there's something very wrong.
>
> I was talking about messages if we log each -EPROBE_DEFER, not devices
> that remain to be probed. The point being that right now we don't have
> a way to know if we are deferring because the dependency will be
> around later, or if we have a problem and the dependency isn't going
> to be there at all.
the driver is not built into the kernel but is available as a module,
and a dependency that isn't around because the module hasn't been
loaded yet?
How do you distinguish between those two scenarios? In the former
scenario, the device will eventually come up when udev loads the
module. In the latter case, it's a persistent failing case.
> Agreed, with the note from above on why it would be better to only
> print such a message only when the -EPROBE_DEFER is likely to be a
> problem.
deferred probes will be a problem, and which won't. The only way to
definitely know that is if you disable kernel modules, and require
all drivers to be built into the kernel.
What you can do is print those devices which have failed to probe at
late_initcall() time - possibly augmenting that with reports from
subsystems showing what resources are not available, but that's only
a guide, because of the "it might or might not be in a kernel module"
problem.
Mark Brown
Oct 19, 2015, 11:00:09 AM10/19/15
to
On Mon, Oct 19, 2015 at 01:47:50PM +0100, David Woodhouse wrote:
> locations [the 'DT look-up code' you mention above] are DT specific
> functions anyway.
> But the point I'm making is that we are working towards *fixing* that,
> and *not* using DT-specific code in places where we should be using the
> generic APIs.
What is the plan for fixing things here? It's not obvious (at least to
me) that we don't want to have the subsystems having knowledge of how
they are bound to a specific firmware which is what you seem to imply
here. That seems like it's going to fall down since the different
firmware interfaces do have quite different ideas about how things fit
together at a system level and different compatibility needs which do
suggest that just trying to do a direct mapping from DT into ACPI may
well not make people happy but it sounds like that's the intention.
When it gets to drivers the situation is much more clear since it's
normally just simple properties, it's generally a bit more worrying if
drivers are needing to directly interact with cross-device linkage.
This is all subsystem level code though.
> None of that really negates that fact that we are *working* on cleaning
> these code paths up to be firmware-agnostic, and the fact that we
> haven't got to this one *yet* isn't necessarily a good reason to make
> it *worse* by adding new firmware-specificity to it.
It seems like we're going to have to refactor these bits of code when
they get generalised anyway so I'm not sure that the additional cost
here is that big.
> On Mon, 2015-10-19 at 07:35 -0500, Rob Herring wrote:
> > See version 2 of the series[1] which did that. It became obvious that
> > was pointless because the call paths ended up looking like this:
> > Generic subsystem code -> DT look-up code -> fwnode_probe_device ->
> > of_probe_device
> You link to a thread which says that "AT LEAST CURRENTLY, the calling
> > See version 2 of the series[1] which did that. It became obvious that
> > was pointless because the call paths ended up looking like this:
> > Generic subsystem code -> DT look-up code -> fwnode_probe_device ->
> > of_probe_device
> locations [the 'DT look-up code' you mention above] are DT specific
> functions anyway.
> But the point I'm making is that we are working towards *fixing* that,
> and *not* using DT-specific code in places where we should be using the
> generic APIs.
What is the plan for fixing things here? It's not obvious (at least to
me) that we don't want to have the subsystems having knowledge of how
they are bound to a specific firmware which is what you seem to imply
here. That seems like it's going to fall down since the different
firmware interfaces do have quite different ideas about how things fit
together at a system level and different compatibility needs which do
suggest that just trying to do a direct mapping from DT into ACPI may
well not make people happy but it sounds like that's the intention.
When it gets to drivers the situation is much more clear since it's
normally just simple properties, it's generally a bit more worrying if
drivers are needing to directly interact with cross-device linkage.
This is all subsystem level code though.
> None of that really negates that fact that we are *working* on cleaning
> these code paths up to be firmware-agnostic, and the fact that we
> haven't got to this one *yet* isn't necessarily a good reason to make
> it *worse* by adding new firmware-specificity to it.
It seems like we're going to have to refactor these bits of code when
they get generalised anyway so I'm not sure that the additional cost
here is that big.
Tomeu Vizoso
Oct 19, 2015, 11:10:09 AM10/19/15
to
On 19 October 2015 at 16:30, Russell King - ARM Linux
I see, in this situation the person trying to find out why some device
hadn't probed would enable debug logging of failed probes and would
see one spurious message if there was a deferred probe because of the
module.
>> >> Having a specific switch for enabling deferred probe logging sounds
>> >> good, but there's going to be hundreds of spurious messages about
>> >> deferred probes that were just deferrals and only one of them is going
>> >> to be the actual error in which a device failed to find a dependency.
>> >
>> > Why would there be? Sounds like something's very wrong there.
>>
>> Sorry about that, I have checked that only now and I "only" get 39
>> deferred probe messages on exynos5250-snow.
>
> I typically see one or two, maybe five maximum on the platforms I have
> here, but normally zero.
Hmm, I have given a look at our lava farm and have seen 2 dozens as
common (with multi_v7).
>> > So, really, after boot and all appropriate modules have been loaded,
>> > you should end up with no deferred probes. Are you saying that you
>> > still have "hundreds" at that point? If you do, that sounds like
>> > there's something very wrong.
>>
>> I was talking about messages if we log each -EPROBE_DEFER, not devices
>> that remain to be probed. The point being that right now we don't have
>> a way to know if we are deferring because the dependency will be
>> around later, or if we have a problem and the dependency isn't going
>> to be there at all.
>
> What's the difference between a dependency which isn't around because
> the driver is not built into the kernel but is available as a module,
> and a dependency that isn't around because the module hasn't been
> loaded yet?
>
> How do you distinguish between those two scenarios? In the former
> scenario, the device will eventually come up when udev loads the
> module. In the latter case, it's a persistent failing case.
Agreed, but it's something that doesn't happen often and that's why
such messages would be at the debug level instead of being warns or
errors.
>> Agreed, with the note from above on why it would be better to only
>> print such a message only when the -EPROBE_DEFER is likely to be a
>> problem.
>
> ... and my argument is that there's _no way_ to know for certain which
> deferred probes will be a problem, and which won't. The only way to
> definitely know that is if you disable kernel modules, and require
> all drivers to be built into the kernel.
>
> What you can do is print those devices which have failed to probe at
> late_initcall() time - possibly augmenting that with reports from
> subsystems showing what resources are not available, but that's only
> a guide, because of the "it might or might not be in a kernel module"
> problem.
Well, adding those reports would give you a changelog similar to the
one in this series...
Thanks,
Tomeu
hadn't probed would enable debug logging of failed probes and would
see one spurious message if there was a deferred probe because of the
module.
>> >> Having a specific switch for enabling deferred probe logging sounds
>> >> good, but there's going to be hundreds of spurious messages about
>> >> deferred probes that were just deferrals and only one of them is going
>> >> to be the actual error in which a device failed to find a dependency.
>> >
>> > Why would there be? Sounds like something's very wrong there.
>>
>> Sorry about that, I have checked that only now and I "only" get 39
>> deferred probe messages on exynos5250-snow.
>
> I typically see one or two, maybe five maximum on the platforms I have
> here, but normally zero.
common (with multi_v7).
>> > So, really, after boot and all appropriate modules have been loaded,
>> > you should end up with no deferred probes. Are you saying that you
>> > still have "hundreds" at that point? If you do, that sounds like
>> > there's something very wrong.
>>
>> I was talking about messages if we log each -EPROBE_DEFER, not devices
>> that remain to be probed. The point being that right now we don't have
>> a way to know if we are deferring because the dependency will be
>> around later, or if we have a problem and the dependency isn't going
>> to be there at all.
>
> What's the difference between a dependency which isn't around because
> the driver is not built into the kernel but is available as a module,
> and a dependency that isn't around because the module hasn't been
> loaded yet?
>
> How do you distinguish between those two scenarios? In the former
> scenario, the device will eventually come up when udev loads the
> module. In the latter case, it's a persistent failing case.
such messages would be at the debug level instead of being warns or
errors.
>> Agreed, with the note from above on why it would be better to only
>> print such a message only when the -EPROBE_DEFER is likely to be a
>> problem.
>
> ... and my argument is that there's _no way_ to know for certain which
> deferred probes will be a problem, and which won't. The only way to
> definitely know that is if you disable kernel modules, and require
> all drivers to be built into the kernel.
>
> What you can do is print those devices which have failed to probe at
> late_initcall() time - possibly augmenting that with reports from
> subsystems showing what resources are not available, but that's only
> a guide, because of the "it might or might not be in a kernel module"
> problem.
one in this series...
Thanks,
Tomeu
Russell King - ARM Linux
Oct 19, 2015, 11:40:09 AM10/19/15
to
On Mon, Oct 19, 2015 at 05:00:54PM +0200, Tomeu Vizoso wrote:
> On 19 October 2015 at 16:30, Russell King - ARM Linux
> <li...@arm.linux.org.uk> wrote:
> > I typically see one or two, maybe five maximum on the platforms I have
> > here, but normally zero.
>
> Hmm, I have given a look at our lava farm and have seen 2 dozens as
> common (with multi_v7).
No, because the lava farms tend not to be public.
> On 19 October 2015 at 16:30, Russell King - ARM Linux
> <li...@arm.linux.org.uk> wrote:
> > I typically see one or two, maybe five maximum on the platforms I have
> > here, but normally zero.
>
> Hmm, I have given a look at our lava farm and have seen 2 dozens as
> common (with multi_v7).
> > What you can do is print those devices which have failed to probe at
> > late_initcall() time - possibly augmenting that with reports from
> > subsystems showing what resources are not available, but that's only
> > a guide, because of the "it might or might not be in a kernel module"
> > problem.
>
> Well, adding those reports would give you a changelog similar to the
> one in this series...
a flag which would be set at late_initcall() time prior to running
a final round of deferred device probing.
This flag would then be used in a deferred_warn() printk function
which would normally be silent, but when this flag is set, it would
print the reason for the deferral - and this would replace (or be
added) to the subsystems and drivers which return -EPROBE_DEFER.
That has the effect of hiding all the deferrals up until just before
launching into userspace, which should then acomplish two things -
firstly, getting rid of the rather useless deferred messages up to
that point, and secondly printing the reason why the remaining
deferrals are happening.
That should be a small number of new lines plus a one-line change
in subsystems and drivers.
It is loading more messages.
0 new messages