Linux-mtd Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2 1/4] dt-bindings: mtd: rawnand: ingenic: Clarify the active state of the RB pin
@ 2020-05-19 23:24 Boris Brezillon
  2020-05-19 23:24 ` [PATCH v2 2/4] dt-bindings: mtd: nand: Document the generic rb-gpios property Boris Brezillon
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Boris Brezillon @ 2020-05-19 23:24 UTC (permalink / raw)
  To: Paul Cercueil, Harvey Hunt, Miquel Raynal, linux-mtd
  Cc: Mark Rutland, devicetree, Vignesh Raghavendra, Tudor Ambarus,
	Richard Weinberger, Rob Herring, Boris Brezillon

Let's make things consistent with other bindings and clarify the
expected active state.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
Changes in v2:
* New patch
---
 Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt b/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
index c02259353327..4cbe13f94564 100644
--- a/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
@@ -30,7 +30,8 @@ Optional children node properties:
 - nand-ecc-strength: ECC strength (max number of correctable bits).
 - nand-ecc-mode: String, operation mode of the NAND ecc mode. "hw" by default
 - nand-on-flash-bbt: boolean to enable on flash bbt option, if not present false
-- rb-gpios: GPIO specifier for the busy pin.
+- rb-gpios: GPIO specifier for the ready/busy pin. The active state (ready)
+            should be high unless the signal is inverted.
 - wp-gpios: GPIO specifier for the write protect pin.
 
 Optional child node of NAND chip nodes:
-- 
2.25.4


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* [PATCH v2 2/4] dt-bindings: mtd: nand: Document the generic rb-gpios property
  2020-05-19 23:24 [PATCH v2 1/4] dt-bindings: mtd: rawnand: ingenic: Clarify the active state of the RB pin Boris Brezillon
@ 2020-05-19 23:24 ` Boris Brezillon
  2020-05-28 21:22   ` Rob Herring
  2020-05-19 23:24 ` [PATCH v2 3/4] mtd: rawnand: ingenic: Fix the RB gpio active-high property on qi, lb60 Boris Brezillon
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Boris Brezillon @ 2020-05-19 23:24 UTC (permalink / raw)
  To: Paul Cercueil, Harvey Hunt, Miquel Raynal, linux-mtd
  Cc: Mark Rutland, devicetree, Vignesh Raghavendra, Tudor Ambarus,
	Richard Weinberger, Rob Herring, Boris Brezillon

A few drivers use this property to describe GPIO pins used to sample
the NAND Ready/Busy state. Let's make it part of the generic binding
doc.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
Changes in v2:
* New patch
---
 Documentation/devicetree/bindings/mtd/nand-controller.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/nand-controller.yaml b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
index d261b7096c69..17f4dc8f8cab 100644
--- a/Documentation/devicetree/bindings/mtd/nand-controller.yaml
+++ b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
@@ -119,6 +119,13 @@ patternProperties:
         description:
           Contains the native Ready/Busy IDs.
 
+      rb-gpios:
+        description:
+          Contains one or more GPIO descriptor (the numper of descriptor
+          depends on the number of R/B pins exposed by the flash) for the
+          Ready/Busy pins. Active state refers to the NAND ready state and
+          should be set to GPIOD_ACTIVE_HIGH unless the signal is inverted.
+
     required:
       - reg
 
-- 
2.25.4


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* [PATCH v2 3/4] mtd: rawnand: ingenic: Fix the RB gpio active-high property on qi, lb60
  2020-05-19 23:24 [PATCH v2 1/4] dt-bindings: mtd: rawnand: ingenic: Clarify the active state of the RB pin Boris Brezillon
  2020-05-19 23:24 ` [PATCH v2 2/4] dt-bindings: mtd: nand: Document the generic rb-gpios property Boris Brezillon
@ 2020-05-19 23:24 ` Boris Brezillon
  2020-05-24 19:03   ` Miquel Raynal
  2020-05-19 23:24 ` [PATCH v2 4/4] mtd: rawnand: ingenic: Convert the driver to exec_op() Boris Brezillon
  2020-05-20  0:25 ` [PATCH v2 1/4] dt-bindings: mtd: rawnand: ingenic: Clarify the active state of the RB pin Paul Cercueil
  3 siblings, 1 reply; 9+ messages in thread
From: Boris Brezillon @ 2020-05-19 23:24 UTC (permalink / raw)
  To: Paul Cercueil, Harvey Hunt, Miquel Raynal, linux-mtd
  Cc: Mark Rutland, devicetree, Vignesh Raghavendra, Tudor Ambarus,
	Richard Weinberger, Rob Herring, Boris Brezillon

The rb-gpios semantics was undocumented and qi,lb60 (along with the
ingenic driver) got it wrong. The active state encodes the NAND ready
state, which is high level. Since there's no signal inverter on this
board, it should be active-high. Let's fix that here for older DTs so
we can re-use the generic nand_gpio_waitrdy() helper, and be consistent
with what other drivers do.

Suggested-by: Paul Cercueil <paul@crapouillou.net>
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
Changes in v2:
* New patch
---
 drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c b/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c
index e7bd845fdbf5..e939404e1383 100644
--- a/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c
+++ b/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c
@@ -184,7 +184,7 @@ static int ingenic_nand_dev_ready(struct nand_chip *chip)
 {
 	struct ingenic_nand *nand = to_ingenic_nand(nand_to_mtd(chip));
 
-	return !gpiod_get_value_cansleep(nand->busy_gpio);
+	return gpiod_get_value_cansleep(nand->busy_gpio);
 }
 
 static void ingenic_nand_ecc_hwctl(struct nand_chip *chip, int mode)
@@ -343,6 +343,18 @@ static int ingenic_nand_init_chip(struct platform_device *pdev,
 		nand->chip.legacy.dev_ready = ingenic_nand_dev_ready;
 	}
 
+	/*
+	 * The rb-gpios semantics was undocumented and qi,lb60 (along with
+	 * the ingenic driver) got it wrong. The active state encodes the
+	 * NAND ready state, which is high level. Since there's no signal
+	 * inverter on this board, it should be active-high. Let's fix that
+	 * here for older DTs so we can re-use the generic nand_gpio_waitrdy()
+	 * helper, and be consistent with what other drivers do.
+	 */
+	if (of_machine_is_compatible("qi,lb60") &&
+	    gpiod_is_active_low(nand->busy_gpio))
+		gpiod_toggle_active_low(nand->busy_gpio);
+
 	nand->wp_gpio = devm_gpiod_get_optional(dev, "wp", GPIOD_OUT_LOW);
 
 	if (IS_ERR(nand->wp_gpio)) {
-- 
2.25.4


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* [PATCH v2 4/4] mtd: rawnand: ingenic: Convert the driver to exec_op()
  2020-05-19 23:24 [PATCH v2 1/4] dt-bindings: mtd: rawnand: ingenic: Clarify the active state of the RB pin Boris Brezillon
  2020-05-19 23:24 ` [PATCH v2 2/4] dt-bindings: mtd: nand: Document the generic rb-gpios property Boris Brezillon
  2020-05-19 23:24 ` [PATCH v2 3/4] mtd: rawnand: ingenic: Fix the RB gpio active-high property on qi, lb60 Boris Brezillon
@ 2020-05-19 23:24 ` Boris Brezillon
  2020-05-20  0:37   ` Paul Cercueil
  2020-05-24 19:03   ` Miquel Raynal
  2020-05-20  0:25 ` [PATCH v2 1/4] dt-bindings: mtd: rawnand: ingenic: Clarify the active state of the RB pin Paul Cercueil
  3 siblings, 2 replies; 9+ messages in thread
From: Boris Brezillon @ 2020-05-19 23:24 UTC (permalink / raw)
  To: Paul Cercueil, Harvey Hunt, Miquel Raynal, linux-mtd
  Cc: Mark Rutland, devicetree, Vignesh Raghavendra, Tudor Ambarus,
	Richard Weinberger, Rob Herring, Boris Brezillon

Let's convert the driver to exec_op() to have one less driver relying
on the legacy interface.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
Changes in v2:
* Add a delay after instructions when needed
* s/cmd_offset/addr_offset/

Paul, I didn't add your T-b since this new version follows the path
you proposed for the R/B polarity inversion issue. Feel free to add
it back if it still works.
---
 .../mtd/nand/raw/ingenic/ingenic_nand_drv.c   | 139 +++++++++++-------
 1 file changed, 83 insertions(+), 56 deletions(-)

diff --git a/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c b/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c
index e939404e1383..3659e62829f9 100644
--- a/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c
+++ b/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c
@@ -27,9 +27,6 @@
 
 #define DRV_NAME	"ingenic-nand"
 
-/* Command delay when there is no R/B pin. */
-#define RB_DELAY_US	100
-
 struct jz_soc_info {
 	unsigned long data_offset;
 	unsigned long addr_offset;
@@ -49,7 +46,6 @@ struct ingenic_nfc {
 	struct nand_controller controller;
 	unsigned int num_banks;
 	struct list_head chips;
-	int selected;
 	struct ingenic_nand_cs cs[];
 };
 
@@ -142,51 +138,6 @@ static const struct mtd_ooblayout_ops jz4725b_ooblayout_ops = {
 	.free = jz4725b_ooblayout_free,
 };
 
-static void ingenic_nand_select_chip(struct nand_chip *chip, int chipnr)
-{
-	struct ingenic_nand *nand = to_ingenic_nand(nand_to_mtd(chip));
-	struct ingenic_nfc *nfc = to_ingenic_nfc(nand->chip.controller);
-	struct ingenic_nand_cs *cs;
-
-	/* Ensure the currently selected chip is deasserted. */
-	if (chipnr == -1 && nfc->selected >= 0) {
-		cs = &nfc->cs[nfc->selected];
-		jz4780_nemc_assert(nfc->dev, cs->bank, false);
-	}
-
-	nfc->selected = chipnr;
-}
-
-static void ingenic_nand_cmd_ctrl(struct nand_chip *chip, int cmd,
-				  unsigned int ctrl)
-{
-	struct ingenic_nand *nand = to_ingenic_nand(nand_to_mtd(chip));
-	struct ingenic_nfc *nfc = to_ingenic_nfc(nand->chip.controller);
-	struct ingenic_nand_cs *cs;
-
-	if (WARN_ON(nfc->selected < 0))
-		return;
-
-	cs = &nfc->cs[nfc->selected];
-
-	jz4780_nemc_assert(nfc->dev, cs->bank, ctrl & NAND_NCE);
-
-	if (cmd == NAND_CMD_NONE)
-		return;
-
-	if (ctrl & NAND_ALE)
-		writeb(cmd, cs->base + nfc->soc_info->addr_offset);
-	else if (ctrl & NAND_CLE)
-		writeb(cmd, cs->base + nfc->soc_info->cmd_offset);
-}
-
-static int ingenic_nand_dev_ready(struct nand_chip *chip)
-{
-	struct ingenic_nand *nand = to_ingenic_nand(nand_to_mtd(chip));
-
-	return gpiod_get_value_cansleep(nand->busy_gpio);
-}
-
 static void ingenic_nand_ecc_hwctl(struct nand_chip *chip, int mode)
 {
 	struct ingenic_nand *nand = to_ingenic_nand(nand_to_mtd(chip));
@@ -298,8 +249,91 @@ static int ingenic_nand_attach_chip(struct nand_chip *chip)
 	return 0;
 }
 
+static int ingenic_nand_exec_instr(struct nand_chip *chip,
+				   struct ingenic_nand_cs *cs,
+				   const struct nand_op_instr *instr)
+{
+	struct ingenic_nand *nand = to_ingenic_nand(nand_to_mtd(chip));
+	struct ingenic_nfc *nfc = to_ingenic_nfc(chip->controller);
+	unsigned int i;
+
+	switch (instr->type) {
+	case NAND_OP_CMD_INSTR:
+		writeb(instr->ctx.cmd.opcode,
+		       cs->base + nfc->soc_info->cmd_offset);
+		return 0;
+	case NAND_OP_ADDR_INSTR:
+		for (i = 0; i < instr->ctx.addr.naddrs; i++)
+			writeb(instr->ctx.addr.addrs[i],
+			       cs->base + nfc->soc_info->addr_offset);
+		return 0;
+	case NAND_OP_DATA_IN_INSTR:
+		if (instr->ctx.data.force_8bit ||
+		    !(chip->options & NAND_BUSWIDTH_16))
+			ioread8_rep(cs->base + nfc->soc_info->data_offset,
+				    instr->ctx.data.buf.in,
+				    instr->ctx.data.len);
+		else
+			ioread16_rep(cs->base + nfc->soc_info->data_offset,
+				     instr->ctx.data.buf.in,
+				     instr->ctx.data.len);
+		return 0;
+	case NAND_OP_DATA_OUT_INSTR:
+		if (instr->ctx.data.force_8bit ||
+		    !(chip->options & NAND_BUSWIDTH_16))
+			iowrite8_rep(cs->base + nfc->soc_info->data_offset,
+				     instr->ctx.data.buf.out,
+				     instr->ctx.data.len);
+		else
+			iowrite16_rep(cs->base + nfc->soc_info->data_offset,
+				      instr->ctx.data.buf.out,
+				      instr->ctx.data.len);
+		return 0;
+	case NAND_OP_WAITRDY_INSTR:
+		if (!nand->busy_gpio)
+			return nand_soft_waitrdy(chip,
+						 instr->ctx.waitrdy.timeout_ms);
+
+		return nand_gpio_waitrdy(chip, nand->busy_gpio,
+					 instr->ctx.waitrdy.timeout_ms);
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static int ingenic_nand_exec_op(struct nand_chip *chip,
+				const struct nand_operation *op,
+				bool check_only)
+{
+	struct ingenic_nand *nand = to_ingenic_nand(nand_to_mtd(chip));
+	struct ingenic_nfc *nfc = to_ingenic_nfc(nand->chip.controller);
+	struct ingenic_nand_cs *cs;
+	unsigned int i;
+	int ret = 0;
+
+	if (check_only)
+		return 0;
+
+	cs = &nfc->cs[op->cs];
+	jz4780_nemc_assert(nfc->dev, cs->bank, true);
+	for (i = 0; i < op->ninstrs; i++) {
+		ret = ingenic_nand_exec_instr(chip, cs, &op->instrs[i]);
+		if (ret)
+			break;
+
+		if (op->instrs[i].delay_ns)
+			ndelay(op->instrs[i].delay_ns);
+	}
+	jz4780_nemc_assert(nfc->dev, cs->bank, false);
+
+	return ret;
+}
+
 static const struct nand_controller_ops ingenic_nand_controller_ops = {
 	.attach_chip = ingenic_nand_attach_chip,
+	.exec_op = ingenic_nand_exec_op,
 };
 
 static int ingenic_nand_init_chip(struct platform_device *pdev,
@@ -339,8 +373,6 @@ static int ingenic_nand_init_chip(struct platform_device *pdev,
 		ret = PTR_ERR(nand->busy_gpio);
 		dev_err(dev, "failed to request busy GPIO: %d\n", ret);
 		return ret;
-	} else if (nand->busy_gpio) {
-		nand->chip.legacy.dev_ready = ingenic_nand_dev_ready;
 	}
 
 	/*
@@ -371,12 +403,7 @@ static int ingenic_nand_init_chip(struct platform_device *pdev,
 		return -ENOMEM;
 	mtd->dev.parent = dev;
 
-	chip->legacy.IO_ADDR_R = cs->base + nfc->soc_info->data_offset;
-	chip->legacy.IO_ADDR_W = cs->base + nfc->soc_info->data_offset;
-	chip->legacy.chip_delay = RB_DELAY_US;
 	chip->options = NAND_NO_SUBPAGE_WRITE;
-	chip->legacy.select_chip = ingenic_nand_select_chip;
-	chip->legacy.cmd_ctrl = ingenic_nand_cmd_ctrl;
 	chip->ecc.mode = NAND_ECC_HW;
 	chip->controller = &nfc->controller;
 	nand_set_flash_node(chip, np);
-- 
2.25.4


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH v2 1/4] dt-bindings: mtd: rawnand: ingenic: Clarify the active state of the RB pin
  2020-05-19 23:24 [PATCH v2 1/4] dt-bindings: mtd: rawnand: ingenic: Clarify the active state of the RB pin Boris Brezillon
                   ` (2 preceding siblings ...)
  2020-05-19 23:24 ` [PATCH v2 4/4] mtd: rawnand: ingenic: Convert the driver to exec_op() Boris Brezillon
@ 2020-05-20  0:25 ` Paul Cercueil
  3 siblings, 0 replies; 9+ messages in thread
From: Paul Cercueil @ 2020-05-20  0:25 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Mark Rutland, devicetree, Vignesh Raghavendra, Tudor Ambarus,
	Richard Weinberger, Rob Herring, Harvey Hunt, linux-mtd,
	Miquel Raynal

Hi Boris,

This .txt file is going away, so there's no need to apply this one 
patch.

I Cc'd you on the V2 of the patchset that drops this file.

-Paul


Le mer. 20 mai 2020 à 1:24, Boris Brezillon 
<boris.brezillon@collabora.com> a écrit :
> Let's make things consistent with other bindings and clarify the
> expected active state.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> Changes in v2:
> * New patch
> ---
>  Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git 
> a/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt 
> b/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
> index c02259353327..4cbe13f94564 100644
> --- a/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
> @@ -30,7 +30,8 @@ Optional children node properties:
>  - nand-ecc-strength: ECC strength (max number of correctable bits).
>  - nand-ecc-mode: String, operation mode of the NAND ecc mode. "hw" 
> by default
>  - nand-on-flash-bbt: boolean to enable on flash bbt option, if not 
> present false
> -- rb-gpios: GPIO specifier for the busy pin.
> +- rb-gpios: GPIO specifier for the ready/busy pin. The active state 
> (ready)
> +            should be high unless the signal is inverted.
>  - wp-gpios: GPIO specifier for the write protect pin.
> 
>  Optional child node of NAND chip nodes:
> --
> 2.25.4
> 



______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH v2 4/4] mtd: rawnand: ingenic: Convert the driver to exec_op()
  2020-05-19 23:24 ` [PATCH v2 4/4] mtd: rawnand: ingenic: Convert the driver to exec_op() Boris Brezillon
@ 2020-05-20  0:37   ` Paul Cercueil
  2020-05-24 19:03   ` Miquel Raynal
  1 sibling, 0 replies; 9+ messages in thread
From: Paul Cercueil @ 2020-05-20  0:37 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Mark Rutland, devicetree, Vignesh Raghavendra, Tudor Ambarus,
	Richard Weinberger, Rob Herring, Harvey Hunt, linux-mtd,
	Miquel Raynal

Hi,

Le mer. 20 mai 2020 à 1:24, Boris Brezillon 
<boris.brezillon@collabora.com> a écrit :
> Let's convert the driver to exec_op() to have one less driver relying
> on the legacy interface.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>

Tested-by: Paul Cercueil <paul@crapouillou.net>

Cheers,
-Paul

> ---
> Changes in v2:
> * Add a delay after instructions when needed
> * s/cmd_offset/addr_offset/
> 
> Paul, I didn't add your T-b since this new version follows the path
> you proposed for the R/B polarity inversion issue. Feel free to add
> it back if it still works.
> ---
>  .../mtd/nand/raw/ingenic/ingenic_nand_drv.c   | 139 
> +++++++++++-------
>  1 file changed, 83 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c 
> b/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c
> index e939404e1383..3659e62829f9 100644
> --- a/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c
> +++ b/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c
> @@ -27,9 +27,6 @@
> 
>  #define DRV_NAME	"ingenic-nand"
> 
> -/* Command delay when there is no R/B pin. */
> -#define RB_DELAY_US	100
> -
>  struct jz_soc_info {
>  	unsigned long data_offset;
>  	unsigned long addr_offset;
> @@ -49,7 +46,6 @@ struct ingenic_nfc {
>  	struct nand_controller controller;
>  	unsigned int num_banks;
>  	struct list_head chips;
> -	int selected;
>  	struct ingenic_nand_cs cs[];
>  };
> 
> @@ -142,51 +138,6 @@ static const struct mtd_ooblayout_ops 
> jz4725b_ooblayout_ops = {
>  	.free = jz4725b_ooblayout_free,
>  };
> 
> -static void ingenic_nand_select_chip(struct nand_chip *chip, int 
> chipnr)
> -{
> -	struct ingenic_nand *nand = to_ingenic_nand(nand_to_mtd(chip));
> -	struct ingenic_nfc *nfc = to_ingenic_nfc(nand->chip.controller);
> -	struct ingenic_nand_cs *cs;
> -
> -	/* Ensure the currently selected chip is deasserted. */
> -	if (chipnr == -1 && nfc->selected >= 0) {
> -		cs = &nfc->cs[nfc->selected];
> -		jz4780_nemc_assert(nfc->dev, cs->bank, false);
> -	}
> -
> -	nfc->selected = chipnr;
> -}
> -
> -static void ingenic_nand_cmd_ctrl(struct nand_chip *chip, int cmd,
> -				  unsigned int ctrl)
> -{
> -	struct ingenic_nand *nand = to_ingenic_nand(nand_to_mtd(chip));
> -	struct ingenic_nfc *nfc = to_ingenic_nfc(nand->chip.controller);
> -	struct ingenic_nand_cs *cs;
> -
> -	if (WARN_ON(nfc->selected < 0))
> -		return;
> -
> -	cs = &nfc->cs[nfc->selected];
> -
> -	jz4780_nemc_assert(nfc->dev, cs->bank, ctrl & NAND_NCE);
> -
> -	if (cmd == NAND_CMD_NONE)
> -		return;
> -
> -	if (ctrl & NAND_ALE)
> -		writeb(cmd, cs->base + nfc->soc_info->addr_offset);
> -	else if (ctrl & NAND_CLE)
> -		writeb(cmd, cs->base + nfc->soc_info->cmd_offset);
> -}
> -
> -static int ingenic_nand_dev_ready(struct nand_chip *chip)
> -{
> -	struct ingenic_nand *nand = to_ingenic_nand(nand_to_mtd(chip));
> -
> -	return gpiod_get_value_cansleep(nand->busy_gpio);
> -}
> -
>  static void ingenic_nand_ecc_hwctl(struct nand_chip *chip, int mode)
>  {
>  	struct ingenic_nand *nand = to_ingenic_nand(nand_to_mtd(chip));
> @@ -298,8 +249,91 @@ static int ingenic_nand_attach_chip(struct 
> nand_chip *chip)
>  	return 0;
>  }
> 
> +static int ingenic_nand_exec_instr(struct nand_chip *chip,
> +				   struct ingenic_nand_cs *cs,
> +				   const struct nand_op_instr *instr)
> +{
> +	struct ingenic_nand *nand = to_ingenic_nand(nand_to_mtd(chip));
> +	struct ingenic_nfc *nfc = to_ingenic_nfc(chip->controller);
> +	unsigned int i;
> +
> +	switch (instr->type) {
> +	case NAND_OP_CMD_INSTR:
> +		writeb(instr->ctx.cmd.opcode,
> +		       cs->base + nfc->soc_info->cmd_offset);
> +		return 0;
> +	case NAND_OP_ADDR_INSTR:
> +		for (i = 0; i < instr->ctx.addr.naddrs; i++)
> +			writeb(instr->ctx.addr.addrs[i],
> +			       cs->base + nfc->soc_info->addr_offset);
> +		return 0;
> +	case NAND_OP_DATA_IN_INSTR:
> +		if (instr->ctx.data.force_8bit ||
> +		    !(chip->options & NAND_BUSWIDTH_16))
> +			ioread8_rep(cs->base + nfc->soc_info->data_offset,
> +				    instr->ctx.data.buf.in,
> +				    instr->ctx.data.len);
> +		else
> +			ioread16_rep(cs->base + nfc->soc_info->data_offset,
> +				     instr->ctx.data.buf.in,
> +				     instr->ctx.data.len);
> +		return 0;
> +	case NAND_OP_DATA_OUT_INSTR:
> +		if (instr->ctx.data.force_8bit ||
> +		    !(chip->options & NAND_BUSWIDTH_16))
> +			iowrite8_rep(cs->base + nfc->soc_info->data_offset,
> +				     instr->ctx.data.buf.out,
> +				     instr->ctx.data.len);
> +		else
> +			iowrite16_rep(cs->base + nfc->soc_info->data_offset,
> +				      instr->ctx.data.buf.out,
> +				      instr->ctx.data.len);
> +		return 0;
> +	case NAND_OP_WAITRDY_INSTR:
> +		if (!nand->busy_gpio)
> +			return nand_soft_waitrdy(chip,
> +						 instr->ctx.waitrdy.timeout_ms);
> +
> +		return nand_gpio_waitrdy(chip, nand->busy_gpio,
> +					 instr->ctx.waitrdy.timeout_ms);
> +	default:
> +		break;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int ingenic_nand_exec_op(struct nand_chip *chip,
> +				const struct nand_operation *op,
> +				bool check_only)
> +{
> +	struct ingenic_nand *nand = to_ingenic_nand(nand_to_mtd(chip));
> +	struct ingenic_nfc *nfc = to_ingenic_nfc(nand->chip.controller);
> +	struct ingenic_nand_cs *cs;
> +	unsigned int i;
> +	int ret = 0;
> +
> +	if (check_only)
> +		return 0;
> +
> +	cs = &nfc->cs[op->cs];
> +	jz4780_nemc_assert(nfc->dev, cs->bank, true);
> +	for (i = 0; i < op->ninstrs; i++) {
> +		ret = ingenic_nand_exec_instr(chip, cs, &op->instrs[i]);
> +		if (ret)
> +			break;
> +
> +		if (op->instrs[i].delay_ns)
> +			ndelay(op->instrs[i].delay_ns);
> +	}
> +	jz4780_nemc_assert(nfc->dev, cs->bank, false);
> +
> +	return ret;
> +}
> +
>  static const struct nand_controller_ops ingenic_nand_controller_ops 
> = {
>  	.attach_chip = ingenic_nand_attach_chip,
> +	.exec_op = ingenic_nand_exec_op,
>  };
> 
>  static int ingenic_nand_init_chip(struct platform_device *pdev,
> @@ -339,8 +373,6 @@ static int ingenic_nand_init_chip(struct 
> platform_device *pdev,
>  		ret = PTR_ERR(nand->busy_gpio);
>  		dev_err(dev, "failed to request busy GPIO: %d\n", ret);
>  		return ret;
> -	} else if (nand->busy_gpio) {
> -		nand->chip.legacy.dev_ready = ingenic_nand_dev_ready;
>  	}
> 
>  	/*
> @@ -371,12 +403,7 @@ static int ingenic_nand_init_chip(struct 
> platform_device *pdev,
>  		return -ENOMEM;
>  	mtd->dev.parent = dev;
> 
> -	chip->legacy.IO_ADDR_R = cs->base + nfc->soc_info->data_offset;
> -	chip->legacy.IO_ADDR_W = cs->base + nfc->soc_info->data_offset;
> -	chip->legacy.chip_delay = RB_DELAY_US;
>  	chip->options = NAND_NO_SUBPAGE_WRITE;
> -	chip->legacy.select_chip = ingenic_nand_select_chip;
> -	chip->legacy.cmd_ctrl = ingenic_nand_cmd_ctrl;
>  	chip->ecc.mode = NAND_ECC_HW;
>  	chip->controller = &nfc->controller;
>  	nand_set_flash_node(chip, np);
> --
> 2.25.4
> 



______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH v2 4/4] mtd: rawnand: ingenic: Convert the driver to exec_op()
  2020-05-19 23:24 ` [PATCH v2 4/4] mtd: rawnand: ingenic: Convert the driver to exec_op() Boris Brezillon
  2020-05-20  0:37   ` Paul Cercueil
@ 2020-05-24 19:03   ` Miquel Raynal
  1 sibling, 0 replies; 9+ messages in thread
From: Miquel Raynal @ 2020-05-24 19:03 UTC (permalink / raw)
  To: Boris Brezillon, Paul Cercueil, Harvey Hunt, Miquel Raynal, linux-mtd
  Cc: Mark Rutland, devicetree, Vignesh Raghavendra, Tudor Ambarus,
	Richard Weinberger, Rob Herring

On Tue, 2020-05-19 at 23:24:54 UTC, Boris Brezillon wrote:
> Let's convert the driver to exec_op() to have one less driver relying
> on the legacy interface.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> Tested-by: Paul Cercueil <paul@crapouillou.net>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.

Miquel

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH v2 3/4] mtd: rawnand: ingenic: Fix the RB gpio active-high property on qi, lb60
  2020-05-19 23:24 ` [PATCH v2 3/4] mtd: rawnand: ingenic: Fix the RB gpio active-high property on qi, lb60 Boris Brezillon
@ 2020-05-24 19:03   ` Miquel Raynal
  0 siblings, 0 replies; 9+ messages in thread
From: Miquel Raynal @ 2020-05-24 19:03 UTC (permalink / raw)
  To: Boris Brezillon, Paul Cercueil, Harvey Hunt, Miquel Raynal, linux-mtd
  Cc: Mark Rutland, devicetree, Vignesh Raghavendra, Tudor Ambarus,
	Richard Weinberger, Rob Herring

On Tue, 2020-05-19 at 23:24:53 UTC, Boris Brezillon wrote:
> The rb-gpios semantics was undocumented and qi,lb60 (along with the
> ingenic driver) got it wrong. The active state encodes the NAND ready
> state, which is high level. Since there's no signal inverter on this
> board, it should be active-high. Let's fix that here for older DTs so
> we can re-use the generic nand_gpio_waitrdy() helper, and be consistent
> with what other drivers do.
> 
> Suggested-by: Paul Cercueil <paul@crapouillou.net>
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.

Miquel

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH v2 2/4] dt-bindings: mtd: nand: Document the generic rb-gpios property
  2020-05-19 23:24 ` [PATCH v2 2/4] dt-bindings: mtd: nand: Document the generic rb-gpios property Boris Brezillon
@ 2020-05-28 21:22   ` Rob Herring
  0 siblings, 0 replies; 9+ messages in thread
From: Rob Herring @ 2020-05-28 21:22 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Mark Rutland, devicetree, Vignesh Raghavendra, Tudor Ambarus,
	Richard Weinberger, Paul Cercueil, Rob Herring, linux-mtd,
	Harvey Hunt, Miquel Raynal

On Wed, 20 May 2020 01:24:52 +0200, Boris Brezillon wrote:
> A few drivers use this property to describe GPIO pins used to sample
> the NAND Ready/Busy state. Let's make it part of the generic binding
> doc.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> Changes in v2:
> * New patch
> ---
>  Documentation/devicetree/bindings/mtd/nand-controller.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

end of thread, back to index

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-19 23:24 [PATCH v2 1/4] dt-bindings: mtd: rawnand: ingenic: Clarify the active state of the RB pin Boris Brezillon
2020-05-19 23:24 ` [PATCH v2 2/4] dt-bindings: mtd: nand: Document the generic rb-gpios property Boris Brezillon
2020-05-28 21:22   ` Rob Herring
2020-05-19 23:24 ` [PATCH v2 3/4] mtd: rawnand: ingenic: Fix the RB gpio active-high property on qi, lb60 Boris Brezillon
2020-05-24 19:03   ` Miquel Raynal
2020-05-19 23:24 ` [PATCH v2 4/4] mtd: rawnand: ingenic: Convert the driver to exec_op() Boris Brezillon
2020-05-20  0:37   ` Paul Cercueil
2020-05-24 19:03   ` Miquel Raynal
2020-05-20  0:25 ` [PATCH v2 1/4] dt-bindings: mtd: rawnand: ingenic: Clarify the active state of the RB pin Paul Cercueil

Linux-mtd Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mtd/0 linux-mtd/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mtd linux-mtd/ https://lore.kernel.org/linux-mtd \
		linux-mtd@lists.infradead.org
	public-inbox-index linux-mtd

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-mtd


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git