linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] mtd: migrate 'of_node' handling to core, not in mtd_part_parser_data
@ 2015-10-27  2:31 Brian Norris
  2015-10-27  2:31 ` [PATCH 1/5] mtd: ofpart: grab device tree node directly from master device node Brian Norris
                   ` (4 more replies)
  0 siblings, 5 replies; 24+ messages in thread
From: Brian Norris @ 2015-10-27  2:31 UTC (permalink / raw)
  To: linux-mtd
  Cc: linux-kernel, Brian Norris, Boris Brezillon, Ezequiel Garcia,
	Marek Vasut, Scott Wood, Josh Wu, Robert Jarzmik, Kyungmin Park,
	Han Xu, Huang Shijie

Hi,

I noticed that, for MTD drivers that want to support DT partition parsing
(essentially all recent ones), it's a bit awkward to set the tell MTD which DT
node to use. I also noticed that this info is duplicated in a few places;
namely, some sub-subsystems (like SPI NOR and NAND) need their own field to
track the DT node, so let's have the propagate the 'MTD' DT node down for us.

Besides simplifying the boilerplate throughout MTD, this series also has the
side effect of producing 'of_node' symlinks in sysfs. e.g.:

  # ls -al /sys/class/mtd/mtd0/of_node 
  lrwxrwxrwx 1 root root 0 Oct 26 19:17 /sys/class/mtd/mtd0/of_node -> ../../../../firmware/devicetree/base/spi@ff130000/flash@0

For NAND, this potentially has some other bigger initial side effects, since
nand_dt_init() handles a bit more automagically for drivers that defined their
->flash_node. But these drivers should probably convert anyway. So, please test
your favorite driver!

Thanks,
Brian

Brian Norris (5):
  mtd: ofpart: grab device tree node directly from master device node
  mtd: nand: drop unnecessary partition parser data
  mtd: spi-nor: drop unnecessary partition parser data
  mtd: drop unnecessary partition parser data
  mtd: drop 'of_node' partition parser data

 drivers/mtd/devices/m25p80.c                  |  8 ++------
 drivers/mtd/devices/mtd_dataflash.c           |  5 ++---
 drivers/mtd/devices/spear_smi.c               |  6 ++----
 drivers/mtd/devices/st_spi_fsm.c              |  5 ++---
 drivers/mtd/maps/lantiq-flash.c               |  5 ++---
 drivers/mtd/maps/physmap_of.c                 |  5 ++---
 drivers/mtd/nand/atmel_nand.c                 |  7 +++----
 drivers/mtd/nand/brcmnand/brcmnand.c          |  3 +--
 drivers/mtd/nand/davinci_nand.c               | 10 +++-------
 drivers/mtd/nand/fsl_elbc_nand.c              |  5 ++---
 drivers/mtd/nand/fsl_ifc_nand.c               |  5 ++---
 drivers/mtd/nand/fsl_upm.c                    |  5 ++---
 drivers/mtd/nand/fsmc_nand.c                  |  7 +++----
 drivers/mtd/nand/gpio.c                       |  8 +++-----
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c        |  5 ++---
 drivers/mtd/nand/hisi504_nand.c               |  5 ++---
 drivers/mtd/nand/lpc32xx_mlc.c                |  7 +++----
 drivers/mtd/nand/lpc32xx_slc.c                |  7 +++----
 drivers/mtd/nand/mpc5121_nfc.c                |  5 ++---
 drivers/mtd/nand/mxc_nand.c                   |  5 ++---
 drivers/mtd/nand/nand_base.c                  |  3 +++
 drivers/mtd/nand/ndfc.c                       |  5 ++---
 drivers/mtd/nand/omap2.c                      |  6 ++----
 drivers/mtd/nand/orion_nand.c                 |  6 ++----
 drivers/mtd/nand/plat_nand.c                  |  5 ++---
 drivers/mtd/nand/pxa3xx_nand.c                | 10 +++++-----
 drivers/mtd/nand/sh_flctl.c                   |  6 ++----
 drivers/mtd/nand/socrates_nand.c              |  5 ++---
 drivers/mtd/nand/sunxi_nand.c                 |  4 +---
 drivers/mtd/nand/vf610_nfc.c                  |  6 +-----
 drivers/mtd/ofpart.c                          | 12 ++++--------
 drivers/mtd/onenand/omap2.c                   |  8 +++-----
 drivers/mtd/spi-nor/fsl-quadspi.c             |  4 +---
 drivers/mtd/spi-nor/nxp-spifi.c               |  4 +---
 drivers/mtd/spi-nor/spi-nor.c                 |  1 +
 drivers/staging/mt29f_spinand/mt29f_spinand.c |  5 ++---
 include/linux/mtd/partitions.h                |  2 --
 37 files changed, 79 insertions(+), 131 deletions(-)

-- 
2.6.0.rc2.230.g3dd15c0


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 1/5] mtd: ofpart: grab device tree node directly from master device node
  2015-10-27  2:31 [PATCH 0/5] mtd: migrate 'of_node' handling to core, not in mtd_part_parser_data Brian Norris
@ 2015-10-27  2:31 ` Brian Norris
  2015-10-27  7:42   ` Boris Brezillon
  2015-10-27  2:31 ` [PATCH 2/5] mtd: nand: drop unnecessary partition parser data Brian Norris
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 24+ messages in thread
From: Brian Norris @ 2015-10-27  2:31 UTC (permalink / raw)
  To: linux-mtd
  Cc: linux-kernel, Brian Norris, Boris Brezillon, Ezequiel Garcia,
	Marek Vasut, Scott Wood, Josh Wu, Robert Jarzmik, Kyungmin Park,
	Han Xu, Huang Shijie

It seems more logical to use a device node directly associated with the
MTD master device (i.e., mtd->dev.of_node field) rather than requiring
auxiliary partition parser information to be passed in by the driver in
a separate struct.

This patch supports the mtd->dev.of_node field, deprecates the parser
data 'of_node' field, and begins using the new convention for nand_base.
Other NAND driver conversions may now follow.

Additional side benefit to assigning mtd->dev.of_node rather than using
parser data: the driver core will automatically create a device -> node
symlink for us.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/nand/nand_base.c   |  3 +++
 drivers/mtd/ofpart.c           | 18 ++++++++++--------
 drivers/mtd/spi-nor/spi-nor.c  |  1 +
 include/linux/mtd/partitions.h |  4 +++-
 4 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index cc74142938b0..d2e7fee2ac37 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3990,6 +3990,9 @@ int nand_scan_ident(struct mtd_info *mtd, int maxchips,
 	int ret;
 
 	if (chip->flash_node) {
+		/* MTD can automatically handle DT partitions, etc. */
+		mtd->dev.of_node = chip->flash_node;
+
 		ret = nand_dt_init(mtd, chip, chip->flash_node);
 		if (ret)
 			return ret;
diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/ofpart.c
index aa26c32e1bc2..a5dfd73cfc7d 100644
--- a/drivers/mtd/ofpart.c
+++ b/drivers/mtd/ofpart.c
@@ -35,10 +35,11 @@ static int parse_ofpart_partitions(struct mtd_info *master,
 	int nr_parts, i;
 
 
-	if (!data)
-		return 0;
-
-	node = data->of_node;
+	/*
+	 * of_node can be provided through auxiliary parser data or (preferred)
+	 * by assigning the master device
+	 */
+	node = data && data->of_node ? data->of_node : master->dev.of_node;
 	if (!node)
 		return 0;
 
@@ -120,10 +121,11 @@ static int parse_ofoldpart_partitions(struct mtd_info *master,
 	} *part;
 	const char *names;
 
-	if (!data)
-		return 0;
-
-	dp = data->of_node;
+	/*
+	 * of_node can be provided through auxiliary parser data or (preferred)
+	 * by assigning the master device
+	 */
+	dp = data && data->of_node ? data->of_node : master->dev.of_node;
 	if (!dp)
 		return 0;
 
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 49883905a434..8f9080c6db63 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1258,6 +1258,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 		mtd->flags |= MTD_NO_ERASE;
 
 	mtd->dev.parent = dev;
+	mtd->dev.of_node = np;
 	nor->page_size = info->page_size;
 	mtd->writebufsize = nor->page_size;
 
diff --git a/include/linux/mtd/partitions.h b/include/linux/mtd/partitions.h
index 6a35e6de5da1..e742f34b67eb 100644
--- a/include/linux/mtd/partitions.h
+++ b/include/linux/mtd/partitions.h
@@ -56,7 +56,9 @@ struct device_node;
 /**
  * struct mtd_part_parser_data - used to pass data to MTD partition parsers.
  * @origin: for RedBoot, start address of MTD device
- * @of_node: for OF parsers, device node containing partitioning information
+ * @of_node: for OF parsers, device node containing partitioning information.
+ *           This field is deprecated, as the device node should simply be
+ *           assigned to the master struct device.
  */
 struct mtd_part_parser_data {
 	unsigned long origin;
-- 
2.6.0.rc2.230.g3dd15c0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 2/5] mtd: nand: drop unnecessary partition parser data
  2015-10-27  2:31 [PATCH 0/5] mtd: migrate 'of_node' handling to core, not in mtd_part_parser_data Brian Norris
  2015-10-27  2:31 ` [PATCH 1/5] mtd: ofpart: grab device tree node directly from master device node Brian Norris
@ 2015-10-27  2:31 ` Brian Norris
  2015-10-27  7:52   ` Boris Brezillon
  2015-10-28  0:45   ` Brian Norris
  2015-10-27  2:31 ` [PATCH 3/5] mtd: spi-nor: " Brian Norris
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 24+ messages in thread
From: Brian Norris @ 2015-10-27  2:31 UTC (permalink / raw)
  To: linux-mtd
  Cc: linux-kernel, Brian Norris, Boris Brezillon, Ezequiel Garcia,
	Marek Vasut, Scott Wood, Josh Wu, Robert Jarzmik, Kyungmin Park,
	Han Xu, Huang Shijie

All of these drivers set up a parser data struct just to communicate DT
partition data. This field has been deprecated and is instead supported
by telling nand_scan_ident() about the 'flash_node'.

This patch:
 * sets chip->flash_node for those drivers that didn't already (but used
   OF partitioning)
 * drops the parser data
 * switches to the simpler mtd_device_register() where possible, now
   that we've eliminated one of the auxiliary parameters

Now that we've assigned chip->flash_node for these drivers, we can
probably rely on nand_dt_init() to do more of the DT parsing for us, but
for now, I don't want to fiddle with each of these drivers. The parsing
is done in duplicate for now on some drivers. I don't think this should
break things. (Famous last words.)

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/nand/atmel_nand.c                 |  7 +++----
 drivers/mtd/nand/brcmnand/brcmnand.c          |  3 +--
 drivers/mtd/nand/davinci_nand.c               | 10 +++-------
 drivers/mtd/nand/fsl_elbc_nand.c              |  5 ++---
 drivers/mtd/nand/fsl_ifc_nand.c               |  5 ++---
 drivers/mtd/nand/fsl_upm.c                    |  5 ++---
 drivers/mtd/nand/fsmc_nand.c                  |  7 +++----
 drivers/mtd/nand/gpio.c                       |  8 +++-----
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c        |  5 ++---
 drivers/mtd/nand/hisi504_nand.c               |  5 ++---
 drivers/mtd/nand/lpc32xx_mlc.c                |  7 +++----
 drivers/mtd/nand/lpc32xx_slc.c                |  7 +++----
 drivers/mtd/nand/mpc5121_nfc.c                |  5 ++---
 drivers/mtd/nand/mxc_nand.c                   |  5 ++---
 drivers/mtd/nand/ndfc.c                       |  5 ++---
 drivers/mtd/nand/omap2.c                      |  6 ++----
 drivers/mtd/nand/orion_nand.c                 |  6 ++----
 drivers/mtd/nand/plat_nand.c                  |  5 ++---
 drivers/mtd/nand/pxa3xx_nand.c                | 10 +++++-----
 drivers/mtd/nand/sh_flctl.c                   |  6 ++----
 drivers/mtd/nand/socrates_nand.c              |  5 ++---
 drivers/mtd/nand/sunxi_nand.c                 |  4 +---
 drivers/mtd/nand/vf610_nfc.c                  |  6 +-----
 drivers/staging/mt29f_spinand/mt29f_spinand.c |  5 ++---
 24 files changed, 54 insertions(+), 88 deletions(-)

diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index 583cdd9bb971..dd157b0d8041 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -2093,7 +2093,6 @@ static int atmel_nand_probe(struct platform_device *pdev)
 	struct mtd_info *mtd;
 	struct nand_chip *nand_chip;
 	struct resource *mem;
-	struct mtd_part_parser_data ppdata = {};
 	int res, irq;
 
 	/* Allocate memory for the device structure (and zero it) */
@@ -2117,6 +2116,7 @@ static int atmel_nand_probe(struct platform_device *pdev)
 	nand_chip = &host->nand_chip;
 	host->dev = &pdev->dev;
 	if (IS_ENABLED(CONFIG_OF) && pdev->dev.of_node) {
+		mtd->dev.of_node = pdev->dev.of_node;
 		/* Only when CONFIG_OF is enabled of_node can be parsed */
 		res = atmel_of_init_port(host, pdev->dev.of_node);
 		if (res)
@@ -2259,9 +2259,8 @@ static int atmel_nand_probe(struct platform_device *pdev)
 	}
 
 	mtd->name = "atmel_nand";
-	ppdata.of_node = pdev->dev.of_node;
-	res = mtd_device_parse_register(mtd, NULL, &ppdata,
-			host->board.parts, host->board.num_parts);
+	res = mtd_device_register(mtd, host->board.parts,
+				  host->board.num_parts);
 	if (!res)
 		return res;
 
diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c b/drivers/mtd/nand/brcmnand/brcmnand.c
index 7c1c306650a4..961a9ee4369c 100644
--- a/drivers/mtd/nand/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/brcmnand/brcmnand.c
@@ -1889,7 +1889,6 @@ static int brcmnand_init_cs(struct brcmnand_host *host)
 	struct mtd_info *mtd;
 	struct nand_chip *chip;
 	int ret;
-	struct mtd_part_parser_data ppdata = { .of_node = dn };
 
 	ret = of_property_read_u32(dn, "reg", &host->cs);
 	if (ret) {
@@ -1959,7 +1958,7 @@ static int brcmnand_init_cs(struct brcmnand_host *host)
 	if (nand_scan_tail(mtd))
 		return -ENXIO;
 
-	return mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0);
+	return mtd_device_register(mtd, NULL, 0);
 }
 
 static void brcmnand_save_restore_cs_config(struct brcmnand_host *host,
diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
index c72313d66cf6..a8448d73f56c 100644
--- a/drivers/mtd/nand/davinci_nand.c
+++ b/drivers/mtd/nand/davinci_nand.c
@@ -684,6 +684,7 @@ static int nand_davinci_probe(struct platform_device *pdev)
 
 	info->mtd.priv		= &info->chip;
 	info->mtd.dev.parent	= &pdev->dev;
+	info->mtd.dev.of_node	= pdev->dev.of_node;
 
 	info->chip.IO_ADDR_R	= vaddr;
 	info->chip.IO_ADDR_W	= vaddr;
@@ -839,13 +840,8 @@ syndrome_done:
 	if (pdata->parts)
 		ret = mtd_device_parse_register(&info->mtd, NULL, NULL,
 					pdata->parts, pdata->nr_parts);
-	else {
-		struct mtd_part_parser_data	ppdata;
-
-		ppdata.of_node = pdev->dev.of_node;
-		ret = mtd_device_parse_register(&info->mtd, NULL, &ppdata,
-						NULL, 0);
-	}
+	else
+		ret = mtd_device_register(&info->mtd, NULL, 0);
 	if (ret < 0)
 		goto err;
 
diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
index dcb1f7f4873f..f3d6e83e193b 100644
--- a/drivers/mtd/nand/fsl_elbc_nand.c
+++ b/drivers/mtd/nand/fsl_elbc_nand.c
@@ -748,6 +748,7 @@ static int fsl_elbc_chip_init(struct fsl_elbc_mtd *priv)
 	/* Fill in fsl_elbc_mtd structure */
 	priv->mtd.priv = chip;
 	priv->mtd.dev.parent = priv->dev;
+	chip->flash_node = priv->dev.of_node;
 
 	/* set timeout to maximum */
 	priv->fmr = 15 << FMR_CWTO_SHIFT;
@@ -823,9 +824,7 @@ static int fsl_elbc_nand_probe(struct platform_device *pdev)
 	int bank;
 	struct device *dev;
 	struct device_node *node = pdev->dev.of_node;
-	struct mtd_part_parser_data ppdata;
 
-	ppdata.of_node = pdev->dev.of_node;
 	if (!fsl_lbc_ctrl_dev || !fsl_lbc_ctrl_dev->regs)
 		return -ENODEV;
 	lbc = fsl_lbc_ctrl_dev->regs;
@@ -911,7 +910,7 @@ static int fsl_elbc_nand_probe(struct platform_device *pdev)
 
 	/* First look for RedBoot table or partitions on the command
 	 * line, these take precedence over device tree information */
-	mtd_device_parse_register(&priv->mtd, part_probe_types, &ppdata,
+	mtd_device_parse_register(&priv->mtd, part_probe_types, NULL,
 				  NULL, 0);
 
 	printk(KERN_INFO "eLBC NAND device at 0x%llx, bank %d\n",
diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c
index 7f4ac8c19001..60978b737cac 100644
--- a/drivers/mtd/nand/fsl_ifc_nand.c
+++ b/drivers/mtd/nand/fsl_ifc_nand.c
@@ -883,6 +883,7 @@ static int fsl_ifc_chip_init(struct fsl_ifc_mtd *priv)
 	/* Fill in fsl_ifc_mtd structure */
 	priv->mtd.priv = chip;
 	priv->mtd.dev.parent = priv->dev;
+	chip->flash_node = priv->dev.of_node;
 
 	/* fill in nand_chip structure */
 	/* set up function call table */
@@ -1030,9 +1031,7 @@ static int fsl_ifc_nand_probe(struct platform_device *dev)
 	int ret;
 	int bank;
 	struct device_node *node = dev->dev.of_node;
-	struct mtd_part_parser_data ppdata;
 
-	ppdata.of_node = dev->dev.of_node;
 	if (!fsl_ifc_ctrl_dev || !fsl_ifc_ctrl_dev->regs)
 		return -ENODEV;
 	ifc = fsl_ifc_ctrl_dev->regs;
@@ -1128,7 +1127,7 @@ static int fsl_ifc_nand_probe(struct platform_device *dev)
 
 	/* First look for RedBoot table or partitions on the command
 	 * line, these take precedence over device tree information */
-	mtd_device_parse_register(&priv->mtd, part_probe_types, &ppdata,
+	mtd_device_parse_register(&priv->mtd, part_probe_types, NULL,
 						NULL, 0);
 
 	dev_info(priv->dev, "IFC NAND device at 0x%llx, bank %d\n",
diff --git a/drivers/mtd/nand/fsl_upm.c b/drivers/mtd/nand/fsl_upm.c
index d326369980c4..ffe416c08914 100644
--- a/drivers/mtd/nand/fsl_upm.c
+++ b/drivers/mtd/nand/fsl_upm.c
@@ -159,7 +159,6 @@ static int fun_chip_init(struct fsl_upm_nand *fun,
 {
 	int ret;
 	struct device_node *flash_np;
-	struct mtd_part_parser_data ppdata;
 
 	fun->chip.IO_ADDR_R = fun->io_base;
 	fun->chip.IO_ADDR_W = fun->io_base;
@@ -182,6 +181,7 @@ static int fun_chip_init(struct fsl_upm_nand *fun,
 	if (!flash_np)
 		return -ENODEV;
 
+	fun->chip.flash_node = flash_np;
 	fun->mtd.name = kasprintf(GFP_KERNEL, "0x%llx.%s", (u64)io_res->start,
 				  flash_np->name);
 	if (!fun->mtd.name) {
@@ -193,8 +193,7 @@ static int fun_chip_init(struct fsl_upm_nand *fun,
 	if (ret)
 		goto err;
 
-	ppdata.of_node = flash_np;
-	ret = mtd_device_parse_register(&fun->mtd, NULL, &ppdata, NULL, 0);
+	ret = mtd_device_register(&fun->mtd, NULL, 0);
 err:
 	of_node_put(flash_np);
 	if (ret)
diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c
index 07af3dc7a4d2..23f34d6aa242 100644
--- a/drivers/mtd/nand/fsmc_nand.c
+++ b/drivers/mtd/nand/fsmc_nand.c
@@ -926,7 +926,6 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
 {
 	struct fsmc_nand_platform_data *pdata = dev_get_platdata(&pdev->dev);
 	struct device_node __maybe_unused *np = pdev->dev.of_node;
-	struct mtd_part_parser_data ppdata = {};
 	struct fsmc_nand_data *host;
 	struct mtd_info *mtd;
 	struct nand_chip *nand;
@@ -1016,6 +1015,7 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
 	nand = &host->nand;
 	mtd->priv = nand;
 	nand->priv = host;
+	nand->flash_node = np;
 
 	host->mtd.dev.parent = &pdev->dev;
 	nand->IO_ADDR_R = host->data_va;
@@ -1175,9 +1175,8 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
 	 * Check for partition info passed
 	 */
 	host->mtd.name = "nand";
-	ppdata.of_node = np;
-	ret = mtd_device_parse_register(&host->mtd, NULL, &ppdata,
-					host->partitions, host->nr_partitions);
+	ret = mtd_device_register(&host->mtd, host->partitions,
+				  host->nr_partitions);
 	if (ret)
 		goto err_probe;
 
diff --git a/drivers/mtd/nand/gpio.c b/drivers/mtd/nand/gpio.c
index 9ab97f934c37..289a48c2ca50 100644
--- a/drivers/mtd/nand/gpio.c
+++ b/drivers/mtd/nand/gpio.c
@@ -209,7 +209,6 @@ static int gpio_nand_probe(struct platform_device *pdev)
 	struct gpiomtd *gpiomtd;
 	struct nand_chip *chip;
 	struct resource *res;
-	struct mtd_part_parser_data ppdata = {};
 	int ret = 0;
 
 	if (!pdev->dev.of_node && !dev_get_platdata(&pdev->dev))
@@ -268,6 +267,7 @@ static int gpio_nand_probe(struct platform_device *pdev)
 		chip->dev_ready = gpio_nand_devready;
 	}
 
+	chip->flash_node	= pdev->dev.of_node;
 	chip->IO_ADDR_W		= chip->IO_ADDR_R;
 	chip->ecc.mode		= NAND_ECC_SOFT;
 	chip->options		= gpiomtd->plat.options;
@@ -291,10 +291,8 @@ static int gpio_nand_probe(struct platform_device *pdev)
 		gpiomtd->plat.adjust_parts(&gpiomtd->plat,
 					   gpiomtd->mtd_info.size);
 
-	ppdata.of_node = pdev->dev.of_node;
-	ret = mtd_device_parse_register(&gpiomtd->mtd_info, NULL, &ppdata,
-					gpiomtd->plat.parts,
-					gpiomtd->plat.num_parts);
+	ret = mtd_device_register(&gpiomtd->mtd_info, gpiomtd->plat.parts,
+				  gpiomtd->plat.num_parts);
 	if (!ret)
 		return 0;
 
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 2064adac1d17..90e49ccade45 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -1888,7 +1888,6 @@ static int gpmi_nand_init(struct gpmi_nand_data *this)
 {
 	struct mtd_info  *mtd = &this->mtd;
 	struct nand_chip *chip = &this->nand;
-	struct mtd_part_parser_data ppdata = {};
 	int ret;
 
 	/* init current chip */
@@ -1901,6 +1900,7 @@ static int gpmi_nand_init(struct gpmi_nand_data *this)
 
 	/* init the nand_chip{}, we don't support a 16-bit NAND Flash bus. */
 	chip->priv		= this;
+	chip->flash_node	= this->pdev->dev.of_node;
 	chip->select_chip	= gpmi_select_chip;
 	chip->cmd_ctrl		= gpmi_cmd_ctrl;
 	chip->dev_ready		= gpmi_dev_ready;
@@ -1954,8 +1954,7 @@ static int gpmi_nand_init(struct gpmi_nand_data *this)
 	if (ret)
 		goto err_out;
 
-	ppdata.of_node = this->pdev->dev.of_node;
-	ret = mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0);
+	ret = mtd_device_register(mtd, NULL, 0);
 	if (ret)
 		goto err_out;
 	return 0;
diff --git a/drivers/mtd/nand/hisi504_nand.c b/drivers/mtd/nand/hisi504_nand.c
index 0cb2e886937d..7c677eb5a3a9 100644
--- a/drivers/mtd/nand/hisi504_nand.c
+++ b/drivers/mtd/nand/hisi504_nand.c
@@ -704,7 +704,6 @@ static int hisi_nfc_probe(struct platform_device *pdev)
 	struct mtd_info   *mtd;
 	struct resource	  *res;
 	struct device_node *np = dev->of_node;
-	struct mtd_part_parser_data ppdata;
 
 	host = devm_kzalloc(dev, sizeof(*host), GFP_KERNEL);
 	if (!host)
@@ -742,6 +741,7 @@ static int hisi_nfc_probe(struct platform_device *pdev)
 	mtd->dev.parent         = &pdev->dev;
 
 	chip->priv		= host;
+	chip->flash_node	= np;
 	chip->cmdfunc		= hisi_nfc_cmdfunc;
 	chip->select_chip	= hisi_nfc_select_chip;
 	chip->read_byte		= hisi_nfc_read_byte;
@@ -805,8 +805,7 @@ static int hisi_nfc_probe(struct platform_device *pdev)
 		goto err_res;
 	}
 
-	ppdata.of_node = np;
-	ret = mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0);
+	ret = mtd_device_register(mtd, NULL, 0);
 	if (ret) {
 		dev_err(dev, "Err MTD partition=%d\n", ret);
 		goto err_mtd;
diff --git a/drivers/mtd/nand/lpc32xx_mlc.c b/drivers/mtd/nand/lpc32xx_mlc.c
index 347510978484..3ccad8ce70fa 100644
--- a/drivers/mtd/nand/lpc32xx_mlc.c
+++ b/drivers/mtd/nand/lpc32xx_mlc.c
@@ -647,7 +647,6 @@ static int lpc32xx_nand_probe(struct platform_device *pdev)
 	struct nand_chip *nand_chip;
 	struct resource *rc;
 	int res;
-	struct mtd_part_parser_data ppdata = {};
 
 	/* Allocate memory for the device structure (and zero it) */
 	host = devm_kzalloc(&pdev->dev, sizeof(*host), GFP_KERNEL);
@@ -682,6 +681,7 @@ static int lpc32xx_nand_probe(struct platform_device *pdev)
 	host->pdata = dev_get_platdata(&pdev->dev);
 
 	nand_chip->priv = host;		/* link the private data structures */
+	nand_chip->flash_node = pdev->dev.of_node;
 	mtd->priv = nand_chip;
 	mtd->dev.parent = &pdev->dev;
 
@@ -786,9 +786,8 @@ static int lpc32xx_nand_probe(struct platform_device *pdev)
 
 	mtd->name = DRV_NAME;
 
-	ppdata.of_node = pdev->dev.of_node;
-	res = mtd_device_parse_register(mtd, NULL, &ppdata, host->ncfg->parts,
-					host->ncfg->num_parts);
+	res = mtd_device_register(mtd, host->ncfg->parts,
+				  host->ncfg->num_parts);
 	if (!res)
 		return res;
 
diff --git a/drivers/mtd/nand/lpc32xx_slc.c b/drivers/mtd/nand/lpc32xx_slc.c
index 4f3d4eb17da0..9cb82066c522 100644
--- a/drivers/mtd/nand/lpc32xx_slc.c
+++ b/drivers/mtd/nand/lpc32xx_slc.c
@@ -763,7 +763,6 @@ static int lpc32xx_nand_probe(struct platform_device *pdev)
 	struct mtd_info *mtd;
 	struct nand_chip *chip;
 	struct resource *rc;
-	struct mtd_part_parser_data ppdata = {};
 	int res;
 
 	rc = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -803,6 +802,7 @@ static int lpc32xx_nand_probe(struct platform_device *pdev)
 	mtd = &host->mtd;
 	chip = &host->nand_chip;
 	chip->priv = host;
+	chip->flash_node = pdev->dev.of_node;
 	mtd->priv = chip;
 	mtd->owner = THIS_MODULE;
 	mtd->dev.parent = &pdev->dev;
@@ -908,9 +908,8 @@ static int lpc32xx_nand_probe(struct platform_device *pdev)
 	}
 
 	mtd->name = "nxp_lpc3220_slc";
-	ppdata.of_node = pdev->dev.of_node;
-	res = mtd_device_parse_register(mtd, NULL, &ppdata, host->ncfg->parts,
-					host->ncfg->num_parts);
+	res = mtd_device_register(mtd, host->ncfg->parts,
+				  host->ncfg->num_parts);
 	if (!res)
 		return res;
 
diff --git a/drivers/mtd/nand/mpc5121_nfc.c b/drivers/mtd/nand/mpc5121_nfc.c
index d6bbde4a5331..f69f146e4916 100644
--- a/drivers/mtd/nand/mpc5121_nfc.c
+++ b/drivers/mtd/nand/mpc5121_nfc.c
@@ -639,7 +639,6 @@ static int mpc5121_nfc_probe(struct platform_device *op)
 	int resettime = 0;
 	int retval = 0;
 	int rev, len;
-	struct mtd_part_parser_data ppdata;
 
 	/*
 	 * Check SoC revision. This driver supports only NFC
@@ -661,6 +660,7 @@ static int mpc5121_nfc_probe(struct platform_device *op)
 	mtd->priv = chip;
 	mtd->dev.parent = dev;
 	chip->priv = prv;
+	chip->flash_node = dn;
 	prv->dev = dev;
 
 	/* Read NFC configuration from Reset Config Word */
@@ -703,7 +703,6 @@ static int mpc5121_nfc_probe(struct platform_device *op)
 	}
 
 	mtd->name = "MPC5121 NAND";
-	ppdata.of_node = dn;
 	chip->dev_ready = mpc5121_nfc_dev_ready;
 	chip->cmdfunc = mpc5121_nfc_command;
 	chip->read_byte = mpc5121_nfc_read_byte;
@@ -815,7 +814,7 @@ static int mpc5121_nfc_probe(struct platform_device *op)
 	dev_set_drvdata(dev, mtd);
 
 	/* Register device in MTD */
-	retval = mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0);
+	retval = mtd_device_register(mtd, NULL, 0);
 	if (retval) {
 		dev_err(dev, "Error adding MTD device!\n");
 		goto error;
diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
index 136e73a3e07e..0fc4f3a2412d 100644
--- a/drivers/mtd/nand/mxc_nand.c
+++ b/drivers/mtd/nand/mxc_nand.c
@@ -1524,6 +1524,7 @@ static int mxcnd_probe(struct platform_device *pdev)
 	this->chip_delay = 5;
 
 	this->priv = host;
+	this->flash_node = pdev->dev.of_node,
 	this->dev_ready = mxc_nand_dev_ready;
 	this->cmdfunc = mxc_nand_command;
 	this->read_byte = mxc_nand_read_byte;
@@ -1683,9 +1684,7 @@ static int mxcnd_probe(struct platform_device *pdev)
 
 	/* Register the partitions */
 	mtd_device_parse_register(mtd, part_probes,
-			&(struct mtd_part_parser_data){
-				.of_node = pdev->dev.of_node,
-			},
+			NULL,
 			host->pdata.parts,
 			host->pdata.nr_parts);
 
diff --git a/drivers/mtd/nand/ndfc.c b/drivers/mtd/nand/ndfc.c
index 4f0d62f9d22c..119a775f4dff 100644
--- a/drivers/mtd/nand/ndfc.c
+++ b/drivers/mtd/nand/ndfc.c
@@ -147,7 +147,6 @@ static int ndfc_chip_init(struct ndfc_controller *ndfc,
 {
 	struct device_node *flash_np;
 	struct nand_chip *chip = &ndfc->chip;
-	struct mtd_part_parser_data ppdata;
 	int ret;
 
 	chip->IO_ADDR_R = ndfc->ndfcbase + NDFC_DATA;
@@ -174,8 +173,8 @@ static int ndfc_chip_init(struct ndfc_controller *ndfc,
 	flash_np = of_get_next_child(node, NULL);
 	if (!flash_np)
 		return -ENODEV;
+	chip->flash_node = flash_np;
 
-	ppdata.of_node = flash_np;
 	ndfc->mtd.name = kasprintf(GFP_KERNEL, "%s.%s",
 			dev_name(&ndfc->ofdev->dev), flash_np->name);
 	if (!ndfc->mtd.name) {
@@ -187,7 +186,7 @@ static int ndfc_chip_init(struct ndfc_controller *ndfc,
 	if (ret)
 		goto err;
 
-	ret = mtd_device_parse_register(&ndfc->mtd, NULL, &ppdata, NULL, 0);
+	ret = mtd_device_register(&ndfc->mtd, NULL, 0);
 
 err:
 	of_node_put(flash_np);
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 93f664cd1c90..33718dc2be58 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -1663,7 +1663,6 @@ static int omap_nand_probe(struct platform_device *pdev)
 	unsigned			sig;
 	unsigned			oob_index;
 	struct resource			*res;
-	struct mtd_part_parser_data	ppdata = {};
 
 	pdata = dev_get_platdata(&pdev->dev);
 	if (pdata == NULL) {
@@ -1688,6 +1687,7 @@ static int omap_nand_probe(struct platform_device *pdev)
 	mtd->dev.parent		= &pdev->dev;
 	nand_chip		= &info->nand;
 	nand_chip->ecc.priv	= NULL;
+	nand_chip->flash_node	= pdata->of_node;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	nand_chip->IO_ADDR_R = devm_ioremap_resource(&pdev->dev, res);
@@ -2037,9 +2037,7 @@ scan_tail:
 		goto return_error;
 	}
 
-	ppdata.of_node = pdata->of_node;
-	mtd_device_parse_register(mtd, NULL, &ppdata, pdata->parts,
-				  pdata->nr_parts);
+	mtd_device_register(mtd, pdata->parts, pdata->nr_parts);
 
 	platform_set_drvdata(pdev, mtd);
 
diff --git a/drivers/mtd/nand/orion_nand.c b/drivers/mtd/nand/orion_nand.c
index ee83749fb1d3..0728ddc32914 100644
--- a/drivers/mtd/nand/orion_nand.c
+++ b/drivers/mtd/nand/orion_nand.c
@@ -76,7 +76,6 @@ static void orion_nand_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
 static int __init orion_nand_probe(struct platform_device *pdev)
 {
 	struct mtd_info *mtd;
-	struct mtd_part_parser_data ppdata = {};
 	struct nand_chip *nc;
 	struct orion_nand_data *board;
 	struct resource *res;
@@ -127,6 +126,7 @@ static int __init orion_nand_probe(struct platform_device *pdev)
 	mtd->dev.parent = &pdev->dev;
 
 	nc->priv = board;
+	nc->flash_node = pdev->dev.of_node;
 	nc->IO_ADDR_R = nc->IO_ADDR_W = io_base;
 	nc->cmd_ctrl = orion_nand_cmd_ctrl;
 	nc->read_buf = orion_nand_read_buf;
@@ -161,9 +161,7 @@ static int __init orion_nand_probe(struct platform_device *pdev)
 	}
 
 	mtd->name = "orion_nand";
-	ppdata.of_node = pdev->dev.of_node;
-	ret = mtd_device_parse_register(mtd, NULL, &ppdata,
-			board->parts, board->nr_parts);
+	ret = mtd_device_register(mtd, board->parts, board->nr_parts);
 	if (ret) {
 		nand_release(mtd);
 		goto no_dev;
diff --git a/drivers/mtd/nand/plat_nand.c b/drivers/mtd/nand/plat_nand.c
index 65b9dbbe6d6a..034beccc863c 100644
--- a/drivers/mtd/nand/plat_nand.c
+++ b/drivers/mtd/nand/plat_nand.c
@@ -30,7 +30,6 @@ struct plat_nand_data {
 static int plat_nand_probe(struct platform_device *pdev)
 {
 	struct platform_nand_data *pdata = dev_get_platdata(&pdev->dev);
-	struct mtd_part_parser_data ppdata;
 	struct plat_nand_data *data;
 	struct resource *res;
 	const char **part_types;
@@ -58,6 +57,7 @@ static int plat_nand_probe(struct platform_device *pdev)
 		return PTR_ERR(data->io_base);
 
 	data->chip.priv = &data;
+	data->chip.flash_node = pdev->dev.of_node;
 	data->mtd.priv = &data->chip;
 	data->mtd.dev.parent = &pdev->dev;
 
@@ -94,8 +94,7 @@ static int plat_nand_probe(struct platform_device *pdev)
 
 	part_types = pdata->chip.part_probe_types;
 
-	ppdata.of_node = pdev->dev.of_node;
-	err = mtd_device_parse_register(&data->mtd, part_types, &ppdata,
+	err = mtd_device_parse_register(&data->mtd, part_types, NULL,
 					pdata->chip.partitions,
 					pdata->chip.nr_partitions);
 
diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index e453ae9a17fa..4b7fbc55749d 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -1697,6 +1697,7 @@ KEEP_CONFIG:
 
 static int alloc_nand_resource(struct platform_device *pdev)
 {
+	struct device_node *np = pdev->dev.of_node;
 	struct pxa3xx_nand_platform_data *pdata;
 	struct pxa3xx_nand_info *info;
 	struct pxa3xx_nand_host *host;
@@ -1725,6 +1726,8 @@ static int alloc_nand_resource(struct platform_device *pdev)
 		host->info_data = info;
 		mtd->priv = host;
 		mtd->dev.parent = &pdev->dev;
+		/* FIXME: all chips use the same device tree partitions */
+		chip->flash_node = np;
 
 		chip->ecc.read_page	= pxa3xx_nand_read_page_hwecc;
 		chip->ecc.write_page	= pxa3xx_nand_write_page_hwecc;
@@ -1886,7 +1889,6 @@ static int pxa3xx_nand_probe_dt(struct platform_device *pdev)
 static int pxa3xx_nand_probe(struct platform_device *pdev)
 {
 	struct pxa3xx_nand_platform_data *pdata;
-	struct mtd_part_parser_data ppdata = {};
 	struct pxa3xx_nand_info *info;
 	int ret, cs, probe_success, dma_available;
 
@@ -1933,10 +1935,8 @@ static int pxa3xx_nand_probe(struct platform_device *pdev)
 			continue;
 		}
 
-		ppdata.of_node = pdev->dev.of_node;
-		ret = mtd_device_parse_register(mtd, NULL,
-						&ppdata, pdata->parts[cs],
-						pdata->nr_parts[cs]);
+		ret = mtd_device_register(mtd, pdata->parts[cs],
+					  pdata->nr_parts[cs]);
 		if (!ret)
 			probe_success = 1;
 	}
diff --git a/drivers/mtd/nand/sh_flctl.c b/drivers/mtd/nand/sh_flctl.c
index bcba1a924c75..ec2ade986b4e 100644
--- a/drivers/mtd/nand/sh_flctl.c
+++ b/drivers/mtd/nand/sh_flctl.c
@@ -1086,7 +1086,6 @@ static int flctl_probe(struct platform_device *pdev)
 	struct sh_flctl_platform_data *pdata;
 	int ret;
 	int irq;
-	struct mtd_part_parser_data ppdata = {};
 
 	flctl = devm_kzalloc(&pdev->dev, sizeof(struct sh_flctl), GFP_KERNEL);
 	if (!flctl)
@@ -1123,6 +1122,7 @@ static int flctl_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, flctl);
 	flctl_mtd = &flctl->mtd;
 	nand = &flctl->chip;
+	nand->flash_node = pdev->dev.of_node;
 	flctl_mtd->priv = nand;
 	flctl_mtd->dev.parent = &pdev->dev;
 	flctl->pdev = pdev;
@@ -1163,9 +1163,7 @@ static int flctl_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_chip;
 
-	ppdata.of_node = pdev->dev.of_node;
-	ret = mtd_device_parse_register(flctl_mtd, NULL, &ppdata, pdata->parts,
-			pdata->nr_parts);
+	ret = mtd_device_register(flctl_mtd, pdata->parts, pdata->nr_parts);
 
 	return 0;
 
diff --git a/drivers/mtd/nand/socrates_nand.c b/drivers/mtd/nand/socrates_nand.c
index b94f53427f0f..f93598578734 100644
--- a/drivers/mtd/nand/socrates_nand.c
+++ b/drivers/mtd/nand/socrates_nand.c
@@ -147,7 +147,6 @@ static int socrates_nand_probe(struct platform_device *ofdev)
 	struct mtd_info *mtd;
 	struct nand_chip *nand_chip;
 	int res;
-	struct mtd_part_parser_data ppdata;
 
 	/* Allocate memory for the device structure (and zero it) */
 	host = devm_kzalloc(&ofdev->dev, sizeof(*host), GFP_KERNEL);
@@ -165,10 +164,10 @@ static int socrates_nand_probe(struct platform_device *ofdev)
 	host->dev = &ofdev->dev;
 
 	nand_chip->priv = host;		/* link the private data structures */
+	nand_chip->flash_node = ofdev->dev.of_node;
 	mtd->priv = nand_chip;
 	mtd->name = "socrates_nand";
 	mtd->dev.parent = &ofdev->dev;
-	ppdata.of_node = ofdev->dev.of_node;
 
 	/*should never be accessed directly */
 	nand_chip->IO_ADDR_R = (void *)0xdeadbeef;
@@ -200,7 +199,7 @@ static int socrates_nand_probe(struct platform_device *ofdev)
 		goto out;
 	}
 
-	res = mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0);
+	res = mtd_device_register(mtd, NULL, 0);
 	if (!res)
 		return res;
 
diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
index ef46ac66248b..82bbddd63c5a 100644
--- a/drivers/mtd/nand/sunxi_nand.c
+++ b/drivers/mtd/nand/sunxi_nand.c
@@ -1232,7 +1232,6 @@ static int sunxi_nand_chip_init(struct device *dev, struct sunxi_nfc *nfc,
 {
 	const struct nand_sdr_timings *timings;
 	struct sunxi_nand_chip *chip;
-	struct mtd_part_parser_data ppdata;
 	struct mtd_info *mtd;
 	struct nand_chip *nand;
 	int nsels;
@@ -1366,8 +1365,7 @@ static int sunxi_nand_chip_init(struct device *dev, struct sunxi_nfc *nfc,
 		return ret;
 	}
 
-	ppdata.of_node = np;
-	ret = mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0);
+	ret = mtd_device_register(mtd, NULL, 0);
 	if (ret) {
 		dev_err(dev, "failed to register mtd device: %d\n", ret);
 		nand_release(mtd);
diff --git a/drivers/mtd/nand/vf610_nfc.c b/drivers/mtd/nand/vf610_nfc.c
index 8805d6325579..d837c60caf03 100644
--- a/drivers/mtd/nand/vf610_nfc.c
+++ b/drivers/mtd/nand/vf610_nfc.c
@@ -811,11 +811,7 @@ static int vf610_nfc_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, mtd);
 
 	/* Register device in MTD */
-	return mtd_device_parse_register(mtd, NULL,
-		&(struct mtd_part_parser_data){
-			.of_node = chip->flash_node,
-		},
-		NULL, 0);
+	return mtd_device_register(mtd, NULL, 0);
 
 error:
 	of_node_put(chip->flash_node);
diff --git a/drivers/staging/mt29f_spinand/mt29f_spinand.c b/drivers/staging/mt29f_spinand/mt29f_spinand.c
index 405b643189fd..bd9b5b0946f5 100644
--- a/drivers/staging/mt29f_spinand/mt29f_spinand.c
+++ b/drivers/staging/mt29f_spinand/mt29f_spinand.c
@@ -853,7 +853,6 @@ static int spinand_probe(struct spi_device *spi_nand)
 	struct nand_chip *chip;
 	struct spinand_info *info;
 	struct spinand_state *state;
-	struct mtd_part_parser_data ppdata;
 
 	info  = devm_kzalloc(&spi_nand->dev, sizeof(struct spinand_info),
 			GFP_KERNEL);
@@ -897,6 +896,7 @@ static int spinand_probe(struct spi_device *spi_nand)
 		pr_info("%s: disable ecc failed!\n", __func__);
 #endif
 
+	chip->flash_node = spi_nand->dev.of_node;
 	chip->priv	= info;
 	chip->read_buf	= spinand_read_buf;
 	chip->write_buf	= spinand_write_buf;
@@ -919,8 +919,7 @@ static int spinand_probe(struct spi_device *spi_nand)
 	if (nand_scan(mtd, 1))
 		return -ENXIO;
 
-	ppdata.of_node = spi_nand->dev.of_node;
-	return mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0);
+	return mtd_device_register(mtd, NULL, 0);
 }
 
 /*
-- 
2.6.0.rc2.230.g3dd15c0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 3/5] mtd: spi-nor: drop unnecessary partition parser data
  2015-10-27  2:31 [PATCH 0/5] mtd: migrate 'of_node' handling to core, not in mtd_part_parser_data Brian Norris
  2015-10-27  2:31 ` [PATCH 1/5] mtd: ofpart: grab device tree node directly from master device node Brian Norris
  2015-10-27  2:31 ` [PATCH 2/5] mtd: nand: drop unnecessary partition parser data Brian Norris
@ 2015-10-27  2:31 ` Brian Norris
  2015-10-27  2:31 ` [PATCH 4/5] mtd: " Brian Norris
  2015-10-27  2:31 ` [PATCH 5/5] mtd: drop 'of_node' " Brian Norris
  4 siblings, 0 replies; 24+ messages in thread
From: Brian Norris @ 2015-10-27  2:31 UTC (permalink / raw)
  To: linux-mtd
  Cc: linux-kernel, Brian Norris, Boris Brezillon, Ezequiel Garcia,
	Marek Vasut, Scott Wood, Josh Wu, Robert Jarzmik, Kyungmin Park,
	Han Xu, Huang Shijie

Now that the SPI-NOR/MTD framework pass the 'flash_node' through to the
partition parsing code, we don't have to do it ourselves.

Also convert to mtd_device_register(), since we don't need the 2nd and
3rd parameters anymore.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/devices/m25p80.c      | 8 ++------
 drivers/mtd/spi-nor/fsl-quadspi.c | 4 +---
 drivers/mtd/spi-nor/nxp-spifi.c   | 4 +---
 3 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index 4b5d7a4655fd..d86fa1ae6d1f 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -175,7 +175,6 @@ static int m25p80_erase(struct spi_nor *nor, loff_t offset)
  */
 static int m25p_probe(struct spi_device *spi)
 {
-	struct mtd_part_parser_data	ppdata;
 	struct flash_platform_data	*data;
 	struct m25p *flash;
 	struct spi_nor *nor;
@@ -227,11 +226,8 @@ static int m25p_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	ppdata.of_node = spi->dev.of_node;
-
-	return mtd_device_parse_register(&nor->mtd, NULL, &ppdata,
-			data ? data->parts : NULL,
-			data ? data->nr_parts : 0);
+	return mtd_device_register(&nor->mtd, data ? data->parts : NULL,
+				   data ? data->nr_parts : 0);
 }
 
 
diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index 7b10ed413983..95991a2197e8 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -927,7 +927,6 @@ static void fsl_qspi_unprep(struct spi_nor *nor, enum spi_nor_ops ops)
 static int fsl_qspi_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
-	struct mtd_part_parser_data ppdata;
 	struct device *dev = &pdev->dev;
 	struct fsl_qspi *q;
 	struct resource *res;
@@ -1038,8 +1037,7 @@ static int fsl_qspi_probe(struct platform_device *pdev)
 		if (ret)
 			goto mutex_failed;
 
-		ppdata.of_node = np;
-		ret = mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0);
+		ret = mtd_device_register(mtd, NULL, 0);
 		if (ret)
 			goto mutex_failed;
 
diff --git a/drivers/mtd/spi-nor/nxp-spifi.c b/drivers/mtd/spi-nor/nxp-spifi.c
index 9e82098ae644..9414495a73cb 100644
--- a/drivers/mtd/spi-nor/nxp-spifi.c
+++ b/drivers/mtd/spi-nor/nxp-spifi.c
@@ -271,7 +271,6 @@ static void nxp_spifi_dummy_id_read(struct spi_nor *nor)
 static int nxp_spifi_setup_flash(struct nxp_spifi *spifi,
 				 struct device_node *np)
 {
-	struct mtd_part_parser_data ppdata;
 	enum read_mode flash_read;
 	u32 ctrl, property;
 	u16 mode = 0;
@@ -361,8 +360,7 @@ static int nxp_spifi_setup_flash(struct nxp_spifi *spifi,
 		return ret;
 	}
 
-	ppdata.of_node = np;
-	ret = mtd_device_parse_register(&spifi->nor.mtd, NULL, &ppdata, NULL, 0);
+	ret = mtd_device_register(&spifi->nor.mtd, NULL, 0);
 	if (ret) {
 		dev_err(spifi->dev, "mtd device parse failed\n");
 		return ret;
-- 
2.6.0.rc2.230.g3dd15c0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 4/5] mtd: drop unnecessary partition parser data
  2015-10-27  2:31 [PATCH 0/5] mtd: migrate 'of_node' handling to core, not in mtd_part_parser_data Brian Norris
                   ` (2 preceding siblings ...)
  2015-10-27  2:31 ` [PATCH 3/5] mtd: spi-nor: " Brian Norris
@ 2015-10-27  2:31 ` Brian Norris
  2015-10-27  2:31 ` [PATCH 5/5] mtd: drop 'of_node' " Brian Norris
  4 siblings, 0 replies; 24+ messages in thread
From: Brian Norris @ 2015-10-27  2:31 UTC (permalink / raw)
  To: linux-mtd
  Cc: linux-kernel, Brian Norris, Boris Brezillon, Ezequiel Garcia,
	Marek Vasut, Scott Wood, Josh Wu, Robert Jarzmik, Kyungmin Park,
	Han Xu, Huang Shijie

We should assign the MTD dev.of_node instead of the parser data field.
This gets us the equivalent partition parser behavior with fewer special
fields and parameter passing.

Also convert several of these to mtd_device_register(), since we don't
need the 2nd and 3rd parameters anymore.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/devices/mtd_dataflash.c | 5 ++---
 drivers/mtd/devices/spear_smi.c     | 6 ++----
 drivers/mtd/devices/st_spi_fsm.c    | 5 ++---
 drivers/mtd/maps/lantiq-flash.c     | 5 ++---
 drivers/mtd/maps/physmap_of.c       | 5 ++---
 drivers/mtd/onenand/omap2.c         | 8 +++-----
 6 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/drivers/mtd/devices/mtd_dataflash.c b/drivers/mtd/devices/mtd_dataflash.c
index 39666d552682..0de40f5a7509 100644
--- a/drivers/mtd/devices/mtd_dataflash.c
+++ b/drivers/mtd/devices/mtd_dataflash.c
@@ -624,7 +624,6 @@ static int add_dataflash_otp(struct spi_device *spi, char *name, int nr_pages,
 {
 	struct dataflash		*priv;
 	struct mtd_info			*device;
-	struct mtd_part_parser_data	ppdata;
 	struct flash_platform_data	*pdata = dev_get_platdata(&spi->dev);
 	char				*otp_tag = "";
 	int				err = 0;
@@ -656,6 +655,7 @@ static int add_dataflash_otp(struct spi_device *spi, char *name, int nr_pages,
 	device->priv = priv;
 
 	device->dev.parent = &spi->dev;
+	device->dev.of_node = spi->dev.of_node;
 
 	if (revision >= 'c')
 		otp_tag = otp_setup(device, revision);
@@ -665,8 +665,7 @@ static int add_dataflash_otp(struct spi_device *spi, char *name, int nr_pages,
 			pagesize, otp_tag);
 	spi_set_drvdata(spi, priv);
 
-	ppdata.of_node = spi->dev.of_node;
-	err = mtd_device_parse_register(device, NULL, &ppdata,
+	err = mtd_device_register(device,
 			pdata ? pdata->parts : NULL,
 			pdata ? pdata->nr_parts : 0);
 
diff --git a/drivers/mtd/devices/spear_smi.c b/drivers/mtd/devices/spear_smi.c
index 64c7458344d4..8c41cc82d860 100644
--- a/drivers/mtd/devices/spear_smi.c
+++ b/drivers/mtd/devices/spear_smi.c
@@ -810,7 +810,6 @@ static int spear_smi_setup_banks(struct platform_device *pdev,
 				 u32 bank, struct device_node *np)
 {
 	struct spear_smi *dev = platform_get_drvdata(pdev);
-	struct mtd_part_parser_data ppdata = {};
 	struct spear_smi_flash_info *flash_info;
 	struct spear_smi_plat_data *pdata;
 	struct spear_snor_flash *flash;
@@ -855,6 +854,7 @@ static int spear_smi_setup_banks(struct platform_device *pdev,
 		flash->mtd.name = flash_devices[flash_index].name;
 
 	flash->mtd.dev.parent = &pdev->dev;
+	flash->mtd.dev.of_node = np;
 	flash->mtd.type = MTD_NORFLASH;
 	flash->mtd.writesize = 1;
 	flash->mtd.flags = MTD_CAP_NORFLASH;
@@ -881,10 +881,8 @@ static int spear_smi_setup_banks(struct platform_device *pdev,
 		count = flash_info->nr_partitions;
 	}
 #endif
-	ppdata.of_node = np;
 
-	ret = mtd_device_parse_register(&flash->mtd, NULL, &ppdata, parts,
-					count);
+	ret = mtd_device_register(&flash->mtd, parts, count);
 	if (ret) {
 		dev_err(&dev->pdev->dev, "Err MTD partition=%d\n", ret);
 		return ret;
diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
index 3060025c8af4..16cd44f728c1 100644
--- a/drivers/mtd/devices/st_spi_fsm.c
+++ b/drivers/mtd/devices/st_spi_fsm.c
@@ -2025,7 +2025,6 @@ boot_device_fail:
 static int stfsm_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
-	struct mtd_part_parser_data ppdata;
 	struct flash_info *info;
 	struct resource *res;
 	struct stfsm *fsm;
@@ -2035,7 +2034,6 @@ static int stfsm_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "No DT found\n");
 		return -EINVAL;
 	}
-	ppdata.of_node = np;
 
 	fsm = devm_kzalloc(&pdev->dev, sizeof(*fsm), GFP_KERNEL);
 	if (!fsm)
@@ -2106,6 +2104,7 @@ static int stfsm_probe(struct platform_device *pdev)
 
 	fsm->mtd.name		= info->name;
 	fsm->mtd.dev.parent	= &pdev->dev;
+	fsm->mtd.dev.of_node	= np;
 	fsm->mtd.type		= MTD_NORFLASH;
 	fsm->mtd.writesize	= 4;
 	fsm->mtd.writebufsize	= fsm->mtd.writesize;
@@ -2124,7 +2123,7 @@ static int stfsm_probe(struct platform_device *pdev)
 		(long long)fsm->mtd.size, (long long)(fsm->mtd.size >> 20),
 		fsm->mtd.erasesize, (fsm->mtd.erasesize >> 10));
 
-	return mtd_device_parse_register(&fsm->mtd, NULL, &ppdata, NULL, 0);
+	return mtd_device_register(&fsm->mtd, NULL, 0);
 }
 
 static int stfsm_remove(struct platform_device *pdev)
diff --git a/drivers/mtd/maps/lantiq-flash.c b/drivers/mtd/maps/lantiq-flash.c
index 93852054977e..048cc33226ea 100644
--- a/drivers/mtd/maps/lantiq-flash.c
+++ b/drivers/mtd/maps/lantiq-flash.c
@@ -110,7 +110,6 @@ ltq_copy_to(struct map_info *map, unsigned long to,
 static int
 ltq_mtd_probe(struct platform_device *pdev)
 {
-	struct mtd_part_parser_data ppdata;
 	struct ltq_mtd *ltq_mtd;
 	struct cfi_private *cfi;
 	int err;
@@ -161,13 +160,13 @@ ltq_mtd_probe(struct platform_device *pdev)
 	}
 
 	ltq_mtd->mtd->dev.parent = &pdev->dev;
+	ltq_mtd->mtd->dev.of_node = pdev->dev.of_node;
 
 	cfi = ltq_mtd->map->fldrv_priv;
 	cfi->addr_unlock1 ^= 1;
 	cfi->addr_unlock2 ^= 1;
 
-	ppdata.of_node = pdev->dev.of_node;
-	err = mtd_device_parse_register(ltq_mtd->mtd, NULL, &ppdata, NULL, 0);
+	err = mtd_device_register(ltq_mtd->mtd, NULL, 0);
 	if (err) {
 		dev_err(&pdev->dev, "failed to add partitions\n");
 		goto err_destroy;
diff --git a/drivers/mtd/maps/physmap_of.c b/drivers/mtd/maps/physmap_of.c
index e46b4e983666..6ebe674262cc 100644
--- a/drivers/mtd/maps/physmap_of.c
+++ b/drivers/mtd/maps/physmap_of.c
@@ -166,7 +166,6 @@ static int of_flash_probe(struct platform_device *dev)
 	int reg_tuple_size;
 	struct mtd_info **mtd_list = NULL;
 	resource_size_t res_size;
-	struct mtd_part_parser_data ppdata;
 	bool map_indirect;
 	const char *mtd_name = NULL;
 
@@ -310,13 +309,13 @@ static int of_flash_probe(struct platform_device *dev)
 	if (err)
 		goto err_out;
 
-	ppdata.of_node = dp;
+	info->cmtd->dev.of_node = dp;
 	part_probe_types = of_get_probes(dp);
 	if (!part_probe_types) {
 		err = -ENOMEM;
 		goto err_out;
 	}
-	mtd_device_parse_register(info->cmtd, part_probe_types, &ppdata,
+	mtd_device_parse_register(info->cmtd, part_probe_types, NULL,
 			NULL, 0);
 	of_free_probes(part_probe_types);
 
diff --git a/drivers/mtd/onenand/omap2.c b/drivers/mtd/onenand/omap2.c
index 3e0285696227..2a539e3fa1b6 100644
--- a/drivers/mtd/onenand/omap2.c
+++ b/drivers/mtd/onenand/omap2.c
@@ -614,7 +614,6 @@ static int omap2_onenand_probe(struct platform_device *pdev)
 	struct onenand_chip *this;
 	int r;
 	struct resource *res;
-	struct mtd_part_parser_data ppdata = {};
 
 	pdata = dev_get_platdata(&pdev->dev);
 	if (pdata == NULL) {
@@ -713,6 +712,7 @@ static int omap2_onenand_probe(struct platform_device *pdev)
 	c->mtd.priv = &c->onenand;
 
 	c->mtd.dev.parent = &pdev->dev;
+	c->mtd.dev.of_node = pdata->of_node;
 
 	this = &c->onenand;
 	if (c->dma_channel >= 0) {
@@ -743,10 +743,8 @@ static int omap2_onenand_probe(struct platform_device *pdev)
 	if ((r = onenand_scan(&c->mtd, 1)) < 0)
 		goto err_release_regulator;
 
-	ppdata.of_node = pdata->of_node;
-	r = mtd_device_parse_register(&c->mtd, NULL, &ppdata,
-				      pdata ? pdata->parts : NULL,
-				      pdata ? pdata->nr_parts : 0);
+	r = mtd_device_register(&c->mtd, pdata ? pdata->parts : NULL,
+				pdata ? pdata->nr_parts : 0);
 	if (r)
 		goto err_release_onenand;
 
-- 
2.6.0.rc2.230.g3dd15c0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 5/5] mtd: drop 'of_node' partition parser data
  2015-10-27  2:31 [PATCH 0/5] mtd: migrate 'of_node' handling to core, not in mtd_part_parser_data Brian Norris
                   ` (3 preceding siblings ...)
  2015-10-27  2:31 ` [PATCH 4/5] mtd: " Brian Norris
@ 2015-10-27  2:31 ` Brian Norris
  4 siblings, 0 replies; 24+ messages in thread
From: Brian Norris @ 2015-10-27  2:31 UTC (permalink / raw)
  To: linux-mtd
  Cc: linux-kernel, Brian Norris, Boris Brezillon, Ezequiel Garcia,
	Marek Vasut, Scott Wood, Josh Wu, Robert Jarzmik, Kyungmin Park,
	Han Xu, Huang Shijie

This field is no longer used anywhere, as it is superseded by
mtd->dev.of_node.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/ofpart.c           | 14 ++++----------
 include/linux/mtd/partitions.h |  4 ----
 2 files changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/ofpart.c
index a5dfd73cfc7d..4368badfc653 100644
--- a/drivers/mtd/ofpart.c
+++ b/drivers/mtd/ofpart.c
@@ -35,11 +35,8 @@ static int parse_ofpart_partitions(struct mtd_info *master,
 	int nr_parts, i;
 
 
-	/*
-	 * of_node can be provided through auxiliary parser data or (preferred)
-	 * by assigning the master device
-	 */
-	node = data && data->of_node ? data->of_node : master->dev.of_node;
+	/* Pull of_node from the master device */
+	node = master->dev.of_node;
 	if (!node)
 		return 0;
 
@@ -121,11 +118,8 @@ static int parse_ofoldpart_partitions(struct mtd_info *master,
 	} *part;
 	const char *names;
 
-	/*
-	 * of_node can be provided through auxiliary parser data or (preferred)
-	 * by assigning the master device
-	 */
-	dp = data && data->of_node ? data->of_node : master->dev.of_node;
+	/* Pull of_node from the master device */
+	dp = master->dev.of_node;
 	if (!dp)
 		return 0;
 
diff --git a/include/linux/mtd/partitions.h b/include/linux/mtd/partitions.h
index e742f34b67eb..773975a3c9e6 100644
--- a/include/linux/mtd/partitions.h
+++ b/include/linux/mtd/partitions.h
@@ -56,13 +56,9 @@ struct device_node;
 /**
  * struct mtd_part_parser_data - used to pass data to MTD partition parsers.
  * @origin: for RedBoot, start address of MTD device
- * @of_node: for OF parsers, device node containing partitioning information.
- *           This field is deprecated, as the device node should simply be
- *           assigned to the master struct device.
  */
 struct mtd_part_parser_data {
 	unsigned long origin;
-	struct device_node *of_node;
 };
 
 
-- 
2.6.0.rc2.230.g3dd15c0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/5] mtd: ofpart: grab device tree node directly from master device node
  2015-10-27  2:31 ` [PATCH 1/5] mtd: ofpart: grab device tree node directly from master device node Brian Norris
@ 2015-10-27  7:42   ` Boris Brezillon
  2015-10-27 17:54     ` Brian Norris
  0 siblings, 1 reply; 24+ messages in thread
From: Boris Brezillon @ 2015-10-27  7:42 UTC (permalink / raw)
  To: Brian Norris
  Cc: linux-mtd, linux-kernel, Ezequiel Garcia, Marek Vasut,
	Scott Wood, Josh Wu, Robert Jarzmik, Kyungmin Park, Han Xu,
	Huang Shijie

Hi Brian,

On Mon, 26 Oct 2015 19:31:06 -0700
Brian Norris <computersforpeace@gmail.com> wrote:

> It seems more logical to use a device node directly associated with the
> MTD master device (i.e., mtd->dev.of_node field) rather than requiring
> auxiliary partition parser information to be passed in by the driver in
> a separate struct.
> 
> This patch supports the mtd->dev.of_node field, deprecates the parser
> data 'of_node' field, and begins using the new convention for nand_base.
> Other NAND driver conversions may now follow.
> 
> Additional side benefit to assigning mtd->dev.of_node rather than using
> parser data: the driver core will automatically create a device -> node
> symlink for us.

I like the idea, but how about pushing the solution even further and
killing the ->flash_node field which AFAICT is rendered useless by
your patch?

Best Regards,

Boris

> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> ---
>  drivers/mtd/nand/nand_base.c   |  3 +++
>  drivers/mtd/ofpart.c           | 18 ++++++++++--------
>  drivers/mtd/spi-nor/spi-nor.c  |  1 +
>  include/linux/mtd/partitions.h |  4 +++-
>  4 files changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index cc74142938b0..d2e7fee2ac37 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -3990,6 +3990,9 @@ int nand_scan_ident(struct mtd_info *mtd, int maxchips,
>  	int ret;
>  
>  	if (chip->flash_node) {
> +		/* MTD can automatically handle DT partitions, etc. */
> +		mtd->dev.of_node = chip->flash_node;
> +
>  		ret = nand_dt_init(mtd, chip, chip->flash_node);
>  		if (ret)
>  			return ret;
> diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/ofpart.c
> index aa26c32e1bc2..a5dfd73cfc7d 100644
> --- a/drivers/mtd/ofpart.c
> +++ b/drivers/mtd/ofpart.c
> @@ -35,10 +35,11 @@ static int parse_ofpart_partitions(struct mtd_info *master,
>  	int nr_parts, i;
>  
>  
> -	if (!data)
> -		return 0;
> -
> -	node = data->of_node;
> +	/*
> +	 * of_node can be provided through auxiliary parser data or (preferred)
> +	 * by assigning the master device
> +	 */
> +	node = data && data->of_node ? data->of_node : master->dev.of_node;
>  	if (!node)
>  		return 0;
>  
> @@ -120,10 +121,11 @@ static int parse_ofoldpart_partitions(struct mtd_info *master,
>  	} *part;
>  	const char *names;
>  
> -	if (!data)
> -		return 0;
> -
> -	dp = data->of_node;
> +	/*
> +	 * of_node can be provided through auxiliary parser data or (preferred)
> +	 * by assigning the master device
> +	 */
> +	dp = data && data->of_node ? data->of_node : master->dev.of_node;
>  	if (!dp)
>  		return 0;
>  
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 49883905a434..8f9080c6db63 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -1258,6 +1258,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>  		mtd->flags |= MTD_NO_ERASE;
>  
>  	mtd->dev.parent = dev;
> +	mtd->dev.of_node = np;
>  	nor->page_size = info->page_size;
>  	mtd->writebufsize = nor->page_size;
>  
> diff --git a/include/linux/mtd/partitions.h b/include/linux/mtd/partitions.h
> index 6a35e6de5da1..e742f34b67eb 100644
> --- a/include/linux/mtd/partitions.h
> +++ b/include/linux/mtd/partitions.h
> @@ -56,7 +56,9 @@ struct device_node;
>  /**
>   * struct mtd_part_parser_data - used to pass data to MTD partition parsers.
>   * @origin: for RedBoot, start address of MTD device
> - * @of_node: for OF parsers, device node containing partitioning information
> + * @of_node: for OF parsers, device node containing partitioning information.
> + *           This field is deprecated, as the device node should simply be
> + *           assigned to the master struct device.
>   */
>  struct mtd_part_parser_data {
>  	unsigned long origin;



-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/5] mtd: nand: drop unnecessary partition parser data
  2015-10-27  2:31 ` [PATCH 2/5] mtd: nand: drop unnecessary partition parser data Brian Norris
@ 2015-10-27  7:52   ` Boris Brezillon
  2015-10-28  0:45   ` Brian Norris
  1 sibling, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2015-10-27  7:52 UTC (permalink / raw)
  To: Brian Norris
  Cc: linux-mtd, linux-kernel, Ezequiel Garcia, Marek Vasut,
	Scott Wood, Josh Wu, Robert Jarzmik, Kyungmin Park, Han Xu,
	Huang Shijie

On Mon, 26 Oct 2015 19:31:07 -0700
Brian Norris <computersforpeace@gmail.com> wrote:

> All of these drivers set up a parser data struct just to communicate DT
> partition data. This field has been deprecated and is instead supported
> by telling nand_scan_ident() about the 'flash_node'.
> 
> This patch:
>  * sets chip->flash_node for those drivers that didn't already (but used
>    OF partitioning)

As mentioned in patch 1, I think we should get rid of the ->flash_node
field and directly set mtd->dev.of_node instead. If we want to hide
MTD internals we could provide this kind of helper:

static inline void mtd_set_of_node(struct mtd_device *mtd,
				   struct device_node *np)
{
	mtd->dev.of_node = np;
}

>  * drops the parser data
>  * switches to the simpler mtd_device_register() where possible, now
>    that we've eliminated one of the auxiliary parameters
> 
> Now that we've assigned chip->flash_node for these drivers, we can
> probably rely on nand_dt_init() to do more of the DT parsing for us, but
> for now, I don't want to fiddle with each of these drivers. The parsing
> is done in duplicate for now on some drivers. I don't think this should
> break things. (Famous last words.)
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>

For the sunxi_nand driver

Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>

Thanks,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/5] mtd: ofpart: grab device tree node directly from master device node
  2015-10-27  7:42   ` Boris Brezillon
@ 2015-10-27 17:54     ` Brian Norris
  2015-10-28  1:01       ` Brian Norris
  2015-10-28  7:58       ` Boris Brezillon
  0 siblings, 2 replies; 24+ messages in thread
From: Brian Norris @ 2015-10-27 17:54 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: linux-mtd, linux-kernel, Ezequiel Garcia, Marek Vasut,
	Scott Wood, Josh Wu, Robert Jarzmik, Kyungmin Park, Han Xu,
	Huang Shijie

Hi Boris,

On Tue, Oct 27, 2015 at 08:42:00AM +0100, Boris Brezillon wrote:
> On Mon, 26 Oct 2015 19:31:06 -0700
> Brian Norris <computersforpeace@gmail.com> wrote:
> 
> > It seems more logical to use a device node directly associated with the
> > MTD master device (i.e., mtd->dev.of_node field) rather than requiring
> > auxiliary partition parser information to be passed in by the driver in
> > a separate struct.
> > 
> > This patch supports the mtd->dev.of_node field, deprecates the parser
> > data 'of_node' field, and begins using the new convention for nand_base.
> > Other NAND driver conversions may now follow.
> > 
> > Additional side benefit to assigning mtd->dev.of_node rather than using
> > parser data: the driver core will automatically create a device -> node
> > symlink for us.
> 
> I like the idea, but how about pushing the solution even further and
> killing the ->flash_node field which AFAICT is rendered useless by
> your patch?

I suppose we could do that. I do think there's something to be said for
layering, though. Historically, we haven't done a very good job of
layering in MTD, so low-level drivers often have to poke around in the
MTD structures, even if they really should only have to know a few
things about their helper subsystem/library, like NAND or SPI NOR. So
with that in mind, I think the ->flash_node serves some purpose --
drivers can just initialize struct nand_chip/spi_nor and be assured that
the NAND/SPI-NOR subsystems will take care of things.

Now, I don't think there's much reason to suspect that we'd have a more
complex mapping than 1:1 between struct mtd_info and struct nand_chip or
struct spi_nor, so maybe we don't actually need duplicate storage
(mtd.dev.of_node and {spi_nor,nand_chip}.flash_node), and the layering
is just have these APIs:

	nand_set_flash_node()
	spi_nor_set_flash_node()

which just call mtd_set_of_node()?

Speaking of layering: why do we have NAND drivers initializing mtd->priv
for us, yet nand_base just assumes that it points to a struct nand_chip?
And why isn't struct mtd_info just embedded in struct nand_chip? Are
there ever cases we want more than one (master) MTD per nand_chip? Or
vice versa?

Thanks for the review,
Brian

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/5] mtd: nand: drop unnecessary partition parser data
  2015-10-27  2:31 ` [PATCH 2/5] mtd: nand: drop unnecessary partition parser data Brian Norris
  2015-10-27  7:52   ` Boris Brezillon
@ 2015-10-28  0:45   ` Brian Norris
  1 sibling, 0 replies; 24+ messages in thread
From: Brian Norris @ 2015-10-28  0:45 UTC (permalink / raw)
  To: linux-mtd
  Cc: linux-kernel, Boris Brezillon, Ezequiel Garcia, Marek Vasut,
	Scott Wood, Josh Wu, Robert Jarzmik, Kyungmin Park, Han Xu,
	Huang Shijie

On Mon, Oct 26, 2015 at 07:31:07PM -0700, Brian Norris wrote:
> All of these drivers set up a parser data struct just to communicate DT
> partition data. This field has been deprecated and is instead supported
> by telling nand_scan_ident() about the 'flash_node'.
> 
> This patch:
>  * sets chip->flash_node for those drivers that didn't already (but used
>    OF partitioning)
>  * drops the parser data
>  * switches to the simpler mtd_device_register() where possible, now
>    that we've eliminated one of the auxiliary parameters
> 
> Now that we've assigned chip->flash_node for these drivers, we can
> probably rely on nand_dt_init() to do more of the DT parsing for us, but
> for now, I don't want to fiddle with each of these drivers. The parsing
> is done in duplicate for now on some drivers. I don't think this should
> break things. (Famous last words.)
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> ---
>  drivers/mtd/nand/atmel_nand.c                 |  7 +++----
>  drivers/mtd/nand/brcmnand/brcmnand.c          |  3 +--
>  drivers/mtd/nand/davinci_nand.c               | 10 +++-------
>  drivers/mtd/nand/fsl_elbc_nand.c              |  5 ++---
>  drivers/mtd/nand/fsl_ifc_nand.c               |  5 ++---

Found some compile errors in the above 2 FSL drivers. Will squash the
below (or similar) into v2.

>  drivers/mtd/nand/fsl_upm.c                    |  5 ++---
>  drivers/mtd/nand/fsmc_nand.c                  |  7 +++----
>  drivers/mtd/nand/gpio.c                       |  8 +++-----
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.c        |  5 ++---
>  drivers/mtd/nand/hisi504_nand.c               |  5 ++---
>  drivers/mtd/nand/lpc32xx_mlc.c                |  7 +++----
>  drivers/mtd/nand/lpc32xx_slc.c                |  7 +++----
>  drivers/mtd/nand/mpc5121_nfc.c                |  5 ++---
>  drivers/mtd/nand/mxc_nand.c                   |  5 ++---
>  drivers/mtd/nand/ndfc.c                       |  5 ++---
>  drivers/mtd/nand/omap2.c                      |  6 ++----
>  drivers/mtd/nand/orion_nand.c                 |  6 ++----
>  drivers/mtd/nand/plat_nand.c                  |  5 ++---
>  drivers/mtd/nand/pxa3xx_nand.c                | 10 +++++-----
>  drivers/mtd/nand/sh_flctl.c                   |  6 ++----
>  drivers/mtd/nand/socrates_nand.c              |  5 ++---
>  drivers/mtd/nand/sunxi_nand.c                 |  4 +---
>  drivers/mtd/nand/vf610_nfc.c                  |  6 +-----
>  drivers/staging/mt29f_spinand/mt29f_spinand.c |  5 ++---
>  24 files changed, 54 insertions(+), 88 deletions(-)

Brian

diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
index f3d6e83e193b..850546dc98c8 100644
--- a/drivers/mtd/nand/fsl_elbc_nand.c
+++ b/drivers/mtd/nand/fsl_elbc_nand.c
@@ -748,7 +748,7 @@ static int fsl_elbc_chip_init(struct fsl_elbc_mtd *priv)
 	/* Fill in fsl_elbc_mtd structure */
 	priv->mtd.priv = chip;
 	priv->mtd.dev.parent = priv->dev;
-	chip->flash_node = priv->dev.of_node;
+	chip->flash_node = priv->dev->of_node;
 
 	/* set timeout to maximum */
 	priv->fmr = 15 << FMR_CWTO_SHIFT;
diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c
index 60978b737cac..8ae2a237ed4d 100644
--- a/drivers/mtd/nand/fsl_ifc_nand.c
+++ b/drivers/mtd/nand/fsl_ifc_nand.c
@@ -883,7 +883,7 @@ static int fsl_ifc_chip_init(struct fsl_ifc_mtd *priv)
 	/* Fill in fsl_ifc_mtd structure */
 	priv->mtd.priv = chip;
 	priv->mtd.dev.parent = priv->dev;
-	chip->flash_node = priv->dev.of_node;
+	chip->flash_node = priv->dev->of_node;
 
 	/* fill in nand_chip structure */
 	/* set up function call table */

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/5] mtd: ofpart: grab device tree node directly from master device node
  2015-10-27 17:54     ` Brian Norris
@ 2015-10-28  1:01       ` Brian Norris
  2015-10-28  8:02         ` Boris Brezillon
  2015-10-28  7:58       ` Boris Brezillon
  1 sibling, 1 reply; 24+ messages in thread
From: Brian Norris @ 2015-10-28  1:01 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: linux-mtd, linux-kernel, Ezequiel Garcia, Marek Vasut,
	Scott Wood, Josh Wu, Robert Jarzmik, Kyungmin Park, Han Xu,
	Huang Shijie

On Tue, Oct 27, 2015 at 10:54:46AM -0700, Brian Norris wrote:
> On Tue, Oct 27, 2015 at 08:42:00AM +0100, Boris Brezillon wrote:
> > On Mon, 26 Oct 2015 19:31:06 -0700
> > I like the idea, but how about pushing the solution even further and
> > killing the ->flash_node field which AFAICT is rendered useless by
> > your patch?
> 
> I suppose we could do that. I do think there's something to be said for
> layering, though. Historically, we haven't done a very good job of
> layering in MTD, so low-level drivers often have to poke around in the
> MTD structures, even if they really should only have to know a few
> things about their helper subsystem/library, like NAND or SPI NOR. So
> with that in mind, I think the ->flash_node serves some purpose --
> drivers can just initialize struct nand_chip/spi_nor and be assured that
> the NAND/SPI-NOR subsystems will take care of things.
> 
> Now, I don't think there's much reason to suspect that we'd have a more
> complex mapping than 1:1 between struct mtd_info and struct nand_chip or
> struct spi_nor, so maybe we don't actually need duplicate storage
> (mtd.dev.of_node and {spi_nor,nand_chip}.flash_node), and the layering
> is just have these APIs:
> 
> 	nand_set_flash_node()
> 	spi_nor_set_flash_node()
> 
> which just call mtd_set_of_node()?

I looked at this quickly for NAND, and it's hard to do right now because
of the below quote. The SPI NOR layering is better though, so that
works. Mind if I defer the dropping the flash_node in NAND but do the
SPI NOR one?

> Speaking of layering: why do we have NAND drivers initializing mtd->priv
> for us, yet nand_base just assumes that it points to a struct nand_chip?
> And why isn't struct mtd_info just embedded in struct nand_chip? Are
> there ever cases we want more than one (master) MTD per nand_chip? Or
> vice versa?

The layering (or lack thereof) make it hard to extract a struct mtd_info
from a struct nand_chip.

Brian

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/5] mtd: ofpart: grab device tree node directly from master device node
  2015-10-27 17:54     ` Brian Norris
  2015-10-28  1:01       ` Brian Norris
@ 2015-10-28  7:58       ` Boris Brezillon
  2015-10-28 16:11         ` Marek Vasut
  1 sibling, 1 reply; 24+ messages in thread
From: Boris Brezillon @ 2015-10-28  7:58 UTC (permalink / raw)
  To: Brian Norris
  Cc: linux-mtd, linux-kernel, Ezequiel Garcia, Marek Vasut,
	Scott Wood, Josh Wu, Robert Jarzmik, Kyungmin Park, Han Xu,
	Huang Shijie

Hi Brian,

On Tue, 27 Oct 2015 10:54:46 -0700
Brian Norris <computersforpeace@gmail.com> wrote:

> Hi Boris,
> 
> On Tue, Oct 27, 2015 at 08:42:00AM +0100, Boris Brezillon wrote:
> > On Mon, 26 Oct 2015 19:31:06 -0700
> > Brian Norris <computersforpeace@gmail.com> wrote:
> > 
> > > It seems more logical to use a device node directly associated with the
> > > MTD master device (i.e., mtd->dev.of_node field) rather than requiring
> > > auxiliary partition parser information to be passed in by the driver in
> > > a separate struct.
> > > 
> > > This patch supports the mtd->dev.of_node field, deprecates the parser
> > > data 'of_node' field, and begins using the new convention for nand_base.
> > > Other NAND driver conversions may now follow.
> > > 
> > > Additional side benefit to assigning mtd->dev.of_node rather than using
> > > parser data: the driver core will automatically create a device -> node
> > > symlink for us.
> > 
> > I like the idea, but how about pushing the solution even further and
> > killing the ->flash_node field which AFAICT is rendered useless by
> > your patch?
> 
> I suppose we could do that. I do think there's something to be said for
> layering, though. Historically, we haven't done a very good job of
> layering in MTD, so low-level drivers often have to poke around in the
> MTD structures, even if they really should only have to know a few
> things about their helper subsystem/library, like NAND or SPI NOR. So
> with that in mind, I think the ->flash_node serves some purpose --
> drivers can just initialize struct nand_chip/spi_nor and be assured that
> the NAND/SPI-NOR subsystems will take care of things.
> 
> Now, I don't think there's much reason to suspect that we'd have a more
> complex mapping than 1:1 between struct mtd_info and struct nand_chip or
> struct spi_nor, so maybe we don't actually need duplicate storage
> (mtd.dev.of_node and {spi_nor,nand_chip}.flash_node), and the layering
> is just have these APIs:
> 
> 	nand_set_flash_node()
> 	spi_nor_set_flash_node()
> 
> which just call mtd_set_of_node()?

I'm fine with that, but as you stated in your next answer this is not
that simple for the NAND subsystem since a lot of drivers are doing
their own custom initialization.

I'll try to come up with something to address that soon (I'll probably
revive the nand_chip nand_controller separation series too) based on
other subsystems like SPI or I2C.
How about providing several simple functions and progressively migrating
all NAND controller drivers to it:

struct nand_controller *nand_controller_alloc(struct device *dev, ...);
struct nand_chip *nand_alloc(struct nand_controller *ctrl, ...);

...

> 
> Speaking of layering: why do we have NAND drivers initializing mtd->priv
> for us, yet nand_base just assumes that it points to a struct nand_chip?

I guess it's been like this from the beginning, and nobody bothered
moving this boilerplate into a function provided by the NAND subsystem.

> And why isn't struct mtd_info just embedded in struct nand_chip?

I always wondered the same thing.

> Are
> there ever cases we want more than one (master) MTD per nand_chip? Or
> vice versa?

Nope, I'd say that you always have a 1:1 relationship between a master
MTD device and a NAND device.

Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/5] mtd: ofpart: grab device tree node directly from master device node
  2015-10-28  1:01       ` Brian Norris
@ 2015-10-28  8:02         ` Boris Brezillon
  0 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2015-10-28  8:02 UTC (permalink / raw)
  To: Brian Norris
  Cc: linux-mtd, linux-kernel, Ezequiel Garcia, Marek Vasut,
	Scott Wood, Josh Wu, Robert Jarzmik, Kyungmin Park, Han Xu,
	Huang Shijie

Hi Brian,

On Tue, 27 Oct 2015 18:01:02 -0700
Brian Norris <computersforpeace@gmail.com> wrote:

> On Tue, Oct 27, 2015 at 10:54:46AM -0700, Brian Norris wrote:
> > On Tue, Oct 27, 2015 at 08:42:00AM +0100, Boris Brezillon wrote:
> > > On Mon, 26 Oct 2015 19:31:06 -0700
> > > I like the idea, but how about pushing the solution even further and
> > > killing the ->flash_node field which AFAICT is rendered useless by
> > > your patch?
> > 
> > I suppose we could do that. I do think there's something to be said for
> > layering, though. Historically, we haven't done a very good job of
> > layering in MTD, so low-level drivers often have to poke around in the
> > MTD structures, even if they really should only have to know a few
> > things about their helper subsystem/library, like NAND or SPI NOR. So
> > with that in mind, I think the ->flash_node serves some purpose --
> > drivers can just initialize struct nand_chip/spi_nor and be assured that
> > the NAND/SPI-NOR subsystems will take care of things.
> > 
> > Now, I don't think there's much reason to suspect that we'd have a more
> > complex mapping than 1:1 between struct mtd_info and struct nand_chip or
> > struct spi_nor, so maybe we don't actually need duplicate storage
> > (mtd.dev.of_node and {spi_nor,nand_chip}.flash_node), and the layering
> > is just have these APIs:
> > 
> > 	nand_set_flash_node()
> > 	spi_nor_set_flash_node()
> > 
> > which just call mtd_set_of_node()?
> 
> I looked at this quickly for NAND, and it's hard to do right now because
> of the below quote. The SPI NOR layering is better though, so that
> works. Mind if I defer the dropping the flash_node in NAND but do the
> SPI NOR one?

No, I don't mind (we'll solve that later).

> 
> > Speaking of layering: why do we have NAND drivers initializing mtd->priv
> > for us, yet nand_base just assumes that it points to a struct nand_chip?
> > And why isn't struct mtd_info just embedded in struct nand_chip? Are
> > there ever cases we want more than one (master) MTD per nand_chip? Or
> > vice versa?
> 
> The layering (or lack thereof) make it hard to extract a struct mtd_info
> from a struct nand_chip.

That's true, and that's why we'd better rework a few things (embed
or point to an allocated mtd struct in nand_chip) before trying to
automatically assign mtd->priv to a nand_chip pointer.

Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/5] mtd: ofpart: grab device tree node directly from master device node
  2015-10-28  7:58       ` Boris Brezillon
@ 2015-10-28 16:11         ` Marek Vasut
  2015-10-28 16:32           ` Boris Brezillon
  0 siblings, 1 reply; 24+ messages in thread
From: Marek Vasut @ 2015-10-28 16:11 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Brian Norris, linux-mtd, linux-kernel, Ezequiel Garcia,
	Scott Wood, Josh Wu, Robert Jarzmik, Kyungmin Park, Han Xu,
	Huang Shijie

On Wednesday, October 28, 2015 at 08:58:13 AM, Boris Brezillon wrote:
> Hi Brian,

Hi,

[...]

> > Are
> > there ever cases we want more than one (master) MTD per nand_chip? Or
> > vice versa?
> 
> Nope, I'd say that you always have a 1:1 relationship between a master
> MTD device and a NAND device.

Do some sorts of chipselects come into play here ? Ie. you can have one master
with multiple NAND chips connected to it.

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/5] mtd: ofpart: grab device tree node directly from master device node
  2015-10-28 16:11         ` Marek Vasut
@ 2015-10-28 16:32           ` Boris Brezillon
  2015-10-28 17:14             ` Brian Norris
  2015-10-28 20:38             ` Marek Vasut
  0 siblings, 2 replies; 24+ messages in thread
From: Boris Brezillon @ 2015-10-28 16:32 UTC (permalink / raw)
  To: Marek Vasut
  Cc: Brian Norris, linux-mtd, linux-kernel, Ezequiel Garcia,
	Scott Wood, Josh Wu, Robert Jarzmik, Kyungmin Park, Han Xu,
	Huang Shijie

Hi Marek,

On Wed, 28 Oct 2015 17:11:14 +0100
Marek Vasut <marex@denx.de> wrote:

> On Wednesday, October 28, 2015 at 08:58:13 AM, Boris Brezillon wrote:
> > Hi Brian,
> 
> Hi,
> 
> [...]
> 
> > > Are
> > > there ever cases we want more than one (master) MTD per nand_chip? Or
> > > vice versa?
> > 
> > Nope, I'd say that you always have a 1:1 relationship between a master
> > MTD device and a NAND device.
> 
> Do some sorts of chipselects come into play here ? Ie. you can have one master
> with multiple NAND chips connected to it.

Most NAND controllers support interacting with several chips (or
dies in case your chip embeds several NAND dies), but I keep thinking
each physical chip should have its own instance of nand_chip + mtd_info.
If you want to have a single mtd device aggregating several chips you
can use mtdconcat.

This leaves the multi-dies chip case, and IHMO we should represent those
chips as a single entity, and I guess that's the purpose of the
->numchips field in nand_chip (if your chip embeds 2 dies with 2 CS
lines, then ->numchips should be 2).

Anyway, I think the whole problem here is that most NAND drivers are
mixing the concepts of NAND controller (the controller driving one or
several NAND chips) and NAND chip (a chip connected to a NAND
controller).
The NAND controller should not be represented with a nand_chip
instance, but with a nand_hw_control instance, which is rarely done
except in a few drivers.

I sent an RFC a while ago [1] to clarify that, but didn't have time to
post a new version.

Best Regards,

Boris

[1]http://thread.gmane.org/gmane.linux.drivers.mtd/57614/focus=58552

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/5] mtd: ofpart: grab device tree node directly from master device node
  2015-10-28 16:32           ` Boris Brezillon
@ 2015-10-28 17:14             ` Brian Norris
  2015-10-28 20:55               ` Robert Jarzmik
  2015-10-28 20:38             ` Marek Vasut
  1 sibling, 1 reply; 24+ messages in thread
From: Brian Norris @ 2015-10-28 17:14 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Marek Vasut, linux-mtd, linux-kernel, Ezequiel Garcia,
	Scott Wood, Josh Wu, Robert Jarzmik, Kyungmin Park, Han Xu,
	Huang Shijie

On Wed, Oct 28, 2015 at 05:32:15PM +0100, Boris Brezillon wrote:
> On Wed, 28 Oct 2015 17:11:14 +0100
> Marek Vasut <marex@denx.de> wrote:
> > On Wednesday, October 28, 2015 at 08:58:13 AM, Boris Brezillon wrote:
> > > Hi Brian,
> > 
> > Hi,
> > 
> > [...]
> > 
> > > > Are
> > > > there ever cases we want more than one (master) MTD per nand_chip? Or
> > > > vice versa?
> > > 
> > > Nope, I'd say that you always have a 1:1 relationship between a master
> > > MTD device and a NAND device.
> > 
> > Do some sorts of chipselects come into play here ? Ie. you can have one master
> > with multiple NAND chips connected to it.
> 
> Most NAND controllers support interacting with several chips (or
> dies in case your chip embeds several NAND dies), but I keep thinking
> each physical chip should have its own instance of nand_chip + mtd_info.
> If you want to have a single mtd device aggregating several chips you
> can use mtdconcat.
> 
> This leaves the multi-dies chip case, and IHMO we should represent those
> chips as a single entity, and I guess that's the purpose of the
> ->numchips field in nand_chip (if your chip embeds 2 dies with 2 CS
> lines, then ->numchips should be 2).

Yes, I think that's some of the intention there. And so even in that
case, a multi-die chip gets represented as a single struct nand_chip.

Brian

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/5] mtd: ofpart: grab device tree node directly from master device node
  2015-10-28 16:32           ` Boris Brezillon
  2015-10-28 17:14             ` Brian Norris
@ 2015-10-28 20:38             ` Marek Vasut
  1 sibling, 0 replies; 24+ messages in thread
From: Marek Vasut @ 2015-10-28 20:38 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Brian Norris, linux-mtd, linux-kernel, Ezequiel Garcia,
	Scott Wood, Josh Wu, Robert Jarzmik, Kyungmin Park, Han Xu,
	Huang Shijie

On Wednesday, October 28, 2015 at 05:32:15 PM, Boris Brezillon wrote:
> Hi Marek,

Hi Boris,

> On Wed, 28 Oct 2015 17:11:14 +0100
> 
> Marek Vasut <marex@denx.de> wrote:
> > On Wednesday, October 28, 2015 at 08:58:13 AM, Boris Brezillon wrote:
> > > Hi Brian,
> > 
> > Hi,
> > 
> > [...]
> > 
> > > > Are
> > > > there ever cases we want more than one (master) MTD per nand_chip? Or
> > > > vice versa?
> > > 
> > > Nope, I'd say that you always have a 1:1 relationship between a master
> > > MTD device and a NAND device.
> > 
> > Do some sorts of chipselects come into play here ? Ie. you can have one
> > master with multiple NAND chips connected to it.
> 
> Most NAND controllers support interacting with several chips (or
> dies in case your chip embeds several NAND dies), but I keep thinking
> each physical chip should have its own instance of nand_chip + mtd_info.
> If you want to have a single mtd device aggregating several chips you
> can use mtdconcat.

I agree.

> This leaves the multi-dies chip case, and IHMO we should represent those
> chips as a single entity, and I guess that's the purpose of the
> ->numchips field in nand_chip (if your chip embeds 2 dies with 2 CS
> lines, then ->numchips should be 2).

I'd expect this to be represented as two physical chips, no ?

> Anyway, I think the whole problem here is that most NAND drivers are
> mixing the concepts of NAND controller (the controller driving one or
> several NAND chips) and NAND chip (a chip connected to a NAND
> controller).
> The NAND controller should not be represented with a nand_chip
> instance, but with a nand_hw_control instance, which is rarely done
> except in a few drivers.

OK, understood.

> I sent an RFC a while ago [1] to clarify that, but didn't have time to
> post a new version.
> 
> Best Regards,
> 
> Boris
> 
> [1]http://thread.gmane.org/gmane.linux.drivers.mtd/57614/focus=58552

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/5] mtd: ofpart: grab device tree node directly from master device node
  2015-10-28 17:14             ` Brian Norris
@ 2015-10-28 20:55               ` Robert Jarzmik
  2015-10-28 22:47                 ` Marek Vasut
  0 siblings, 1 reply; 24+ messages in thread
From: Robert Jarzmik @ 2015-10-28 20:55 UTC (permalink / raw)
  To: Brian Norris
  Cc: Boris Brezillon, Marek Vasut, linux-mtd, linux-kernel,
	Ezequiel Garcia, Scott Wood, Josh Wu, Kyungmin Park, Han Xu,
	Huang Shijie

Brian Norris <computersforpeace@gmail.com> writes:

>> > 
>> > Do some sorts of chipselects come into play here ? Ie. you can have one master
>> > with multiple NAND chips connected to it.
>> 
>> Most NAND controllers support interacting with several chips (or
>> dies in case your chip embeds several NAND dies), but I keep thinking
>> each physical chip should have its own instance of nand_chip + mtd_info.
>> If you want to have a single mtd device aggregating several chips you
>> can use mtdconcat.
>> 
>> This leaves the multi-dies chip case, and IHMO we should represent those
>> chips as a single entity, and I guess that's the purpose of the
>> ->numchips field in nand_chip (if your chip embeds 2 dies with 2 CS
>> lines, then ->numchips should be 2).
> Yes, I think that's some of the intention there. And so even in that
> case, a multi-die chip gets represented as a single struct nand_chip.

Isn't there the case of a single NAND controller with 2 identical chips, each a
8 bit NAND chip, and the controller aggregating them to offer the OS a single
16-bit NAND chip ?

In this case, the controller (pxa3xx is a good example) will be programmed to
handle both chips at the same time, and calculate CRC on both chips, etc ... I
hope the assertion "physical chip should have its own instance of nand_chip +
mtd_info" does take into account this example.

I don't know if there is actually any user of this for either pxa3xx or another
controller, nor if there is any value in this.

Cheers.

-- 
Robert

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/5] mtd: ofpart: grab device tree node directly from master device node
  2015-10-28 20:55               ` Robert Jarzmik
@ 2015-10-28 22:47                 ` Marek Vasut
  2015-10-29  6:32                   ` Robert Jarzmik
  0 siblings, 1 reply; 24+ messages in thread
From: Marek Vasut @ 2015-10-28 22:47 UTC (permalink / raw)
  To: Robert Jarzmik
  Cc: Brian Norris, Boris Brezillon, linux-mtd, linux-kernel,
	Ezequiel Garcia, Scott Wood, Josh Wu, Kyungmin Park, Han Xu,
	Huang Shijie

On Wednesday, October 28, 2015 at 09:55:24 PM, Robert Jarzmik wrote:
> Brian Norris <computersforpeace@gmail.com> writes:
> >> > Do some sorts of chipselects come into play here ? Ie. you can have
> >> > one master with multiple NAND chips connected to it.
> >> 
> >> Most NAND controllers support interacting with several chips (or
> >> dies in case your chip embeds several NAND dies), but I keep thinking
> >> each physical chip should have its own instance of nand_chip + mtd_info.
> >> If you want to have a single mtd device aggregating several chips you
> >> can use mtdconcat.
> >> 
> >> This leaves the multi-dies chip case, and IHMO we should represent those
> >> chips as a single entity, and I guess that's the purpose of the
> >> ->numchips field in nand_chip (if your chip embeds 2 dies with 2 CS
> >> lines, then ->numchips should be 2).
> > 
> > Yes, I think that's some of the intention there. And so even in that
> > case, a multi-die chip gets represented as a single struct nand_chip.
> 
> Isn't there the case of a single NAND controller with 2 identical chips,
> each a 8 bit NAND chip, and the controller aggregating them to offer the
> OS a single 16-bit NAND chip ?

Is that using 1 or 2 physical chipselect lines on the CPU (controller) ?

> In this case, the controller (pxa3xx is a good example) will be programmed
> to handle both chips at the same time, and calculate CRC on both chips,
> etc ... I hope the assertion "physical chip should have its own instance
> of nand_chip + mtd_info" does take into account this example.
> 
> I don't know if there is actually any user of this for either pxa3xx or
> another controller, nor if there is any value in this.
> 
> Cheers.

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/5] mtd: ofpart: grab device tree node directly from master device node
  2015-10-28 22:47                 ` Marek Vasut
@ 2015-10-29  6:32                   ` Robert Jarzmik
  2015-10-29  7:24                     ` Boris Brezillon
  0 siblings, 1 reply; 24+ messages in thread
From: Robert Jarzmik @ 2015-10-29  6:32 UTC (permalink / raw)
  To: Marek Vasut
  Cc: Brian Norris, Boris Brezillon, linux-mtd, linux-kernel,
	Ezequiel Garcia, Scott Wood, Josh Wu, Kyungmin Park, Han Xu,
	Huang Shijie

Marek Vasut <marex@denx.de> writes:

>> Isn't there the case of a single NAND controller with 2 identical chips,
>> each a 8 bit NAND chip, and the controller aggregating them to offer the
>> OS a single 16-bit NAND chip ?
>
> Is that using 1 or 2 physical chipselect lines on the CPU (controller) ?
I think it's 2 physical chipselects (CS0 and CS1). The way I understand it is
that the NAND controller asserts them both, issues the command on the command
line (shared between the 2 chips), and reads/writes on the separate data lines.

Cheers.

-- 
Robert

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/5] mtd: ofpart: grab device tree node directly from master device node
  2015-10-29  6:32                   ` Robert Jarzmik
@ 2015-10-29  7:24                     ` Boris Brezillon
  2015-10-29 17:23                       ` Marek Vasut
  0 siblings, 1 reply; 24+ messages in thread
From: Boris Brezillon @ 2015-10-29  7:24 UTC (permalink / raw)
  To: Robert Jarzmik
  Cc: Marek Vasut, Brian Norris, linux-mtd, linux-kernel,
	Ezequiel Garcia, Scott Wood, Josh Wu, Kyungmin Park, Han Xu,
	Huang Shijie

Hi Robert,

On Thu, 29 Oct 2015 07:32:33 +0100
Robert Jarzmik <robert.jarzmik@free.fr> wrote:

> Marek Vasut <marex@denx.de> writes:
> 
> >> Isn't there the case of a single NAND controller with 2 identical chips,
> >> each a 8 bit NAND chip, and the controller aggregating them to offer the
> >> OS a single 16-bit NAND chip ?

Honestly, I don't know how this can possibly work, do you have a real
example of that use case.

Here are a few reasons making it impossible:

1/ NAND are accessed using specific command sequences, and those
commands and addresses cycles are sent on through the data bus (AFAIR
only the lower 8bits of a 16bits bus are used for those
command/address cycles), so even if you connect the CLE/ALE/CS/RB pins
on both chips, the one connected on the MSB side of the data bus will
just receive garbage during the command/address sequences, and your
program/read operations won't work

2/ NAND chips can have bad blocks, so even if you were able to address
2 chips (which according to #1 is impossible), you might try to write
on a bad block on the chip connected on the MSB side of the data bus.

3/ There probably are plenty of other reasons why this is not
possible ;-).

Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/5] mtd: ofpart: grab device tree node directly from master device node
  2015-10-29  7:24                     ` Boris Brezillon
@ 2015-10-29 17:23                       ` Marek Vasut
  2015-10-29 17:34                         ` Boris Brezillon
  0 siblings, 1 reply; 24+ messages in thread
From: Marek Vasut @ 2015-10-29 17:23 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Robert Jarzmik, Brian Norris, linux-mtd, linux-kernel,
	Ezequiel Garcia, Scott Wood, Josh Wu, Kyungmin Park, Han Xu,
	Huang Shijie

On Thursday, October 29, 2015 at 08:24:48 AM, Boris Brezillon wrote:
> Hi Robert,

Hi!

> On Thu, 29 Oct 2015 07:32:33 +0100
> 
> Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> > Marek Vasut <marex@denx.de> writes:
> > >> Isn't there the case of a single NAND controller with 2 identical
> > >> chips, each a 8 bit NAND chip, and the controller aggregating them to
> > >> offer the OS a single 16-bit NAND chip ?
> 
> Honestly, I don't know how this can possibly work, do you have a real
> example of that use case.
> 
> Here are a few reasons making it impossible:
> 
> 1/ NAND are accessed using specific command sequences, and those
> commands and addresses cycles are sent on through the data bus (AFAIR
> only the lower 8bits of a 16bits bus are used for those
> command/address cycles), so even if you connect the CLE/ALE/CS/RB pins
> on both chips, the one connected on the MSB side of the data bus will
> just receive garbage during the command/address sequences, and your
> program/read operations won't work

Unless you duplicate the command to both MSB and LSB.

> 2/ NAND chips can have bad blocks, so even if you were able to address
> 2 chips (which according to #1 is impossible), you might try to write
> on a bad block on the chip connected on the MSB side of the data bus.

This one is a valid problem. The other valid issue here is where the
command might fail on one chip and pass on the other.

> 3/ There probably are plenty of other reasons why this is not
> possible ;-).

It's possible, implementable, but a really bad idea.

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/5] mtd: ofpart: grab device tree node directly from master device node
  2015-10-29 17:23                       ` Marek Vasut
@ 2015-10-29 17:34                         ` Boris Brezillon
  2015-10-29 20:28                           ` Robert Jarzmik
  0 siblings, 1 reply; 24+ messages in thread
From: Boris Brezillon @ 2015-10-29 17:34 UTC (permalink / raw)
  To: Marek Vasut
  Cc: Robert Jarzmik, Brian Norris, linux-mtd, linux-kernel,
	Ezequiel Garcia, Scott Wood, Josh Wu, Kyungmin Park, Han Xu,
	Huang Shijie

On Thu, 29 Oct 2015 18:23:47 +0100
Marek Vasut <marex@denx.de> wrote:

> On Thursday, October 29, 2015 at 08:24:48 AM, Boris Brezillon wrote:
> > Hi Robert,
> 
> Hi!
> 
> > On Thu, 29 Oct 2015 07:32:33 +0100
> > 
> > Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> > > Marek Vasut <marex@denx.de> writes:
> > > >> Isn't there the case of a single NAND controller with 2 identical
> > > >> chips, each a 8 bit NAND chip, and the controller aggregating them to
> > > >> offer the OS a single 16-bit NAND chip ?
> > 
> > Honestly, I don't know how this can possibly work, do you have a real
> > example of that use case.
> > 
> > Here are a few reasons making it impossible:
> > 
> > 1/ NAND are accessed using specific command sequences, and those
> > commands and addresses cycles are sent on through the data bus (AFAIR
> > only the lower 8bits of a 16bits bus are used for those
> > command/address cycles), so even if you connect the CLE/ALE/CS/RB pins
> > on both chips, the one connected on the MSB side of the data bus will
> > just receive garbage during the command/address sequences, and your
> > program/read operations won't work
> 
> Unless you duplicate the command to both MSB and LSB.

Except it's now how devices supporting 16 bits data bus are supposed to
work, which means your NAND controller will probably not be able to
send the command/address value on the higher 8 bits...

> 
> > 2/ NAND chips can have bad blocks, so even if you were able to address
> > 2 chips (which according to #1 is impossible), you might try to write
> > on a bad block on the chip connected on the MSB side of the data bus.
> 
> This one is a valid problem. The other valid issue here is where the
> command might fail on one chip and pass on the other.
> 
> > 3/ There probably are plenty of other reasons why this is not
> > possible ;-).
> 
> It's possible, implementable, but a really bad idea.

Possible and implementable, maybe with an adapted software stack
and a customized NAND controller. I know you're working on emulating
flash devices using FPGA, so the next step is to create a new NAND
controller IP supporting that kind of stuff and adding support for
this feature to the NAND framework ;-).
Anyway, it"s definitely a bad idea.

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/5] mtd: ofpart: grab device tree node directly from master device node
  2015-10-29 17:34                         ` Boris Brezillon
@ 2015-10-29 20:28                           ` Robert Jarzmik
  0 siblings, 0 replies; 24+ messages in thread
From: Robert Jarzmik @ 2015-10-29 20:28 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Marek Vasut, Brian Norris, linux-mtd, linux-kernel,
	Ezequiel Garcia, Scott Wood, Josh Wu, Kyungmin Park, Han Xu,
	Huang Shijie

Boris Brezillon <boris.brezillon@free-electrons.com> writes:

> On Thu, 29 Oct 2015 18:23:47 +0100
> Marek Vasut <marex@denx.de> wrote:
> Except it's now how devices supporting 16 bits data bus are supposed to
> work, which means your NAND controller will probably not be able to
> send the command/address value on the higher 8 bits...
Correct.

>> > 2/ NAND chips can have bad blocks, so even if you were able to address
>> > 2 chips (which according to #1 is impossible), you might try to write
>> > on a bad block on the chip connected on the MSB side of the data bus.
>> 
>> This one is a valid problem. The other valid issue here is where the
>> command might fail on one chip and pass on the other.
Probably the bad block wins, and the command is reported as a failure.

>> > 3/ There probably are plenty of other reasons why this is not
>> > possible ;-).
>> 
>> It's possible, implementable, but a really bad idea.
>
> Possible and implementable, maybe with an adapted software stack
> and a customized NAND controller. I know you're working on emulating
> flash devices using FPGA, so the next step is to create a new NAND
> controller IP supporting that kind of stuff and adding support for
> this feature to the NAND framework ;-).
> Anyway, it"s definitely a bad idea.
I agree, a bad idea.

Just to finish the discussion, let me quote the pxa3xx developper manual volume
2, chapter 3.7.6.1 "Flash Memory Data Width when Two Flash Devices Connect to
the Same Chip" :
    When NDCR[DWIDTH_C] = 1 and NDCR[DWIDTH_M] = 0, two flash devices are
    connected to the same chip select with DF_IO<7:0> interfacing to one device,
    and DF_IO<15:8> interfacing to the second device. In this scenario, since
    two devices are accessed simultaneously, the command and address are
    replicated between the lower and upper byte of the interface.

Now, regardless of this, please continue with your patches, this corner case is
not important, at least I don't care at all.

Cheers.

-- 
Robert

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2015-10-29 20:34 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-27  2:31 [PATCH 0/5] mtd: migrate 'of_node' handling to core, not in mtd_part_parser_data Brian Norris
2015-10-27  2:31 ` [PATCH 1/5] mtd: ofpart: grab device tree node directly from master device node Brian Norris
2015-10-27  7:42   ` Boris Brezillon
2015-10-27 17:54     ` Brian Norris
2015-10-28  1:01       ` Brian Norris
2015-10-28  8:02         ` Boris Brezillon
2015-10-28  7:58       ` Boris Brezillon
2015-10-28 16:11         ` Marek Vasut
2015-10-28 16:32           ` Boris Brezillon
2015-10-28 17:14             ` Brian Norris
2015-10-28 20:55               ` Robert Jarzmik
2015-10-28 22:47                 ` Marek Vasut
2015-10-29  6:32                   ` Robert Jarzmik
2015-10-29  7:24                     ` Boris Brezillon
2015-10-29 17:23                       ` Marek Vasut
2015-10-29 17:34                         ` Boris Brezillon
2015-10-29 20:28                           ` Robert Jarzmik
2015-10-28 20:38             ` Marek Vasut
2015-10-27  2:31 ` [PATCH 2/5] mtd: nand: drop unnecessary partition parser data Brian Norris
2015-10-27  7:52   ` Boris Brezillon
2015-10-28  0:45   ` Brian Norris
2015-10-27  2:31 ` [PATCH 3/5] mtd: spi-nor: " Brian Norris
2015-10-27  2:31 ` [PATCH 4/5] mtd: " Brian Norris
2015-10-27  2:31 ` [PATCH 5/5] mtd: drop 'of_node' " Brian Norris

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).