[PATCH 00/13] mtd: nand: fsmc: fixes, improvements and cleanups

[PATCH 00/13] mtd: nand: fsmc: fixes, improvements and cleanups

[Thomas Petazzoni]

Hello,

This patch series brings a number of fixes, improvements and cleanups
to the fsmc MTD NAND driver.

The first patch is really a fix for the NAND bus width selection
through the Device Tree, which was broken since the Device Tree
support was added in the driver.

The next two patches add support for configuring the NAND controller
timings according to the connected NAND chip automatically.

The remaining patches are basically cleanups, removing useless code
and logic from the driver.

This patch series is based on v4.11-rc1.

Best regards,

Thomas

Thomas Petazzoni (13):
  mtd: nand: fsmc: fix NAND width handling
  mtd: nand: fsmc: rework fsmc_nand_setup() to use
    ->setup_data_interface()
  mtd: nand: fsmc: add support to use NAND timings
  mtd: nand: fsmc: move fsmc_nand_data definition
  mtd: nand: fsmc: remove ->select_bank() from fsmc_nand_platform_data
  mtd: nand: fsmc: remove fsmc_select_chip()
  mtd: nand: fmsc: kill {read,write}_dma_priv from
    fsmc_nand_platform_data
  mtd: nand: fsmc: kill {nr_,}partitions structure fields
  mtd: nand: fsmc: remove duplicate nand_set_flash_node()
  mtd: nand: fsmc: finally remove fsmc_nand_platform_data
  mtd: nand: fsmc: use devm_clk_get()
  mtd: nand: fsmc: remove unused definitions
  mtd: nand: fsmc: remove CONFIG_OF conditional

 drivers/mtd/nand/fsmc_nand.c | 342 ++++++++++++++++++++-----------------------
 1 file changed, 156 insertions(+), 186 deletions(-)

-- 
2.7.4

[PATCH 01/13] mtd: nand: fsmc: fix NAND width handling

[Thomas Petazzoni]

In commit eea628199d5b ("mtd: Add device-tree support to fsmc_nand"),
Device Tree support was added to the fmsc_nand driver. However, this
code has a bug in how it handles the bank-width DT property to set the
bus width.

Indeed, in the function fsmc_nand_probe_config_dt() that parses the
Device Tree, it sets pdata->width to either 8 or 16 depending on the
value of the bank-width DT property.

Then, the ->probe() function will test if pdata->width is equal to
FSMC_NAND_BW16 (which is 2) to set NAND_BUSWIDTH_16 in
nand->options. Therefore, with the DT probing, this condition will never
match.

This commit fixes that by removing the "width" field from
fsmc_nand_platform_data and instead have the fsmc_nand_probe_config_dt()
function directly set the appropriate nand->options value.

It is worth mentioning that if this commit gets backported to older
kernels, prior to the drop of non-DT probing, then non-DT probing will
be broken because nand->options will no longer be set to
NAND_BUSWIDTH_16.

Fixes: eea628199d5b ("mtd: Add device-tree support to fsmc_nand")
Cc: <stable@vger.kernel.org>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/mtd/nand/fsmc_nand.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c
index bda1e46..66aece9 100644
--- a/drivers/mtd/nand/fsmc_nand.c
+++ b/drivers/mtd/nand/fsmc_nand.c
@@ -150,7 +150,6 @@ struct fsmc_nand_platform_data {
 	struct mtd_partition	*partitions;
 	unsigned int		nr_partitions;
 	unsigned int		options;
-	unsigned int		width;
 	unsigned int		bank;
 
 	enum access_mode	mode;
@@ -844,18 +843,19 @@ static int fsmc_nand_probe_config_dt(struct platform_device *pdev,
 	u32 val;
 	int ret;
 
-	/* Set default NAND width to 8 bits */
-	pdata->width = 8;
+	pdata->options = 0;
+
 	if (!of_property_read_u32(np, "bank-width", &val)) {
 		if (val == 2) {
-			pdata->width = 16;
+			pdata->options |= NAND_BUSWIDTH_16;
 		} else if (val != 1) {
 			dev_err(&pdev->dev, "invalid bank-width %u\n", val);
 			return -EINVAL;
 		}
 	}
+
 	if (of_get_property(np, "nand-skip-bbtscan", NULL))
-		pdata->options = NAND_SKIP_BBTSCAN;
+		pdata->options |= NAND_SKIP_BBTSCAN;
 
 	pdata->nand_timings = devm_kzalloc(&pdev->dev,
 				sizeof(*pdata->nand_timings), GFP_KERNEL);
@@ -992,9 +992,6 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
 	nand->badblockbits = 7;
 	nand_set_flash_node(nand, np);
 
-	if (pdata->width == FSMC_NAND_BW16)
-		nand->options |= NAND_BUSWIDTH_16;
-
 	switch (host->mode) {
 	case USE_DMA_ACCESS:
 		dma_cap_zero(mask);
-- 
2.7.4

Re: [PATCH 01/13] mtd: nand: fsmc: fix NAND width handling

[Boris Brezillon]

+Greg for the backport to stable question.

On Tue, 21 Mar 2017 11:03:53 +0100
Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:

> In commit eea628199d5b ("mtd: Add device-tree support to fsmc_nand"),
> Device Tree support was added to the fmsc_nand driver. However, this
> code has a bug in how it handles the bank-width DT property to set the
> bus width.
> 
> Indeed, in the function fsmc_nand_probe_config_dt() that parses the
> Device Tree, it sets pdata->width to either 8 or 16 depending on the
> value of the bank-width DT property.
> 
> Then, the ->probe() function will test if pdata->width is equal to
> FSMC_NAND_BW16 (which is 2) to set NAND_BUSWIDTH_16 in
> nand->options. Therefore, with the DT probing, this condition will never
> match.
> 
> This commit fixes that by removing the "width" field from
> fsmc_nand_platform_data and instead have the fsmc_nand_probe_config_dt()
> function directly set the appropriate nand->options value.
> 
> It is worth mentioning that if this commit gets backported to older
> kernels, prior to the drop of non-DT probing, then non-DT probing will
> be broken because nand->options will no longer be set to
> NAND_BUSWIDTH_16.

Then maybe we should the drop the Cc-stable tag, or put # vX.Y+ to
prevent this patch from being applied to versions where it could break
things.

Note that no-one complained about this bug so far, so I guess no-one
cares about this fix (probably because 16-bits NANDs are not widely
used) ;-).

> 
> Fixes: eea628199d5b ("mtd: Add device-tree support to fsmc_nand")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  drivers/mtd/nand/fsmc_nand.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c
> index bda1e46..66aece9 100644
> --- a/drivers/mtd/nand/fsmc_nand.c
> +++ b/drivers/mtd/nand/fsmc_nand.c
> @@ -150,7 +150,6 @@ struct fsmc_nand_platform_data {
>  	struct mtd_partition	*partitions;
>  	unsigned int		nr_partitions;
>  	unsigned int		options;
> -	unsigned int		width;
>  	unsigned int		bank;
>  
>  	enum access_mode	mode;
> @@ -844,18 +843,19 @@ static int fsmc_nand_probe_config_dt(struct platform_device *pdev,
>  	u32 val;
>  	int ret;
>  
> -	/* Set default NAND width to 8 bits */
> -	pdata->width = 8;
> +	pdata->options = 0;
> +
>  	if (!of_property_read_u32(np, "bank-width", &val)) {
>  		if (val == 2) {
> -			pdata->width = 16;
> +			pdata->options |= NAND_BUSWIDTH_16;
>  		} else if (val != 1) {
>  			dev_err(&pdev->dev, "invalid bank-width %u\n", val);
>  			return -EINVAL;
>  		}
>  	}
> +
>  	if (of_get_property(np, "nand-skip-bbtscan", NULL))
> -		pdata->options = NAND_SKIP_BBTSCAN;
> +		pdata->options |= NAND_SKIP_BBTSCAN;
>  
>  	pdata->nand_timings = devm_kzalloc(&pdev->dev,
>  				sizeof(*pdata->nand_timings), GFP_KERNEL);
> @@ -992,9 +992,6 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
>  	nand->badblockbits = 7;
>  	nand_set_flash_node(nand, np);
>  
> -	if (pdata->width == FSMC_NAND_BW16)
> -		nand->options |= NAND_BUSWIDTH_16;
> -
>  	switch (host->mode) {
>  	case USE_DMA_ACCESS:
>  		dma_cap_zero(mask);

Re: [PATCH 01/13] mtd: nand: fsmc: fix NAND width handling

[Thomas Petazzoni]

Hello,

On Wed, 22 Mar 2017 22:42:37 +0100, Boris Brezillon wrote:

> > It is worth mentioning that if this commit gets backported to older
> > kernels, prior to the drop of non-DT probing, then non-DT probing will
> > be broken because nand->options will no longer be set to
> > NAND_BUSWIDTH_16.  
> 
> Then maybe we should the drop the Cc-stable tag, or put # vX.Y+ to
> prevent this patch from being applied to versions where it could break
> things.
> 
> Note that no-one complained about this bug so far, so I guess no-one
> cares about this fix (probably because 16-bits NANDs are not widely
> used) ;-).

It's really up to you. I didn't encounter the bug myself, I just found
out the mistake by reading the code and cooking this cleanup series.

Feel free to drop the Cc: to stable when applying, I don't particularly
care about this specific feature, and since no-one noticed so far, I
doubt anyone cares :)

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH 01/13] mtd: nand: fsmc: fix NAND width handling

[Linus Walleij]

On Tue, Mar 21, 2017 at 11:03 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:

> In commit eea628199d5b ("mtd: Add device-tree support to fsmc_nand"),
> Device Tree support was added to the fmsc_nand driver. However, this
> code has a bug in how it handles the bank-width DT property to set the
> bus width.
>
> Indeed, in the function fsmc_nand_probe_config_dt() that parses the
> Device Tree, it sets pdata->width to either 8 or 16 depending on the
> value of the bank-width DT property.
>
> Then, the ->probe() function will test if pdata->width is equal to
> FSMC_NAND_BW16 (which is 2) to set NAND_BUSWIDTH_16 in
> nand->options. Therefore, with the DT probing, this condition will never
> match.
>
> This commit fixes that by removing the "width" field from
> fsmc_nand_platform_data and instead have the fsmc_nand_probe_config_dt()
> function directly set the appropriate nand->options value.
>
> It is worth mentioning that if this commit gets backported to older
> kernels, prior to the drop of non-DT probing, then non-DT probing will
> be broken because nand->options will no longer be set to
> NAND_BUSWIDTH_16.
>
> Fixes: eea628199d5b ("mtd: Add device-tree support to fsmc_nand")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Good catch.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 02/13] mtd: nand: fsmc: rework fsmc_nand_setup() to use ->setup_data_interface()

[Thomas Petazzoni]

Since a few kernel versions, the NAND subsystem has introduced a
callback called ->setup_data_interface(), which NAND controller drivers
are supposed to use to perform all the NAND data interface.

This commit therefore converts the fsmc_nand driver to use this callback
instead of the home-grown fsmc_nand_setup() function.

For now, the NAND timings are not used, and we simply use the timings
provided from the Device Tree, or otherwise the default timings. Support
for the NAND timings is added in a follow-up patch.

The call to fsmc_nand_setup() in the ->resume() hook is simply replaced
by a call to nand_reset(), which internally will call the
->setup_data_interface() callback.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/mtd/nand/fsmc_nand.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c
index 66aece9..4a98433 100644
--- a/drivers/mtd/nand/fsmc_nand.c
+++ b/drivers/mtd/nand/fsmc_nand.c
@@ -370,9 +370,12 @@ static void fsmc_cmd_ctrl(struct mtd_info *mtd, int cmd, unsigned int ctrl)
  * This routine initializes timing parameters related to NAND memory access in
  * FSMC registers
  */
-static void fsmc_nand_setup(void __iomem *regs, uint32_t bank,
-			   uint32_t busw, struct fsmc_nand_timings *timings)
+static int fsmc_setup_data_interface(struct mtd_info *mtd,
+				     const struct nand_data_interface *conf,
+				     bool check_only)
 {
+	struct nand_chip *nand = mtd_to_nand(mtd);
+	struct fsmc_nand_data *host = nand_get_controller_data(nand);
 	uint32_t value = FSMC_DEVTYPE_NAND | FSMC_ENABLE | FSMC_WAITON;
 	uint32_t tclr, tar, thiz, thold, twait, tset;
 	struct fsmc_nand_timings *tims;
@@ -384,12 +387,18 @@ static void fsmc_nand_setup(void __iomem *regs, uint32_t bank,
 		.twait	= FSMC_TWAIT_6,
 		.tset	= FSMC_TSET_0,
 	};
+	unsigned int bank = host->bank;
+	void __iomem *regs = host->regs_va;
+	int ret;
 
-	if (timings)
-		tims = timings;
+	if (host->dev_timings)
+		tims = host->dev_timings;
 	else
 		tims = &default_timings;
 
+	if (check_only)
+		return 0;
+
 	tclr = (tims->tclr & FSMC_TCLR_MASK) << FSMC_TCLR_SHIFT;
 	tar = (tims->tar & FSMC_TAR_MASK) << FSMC_TAR_SHIFT;
 	thiz = (tims->thiz & FSMC_THIZ_MASK) << FSMC_THIZ_SHIFT;
@@ -397,7 +406,7 @@ static void fsmc_nand_setup(void __iomem *regs, uint32_t bank,
 	twait = (tims->twait & FSMC_TWAIT_MASK) << FSMC_TWAIT_SHIFT;
 	tset = (tims->tset & FSMC_TSET_MASK) << FSMC_TSET_SHIFT;
 
-	if (busw)
+	if (nand->options & NAND_BUSWIDTH_16)
 		writel_relaxed(value | FSMC_DEVWID_16,
 				FSMC_NAND_REG(regs, bank, PC));
 	else
@@ -410,6 +419,8 @@ static void fsmc_nand_setup(void __iomem *regs, uint32_t bank,
 			FSMC_NAND_REG(regs, bank, COMM));
 	writel_relaxed(thiz | thold | twait | tset,
 			FSMC_NAND_REG(regs, bank, ATTRIB));
+
+	return 0;
 }
 
 /*
@@ -991,6 +1002,7 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
 	nand->select_chip = fsmc_select_chip;
 	nand->badblockbits = 7;
 	nand_set_flash_node(nand, np);
+	nand->setup_data_interface = fsmc_setup_data_interface;
 
 	switch (host->mode) {
 	case USE_DMA_ACCESS:
@@ -1019,10 +1031,6 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
 		break;
 	}
 
-	fsmc_nand_setup(host->regs_va, host->bank,
-			nand->options & NAND_BUSWIDTH_16,
-			host->dev_timings);
-
 	if (AMBA_REV_BITS(host->pid) >= 8) {
 		nand->ecc.read_page = fsmc_read_page_hwecc;
 		nand->ecc.calculate = fsmc_read_hwecc_ecc4;
@@ -1121,6 +1129,7 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, host);
 	dev_info(&pdev->dev, "FSMC NAND driver registration successful\n");
+
 	return 0;
 
 err_probe:
@@ -1172,9 +1181,7 @@ static int fsmc_nand_resume(struct device *dev)
 	struct fsmc_nand_data *host = dev_get_drvdata(dev);
 	if (host) {
 		clk_prepare_enable(host->clk);
-		fsmc_nand_setup(host->regs_va, host->bank,
-				host->nand.options & NAND_BUSWIDTH_16,
-				host->dev_timings);
+		nand_reset(&host->nand, 0);
 	}
 	return 0;
 }
-- 
2.7.4

Re: [PATCH 02/13] mtd: nand: fsmc: rework fsmc_nand_setup() to use ->setup_data_interface()

[Boris Brezillon]

On Tue, 21 Mar 2017 11:03:54 +0100
Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:

> Since a few kernel versions, the NAND subsystem has introduced a
> callback called ->setup_data_interface(), which NAND controller drivers
> are supposed to use to perform all the NAND data interface.
> 
> This commit therefore converts the fsmc_nand driver to use this callback
> instead of the home-grown fsmc_nand_setup() function.
> 
> For now, the NAND timings are not used, and we simply use the timings
> provided from the Device Tree, or otherwise the default timings. Support
> for the NAND timings is added in a follow-up patch.
> 
> The call to fsmc_nand_setup() in the ->resume() hook is simply replaced
> by a call to nand_reset(), which internally will call the
> ->setup_data_interface() callback.  
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  drivers/mtd/nand/fsmc_nand.c | 31 +++++++++++++++++++------------
>  1 file changed, 19 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c
> index 66aece9..4a98433 100644
> --- a/drivers/mtd/nand/fsmc_nand.c
> +++ b/drivers/mtd/nand/fsmc_nand.c
> @@ -370,9 +370,12 @@ static void fsmc_cmd_ctrl(struct mtd_info *mtd, int cmd, unsigned int ctrl)
>   * This routine initializes timing parameters related to NAND memory access in
>   * FSMC registers
>   */
> -static void fsmc_nand_setup(void __iomem *regs, uint32_t bank,
> -			   uint32_t busw, struct fsmc_nand_timings *timings)
> +static int fsmc_setup_data_interface(struct mtd_info *mtd,
> +				     const struct nand_data_interface *conf,
> +				     bool check_only)
>  {
> +	struct nand_chip *nand = mtd_to_nand(mtd);
> +	struct fsmc_nand_data *host = nand_get_controller_data(nand);
>  	uint32_t value = FSMC_DEVTYPE_NAND | FSMC_ENABLE | FSMC_WAITON;
>  	uint32_t tclr, tar, thiz, thold, twait, tset;
>  	struct fsmc_nand_timings *tims;
> @@ -384,12 +387,18 @@ static void fsmc_nand_setup(void __iomem *regs, uint32_t bank,
>  		.twait	= FSMC_TWAIT_6,
>  		.tset	= FSMC_TSET_0,
>  	};
> +	unsigned int bank = host->bank;
> +	void __iomem *regs = host->regs_va;
> +	int ret;
>  
> -	if (timings)
> -		tims = timings;
> +	if (host->dev_timings)
> +		tims = host->dev_timings;
>  	else
>  		tims = &default_timings;
>  
> +	if (check_only)
> +		return 0;
> +

I'm not sure this is such a good idea to move default and DT timings
setting in the ->setup_data_interface() hook.

ONFI NANDs are changing an internal parameter to switch to a specific
timing mode. If you let the core think that you configured the
controller to support this timing mode, while you actually configured
it with the default or DT timings it might not work as expected.

>  	tclr = (tims->tclr & FSMC_TCLR_MASK) << FSMC_TCLR_SHIFT;
>  	tar = (tims->tar & FSMC_TAR_MASK) << FSMC_TAR_SHIFT;
>  	thiz = (tims->thiz & FSMC_THIZ_MASK) << FSMC_THIZ_SHIFT;
> @@ -397,7 +406,7 @@ static void fsmc_nand_setup(void __iomem *regs, uint32_t bank,
>  	twait = (tims->twait & FSMC_TWAIT_MASK) << FSMC_TWAIT_SHIFT;
>  	tset = (tims->tset & FSMC_TSET_MASK) << FSMC_TSET_SHIFT;
>  
> -	if (busw)
> +	if (nand->options & NAND_BUSWIDTH_16)
>  		writel_relaxed(value | FSMC_DEVWID_16,
>  				FSMC_NAND_REG(regs, bank, PC));
>  	else
> @@ -410,6 +419,8 @@ static void fsmc_nand_setup(void __iomem *regs, uint32_t bank,
>  			FSMC_NAND_REG(regs, bank, COMM));
>  	writel_relaxed(thiz | thold | twait | tset,
>  			FSMC_NAND_REG(regs, bank, ATTRIB));
> +
> +	return 0;
>  }
>  
>  /*
> @@ -991,6 +1002,7 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
>  	nand->select_chip = fsmc_select_chip;
>  	nand->badblockbits = 7;
>  	nand_set_flash_node(nand, np);
> +	nand->setup_data_interface = fsmc_setup_data_interface;
>  
>  	switch (host->mode) {
>  	case USE_DMA_ACCESS:
> @@ -1019,10 +1031,6 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
>  		break;
>  	}
>  
> -	fsmc_nand_setup(host->regs_va, host->bank,
> -			nand->options & NAND_BUSWIDTH_16,
> -			host->dev_timings);
> -
>  	if (AMBA_REV_BITS(host->pid) >= 8) {
>  		nand->ecc.read_page = fsmc_read_page_hwecc;
>  		nand->ecc.calculate = fsmc_read_hwecc_ecc4;
> @@ -1121,6 +1129,7 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, host);
>  	dev_info(&pdev->dev, "FSMC NAND driver registration successful\n");
> +
>  	return 0;
>  
>  err_probe:
> @@ -1172,9 +1181,7 @@ static int fsmc_nand_resume(struct device *dev)
>  	struct fsmc_nand_data *host = dev_get_drvdata(dev);
>  	if (host) {
>  		clk_prepare_enable(host->clk);
> -		fsmc_nand_setup(host->regs_va, host->bank,
> -				host->nand.options & NAND_BUSWIDTH_16,
> -				host->dev_timings);
> +		nand_reset(&host->nand, 0);
>  	}
>  	return 0;
>  }

Re: [PATCH 02/13] mtd: nand: fsmc: rework fsmc_nand_setup() to use ->setup_data_interface()

[Thomas Petazzoni]

Hello,

On Wed, 22 Mar 2017 22:56:17 +0100, Boris Brezillon wrote:

> I'm not sure this is such a good idea to move default and DT timings
> setting in the ->setup_data_interface() hook.
> 
> ONFI NANDs are changing an internal parameter to switch to a specific
> timing mode. If you let the core think that you configured the
> controller to support this timing mode, while you actually configured
> it with the default or DT timings it might not work as expected.

So what do you suggest to keep the compatibility with the existing DT
binding for this NAND controller?

We also need to take into account that the timings need to be
reconfigured upon ->resume().

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH 02/13] mtd: nand: fsmc: rework fsmc_nand_setup() to use ->setup_data_interface()

[Boris Brezillon]

On Wed, 22 Mar 2017 23:05:53 +0100
Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:

> Hello,
> 
> On Wed, 22 Mar 2017 22:56:17 +0100, Boris Brezillon wrote:
> 
> > I'm not sure this is such a good idea to move default and DT timings
> > setting in the ->setup_data_interface() hook.
> > 
> > ONFI NANDs are changing an internal parameter to switch to a specific
> > timing mode. If you let the core think that you configured the
> > controller to support this timing mode, while you actually configured
> > it with the default or DT timings it might not work as expected.  
> 
> So what do you suggest to keep the compatibility with the existing DT
> binding for this NAND controller?

We have 2 choices here:
1/ drop the old/static timings config and only rely on the dynamic
   config. This is a bit risky, because we've only tested the new
   timing conversion logic on one board.
2/ keep both solutions around until all boards have switched to the new
   solution. In this case, you'll only set the ->setup_data_interface()
   hook if timings are not defined in the DT

> 
> We also need to take into account that the timings need to be
> reconfigured upon ->resume().

Yes, I know, and I also know how painful it is to add extra code for
backward compat, but I think abusing ->setup_data_interface() is even
worse.

To sum-up, if we go for solution #2, in your resume you'll have to
conditionally call the legacy timing setup logic (only if timings are
defined in the DT) in addition to the nand_reset() call.

Re: [PATCH 02/13] mtd: nand: fsmc: rework fsmc_nand_setup() to use ->setup_data_interface()

[Thomas Petazzoni]

Hello,

On Wed, 22 Mar 2017 23:23:33 +0100, Boris Brezillon wrote:

> > So what do you suggest to keep the compatibility with the existing DT
> > binding for this NAND controller?  
> 
> We have 2 choices here:
> 1/ drop the old/static timings config and only rely on the dynamic
>    config. This is a bit risky, because we've only tested the new
>    timing conversion logic on one board.
> 2/ keep both solutions around until all boards have switched to the new
>    solution. In this case, you'll only set the ->setup_data_interface()
>    hook if timings are not defined in the DT

What about the "default" timings that are currently hardcoded in the
driver, if no timings are specified in the DT? My patch currently does
the following:

 - Use DT timings if provided
 - Use NAND timings if provided
 - Use "default" timing otherwise

Should we drop the "default" timing thing? If not, how does it fit in
your proposal? Indeed your proposal is simply: if no timings in DT, use
the NAND timings. But my patch does more than that.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH 02/13] mtd: nand: fsmc: rework fsmc_nand_setup() to use ->setup_data_interface()

[Boris Brezillon]

On Wed, 22 Mar 2017 23:39:15 +0100
Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:

> Hello,
> 
> On Wed, 22 Mar 2017 23:23:33 +0100, Boris Brezillon wrote:
> 
> > > So what do you suggest to keep the compatibility with the existing DT
> > > binding for this NAND controller?    
> > 
> > We have 2 choices here:
> > 1/ drop the old/static timings config and only rely on the dynamic
> >    config. This is a bit risky, because we've only tested the new
> >    timing conversion logic on one board.
> > 2/ keep both solutions around until all boards have switched to the new
> >    solution. In this case, you'll only set the ->setup_data_interface()
> >    hook if timings are not defined in the DT  
> 
> What about the "default" timings that are currently hardcoded in the
> driver, if no timings are specified in the DT? My patch currently does
> the following:
> 
>  - Use DT timings if provided
>  - Use NAND timings if provided
>  - Use "default" timing otherwise

Okay, I didn't notice the 'fallback to "default" timings' in patch 3.
This portion of code will actually never be reached, because the core
always provide valid SDR timings. If the NAND does not support ONFI
timing modes and nothing is specified in the NAND ids table (or the in
vendor specific init code), then the lowest timing mode is assumed
(timing mode 0), and if you implement ->setup_data_interface(), this is
what you will end-up with.

> 
> Should we drop the "default" timing thing?

I think so.

> If not, how does it fit in
> your proposal? Indeed your proposal is simply: if no timings in DT, use
> the NAND timings. But my patch does more than that.

If we really want to keep "default" timmings around, then we'll have to
add an extra Kconfig option, a kernel parameter (or a DT property :-)),
but I'm not a big fan of these solutions.

Re: [PATCH 02/13] mtd: nand: fsmc: rework fsmc_nand_setup() to use ->setup_data_interface()

[Linus Walleij]

On Tue, Mar 21, 2017 at 11:03 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:

> Since a few kernel versions, the NAND subsystem has introduced a
> callback called ->setup_data_interface(), which NAND controller drivers
> are supposed to use to perform all the NAND data interface.
>
> This commit therefore converts the fsmc_nand driver to use this callback
> instead of the home-grown fsmc_nand_setup() function.
>
> For now, the NAND timings are not used, and we simply use the timings
> provided from the Device Tree, or otherwise the default timings. Support
> for the NAND timings is added in a follow-up patch.
>
> The call to fsmc_nand_setup() in the ->resume() hook is simply replaced
> by a call to nand_reset(), which internally will call the
> ->setup_data_interface() callback.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Looks good to me, whether you go with Boris' suggestion or not.
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 03/13] mtd: nand: fsmc: add support to use NAND timings

[Thomas Petazzoni]

Until now, the fsmc_nand driver was either using controller timings
specified in the Device Tree (through FSMC specific DT properties) or
alternatively default/fallback timings.

This commit implements support to use the timings advertised by the NAND
chip itself, and passed by the NAND core to the ->setup_data_interface()
callback.

Many thanks to Boris Brezillon for coming up with the logic to convert
the NAND chip timings into the timings expected by the FSMC controller.

Also, since the timings are now not only coming from the DT, the message
warning that default timings will be used is removed.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/mtd/nand/fsmc_nand.c | 78 ++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 72 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c
index 4a98433..d2e85c6 100644
--- a/drivers/mtd/nand/fsmc_nand.c
+++ b/drivers/mtd/nand/fsmc_nand.c
@@ -364,6 +364,62 @@ static void fsmc_cmd_ctrl(struct mtd_info *mtd, int cmd, unsigned int ctrl)
 		writeb_relaxed(cmd, this->IO_ADDR_W);
 }
 
+static int fsmc_calc_timings(struct fsmc_nand_data *host,
+			     const struct nand_sdr_timings *sdrt,
+			     struct fsmc_nand_timings *tims)
+{
+	unsigned long hclk = clk_get_rate(host->clk);
+	unsigned long hclkn = NSEC_PER_SEC / hclk;
+	uint32_t thiz, thold, twait, tset;
+
+	if (sdrt->tRC_min < 30000)
+		return -EOPNOTSUPP;
+
+	tims->tar = DIV_ROUND_UP(sdrt->tAR_min / 1000, hclkn) - 1;
+	if (tims->tar > FSMC_TAR_MASK)
+		tims->tar = FSMC_TAR_MASK;
+	tims->tclr = DIV_ROUND_UP(sdrt->tCLR_min / 1000, hclkn) - 1;
+	if (tims->tclr > FSMC_TCLR_MASK)
+		tims->tclr = FSMC_TCLR_MASK;
+
+	thiz = sdrt->tCS_min - sdrt->tWP_min;
+	tims->thiz = DIV_ROUND_UP(thiz / 1000, hclkn);
+
+	thold = sdrt->tDH_min;
+	if (thold < sdrt->tCH_min)
+		thold = sdrt->tCH_min;
+	if (thold < sdrt->tCLH_min)
+		thold = sdrt->tCLH_min;
+	if (thold < sdrt->tWH_min)
+		thold = sdrt->tWH_min;
+	if (thold < sdrt->tALH_min)
+		thold = sdrt->tALH_min;
+	if (thold < sdrt->tREH_min)
+		thold = sdrt->tREH_min;
+	tims->thold = DIV_ROUND_UP(thold / 1000, hclkn);
+	if (tims->thold == 0)
+		tims->thold = 1;
+	else if (tims->thold > FSMC_THOLD_MASK)
+		tims->thold = FSMC_THOLD_MASK;
+
+	twait = max(sdrt->tRP_min, sdrt->tWP_min);
+	tims->twait = DIV_ROUND_UP(twait / 1000, hclkn) - 1;
+	if (tims->twait == 0)
+		tims->twait = 1;
+	else if (tims->twait > FSMC_TWAIT_MASK)
+		tims->twait = FSMC_TWAIT_MASK;
+
+	tset = max(sdrt->tCS_min - sdrt->tWP_min,
+		   sdrt->tCEA_max - sdrt->tREA_max);
+	tims->tset = DIV_ROUND_UP(tset / 1000, hclkn) - 1;
+	if (tims->tset == 0)
+		tims->tset = 1;
+	else if (tims->tset > FSMC_TSET_MASK)
+		tims->tset = FSMC_TSET_MASK;
+
+	return 0;
+}
+
 /*
  * fsmc_nand_setup - FSMC (Flexible Static Memory Controller) init routine
  *
@@ -387,14 +443,26 @@ static int fsmc_setup_data_interface(struct mtd_info *mtd,
 		.twait	= FSMC_TWAIT_6,
 		.tset	= FSMC_TSET_0,
 	};
+	struct fsmc_nand_timings nand_timings;
 	unsigned int bank = host->bank;
 	void __iomem *regs = host->regs_va;
 	int ret;
 
-	if (host->dev_timings)
+	if (host->dev_timings) {
 		tims = host->dev_timings;
-	else
-		tims = &default_timings;
+	} else {
+		const struct nand_sdr_timings *sdrt;
+
+		sdrt = nand_get_sdr_timings(conf);
+		if (IS_ERR(sdrt)) {
+			tims = &default_timings;
+		} else {
+			ret = fsmc_calc_timings(host, sdrt, &nand_timings);
+			if (ret)
+				return ret;
+			tims = &nand_timings;
+		}
+	}
 
 	if (check_only)
 		return 0;
@@ -874,10 +942,8 @@ static int fsmc_nand_probe_config_dt(struct platform_device *pdev,
 		return -ENOMEM;
 	ret = of_property_read_u8_array(np, "timings", (u8 *)pdata->nand_timings,
 						sizeof(*pdata->nand_timings));
-	if (ret) {
-		dev_info(&pdev->dev, "No timings in dts specified, using default timings!\n");
+	if (ret)
 		pdata->nand_timings = NULL;
-	}
 
 	/* Set default NAND bank to 0 */
 	pdata->bank = 0;
-- 
2.7.4

Re: [PATCH 03/13] mtd: nand: fsmc: add support to use NAND timings

[Linus Walleij]

On Tue, Mar 21, 2017 at 11:03 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:

> Until now, the fsmc_nand driver was either using controller timings
> specified in the Device Tree (through FSMC specific DT properties) or
> alternatively default/fallback timings.
>
> This commit implements support to use the timings advertised by the NAND
> chip itself, and passed by the NAND core to the ->setup_data_interface()
> callback.
>
> Many thanks to Boris Brezillon for coming up with the logic to convert
> the NAND chip timings into the timings expected by the FSMC controller.
>
> Also, since the timings are now not only coming from the DT, the message
> warning that default timings will be used is removed.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Obviously the right thing to do. If NAND manufacturers give us
plug-n-play capabilities, we should use it.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 04/13] mtd: nand: fsmc: move fsmc_nand_data definition

[Thomas Petazzoni]

This commit simply moves the "struct fsmc_nand_data" definition to be
towards the beginning of the file, with the other defines and type
definitions, instead of in the middle of the driver code. This is much
more consistent with what most Linux drivers do.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/mtd/nand/fsmc_nand.c | 98 ++++++++++++++++++++++----------------------
 1 file changed, 49 insertions(+), 49 deletions(-)

diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c
index d2e85c6..bc1c764 100644
--- a/drivers/mtd/nand/fsmc_nand.c
+++ b/drivers/mtd/nand/fsmc_nand.c
@@ -161,6 +161,55 @@ struct fsmc_nand_platform_data {
 	void			*write_dma_priv;
 };
 
+/**
+ * struct fsmc_nand_data - structure for FSMC NAND device state
+ *
+ * @pid:		Part ID on the AMBA PrimeCell format
+ * @mtd:		MTD info for a NAND flash.
+ * @nand:		Chip related info for a NAND flash.
+ * @partitions:		Partition info for a NAND Flash.
+ * @nr_partitions:	Total number of partition of a NAND flash.
+ *
+ * @bank:		Bank number for probed device.
+ * @clk:		Clock structure for FSMC.
+ *
+ * @read_dma_chan:	DMA channel for read access
+ * @write_dma_chan:	DMA channel for write access to NAND
+ * @dma_access_complete: Completion structure
+ *
+ * @data_pa:		NAND Physical port for Data.
+ * @data_va:		NAND port for Data.
+ * @cmd_va:		NAND port for Command.
+ * @addr_va:		NAND port for Address.
+ * @regs_va:		FSMC regs base address.
+ */
+struct fsmc_nand_data {
+	u32			pid;
+	struct nand_chip	nand;
+	struct mtd_partition	*partitions;
+	unsigned int		nr_partitions;
+
+	unsigned int		bank;
+	struct device		*dev;
+	enum access_mode	mode;
+	struct clk		*clk;
+
+	/* DMA related objects */
+	struct dma_chan		*read_dma_chan;
+	struct dma_chan		*write_dma_chan;
+	struct completion	dma_access_complete;
+
+	struct fsmc_nand_timings *dev_timings;
+
+	dma_addr_t		data_pa;
+	void __iomem		*data_va;
+	void __iomem		*cmd_va;
+	void __iomem		*addr_va;
+	void __iomem		*regs_va;
+
+	void			(*select_chip)(uint32_t bank, uint32_t busw);
+};
+
 static int fsmc_ecc1_ooblayout_ecc(struct mtd_info *mtd, int section,
 				   struct mtd_oob_region *oobregion)
 {
@@ -245,55 +294,6 @@ static const struct mtd_ooblayout_ops fsmc_ecc4_ooblayout_ops = {
 	.free = fsmc_ecc4_ooblayout_free,
 };
 
-/**
- * struct fsmc_nand_data - structure for FSMC NAND device state
- *
- * @pid:		Part ID on the AMBA PrimeCell format
- * @mtd:		MTD info for a NAND flash.
- * @nand:		Chip related info for a NAND flash.
- * @partitions:		Partition info for a NAND Flash.
- * @nr_partitions:	Total number of partition of a NAND flash.
- *
- * @bank:		Bank number for probed device.
- * @clk:		Clock structure for FSMC.
- *
- * @read_dma_chan:	DMA channel for read access
- * @write_dma_chan:	DMA channel for write access to NAND
- * @dma_access_complete: Completion structure
- *
- * @data_pa:		NAND Physical port for Data.
- * @data_va:		NAND port for Data.
- * @cmd_va:		NAND port for Command.
- * @addr_va:		NAND port for Address.
- * @regs_va:		FSMC regs base address.
- */
-struct fsmc_nand_data {
-	u32			pid;
-	struct nand_chip	nand;
-	struct mtd_partition	*partitions;
-	unsigned int		nr_partitions;
-
-	unsigned int		bank;
-	struct device		*dev;
-	enum access_mode	mode;
-	struct clk		*clk;
-
-	/* DMA related objects */
-	struct dma_chan		*read_dma_chan;
-	struct dma_chan		*write_dma_chan;
-	struct completion	dma_access_complete;
-
-	struct fsmc_nand_timings *dev_timings;
-
-	dma_addr_t		data_pa;
-	void __iomem		*data_va;
-	void __iomem		*cmd_va;
-	void __iomem		*addr_va;
-	void __iomem		*regs_va;
-
-	void			(*select_chip)(uint32_t bank, uint32_t busw);
-};
-
 static inline struct fsmc_nand_data *mtd_to_fsmc(struct mtd_info *mtd)
 {
 	return container_of(mtd_to_nand(mtd), struct fsmc_nand_data, nand);
-- 
2.7.4

Re: [PATCH 04/13] mtd: nand: fsmc: move fsmc_nand_data definition

[Linus Walleij]

On Tue, Mar 21, 2017 at 11:03 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:

> This commit simply moves the "struct fsmc_nand_data" definition to be
> towards the beginning of the file, with the other defines and type
> definitions, instead of in the middle of the driver code. This is much
> more consistent with what most Linux drivers do.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 05/13] mtd: nand: fsmc: remove ->select_bank() from fsmc_nand_platform_data

[Thomas Petazzoni]

Since commit 4404d7d821c3 ("mtd: nand: fsmc: remove stale non-DT probe
path"), only DT probing is used for the fsmc_nand driver. Due to this,
the ->select_bank() field of fsmc_nand_platform_data is never used, so
this commit gets rid of it.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/mtd/nand/fsmc_nand.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c
index bc1c764..928ff98 100644
--- a/drivers/mtd/nand/fsmc_nand.c
+++ b/drivers/mtd/nand/fsmc_nand.c
@@ -141,7 +141,6 @@ enum access_mode {
  * @options: different options for the driver
  * @width: bus width
  * @bank: default bank
- * @select_bank: callback to select a certain bank, this is
  * platform-specific. If the controller only supports one bank
  * this may be set to NULL
  */
@@ -154,8 +153,6 @@ struct fsmc_nand_platform_data {
 
 	enum access_mode	mode;
 
-	void			(*select_bank)(uint32_t bank, uint32_t busw);
-
 	/* priv structures for dma accesses */
 	void			*read_dma_priv;
 	void			*write_dma_priv;
@@ -1035,7 +1032,6 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
 		 AMBA_REV_BITS(pid), AMBA_CONFIG_BITS(pid));
 
 	host->bank = pdata->bank;
-	host->select_chip = pdata->select_bank;
 	host->partitions = pdata->partitions;
 	host->nr_partitions = pdata->nr_partitions;
 	host->dev = &pdev->dev;
-- 
2.7.4

Re: [PATCH 05/13] mtd: nand: fsmc: remove ->select_bank() from fsmc_nand_platform_data

[Linus Walleij]

On Tue, Mar 21, 2017 at 11:03 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:

> Since commit 4404d7d821c3 ("mtd: nand: fsmc: remove stale non-DT probe
> path"), only DT probing is used for the fsmc_nand driver. Due to this,
> the ->select_bank() field of fsmc_nand_platform_data is never used, so
> this commit gets rid of it.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 06/13] mtd: nand: fsmc: remove fsmc_select_chip()

[Thomas Petazzoni]

host->select_chip used to point to the ->select_bank() function provided
by the platform data, but the latter no longer exists. Therefore
host->select_chip is always NULL.

Due to this, the fsmc_select_chip() does nothing, except:

  chip->cmd_ctrl(mtd, NAND_CMD_NONE, 0 | NAND_CTRL_CHANGE);

when chipnr is -1, which is exactly what the default implementation of
->select_chip() does in the NAND framework. So, this commit kills
fsmc_select_chip() entirely.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/mtd/nand/fsmc_nand.c | 29 -----------------------------
 1 file changed, 29 deletions(-)

diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c
index 928ff98..1c9744b 100644
--- a/drivers/mtd/nand/fsmc_nand.c
+++ b/drivers/mtd/nand/fsmc_nand.c
@@ -203,8 +203,6 @@ struct fsmc_nand_data {
 	void __iomem		*cmd_va;
 	void __iomem		*addr_va;
 	void __iomem		*regs_va;
-
-	void			(*select_chip)(uint32_t bank, uint32_t busw);
 };
 
 static int fsmc_ecc1_ooblayout_ecc(struct mtd_info *mtd, int section,
@@ -296,32 +294,6 @@ static inline struct fsmc_nand_data *mtd_to_fsmc(struct mtd_info *mtd)
 	return container_of(mtd_to_nand(mtd), struct fsmc_nand_data, nand);
 }
 
-/* Assert CS signal based on chipnr */
-static void fsmc_select_chip(struct mtd_info *mtd, int chipnr)
-{
-	struct nand_chip *chip = mtd_to_nand(mtd);
-	struct fsmc_nand_data *host;
-
-	host = mtd_to_fsmc(mtd);
-
-	switch (chipnr) {
-	case -1:
-		chip->cmd_ctrl(mtd, NAND_CMD_NONE, 0 | NAND_CTRL_CHANGE);
-		break;
-	case 0:
-	case 1:
-	case 2:
-	case 3:
-		if (host->select_chip)
-			host->select_chip(chipnr,
-					chip->options & NAND_BUSWIDTH_16);
-		break;
-
-	default:
-		dev_err(host->dev, "unsupported chip-select %d\n", chipnr);
-	}
-}
-
 /*
  * fsmc_cmd_ctrl - For facilitaing Hardware access
  * This routine allows hardware specific access to control-lines(ALE,CLE)
@@ -1061,7 +1033,6 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
 	nand->ecc.hwctl = fsmc_enable_hwecc;
 	nand->ecc.size = 512;
 	nand->options = pdata->options;
-	nand->select_chip = fsmc_select_chip;
 	nand->badblockbits = 7;
 	nand_set_flash_node(nand, np);
 	nand->setup_data_interface = fsmc_setup_data_interface;
-- 
2.7.4

Re: [PATCH 06/13] mtd: nand: fsmc: remove fsmc_select_chip()

[Linus Walleij]

On Tue, Mar 21, 2017 at 11:03 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:

> host->select_chip used to point to the ->select_bank() function provided
> by the platform data, but the latter no longer exists. Therefore
> host->select_chip is always NULL.
>
> Due to this, the fsmc_select_chip() does nothing, except:
>
>   chip->cmd_ctrl(mtd, NAND_CMD_NONE, 0 | NAND_CTRL_CHANGE);
>
> when chipnr is -1, which is exactly what the default implementation of
> ->select_chip() does in the NAND framework. So, this commit kills
> fsmc_select_chip() entirely.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

As usual, less is more.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 07/13] mtd: nand: fmsc: kill {read, write}_dma_priv from fsmc_nand_platform_data

[Thomas Petazzoni]

The read_dma_priv and write_dma_priv fields of fsmc_nand_platform_data
are never set, so this commit removes them.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/mtd/nand/fsmc_nand.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c
index 1c9744b..0d618ad 100644
--- a/drivers/mtd/nand/fsmc_nand.c
+++ b/drivers/mtd/nand/fsmc_nand.c
@@ -152,10 +152,6 @@ struct fsmc_nand_platform_data {
 	unsigned int		bank;
 
 	enum access_mode	mode;
-
-	/* priv structures for dma accesses */
-	void			*read_dma_priv;
-	void			*write_dma_priv;
 };
 
 /**
@@ -1041,14 +1037,12 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
 	case USE_DMA_ACCESS:
 		dma_cap_zero(mask);
 		dma_cap_set(DMA_MEMCPY, mask);
-		host->read_dma_chan = dma_request_channel(mask, filter,
-				pdata->read_dma_priv);
+		host->read_dma_chan = dma_request_channel(mask, filter, NULL);
 		if (!host->read_dma_chan) {
 			dev_err(&pdev->dev, "Unable to get read dma channel\n");
 			goto err_req_read_chnl;
 		}
-		host->write_dma_chan = dma_request_channel(mask, filter,
-				pdata->write_dma_priv);
+		host->write_dma_chan = dma_request_channel(mask, filter, NULL);
 		if (!host->write_dma_chan) {
 			dev_err(&pdev->dev, "Unable to get write dma channel\n");
 			goto err_req_write_chnl;
-- 
2.7.4

Re: [PATCH 07/13] mtd: nand: fmsc: kill {read, write}_dma_priv from fsmc_nand_platform_data

[Linus Walleij]

On Tue, Mar 21, 2017 at 11:03 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:

> The read_dma_priv and write_dma_priv fields of fsmc_nand_platform_data
> are never set, so this commit removes them.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 08/13] mtd: nand: fsmc: kill {nr_, }partitions structure fields

[Thomas Petazzoni]

The ->partitions and ->nr_partitions fields of struct
fsmc_nand_platform_data are never set anywhere, so they are always
NULL/0. The corresponding fields in 'struct fsmc_nand_data' are set to the
value of the same fields in fsmc_nand_platform_data, i.e NULL/0.

Therefore, we remove those two fields, and pass NULL/0 directly to
mtd_device_register(), like many other NAND drivers already do.

At the same time, we remove the comment about the fact that we pass
partition info, since we are no longer doing this.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/mtd/nand/fsmc_nand.c | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c
index 0d618ad..8342c39 100644
--- a/drivers/mtd/nand/fsmc_nand.c
+++ b/drivers/mtd/nand/fsmc_nand.c
@@ -146,8 +146,6 @@ enum access_mode {
  */
 struct fsmc_nand_platform_data {
 	struct fsmc_nand_timings *nand_timings;
-	struct mtd_partition	*partitions;
-	unsigned int		nr_partitions;
 	unsigned int		options;
 	unsigned int		bank;
 
@@ -179,8 +177,6 @@ struct fsmc_nand_platform_data {
 struct fsmc_nand_data {
 	u32			pid;
 	struct nand_chip	nand;
-	struct mtd_partition	*partitions;
-	unsigned int		nr_partitions;
 
 	unsigned int		bank;
 	struct device		*dev;
@@ -1000,8 +996,6 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
 		 AMBA_REV_BITS(pid), AMBA_CONFIG_BITS(pid));
 
 	host->bank = pdata->bank;
-	host->partitions = pdata->partitions;
-	host->nr_partitions = pdata->nr_partitions;
 	host->dev = &pdev->dev;
 	host->dev_timings = pdata->nand_timings;
 	host->mode = pdata->mode;
@@ -1139,18 +1133,8 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_probe;
 
-	/*
-	 * The partition information can is accessed by (in the same precedence)
-	 *
-	 * command line through Bootloader,
-	 * platform data,
-	 * default partition information present in driver.
-	 */
-	/*
-	 * Check for partition info passed
-	 */
 	mtd->name = "nand";
-	ret = mtd_device_register(mtd, host->partitions, host->nr_partitions);
+	ret = mtd_device_register(mtd, NULL, 0);
 	if (ret)
 		goto err_probe;
 
-- 
2.7.4

Re: [PATCH 08/13] mtd: nand: fsmc: kill {nr_, }partitions structure fields

[Linus Walleij]

On Tue, Mar 21, 2017 at 11:04 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:

> The ->partitions and ->nr_partitions fields of struct
> fsmc_nand_platform_data are never set anywhere, so they are always
> NULL/0. The corresponding fields in 'struct fsmc_nand_data' are set to the
> value of the same fields in fsmc_nand_platform_data, i.e NULL/0.
>
> Therefore, we remove those two fields, and pass NULL/0 directly to
> mtd_device_register(), like many other NAND drivers already do.
>
> At the same time, we remove the comment about the fact that we pass
> partition info, since we are no longer doing this.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 09/13] mtd: nand: fsmc: remove duplicate nand_set_flash_node()

[Thomas Petazzoni]

It is already done a few lines before.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/mtd/nand/fsmc_nand.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c
index 8342c39..990c83a 100644
--- a/drivers/mtd/nand/fsmc_nand.c
+++ b/drivers/mtd/nand/fsmc_nand.c
@@ -1024,7 +1024,6 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
 	nand->ecc.size = 512;
 	nand->options = pdata->options;
 	nand->badblockbits = 7;
-	nand_set_flash_node(nand, np);
 	nand->setup_data_interface = fsmc_setup_data_interface;
 
 	switch (host->mode) {
-- 
2.7.4

Re: [PATCH 09/13] mtd: nand: fsmc: remove duplicate nand_set_flash_node()

[Linus Walleij]

On Tue, Mar 21, 2017 at 11:04 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:

> It is already done a few lines before.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 10/13] mtd: nand: fsmc: finally remove fsmc_nand_platform_data

[Thomas Petazzoni]

Since the driver now only supports DT probing, it doesn't make a lot of
sense to have a private data structure called platform_data, fill it in
with information coming from the DT, and then copying this into the
driver-specific structure fsmc_nand_data.

So instead, we remove fsmc_nand_platform_data entirely, and have
fsmc_nand_probe_config_dt() fill in the fsmc_nand_data structure
directly.

This requires calling fsmc_nand_probe_config_dt() after fsmc_nand_data
has been allocated instead of before.

Also, as an added bonus, we now propagate properly the return value of
fsmc_nand_probe_config_dt() instead of returning -ENODEV on failure. The
error message is also removed, since it no longer made any sense.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/mtd/nand/fsmc_nand.c | 74 +++++++++++++-------------------------------
 1 file changed, 22 insertions(+), 52 deletions(-)

diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c
index 990c83a..12afae62 100644
--- a/drivers/mtd/nand/fsmc_nand.c
+++ b/drivers/mtd/nand/fsmc_nand.c
@@ -133,26 +133,6 @@ enum access_mode {
 };
 
 /**
- * fsmc_nand_platform_data - platform specific NAND controller config
- * @nand_timings: timing setup for the physical NAND interface
- * @partitions: partition table for the platform, use a default fallback
- * if this is NULL
- * @nr_partitions: the number of partitions in the previous entry
- * @options: different options for the driver
- * @width: bus width
- * @bank: default bank
- * platform-specific. If the controller only supports one bank
- * this may be set to NULL
- */
-struct fsmc_nand_platform_data {
-	struct fsmc_nand_timings *nand_timings;
-	unsigned int		options;
-	unsigned int		bank;
-
-	enum access_mode	mode;
-};
-
-/**
  * struct fsmc_nand_data - structure for FSMC NAND device state
  *
  * @pid:		Part ID on the AMBA PrimeCell format
@@ -877,17 +857,18 @@ static bool filter(struct dma_chan *chan, void *slave)
 }
 
 static int fsmc_nand_probe_config_dt(struct platform_device *pdev,
-				     struct device_node *np)
+				     struct fsmc_nand_data *host,
+				     struct nand_chip *nand)
 {
-	struct fsmc_nand_platform_data *pdata = dev_get_platdata(&pdev->dev);
+	struct device_node *np = pdev->dev.of_node;
 	u32 val;
 	int ret;
 
-	pdata->options = 0;
+	nand->options = 0;
 
 	if (!of_property_read_u32(np, "bank-width", &val)) {
 		if (val == 2) {
-			pdata->options |= NAND_BUSWIDTH_16;
+			nand->options |= NAND_BUSWIDTH_16;
 		} else if (val != 1) {
 			dev_err(&pdev->dev, "invalid bank-width %u\n", val);
 			return -EINVAL;
@@ -895,25 +876,26 @@ static int fsmc_nand_probe_config_dt(struct platform_device *pdev,
 	}
 
 	if (of_get_property(np, "nand-skip-bbtscan", NULL))
-		pdata->options |= NAND_SKIP_BBTSCAN;
+		nand->options |= NAND_SKIP_BBTSCAN;
 
-	pdata->nand_timings = devm_kzalloc(&pdev->dev,
-				sizeof(*pdata->nand_timings), GFP_KERNEL);
-	if (!pdata->nand_timings)
+	host->dev_timings =
+		devm_kzalloc(&pdev->dev, sizeof(*host->dev_timings),
+			     GFP_KERNEL);
+	if (!host->dev_timings)
 		return -ENOMEM;
-	ret = of_property_read_u8_array(np, "timings", (u8 *)pdata->nand_timings,
-						sizeof(*pdata->nand_timings));
+	ret = of_property_read_u8_array(np, "timings", (u8 *)host->dev_timings,
+						sizeof(*host->dev_timings));
 	if (ret)
-		pdata->nand_timings = NULL;
+		host->dev_timings = NULL;
 
 	/* Set default NAND bank to 0 */
-	pdata->bank = 0;
+	host->bank = 0;
 	if (!of_property_read_u32(np, "bank", &val)) {
 		if (val > 3) {
 			dev_err(&pdev->dev, "invalid bank %u\n", val);
 			return -EINVAL;
 		}
-		pdata->bank = val;
+		host->bank = val;
 	}
 	return 0;
 }
@@ -924,8 +906,6 @@ static int fsmc_nand_probe_config_dt(struct platform_device *pdev,
  */
 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 fsmc_nand_data *host;
 	struct mtd_info *mtd;
 	struct nand_chip *nand;
@@ -935,22 +915,17 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
 	u32 pid;
 	int i;
 
-	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
-	if (!pdata)
-		return -ENOMEM;
-
-	pdev->dev.platform_data = pdata;
-	ret = fsmc_nand_probe_config_dt(pdev, np);
-	if (ret) {
-		dev_err(&pdev->dev, "no platform data\n");
-		return -ENODEV;
-	}
-
 	/* Allocate memory for the device structure (and zero it) */
 	host = devm_kzalloc(&pdev->dev, sizeof(*host), GFP_KERNEL);
 	if (!host)
 		return -ENOMEM;
 
+	nand = &host->nand;
+
+	ret = fsmc_nand_probe_config_dt(pdev, host, nand);
+	if (ret)
+		return ret;
+
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "nand_data");
 	host->data_va = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(host->data_va))
@@ -995,19 +970,15 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
 		 AMBA_PART_BITS(pid), AMBA_MANF_BITS(pid),
 		 AMBA_REV_BITS(pid), AMBA_CONFIG_BITS(pid));
 
-	host->bank = pdata->bank;
 	host->dev = &pdev->dev;
-	host->dev_timings = pdata->nand_timings;
-	host->mode = pdata->mode;
 
 	if (host->mode == USE_DMA_ACCESS)
 		init_completion(&host->dma_access_complete);
 
 	/* Link all private pointers */
 	mtd = nand_to_mtd(&host->nand);
-	nand = &host->nand;
 	nand_set_controller_data(nand, host);
-	nand_set_flash_node(nand, np);
+	nand_set_flash_node(nand, pdev->dev.of_node);
 
 	mtd->dev.parent = &pdev->dev;
 	nand->IO_ADDR_R = host->data_va;
@@ -1022,7 +993,6 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
 	nand->ecc.mode = NAND_ECC_HW;
 	nand->ecc.hwctl = fsmc_enable_hwecc;
 	nand->ecc.size = 512;
-	nand->options = pdata->options;
 	nand->badblockbits = 7;
 	nand->setup_data_interface = fsmc_setup_data_interface;
 
-- 
2.7.4

Re: [PATCH 10/13] mtd: nand: fsmc: finally remove fsmc_nand_platform_data

[Linus Walleij]

On Tue, Mar 21, 2017 at 11:04 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:

> Since the driver now only supports DT probing, it doesn't make a lot of
> sense to have a private data structure called platform_data, fill it in
> with information coming from the DT, and then copying this into the
> driver-specific structure fsmc_nand_data.
>
> So instead, we remove fsmc_nand_platform_data entirely, and have
> fsmc_nand_probe_config_dt() fill in the fsmc_nand_data structure
> directly.
>
> This requires calling fsmc_nand_probe_config_dt() after fsmc_nand_data
> has been allocated instead of before.
>
> Also, as an added bonus, we now propagate properly the return value of
> fsmc_nand_probe_config_dt() instead of returning -ENODEV on failure. The
> error message is also removed, since it no longer made any sense.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Sweet!
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 11/13] mtd: nand: fsmc: use devm_clk_get()

[Thomas Petazzoni]

This commit switches the fsmc_nand driver from clk_get() to
devm_clk_get(), which saves a few clk_put().

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/mtd/nand/fsmc_nand.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c
index 12afae62..296222f 100644
--- a/drivers/mtd/nand/fsmc_nand.c
+++ b/drivers/mtd/nand/fsmc_nand.c
@@ -948,7 +948,7 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
 	if (IS_ERR(host->regs_va))
 		return PTR_ERR(host->regs_va);
 
-	host->clk = clk_get(&pdev->dev, NULL);
+	host->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(host->clk)) {
 		dev_err(&pdev->dev, "failed to fetch block clock\n");
 		return PTR_ERR(host->clk);
@@ -956,7 +956,7 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
 
 	ret = clk_prepare_enable(host->clk);
 	if (ret)
-		goto err_clk_prepare_enable;
+		return ret;
 
 	/*
 	 * This device ID is actually a common AMBA ID as used on the
@@ -1121,8 +1121,6 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
 		dma_release_channel(host->read_dma_chan);
 err_req_read_chnl:
 	clk_disable_unprepare(host->clk);
-err_clk_prepare_enable:
-	clk_put(host->clk);
 	return ret;
 }
 
@@ -1141,7 +1139,6 @@ static int fsmc_nand_remove(struct platform_device *pdev)
 			dma_release_channel(host->read_dma_chan);
 		}
 		clk_disable_unprepare(host->clk);
-		clk_put(host->clk);
 	}
 
 	return 0;
-- 
2.7.4

Re: [PATCH 11/13] mtd: nand: fsmc: use devm_clk_get()

[Linus Walleij]

On Tue, Mar 21, 2017 at 11:04 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:

> This commit switches the fsmc_nand driver from clk_get() to
> devm_clk_get(), which saves a few clk_put().
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 12/13] mtd: nand: fsmc: remove unused definitions

[Thomas Petazzoni]

These definitions are not used anywhere in the driver, so remove them.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/mtd/nand/fsmc_nand.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c
index 296222f..c50c3ed5 100644
--- a/drivers/mtd/nand/fsmc_nand.c
+++ b/drivers/mtd/nand/fsmc_nand.c
@@ -38,15 +38,6 @@
 #include <linux/amba/bus.h>
 #include <mtd/mtd-abi.h>
 
-#define FSMC_NAND_BW8		1
-#define FSMC_NAND_BW16		2
-
-#define FSMC_MAX_NOR_BANKS	4
-#define FSMC_MAX_NAND_BANKS	4
-
-#define FSMC_FLASH_WIDTH8	1
-#define FSMC_FLASH_WIDTH16	2
-
 /* fsmc controller registers for NOR flash */
 #define CTRL			0x0
 	/* ctrl register definitions */
-- 
2.7.4

Re: [PATCH 12/13] mtd: nand: fsmc: remove unused definitions

[Linus Walleij]

On Tue, Mar 21, 2017 at 11:04 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:

> These definitions are not used anywhere in the driver, so remove them.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 13/13] mtd: nand: fsmc: remove CONFIG_OF conditional

[Thomas Petazzoni]

Since commit 4404d7d821c33 ("mtd: nand: fsmc: remove stale non-DT probe
path"), the fsmc NAND driver only supports Device Tree probing, and
therefore has a "depends on OF" in its Kconfig option.

Due to this the #ifdef CONFIG_OF ... #endif condition in the driver code
is no longer necessary.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/mtd/nand/fsmc_nand.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c
index c50c3ed5..fbab0bb 100644
--- a/drivers/mtd/nand/fsmc_nand.c
+++ b/drivers/mtd/nand/fsmc_nand.c
@@ -1157,14 +1157,12 @@ static int fsmc_nand_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(fsmc_nand_pm_ops, fsmc_nand_suspend, fsmc_nand_resume);
 
-#ifdef CONFIG_OF
 static const struct of_device_id fsmc_nand_id_table[] = {
 	{ .compatible = "st,spear600-fsmc-nand" },
 	{ .compatible = "stericsson,fsmc-nand" },
 	{}
 };
 MODULE_DEVICE_TABLE(of, fsmc_nand_id_table);
-#endif
 
 static struct platform_driver fsmc_nand_driver = {
 	.remove = fsmc_nand_remove,
-- 
2.7.4

Re: [PATCH 13/13] mtd: nand: fsmc: remove CONFIG_OF conditional

[Boris Brezillon]

On Tue, 21 Mar 2017 11:04:05 +0100
Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:

> Since commit 4404d7d821c33 ("mtd: nand: fsmc: remove stale non-DT probe
> path"), the fsmc NAND driver only supports Device Tree probing, and
> therefore has a "depends on OF" in its Kconfig option.
> 
> Due to this the #ifdef CONFIG_OF ... #endif condition in the driver code
> is no longer necessary.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  drivers/mtd/nand/fsmc_nand.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c
> index c50c3ed5..fbab0bb 100644
> --- a/drivers/mtd/nand/fsmc_nand.c
> +++ b/drivers/mtd/nand/fsmc_nand.c
> @@ -1157,14 +1157,12 @@ static int fsmc_nand_resume(struct device *dev)
>  
>  static SIMPLE_DEV_PM_OPS(fsmc_nand_pm_ops, fsmc_nand_suspend, fsmc_nand_resume);
>  
> -#ifdef CONFIG_OF
>  static const struct of_device_id fsmc_nand_id_table[] = {
>  	{ .compatible = "st,spear600-fsmc-nand" },
>  	{ .compatible = "stericsson,fsmc-nand" },
>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, fsmc_nand_id_table);
> -#endif
>  
>  static struct platform_driver fsmc_nand_driver = {
>  	.remove = fsmc_nand_remove,

You probably also want to remove of_match_ptr() in

		.of_match_table = of_match_ptr(fsmc_nand_id_table),

Re: [PATCH 13/13] mtd: nand: fsmc: remove CONFIG_OF conditional

[Linus Walleij]

On Tue, Mar 21, 2017 at 11:04 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:

> Since commit 4404d7d821c33 ("mtd: nand: fsmc: remove stale non-DT probe
> path"), the fsmc NAND driver only supports Device Tree probing, and
> therefore has a "depends on OF" in its Kconfig option.
>
> Due to this the #ifdef CONFIG_OF ... #endif condition in the driver code
> is no longer necessary.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 00/13] mtd: nand: fsmc: fixes, improvements and cleanups

[Linus Walleij]

On Tue, Mar 21, 2017 at 11:03 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:

> This patch series brings a number of fixes, improvements and cleanups
> to the fsmc MTD NAND driver.

Are you testing this on some specific hardware?

I can give it a spin on the Nomadik later, will review
the series.

Yours,
Linus Walleij

Re: [PATCH 00/13] mtd: nand: fsmc: fixes, improvements and cleanups

[Thomas Petazzoni]

Hello,

On Thu, 23 Mar 2017 10:50:39 +0100, Linus Walleij wrote:

> > This patch series brings a number of fixes, improvements and cleanups
> > to the fsmc MTD NAND driver.  
> 
> Are you testing this on some specific hardware?

I am testing on a SPEAr600 based custom HW platform.

> I can give it a spin on the Nomadik later, will review
> the series.

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH 00/13] mtd: nand: fsmc: fixes, improvements and cleanups

[Boris Brezillon]

Hi Thomas,

On Tue, 21 Mar 2017 11:03:52 +0100
Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:

> Hello,
> 
> This patch series brings a number of fixes, improvements and cleanups
> to the fsmc MTD NAND driver.
> 
> The first patch is really a fix for the NAND bus width selection
> through the Device Tree, which was broken since the Device Tree
> support was added in the driver.
> 
> The next two patches add support for configuring the NAND controller
> timings according to the connected NAND chip automatically.
> 
> The remaining patches are basically cleanups, removing useless code
> and logic from the driver.
> 
> This patch series is based on v4.11-rc1.
> 
> Best regards,
> 
> Thomas
> 
> Thomas Petazzoni (13):
>   mtd: nand: fsmc: fix NAND width handling
>   mtd: nand: fsmc: rework fsmc_nand_setup() to use
>     ->setup_data_interface()  
>   mtd: nand: fsmc: add support to use NAND timings
>   mtd: nand: fsmc: move fsmc_nand_data definition
>   mtd: nand: fsmc: remove ->select_bank() from fsmc_nand_platform_data
>   mtd: nand: fsmc: remove fsmc_select_chip()
>   mtd: nand: fmsc: kill {read,write}_dma_priv from
>     fsmc_nand_platform_data
>   mtd: nand: fsmc: kill {nr_,}partitions structure fields
>   mtd: nand: fsmc: remove duplicate nand_set_flash_node()
>   mtd: nand: fsmc: finally remove fsmc_nand_platform_data
>   mtd: nand: fsmc: use devm_clk_get()
>   mtd: nand: fsmc: remove unused definitions
>   mtd: nand: fsmc: remove CONFIG_OF conditional

Applied all patches except 2 and 3.

Thanks,

Boris