Re: [PATCH] mtd: mtdchar: Exit write loop when hitting end of OTP memory

[Artem Bityutskiy <dedekind1@gmail.com>]

On Wed, 2013-05-29 at 16:27 +0200, Christian Riesch wrote:
> [cc'ed the author of onenand otp support]
> 
> On 2013-05-29 15:56, Artem Bityutskiy wrote:
> > On Wed, 2013-05-29 at 15:19 +0200, Christian Riesch wrote:
> >> The OTP code for the AMD command set in my recent patchset is modeled
> >> after the existing code in drivers/mtd/chips/cfi_cmdset_0001.c.
> >> Therefore it has a ...walk() function that walks from chip to chip and
> >> tries to write as much data as possible into the OTP memories of these
> >> chips. Until the last iteration of this loop it does not know how much
> >> OTP memory is available. Therefore, a check for insufficient OTP memory
> >> and returning an error before writing any data is not possible.
> >>
> >> Of course I could change my code to obtain the available OTP memory
> >> before writing any data. But then the code in cfi_cmdset_0001.c would
> >> still suffer from this issue.
> >
> > Could you please check OneNAND and other drivers which implement OTP and
> > see whether they check for space availability?
> 
> mtd->_write_user_prot_reg is currently implemented by 
> drivers/mtd/chips/cfi_cmdset_0001.c, drivers/mtd/onenand/onenand_base.c, 
> and drivers/mtd/devices/mtd_dataflash.c.
> 
> mtd_dataflash checks if the offset is larger than 64 and returns -EINVAL 
> in this case. If offset <= 64, but offset + len > 64, len is decreased 
> to fit into the OTP memory, the number of actually written bytes is 
> returned. It therefore suffers from the same issue in mtdchar.c as the 
> Intel command set.
> 
> onenand seems to do some kind of length check (although I do not fully 
> understand why it does not include the offset in 'from' in this check if 
> the factory OTP is addressed), but if the data does not fit into the 
> memory it returns 0 instead of an error code, resulting in an infinite 
> loop as well.

Would you be able to harmonize the implementations and switch them all
to the interface which is consistent with normal read/write, i.e., has
the retlen parameter?
 
-- 
Best Regards,
Artem Bityutskiy