RE: [PATCH] spi/zynqmp: remove entry that causes a cs glitch

[Naga Sureshkumar Relli <nagasure@xxxxxxxxxx>]

Hi Thommy,

> -----Original Message-----
> From: linux-spi-owner@xxxxxxxxxxxxxxx <linux-spi-owner@xxxxxxxxxxxxxxx> On Behalf Of
> Thommy Jakobsson
> Sent: Monday, February 24, 2020 9:57 PM
> To: broonie@xxxxxxxxxx; Michal Simek <michals@xxxxxxxxxx>; linux-spi@xxxxxxxxxxxxxxx
> Cc: Thommy Jakobsson <thommyj@xxxxxxxxx>
> Subject: [PATCH] spi/zynqmp: remove entry that causes a cs glitch
> 
> In the public interface for chipselect, there is always an entry commented as "Dummy generic
> FIFO entry" pushed down to the fifo right after the activate/deactivate command. The dummy
> entry is 0x0, irregardless if the intention was to activate or deactive the cs. This causes the cs
> line to glitch rather than beeing activated in the case when there was an activate command.
> 
> This has been observed on oscilloscope, and have caused problems for at least one specific
> flash device type connected to the qspi port. After the change the glitch is gone and cs goes
> active when intended.
> 
> The reason why this worked before (except for the glitch) was because when sending the actual
> data, the CS bits are once again set. Since most flashes uses mode 0, there is always a half clk
> period anyway for cs to clk active setup time. If someone would rely on timing from a
> chip_select call to a transfer_one, it would fail though.
> 
> It is unknown why the dummy entry was there in the first place, git log seems to be of no help
> in this case. The reference manual gives no indication of the necessity of this. In fact the lower
> 8 bits are a setup (or hold in case of deactivate) time expressed in cycles. So this should not be
> needed to fulfill any setup/hold timings.
> 
> Signed-off-by: Thommy Jakobsson <thommyj@xxxxxxxxx>
> ---
>  drivers/spi/spi-zynqmp-gqspi.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/spi/spi-zynqmp-gqspi.c b/drivers/spi/spi-zynqmp-gqspi.c index
> 60c4de4e4485..7412a3042a8d 100644
> --- a/drivers/spi/spi-zynqmp-gqspi.c
> +++ b/drivers/spi/spi-zynqmp-gqspi.c
> @@ -401,9 +401,6 @@ static void zynqmp_qspi_chipselect(struct spi_device *qspi, bool
> is_high)
> 
>  	zynqmp_gqspi_write(xqspi, GQSPI_GEN_FIFO_OFST, genfifoentry);
> 
> -	/* Dummy generic FIFO entry */
> -	zynqmp_gqspi_write(xqspi, GQSPI_GEN_FIFO_OFST, 0x0);
> -
>  	/* Manually start the generic FIFO command */
>  	zynqmp_gqspi_write(xqspi, GQSPI_CONFIG_OFST,
>  			zynqmp_gqspi_read(xqspi, GQSPI_CONFIG_OFST) |
> --
> 2.17.1
Reviewed-by: Naga Sureshkumar Relli <naga.sureshkumar.relli@xxxxxxxxxx>