linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/8] mtd: rawnand: bcm47xx: Convert to exec_op() (and more)
@ 2020-05-18 16:28 Boris Brezillon
  2020-05-18 16:28 ` [PATCH v2 1/8] mtd: rawnand: Add an is_last flag to nand_subop Boris Brezillon
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Boris Brezillon @ 2020-05-18 16:28 UTC (permalink / raw)
  To: Miquel Raynal, linux-mtd, Hauke Mehrtens,
	Rafał Miłecki, linux-mips
  Cc: Richard Weinberger, Boris Brezillon, Vignesh Raghavendra, Tudor Ambarus

Hello,

A bit of context to explain the motivation behind those conversions
I've been sending for the last couple of weeks. The raw NAND subsystem
carries a lot of history which makes any rework not only painful, but
also subject to regressions which we only detect when someone dares to
update its kernel on one of those ancient HW. While carrying drivers
for old HW is not a problem per se, carrying ancient and unmaintained
drivers that are not converted to new APIs is a maintenance burden,
hence this massive conversion attempt I'm conducting here.

So here is a series converting the BCM47XX NAND controller driver to
exec_op(), plus a bunch of minor improvements done along the way.
I hope I'll find someone to test those changes, but if there's no one
still having access to this  HW or no interest in keeping it supported
in recent kernel versions, we should definitely consider removing the
driver instead.

No major changes in this v2, apart from fixes for things reported by
Miquel. See the changelog on each patch for more details.

Regards,

Boris

Boris Brezillon (8):
  mtd: rawnand: Add an is_last flag to nand_subop
  mtd: rawnand: bcm47xx: Drop dependency on BCMA
  mtd: rawnand: bcm47xx: Allow compiling the driver when COMPILE_TEST=y
  mtd: rawnand: bcm47xx: Demistify a few more things
  mtd: rawnand: bcm47xx: Implement the exec_op() interface
  mtd: rawnand: bcm47xx: Get rid of the legacy implementation
  mtd: rawnand: bcm47xx: Simplify the init() function
  mtd: rawnand: bcm47xx: Merge all source files

 drivers/mtd/nand/raw/Kconfig                  |   3 +-
 drivers/mtd/nand/raw/Makefile                 |   2 +-
 .../mtd/nand/raw/bcm47xx-nand-controller.c    | 343 +++++++++++++
 drivers/mtd/nand/raw/bcm47xxnflash/Makefile   |   5 -
 .../nand/raw/bcm47xxnflash/bcm47xxnflash.h    |  26 -
 drivers/mtd/nand/raw/bcm47xxnflash/main.c     |  77 ---
 .../mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c  | 450 ------------------
 drivers/mtd/nand/raw/nand_base.c              |   2 +
 include/linux/mtd/rawnand.h                   |   2 +
 9 files changed, 349 insertions(+), 561 deletions(-)
 create mode 100644 drivers/mtd/nand/raw/bcm47xx-nand-controller.c
 delete mode 100644 drivers/mtd/nand/raw/bcm47xxnflash/Makefile
 delete mode 100644 drivers/mtd/nand/raw/bcm47xxnflash/bcm47xxnflash.h
 delete mode 100644 drivers/mtd/nand/raw/bcm47xxnflash/main.c
 delete mode 100644 drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c

-- 
2.25.4


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

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

* [PATCH v2 1/8] mtd: rawnand: Add an is_last flag to nand_subop
  2020-05-18 16:28 [PATCH v2 0/8] mtd: rawnand: bcm47xx: Convert to exec_op() (and more) Boris Brezillon
@ 2020-05-18 16:28 ` Boris Brezillon
  2020-05-18 16:28 ` [PATCH v2 2/8] mtd: rawnand: bcm47xx: Drop dependency on BCMA Boris Brezillon
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Boris Brezillon @ 2020-05-18 16:28 UTC (permalink / raw)
  To: Miquel Raynal, linux-mtd, Hauke Mehrtens,
	Rafał Miłecki, linux-mips
  Cc: Richard Weinberger, Boris Brezillon, Vignesh Raghavendra, Tudor Ambarus

Some controllers need to know when they're passed the last subop so
they can de-assert the CE pin.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
Changes in v2:
* Add R-b
---
 drivers/mtd/nand/raw/nand_base.c | 2 ++
 include/linux/mtd/rawnand.h      | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 169150a7c140..21f8771b00ba 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -2169,6 +2169,8 @@ nand_op_parser_match_pat(const struct nand_op_parser_pattern *pat,
 	 */
 	ctx->subop.ninstrs = ninstrs;
 	ctx->subop.last_instr_end_off = instr_offset;
+	if (ctx->subop.instrs + ninstrs == end && !instr_offset)
+		ctx->subop.is_last = true;
 
 	return true;
 }
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 0f45b6984ad1..69f1c1652187 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -715,6 +715,7 @@ struct nand_op_instr {
  *			   of the sub-operation
  * @last_instr_end_off: offset to end at (excluded) for the last instruction
  *			of the sub-operation
+ * @is_last: this sub-operation is the last one
  *
  * Both @first_instr_start_off and @last_instr_end_off only apply to data or
  * address instructions.
@@ -728,6 +729,7 @@ struct nand_subop {
 	unsigned int ninstrs;
 	unsigned int first_instr_start_off;
 	unsigned int last_instr_end_off;
+	bool is_last;
 };
 
 unsigned int nand_subop_get_addr_start_off(const struct nand_subop *subop,
-- 
2.25.4


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

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

* [PATCH v2 2/8] mtd: rawnand: bcm47xx: Drop dependency on BCMA
  2020-05-18 16:28 [PATCH v2 0/8] mtd: rawnand: bcm47xx: Convert to exec_op() (and more) Boris Brezillon
  2020-05-18 16:28 ` [PATCH v2 1/8] mtd: rawnand: Add an is_last flag to nand_subop Boris Brezillon
@ 2020-05-18 16:28 ` Boris Brezillon
  2020-05-18 16:28 ` [PATCH v2 3/8] mtd: rawnand: bcm47xx: Allow compiling the driver when COMPILE_TEST=y Boris Brezillon
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Boris Brezillon @ 2020-05-18 16:28 UTC (permalink / raw)
  To: Miquel Raynal, linux-mtd, Hauke Mehrtens,
	Rafał Miłecki, linux-mips
  Cc: Richard Weinberger, Boris Brezillon, Vignesh Raghavendra, Tudor Ambarus

BCMA_NFLASH can only be selected if BCMA is enabled and we already
depend on BCMA_NFLASH, making the dependency on BCMA useless.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
Changes in v2:
* None
---
 drivers/mtd/nand/raw/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
index a80a46bb5b8b..60c4eb9d382f 100644
--- a/drivers/mtd/nand/raw/Kconfig
+++ b/drivers/mtd/nand/raw/Kconfig
@@ -243,7 +243,6 @@ config MTD_NAND_BRCMNAND
 config MTD_NAND_BCM47XXNFLASH
 	tristate "BCM4706 BCMA NAND controller"
 	depends on BCMA_NFLASH
-	depends on BCMA
 	help
 	  BCMA bus can have various flash memories attached, they are
 	  registered by bcma as platform devices. This enables driver for
-- 
2.25.4


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

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

* [PATCH v2 3/8] mtd: rawnand: bcm47xx: Allow compiling the driver when COMPILE_TEST=y
  2020-05-18 16:28 [PATCH v2 0/8] mtd: rawnand: bcm47xx: Convert to exec_op() (and more) Boris Brezillon
  2020-05-18 16:28 ` [PATCH v2 1/8] mtd: rawnand: Add an is_last flag to nand_subop Boris Brezillon
  2020-05-18 16:28 ` [PATCH v2 2/8] mtd: rawnand: bcm47xx: Drop dependency on BCMA Boris Brezillon
@ 2020-05-18 16:28 ` Boris Brezillon
  2020-05-18 16:28 ` [PATCH v2 4/8] mtd: rawnand: bcm47xx: Demistify a few more things Boris Brezillon
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Boris Brezillon @ 2020-05-18 16:28 UTC (permalink / raw)
  To: Miquel Raynal, linux-mtd, Hauke Mehrtens,
	Rafał Miłecki, linux-mips
  Cc: Richard Weinberger, Boris Brezillon, Vignesh Raghavendra, Tudor Ambarus

Makes it easier to spot compile-time issues.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
Changes in v2:
* None
---
 drivers/mtd/nand/raw/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
index 60c4eb9d382f..1f8aa353f764 100644
--- a/drivers/mtd/nand/raw/Kconfig
+++ b/drivers/mtd/nand/raw/Kconfig
@@ -242,7 +242,7 @@ config MTD_NAND_BRCMNAND
 
 config MTD_NAND_BCM47XXNFLASH
 	tristate "BCM4706 BCMA NAND controller"
-	depends on BCMA_NFLASH
+	depends on BCMA_NFLASH || COMPILE_TEST
 	help
 	  BCMA bus can have various flash memories attached, they are
 	  registered by bcma as platform devices. This enables driver for
-- 
2.25.4


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

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

* [PATCH v2 4/8] mtd: rawnand: bcm47xx: Demistify a few more things
  2020-05-18 16:28 [PATCH v2 0/8] mtd: rawnand: bcm47xx: Convert to exec_op() (and more) Boris Brezillon
                   ` (2 preceding siblings ...)
  2020-05-18 16:28 ` [PATCH v2 3/8] mtd: rawnand: bcm47xx: Allow compiling the driver when COMPILE_TEST=y Boris Brezillon
@ 2020-05-18 16:28 ` Boris Brezillon
  2020-05-18 16:28 ` [PATCH v2 5/8] mtd: rawnand: bcm47xx: Implement the exec_op() interface Boris Brezillon
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Boris Brezillon @ 2020-05-18 16:28 UTC (permalink / raw)
  To: Miquel Raynal, linux-mtd, Hauke Mehrtens,
	Rafał Miłecki, linux-mips
  Cc: Richard Weinberger, Boris Brezillon, Vignesh Raghavendra, Tudor Ambarus

There were a few places where raw hex values were used instead of the
register/field definitions. Let's replace them by proper definitions.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
Changes in v2:
* Fix the commit message
* Fix a typo in a comment
* Add R-b
---
 .../mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c  | 34 +++++++++++++++----
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c b/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c
index 591775173034..543fcff6e4d2 100644
--- a/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c
+++ b/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c
@@ -25,12 +25,29 @@
 #define NCTL_CMD1W			0x00080000
 #define NCTL_READ			0x00100000
 #define NCTL_WRITE			0x00200000
+/* When the SPECADDR is set CMD1 is interpreted as a single ADDR cycle */
 #define NCTL_SPECADDR			0x01000000
 #define NCTL_READY			0x04000000
 #define NCTL_ERR			0x08000000
+/*
+ * Number of DATA cycles to issue when NCTL_{READ,WRITE} is set. The minimum
+ * value is 1 and the maximum value is 4. Those bytes are then stored in the
+ * BCMA_CC_NFLASH_DATA register.
+ */
+#define NCTL_DATA_CYCLES(x)		((((x) - 1) & 0x3) << 28)
+/*
+ * The CS pin seems to be asserted even if NCTL_CSA is not set. All this bit
+ * seems to encode is whether the CS line should stay asserted after the
+ * operation has been executed. In other words, you should only set it if you
+ * intend to do more operations on the NAND bus.
+ */
 #define NCTL_CSA			0x40000000
 #define NCTL_START			0x80000000
 
+#define CONF_MAGIC_BIT			0x00000002
+#define CONF_COL_BYTES(x)		(((x) - 1) << 4)
+#define CONF_ROW_BYTES(x)		(((x) - 1) << 6)
+
 /**************************************************
  * Various helpers
  **************************************************/
@@ -118,7 +135,7 @@ static void bcm47xxnflash_ops_bcm4706_read(struct mtd_info *mtd, uint8_t *buf,
 
 		/* Eventually read some data :) */
 		for (i = 0; i < toread; i += 4, dest++) {
-			ctlcode = NCTL_CSA | 0x30000000 | NCTL_READ;
+			ctlcode = NCTL_CSA | NCTL_DATA_CYCLES(4) | NCTL_READ;
 			if (i == toread - 4) /* Last read goes without that */
 				ctlcode &= ~NCTL_CSA;
 			if (bcm47xxnflash_ops_bcm4706_ctl_cmd(b47n->cc,
@@ -150,7 +167,7 @@ static void bcm47xxnflash_ops_bcm4706_write(struct mtd_info *mtd,
 	for (i = 0; i < len; i += 4, data++) {
 		bcma_cc_write32(cc, BCMA_CC_NFLASH_DATA, *data);
 
-		ctlcode = NCTL_CSA | 0x30000000 | NCTL_WRITE;
+		ctlcode = NCTL_CSA | NCTL_DATA_CYCLES(4) | NCTL_WRITE;
 		if (i == len - 4) /* Last read goes without that */
 			ctlcode &= ~NCTL_CSA;
 		if (bcm47xxnflash_ops_bcm4706_ctl_cmd(cc, ctlcode)) {
@@ -229,7 +246,7 @@ static void bcm47xxnflash_ops_bcm4706_cmdfunc(struct nand_chip *nand_chip,
 		nand_wait_ready(nand_chip);
 		break;
 	case NAND_CMD_READID:
-		ctlcode = NCTL_CSA | 0x01000000 | NCTL_CMD1W | NCTL_CMD0;
+		ctlcode = NCTL_CSA | NCTL_SPECADDR | NCTL_CMD1W | NCTL_CMD0;
 		ctlcode |= NAND_CMD_READID;
 		if (bcm47xxnflash_ops_bcm4706_ctl_cmd(b47n->cc, ctlcode)) {
 			pr_err("READID error\n");
@@ -242,7 +259,7 @@ static void bcm47xxnflash_ops_bcm4706_cmdfunc(struct nand_chip *nand_chip,
 		 * to perform, so cache everything.
 		 */
 		for (i = 0; i < ARRAY_SIZE(b47n->id_data); i++) {
-			ctlcode = NCTL_CSA | NCTL_READ;
+			ctlcode = NCTL_CSA | NCTL_READ | NCTL_DATA_CYCLES(1);
 			if (i == ARRAY_SIZE(b47n->id_data) - 1)
 				ctlcode &= ~NCTL_CSA;
 			if (bcm47xxnflash_ops_bcm4706_ctl_cmd(b47n->cc,
@@ -285,7 +302,7 @@ static void bcm47xxnflash_ops_bcm4706_cmdfunc(struct nand_chip *nand_chip,
 				b47n->curr_page_addr);
 
 		/* Prepare to write */
-		ctlcode = 0x40000000 | NCTL_ROW | NCTL_COL | NCTL_CMD0;
+		ctlcode = NCTL_CSA | NCTL_ROW | NCTL_COL | NCTL_CMD0;
 		ctlcode |= NAND_CMD_SEQIN;
 		if (bcm47xxnflash_ops_bcm4706_ctl_cmd(cc, ctlcode))
 			pr_err("SEQIN failed\n");
@@ -320,7 +337,9 @@ static u8 bcm47xxnflash_ops_bcm4706_read_byte(struct nand_chip *nand_chip)
 		}
 		return b47n->id_data[b47n->curr_column++];
 	case NAND_CMD_STATUS:
-		if (bcm47xxnflash_ops_bcm4706_ctl_cmd(cc, NCTL_READ))
+		if (bcm47xxnflash_ops_bcm4706_ctl_cmd(cc,
+						      NCTL_READ |
+						      NCTL_DATA_CYCLES(1)))
 			return 0;
 		return bcma_cc_read32(cc, BCMA_CC_NFLASH_DATA) & 0xff;
 	case NAND_CMD_READOOB:
@@ -439,7 +458,8 @@ int bcm47xxnflash_ops_bcm4706_init(struct bcm47xxnflash *b47n)
 	row_bits = tbits - col_bits + 1;
 	row_bsize = (row_bits + 7) / 8;
 
-	val = ((row_bsize - 1) << 6) | ((col_size - 1) << 4) | 2;
+	val = CONF_ROW_BYTES(row_bsize) | CONF_COL_BYTES(col_size) |
+	      CONF_MAGIC_BIT;
 	bcma_cc_write32(b47n->cc, BCMA_CC_NFLASH_CONF, val);
 
 exit:
-- 
2.25.4


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

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

* [PATCH v2 5/8] mtd: rawnand: bcm47xx: Implement the exec_op() interface
  2020-05-18 16:28 [PATCH v2 0/8] mtd: rawnand: bcm47xx: Convert to exec_op() (and more) Boris Brezillon
                   ` (3 preceding siblings ...)
  2020-05-18 16:28 ` [PATCH v2 4/8] mtd: rawnand: bcm47xx: Demistify a few more things Boris Brezillon
@ 2020-05-18 16:28 ` Boris Brezillon
  2020-05-18 16:28 ` [PATCH v2 6/8] mtd: rawnand: bcm47xx: Get rid of the legacy implementation Boris Brezillon
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Boris Brezillon @ 2020-05-18 16:28 UTC (permalink / raw)
  To: Miquel Raynal, linux-mtd, Hauke Mehrtens,
	Rafał Miłecki, linux-mips
  Cc: Richard Weinberger, Boris Brezillon, Vignesh Raghavendra, Tudor Ambarus

Implement the exec_op() interface so we can get rid of the convoluted
cmdfunc() implementation.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
This is based on my understanding of how this controller works, and I
think it covers all the use cases covered by the custom cmdfunc()
implementation. I might be wrong of course, so it'd be great to have
someone test on real HW.

Changes in v2:
 * s/i/j/ in exec_cmd_addr()
 * Drop WARN_ON()s
---
 .../nand/raw/bcm47xxnflash/bcm47xxnflash.h    |   1 +
 .../mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c  | 140 ++++++++++++++++++
 2 files changed, 141 insertions(+)

diff --git a/drivers/mtd/nand/raw/bcm47xxnflash/bcm47xxnflash.h b/drivers/mtd/nand/raw/bcm47xxnflash/bcm47xxnflash.h
index 201b9baa52a0..00d0974b73cb 100644
--- a/drivers/mtd/nand/raw/bcm47xxnflash/bcm47xxnflash.h
+++ b/drivers/mtd/nand/raw/bcm47xxnflash/bcm47xxnflash.h
@@ -10,6 +10,7 @@
 #include <linux/mtd/rawnand.h>
 
 struct bcm47xxnflash {
+	struct nand_controller base;
 	struct bcma_drv_cc *cc;
 
 	struct nand_chip nand_chip;
diff --git a/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c b/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c
index 543fcff6e4d2..edf0c3d7b561 100644
--- a/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c
+++ b/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c
@@ -382,6 +382,143 @@ static void bcm47xxnflash_ops_bcm4706_write_buf(struct nand_chip *nand_chip,
 	pr_err("Invalid command for buf write: 0x%X\n", b47n->curr_command);
 }
 
+static int
+bcm47xxnflash_ops_bcm4706_exec_cmd_addr(struct nand_chip *chip,
+					const struct nand_subop *subop)
+{
+	struct bcm47xxnflash *b47n = nand_get_controller_data(chip);
+	u32 nctl = 0, col = 0, row = 0, ncols = 0, nrows = 0;
+	unsigned int i, j;
+
+	for (i = 0; i < subop->ninstrs; i++) {
+		const struct nand_op_instr *instr = &subop->instrs[i];
+
+		switch (instr->type) {
+		case NAND_OP_CMD_INSTR:
+			if (WARN_ON_ONCE((nctl & NCTL_CMD0) &&
+					 (nctl & NCTL_CMD1W)))
+				return -EINVAL;
+			else if (nctl & NCTL_CMD0)
+				nctl |= NCTL_CMD1W |
+					((u32)instr->ctx.cmd.opcode << 8);
+			else
+				nctl |= NCTL_CMD0 | instr->ctx.cmd.opcode;
+			break;
+		case NAND_OP_ADDR_INSTR:
+			for (j = 0; j < instr->ctx.addr.naddrs; j++) {
+				u32 addr = instr->ctx.addr.addrs[j];
+
+				if (j < 2) {
+					col |= addr << (j * 8);
+					nctl |= NCTL_COL;
+					ncols++;
+				} else {
+					row |= addr << ((j - 2) * 8);
+					nctl |= NCTL_ROW;
+					nrows++;
+				}
+			}
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
+
+	/* Keep the CS line asserted if there's something else to execute. */
+	if (!subop->is_last)
+		nctl |= NCTL_CSA;
+
+	bcma_cc_write32(b47n->cc, BCMA_CC_NFLASH_CONF,
+			CONF_MAGIC_BIT |
+			CONF_COL_BYTES(ncols) |
+			CONF_ROW_BYTES(nrows));
+	return bcm47xxnflash_ops_bcm4706_ctl_cmd(b47n->cc, nctl);
+}
+
+static int
+bcm47xxnflash_ops_bcm4706_exec_waitrdy(struct nand_chip *chip,
+				       const struct nand_subop *subop)
+{
+	struct bcm47xxnflash *b47n = nand_get_controller_data(chip);
+	const struct nand_op_instr *instr = &subop->instrs[0];
+	unsigned long timeout_jiffies = jiffies;
+
+	timeout_jiffies += msecs_to_jiffies(instr->ctx.waitrdy.timeout_ms) + 1;
+	do {
+		if (bcma_cc_read32(b47n->cc, BCMA_CC_NFLASH_CTL) & NCTL_READY)
+			return 0;
+
+		usleep_range(10, 100);
+	} while (time_before(jiffies, timeout_jiffies));
+
+	return -ETIMEDOUT;
+}
+
+static int
+bcm47xxnflash_ops_bcm4706_exec_rw(struct nand_chip *chip,
+				  const struct nand_subop *subop)
+{
+	struct bcm47xxnflash *b47n = nand_get_controller_data(chip);
+	const struct nand_op_instr *instr = &subop->instrs[0];
+	unsigned int i;
+	int ret;
+
+	for (i = 0; i < instr->ctx.data.len; i += 4) {
+		unsigned int nbytes = min_t(unsigned int,
+					    instr->ctx.data.len - i, 4);
+		u32 nctl, data;
+
+		nctl = NCTL_CSA | NCTL_DATA_CYCLES(nbytes);
+		if (instr->type == NAND_OP_DATA_IN_INSTR) {
+			nctl |= NCTL_READ;
+		} else {
+			nctl |= NCTL_WRITE;
+			memcpy(&data, instr->ctx.data.buf.in + i, nbytes);
+			bcma_cc_write32(b47n->cc, BCMA_CC_NFLASH_DATA, data);
+		}
+
+		if (i + nbytes < instr->ctx.data.len)
+			nctl |= NCTL_CSA;
+
+		ret = bcm47xxnflash_ops_bcm4706_ctl_cmd(b47n->cc, nctl);
+		if (ret)
+			return ret;
+
+		if (instr->type == NAND_OP_DATA_IN_INSTR) {
+			data = bcma_cc_read32(b47n->cc, BCMA_CC_NFLASH_DATA);
+			memcpy(instr->ctx.data.buf.in + i, &data, nbytes);
+		}
+	}
+
+	return 0;
+}
+
+static const struct nand_op_parser bcm47xxnflash_op_parser = NAND_OP_PARSER(
+	NAND_OP_PARSER_PATTERN(bcm47xxnflash_ops_bcm4706_exec_cmd_addr,
+			       NAND_OP_PARSER_PAT_CMD_ELEM(true),
+			       NAND_OP_PARSER_PAT_ADDR_ELEM(true, 5),
+			       NAND_OP_PARSER_PAT_CMD_ELEM(true)),
+	NAND_OP_PARSER_PATTERN(bcm47xxnflash_ops_bcm4706_exec_waitrdy,
+			       NAND_OP_PARSER_PAT_WAITRDY_ELEM(false)),
+	NAND_OP_PARSER_PATTERN(bcm47xxnflash_ops_bcm4706_exec_rw,
+			       NAND_OP_PARSER_PAT_DATA_IN_ELEM(false, 0x200)),
+	NAND_OP_PARSER_PATTERN(bcm47xxnflash_ops_bcm4706_exec_rw,
+			       NAND_OP_PARSER_PAT_DATA_OUT_ELEM(false, 0x200)),
+);
+
+static int
+bcm47xxnflash_ops_bcm4706_exec_op(struct nand_chip *chip,
+				  const struct nand_operation *op,
+				  bool check_only)
+{
+	return nand_op_parser_exec_op(chip, &bcm47xxnflash_op_parser, op,
+				      check_only);
+}
+
+static const struct nand_controller_ops bcm47xxnflash_ops = {
+	.exec_op = bcm47xxnflash_ops_bcm4706_exec_op,
+};
+
 /**************************************************
  * Init
  **************************************************/
@@ -398,6 +535,9 @@ int bcm47xxnflash_ops_bcm4706_init(struct bcm47xxnflash *b47n)
 	u8 tbits, col_bits, col_size, row_bits, row_bsize;
 	u32 val;
 
+	nand_controller_init(&b47n->base);
+	b47n->base.ops = &bcm47xxnflash_ops;
+	b47n->nand_chip.controller = &b47n->base;
 	nand_chip->legacy.select_chip = bcm47xxnflash_ops_bcm4706_select_chip;
 	nand_chip->legacy.cmd_ctrl = bcm47xxnflash_ops_bcm4706_cmd_ctrl;
 	nand_chip->legacy.dev_ready = bcm47xxnflash_ops_bcm4706_dev_ready;
-- 
2.25.4


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

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

* [PATCH v2 6/8] mtd: rawnand: bcm47xx: Get rid of the legacy implementation
  2020-05-18 16:28 [PATCH v2 0/8] mtd: rawnand: bcm47xx: Convert to exec_op() (and more) Boris Brezillon
                   ` (4 preceding siblings ...)
  2020-05-18 16:28 ` [PATCH v2 5/8] mtd: rawnand: bcm47xx: Implement the exec_op() interface Boris Brezillon
@ 2020-05-18 16:28 ` Boris Brezillon
  2020-05-18 16:28 ` [PATCH v2 7/8] mtd: rawnand: bcm47xx: Simplify the init() function Boris Brezillon
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Boris Brezillon @ 2020-05-18 16:28 UTC (permalink / raw)
  To: Miquel Raynal, linux-mtd, Hauke Mehrtens,
	Rafał Miłecki, linux-mips
  Cc: Richard Weinberger, Boris Brezillon, Vignesh Raghavendra, Tudor Ambarus

Now that exec_op() is implemented we don't need to implement all those
legacy hooks, which simplifies the code quite a bit.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
Changes in v2:
* Add R-b
---
 .../nand/raw/bcm47xxnflash/bcm47xxnflash.h    |   6 -
 .../mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c  | 315 ------------------
 2 files changed, 321 deletions(-)

diff --git a/drivers/mtd/nand/raw/bcm47xxnflash/bcm47xxnflash.h b/drivers/mtd/nand/raw/bcm47xxnflash/bcm47xxnflash.h
index 00d0974b73cb..8de0e7e0a3a4 100644
--- a/drivers/mtd/nand/raw/bcm47xxnflash/bcm47xxnflash.h
+++ b/drivers/mtd/nand/raw/bcm47xxnflash/bcm47xxnflash.h
@@ -14,12 +14,6 @@ struct bcm47xxnflash {
 	struct bcma_drv_cc *cc;
 
 	struct nand_chip nand_chip;
-
-	unsigned curr_command;
-	int curr_page_addr;
-	int curr_column;
-
-	u8 id_data[8];
 };
 
 int bcm47xxnflash_ops_bcm4706_init(struct bcm47xxnflash *b47n);
diff --git a/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c b/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c
index edf0c3d7b561..b6c5db9acac9 100644
--- a/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c
+++ b/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c
@@ -75,313 +75,10 @@ static int bcm47xxnflash_ops_bcm4706_ctl_cmd(struct bcma_drv_cc *cc, u32 code)
 	return 0;
 }
 
-static int bcm47xxnflash_ops_bcm4706_poll(struct bcma_drv_cc *cc)
-{
-	int i;
-
-	for (i = 0; i < NFLASH_READY_RETRIES; i++) {
-		if (bcma_cc_read32(cc, BCMA_CC_NFLASH_CTL) & NCTL_READY) {
-			if (bcma_cc_read32(cc, BCMA_CC_NFLASH_CTL) &
-			    BCMA_CC_NFLASH_CTL_ERR) {
-				pr_err("Error on polling\n");
-				return -EBUSY;
-			} else {
-				return 0;
-			}
-		}
-	}
-
-	pr_err("Polling timeout!\n");
-	return -EBUSY;
-}
-
-/**************************************************
- * R/W
- **************************************************/
-
-static void bcm47xxnflash_ops_bcm4706_read(struct mtd_info *mtd, uint8_t *buf,
-					   int len)
-{
-	struct nand_chip *nand_chip = mtd_to_nand(mtd);
-	struct bcm47xxnflash *b47n = nand_get_controller_data(nand_chip);
-
-	u32 ctlcode;
-	u32 *dest = (u32 *)buf;
-	int i;
-	int toread;
-
-	BUG_ON(b47n->curr_page_addr & ~nand_chip->pagemask);
-	/* Don't validate column using nand_chip->page_shift, it may be bigger
-	 * when accessing OOB */
-
-	while (len) {
-		/* We can read maximum of 0x200 bytes at once */
-		toread = min(len, 0x200);
-
-		/* Set page and column */
-		bcma_cc_write32(b47n->cc, BCMA_CC_NFLASH_COL_ADDR,
-				b47n->curr_column);
-		bcma_cc_write32(b47n->cc, BCMA_CC_NFLASH_ROW_ADDR,
-				b47n->curr_page_addr);
-
-		/* Prepare to read */
-		ctlcode = NCTL_CSA | NCTL_CMD1W | NCTL_ROW | NCTL_COL |
-			  NCTL_CMD0;
-		ctlcode |= NAND_CMD_READSTART << 8;
-		if (bcm47xxnflash_ops_bcm4706_ctl_cmd(b47n->cc, ctlcode))
-			return;
-		if (bcm47xxnflash_ops_bcm4706_poll(b47n->cc))
-			return;
-
-		/* Eventually read some data :) */
-		for (i = 0; i < toread; i += 4, dest++) {
-			ctlcode = NCTL_CSA | NCTL_DATA_CYCLES(4) | NCTL_READ;
-			if (i == toread - 4) /* Last read goes without that */
-				ctlcode &= ~NCTL_CSA;
-			if (bcm47xxnflash_ops_bcm4706_ctl_cmd(b47n->cc,
-							      ctlcode))
-				return;
-			*dest = bcma_cc_read32(b47n->cc, BCMA_CC_NFLASH_DATA);
-		}
-
-		b47n->curr_column += toread;
-		len -= toread;
-	}
-}
-
-static void bcm47xxnflash_ops_bcm4706_write(struct mtd_info *mtd,
-					    const uint8_t *buf, int len)
-{
-	struct nand_chip *nand_chip = mtd_to_nand(mtd);
-	struct bcm47xxnflash *b47n = nand_get_controller_data(nand_chip);
-	struct bcma_drv_cc *cc = b47n->cc;
-
-	u32 ctlcode;
-	const u32 *data = (u32 *)buf;
-	int i;
-
-	BUG_ON(b47n->curr_page_addr & ~nand_chip->pagemask);
-	/* Don't validate column using nand_chip->page_shift, it may be bigger
-	 * when accessing OOB */
-
-	for (i = 0; i < len; i += 4, data++) {
-		bcma_cc_write32(cc, BCMA_CC_NFLASH_DATA, *data);
-
-		ctlcode = NCTL_CSA | NCTL_DATA_CYCLES(4) | NCTL_WRITE;
-		if (i == len - 4) /* Last read goes without that */
-			ctlcode &= ~NCTL_CSA;
-		if (bcm47xxnflash_ops_bcm4706_ctl_cmd(cc, ctlcode)) {
-			pr_err("%s ctl_cmd didn't work!\n", __func__);
-			return;
-		}
-	}
-
-	b47n->curr_column += len;
-}
-
 /**************************************************
  * NAND chip ops
  **************************************************/
 
-static void bcm47xxnflash_ops_bcm4706_cmd_ctrl(struct nand_chip *nand_chip,
-					       int cmd, unsigned int ctrl)
-{
-	struct bcm47xxnflash *b47n = nand_get_controller_data(nand_chip);
-	u32 code = 0;
-
-	if (cmd == NAND_CMD_NONE)
-		return;
-
-	if (cmd & NAND_CTRL_CLE)
-		code = cmd | NCTL_CMD0;
-
-	/* nCS is not needed for reset command */
-	if (cmd != NAND_CMD_RESET)
-		code |= NCTL_CSA;
-
-	bcm47xxnflash_ops_bcm4706_ctl_cmd(b47n->cc, code);
-}
-
-/* Default nand_select_chip calls cmd_ctrl, which is not used in BCM4706 */
-static void bcm47xxnflash_ops_bcm4706_select_chip(struct nand_chip *chip,
-						  int cs)
-{
-	return;
-}
-
-static int bcm47xxnflash_ops_bcm4706_dev_ready(struct nand_chip *nand_chip)
-{
-	struct bcm47xxnflash *b47n = nand_get_controller_data(nand_chip);
-
-	return !!(bcma_cc_read32(b47n->cc, BCMA_CC_NFLASH_CTL) & NCTL_READY);
-}
-
-/*
- * Default nand_command and nand_command_lp don't match BCM4706 hardware layout.
- * For example, reading chip id is performed in a non-standard way.
- * Setting column and page is also handled differently, we use a special
- * registers of ChipCommon core. Hacking cmd_ctrl to understand and convert
- * standard commands would be much more complicated.
- */
-static void bcm47xxnflash_ops_bcm4706_cmdfunc(struct nand_chip *nand_chip,
-					      unsigned command, int column,
-					      int page_addr)
-{
-	struct mtd_info *mtd = nand_to_mtd(nand_chip);
-	struct bcm47xxnflash *b47n = nand_get_controller_data(nand_chip);
-	struct bcma_drv_cc *cc = b47n->cc;
-	u32 ctlcode;
-	int i;
-
-	if (column != -1)
-		b47n->curr_column = column;
-	if (page_addr != -1)
-		b47n->curr_page_addr = page_addr;
-
-	switch (command) {
-	case NAND_CMD_RESET:
-		nand_chip->legacy.cmd_ctrl(nand_chip, command, NAND_CTRL_CLE);
-
-		ndelay(100);
-		nand_wait_ready(nand_chip);
-		break;
-	case NAND_CMD_READID:
-		ctlcode = NCTL_CSA | NCTL_SPECADDR | NCTL_CMD1W | NCTL_CMD0;
-		ctlcode |= NAND_CMD_READID;
-		if (bcm47xxnflash_ops_bcm4706_ctl_cmd(b47n->cc, ctlcode)) {
-			pr_err("READID error\n");
-			break;
-		}
-
-		/*
-		 * Reading is specific, last one has to go without NCTL_CSA
-		 * bit. We don't know how many reads NAND subsystem is going
-		 * to perform, so cache everything.
-		 */
-		for (i = 0; i < ARRAY_SIZE(b47n->id_data); i++) {
-			ctlcode = NCTL_CSA | NCTL_READ | NCTL_DATA_CYCLES(1);
-			if (i == ARRAY_SIZE(b47n->id_data) - 1)
-				ctlcode &= ~NCTL_CSA;
-			if (bcm47xxnflash_ops_bcm4706_ctl_cmd(b47n->cc,
-							      ctlcode)) {
-				pr_err("READID error\n");
-				break;
-			}
-			b47n->id_data[i] =
-				bcma_cc_read32(b47n->cc, BCMA_CC_NFLASH_DATA)
-				& 0xFF;
-		}
-
-		break;
-	case NAND_CMD_STATUS:
-		ctlcode = NCTL_CSA | NCTL_CMD0 | NAND_CMD_STATUS;
-		if (bcm47xxnflash_ops_bcm4706_ctl_cmd(cc, ctlcode))
-			pr_err("STATUS command error\n");
-		break;
-	case NAND_CMD_READ0:
-		break;
-	case NAND_CMD_READOOB:
-		if (page_addr != -1)
-			b47n->curr_column += mtd->writesize;
-		break;
-	case NAND_CMD_ERASE1:
-		bcma_cc_write32(cc, BCMA_CC_NFLASH_ROW_ADDR,
-				b47n->curr_page_addr);
-		ctlcode = NCTL_ROW | NCTL_CMD1W | NCTL_CMD0 |
-			  NAND_CMD_ERASE1 | (NAND_CMD_ERASE2 << 8);
-		if (bcm47xxnflash_ops_bcm4706_ctl_cmd(cc, ctlcode))
-			pr_err("ERASE1 failed\n");
-		break;
-	case NAND_CMD_ERASE2:
-		break;
-	case NAND_CMD_SEQIN:
-		/* Set page and column */
-		bcma_cc_write32(cc, BCMA_CC_NFLASH_COL_ADDR,
-				b47n->curr_column);
-		bcma_cc_write32(cc, BCMA_CC_NFLASH_ROW_ADDR,
-				b47n->curr_page_addr);
-
-		/* Prepare to write */
-		ctlcode = NCTL_CSA | NCTL_ROW | NCTL_COL | NCTL_CMD0;
-		ctlcode |= NAND_CMD_SEQIN;
-		if (bcm47xxnflash_ops_bcm4706_ctl_cmd(cc, ctlcode))
-			pr_err("SEQIN failed\n");
-		break;
-	case NAND_CMD_PAGEPROG:
-		if (bcm47xxnflash_ops_bcm4706_ctl_cmd(cc, NCTL_CMD0 |
-							  NAND_CMD_PAGEPROG))
-			pr_err("PAGEPROG failed\n");
-		if (bcm47xxnflash_ops_bcm4706_poll(cc))
-			pr_err("PAGEPROG not ready\n");
-		break;
-	default:
-		pr_err("Command 0x%X unsupported\n", command);
-		break;
-	}
-	b47n->curr_command = command;
-}
-
-static u8 bcm47xxnflash_ops_bcm4706_read_byte(struct nand_chip *nand_chip)
-{
-	struct mtd_info *mtd = nand_to_mtd(nand_chip);
-	struct bcm47xxnflash *b47n = nand_get_controller_data(nand_chip);
-	struct bcma_drv_cc *cc = b47n->cc;
-	u32 tmp = 0;
-
-	switch (b47n->curr_command) {
-	case NAND_CMD_READID:
-		if (b47n->curr_column >= ARRAY_SIZE(b47n->id_data)) {
-			pr_err("Requested invalid id_data: %d\n",
-			       b47n->curr_column);
-			return 0;
-		}
-		return b47n->id_data[b47n->curr_column++];
-	case NAND_CMD_STATUS:
-		if (bcm47xxnflash_ops_bcm4706_ctl_cmd(cc,
-						      NCTL_READ |
-						      NCTL_DATA_CYCLES(1)))
-			return 0;
-		return bcma_cc_read32(cc, BCMA_CC_NFLASH_DATA) & 0xff;
-	case NAND_CMD_READOOB:
-		bcm47xxnflash_ops_bcm4706_read(mtd, (u8 *)&tmp, 4);
-		return tmp & 0xFF;
-	}
-
-	pr_err("Invalid command for byte read: 0x%X\n", b47n->curr_command);
-	return 0;
-}
-
-static void bcm47xxnflash_ops_bcm4706_read_buf(struct nand_chip *nand_chip,
-					       uint8_t *buf, int len)
-{
-	struct bcm47xxnflash *b47n = nand_get_controller_data(nand_chip);
-
-	switch (b47n->curr_command) {
-	case NAND_CMD_READ0:
-	case NAND_CMD_READOOB:
-		bcm47xxnflash_ops_bcm4706_read(nand_to_mtd(nand_chip), buf,
-					       len);
-		return;
-	}
-
-	pr_err("Invalid command for buf read: 0x%X\n", b47n->curr_command);
-}
-
-static void bcm47xxnflash_ops_bcm4706_write_buf(struct nand_chip *nand_chip,
-						const uint8_t *buf, int len)
-{
-	struct bcm47xxnflash *b47n = nand_get_controller_data(nand_chip);
-
-	switch (b47n->curr_command) {
-	case NAND_CMD_SEQIN:
-		bcm47xxnflash_ops_bcm4706_write(nand_to_mtd(nand_chip), buf,
-						len);
-		return;
-	}
-
-	pr_err("Invalid command for buf write: 0x%X\n", b47n->curr_command);
-}
-
 static int
 bcm47xxnflash_ops_bcm4706_exec_cmd_addr(struct nand_chip *chip,
 					const struct nand_subop *subop)
@@ -525,7 +222,6 @@ static const struct nand_controller_ops bcm47xxnflash_ops = {
 
 int bcm47xxnflash_ops_bcm4706_init(struct bcm47xxnflash *b47n)
 {
-	struct nand_chip *nand_chip = (struct nand_chip *)&b47n->nand_chip;
 	int err;
 	u32 freq;
 	u16 clock;
@@ -538,17 +234,6 @@ int bcm47xxnflash_ops_bcm4706_init(struct bcm47xxnflash *b47n)
 	nand_controller_init(&b47n->base);
 	b47n->base.ops = &bcm47xxnflash_ops;
 	b47n->nand_chip.controller = &b47n->base;
-	nand_chip->legacy.select_chip = bcm47xxnflash_ops_bcm4706_select_chip;
-	nand_chip->legacy.cmd_ctrl = bcm47xxnflash_ops_bcm4706_cmd_ctrl;
-	nand_chip->legacy.dev_ready = bcm47xxnflash_ops_bcm4706_dev_ready;
-	b47n->nand_chip.legacy.cmdfunc = bcm47xxnflash_ops_bcm4706_cmdfunc;
-	b47n->nand_chip.legacy.read_byte = bcm47xxnflash_ops_bcm4706_read_byte;
-	b47n->nand_chip.legacy.read_buf = bcm47xxnflash_ops_bcm4706_read_buf;
-	b47n->nand_chip.legacy.write_buf = bcm47xxnflash_ops_bcm4706_write_buf;
-	b47n->nand_chip.legacy.set_features = nand_get_set_features_notsupp;
-	b47n->nand_chip.legacy.get_features = nand_get_set_features_notsupp;
-
-	nand_chip->legacy.chip_delay = 50;
 	b47n->nand_chip.bbt_options = NAND_BBT_USE_FLASH;
 	b47n->nand_chip.ecc.mode = NAND_ECC_NONE; /* TODO: implement ECC */
 
-- 
2.25.4


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

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

* [PATCH v2 7/8] mtd: rawnand: bcm47xx: Simplify the init() function
  2020-05-18 16:28 [PATCH v2 0/8] mtd: rawnand: bcm47xx: Convert to exec_op() (and more) Boris Brezillon
                   ` (5 preceding siblings ...)
  2020-05-18 16:28 ` [PATCH v2 6/8] mtd: rawnand: bcm47xx: Get rid of the legacy implementation Boris Brezillon
@ 2020-05-18 16:28 ` Boris Brezillon
  2020-05-18 16:28 ` [PATCH v2 8/8] mtd: rawnand: bcm47xx: Merge all source files Boris Brezillon
  2020-06-03 14:22 ` [PATCH v2 0/8] mtd: rawnand: bcm47xx: Convert to exec_op() (and more) Miquel Raynal
  8 siblings, 0 replies; 10+ messages in thread
From: Boris Brezillon @ 2020-05-18 16:28 UTC (permalink / raw)
  To: Miquel Raynal, linux-mtd, Hauke Mehrtens,
	Rafał Miłecki, linux-mips
  Cc: Richard Weinberger, Boris Brezillon, Vignesh Raghavendra, Tudor Ambarus

The row and column sizes are now set as part of the exec_op() procedure
and adjusted to match the requested number of address cycles. No need
to set them in the init() function since those values will be
overwritten anyway. As for the other sanity check that was done on the
chip size, I don't think it's really needed.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
Changes in v2:
* Add R-b
---
 .../mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c  | 31 ++-----------------
 1 file changed, 2 insertions(+), 29 deletions(-)

diff --git a/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c b/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c
index b6c5db9acac9..e34a13b7f919 100644
--- a/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c
+++ b/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c
@@ -227,10 +227,6 @@ int bcm47xxnflash_ops_bcm4706_init(struct bcm47xxnflash *b47n)
 	u16 clock;
 	u8 w0, w1, w2, w3, w4;
 
-	unsigned long chipsize; /* MiB */
-	u8 tbits, col_bits, col_size, row_bits, row_bsize;
-	u32 val;
-
 	nand_controller_init(&b47n->base);
 	b47n->base.ops = &bcm47xxnflash_ops;
 	b47n->nand_chip.controller = &b47n->base;
@@ -264,32 +260,9 @@ int bcm47xxnflash_ops_bcm4706_init(struct bcm47xxnflash *b47n)
 	err = nand_scan(&b47n->nand_chip, 1);
 	if (err) {
 		pr_err("Could not scan NAND flash: %d\n", err);
-		goto exit;
-	}
-
-	/* Configure FLASH */
-	chipsize = nanddev_target_size(&b47n->nand_chip.base) >> 20;
-	tbits = ffs(chipsize); /* find first bit set */
-	if (!tbits || tbits != fls(chipsize)) {
-		pr_err("Invalid flash size: 0x%lX\n", chipsize);
-		err = -ENOTSUPP;
-		goto exit;
-	}
-	tbits += 19; /* Broadcom increases *index* by 20, we increase *pos* */
-
-	col_bits = b47n->nand_chip.page_shift + 1;
-	col_size = (col_bits + 7) / 8;
-
-	row_bits = tbits - col_bits + 1;
-	row_bsize = (row_bits + 7) / 8;
-
-	val = CONF_ROW_BYTES(row_bsize) | CONF_COL_BYTES(col_size) |
-	      CONF_MAGIC_BIT;
-	bcma_cc_write32(b47n->cc, BCMA_CC_NFLASH_CONF, val);
-
-exit:
-	if (err)
 		bcma_cc_mask32(b47n->cc, BCMA_CC_4706_FLASHSCFG,
 			       ~BCMA_CC_4706_FLASHSCFG_NF1);
+	}
+
 	return err;
 }
-- 
2.25.4


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

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

* [PATCH v2 8/8] mtd: rawnand: bcm47xx: Merge all source files
  2020-05-18 16:28 [PATCH v2 0/8] mtd: rawnand: bcm47xx: Convert to exec_op() (and more) Boris Brezillon
                   ` (6 preceding siblings ...)
  2020-05-18 16:28 ` [PATCH v2 7/8] mtd: rawnand: bcm47xx: Simplify the init() function Boris Brezillon
@ 2020-05-18 16:28 ` Boris Brezillon
  2020-06-03 14:22 ` [PATCH v2 0/8] mtd: rawnand: bcm47xx: Convert to exec_op() (and more) Miquel Raynal
  8 siblings, 0 replies; 10+ messages in thread
From: Boris Brezillon @ 2020-05-18 16:28 UTC (permalink / raw)
  To: Miquel Raynal, linux-mtd, Hauke Mehrtens,
	Rafał Miłecki, linux-mips
  Cc: Richard Weinberger, Boris Brezillon, Vignesh Raghavendra, Tudor Ambarus

Given the number of lines it doesn't make sense to split the code.
Let's merge everything and move the driver to drivers/mtd/nand/raw.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
Changes in v2:
* Fix the Makefile entry
* Merge commits doing the source merge and the file move
* Rename the driver bcm47xx-nand-controller
---
 drivers/mtd/nand/raw/Makefile                 |  2 +-
 ...ps_bcm4706.c => bcm47xx-nand-controller.c} | 81 ++++++++++++++++++-
 drivers/mtd/nand/raw/bcm47xxnflash/Makefile   |  5 --
 .../nand/raw/bcm47xxnflash/bcm47xxnflash.h    | 21 -----
 drivers/mtd/nand/raw/bcm47xxnflash/main.c     | 77 ------------------
 5 files changed, 79 insertions(+), 107 deletions(-)
 rename drivers/mtd/nand/raw/{bcm47xxnflash/ops_bcm4706.c => bcm47xx-nand-controller.c} (81%)
 delete mode 100644 drivers/mtd/nand/raw/bcm47xxnflash/Makefile
 delete mode 100644 drivers/mtd/nand/raw/bcm47xxnflash/bcm47xxnflash.h
 delete mode 100644 drivers/mtd/nand/raw/bcm47xxnflash/main.c

diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile
index 2d136b158fb7..854107365774 100644
--- a/drivers/mtd/nand/raw/Makefile
+++ b/drivers/mtd/nand/raw/Makefile
@@ -47,7 +47,7 @@ obj-$(CONFIG_MTD_NAND_RICOH)		+= r852.o
 obj-y					+= ingenic/
 obj-$(CONFIG_MTD_NAND_GPMI_NAND)	+= gpmi-nand/
 obj-$(CONFIG_MTD_NAND_XWAY)		+= xway_nand.o
-obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH)	+= bcm47xxnflash/
+obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH)	+= bcm47xx-nand-controller.o
 obj-$(CONFIG_MTD_NAND_SUNXI)		+= sunxi_nand.o
 obj-$(CONFIG_MTD_NAND_HISI504)	        += hisi504_nand.o
 obj-$(CONFIG_MTD_NAND_BRCMNAND)		+= brcmnand/
diff --git a/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c b/drivers/mtd/nand/raw/bcm47xx-nand-controller.c
similarity index 81%
rename from drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c
rename to drivers/mtd/nand/raw/bcm47xx-nand-controller.c
index e34a13b7f919..ff8b86a8e923 100644
--- a/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c
+++ b/drivers/mtd/nand/raw/bcm47xx-nand-controller.c
@@ -5,13 +5,14 @@
  * Copyright (C) 2012 Rafał Miłecki <zajec5@gmail.com>
  */
 
-#include "bcm47xxnflash.h"
-
+#include <linux/delay.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/slab.h>
-#include <linux/delay.h>
+#include <linux/platform_device.h>
 #include <linux/bcma/bcma.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/rawnand.h>
 
 /* Broadcom uses 1'000'000 but it seems to be too many. Tests on WNDR4500 has
  * shown ~1000 retries as maxiumum. */
@@ -48,6 +49,17 @@
 #define CONF_COL_BYTES(x)		(((x) - 1) << 4)
 #define CONF_ROW_BYTES(x)		(((x) - 1) << 6)
 
+#ifndef pr_fmt
+#define pr_fmt(fmt)		KBUILD_MODNAME ": " fmt
+#endif
+
+struct bcm47xxnflash {
+	struct nand_controller base;
+	struct bcma_drv_cc *cc;
+
+	struct nand_chip nand_chip;
+};
+
 /**************************************************
  * Various helpers
  **************************************************/
@@ -266,3 +278,66 @@ int bcm47xxnflash_ops_bcm4706_init(struct bcm47xxnflash *b47n)
 
 	return err;
 }
+
+static const char *probes[] = { "bcm47xxpart", NULL };
+
+static int bcm47xxnflash_probe(struct platform_device *pdev)
+{
+	struct bcma_nflash *nflash = dev_get_platdata(&pdev->dev);
+	struct bcm47xxnflash *b47n;
+	struct mtd_info *mtd;
+	int err = 0;
+
+	b47n = devm_kzalloc(&pdev->dev, sizeof(*b47n), GFP_KERNEL);
+	if (!b47n)
+		return -ENOMEM;
+
+	nand_set_controller_data(&b47n->nand_chip, b47n);
+	mtd = nand_to_mtd(&b47n->nand_chip);
+	mtd->dev.parent = &pdev->dev;
+	b47n->cc = container_of(nflash, struct bcma_drv_cc, nflash);
+
+	if (b47n->cc->core->bus->chipinfo.id == BCMA_CHIP_ID_BCM4706) {
+		err = bcm47xxnflash_ops_bcm4706_init(b47n);
+	} else {
+		pr_err("Device not supported\n");
+		err = -ENOTSUPP;
+	}
+	if (err) {
+		pr_err("Initialization failed: %d\n", err);
+		return err;
+	}
+
+	platform_set_drvdata(pdev, b47n);
+
+	err = mtd_device_parse_register(mtd, probes, NULL, NULL, 0);
+	if (err) {
+		pr_err("Failed to register MTD device: %d\n", err);
+		return err;
+	}
+
+	return 0;
+}
+
+static int bcm47xxnflash_remove(struct platform_device *pdev)
+{
+	struct bcm47xxnflash *nflash = platform_get_drvdata(pdev);
+
+	nand_release(&nflash->nand_chip);
+
+	return 0;
+}
+
+static struct platform_driver bcm47xxnflash_driver = {
+	.probe	= bcm47xxnflash_probe,
+	.remove = bcm47xxnflash_remove,
+	.driver = {
+		.name = "bcma_nflash",
+	},
+};
+module_platform_driver(bcm47xxnflash_driver);
+
+MODULE_DESCRIPTION("NAND flash driver for BCMA bus");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Rafał Miłecki");
+
diff --git a/drivers/mtd/nand/raw/bcm47xxnflash/Makefile b/drivers/mtd/nand/raw/bcm47xxnflash/Makefile
deleted file mode 100644
index b531a630c9cf..000000000000
--- a/drivers/mtd/nand/raw/bcm47xxnflash/Makefile
+++ /dev/null
@@ -1,5 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0-only
-bcm47xxnflash-y				+= main.o
-bcm47xxnflash-y				+= ops_bcm4706.o
-
-obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH)	+= bcm47xxnflash.o
diff --git a/drivers/mtd/nand/raw/bcm47xxnflash/bcm47xxnflash.h b/drivers/mtd/nand/raw/bcm47xxnflash/bcm47xxnflash.h
deleted file mode 100644
index 8de0e7e0a3a4..000000000000
--- a/drivers/mtd/nand/raw/bcm47xxnflash/bcm47xxnflash.h
+++ /dev/null
@@ -1,21 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef __BCM47XXNFLASH_H
-#define __BCM47XXNFLASH_H
-
-#ifndef pr_fmt
-#define pr_fmt(fmt)		KBUILD_MODNAME ": " fmt
-#endif
-
-#include <linux/mtd/mtd.h>
-#include <linux/mtd/rawnand.h>
-
-struct bcm47xxnflash {
-	struct nand_controller base;
-	struct bcma_drv_cc *cc;
-
-	struct nand_chip nand_chip;
-};
-
-int bcm47xxnflash_ops_bcm4706_init(struct bcm47xxnflash *b47n);
-
-#endif /* BCM47XXNFLASH */
diff --git a/drivers/mtd/nand/raw/bcm47xxnflash/main.c b/drivers/mtd/nand/raw/bcm47xxnflash/main.c
deleted file mode 100644
index 8dae97c1dbe7..000000000000
--- a/drivers/mtd/nand/raw/bcm47xxnflash/main.c
+++ /dev/null
@@ -1,77 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * BCM47XX NAND flash driver
- *
- * Copyright (C) 2012 Rafał Miłecki <zajec5@gmail.com>
- */
-
-#include "bcm47xxnflash.h"
-
-#include <linux/module.h>
-#include <linux/kernel.h>
-#include <linux/slab.h>
-#include <linux/platform_device.h>
-#include <linux/bcma/bcma.h>
-
-MODULE_DESCRIPTION("NAND flash driver for BCMA bus");
-MODULE_LICENSE("GPL");
-MODULE_AUTHOR("Rafał Miłecki");
-
-static const char *probes[] = { "bcm47xxpart", NULL };
-
-static int bcm47xxnflash_probe(struct platform_device *pdev)
-{
-	struct bcma_nflash *nflash = dev_get_platdata(&pdev->dev);
-	struct bcm47xxnflash *b47n;
-	struct mtd_info *mtd;
-	int err = 0;
-
-	b47n = devm_kzalloc(&pdev->dev, sizeof(*b47n), GFP_KERNEL);
-	if (!b47n)
-		return -ENOMEM;
-
-	nand_set_controller_data(&b47n->nand_chip, b47n);
-	mtd = nand_to_mtd(&b47n->nand_chip);
-	mtd->dev.parent = &pdev->dev;
-	b47n->cc = container_of(nflash, struct bcma_drv_cc, nflash);
-
-	if (b47n->cc->core->bus->chipinfo.id == BCMA_CHIP_ID_BCM4706) {
-		err = bcm47xxnflash_ops_bcm4706_init(b47n);
-	} else {
-		pr_err("Device not supported\n");
-		err = -ENOTSUPP;
-	}
-	if (err) {
-		pr_err("Initialization failed: %d\n", err);
-		return err;
-	}
-
-	platform_set_drvdata(pdev, b47n);
-
-	err = mtd_device_parse_register(mtd, probes, NULL, NULL, 0);
-	if (err) {
-		pr_err("Failed to register MTD device: %d\n", err);
-		return err;
-	}
-
-	return 0;
-}
-
-static int bcm47xxnflash_remove(struct platform_device *pdev)
-{
-	struct bcm47xxnflash *nflash = platform_get_drvdata(pdev);
-
-	nand_release(&nflash->nand_chip);
-
-	return 0;
-}
-
-static struct platform_driver bcm47xxnflash_driver = {
-	.probe	= bcm47xxnflash_probe,
-	.remove = bcm47xxnflash_remove,
-	.driver = {
-		.name = "bcma_nflash",
-	},
-};
-
-module_platform_driver(bcm47xxnflash_driver);
-- 
2.25.4


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

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

* Re: [PATCH v2 0/8] mtd: rawnand: bcm47xx: Convert to exec_op() (and more)
  2020-05-18 16:28 [PATCH v2 0/8] mtd: rawnand: bcm47xx: Convert to exec_op() (and more) Boris Brezillon
                   ` (7 preceding siblings ...)
  2020-05-18 16:28 ` [PATCH v2 8/8] mtd: rawnand: bcm47xx: Merge all source files Boris Brezillon
@ 2020-06-03 14:22 ` Miquel Raynal
  8 siblings, 0 replies; 10+ messages in thread
From: Miquel Raynal @ 2020-06-03 14:22 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Vignesh Raghavendra, Tudor Ambarus, Hauke Mehrtens,
	Rafał Miłecki, linux-mips, linux-mtd,
	Richard Weinberger

Hello,

Boris Brezillon <boris.brezillon@collabora.com> wrote on Mon, 18 May
2020 18:28:29 +0200:

> Hello,
> 
> A bit of context to explain the motivation behind those conversions
> I've been sending for the last couple of weeks. The raw NAND subsystem
> carries a lot of history which makes any rework not only painful, but
> also subject to regressions which we only detect when someone dares to
> update its kernel on one of those ancient HW. While carrying drivers
> for old HW is not a problem per se, carrying ancient and unmaintained
> drivers that are not converted to new APIs is a maintenance burden,
> hence this massive conversion attempt I'm conducting here.
> 
> So here is a series converting the BCM47XX NAND controller driver to
> exec_op(), plus a bunch of minor improvements done along the way.
> I hope I'll find someone to test those changes, but if there's no one
> still having access to this  HW or no interest in keeping it supported
> in recent kernel versions, we should definitely consider removing the
> driver instead.
> 
> No major changes in this v2, apart from fixes for things reported by
> Miquel. See the changelog on each patch for more details.
> 
> Regards,
> 
> Boris
> 
> Boris Brezillon (8):
>   mtd: rawnand: Add an is_last flag to nand_subop
>   mtd: rawnand: bcm47xx: Drop dependency on BCMA
>   mtd: rawnand: bcm47xx: Allow compiling the driver when COMPILE_TEST=y
>   mtd: rawnand: bcm47xx: Demistify a few more things
>   mtd: rawnand: bcm47xx: Implement the exec_op() interface
>   mtd: rawnand: bcm47xx: Get rid of the legacy implementation
>   mtd: rawnand: bcm47xx: Simplify the init() function
>   mtd: rawnand: bcm47xx: Merge all source files
> 
>  drivers/mtd/nand/raw/Kconfig                  |   3 +-
>  drivers/mtd/nand/raw/Makefile                 |   2 +-
>  .../mtd/nand/raw/bcm47xx-nand-controller.c    | 343 +++++++++++++
>  drivers/mtd/nand/raw/bcm47xxnflash/Makefile   |   5 -
>  .../nand/raw/bcm47xxnflash/bcm47xxnflash.h    |  26 -
>  drivers/mtd/nand/raw/bcm47xxnflash/main.c     |  77 ---
>  .../mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c  | 450 ------------------
>  drivers/mtd/nand/raw/nand_base.c              |   2 +
>  include/linux/mtd/rawnand.h                   |   2 +
>  9 files changed, 349 insertions(+), 561 deletions(-)
>  create mode 100644 drivers/mtd/nand/raw/bcm47xx-nand-controller.c
>  delete mode 100644 drivers/mtd/nand/raw/bcm47xxnflash/Makefile
>  delete mode 100644 drivers/mtd/nand/raw/bcm47xxnflash/bcm47xxnflash.h
>  delete mode 100644 drivers/mtd/nand/raw/bcm47xxnflash/main.c
>  delete mode 100644 drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c
> 

Anyone to test this series?

If not I will apply it as soon as v5.8-rc1 is released.


Thanks,
Miquèl

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

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

end of thread, other threads:[~2020-06-03 14:22 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-18 16:28 [PATCH v2 0/8] mtd: rawnand: bcm47xx: Convert to exec_op() (and more) Boris Brezillon
2020-05-18 16:28 ` [PATCH v2 1/8] mtd: rawnand: Add an is_last flag to nand_subop Boris Brezillon
2020-05-18 16:28 ` [PATCH v2 2/8] mtd: rawnand: bcm47xx: Drop dependency on BCMA Boris Brezillon
2020-05-18 16:28 ` [PATCH v2 3/8] mtd: rawnand: bcm47xx: Allow compiling the driver when COMPILE_TEST=y Boris Brezillon
2020-05-18 16:28 ` [PATCH v2 4/8] mtd: rawnand: bcm47xx: Demistify a few more things Boris Brezillon
2020-05-18 16:28 ` [PATCH v2 5/8] mtd: rawnand: bcm47xx: Implement the exec_op() interface Boris Brezillon
2020-05-18 16:28 ` [PATCH v2 6/8] mtd: rawnand: bcm47xx: Get rid of the legacy implementation Boris Brezillon
2020-05-18 16:28 ` [PATCH v2 7/8] mtd: rawnand: bcm47xx: Simplify the init() function Boris Brezillon
2020-05-18 16:28 ` [PATCH v2 8/8] mtd: rawnand: bcm47xx: Merge all source files Boris Brezillon
2020-06-03 14:22 ` [PATCH v2 0/8] mtd: rawnand: bcm47xx: Convert to exec_op() (and more) Miquel Raynal

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