[RFC PATCH 00/12] ARM: Decompressor multiplatform support

[RFC PATCH 00/12] ARM: Decompressor multiplatform support

[Domenico Andreoli]

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

[RFC PATCH 01/12] ARM: Add strstr to the decompressor

[Domenico Andreoli]

An embedded and charset-unspecified text was scrubbed...
Name: decomp-add-strstr.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120715/8a47430b/attachment.ksh>

[RFC PATCH 02/12] ARM: kernel -> decompressor relocation handling

[Domenico Andreoli]

An embedded and charset-unspecified text was scrubbed...
Name: decomp-arch-reloc.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120715/f4f3a31e/attachment.ksh>

[RFC PATCH 03/12] ARM: Add indirection around arch_decomp_setup()/arch_error()

[Domenico Andreoli]

An embedded and charset-unspecified text was scrubbed...
Name: decomp-arch-fallbacks.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120715/defe8838/attachment.ksh>

[RFC PATCH 04/12] ARM: Add architecture specific decompressor tags

[Domenico Andreoli]

An embedded and charset-unspecified text was scrubbed...
Name: decomp-arch.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120715/9143d173/attachment.ksh>

[RFC PATCH 05/12] ARM: Add DT support to decompressor tags

[Domenico Andreoli]

An embedded and charset-unspecified text was scrubbed...
Name: decomp-arch-dt.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120715/2c282683/attachment.ksh>

[RFC PATCH 05/12] ARM: Add DT support to decompressor tags

[Arnd Bergmann]

On Sunday 15 July 2012, Domenico Andreoli wrote:
> -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

[RFC PATCH 05/12] ARM: Add DT support to decompressor tags

[Domenico Andreoli]

On Tue, Jul 17, 2012 at 02:14:19PM +0000, Arnd Bergmann wrote:
> On Sunday 15 July 2012, Domenico Andreoli wrote:
> > -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

[RFC PATCH 06/12] ARM: Work around OMAPs arch_decomp_setup()

[Domenico Andreoli]

An embedded and charset-unspecified text was scrubbed...
Name: decomp-arch-omap-hack.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120715/59a8affb/attachment.ksh>

[RFC PATCH 07/12] ARM: Add multi-arch console support to the decompressor

[Domenico Andreoli]

An embedded and charset-unspecified text was scrubbed...
Name: decomp-console.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120715/97856051/attachment.ksh>

[RFC PATCH 08/12] ARM: Add DT support to the decompressor console

[Domenico Andreoli]

An embedded and charset-unspecified text was scrubbed...
Name: decomp-console-dt.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120715/45e10e50/attachment.ksh>

[RFC PATCH 09/12] ARM: Decompressor support for AMBA PL010 UARTs

[Domenico Andreoli]

An embedded and charset-unspecified text was scrubbed...
Name: decomp-console-uart-amba-pl010.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120715/3333eb47/attachment.ksh>

[RFC PATCH 10/12] ARM: Decompressor support for AMBA PL011 UARTs

[Domenico Andreoli]

An embedded and charset-unspecified text was scrubbed...
Name: decomp-console-uart-amba-pl011.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120715/81f7ca6e/attachment.ksh>

[RFC PATCH 11/12] ARM: Decompressor support for Samsung UARTs

[Domenico Andreoli]

An embedded and charset-unspecified text was scrubbed...
Name: decomp-console-uart-samsung.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120715/3eaf0aff/attachment.ksh>

[RFC PATCH 12/12] ARM: Decompressor support for OMAP UARTs

[Domenico Andreoli]

An embedded and charset-unspecified text was scrubbed...
Name: decomp-console-uart-omap.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120715/a3fcf7f7/attachment.ksh>

[RFC PATCH 00/12] ARM: Decompressor multiplatform support

[Arnd Bergmann]

On Sunday 15 July 2012, Domenico Andreoli wrote:
> 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

[RFC PATCH 00/12] ARM: Decompressor multiplatform support

[Domenico Andreoli]

On Tue, Jul 17, 2012 at 02:31:15PM +0000, Arnd Bergmann wrote:
> On Sunday 15 July 2012, Domenico Andreoli wrote:
> > 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.

yep, thanks.

> 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 ;)

> 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 :)

> 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