Skip to contentSkip to conversation

ARM: Decompressor multiplatform support

RFC PATCH New Domenico Andreoli 2 participants 17 messages Jul 15, 2012 – Jul 17, 2012

lore
Share & tools
lore

Domenico Andreoli

author
#0 lore raw

cover ARM: Decompressor multiplatform support

Hi all,

here is a new edition of my work in the decompressor area.

My intent is to get rid of uncompress.h and prepare the decompressor to dynamically select the various machine specific decompressor init steps, included the selection of the appropriate console driver.

Currently the mainline kernel defines these steps statically and indeed has some trouble to boot on different boards with the same binary.

What this patch does is allowing the four functions (arch_decomp_setup, arch_error, putc and flush) currently used to define such static steps to be packed in quantity and to be selected/executed by the decompressor accordingly to the machid or DT passed by the boot loader.

I forgot to say that all this code and data is relocated from the kernel image to the decompressor during the build. So some pointer arithmetic is required on the decompressor side. This unfortunately brings in also some limitations in the code/data to be relocated.

The patchset is of course far from being ready for anything but development and testing. I really looked into only three SoCs but I know that aside from a kind of graphic adapter somewhere all the other are a variation of these three players.

I'm able to test it on a out-of-tree bcm4760 board and a Samsung S5P6450. I also build tested a dozen of other random samsung and ti/omap defconfigs, sometimes nailing down problems. I've also my qq2440 ;)

The patchset also applies on top of Arnd's multiplatform tree and the resulting binary boots on the bcm4760 board. In that configuration I get the most fat relocation of code and data, with an amazing number of four drivers and don't known how many console tags. I had also some fun efficiently packing all that stuff :)

That said, thank you for the comments :)

cheers,
Domenico

#6 lore raw

RFC PATCH 5/12 ARM: Add DT support to decompressor tags

Review

Quoted 3 lines · Domenico Andreoli
(quoted diff) +0 -2
-ifeq ($(CONFIG_ARM_ATAG_DTB_COMPAT),y)
 OBJS   += $(libfdt_objs) atags_to_fdt.o
-endif

Can you explain why this is necessary? I don't see any code of that file being called from the newly added code for the case where CONFIG_ARM_ATAG_DTB_COMPAT is disabled.

Arnd

author
#7 lore raw

RFC PATCH 5/12 ARM: Add DT support to decompressor tags

Quoted 8 lines · Arnd Bergmann

Review

Quoted 3 lines · Domenico Andreoli
(quoted diff) +0 -2
-ifeq ($(CONFIG_ARM_ATAG_DTB_COMPAT),y)
 OBJS   += $(libfdt_objs) atags_to_fdt.o
-endif

Can you explain why this is necessary? I don't see any code of that file being called from the newly added code for the case where CONFIG_ARM_ATAG_DTB_COMPAT is disabled.

I put function get_fdt_prop() in atags_to_fdt.c but what I basically needed was the inclusion of $(libfdt_objs). I agree this can be achieved out of CONFIG_ARM_ATAG_DTB_COMPAT's stuff and I'll do if the patchest is allowed to mature.

I guess a better place could be libfdt itself but feel free to suggest a better one.

Thanks,
Domenico

#15 lore raw

cover ARM: Decompressor multiplatform support

In reply to Domenico Andreoli
Quoted 11 lines · Domenico Andreoli

My intent is to get rid of uncompress.h and prepare the decompressor to dynamically select the various machine specific decompressor init steps, included the selection of the appropriate console driver.

Currently the mainline kernel defines these steps statically and indeed has some trouble to boot on different boards with the same binary.

What this patch does is allowing the four functions (arch_decomp_setup, arch_error, putc and flush) currently used to define such static steps to be packed in quantity and to be selected/executed by the decompressor accordingly to the machid or DT passed by the boot loader.

I definitely like the implementation, it looks very nicely done with the driver specific code being right inside of the actual device drivers.

I think the main question we have to answer is whether we want to go this far for the decompressor output. IIRC the last time this was debated, the argument was made (I don't remember if it was by Russell or someone else) that the decompressor code is designed to be as simple as possible and we should add too much complexity in it that would make it harder to debug when the only purpose of that code is to debug the decompressor code itself.

I find it hard to judge what the benefit of your implementation is compared to the risk of introducing bugs.

The other part I don't understand is how it relates to the early_print() infrastructure that has some of the same requirements.

Arnd

author
#16 lore raw

cover ARM: Decompressor multiplatform support

Quoted 8 lines · Arnd Bergmann
Quoted 7 lines · Domenico Andreoli

My intent is to get rid of uncompress.h and prepare the decompressor to dynamically select the various machine specific decompressor init steps, included the selection of the appropriate console driver.

Currently the mainline kernel defines these steps statically and indeed has some trouble to boot on different boards with the same binary.

Quoted 7 lines
Quoted 4 lines

What this patch does is allowing the four functions (arch_decomp_setup, arch_error, putc and flush) currently used to define such static steps to be packed in quantity and to be selected/executed by the decompressor accordingly to the machid or DT passed by the boot loader.

I definitely like the implementation, it looks very nicely done with the driver specific code being right inside of the actual device drivers.

yep, thanks.

Quoted 7 lines

I think the main question we have to answer is whether we want to go this far for the decompressor output. IIRC the last time this was debated, the argument was made (I don't remember if it was by Russell or someone else) that the decompressor code is designed to be as simple as possible and we should add too much complexity in it that would make it harder to debug when the only purpose of that code is to debug the decompressor code itself.

I also made this question [0] but probably the message was too long and nobody bothered to read it fully :)

The question applies almost unchanged for the arch_decomp_setup()/arch_error() calls. Those could be worthy to preserve and probably less easy to dismiss than the console ;)

Quoted 2 lines

I find it hard to judge what the benefit of your implementation is compared to the risk of introducing bugs.

Indeed. I can also add that without a sound extraction of console data from DT, where arm is heading, the thing looks even less appealing.

But I needed to publish it :)

Quoted 2 lines

The other part I don't understand is how it relates to the early_print() infrastructure that has some of the same requirements.

If there is, it's not intentional.

cheers,
Domenico

[0] http://www.spinics.net/lists/arm-kernel/msg177843.html