Re: [PATCH 28/74] Incrementing the ecc_pos array to contain 128 char

[Ryan Mallon <ryan@bluewatersys.com>]

On 09/02/2010 11:37 AM, Ryan Mallon wrote:
> On 09/02/2010 10:53 AM, Artem Bityutskiy wrote:
>> On Thu, 2010-09-02 at 10:21 +1200, Ryan Mallon wrote:
>>> On 09/02/2010 09:54 AM, Kevin Cernekee wrote:
>>>> On Wed, Sep 1, 2010 at 2:23 PM, Ryan Mallon <ryan@bluewatersys.com> wrote:
>>>>> However, we are interested in having a proper solution to this problem.
>>>>
>>>> I would suggest using this patch as a starting point - could you
>>>> please review the changes and try it out?
>>>>
>>>> http://git.infradead.org/users/dedekind/l2-mtd-2.6.git/commitdiff/b6d6ae730be2750fac166ed9df11ee6ea54d9160
>>>
>>> That patch still breaks the ABI by renaming struct nand_ecclayout to
>>> nand_ecclayout_user.
>>
>> Hmm, would you please show why my binary file 'nandwrite' would have to
>> be re-built?
>>
>>>  Any application using the old names will have to be
>>> rewritten to compile against a new kernel.
>>
>> Not necessary. mtd-utils, for example, maintain their own copy of the
>> mtd-abi.h file, which I personally think is a good idea.
>>
>> But if your apps depend on the kernel headers, then yes, you will need
>> to amend it or copy the old header into your project.
>>
>> IOW, the patch breaks API by renaming, but not ABI, I think. I do not
>> see this as a major problem - just a small inconvenience.
> 
> Sorry, you are correct, it breaks the API not the ABI. I understood that
> such changes were still to be avoided.
> 
>>> The old interface should remain unchanged in that include/mtd/mtd-abi.h.
>>> If an application using the old interface calls any of the ecc ioctls
>>> for a nand chip with > 64 bytes ecc it should return an error.
>>
>> It will, because size of the structure is part of the ioctl number,
>> even. See the corresponding ioctl macro definition.
> 
> Right, but I don't think it needs the complexity of the shrink_ecc
> function from Kevin's patch. The old ioctl should return an error
> (possibly with a kernel message) if the requested nand chip has > 64
> bytes ecc. Otherwise it should just copy upto 64 bytes into the old
> nand_ecclayout structure and return.

Oops, I misread the patch. The shrink_ecclayout function is actually
fine. The only change I would make is to possible add a warning/error if
the data is being truncated.

~Ryan

-- 
Bluewater Systems Ltd - ARM Technology Solution Centre

Ryan Mallon         		5 Amuri Park, 404 Barbadoes St
ryan@bluewatersys.com         	PO Box 13 889, Christchurch 8013
http://www.bluewatersys.com	New Zealand
Phone: +64 3 3779127		Freecall: Australia 1800 148 751
Fax:   +64 3 3779135			  USA 1800 261 2934