Re: [PATCH] mtd: cqspi: Wait for transfer completion

2021-12-02 Thread Pratyush Yadav
Hi Marek,

On 25/10/21 10:25PM, Marek Vasut wrote:
> On 10/25/21 9:53 PM, Pratyush Yadav wrote:
> > On 08/10/21 06:06PM, Jagan Teki wrote:
> > > On Wed, Sep 15, 2021 at 2:05 PM Marek Vasut  wrote:
> > > > 
> > > > On 9/15/21 10:28 AM, Pratyush Yadav wrote:
> > > > > On 14/09/21 08:22PM, Marek Vasut wrote:
> > > > > > On 9/14/21 7:42 PM, Pratyush Yadav wrote:
> > > > > > > On 14/09/21 05:22AM, Marek Vasut wrote:
> > > > > > > > Wait for the read/write transfer finish bit get actually 
> > > > > > > > cleared,
> > > > > > > > this does not happen immediately on at least SoCFPGA Gen5.
> > > > > > > > 
> > > > > > > > Signed-off-by: Marek Vasut 
> > > > > > > > Cc: Jagan Teki 
> > > > > > > > Cc: Vignesh R 
> > > > > > > > Cc: Pratyush Yadav 
> > > > > > > > ---
> > > > > > > > drivers/spi/cadence_qspi_apb.c | 17 +
> > > > > > > > 1 file changed, 17 insertions(+)
> > > > > > > > 
> > > > > > > > diff --git a/drivers/spi/cadence_qspi_apb.c 
> > > > > > > > b/drivers/spi/cadence_qspi_apb.c
> > > > > > > > index 429ee335db6..2cdf4c9c9f8 100644
> > > > > > > > --- a/drivers/spi/cadence_qspi_apb.c
> > > > > > > > +++ b/drivers/spi/cadence_qspi_apb.c
> > > > > > > > @@ -858,6 +858,14 @@ 
> > > > > > > > cadence_qspi_apb_indirect_read_execute(struct cadence_spi_plat 
> > > > > > > > *plat,
> > > > > > > > writel(CQSPI_REG_INDIRECTRD_DONE,
> > > > > > > >plat->regbase + CQSPI_REG_INDIRECTRD);
> > > > > > > > +  /* Check indirect done status */
> > > > > > > > +  ret = wait_for_bit_le32(plat->regbase + CQSPI_REG_INDIRECTRD,
> > > > > > > > +  CQSPI_REG_INDIRECTRD_DONE, 0, 10, 0);
> > > > > > > > +  if (ret) {
> > > > > > > > +  printf("Indirect read clear completion error 
> > > > > > > > (%i)\n", ret);
> > > > > > > > +  goto failrd;
> > > > > > > > +  }
> > > > > > > > +
> > > > > > > 
> > > > > > > Huh, this is strange. I would expect the bit to clear immediately 
> > > > > > > since
> > > > > > > it doesn't really do any operation on the flash. How long does it
> > > > > > > usually take to clear? If you don't wait for it to clear does 
> > > > > > > anything
> > > > > > > break?
> > > > > > 
> > > > > > Often it does clear immediately, but there were a few odd cases 
> > > > > > where it did
> > > > > > not happen right away, in which case I had transfer corruption.
> > > > > 
> > > > > By "transfer corruption" do you mean the current transfer gets 
> > > > > corrupted
> > > > > or the next one?
> > > > > 
> > > > > We get here _after_ the transfer is completed and this bit should have
> > > > > little to do with the data received. If the current transfer fails 
> > > > > then
> > > > > I suspect something else might be going wrong the this is just a 
> > > > > symptom
> > > > > of the problem.
> > > > 
> > > > As far as I recall, the problem was triggered when using UBI on the SPI
> > > > NOR, so that could very well be the next transfer.
> > > 
> > > Any further decisions here? shall I take it or v2?
> > 
> > I think we need more information here. I don't see why checking this bit
> > would interfere with the current transfer since that is already finished
> > by the time we get here. If it is the next transfer then this might just
> > be a symptom and the real problem might be somewhere else.
> 
> What additional information do you need ?

Sorry for the late reply. I dropped the ball on this one.

The additional information I needed was to know what exactly happens if 
we don't wait for this bit to be cleared. Does the _current_ transfer go 
though fine? If it does, what gets corrupted in the next transfer? Do 
you only get partial data? Do you get garbage data? Do you get no data 
at all?

All this information would be useful for understanding if this fix is 
the right one, and if so why it is the right one.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.


Re: [PATCH] mtd: cqspi: Wait for transfer completion

2021-12-01 Thread Jagan Teki
On Thu, Dec 2, 2021 at 11:20 AM Marek Vasut  wrote:
>
> On 12/2/21 06:48, Jagan Teki wrote:
> > Hi Marek,
> >
> > On Tue, Sep 14, 2021 at 8:52 AM Marek Vasut  wrote:
> >>
> >> Wait for the read/write transfer finish bit get actually cleared,
> >> this does not happen immediately on at least SoCFPGA Gen5.
> >>
> >> Signed-off-by: Marek Vasut 
> >> Cc: Jagan Teki 
> >> Cc: Vignesh R 
> >> Cc: Pratyush Yadav 
> >> ---
> >>   drivers/spi/cadence_qspi_apb.c | 17 +
> >>   1 file changed, 17 insertions(+)
> >>
> >> diff --git a/drivers/spi/cadence_qspi_apb.c 
> >> b/drivers/spi/cadence_qspi_apb.c
> >> index 429ee335db6..2cdf4c9c9f8 100644
> >> --- a/drivers/spi/cadence_qspi_apb.c
> >> +++ b/drivers/spi/cadence_qspi_apb.c
> >> @@ -858,6 +858,14 @@ cadence_qspi_apb_indirect_read_execute(struct 
> >> cadence_spi_plat *plat,
> >>  writel(CQSPI_REG_INDIRECTRD_DONE,
> >> plat->regbase + CQSPI_REG_INDIRECTRD);
> >>
> >> +   /* Check indirect done status */
> >> +   ret = wait_for_bit_le32(plat->regbase + CQSPI_REG_INDIRECTRD,
> >> +   CQSPI_REG_INDIRECTRD_DONE, 0, 10, 0);
> >> +   if (ret) {
> >> +   printf("Indirect read clear completion error (%i)\n", ret);
> >> +   goto failrd;
> >> +   }
> >> +
> >>  return 0;
> >>
> >>   failrd:
> >> @@ -1012,6 +1020,15 @@ cadence_qspi_apb_indirect_write_execute(struct 
> >> cadence_spi_plat *plat,
> >>  /* Clear indirect completion status */
> >>  writel(CQSPI_REG_INDIRECTWR_DONE,
> >> plat->regbase + CQSPI_REG_INDIRECTWR);
> >> +
> >> +   /* Check indirect done status */
> >> +   ret = wait_for_bit_le32(plat->regbase + CQSPI_REG_INDIRECTWR,
> >> +   CQSPI_REG_INDIRECTWR_DONE, 0, 10, 0);
> >> +   if (ret) {
> >> +   printf("Indirect write clear completion error (%i)\n", 
> >> ret);
> >> +   goto failwr;
> >> +   }
> >> +
> >
> > Does this patch to be part of the release?
>
> Yes, this fix was posted way before the 2022.01 MR even opened, it can
> be added.

Yeah. was delayed. in fact I was waited response from Pratyush
previous comments. I will send the PR anyway.

Thanks,
Jagan.


Re: [PATCH] mtd: cqspi: Wait for transfer completion

2021-12-01 Thread Marek Vasut

On 12/2/21 06:48, Jagan Teki wrote:

Hi Marek,

On Tue, Sep 14, 2021 at 8:52 AM Marek Vasut  wrote:


Wait for the read/write transfer finish bit get actually cleared,
this does not happen immediately on at least SoCFPGA Gen5.

Signed-off-by: Marek Vasut 
Cc: Jagan Teki 
Cc: Vignesh R 
Cc: Pratyush Yadav 
---
  drivers/spi/cadence_qspi_apb.c | 17 +
  1 file changed, 17 insertions(+)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index 429ee335db6..2cdf4c9c9f8 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -858,6 +858,14 @@ cadence_qspi_apb_indirect_read_execute(struct 
cadence_spi_plat *plat,
 writel(CQSPI_REG_INDIRECTRD_DONE,
plat->regbase + CQSPI_REG_INDIRECTRD);

+   /* Check indirect done status */
+   ret = wait_for_bit_le32(plat->regbase + CQSPI_REG_INDIRECTRD,
+   CQSPI_REG_INDIRECTRD_DONE, 0, 10, 0);
+   if (ret) {
+   printf("Indirect read clear completion error (%i)\n", ret);
+   goto failrd;
+   }
+
 return 0;

  failrd:
@@ -1012,6 +1020,15 @@ cadence_qspi_apb_indirect_write_execute(struct 
cadence_spi_plat *plat,
 /* Clear indirect completion status */
 writel(CQSPI_REG_INDIRECTWR_DONE,
plat->regbase + CQSPI_REG_INDIRECTWR);
+
+   /* Check indirect done status */
+   ret = wait_for_bit_le32(plat->regbase + CQSPI_REG_INDIRECTWR,
+   CQSPI_REG_INDIRECTWR_DONE, 0, 10, 0);
+   if (ret) {
+   printf("Indirect write clear completion error (%i)\n", ret);
+   goto failwr;
+   }
+


Does this patch to be part of the release?


Yes, this fix was posted way before the 2022.01 MR even opened, it can 
be added.


Re: [PATCH] mtd: cqspi: Wait for transfer completion

2021-12-01 Thread Jagan Teki
Hi Marek,

On Tue, Sep 14, 2021 at 8:52 AM Marek Vasut  wrote:
>
> Wait for the read/write transfer finish bit get actually cleared,
> this does not happen immediately on at least SoCFPGA Gen5.
>
> Signed-off-by: Marek Vasut 
> Cc: Jagan Teki 
> Cc: Vignesh R 
> Cc: Pratyush Yadav 
> ---
>  drivers/spi/cadence_qspi_apb.c | 17 +
>  1 file changed, 17 insertions(+)
>
> diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
> index 429ee335db6..2cdf4c9c9f8 100644
> --- a/drivers/spi/cadence_qspi_apb.c
> +++ b/drivers/spi/cadence_qspi_apb.c
> @@ -858,6 +858,14 @@ cadence_qspi_apb_indirect_read_execute(struct 
> cadence_spi_plat *plat,
> writel(CQSPI_REG_INDIRECTRD_DONE,
>plat->regbase + CQSPI_REG_INDIRECTRD);
>
> +   /* Check indirect done status */
> +   ret = wait_for_bit_le32(plat->regbase + CQSPI_REG_INDIRECTRD,
> +   CQSPI_REG_INDIRECTRD_DONE, 0, 10, 0);
> +   if (ret) {
> +   printf("Indirect read clear completion error (%i)\n", ret);
> +   goto failrd;
> +   }
> +
> return 0;
>
>  failrd:
> @@ -1012,6 +1020,15 @@ cadence_qspi_apb_indirect_write_execute(struct 
> cadence_spi_plat *plat,
> /* Clear indirect completion status */
> writel(CQSPI_REG_INDIRECTWR_DONE,
>plat->regbase + CQSPI_REG_INDIRECTWR);
> +
> +   /* Check indirect done status */
> +   ret = wait_for_bit_le32(plat->regbase + CQSPI_REG_INDIRECTWR,
> +   CQSPI_REG_INDIRECTWR_DONE, 0, 10, 0);
> +   if (ret) {
> +   printf("Indirect write clear completion error (%i)\n", ret);
> +   goto failwr;
> +   }
> +

Does this patch to be part of the release?

Jagan.


Re: [PATCH] mtd: cqspi: Wait for transfer completion

2021-11-01 Thread Jagan Teki
Hi Pratyush,

On Tue, Oct 26, 2021 at 1:23 AM Pratyush Yadav  wrote:
>
> On 08/10/21 06:06PM, Jagan Teki wrote:
> > On Wed, Sep 15, 2021 at 2:05 PM Marek Vasut  wrote:
> > >
> > > On 9/15/21 10:28 AM, Pratyush Yadav wrote:
> > > > On 14/09/21 08:22PM, Marek Vasut wrote:
> > > >> On 9/14/21 7:42 PM, Pratyush Yadav wrote:
> > > >>> On 14/09/21 05:22AM, Marek Vasut wrote:
> > >  Wait for the read/write transfer finish bit get actually cleared,
> > >  this does not happen immediately on at least SoCFPGA Gen5.
> > > 
> > >  Signed-off-by: Marek Vasut 
> > >  Cc: Jagan Teki 
> > >  Cc: Vignesh R 
> > >  Cc: Pratyush Yadav 
> > >  ---
> > > drivers/spi/cadence_qspi_apb.c | 17 +
> > > 1 file changed, 17 insertions(+)
> > > 
> > >  diff --git a/drivers/spi/cadence_qspi_apb.c 
> > >  b/drivers/spi/cadence_qspi_apb.c
> > >  index 429ee335db6..2cdf4c9c9f8 100644
> > >  --- a/drivers/spi/cadence_qspi_apb.c
> > >  +++ b/drivers/spi/cadence_qspi_apb.c
> > >  @@ -858,6 +858,14 @@ cadence_qspi_apb_indirect_read_execute(struct 
> > >  cadence_spi_plat *plat,
> > > writel(CQSPI_REG_INDIRECTRD_DONE,
> > >    plat->regbase + CQSPI_REG_INDIRECTRD);
> > >  +  /* Check indirect done status */
> > >  +  ret = wait_for_bit_le32(plat->regbase + CQSPI_REG_INDIRECTRD,
> > >  +  CQSPI_REG_INDIRECTRD_DONE, 0, 10, 0);
> > >  +  if (ret) {
> > >  +  printf("Indirect read clear completion error (%i)\n", 
> > >  ret);
> > >  +  goto failrd;
> > >  +  }
> > >  +
> > > >>>
> > > >>> Huh, this is strange. I would expect the bit to clear immediately 
> > > >>> since
> > > >>> it doesn't really do any operation on the flash. How long does it
> > > >>> usually take to clear? If you don't wait for it to clear does anything
> > > >>> break?
> > > >>
> > > >> Often it does clear immediately, but there were a few odd cases where 
> > > >> it did
> > > >> not happen right away, in which case I had transfer corruption.
> > > >
> > > > By "transfer corruption" do you mean the current transfer gets corrupted
> > > > or the next one?
> > > >
> > > > We get here _after_ the transfer is completed and this bit should have
> > > > little to do with the data received. If the current transfer fails then
> > > > I suspect something else might be going wrong the this is just a symptom
> > > > of the problem.
> > >
> > > As far as I recall, the problem was triggered when using UBI on the SPI
> > > NOR, so that could very well be the next transfer.
> >
> > Any further decisions here? shall I take it or v2?
>
> I think we need more information here. I don't see why checking this bit
> would interfere with the current transfer since that is already finished
> by the time we get here. If it is the next transfer then this might just
> be a symptom and the real problem might be somewhere else.

Do you mean the bit is using on the patch is not proper for all
transfers? did you find any issues on your tests some where? please
indicate what's wrong?

Jagan.


Re: [PATCH] mtd: cqspi: Wait for transfer completion

2021-10-25 Thread Marek Vasut

On 10/25/21 9:53 PM, Pratyush Yadav wrote:

On 08/10/21 06:06PM, Jagan Teki wrote:

On Wed, Sep 15, 2021 at 2:05 PM Marek Vasut  wrote:


On 9/15/21 10:28 AM, Pratyush Yadav wrote:

On 14/09/21 08:22PM, Marek Vasut wrote:

On 9/14/21 7:42 PM, Pratyush Yadav wrote:

On 14/09/21 05:22AM, Marek Vasut wrote:

Wait for the read/write transfer finish bit get actually cleared,
this does not happen immediately on at least SoCFPGA Gen5.

Signed-off-by: Marek Vasut 
Cc: Jagan Teki 
Cc: Vignesh R 
Cc: Pratyush Yadav 
---
drivers/spi/cadence_qspi_apb.c | 17 +
1 file changed, 17 insertions(+)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index 429ee335db6..2cdf4c9c9f8 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -858,6 +858,14 @@ cadence_qspi_apb_indirect_read_execute(struct 
cadence_spi_plat *plat,
writel(CQSPI_REG_INDIRECTRD_DONE,
   plat->regbase + CQSPI_REG_INDIRECTRD);
+  /* Check indirect done status */
+  ret = wait_for_bit_le32(plat->regbase + CQSPI_REG_INDIRECTRD,
+  CQSPI_REG_INDIRECTRD_DONE, 0, 10, 0);
+  if (ret) {
+  printf("Indirect read clear completion error (%i)\n", ret);
+  goto failrd;
+  }
+


Huh, this is strange. I would expect the bit to clear immediately since
it doesn't really do any operation on the flash. How long does it
usually take to clear? If you don't wait for it to clear does anything
break?


Often it does clear immediately, but there were a few odd cases where it did
not happen right away, in which case I had transfer corruption.


By "transfer corruption" do you mean the current transfer gets corrupted
or the next one?

We get here _after_ the transfer is completed and this bit should have
little to do with the data received. If the current transfer fails then
I suspect something else might be going wrong the this is just a symptom
of the problem.


As far as I recall, the problem was triggered when using UBI on the SPI
NOR, so that could very well be the next transfer.


Any further decisions here? shall I take it or v2?


I think we need more information here. I don't see why checking this bit
would interfere with the current transfer since that is already finished
by the time we get here. If it is the next transfer then this might just
be a symptom and the real problem might be somewhere else.


What additional information do you need ?


Re: [PATCH] mtd: cqspi: Wait for transfer completion

2021-10-25 Thread Pratyush Yadav
On 08/10/21 06:06PM, Jagan Teki wrote:
> On Wed, Sep 15, 2021 at 2:05 PM Marek Vasut  wrote:
> >
> > On 9/15/21 10:28 AM, Pratyush Yadav wrote:
> > > On 14/09/21 08:22PM, Marek Vasut wrote:
> > >> On 9/14/21 7:42 PM, Pratyush Yadav wrote:
> > >>> On 14/09/21 05:22AM, Marek Vasut wrote:
> >  Wait for the read/write transfer finish bit get actually cleared,
> >  this does not happen immediately on at least SoCFPGA Gen5.
> > 
> >  Signed-off-by: Marek Vasut 
> >  Cc: Jagan Teki 
> >  Cc: Vignesh R 
> >  Cc: Pratyush Yadav 
> >  ---
> > drivers/spi/cadence_qspi_apb.c | 17 +
> > 1 file changed, 17 insertions(+)
> > 
> >  diff --git a/drivers/spi/cadence_qspi_apb.c 
> >  b/drivers/spi/cadence_qspi_apb.c
> >  index 429ee335db6..2cdf4c9c9f8 100644
> >  --- a/drivers/spi/cadence_qspi_apb.c
> >  +++ b/drivers/spi/cadence_qspi_apb.c
> >  @@ -858,6 +858,14 @@ cadence_qspi_apb_indirect_read_execute(struct 
> >  cadence_spi_plat *plat,
> > writel(CQSPI_REG_INDIRECTRD_DONE,
> >    plat->regbase + CQSPI_REG_INDIRECTRD);
> >  +  /* Check indirect done status */
> >  +  ret = wait_for_bit_le32(plat->regbase + CQSPI_REG_INDIRECTRD,
> >  +  CQSPI_REG_INDIRECTRD_DONE, 0, 10, 0);
> >  +  if (ret) {
> >  +  printf("Indirect read clear completion error (%i)\n", ret);
> >  +  goto failrd;
> >  +  }
> >  +
> > >>>
> > >>> Huh, this is strange. I would expect the bit to clear immediately since
> > >>> it doesn't really do any operation on the flash. How long does it
> > >>> usually take to clear? If you don't wait for it to clear does anything
> > >>> break?
> > >>
> > >> Often it does clear immediately, but there were a few odd cases where it 
> > >> did
> > >> not happen right away, in which case I had transfer corruption.
> > >
> > > By "transfer corruption" do you mean the current transfer gets corrupted
> > > or the next one?
> > >
> > > We get here _after_ the transfer is completed and this bit should have
> > > little to do with the data received. If the current transfer fails then
> > > I suspect something else might be going wrong the this is just a symptom
> > > of the problem.
> >
> > As far as I recall, the problem was triggered when using UBI on the SPI
> > NOR, so that could very well be the next transfer.
> 
> Any further decisions here? shall I take it or v2?

I think we need more information here. I don't see why checking this bit 
would interfere with the current transfer since that is already finished 
by the time we get here. If it is the next transfer then this might just 
be a symptom and the real problem might be somewhere else.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.


Re: [PATCH] mtd: cqspi: Wait for transfer completion

2021-10-08 Thread Jagan Teki
On Wed, Sep 15, 2021 at 2:05 PM Marek Vasut  wrote:
>
> On 9/15/21 10:28 AM, Pratyush Yadav wrote:
> > On 14/09/21 08:22PM, Marek Vasut wrote:
> >> On 9/14/21 7:42 PM, Pratyush Yadav wrote:
> >>> On 14/09/21 05:22AM, Marek Vasut wrote:
>  Wait for the read/write transfer finish bit get actually cleared,
>  this does not happen immediately on at least SoCFPGA Gen5.
> 
>  Signed-off-by: Marek Vasut 
>  Cc: Jagan Teki 
>  Cc: Vignesh R 
>  Cc: Pratyush Yadav 
>  ---
> drivers/spi/cadence_qspi_apb.c | 17 +
> 1 file changed, 17 insertions(+)
> 
>  diff --git a/drivers/spi/cadence_qspi_apb.c 
>  b/drivers/spi/cadence_qspi_apb.c
>  index 429ee335db6..2cdf4c9c9f8 100644
>  --- a/drivers/spi/cadence_qspi_apb.c
>  +++ b/drivers/spi/cadence_qspi_apb.c
>  @@ -858,6 +858,14 @@ cadence_qspi_apb_indirect_read_execute(struct 
>  cadence_spi_plat *plat,
> writel(CQSPI_REG_INDIRECTRD_DONE,
>    plat->regbase + CQSPI_REG_INDIRECTRD);
>  +  /* Check indirect done status */
>  +  ret = wait_for_bit_le32(plat->regbase + CQSPI_REG_INDIRECTRD,
>  +  CQSPI_REG_INDIRECTRD_DONE, 0, 10, 0);
>  +  if (ret) {
>  +  printf("Indirect read clear completion error (%i)\n", ret);
>  +  goto failrd;
>  +  }
>  +
> >>>
> >>> Huh, this is strange. I would expect the bit to clear immediately since
> >>> it doesn't really do any operation on the flash. How long does it
> >>> usually take to clear? If you don't wait for it to clear does anything
> >>> break?
> >>
> >> Often it does clear immediately, but there were a few odd cases where it 
> >> did
> >> not happen right away, in which case I had transfer corruption.
> >
> > By "transfer corruption" do you mean the current transfer gets corrupted
> > or the next one?
> >
> > We get here _after_ the transfer is completed and this bit should have
> > little to do with the data received. If the current transfer fails then
> > I suspect something else might be going wrong the this is just a symptom
> > of the problem.
>
> As far as I recall, the problem was triggered when using UBI on the SPI
> NOR, so that could very well be the next transfer.

Any further decisions here? shall I take it or v2?

Jagan.


Re: [PATCH] mtd: cqspi: Wait for transfer completion

2021-09-15 Thread Marek Vasut

On 9/15/21 10:28 AM, Pratyush Yadav wrote:

On 14/09/21 08:22PM, Marek Vasut wrote:

On 9/14/21 7:42 PM, Pratyush Yadav wrote:

On 14/09/21 05:22AM, Marek Vasut wrote:

Wait for the read/write transfer finish bit get actually cleared,
this does not happen immediately on at least SoCFPGA Gen5.

Signed-off-by: Marek Vasut 
Cc: Jagan Teki 
Cc: Vignesh R 
Cc: Pratyush Yadav 
---
   drivers/spi/cadence_qspi_apb.c | 17 +
   1 file changed, 17 insertions(+)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index 429ee335db6..2cdf4c9c9f8 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -858,6 +858,14 @@ cadence_qspi_apb_indirect_read_execute(struct 
cadence_spi_plat *plat,
writel(CQSPI_REG_INDIRECTRD_DONE,
   plat->regbase + CQSPI_REG_INDIRECTRD);
+   /* Check indirect done status */
+   ret = wait_for_bit_le32(plat->regbase + CQSPI_REG_INDIRECTRD,
+   CQSPI_REG_INDIRECTRD_DONE, 0, 10, 0);
+   if (ret) {
+   printf("Indirect read clear completion error (%i)\n", ret);
+   goto failrd;
+   }
+


Huh, this is strange. I would expect the bit to clear immediately since
it doesn't really do any operation on the flash. How long does it
usually take to clear? If you don't wait for it to clear does anything
break?


Often it does clear immediately, but there were a few odd cases where it did
not happen right away, in which case I had transfer corruption.


By "transfer corruption" do you mean the current transfer gets corrupted
or the next one?

We get here _after_ the transfer is completed and this bit should have
little to do with the data received. If the current transfer fails then
I suspect something else might be going wrong the this is just a symptom
of the problem.


As far as I recall, the problem was triggered when using UBI on the SPI 
NOR, so that could very well be the next transfer.


Re: [PATCH] mtd: cqspi: Wait for transfer completion

2021-09-15 Thread Pratyush Yadav
On 14/09/21 08:22PM, Marek Vasut wrote:
> On 9/14/21 7:42 PM, Pratyush Yadav wrote:
> > On 14/09/21 05:22AM, Marek Vasut wrote:
> > > Wait for the read/write transfer finish bit get actually cleared,
> > > this does not happen immediately on at least SoCFPGA Gen5.
> > > 
> > > Signed-off-by: Marek Vasut 
> > > Cc: Jagan Teki 
> > > Cc: Vignesh R 
> > > Cc: Pratyush Yadav 
> > > ---
> > >   drivers/spi/cadence_qspi_apb.c | 17 +
> > >   1 file changed, 17 insertions(+)
> > > 
> > > diff --git a/drivers/spi/cadence_qspi_apb.c 
> > > b/drivers/spi/cadence_qspi_apb.c
> > > index 429ee335db6..2cdf4c9c9f8 100644
> > > --- a/drivers/spi/cadence_qspi_apb.c
> > > +++ b/drivers/spi/cadence_qspi_apb.c
> > > @@ -858,6 +858,14 @@ cadence_qspi_apb_indirect_read_execute(struct 
> > > cadence_spi_plat *plat,
> > >   writel(CQSPI_REG_INDIRECTRD_DONE,
> > >  plat->regbase + CQSPI_REG_INDIRECTRD);
> > > + /* Check indirect done status */
> > > + ret = wait_for_bit_le32(plat->regbase + CQSPI_REG_INDIRECTRD,
> > > + CQSPI_REG_INDIRECTRD_DONE, 0, 10, 0);
> > > + if (ret) {
> > > + printf("Indirect read clear completion error (%i)\n", ret);
> > > + goto failrd;
> > > + }
> > > +
> > 
> > Huh, this is strange. I would expect the bit to clear immediately since
> > it doesn't really do any operation on the flash. How long does it
> > usually take to clear? If you don't wait for it to clear does anything
> > break?
> 
> Often it does clear immediately, but there were a few odd cases where it did
> not happen right away, in which case I had transfer corruption.

By "transfer corruption" do you mean the current transfer gets corrupted 
or the next one?

We get here _after_ the transfer is completed and this bit should have 
little to do with the data received. If the current transfer fails then 
I suspect something else might be going wrong the this is just a symptom 
of the problem.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.


Re: [PATCH] mtd: cqspi: Wait for transfer completion

2021-09-14 Thread Marek Vasut

On 9/14/21 7:42 PM, Pratyush Yadav wrote:

On 14/09/21 05:22AM, Marek Vasut wrote:

Wait for the read/write transfer finish bit get actually cleared,
this does not happen immediately on at least SoCFPGA Gen5.

Signed-off-by: Marek Vasut 
Cc: Jagan Teki 
Cc: Vignesh R 
Cc: Pratyush Yadav 
---
  drivers/spi/cadence_qspi_apb.c | 17 +
  1 file changed, 17 insertions(+)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index 429ee335db6..2cdf4c9c9f8 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -858,6 +858,14 @@ cadence_qspi_apb_indirect_read_execute(struct 
cadence_spi_plat *plat,
writel(CQSPI_REG_INDIRECTRD_DONE,
   plat->regbase + CQSPI_REG_INDIRECTRD);
  
+	/* Check indirect done status */

+   ret = wait_for_bit_le32(plat->regbase + CQSPI_REG_INDIRECTRD,
+   CQSPI_REG_INDIRECTRD_DONE, 0, 10, 0);
+   if (ret) {
+   printf("Indirect read clear completion error (%i)\n", ret);
+   goto failrd;
+   }
+


Huh, this is strange. I would expect the bit to clear immediately since
it doesn't really do any operation on the flash. How long does it
usually take to clear? If you don't wait for it to clear does anything
break?


Often it does clear immediately, but there were a few odd cases where it 
did not happen right away, in which case I had transfer corruption.


Re: [PATCH] mtd: cqspi: Wait for transfer completion

2021-09-14 Thread Pratyush Yadav
On 14/09/21 05:22AM, Marek Vasut wrote:
> Wait for the read/write transfer finish bit get actually cleared,
> this does not happen immediately on at least SoCFPGA Gen5.
> 
> Signed-off-by: Marek Vasut 
> Cc: Jagan Teki 
> Cc: Vignesh R 
> Cc: Pratyush Yadav 
> ---
>  drivers/spi/cadence_qspi_apb.c | 17 +
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
> index 429ee335db6..2cdf4c9c9f8 100644
> --- a/drivers/spi/cadence_qspi_apb.c
> +++ b/drivers/spi/cadence_qspi_apb.c
> @@ -858,6 +858,14 @@ cadence_qspi_apb_indirect_read_execute(struct 
> cadence_spi_plat *plat,
>   writel(CQSPI_REG_INDIRECTRD_DONE,
>  plat->regbase + CQSPI_REG_INDIRECTRD);
>  
> + /* Check indirect done status */
> + ret = wait_for_bit_le32(plat->regbase + CQSPI_REG_INDIRECTRD,
> + CQSPI_REG_INDIRECTRD_DONE, 0, 10, 0);
> + if (ret) {
> + printf("Indirect read clear completion error (%i)\n", ret);
> + goto failrd;
> + }
> +

Huh, this is strange. I would expect the bit to clear immediately since 
it doesn't really do any operation on the flash. How long does it 
usually take to clear? If you don't wait for it to clear does anything 
break?

>   return 0;
>  
>  failrd:
> @@ -1012,6 +1020,15 @@ cadence_qspi_apb_indirect_write_execute(struct 
> cadence_spi_plat *plat,
>   /* Clear indirect completion status */
>   writel(CQSPI_REG_INDIRECTWR_DONE,
>  plat->regbase + CQSPI_REG_INDIRECTWR);
> +
> + /* Check indirect done status */
> + ret = wait_for_bit_le32(plat->regbase + CQSPI_REG_INDIRECTWR,
> + CQSPI_REG_INDIRECTWR_DONE, 0, 10, 0);
> + if (ret) {
> + printf("Indirect write clear completion error (%i)\n", ret);
> + goto failwr;
> + }
> +
>   if (bounce_buf)
>   free(bounce_buf);
>   return 0;
> -- 
> 2.33.0
> 

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.


[PATCH] mtd: cqspi: Wait for transfer completion

2021-09-13 Thread Marek Vasut
Wait for the read/write transfer finish bit get actually cleared,
this does not happen immediately on at least SoCFPGA Gen5.

Signed-off-by: Marek Vasut 
Cc: Jagan Teki 
Cc: Vignesh R 
Cc: Pratyush Yadav 
---
 drivers/spi/cadence_qspi_apb.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index 429ee335db6..2cdf4c9c9f8 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -858,6 +858,14 @@ cadence_qspi_apb_indirect_read_execute(struct 
cadence_spi_plat *plat,
writel(CQSPI_REG_INDIRECTRD_DONE,
   plat->regbase + CQSPI_REG_INDIRECTRD);
 
+   /* Check indirect done status */
+   ret = wait_for_bit_le32(plat->regbase + CQSPI_REG_INDIRECTRD,
+   CQSPI_REG_INDIRECTRD_DONE, 0, 10, 0);
+   if (ret) {
+   printf("Indirect read clear completion error (%i)\n", ret);
+   goto failrd;
+   }
+
return 0;
 
 failrd:
@@ -1012,6 +1020,15 @@ cadence_qspi_apb_indirect_write_execute(struct 
cadence_spi_plat *plat,
/* Clear indirect completion status */
writel(CQSPI_REG_INDIRECTWR_DONE,
   plat->regbase + CQSPI_REG_INDIRECTWR);
+
+   /* Check indirect done status */
+   ret = wait_for_bit_le32(plat->regbase + CQSPI_REG_INDIRECTWR,
+   CQSPI_REG_INDIRECTWR_DONE, 0, 10, 0);
+   if (ret) {
+   printf("Indirect write clear completion error (%i)\n", ret);
+   goto failwr;
+   }
+
if (bounce_buf)
free(bounce_buf);
return 0;
-- 
2.33.0