Re: [PATCH 16/16] phy: phy-core: fix initcall level
Am 18.09.2015 um 08:16 schrieb Kishon Vijay Abraham I:
Hi,
On Wednesday 26 August 2015 05:58 PM, Alexander Holler wrote:
The phy-core has to be initialized before other dependent usb-drivers,
otherwise a crash might occur.
Currently phy_core_init() is called in the initcall-level device, which is
the same level where most usb-drivers will end up. By luck this seemed to
have been called most of the time before other usb-drivers without having
been explicitly enforced. But if phy_core_init() is not called before a
dependent driver, a null-pointer exception might occur (e.g. because the
phy device class isn't registered).
Did you actually face a problem? IIUC the modules get loaded based on
the drivers/Makefile order (unless the other modules are in a different
initcall table).
I had a problem while playing with a modified init-system (based on
dependencies). So not an actual problem.
IMHO the fix should be in the module that caused the crash. Change it to
use module_init?
The problem arises if the init-system ignores the link order and assumes
all drivers in the same initcall level can be called without any special
ordering.
The problem might also appear if a driver changes its name, directory or
position in file system. E.g. how to you make sure that a driver in
staging will be linked after the phy-core? Actually this happens, but I
would assume its by luck. I assume if staging would be renamed to
'beta-quality' a lot of stuff would actually fail because of the problem
with the implicit link order.
Anyway, nothing which really has to be fixed. It's just a notice that
maybe another initcall level of 'subsys' or something else before
'device' might be a better place for phy-core. I've chosen fs_sync
instead of subsys because otherwise I would have had to look up if
phy-core depends on another subsystem and therefore has to be
initialized after subsys.
Regards,
Alexander Holler
Thanks
Kishon
To fix this, phy_core_init() is moved to the initcall-level fs (right
before the standard initcall level device).
Signed-off-by: Alexander Holler
---
drivers/phy/phy-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index fc48fac..4945029 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -930,7 +930,7 @@ static int __init phy_core_init(void)
return 0;
}
-module_init(phy_core_init);
+fs_initcall_sync(phy_core_init);
static void __exit phy_core_exit(void)
{
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] deps: parallel initialization of (device-)drivers
Am 09.09.2015 um 20:35 schrieb Alexander Holler: Hello, as already mentioned, I've implemented the stuff to initialize drivers in parallel. What follows are two patches to be used on top of my already posted series (for 4.2) which implements annotated initcalls and DT based dependencies. But be warned: many drivers which are in the same initcall level still depend on the link order given by the Makefile and directoy (-name) and therefor will fail. That means without moving them to other initcall levels or explicit dependencies (which are a TODO) for these drivers, the whole stuff currently works only for some configurations and you likely will need to add several patches for your board. Another update: I've now did what I've described as TODO above. That means I have everything working to parallelize the (whole) init-system regardless the arch or DT/ACPI or whatever. Cleaning up the new stuff to post it here will need some time. And collecting the _mandatory_ dependencies to parallelize all static linked drivers (from all initcall levels) will need much more time. Even on systems where most stuff is build as a module, the list of drivers initialized through initcalls is usually several dozens or even hundreds. You might use 'grep initcall_ System.map | wc -l' to get an idea. Therefor I don't know when I will post cleaned up patches and/or some benchmark times. The interest seems rather low. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] deps: avoid multiple calls to memmove by just setting duplicates to 0
Besides make the code (almost unmeasurable) faster, this makes the
ugly looking loop I've used to remove duplicates cleaner.
Disadvantage is that the ordered array now contains 'holes' and the
number of elements in the array doesn't really match the number
of ordered elements. But this only makes a difference for debugging.
This patch also adds an of_node_put() for duplicate dt nodes, something
I previously had forgotten.
Signed-off-by: Alexander Holler
---
drivers/of/of_dependencies.c | 34 --
1 file changed, 20 insertions(+), 14 deletions(-)
diff --git a/drivers/of/of_dependencies.c b/drivers/of/of_dependencies.c
index 85cef84..ac0c0f5 100644
--- a/drivers/of/of_dependencies.c
+++ b/drivers/of/of_dependencies.c
@@ -323,21 +323,20 @@ static bool __init all_compatibles_same(struct
device_node *node1,
/*
* The order is based on devices but we are calling drivers.
* Therefor the order contains some drivers more than once.
- * Remove the duplicates.
+ * Disable the duplicates by setting them to 0.
*/
-static void __init of_init_remove_duplicates(void)
+static void __init of_init_disable_duplicates(void)
{
unsigned i, j;
for (i = 1; i < order.count; ++i)
for (j = 0; j < i; ++j) {
+ if (!order.order[j])
+ continue;
if (all_compatibles_same(order.order[j],
order.order[i])) {
- --order.count;
- memmove(&order.order[i], &order.order[i+1],
- (order.count - i) *
- sizeof(order.order[0]));
- --i;
+ of_node_put(order.order[i]);
+ order.order[i] = 0;
break;
}
}
@@ -416,7 +415,8 @@ static void __init build_tgroups(void)
unsigned dist = 0;
for (i = 0; i < order.count; ++i) {
- if (distance[order.order[i]->phandle] != dist) {
+ if (order.order[i] &&
+ distance[order.order[i]->phandle] != dist) {
dist = distance[order.order[i]->phandle];
count_groups++;
tgroup[count_groups].start = i;
@@ -436,6 +436,8 @@ static void __init of_init_print_order(void)
pr_info("Initialization order:\n");
for (i = 0; i < order.count; ++i) {
+ if (!order.order[i])
+ continue;
#ifdef CONFIG_OF_DEPENDENCIES_PARALLEL
pr_info("init %u 0x%x (group %u)", i,
order.order[i]->phandle,
@@ -496,10 +498,10 @@ static int __init of_init_build_order(void)
#ifdef CONFIG_OF_DEPENDENCIES_PARALLEL
build_order_by_distance();
- of_init_remove_duplicates();
+ of_init_disable_duplicates();
build_tgroups();
#else
- of_init_remove_duplicates();
+ of_init_disable_duplicates();
#endif
#ifdef CONFIG_OF_DEPENDENCIES_PRINT_INIT_ORDER
@@ -516,7 +518,8 @@ static void __init of_init_free_order(void)
unsigned i;
for (i = 0; i < order.count; ++i)
- of_node_put(order.order[i]);
+ if (order.order[i])
+ of_node_put(order.order[i]);
order.count = 0;
/* remove_new_phandles(); */
}
@@ -593,8 +596,10 @@ static int __init initcall_thread(void *thread_nr)
start = atomic_read(&ostart);
count = atomic_read(&ocount);
while ((i = atomic_dec_return(&shared_counter)) >= 0)
- init_if_matched(order.order[start + count - 1 - i],
- (unsigned)thread_nr);
+ if (order.order[start + count - 1 - i])
+ init_if_matched(
+ order.order[start + count - 1 - i],
+(unsigned)thread_nr);
prepare_to_wait(&group_waitqueue, &wait, TASK_UNINTERRUPTIBLE);
if (!atomic_dec_and_test(&count_initcall_threads)) {
schedule();
@@ -629,7 +634,8 @@ static void __init of_init_drivers_non_threaded(void)
if (!of_init_build_order()) {
for (i = 0; i < order.count; ++i)
- init_if_matched(order.order[i], 0);
+ if (order.order[i])
+ init_if_matched(order.order[i], 0);
of_init_free_order();
}
ac = __annotated_initcall_start;
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] deps: parallel initialization of (device-)drivers
This initializes drivers (annotated or in the initcall level device)
in parallel.
Which drivers can be initialized in parallel is calculated by using
the dependencies. That means, currently, only annotated drivers which
are are referenced in the used DT will be in order. For all others it
is assumed that, as long as they belong to the same initcall level
(device), they can be called in any order.
Unfortunately this isn't allways true and several drivers are depending
on the link-order (based on the Makefile and the directory). This is,
imho, a bug or at least a very fragile way to do such and should be,
again imho, fixed. Otherwise problems might arise if e.g. a driver is
moved from staging to its final position (which changes its place in
the list of initcalls too).
But this isn't really the topic of this patch and I'm mentioning this
here just as a warning or as hint in case someone experiences problems
when enabling the feature this patch provides.
Signed-off-by: Alexander Holler
---
drivers/of/Kconfig | 20
drivers/of/of_dependencies.c | 245 ++-
2 files changed, 261 insertions(+), 4 deletions(-)
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index 26c4b1a..7e6e910 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -132,4 +132,24 @@ config OF_DEPENDENCIES_DEBUG_CALLS_OF_ANNOTATED_INITCALLS
help
Used for debugging purposes.
+config OF_DEPENDENCIES_PARALLEL
+ bool "Initialize annotated initcalls in parallel"
+ depends on OF_DEPENDENCIES
+ help
+ Calculates which (annotated) initcalls can be called in parallel
+ and calls them using multiple threads. Be warned, this doesn't
+ work always as it should because of missing dependencies and
+ because it assumes that drivers belonging to the same initcall
+ level can be called in an order different than the order they
+ are linked.
+
+config OF_DEPENDENCIES_THREADS
+ int "Number of threads to use for parallel initialization"
+ depends on OF_DEPENDENCIES_PARALLEL
+ default 0
+ help
+ 0 means the number of threads used for parallel initialization
+ of drivers equals the number of online CPUs.
+ 1 means the threaded initialization is disabled.
+
endif # OF
diff --git a/drivers/of/of_dependencies.c b/drivers/of/of_dependencies.c
index 06435d5..85cef84 100644
--- a/drivers/of/of_dependencies.c
+++ b/drivers/of/of_dependencies.c
@@ -11,12 +11,16 @@
*/
#include
+#include
#define MAX_DT_NODES 1000 /* maximum number of vertices */
#define MAX_EDGES (MAX_DT_NODES*2) /* maximum number of edges (dependencies) */
struct edgenode {
uint32_t y; /* phandle */
+#ifdef CONFIG_OF_DEPENDENCIES_PARALLEL
+ uint32_t x;
+#endif
struct edgenode *next; /* next edge in list */
};
@@ -120,6 +124,9 @@ static int __init insert_edge(uint32_t x, uint32_t y)
graph.include_node[x] = 1;
graph.include_node[y] = 1;
p->y = y;
+#ifdef CONFIG_OF_DEPENDENCIES_PARALLEL
+ p->x = x;
+#endif
p->next = graph.edges[x];
graph.edges[x] = p; /* insert at head of list */
@@ -336,6 +343,90 @@ static void __init of_init_remove_duplicates(void)
}
}
+#ifdef CONFIG_OF_DEPENDENCIES_PARALLEL
+/*
+ * The algorithm I've used below to calculate the max. distance for
+ * nodes to the root node likely isn't the fasted. But based on the
+ * already done implementation of the topological sort, this is an
+ * easy way to achieve this. Instead of first doing an topological
+ * sort and then using the stuff below to calculate the distances,
+ * using an algorithm which does spit out distances directly would
+ * be likely faster.
+ * If you want to spend the time, you could have a look e.g. at the
+ * topic 'layered graph drawing'.
+ */
+/* max. distance from a node to root */
+static unsigned distance[MAX_DT_NODES+1] __initdata;
+static struct device_node *order_by_distance[MAX_DT_NODES+1] __initdata;
+
+static void __init calc_max_distance(uint32_t v)
+{
+ unsigned i;
+ unsigned max_dist = 0;
+
+ for (i = 0; i < graph.nedges; ++i)
+ if (graph.edge_slots[i].x == v)
+ max_dist = max(max_dist,
+ distance[graph.edge_slots[i].y] + 1);
+ distance[v] = max_dist;
+}
+
+static void __init calc_distances(void)
+{
+ unsigned i;
+
+ for (i = 0; i < order.count; ++i)
+ calc_max_distance(order.order[i]->phandle);
+}
+
+static void __init build_order_by_distance(void)
+{
+ unsigned i, j;
+ unsigned max_distance = 0;
+ unsigned new_order_count = 0;
+
+ calc_distances();
+ order_by_distance[new_order_count++] = order.order[0];
+ for (i = 1; i < order.count; ++i) {
+ if (distance[order.o
[PATCH 0/2] deps: parallel initialization of (device-)drivers
Hello, as already mentioned, I've implemented the stuff to initialize drivers in parallel. What follows are two patches to be used on top of my already posted series (for 4.2) which implements annotated initcalls and DT based dependencies. But be warned: many drivers which are in the same initcall level still depend on the link order given by the Makefile and directoy (-name) and therefor will fail. That means without moving them to other initcall levels or explicit dependencies (which are a TODO) for these drivers, the whole stuff currently works only for some configurations and you likely will need to add several patches for your board. I've already posted two patches to move two drivers into another initcall level. While playing with the stuff, I've found several more drivers which are suffering under the same problem and need the same modification: block/noop-iosched.c crypto/hmac.c cryoto/sha_generic.c drivers/mtd/ofpart.c drivers/tty/serial/8250/8250_core.c To offer an impression how this patch series might work in action, below is the relevant part from the kernel log for a configuration I'm using successfully on an imx6q. Maybe someone has interest in that stuff. Regards, Alexander Holler --- (...) [2.628325] Thread 0 calling (ordered) initcall for driver reg-fixed-voltage (regulator-fixed) [2.629196] Thread 3 calling (ordered) initcall for driver imx6q-pinctrl (fsl,imx6q-iomuxc) [2.629331] Thread 0 calling (ordered) initcall for driver gpio-mxc (fsl,imx1-gpio) [2.630276] imx6q-pinctrl 20e.iomuxc: initialized IMX pinctrl driver [2.632543] Thread 0 calling (ordered) initcall for driver anatop_regulator (fsl,anatop-regulator) [2.632556] Thread 1 calling (ordered) initcall for driver imx-sdma (fsl,imx6q-sdma) [2.632563] Thread 2 calling (ordered) initcall for driver mxs-dma (fsl,imx23-dma-apbh) [2.632598] Thread 3 calling (ordered) initcall for driver sram (mmio-sram) [2.634502] mxs-dma 11.dma-apbh: initialized [2.634848] Thread 3 calling (ordered) initcall for driver mxs_phy (fsl,imx6sx-usbphy) [2.635165] Thread 0 calling (ordered) initcall for driver imx2-wdt (fsl,imx21-wdt) [2.635181] Thread 2 calling (ordered) initcall for driver snvs_rtc (fsl,sec-v4.0-mon-rtc-lp) [2.635493] snvs_rtc 20cc034.snvs-rtc-lp: rtc core: setting system clock to 2015-09-09 16:37:09 UTC (1441816629) [2.635813] snvs_rtc 20cc034.snvs-rtc-lp: rtc core: registered 20cc034.snvs-rtc-lp as rtc0 [2.635978] Thread 2 calling (ordered) initcall for driver ahci-imx (fsl,imx53-ahci) [2.636317] imx-sdma 20ec000.sdma: initialized [2.636322] ahci-imx 220.sata: fsl,transmit-level-mV not specified, using 0024 [2.636332] ahci-imx 220.sata: fsl,transmit-boost-mdB not specified, using 0480 [2.636338] ahci-imx 220.sata: fsl,transmit-atten-16ths not specified, using 2000 [2.636347] ahci-imx 220.sata: fsl,receive-eq-mdB not specified, using 0500 [2.636690] Thread 1 calling (ordered) initcall for driver wandboard-rfkill (wand,imx6q-wandboard-rfkill) [2.637160] imx-sdma 20ec000.sdma: loaded firmware 1.1 [2.637166] imx2-wdt 20bc000.wdog: timeout 60 sec (nowayout=0) [2.637283] wandboard-rfkill rfkill: Wandboard rfkill initialization [2.637402] Thread 0 calling (ordered) initcall for driver leds-gpio (gpio-leds) [2.637422] wandboard-rfkill rfkill: Turning of power [2.639193] ahci-imx 220.sata: SSS flag set, parallel bus scan disabled [2.639253] ahci-imx 220.sata: AHCI 0001.0300 32 slots 1 ports 3 Gbps 0x1 impl platform mode [2.639299] ahci-imx 220.sata: flags: ncq sntf stag pm led clo only pmp pio slum part ccc apst [2.640579] scsi host0: ahci-imx [2.640902] ata1: SATA max UDMA/133 mmio [mem 0x0220-0x02203fff] port 0x100 irq 67 [2.663463] wandboard-rfkill rfkill: initialize wifi chip [2.663642] wandboard-rfkill rfkill: wifi-rfkill registered. [2.663720] wandboard-rfkill rfkill: initialize bluetooth chip [2.663919] wandboard-rfkill rfkill: bluetooth-rfkill registered. [2.664289] Thread 1 calling (ordered) initcall for driver imx-gpc (fsl,imx6q-gpc) [2.664335] Thread 3 calling (ordered) initcall for driver imx-uart (fsl,imx6q-uart) [2.664769] 202.serial: ttymxc0 at MMIO 0x202 (irq = 23, base_baud = 500) is a IMX [2.983471] ata1: SATA link up 1.5 Gbps (SStatus 113 SControl 300) [2.984610] ata1.00: ATA-8: Hitachi HTS542525K9SA00, BBFOC31P, max UDMA/133 [2.984620] ata1.00: 488397168 sectors, multi 0: LBA48 NCQ (depth 31/32) [2.985793] ata1.00: configured for UDMA/133 [2.985912] Default I/O scheduler not found. Using noop. [2.986213] scsi 0:0:0:0: Direct-Access ATA Hitachi HTS54252 C31P PQ: 0 ANSI: 5 [2.986776] scsi 0:0:0:0: Failed to register bsg queue, errno=-19 [3.734832] console [ttymxc0] enabled [3.739241] 21ec000.serial: ttymx
Re: [PATCH 15/16] mtd: mtdcore: fix initcall level
Am 04.09.2015 um 06:00 schrieb Alexander Holler: Am 02.09.2015 um 07:34 schrieb Alexander Holler: Am 01.09.2015 um 23:19 schrieb Brian Norris: Hi Alexander, No judgment here for the rest of this series, but for this patch: On Wed, Aug 26, 2015 at 02:28:27PM +0200, Alexander Holler wrote: The mtd-core has to be initialized before other dependent mtd-drivers, otherwise a crash might occur. Currently mtd_init() is called in the initcall-level device, which is the same level where most mtd-drivers will end up. By luck this seemed to have been called most of the time before other mtd-drivers without having been explicitly enforced. I can't really speak for the original authors, but it does not appear to be entirely "by luck." Link order was one of the de facto ways to get this ordering (though it's not really a great one), and mtdcore was always linked first within the drivers/mtd/ directory structure. But that's just background, I think this is worth fixing anyway. It could, for instance, become a problem if drivers are located outside drivers/mtd/; I see random board files in arch/ that register with MTD, and I'm actually not sure how they have never tripped on this. As I've just had a look at my patches in order to clean up the patch for parallel initialization (to post it here too): drivers/mtd/ofparts.c has the same problem. In order to let the NAND-driver see the partitions defined in the DT I had to move this into another initcall level (fs sync) too. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
deps: update in regard to parallel initialization of static linked drivers
Am 26.08.2015 um 14:28 schrieb Alexander Holler: Hello, (...) Some numbers (5 boots on each board, without and with ordering drivers), all times are seconds. (...) imx6q (armv7): unordered: 3.451998 3.418864 3.446952 3.429974 3.440996 (3.4377568) ordered: 3.538312 3.549019 3.538105 3.515916 3.555715 (3.5394134) (...) Further improvements could be to initialize drivers in parallel (using multiple cores) and/or to use this stuff on x86 (ACPI) too (e.g. by using a minimal DT which just includes dependencies). I've now made a quick implementation which uses multiple threads to initialize annotated drivers in parallel. It worked out of the box. Here are the times (dmesg | grep Freeing) I've now measured on the imx6q (4 cores): 3.388273 3.399892 3.411615 3.410523 3.388802 (3.399821) So it's now at least as fast as without ordering the drivers. Even on the one system where ordering didn't help without parallel initialization (likely because the unordered sequence of initcalls is already quiet good on that system, in comparison to the other systems I've tested). And my current implementation for the parallel stuff is far from being perfect and can be optimized much more (enough to not post a patch here). In addition I've added another driver to my config since I measure the old times, so the new times are including one more initialization of a driver (it now calls 30 annotated (static linked) drivers at boot, most stuff is still in modules (and some not annotated static linked drivers) on that system). Maybe this helps raising interest enough that someone else will test and maybe give some (reasonable, not about style) feedback on my patches. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 15/16] mtd: mtdcore: fix initcall level
Am 02.09.2015 um 07:34 schrieb Alexander Holler: Am 01.09.2015 um 23:19 schrieb Brian Norris: Hi Alexander, No judgment here for the rest of this series, but for this patch: On Wed, Aug 26, 2015 at 02:28:27PM +0200, Alexander Holler wrote: The mtd-core has to be initialized before other dependent mtd-drivers, otherwise a crash might occur. Currently mtd_init() is called in the initcall-level device, which is the same level where most mtd-drivers will end up. By luck this seemed to have been called most of the time before other mtd-drivers without having been explicitly enforced. I can't really speak for the original authors, but it does not appear to be entirely "by luck." Link order was one of the de facto ways to get this ordering (though it's not really a great one), and mtdcore was always linked first within the drivers/mtd/ directory structure. But that's just background, I think this is worth fixing anyway. It could, for instance, become a problem if drivers are located outside drivers/mtd/; I see random board files in arch/ that register with MTD, and I'm actually not sure how they have never tripped on this. I've already found at least a half a dozen other drivers with the same problem through my shuffling of the drivers which all end up in the standard initcall level device. I'm aware that this is based on the link order, which itself is based on linker behaviour (and maybe other things like make or other build tools). I'm just calling it luck, because this is nowhere explicitly stated, at least I've never seen such a statement, neither in any source, nor somewhere in Documentation. So I've choosen the term "by luck" in order to provoke a bit (or to stimulate a discussion about that widespread problem). A good example why "luck" might not be far away from the truth is what happens when a drivers moves e.g. from staging to it's real position. Also the position will stay inside the same initcall level, the move of the driver into another directory (maybe together with a rename) will likely change its position in the initcall-sequence, without anyone really expecting this. But if mtd_init() is not called before a dependent driver, a null-pointer exception might occur (e.g. because the mtd device class isn't registered). To fix this, mtd-init() is moved to the initcall-level fs (right before the standard initcall level device). Is there a good reason we shouldn't just make this a subsys_initcall()? That would match the naming better, and it mirrors what, e.g., block/genhd uses. It would also allow flexibility if there are other current/future use cases that might need to sit between the subsystem and the drivers. No real reason here. The names for the initcall levels seem to be outdated since a long time anyway, and I think just speaking about the numbers 1-7 (or 0-14) would be better anyways. The only reason why I've used the fs (sync) level is because I was too lazy to check out if mtdcore might depend on something else in any other level. Therefor I've used the one most close to were it was before. Lazy was the wrong term. It is time consuming, cumbersome and therefor error-prone to check on what other stuff a driver depends. One reason why choosing the right place in the initcall sequence isn't that easy and why some automation make sense. Also I've an idea about how to fix that in general for all drivers (by using the same algorithm I've now introduced just for DT-based drivers with a device description). Wouldn't be that hard to use that for all drivers, but as I've written in a follow up to the introductory mail, one step after another. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 15/16] mtd: mtdcore: fix initcall level
Am 01.09.2015 um 23:19 schrieb Brian Norris: Hi Alexander, No judgment here for the rest of this series, but for this patch: On Wed, Aug 26, 2015 at 02:28:27PM +0200, Alexander Holler wrote: The mtd-core has to be initialized before other dependent mtd-drivers, otherwise a crash might occur. Currently mtd_init() is called in the initcall-level device, which is the same level where most mtd-drivers will end up. By luck this seemed to have been called most of the time before other mtd-drivers without having been explicitly enforced. I can't really speak for the original authors, but it does not appear to be entirely "by luck." Link order was one of the de facto ways to get this ordering (though it's not really a great one), and mtdcore was always linked first within the drivers/mtd/ directory structure. But that's just background, I think this is worth fixing anyway. It could, for instance, become a problem if drivers are located outside drivers/mtd/; I see random board files in arch/ that register with MTD, and I'm actually not sure how they have never tripped on this. I've already found at least a half a dozen other drivers with the same problem through my shuffling of the drivers which all end up in the standard initcall level device. I'm aware that this is based on the link order, which itself is based on linker behaviour (and maybe other things like make or other build tools). I'm just calling it luck, because this is nowhere explicitly stated, at least I've never seen such a statement, neither in any source, nor somewhere in Documentation. So I've choosen the term "by luck" in order to provoke a bit (or to stimulate a discussion about that widespread problem). But if mtd_init() is not called before a dependent driver, a null-pointer exception might occur (e.g. because the mtd device class isn't registered). To fix this, mtd-init() is moved to the initcall-level fs (right before the standard initcall level device). Is there a good reason we shouldn't just make this a subsys_initcall()? That would match the naming better, and it mirrors what, e.g., block/genhd uses. It would also allow flexibility if there are other current/future use cases that might need to sit between the subsystem and the drivers. No real reason here. The names for the initcall levels seem to be outdated since a long time anyway, and I think just speaking about the numbers 1-7 (or 0-14) would be better anyways. The only reason why I've used the fs (sync) level is because I was too lazy to check out if mtdcore might depend on something else in any other level. Therefor I've used the one most close to were it was before. Also I've an idea about how to fix that in general for all drivers (by using the same algorithm I've now introduced just for DT-based drivers with a device description). Wouldn't be that hard to use that for all drivers, but as I've written in a follow up to the introductory mail, one step after another. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/16] deps: deterministic driver initialization order
Am 27.08.2015 um 21:01 schrieb Alexander Holler: Am 26.08.2015 um 14:28 schrieb Alexander Holler: The final 2 patches are fixes which should end up in mainline, regardless if this feature is merged or not. Just in case your serial or MTD-NAND partitions don't work, the drivers drivers/tty/serial/8250/8250_core.c and drivers/mtd/ofpart.c need the same trivial fix for their initcall level as found in the last two patches of my series. I'm not going to post these patches now until I've got some feedback for the other stuff. And just in case, I also have an idea how to fix such dependencies for drivers without DT (by adding hard-wired dependencies directly to drivers). That would also be usable for ACPI and x86. But one step after another. Based on my past experiences, I don't even think the stuff I've just posted will ever end up in the kernel. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/16] deps: deterministic driver initialization order
Am 26.08.2015 um 14:28 schrieb Alexander Holler: The final 2 patches are fixes which should end up in mainline, regardless if this feature is merged or not. Just in case your serial or MTD-NAND partitions don't work, the drivers drivers/tty/serial/8250/8250_core.c and drivers/mtd/ofpart.c need the same trivial fix for their initcall level as found in the last two patches of my series. I'm not going to post these patches now until I've got some feedback for the other stuff. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 05/16] deps: introduce initcalls annotated with a struct device_driver
Make it possible to identify initcalls before calling them by adding
a pointer to a struct device_driver to the stored pointer to an initcall.
This is e.g. necessary in order to sort initcalls by whatever means
before calling them.
To annotate an initcall, the following changes are necessary on
drivers which want to offer that feature:
now annotated
--
pure_initcall(fn) annotated_initcall(pure, fn, dev_drv)
core_initcall(fn) annotated_initcall(core, fn, dev_drv)
core_initcall_sync(fn) annotated_initcall_sync(core, fn, dev_drv)
...
late_initcall(fn) annotated_initcall(late, fn, dev_drv)
module_init(fn) annotated_module_init(fn, dev_drv)
module_platform_driver(drv) no changes necessary, done automatically
module_platform_driver_probe(drv, probe)no changes necessary
module_i2c_driver(i2c_drv) no changes necessary, done automatically
E.g. to make the driver sram offering an annotated initcall the
following patch is necessary:
-postcore_initcall(sram_init);
+annotated_initcall(postcore, sram_init, sram_driver.driver);
These changes can be done without any fear. If the feature is disabled,
which is the default, the new macros will just map to the old ones and
nothing is changed at all.
Signed-off-by: Alexander Holler
---
arch/arm/kernel/vmlinux.lds.S | 1 +
include/asm-generic/vmlinux.lds.h | 6 ++
include/linux/device.h| 12
include/linux/i2c.h | 2 +-
include/linux/init.h | 33 +
include/linux/module.h| 2 ++
include/linux/platform_device.h | 16
init/Kconfig | 3 +++
8 files changed, 70 insertions(+), 5 deletions(-)
diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index 8b60fde..10a328f 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -213,6 +213,7 @@ SECTIONS
#endif
INIT_SETUP(16)
INIT_CALLS
+ ANNOTATED_INITCALL
CON_INITCALL
SECURITY_INITCALL
INIT_RAM_FS
diff --git a/include/asm-generic/vmlinux.lds.h
b/include/asm-generic/vmlinux.lds.h
index 8bd374d..7318ba7 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -660,6 +660,11 @@
INIT_CALLS_LEVEL(7) \
VMLINUX_SYMBOL(__initcall_end) = .;
+#define ANNOTATED_INITCALL \
+ VMLINUX_SYMBOL(__annotated_initcall_start) = .; \
+ *(.annotated_initcall.init) \
+ VMLINUX_SYMBOL(__annotated_initcall_end) = .;
+
#define CON_INITCALL \
VMLINUX_SYMBOL(__con_initcall_start) = .; \
*(.con_initcall.init) \
@@ -816,6 +821,7 @@
INIT_DATA \
INIT_SETUP(initsetup_align) \
INIT_CALLS \
+ ANNOTATED_INITCALL \
CON_INITCALL\
SECURITY_INITCALL \
INIT_RAM_FS \
diff --git a/include/linux/device.h b/include/linux/device.h
index a2b4ea7..128fddd 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1321,4 +1321,16 @@ static int __init __driver##_init(void) \
} \
device_initcall(__driver##_init);
+#define annotated_module_driver(__driver, __register, __unregister, ...) \
+static int __init __driver##_init(void) \
+{ \
+ return __register(&(__driver), ##__VA_ARGS__); \
+} \
+annotated_module_init(__driver##_init, __driver.driver); \
+static void __exit __driver##_exit(void) \
+{ \
+ __unregister(&(__driver), ##__VA_ARGS__); \
+} \
+module_exit(__driver##_exit)
+
#endif /* _DEVICE_H_ */
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index e83a738..fa63ec1 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -626,7 +626,7 @@ static inline int i2c_adapter_id(struct i2c_adapter *adap)
* use this macro once, and calling it replaces module_init() and module_exit()
*/
#define module_i2c_driver(__i2c_driver) \
- module_driver(__i2c_driver, i2c_add_driver, \
+ annotated_module_driver(__i2c_driver, i2c_add_driver, \
i2c_del_driver)
#endif /* I2C */
diff --git a/include/linux/init.h b/include/linux/init.h
index b449f37..52ea986 100644
--- a/include/linux/init.h
+++ b
[PATCH 04/16] deps: dtc: introduce new (virtual) property no-dependencies
In some cases it makes sense to handle some phandles not as dependencies.
This is escpecially true for 'remote-endpoint' properties, because these
otherwise introducing dependency cycles into the graph. To avoid these,
one end of each remote-endpoint pairs has not to be handled as a
dependency.
The syntax is like
foo {
remote-endpoint = <&bar>;
};
bar {
remote-endpoint = <&foo>;
no-dependencies = <&foo>;
};
Without that 'no-dependencies' property dtc would automatically add a
dependency to foo to the property 'dependencies' of the node bar.
But with that 'no-dependencies' it will not automatically add the
listed dependencies.
The property 'no-dependencies' is virtual property and will not be added
to any output file.
Signed-off-by: Alexander Holler
---
scripts/dtc/dependencies.c | 33 +
1 file changed, 33 insertions(+)
diff --git a/scripts/dtc/dependencies.c b/scripts/dtc/dependencies.c
index 2c1e0d4..76e4d91 100644
--- a/scripts/dtc/dependencies.c
+++ b/scripts/dtc/dependencies.c
@@ -44,6 +44,23 @@ static bool is_parent_of(struct node *node1, struct node
*node2)
}
+static bool is_no_dependency(struct node *dt, struct property *prop, cell_t ph)
+{
+ struct node *node;
+ unsigned i;
+ cell_t *cell = (cell_t *)(prop->val.val);
+
+ for (i = 0; i < prop->val.len/4; ++i) {
+ node = get_node_by_phandle(dt, cpu_to_fdt32(*cell++));
+ if (node) {
+ node = find_compatible_not_disabled(node);
+ if (node && get_node_phandle(dt, node) == ph)
+ return true;
+ }
+ }
+ return false;
+}
+
static void add_deps(struct node *dt, struct node *node, struct property *prop)
{
struct marker *m = prop->val.markers;
@@ -73,6 +90,10 @@ static void add_deps(struct node *dt, struct node *node,
struct property *prop)
is_parent_of(target, source))
continue;
phandle = get_node_phandle(dt, target);
+ prop_deps = get_property(node, "no-dependencies");
+ if (prop_deps && is_no_dependency(dt, prop_deps, phandle))
+ /* avoid adding non-dependencies */
+ continue;
prop_deps = get_property(source, "dependencies");
if (!prop_deps) {
add_property(source,
@@ -102,9 +123,21 @@ static void process_nodes_props(struct node *dt, struct
node *node)
process_nodes_props(dt, child);
}
+static void del_prop_no_dependencies(struct node *node)
+{
+ struct node *child;
+
+ if (!node)
+ return;
+ delete_property_by_name(node, "no-dependencies");
+ for_each_child(node, child)
+ del_prop_no_dependencies(child);
+}
+
void add_dependencies(struct boot_info *bi)
{
process_nodes_props(bi->dt, bi->dt);
+ del_prop_no_dependencies(bi->dt);
}
/*
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 06/16] deps: deterministic driver initialization sequence based on dependencies from the DT
Use annotated initcalls along with dependencies found in the DT to sort
the initialization of drivers.
This makes stuff like in-driver hacks (e.g. omap), bruteforce
trial-and-error (deferred probe calls which are killing error messages)
and similar workarounds to circumvent the limited level based
driver initialization sequence unnecessary.
If enabled, this changes the driver initialization sequence as
described in the following table.
old (level based) new (ordered, in parts)
-
early early
pure (0)pure (0)
core (1, 1s)core (1, 1s)
postcore (2, 2s)postcore (2, 2s)
arch (3, 3s)arch (3, 3s)
subsys (4. 4s) subsys (4, 4s)
fs (5, 5s) fs (5, 5s)
rootfs rootfs
device (6. 6s) annotated initcalls (ordered by DT)
annotated initcalls (not found in DT)
device (6. 6s)
late (7, 7s)late (7, 7s)
To use this feature new binary DT blobs which are including dependencies
(type information for phandles) have to be used and most drivers found
in the DT should have been annotated.
Signed-off-by: Alexander Holler
---
drivers/of/Kconfig | 12 ++
drivers/of/Makefile | 1 +
drivers/of/of_dependencies.c| 410
include/linux/of_dependencies.h | 20 ++
init/main.c | 11 +-
5 files changed, 453 insertions(+), 1 deletion(-)
create mode 100644 drivers/of/of_dependencies.c
create mode 100644 include/linux/of_dependencies.h
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index 59bb855..9bf6c73 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -102,4 +102,16 @@ config OF_OVERLAY
While this option is selected automatically when needed, you can
enable it manually to improve device tree unit test coverage.
+config OF_DEPENDENCIES
+ bool "Device Tree dependency based initialization order (EXPERIMENTAL)"
+ select ANNOTATED_INITCALLS
+ help
+ Enables dependency based initialization order of drivers.
+
+ For correct operation the binary DT blob should have been
+ populated with properties of type "dependencies" and the
+ drivers referenced in the DT should have been annotated.
+
+ If unsure, say N here.
+
endif # OF
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index 156c072..05ced0d 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -14,5 +14,6 @@ obj-$(CONFIG_OF_MTD) += of_mtd.o
obj-$(CONFIG_OF_RESERVED_MEM) += of_reserved_mem.o
obj-$(CONFIG_OF_RESOLVE) += resolver.o
obj-$(CONFIG_OF_OVERLAY) += overlay.o
+obj-$(CONFIG_OF_DEPENDENCIES) += of_dependencies.o
obj-$(CONFIG_OF_UNITTEST) += unittest-data/
diff --git a/drivers/of/of_dependencies.c b/drivers/of/of_dependencies.c
new file mode 100644
index 000..64ed049
--- /dev/null
+++ b/drivers/of/of_dependencies.c
@@ -0,0 +1,410 @@
+/*
+ * Code for building a deterministic initialization order based on dependencies
+ * defined in the device tree.
+ *
+ * Copyright (C) 2014 Alexander Holler
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include
+
+#define MAX_DT_NODES 1000 /* maximum number of vertices */
+#define MAX_EDGES (MAX_DT_NODES*2) /* maximum number of edges (dependencies) */
+
+struct edgenode {
+ uint32_t y; /* phandle */
+ struct edgenode *next; /* next edge in list */
+};
+
+/*
+ * Vertex numbers do correspond to phandle numbers. That means the graph
+ * does contain as much vertices as the maximum of all phandles.
+ * Or in other words, we assume that for all phandles in the device tree
+ * 0 < phandle < MAX_DT_NODES+1 is true.
+ */
+struct dep_graph {
+ struct edgenode edge_slots[MAX_EDGES]; /* used to avoid kmalloc */
+ struct edgenode *edges[MAX_DT_NODES+1]; /* adjacency info */
+ unsigned nvertices; /* number of vertices in graph */
+ unsigned nedges; /* number of edges in graph */
+ bool processed[MAX_DT_NODES+1]; /* which vertices have been processed */
+ bool include_node[MAX_DT_NODES+1]; /* which nodes to consider */
+ bool discovered[MAX_DT_NODES+1]; /* which vertices have been found */
+ bool finished; /* if true, cut off search immediately */
+};
+static struct dep_graph graph __initdata;
+
+struct init_order {
+ uint32_t max_phandle; /* the max used phandle */
+ uint32_t old_max_phandle; /* used to keep track of added phandles */
+ struct device_node *order[MAX_DT_NODES+1];
+ unsigned count;
+ uint32_t ph_root; /* the phandle of the root */
+};
+static struct init_order order __i
[PATCH 09/16] deps: dts: imx6q: make some remote-endpoints non-dependencies
This is necessary to remove dependency cycles introduced by
'remote-endpoints'.
Signed-off-by: Alexander Holler
---
arch/arm/boot/dts/imx6q.dtsi | 2 ++
arch/arm/boot/dts/imx6qdl.dtsi | 4
2 files changed, 6 insertions(+)
diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index 399103b..8db7f25 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -184,6 +184,7 @@
ipu2_di0_hdmi: endpoint@1 {
remote-endpoint = <&hdmi_mux_2>;
+ no-dependencies = <&hdmi_mux_2>;
};
ipu2_di0_mipi: endpoint@2 {
@@ -205,6 +206,7 @@
ipu2_di1_hdmi: endpoint@1 {
remote-endpoint = <&hdmi_mux_3>;
+ no-dependencies = <&hdmi_mux_3>;
};
ipu2_di1_mipi: endpoint@2 {
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index b57033e..db3d0d0 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -1150,10 +1150,12 @@
ipu1_di0_hdmi: endpoint@1 {
remote-endpoint = <&hdmi_mux_0>;
+ no-dependencies = <&hdmi_mux_0>;
};
ipu1_di0_mipi: endpoint@2 {
remote-endpoint = <&mipi_mux_0>;
+ no-dependencies = <&mipi_mux_0>;
};
ipu1_di0_lvds0: endpoint@3 {
@@ -1175,10 +1177,12 @@
ipu1_di1_hdmi: endpoint@1 {
remote-endpoint = <&hdmi_mux_1>;
+ no-dependencies = <&hdmi_mux_1>;
};
ipu1_di1_mipi: endpoint@2 {
remote-endpoint = <&mipi_mux_1>;
+ no-dependencies = <&mipi_mux_1>;
};
ipu1_di1_lvds0: endpoint@3 {
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 08/16] deps: dts: kirkwood: dockstar: add dependency ehci -> usb power regulator
This serves as an example how easy it is to fix an initialization order
if the order depends on the DT. No source code changes will be necessary.
If you look at the dependency graph for the dockstar, you will see that
there is no dependency between ehci and the usb power regulator. This
ends up with the fact that the regulator will be initialized after ehci.
Fix this by adding one dependency to the .dts.
Signed-off-by: Alexander Holler
---
arch/arm/boot/dts/kirkwood-dockstar.dts | 4
1 file changed, 4 insertions(+)
diff --git a/arch/arm/boot/dts/kirkwood-dockstar.dts
b/arch/arm/boot/dts/kirkwood-dockstar.dts
index 8497363..426d8840 100644
--- a/arch/arm/boot/dts/kirkwood-dockstar.dts
+++ b/arch/arm/boot/dts/kirkwood-dockstar.dts
@@ -107,3 +107,7 @@
phy-handle = <ðphy0>;
};
};
+
+&usb0 {
+ dependencies = <&usb_power>;
+};
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 07/16] deps: add debug configuration options
Add some debug options to print annotated initcalls, the ordered list of
annotated initcalls and to print a message right before an annotated
initcall is called.
Signed-off-by: Alexander Holler
---
drivers/of/Kconfig | 18 ++
drivers/of/of_dependencies.c | 57 ++--
2 files changed, 73 insertions(+), 2 deletions(-)
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index 9bf6c73..26c4b1a 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -114,4 +114,22 @@ config OF_DEPENDENCIES
If unsure, say N here.
+config OF_DEPENDENCIES_PRINT_INIT_ORDER
+ bool "Print dependency based initialization order"
+ depends on OF_DEPENDENCIES
+ help
+ Used for debugging purposes.
+
+config OF_DEPENDENCIES_PRINT_ANNOTATED_INITCALLS
+ bool "Print names of annotated drivers"
+ depends on OF_DEPENDENCIES
+ help
+ Used for debugging purposes.
+
+config OF_DEPENDENCIES_DEBUG_CALLS_OF_ANNOTATED_INITCALLS
+ bool "Show when annotated initcalls are actually called"
+ depends on OF_DEPENDENCIES
+ help
+ Used for debugging purposes.
+
endif # OF
diff --git a/drivers/of/of_dependencies.c b/drivers/of/of_dependencies.c
index 64ed049..06435d5 100644
--- a/drivers/of/of_dependencies.c
+++ b/drivers/of/of_dependencies.c
@@ -336,6 +336,41 @@ static void __init of_init_remove_duplicates(void)
}
}
+#ifdef CONFIG_OF_DEPENDENCIES_PRINT_INIT_ORDER
+static void __init of_init_print_order(void)
+{
+ unsigned i;
+ struct property *prop;
+ const char *cp;
+
+ pr_info("Initialization order:\n");
+ for (i = 0; i < order.count; ++i) {
+ pr_info("init %u 0x%x", i, order.order[i]->phandle);
+ if (order.order[i]->name)
+ pr_cont(" %s", order.order[i]->name);
+ if (order.order[i]->full_name)
+ pr_cont(" (%s)", order.order[i]->full_name);
+ prop = __of_find_property(order.order[i], "compatible", NULL);
+ for (cp = of_prop_next_string(prop, NULL); cp;
+cp = of_prop_next_string(prop, cp))
+ pr_cont(" %s", cp);
+ }
+}
+#endif
+
+#ifdef CONFIG_OF_DEPENDENCIES_PRINT_ANNOTATED_INITCALLS
+static void __init of_init_print_annotated(void)
+{
+ struct _annotated_initcall *ac;
+
+ pr_info("Annotated drivers:\n");
+ ac = __annotated_initcall_start;
+ for (; ac < __annotated_initcall_end; ++ac)
+ pr_info("Driver '%s' (%s)\n", ac->driver->name,
+ ac->driver->of_match_table->compatible);
+}
+#endif
+
static int __init of_init_build_order(void)
{
int rc = 0;
@@ -363,7 +398,12 @@ static int __init of_init_build_order(void)
return -EINVAL; /* cycle found */
of_init_remove_duplicates();
-
+#ifdef CONFIG_OF_DEPENDENCIES_PRINT_INIT_ORDER
+ of_init_print_order();
+#endif
+#ifdef CONFIG_OF_DEPENDENCIES_PRINT_ANNOTATED_INITCALLS
+ of_init_print_annotated();
+#endif
return rc;
}
@@ -386,6 +426,12 @@ static void __init init_if_matched(struct device_node
*node)
if (ac->initcall && ac->driver->of_match_table)
if (of_match_node(ac->driver->of_match_table,
node)) {
+#ifdef CONFIG_OF_DEPENDENCIES_DEBUG_CALLS_OF_ANNOTATED_INITCALLS
+ pr_info("Calling (ordered) initcall for driver
%s (%s)\n",
+ ac->driver->name,
+ ac->driver->of_match_table ?
+
ac->driver->of_match_table->compatible : "");
+#endif
do_one_initcall(*ac->initcall);
ac->initcall = 0;
}
@@ -404,7 +450,14 @@ void __init of_init_drivers(void)
}
ac = __annotated_initcall_start;
for (; ac < __annotated_initcall_end; ++ac) {
- if (ac->initcall)
+ if (ac->initcall) {
+#ifdef CONFIG_OF_DEPENDENCIES_DEBUG_CALLS_OF_ANNOTATED_INITCALLS
+ pr_info("Calling (unordered) initcall for driver %s
(%s)\n",
+ ac->driver->name,
+ ac->driver->of_match_table ?
+ ac->driver->of_match_table->compatible
: "");
+#endif
do_one_initcall(*ac->initcall);
+ }
}
}
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 10/16] deps: dts: omap: beagle: make some remote-endpoints non-dependencies
This is necessary to remove dependency cycles introduced by
'remote-endpoints'.
Signed-off-by: Alexander Holler
---
arch/arm/boot/dts/omap3-beagle.dts | 4
1 file changed, 4 insertions(+)
diff --git a/arch/arm/boot/dts/omap3-beagle.dts
b/arch/arm/boot/dts/omap3-beagle.dts
index a547411..78ba39e 100644
--- a/arch/arm/boot/dts/omap3-beagle.dts
+++ b/arch/arm/boot/dts/omap3-beagle.dts
@@ -101,6 +101,7 @@
tfp410_in: endpoint@0 {
remote-endpoint = <&dpi_out>;
+ no-dependencies = <&dpi_out>;
};
};
@@ -109,6 +110,7 @@
tfp410_out: endpoint@0 {
remote-endpoint = <&dvi_connector_in>;
+ no-dependencies = <&dvi_connector_in>;
};
};
};
@@ -150,6 +152,7 @@
etb_in: endpoint {
slave-mode;
remote-endpoint = <&etm_out>;
+ no-dependencies = <&etm_out>;
};
};
};
@@ -373,6 +376,7 @@
port {
venc_out: endpoint {
remote-endpoint = <&tv_connector_in>;
+ no-dependencies = <&tv_connector_in>;
ti,channels = <2>;
};
};
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 11/16] deps: WIP: generic: annotate some initcalls
WIP means Work In Progress.
Change some generic drivers to offer annotated initcalls.
Signed-off-by: Alexander Holler
---
drivers/input/keyboard/gpio_keys.c | 2 +-
drivers/misc/sram.c| 2 +-
drivers/regulator/fixed.c | 3 ++-
drivers/usb/phy/phy-generic.c | 2 +-
4 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/input/keyboard/gpio_keys.c
b/drivers/input/keyboard/gpio_keys.c
index ddf4045..319bcf7 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -855,7 +855,7 @@ static void __exit gpio_keys_exit(void)
platform_driver_unregister(&gpio_keys_device_driver);
}
-late_initcall(gpio_keys_init);
+annotated_initcall(late, gpio_keys_init, gpio_keys_device_driver.driver);
module_exit(gpio_keys_exit);
MODULE_LICENSE("GPL");
diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c
index 15c33cc..eb662bd 100644
--- a/drivers/misc/sram.c
+++ b/drivers/misc/sram.c
@@ -243,4 +243,4 @@ static int __init sram_init(void)
return platform_driver_register(&sram_driver);
}
-postcore_initcall(sram_init);
+annotated_initcall(postcore, sram_init, sram_driver.driver);
diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index ff62d69..ec35d10 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -221,7 +221,8 @@ static int __init regulator_fixed_voltage_init(void)
{
return platform_driver_register(®ulator_fixed_voltage_driver);
}
-subsys_initcall(regulator_fixed_voltage_init);
+annotated_initcall(subsys, regulator_fixed_voltage_init,
+ regulator_fixed_voltage_driver.driver);
static void __exit regulator_fixed_voltage_exit(void)
{
diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
index deee68e..baf95d0 100644
--- a/drivers/usb/phy/phy-generic.c
+++ b/drivers/usb/phy/phy-generic.c
@@ -360,7 +360,7 @@ static int __init usb_phy_generic_init(void)
{
return platform_driver_register(&usb_phy_generic_driver);
}
-subsys_initcall(usb_phy_generic_init);
+annotated_initcall(subsys, usb_phy_generic_init,
usb_phy_generic_driver.driver);
static void __exit usb_phy_generic_exit(void)
{
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 16/16] phy: phy-core: fix initcall level
The phy-core has to be initialized before other dependent usb-drivers,
otherwise a crash might occur.
Currently phy_core_init() is called in the initcall-level device, which is
the same level where most usb-drivers will end up. By luck this seemed to
have been called most of the time before other usb-drivers without having
been explicitly enforced. But if phy_core_init() is not called before a
dependent driver, a null-pointer exception might occur (e.g. because the
phy device class isn't registered).
To fix this, phy_core_init() is moved to the initcall-level fs (right
before the standard initcall level device).
Signed-off-by: Alexander Holler
---
drivers/phy/phy-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index fc48fac..4945029 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -930,7 +930,7 @@ static int __init phy_core_init(void)
return 0;
}
-module_init(phy_core_init);
+fs_initcall_sync(phy_core_init);
static void __exit phy_core_exit(void)
{
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 13/16] deps: WIP: omap: annotate some initcalls
WIP means Work In Progress.
Change some omap drivers to offer annotated initcalls.
Signed-off-by: Alexander Holler
---
arch/arm/common/edma.c | 2 +-
drivers/bus/omap_l3_smx.c | 2 +-
drivers/dma/omap-dma.c | 2 +-
drivers/gpio/gpio-twl4030.c| 2 +-
drivers/gpu/drm/tilcdc/tilcdc_drv.c| 2 +-
drivers/i2c/busses/i2c-omap.c | 2 +-
drivers/iommu/omap-iommu.c | 2 +-
drivers/mailbox/omap-mailbox.c | 2 +-
drivers/memory/omap-gpmc.c | 2 +-
drivers/mfd/omap-usb-host.c| 2 +-
drivers/mfd/omap-usb-tll.c | 2 +-
drivers/mfd/tps65217.c | 2 +-
drivers/net/ethernet/ti/cpsw.c | 2 +-
drivers/net/ethernet/ti/davinci_mdio.c | 2 +-
drivers/phy/phy-twl4030-usb.c | 2 +-
drivers/regulator/twl-regulator.c | 2 +-
drivers/tty/serial/omap-serial.c | 2 +-
drivers/usb/host/ehci-omap.c | 2 +-
drivers/usb/host/ohci-omap3.c | 2 +-
drivers/usb/musb/omap2430.c| 2 +-
20 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
index 873dbfc..29f363c 100644
--- a/arch/arm/common/edma.c
+++ b/arch/arm/common/edma.c
@@ -1872,5 +1872,5 @@ static int __init edma_init(void)
{
return platform_driver_probe(&edma_driver, edma_probe);
}
-arch_initcall(edma_init);
+annotated_initcall(arch, edma_init, edma_driver.driver);
diff --git a/drivers/bus/omap_l3_smx.c b/drivers/bus/omap_l3_smx.c
index 360a5c0..e2172b8 100644
--- a/drivers/bus/omap_l3_smx.c
+++ b/drivers/bus/omap_l3_smx.c
@@ -302,7 +302,7 @@ static int __init omap3_l3_init(void)
{
return platform_driver_register(&omap3_l3_driver);
}
-postcore_initcall_sync(omap3_l3_init);
+annotated_initcall_sync(postcore, omap3_l3_init, omap3_l3_driver.driver);
static void __exit omap3_l3_exit(void)
{
diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
index 249445c..0866ae9 100644
--- a/drivers/dma/omap-dma.c
+++ b/drivers/dma/omap-dma.c
@@ -1285,7 +1285,7 @@ static int omap_dma_init(void)
{
return platform_driver_register(&omap_dma_driver);
}
-subsys_initcall(omap_dma_init);
+annotated_initcall(subsys, omap_dma_init, omap_dma_driver.driver);
static void __exit omap_dma_exit(void)
{
diff --git a/drivers/gpio/gpio-twl4030.c b/drivers/gpio/gpio-twl4030.c
index 9e1dbb9..60c0d1a 100644
--- a/drivers/gpio/gpio-twl4030.c
+++ b/drivers/gpio/gpio-twl4030.c
@@ -615,7 +615,7 @@ static int __init gpio_twl4030_init(void)
{
return platform_driver_register(&gpio_twl4030_driver);
}
-subsys_initcall(gpio_twl4030_init);
+annotated_initcall(subsys, gpio_twl4030_init, gpio_twl4030_driver.driver);
static void __exit gpio_twl4030_exit(void)
{
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 0f283a3..8c50f23 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -715,7 +715,7 @@ static void __exit tilcdc_drm_fini(void)
tilcdc_tfp410_fini();
}
-module_init(tilcdc_drm_init);
+annotated_module_init(tilcdc_drm_init, tilcdc_platform_driver.driver);
module_exit(tilcdc_drm_fini);
MODULE_AUTHOR("Rob Clark http://vger.kernel.org/majordomo-info.html
[PATCH 12/16] deps: WIP: imx: annotate some initcalls
WIP means Work In Progress.
Change some imx drivers to offer annotated initcalls.
Signed-off-by: Alexander Holler
---
arch/arm/mach-imx/gpc.c | 2 +-
arch/arm/mach-imx/mmdc.c | 2 +-
drivers/dma/mxs-dma.c | 2 +-
drivers/gpio/gpio-mxc.c | 2 +-
drivers/i2c/busses/i2c-imx.c | 2 +-
drivers/pinctrl/freescale/pinctrl-imx6q.c | 2 +-
drivers/power/reset/imx-snvs-poweroff.c | 2 +-
drivers/regulator/anatop-regulator.c | 3 ++-
drivers/tty/serial/imx.c | 2 +-
drivers/usb/phy/phy-mxs-usb.c | 2 +-
sound/soc/fsl/imx-audmux.c| 2 +-
11 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/arch/arm/mach-imx/gpc.c b/arch/arm/mach-imx/gpc.c
index 8c4467f..3802614 100644
--- a/arch/arm/mach-imx/gpc.c
+++ b/arch/arm/mach-imx/gpc.c
@@ -468,4 +468,4 @@ static int __init imx_pgc_init(void)
{
return platform_driver_register(&imx_gpc_driver);
}
-subsys_initcall(imx_pgc_init);
+annotated_initcall(subsys, imx_pgc_init, imx_gpc_driver.driver);
diff --git a/arch/arm/mach-imx/mmdc.c b/arch/arm/mach-imx/mmdc.c
index db9621c..cf84c03 100644
--- a/arch/arm/mach-imx/mmdc.c
+++ b/arch/arm/mach-imx/mmdc.c
@@ -87,4 +87,4 @@ static int __init imx_mmdc_init(void)
{
return platform_driver_register(&imx_mmdc_driver);
}
-postcore_initcall(imx_mmdc_init);
+annotated_initcall(postcore, imx_mmdc_init, imx_mmdc_driver.driver);
diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
index 60de352..5bfa232 100644
--- a/drivers/dma/mxs-dma.c
+++ b/drivers/dma/mxs-dma.c
@@ -884,4 +884,4 @@ static int __init mxs_dma_module_init(void)
{
return platform_driver_probe(&mxs_dma_driver, mxs_dma_probe);
}
-subsys_initcall(mxs_dma_module_init);
+annotated_initcall(subsys, mxs_dma_module_init, mxs_dma_driver.driver);
diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
index ec1eb1b..0a22a62 100644
--- a/drivers/gpio/gpio-mxc.c
+++ b/drivers/gpio/gpio-mxc.c
@@ -506,7 +506,7 @@ static int __init gpio_mxc_init(void)
{
return platform_driver_register(&mxc_gpio_driver);
}
-postcore_initcall(gpio_mxc_init);
+annotated_initcall(postcore, gpio_mxc_init, mxc_gpio_driver.driver);
MODULE_AUTHOR("Freescale Semiconductor, "
"Daniel Mack , "
diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 785aa67..c20d03c 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -1110,7 +1110,7 @@ static int __init i2c_adap_imx_init(void)
{
return platform_driver_register(&i2c_imx_driver);
}
-subsys_initcall(i2c_adap_imx_init);
+annotated_initcall(subsys, i2c_adap_imx_init, i2c_imx_driver.driver);
static void __exit i2c_adap_imx_exit(void)
{
diff --git a/drivers/pinctrl/freescale/pinctrl-imx6q.c
b/drivers/pinctrl/freescale/pinctrl-imx6q.c
index 4d1fcb8..77a1ffd 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx6q.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx6q.c
@@ -489,7 +489,7 @@ static int __init imx6q_pinctrl_init(void)
{
return platform_driver_register(&imx6q_pinctrl_driver);
}
-arch_initcall(imx6q_pinctrl_init);
+annotated_initcall(arch, imx6q_pinctrl_init, imx6q_pinctrl_driver.driver);
static void __exit imx6q_pinctrl_exit(void)
{
diff --git a/drivers/power/reset/imx-snvs-poweroff.c
b/drivers/power/reset/imx-snvs-poweroff.c
index ad6ce50..57a06dc 100644
--- a/drivers/power/reset/imx-snvs-poweroff.c
+++ b/drivers/power/reset/imx-snvs-poweroff.c
@@ -63,4 +63,4 @@ static int __init imx_poweroff_init(void)
{
return platform_driver_register(&imx_poweroff_driver);
}
-device_initcall(imx_poweroff_init);
+annotated_initcall(device, imx_poweroff_init, imx_poweroff_driver.driver);
diff --git a/drivers/regulator/anatop-regulator.c
b/drivers/regulator/anatop-regulator.c
index 738adfa..97808da 100644
--- a/drivers/regulator/anatop-regulator.c
+++ b/drivers/regulator/anatop-regulator.c
@@ -331,7 +331,8 @@ static int __init anatop_regulator_init(void)
{
return platform_driver_register(&anatop_regulator_driver);
}
-postcore_initcall(anatop_regulator_init);
+annotated_initcall(postcore, anatop_regulator_init,
+ anatop_regulator_driver.driver);
static void __exit anatop_regulator_exit(void)
{
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 54fdc78..c79e19b 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -2024,7 +2024,7 @@ static void __exit imx_serial_exit(void)
uart_unregister_driver(&imx_reg);
}
-module_init(imx_serial_init);
+annotated_module_init(imx_serial_init, serial_imx_driver.driver);
module_exit(imx_serial_exit);
MODULE_AUTHOR("Sascha Hauer");
diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
index 3fcc048..dd2768d 100644
--- a/drivers/usb/phy/phy-mxs-usb.c
+++ b/d
[PATCH 15/16] mtd: mtdcore: fix initcall level
The mtd-core has to be initialized before other dependent mtd-drivers,
otherwise a crash might occur.
Currently mtd_init() is called in the initcall-level device, which is the
same level where most mtd-drivers will end up. By luck this seemed to have
been called most of the time before other mtd-drivers without having been
explicitly enforced. But if mtd_init() is not called before a dependent
driver, a null-pointer exception might occur (e.g. because the mtd device
class isn't registered).
To fix this, mtd-init() is moved to the initcall-level fs (right before
the standard initcall level device).
Signed-off-by: Alexander Holler
---
drivers/mtd/mtdcore.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 875..fa8e6452 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -1303,7 +1303,7 @@ static void __exit cleanup_mtd(void)
bdi_destroy(&mtd_bdi);
}
-module_init(init_mtd);
+fs_initcall_sync(init_mtd);
module_exit(cleanup_mtd);
MODULE_LICENSE("GPL");
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 14/16] deps: WIP: kirkwood: annotate some initcalls
WIP means Work In Progress.
Change some kirkwood drivers to offer annotated initcalls.
Signed-off-by: Alexander Holler
---
drivers/dma/mv_xor.c | 2 +-
drivers/net/ethernet/marvell/mv643xx_eth.c | 3 ++-
drivers/usb/host/ehci-orion.c | 2 +-
3 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/dma/mv_xor.c b/drivers/dma/mv_xor.c
index f1325f6..e766ab2 100644
--- a/drivers/dma/mv_xor.c
+++ b/drivers/dma/mv_xor.c
@@ -1295,7 +1295,7 @@ static int __init mv_xor_init(void)
{
return platform_driver_register(&mv_xor_driver);
}
-module_init(mv_xor_init);
+annotated_module_init(mv_xor_init, mv_xor_driver.driver);
/* it's currently unsafe to unload this module */
#if 0
diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c
b/drivers/net/ethernet/marvell/mv643xx_eth.c
index d52639b..5eec4d8 100644
--- a/drivers/net/ethernet/marvell/mv643xx_eth.c
+++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
@@ -3239,7 +3239,8 @@ static int __init mv643xx_eth_init_module(void)
return rc;
}
-module_init(mv643xx_eth_init_module);
+annotated_module_init(mv643xx_eth_init_module,
+ mv643xx_eth_shared_driver.driver);
static void __exit mv643xx_eth_cleanup_module(void)
{
diff --git a/drivers/usb/host/ehci-orion.c b/drivers/usb/host/ehci-orion.c
index bfcbb9a..28f8793 100644
--- a/drivers/usb/host/ehci-orion.c
+++ b/drivers/usb/host/ehci-orion.c
@@ -333,7 +333,7 @@ static int __init ehci_orion_init(void)
ehci_init_driver(&ehci_orion_hc_driver, &orion_overrides);
return platform_driver_register(&ehci_orion_driver);
}
-module_init(ehci_orion_init);
+annotated_module_init(ehci_orion_init, ehci_orion_driver.driver);
static void __exit ehci_orion_cleanup(void)
{
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 03/16] deps: dtc: Add option to print dependency graph as dot (Graphviz)
Add option -T do print a dependency graph in dot format for
generating a picture with Graphviz.
E.g.
dtc -T foo.dts | dot -T svg -o foo.svg
would generate the picture foo.png with the dependency graph.
Convential dependencies (those based on the tree structure) are having
black arrows, dependencies based on the property 'dependencies' are
having cyan arrows.
Option -D to not automatically add dependencies does still work, so
you could build a classic dependency graph with
dtc -D -T foo.dts | dot -T png -o foo_no_auto_deps.png
This works with binary blobs as input too. E.g.
CROSS_COMPILE=gcc-foo ARCH=arm make foo.dtb
scripts/dtc/dtc -I dtb -T arch/arm/boot/dts/foo.dtb
would print the dot file.
Signed-off-by: Alexander Holler
---
scripts/dtc/dependencies.c | 49 --
scripts/dtc/dtc.c | 19 +++---
scripts/dtc/dtc.h | 2 +-
3 files changed, 60 insertions(+), 10 deletions(-)
diff --git a/scripts/dtc/dependencies.c b/scripts/dtc/dependencies.c
index 3fb5cef..2c1e0d4 100644
--- a/scripts/dtc/dependencies.c
+++ b/scripts/dtc/dependencies.c
@@ -298,7 +298,7 @@ static void __init topological_sort(void)
depth_first_search(i);
}
-static int __init add_dep_list(struct device_node *node)
+static int __init add_dep_list(struct device_node *node, bool print_dot)
{
const __be32 *list, *list_end;
uint32_t ph;
@@ -328,6 +328,9 @@ static int __init add_dep_list(struct device_node *node)
rc = insert_edge(node->phandle, ph);
if (rc)
break;
+ if (print_dot)
+ printf(" node0x%x -> node0x%x [color=cyan]\n",
+ node->phandle, ph);
}
return rc;
@@ -352,9 +355,10 @@ static const char *of_prop_next_string(struct property
*prop, const char *cur)
}
static int __init add_deps_lnx(struct device_node *parent,
- struct device_node *node)
+ struct device_node *node, bool print_dot)
{
struct device_node *child;
+ const char *cp;
int rc = 0;
if (!__of_device_is_available(node))
@@ -375,13 +379,34 @@ static int __init add_deps_lnx(struct device_node *parent,
return -EINVAL;
}
order.parent_by_phandle[node->phandle] = parent->phandle;
- rc = add_dep_list(node);
+ if (print_dot) {
+ struct property *prop;
+
+ printf("node0x%x [label=\"0x%x %s", node->phandle,
+ node->phandle, node->name);
+ if (node->full_name)
+ printf(" (%s)", node->full_name);
+ prop = get_property(node, "compatible");
+ if (prop) {
+ printf("\\n");
+ for (cp = of_prop_next_string(prop, NULL); cp;
+cp = of_prop_next_string(prop, cp)) {
+ if (cp != prop->val.val)
+ putchar(' ');
+ printf("%s", cp);
+ }
+ }
+ printf("\"];\n");
+ printf(" node0x%x -> node0x%x\n", node->phandle,
+ parent->phandle);
+ }
+ rc = add_dep_list(node, print_dot);
if (unlikely(rc))
return rc;
parent = node; /* change the parent only if node is a driver */
}
for_each_child_of_node(node, child) {
- rc = add_deps_lnx(parent, child);
+ rc = add_deps_lnx(parent, child, print_dot);
if (unlikely(rc))
break;
}
@@ -424,7 +449,7 @@ void __init of_init_print_order(const char *name)
}
}
-int __init of_init_build_order(struct device_node *root)
+int __init of_init_build_order(struct device_node *root, const char *print_dot)
{
struct device_node *child;
int rc = 0;
@@ -436,12 +461,24 @@ int __init of_init_build_order(struct device_node *root)
calc_max_phandle(root);
order.old_max_phandle = order.max_phandle;
+ if (print_dot) {
+ printf("digraph G {\n");
+ printf("node0x%x [label=\"0x%x root (/)\"];\n",
+ order.max_phandle+1, order.max_phandle+1);
+ }
+
for_each_child_of_node(root, child) {
- rc = ad
[PATCH 02/16] deps: dtc: Add option to print initialization order
Add option -t to print the default initialization order.
No other output will be generated.
To print the order, just use something like this:
CROSS_COMPILE=gcc-foo ARCH=arm make foo.dtb
scripts/dtc/dtc -I dtb -t arch/arm/boot/dts/foo.dtb
Since it's now possible to check to for cycles in the dependency graph,
this is now done too.
Signed-off-by: Alexander Holler
---
scripts/dtc/dependencies.c | 344 +
scripts/dtc/dtc.c | 24 +++-
scripts/dtc/dtc.h | 2 +
3 files changed, 369 insertions(+), 1 deletion(-)
diff --git a/scripts/dtc/dependencies.c b/scripts/dtc/dependencies.c
index dd4658c..3fb5cef 100644
--- a/scripts/dtc/dependencies.c
+++ b/scripts/dtc/dependencies.c
@@ -106,3 +106,347 @@ void add_dependencies(struct boot_info *bi)
{
process_nodes_props(bi->dt, bi->dt);
}
+
+/*
+ * The code below is in large parts a copy of drivers/of/of_dependencies.c
+ * in the Linux kernel. So both files do share the same bugs.
+ * The next few ugly defines do exist to keep the differences at a minimum.
+ */
+static struct node *tree;
+#define pr_cont(format, ...) printf(format, ##__VA_ARGS__)
+#define pr_info(format, ...) printf(format, ##__VA_ARGS__)
+#define pr_warn(format, ...) printf(format, ##__VA_ARGS__)
+#define pr_err(format, ...) fprintf(stderr, format, ##__VA_ARGS__)
+typedef cell_t __be32;
+#define device_node node
+#define full_name fullpath
+#define __initdata
+#define __init
+#define unlikely(a) (a)
+#define of_node_put(a)
+#define of_find_node_by_phandle(v) get_node_by_phandle(tree, v)
+#define __of_get_property(a, b, c) get_property(a, b)
+#define for_each_child_of_node(a, b) for_each_child(a, b)
+
+
+#define MAX_DT_NODES 1000 /* maximum number of vertices */
+#define MAX_EDGES (MAX_DT_NODES*2) /* maximum number of edges (dependencies) */
+
+struct edgenode {
+ uint32_t y; /* phandle */
+ struct edgenode *next; /* next edge in list */
+};
+
+/*
+ * Vertex numbers do correspond to phandle numbers. That means the graph
+ * does contain as much vertices as the maximum of all phandles.
+ * Or in other words, we assume that for all phandles in the device tree
+ * 0 < phandle < MAX_DT_NODES+1 is true.
+ */
+struct dep_graph {
+ struct edgenode edge_slots[MAX_EDGES]; /* used to avoid kmalloc */
+ struct edgenode *edges[MAX_DT_NODES+1]; /* adjacency info */
+ unsigned nvertices; /* number of vertices in graph */
+ unsigned nedges; /* number of edges in graph */
+ bool processed[MAX_DT_NODES+1]; /* which vertices have been processed */
+ bool include_node[MAX_DT_NODES+1]; /* which nodes to consider */
+ bool discovered[MAX_DT_NODES+1]; /* which vertices have been found */
+ bool finished; /* if true, cut off search immediately */
+};
+static struct dep_graph graph __initdata;
+
+struct init_order {
+ uint32_t max_phandle; /* the max used phandle */
+ uint32_t old_max_phandle; /* used to keep track of added phandles */
+ struct device_node *order[MAX_DT_NODES+1];
+ unsigned count;
+ /* Used to keep track of parent devices in regard to the DT */
+ uint32_t parent_by_phandle[MAX_DT_NODES+1];
+ struct device *device_by_phandle[MAX_DT_NODES+1];
+};
+static struct init_order order __initdata;
+
+
+/* Copied from drivers/of/base.c (because it's lockless). */
+static int __init __of_device_is_available(struct device_node *device)
+{
+ struct property *status;
+
+ if (!device)
+ return 0;
+
+ status = get_property(device, "status");
+ if (status == NULL)
+ return 1;
+
+ if (status->val.len > 0) {
+ if (!strcmp(status->val.val, "okay") ||
+ !strcmp(status->val.val, "ok"))
+ return 1;
+ }
+
+ return 0;
+}
+
+/*
+ * x is a dependant of y or in other words
+ * y will be initialized before x.
+ */
+static int __init insert_edge(uint32_t x, uint32_t y)
+{
+ struct edgenode *p; /* temporary pointer */
+
+ if (unlikely(x > MAX_DT_NODES || y > MAX_DT_NODES)) {
+ pr_err("Node found with phandle 0x%x > MAX_DT_NODES (%d)!\n",
+ x > MAX_DT_NODES ? x : y, MAX_DT_NODES);
+ return -EINVAL;
+ }
+ if (unlikely(!x || !y))
+ return 0;
+ if (unlikely(graph.nedges >= MAX_EDGES)) {
+ pr_err("Maximum number of edges (%d) reached!\n", MAX_EDGES);
+ return -EINVAL;
+ }
+ p = &graph.edge_slots[graph.nedges++];
+ graph.include_node[x] = 1;
+ graph.include_node[y] = 1;
+ p->y = y;
+ p->next = graph.edges[x];
+ graph.edges[x] = p; /* insert at head of list */
+
+ graph.nvertices = (x > graph.nvertices) ? x : graph.nvertices;
+ graph.nve
[PATCH 01/16] deps: dtc: Automatically add new property 'dependencies' which contains a list of referenced phandles
During the step from .dts to .dtb the information about dependcies
contained in the .dts through phandle references is lost. This makes it
impossible to use the binary blob to create a dependency graph without
knowing the semantic of all cell arrays.
Therefor automatically add a new property called 'dependencies' to all
nodes which have phandle references in one of their properties.
This new property will contain an array of phandles with one value for
every phandle referenced by other properties in the node.
If such a property already exists (e.g. to manually add dependencies
through the .dts), the existing list will be expanded.
Added phandles will be the phandle of either the referenced node itself
(if it has a property named 'compatible', or of the next parent of the
referenced node which as property named 'compatible'. This ensures only
dependencies to drivers will be added.
References to phandles of parent or child nodes will not be added to this
property, because this information is already contained in the blob (in
the form of the tree itself).
No dependencies to disabled nodes will be added.
Signed-off-by: Alexander Holler
---
scripts/dtc/Makefile | 3 +-
scripts/dtc/Makefile.dtc | 1 +
scripts/dtc/dependencies.c | 108 +
scripts/dtc/dtc.c | 12 -
scripts/dtc/dtc.h | 3 ++
5 files changed, 125 insertions(+), 2 deletions(-)
create mode 100644 scripts/dtc/dependencies.c
diff --git a/scripts/dtc/Makefile b/scripts/dtc/Makefile
index 2a48022..1174cf9 100644
--- a/scripts/dtc/Makefile
+++ b/scripts/dtc/Makefile
@@ -4,7 +4,7 @@ hostprogs-y := dtc
always := $(hostprogs-y)
dtc-objs := dtc.o flattree.o fstree.o data.o livetree.o treesource.o \
- srcpos.o checks.o util.o
+ srcpos.o checks.o util.o dependencies.o
dtc-objs += dtc-lexer.lex.o dtc-parser.tab.o
# Source files need to get at the userspace version of libfdt_env.h to compile
@@ -13,6 +13,7 @@ HOSTCFLAGS_DTC := -I$(src) -I$(src)/libfdt
HOSTCFLAGS_checks.o := $(HOSTCFLAGS_DTC)
HOSTCFLAGS_data.o := $(HOSTCFLAGS_DTC)
+HOSTCFLAGS_dependencies.o := $(HOSTCFLAGS_DTC)
HOSTCFLAGS_dtc.o := $(HOSTCFLAGS_DTC)
HOSTCFLAGS_flattree.o := $(HOSTCFLAGS_DTC)
HOSTCFLAGS_fstree.o := $(HOSTCFLAGS_DTC)
diff --git a/scripts/dtc/Makefile.dtc b/scripts/dtc/Makefile.dtc
index bece49b..5fb5343 100644
--- a/scripts/dtc/Makefile.dtc
+++ b/scripts/dtc/Makefile.dtc
@@ -6,6 +6,7 @@
DTC_SRCS = \
checks.c \
data.c \
+ dependencies.c \
dtc.c \
flattree.c \
fstree.c \
diff --git a/scripts/dtc/dependencies.c b/scripts/dtc/dependencies.c
new file mode 100644
index 000..dd4658c
--- /dev/null
+++ b/scripts/dtc/dependencies.c
@@ -0,0 +1,108 @@
+/*
+ * Code to add a property which contains dependencies (used phandle references)
+ * to all (driver) nodes which are having phandle references.
+ *
+ * Copyright (C) 2014 Alexander Holler
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include
+
+/* Searches upwards for a node with a property 'compatible' */
+static struct node *find_compatible_not_disabled(struct node *node)
+{
+ struct property *prop;
+
+ while (node) {
+ prop = get_property(node, "compatible");
+ if (prop) {
+ prop = get_property(node, "status");
+ if (prop)
+ if (!prop->val.len ||
+ (strcmp(prop->val.val, "okay") &&
+ strcmp(prop->val.val, "ok")))
+ return NULL; /* disabled */
+ return node;
+ }
+ node = node->parent;
+ }
+ return NULL;
+}
+
+static bool is_parent_of(struct node *node1, struct node *node2)
+{
+ while (node2) {
+ if (node2->parent == node1)
+ return true;
+ node2 = node2->parent;
+ }
+ return false;
+
+}
+
+static void add_deps(struct node *dt, struct node *node, struct property *prop)
+{
+ struct marker *m = prop->val.markers;
+ struct node *refnode;
+ cell_t phandle;
+ struct property *prop_deps;
+ unsigned i;
+ cell_t *cell;
+ struct node *source;
+ struct node *target;
+
+ for_each_marker_of_type(m, REF_PHANDLE) {
+ assert(m->offset + sizeof(cell_t) <= prop->val.len);
+
+ refnode = get_node_by_ref(dt, m->ref);
+ if (!refnode) {
+
[PATCH 00/16] deps: deterministic driver initialization order
Hello, I've already written a lot on that topic, therefor this introductory mail doesn't explain everything. Please have a look at the threads "dt: dependencies (for deterministic driver initialization order based on the DT)" and "On-demand device registration" on LKML for previous discussions. In short, the current init system is quiet old and doesn't really match todays HW which needs much more drivers than 20 years ago. As I've mentioned in those threads, initcalls should be identifiable in order to sort them. My previous attempt intercepted the driver registrations in order to do that, which unfortunately ended up with having to deal how drivers are creating devices. I've now spend some more time and this partly new approach now annotates the initcalls directly, by storing a pointer to a struct device_driver in addition to the pointer to the initcall. This makes my previous patches smaller, cleaner and better to understand, the result which you can see in this little patch series. The whole patch series is made up of several parts. The first 4 patches are modifying dtc to include dependencies (type information for phandles). The next 3 patches are the ones which are implementing those annotated initcalls and the driver sort algorithm. Then there are 3 patches with small changes on some dts. The following 4 patches with the WIP (Work In Progress) are changing some initcalls to annotated initcalls. I would split them up in a bunch of patches (one for each driver), if I get the promise that the other patches will be merged into mainline (but not without that promise). The final 2 patches are fixes which should end up in mainline, regardless if this feature is merged or not. Some numbers (5 boots on each board, without and with ordering drivers), all times are seconds. Kirkwood (dockstar, armv5): Boot to "Freeing unused kernel memory" (includes mounting the rootfs), unordered: 4.456016 3.937801 4.114788 4.114526 3.949480 (average 4.1145222) ordered: 3.173054 3.164045 3.141418 3.480679 3.459298 (3.2836988) Time needed to sort (of_init_build_order()): 0.003024 Time needed to match drivers to the order (without calling them): 0.002884 Beagleboard (rev C4, armv7): unordered: 6.706024 6.821746 6.696014 6.673675 6.769866 (6.733465) ordered: 5.544860 5.514160 5.505859 5.527374 5.496795 (5.5178096) sorting: 0.021209 matching: 0.006165 Beaglebone Black (rev A5, armv7): unordered: 3.826531 3.825662 3.826648 3.825434 3.825263 (3.8259076) ordered: 2.838554 2.838322 2.839459 2.838467 2.838421 (2.8386446) sorting: 0.004769 matching: 0.004860 imx6q (armv7): unordered: 3.451998 3.418864 3.446952 3.429974 3.440996 (3.4377568) ordered: 3.538312 3.549019 3.538105 3.515916 3.555715 (3.5394134) sorting: 0.004622 matching: 0.003868 Because I'm using different configuration options on these boards, the absolute times aren't comparable. But in conclusion, most boards are booting faster when the initcalls have been ordered. In addition, ordering the initcalls makes the initialization sequence more deterministic (it can even be seen without booting a machine). But I still think the most benefit of these little series is by not having to use in-driver hacks or that ugly brute force trial-and-error deferred probing in order to fix a wrong default call order of initcalls. Further improvements could be to initialize drivers in parallel (using multiple cores) and/or to use this stuff on x86 (ACPI) too (e.g. by using a minimal DT which just includes dependencies). To test this series with any other DT enabled board, look at which drivers are referenced in the DT (see patch 2 or 3), annotate these drivers (trivial, see patch 5 or the examples with WIP in the subject), turn on CONFIG_OF_DEPENDENCIES, compile and boot. Just to mention it, unless CONFIG_OF_DEPENDENCIES is turned on (which is marked as experimental), nothing will change, even if you've annotated some drivers (the new macros will just map to the old ones). Regards, Alexander Holler PS: Please keep in mind that these patches are an offer. I'm already 100+ patches above mainline and don't really care to maintain some more for myself. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/21] On-demand device registration
Am 15.06.2015 um 10:58 schrieb Linus Walleij: On Sat, Jun 13, 2015 at 8:27 PM, Alexander Holler wrote: And because you've said that "problem space is a bit convoluted" and I disagree, here's a summary from my point of view: 1. All the necessary information (dependencies between drivers) already exists at compile time. The set of dependencies between drivers might become smaller by configuration, but will not become larger. So there should be NO need to collect them at runtime, e.g. by instrumenting function calls. I think you arrived at the core of the crux here. I've hoped so, that's why I've written it. I guess your suggested approach then need to introduce a special build tool to order the initcalls accordingly. Again this will fall short if you don't know at compile time exactly *which* board file will be executed. I've just tried to describe the facts in order to make the problem space more clear, because, as said, I don't think it's convoluted. Besides that, I didn't want to suggest anything else other than what I've already posted working patches for. What I've mentioned as possible other solutions above is stuff which might be possible too in order to give some starting points for people which are searching another solution. But I wouldn't have written my patches as they are, if I would think there is another more easier solution. And of course, there is still a bit to resolve at runtime, even in the DT case (look at the "compatible" attribute). But there is already a runtime solution to find the right driver (in case of DT) and I haven't mentioned it in order to no confuse people again. Mentioning every little detail doesn't make sense if you want to describe something understandable (which is what I've tried). So the only practical way to solve this at compile time is to predict an initcall ordering sequence for all possible boot paths, compile in all of them, and choose the right one at boot. But the number of boot paths is equal to the number of device trees / ACPI tables or board files supported, and that space is uncontrolled and ordered infinite. You just need one working ordered sequence which includes all options. This one will work for all others too. Basically I think the root problem with your approach is that you assume we know what hardware we will boot on at compile time. We Totally wrong. If you assume that I assume this, than either I was totally unable to describe something clearly, or you were unable or unwilling to understand what I've written. And as the result is the same, we don't need to find out which was reason. Anyway, have fun. I'm quitting the discussion here as I don't have any business with the kernel and already decided some time again to not post patches anymore as it seems to be a waste of my (and maybe others) time. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/21] On-demand device registration
Am 12.06.2015 um 13:36 schrieb Alexander Holler: Am 12.06.2015 um 13:19 schrieb Alexander Holler: Am 12.06.2015 um 09:25 schrieb Linus Walleij: On Thu, Jun 11, 2015 at 6:40 PM, Alexander Holler wrote: Am 11.06.2015 um 14:30 schrieb Linus Walleij: Certainly it is possible to create deadlocks in this scenario, but the scope is not to create an ubreakable system. IAnd what happens if you run into a deadlock? Do you print "you've lost, try changing your kernel config" in some output hidden by a splash-screen? ;) Sorry it sounds like a blanket argument, the fact that there are mutexes in the kernel makes it possible to deadlock, it doesn't mean we don't use mutexes. Some programming problems are just like such. I'm not talking about specific deadlocks through mutexes. I'm talking about what happens when driver A needs driver B which needs driver A. How do you recognise and handle that with your instrumented on-demand device initialization? Such a circular dependency might happen by just adding a new fucntion call or by changing the kernel configuration. And with the on-demand stuff, the possibility that the developer introducing this new (maybe optional) call will never hit such a circular dependency is high. So you will end up with a never ending stream of problem reports whenever someone introduced such a circular dependecy without having noticed it. And to come back to specific deadlocks, if you are extending function calls from something former simple to something which might initialize a whole bunch of drivers, needing maybe seconds, I wouldn't say this is a blanket argument, but a real thread. Keep in mind, that the possibility that a function call ends up with initializing a whole bunch of other drivers, is not determined statically, but depends on the configuration and runtime behaviour of the actual system the on-demand stuff actually happens. E.g. if driver A is faster one system that driver B, the whole bunch of drivers might become initialized by a call in driver A. But if driver B was faster on the developers system (or the system is configured to first init driver B), than the whole bunch of drivers might have become initialized by driver B on the developers system. Thus he never might have hit a possible problem when the whole bunch of drivers got initialized in driver A. That means it isn't always a good idea to create dynamic systems (like on-demand device initialization), because it's very hard to foresee and correctly handle their runtime behaviour. And because you've said that "problem space is a bit convoluted" and I disagree, here's a summary from my point of view: 1. All the necessary information (dependencies between drivers) already exists at compile time. The set of dependencies between drivers might become smaller by configuration, but will not become larger. So there should be NO need to collect them at runtime, e.g. by instrumenting function calls. I've described the problems I see with that above. I've choosen DT as source of dependencies because it offers an easy accessible and almost complete set of dependencies. I just had to add some type information to the dtb in order to identify the dependencies (phandles). But other ways to collect the dependencies would work too. Even the most simple way to add a static list of dependencies to each driver (which later on might be automated by some more clever stuff than adding them manually) would do the trick. 2. The problem to sort a set of nodes (drivers) with dependencies is solved since a long time and almost any developers uses it regularly in form of make. And everyone who used make -jN knows that the possible parallel initialization of drivers I've talked about, is already solved too. 3. In order to initialize the drivers in some specific order, their initcalls must be identified. I've offered a possible solution to that without much changes, but many other, even better ways, are possible too. It just depends on how much you want to change and on how much of these changes you will be able to feed into mainline kernel (which depends on your connections/relations inside the core kernel crew). E.g. instead of still just relying on one-dimensional arrays with (anonymous) pointers to initcalls, a multidimensional array of initcalls and drivername (and maybe more information) might be thinkable. 4. x86/amd64/ACPI-people, so most longtime and core kernel maintainers obviously don't have much interest until you've solved 1. in a way they can use too. So the necessary changes for 2. or 3. will have a big hurdle to take if 1. isn't solved usable for them too. Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/21] On-demand device registration
Am 12.06.2015 um 13:19 schrieb Alexander Holler: Am 12.06.2015 um 09:25 schrieb Linus Walleij: On Thu, Jun 11, 2015 at 6:40 PM, Alexander Holler wrote: Am 11.06.2015 um 14:30 schrieb Linus Walleij: Certainly it is possible to create deadlocks in this scenario, but the scope is not to create an ubreakable system. IAnd what happens if you run into a deadlock? Do you print "you've lost, try changing your kernel config" in some output hidden by a splash-screen? ;) Sorry it sounds like a blanket argument, the fact that there are mutexes in the kernel makes it possible to deadlock, it doesn't mean we don't use mutexes. Some programming problems are just like such. I'm not talking about specific deadlocks through mutexes. I'm talking about what happens when driver A needs driver B which needs driver A. How do you recognise and handle that with your instrumented on-demand device initialization? Such a circular dependency might happen by just adding a new fucntion call or by changing the kernel configuration. And with the on-demand stuff, the possibility that the developer introducing this new (maybe optional) call will never hit such a circular dependency is high. So you will end up with a never ending stream of problem reports whenever someone introduced such a circular dependecy without having noticed it. And to come back to specific deadlocks, if you are extending function calls from something former simple to something which might initialize a whole bunch of drivers, needing maybe seconds, I wouldn't say this is a blanket argument, but a real thread. Keep in mind, that the possibility that a function call ends up with initializing a whole bunch of other drivers, is not determined statically, but depends on the configuration and runtime behaviour of the actual system the on-demand stuff actually happens. E.g. if driver A is faster one system that driver B, the whole bunch of drivers might become initialized by a call in driver A. But if driver B was faster on the developers system (or the system is configured to first init driver B), than the whole bunch of drivers might have become initialized by driver B on the developers system. Thus he never might have hit a possible problem when the whole bunch of drivers got initialized in driver A. That means it isn't always a good idea to create dynamic systems (like on-demand device initialization), because it's very hard to foresee and correctly handle their runtime behaviour. Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/21] On-demand device registration
Am 12.06.2015 um 09:25 schrieb Linus Walleij: On Thu, Jun 11, 2015 at 6:40 PM, Alexander Holler wrote: Am 11.06.2015 um 14:30 schrieb Linus Walleij: Certainly it is possible to create deadlocks in this scenario, but the scope is not to create an ubreakable system. IAnd what happens if you run into a deadlock? Do you print "you've lost, try changing your kernel config" in some output hidden by a splash-screen? ;) Sorry it sounds like a blanket argument, the fact that there are mutexes in the kernel makes it possible to deadlock, it doesn't mean we don't use mutexes. Some programming problems are just like such. I'm not talking about specific deadlocks through mutexes. I'm talking about what happens when driver A needs driver B which needs driver A. How do you recognise and handle that with your instrumented on-demand device initialization? Such a circular dependency might happen by just adding a new fucntion call or by changing the kernel configuration. And with the on-demand stuff, the possibility that the developer introducing this new (maybe optional) call will never hit such a circular dependency is high. So you will end up with a never ending stream of problem reports whenever someone introduced such a circular dependecy without having noticed it. And to come back to specific deadlocks, if you are extending function calls from something former simple to something which might initialize a whole bunch of drivers, needing maybe seconds, I wouldn't say this is a blanket argument, but a real thread. Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/21] On-demand device registration
Am 11.06.2015 um 14:30 schrieb Linus Walleij: On Thu, Jun 11, 2015 at 12:17 PM, Alexander Holler wrote: Am 11.06.2015 um 10:12 schrieb Linus Walleij: On Wed, Jun 10, 2015 at 10:28 AM, Alexander Holler wrote: You would end up with the same problem of deadlocks as currently, and you would still need something ugly like the defered probe brutforce to avoid them. Sorry I don't get that. Care to elaborate on why? Because loading/initializing on demand doesn't give you any solved order of drivers to initialize. And it can't because it has no idea about the requirements of other drivers. The reason why it might work better in the case of the tegra is that it might give you another initialization order than the one which is currently choosen, which, by luck, might be a better one. But maybe I missed something, I haven't looked at the patches at all. But just loading on demand, can't magically give you a working order of drivers to initialize. E.g. how do you choose the first driver to initialize? So the current patch set introduces dependencies (just for device tree) and Tomeu is working on a more generic dependency approach for any HW description. The first driver to initialize will be as usual the first one in the list for that initlevel, then walking up the initilevels. However if any driver runs into a resource roadblock it will postpone and wait for dependencies to probe first. Certainly it is possible to create deadlocks in this scenario, but the scope is not to create an ubreakable system. IAnd what happens if you run into a deadlock? Do you print "you've lost, try changing your kernel config" in some output hidden by a splash-screen? ;) That sounds like the fun with duck typed languages where you have to test any and every possible screnario (something which is almost impossible) in order to not run into something unexpected. Anyway, have fun, good luck. Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/21] On-demand device registration
Am 11.06.2015 um 13:24 schrieb Alexander Holler: > Am 11.06.2015 um 12:17 schrieb Alexander Holler: >> Am 11.06.2015 um 10:12 schrieb Linus Walleij: >>> On Wed, Jun 10, 2015 at 10:28 AM, Alexander Holler >>> wrote: >>>> Am 10.06.2015 um 09:30 schrieb Linus Walleij: >>> >>>>> i2c host comes out, probes the regulator driver, regulator driver >>>>> probes and then the regulator_get() call returns. >>>>> >>>>> This requires instrumentation on anything providing a resource >>>>> to another driver like those I mentioned and a lot of overhead >>>>> infrastructure, but I think it's the right approach. However I don't >>>>> know if I would ever be able to pull that off myself, I know talk >>>>> is cheap and I should show the code instead. >>>> >>>> You would end up with the same problem of deadlocks as currently, and >>>> you >>>> would still need something ugly like the defered probe brutforce to >>>> avoid >>>> them. >>> >>> Sorry I don't get that. Care to elaborate on why? >> >> Because loading/initializing on demand doesn't give you any solved order >> of drivers to initialize. And it can't because it has no idea about the >> requirements of other drivers. The reason why it might work better in >> the case of the tegra is that it might give you another initialization >> order than the one which is currently choosen, which, by luck, might be >> a better one. >> >> But maybe I missed something, I haven't looked at the patches at all. >> But just loading on demand, can't magically give you a working order of >> drivers to initialize. E.g. how do you choose the first driver to >> initialize? > > Other problems you will run into are time constraints and multithreaded > drivers. > > E.g. we all should know how tricky it sometimes is to avoid deadlocks. > And with loading on demand, you are extending this problem over the > initialization of maybe a whole bunch of other drivers which might be > started by calling one function of another driver. And a function call > might need a very long time to finish during which an unpredictable > amount of things may happen. > > It would make me wonder if that will end up with a good, usable and as > simple as possible solution. Besides that instrumenting every call to another driver in order to fix a onetime operation (the initialization) sounds like an enormous overhead. Initialization is done pnly once, regardless how long a system runs, but the instrumentation to fix this onetime operation would slow down the operation during the whole runtime of a system. I don't think this is what should be done. > > Regards, > > Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/21] On-demand device registration
Am 11.06.2015 um 12:17 schrieb Alexander Holler: Am 11.06.2015 um 10:12 schrieb Linus Walleij: On Wed, Jun 10, 2015 at 10:28 AM, Alexander Holler wrote: Am 10.06.2015 um 09:30 schrieb Linus Walleij: i2c host comes out, probes the regulator driver, regulator driver probes and then the regulator_get() call returns. This requires instrumentation on anything providing a resource to another driver like those I mentioned and a lot of overhead infrastructure, but I think it's the right approach. However I don't know if I would ever be able to pull that off myself, I know talk is cheap and I should show the code instead. You would end up with the same problem of deadlocks as currently, and you would still need something ugly like the defered probe brutforce to avoid them. Sorry I don't get that. Care to elaborate on why? Because loading/initializing on demand doesn't give you any solved order of drivers to initialize. And it can't because it has no idea about the requirements of other drivers. The reason why it might work better in the case of the tegra is that it might give you another initialization order than the one which is currently choosen, which, by luck, might be a better one. But maybe I missed something, I haven't looked at the patches at all. But just loading on demand, can't magically give you a working order of drivers to initialize. E.g. how do you choose the first driver to initialize? Other problems you will run into are time constraints and multithreaded drivers. E.g. we all should know how tricky it sometimes is to avoid deadlocks. And with loading on demand, you are extending this problem over the initialization of maybe a whole bunch of other drivers which might be started by calling one function of another driver. And a function call might need a very long time to finish during which an unpredictable amount of things may happen. It would make me wonder if that will end up with a good, usable and as simple as possible solution. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/21] On-demand device registration
Am 11.06.2015 um 10:12 schrieb Linus Walleij: On Wed, Jun 10, 2015 at 10:28 AM, Alexander Holler wrote: Am 10.06.2015 um 09:30 schrieb Linus Walleij: i2c host comes out, probes the regulator driver, regulator driver probes and then the regulator_get() call returns. This requires instrumentation on anything providing a resource to another driver like those I mentioned and a lot of overhead infrastructure, but I think it's the right approach. However I don't know if I would ever be able to pull that off myself, I know talk is cheap and I should show the code instead. You would end up with the same problem of deadlocks as currently, and you would still need something ugly like the defered probe brutforce to avoid them. Sorry I don't get that. Care to elaborate on why? Because loading/initializing on demand doesn't give you any solved order of drivers to initialize. And it can't because it has no idea about the requirements of other drivers. The reason why it might work better in the case of the tegra is that it might give you another initialization order than the one which is currently choosen, which, by luck, might be a better one. But maybe I missed something, I haven't looked at the patches at all. But just loading on demand, can't magically give you a working order of drivers to initialize. E.g. how do you choose the first driver to initialize? Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/21] On-demand device registration
Am 08.06.2015 um 20:14 schrieb Alexander Holler: Am 08.06.2015 um 14:26 schrieb Enrico Weigelt, metux IT consult: Am 04.06.2015 um 22:39 schrieb Alexander Holler: > As it seems to have been forgotten or overread, I've mentioned in my series of patches last year that, with a few changes, it's possible to let the algorithm I've used (dfs) to spit out all drivers which can be initialized in parallel. Unfortunately, I've missed that ... could you please resend you patches? Boot time reduction is one of the topics on my 2do in several weeks. https://lkml.org/lkml/2014/5/12/452 And don't forget patch 10/9 which fixed a bug in my previous patch series and which alos was the reason for the large difference in boot times with and without deps: https://lkml.org/lkml/2014/5/13/567 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/21] On-demand device registration
Am 08.06.2015 um 14:26 schrieb Enrico Weigelt, metux IT consult: Am 04.06.2015 um 22:39 schrieb Alexander Holler: > As it seems to have been forgotten or overread, I've mentioned in my series of patches last year that, with a few changes, it's possible to let the algorithm I've used (dfs) to spit out all drivers which can be initialized in parallel. Unfortunately, I've missed that ... could you please resend you patches? Boot time reduction is one of the topics on my 2do in several weeks. https://lkml.org/lkml/2014/5/12/452 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/21] On-demand device registration
Am 03.06.2015 um 23:12 schrieb Rob Clark: > On Mon, May 25, 2015 at 10:53 AM, Tomeu Vizoso > wrote: >> Hello, >> >> I have a problem with the panel on my Tegra Chromebook taking longer than >> expected to be ready during boot (Stéphane Marchesin reported what is >> basically the same issue in [0]), and have looked into ordered probing as a >> better way of solving this than moving nodes around in the DT or playing with >> initcall levels. >> >> While reading the thread [1] that Alexander Holler started with his series to >> make probing order deterministic, it occurred to me that it should be >> possible >> to achieve the same by registering devices as they are referenced by other >> devices. >> >> This basically reuses the information that is already implicit in the probe() >> implementations, saving us from refactoring existing drivers or adding >> information to DTBs. >> >> Something I'm not completely happy with is that I have had to move the call >> to >> of_platform_populate after all platform drivers have been registered. >> Otherwise I don't see how I could register drivers on demand as we don't have >> yet each driver's compatible strings. >> >> For machs that don't move of_platform_populate() to a later point, these >> patches shouldn't cause any problems but it's not guaranteed that we'll avoid >> all the deferred probes as some drivers may not be registered yet. >> >> I have tested this on boards with Tegra, iMX.6 and Exynos SoCs, and these >> patches were enough to eliminate all the deferred probes. >> >> With this series I get the kernel to output to the panel in 0.5s, instead of >> 2.8s. > > So, complete drive-by comment (and I won't claim to be a DT expert, > etc, etc, so take this with a few grains of salt), but why not push > the problem to the DT compiler (or a pre-process step that could be > run on existing DT blobs), which generates an optional DT node that is > the recommended probe order? That seems like it avoids adding > complexity into the early boot code (which seems like a good thing).. I've played with that approach too (as my patches for dtc do contain the same code I've put into the kernel, but decided that it doesn't make much sense. The sort algorithm is really small (some dozen lines), very fast (around 3-5ms on a omap) and might be later used to sort necessary module loading too. So there would be no advantage to put a sorted list into the DT. And having the sort algorithm in the kernel, would make it possible to use it for acpi or something else too, if they manage it to provide the necessary dependencies. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/21] On-demand device registration
Am 03.06.2015 um 21:57 schrieb grygorii.stras...@linaro.org: ... > So few comments from above: > - registering devices later during the System boot may improve boot time. > But resolving of all deferred probes may NOT improve boot time ;) > Have you seen smth like this? If someone is out for boot time reduction, I think one of the best ways would by making driver initialization parallel. Keep in mind that all linked in drivers currently are initialized in series. As it seems to have been forgotten or overread, I've mentioned in my series of patches last year that, with a few changes, it's possible to let the algorithm I've used (dfs) to spit out all drivers which can be initialized in parallel. But as I'm not paid for the work I've done and just did it out of curiosity, interest or how ever you want name it, I haven't spend any more time into that topic, especially as I'm missing the necessary connections to get patches into the kernel. ;) But, as said, it's easy (at least if aren't getting panic when it comes to a bit of algorithm theory) to get a list drivers you can start in parallel if you have such a complete list of dependencies as DT already offers. Just look at the pictures generate by dtc (using my patches), you will see, they already show which drivers can be initialized in parallel. So it would be easy to use e.g. all cores already very early at boot to initialize drivers, not just after init got started. Besides that the würgaround of defered init (which, btw. leads devs to supress error messages, which is especially bad if you are searching a problem) isn't needed anymore if you have a list of dependecies (however you get it, I've used DT because the dependencies already are all there). Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 00/12] On-demand device registration
Am 28.04.2015 um 14:49 schrieb Tomeu Vizoso: On 25 April 2015 at 01:15, Alexander Holler wrote: Am 24.04.2015 um 16:47 schrieb Tomeu Vizoso: Hi, while reading the thread [0] 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 embedded in the probe() implementations, saving us from refactoring existing drivers or adding information to DTBs. The main issue I see is that the registration code path in some subsystems may not be reentrant, so some refactoring of the locking will be needed. In my testing I have found this problem with regulators, as the supply of a regulator might end up being registered during the registration of the first one. Something I'm not completely happy with is that I have had to move the population of the device tree after all platform drivers have been registered. Otherwise I don't see how I could register drivers on demand as we don't have yet each driver's compatible strings. I have done my testing on a Tegra124-based Chromebook, and these patches were enough to eliminate all the deferred probes. First you have to solve a problem which is totally unrelated to DT or ACPI or x86 or ARM: I think as long as drivers don't register themself whithout any side effect, this problem isn't solvable. In order to make an ordered list of drivers to start, you need to know which drivers are actually available. Yeah, I kind of side-stepped that issue by waiting until all drivers have been registered before registering devices. I think someone suggested doing so in your thread (maybe Grant?). That doesn't work. As said above, several drivers doing a lot more than just registering in their initcall. They might even crash if some prerequisits aren't given. And several of these prerequisits (init orders) are hardcoded by various means. There's lots of things that can be improved regarding driver and device initialization, but we have to start somewhere :) That's what I've tried by marking good drivers as such (and by placing them in their own initcall level group). But as said, good luck. I've tried it already and nobody seemed interested. Some people even seem to panic if they see a patch which changes something in linux/init/ ;) Regards, Alexander Holler Thanks, Tomeu And also drivers are registering themself with their initcall, they might do an awfull lot of stuff besides just registering themself. That means several drivers already have prerequisites and dependcies for their initcall. That means you can't just call their initcall to get and idea of which driver an initcall is even part of. That ends up with the fact that you just don't have a list of drivers you can sort, based on whatever algorithm you might have in mind. I've tried to solve that problem with marking drivers which don't have any prerequisits (and side effects) as "well done". The patch which did that was 5/9 in my series, this one: https://lkml.org/lkml/2014/5/12/414 Unfortunately nobody seemed really interested and without one of the few "big guys" in your pocket, it's absolutely impossible to get such changes into the kernel. Not to speak about all the unavoidable discussions about absolutely silly things. But maybe I'm the problem here. No idea. I wish you more luck than I had in the past two or three years. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 00/12] On-demand device registration
Am 24.04.2015 um 16:47 schrieb Tomeu Vizoso: > Hi, > > while reading the thread [0] 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 embedded in the probe() > implementations, saving us from refactoring existing drivers or adding > information to DTBs. > > The main issue I see is that the registration code path in some subsystems > may not be reentrant, so some refactoring of the locking will be needed. In > my testing I have found this problem with regulators, as the supply of a > regulator might end up being registered during the registration of the first > one. > > Something I'm not completely happy with is that I have had to move the > population of the device tree after all platform drivers have been > registered. Otherwise I don't see how I could register drivers on demand as > we don't have yet each driver's compatible strings. > > I have done my testing on a Tegra124-based Chromebook, and these patches were > enough to eliminate all the deferred probes. First you have to solve a problem which is totally unrelated to DT or ACPI or x86 or ARM: I think as long as drivers don't register themself whithout any side effect, this problem isn't solvable. In order to make an ordered list of drivers to start, you need to know which drivers are actually available. And also drivers are registering themself with their initcall, they might do an awfull lot of stuff besides just registering themself. That means several drivers already have prerequisites and dependcies for their initcall. That means you can't just call their initcall to get and idea of which driver an initcall is even part of. That ends up with the fact that you just don't have a list of drivers you can sort, based on whatever algorithm you might have in mind. I've tried to solve that problem with marking drivers which don't have any prerequisits (and side effects) as "well done". The patch which did that was 5/9 in my series, this one: https://lkml.org/lkml/2014/5/12/414 Unfortunately nobody seemed really interested and without one of the few "big guys" in your pocket, it's absolutely impossible to get such changes into the kernel. Not to speak about all the unavoidable discussions about absolutely silly things. But maybe I'm the problem here. No idea. I wish you more luck than I had in the past two or three years. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] ARM: dts: imx6dl: disable dma support for spi on i.mx6dl
Am 17.09.2014 10:51, schrieb Robin Gong: On Tue, Sep 16, 2014 at 11:41:55AM +0200, Alexander Holler wrote: Am 16.09.2014 05:52, schrieb Robin Gong: On Mon, Sep 15, 2014 at 01:50:02PM +0200, Alexander Holler wrote: Am 10.09.2014 07:30, schrieb Robin Gong: There is one weird data in rxfifo after one full rx/tx transfer done sometimes. It looks a design issue and hard to workaround totally, so disable dma functhion here. And will re-enable it once the root cause found. Hmm, I experience problems with DMA too but on uart3. I'm using the same workaround for the uart (I've just commented out the dma entries in the DT). The problem manifests itself here such, that brcm_patchram_plus hangs while uploading the firmware to a BCM4330 connected at uart3 (reproducible). (...) Yes, we just have fixed the BT issue over high speed UART last month. Suggest you trying our patches based on v3.10. Thanks for the pointer. Will look if I will find these patches. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] ARM: dts: imx6dl: disable dma support for spi on i.mx6dl
Am 16.09.2014 05:52, schrieb Robin Gong: On Mon, Sep 15, 2014 at 01:50:02PM +0200, Alexander Holler wrote: Am 10.09.2014 07:30, schrieb Robin Gong: There is one weird data in rxfifo after one full rx/tx transfer done sometimes. It looks a design issue and hard to workaround totally, so disable dma functhion here. And will re-enable it once the root cause found. Hmm, I experience problems with DMA too but on uart3. I'm using the same workaround for the uart (I've just commented out the dma entries in the DT). The problem manifests itself here such, that brcm_patchram_plus hangs while uploading the firmware to a BCM4330 connected at uart3 (reproducible). So maybe there is a bug in the DMA-engine which not only effects SPI. Or both drivers contain the same error in handling DMA (maybe through c&p). But that's just specualtion from me, I haven't looked further into that problem. Regards, Alexander Holler Thanks for your information share. But my issue should be caused by hardware, since everything is ok if it runs on other i.mx6 chip. Is your board also based on i.mx6 chip? If yes, hope you can raise your issue in freescale community or contact with Andy whose mail address added in CC list fugang.d...@freescale.cm. We have fix some bugs in UART DMA case. It's an i.mx6q (Wandboard quad c1) where I have this problem with mainline and much older (but heavily patched freescale 3.10.x based) kernels. A quick web-search suggests that this problem exists since a long time (noticed mainly by people which try to use BT as this seems to be the major use case for high speed serial communication). Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] ARM: dts: imx6dl: disable dma support for spi on i.mx6dl
Am 10.09.2014 07:30, schrieb Robin Gong: There is one weird data in rxfifo after one full rx/tx transfer done sometimes. It looks a design issue and hard to workaround totally, so disable dma functhion here. And will re-enable it once the root cause found. Hmm, I experience problems with DMA too but on uart3. I'm using the same workaround for the uart (I've just commented out the dma entries in the DT). The problem manifests itself here such, that brcm_patchram_plus hangs while uploading the firmware to a BCM4330 connected at uart3 (reproducible). So maybe there is a bug in the DMA-engine which not only effects SPI. Or both drivers contain the same error in handling DMA (maybe through c&p). But that's just specualtion from me, I haven't looked further into that problem. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/9] dt: dependencies (for deterministic driver initialization order based on the DT)
Am 28.08.2014 11:23, schrieb Catalin Marinas:
On Thu, Aug 28, 2014 at 07:50:36AM +0100, Alexander Holler wrote:
And I wonder how the ACPI world solves that problem. My guess would be
hardcoded stuff in the firmware-blob (BIOS), just like it happened with
board files, but I've never seen BIOS code and my knowledge about ACPI
is almost zero. ;)
ACPI doesn't even attempt to solve such problems at the OS level. SoCs
aimed at ACPI should have simple hardware configuration (from a Linux
perspective) initialised by firmware (e.g. clocks) and devices living on
a standard bus like PCIe. If a SoC requires specific low-level code to
initialise the hardware in a specific order (other than architected
peripherals like GIC, timers; e.g. MFD devices), we should deem it
unsuitable for ACPI.
ACPI should only be used by vendors who know exactly why they need and
how to implement it properly and not just because the marketing
department told them to (it would also be nice if the Linux kernel
community was informed about such reasons).
Hmm, Jon Masters from Red Hat sounds too like UEFI/ACPI is the way to go.
But maybe he's right and hiding ugly code in proprietary/binary firmware
blobs where no one can see and criticize it is really the way to go.
Personally I think it's a way back to the past ("when everything was
better", at least that is what most human brains do like to suggest) and
just a dream. And I don't believe that the stuff which was and is hidden
away always just works and doesn't need fixes (one can't do themself).
In my humble opinion that's a nice but wrong myth and doesn't take into
consideration that HW (and SW) gets more and more complicated too (and
thus more faulty).
And even the biggest companies already have problems to produce stuff
which just works (they just employ humans too), not to speak from
smaller board vendors. So it's much better to keep the source as visible
as possible to as much people as possible (in the OS) and not to rely on
some binary blob as most BIOSes are.
Regards,
Alexander Holler
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/9] dt: dependencies (for deterministic driver initialization order based on the DT)
Am 27.08.2014 18:37, schrieb Stephen Warren: Of course, there are probably cases where we could/should add some more phandles/... and likewise cases where we shouldn't. That's why detailed research is needed. Just because I'm curious, I wonder how this research does or shoud look like. Defered probes did come to light with 3.4, that was more than 2 years ago. Ok, most people (like me) just noticed it during the last months when they switched to DT and have run into a problem (the deferred probe mechanism is an error-message killer), but some must have seen it already 2 years ago. And I wonder how the ACPI world solves that problem. My guess would be hardcoded stuff in the firmware-blob (BIOS), just like it happened with board files, but I've never seen BIOS code and my knowledge about ACPI is almost zero. ;) Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/9] dt: dependencies (for deterministic driver initialization order based on the DT)
Am 27.08.2014 19:52, schrieb Catalin Marinas: Irrespective though, a new kernel needs to work against an old DT, I fully agree. But we shouldn't really extend the "old DT" statement to a new ARMv8 SoC ;). Or any new v7 SoC. And even poor users of current ARM HW do want use their HW. And they don't care if they have to change the DT if they finally are able to use their board, which happens seldom enough. (I'm not speaking about companies which are able to spend many man-years to fix one kernel version for use with one specific HW). Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/9] dt: dependencies (for deterministic driver initialization order based on the DT)
Am 27.08.2014 18:37, schrieb Stephen Warren: On 08/27/2014 10:30 AM, Alexander Holler wrote: Am 27.08.2014 18:22, schrieb Stephen Warren: On 08/27/2014 08:44 AM, Catalin Marinas wrote: It's not just optimisation but an important feature for new arm64 SoCs. Given some Tegra discussions recently, in many cases the machine_desc use on arm is primarily to initialise devices in the right order. If we can solve this in a more deterministic way (other than deferred probing), we avoid the need for a dedicated SoC platform driver (or machine_desc) or workarounds like different initcall levels and explicit DT parsing. A lot of the ordering is SW driver dependencies. I'm not sure how much of that can accurately be claimed as HW dependencies. As such, I'm not sure that putting dependencies into DT would be a good idea; it doesn't feel like HW data, and might well change if we restructure SW. It'd need some detailed research though. Almost every phandle is a dependency, so the DT is already full with them. That's true, but not entirely relevant. phandles in DT should only be present where there's an obvious HW dependency. It's obvious that, for example, there's a real HW dependency between an IRQ controller and a device that has an IRQ signal fed into the IRQ controller. It makes perfect sense to represent that as a phandle (+args). However, most of the ordering imposed by the Tegra machine descriptor callbacks is nothing to do with this. It's more that the SW driver for component X needs some low level data (e.g. SKU/fuse information) before it can run. However, there's no real HW dependency between the HW component X and the fuse module. As such, it doesn't make sense to represent such a dependency in DT, using a phandle or by any other means. Of course, there are probably cases where we could/should add some more phandles/... and likewise cases where we shouldn't. That's why detailed research is needed. Irrespective though, a new kernel needs to work against an old DT, so always needs to work without any (of these new) dependencies being represented in DT, since they aren't represented there today. So, I think pushing the issue into DT is a non-starter either way, unless we accept yet another ABI-breaking change, in which case we should just give up on any claims of ABI and make everyone's lives simpler. If I hear research, my response is usually "how many years"? Fact is that there are already a lot of usable dependencies in the DT, they just didn't find their way into the kernel and weren't used. And I think it doesn't help much to make the picture more complicated than it already is. Solve one step by another and not try to solve everything at once. So first enable the kernel to use dependencies at all. I've shown that it doesn't need magic to do so. Afterwards you can extend or change the existing solution. It's not always the best approach, but for complicated things it often doesn't make sense trying to solve everything at first. Of course, my approach isn't perfect, but at least it is something people can already use to play with. Ok, the way how my patches do handle devices (not drivers) might be completely wrong, but that's just because I've never got in contact with the device-model before, it always just worked. So I haven't spend any time to look into that before and I didn't spend much time to look into that for my patches (I just discoverd that device-handling by drivers looks sometimes awkward). I was happy with what I've achieved in the short time I've spend, and therfor posted the patches to give other people an easy possibility to try the stuff themself. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/9] dt: dependencies (for deterministic driver initialization order based on the DT)
Am 27.08.2014 18:22, schrieb Stephen Warren: On 08/27/2014 08:44 AM, Catalin Marinas wrote: It's not just optimisation but an important feature for new arm64 SoCs. Given some Tegra discussions recently, in many cases the machine_desc use on arm is primarily to initialise devices in the right order. If we can solve this in a more deterministic way (other than deferred probing), we avoid the need for a dedicated SoC platform driver (or machine_desc) or workarounds like different initcall levels and explicit DT parsing. A lot of the ordering is SW driver dependencies. I'm not sure how much of that can accurately be claimed as HW dependencies. As such, I'm not sure that putting dependencies into DT would be a good idea; it doesn't feel like HW data, and might well change if we restructure SW. It'd need some detailed research though. Almost every phandle is a dependency, so the DT is already full with them. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/9] dt: dependencies (for deterministic driver initialization order based on the DT)
Am 27.08.2014 09:16, schrieb Alexander Holler: Why should I? I've posted patches along with a lot of comments and explanations, and e.g. you are just talking that it should be made like my patches already did. And others do talk too like my patches and the accompanying comments from me which explain most stuff never have existed and just repeat what the patches already do without refering to them. Just to repeat myself: These patches which started this thread are not just some ideas without any sense for the amount of work necessary to implement them (as seen so often). These patches are real working code everyone can apply to the mentioned kernel version and see what happens with his board. They are even checkpatched to avoid bean counting discussion. (Don't forget to use patch 10/9 too) Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/9] dt: dependencies (for deterministic driver initialization order based on the DT)
Am 26.08.2014 15:58, schrieb Jon Loeliger: I think we need to do the complete topsort *before* we attempt to do any probing. So three steps: 1) Graph Construction Add a new "emit dependencies" function to driver bindings. Iterate over known devices or nodes in the DT in any order. Call the "emit dependencies" function. It adds all dependency edges to a global graph by knowing what phandles or other pieces it will need. A driver with no "emit dependencies" function can be added to the graph anywhere without loss of generality. Add any additional edges for whatever reason. 2) Topsort the generated driver graph 3) Call probe for real in topsort order Alexander, I don't recall the details of your patch series. Can you please remind us if it took this approach in the kernel? Anyway, I'm leaving this discussion. I've already made a proposal which solved most mentioned problems (imho) and even offered usable patches Why should I? I've posted patches along with a lot of comments and explanations, and e.g. you are just talking that it should be made like my patches already did. And others do talk too like my patches and the accompanying comments from me which explain most stuff never have existed and just repeat what the patches already do without refering to them. Darn. I think you clearly have a pony in this race, and it would be good if you still participated. Really. Thanks. But I don't see it as a race and I don't want to take part in a race (and I usually avoid those silly contests which have become chic in the IT world). I just offered a solution (or at least a working starting point to a solution). (ok, they suffer under the "not invented here" syndrom, but ...). ;) There isn't a single thing in the entire Linux Kernel community that was "invented here"; every aspect of it was NIH'ed by *someone*. That's how it gets built, changed, maintained, fixed, etc. Might be true in an ideal open source world and might have been true for past kernel development when most people weren't full time kernel developers. But nowadays it appears to me like many parts of the kernel have become in the hands of closed groups. And they build and enforce hurdles that high, that nobody else can take them without spending an idiotic amount of time. Just like many other "consortiums" do, you only have to build enough rules to protect from the outside while still looking open. E.g. an example I've seen often is that someone spend a lot of time to examine and fix a bug and write a commented patch. And the only response from the maintainer was that he should add an emtpy line before a return statement and similiar silly things to enforce patch-ping-pong. Such just drives people on the other end crazy and they likely won't spend the time to post another patch (they still might fix other bugs, but just for themself). Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/9] dt: dependencies (for deterministic driver initialization order based on the DT)
Am 26.08.2014 13:47, schrieb Thierry Reding: On Tue, Aug 26, 2014 at 01:23:54PM +0200, Alexander Holler wrote: Am 26.08.2014 13:08, schrieb Thierry Reding: On Tue, Aug 26, 2014 at 12:44:35PM +0200, Alexander Holler wrote: Am 26.08.2014 12:25, schrieb Thierry Reding: On Tue, Aug 26, 2014 at 11:42:04AM +0200, Alexander Holler wrote: You need either the type information in the DTB (that's why I've add those "dependencies" to identify phandles), or you need to know every binding (at "dependency-resolve-time" to identify phandles. The latter is impracticable to implement in a generic way (for use with every possible binding). Like I already mentioned, this could be done in drivers who contain that information already anyway. Or parts of it could be done in subsystem- specific callbacks where a generic binding is available. That would end up with almost the same ugly driver-based workarounds as now. It's much better if a driver author only has to define it's prerequisits (in form of dependencies in the dts) and could be sure the driver will only be probed if those are met, than to do that stuff based on a subsystem or even driver level. If you add dependency-information to drivers, you have two problems: We already have all that dependency information in drivers anyway. Each driver requests the resources at .probe() time. What I proposed (it was really Arnd who proposed it first) is to move that information out of code and into some sort of table that could be used by the driver core to figure out dependencies. - How do you get these information from the driver (remember, currently there is only one initial call, a initcall which might do almost anything) While I don't think it's necessary, that's something that could be changed. I mean, we have access to the full source code of this operating system, so we can change every aspect of it. If we can't find a way to make this work with the current initcall sequence it's always an option to extend that sequence so that it meets our needs. - These information might become outdated and you would have to change all drivers. E.g. if the name of a dependency (driver) changes it wouldn't be done with changing the dts (maybe plural), but you would have to change the source of all dependant drivers too. No. Drivers implement a DT binding. That binding defines what power supplies, clocks, pinmux, ... the device needs. Those constitute the dependencies. We most certainly don't want to depend on driver names since there can be a multitude of different drivers that provide a given dependency. What drivers should provide (and what they already provide today) is the name of the property and the index of the cell that they expect to find a phandle in as well as the type of the phandle. That's all that's necessary, really. Everything else can be derived from that phandle and the type. Drivers don't provide that information (dependencies) in any usable way. And as you said yourself, it's already contained in phandles. So what we are discussing here about? The proposal to use phandles for that is already on the table since several month. ;) Sorry, but I don't understand what you want to propose. In many cases we simply don't know where phandles are stored since we don't have the type information in DT. But drivers already know the type of a specific phandle and where to get it from, so the proposal is to make that knowledge more generally useful so that it can be used for dependency resolution. How? Anyway, I'm leaving this discussion. I've already made a proposal which solved most mentioned problems (imho) and even offered usable patches (ok, they suffer under the "not invented here" syndrom, but ...). ;) But please continue this discussion, I will try to not disturb it anymore. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/9] dt: dependencies (for deterministic driver initialization order based on the DT)
Am 26.08.2014 13:08, schrieb Thierry Reding: On Tue, Aug 26, 2014 at 12:44:35PM +0200, Alexander Holler wrote: Am 26.08.2014 12:25, schrieb Thierry Reding: On Tue, Aug 26, 2014 at 11:42:04AM +0200, Alexander Holler wrote: You need either the type information in the DTB (that's why I've add those "dependencies" to identify phandles), or you need to know every binding (at "dependency-resolve-time" to identify phandles. The latter is impracticable to implement in a generic way (for use with every possible binding). Like I already mentioned, this could be done in drivers who contain that information already anyway. Or parts of it could be done in subsystem- specific callbacks where a generic binding is available. That would end up with almost the same ugly driver-based workarounds as now. It's much better if a driver author only has to define it's prerequisits (in form of dependencies in the dts) and could be sure the driver will only be probed if those are met, than to do that stuff based on a subsystem or even driver level. If you add dependency-information to drivers, you have two problems: We already have all that dependency information in drivers anyway. Each driver requests the resources at .probe() time. What I proposed (it was really Arnd who proposed it first) is to move that information out of code and into some sort of table that could be used by the driver core to figure out dependencies. - How do you get these information from the driver (remember, currently there is only one initial call, a initcall which might do almost anything) While I don't think it's necessary, that's something that could be changed. I mean, we have access to the full source code of this operating system, so we can change every aspect of it. If we can't find a way to make this work with the current initcall sequence it's always an option to extend that sequence so that it meets our needs. - These information might become outdated and you would have to change all drivers. E.g. if the name of a dependency (driver) changes it wouldn't be done with changing the dts (maybe plural), but you would have to change the source of all dependant drivers too. No. Drivers implement a DT binding. That binding defines what power supplies, clocks, pinmux, ... the device needs. Those constitute the dependencies. We most certainly don't want to depend on driver names since there can be a multitude of different drivers that provide a given dependency. What drivers should provide (and what they already provide today) is the name of the property and the index of the cell that they expect to find a phandle in as well as the type of the phandle. That's all that's necessary, really. Everything else can be derived from that phandle and the type. Drivers don't provide that information (dependencies) in any usable way. And as you said yourself, it's already contained in phandles. So what we are discussing here about? The proposal to use phandles for that is already on the table since several month. ;) Sorry, but I don't understand what you want to propose. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/9] dt: dependencies (for deterministic driver initialization order based on the DT)
Am 26.08.2014 12:44, schrieb Alexander Holler: Am 26.08.2014 12:25, schrieb Thierry Reding: On Tue, Aug 26, 2014 at 11:42:04AM +0200, Alexander Holler wrote: You need either the type information in the DTB (that's why I've add those "dependencies" to identify phandles), or you need to know every binding (at "dependency-resolve-time" to identify phandles. The latter is impracticable to implement in a generic way (for use with every possible binding). Like I already mentioned, this could be done in drivers who contain that information already anyway. Or parts of it could be done in subsystem- specific callbacks where a generic binding is available. That would end up with almost the same ugly driver-based workarounds as now. It's much better if a driver author only has to define it's prerequisits (in form of dependencies in the dts) and could be sure the driver will only be probed if those are met, than to do that stuff based on a subsystem or even driver level. If you add dependency-information to drivers, you have two problems: - How do you get these information from the driver (remember, currently there is only one initial call, a initcall which might do almost anything) - These information might become outdated and you would have to change all drivers. E.g. if the name of a dependency (driver) changes it wouldn't be done with changing the dts (maybe plural), but you would have to change the source of all dependant drivers too. And after having sorted my brain: A driver depends on a binding (and its API), but not on explicit named other drivers. So trying it (again) on driver level is doomed to fail. Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/9] dt: dependencies (for deterministic driver initialization order based on the DT)
Am 26.08.2014 12:25, schrieb Thierry Reding: On Tue, Aug 26, 2014 at 11:42:04AM +0200, Alexander Holler wrote: You need either the type information in the DTB (that's why I've add those "dependencies" to identify phandles), or you need to know every binding (at "dependency-resolve-time" to identify phandles. The latter is impracticable to implement in a generic way (for use with every possible binding). Like I already mentioned, this could be done in drivers who contain that information already anyway. Or parts of it could be done in subsystem- specific callbacks where a generic binding is available. That would end up with almost the same ugly driver-based workarounds as now. It's much better if a driver author only has to define it's prerequisits (in form of dependencies in the dts) and could be sure the driver will only be probed if those are met, than to do that stuff based on a subsystem or even driver level. If you add dependency-information to drivers, you have two problems: - How do you get these information from the driver (remember, currently there is only one initial call, a initcall which might do almost anything) - These information might become outdated and you would have to change all drivers. E.g. if the name of a dependency (driver) changes it wouldn't be done with changing the dts (maybe plural), but you would have to change the source of all dependant drivers too. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/9] dt: dependencies (for deterministic driver initialization order based on the DT)
Am 26.08.2014 10:51, schrieb Grant Likely: Getting the dependency tree I think is only half the problem. The other have is how to get the driver model to actually order probing using that list. That problem is hard because the order drivers are probed is currently determined by the interaction of driver link order, driver initcall level, and device registration order. The first devices are > registered at an early initcall, before their drivers, and therefore > bind order is primarily determined by initcall level and driver link > order. However, later devices (ie. i2c clients) are registered by the > bus driver (ie. again, i2c) and probe order may be primarily link order > (if the driver is not yet registered) or registration order (if the > driver was registered before the parent bus). Using my patches, the problem which still exists is how to handle devices (not drivers). I've build the patches based on the assumption that device-handling happens automatically. Unfortunately that isn't really true and device-handling looks awkward. Some drivers build them themself, some require that a device already exists and some require that a device doesn't already exist. But I haven't looked in deep at that. I'm sure that can be fixed by fixing drivers which do things differently than they should (maybe because they needed to do such for dirty workarounds because no order was guaranteed, which wouldn't be true anymore). Anyway, I've not looked further into that problem (with devices, not drivers) as it already seems quiet impossible to get the other necessary stuff into the kernel in a reasonable time (before 32bit-HW which does use DT will not be available anymore). Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/9] dt: dependencies (for deterministic driver initialization order based on the DT)
Am 26.08.2014 10:51, schrieb Grant Likely: On Mon, 25 Aug 2014 08:08:59 -0500, Jon Loeliger wrote: Anyway, instead of going back and forth between "deferred probe is good" and "deferred probe is bad", how about we do something useful now and concentrate on how to make use of the information we have in DT with the goal to reduce the number of cases where deferred probing is required? Good idea. The proposal on the table is to allow the probe code to make a topological sort of the devices based on dependency information either implied, explicitly stated or both. That is likely a fundamentally correct approach. I believe some of the issues that need to be resolved are: 1) What constitutes a dependency? 2) How is that dependency expressed? 3) How do we add missing dependencies? 4) Backward compatability problems. There are other questions, of course. Is it a topsort per bus? Are there required "early devices"? Should the inter-node dependencies be expressed at each node, or in a separate hierarchy within the DTS? Others. Topsort by bus wouldn't work. That would imply that nothing uses something involved with another bus. And early devices are handled fine by normal dependencies too (as long as they are complete), so there is no need to make an distinction. Getting the dependency tree I think is only half the problem. The other have is how to get the driver model to actually order probing using that list. That problem is hard because the order drivers are probed is currently determined by the interaction of driver link order, driver initcall level, and device registration order. The first devices are registered at an early initcall, before their drivers, and therefore bind order is primarily determined by initcall level and driver link order. However, later devices (ie. i2c clients) are registered by the bus driver (ie. again, i2c) and probe order may be primarily link order (if the driver is not yet registered) or registration order (if the driver was registered before the parent bus). That's why I've invented those "well-done"-initcalls. These are initcalls which just register a driver and don't probe it. Thats necessary to build a catalog of existing in-kernel-drivers (you have to know what you are sorting). Fortunately most drivers are already of that type. And those which aren't can be either just used like before (if it works) or they can be changed. Changing them can be done per board (only enable dependency based order for a board if necessary drivers have changed). Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/9] dt: dependencies (for deterministic driver initialization order based on the DT)
Am 26.08.2014 10:49, schrieb Thierry Reding: On Tue, Aug 26, 2014 at 09:42:08AM +0100, Grant Likely wrote: On Mon, 25 Aug 2014 15:37:16 +0200, Thierry Reding wrote: [...] There are somewhat standardized bindings for the above and especially for bindings of the type that clocks implement this is trivial. We can simply iterate over each (phandle, specifier) tuple and check that the corresponding clock provider can be resolved (which typically means that it's been registered with the common clock framework). For regulators (and regulator-like bindings) the problem is somewhat more difficult because they property names are not standardized. One way to solve this would be to look for property names with a -supply suffix, but that could obviously lead to false positives. One alternative that I think could eliminate this would be to explicitly list dependencies in drivers. This would allow core code to step through such a list and resolve the (phandle, specifier) tuples. False positives and negatives may not actually be a problem. It is suboptimal, certainly, but it shouldn't outright break the kernel. There could be cases where some random integer in a cell could be interpreted as a phandle and resolve to a struct device_node. I suppose it might be unlikely, but not impossible, that the device_node could even match a device in the correct subsystem and you'd get a wrong dependency. Granted, a wrong dependency may not be catastrophic in that it won't lead to a crash, but it could lead to various kinds of weirdness and hard to diagnose problems. You need either the type information in the DTB (that's why I've add those "dependencies" to identify phandles), or you need to know every binding (at "dependency-resolve-time" to identify phandles. The latter is impracticable to implement in a generic way (for use with every possible binding). Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/9] dt: dependencies (for deterministic driver initialization order based on the DT)
Am 25.08.2014 15:08, schrieb Jon Loeliger: Anyway, instead of going back and forth between "deferred probe is good" and "deferred probe is bad", how about we do something useful now and concentrate on how to make use of the information we have in DT with the goal to reduce the number of cases where deferred probing is required? Good idea. The proposal on the table is to allow the probe code to make a topological sort of the devices based on dependency information either implied, explicitly stated or both. That is likely a fundamentally correct approach. I believe some of the issues that need to be resolved are: 1) What constitutes a dependency? In my patches phandles are used. That works pretty good for almost all DTs. So almost all dependencies are already declared in a DT and almost no changes to the DT are necessary. The only binding I've seen where it doesn't work is remote-endpoint, because that binding isn't a directed dependency. So one of the two places where such a binding occurs needs a "no-dependencies = " to avoid circular dependencies which can be solved. 2) How is that dependency expressed? Already there in form of phandles. 3) How do we add missing dependencies? My patches offer the possibility to extend or reduce the list of (automatically generated) dependencies by using "[no-]dependencies = < list of phandles >;" 4) Backward compatability problems. None in my approach. The DT just includes an additional binding to circumvent the missing but needed type information for phandles. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/9] dt: dependencies (for deterministic driver initialization order based on the DT)
Am 22.08.2014 15:19, schrieb Mark Rutland: On Thu, Aug 21, 2014 at 08:19:00PM +0100, Alexander Holler wrote: Am 21.08.2014 16:02, schrieb Thierry Reding: Anyway, those are all fairly standard reasons for where deferred probe triggers, and since I do like deferred probe for it's simplicity and reliability I'd rather not try to work around it if boot time is all that people are concerned about. It's neither simple nor reliable. It's non deterministic brutforcing while making it almost impossible to identify real errors. It's horrible, yes. In my humble opinion the worst way to solve something. I'm pretty sure if I would have suggest such a solution, the maintainer crowd would have eaten me without cooking. We didn't have a better workable solution at the time. Having a hack that got boards booting was considered better than not having them boot. I don't remember people being particularly enthralled by the idea. Agreed. And usually I don't flame about workarounds. They are needed practice usually born out of a time limited background or similiar constraints. Only Linux kernel maintainers do demand perfect stuff from others as the kernel seems to have to be a perfect school project. I for myself already think checkpatch is a ridiculous tool, only invented to drive people crazy. Of course, it's better a tool drives people crazy than a maintainer who make decisions based on the phase of the moon, but ... ;) And I haven't flamed much about deferred probe before, but if I read it's simple and reliable I couldn't stand still. Sorry, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/9] dt: dependencies (for deterministic driver initialization order based on the DT)
Am 21.08.2014 16:02, schrieb Thierry Reding: Anyway, those are all fairly standard reasons for where deferred probe triggers, and since I do like deferred probe for it's simplicity and reliability I'd rather not try to work around it if boot time is all that people are concerned about. It's neither simple nor reliable. It's non deterministic brutforcing while making it almost impossible to identify real errors. In my humble opinion the worst way to solve something. I'm pretty sure if I would have suggest such a solution, the maintainer crowd would have eaten me without cooking. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/9] dt: deps: dtc: Automatically add new property 'dependencies' which contains a list of referenced phandles
Am 27.05.2014 22:02, schrieb Grant Likely:
> On Mon, 19 May 2014 14:35:49 +0200, Alexander Holler
> wrote:
>> What's still questionable about the patches for dtc is if dependencies
>> to devices and not just drivers should be included in the new property
>> dependencies too. My current assumption is that all devices belonging to
>> one and the same driver don't have dependencies between each other. In
>> other words the order in which devices will be attached to one and the
>> same driver isn't important. If that assumption is correct it would be
>> possible to just attach all devices belonging to a driver after the
>> driver was loaded (also I haven't that done in my patches).
>
> There aren't really any guarantees here. It is perfectly valid to have
> two of the same device depending on the other, or even a device with a
> different driver between the two.
>
> There's always going to be corner cases on the dependency chain. The
> question is whether or not it is worth trying to solve every concievable
> order, or if a partway solution is good enough.
Solving dependencies always happens automatically, with or without
dependencies inbetween devices. I just ignored dependencies between pure
devices (instead changed them into dependencies between drivers) because
I'm still not sure how to handle devices at all. Below is a diff ontop
my dtc-patches to include dependencies between devices too. As said, the
changes to do so are minimal. Of course, the graphs are a bit more
complex, because they then include devices too, but that isn't any
problem for the solving algorithm at all.
diff --git a/scripts/dtc/dependencies.c b/scripts/dtc/dependencies.c
index 06f447b..602ec01 100644
--- a/scripts/dtc/dependencies.c
+++ b/scripts/dtc/dependencies.c
@@ -66,8 +66,10 @@ static void add_deps(struct node *dt, struct node
*node, struct property *prop)
continue;
}
- source = find_compatible_not_disabled(node);
- target = find_compatible_not_disabled(refnode);
+ //source = find_compatible_not_disabled(node);
+ //target = find_compatible_not_disabled(refnode);
+ source = node;
+ target = refnode;
if (!source || !target || source == target ||
is_parent_of(source, target) ||
is_parent_of(target, source))
@@ -385,9 +387,9 @@ static int __init add_deps_lnx(struct device_node
*parent,
if (!__of_device_is_available(node))
return 0;
- if (__of_get_property(node, "compatible", NULL)) {
+// if (__of_get_property(node, "compatible", NULL)) {
if (!parent->phandle) {
- if (__of_get_property(parent, "compatible", NULL))
+// if (__of_get_property(parent, "compatible", NULL))
parent->phandle = 1 + order.max_phandle++;
}
if (!node->phandle)
@@ -425,7 +427,7 @@ static int __init add_deps_lnx(struct device_node
*parent,
if (unlikely(rc))
return rc;
parent = node; /* change the parent only if node is a driver */
- }
+// }
for_each_child_of_node(node, child) {
rc = add_deps_lnx(parent, child, print_dot);
if (unlikely(rc))
--
1.8.3.2
To make it easier to see devices in the produced order, here is another
patch on top:
@@ -464,6 +467,8 @@ void __init of_init_print_order(const char *name)
if (order.order[i]->full_name)
pr_cont(" (%s)", order.order[i]->full_name);
prop = get_property(order.order[i], "compatible");
+ if (!prop)
+ pr_cont(" -");
for (cp = of_prop_next_string(prop, NULL); cp;
cp = of_prop_next_string(prop, cp))
pr_cont(" %s", cp);
With that patch one can do e.g. dtc -t | grep ' - ' to see which device
nodes are included which don't have a compatible property. For the
omap3-beagle this produces the following 21 additional entries in the
init-order:
aholler@laptopahbt ~/Source/aholler/dtc.git $ dts/make_dtb.sh
dts/omap3-beagle.dts -t | grep ' - '
init 4 0xe4 pinmux_twl4030_pins
(/ocp/pinmux@48002030/pinmux_twl4030_pins) - (parent 0x14b)
init 10 0x107 clocks (/ocp/cm@48004000/clocks) - (parent 0x106)
init 17 0x101 clocks (/ocp/prm@48306000/clocks) - (parent 0x100)
init 226 0x143 clocks (/ocp/scrm@48002000/clocks) - (parent 0x142)
init 237 0xe1 pinmux_hsusb2_pins
(/ocp/pinmux@48002030/pinmux_hsusb2_pins) - (parent 0x14b)
init 239 0xe2 pinmux_gpio1_pins (/ocp/pinmux@480
Re: [RFC PATCH 1/9] dt: deps: dtc: Automatically add new property 'dependencies' which contains a list of referenced phandles
Am 19.05.2014 17:49, schrieb Jon Loeliger: [ Crap. Sorry about the duplicate post. Stupid HTML; didn't hit the lists. -- jdl ] What's still questionable about the patches for dtc is if dependencies to devices and not just drivers should be included in the new property dependencies too. I don't think the DTC should have any semantic knowledge of why these dependency arcs are being added to the graph. Sure, it could be that different types of arcs are added, and that the total dependency graph travels multiple such arc types to obtains some valid topological sort, but the DTC itself should just not care. I will remove those policies (which means including all dependencies). As said below, I already thought it was evil premature optimization (I did in order to make the graph a bit smaller and to save some bytes). (No date, it isn't a paid project so I will do it whenever I feel good to do so). After saying that, there are likely semantic checks that could be added to ensure some policy about those arcs was followed. Separate the implementation from the policy. There is already plenty of discussion down that line within the DTC ongoing. Hmm, discussion about what? Those dependencies or about semantic checks? Btw., if someone has a problem with the necessary time to do the topological sort at boot time (needs a few ms on a single core omap with 600 MHz), there could be an additional option to add a new property which includes the whole (already topological sorted) list. That wouldn't be much effort. But currently I don't think any DT enabled device is in need of having to avoid doing the topological sort itself. Regards, Alexander Holler HTH, jdl On Mon, May 19, 2014 at 7:35 AM, Alexander Holler wrote: Am 17.05.2014 14:16, schrieb Tomasz Figa: References to phandles of parent or child nodes will not be added to this property, because this information is already contained in the blob (in the form of the tree itself). I wonder if we shouldn't be including them too for consistency related reasons, so we have all the necessary information in one place. References to child nodes are great recipes for cycles, though... No strong opinion, though, just an idea. As said, they are already in the tree itself. And they are already included in the graph (these are the black edges), so they just don't appear in the property dependencies. No dependencies to disabled nodes will be added. Same here. IMHO it might be wise to let the parsing entity (e.g. kernel) decide whether to ignore a dependency to disabled node or not. Otherwise, I like the simplicity of compile-time dependency list creation. Quite a nice work. Thanks. What's still questionable about the patches for dtc is if dependencies to devices and not just drivers should be included in the new property dependencies too. My current assumption is that all devices belonging to one and the same driver don't have dependencies between each other. In other words the order in which devices will be attached to one and the same driver isn't important. If that assumption is correct it would be possible to just attach all devices belonging to a driver after the driver was loaded (also I haven't that done in my patches). And thinking about that again, I think I was wrong and doing so have been some kind of evil premature optimization I did in order to spare a few dependencies/edges. But changing this can done by removing a few lines in the code for dtc (patch 1). Regards, Alexander Holler ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/9] dt: deps: dtc: Automatically add new property 'dependencies' which contains a list of referenced phandles
Am 17.05.2014 14:16, schrieb Tomasz Figa: References to phandles of parent or child nodes will not be added to this property, because this information is already contained in the blob (in the form of the tree itself). I wonder if we shouldn't be including them too for consistency related reasons, so we have all the necessary information in one place. References to child nodes are great recipes for cycles, though... No strong opinion, though, just an idea. As said, they are already in the tree itself. And they are already included in the graph (these are the black edges), so they just don't appear in the property dependencies. No dependencies to disabled nodes will be added. Same here. IMHO it might be wise to let the parsing entity (e.g. kernel) decide whether to ignore a dependency to disabled node or not. Otherwise, I like the simplicity of compile-time dependency list creation. Quite a nice work. Thanks. What's still questionable about the patches for dtc is if dependencies to devices and not just drivers should be included in the new property dependencies too. My current assumption is that all devices belonging to one and the same driver don't have dependencies between each other. In other words the order in which devices will be attached to one and the same driver isn't important. If that assumption is correct it would be possible to just attach all devices belonging to a driver after the driver was loaded (also I haven't that done in my patches). And thinking about that again, I think I was wrong and doing so have been some kind of evil premature optimization I did in order to spare a few dependencies/edges. But changing this can done by removing a few lines in the code for dtc (patch 1). Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/9] dt: deps: dependency based device creation
Am 18.05.2014 16:59, schrieb Grant Likely: Hi Tomasz, Thanks for weighing in on this. Thoughts and comments below. On Sat, 17 May 2014 16:24:19 +0200, Tomasz Figa wrote: registration order. Alexander had to hook into the driver registration patch to get the optimal probe order. For device order to be effective, I had to hook into the driver registration to get information about the (available) drivers. Without the hook it is currently impossible to identify drivers before they start doing things. To reccover, I had to solve several problems: - Getting dependencies (happens almost automatically by using phandle references) - Get them to the kernel (done by using a new property) - Build order (already a solved problem, think at make) - Identify available drivers (invented hook, "well done" is meant in regard to this feature, I needed a name and found "well done" apropriate because it too might stimulate driver authors to use it) - Check out how to handle/start/register devices and drivers (in order). I think the last one is the most unfinished and questionable part. The part to identify drivers could be done much better by linking an array of struct platform_driver, but in order to use such an array, drivers have to be done "well done" too (which means no action before probe). So that well-done hook can be seen as an intermediate step. Having said that, there are some things that I worry about. I worry about the cost of doing dependency sorting, both in calculating the dependency tree and in pushing back probe calls to the end of initcalls. Building and calculating the dependency tree just needs a few ms and I think it's much faster than what is necessary afterwards, all those string compares to match drivers/devices. But this string compares already do happen, and I think this part could optimized a lot, when a list of drivers and their compatibility strings is available. Then it's possible to build a hash or e.g. radix tree which leads from the compatibility string to the available driver(s). I worry that incorrect dependency information will cause some devices to not get bound (say because the kernel things the dependency isn't met when it actually is). All (not started) drivers and (unbounded) devices can still be registered/bound after those which appear in the order. That would be just like before. But as said, the whole handling which happens after the order was build is done quick & dirty, done with missing knownledge about the device model, and might contain a lot of bugs and even might need that some drivers will be changed. Therefor all changes disappear when CONFIG_OF_DEPENDENCIES is disabled. So tested platforms might use it (taking advantage of a deterministic order in order to get rid of hardcoded stuff to fix the order) and others don't have to care. So, as already said, I've posted these patches to make evaluation easy, without the need to discuss just ideas but something real to play with (in order to get something happen on this front, not just hardcoded hacks done in individual drivers because such passes maintainers easier). I didn't cared much about form or how to split those patches into more convenient parts. That is stuff where I just do it like a maintainer does want it. I did them as I like them, and I don't want to end up in a time wasting discussions about form, style or similiar questions. So if anyone would be comfortable to merge these patches (for evaluation by others) in other form or splitted in more parts, I will just hear and do. I also don't have any objections in changes in the stuff which happens after the order was build. In fact I would even like it if someone with more experience with the driver model would do it. I just had to do something there too, otherwise it would still have been just an idea which wouldn't offer much motivation to actually look at it. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/9] dt: deps: dependency based device creation
Am 16.05.2014 13:00, schrieb Grant Likely: On Wed, 14 May 2014 23:10:39 +0200, Alexander Holler wrote: Am 14.05.2014 22:06, schrieb Grant Likely: On Wed, 14 May 2014 16:49:05 +0200, Alexander Holler wrote: Am 14.05.2014 16:05, schrieb Grant Likely: On Mon, 12 May 2014 18:47:53 +0200, Alexander Holler wrote: Hmm, I don't really care if that will be merged. I have no motivation to fight with Linux kernel maintainers and I don't know if I will spend the necessary time to do so. The people you need to convince are Rob, Greg, and me, and you've got my attention. If I'm convinced, then I can probably convince Greg also. You've got an interesting approach, and I hope you won't give up on it. Sorry, but that point of the view is already a problem. Why do I have to convince you people? To summarize: Linux kernel maintainers - don't like code they haven't written theirself, - don't like code which doesn't look like they have written it theirself, - don't like ideas they haven't had themself, - do feel good in their closed group they have formed, I'm sorry that you feel that way and that you don't want to continue with this. Best wishes. Sorry to hit you with reality, but it just will not happen that I will try to convience any Linux kernel maintainer (anymore). It is their job to decide what they want and not mine to convince them. I offer code and arguments, but I will not offer my pride, ego or how you want to name it. If Linux kernel maintainers are unable to deal with the power they've got, that isn't my problem. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/9] dt: dependencies (for deterministic driver initialization order based on the DT)
Am 14.05.2014 21:24, schrieb Alexander Holler:
> Am 14.05.2014 21:06, schrieb Rob Herring:
>> I still have not seen an example of A depends on B, deferred probe
>> fails because of ? and here is the code for A that works around the
>> problem.
>>
>>> Anyway, this feature is totally independ of the deferred probe stuff and
>>> both can friendly live together.
>>
>> Yes, except then we get to maintain both.
And just in case someone still hasn't realized what the goal of a
deterministic initialization order is, have a look at this snippet from
arch/arm/mach-omap2/gpio.c:
/*
* gpio_init needs to be done before
* machine_init functions access gpio APIs.
* Hence gpio_init is a omap_postcore_initcall.
*/
static int __init omap2_gpio_init(void)
{
/* If dtb is there, the devices will be created dynamically */
if (of_have_populated_dt())
return -ENODEV;
return omap_hwmod_for_each_by_class("gpio", omap2_gpio_dev_init, NULL);
}
omap_postcore_initcall(omap2_gpio_init);
(Sorry to the OMAP guys, it isn't there fault that it has to look like
this.)
But this is ecactly what should be avoided and why the kernel is in need
of a deterministic, easy to setup, initialization order. And deferred
probes are in now way a help to reach that target, in fact they even
support such stuff. Does anybody outside the OMAP crew do understand
what that piece of code does? The answer is pretty likely no. Again
sorry to the OMAP guys, I'm pretty sure that code was born out of
necessity because no other mechanism is available to get things in order.
Regards,
Alexander Holler
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/9] dt: dependencies (for deterministic driver initialization order based on the DT)
Am 14.05.2014 21:06, schrieb Rob Herring: > On Wed, May 14, 2014 at 12:45 PM, Alexander Holler > wrote: >> Am 14.05.2014 19:30, schrieb Rob Herring: >> >>> On Wed, May 14, 2014 at 11:23 AM, Alexander Holler >>> wrote: >>>> >>>> Am 14.05.2014 18:05, schrieb Grant Likely: >>>> >>>>> On Wed, May 14, 2014 at 4:02 PM, Alexander Holler >>>>> wrote: >>>>>> >>>>>> >>>>>> Am 14.05.2014 16:19, schrieb Grant Likely: >>>>>> >>>>>> >>>>>>> Rather than a dtb schema change, for the most common properties (irqs, >>>>>>> clocks, gpios), we could extract dependencies at boot time. I don't >>>>>>> like >>>>>>> the idea of adding a separate depends-on property because it is very >>>>>>> easy to get it out of sync with the actual binding data (dtc is not >>>>>>> the >>>>>>> only tool that manipulates .dtbs. Firmware will fiddle with it too). >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> Then that stuff has to fiddle correct. Sorry, but trying to solve all >>>>>> problems right from the beginning just leads to endless talks with no >>>>>> end >>>>>> and nothing will happen at all because nobody aggrees how to start. >>>>> >>>>> >>>>> >>>>> I appreciate the problem that you're trying to solve and why you're >>>>> using the dtc approach. My job is to poke at the solution and make >>>>> sure it is going to be reliable. Making sure all users know how to >>>>> fiddle with the new property correctly is not a trivial problem, >>>>> especially when it is firmware that will not necessarily be updated. >>>> >>>> >>>> >>>> The answer is just that they don't have to use this feature. >>> >>> >>> It's not just about users, but maintainers have to carry the code and >>> anything tied to DT is difficult to change or remove. >>> >>> Lots of inter-dependencies are already described in DT. We should >>> leverage those first and then look at how to add dependencies that are >>> not described. >> >> >> Again, that's what this feature is about. One of the problems it solves is >> that those dependencies which are described in the DT source in form of >> phandle reference, do disappear in the blobs because the init-system would >> have to know all bindings in order to identify phandle references (the >> dependencies) again. > > They don't disappear, but they become binding specific to recover. > What you are loosing is type information which is something we would > like to solve as well. > > You can regenerate or figure out the dependencies with knowledge of > the binding. The of_irq_init code does this. Maintaining this > information in the dtb that can be parsed in a generic way and having > the kernel handle non-bus oriented dependencies are really 2 separate > problems. Let's not try to solve it all at once. > Btw. I wonder if you have really read what I did and what I have written. At first the need for knowledge of the binding is broken by design and will never work for any general solution. Second, I've already written almost the same as you've written in your first paragraph in my pragraph you've just quoted directly above. And third, if you like to solve that problem, I just posted a possible solution. ;) You only have to take it, feel free to so, all the patches do contain a Signed-off-by which means that I don't care much what you do with them. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/9] dt: deps: dependency based device creation
Am 14.05.2014 22:06, schrieb Grant Likely: > On Wed, 14 May 2014 16:49:05 +0200, Alexander Holler > wrote: >> Am 14.05.2014 16:05, schrieb Grant Likely: >>> On Mon, 12 May 2014 18:47:53 +0200, Alexander Holler >>> wrote: >> Hmm, I don't really care if that will be merged. I have no motivation to >> fight with Linux kernel maintainers and I don't know if I will spend the >> necessary time to do so. > > The people you need to convince are Rob, Greg, and me, and you've got my > attention. If I'm convinced, then I can probably convince Greg also. > You've got an interesting approach, and I hope you won't give up on it. Sorry, but that point of the view is already a problem. Why do I have to convince you people? To summarize: Linux kernel maintainers - don't like code they haven't written theirself, - don't like code which doesn't look like they have written it theirself, - don't like ideas they haven't had themself, - do feel good in their closed group they have formed, ... You see, I long have given up. The reason I still posted these patches is just because I don't care anymore about Linux kernel maintainers at all. I was prepared that I do that just for the fun of annoying some Linux kernel maintainers. I've already decided some time ago that I just will post my patches once to the LKML (so that other poor Linux kernel users may find them) and will ignore Linux kernel maintainers. Sorry, but I have a long and very unpleasant history in dealing with Linux kernel maintainers, and they already have called me almost anything like "ugly code writer", "frustrated" and such things more. Fortunately I'm too old to care about such, that's why I still post patches (besides that I like to solve problems and help other people). ;) Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/9] dt: dependencies (for deterministic driver initialization order based on the DT)
Am 14.05.2014 21:06, schrieb Rob Herring: On Wed, May 14, 2014 at 12:45 PM, Alexander Holler wrote: Am 14.05.2014 19:30, schrieb Rob Herring: On Wed, May 14, 2014 at 11:23 AM, Alexander Holler wrote: Am 14.05.2014 18:05, schrieb Grant Likely: On Wed, May 14, 2014 at 4:02 PM, Alexander Holler wrote: Am 14.05.2014 16:19, schrieb Grant Likely: Rather than a dtb schema change, for the most common properties (irqs, clocks, gpios), we could extract dependencies at boot time. I don't like the idea of adding a separate depends-on property because it is very easy to get it out of sync with the actual binding data (dtc is not the only tool that manipulates .dtbs. Firmware will fiddle with it too). Then that stuff has to fiddle correct. Sorry, but trying to solve all problems right from the beginning just leads to endless talks with no end and nothing will happen at all because nobody aggrees how to start. I appreciate the problem that you're trying to solve and why you're using the dtc approach. My job is to poke at the solution and make sure it is going to be reliable. Making sure all users know how to fiddle with the new property correctly is not a trivial problem, especially when it is firmware that will not necessarily be updated. The answer is just that they don't have to use this feature. It's not just about users, but maintainers have to carry the code and anything tied to DT is difficult to change or remove. Lots of inter-dependencies are already described in DT. We should leverage those first and then look at how to add dependencies that are not described. Again, that's what this feature is about. One of the problems it solves is that those dependencies which are described in the DT source in form of phandle reference, do disappear in the blobs because the init-system would have to know all bindings in order to identify phandle references (the dependencies) again. They don't disappear, but they become binding specific to recover. What you are loosing is type information which is something we would like to solve as well. You can regenerate or figure out the dependencies with knowledge of the binding. The of_irq_init code does this. Maintaining this information in the dtb that can be parsed in a generic way and having the kernel handle non-bus oriented dependencies are really 2 separate problems. Let's not try to solve it all at once. I'm not saying flat out 'no' here, but before I merge anything, I have to be reasonably certain that the feature is not going to represent a maintenance nightmare over the long term. The maintenance nightmare is already present in form of all the workarounds which are trying to fix the initialzation order necessary for modern hardware. Do you have concrete examples or cases where deferred probe does not work? Why do people come back to the deferred probe stuff? Because it is there today and generally works. One of the biggest problem of the deferred probe stuff is the problem how to identify real problems if everything ends up with a deferred probe when an error occurs? That means if you display an error whenever something is deferred, the log becomes almost unreadable. If you don't display an error, you never will see an error. And how do you display the real error when deferred probes finally do fail? The deferred probe stuff doesn't has any information about the underlying error, so it can't display it. This all sounds like "I don't like deferred probe because it is hard to debug" to me. This all sounds solvable with better instrumentation and debug capability. Why probe is deferred should be available at the source when deciding to return -EPROBE_DEFER. I still have not seen an example of A depends on B, deferred probe fails because of ? and here is the code for A that works around the problem. Anyway, this feature is totally independ of the deferred probe stuff and both can friendly live together. Yes, except then we get to maintain both. Goodbye and thanks for all the fish. Sorry, but my patience in dealing with Linux kernel maintainers was already almost zero before I've posted these patches and I have to realize that only fools still try to do so. Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/9] dt: dependencies (for deterministic driver initialization order based on the DT)
Am 14.05.2014 20:16, schrieb Alexander Holler: Am 14.05.2014 19:53, schrieb Alexander Holler: Am 14.05.2014 19:45, schrieb Alexander Holler: One of the biggest problem of the deferred probe stuff is the problem how to identify real problems if everything ends up with a deferred probe when an error occurs? That means if you display an error whenever something is deferred, the log becomes almost unreadable. If you don't display an error, you never will see an error. And how do you display the real error when deferred probes finally do fail? The deferred probe stuff doesn't has any information about the underlying error, so it can't display it. And that is a real problem. I've recently tried to identify why a driver failed and it was a nightmare because nothing offered any message (debug or not) when a probe was deferred. So I had to insert tons of printks to walk upwards to find the finally place where the probe failed. Everything afterwards just has forwarded the -EPROBE_DEFER without printing any message. To add some numbers, I had to insert around 20-30 printks in around 10 or more files to find the underlying problem. Having to do such whenever an error happens because everything assumes the error will disappear in a later try, which doesn't happen for real errors, is just a nightmare. And to give other people an idea how such a nightmare which has become reality does look like: You see a driver fails (through the deferred stuff). You look at that the driver and see around 5-10 places which return or forward an -EPROBE_DEFER. You add printks (to all or just some of them, hopefully the right ones, but Murphy ...). Then you go to the underlying functions. You see again several places which do the same, you add again printks. You go to the underlying functions ... Finally you've created a tree full with nodes of printks, searching for the one leaf which is the origin of the -EPROBE_DEFER for your problem. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/9] dt: dependencies (for deterministic driver initialization order based on the DT)
Am 14.05.2014 19:53, schrieb Alexander Holler: Am 14.05.2014 19:45, schrieb Alexander Holler: One of the biggest problem of the deferred probe stuff is the problem how to identify real problems if everything ends up with a deferred probe when an error occurs? That means if you display an error whenever something is deferred, the log becomes almost unreadable. If you don't display an error, you never will see an error. And how do you display the real error when deferred probes finally do fail? The deferred probe stuff doesn't has any information about the underlying error, so it can't display it. And that is a real problem. I've recently tried to identify why a driver failed and it was a nightmare because nothing offered any message (debug or not) when a probe was deferred. So I had to insert tons of printks to walk upwards to find the finally place where the probe failed. Everything afterwards just has forwarded the -EPROBE_DEFER without printing any message. To add some numbers, I had to insert around 20-30 printks in around 10 or more files to find the underlying problem. Having to do such whenever an error happens because everything assumes the error will disappear in a later try, which doesn't happen for real errors, is just a nightmare. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/9] dt: dependencies (for deterministic driver initialization order based on the DT)
Am 14.05.2014 19:45, schrieb Alexander Holler: One of the biggest problem of the deferred probe stuff is the problem how to identify real problems if everything ends up with a deferred probe when an error occurs? That means if you display an error whenever something is deferred, the log becomes almost unreadable. If you don't display an error, you never will see an error. And how do you display the real error when deferred probes finally do fail? The deferred probe stuff doesn't has any information about the underlying error, so it can't display it. And that is a real problem. I've recently tried to identify why a driver failed and it was a nightmare because nothing offered any message (debug or not) when a probe was deferred. So I had to insert tons of printks to walk upwards to find the finally place where the probe failed. Everything afterwards just has forwarded the -EPROBE_DEFER without printing any message. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/9] dt: dependencies (for deterministic driver initialization order based on the DT)
Am 14.05.2014 19:30, schrieb Rob Herring: On Wed, May 14, 2014 at 11:23 AM, Alexander Holler wrote: Am 14.05.2014 18:05, schrieb Grant Likely: On Wed, May 14, 2014 at 4:02 PM, Alexander Holler wrote: Am 14.05.2014 16:19, schrieb Grant Likely: Rather than a dtb schema change, for the most common properties (irqs, clocks, gpios), we could extract dependencies at boot time. I don't like the idea of adding a separate depends-on property because it is very easy to get it out of sync with the actual binding data (dtc is not the only tool that manipulates .dtbs. Firmware will fiddle with it too). Then that stuff has to fiddle correct. Sorry, but trying to solve all problems right from the beginning just leads to endless talks with no end and nothing will happen at all because nobody aggrees how to start. I appreciate the problem that you're trying to solve and why you're using the dtc approach. My job is to poke at the solution and make sure it is going to be reliable. Making sure all users know how to fiddle with the new property correctly is not a trivial problem, especially when it is firmware that will not necessarily be updated. The answer is just that they don't have to use this feature. It's not just about users, but maintainers have to carry the code and anything tied to DT is difficult to change or remove. Lots of inter-dependencies are already described in DT. We should leverage those first and then look at how to add dependencies that are not described. Again, that's what this feature is about. One of the problems it solves is that those dependencies which are described in the DT source in form of phandle reference, do disappear in the blobs because the init-system would have to know all bindings in order to identify phandle references (the dependencies) again. It is more meant as a long-term solution to fix for the problem of increasing hard-coded workarounds which all are trying to fix the initialization order of drivers. Hardware has become a lot more complicated than it was in the good old days, and I think the time is right trying to adopt the init-system to this new century instead of still adding workarounds here and there. I don't know when the good old days were, but this has been a problem in embedded systems for as long as I have worked on Linux. Yes, but stuff wasn't as complicated as today, which means it was relatively easy to manualy solve dependency problems. But if you look at complicated SOCs like the OMAP, it's much better to let the machine solve the dependencies to get the initialization order instead of still trying to do this manually. I'm not saying flat out 'no' here, but before I merge anything, I have to be reasonably certain that the feature is not going to represent a maintenance nightmare over the long term. The maintenance nightmare is already present in form of all the workarounds which are trying to fix the initialzation order necessary for modern hardware. Do you have concrete examples or cases where deferred probe does not work? Why do people come back to the deferred probe stuff? One of the biggest problem of the deferred probe stuff is the problem how to identify real problems if everything ends up with a deferred probe when an error occurs? That means if you display an error whenever something is deferred, the log becomes almost unreadable. If you don't display an error, you never will see an error. And how do you display the real error when deferred probes finally do fail? The deferred probe stuff doesn't has any information about the underlying error, so it can't display it. Anyway, this feature is totally independ of the deferred probe stuff and both can friendly live together. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/9] dt: deps: dependency based device creation
Am 14.05.2014 16:49, schrieb Alexander Holler: Am 14.05.2014 16:05, schrieb Grant Likely: On Mon, 12 May 2014 18:47:53 +0200, Alexander Holler wrote: Personally, I think the parts of this patch that manipulate the device registration order is entirely the wrong way to handle it. If anything, I would say continue to register the devices, even if the dependencies are unmet. (...) How does the dependency code decide which devices can be platform_devices? It's not clear to me from what I've read so far. Dependencies currently are only considered on stuff which has a "compatibility" property, thus drivers. I wanted to get the drivers loaded in order, not really caring for devices. Let me quote from (outdated) ldd3: "For the most part, the Linux device model code takes care of all these considerations without imposing itself upon driver authors. It sits mostly in the background; direct interaction with the device model is generally handled by bus-level logic and various other kernel subsystems. As a result, many driver authors can ignore the device model entirely, and trust it to take care of itself." So do I. ;) To explain a bit further, I've started with totally ignoring the device model just careing for the order in why drivers will be initialized. Than the device model did come into my way. ;) But it isn't any problem at all to extend the stuff to care for devices. That even would reduce some code in dtc with the disadvantage that the sizes of blobs will slightly increase a bit more, because they then would include dependencies to devices too (instead of just dependencies between drivers). So I'm absolutely open here. If using dependencies between devices is necessary or has advantages, that could be changed with changing a few lines. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/9] dt: dependencies (for deterministic driver initialization order based on the DT)
Am 14.05.2014 18:05, schrieb Grant Likely: On Wed, May 14, 2014 at 4:02 PM, Alexander Holler wrote: Am 14.05.2014 16:19, schrieb Grant Likely: Rather than a dtb schema change, for the most common properties (irqs, clocks, gpios), we could extract dependencies at boot time. I don't like the idea of adding a separate depends-on property because it is very easy to get it out of sync with the actual binding data (dtc is not the only tool that manipulates .dtbs. Firmware will fiddle with it too). Then that stuff has to fiddle correct. Sorry, but trying to solve all problems right from the beginning just leads to endless talks with no end and nothing will happen at all because nobody aggrees how to start. I appreciate the problem that you're trying to solve and why you're using the dtc approach. My job is to poke at the solution and make sure it is going to be reliable. Making sure all users know how to fiddle with the new property correctly is not a trivial problem, especially when it is firmware that will not necessarily be updated. The answer is just that they don't have to use this feature. It is more meant as a long-term solution to fix for the problem of increasing hard-coded workarounds which all are trying to fix the initialization order of drivers. Hardware has become a lot more complicated than it was in the good old days, and I think the time is right trying to adopt the init-system to this new century instead of still adding workarounds here and there. I'm not saying flat out 'no' here, but before I merge anything, I have to be reasonably certain that the feature is not going to represent a maintenance nightmare over the long term. The maintenance nightmare is already present in form of all the workarounds which are trying to fix the initialzation order necessary for modern hardware. Regards, Alexander -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/9] dt: deps: dependency based device creation
Am 14.05.2014 16:05, schrieb Grant Likely:
+
+ if (graph.finished)
+ return -EINVAL; /* cycle found */
+
+ /* of_init_print_order(); */
If you wrap of_init_print_order with a #ifdef DEBUG/#else/#endif, then
you don't need to comment out the call to of_init_print_order().
To explain why I didn't use DEBUG here: DEBUG enables a lot of
distracting messages. which is true for CONFIG_DEBUG_DRIVER too.
Therefor I often prefer to use just pr_info("AHO: ..."); with which I
print only stuff I want (and can easily grep for).
And as said, the patches I presented are meant for evaluation. I did
patches without discussing them before in order to avoid endless
discussion which likely never would have found an end and therfor would
have prevented a start. So now you already have some to play and test
with, without anyone had to discuss stuff before. ;)
Regards,
Alexander Holler
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/9] dt: dependencies (for deterministic driver initialization order based on the DT)
Am 14.05.2014 16:19, schrieb Grant Likely: Rather than a dtb schema change, for the most common properties (irqs, clocks, gpios), we could extract dependencies at boot time. I don't like the idea of adding a separate depends-on property because it is very easy to get it out of sync with the actual binding data (dtc is not the only tool that manipulates .dtbs. Firmware will fiddle with it too). Then that stuff has to fiddle correct. Sorry, but trying to solve all problems right from the beginning just leads to endless talks with no end and nothing will happen at all because nobody aggrees how to start. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 5/9] dt: deps: register drivers based on the initialization order based on DT
Am 14.05.2014 16:13, schrieb Grant Likely: On Mon, 12 May 2014 18:47:56 +0200, Alexander Holler wrote: The init system currently calls unknown functions with almost unknown functionality in an almost random order. Correct, we've got a module system. Some would say that is a strength! :-) That said, I don't object to optimizing to the optimal order when possible. Modules do work after init happened, that isn't what this feature is about. Fixing this is on a short-term basis is a bit tricky. In order to register drivers with a deterministic order, a list of all available in-kernel drivers is needed. Unfortunately such a list doesn't exist, just a list of initcalls does exist. The trick now is to first call special annotated initcalls (I call those "well done") for platform drivers, but not actualy starting those drivers by calling their probe function, but just collectiong their meta datas (struct platform_driver). After all those informations were collected, available the drivers will be started according to the previously determined order. Why does the initcall level matter? Why not just let the initcalls happen, capture the calls that register a driver, and then use that list later? Some initcalls assume that stuff is present when they called probe or register and do further action based on the rc code. The long range target to fix the problem should be to include a list (array) of struct platform_driver in the kernel for all in-kernel platform drivers, instead of just initcalls. This will be easy if all platform drivers have become "well done". How will that list be constructed? How will it account for multiple platforms, each requiring a different init order? The list could be build just like the list of initcalls, but containing structs platform instead of function pointers. The order is in now way part of this list, after all that's what this feature is about. The order is determined by metadatas in the DT, to get rid of a lot of otherwise necessary hardcoded stuff to fix the order in drivers. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/9] dt: deps: dependency based device creation
Am 14.05.2014 16:05, schrieb Grant Likely:
On Mon, 12 May 2014 18:47:53 +0200, Alexander Holler
wrote:
Use the properties named 'dependencies' in binary device tree blobs to build
a dependency based initialization order for platform devices and drivers.
This is done by building a directed acyclic graph using an adjacency list
and doing a topological sort to retrieve the order in which devices/drivers
should be created/initialized.
Signed-off-by: Alexander Holler
Hi Alexander,
Thanks for looking at this. It is a difficult problem. I've made
comments below, but first I've got some general comments...
First, I'm going to be very cautious about this. It is a complicated
piece of code making the device initialization process a lot more
complicated than it already is. I'm the first one to admit that deferred
probe handles the problem in quite a naive manner, but it is simple,
correct (when drivers support deferred probe) and easy to audit. This
series digs deep into the registration order of *both* devices and
drivers which gives me the heebee jeebees.
Sure, but the approach I present is deterministic. The deferred stuff,
while it's simple and works, is imho just a workaround. Besides that
this series isn't about pro or cons of the deferred probe, the deferred
probes I have seen where just the reason why I had a look at what
happens. To conclude, I like the deferred probe because it fixes
problems, but trying to do things right is much better. And there are
already many workarounds trying fix the initialization order (e.g.
drivers which classify themself as a subsys), so starting do it right
makes imho sense.
So, I'm sorry if you see this feature as an attack on the deferred probe
stuff, it isn't meant as such, no offense here or there.
Personally, I think the parts of this patch that manipulate the device
registration
order is entirely the wrong way to handle it. If anything, I would say
continue to register the devices, even if the dependencies are unmet.
That would just be a removal of 2 lines. I've no problem with that. ;)
Instead, I would focus on the driver core (drivers/base) to catch
device probe attempts when a known dependency is not met, remember it,
and perform the probe after the other driver shows up. That is also
going to be a complicated bit of code, but it works for every kind of
device, not just platform_devices, and has far less impact on the
platform setup code.
BTW, this has to be able to work at the level of struct device instead
of struct platform_device. There are far more kinds of devices than just
platform_device, and they all have the same problem.
Changing to care for devices instead of just drivers is easy to do.
Also, may I suggest that the more pieces that you can break this series
up into, the greater chance you'll have of getting a smaller subset
merged earlier if it can be proven to be useful on its own.
Hmm, I don't really care if that will be merged. I have no motivation to
fight with Linux kernel maintainers and I don't know if I will spend the
necessary time to do so.
+#ifdef CONFIG_OF_DEPENDENCIES
+ if (!of_init_build_order(NULL, NULL))
+ of_init_create_devices(NULL, NULL);
+ else
+ of_init_free_order();
What happens when of_init_build_order() fails? Does the whole system
fall over?
Yes. The only reason it can fail is when there is a cycle, and dtc
checks (and fails) for that when building the blob (dtb).
+#else
of_platform_populate(NULL, of_default_bus_match_table,
NULL, NULL);
#endif
+ }
+#endif
+
return 0;
}
arch_initcall(customize_machine);
@@ -914,7 +924,13 @@ void __init setup_arch(char **cmdline_p)
arm_pm_restart = mdesc->restart;
unflatten_device_tree();
-
+#ifdef CONFIG_OF_DEPENDENCIES
+ /*
+* No alloc used in of_init_build_order(), therefor it would work
+* already here too.
+*/
+ /* of_init_build_order(NULL, NULL); */
+#endif
Stale hunk left in patch?
See here: https://lkml.org/lkml/2014/5/14/102
This are NOT patches meant for final merging!
I would suggest splitting the core graph support into a separate patch
to keep things smaller and to keep the behaviour changes separate from
the support function additions.
Could be done.
+
+
+/* Copied from drivers/of/base.c (because it's lockless). */
Copying isn't a good idea. The function will need to be made accessible
to other files in the drivers/of directory.
See above.
+int __init of_init_build_order(struct device_node *root,
+ const struct of_device_id *matches)
+{
+ struct device_node *child;
+ int rc = 0;
+
+ root = root ? of_node_get(root) : of_find_node_by_path("/");
+ if (unlikely(!root)
dt: deps: some tips about how to debug/evaluate this feature
Hello,
to make it a bit more easier to evaluate or debug this new feature, here
are some tips:
- To see the initialization order you can use dtc:
CROSS_COMPILE=arm-linux-gnu- ARCH=arm make foo.dtb
scripts/dtc/dtc -I dtb -t arch/arm/boot/dts/foo.dtb
- To see that order in dmesg, I've left a commented function
in patch 2/9. Just enable of_init_print_order() in
drivers/of/of_dependencies.c
- To see which drivers do call of_platform_populate() theirself
(which is not necessary when using this new feature) uncomment the
WARN_ON(np->dev_created); in drivers/of/platform.c (patch 2/9).
- To see which drivers are already "well done" or not, add a small
debug line to of_init_register_platform_driver() in
drivers/of/of_dependencies.c:
@@ -416,39 +416,41 @@ int of_init_register_platform_driver(struct
platform_driver *drv)
{
BUG_ON(!is_recording);
order.platform_drivers[order.count_drivers++] = drv;
+ pr_info("DEPS: recording of drv %s\n", drv->driver.name);
return 0;
}
Now "well done" drivers linked to the kernel will be seen in dmesg as
beeing recorded. This also shows most drivers which are not "well
done", they will be started before the recording of drivers (except
late ones).
- To see when "well done" drivers will be registered (in order) add
something like that to drivers/of/of_dependencies.c:
@@ -445,8 +445,10 @@ void __init of_init_register_drivers(void)
if (of_driver_match_device(dev, &drv->driver)) {
if (!driver_find(drv->driver.name,
- drv->driver.bus))
+ drv->driver.bus)) {
platform_driver_register(drv);
+ pr_info("DEPS: driver %s
registered\n", drv->name);
+ }
if (dev->parent)
device_lock(dev->parent);
rc = device_attach(dev);
Regards,
Alexander Holler
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] deps: introduce new (virtual) property no-dependencies
Am 14.05.2014 03:55, schrieb Alexander Holler: Am 13.05.2014 20:48, schrieb Alexander Holler: The property 'no-dependencies' is virtual property and will not be added to any output file. Hmm, I think it should be non-virtual, otherwise a dtb isn't reproducible. That means dtc -> dtb -> dts (fdtdump) -> dtb would produce a different dtb if no-dependencies was used to avoid an automatic dependency. But a dtb doesn't have phandles. After all that's why I had to introduce the property dependencies. So no-dependencies doesn't make sense in a .dtb, the virtual nature of no-dependencies makes sense and the patch is fine. Looks like I had a weak moment yesterday evening. ;) Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] deps: introduce new (virtual) property no-dependencies
Am 13.05.2014 20:48, schrieb Alexander Holler: > > The property 'no-dependencies' is virtual property and will not be added > to any output file. > Hmm, I think it should be non-virtual, otherwise a dtb isn't reproducible. That means dtc -> dtb -> dts (fdtdump) -> dtb would produce a different dtb if no-dependencies was used to avoid an automatic dependency. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 11/9] dt: deps: dtc: introduce new (virtual) property no-dependencies
In some rare cases it might make sense for not wanting an automatic
dependency for some used phandles.
E.g. if a cpu depends on a regulator which depends on i2c which depends
on some bus and you want that the bus depends on the cpu. That would
end up in a cycle. Usually such doesn't make sense because the hw doesn't
support such circular dependencies too (you always have to start somewhere
with initializing things). But e.g. in the case of the regulator the cpu
depends on, it's very likely that the regulator was initialized
automatically (otherwise the cpu won't run), so there is no real need to
initialize the regulator before the cpu. But that's just an example for
one of such rare cases where it might make sense to avoid an otherwise
automatically added dependency.
The syntax is like
bar: whatever {
...
};
foo {
my-option = <&bar 1 2 3>;
no-dependencies = <&bar>;
};
Without that 'no-dependencies' property dtc would automatically add a
dependency to bar to the property 'dependencies' of the node foo.
But with that 'no-dependencies' it will not automatically add the
listed dependencies.
The property 'no-dependencies' is virtual property and will not be added
to any output file.
Signed-off-by: Alexander Holler
---
scripts/dtc/dependencies.c | 22 ++
1 file changed, 22 insertions(+)
diff --git a/scripts/dtc/dependencies.c b/scripts/dtc/dependencies.c
index 4579f6f..06f447b 100644
--- a/scripts/dtc/dependencies.c
+++ b/scripts/dtc/dependencies.c
@@ -73,6 +73,16 @@ static void add_deps(struct node *dt, struct node *node,
struct property *prop)
is_parent_of(target, source))
continue;
phandle = get_node_phandle(dt, target);
+ prop_deps = get_property(node, "no-dependencies");
+ if (prop_deps) {
+ /* avoid adding non-dependencies */
+ cell = (cell_t *)prop_deps->val.val;
+ for (i = 0; i < prop_deps->val.len/4; ++i)
+ if (*cell++ == cpu_to_fdt32(phandle))
+ break;
+ if (i < prop_deps->val.len/4)
+ continue;
+ }
prop_deps = get_property(source, "dependencies");
if (!prop_deps) {
add_property(source,
@@ -102,9 +112,21 @@ static void process_nodes_props(struct node *dt, struct
node *node)
process_nodes_props(dt, child);
}
+static void del_prop_no_dependencies(struct node *node)
+{
+ struct node *child;
+
+ if (!node)
+ return;
+ delete_property_by_name(node, "no-dependencies");
+ for_each_child(node, child)
+ del_prop_no_dependencies(child);
+}
+
void add_dependencies(struct boot_info *bi)
{
process_nodes_props(bi->dt, bi->dt);
+ del_prop_no_dependencies(bi->dt);
}
/*
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] deps: introduce new (virtual) property no-dependencies
In some rare cases it might make sense for not wanting an automatic
dependency for some used phandles.
E.g. if a cpu depends on a regulator which depends on i2c which depends
on some bus and you want that the bus depends on the cpu. That would
end up in a cycle. Usually such doesn't make sense because the hw doesn't
support such circular dependencies too (you always have to start somewhere
with initializing things). But e.g. in the case of the regulator the cpu
depends on, it's very likely that the regulator was initialized
automatically (otherwise the cpu won't run), so there is no real need to
initialize the regulator before the cpu. But that's just an example for
one of such rare cases where it might make sense to avoid an otherwise
automatically added dependency.
The syntax is like
bar: whatever {
...
};
foo {
my-option = <&bar 1 2 3>;
no-dependencies = <&bar>;
};
Without that 'no-dependencies' property dtc would automatically add a
dependency to bar to the property 'dependencies' of the node foo.
But with that 'no-dependencies' it will not automatically add the
listed dependencies.
The property 'no-dependencies' is virtual property and will not be added
to any output file.
Signed-off-by: Alexander Holler
---
dependencies.c | 22 ++
1 file changed, 22 insertions(+)
diff --git a/dependencies.c b/dependencies.c
index 4579f6f..06f447b 100644
--- a/dependencies.c
+++ b/dependencies.c
@@ -73,6 +73,16 @@ static void add_deps(struct node *dt, struct node *node,
struct property *prop)
is_parent_of(target, source))
continue;
phandle = get_node_phandle(dt, target);
+ prop_deps = get_property(node, "no-dependencies");
+ if (prop_deps) {
+ /* avoid adding non-dependencies */
+ cell = (cell_t *)prop_deps->val.val;
+ for (i = 0; i < prop_deps->val.len/4; ++i)
+ if (*cell++ == cpu_to_fdt32(phandle))
+ break;
+ if (i < prop_deps->val.len/4)
+ continue;
+ }
prop_deps = get_property(source, "dependencies");
if (!prop_deps) {
add_property(source,
@@ -102,9 +112,21 @@ static void process_nodes_props(struct node *dt, struct
node *node)
process_nodes_props(dt, child);
}
+static void del_prop_no_dependencies(struct node *node)
+{
+ struct node *child;
+
+ if (!node)
+ return;
+ delete_property_by_name(node, "no-dependencies");
+ for_each_child(node, child)
+ del_prop_no_dependencies(child);
+}
+
void add_dependencies(struct boot_info *bi)
{
process_nodes_props(bi->dt, bi->dt);
+ del_prop_no_dependencies(bi->dt);
}
/*
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 10/9] dt: deps: fix bug not registering late drivers when OF_DEPENDENCIES is disabled
The subject says all. Patch 5/9 has a bug which avoids registering late drivers if OF_DEPENDENCIES is disabled. This also explains the large differences in boot times I've experienced when comparing boot times with and without DT dependency based initialization order. Signed-off-by: Alexander Holler --- init/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/init/main.c b/init/main.c index 7591cd1..e16e2b4 100644 --- a/init/main.c +++ b/init/main.c @@ -799,9 +799,9 @@ static void __init do_basic_setup(void) of_init_set_recording(false); /* probe available platform drivers with deterministic order */ of_init_register_drivers(); +#endif /* register late drivers */ do_initcall_level(ARRAY_SIZE(initcall_levels) - 3); -#endif random_int_secret_init(); } -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] deps: Add option to print initialization order
Add option -t to print the default initialization order.
No other output will be generated.
To print the order, just use something like this:
CROSS_COMPILE=gcc-foo ARCH=arm make foo.dtb
scripts/dtc/dtc -I dtb -t arch/arm/boot/dts/foo.dtb
Since it's now possible to check to for cycles in the dependency graph,
this is now done too.
Signed-off-by: Alexander Holler
---
dependencies.c | 346 +
dtc.c | 24 +++-
dtc.h | 2 +
3 files changed, 371 insertions(+), 1 deletion(-)
diff --git a/dependencies.c b/dependencies.c
index dd4658c..8fe1a8c 100644
--- a/dependencies.c
+++ b/dependencies.c
@@ -106,3 +106,349 @@ void add_dependencies(struct boot_info *bi)
{
process_nodes_props(bi->dt, bi->dt);
}
+
+/*
+ * The code below is in large parts a copy of drivers/of/of_dependencies.c
+ * in the Linux kernel. So both files do share the same bugs.
+ * The next few ugly defines do exist to keep the differences at a minimum.
+ */
+static struct node *tree;
+#define pr_cont(format, ...) printf(format, ##__VA_ARGS__)
+#define pr_info(format, ...) printf(format, ##__VA_ARGS__)
+#define pr_warn(format, ...) printf(format, ##__VA_ARGS__)
+#define pr_err(format, ...) fprintf(stderr, format, ##__VA_ARGS__)
+typedef cell_t __be32;
+#define device_node node
+#define full_name fullpath
+#define __initdata
+#define __init
+#define unlikely(a) (a)
+#define of_node_put(a)
+#define of_find_node_by_phandle(v) get_node_by_phandle(tree, v)
+#define __of_get_property(a, b, c) get_property(a, b)
+#define for_each_child_of_node(a, b) for_each_child(a, b)
+
+
+#define MAX_DT_NODES 1000 /* maximum number of vertices */
+#define MAX_EDGES (MAX_DT_NODES*2) /* maximum number of edges (dependencies) */
+
+struct edgenode {
+ uint32_t y; /* phandle */
+ struct edgenode *next; /* next edge in list */
+};
+
+/*
+ * Vertex numbers do correspond to phandle numbers. That means the graph
+ * does contain as much vertices as the maximum of all phandles.
+ * Or in other words, we assume that for all phandles in the device tree
+ * 0 < phandle < MAX_DT_NODES+1 is true.
+ */
+struct dep_graph {
+ struct edgenode edge_slots[MAX_EDGES]; /* used to avoid kmalloc */
+ struct edgenode *edges[MAX_DT_NODES+1]; /* adjacency info */
+ unsigned nvertices; /* number of vertices in graph */
+ unsigned nedges; /* number of edges in graph */
+ bool processed[MAX_DT_NODES+1]; /* which vertices have been processed */
+ bool include_node[MAX_DT_NODES+1]; /* which nodes to consider */
+ bool discovered[MAX_DT_NODES+1]; /* which vertices have been found */
+ bool finished; /* if true, cut off search immediately */
+};
+static struct dep_graph graph __initdata;
+
+struct init_order {
+ uint32_t max_phandle; /* the max used phandle */
+ uint32_t old_max_phandle; /* used to keep track of added phandles */
+ struct device_node *order[MAX_DT_NODES+1];
+ unsigned count;
+ /* Used to keep track of parent devices in regard to the DT */
+ uint32_t parent_by_phandle[MAX_DT_NODES+1];
+ struct device *device_by_phandle[MAX_DT_NODES+1];
+};
+static struct init_order order __initdata;
+
+
+/* Copied from drivers/of/base.c (because it's lockless). */
+static int __init __of_device_is_available(struct device_node *device)
+{
+ struct property *status;
+
+ if (!device)
+ return 0;
+
+ status = get_property(device, "status");
+ if (status == NULL)
+ return 1;
+
+ if (status->val.len > 0) {
+ if (!strcmp(status->val.val, "okay") ||
+ !strcmp(status->val.val, "ok"))
+ return 1;
+ }
+
+ return 0;
+}
+
+/*
+ * x is a dependant of y or in other words
+ * y will be initialized before x.
+ */
+static int __init insert_edge(uint32_t x, uint32_t y)
+{
+ struct edgenode *p; /* temporary pointer */
+
+ if (unlikely(x > MAX_DT_NODES || y > MAX_DT_NODES)) {
+ pr_err("Node found with phandle 0x%x > MAX_DT_NODES (%d)!\n",
+ x > MAX_DT_NODES ? x : y, MAX_DT_NODES);
+ return -EINVAL;
+ }
+ if (unlikely(!x || !y))
+ return 0;
+ if (unlikely(graph.nedges >= MAX_EDGES)) {
+ pr_err("Maximum number of edges (%d) reached!\n", MAX_EDGES);
+ return -EINVAL;
+ }
+ p = &graph.edge_slots[graph.nedges++];
+ graph.include_node[x] = 1;
+ graph.include_node[y] = 1;
+ p->y = y;
+ p->next = graph.edges[x];
+ graph.edges[x] = p; /* insert at head of list */
+
+ graph.nvertices = (x > graph.nvertices) ? x : graph.nvertices;
+ graph.nvertices = (y > graph.nvertices) ? y : graph.nvertices;
+
[PATCH 3/3] deps: Add option to print dependency graph as dot (Graphviz)
Add option -T do print a dependency graph in dot format for
generating a picture with Graphviz.
E.g.
dtc -T foo.dts | dot -T svg -o foo.svg
would generate the picture foo.png with the dependency graph.
Convential dependencies (those based on the tree structure) are having
black arrows, dependencies based on the property 'dependencies' are
having cyan arrows.
Option -D to not automatically add dependencies does still work, so
you could build a classic dependency graph with
dtc -D -T foo.dts | dot -T png -o foo_no_auto_deps.png
This works with binary blobs as input too. E.g.
CROSS_COMPILE=gcc-foo ARCH=arm make foo.dtb
scripts/dtc/dtc -I dtb -T arch/arm/boot/dts/foo.dtb
would print the dot file.
Signed-off-by: Alexander Holler
---
dependencies.c | 48 ++--
dtc.c | 19 ---
dtc.h | 2 +-
3 files changed, 59 insertions(+), 10 deletions(-)
diff --git a/dependencies.c b/dependencies.c
index 8fe1a8c..4579f6f 100644
--- a/dependencies.c
+++ b/dependencies.c
@@ -298,7 +298,7 @@ static void __init topological_sort(void)
depth_first_search(i);
}
-static int __init add_dep_list(struct device_node *node)
+static int __init add_dep_list(struct device_node *node, bool print_dot)
{
const __be32 *list, *list_end;
uint32_t ph;
@@ -328,6 +328,9 @@ static int __init add_dep_list(struct device_node *node)
rc = insert_edge(node->phandle, ph);
if (rc)
break;
+ if (print_dot)
+ printf(" node0x%x -> node0x%x [color=cyan]\n",
+ node->phandle, ph);
}
return rc;
@@ -352,9 +355,10 @@ static const char *of_prop_next_string(struct property
*prop, const char *cur)
}
static int __init add_deps_lnx(struct device_node *parent,
- struct device_node *node)
+ struct device_node *node, bool print_dot)
{
struct device_node *child;
+ const char *cp;
int rc = 0;
if (!__of_device_is_available(node))
@@ -375,13 +379,33 @@ static int __init add_deps_lnx(struct device_node *parent,
return -EINVAL;
}
order.parent_by_phandle[node->phandle] = parent->phandle;
- rc = add_dep_list(node);
+ if (print_dot) {
+ struct property *prop;
+ printf("node0x%x [label=\"0x%x %s", node->phandle,
+ node->phandle, node->name);
+ if (node->full_name)
+ printf(" (%s)", node->full_name);
+ prop = get_property(node, "compatible");
+ if (prop) {
+ printf("\\n");
+ for (cp = of_prop_next_string(prop, NULL); cp;
+cp = of_prop_next_string(prop, cp)) {
+ if (cp != prop->val.val)
+ putchar(' ');
+ printf("%s", cp);
+ }
+ }
+ printf("\"];\n");
+ printf(" node0x%x -> node0x%x\n", node->phandle,
+ parent->phandle);
+ }
+ rc = add_dep_list(node, print_dot);
if (unlikely(rc))
return rc;
parent = node; /* change the parent only if node is a driver */
}
for_each_child_of_node(node, child) {
- rc = add_deps_lnx(parent, child);
+ rc = add_deps_lnx(parent, child, print_dot);
if (unlikely(rc))
break;
}
@@ -426,7 +450,7 @@ void __init of_init_print_order(const char *name)
}
}
-int __init of_init_build_order(struct device_node *root)
+int __init of_init_build_order(struct device_node *root, const char *print_dot)
{
struct device_node *child;
int rc = 0;
@@ -438,12 +462,24 @@ int __init of_init_build_order(struct device_node *root)
calc_max_phandle(root);
order.old_max_phandle = order.max_phandle;
+ if (print_dot) {
+ printf("digraph G {\n");
+ printf("node0x%x [label=\"0x%x root (/)\"];\n",
+ order.max_phandle+1 , order.max_phandle+1);
+ }
+
for_each_child_of_node(root, child) {
- rc = add_deps_lnx(root, child);
+ rc = add_deps_lnx(root, child, print_dot)
[PATCH 1/3] deps: Automatically add new property 'dependencies' which contains a list of referenced phandles
During the step from .dts to .dtb the information about dependcies contained
in the .dts through phandle references is lost. This makes it impossible to
use the binary blob to create a dependency graph without knowing the semantic
of all cell arrays.
Therefor automatically add a new property called 'dependencies' to all nodes
which have phandle references in one of their properties.
This new property will contain an array of phandles with one value for every
phandle referenced by other properties in the node.
If such a property already exists (e.g. to manually add dependencies through
the .dts), the existing list will be expanded.
Added phandles will be the phandle of either the referenced node itself (if
it has a property named 'compatible', or of the next parent of the referenced
node which as property named 'compatible'. This ensures only dependencies to
drivers will be added.
References to phandles of parent or child nodes will not be added to this
property, because this information is already contained in the blob (in the
form of the tree itself).
No dependencies to disabled nodes will be added.
Signed-off-by: Alexander Holler
---
Makefile.dtc | 1 +
dependencies.c | 108 +
dtc.c | 12 ++-
dtc.h | 3 ++
4 files changed, 123 insertions(+), 1 deletion(-)
create mode 100644 dependencies.c
diff --git a/Makefile.dtc b/Makefile.dtc
index bece49b..5fb5343 100644
--- a/Makefile.dtc
+++ b/Makefile.dtc
@@ -6,6 +6,7 @@
DTC_SRCS = \
checks.c \
data.c \
+ dependencies.c \
dtc.c \
flattree.c \
fstree.c \
diff --git a/dependencies.c b/dependencies.c
new file mode 100644
index 000..dd4658c
--- /dev/null
+++ b/dependencies.c
@@ -0,0 +1,108 @@
+/*
+ * Code to add a property which contains dependencies (used phandle references)
+ * to all (driver) nodes which are having phandle references.
+ *
+ * Copyright (C) 2014 Alexander Holler
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include
+
+/* Searches upwards for a node with a property 'compatible' */
+static struct node *find_compatible_not_disabled(struct node *node)
+{
+ struct property *prop;
+
+ while (node) {
+ prop = get_property(node, "compatible");
+ if (prop) {
+ prop = get_property(node, "status");
+ if (prop)
+ if (!prop->val.len ||
+ (strcmp(prop->val.val, "okay") &&
+ strcmp(prop->val.val, "ok")))
+ return NULL; /* disabled */
+ return node;
+ }
+ node = node->parent;
+ }
+ return NULL;
+}
+
+static bool is_parent_of(struct node *node1, struct node *node2)
+{
+ while (node2) {
+ if (node2->parent == node1)
+ return true;
+ node2 = node2->parent;
+ }
+ return false;
+
+}
+
+static void add_deps(struct node *dt, struct node *node, struct property *prop)
+{
+ struct marker *m = prop->val.markers;
+ struct node *refnode;
+ cell_t phandle;
+ struct property *prop_deps;
+ unsigned i;
+ cell_t *cell;
+ struct node *source;
+ struct node *target;
+
+ for_each_marker_of_type(m, REF_PHANDLE) {
+ assert(m->offset + sizeof(cell_t) <= prop->val.len);
+
+ refnode = get_node_by_ref(dt, m->ref);
+ if (!refnode) {
+ fprintf(stderr,
+ "ERROR: Reference to non-existent node or label
\"%s\"\n",
+ m->ref);
+ continue;
+ }
+
+ source = find_compatible_not_disabled(node);
+ target = find_compatible_not_disabled(refnode);
+ if (!source || !target || source == target ||
+ is_parent_of(source, target) ||
+ is_parent_of(target, source))
+ continue;
+ phandle = get_node_phandle(dt, target);
+ prop_deps = get_property(source, "dependencies");
+ if (!prop_deps) {
+ add_property(source,
+build_property("dependencies",
+ data_append_cell(empty_data, phandle)));
+ continue;
+ }
+
[PATCH 0/3] add dependencies
Here are the 3 patches for dtc I've posted in the previous series for the kernel ready made for the DTC repository. They are identical except the different dtc.c (help format). I've also changed the topic line from dt: deps: dtc: to just deps: Please keep in mind that adding dependencies will be enabled by default. That means .dtb sizes will increase slightly after the first patch is applied. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 3/9] dt: deps: dtc: Add option to print initialization order
Am 12.05.2014 22:38, schrieb Jon Loeliger: > So, this should be rebased on the actual DTC repository, > and not the kernel's copy of the DTC under scripts. I have patches for the standalone dtc too. I just didn't want to poste almost the same patches twice and wanted to wait for feedback if people do like this stuff at all. Do you already want those 3 patches for the standalone dtc? Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 7/9] dt: deps: kirkwood: make it possible to use CONFIG_OF_DEPENDENCIES
Use the feature of dependency based initialization order for drivers
if CONFIG_OF_DEPENDENCIES is enabled.
Signed-off-by: Alexander Holler
---
arch/arm/mach-kirkwood/board-dt.c | 8
1 file changed, 8 insertions(+)
diff --git a/arch/arm/mach-kirkwood/board-dt.c
b/arch/arm/mach-kirkwood/board-dt.c
index 7818815..5c352b7 100644
--- a/arch/arm/mach-kirkwood/board-dt.c
+++ b/arch/arm/mach-kirkwood/board-dt.c
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -133,7 +134,14 @@ static void __init kirkwood_dt_init(void)
if (of_machine_is_compatible("marvell,mv88f6281gtw-ge"))
mv88f6281gtw_ge_init();
+#ifdef CONFIG_OF_DEPENDENCIES
+ if (!of_init_build_order(NULL, NULL))
+ of_init_create_devices(NULL, NULL);
+ else
+ of_init_free_order();
+#else
of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
+#endif
}
static const char * const kirkwood_dt_board_compat[] = {
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 8/9] dt: deps: dts: kirkwood: dockstar: add dependency ehci -> usb power regulator
This serves as an example how easy it is to fix an initialization order
if the order depends on the DT. No source code changes will be necessary.
If you look at the dependency graph for the dockstar, you will see that
there is no dependency between ehci and the usb power regulator. This
ends up with the fact that the regulator will be initialized after ehci.
Fix this by adding one dependency to the .dts.
Signed-off-by: Alexander Holler
---
arch/arm/boot/dts/kirkwood-dockstar.dts | 4
arch/arm/boot/dts/kirkwood.dtsi | 2 +-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/kirkwood-dockstar.dts
b/arch/arm/boot/dts/kirkwood-dockstar.dts
index f31312e..0ca7456 100644
--- a/arch/arm/boot/dts/kirkwood-dockstar.dts
+++ b/arch/arm/boot/dts/kirkwood-dockstar.dts
@@ -106,3 +106,7 @@
phy-handle = <ðphy0>;
};
};
+
+&ehci {
+ dependencies = <&usb_power>;
+};
diff --git a/arch/arm/boot/dts/kirkwood.dtsi b/arch/arm/boot/dts/kirkwood.dtsi
index 6abf44d..54a1223 100644
--- a/arch/arm/boot/dts/kirkwood.dtsi
+++ b/arch/arm/boot/dts/kirkwood.dtsi
@@ -185,7 +185,7 @@
status = "okay";
};
- ehci@5 {
+ ehci: ehci@5 {
compatible = "marvell,orion-ehci";
reg = <0x5 0x1000>;
interrupts = <19>;
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 9/9] dt: deps: omap2: make it possible to use CONFIG_OF_DEPENDENCIES
Use the feature of dependency based initialization order for drivers
if CONFIG_OF_DEPENDENCIES is enabled.
This patch also includes a change for a driver which does call
of_platform_populate() itself. This shouldn't be done or necessary
when CONFIG_OF_DEPENDENCIES is enabled.
Signed-off-by: Alexander Holler
---
arch/arm/mach-omap2/pdata-quirks.c | 8
drivers/pwm/pwm-tipwmss.c | 5 +
2 files changed, 13 insertions(+)
diff --git a/arch/arm/mach-omap2/pdata-quirks.c
b/arch/arm/mach-omap2/pdata-quirks.c
index c33e07e..80becdb 100644
--- a/arch/arm/mach-omap2/pdata-quirks.c
+++ b/arch/arm/mach-omap2/pdata-quirks.c
@@ -13,6 +13,7 @@
#include
#include
#include
+#include
#include
#include
@@ -312,7 +313,14 @@ void __init pdata_quirks_init(struct of_device_id
*omap_dt_match_table)
{
omap_sdrc_init(NULL, NULL);
pdata_quirks_check(auxdata_quirks);
+#ifdef CONFIG_OF_DEPENDENCIES
+ if (!of_init_build_order(NULL, NULL))
+ of_init_create_devices(NULL, omap_auxdata_lookup);
+ else
+ of_init_free_order();
+#else
of_platform_populate(NULL, omap_dt_match_table,
omap_auxdata_lookup, NULL);
+#endif
pdata_quirks_check(pdata_quirks);
}
diff --git a/drivers/pwm/pwm-tipwmss.c b/drivers/pwm/pwm-tipwmss.c
index 3b119bc..786fa39 100644
--- a/drivers/pwm/pwm-tipwmss.c
+++ b/drivers/pwm/pwm-tipwmss.c
@@ -78,10 +78,15 @@ static int pwmss_probe(struct platform_device *pdev)
pm_runtime_get_sync(&pdev->dev);
platform_set_drvdata(pdev, info);
+#ifndef CONFIG_OF_DEPENDENCIES
/* Populate all the child nodes here... */
ret = of_platform_populate(node, NULL, NULL, &pdev->dev);
if (ret)
dev_err(&pdev->dev, "no child node found\n");
+#else
+ /* dependency based initialization already has setup devices */
+ ret = 0;
+#endif
return ret;
}
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 6/9] dt: deps: WIP: well done drivers
This patch contains the necessary changes for some drivers which are used
by the boards I have. The list isn't complete (therefor the WIP) and is
meant as an example. If considered to be mainlined, I assume these changes
should end up in one patch for every changed driver.
Signed-off-by: Alexander Holler
---
drivers/dma/mv_xor.c | 2 +-
drivers/dma/omap-dma.c | 2 +-
drivers/gpio/gpio-mvebu.c | 2 +-
drivers/gpio/gpio-twl4030.c| 2 +-
drivers/i2c/busses/i2c-omap.c | 2 +-
drivers/iommu/omap-iommu.c | 2 +-
drivers/mailbox/mailbox-omap2.c| 2 +-
drivers/net/ethernet/marvell/mv643xx_eth.c | 2 +-
drivers/regulator/fixed.c | 2 +-
drivers/regulator/twl-regulator.c | 2 +-
drivers/usb/host/ehci-omap.c | 2 +-
drivers/usb/host/ehci-orion.c | 2 +-
drivers/usb/host/ohci-omap3.c | 2 +-
drivers/usb/phy/phy-generic.c | 2 +-
14 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/dma/mv_xor.c b/drivers/dma/mv_xor.c
index 766b68e..7f1091a 100644
--- a/drivers/dma/mv_xor.c
+++ b/drivers/dma/mv_xor.c
@@ -1316,7 +1316,7 @@ static int __init mv_xor_init(void)
{
return platform_driver_register(&mv_xor_driver);
}
-module_init(mv_xor_init);
+well_done_platform_module_init(mv_xor_init);
/* it's currently unsafe to unload this module */
#if 0
diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
index 362e7c4..a523025 100644
--- a/drivers/dma/omap-dma.c
+++ b/drivers/dma/omap-dma.c
@@ -707,7 +707,7 @@ static int omap_dma_init(void)
{
return platform_driver_register(&omap_dma_driver);
}
-subsys_initcall(omap_dma_init);
+well_done_platform_initcall(subsys, omap_dma_init);
static void __exit omap_dma_exit(void)
{
diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index 3b1fd1c..c151f6e 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -736,4 +736,4 @@ static int __init mvebu_gpio_init(void)
{
return platform_driver_register(&mvebu_gpio_driver);
}
-postcore_initcall(mvebu_gpio_init);
+well_done_platform_initcall(postcore, mvebu_gpio_init);
diff --git a/drivers/gpio/gpio-twl4030.c b/drivers/gpio/gpio-twl4030.c
index 8b88ca2..6c18f4a 100644
--- a/drivers/gpio/gpio-twl4030.c
+++ b/drivers/gpio/gpio-twl4030.c
@@ -618,7 +618,7 @@ static int __init gpio_twl4030_init(void)
{
return platform_driver_register(&gpio_twl4030_driver);
}
-subsys_initcall(gpio_twl4030_init);
+well_done_platform_initcall(subsys, gpio_twl4030_init);
static void __exit gpio_twl4030_exit(void)
{
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 90dcc2e..4df05c0 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -1352,7 +1352,7 @@ omap_i2c_init_driver(void)
{
return platform_driver_register(&omap_i2c_driver);
}
-subsys_initcall(omap_i2c_init_driver);
+well_done_platform_initcall(subsys, omap_i2c_init_driver);
static void __exit omap_i2c_exit_driver(void)
{
diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index bcd78a7..c121708 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -1282,7 +1282,7 @@ static int __init omap_iommu_init(void)
return platform_driver_register(&omap_iommu_driver);
}
/* must be ready before omap3isp is probed */
-subsys_initcall(omap_iommu_init);
+well_done_platform_initcall(subsys, omap_iommu_init);
static void __exit omap_iommu_exit(void)
{
diff --git a/drivers/mailbox/mailbox-omap2.c b/drivers/mailbox/mailbox-omap2.c
index 42d2b89..919da67 100644
--- a/drivers/mailbox/mailbox-omap2.c
+++ b/drivers/mailbox/mailbox-omap2.c
@@ -347,7 +347,7 @@ static void __exit omap2_mbox_exit(void)
platform_driver_unregister(&omap2_mbox_driver);
}
-module_init(omap2_mbox_init);
+well_done_platform_module_init(omap2_mbox_init);
module_exit(omap2_mbox_exit);
MODULE_LICENSE("GPL v2");
diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c
b/drivers/net/ethernet/marvell/mv643xx_eth.c
index a2565ce..2fcd832 100644
--- a/drivers/net/ethernet/marvell/mv643xx_eth.c
+++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
@@ -3011,7 +3011,7 @@ static int __init mv643xx_eth_init_module(void)
return rc;
}
-module_init(mv643xx_eth_init_module);
+well_done_platform_module_init(mv643xx_eth_init_module);
static void __exit mv643xx_eth_cleanup_module(void)
{
diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index 5ea64b9..3a71016 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -241,7 +241,7 @@ static int __init regulator_fixed_voltage_init(void)
{
return platform_driver_register(®ulator_fixed_voltage_driver);
}
-subsys_initcall(regulator_fixed_voltage_init);
+well_done_platform_initcall(subsys, regu
[RFC PATCH 5/9] dt: deps: register drivers based on the initialization order based on DT
The init system currently calls unknown functions with almost unknown
functionality in an almost random order.
Fixing this is on a short-term basis is a bit tricky.
In order to register drivers with a deterministic order, a list of all
available in-kernel drivers is needed. Unfortunately such a list doesn't
exist, just a list of initcalls does exist.
The trick now is to first call special annotated initcalls (I call those
"well done") for platform drivers, but not actualy starting those drivers
by calling their probe function, but just collectiong their meta datas
(struct platform_driver). After all those informations were collected,
available the drivers will be started according to the previously
determined order.
The annotation of such platform drivers is necessary because it must be
made sure that those drivers don't care if the probe is actually called in
their initcall or later.
That means that all platform drivers which already do use
module_platform_driver() or
module_platform_driver_probe()
don't need any modification because their initcall is known and already well
done. But all platform drivers which do use
module_init() or
*_initcall()
have to be reviewed if they are "well done". If they are, they need a change
like
-module_init(foo_init);
+well_done_platform_module_init(foo_init);
or
-subsys_initcall(foo_init);
+well_done_platform_initcall(subsys, foo_init);
to become included in the deterministic order in which platform drivers
will be initialized.
All other platform drivers will still be initialized in random order before
platform drivers included in the deterministic order will be initialized.
"Well done" drivers which don't appear in the order (because they don't appear
in the DT) will be initialized after those which do appear in the order.
If CONFIG_OF_DEPENDENCIES is disabled, nothing is changed at all.
The long range target to fix the problem should be to include a list (array)
of struct platform_driver in the kernel for all in-kernel platform drivers,
instead of just initcalls. This will be easy if all platform drivers have
become "well done".
Unfortunately there are some drivers which will need quiet some changes
to become "well done". As an example for such an initcall look e.g. at
drivers/tty/serial/8250/8250_core.c.
Signed-off-by: Alexander Holler
---
drivers/base/platform.c | 13 +++
drivers/of/of_dependencies.c | 79 +++
include/asm-generic/vmlinux.lds.h | 1 +
include/linux/init.h | 19 ++
include/linux/of_dependencies.h | 5 +++
include/linux/platform_device.h | 16 ++--
init/main.c | 17 -
7 files changed, 145 insertions(+), 5 deletions(-)
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index bc78848..b9c9b33 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -13,6 +13,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -541,6 +542,12 @@ int __platform_driver_register(struct platform_driver *drv,
if (drv->shutdown)
drv->driver.shutdown = platform_drv_shutdown;
+#ifdef CONFIG_OF_DEPENDENCIES
+ if (of_init_is_recording())
+ /* Just record the driver */
+ return of_init_register_platform_driver(drv);
+ else
+#endif
return driver_register(&drv->driver);
}
EXPORT_SYMBOL_GPL(__platform_driver_register);
@@ -590,8 +597,14 @@ int __init_or_module platform_driver_probe(struct
platform_driver *drv,
/* temporary section violation during probe() */
drv->probe = probe;
+
retval = code = platform_driver_register(drv);
+#ifdef CONFIG_OF_DEPENDENCIES
+ if (of_init_is_recording())
+ /* Just record the driver */
+ return retval;
+#endif
/*
* Fixup that section violation, being paranoid about code scanning
* the list of drivers in order to probe new devices. Check to see
diff --git a/drivers/of/of_dependencies.c b/drivers/of/of_dependencies.c
index 7905172..4af62d5 100644
--- a/drivers/of/of_dependencies.c
+++ b/drivers/of/of_dependencies.c
@@ -46,9 +46,12 @@ struct init_order {
/* Used to keep track of parent devices in regard to the DT */
uint32_t parent_by_phandle[MAX_DT_NODES+1];
struct device *device_by_phandle[MAX_DT_NODES+1];
+ struct platform_driver *platform_drivers[MAX_DT_NODES+1];
+ unsigned count_drivers;
};
static struct init_order order __initdata;
+static bool is_recording;
/* Copied from drivers/of/base.c (because it's lockless). */
static struct property * __init __of_find_property(const struct device_node
*np,
@@ -401,3 +404,79 @@ void __init of_init_free_order(void)
order.count = 0;
/* remove_ne
[RFC PATCH 2/9] dt: deps: dependency based device creation
Use the properties named 'dependencies' in binary device tree blobs to build
a dependency based initialization order for platform devices and drivers.
This is done by building a directed acyclic graph using an adjacency list
and doing a topological sort to retrieve the order in which devices/drivers
should be created/initialized.
Signed-off-by: Alexander Holler
---
arch/arm/kernel/setup.c | 20 +-
drivers/of/Kconfig | 10 +
drivers/of/Makefile | 1 +
drivers/of/of_dependencies.c| 403
drivers/of/platform.c | 32 +++-
include/linux/of.h | 15 ++
include/linux/of_dependencies.h | 61 ++
include/linux/of_platform.h | 5 +
8 files changed, 543 insertions(+), 4 deletions(-)
create mode 100644 drivers/of/of_dependencies.c
create mode 100644 include/linux/of_dependencies.h
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 1e8b030..f67387d 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -787,10 +788,19 @@ static int __init customize_machine(void)
if (machine_desc->init_machine)
machine_desc->init_machine();
#ifdef CONFIG_OF
- else
+ else {
+#ifdef CONFIG_OF_DEPENDENCIES
+ if (!of_init_build_order(NULL, NULL))
+ of_init_create_devices(NULL, NULL);
+ else
+ of_init_free_order();
+#else
of_platform_populate(NULL, of_default_bus_match_table,
NULL, NULL);
#endif
+ }
+#endif
+
return 0;
}
arch_initcall(customize_machine);
@@ -914,7 +924,13 @@ void __init setup_arch(char **cmdline_p)
arm_pm_restart = mdesc->restart;
unflatten_device_tree();
-
+#ifdef CONFIG_OF_DEPENDENCIES
+ /*
+* No alloc used in of_init_build_order(), therefor it would work
+* already here too.
+*/
+ /* of_init_build_order(NULL, NULL); */
+#endif
arm_dt_init_cpu_maps();
psci_init();
#ifdef CONFIG_SMP
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index c6973f1..a7e1614 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -75,4 +75,14 @@ config OF_MTD
depends on MTD
def_bool y
+config OF_DEPENDENCIES
+ bool "Device Tree dependency based initialization order (EXPERIMENTAL)"
+ help
+ Enables dependency based initialization order of platform drivers.
+
+ For correct operation the binary DT blob should have been
+ populated with properties of type "dependencies".
+
+ If unsure, say N here.
+
endmenu # OF
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index efd0510..3888d9c 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_OF_MDIO) += of_mdio.o
obj-$(CONFIG_OF_PCI) += of_pci.o
obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o
obj-$(CONFIG_OF_MTD) += of_mtd.o
+obj-$(CONFIG_OF_DEPENDENCIES) += of_dependencies.o
diff --git a/drivers/of/of_dependencies.c b/drivers/of/of_dependencies.c
new file mode 100644
index 000..7905172
--- /dev/null
+++ b/drivers/of/of_dependencies.c
@@ -0,0 +1,403 @@
+/*
+ * Code for building a deterministic initialization order based on dependencies
+ * defined in the device tree.
+ *
+ * Copyright (C) 2014 Alexander Holler
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include
+
+#define MAX_DT_NODES 1000 /* maximum number of vertices */
+#define MAX_EDGES (MAX_DT_NODES*2) /* maximum number of edges (dependencies) */
+
+struct edgenode {
+ uint32_t y; /* phandle */
+ struct edgenode *next; /* next edge in list */
+};
+
+/*
+ * Vertex numbers do correspond to phandle numbers. That means the graph
+ * does contain as much vertices as the maximum of all phandles.
+ * Or in other words, we assume that for all phandles in the device tree
+ * 0 < phandle < MAX_DT_NODES+1 is true.
+ */
+struct dep_graph {
+ struct edgenode edge_slots[MAX_EDGES]; /* used to avoid kmalloc */
+ struct edgenode *edges[MAX_DT_NODES+1]; /* adjacency info */
+ unsigned nvertices; /* number of vertices in graph */
+ unsigned nedges; /* number of edges in graph */
+ bool processed[MAX_DT_NODES+1]; /* which vertices have been processed */
+ bool include_node[MAX_DT_NODES+1]; /* which nodes to consider */
+ bool discovered[MAX_DT_NODES+1]; /* which vertices have been found */
+ bool finished; /* if true, cut off search immediately */
+};
+static struct dep_graph graph __initdata;
+
+struct init_order {
+
[RFC PATCH 4/9] dt: deps: dtc: Add option to print dependency graph as dot (Graphviz)
Add option -T do print a dependency graph in dot format for
generating a picture with Graphviz.
E.g.
dtc -T foo.dts | dot -T svg -o foo.svg
would generate the picture foo.png with the dependency graph.
Convential dependencies (those based on the tree structure) are having
black arrows, dependencies based on the property 'dependencies' are
having cyan arrows.
Option -D to not automatically add dependencies does still work, so
you could build a classic dependency graph with
dtc -D -T foo.dts | dot -T png -o foo_no_auto_deps.png
This works with binary blobs as input too. E.g.
CROSS_COMPILE=gcc-foo ARCH=arm make foo.dtb
scripts/dtc/dtc -I dtb -T arch/arm/boot/dts/foo.dtb
would print the dot file.
Signed-off-by: Alexander Holler
---
scripts/dtc/dependencies.c | 48 --
scripts/dtc/dtc.c | 19 +++---
scripts/dtc/dtc.h | 2 +-
3 files changed, 59 insertions(+), 10 deletions(-)
diff --git a/scripts/dtc/dependencies.c b/scripts/dtc/dependencies.c
index 8fe1a8c..4579f6f 100644
--- a/scripts/dtc/dependencies.c
+++ b/scripts/dtc/dependencies.c
@@ -298,7 +298,7 @@ static void __init topological_sort(void)
depth_first_search(i);
}
-static int __init add_dep_list(struct device_node *node)
+static int __init add_dep_list(struct device_node *node, bool print_dot)
{
const __be32 *list, *list_end;
uint32_t ph;
@@ -328,6 +328,9 @@ static int __init add_dep_list(struct device_node *node)
rc = insert_edge(node->phandle, ph);
if (rc)
break;
+ if (print_dot)
+ printf(" node0x%x -> node0x%x [color=cyan]\n",
+ node->phandle, ph);
}
return rc;
@@ -352,9 +355,10 @@ static const char *of_prop_next_string(struct property
*prop, const char *cur)
}
static int __init add_deps_lnx(struct device_node *parent,
- struct device_node *node)
+ struct device_node *node, bool print_dot)
{
struct device_node *child;
+ const char *cp;
int rc = 0;
if (!__of_device_is_available(node))
@@ -375,13 +379,33 @@ static int __init add_deps_lnx(struct device_node *parent,
return -EINVAL;
}
order.parent_by_phandle[node->phandle] = parent->phandle;
- rc = add_dep_list(node);
+ if (print_dot) {
+ struct property *prop;
+ printf("node0x%x [label=\"0x%x %s", node->phandle,
+ node->phandle, node->name);
+ if (node->full_name)
+ printf(" (%s)", node->full_name);
+ prop = get_property(node, "compatible");
+ if (prop) {
+ printf("\\n");
+ for (cp = of_prop_next_string(prop, NULL); cp;
+cp = of_prop_next_string(prop, cp)) {
+ if (cp != prop->val.val)
+ putchar(' ');
+ printf("%s", cp);
+ }
+ }
+ printf("\"];\n");
+ printf(" node0x%x -> node0x%x\n", node->phandle,
+ parent->phandle);
+ }
+ rc = add_dep_list(node, print_dot);
if (unlikely(rc))
return rc;
parent = node; /* change the parent only if node is a driver */
}
for_each_child_of_node(node, child) {
- rc = add_deps_lnx(parent, child);
+ rc = add_deps_lnx(parent, child, print_dot);
if (unlikely(rc))
break;
}
@@ -426,7 +450,7 @@ void __init of_init_print_order(const char *name)
}
}
-int __init of_init_build_order(struct device_node *root)
+int __init of_init_build_order(struct device_node *root, const char *print_dot)
{
struct device_node *child;
int rc = 0;
@@ -438,12 +462,24 @@ int __init of_init_build_order(struct device_node *root)
calc_max_phandle(root);
order.old_max_phandle = order.max_phandle;
+ if (print_dot) {
+ printf("digraph G {\n");
+ printf("node0x%x [label=\"0x%x root (/)\"];\n",
+ order.max_phandle+1 , order.max_phandle+1);
+ }
+
for_each_child_of_node(root, child) {
- rc = ad
[RFC PATCH 3/9] dt: deps: dtc: Add option to print initialization order
Add option -t to print the default initialization order.
No other output will be generated.
To print the order, just use something like this:
CROSS_COMPILE=gcc-foo ARCH=arm make foo.dtb
scripts/dtc/dtc -I dtb -t arch/arm/boot/dts/foo.dtb
Since it's now possible to check to for cycles in the dependency graph,
this is now done too.
Signed-off-by: Alexander Holler
---
scripts/dtc/dependencies.c | 346 +
scripts/dtc/dtc.c | 24 +++-
scripts/dtc/dtc.h | 2 +
3 files changed, 371 insertions(+), 1 deletion(-)
diff --git a/scripts/dtc/dependencies.c b/scripts/dtc/dependencies.c
index dd4658c..8fe1a8c 100644
--- a/scripts/dtc/dependencies.c
+++ b/scripts/dtc/dependencies.c
@@ -106,3 +106,349 @@ void add_dependencies(struct boot_info *bi)
{
process_nodes_props(bi->dt, bi->dt);
}
+
+/*
+ * The code below is in large parts a copy of drivers/of/of_dependencies.c
+ * in the Linux kernel. So both files do share the same bugs.
+ * The next few ugly defines do exist to keep the differences at a minimum.
+ */
+static struct node *tree;
+#define pr_cont(format, ...) printf(format, ##__VA_ARGS__)
+#define pr_info(format, ...) printf(format, ##__VA_ARGS__)
+#define pr_warn(format, ...) printf(format, ##__VA_ARGS__)
+#define pr_err(format, ...) fprintf(stderr, format, ##__VA_ARGS__)
+typedef cell_t __be32;
+#define device_node node
+#define full_name fullpath
+#define __initdata
+#define __init
+#define unlikely(a) (a)
+#define of_node_put(a)
+#define of_find_node_by_phandle(v) get_node_by_phandle(tree, v)
+#define __of_get_property(a, b, c) get_property(a, b)
+#define for_each_child_of_node(a, b) for_each_child(a, b)
+
+
+#define MAX_DT_NODES 1000 /* maximum number of vertices */
+#define MAX_EDGES (MAX_DT_NODES*2) /* maximum number of edges (dependencies) */
+
+struct edgenode {
+ uint32_t y; /* phandle */
+ struct edgenode *next; /* next edge in list */
+};
+
+/*
+ * Vertex numbers do correspond to phandle numbers. That means the graph
+ * does contain as much vertices as the maximum of all phandles.
+ * Or in other words, we assume that for all phandles in the device tree
+ * 0 < phandle < MAX_DT_NODES+1 is true.
+ */
+struct dep_graph {
+ struct edgenode edge_slots[MAX_EDGES]; /* used to avoid kmalloc */
+ struct edgenode *edges[MAX_DT_NODES+1]; /* adjacency info */
+ unsigned nvertices; /* number of vertices in graph */
+ unsigned nedges; /* number of edges in graph */
+ bool processed[MAX_DT_NODES+1]; /* which vertices have been processed */
+ bool include_node[MAX_DT_NODES+1]; /* which nodes to consider */
+ bool discovered[MAX_DT_NODES+1]; /* which vertices have been found */
+ bool finished; /* if true, cut off search immediately */
+};
+static struct dep_graph graph __initdata;
+
+struct init_order {
+ uint32_t max_phandle; /* the max used phandle */
+ uint32_t old_max_phandle; /* used to keep track of added phandles */
+ struct device_node *order[MAX_DT_NODES+1];
+ unsigned count;
+ /* Used to keep track of parent devices in regard to the DT */
+ uint32_t parent_by_phandle[MAX_DT_NODES+1];
+ struct device *device_by_phandle[MAX_DT_NODES+1];
+};
+static struct init_order order __initdata;
+
+
+/* Copied from drivers/of/base.c (because it's lockless). */
+static int __init __of_device_is_available(struct device_node *device)
+{
+ struct property *status;
+
+ if (!device)
+ return 0;
+
+ status = get_property(device, "status");
+ if (status == NULL)
+ return 1;
+
+ if (status->val.len > 0) {
+ if (!strcmp(status->val.val, "okay") ||
+ !strcmp(status->val.val, "ok"))
+ return 1;
+ }
+
+ return 0;
+}
+
+/*
+ * x is a dependant of y or in other words
+ * y will be initialized before x.
+ */
+static int __init insert_edge(uint32_t x, uint32_t y)
+{
+ struct edgenode *p; /* temporary pointer */
+
+ if (unlikely(x > MAX_DT_NODES || y > MAX_DT_NODES)) {
+ pr_err("Node found with phandle 0x%x > MAX_DT_NODES (%d)!\n",
+ x > MAX_DT_NODES ? x : y, MAX_DT_NODES);
+ return -EINVAL;
+ }
+ if (unlikely(!x || !y))
+ return 0;
+ if (unlikely(graph.nedges >= MAX_EDGES)) {
+ pr_err("Maximum number of edges (%d) reached!\n", MAX_EDGES);
+ return -EINVAL;
+ }
+ p = &graph.edge_slots[graph.nedges++];
+ graph.include_node[x] = 1;
+ graph.include_node[y] = 1;
+ p->y = y;
+ p->next = graph.edges[x];
+ graph.edges[x] = p; /* insert at head of list */
+
+ graph.nvertices = (x > graph.nvertices) ? x : graph.nvertices;
+ graph.nve
