Hi Todd,
Todd Poynor wrote:
> Hi Vitaly, my comments...
>
>> +++ util/nanddump.c 2 Dec 2005 06:54:11 -0000
>> @@ -163,6 +163,7 @@
>> struct mtd_oob_buf oob = {0, 16, oobbuf};
>> mtd_info_t meminfo;
>> char pretty_buf[80];
>> + uint32_t oobavail;
>>
>> process_options(argc, argv);
>>
>> @@ -179,6 +180,13 @@
>> exit (1);
>> }
>>
>> + if (ioctl(fd, MEMGETOOBAVAIL, &oobavail) != 0) {
>> + perror("unable to get NAND oobavail");
>> + close(fd);
>> + exit(1);
>> + }
>
>
> How about fall back to all 16/64/etc. avail?
>
> For debugging purposes it can be nice to see the raw OOB, in case that
> can be shoehorned in here somehow.
Well, the main idea of what I'm implementing is to hide the internal
structure of the OOB data from user. Therefore it's no guaranteed way in
this implementation to read the raw OOB.
On the other hand, not being able to get oobavail means a problem so I
wouldn't try to read OOB in this situation.
However I can a) add a flag to both read_oob/write_oob whether to read
oobavail or raw OOB and implement corresponding functionality/flag in
nanddump, or) fall back to considering oobavail being equal to oobsize
if getting oobavail fails but neither variant seems good to me...
>
>> +++ util/nandwrite.c 2 Dec 2005 06:54:11 -0000
>
>
>> - " -j, --jffs2 force jffs2 oob layout (legacy
>> support)\n"
>> - " -y, --yaffs force yaffs oob layout (legacy
>> support)\n"
>
>
> Again, it may be good to keep a legacy mode around during the
> transition period.
I'm afraid it's not possible. And this legacy support has been sheduled
to go away long ago, I hope now is a good time :)
>
>> if (writeoob) {
>> /* Read OOB data from input file, exit on failure */
>> - if ((cnt = read(ifd, oobreadbuf, meminfo.oobsize)) !=
>> meminfo.oobsize) {
>> + if ((cnt = read(ifd, oobreadbuf, oobavail)) != oobavail) {
>
>
> This requires the input file to be tailored to the oobavail of a
> specific destination device, reducing the benefit of autoplacement.
> It may be best to continue to pad input files oob data to the full
> oobsize, much like the data portion is handled.
Well, padding doesn't work in nandwrite if the image to be written
contains OOB data. The idea as I see it is if you are trying to write
the image with OOB data, you should know what you're doing, and that
implies knowledge of the oobavail size.
On the other hand, it might be useful to implement an option for
nandwrite which specifies what OOB data length user supposes (default
will be the oobavail).
>
>> + memcpy(oobbuf, oobreadbuf, oobavail);
>
>
> No need for oobreadbuf and copy if no OOB layout processing done anymore?
>
Yeah, thanks.
And 'noecc' parameter should also go away. An update of the patch will
follow...
Vitaly