On 17-09-2012 17:57, Tom Rini wrote:
On Sun, Sep 16, 2012 at 10:16:47AM +0100, Jos? Miguel Gon?alves wrote:
On 09/14/2012 08:01 PM, Tom Rini wrote:
On Fri, Sep 14, 2012 at 07:45:40PM +0100, Jos? Miguel Gon?alves wrote:
On 14-09-2012 19:21, Marek Vasut wrote:
Dear Jos? Miguel Gon?alves,

NAND Flash driver with HW ECC for the S3C24XX SoCs.
Currently it only supports SLC NAND chips.

Signed-off-by: Jos? Miguel Gon?alves <jose.goncal...@inov.pt>
[...]

+#include <common.h>
+#include <nand.h>
+#include <asm/io.h>
+#include <asm/arch/s3c24xx_cpu.h>
+#include <asm/errno.h>
+
+#define MAX_CHIPS      2
+static int nand_cs[MAX_CHIPS] = { 0, 1 };
+
+#ifdef CONFIG_SPL_BUILD
+#define printf(arg...) do {} while (0)
This doesn't seem quite right ...

1) this should be in CPU directory
2) should be enabled only if CONFIG_SPL_SERIAL_SUPPORT is not set
3) should be inline function, not a macro
1) and 3) OK.
Don't quite understand 2). I want to remove the printfs in the SPL
build, as it would blown up the internal SoC RAM space available.
So why add a condition with CONFIG_SPL_SERIAL_SUPPORT?
You've got 8KB, based on the final patch in the series.  At least in my
SPL series that's still enough to get you printf/puts (I believe 4kb was
the cutoff where that had to be dropped).

Barely:

$ size u-boot-spl
    text       data        bss        dec        hex    filename
    3337          8        588       3933        f5d    u-boot-spl

$ size u-boot-spl-printf
    text       data        bss        dec        hex    filename
    7968          8        604       8580       2184 u-boot-spl-printf

The printf is not so important that justifies exhausting the IRAM
space available and preventing any future SPL expansion...
There's two parts to this:
- What else can you do in a single binary, in theory?  Is there boot
   medium detection and you would want to have, for example, NAND and SD
   support in the same binary?  I would say memory is meant for using,
   but this is a board maintainer decision and that's you :)

That's exactly what I've got in mind when I talked about a future expansion! Being able to boot also from an SD card. With only 8KB for .text and .data, I can not use printfs in the SPL for this platform (at least with the present printf support for SPL).

- We have a define today (CONFIG_SPL_LIBCOMMON_SUPPORT) that toggles
   printf or no printf.  If we really need to say yes to
   LIBCOMMON_SUPPORT and no to printf, we need finer grained config
   options and then a do-nothing printf is used for SPL.  Doing the
   opt-out driver by driver just punts this problem down the road to the
   next developer and that's not very nice (and adding
   CONFIG_SPL_PRINTF_SUPPORT shouldn't be a big patch, modify a few
   Makefiles, update a bunch of config files, add
   common/spl/dummy_funcs.c and a __weak printf).


_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to