| Message ID | 1416588884-21701-2-git-send-email-hector.palacios@digi.com |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Stefano Babic |
| Headers | show |
Hi Hector, On Fri, Nov 21, 2014 at 2:54 PM, Hector Palacios <hector.palacios@digi.com> wrote: > The write operation may fail when trying to write to a locked area. In > this case the ERROR bit is set in the CTRL register. Check for that > condition and return an error. > > Signed-off-by: Hector Palacios <hector.palacios@digi.com> > --- > drivers/misc/mxs_ocotp.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/misc/mxs_ocotp.c b/drivers/misc/mxs_ocotp.c > index 09002814f2f0..1659ee6a5eec 100644 > --- a/drivers/misc/mxs_ocotp.c > +++ b/drivers/misc/mxs_ocotp.c > @@ -221,6 +221,13 @@ static int mxs_ocotp_write_fuse(uint32_t addr, uint32_t mask) > goto fail; > } > > + /* Check for errors */ > + if (readl(&ocotp_regs->hw_ocotp_ctrl) & OCOTP_CTRL_ERROR) { > + puts("Failed writing fuses!\n"); > + ret = -EPERM; > + goto fail; > + } > + > fail: What about doing this instead? /* Check for errors */ ret = readl(&ocotp_regs->hw_ocotp_ctrl) & OCOTP_CTRL_ERROR); if (ret) { printfs("Failed writing the fuses: %d\n", ret); goto fail; }
Hi Fabio, On 11/21/2014 06:10 PM, Fabio Estevam wrote: > Hi Hector, > > On Fri, Nov 21, 2014 at 2:54 PM, Hector Palacios > <hector.palacios@digi.com> wrote: >> The write operation may fail when trying to write to a locked area. In >> this case the ERROR bit is set in the CTRL register. Check for that >> condition and return an error. >> >> Signed-off-by: Hector Palacios <hector.palacios@digi.com> >> --- >> drivers/misc/mxs_ocotp.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/drivers/misc/mxs_ocotp.c b/drivers/misc/mxs_ocotp.c >> index 09002814f2f0..1659ee6a5eec 100644 >> --- a/drivers/misc/mxs_ocotp.c >> +++ b/drivers/misc/mxs_ocotp.c >> @@ -221,6 +221,13 @@ static int mxs_ocotp_write_fuse(uint32_t addr, uint32_t mask) >> goto fail; >> } >> >> + /* Check for errors */ >> + if (readl(&ocotp_regs->hw_ocotp_ctrl) & OCOTP_CTRL_ERROR) { >> + puts("Failed writing fuses!\n"); >> + ret = -EPERM; >> + goto fail; >> + } >> + >> fail: > > What about doing this instead? > > /* Check for errors */ > ret = readl(&ocotp_regs->hw_ocotp_ctrl) & OCOTP_CTRL_ERROR); > if (ret) { > printfs("Failed writing the fuses: %d\n", ret); > goto fail; > } > Although the code looks nicer you are not returning a meaningful error code, just a mask value (and yes, I know the error code does not get anywhere, but still). -- Hector Palacios PS. Sorry about the resending of the message but the mailing list server kept rejecting it.
On Fri, Nov 21, 2014 at 4:04 PM, Hector Palacios <hector.palacios@digi.com> wrote: >> What about doing this instead? >> >> /* Check for errors */ >> ret = readl(&ocotp_regs->hw_ocotp_ctrl) & OCOTP_CTRL_ERROR); >> if (ret) { >> printfs("Failed writing the fuses: %d\n", ret); >> goto fail; >> } >> > > Although the code looks nicer you are not returning a meaningful error code, just a > mask value (and yes, I know the error code does not get anywhere, but still). I am just returning the real error code, not a 'fake' one :-)
On Sat, Nov 22, 2014 at 5:29 PM, Fabio Estevam <festevam@gmail.com> wrote: > On Fri, Nov 21, 2014 at 4:04 PM, Hector Palacios > <hector.palacios@digi.com> wrote: > >>> What about doing this instead? >>> >>> /* Check for errors */ >>> ret = readl(&ocotp_regs->hw_ocotp_ctrl) & OCOTP_CTRL_ERROR); >>> if (ret) { >>> printfs("Failed writing the fuses: %d\n", ret); >>> goto fail; >>> } >>> >> >> Although the code looks nicer you are not returning a meaningful error code, just a >> mask value (and yes, I know the error code does not get anywhere, but still). > > I am just returning the real error code, not a 'fake' one :-) Actually your code is correct and I misread it. Sorry about that.
On Fri, Nov 21, 2014 at 2:54 PM, Hector Palacios <hector.palacios@digi.com> wrote: > The write operation may fail when trying to write to a locked area. In > this case the ERROR bit is set in the CTRL register. Check for that > condition and return an error. > > Signed-off-by: Hector Palacios <hector.palacios@digi.com> Reviewed-by: Fabio Estevam <fabio.estevam@freescale.com>
On 21/11/2014 17:54, Hector Palacios wrote: > The write operation may fail when trying to write to a locked area. In > this case the ERROR bit is set in the CTRL register. Check for that > condition and return an error. > > Signed-off-by: Hector Palacios <hector.palacios@digi.com> > --- > drivers/misc/mxs_ocotp.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/misc/mxs_ocotp.c b/drivers/misc/mxs_ocotp.c > index 09002814f2f0..1659ee6a5eec 100644 > --- a/drivers/misc/mxs_ocotp.c > +++ b/drivers/misc/mxs_ocotp.c > @@ -221,6 +221,13 @@ static int mxs_ocotp_write_fuse(uint32_t addr, uint32_t mask) > goto fail; > } > > + /* Check for errors */ > + if (readl(&ocotp_regs->hw_ocotp_ctrl) & OCOTP_CTRL_ERROR) { > + puts("Failed writing fuses!\n"); > + ret = -EPERM; > + goto fail; > + } > + > fail: > mxs_ocotp_scale_vddio(0, &vddio_val); > if (mxs_ocotp_scale_hclk(0, &hclk_val)) > Applied to u-boot-imx, thanks ! Best regards, Stefano Babic
diff --git a/drivers/misc/mxs_ocotp.c b/drivers/misc/mxs_ocotp.c index 09002814f2f0..1659ee6a5eec 100644 --- a/drivers/misc/mxs_ocotp.c +++ b/drivers/misc/mxs_ocotp.c @@ -221,6 +221,13 @@ static int mxs_ocotp_write_fuse(uint32_t addr, uint32_t mask) goto fail; } + /* Check for errors */ + if (readl(&ocotp_regs->hw_ocotp_ctrl) & OCOTP_CTRL_ERROR) { + puts("Failed writing fuses!\n"); + ret = -EPERM; + goto fail; + } + fail: mxs_ocotp_scale_vddio(0, &vddio_val); if (mxs_ocotp_scale_hclk(0, &hclk_val))
The write operation may fail when trying to write to a locked area. In this case the ERROR bit is set in the CTRL register. Check for that condition and return an error. Signed-off-by: Hector Palacios <hector.palacios@digi.com> --- drivers/misc/mxs_ocotp.c | 7 +++++++ 1 file changed, 7 insertions(+)