diff mbox series

[v2,04/10] mtd: rawnand: denali: switch over to ->exec_op() from legacy hooks

Message ID 1549955582-30346-5-git-send-email-yamada.masahiro@socionext.com
State New
Headers show
Series None | expand

Commit Message

Masahiro Yamada Feb. 12, 2019, 7:12 a.m. UTC 
Implement ->exec_op(), and remove the deprecated hooks.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

---

Changes in v2: None

 drivers/mtd/nand/raw/denali.c | 234 +++++++++++++++++++++++-------------------
 1 file changed, 126 insertions(+), 108 deletions(-)

-- 
2.7.4

Comments

Miquel Raynal March 4, 2019, 9:30 a.m. UTC | #1 
Hi Masahiro,

Masahiro Yamada <yamada.masahiro@socionext.com> wrote on Tue, 12 Feb
2019 16:12:56 +0900:

> Implement ->exec_op(), and remove the deprecated hooks.

> 

> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

> ---


Thanks for working on this, I like it very much!

> 

> Changes in v2: None

> 

>  drivers/mtd/nand/raw/denali.c | 234 +++++++++++++++++++++++-------------------

>  1 file changed, 126 insertions(+), 108 deletions(-)

> 

> diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c

> index a2fe2ff..bd7df25 100644

> --- a/drivers/mtd/nand/raw/denali.c

> +++ b/drivers/mtd/nand/raw/denali.c


[...]

> +

> +static int denali_exec_instr(struct nand_chip *chip,

> +			     const struct nand_op_instr *instr)

> +{

> +	struct denali_nand_info *denali = to_denali(chip);

> +	bool width16 = chip->options & NAND_BUSWIDTH_16;

> +

> +	switch (instr->type) {

> +	case NAND_OP_CMD_INSTR:

> +		denali_exec_out8(denali, DENALI_MAP11_CMD,

> +				 &instr->ctx.cmd.opcode, 1);

> +		return 0;

> +	case NAND_OP_ADDR_INSTR:

> +		denali_exec_out8(denali, DENALI_MAP11_ADDR,

> +				 instr->ctx.addr.addrs,

> +				 instr->ctx.addr.naddrs);

> +		return 0;

> +	case NAND_OP_DATA_IN_INSTR:

> +		(!instr->ctx.data.force_8bit && width16 ?

> +		 denali_exec_in16 :

> +		 denali_exec_in8)(denali, DENALI_MAP11_DATA,

> +				  instr->ctx.data.buf.in,

> +				  instr->ctx.data.len);


I think this is abusing the ternary operator, can you please find
another way for writing this? Otherwise it is not easily readable... If
it is really too complicated within 80 chars, then never mind.

> +		return 0;

> +	case NAND_OP_DATA_OUT_INSTR:

> +		(!instr->ctx.data.force_8bit && width16 ?

> +		 denali_exec_out16 :

> +		 denali_exec_out8)(denali, DENALI_MAP11_DATA,

> +				   instr->ctx.data.buf.out,

> +				   instr->ctx.data.len);

> +		return 0;

> +	case NAND_OP_WAITRDY_INSTR:

> +		return denali_exec_waitrdy(denali);

> +	default:

> +		WARN_ONCE(1, "unsupported NAND instruction type: %d\n",

> +			  instr->type);

> +

> +		return -EINVAL;

> +	}

> +}

> +



Thanks,
Miquèl
Masahiro Yamada March 5, 2019, 7:13 a.m. UTC | #2 
Hi Miquel,


On Mon, Mar 4, 2019 at 6:31 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>

> Hi Masahiro,

>

> Masahiro Yamada <yamada.masahiro@socionext.com> wrote on Tue, 12 Feb

> 2019 16:12:56 +0900:

>

> > Implement ->exec_op(), and remove the deprecated hooks.

> >

> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

> > ---

>

> Thanks for working on this, I like it very much!

>

> >

> > Changes in v2: None

> >

> >  drivers/mtd/nand/raw/denali.c | 234 +++++++++++++++++++++++-------------------

> >  1 file changed, 126 insertions(+), 108 deletions(-)

> >

> > diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c

> > index a2fe2ff..bd7df25 100644

> > --- a/drivers/mtd/nand/raw/denali.c

> > +++ b/drivers/mtd/nand/raw/denali.c

>

> [...]

>

> > +

> > +static int denali_exec_instr(struct nand_chip *chip,

> > +                          const struct nand_op_instr *instr)

> > +{

> > +     struct denali_nand_info *denali = to_denali(chip);

> > +     bool width16 = chip->options & NAND_BUSWIDTH_16;

> > +

> > +     switch (instr->type) {

> > +     case NAND_OP_CMD_INSTR:

> > +             denali_exec_out8(denali, DENALI_MAP11_CMD,

> > +                              &instr->ctx.cmd.opcode, 1);

> > +             return 0;

> > +     case NAND_OP_ADDR_INSTR:

> > +             denali_exec_out8(denali, DENALI_MAP11_ADDR,

> > +                              instr->ctx.addr.addrs,

> > +                              instr->ctx.addr.naddrs);

> > +             return 0;

> > +     case NAND_OP_DATA_IN_INSTR:

> > +             (!instr->ctx.data.force_8bit && width16 ?

> > +              denali_exec_in16 :

> > +              denali_exec_in8)(denali, DENALI_MAP11_DATA,

> > +                               instr->ctx.data.buf.in,

> > +                               instr->ctx.data.len);

>

> I think this is abusing the ternary operator, can you please find

> another way for writing this? Otherwise it is not easily readable... If

> it is really too complicated within 80 chars, then never mind.


Yes, I am abusing it,
but the intention is quite clear.


I could stupidly repeat the same function arguments like follows,
but I did not want to do it.

case NAND_OP_DATA_IN_INSTR:
        if (!instr->ctx.data.force_8bit && width16)
                denali_exec_in16(denali, DENALI_MAP11_DATA,
                                 instr->ctx.data.buf.in,
                                 instr->ctx.data.len);
        else
                denali_exec_in8(denali, DENALI_MAP11_DATA,
                                instr->ctx.data.buf.in,
                                instr->ctx.data.len);





> > +             return 0;

> > +     case NAND_OP_DATA_OUT_INSTR:

> > +             (!instr->ctx.data.force_8bit && width16 ?

> > +              denali_exec_out16 :

> > +              denali_exec_out8)(denali, DENALI_MAP11_DATA,

> > +                                instr->ctx.data.buf.out,

> > +                                instr->ctx.data.len);

> > +             return 0;

> > +     case NAND_OP_WAITRDY_INSTR:

> > +             return denali_exec_waitrdy(denali);

> > +     default:

> > +             WARN_ONCE(1, "unsupported NAND instruction type: %d\n",

> > +                       instr->type);

> > +

> > +             return -EINVAL;

> > +     }

> > +}

> > +

>

>

> Thanks,

> Miquèl

>

> ______________________________________________________

> Linux MTD discussion mailing list

> http://lists.infradead.org/mailman/listinfo/linux-mtd/




-- 
Best Regards
Masahiro Yamada
diff mbox series

Patch

diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c
index a2fe2ff..bd7df25 100644
--- a/drivers/mtd/nand/raw/denali.c
+++ b/drivers/mtd/nand/raw/denali.c
@@ -206,85 +206,11 @@  static uint32_t denali_wait_for_irq(struct denali_nand_info *denali,
 	return denali->irq_status;
 }
 
-static void denali_read_buf(struct nand_chip *chip, uint8_t *buf, int len)
+static void denali_select_target(struct nand_chip *chip, int cs)
 {
-	struct mtd_info *mtd = nand_to_mtd(chip);
-	struct denali_nand_info *denali = mtd_to_denali(mtd);
-	u32 addr = DENALI_MAP11_DATA | DENALI_BANK(denali);
-	int i;
-
-	for (i = 0; i < len; i++)
-		buf[i] = denali->host_read(denali, addr);
-}
-
-static void denali_write_buf(struct nand_chip *chip, const uint8_t *buf,
-			     int len)
-{
-	struct denali_nand_info *denali = mtd_to_denali(nand_to_mtd(chip));
-	u32 addr = DENALI_MAP11_DATA | DENALI_BANK(denali);
-	int i;
-
-	for (i = 0; i < len; i++)
-		denali->host_write(denali, addr, buf[i]);
-}
-
-static void denali_read_buf16(struct nand_chip *chip, uint8_t *buf, int len)
-{
-	struct denali_nand_info *denali = mtd_to_denali(nand_to_mtd(chip));
-	u32 addr = DENALI_MAP11_DATA | DENALI_BANK(denali);
-	uint16_t *buf16 = (uint16_t *)buf;
-	int i;
-
-	for (i = 0; i < len / 2; i++)
-		buf16[i] = denali->host_read(denali, addr);
-}
-
-static void denali_write_buf16(struct nand_chip *chip, const uint8_t *buf,
-			       int len)
-{
-	struct denali_nand_info *denali = mtd_to_denali(nand_to_mtd(chip));
-	u32 addr = DENALI_MAP11_DATA | DENALI_BANK(denali);
-	const uint16_t *buf16 = (const uint16_t *)buf;
-	int i;
-
-	for (i = 0; i < len / 2; i++)
-		denali->host_write(denali, addr, buf16[i]);
-}
-
-static uint8_t denali_read_byte(struct nand_chip *chip)
-{
-	uint8_t byte;
-
-	denali_read_buf(chip, &byte, 1);
-
-	return byte;
-}
-
-static void denali_write_byte(struct nand_chip *chip, uint8_t byte)
-{
-	denali_write_buf(chip, &byte, 1);
-}
-
-static void denali_cmd_ctrl(struct nand_chip *chip, int dat, unsigned int ctrl)
-{
-	struct denali_nand_info *denali = mtd_to_denali(nand_to_mtd(chip));
-	uint32_t type;
-
-	if (ctrl & NAND_CLE)
-		type = DENALI_MAP11_CMD;
-	else if (ctrl & NAND_ALE)
-		type = DENALI_MAP11_ADDR;
-	else
-		return;
-
-	/*
-	 * Some commands are followed by chip->legacy.waitfunc.
-	 * irq_status must be cleared here to catch the R/B# interrupt later.
-	 */
-	if (ctrl & NAND_CTRL_CHANGE)
-		denali_reset_irq(denali);
+	struct denali_nand_info *denali = to_denali(chip);
 
-	denali->host_write(denali, DENALI_BANK(denali) | type, dat);
+	denali->active_bank = cs;
 }
 
 static int denali_check_erased_page(struct nand_chip *chip,
@@ -596,6 +522,8 @@  static int denali_data_xfer(struct nand_chip *chip, void *buf, size_t size,
 {
 	struct denali_nand_info *denali = to_denali(chip);
 
+	denali_select_target(chip, chip->cur_cs);
+
 	iowrite32(raw ? 0 : ECC_ENABLE__FLAG, denali->reg + ECC_ENABLE);
 	iowrite32(raw ? TRANSFER_SPARE_REG__FLAG : 0,
 		  denali->reg + TRANSFER_SPARE_REG);
@@ -856,24 +784,6 @@  static int denali_write_page(struct nand_chip *chip, const uint8_t *buf,
 				0, 1);
 }
 
-static void denali_select_chip(struct nand_chip *chip, int cs)
-{
-	struct denali_nand_info *denali = mtd_to_denali(nand_to_mtd(chip));
-
-	denali->active_bank = cs;
-}
-
-static int denali_waitfunc(struct nand_chip *chip)
-{
-	struct denali_nand_info *denali = mtd_to_denali(nand_to_mtd(chip));
-	uint32_t irq_status;
-
-	/* R/B# pin transitioned from low to high? */
-	irq_status = denali_wait_for_irq(denali, INTR__INT_ACT);
-
-	return irq_status & INTR__INT_ACT ? 0 : NAND_STATUS_FAIL;
-}
-
 static int denali_setup_data_interface(struct nand_chip *chip, int chipnr,
 				       const struct nand_data_interface *conf)
 {
@@ -1185,13 +1095,6 @@  static int denali_attach_chip(struct nand_chip *chip)
 
 	mtd_set_ooblayout(mtd, &denali_ooblayout_ops);
 
-	if (chip->options & NAND_BUSWIDTH_16) {
-		chip->legacy.read_buf = denali_read_buf16;
-		chip->legacy.write_buf = denali_write_buf16;
-	} else {
-		chip->legacy.read_buf = denali_read_buf;
-		chip->legacy.write_buf = denali_write_buf;
-	}
 	chip->ecc.read_page = denali_read_page;
 	chip->ecc.read_page_raw = denali_read_page_raw;
 	chip->ecc.write_page = denali_write_page;
@@ -1223,9 +1126,130 @@  static void denali_detach_chip(struct nand_chip *chip)
 	kfree(denali->buf);
 }
 
+static void denali_exec_in8(struct denali_nand_info *denali, u32 type,
+			    u8 *buf, unsigned int len)
+{
+	int i;
+
+	for (i = 0; i < len; i++)
+		buf[i] = denali->host_read(denali, type | DENALI_BANK(denali));
+}
+
+static void denali_exec_in16(struct denali_nand_info *denali, u32 type,
+			     u8 *buf, unsigned int len)
+{
+	u32 data;
+	int i;
+
+	for (i = 0; i < len; i += 2) {
+		data = denali->host_read(denali, type | DENALI_BANK(denali));
+		/* bit 31:24 and 15:8 are used for DDR */
+		buf[i] = data >> 16;
+		buf[i + 1] = data;
+	}
+}
+
+static void denali_exec_out8(struct denali_nand_info *denali, u32 type,
+			     const u8 *buf, unsigned int len)
+{
+	int i;
+
+	for (i = 0; i < len; i++)
+		denali->host_write(denali, type | DENALI_BANK(denali), buf[i]);
+}
+
+static void denali_exec_out16(struct denali_nand_info *denali, u32 type,
+			      const u8 *buf, unsigned int len)
+{
+	int i;
+
+	for (i = 0; i < len; i += 2)
+		denali->host_write(denali, type | DENALI_BANK(denali),
+				   buf[i + 1] << 16 | buf[i]);
+}
+
+static int denali_exec_waitrdy(struct denali_nand_info *denali)
+{
+	u32 irq_stat;
+
+	/* R/B# pin transitioned from low to high? */
+	irq_stat = denali_wait_for_irq(denali, INTR__INT_ACT);
+
+	/* Just in case nand_operation has multiple NAND_OP_WAITRDY_INSTR. */
+	denali_reset_irq(denali);
+
+	return irq_stat & INTR__INT_ACT ? 0 : -EIO;
+}
+
+static int denali_exec_instr(struct nand_chip *chip,
+			     const struct nand_op_instr *instr)
+{
+	struct denali_nand_info *denali = to_denali(chip);
+	bool width16 = chip->options & NAND_BUSWIDTH_16;
+
+	switch (instr->type) {
+	case NAND_OP_CMD_INSTR:
+		denali_exec_out8(denali, DENALI_MAP11_CMD,
+				 &instr->ctx.cmd.opcode, 1);
+		return 0;
+	case NAND_OP_ADDR_INSTR:
+		denali_exec_out8(denali, DENALI_MAP11_ADDR,
+				 instr->ctx.addr.addrs,
+				 instr->ctx.addr.naddrs);
+		return 0;
+	case NAND_OP_DATA_IN_INSTR:
+		(!instr->ctx.data.force_8bit && width16 ?
+		 denali_exec_in16 :
+		 denali_exec_in8)(denali, DENALI_MAP11_DATA,
+				  instr->ctx.data.buf.in,
+				  instr->ctx.data.len);
+		return 0;
+	case NAND_OP_DATA_OUT_INSTR:
+		(!instr->ctx.data.force_8bit && width16 ?
+		 denali_exec_out16 :
+		 denali_exec_out8)(denali, DENALI_MAP11_DATA,
+				   instr->ctx.data.buf.out,
+				   instr->ctx.data.len);
+		return 0;
+	case NAND_OP_WAITRDY_INSTR:
+		return denali_exec_waitrdy(denali);
+	default:
+		WARN_ONCE(1, "unsupported NAND instruction type: %d\n",
+			  instr->type);
+
+		return -EINVAL;
+	}
+}
+
+static int denali_exec_op(struct nand_chip *chip,
+			  const struct nand_operation *op, bool check_only)
+{
+	int i, ret;
+
+	if (check_only)
+		return 0;
+
+	denali_select_target(chip, op->cs);
+
+	/*
+	 * Some commands contain NAND_OP_WAITRDY_INSTR.
+	 * irq must be cleared here to catch the R/B# interrupt there.
+	 */
+	denali_reset_irq(to_denali(chip));
+
+	for (i = 0; i < op->ninstrs; i++) {
+		ret = denali_exec_instr(chip, &op->instrs[i]);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 static const struct nand_controller_ops denali_controller_ops = {
 	.attach_chip = denali_attach_chip,
 	.detach_chip = denali_detach_chip,
+	.exec_op = denali_exec_op,
 	.setup_data_interface = denali_setup_data_interface,
 };
 
@@ -1260,12 +1284,6 @@  int denali_init(struct denali_nand_info *denali)
 	if (!mtd->name)
 		mtd->name = "denali-nand";
 
-	chip->legacy.select_chip = denali_select_chip;
-	chip->legacy.read_byte = denali_read_byte;
-	chip->legacy.write_byte = denali_write_byte;
-	chip->legacy.cmd_ctrl = denali_cmd_ctrl;
-	chip->legacy.waitfunc = denali_waitfunc;
-
 	if (features & FEATURES__INDEX_ADDR) {
 		denali->host_read = denali_indexed_read;
 		denali->host_write = denali_indexed_write;