On Wednesday, September 28, 2011 11:57:49 PM Scott Wood wrote:
> On 09/28/2011 04:42 PM, Marek Vasut wrote:
> > On Wednesday, September 28, 2011 11:26:45 PM Scott Wood wrote:
> >> On 09/11/2011 11:06 PM, Marek Vasut wrote:
> >>> + desc = info->desc[i];
> >>> + memset(desc, 0, sizeof(struct mxs_dma_desc));
> >>> + desc->address = (dma_addr_t)desc;
> >>> + }
> >>> +
> >>> + info->desc_index = 0;
> >>> +}
> >>> +
> >>> +static inline uint32_t mxs_nand_ecc_chunk_cnt(uint32_t page_data_size)
> >>> +{
> >>> + return page_data_size / MXS_NAND_CHUNK_DATA_CHUNK_SIZE;
> >>> +}
> >>
> >> No need for inline in .c files, GCC should take care of this
> >> automatically.
> >
> > Does it really ?
>
> Very small functions (and anything used only once) get inlined
> automatically.
>
> It might need the hint if you have a larger function that collapses down
> to something small due to constant propagation (and of course if it's a
> header or otherwise *must* be inlined), but usually it does a decent job
> on its own.
>
> >> Hmm, I thought raw was just supposed to disable ECC, not change the
> >> layout from what is used in normal operation.
> >
> > You see the page as is ... I see no problem with this part.
>
> What if the raw access is being done to e.g. force bit flips for testing?
>
> There seems to be a difference between U-Boot and Linux here.
>
> Linux has this in mtd.h:
> > * MTD_OOB_RAW: mode to read oob and data without doing ECC
> > checking
>
> Doesn't say anything about layout.
>
> Whereas U-Boot says:
> > * MTD_OOB_RAW: mode to read raw data+oob in one chunk. The oob
> > data * is inserted into the data. Thats a raw
> > image of the * flash contents.
>
> Linux used to say what U-Boot says, but changed in commit
> b64d39d8b03fea88417d53715ccbebf71d4dcc9f
>
> This commit message includes the comment "Now MTD_OOB_RAW behaves just
> like MTD_OOB_PLACE, but doesn't do ECC validation".
>
> So I think if you need something that changes the layout from normal
> operations, it needs to be a new mode. And it's about time to sync up
> U-Boot's NAND code with Linux again...
Well aren't you the maintainer that should take care of it ? ;-)
>
> >>> + /*
> >>> + * There are fundamental incompatibilities between the i.MX GPMI NFC
> >>> and + * the NAND Flash MTD model that make it essentially impossible
> >
> > to
> >
> >>> write + * the out-of-band bytes.
> >>> + *
> >>> + * We permit *ONE* exception. If the *intent* of writing the OOB is
> >>> to + * mark a block bad, we can do that.
> >>> + */
> >>
> >> Is this just an issue with writing OOB separately from the main data
> >> (which would also be an issue on MLC chips that don't allow multiple
> >> partial programming), or can you not even write user OOB bytes as part
> >> of a full page write?
> >>
> >> Based on fake_ecc_layout I'm guessing the latter.
> >
> > My understanding of the original FSL driver is that you should never be
> > allowed to access the physical NAND media at all. Only through the
> > driver, which does the magic.
>
> I'm not talking about circumventing the driver, just accessing some user
> OOB bytes through it.
There are no user OOB bytes, the driver does the ECC so user has no need to
write any OOB.
>
> >> The nand_base.c code actually does have a split here
> >> (nand_scan_ident/nand_scan_tail), but U-Boot's glue code is too
> >> inflexible, and insists on calling nand_scan. The right fix is to let
> >> drivers call nand_scan_ident/nand_scan_tail themselves.
> >
> > I can't test now, so this has to wait. I'd prefer to get this mainline
> > and then start poking around fixing this.
>
> OK. It's been on my TODO list for a while now...
>
> -Scott
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
