From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.free-electrons.com ([62.4.15.54]) by bombadil.infradead.org with esmtp (Exim 4.89 #1 (Red Hat Linux)) id 1eZcrI-0001CT-EW for linux-mtd@lists.infradead.org; Thu, 11 Jan 2018 13:25:30 +0000 Date: Thu, 11 Jan 2018 14:25:06 +0100 From: Boris Brezillon To: Fabio Estevam Cc: , , , , Subject: Re: [PATCH v2] mtd: fsl-quadspi: Distinguish the mtd device names Message-ID: <20180111142506.13a05281@bbrezillon> In-Reply-To: <1515676504-11510-1-git-send-email-fabio.estevam@nxp.com> References: <1515676504-11510-1-git-send-email-fabio.estevam@nxp.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Thu, 11 Jan 2018 11:15:04 -0200 Fabio Estevam wrote: > Currently on a imx6sx-sdb board, which has two SPI NOR chips connected > to QSPI2 the following output from /proc/mtd is seen: > > # cat /proc/mtd > dev: size erasesize name > mtd0: 01000000 00010000 "21e4000.qspi" > mtd1: 01000000 00010000 "21e4000.qspi" > > Attempts to partition them on the kernel command line result in both > chips with identical (and identically named) partitions, which is > an inconvenient behavior. > > Assign a different mtd->name for each mtd device to avoid this problem. > > After this change the output from /proc/mtd becomes: > > # cat /proc/mtd > dev: size erasesize name > mtd0: 01000000 00010000 "21e4000.qspi-0" > mtd1: 01000000 00010000 "21e4000.qspi-1" > > Reported-by: David Wolfe > Signed-off-by: Fabio Estevam > --- > Changes since v1: > - Use 'spiflash_idx' for the variable name > - Use the mtd->name in the format 21e4000.qspi-0 > - Do not assing the mtd name relying on the dts ordering and use the reg > property instead > - Call devm_kasprintf() after spi_nor_set_flash_node(). > > drivers/mtd/spi-nor/fsl-quadspi.c | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c > index 2901c7b..b28ad8b 100644 > --- a/drivers/mtd/spi-nor/fsl-quadspi.c > +++ b/drivers/mtd/spi-nor/fsl-quadspi.c > @@ -967,7 +967,7 @@ static int fsl_qspi_probe(struct platform_device *pdev) > struct resource *res; > struct spi_nor *nor; > struct mtd_info *mtd; > - int ret, i = 0; > + int ret, i = 0, spiflash_idx; > > q = devm_kzalloc(dev, sizeof(*q), GFP_KERNEL); > if (!q) > @@ -1051,6 +1051,22 @@ static int fsl_qspi_probe(struct platform_device *pdev) > spi_nor_set_flash_node(nor, np); > nor->priv = q; > > + ret = of_property_read_u32(np, "reg", &spiflash_idx); > + if (ret) { > + dev_err(dev, "could not get reg property: %d\n", ret); > + ret = -EINVAL; > + goto mutex_failed; Was reg already mandatory before this commit? If it's not the case, you're breaking backward compat. > + } You can move the above block > + > + if (!mtd->name) { here. > + mtd->name = devm_kasprintf(dev, GFP_KERNEL, "%s-%d", > + dev_name(dev), spiflash_idx); By changing the name you're breaking existing users that pass mtdparts=21e4000.qspi: :-(. There's no easy solution for the problem you're trying to solve, but maybe you can make reg optional, and if it's not present set mtd->name to dev_name(dev). And, you should complain when you have more than one child node and reg is missing. > + if (!mtd->name) { > + ret = -ENOMEM; > + goto mutex_failed; > + } > + } > + > /* fill the hooks */ > nor->read_reg = fsl_qspi_read_reg; > nor->write_reg = fsl_qspi_write_reg;