Re: [RESEND PATCH] Revert "spi: zynq_qspi: Use dummy buswidth in dummy byte calculation"

2023-04-27 Thread Michal Simek 




On 4/27/23 08:53, Stefan Herbrechtsmeier wrote:

From: Stefan Herbrechtsmeier 

This reverts commit e09784728689de7949d4cdd559a9590e0bfcc702. The
commit wrongly divides the dummy bytes by dummy bus width to calculate
the dummy bytes. The framework already converts the dummy cycles to the
number of bytes and the controller use the SPI flash command to
determine the dummy cycles via the address width.

Signed-off-by: Stefan Herbrechtsmeier 
Acked-by: Ashok Reddy Soma 

---

  drivers/spi/zynq_qspi.c | 10 ++
  1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/zynq_qspi.c b/drivers/spi/zynq_qspi.c
index 00e3ffcd1d..d1d4048966 100644
--- a/drivers/spi/zynq_qspi.c
+++ b/drivers/spi/zynq_qspi.c
@@ -676,7 +676,6 @@ static int zynq_qspi_exec_op(struct spi_slave *slave,
 const struct spi_mem_op *op)
  {
int op_len, pos = 0, ret, i;
-   u32 dummy_bytes = 0;
unsigned int flag = 0;
const u8 *tx_buf = NULL;
u8 *rx_buf = NULL;
@@ -689,11 +688,6 @@ static int zynq_qspi_exec_op(struct spi_slave *slave,
}
  
  	op_len = op->cmd.nbytes + op->addr.nbytes + op->dummy.nbytes;

-   if (op->dummy.nbytes) {
-   op_len = op->cmd.nbytes + op->addr.nbytes +
-op->dummy.nbytes / op->dummy.buswidth;
-   dummy_bytes = op->dummy.nbytes / op->dummy.buswidth;
-   }
  
  	u8 op_buf[op_len];
  
@@ -707,8 +701,8 @@ static int zynq_qspi_exec_op(struct spi_slave *slave,

pos += op->addr.nbytes;
}
  
-	if (dummy_bytes)

-   memset(op_buf + pos, 0xff, dummy_bytes);
+   if (op->dummy.nbytes)
+   memset(op_buf + pos, 0xff, op->dummy.nbytes);
  
  	/* 1st transfer: opcode + address + dummy cycles */

/* Make sure to set END bit if no tx or rx data messages follow */



Applied.
M

Re: [PATCH] Revert "spi: zynq_qspi: Use dummy buswidth in dummy byte calculation"

2023-04-27 Thread Stefan Herbrechtsmeier 

Hi Michal,

Am 26.04.2023 um 16:37 schrieb Michal Simek:



On 3/31/23 16:44, Stefan Herbrechtsmeier wrote:

From: Stefan Herbrechtsmeier 

This reverts commit e09784728689de7949d4cdd559a9590e0bfcc702. The
commit wrongly divides the dummy bytes by dummy bus width to calculate
the dummy bytes. The framework already converts the dummy cycles to the
number of bytes and the controller use the SPI flash command to
determine the dummy cycles via the address width.

Signed-off-by: Stefan Herbrechtsmeier 



---

  drivers/spi/zynq_qspi.c | 10 ++
  1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/zynq_qspi.c b/drivers/spi/zynq_qspi.c
index 00e3ffcd1d..d1d4048966 100644
--- a/drivers/spi/zynq_qspi.c
+++ b/drivers/spi/zynq_qspi.c
@@ -676,7 +676,6 @@ static int zynq_qspi_exec_op(struct spi_slave 
*slave,

  const struct spi_mem_op *op)
  {
 int op_len, pos = 0, ret, i;
-   u32 dummy_bytes = 0;
 unsigned int flag = 0;
 const u8 *tx_buf = NULL;
 u8 *rx_buf = NULL;
@@ -689,11 +688,6 @@ static int zynq_qspi_exec_op(struct spi_slave 
*slave,

 }

 op_len = op->cmd.nbytes + op->addr.nbytes + op->dummy.nbytes;
-   if (op->dummy.nbytes) {
-   op_len = op->cmd.nbytes + op->addr.nbytes +
-    op->dummy.nbytes / op->dummy.buswidth;
-   dummy_bytes = op->dummy.nbytes / op->dummy.buswidth;
-   }

 u8 op_buf[op_len];

@@ -707,8 +701,8 @@ static int zynq_qspi_exec_op(struct spi_slave 
*slave,

 pos += op->addr.nbytes;
 }

-   if (dummy_bytes)
-   memset(op_buf + pos, 0xff, dummy_bytes);
+   if (op->dummy.nbytes)
+   memset(op_buf + pos, 0xff, op->dummy.nbytes);

 /* 1st transfer: opcode + address + dummy cycles */
 /* Make sure to set END bit if no tx or rx data messages 
follow */

--
2.30.2


Kommanditgesellschaft - Sitz: Detmold - Amtsgericht Lemgo HRA 2790 -
Komplementärin: Weidmüller Interface Führungsgesellschaft mbH -
Sitz: Detmold - Amtsgericht Lemgo HRB 3924;
Geschäftsführer: Dr. Timo Berger, Volker Bibelhausen, Dr. Sebastian 
Durst, André Sombecki;

USt-ID-Nr. DE124599660


Can you please rebase it on the top of the latest version?


I have resend the patch hopefully without reformation.

Regards
  Stefan

[RESEND PATCH] Revert "spi: zynq_qspi: Use dummy buswidth in dummy byte calculation"

2023-04-27 Thread Stefan Herbrechtsmeier 
From: Stefan Herbrechtsmeier 

This reverts commit e09784728689de7949d4cdd559a9590e0bfcc702. The
commit wrongly divides the dummy bytes by dummy bus width to calculate
the dummy bytes. The framework already converts the dummy cycles to the
number of bytes and the controller use the SPI flash command to
determine the dummy cycles via the address width.

Signed-off-by: Stefan Herbrechtsmeier 
Acked-by: Ashok Reddy Soma 

---

 drivers/spi/zynq_qspi.c | 10 ++
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/zynq_qspi.c b/drivers/spi/zynq_qspi.c
index 00e3ffcd1d..d1d4048966 100644
--- a/drivers/spi/zynq_qspi.c
+++ b/drivers/spi/zynq_qspi.c
@@ -676,7 +676,6 @@ static int zynq_qspi_exec_op(struct spi_slave *slave,
 const struct spi_mem_op *op)
 {
int op_len, pos = 0, ret, i;
-   u32 dummy_bytes = 0;
unsigned int flag = 0;
const u8 *tx_buf = NULL;
u8 *rx_buf = NULL;
@@ -689,11 +688,6 @@ static int zynq_qspi_exec_op(struct spi_slave *slave,
}
 
op_len = op->cmd.nbytes + op->addr.nbytes + op->dummy.nbytes;
-   if (op->dummy.nbytes) {
-   op_len = op->cmd.nbytes + op->addr.nbytes +
-op->dummy.nbytes / op->dummy.buswidth;
-   dummy_bytes = op->dummy.nbytes / op->dummy.buswidth;
-   }
 
u8 op_buf[op_len];
 
@@ -707,8 +701,8 @@ static int zynq_qspi_exec_op(struct spi_slave *slave,
pos += op->addr.nbytes;
}
 
-   if (dummy_bytes)
-   memset(op_buf + pos, 0xff, dummy_bytes);
+   if (op->dummy.nbytes)
+   memset(op_buf + pos, 0xff, op->dummy.nbytes);
 
/* 1st transfer: opcode + address + dummy cycles */
/* Make sure to set END bit if no tx or rx data messages follow */
-- 
2.30.2

Re: [PATCH] Revert "spi: zynq_qspi: Use dummy buswidth in dummy byte calculation"

2023-04-26 Thread Michal Simek 




On 3/31/23 16:44, Stefan Herbrechtsmeier wrote:

From: Stefan Herbrechtsmeier 

This reverts commit e09784728689de7949d4cdd559a9590e0bfcc702. The
commit wrongly divides the dummy bytes by dummy bus width to calculate
the dummy bytes. The framework already converts the dummy cycles to the
number of bytes and the controller use the SPI flash command to
determine the dummy cycles via the address width.

Signed-off-by: Stefan Herbrechtsmeier 

---

  drivers/spi/zynq_qspi.c | 10 ++
  1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/zynq_qspi.c b/drivers/spi/zynq_qspi.c
index 00e3ffcd1d..d1d4048966 100644
--- a/drivers/spi/zynq_qspi.c
+++ b/drivers/spi/zynq_qspi.c
@@ -676,7 +676,6 @@ static int zynq_qspi_exec_op(struct spi_slave *slave,
  const struct spi_mem_op *op)
  {
 int op_len, pos = 0, ret, i;
-   u32 dummy_bytes = 0;
 unsigned int flag = 0;
 const u8 *tx_buf = NULL;
 u8 *rx_buf = NULL;
@@ -689,11 +688,6 @@ static int zynq_qspi_exec_op(struct spi_slave *slave,
 }

 op_len = op->cmd.nbytes + op->addr.nbytes + op->dummy.nbytes;
-   if (op->dummy.nbytes) {
-   op_len = op->cmd.nbytes + op->addr.nbytes +
-op->dummy.nbytes / op->dummy.buswidth;
-   dummy_bytes = op->dummy.nbytes / op->dummy.buswidth;
-   }

 u8 op_buf[op_len];

@@ -707,8 +701,8 @@ static int zynq_qspi_exec_op(struct spi_slave *slave,
 pos += op->addr.nbytes;
 }

-   if (dummy_bytes)
-   memset(op_buf + pos, 0xff, dummy_bytes);
+   if (op->dummy.nbytes)
+   memset(op_buf + pos, 0xff, op->dummy.nbytes);

 /* 1st transfer: opcode + address + dummy cycles */
 /* Make sure to set END bit if no tx or rx data messages follow */
--
2.30.2


Kommanditgesellschaft - Sitz: Detmold - Amtsgericht Lemgo HRA 2790 -
Komplementärin: Weidmüller Interface Führungsgesellschaft mbH -
Sitz: Detmold - Amtsgericht Lemgo HRB 3924;
Geschäftsführer: Dr. Timo Berger, Volker Bibelhausen, Dr. Sebastian Durst, 
André Sombecki;
USt-ID-Nr. DE124599660


Can you please rebase it on the top of the latest version?

$ git am -s 
./20230331_stefan_herbrechtsmeier_oss_revert_spi_zynq_qspi_use_dummy_buswidth_in_dummy_byte_calculatio.mbx

Adding link to lore.kernel.org
Applying: Revert "spi: zynq_qspi: Use dummy buswidth in dummy byte calculation"
error: patch failed: drivers/spi/zynq_qspi.c:676
error: drivers/spi/zynq_qspi.c: patch does not apply
Patch failed at 0001 Revert "spi: zynq_qspi: Use dummy buswidth in dummy byte 
calculation"

hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Thanks,
Michal

RE: [PATCH] Revert "spi: zynq_qspi: Use dummy buswidth in dummy byte calculation"

2023-04-26 Thread Soma, Ashok Reddy 
Acked-by: Ashok Reddy Soma 

Hi Stefan,

> -Original Message-
> From: Stefan Herbrechtsmeier  o...@weidmueller.com>
> Sent: Tuesday, April 18, 2023 3:58 PM
> To: Soma, Ashok Reddy ; Simek, Michal
> ; u-boot@lists.denx.de
> Cc: Stefan Herbrechtsmeier ;
> Jagan Teki 
> Subject: Re: [PATCH] Revert "spi: zynq_qspi: Use dummy buswidth in dummy
> byte calculation"
> 
> Hi Ashok,
> 
> Am 18.04.2023 um 10:43 schrieb Soma, Ashok Reddy:
> 
> >
> >> -Original Message-
> >> From: Simek, Michal 
> >> Sent: Monday, April 17, 2023 3:47 PM
> >> To: Stefan Herbrechtsmeier
> >> ;
> >> u-boot@lists.denx.de; Soma, Ashok Reddy
> 
> >> Cc: Stefan Herbrechtsmeier ;
> >> Jagan Teki 
> >> Subject: Re: [PATCH] Revert "spi: zynq_qspi: Use dummy buswidth in
> >> dummy byte calculation"
> >>
> >>
> >>
> >> On 3/31/23 16:44, Stefan Herbrechtsmeier wrote:
> >>> From: Stefan Herbrechtsmeier
> >>> 
> >>>
> >>> This reverts commit e09784728689de7949d4cdd559a9590e0bfcc702.
> The
> >>> commit wrongly divides the dummy bytes by dummy bus width to
> >>> calculate the dummy bytes. The framework already converts the dummy
> >>> cycles to the number of bytes and the controller use the SPI flash
> >>> command to determine the dummy cycles via the address width.
> > As per my understanding dummy bus width should be equal to data
> buswidth and not equal to address bus width.
> > Please let me know if this understanding in incorrect.
> 
> Why? The kernel use the addr bus width too.
> 
> Independent of the correct answer the core should handle this because it
> forward the dummy *bytes* to the driver:
> 
>  /* convert the dummy cycles to the number of bytes */
>  op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;
> 
> 
> Furthermore, the driver send the dummy as part of the address:
> 
>  /* 1st transfer: opcode + address + dummy cycles */
> 
> > Based on the above understanding we have changed in Xilinx repo, at
> > below code
> > https://github.com/Xilinx/u-boot-
> xlnx/blob/024eb37c1e38ab811abe5408d42
> > 069fbd7901824/drivers/mtd/spi/spi-nor-core.c#L262
> > from
> > op->dummy.buswidth = spi_nor_get_protocol_addr_nbits(proto);
> > to
> > op->dummy.buswidth = spi_nor_get_protocol_data_nbits(proto);
> >
> > I have sent the same as RFC. Please see the attached patch.
> >
> > Currently this patch is present as part of Xilinx repo, and hence based on 
> > this
> below implementation dymmy_bytes are recalculated.
> 
> In this case the patch should be reverted until the other patch is accepted.
Agree with you, we can revert until I send the other patch. Ack'ing this patch.

Thanks,
Ashok
> 
> > I have sent controller driver code to upstream but, I have not yet sent spi-
> nor-core.c code, which I will be sending soon with some other changes.
> >
> > Please let me know your thoughts about this patch and the changes.
> 
> It is unclear to me why the dummy should be part of the data. The current
> implementation is wrong because it doesn't respects the address width and
> send the dummy bytes as part of the addr.
> 
> Regards
>Stefan
> 
> 
> Kommanditgesellschaft - Sitz: Detmold - Amtsgericht Lemgo HRA 2790 -
> Komplementärin: Weidmüller Interface Führungsgesellschaft mbH -
> Sitz: Detmold - Amtsgericht Lemgo HRB 3924;
> Geschäftsführer: Dr. Timo Berger, Volker Bibelhausen, Dr. Sebastian Durst,
> André Sombecki; USt-ID-Nr. DE124599660

Re: [PATCH] Revert "spi: zynq_qspi: Use dummy buswidth in dummy byte calculation"

2023-04-19 Thread Stefan Herbrechtsmeier 

Hi Ashok,

have you test your patches with 1-4-4 mode?

I think your patches are a nop for 1-1-4 mode and break other modes:

spi-nor-core
op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;

zynq_qspi:
dummy_bytes = op->dummy.nbytes / op->dummy.buswidth;

=> dummy_bytes = (nor->read_dummy * op.dummy.buswidth) / 8) /
op->dummy.buswidth
=> dummy_bytes = nor->read_dummy / 8

You changes ignores the bus width and breaks if the addr bus width is not 1!

Regards
  Stefan

Am 18.04.2023 um 12:27 schrieb Stefan Herbrechtsmeier:

Hi Ashok,

Am 18.04.2023 um 10:43 schrieb Soma, Ashok Reddy:




-Original Message-
From: Simek, Michal 
Sent: Monday, April 17, 2023 3:47 PM
To: Stefan Herbrechtsmeier
;
u-boot@lists.denx.de; Soma, Ashok Reddy 
Cc: Stefan Herbrechtsmeier ;
Jagan Teki 
Subject: Re: [PATCH] Revert "spi: zynq_qspi: Use dummy buswidth in
dummy
byte calculation"



On 3/31/23 16:44, Stefan Herbrechtsmeier wrote:

From: Stefan Herbrechtsmeier 

This reverts commit e09784728689de7949d4cdd559a9590e0bfcc702. The
commit wrongly divides the dummy bytes by dummy bus width to calculate
the dummy bytes. The framework already converts the dummy cycles to
the number of bytes and the controller use the SPI flash command to
determine the dummy cycles via the address width.

As per my understanding dummy bus width should be equal to data
buswidth and not equal to address bus width.
Please let me know if this understanding in incorrect.


Why? The kernel use the addr bus width too.

Independent of the correct answer the core should handle this because
it forward the dummy *bytes* to the driver:

/* convert the dummy cycles to the number of bytes */
op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;


Furthermore, the driver send the dummy as part of the address:

/* 1st transfer: opcode + address + dummy cycles */


Based on the above understanding we have changed in Xilinx repo, at
below code
https://github.com/Xilinx/u-boot-xlnx/blob/024eb37c1e38ab811abe5408d42069fbd7901824/drivers/mtd/spi/spi-nor-core.c#L262

from
op->dummy.buswidth = spi_nor_get_protocol_addr_nbits(proto);
to
op->dummy.buswidth = spi_nor_get_protocol_data_nbits(proto);

I have sent the same as RFC. Please see the attached patch.

Currently this patch is present as part of Xilinx repo, and hence
based on this below implementation dymmy_bytes are recalculated.


In this case the patch should be reverted until the other patch is
accepted.


I have sent controller driver code to upstream but, I have not yet
sent spi-nor-core.c code, which I will be sending soon with some
other changes.

Please let me know your thoughts about this patch and the changes.


It is unclear to me why the dummy should be part of the data. The
current implementation is wrong because it doesn't respects the
address width and send the dummy bytes as part of the addr.

Regards
  Stefan



Kommanditgesellschaft - Sitz: Detmold - Amtsgericht Lemgo HRA 2790 -
Komplementärin: Weidmüller Interface Führungsgesellschaft mbH -
Sitz: Detmold - Amtsgericht Lemgo HRB 3924;
Geschäftsführer: Dr. Timo Berger, Volker Bibelhausen, Dr. Sebastian Durst, 
André Sombecki;
USt-ID-Nr. DE124599660

Re: [PATCH] Revert "spi: zynq_qspi: Use dummy buswidth in dummy byte calculation"

2023-04-18 Thread Stefan Herbrechtsmeier 

Hi Ashok,

Am 18.04.2023 um 10:43 schrieb Soma, Ashok Reddy:




-Original Message-
From: Simek, Michal 
Sent: Monday, April 17, 2023 3:47 PM
To: Stefan Herbrechtsmeier ;
u-boot@lists.denx.de; Soma, Ashok Reddy 
Cc: Stefan Herbrechtsmeier ;
Jagan Teki 
Subject: Re: [PATCH] Revert "spi: zynq_qspi: Use dummy buswidth in dummy
byte calculation"



On 3/31/23 16:44, Stefan Herbrechtsmeier wrote:

From: Stefan Herbrechtsmeier 

This reverts commit e09784728689de7949d4cdd559a9590e0bfcc702. The
commit wrongly divides the dummy bytes by dummy bus width to calculate
the dummy bytes. The framework already converts the dummy cycles to
the number of bytes and the controller use the SPI flash command to
determine the dummy cycles via the address width.

As per my understanding dummy bus width should be equal to data buswidth and 
not equal to address bus width.
Please let me know if this understanding in incorrect.


Why? The kernel use the addr bus width too.

Independent of the correct answer the core should handle this because it
forward the dummy *bytes* to the driver:

/* convert the dummy cycles to the number of bytes */
op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;


Furthermore, the driver send the dummy as part of the address:

/* 1st transfer: opcode + address + dummy cycles */


Based on the above understanding we have changed in Xilinx repo, at below code
https://github.com/Xilinx/u-boot-xlnx/blob/024eb37c1e38ab811abe5408d42069fbd7901824/drivers/mtd/spi/spi-nor-core.c#L262
from
op->dummy.buswidth = spi_nor_get_protocol_addr_nbits(proto);
to
op->dummy.buswidth = spi_nor_get_protocol_data_nbits(proto);

I have sent the same as RFC. Please see the attached patch.

Currently this patch is present as part of Xilinx repo, and hence based on this 
below implementation dymmy_bytes are recalculated.


In this case the patch should be reverted until the other patch is accepted.


I have sent controller driver code to upstream but, I have not yet sent 
spi-nor-core.c code, which I will be sending soon with some other changes.

Please let me know your thoughts about this patch and the changes.


It is unclear to me why the dummy should be part of the data. The
current implementation is wrong because it doesn't respects the address
width and send the dummy bytes as part of the addr.

Regards
  Stefan


Kommanditgesellschaft - Sitz: Detmold - Amtsgericht Lemgo HRA 2790 -
Komplementärin: Weidmüller Interface Führungsgesellschaft mbH -
Sitz: Detmold - Amtsgericht Lemgo HRB 3924;
Geschäftsführer: Dr. Timo Berger, Volker Bibelhausen, Dr. Sebastian Durst, 
André Sombecki;
USt-ID-Nr. DE124599660

RE: [PATCH] Revert "spi: zynq_qspi: Use dummy buswidth in dummy byte calculation"

2023-04-18 Thread Soma, Ashok Reddy 
Hi Stefan,


> -Original Message-
> From: Simek, Michal 
> Sent: Monday, April 17, 2023 3:47 PM
> To: Stefan Herbrechtsmeier ;
> u-boot@lists.denx.de; Soma, Ashok Reddy 
> Cc: Stefan Herbrechtsmeier ;
> Jagan Teki 
> Subject: Re: [PATCH] Revert "spi: zynq_qspi: Use dummy buswidth in dummy
> byte calculation"
> 
> 
> 
> On 3/31/23 16:44, Stefan Herbrechtsmeier wrote:
> > From: Stefan Herbrechtsmeier 
> >
> > This reverts commit e09784728689de7949d4cdd559a9590e0bfcc702. The
> > commit wrongly divides the dummy bytes by dummy bus width to calculate
> > the dummy bytes. The framework already converts the dummy cycles to
> > the number of bytes and the controller use the SPI flash command to
> > determine the dummy cycles via the address width.

As per my understanding dummy bus width should be equal to data buswidth and 
not equal to address bus width.
Please let me know if this understanding in incorrect. 

Based on the above understanding we have changed in Xilinx repo, at below code 
https://github.com/Xilinx/u-boot-xlnx/blob/024eb37c1e38ab811abe5408d42069fbd7901824/drivers/mtd/spi/spi-nor-core.c#L262
from
op->dummy.buswidth = spi_nor_get_protocol_addr_nbits(proto);
to
op->dummy.buswidth = spi_nor_get_protocol_data_nbits(proto);

I have sent the same as RFC. Please see the attached patch.

Currently this patch is present as part of Xilinx repo, and hence based on this 
below implementation dymmy_bytes are recalculated.
I have sent controller driver code to upstream but, I have not yet sent 
spi-nor-core.c code, which I will be sending soon with some other changes.

Please let me know your thoughts about this patch and the changes.

Thanks,
Ashok


> >
> > Signed-off-by: Stefan Herbrechtsmeier
> > 
> >
> > ---
> >
> >   drivers/spi/zynq_qspi.c | 10 ++
> >   1 file changed, 2 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/spi/zynq_qspi.c b/drivers/spi/zynq_qspi.c index
> > 00e3ffcd1d..d1d4048966 100644
> > --- a/drivers/spi/zynq_qspi.c
> > +++ b/drivers/spi/zynq_qspi.c
> > @@ -676,7 +676,6 @@ static int zynq_qspi_exec_op(struct spi_slave *slave,
> >   const struct spi_mem_op *op)
> >   {
> >  int op_len, pos = 0, ret, i;
> > -   u32 dummy_bytes = 0;
> >  unsigned int flag = 0;
> >  const u8 *tx_buf = NULL;
> >  u8 *rx_buf = NULL;
> > @@ -689,11 +688,6 @@ static int zynq_qspi_exec_op(struct spi_slave
> *slave,
> >  }
> >
> >  op_len = op->cmd.nbytes + op->addr.nbytes + op->dummy.nbytes;
> > -   if (op->dummy.nbytes) {
> > -   op_len = op->cmd.nbytes + op->addr.nbytes +
> > -op->dummy.nbytes / op->dummy.buswidth;
> > -   dummy_bytes = op->dummy.nbytes / op->dummy.buswidth;
> > -   }
> >
> >  u8 op_buf[op_len];
> >
> > @@ -707,8 +701,8 @@ static int zynq_qspi_exec_op(struct spi_slave *slave,
> >  pos += op->addr.nbytes;
> >  }
> >
> > -   if (dummy_bytes)
> > -   memset(op_buf + pos, 0xff, dummy_bytes);
> > +   if (op->dummy.nbytes)
> > +   memset(op_buf + pos, 0xff, op->dummy.nbytes);
> >
> >  /* 1st transfer: opcode + address + dummy cycles */
> >  /* Make sure to set END bit if no tx or rx data messages
> > follow */
> > --
> > 2.30.2
> >
> 
> Ashok: Can you please comment on this one?
> 
> Thanks,
> Michal
--- Begin Message ---
In current implementation dummy buswidth is set equal to address
buswidth. In case of quad spi (mode 1-1-4), where address width is 1
the dummy bytes will be calculated to 1(8 dummy cycles) and dummy
buswidth is set to 1. Due to this, the controller driver will introduce
8 dummy cycles on data line(D0) during read operation.

But since we are using 4 data lines in case of qspi, we need to change
this dummy bus width to 4. This will make dummy bytes to 4 inplace of 1.
This will be taken care in controller driver by dividing with dummy
buswidth again as in below code, which makes dummy cycles to 8 as
earlier.

dummy_cycles = op->dummy.nbytes * 8 / op->dummy.buswidth;

https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/spi/zynqmp_gqspi.c#L511

So with this change dummy cycles will be on all data lines(D0-D3) and it
is taken care for all the configurations(single, dual, quad and octal).

SPI experts, please advice if this change is good. What is the reason
we are using dummy bus width as address bus width so far ?

I have tested this change on all xilinx platforms with single, quad and
octal configura

Re: [PATCH] Revert "spi: zynq_qspi: Use dummy buswidth in dummy byte calculation"

2023-04-17 Thread Michal Simek 




On 3/31/23 16:44, Stefan Herbrechtsmeier wrote:

From: Stefan Herbrechtsmeier 

This reverts commit e09784728689de7949d4cdd559a9590e0bfcc702. The
commit wrongly divides the dummy bytes by dummy bus width to calculate
the dummy bytes. The framework already converts the dummy cycles to the
number of bytes and the controller use the SPI flash command to
determine the dummy cycles via the address width.

Signed-off-by: Stefan Herbrechtsmeier 

---

  drivers/spi/zynq_qspi.c | 10 ++
  1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/zynq_qspi.c b/drivers/spi/zynq_qspi.c
index 00e3ffcd1d..d1d4048966 100644
--- a/drivers/spi/zynq_qspi.c
+++ b/drivers/spi/zynq_qspi.c
@@ -676,7 +676,6 @@ static int zynq_qspi_exec_op(struct spi_slave *slave,
  const struct spi_mem_op *op)
  {
 int op_len, pos = 0, ret, i;
-   u32 dummy_bytes = 0;
 unsigned int flag = 0;
 const u8 *tx_buf = NULL;
 u8 *rx_buf = NULL;
@@ -689,11 +688,6 @@ static int zynq_qspi_exec_op(struct spi_slave *slave,
 }

 op_len = op->cmd.nbytes + op->addr.nbytes + op->dummy.nbytes;
-   if (op->dummy.nbytes) {
-   op_len = op->cmd.nbytes + op->addr.nbytes +
-op->dummy.nbytes / op->dummy.buswidth;
-   dummy_bytes = op->dummy.nbytes / op->dummy.buswidth;
-   }

 u8 op_buf[op_len];

@@ -707,8 +701,8 @@ static int zynq_qspi_exec_op(struct spi_slave *slave,
 pos += op->addr.nbytes;
 }

-   if (dummy_bytes)
-   memset(op_buf + pos, 0xff, dummy_bytes);
+   if (op->dummy.nbytes)
+   memset(op_buf + pos, 0xff, op->dummy.nbytes);

 /* 1st transfer: opcode + address + dummy cycles */
 /* Make sure to set END bit if no tx or rx data messages follow */
--
2.30.2



Ashok: Can you please comment on this one?

Thanks,
Michal

[PATCH] Revert "spi: zynq_qspi: Use dummy buswidth in dummy byte calculation"

2023-03-31 Thread Stefan Herbrechtsmeier 
From: Stefan Herbrechtsmeier 

This reverts commit e09784728689de7949d4cdd559a9590e0bfcc702. The
commit wrongly divides the dummy bytes by dummy bus width to calculate
the dummy bytes. The framework already converts the dummy cycles to the
number of bytes and the controller use the SPI flash command to
determine the dummy cycles via the address width.

Signed-off-by: Stefan Herbrechtsmeier 

---

 drivers/spi/zynq_qspi.c | 10 ++
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/zynq_qspi.c b/drivers/spi/zynq_qspi.c
index 00e3ffcd1d..d1d4048966 100644
--- a/drivers/spi/zynq_qspi.c
+++ b/drivers/spi/zynq_qspi.c
@@ -676,7 +676,6 @@ static int zynq_qspi_exec_op(struct spi_slave *slave,
 const struct spi_mem_op *op)
 {
int op_len, pos = 0, ret, i;
-   u32 dummy_bytes = 0;
unsigned int flag = 0;
const u8 *tx_buf = NULL;
u8 *rx_buf = NULL;
@@ -689,11 +688,6 @@ static int zynq_qspi_exec_op(struct spi_slave *slave,
}

op_len = op->cmd.nbytes + op->addr.nbytes + op->dummy.nbytes;
-   if (op->dummy.nbytes) {
-   op_len = op->cmd.nbytes + op->addr.nbytes +
-op->dummy.nbytes / op->dummy.buswidth;
-   dummy_bytes = op->dummy.nbytes / op->dummy.buswidth;
-   }

u8 op_buf[op_len];

@@ -707,8 +701,8 @@ static int zynq_qspi_exec_op(struct spi_slave *slave,
pos += op->addr.nbytes;
}

-   if (dummy_bytes)
-   memset(op_buf + pos, 0xff, dummy_bytes);
+   if (op->dummy.nbytes)
+   memset(op_buf + pos, 0xff, op->dummy.nbytes);

/* 1st transfer: opcode + address + dummy cycles */
/* Make sure to set END bit if no tx or rx data messages follow */
--
2.30.2


Kommanditgesellschaft - Sitz: Detmold - Amtsgericht Lemgo HRA 2790 -
Komplementärin: Weidmüller Interface Führungsgesellschaft mbH -
Sitz: Detmold - Amtsgericht Lemgo HRB 3924;
Geschäftsführer: Dr. Timo Berger, Volker Bibelhausen, Dr. Sebastian Durst, 
André Sombecki;
USt-ID-Nr. DE124599660