Commit Message

The Micron M29EW has a 256 byte one time programmable (OTP) memory.
This patch adds support for reading this memory. This support will be
extended for locking and writing in subsequent patches.
Signed-off-by: Christian Riesch <christian.riesch@omicron.at>
---
drivers/mtd/chips/cfi_cmdset_0002.c | 163 +++++++++++++++++++++++++++++++++++
1 file changed, 163 insertions(+)

Artem,
Thank you very much for your comments!
On 2013-05-29 09:11, Artem Bityutskiy wrote:
> On Fri, 2013-04-26 at 21:10 +0200, Christian Riesch wrote:>> +static int cfi_amdstd_get_fact_prot_info(struct mtd_info *mtd,>> + struct otp_info *buf,>> size_t len)>> +{>> + size_t retlen;>> + int ret;>> +>> + ret = cfi_amdstd_otp_walk(mtd, 0, len, &retlen, (u_char *)buf,>> NULL, 0);>> + return ret ? : retlen;>> +}>> Hmm, I thought we would return 0 in case of success and a negative error> code in case of error. Returning retlen looks wrong.>
I don't think so. Somehow the caller of this function should be told how
many bytes have been written into buf, there is no other way to return
the data length than the return value. Anyway, I just stole this code
from cfi_cmdset_0001.c (function cfi_intelext_get_fact_prot_info), it is
done in the same way there.
Christian

On Wed, 2013-05-29 at 14:54 +0200, Christian Riesch wrote:
> Artem,> Thank you very much for your comments!> > On 2013-05-29 09:11, Artem Bityutskiy wrote:> > On Fri, 2013-04-26 at 21:10 +0200, Christian Riesch wrote:> >> +static int cfi_amdstd_get_fact_prot_info(struct mtd_info *mtd,> >> + struct otp_info *buf,> >> size_t len)> >> +{> >> + size_t retlen;> >> + int ret;> >> +> >> + ret = cfi_amdstd_otp_walk(mtd, 0, len, &retlen, (u_char *)buf,> >> NULL, 0);> >> + return ret ? : retlen;> >> +}> >> > Hmm, I thought we would return 0 in case of success and a negative error> > code in case of error. Returning retlen looks wrong.> >> > I don't think so. Somehow the caller of this function should be told how > many bytes have been written into buf, there is no other way to return > the data length than the return value. Anyway, I just stole this code > from cfi_cmdset_0001.c (function cfi_intelext_get_fact_prot_info), it is > done in the same way there.
But retlen is where you return the amount of bytes you actually wrote.
The return code is either 0 or a negative error code number. This is
what we do in mtd_write() and I'd expect the OTP functions to be
consistent.

On 2013-05-29 15:53, Artem Bityutskiy wrote:
> On Wed, 2013-05-29 at 14:54 +0200, Christian Riesch wrote:>> Artem,>> Thank you very much for your comments!>>>> On 2013-05-29 09:11, Artem Bityutskiy wrote:>>> On Fri, 2013-04-26 at 21:10 +0200, Christian Riesch wrote:>>>> +static int cfi_amdstd_get_fact_prot_info(struct mtd_info *mtd,>>>> + struct otp_info *buf,>>>> size_t len)>>>> +{>>>> + size_t retlen;>>>> + int ret;>>>> +>>>> + ret = cfi_amdstd_otp_walk(mtd, 0, len, &retlen, (u_char *)buf,>>>> NULL, 0);>>>> + return ret ? : retlen;>>>> +}>>>>>> Hmm, I thought we would return 0 in case of success and a negative error>>> code in case of error. Returning retlen looks wrong.>>>>>>> I don't think so. Somehow the caller of this function should be told how>> many bytes have been written into buf, there is no other way to return>> the data length than the return value. Anyway, I just stole this code>> from cfi_cmdset_0001.c (function cfi_intelext_get_fact_prot_info), it is>> done in the same way there.>> But retlen is where you return the amount of bytes you actually wrote.> The return code is either 0 or a negative error code number. This is> what we do in mtd_write() and I'd expect the OTP functions to be> consistent.
Unlike mtd_write, mtd->_get_fact_prot_info (which is a pointer to
cfi_amdstd_get_fact_prot_info here) does not have a retlen parameter. Do
you suggest to change this?
Regards, Christian

On Wed, 2013-05-29 at 16:36 +0200, Christian Riesch wrote:
> > But retlen is where you return the amount of bytes you actually> wrote.> > The return code is either 0 or a negative error code number. This is> > what we do in mtd_write() and I'd expect the OTP functions to be> > consistent.> > Unlike mtd_write, mtd->_get_fact_prot_info (which is a pointer to > cfi_amdstd_get_fact_prot_info here) does not have a retlen parameter.> Do > you suggest to change this?
Yes, I would suggest this.

On 2013-06-03 11:40, Artem Bityutskiy wrote:
> On Wed, 2013-05-29 at 16:36 +0200, Christian Riesch wrote:>>> But retlen is where you return the amount of bytes you actually>> wrote.>>> The return code is either 0 or a negative error code number. This is>>> what we do in mtd_write() and I'd expect the OTP functions to be>>> consistent.>>>> Unlike mtd_write, mtd->_get_fact_prot_info (which is a pointer to>> cfi_amdstd_get_fact_prot_info here) does not have a retlen parameter.>> Do>> you suggest to change this?>> Yes, I would suggest this.>
Ok, I will prepare a patch for this.
Thanks, Christian