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

Hi Stefan,

-----Original Message----- From: Simek, Michal michal.simek@amd.com Sent: Monday, April 17, 2023 3:47 PM To: Stefan Herbrechtsmeier stefan.herbrechtsmeier-oss@weidmueller.com; u-boot@lists.denx.de; Soma, Ashok Reddy ashok.reddy.soma@amd.com Cc: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com; Jagan Teki jagan@amarulasolutions.com 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 stefan.herbrechtsmeier@weidmueller.com

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/024eb37c1e38ab811abe5408d42069fbd... 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 stefan.herbrechtsmeier@weidmueller.com

---

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