Hello, On Mon, Jan 30, 2017 at 06:54:44PM +0000, Russell King - ARM Linux wrote: > On Wed, Jan 25, 2017 at 01:13:23PM -0800, Frank Rowand wrote: > > Adding back the chosen nodes is a bandaid that papers over the actual bug > > in the decompressor. > > > > A comment about Fabio's attempt to fix the decompressor noted a possible > > issue with the method taken to fix the problem, but then the discussion > > of fixing the decompressor was dropped and v3 of the patch to add chosen > > into a bunch of .dtsi files was applied by Shawn. > > > > Can someone please help Fabio to create a correct patch to fix the > > decompressor? > > I refuse to assist, see below. > > The reason this has come up is that this problem has _nothing_ to do > with the decompressor - see recent context on the linux-arm-kernel > list. Several people reported problems with the patch that removes skeleton.dtsi. There are at least: - Russell, who saw U-Boot failing to specify the amount of RAM; - somebody (I forgot and I'm lazy so I didn't look up the name) who noticed some i.MX6 boards fail in kernelci with barebox because barebox assumed /chosen to be present; and - somebody else (...) who reported the ATAGS_DTB_COMPAT failure (also wrongly called "decompressor failure" in this thread). I agree that for each single of the above issues it is worth to readd the /chosen and /memory nodes for now. Still all three are IMHO worth to be fixed. barebox is already fixed in v2017.01.0 (commit f49b415b9231 (of: base: add chosen node if it does not exist when adding initrd), https://git.pengutronix.de/cgit/barebox/commit/?id=f49b415b9231). Looking at the ATAGS_DTB_COMPAT problem: The code in question is (if I understood correctly) merge_fdt_bootargs in arch/arm/boot/compressed/atags_to_fdt.c: setprop_string(fdt, "/chosen", "bootargs", cmdline); looking further: static int node_offset(void *fdt, const char *node_path) { int offset = fdt_path_offset(fdt, node_path); if (offset == -FDT_ERR_NOTFOUND) offset = fdt_add_subnode(fdt, 0, node_path); return offset; } static int setprop_string(void *fdt, const char *node_path, const char *property, const char *string) { int offset = node_offset(fdt, node_path); if (offset < 0) return offset; return fdt_setprop_string(fdt, offset, property, string); } So it seems "/chosen" not being there should be handled. I don't have a reproducer for this problem, but it doesn't look that hard to fix with DEBUG_LL and a system that shows the problem. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ | -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
