diff mbox series

[2/3] mtd: rawnand: bcrmnand: Add exec_op() support

Message ID 20200502163432.1543243-3-boris.brezillon@collabora.com
State Changes Requested
Delegated to: Miquel Raynal
Headers show
Series mtd: rawnand: brcmnand: Convert to exec_op() | expand

Commit Message

Boris Brezillon May 2, 2020, 4:34 p.m. UTC
This implementation of exec_op() relies on low-level operations only. We
could add support for high-level operations too through an op parser,
but the gain is likely to be negligible since read/write page operations
already have a fast path ({readwrite}_page[raw]() implementations).

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/mtd/nand/raw/brcmnand/brcmnand.c | 72 ++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

Comments

Miquel Raynal May 11, 2020, 4:57 p.m. UTC | #1
Hi Boris,

Boris Brezillon <boris.brezillon@collabora.com> wrote on Sat,  2 May
2020 18:34:31 +0200:

> This implementation of exec_op() relies on low-level operations only. We
> could add support for high-level operations too through an op parser,
> but the gain is likely to be negligible since read/write page operations
> already have a fast path ({readwrite}_page[raw]() implementations).

Agreed.

> 
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
>  drivers/mtd/nand/raw/brcmnand/brcmnand.c | 72 ++++++++++++++++++++++++
>  1 file changed, 72 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> index e4e3ceeac38f..e70117146755 100644
> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> @@ -1599,6 +1599,77 @@ static int brcmnand_low_level_op(struct brcmnand_host *host,
>  	return brcmnand_waitfunc(chip);
>  }
>  
> +static void brcmnand_exec_instr(struct brcmnand_host *host,
> +				const struct nand_op_instr *instr,
> +				bool last_op)
> +{
> +	unsigned int i;
> +	const u8 *out;
> +	u8 *in;
> +
> +	switch (instr->type) {
> +	case NAND_OP_CMD_INSTR:
> +		brcmnand_low_level_op(host, LL_OP_CMD,
> +				      instr->ctx.cmd.opcode, last_op);
> +		break;
> +
> +	case NAND_OP_ADDR_INSTR:
> +		for (i = 0; i < instr->ctx.addr.naddrs; i++)
> +			brcmnand_low_level_op(host, LL_OP_ADDR,
> +					      instr->ctx.addr.addrs[i],
> +					      last_op);
> +		break;
> +
> +	case NAND_OP_DATA_IN_INSTR:
> +		in = instr->ctx.data.buf.in;
> +		for (i = 0; i < instr->ctx.data.len; i++) {
> +			brcmnand_low_level_op(host, LL_OP_RD, 0, last_op);
> +			in[i] = brcmnand_read_reg(host->ctrl,
> +						  BRCMNAND_LL_RDATA);
> +		}
> +		break;
> +
> +	case NAND_OP_DATA_OUT_INSTR:
> +		out = instr->ctx.data.buf.out;
> +		for (i = 0; i < instr->ctx.data.len; i++)
> +			brcmnand_low_level_op(host, LL_OP_WR, out[i], last_op);
> +		break;
> +
> +	case NAND_OP_WAITRDY_INSTR:
> +		/*
> +		 * Nothing to do here, brcmnand_low_level_op() already waits on
> +		 * FLASH_READY every time it's called.
> +		 */
> +		break;
> +
> +	default:
> +		break;
> +	}
> +}
> +
> +static int brcmnand_exec_op(struct nand_chip *chip,
> +			    const struct nand_operation *op,
> +			    bool check_only)
> +{
> +	struct brcmnand_host *host = nand_get_controller_data(chip);
> +	struct mtd_info *mtd = nand_to_mtd(chip);
> +	unsigned int i;
> +
> +	if (check_only)
> +		return 0;
> +
> +	if (op->deassert_wp)
> +		brcmnand_wp(mtd, 0);
> +
> +	for (i = 0; i < op->ninstrs; i++)
> +		brcmnand_exec_instr(host, &op->instrs[i], i == op->ninstrs - 1);

Maybe                                                          (              ) 

to improve readability?

Or even maybe using an intermediate boolean?

> +
> +	if (op->deassert_wp)
> +		brcmnand_wp(mtd, 1);
> +
> +	return 0;
> +}
> +
>  static void brcmnand_cmdfunc(struct nand_chip *chip, unsigned command,
>  			     int column, int page_addr)
>  {
> @@ -2597,6 +2668,7 @@ static int brcmnand_attach_chip(struct nand_chip *chip)
>  
>  static const struct nand_controller_ops brcmnand_controller_ops = {
>  	.attach_chip = brcmnand_attach_chip,
> +	.exec_op = brcmnand_exec_op,
>  };
>  
>  static int brcmnand_init_cs(struct brcmnand_host *host, struct device_node *dn)

With this tiny change (don't resend if unneeded):

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>


Thanks,
Miquèl
diff mbox series

Patch

diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index e4e3ceeac38f..e70117146755 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -1599,6 +1599,77 @@  static int brcmnand_low_level_op(struct brcmnand_host *host,
 	return brcmnand_waitfunc(chip);
 }
 
+static void brcmnand_exec_instr(struct brcmnand_host *host,
+				const struct nand_op_instr *instr,
+				bool last_op)
+{
+	unsigned int i;
+	const u8 *out;
+	u8 *in;
+
+	switch (instr->type) {
+	case NAND_OP_CMD_INSTR:
+		brcmnand_low_level_op(host, LL_OP_CMD,
+				      instr->ctx.cmd.opcode, last_op);
+		break;
+
+	case NAND_OP_ADDR_INSTR:
+		for (i = 0; i < instr->ctx.addr.naddrs; i++)
+			brcmnand_low_level_op(host, LL_OP_ADDR,
+					      instr->ctx.addr.addrs[i],
+					      last_op);
+		break;
+
+	case NAND_OP_DATA_IN_INSTR:
+		in = instr->ctx.data.buf.in;
+		for (i = 0; i < instr->ctx.data.len; i++) {
+			brcmnand_low_level_op(host, LL_OP_RD, 0, last_op);
+			in[i] = brcmnand_read_reg(host->ctrl,
+						  BRCMNAND_LL_RDATA);
+		}
+		break;
+
+	case NAND_OP_DATA_OUT_INSTR:
+		out = instr->ctx.data.buf.out;
+		for (i = 0; i < instr->ctx.data.len; i++)
+			brcmnand_low_level_op(host, LL_OP_WR, out[i], last_op);
+		break;
+
+	case NAND_OP_WAITRDY_INSTR:
+		/*
+		 * Nothing to do here, brcmnand_low_level_op() already waits on
+		 * FLASH_READY every time it's called.
+		 */
+		break;
+
+	default:
+		break;
+	}
+}
+
+static int brcmnand_exec_op(struct nand_chip *chip,
+			    const struct nand_operation *op,
+			    bool check_only)
+{
+	struct brcmnand_host *host = nand_get_controller_data(chip);
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	unsigned int i;
+
+	if (check_only)
+		return 0;
+
+	if (op->deassert_wp)
+		brcmnand_wp(mtd, 0);
+
+	for (i = 0; i < op->ninstrs; i++)
+		brcmnand_exec_instr(host, &op->instrs[i], i == op->ninstrs - 1);
+
+	if (op->deassert_wp)
+		brcmnand_wp(mtd, 1);
+
+	return 0;
+}
+
 static void brcmnand_cmdfunc(struct nand_chip *chip, unsigned command,
 			     int column, int page_addr)
 {
@@ -2597,6 +2668,7 @@  static int brcmnand_attach_chip(struct nand_chip *chip)
 
 static const struct nand_controller_ops brcmnand_controller_ops = {
 	.attach_chip = brcmnand_attach_chip,
+	.exec_op = brcmnand_exec_op,
 };
 
 static int brcmnand_init_cs(struct brcmnand_host *host, struct device_node *dn)