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

[Soma, Ashok Reddy]

Acked-by: Ashok Reddy Soma <ashok.reddy.soma at amd.com>

Hi Stefan,

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