[PATCH 0/4] MTD: Change meaning of -EUCLEAN return code on reads

Mike Dunn mikedunn at newsguy.com
Tue Mar 13 13:46:14 EDT 2012


Thanks for having a look Artem.

On 03/13/2012 05:03 AM, Artem Bityutskiy wrote:
>> (1) The element 'ecc_strength' is added to struct mtd_info, which will store the
>>     maximum number of bit errors that can be corrected in one writesize region.
> 
> I think this attribute should be exported via sysfs as R/O.


Do you think bitflip_threshold and/or ecc_strength should be accessible through
mtdchar ioctls as well?


>> (3) The element 'euclean_threshold' is added to struct mtd_info.  If the driver
>>     leaves this uninitialized, mtd sets it to ecc_strength when the device or
>>     partition is registered.  This element is also exposed for reading and
>>     writing from userspace through sysfs.
> 
> Would you please rename it to bitflip_threshold. It is bearable when it
> is just a  'struct mtd_info' member, but you also export
> 'euclean_threshold' sysfs file which is really confusing from user
> perspective, I think.


OK, maybe bitflip is the better choice.


>> So basically, the meaning of -EUCLEAN is changed from "one or more bit errors
>> were corrected over the entire read operation", to "a dangerously high number of
>> bit errors were corrected on one or more writesize regions".  By default,
>> "dangerously high" is interpreted as the maximum number of correctible bit
>> errors per writesize.  Drivers can specify a different value, and the user can
>> override it if more or less caution regarding data integrity is desired.
> 
> Please, make sure we have a good comment like this in the mtd.h file as
> well. I think the one you added is not verbose enough.


OK.  Usually I have the opposite problem - comments too verbose!


>> Patch #2 touches a lot of files, but they are small changes in most cases.  If
>> you can verify the correctness of the device's ecc strength, an ACK would be
>> much appreciated!
> 
> I'd be pro-active and just CC'ed maintainers/possibly relevant people.
> scripts/get_maintainer.pl would give you the ones, I think. Just spend a
> little time and come up with a list of people and CC them.


I actually did do this, and git-send-email kept rejecting it because of "too
many recipients" or some such.  After several iterations of trying to
intelligently whittle down the CC list and getting the same result, I got
frustrated and just CC'd Ivan.  (Hope you don't mind, Ivan :) Anyone know how
many recipients git allows?

I'll forward patch #2 along with a polite request for review to the original CC
list using my regular email client.  I'll also start putting together new
patches that address your comments.

Thanks again,
Mike



More information about the linux-mtd mailing list