devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/2] Meson NAND: waiting w/o wired ready/busy pin
@ 2023-06-07 14:50 Arseniy Krasnov
  2023-06-07 14:50 ` [PATCH v1 1/2] dt-bindings: nand: meson: Fix 'nand-rb' property Arseniy Krasnov
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Arseniy Krasnov @ 2023-06-07 14:50 UTC (permalink / raw)
  To: Liang Yang, Miquel Raynal, Richard Weinberger,
	Vignesh Raghavendra, Rob Herring, Krzysztof Kozlowski,
	Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl
  Cc: oxffffaa, kernel, Arseniy Krasnov, linux-mtd, devicetree,
	linux-arm-kernel, linux-amlogic, linux-kernel

Main thing that this patchset adds is support for waiting for command
completion when there is no wired ready/busy pin. This is implemented
by using Meson specific command which operates without wired pin, but
supports interrupt driven waiting (default way for waiting without
hardware pin is 'nand_soft_waitrdy()' which uses software loop instead
of interrupt). To select which mode to use during driver initialization,
common NAND controller property is used - 'nand-rb'. It is described in
'nand-controller.yaml'. But for Meson, its description was missed in
'amlogic,meson-nand.yaml', so this patchset also adds support for it.

Patch for bindings was tested with dt_binding_check and dtbs_check.

Arseniy Krasnov (2):
  dt-bindings: nand: meson: Fix 'nand-rb' property
  mtd: rawnand: meson: waiting w/o wired ready/busy pin

 .../bindings/mtd/amlogic,meson-nand.yaml      |  7 ++
 drivers/mtd/nand/raw/meson_nand.c             | 82 ++++++++++++++++++-
 2 files changed, 85 insertions(+), 4 deletions(-)

-- 
2.35.0


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

* [PATCH v1 1/2] dt-bindings: nand: meson: Fix 'nand-rb' property
  2023-06-07 14:50 [PATCH v1 0/2] Meson NAND: waiting w/o wired ready/busy pin Arseniy Krasnov
@ 2023-06-07 14:50 ` Arseniy Krasnov
  2023-06-07 18:48   ` Krzysztof Kozlowski
  2023-06-07 14:50 ` [PATCH v1 2/2] mtd: rawnand: meson: waiting w/o wired ready/busy pin Arseniy Krasnov
  2023-06-07 15:18 ` [PATCH v1 0/2] Meson NAND: " Arseniy Krasnov
  2 siblings, 1 reply; 8+ messages in thread
From: Arseniy Krasnov @ 2023-06-07 14:50 UTC (permalink / raw)
  To: Liang Yang, Miquel Raynal, Richard Weinberger,
	Vignesh Raghavendra, Rob Herring, Krzysztof Kozlowski,
	Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl
  Cc: oxffffaa, kernel, Arseniy Krasnov, linux-mtd, devicetree,
	linux-arm-kernel, linux-amlogic, linux-kernel

Add description of 'nand-rb' property. Use "Fixes" because this property
must be supported since the beginning.

Fixes: fbc00b5e746f ("dt-bindings: nand: meson: convert txt to yaml")
Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru>
---
 .../devicetree/bindings/mtd/amlogic,meson-nand.yaml        | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
index 28fb9a7dd70f..54208a4191af 100644
--- a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
+++ b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
@@ -58,6 +58,12 @@ patternProperties:
             meson-gxl-nfc 8, 16, 24, 30, 40, 50, 60
             meson-axg-nfc 8
 
+      nand-rb:
+        maxItems: 1
+        items:
+          minimum: 0
+          maximum: 0
+
 required:
   - compatible
   - reg
@@ -87,6 +93,7 @@ examples:
 
       nand@0 {
         reg = <0>;
+        nand-rb = <0>;
       };
     };
 
-- 
2.35.0


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

* [PATCH v1 2/2] mtd: rawnand: meson: waiting w/o wired ready/busy pin
  2023-06-07 14:50 [PATCH v1 0/2] Meson NAND: waiting w/o wired ready/busy pin Arseniy Krasnov
  2023-06-07 14:50 ` [PATCH v1 1/2] dt-bindings: nand: meson: Fix 'nand-rb' property Arseniy Krasnov
@ 2023-06-07 14:50 ` Arseniy Krasnov
  2023-06-07 15:50   ` Miquel Raynal
  2023-06-07 15:18 ` [PATCH v1 0/2] Meson NAND: " Arseniy Krasnov
  2 siblings, 1 reply; 8+ messages in thread
From: Arseniy Krasnov @ 2023-06-07 14:50 UTC (permalink / raw)
  To: Liang Yang, Miquel Raynal, Richard Weinberger,
	Vignesh Raghavendra, Rob Herring, Krzysztof Kozlowski,
	Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl
  Cc: oxffffaa, kernel, Arseniy Krasnov, linux-mtd, devicetree,
	linux-arm-kernel, linux-amlogic, linux-kernel

If there is no wired ready/busy pin, classic way to wait for command
completion is to use function 'nand_soft_waitrdy()'. Meson NAND has
special command which allows to wait for NAND_STATUS_READY bit without
reading status in a software loop (as 'nand_soft_waitrdy()' does). To
use it send this command along with NAND_CMD_STATUS, then wait for an
interrupt, and after interrupt send NAND_CMD_READ0. So this feature
allows to use interrupt driven waiting without wired ready/busy pin.

Suggested-by: Liang Yang <liang.yang@amlogic.com>
Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru>
---
 drivers/mtd/nand/raw/meson_nand.c | 82 +++++++++++++++++++++++++++++--
 1 file changed, 78 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/raw/meson_nand.c b/drivers/mtd/nand/raw/meson_nand.c
index 074e14225c06..935de8e4934d 100644
--- a/drivers/mtd/nand/raw/meson_nand.c
+++ b/drivers/mtd/nand/raw/meson_nand.c
@@ -38,6 +38,7 @@
 #define NFC_CMD_SCRAMBLER_DISABLE	0
 #define NFC_CMD_SHORTMODE_DISABLE	0
 #define NFC_CMD_RB_INT		BIT(14)
+#define NFC_CMD_RB_INT_NO_PIN	((0xb << 10) | BIT(18) | BIT(16))
 
 #define NFC_CMD_GET_SIZE(x)	(((x) >> 22) & GENMASK(4, 0))
 
@@ -179,6 +180,7 @@ struct meson_nfc {
 	u32 info_bytes;
 
 	unsigned long assigned_cs;
+	bool no_rb_pin;
 };
 
 enum {
@@ -392,7 +394,42 @@ static void meson_nfc_set_data_oob(struct nand_chip *nand,
 	}
 }
 
-static int meson_nfc_queue_rb(struct meson_nfc *nfc, int timeout_ms)
+static int meson_nfc_wait_no_rb_pin(struct meson_nfc *nfc, int timeout_ms,
+				    bool need_cmd_read0)
+{
+	u32 cmd, cfg;
+
+	meson_nfc_cmd_idle(nfc, nfc->timing.twb);
+	meson_nfc_drain_cmd(nfc);
+	meson_nfc_wait_cmd_finish(nfc, CMD_FIFO_EMPTY_TIMEOUT);
+
+	cfg = readl(nfc->reg_base + NFC_REG_CFG);
+	cfg |= NFC_RB_IRQ_EN;
+	writel(cfg, nfc->reg_base + NFC_REG_CFG);
+
+	reinit_completion(&nfc->completion);
+	cmd = nfc->param.chip_select | NFC_CMD_CLE | NAND_CMD_STATUS;
+	writel(cmd, nfc->reg_base + NFC_REG_CMD);
+
+	/* use the max erase time as the maximum clock for waiting R/B */
+	cmd = NFC_CMD_RB | NFC_CMD_RB_INT_NO_PIN | nfc->timing.tbers_max;
+	writel(cmd, nfc->reg_base + NFC_REG_CMD);
+
+	if (!wait_for_completion_timeout(&nfc->completion,
+					 msecs_to_jiffies(timeout_ms)))
+		return -ETIMEDOUT;
+
+	if (need_cmd_read0) {
+		cmd = nfc->param.chip_select | NFC_CMD_CLE | NAND_CMD_READ0;
+		writel(cmd, nfc->reg_base + NFC_REG_CMD);
+		meson_nfc_drain_cmd(nfc);
+		meson_nfc_wait_cmd_finish(nfc, CMD_FIFO_EMPTY_TIMEOUT);
+	}
+
+	return 0;
+}
+
+static int meson_nfc_wait_rb_pin(struct meson_nfc *nfc, int timeout_ms)
 {
 	u32 cmd, cfg;
 	int ret = 0;
@@ -420,6 +457,27 @@ static int meson_nfc_queue_rb(struct meson_nfc *nfc, int timeout_ms)
 	return ret;
 }
 
+static int meson_nfc_queue_rb(struct meson_nfc *nfc, int timeout_ms,
+			      bool need_cmd_read0)
+{
+	if (nfc->no_rb_pin) {
+		/* This mode is used when there is no wired R/B pin.
+		 * It works like 'nand_soft_waitrdy()', but instead of
+		 * polling NAND_CMD_STATUS bit in the software loop,
+		 * it will wait for interrupt - controllers checks IO
+		 * bus and when it detects NAND_CMD_STATUS on it, it
+		 * raises interrupt. After interrupt, NAND_CMD_READ0 is
+		 * sent as terminator of the ready waiting procedure if
+		 * needed (for all cases except page programming - this
+		 * is reason of 'need_cmd_read0' flag).
+		 */
+		return meson_nfc_wait_no_rb_pin(nfc, timeout_ms,
+						need_cmd_read0);
+	} else {
+		return meson_nfc_wait_rb_pin(nfc, timeout_ms);
+	}
+}
+
 static void meson_nfc_set_user_byte(struct nand_chip *nand, u8 *oob_buf)
 {
 	struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
@@ -623,7 +681,7 @@ static int meson_nfc_rw_cmd_prepare_and_execute(struct nand_chip *nand,
 	if (in) {
 		nfc->cmdfifo.rw.cmd1 = cs | NFC_CMD_CLE | NAND_CMD_READSTART;
 		writel(nfc->cmdfifo.rw.cmd1, nfc->reg_base + NFC_REG_CMD);
-		meson_nfc_queue_rb(nfc, PSEC_TO_MSEC(sdr->tR_max));
+		meson_nfc_queue_rb(nfc, PSEC_TO_MSEC(sdr->tR_max), true);
 	} else {
 		meson_nfc_cmd_idle(nfc, nfc->timing.tadl);
 	}
@@ -669,7 +727,7 @@ static int meson_nfc_write_page_sub(struct nand_chip *nand,
 
 	cmd = nfc->param.chip_select | NFC_CMD_CLE | NAND_CMD_PAGEPROG;
 	writel(cmd, nfc->reg_base + NFC_REG_CMD);
-	meson_nfc_queue_rb(nfc, PSEC_TO_MSEC(sdr->tPROG_max));
+	meson_nfc_queue_rb(nfc, PSEC_TO_MSEC(sdr->tPROG_max), false);
 
 	meson_nfc_dma_buffer_release(nand, data_len, info_len, DMA_TO_DEVICE);
 
@@ -952,7 +1010,8 @@ static int meson_nfc_exec_op(struct nand_chip *nand,
 			break;
 
 		case NAND_OP_WAITRDY_INSTR:
-			meson_nfc_queue_rb(nfc, instr->ctx.waitrdy.timeout_ms);
+			meson_nfc_queue_rb(nfc, instr->ctx.waitrdy.timeout_ms,
+					   true);
 			if (instr->delay_ns)
 				meson_nfc_cmd_idle(nfc, delay_idle);
 			break;
@@ -1248,6 +1307,7 @@ meson_nfc_nand_chip_init(struct device *dev,
 	struct mtd_info *mtd;
 	int ret, i;
 	u32 tmp, nsels;
+	u32 nand_rb_val;
 
 	nsels = of_property_count_elems_of_size(np, "reg", sizeof(u32));
 	if (!nsels || nsels > MAX_CE_NUM) {
@@ -1287,6 +1347,20 @@ meson_nfc_nand_chip_init(struct device *dev,
 	mtd->owner = THIS_MODULE;
 	mtd->dev.parent = dev;
 
+	ret = of_property_read_u32(np, "nand-rb", &nand_rb_val);
+	if (ret) {
+		/* If property was not found, don't use rb pin. */
+		if (ret == -EINVAL)
+			nfc->no_rb_pin = true;
+		else
+			return -EINVAL;
+	} else {
+		if (nand_rb_val)
+			return -EINVAL;
+
+		nfc->no_rb_pin = false;
+	}
+
 	ret = nand_scan(nand, nsels);
 	if (ret)
 		return ret;
-- 
2.35.0


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

* Re: [PATCH v1 0/2] Meson NAND: waiting w/o wired ready/busy pin
  2023-06-07 14:50 [PATCH v1 0/2] Meson NAND: waiting w/o wired ready/busy pin Arseniy Krasnov
  2023-06-07 14:50 ` [PATCH v1 1/2] dt-bindings: nand: meson: Fix 'nand-rb' property Arseniy Krasnov
  2023-06-07 14:50 ` [PATCH v1 2/2] mtd: rawnand: meson: waiting w/o wired ready/busy pin Arseniy Krasnov
@ 2023-06-07 15:18 ` Arseniy Krasnov
  2023-06-07 18:48   ` Krzysztof Kozlowski
  2 siblings, 1 reply; 8+ messages in thread
From: Arseniy Krasnov @ 2023-06-07 15:18 UTC (permalink / raw)
  To: Liang Yang, Miquel Raynal, Richard Weinberger,
	Vignesh Raghavendra, Rob Herring, Krzysztof Kozlowski,
	Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl
  Cc: oxffffaa, kernel, linux-mtd, devicetree, linux-arm-kernel,
	linux-amlogic, linux-kernel



On 07.06.2023 17:50, Arseniy Krasnov wrote:
> Main thing that this patchset adds is support for waiting for command
> completion when there is no wired ready/busy pin. This is implemented
> by using Meson specific command which operates without wired pin, but
> supports interrupt driven waiting (default way for waiting without
> hardware pin is 'nand_soft_waitrdy()' which uses software loop instead
> of interrupt). To select which mode to use during driver initialization,
> common NAND controller property is used - 'nand-rb'. It is described in
> 'nand-controller.yaml'. But for Meson, its description was missed in
> 'amlogic,meson-nand.yaml', so this patchset also adds support for it.
> 
> Patch for bindings was tested with dt_binding_check and dtbs_check.
> 
> Arseniy Krasnov (2):
>   dt-bindings: nand: meson: Fix 'nand-rb' property
>   mtd: rawnand: meson: waiting w/o wired ready/busy pin
> 
>  .../bindings/mtd/amlogic,meson-nand.yaml      |  7 ++
>  drivers/mtd/nand/raw/meson_nand.c             | 82 ++++++++++++++++++-
>  2 files changed, 85 insertions(+), 4 deletions(-)
> 

Changelog:
* Two separate patches:
  For bindings: https://lore.kernel.org/linux-mtd/20230606193507.35024-1-AVKrasnov@sberdevices.ru/
  For driver: https://lore.kernel.org/linux-mtd/20230607073015.1280085-1-AVKrasnov@sberdevices.ru/
  Now fixed/updated and combined to this patchset with v1 version.

* In binding patch:
  * Invalid type of 'nand-rb' is fixed, it was 'bool', now it is 'uint32 array' as required. Also
    new declaration of 'nand-rb' contains several restrictions like number of elements and min/max
    value of elements.
  * Location of 'nand-rb' is moved from the controller object to the chip object as required.

* In driver patch:
  * Update comment which describes when it is needed to send NAND_CMD_READ0 to
    leave command waiting loop.
  * Change type of 'no_rb_pin' to 'bool' as in v1. Now it is just a flag which selects
    mode to wait for command completion.

Thanks, Arseniy
  

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

* Re: [PATCH v1 2/2] mtd: rawnand: meson: waiting w/o wired ready/busy pin
  2023-06-07 14:50 ` [PATCH v1 2/2] mtd: rawnand: meson: waiting w/o wired ready/busy pin Arseniy Krasnov
@ 2023-06-07 15:50   ` Miquel Raynal
  0 siblings, 0 replies; 8+ messages in thread
From: Miquel Raynal @ 2023-06-07 15:50 UTC (permalink / raw)
  To: Arseniy Krasnov
  Cc: Liang Yang, Richard Weinberger, Vignesh Raghavendra, Rob Herring,
	Krzysztof Kozlowski, Neil Armstrong, Kevin Hilman, Jerome Brunet,
	Martin Blumenstingl, oxffffaa, kernel, linux-mtd, devicetree,
	linux-arm-kernel, linux-amlogic, linux-kernel

Hi Arseniy,

AVKrasnov@sberdevices.ru wrote on Wed, 7 Jun 2023 17:50:25 +0300:

> If there is no wired ready/busy pin, classic way to wait for command
> completion is to use function 'nand_soft_waitrdy()'. Meson NAND has
> special command which allows to wait for NAND_STATUS_READY bit without
> reading status in a software loop (as 'nand_soft_waitrdy()' does). To
> use it send this command along with NAND_CMD_STATUS, then wait for an
> interrupt, and after interrupt send NAND_CMD_READ0. So this feature
> allows to use interrupt driven waiting without wired ready/busy pin.
> 
> Suggested-by: Liang Yang <liang.yang@amlogic.com>
> Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru>
> ---
>  drivers/mtd/nand/raw/meson_nand.c | 82 +++++++++++++++++++++++++++++--
>  1 file changed, 78 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/meson_nand.c b/drivers/mtd/nand/raw/meson_nand.c
> index 074e14225c06..935de8e4934d 100644
> --- a/drivers/mtd/nand/raw/meson_nand.c
> +++ b/drivers/mtd/nand/raw/meson_nand.c
> @@ -38,6 +38,7 @@
>  #define NFC_CMD_SCRAMBLER_DISABLE	0
>  #define NFC_CMD_SHORTMODE_DISABLE	0
>  #define NFC_CMD_RB_INT		BIT(14)
> +#define NFC_CMD_RB_INT_NO_PIN	((0xb << 10) | BIT(18) | BIT(16))
>  
>  #define NFC_CMD_GET_SIZE(x)	(((x) >> 22) & GENMASK(4, 0))
>  
> @@ -179,6 +180,7 @@ struct meson_nfc {
>  	u32 info_bytes;
>  
>  	unsigned long assigned_cs;
> +	bool no_rb_pin;
>  };
>  
>  enum {
> @@ -392,7 +394,42 @@ static void meson_nfc_set_data_oob(struct nand_chip *nand,
>  	}
>  }
>  
> -static int meson_nfc_queue_rb(struct meson_nfc *nfc, int timeout_ms)
> +static int meson_nfc_wait_no_rb_pin(struct meson_nfc *nfc, int timeout_ms,
> +				    bool need_cmd_read0)
> +{
> +	u32 cmd, cfg;
> +
> +	meson_nfc_cmd_idle(nfc, nfc->timing.twb);
> +	meson_nfc_drain_cmd(nfc);
> +	meson_nfc_wait_cmd_finish(nfc, CMD_FIFO_EMPTY_TIMEOUT);
> +
> +	cfg = readl(nfc->reg_base + NFC_REG_CFG);
> +	cfg |= NFC_RB_IRQ_EN;
> +	writel(cfg, nfc->reg_base + NFC_REG_CFG);
> +
> +	reinit_completion(&nfc->completion);
> +	cmd = nfc->param.chip_select | NFC_CMD_CLE | NAND_CMD_STATUS;
> +	writel(cmd, nfc->reg_base + NFC_REG_CMD);
> +
> +	/* use the max erase time as the maximum clock for waiting R/B */
> +	cmd = NFC_CMD_RB | NFC_CMD_RB_INT_NO_PIN | nfc->timing.tbers_max;
> +	writel(cmd, nfc->reg_base + NFC_REG_CMD);
> +
> +	if (!wait_for_completion_timeout(&nfc->completion,
> +					 msecs_to_jiffies(timeout_ms)))
> +		return -ETIMEDOUT;
> +
> +	if (need_cmd_read0) {
> +		cmd = nfc->param.chip_select | NFC_CMD_CLE | NAND_CMD_READ0;
> +		writel(cmd, nfc->reg_base + NFC_REG_CMD);
> +		meson_nfc_drain_cmd(nfc);
> +		meson_nfc_wait_cmd_finish(nfc, CMD_FIFO_EMPTY_TIMEOUT);
> +	}
> +
> +	return 0;
> +}
> +
> +static int meson_nfc_wait_rb_pin(struct meson_nfc *nfc, int timeout_ms)
>  {
>  	u32 cmd, cfg;
>  	int ret = 0;
> @@ -420,6 +457,27 @@ static int meson_nfc_queue_rb(struct meson_nfc *nfc, int timeout_ms)
>  	return ret;
>  }
>  
> +static int meson_nfc_queue_rb(struct meson_nfc *nfc, int timeout_ms,
> +			      bool need_cmd_read0)
> +{
> +	if (nfc->no_rb_pin) {
> +		/* This mode is used when there is no wired R/B pin.
> +		 * It works like 'nand_soft_waitrdy()', but instead of
> +		 * polling NAND_CMD_STATUS bit in the software loop,
> +		 * it will wait for interrupt - controllers checks IO
> +		 * bus and when it detects NAND_CMD_STATUS on it, it
> +		 * raises interrupt. After interrupt, NAND_CMD_READ0 is
> +		 * sent as terminator of the ready waiting procedure if
> +		 * needed (for all cases except page programming - this
> +		 * is reason of 'need_cmd_read0' flag).
> +		 */
> +		return meson_nfc_wait_no_rb_pin(nfc, timeout_ms,
> +						need_cmd_read0);
> +	} else {
> +		return meson_nfc_wait_rb_pin(nfc, timeout_ms);
> +	}
> +}
> +
>  static void meson_nfc_set_user_byte(struct nand_chip *nand, u8 *oob_buf)
>  {
>  	struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
> @@ -623,7 +681,7 @@ static int meson_nfc_rw_cmd_prepare_and_execute(struct nand_chip *nand,
>  	if (in) {
>  		nfc->cmdfifo.rw.cmd1 = cs | NFC_CMD_CLE | NAND_CMD_READSTART;
>  		writel(nfc->cmdfifo.rw.cmd1, nfc->reg_base + NFC_REG_CMD);
> -		meson_nfc_queue_rb(nfc, PSEC_TO_MSEC(sdr->tR_max));
> +		meson_nfc_queue_rb(nfc, PSEC_TO_MSEC(sdr->tR_max), true);
>  	} else {
>  		meson_nfc_cmd_idle(nfc, nfc->timing.tadl);
>  	}
> @@ -669,7 +727,7 @@ static int meson_nfc_write_page_sub(struct nand_chip *nand,
>  
>  	cmd = nfc->param.chip_select | NFC_CMD_CLE | NAND_CMD_PAGEPROG;
>  	writel(cmd, nfc->reg_base + NFC_REG_CMD);
> -	meson_nfc_queue_rb(nfc, PSEC_TO_MSEC(sdr->tPROG_max));
> +	meson_nfc_queue_rb(nfc, PSEC_TO_MSEC(sdr->tPROG_max), false);
>  
>  	meson_nfc_dma_buffer_release(nand, data_len, info_len, DMA_TO_DEVICE);
>  
> @@ -952,7 +1010,8 @@ static int meson_nfc_exec_op(struct nand_chip *nand,
>  			break;
>  
>  		case NAND_OP_WAITRDY_INSTR:
> -			meson_nfc_queue_rb(nfc, instr->ctx.waitrdy.timeout_ms);
> +			meson_nfc_queue_rb(nfc, instr->ctx.waitrdy.timeout_ms,
> +					   true);
>  			if (instr->delay_ns)
>  				meson_nfc_cmd_idle(nfc, delay_idle);
>  			break;
> @@ -1248,6 +1307,7 @@ meson_nfc_nand_chip_init(struct device *dev,
>  	struct mtd_info *mtd;
>  	int ret, i;
>  	u32 tmp, nsels;
> +	u32 nand_rb_val;
>  
>  	nsels = of_property_count_elems_of_size(np, "reg", sizeof(u32));
>  	if (!nsels || nsels > MAX_CE_NUM) {
> @@ -1287,6 +1347,20 @@ meson_nfc_nand_chip_init(struct device *dev,
>  	mtd->owner = THIS_MODULE;
>  	mtd->dev.parent = dev;
>  
> +	ret = of_property_read_u32(np, "nand-rb", &nand_rb_val);
> +	if (ret) {
> +		/* If property was not found, don't use rb pin. */
> +		if (ret == -EINVAL)
> +			nfc->no_rb_pin = true;
> +		else
> +			return -EINVAL;

You must propagate the real error code, so return ret here.

> +	} else {
> +		if (nand_rb_val)
> +			return -EINVAL;
> +
> +		nfc->no_rb_pin = false;

I expect nfc structure to be allocated with kzalloc, if that's the
case, then you don't need this line. Actually if you reset nand_rb_val
at creation time you could have something more readable, like:

	ret = of_prop(...)
	if (ret == -EINVAL)
		no_rb_pin = true;
	else if (ret)
		return ret;

	if (nand_rb_val)
		return -EINVAL;

Otherwise both patches look good to me.

> +	}
> +
>  	ret = nand_scan(nand, nsels);
>  	if (ret)
>  		return ret;


Thanks,
Miquèl

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

* Re: [PATCH v1 0/2] Meson NAND: waiting w/o wired ready/busy pin
  2023-06-07 15:18 ` [PATCH v1 0/2] Meson NAND: " Arseniy Krasnov
@ 2023-06-07 18:48   ` Krzysztof Kozlowski
  2023-06-07 18:58     ` Arseniy Krasnov
  0 siblings, 1 reply; 8+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-07 18:48 UTC (permalink / raw)
  To: Arseniy Krasnov, Liang Yang, Miquel Raynal, Richard Weinberger,
	Vignesh Raghavendra, Rob Herring, Krzysztof Kozlowski,
	Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl
  Cc: oxffffaa, kernel, linux-mtd, devicetree, linux-arm-kernel,
	linux-amlogic, linux-kernel

On 07/06/2023 17:18, Arseniy Krasnov wrote:
> 
> 
> On 07.06.2023 17:50, Arseniy Krasnov wrote:
>> Main thing that this patchset adds is support for waiting for command
>> completion when there is no wired ready/busy pin. This is implemented
>> by using Meson specific command which operates without wired pin, but
>> supports interrupt driven waiting (default way for waiting without
>> hardware pin is 'nand_soft_waitrdy()' which uses software loop instead
>> of interrupt). To select which mode to use during driver initialization,
>> common NAND controller property is used - 'nand-rb'. It is described in
>> 'nand-controller.yaml'. But for Meson, its description was missed in
>> 'amlogic,meson-nand.yaml', so this patchset also adds support for it.
>>
>> Patch for bindings was tested with dt_binding_check and dtbs_check.
>>
>> Arseniy Krasnov (2):
>>   dt-bindings: nand: meson: Fix 'nand-rb' property
>>   mtd: rawnand: meson: waiting w/o wired ready/busy pin
>>
>>  .../bindings/mtd/amlogic,meson-nand.yaml      |  7 ++
>>  drivers/mtd/nand/raw/meson_nand.c             | 82 ++++++++++++++++++-
>>  2 files changed, 85 insertions(+), 4 deletions(-)
>>
> 
> Changelog:
> * Two separate patches:
>   For bindings: https://lore.kernel.org/linux-mtd/20230606193507.35024-1-AVKrasnov@sberdevices.ru/
>   For driver: https://lore.kernel.org/linux-mtd/20230607073015.1280085-1-AVKrasnov@sberdevices.ru/
>   Now fixed/updated and combined to this patchset with v1 version.
> 
> * In binding patch:
>   * Invalid type of 'nand-rb' is fixed, it was 'bool', now it is 'uint32 array' as required. Also
>     new declaration of 'nand-rb' contains several restrictions like number of elements and min/max
>     value of elements.
>   * Location of 'nand-rb' is moved from the controller object to the chip object as required.

Version your patchsets. This is v2, not v1.

Best regards,
Krzysztof


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

* Re: [PATCH v1 1/2] dt-bindings: nand: meson: Fix 'nand-rb' property
  2023-06-07 14:50 ` [PATCH v1 1/2] dt-bindings: nand: meson: Fix 'nand-rb' property Arseniy Krasnov
@ 2023-06-07 18:48   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 8+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-07 18:48 UTC (permalink / raw)
  To: Arseniy Krasnov, Liang Yang, Miquel Raynal, Richard Weinberger,
	Vignesh Raghavendra, Rob Herring, Krzysztof Kozlowski,
	Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl
  Cc: oxffffaa, kernel, linux-mtd, devicetree, linux-arm-kernel,
	linux-amlogic, linux-kernel

On 07/06/2023 16:50, Arseniy Krasnov wrote:
> Add description of 'nand-rb' property. Use "Fixes" because this property
> must be supported since the beginning.
> 
> Fixes: fbc00b5e746f ("dt-bindings: nand: meson: convert txt to yaml")
> Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru>
> ---


>  .../devicetree/bindings/mtd/amlogic,meson-nand.yaml        | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
> index 28fb9a7dd70f..54208a4191af 100644
> --- a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
> +++ b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
> @@ -58,6 +58,12 @@ patternProperties:
>              meson-gxl-nfc 8, 16, 24, 30, 40, 50, 60
>              meson-axg-nfc 8
>  
> +      nand-rb:
> +        maxItems: 1
> +        items:
> +          minimum: 0

Drop minimum. It's uint.

> +          maximum: 0
> +

Best regards,
Krzysztof


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

* Re: [PATCH v1 0/2] Meson NAND: waiting w/o wired ready/busy pin
  2023-06-07 18:48   ` Krzysztof Kozlowski
@ 2023-06-07 18:58     ` Arseniy Krasnov
  0 siblings, 0 replies; 8+ messages in thread
From: Arseniy Krasnov @ 2023-06-07 18:58 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Liang Yang, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, Rob Herring,
	Krzysztof Kozlowski, Neil Armstrong, Kevin Hilman, Jerome Brunet,
	Martin Blumenstingl
  Cc: oxffffaa, kernel, linux-mtd, devicetree, linux-arm-kernel,
	linux-amlogic, linux-kernel



On 07.06.2023 21:48, Krzysztof Kozlowski wrote:
> On 07/06/2023 17:18, Arseniy Krasnov wrote:
>>
>>
>> On 07.06.2023 17:50, Arseniy Krasnov wrote:
>>> Main thing that this patchset adds is support for waiting for command
>>> completion when there is no wired ready/busy pin. This is implemented
>>> by using Meson specific command which operates without wired pin, but
>>> supports interrupt driven waiting (default way for waiting without
>>> hardware pin is 'nand_soft_waitrdy()' which uses software loop instead
>>> of interrupt). To select which mode to use during driver initialization,
>>> common NAND controller property is used - 'nand-rb'. It is described in
>>> 'nand-controller.yaml'. But for Meson, its description was missed in
>>> 'amlogic,meson-nand.yaml', so this patchset also adds support for it.
>>>
>>> Patch for bindings was tested with dt_binding_check and dtbs_check.
>>>
>>> Arseniy Krasnov (2):
>>>   dt-bindings: nand: meson: Fix 'nand-rb' property
>>>   mtd: rawnand: meson: waiting w/o wired ready/busy pin
>>>
>>>  .../bindings/mtd/amlogic,meson-nand.yaml      |  7 ++
>>>  drivers/mtd/nand/raw/meson_nand.c             | 82 ++++++++++++++++++-
>>>  2 files changed, 85 insertions(+), 4 deletions(-)
>>>
>>
>> Changelog:
>> * Two separate patches:
>>   For bindings: https://lore.kernel.org/linux-mtd/20230606193507.35024-1-AVKrasnov@sberdevices.ru/
>>   For driver: https://lore.kernel.org/linux-mtd/20230607073015.1280085-1-AVKrasnov@sberdevices.ru/
>>   Now fixed/updated and combined to this patchset with v1 version.
>>
>> * In binding patch:
>>   * Invalid type of 'nand-rb' is fixed, it was 'bool', now it is 'uint32 array' as required. Also
>>     new declaration of 'nand-rb' contains several restrictions like number of elements and min/max
>>     value of elements.
>>   * Location of 'nand-rb' is moved from the controller object to the chip object as required.
> 
> Version your patchsets. This is v2, not v1.

Ack, next will be v3

Thanks, Arseniy

> 
> Best regards,
> Krzysztof
> 

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

end of thread, other threads:[~2023-06-07 19:03 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-07 14:50 [PATCH v1 0/2] Meson NAND: waiting w/o wired ready/busy pin Arseniy Krasnov
2023-06-07 14:50 ` [PATCH v1 1/2] dt-bindings: nand: meson: Fix 'nand-rb' property Arseniy Krasnov
2023-06-07 18:48   ` Krzysztof Kozlowski
2023-06-07 14:50 ` [PATCH v1 2/2] mtd: rawnand: meson: waiting w/o wired ready/busy pin Arseniy Krasnov
2023-06-07 15:50   ` Miquel Raynal
2023-06-07 15:18 ` [PATCH v1 0/2] Meson NAND: " Arseniy Krasnov
2023-06-07 18:48   ` Krzysztof Kozlowski
2023-06-07 18:58     ` Arseniy Krasnov

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).