From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dirk Behme Date: Tue, 07 Oct 2008 22:47:48 +0200 Subject: [U-Boot] [PATCH 07/12 v2] ARM: OMAP3: Add memory and syslib common files, add NAND support In-Reply-To: <48EBA2B5.6060403@googlemail.com> References: <48e5f6a5.0305560a.0a0e.ffffe071@mx.google.com> <20081003165203.GA11156@ld0162-tx32.am.freescale.net> <48EB2F0E.4090805@googlemail.com> <48EB4717.7080904@gmail.com> <20081007173016.GB3438@loki.buserror.net> <48EBA2B5.6060403@googlemail.com> Message-ID: <48EBCAF4.6020407@googlemail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Dirk Behme wrote: > Scott Wood wrote: > >> On Tue, Oct 07, 2008 at 06:25:11AM -0500, Nishanth Menon wrote: >> >>> Dirk Behme said the following on 10/07/2008 04:42 AM: >>> >>>> It doesn't differ ;) >>>> >>>> So I removed this and tried to use default nand_read_buf16() instead: >>>> >>>> nand->read_buf = nand_read_buf16; >>>> >>>> in board_nand_init(). But this doesn't compile as nand_read_buf16() is >>>> static in nand_base.c. How do I use it in OMAP3 NAND driver? Marked it >>>> as FIXME in patch. >>> >>> >>> Probably does not need an explicit initialization, mtd nand_scan should >>> populate that. >> >> >> >> Correct, NULL methods will be filled in with defaults if applicable. > > > ok, will do so, this is easy :) > >>>>>> +/* >>>>>> + * omap_calculate_ecc - Generate non-inverted ECC bytes. >>>>>> + * >>>>>> + * Using noninverted ECC can be considered ugly since writing a >>>>>> blank >>>>>> + * page ie. padding will clear the ECC bytes. This is no problem as >>>>>> + * long nobody is trying to write data on the seemingly unused >>>>>> page. >>>>>> + * Reading an erased page will produce an ECC mismatch between >>>>>> + * generated and read ECC bytes that has to be dealt with >>>>>> separately. >>>>> >>>>> >>>>> >>>>> Is this a hardware limitation? If so, say so in the comment. If not, >>>>> why do it this way? >>>> >>>> >>>> Don't know. >>>> >>>> All: Any help? >>> >>> >>> The issue is simple: assume we read a page of 0xFF's(fresh erased), IF >>> using H/w ECC engine within GPMC, the result of read will be 0x0 while >>> the ECC offsets of the spare area will be 0xFF which will result in an >>> ECC mismatch. >> >> >> Right, I'd just like to see an explicit statement that this is the >> only way >> to do HW ECC that the hardware supports (following verification of that >> fact, of course), along with a pointer to where in the code the ECC error >> when reading an empty page is dealt with. > > > Will add Nishanth's explanation to comment and check code for this. > >>>>>> + .eccbytes = 12, >>>>>> + .eccpos = { >>>>>> + 2, 3, 4, 5, >>>>>> + 6, 7, 8, 9, >>>>>> + 10, 11, 12, 13}, >>>>>> + .oobfree = { {20, 50} } /* don't care */ >>>>> >>>>> >>>>> >>>>> Bytes 64-69 of a 64-byte OOB are free? >>>>> What don't we care about? >>>> >>>> >>>> +static struct nand_ecclayout hw_nand_oob_64 = { >>>> >>>> Don't know (or understand?). >>>> >>>> All: Any help? >>> >>> >>> I do not get it either.. ECCPOS is in offset bytes, and oobfree should >>> be {.offset=20,.length=44} /*I always hated struct initialization done >>> as above..*/, but then, >> >> >> >> Why not offset 14, length 50? > > > Seems I need a closer look what we are talking about here ;) > >>>> We need to be able to switch ECC at runtime cause some images have to >>>> be written to NAND with HW ECC and some with SW ECC. This depends on >>>> what user (reader) of these parts expect. >>>> >>>> OMAP3 has a boot ROM which is able to read a second level loader >>>> (called x-loader) from NAND and start/execute it. This 2nd level >>>> loader has to be written by U-Boot using HW ECC as ROM code does HW >>>> ECC to read the image. All other parts, e.g. Linux kernel, use SW ECC >>>> as default. For this we have to use SW ECC to write images, then. >>>> >>>> Therefore we add an additional user command in cmd_nand.c to switch >>>> ECC depending on what user wants to write. >>> >>> >>> why not use h/w ecc which rom code understands in kernel and u-boot. H/w >>> ecc will be faster in comparison to doing s/w ecc and there is good >>> support in MTD to do it, then there would be no reason for s/w ecc >>> IMHO.. >> >> >> >> Agreed. > > > As already mentioned in previous post, I think for the the moment we > have to go with both ways. > > To summarize the open points I will look at: > > a) Remove unnecessary nand->read_buf init Removed in attachment > b) Add comment and check code for HW ecc issue with erased page Added comment. Not sure if we have to do anything in code, though. > c) Fix offset 14, length 50 issue Changed in attachment. > d) Extend MTD API with a call to switch HW/SW ecc, remove > platform-specific ifdefs in generic files Removed nand ecc command from cmd_nand.c and added "nandecc" command to board file (Scott: Thanks for the hint!): --- /dev/null +++ u-boot-arm/cpu/arm_cortexa8/omap3/board.c ... +#ifdef CONFIG_NAND_OMAP3 +/****************************************************************************** + * OMAP3 specific command to switch between NAND HW and SW ecc + *****************************************************************************/ +static int do_switch_ecc(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[]) +{ + if (argc != 2) + goto usage; + if (strncmp(argv[1], "hw", 2) == 0) + omap_nand_switch_ecc(1); + else if (strncmp(argv[1], "sw", 2) == 0) + omap_nand_switch_ecc(0); + else + goto usage; + + return 0; + +usage: + printf ("Usage: nandecc %s\n", cmdtp->help); + return 1; +} + +U_BOOT_CMD( + nandecc, 2, 1, do_switch_ecc, + "nandecc - switch OMAP3 NAND ECC calculation algorithm\n", + "[hw/sw] - Switch between NAND hardware (hw) or software (sw) ecc algorithm\n" + ); + +#endif /* CONFIG_NAND_OMAP3 */ See attachment for latest version of NAND patch. Please let us know if anything else has to be changed. If not, I will prepare OMAP3 v3 patch set. Thanks Dirk -------------- next part -------------- An embedded and charset-unspecified text was scrubbed... Name: 08_omap3_nand.txt Url: http://lists.denx.de/pipermail/u-boot/attachments/20081007/88d2c45d/attachment-0001.txt