| Message ID | 20200519232454.374081-4-boris.brezillon@collabora.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2,1/4] dt-bindings: mtd: rawnand: ingenic: Clarify the active state of the RB pin | expand |
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 >
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
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);
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(-)