2019-04-13 08:43:31

by Richard Weinberger

[permalink] [raw]
Subject: [PATCH 1/2] mtd: nandsim: Embed struct nand_chip in struct nandsim

We well need struct nand_controller soon, so more stuff need to
be parts of struct nandsim.
While we are here, rename "nand" to "ns" to use the same naming scheme
everywhere in nandsim.

Signed-off-by: Richard Weinberger <[email protected]>
---
drivers/mtd/nand/raw/nandsim.c | 49 +++++++++++++++++-----------------
1 file changed, 24 insertions(+), 25 deletions(-)

diff --git a/drivers/mtd/nand/raw/nandsim.c b/drivers/mtd/nand/raw/nandsim.c
index 933d1a629c51..3d80e2d23b6e 100644
--- a/drivers/mtd/nand/raw/nandsim.c
+++ b/drivers/mtd/nand/raw/nandsim.c
@@ -298,6 +298,7 @@ union ns_mem {
* The structure which describes all the internal simulator data.
*/
struct nandsim {
+ struct nand_chip chip;
struct mtd_partition partitions[CONFIG_NANDSIM_MAX_PARTS];
unsigned int nbparts;

@@ -2216,7 +2217,7 @@ static const struct nand_controller_ops ns_controller_ops = {
static int __init ns_init_module(void)
{
struct nand_chip *chip;
- struct nandsim *nand;
+ struct nandsim *ns;
int retval = -ENOMEM, i;

if (bus_width != 8 && bus_width != 16) {
@@ -2224,16 +2225,14 @@ static int __init ns_init_module(void)
return -EINVAL;
}

- /* Allocate and initialize mtd_info, nand_chip and nandsim structures */
- chip = kzalloc(sizeof(struct nand_chip) + sizeof(struct nandsim),
- GFP_KERNEL);
- if (!chip) {
+ ns = kzalloc(sizeof(struct nandsim), GFP_KERNEL);
+ if (!ns) {
NS_ERR("unable to allocate core structures.\n");
return -ENOMEM;
}
+ chip = &ns->chip;
nsmtd = nand_to_mtd(chip);
- nand = (struct nandsim *)(chip + 1);
- nand_set_controller_data(chip, (void *)nand);
+ nand_set_controller_data(chip, (void *)ns);

/*
* Register simulator's callbacks.
@@ -2266,19 +2265,19 @@ static int __init ns_init_module(void)
* the initial ID read command correctly
*/
if (id_bytes[6] != 0xFF || id_bytes[7] != 0xFF)
- nand->geom.idbytes = 8;
+ ns->geom.idbytes = 8;
else if (id_bytes[4] != 0xFF || id_bytes[5] != 0xFF)
- nand->geom.idbytes = 6;
+ ns->geom.idbytes = 6;
else if (id_bytes[2] != 0xFF || id_bytes[3] != 0xFF)
- nand->geom.idbytes = 4;
+ ns->geom.idbytes = 4;
else
- nand->geom.idbytes = 2;
- nand->regs.status = NS_STATUS_OK(nand);
- nand->nxstate = STATE_UNKNOWN;
- nand->options |= OPT_PAGE512; /* temporary value */
- memcpy(nand->ids, id_bytes, sizeof(nand->ids));
+ ns->geom.idbytes = 2;
+ ns->regs.status = NS_STATUS_OK(ns);
+ ns->nxstate = STATE_UNKNOWN;
+ ns->options |= OPT_PAGE512; /* temporary value */
+ memcpy(ns->ids, id_bytes, sizeof(ns->ids));
if (bus_width == 16) {
- nand->busw = 16;
+ ns->busw = 16;
chip->options |= NAND_BUSWIDTH_16;
}

@@ -2323,27 +2322,27 @@ static int __init ns_init_module(void)
if ((retval = nand_create_bbt(chip)) != 0)
goto err_exit;

- if ((retval = parse_badblocks(nand, nsmtd)) != 0)
+ if ((retval = parse_badblocks(ns, nsmtd)) != 0)
goto err_exit;

/* Register NAND partitions */
- retval = mtd_device_register(nsmtd, &nand->partitions[0],
- nand->nbparts);
+ retval = mtd_device_register(nsmtd, &ns->partitions[0],
+ ns->nbparts);
if (retval != 0)
goto err_exit;

- if ((retval = nandsim_debugfs_create(nand)) != 0)
+ if ((retval = nandsim_debugfs_create(ns)) != 0)
goto err_exit;

return 0;

err_exit:
- free_nandsim(nand);
+ free_nandsim(ns);
nand_release(chip);
- for (i = 0;i < ARRAY_SIZE(nand->partitions); ++i)
- kfree(nand->partitions[i].name);
+ for (i = 0;i < ARRAY_SIZE(ns->partitions); ++i)
+ kfree(ns->partitions[i].name);
error:
- kfree(chip);
+ kfree(ns);
free_lists();

return retval;
@@ -2364,7 +2363,7 @@ static void __exit ns_cleanup_module(void)
nand_release(chip); /* Unregister driver */
for (i = 0;i < ARRAY_SIZE(ns->partitions); ++i)
kfree(ns->partitions[i].name);
- kfree(mtd_to_nand(nsmtd)); /* Free other structures */
+ kfree(ns); /* Free other structures */
free_lists();
}

--
2.21.0


2019-04-13 08:42:07

by Richard Weinberger

[permalink] [raw]
Subject: [PATCH 2/2] mtd: nandsim: switch to exec_op interface

Stop using the legacy interface.

Signed-off-by: Richard Weinberger <[email protected]>
---
drivers/mtd/nand/raw/nandsim.c | 78 ++++++++++++++++++++--------------
1 file changed, 47 insertions(+), 31 deletions(-)

diff --git a/drivers/mtd/nand/raw/nandsim.c b/drivers/mtd/nand/raw/nandsim.c
index 3d80e2d23b6e..33c369f9a607 100644
--- a/drivers/mtd/nand/raw/nandsim.c
+++ b/drivers/mtd/nand/raw/nandsim.c
@@ -299,6 +299,7 @@ union ns_mem {
*/
struct nandsim {
struct nand_chip chip;
+ struct nand_controller base;
struct mtd_partition partitions[CONFIG_NANDSIM_MAX_PARTS];
unsigned int nbparts;

@@ -645,9 +646,6 @@ static int __init init_nandsim(struct mtd_info *mtd)
return -EIO;
}

- /* Force mtd to not do delays */
- chip->legacy.chip_delay = 0;
-
/* Initialize the NAND flash parameters */
ns->busw = chip->options & NAND_BUSWIDTH_16 ? 16 : 8;
ns->geom.totsz = mtd->size;
@@ -2077,24 +2075,6 @@ static void ns_nand_write_byte(struct nand_chip *chip, u_char byte)
return;
}

-static void ns_hwcontrol(struct nand_chip *chip, int cmd, unsigned int bitmask)
-{
- struct nandsim *ns = nand_get_controller_data(chip);
-
- ns->lines.cle = bitmask & NAND_CLE ? 1 : 0;
- ns->lines.ale = bitmask & NAND_ALE ? 1 : 0;
- ns->lines.ce = bitmask & NAND_NCE ? 1 : 0;
-
- if (cmd != NAND_CMD_NONE)
- ns_nand_write_byte(chip, cmd);
-}
-
-static int ns_device_ready(struct nand_chip *chip)
-{
- NS_DBG("device_ready\n");
- return 1;
-}
-
static void ns_nand_write_buf(struct nand_chip *chip, const u_char *buf,
int len)
{
@@ -2146,7 +2126,7 @@ static void ns_nand_read_buf(struct nand_chip *chip, u_char *buf, int len)
int i;

for (i = 0; i < len; i++)
- buf[i] = chip->legacy.read_byte(chip);
+ buf[i] = ns_nand_read_byte(chip);

return;
}
@@ -2169,6 +2149,46 @@ static void ns_nand_read_buf(struct nand_chip *chip, u_char *buf, int len)
return;
}

+static int ns_exec_op(struct nand_chip *chip, const struct nand_operation *op,
+ bool check_only)
+{
+ int i;
+ unsigned int op_id;
+ const struct nand_op_instr *instr = NULL;
+ struct nandsim *ns = nand_get_controller_data(chip);
+
+ ns->lines.ce = chip->cur_cs == -1 ? 0 : 1;
+
+ for (op_id = 0; op_id < op->ninstrs; op_id++) {
+ instr = &op->instrs[op_id];
+ ns->lines.cle = 0;
+ ns->lines.ale = 0;
+
+ switch (instr->type) {
+ case NAND_OP_CMD_INSTR:
+ ns->lines.cle = 1;
+ ns_nand_write_byte(chip, instr->ctx.cmd.opcode);
+ break;
+ case NAND_OP_ADDR_INSTR:
+ ns->lines.ale = 1;
+ for (i = 0; i < instr->ctx.addr.naddrs; i++)
+ ns_nand_write_byte(chip, instr->ctx.addr.addrs[i]);
+ break;
+ case NAND_OP_DATA_IN_INSTR:
+ ns_nand_read_buf(chip, instr->ctx.data.buf.in, instr->ctx.data.len);
+ break;
+ case NAND_OP_DATA_OUT_INSTR:
+ ns_nand_write_buf(chip, instr->ctx.data.buf.out, instr->ctx.data.len);
+ break;
+ case NAND_OP_WAITRDY_INSTR:
+ /* we are always ready */
+ break;
+ }
+ }
+
+ return 0;
+}
+
static int ns_attach_chip(struct nand_chip *chip)
{
unsigned int eccsteps, eccbytes;
@@ -2209,6 +2229,7 @@ static int ns_attach_chip(struct nand_chip *chip)

static const struct nand_controller_ops ns_controller_ops = {
.attach_chip = ns_attach_chip,
+ .exec_op = ns_exec_op,
};

/*
@@ -2234,14 +2255,6 @@ static int __init ns_init_module(void)
nsmtd = nand_to_mtd(chip);
nand_set_controller_data(chip, (void *)ns);

- /*
- * Register simulator's callbacks.
- */
- chip->legacy.cmd_ctrl = ns_hwcontrol;
- chip->legacy.read_byte = ns_nand_read_byte;
- chip->legacy.dev_ready = ns_device_ready;
- chip->legacy.write_buf = ns_nand_write_buf;
- chip->legacy.read_buf = ns_nand_read_buf;
chip->ecc.mode = NAND_ECC_SOFT;
chip->ecc.algo = NAND_ECC_HAMMING;
/* The NAND_SKIP_BBTSCAN option is necessary for 'overridesize' */
@@ -2292,7 +2305,10 @@ static int __init ns_init_module(void)
if ((retval = parse_gravepages()) != 0)
goto error;

- chip->legacy.dummy_controller.ops = &ns_controller_ops;
+ nand_controller_init(&ns->base);
+ ns->base.ops = &ns_controller_ops;
+ chip->controller = &ns->base;
+
retval = nand_scan(chip, 1);
if (retval) {
NS_ERR("Could not scan NAND Simulator device\n");
--
2.21.0

2019-04-13 08:52:48

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH 1/2] mtd: nandsim: Embed struct nand_chip in struct nandsim

Hello!

On 13.04.2019 11:40, Richard Weinberger wrote:

> We well need struct nand_controller soon, so more stuff need to

We will, maybe?

> be parts of struct nandsim.
> While we are here, rename "nand" to "ns" to use the same naming scheme
> everywhere in nandsim.
>
> Signed-off-by: Richard Weinberger <[email protected]>
[...]

MBR, Sergei

2019-04-14 08:37:32

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH 2/2] mtd: nandsim: switch to exec_op interface

On Sat, 13 Apr 2019 10:40:52 +0200
Richard Weinberger <[email protected]> wrote:

> Stop using the legacy interface.

Thanks for converting the nandsim driver.

>
> Signed-off-by: Richard Weinberger <[email protected]>
> ---
> drivers/mtd/nand/raw/nandsim.c | 78 ++++++++++++++++++++--------------
> 1 file changed, 47 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/nandsim.c b/drivers/mtd/nand/raw/nandsim.c
> index 3d80e2d23b6e..33c369f9a607 100644
> --- a/drivers/mtd/nand/raw/nandsim.c
> +++ b/drivers/mtd/nand/raw/nandsim.c
> @@ -299,6 +299,7 @@ union ns_mem {
> */
> struct nandsim {
> struct nand_chip chip;
> + struct nand_controller base;
> struct mtd_partition partitions[CONFIG_NANDSIM_MAX_PARTS];
> unsigned int nbparts;
>
> @@ -645,9 +646,6 @@ static int __init init_nandsim(struct mtd_info *mtd)
> return -EIO;
> }
>
> - /* Force mtd to not do delays */
> - chip->legacy.chip_delay = 0;
> -
> /* Initialize the NAND flash parameters */
> ns->busw = chip->options & NAND_BUSWIDTH_16 ? 16 : 8;
> ns->geom.totsz = mtd->size;
> @@ -2077,24 +2075,6 @@ static void ns_nand_write_byte(struct nand_chip *chip, u_char byte)
> return;
> }
>
> -static void ns_hwcontrol(struct nand_chip *chip, int cmd, unsigned int bitmask)
> -{
> - struct nandsim *ns = nand_get_controller_data(chip);
> -
> - ns->lines.cle = bitmask & NAND_CLE ? 1 : 0;
> - ns->lines.ale = bitmask & NAND_ALE ? 1 : 0;
> - ns->lines.ce = bitmask & NAND_NCE ? 1 : 0;
> -
> - if (cmd != NAND_CMD_NONE)
> - ns_nand_write_byte(chip, cmd);
> -}
> -
> -static int ns_device_ready(struct nand_chip *chip)
> -{
> - NS_DBG("device_ready\n");
> - return 1;
> -}
> -
> static void ns_nand_write_buf(struct nand_chip *chip, const u_char *buf,
> int len)
> {
> @@ -2146,7 +2126,7 @@ static void ns_nand_read_buf(struct nand_chip *chip, u_char *buf, int len)
> int i;
>
> for (i = 0; i < len; i++)
> - buf[i] = chip->legacy.read_byte(chip);
> + buf[i] = ns_nand_read_byte(chip);
>
> return;
> }
> @@ -2169,6 +2149,46 @@ static void ns_nand_read_buf(struct nand_chip *chip, u_char *buf, int len)
> return;
> }
>
> +static int ns_exec_op(struct nand_chip *chip, const struct nand_operation *op,
> + bool check_only)
> +{
> + int i;
> + unsigned int op_id;
> + const struct nand_op_instr *instr = NULL;
> + struct nandsim *ns = nand_get_controller_data(chip);
> +
> + ns->lines.ce = chip->cur_cs == -1 ? 0 : 1;

You can assume ns->lines.ce is always 1, since there's no point
executing a NAND operation if the CS pin is not asserted. BTW, in case
you ever consider supporting multi-CS chips, the CS line to select is
passed through op->cs (you should not use nand->cur_cs).

The rest of the patch looks good. Once this tiny detail is addressed you
can add

Reviewed-by: Boris Brezillon <[email protected]>

> +
> + for (op_id = 0; op_id < op->ninstrs; op_id++) {
> + instr = &op->instrs[op_id];
> + ns->lines.cle = 0;
> + ns->lines.ale = 0;
> +
> + switch (instr->type) {
> + case NAND_OP_CMD_INSTR:
> + ns->lines.cle = 1;
> + ns_nand_write_byte(chip, instr->ctx.cmd.opcode);
> + break;
> + case NAND_OP_ADDR_INSTR:
> + ns->lines.ale = 1;
> + for (i = 0; i < instr->ctx.addr.naddrs; i++)
> + ns_nand_write_byte(chip, instr->ctx.addr.addrs[i]);
> + break;
> + case NAND_OP_DATA_IN_INSTR:
> + ns_nand_read_buf(chip, instr->ctx.data.buf.in, instr->ctx.data.len);
> + break;
> + case NAND_OP_DATA_OUT_INSTR:
> + ns_nand_write_buf(chip, instr->ctx.data.buf.out, instr->ctx.data.len);
> + break;
> + case NAND_OP_WAITRDY_INSTR:
> + /* we are always ready */
> + break;
> + }
> + }
> +
> + return 0;
> +}
> +
> static int ns_attach_chip(struct nand_chip *chip)
> {
> unsigned int eccsteps, eccbytes;
> @@ -2209,6 +2229,7 @@ static int ns_attach_chip(struct nand_chip *chip)
>
> static const struct nand_controller_ops ns_controller_ops = {
> .attach_chip = ns_attach_chip,
> + .exec_op = ns_exec_op,
> };
>
> /*
> @@ -2234,14 +2255,6 @@ static int __init ns_init_module(void)
> nsmtd = nand_to_mtd(chip);
> nand_set_controller_data(chip, (void *)ns);
>
> - /*
> - * Register simulator's callbacks.
> - */
> - chip->legacy.cmd_ctrl = ns_hwcontrol;
> - chip->legacy.read_byte = ns_nand_read_byte;
> - chip->legacy.dev_ready = ns_device_ready;
> - chip->legacy.write_buf = ns_nand_write_buf;
> - chip->legacy.read_buf = ns_nand_read_buf;
> chip->ecc.mode = NAND_ECC_SOFT;
> chip->ecc.algo = NAND_ECC_HAMMING;
> /* The NAND_SKIP_BBTSCAN option is necessary for 'overridesize' */
> @@ -2292,7 +2305,10 @@ static int __init ns_init_module(void)
> if ((retval = parse_gravepages()) != 0)
> goto error;
>
> - chip->legacy.dummy_controller.ops = &ns_controller_ops;
> + nand_controller_init(&ns->base);
> + ns->base.ops = &ns_controller_ops;
> + chip->controller = &ns->base;
> +
> retval = nand_scan(chip, 1);
> if (retval) {
> NS_ERR("Could not scan NAND Simulator device\n");

2019-04-14 08:45:33

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH 1/2] mtd: nandsim: Embed struct nand_chip in struct nandsim

On Sat, 13 Apr 2019 10:40:51 +0200
Richard Weinberger <[email protected]> wrote:

> We well need struct nand_controller soon, so more stuff need to
> be parts of struct nandsim.
> While we are here, rename "nand" to "ns" to use the same naming scheme
> everywhere in nandsim.
>
> Signed-off-by: Richard Weinberger <[email protected]>

Reviewed-by: Boris Brezillon <[email protected]>

> ---
> drivers/mtd/nand/raw/nandsim.c | 49 +++++++++++++++++-----------------
> 1 file changed, 24 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/nandsim.c b/drivers/mtd/nand/raw/nandsim.c
> index 933d1a629c51..3d80e2d23b6e 100644
> --- a/drivers/mtd/nand/raw/nandsim.c
> +++ b/drivers/mtd/nand/raw/nandsim.c
> @@ -298,6 +298,7 @@ union ns_mem {
> * The structure which describes all the internal simulator data.
> */
> struct nandsim {
> + struct nand_chip chip;
> struct mtd_partition partitions[CONFIG_NANDSIM_MAX_PARTS];
> unsigned int nbparts;
>
> @@ -2216,7 +2217,7 @@ static const struct nand_controller_ops ns_controller_ops = {
> static int __init ns_init_module(void)
> {
> struct nand_chip *chip;
> - struct nandsim *nand;
> + struct nandsim *ns;
> int retval = -ENOMEM, i;
>
> if (bus_width != 8 && bus_width != 16) {
> @@ -2224,16 +2225,14 @@ static int __init ns_init_module(void)
> return -EINVAL;
> }
>
> - /* Allocate and initialize mtd_info, nand_chip and nandsim structures */
> - chip = kzalloc(sizeof(struct nand_chip) + sizeof(struct nandsim),
> - GFP_KERNEL);
> - if (!chip) {
> + ns = kzalloc(sizeof(struct nandsim), GFP_KERNEL);
> + if (!ns) {
> NS_ERR("unable to allocate core structures.\n");
> return -ENOMEM;
> }
> + chip = &ns->chip;
> nsmtd = nand_to_mtd(chip);
> - nand = (struct nandsim *)(chip + 1);
> - nand_set_controller_data(chip, (void *)nand);
> + nand_set_controller_data(chip, (void *)ns);
>
> /*
> * Register simulator's callbacks.
> @@ -2266,19 +2265,19 @@ static int __init ns_init_module(void)
> * the initial ID read command correctly
> */
> if (id_bytes[6] != 0xFF || id_bytes[7] != 0xFF)
> - nand->geom.idbytes = 8;
> + ns->geom.idbytes = 8;
> else if (id_bytes[4] != 0xFF || id_bytes[5] != 0xFF)
> - nand->geom.idbytes = 6;
> + ns->geom.idbytes = 6;
> else if (id_bytes[2] != 0xFF || id_bytes[3] != 0xFF)
> - nand->geom.idbytes = 4;
> + ns->geom.idbytes = 4;
> else
> - nand->geom.idbytes = 2;
> - nand->regs.status = NS_STATUS_OK(nand);
> - nand->nxstate = STATE_UNKNOWN;
> - nand->options |= OPT_PAGE512; /* temporary value */
> - memcpy(nand->ids, id_bytes, sizeof(nand->ids));
> + ns->geom.idbytes = 2;
> + ns->regs.status = NS_STATUS_OK(ns);
> + ns->nxstate = STATE_UNKNOWN;
> + ns->options |= OPT_PAGE512; /* temporary value */
> + memcpy(ns->ids, id_bytes, sizeof(ns->ids));
> if (bus_width == 16) {
> - nand->busw = 16;
> + ns->busw = 16;
> chip->options |= NAND_BUSWIDTH_16;
> }
>
> @@ -2323,27 +2322,27 @@ static int __init ns_init_module(void)
> if ((retval = nand_create_bbt(chip)) != 0)
> goto err_exit;
>
> - if ((retval = parse_badblocks(nand, nsmtd)) != 0)
> + if ((retval = parse_badblocks(ns, nsmtd)) != 0)
> goto err_exit;
>
> /* Register NAND partitions */
> - retval = mtd_device_register(nsmtd, &nand->partitions[0],
> - nand->nbparts);
> + retval = mtd_device_register(nsmtd, &ns->partitions[0],
> + ns->nbparts);
> if (retval != 0)
> goto err_exit;
>
> - if ((retval = nandsim_debugfs_create(nand)) != 0)
> + if ((retval = nandsim_debugfs_create(ns)) != 0)
> goto err_exit;
>
> return 0;
>
> err_exit:
> - free_nandsim(nand);
> + free_nandsim(ns);
> nand_release(chip);
> - for (i = 0;i < ARRAY_SIZE(nand->partitions); ++i)
> - kfree(nand->partitions[i].name);
> + for (i = 0;i < ARRAY_SIZE(ns->partitions); ++i)
> + kfree(ns->partitions[i].name);
> error:
> - kfree(chip);
> + kfree(ns);
> free_lists();
>
> return retval;
> @@ -2364,7 +2363,7 @@ static void __exit ns_cleanup_module(void)
> nand_release(chip); /* Unregister driver */
> for (i = 0;i < ARRAY_SIZE(ns->partitions); ++i)
> kfree(ns->partitions[i].name);
> - kfree(mtd_to_nand(nsmtd)); /* Free other structures */
> + kfree(ns); /* Free other structures */
> free_lists();
> }
>