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

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

[Richard Weinberger]

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 <richard@nod.at>
---
 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

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

[Richard Weinberger]

Stop using the legacy interface.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 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

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

[Sergei Shtylyov]

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 <richard@nod.at>
[...]

MBR, Sergei

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

[Boris Brezillon]

On Sat, 13 Apr 2019 10:40:52 +0200
Richard Weinberger <richard@nod.at> wrote:

> Stop using the legacy interface.

Thanks for converting the nandsim driver.

> 
> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
>  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 <boris.brezillon@collabora.com>

> +
> +	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");

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

[Boris Brezillon]

On Sat, 13 Apr 2019 10:40:51 +0200
Richard Weinberger <richard@nod.at> 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 <richard@nod.at>

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

> ---
>  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();
>  }
>