| Message ID | 1450256509-17280-3-git-send-email-jteki@openedev.com |
|---|---|
| State | Superseded |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
On Wednesday 16 December 2015 02:31 PM, Jagan Teki wrote: > One macronix quad bit set using SR, it's good to > read back and check the written bit and also if > it's already been set check for the bit and return. > > Cc: Vignesh R <vigneshr@ti.com> > Cc: Mugunthan V N <mugunthanvnm@ti.com> > Cc: Simon Glass <sjg@chromium.org> > Cc: Bin Meng <bmeng.cn@gmail.com> > Signed-off-by: Jagan Teki <jteki@openedev.com> > --- > Changes for v2: > - none > > drivers/mtd/spi/spi_flash.c | 18 ++++++++++++------ > 1 file changed, 12 insertions(+), 6 deletions(-) > > diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c > index ba6651e..c922322 100644 > --- a/drivers/mtd/spi/spi_flash.c > +++ b/drivers/mtd/spi/spi_flash.c > @@ -840,12 +840,18 @@ static int macronix_quad_enable(struct spi_flash *flash) > if (ret < 0) > return ret; > > - if (qeb_status & STATUS_QEB_MXIC) { > - debug("SF: mxic: QEB is already set\n"); > - } else { > - ret = write_sr(flash, STATUS_QEB_MXIC); > - if (ret < 0) > - return ret; > + if (qeb_status & STATUS_QEB_MXIC) > + return 0; > + > + ret = write_sr(flash, STATUS_QEB_MXIC); > + if (ret < 0) > + return ret; > + > + /* read SR and check it */ > + ret = read_sr(flash, &qeb_status); > + if (!(ret > 0 && (qeb_status & STATUS_QEB_MXIC))) { This error check is wrong, it can be either one of the below if (!(ret >= 0 && (qeb_status & STATUS_QEB_MXIC))) { or if (ret < 0 || !(qeb_status & STATUS_QEB_MXIC)) { Regards Mugunthan V N > + printf("SF: Macronix SR Quad bit not clear\n"); > + return -EINVAL; > } > > return ret; >
On 23 December 2015 at 13:22, Mugunthan V N <mugunthanvnm@ti.com> wrote: > On Wednesday 16 December 2015 02:31 PM, Jagan Teki wrote: >> One macronix quad bit set using SR, it's good to >> read back and check the written bit and also if >> it's already been set check for the bit and return. >> >> Cc: Vignesh R <vigneshr@ti.com> >> Cc: Mugunthan V N <mugunthanvnm@ti.com> >> Cc: Simon Glass <sjg@chromium.org> >> Cc: Bin Meng <bmeng.cn@gmail.com> >> Signed-off-by: Jagan Teki <jteki@openedev.com> >> --- >> Changes for v2: >> - none >> >> drivers/mtd/spi/spi_flash.c | 18 ++++++++++++------ >> 1 file changed, 12 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c >> index ba6651e..c922322 100644 >> --- a/drivers/mtd/spi/spi_flash.c >> +++ b/drivers/mtd/spi/spi_flash.c >> @@ -840,12 +840,18 @@ static int macronix_quad_enable(struct spi_flash *flash) >> if (ret < 0) >> return ret; >> >> - if (qeb_status & STATUS_QEB_MXIC) { >> - debug("SF: mxic: QEB is already set\n"); >> - } else { >> - ret = write_sr(flash, STATUS_QEB_MXIC); >> - if (ret < 0) >> - return ret; >> + if (qeb_status & STATUS_QEB_MXIC) >> + return 0; >> + >> + ret = write_sr(flash, STATUS_QEB_MXIC); >> + if (ret < 0) >> + return ret; >> + >> + /* read SR and check it */ >> + ret = read_sr(flash, &qeb_status); >> + if (!(ret > 0 && (qeb_status & STATUS_QEB_MXIC))) { > > This error check is wrong, it can be either one of the below > > if (!(ret >= 0 && (qeb_status & STATUS_QEB_MXIC))) { So =0 is a success case, this is what you pointing to correct? did you test this? > or > if (ret < 0 || !(qeb_status & STATUS_QEB_MXIC)) { > > Regards > Mugunthan V N > >> + printf("SF: Macronix SR Quad bit not clear\n"); >> + return -EINVAL; >> } >> >> return ret; thanks!
On Wednesday 23 December 2015 01:51 PM, Jagan Teki wrote: > On 23 December 2015 at 13:22, Mugunthan V N <mugunthanvnm@ti.com> wrote: >> On Wednesday 16 December 2015 02:31 PM, Jagan Teki wrote: >>> One macronix quad bit set using SR, it's good to >>> read back and check the written bit and also if >>> it's already been set check for the bit and return. >>> >>> Cc: Vignesh R <vigneshr@ti.com> >>> Cc: Mugunthan V N <mugunthanvnm@ti.com> >>> Cc: Simon Glass <sjg@chromium.org> >>> Cc: Bin Meng <bmeng.cn@gmail.com> >>> Signed-off-by: Jagan Teki <jteki@openedev.com> >>> --- >>> Changes for v2: >>> - none >>> >>> drivers/mtd/spi/spi_flash.c | 18 ++++++++++++------ >>> 1 file changed, 12 insertions(+), 6 deletions(-) >>> >>> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c >>> index ba6651e..c922322 100644 >>> --- a/drivers/mtd/spi/spi_flash.c >>> +++ b/drivers/mtd/spi/spi_flash.c >>> @@ -840,12 +840,18 @@ static int macronix_quad_enable(struct spi_flash *flash) >>> if (ret < 0) >>> return ret; >>> >>> - if (qeb_status & STATUS_QEB_MXIC) { >>> - debug("SF: mxic: QEB is already set\n"); >>> - } else { >>> - ret = write_sr(flash, STATUS_QEB_MXIC); >>> - if (ret < 0) >>> - return ret; >>> + if (qeb_status & STATUS_QEB_MXIC) >>> + return 0; >>> + >>> + ret = write_sr(flash, STATUS_QEB_MXIC); >>> + if (ret < 0) >>> + return ret; >>> + >>> + /* read SR and check it */ >>> + ret = read_sr(flash, &qeb_status); >>> + if (!(ret > 0 && (qeb_status & STATUS_QEB_MXIC))) { >> >> This error check is wrong, it can be either one of the below >> >> if (!(ret >= 0 && (qeb_status & STATUS_QEB_MXIC))) { > > So =0 is a success case, this is what you pointing to correct? did you > test this? Yeah, I have tested this on am437x-sk evm. The same fix has to be done for spansion flash as well! Regards Mugunthan V N
diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c index ba6651e..c922322 100644 --- a/drivers/mtd/spi/spi_flash.c +++ b/drivers/mtd/spi/spi_flash.c @@ -840,12 +840,18 @@ static int macronix_quad_enable(struct spi_flash *flash) if (ret < 0) return ret; - if (qeb_status & STATUS_QEB_MXIC) { - debug("SF: mxic: QEB is already set\n"); - } else { - ret = write_sr(flash, STATUS_QEB_MXIC); - if (ret < 0) - return ret; + if (qeb_status & STATUS_QEB_MXIC) + return 0; + + ret = write_sr(flash, STATUS_QEB_MXIC); + if (ret < 0) + return ret; + + /* read SR and check it */ + ret = read_sr(flash, &qeb_status); + if (!(ret > 0 && (qeb_status & STATUS_QEB_MXIC))) { + printf("SF: Macronix SR Quad bit not clear\n"); + return -EINVAL; } return ret;
One macronix quad bit set using SR, it's good to read back and check the written bit and also if it's already been set check for the bit and return. Cc: Vignesh R <vigneshr@ti.com> Cc: Mugunthan V N <mugunthanvnm@ti.com> Cc: Simon Glass <sjg@chromium.org> Cc: Bin Meng <bmeng.cn@gmail.com> Signed-off-by: Jagan Teki <jteki@openedev.com> --- Changes for v2: - none drivers/mtd/spi/spi_flash.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)