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...
>>> + /*
>>> + * 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.
>> 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
