From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tom Rini Date: Mon, 17 Sep 2012 10:56:14 -0700 Subject: [U-Boot] [PATCH v2 09/11] S3C24XX: Add NAND Flash driver In-Reply-To: <505758FA.2090805@inov.pt> References: <1347643742-19966-1-git-send-email-jose.goncalves@inov.pt> <1347643742-19966-10-git-send-email-jose.goncalves@inov.pt> <201209142021.12033.marex@denx.de> <50537B54.2060903@inov.pt> <20120914190120.GL22028@bill-the-cat> <505598FF.7010108@inov.pt> <20120917165757.GA16252@bill-the-cat> <505758FA.2090805@inov.pt> Message-ID: <5057643E.20806@ti.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 09/17/12 10:08, Jos? Miguel Gon?alves wrote: > 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 >>>>>>> >>>>>> [...] >>>>>> >>>>>>> +#include +#include +#include >>>>>>> +#include +#include >>>>>>> + +#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). OK, so please take a stab at option two, on top of my SPL series, keeping in mind what Scott has said (which makes sense) because otherwise you'll be changing a lot of MMC files too to drop out printf :) - -- Tom -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://www.enigmail.net/ iQIcBAEBAgAGBQJQV2Q+AAoJENk4IS6UOR1W0VMP/1ToNzW5XmNApUGYIZi1d779 hJ6MieZoiTHOCnrHiRMMMAfOnPNnt+63TozmXWkhNhlPVRCs1Irx8nCpzMabjevr ZENjnewtKsvgBsICak5rSQLbfyQBUG8tL3iBMPnYwyNhDf8CgED6rNCnxhV8Lr7j o0gELaNHPRD7bJwglXo3TN0BxNtTrww3uSArSYh4WMVaOP908Mk7p7y8qEVSvNeh BrdbVZK1oPmhlke9EUXfCQYqn/JdJ7mtdW1q5PdST7GFtnLZmqpj+FuOEwN3OioA DE6J461Aqr95mGVUnUr7PxglytL6zLKJcE8YpIhu9nL6r9QRg0wm4HIKr3uTKLl5 WBs4ziJsLtm5IAZ7xg2FOsPrCkrAiL3bVQg0+7xVc1cVzKIGmtMR6oGVS+nI38Q6 lzmz0AQSuobeLkiP4+tL8C7kFkwMcj9I5LByN968ZMvbTftecIsdgYSkluOdGU5w dwKPjplU4t8yy6d1TXbh0Xdw7q8cBZ3bjqbAKi3uo9BpEHgCDOwHp7oOTuJTDB/7 C6WxXHdQFHh7m0hxf1zkawOeh+oqd5MHAxjlckQ/zmg5UsmNWDA6RmmYgWrOBw9m jCvN/lhe1soBnxaz2byUKwEkPIDwmBl+JgtSL7DMZ7bLfS59daXiaMNac2JPstG3 j0lBvj7SCVnQrE6SJxxa =71I1 -----END PGP SIGNATURE-----