LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v2 0/2] spi: cadence-quadspi: Fix DTR op checks and timeout in SPI NAND write operations
@ 2021-07-16 23:25 Apurva Nandan
  2021-07-16 23:25 ` [PATCH v2 1/2] spi: cadence-quadspi: Disable Auto-HW polling Apurva Nandan
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Apurva Nandan @ 2021-07-16 23:25 UTC (permalink / raw)
  To: Mark Brown, linux-spi, linux-kernel
  Cc: Apurva Nandan, Pratyush Yadav, Vignesh Raghavendra

Hi,
This series proposes fixes for cadence-quadspi controller for the
following issues with SPI NAND flashes:

- Due to auto-HW polling without address phase, the cadence-quadspi
  controller timeouts when performing any write operation on SPI NAND
  flash.

- When checking for DTR spi_mem_op, cadence-quadspi doesn't ignore a
  zero length phase in the SPI instruction, resulting in false negatives.

This series has been tested on TI J721e EVM with the Winbond W35N01JW
flash.

v1 series: https://lore.kernel.org/linux-spi/20210713125743.1540-1-a-nandan@ti.com/

Changes in v2:
- [PATCH v2 1/2]: Same as v1. This patch has been already applied to
  https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
  commit 9cb2ff111712 ("spi: cadence-quadspi: Disable Auto-HW polling")

- [PATCH v2 2/2]: Add new comments to explain the DTR check conditions

Apurva Nandan (2):
  spi: cadence-quadspi: Disable Auto-HW polling
  spi: cadence-quadspi: Fix check condition for DTR ops

 drivers/spi/spi-cadence-quadspi.c | 48 ++++++++++++++++++++-----------
 1 file changed, 32 insertions(+), 16 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/2] spi: cadence-quadspi: Disable Auto-HW polling
  2021-07-16 23:25 [PATCH v2 0/2] spi: cadence-quadspi: Fix DTR op checks and timeout in SPI NAND write operations Apurva Nandan
@ 2021-07-16 23:25 ` Apurva Nandan
  2021-07-16 23:25 ` [PATCH v2 2/2] spi: cadence-quadspi: Fix check condition for DTR ops Apurva Nandan
  2021-08-06  0:47 ` (subset) [PATCH v2 0/2] spi: cadence-quadspi: Fix DTR op checks and timeout in SPI NAND write operations Mark Brown
  2 siblings, 0 replies; 6+ messages in thread
From: Apurva Nandan @ 2021-07-16 23:25 UTC (permalink / raw)
  To: Mark Brown, linux-spi, linux-kernel
  Cc: Apurva Nandan, Pratyush Yadav, Vignesh Raghavendra

cadence-quadspi has a builtin Auto-HW polling funtionality using which
it keep tracks of completion of write operations. When Auto-HW polling
is enabled, it automatically initiates status register read operation,
until the flash clears its busy bit.

cadence-quadspi controller doesn't allow an address phase when
auto-polling the busy bit on the status register. Unlike SPI NOR
flashes, SPI NAND flashes do require the address of status register
when polling the busy bit using the read register operation. As
Auto-HW polling is enabled by default, cadence-quadspi returns a
timeout for every write operation after an indefinite amount of
polling on SPI NAND flashes.

Disable Auto-HW polling completely as the spi-nor core, spinand core,
etc. take care of polling the busy bit on their own.

Signed-off-by: Apurva Nandan <a-nandan@ti.com>
Link: https://lore.kernel.org/r/20210713125743.1540-2-a-nandan@ti.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 This patch has already been applied to
 https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
 [1/2] spi: cadence-quadspi: Disable Auto-HW polling
       commit: 9cb2ff11171264d10be7ea9e31d9ee5d49ba84a5
 Reposting it for sake of completeness of the v2 series.

 drivers/spi/spi-cadence-quadspi.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index d62d69dd72b9..a2de23516553 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -800,19 +800,20 @@ static int cqspi_write_setup(struct cqspi_flash_pdata *f_pdata,
 	reg = cqspi_calc_rdreg(f_pdata);
 	writel(reg, reg_base + CQSPI_REG_RD_INSTR);
 
-	if (f_pdata->dtr) {
-		/*
-		 * Some flashes like the cypress Semper flash expect a 4-byte
-		 * dummy address with the Read SR command in DTR mode, but this
-		 * controller does not support sending address with the Read SR
-		 * command. So, disable write completion polling on the
-		 * controller's side. spi-nor will take care of polling the
-		 * status register.
-		 */
-		reg = readl(reg_base + CQSPI_REG_WR_COMPLETION_CTRL);
-		reg |= CQSPI_REG_WR_DISABLE_AUTO_POLL;
-		writel(reg, reg_base + CQSPI_REG_WR_COMPLETION_CTRL);
-	}
+	/*
+	 * SPI NAND flashes require the address of the status register to be
+	 * passed in the Read SR command. Also, some SPI NOR flashes like the
+	 * cypress Semper flash expect a 4-byte dummy address in the Read SR
+	 * command in DTR mode.
+	 *
+	 * But this controller does not support address phase in the Read SR
+	 * command when doing auto-HW polling. So, disable write completion
+	 * polling on the controller's side. spinand and spi-nor will take
+	 * care of polling the status register.
+	 */
+	reg = readl(reg_base + CQSPI_REG_WR_COMPLETION_CTRL);
+	reg |= CQSPI_REG_WR_DISABLE_AUTO_POLL;
+	writel(reg, reg_base + CQSPI_REG_WR_COMPLETION_CTRL);
 
 	reg = readl(reg_base + CQSPI_REG_SIZE);
 	reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 2/2] spi: cadence-quadspi: Fix check condition for DTR ops
  2021-07-16 23:25 [PATCH v2 0/2] spi: cadence-quadspi: Fix DTR op checks and timeout in SPI NAND write operations Apurva Nandan
  2021-07-16 23:25 ` [PATCH v2 1/2] spi: cadence-quadspi: Disable Auto-HW polling Apurva Nandan
@ 2021-07-16 23:25 ` Apurva Nandan
  2021-07-21 14:53   ` Nandan, Apurva
  2021-08-06  0:47 ` (subset) [PATCH v2 0/2] spi: cadence-quadspi: Fix DTR op checks and timeout in SPI NAND write operations Mark Brown
  2 siblings, 1 reply; 6+ messages in thread
From: Apurva Nandan @ 2021-07-16 23:25 UTC (permalink / raw)
  To: Mark Brown, linux-spi, linux-kernel
  Cc: Apurva Nandan, Pratyush Yadav, Vignesh Raghavendra

buswidth and dtr fields in spi_mem_op are only valid when the
corresponding spi_mem_op phase has a non-zero length. For example,
SPI NAND core doesn't set buswidth when using SPI_MEM_OP_NO_ADDR
phase.

Fix the dtr checks in set_protocol() and suppports_mem_op() to
ignore empty spi_mem_op phases, as checking for dtr field in
empty phase will result in false negatives.

Signed-off-by: Apurva Nandan <a-nandan@ti.com>
---
 drivers/spi/spi-cadence-quadspi.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index a2de23516553..1cec1c179a94 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -325,7 +325,15 @@ static int cqspi_set_protocol(struct cqspi_flash_pdata *f_pdata,
 	f_pdata->inst_width = CQSPI_INST_TYPE_SINGLE;
 	f_pdata->addr_width = CQSPI_INST_TYPE_SINGLE;
 	f_pdata->data_width = CQSPI_INST_TYPE_SINGLE;
-	f_pdata->dtr = op->data.dtr && op->cmd.dtr && op->addr.dtr;
+
+	/*
+	 * For an op to be DTR, cmd phase along with every other non-empty
+	 * phase should have dtr field set to 1. If an op phase has zero
+	 * nbytes, ignore its dtr field; otherwise, check its dtr field.
+	 */
+	f_pdata->dtr = op->cmd.dtr &&
+		       (!op->addr.nbytes || op->addr.dtr) &&
+		       (!op->data.nbytes || op->data.dtr);
 
 	switch (op->data.buswidth) {
 	case 0:
@@ -1228,8 +1236,15 @@ static bool cqspi_supports_mem_op(struct spi_mem *mem,
 {
 	bool all_true, all_false;
 
-	all_true = op->cmd.dtr && op->addr.dtr && op->dummy.dtr &&
-		   op->data.dtr;
+	/*
+	 * op->dummy.dtr is required for converting nbytes into ncycles.
+	 * Also, don't check the dtr field of the op phase having zero nbytes.
+	 */
+	all_true = op->cmd.dtr &&
+		   (!op->addr.nbytes || op->addr.dtr) &&
+		   (!op->dummy.nbytes || op->dummy.dtr) &&
+		   (!op->data.nbytes || op->data.dtr);
+
 	all_false = !op->cmd.dtr && !op->addr.dtr && !op->dummy.dtr &&
 		    !op->data.dtr;
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/2] spi: cadence-quadspi: Fix check condition for DTR ops
  2021-07-16 23:25 ` [PATCH v2 2/2] spi: cadence-quadspi: Fix check condition for DTR ops Apurva Nandan
@ 2021-07-21 14:53   ` Nandan, Apurva
  2021-07-21 16:36     ` Mark Brown
  0 siblings, 1 reply; 6+ messages in thread
From: Nandan, Apurva @ 2021-07-21 14:53 UTC (permalink / raw)
  To: Mark Brown, linux-spi, linux-kernel; +Cc: Pratyush Yadav, Vignesh Raghavendra



On 17-Jul-21 4:55 AM, Apurva Nandan wrote:
> buswidth and dtr fields in spi_mem_op are only valid when the
> corresponding spi_mem_op phase has a non-zero length. For example,
> SPI NAND core doesn't set buswidth when using SPI_MEM_OP_NO_ADDR
> phase.
> 
> Fix the dtr checks in set_protocol() and suppports_mem_op() to
> ignore empty spi_mem_op phases, as checking for dtr field in
> empty phase will result in false negatives.
> 
> Signed-off-by: Apurva Nandan <a-nandan@ti.com>
> ---
>  drivers/spi/spi-cadence-quadspi.c | 21 ++++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
> index a2de23516553..1cec1c179a94 100644
> --- a/drivers/spi/spi-cadence-quadspi.c
> +++ b/drivers/spi/spi-cadence-quadspi.c
> @@ -325,7 +325,15 @@ static int cqspi_set_protocol(struct cqspi_flash_pdata *f_pdata,
>  	f_pdata->inst_width = CQSPI_INST_TYPE_SINGLE;
>  	f_pdata->addr_width = CQSPI_INST_TYPE_SINGLE;
>  	f_pdata->data_width = CQSPI_INST_TYPE_SINGLE;
> -	f_pdata->dtr = op->data.dtr && op->cmd.dtr && op->addr.dtr;
> +
> +	/*
> +	 * For an op to be DTR, cmd phase along with every other non-empty
> +	 * phase should have dtr field set to 1. If an op phase has zero
> +	 * nbytes, ignore its dtr field; otherwise, check its dtr field.
> +	 */
> +	f_pdata->dtr = op->cmd.dtr &&
> +		       (!op->addr.nbytes || op->addr.dtr) &&
> +		       (!op->data.nbytes || op->data.dtr);
>  
>  	switch (op->data.buswidth) {
>  	case 0:
> @@ -1228,8 +1236,15 @@ static bool cqspi_supports_mem_op(struct spi_mem *mem,
>  {
>  	bool all_true, all_false;
>  
> -	all_true = op->cmd.dtr && op->addr.dtr && op->dummy.dtr &&
> -		   op->data.dtr;
> +	/*
> +	 * op->dummy.dtr is required for converting nbytes into ncycles.
> +	 * Also, don't check the dtr field of the op phase having zero nbytes.
> +	 */
> +	all_true = op->cmd.dtr &&
> +		   (!op->addr.nbytes || op->addr.dtr) &&
> +		   (!op->dummy.nbytes || op->dummy.dtr) &&
> +		   (!op->data.nbytes || op->data.dtr);
> +
>  	all_false = !op->cmd.dtr && !op->addr.dtr && !op->dummy.dtr &&
>  		    !op->data.dtr;
>  
> 

Hi Mark,

Could you please have a look, I fixed the comments as you suggested.

Thanks and regards,
Apurva Nandan

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/2] spi: cadence-quadspi: Fix check condition for DTR ops
  2021-07-21 14:53   ` Nandan, Apurva
@ 2021-07-21 16:36     ` Mark Brown
  0 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2021-07-21 16:36 UTC (permalink / raw)
  To: Nandan, Apurva
  Cc: linux-spi, linux-kernel, Pratyush Yadav, Vignesh Raghavendra

[-- Attachment #1: Type: text/plain, Size: 904 bytes --]

On Wed, Jul 21, 2021 at 08:23:30PM +0530, Nandan, Apurva wrote:

> Could you please have a look, I fixed the comments as you suggested.

Please don't send content free pings and please allow a reasonable time
for review.  People get busy, go on holiday, attend conferences and so 
on so unless there is some reason for urgency (like critical bug fixes)
please allow at least a couple of weeks for review.  If there have been
review comments then people may be waiting for those to be addressed.

Sending content free pings adds to the mail volume (if they are seen at
all) which is often the problem and since they can't be reviewed
directly if something has gone wrong you'll have to resend the patches
anyway, so sending again is generally a better approach though there are
some other maintainers who like them - if in doubt look at how patches
for the subsystem are normally handled.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: (subset) [PATCH v2 0/2] spi: cadence-quadspi: Fix DTR op checks and timeout in SPI NAND write operations
  2021-07-16 23:25 [PATCH v2 0/2] spi: cadence-quadspi: Fix DTR op checks and timeout in SPI NAND write operations Apurva Nandan
  2021-07-16 23:25 ` [PATCH v2 1/2] spi: cadence-quadspi: Disable Auto-HW polling Apurva Nandan
  2021-07-16 23:25 ` [PATCH v2 2/2] spi: cadence-quadspi: Fix check condition for DTR ops Apurva Nandan
@ 2021-08-06  0:47 ` Mark Brown
  2 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2021-08-06  0:47 UTC (permalink / raw)
  To: linux-kernel, linux-spi, Apurva Nandan
  Cc: Mark Brown, Pratyush Yadav, Vignesh Raghavendra

On Fri, 16 Jul 2021 23:25:01 +0000, Apurva Nandan wrote:
> This series proposes fixes for cadence-quadspi controller for the
> following issues with SPI NAND flashes:
> 
> - Due to auto-HW polling without address phase, the cadence-quadspi
>   controller timeouts when performing any write operation on SPI NAND
>   flash.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[2/2] spi: cadence-quadspi: Fix check condition for DTR ops
      commit: 0395be967b067d99494113d78470574e86a02ed4

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-08-06  0:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-16 23:25 [PATCH v2 0/2] spi: cadence-quadspi: Fix DTR op checks and timeout in SPI NAND write operations Apurva Nandan
2021-07-16 23:25 ` [PATCH v2 1/2] spi: cadence-quadspi: Disable Auto-HW polling Apurva Nandan
2021-07-16 23:25 ` [PATCH v2 2/2] spi: cadence-quadspi: Fix check condition for DTR ops Apurva Nandan
2021-07-21 14:53   ` Nandan, Apurva
2021-07-21 16:36     ` Mark Brown
2021-08-06  0:47 ` (subset) [PATCH v2 0/2] spi: cadence-quadspi: Fix DTR op checks and timeout in SPI NAND write operations Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).