[PATCH v2 00/14] pxa3xx-nand patches to support mvebu builds

[PATCH v2 00/14] pxa3xx-nand patches to support mvebu builds

[Ezequiel Garcia]

Hello everyone!

This patchset is part of the work I'm doing to enable the pxa3xx-nand
driver to support the NAND controller in Armada 370/XP. While this is
work in progress, here's a subset of patches I consider good enough
for mainline.

Since pxa3xx has a mach-specific DMA API, I'm including a few patches to
ifdef such pxa-specific code in non PXA/MMP platforms.
This is only an ugly workaround and should be removed once the specific
DMA API is replaced by dmaengine.

Daniel Mack is already working on that, so it's expected this removal
will happen soon.

Other than that, the rest of the patchset is just assorted cleanups
and minor fixes.

This patchset is based in Artem's l2-mtd master branch.

If at all possible and if no regressions are reported I'd like to see
this queued for v3.12.

Changes from v1:

  * Use __maybe_unused instead of inline keyword for a symbol
    that's maybe unused. Suggested by Brian Norris.

  * As Daniel Mack reported, the PXA nand controller (NFCv1)
    lacks an NDBC3 register and it's access is undefined.
    This means we need a way to distinguish between the two
    controllers (NFCv1 and NFCv2).

    We introduce a new 'marvell,armada370-nand' compatible
    string to distinguish the newer controller (NFCv2).

Ezequiel Garcia (14):
  mtd: nand: pxa3xx: Introduce 'marvell,armada370-nand' compatible
    string
  mtd: nand: pxa3xx: Handle ECC and DMA enable/disable properly
  mtd: nand: pxa3xx: Allow to set/clear the 'spare enable' field
  mtd: nand: pxa3xx: Support command buffer #3
  mtd: nand: pxa3xx: Use 'length override' in ONFI paramater page read
  mtd: nand: pxa3xx: Add a local loop variable
  mtd: nand: pxa3xx: Remove hardcoded mtd name
  mtd: nand: pxa3xx: Remove uneeded internal cmdset
  mtd: nand: pxa3xx: Move cached registers to info structure
  mtd: nand: pxa3xx: Make dma code dependent on dma capable platforms
  mtd: nand: pxa3xx: Add __maybe_unused keyword to enable_int()
  mtd: nand: pxa3xx: Allow devices with no dma resources
  mtd: nand: pxa3xx: Increase data buffer size
  mtd: nand: Allow to build pxa3xx_nand on Orion platforms

 drivers/mtd/nand/Kconfig                      |   2 +-
 drivers/mtd/nand/pxa3xx_nand.c                | 269 ++++++++++++++++----------
 include/linux/platform_data/mtd-nand-pxa3xx.h |  13 --
 3 files changed, 171 insertions(+), 113 deletions(-)

-- 
1.8.1.5

[PATCH v2 01/14] mtd: nand: pxa3xx: Introduce 'marvell, armada370-nand' compatible string

[Ezequiel Garcia]

This driver supports NFCv1 (as found in PXA SoC) and NFCv2 (as found in
Armada 370/XP SoC). As both controller has a few differences, a way of
distinguishing between the two is needed.

This commit introduces a new compatible string 'marvell,armada370-nand'
and assigns a compatible data of type enum pxa3xx_nand_variant to allow
such distinction.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 38 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 501e380..2582e1f 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -123,6 +123,11 @@ enum {
 	STATE_READY,
 };
 
+enum pxa3xx_nand_variant {
+	PXA3XX_NAND_VARIANT_PXA,
+	PXA3XX_NAND_VARIANT_ARMADA370,
+};
+
 struct pxa3xx_nand_host {
 	struct nand_chip	chip;
 	struct pxa3xx_nand_cmdset *cmdset;
@@ -171,6 +176,12 @@ struct pxa3xx_nand_info {
 	struct pxa3xx_nand_host *host[NUM_CHIP_SELECT];
 	unsigned int		state;
 
+	/*
+	 * This driver supports NFCv1 (as found in PXA SoC)
+	 * and NFCv2 (as found in Armada 370/XP SoC).
+	 */
+	enum pxa3xx_nand_variant variant;
+
 	int			cs;
 	int			use_ecc;	/* use HW ECC ? */
 	int			use_dma;	/* use DMA ? */
@@ -1192,7 +1203,14 @@ static int pxa3xx_nand_remove(struct platform_device *pdev)
 
 #ifdef CONFIG_OF
 static struct of_device_id pxa3xx_nand_dt_ids[] = {
-	{ .compatible = "marvell,pxa3xx-nand" },
+	{
+		.compatible = "marvell,pxa3xx-nand",
+		.data       = (void *) PXA3XX_NAND_VARIANT_PXA,
+	},
+	{
+		.compatible = "marvell,armada370-nand",
+		.data       = (void *) PXA3XX_NAND_VARIANT_ARMADA370,
+	},
 	{}
 };
 MODULE_DEVICE_TABLE(of, pxa3xx_nand_dt_ids);
@@ -1221,11 +1239,28 @@ static int pxa3xx_nand_probe_dt(struct platform_device *pdev)
 
 	return 0;
 }
+
+static enum pxa3xx_nand_variant
+pxa3xx_nand_get_variant(struct platform_device *pdev)
+{
+	const struct of_device_id *of_id =
+			of_match_device(pxa3xx_nand_dt_ids, &pdev->dev);
+	if (!of_id)
+		return PXA3XX_NAND_VARIANT_PXA;
+	return (enum pxa3xx_nand_variant) of_id->data;
+}
 #else
 static inline int pxa3xx_nand_probe_dt(struct platform_device *pdev)
 {
 	return 0;
 }
+
+static enum pxa3xx_nand_variant
+pxa3xx_nand_get_variant(struct platform_device *pdev)
+{
+	/* Default lefacy (non-DT) variant */
+	return PXA3XX_NAND_VARIANT_PXA;
+}
 #endif
 
 static int pxa3xx_nand_probe(struct platform_device *pdev)
@@ -1252,6 +1287,7 @@ static int pxa3xx_nand_probe(struct platform_device *pdev)
 	}
 
 	info = platform_get_drvdata(pdev);
+	info->variant = pxa3xx_nand_get_variant(pdev);
 	probe_success = 0;
 	for (cs = 0; cs < pdata->num_cs; cs++) {
 		info->cs = cs;
-- 
1.8.1.5

Re: [PATCH v2 01/14] mtd: nand: pxa3xx: Introduce 'marvell, armada370-nand' compatible string

[Brian Norris]

+ devicetree list

On Wed, Aug 7, 2013 at 5:31 AM, Ezequiel Garcia
<ezequiel.garcia@free-electrons.com> wrote:
> This driver supports NFCv1 (as found in PXA SoC) and NFCv2 (as found in
> Armada 370/XP SoC). As both controller has a few differences, a way of
> distinguishing between the two is needed.
>
> This commit introduces a new compatible string 'marvell,armada370-nand'
> and assigns a compatible data of type enum pxa3xx_nand_variant to allow
> such distinction.
>
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
>  drivers/mtd/nand/pxa3xx_nand.c | 38 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> index 501e380..2582e1f 100644
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -123,6 +123,11 @@ enum {
>         STATE_READY,
>  };
>
> +enum pxa3xx_nand_variant {
> +       PXA3XX_NAND_VARIANT_PXA,
> +       PXA3XX_NAND_VARIANT_ARMADA370,
> +};
> +
>  struct pxa3xx_nand_host {
>         struct nand_chip        chip;
>         struct pxa3xx_nand_cmdset *cmdset;
> @@ -171,6 +176,12 @@ struct pxa3xx_nand_info {
>         struct pxa3xx_nand_host *host[NUM_CHIP_SELECT];
>         unsigned int            state;
>
> +       /*
> +        * This driver supports NFCv1 (as found in PXA SoC)
> +        * and NFCv2 (as found in Armada 370/XP SoC).
> +        */
> +       enum pxa3xx_nand_variant variant;
> +
>         int                     cs;
>         int                     use_ecc;        /* use HW ECC ? */
>         int                     use_dma;        /* use DMA ? */
> @@ -1192,7 +1203,14 @@ static int pxa3xx_nand_remove(struct platform_device *pdev)
>
>  #ifdef CONFIG_OF
>  static struct of_device_id pxa3xx_nand_dt_ids[] = {
> -       { .compatible = "marvell,pxa3xx-nand" },
> +       {
> +               .compatible = "marvell,pxa3xx-nand",
> +               .data       = (void *) PXA3XX_NAND_VARIANT_PXA,
> +       },
> +       {
> +               .compatible = "marvell,armada370-nand",
> +               .data       = (void *) PXA3XX_NAND_VARIANT_ARMADA370,
> +       },
>         {}
>  };
>  MODULE_DEVICE_TABLE(of, pxa3xx_nand_dt_ids);
> @@ -1221,11 +1239,28 @@ static int pxa3xx_nand_probe_dt(struct platform_device *pdev)
>
>         return 0;
>  }
> +
> +static enum pxa3xx_nand_variant
> +pxa3xx_nand_get_variant(struct platform_device *pdev)
> +{
> +       const struct of_device_id *of_id =
> +                       of_match_device(pxa3xx_nand_dt_ids, &pdev->dev);
> +       if (!of_id)
> +               return PXA3XX_NAND_VARIANT_PXA;
> +       return (enum pxa3xx_nand_variant) of_id->data;
> +}
>  #else
>  static inline int pxa3xx_nand_probe_dt(struct platform_device *pdev)
>  {
>         return 0;
>  }
> +
> +static enum pxa3xx_nand_variant
> +pxa3xx_nand_get_variant(struct platform_device *pdev)
> +{
> +       /* Default lefacy (non-DT) variant */
> +       return PXA3XX_NAND_VARIANT_PXA;
> +}
>  #endif
>
>  static int pxa3xx_nand_probe(struct platform_device *pdev)
> @@ -1252,6 +1287,7 @@ static int pxa3xx_nand_probe(struct platform_device *pdev)
>         }
>
>         info = platform_get_drvdata(pdev);
> +       info->variant = pxa3xx_nand_get_variant(pdev);
>         probe_success = 0;
>         for (cs = 0; cs < pdata->num_cs; cs++) {
>                 info->cs = cs;
> --
> 1.8.1.5
>

Re: [PATCH v2 01/14] mtd: nand: pxa3xx: Introduce 'marvell,armada370-nand' compatible string

[Brian Norris]

On Wed, Aug 07, 2013 at 09:31:06AM -0300, Ezequiel Garcia wrote:
> This driver supports NFCv1 (as found in PXA SoC) and NFCv2 (as found in
> Armada 370/XP SoC). As both controller has a few differences, a way of
> distinguishing between the two is needed.
> 
> This commit introduces a new compatible string 'marvell,armada370-nand'
> and assigns a compatible data of type enum pxa3xx_nand_variant to allow
> such distinction.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
>  drivers/mtd/nand/pxa3xx_nand.c | 38 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> index 501e380..2582e1f 100644
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -123,6 +123,11 @@ enum {
>  	STATE_READY,
>  };
>  
> +enum pxa3xx_nand_variant {
> +	PXA3XX_NAND_VARIANT_PXA,
> +	PXA3XX_NAND_VARIANT_ARMADA370,
> +};
> +
>  struct pxa3xx_nand_host {
>  	struct nand_chip	chip;
>  	struct pxa3xx_nand_cmdset *cmdset;
> @@ -171,6 +176,12 @@ struct pxa3xx_nand_info {
>  	struct pxa3xx_nand_host *host[NUM_CHIP_SELECT];
>  	unsigned int		state;
>  
> +	/*
> +	 * This driver supports NFCv1 (as found in PXA SoC)
> +	 * and NFCv2 (as found in Armada 370/XP SoC).
> +	 */
> +	enum pxa3xx_nand_variant variant;
> +
>  	int			cs;
>  	int			use_ecc;	/* use HW ECC ? */
>  	int			use_dma;	/* use DMA ? */
> @@ -1192,7 +1203,14 @@ static int pxa3xx_nand_remove(struct platform_device *pdev)
>  
>  #ifdef CONFIG_OF
>  static struct of_device_id pxa3xx_nand_dt_ids[] = {
> -	{ .compatible = "marvell,pxa3xx-nand" },
> +	{
> +		.compatible = "marvell,pxa3xx-nand",
> +		.data       = (void *) PXA3XX_NAND_VARIANT_PXA,

It's a little more common to avoid a space between the cast and the
rvalue. But that's a nitpick that can go either way (it's not in
CodingStyle, for one).

> +	},
> +	{
> +		.compatible = "marvell,armada370-nand",
> +		.data       = (void *) PXA3XX_NAND_VARIANT_ARMADA370,
> +	},
>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, pxa3xx_nand_dt_ids);
> @@ -1221,11 +1239,28 @@ static int pxa3xx_nand_probe_dt(struct platform_device *pdev)
>  
>  	return 0;
>  }
> +
> +static enum pxa3xx_nand_variant
> +pxa3xx_nand_get_variant(struct platform_device *pdev)
> +{
> +	const struct of_device_id *of_id =
> +			of_match_device(pxa3xx_nand_dt_ids, &pdev->dev);
> +	if (!of_id)
> +		return PXA3XX_NAND_VARIANT_PXA;
> +	return (enum pxa3xx_nand_variant) of_id->data;
> +}
>  #else
>  static inline int pxa3xx_nand_probe_dt(struct platform_device *pdev)
>  {
>  	return 0;
>  }

It looks like before this patch, you don't actually need the whole
#ifdef/#else CONFIG_OF block. All the of_* helpers have default inline
implementations that allow things to compile even without CONFIG_OF. So
without CONFIG_OF, of_match_device() will just return NULL and the
compiler can easiliy figure out that pxa3xx_nand_probe_dt() always
should return 0.

IOW, you only need a single pxa3xx_nand_probe_dt() implementation.

And directly related to this patch: you don't need two
pxa3xx_nand_get_variant() implementations either. Again,
of_match_device() returns NULL in the !defined(CONFIG_OF) case.

> +
> +static enum pxa3xx_nand_variant
> +pxa3xx_nand_get_variant(struct platform_device *pdev)
> +{
> +	/* Default lefacy (non-DT) variant */

s/lefacy/legacy/

> +	return PXA3XX_NAND_VARIANT_PXA;
> +}

Given the above comments, you won't need this version of
pxa3xx_nand_get_variant().

>  #endif
>  
>  static int pxa3xx_nand_probe(struct platform_device *pdev)
> @@ -1252,6 +1287,7 @@ static int pxa3xx_nand_probe(struct platform_device *pdev)
>  	}
>  
>  	info = platform_get_drvdata(pdev);
> +	info->variant = pxa3xx_nand_get_variant(pdev);
>  	probe_success = 0;
>  	for (cs = 0; cs < pdata->num_cs; cs++) {
>  		info->cs = cs;

I would recommend rewriting this patch to remove the #ifdef CONFIG_OF.

Thanks,
Brian

Re: [PATCH v2 01/14] mtd: nand: pxa3xx: Introduce 'marvell,armada370-nand' compatible string

[Ezequiel Garcia]

On Sat, Aug 10, 2013 at 11:28:57AM -0700, Brian Norris wrote:
> On Wed, Aug 07, 2013 at 09:31:06AM -0300, Ezequiel Garcia wrote:
> > This driver supports NFCv1 (as found in PXA SoC) and NFCv2 (as found in
> > Armada 370/XP SoC). As both controller has a few differences, a way of
> > distinguishing between the two is needed.
> > 
> > This commit introduces a new compatible string 'marvell,armada370-nand'
> > and assigns a compatible data of type enum pxa3xx_nand_variant to allow
> > such distinction.
> > 
> > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> > ---
> >  drivers/mtd/nand/pxa3xx_nand.c | 38 +++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 37 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> > index 501e380..2582e1f 100644
> > --- a/drivers/mtd/nand/pxa3xx_nand.c
> > +++ b/drivers/mtd/nand/pxa3xx_nand.c
> > @@ -123,6 +123,11 @@ enum {
> >  	STATE_READY,
> >  };
> >  
> > +enum pxa3xx_nand_variant {
> > +	PXA3XX_NAND_VARIANT_PXA,
> > +	PXA3XX_NAND_VARIANT_ARMADA370,
> > +};
> > +
> >  struct pxa3xx_nand_host {
> >  	struct nand_chip	chip;
> >  	struct pxa3xx_nand_cmdset *cmdset;
> > @@ -171,6 +176,12 @@ struct pxa3xx_nand_info {
> >  	struct pxa3xx_nand_host *host[NUM_CHIP_SELECT];
> >  	unsigned int		state;
> >  
> > +	/*
> > +	 * This driver supports NFCv1 (as found in PXA SoC)
> > +	 * and NFCv2 (as found in Armada 370/XP SoC).
> > +	 */
> > +	enum pxa3xx_nand_variant variant;
> > +
> >  	int			cs;
> >  	int			use_ecc;	/* use HW ECC ? */
> >  	int			use_dma;	/* use DMA ? */
> > @@ -1192,7 +1203,14 @@ static int pxa3xx_nand_remove(struct platform_device *pdev)
> >  
> >  #ifdef CONFIG_OF
> >  static struct of_device_id pxa3xx_nand_dt_ids[] = {
> > -	{ .compatible = "marvell,pxa3xx-nand" },
> > +	{
> > +		.compatible = "marvell,pxa3xx-nand",
> > +		.data       = (void *) PXA3XX_NAND_VARIANT_PXA,
> 
> It's a little more common to avoid a space between the cast and the
> rvalue. But that's a nitpick that can go either way (it's not in
> CodingStyle, for one).
> 

Sure.

> > +	},
> > +	{
> > +		.compatible = "marvell,armada370-nand",
> > +		.data       = (void *) PXA3XX_NAND_VARIANT_ARMADA370,
> > +	},
> >  	{}
> >  };
> >  MODULE_DEVICE_TABLE(of, pxa3xx_nand_dt_ids);
> > @@ -1221,11 +1239,28 @@ static int pxa3xx_nand_probe_dt(struct platform_device *pdev)
> >  
> >  	return 0;
> >  }
> > +
> > +static enum pxa3xx_nand_variant
> > +pxa3xx_nand_get_variant(struct platform_device *pdev)
> > +{
> > +	const struct of_device_id *of_id =
> > +			of_match_device(pxa3xx_nand_dt_ids, &pdev->dev);
> > +	if (!of_id)
> > +		return PXA3XX_NAND_VARIANT_PXA;
> > +	return (enum pxa3xx_nand_variant) of_id->data;
> > +}
> >  #else
> >  static inline int pxa3xx_nand_probe_dt(struct platform_device *pdev)
> >  {
> >  	return 0;
> >  }
> 
> It looks like before this patch, you don't actually need the whole
> #ifdef/#else CONFIG_OF block. All the of_* helpers have default inline
> implementations that allow things to compile even without CONFIG_OF. So
> without CONFIG_OF, of_match_device() will just return NULL and the
> compiler can easiliy figure out that pxa3xx_nand_probe_dt() always
> should return 0.
> 
> IOW, you only need a single pxa3xx_nand_probe_dt() implementation.
> 
> And directly related to this patch: you don't need two
> pxa3xx_nand_get_variant() implementations either. Again,
> of_match_device() returns NULL in the !defined(CONFIG_OF) case.
> 
> > +
> > +static enum pxa3xx_nand_variant
> > +pxa3xx_nand_get_variant(struct platform_device *pdev)
> > +{
> > +	/* Default lefacy (non-DT) variant */
> 
> s/lefacy/legacy/
> 
> > +	return PXA3XX_NAND_VARIANT_PXA;
> > +}
> 
> Given the above comments, you won't need this version of
> pxa3xx_nand_get_variant().
> 
> >  #endif
> >  
> >  static int pxa3xx_nand_probe(struct platform_device *pdev)
> > @@ -1252,6 +1287,7 @@ static int pxa3xx_nand_probe(struct platform_device *pdev)
> >  	}
> >  
> >  	info = platform_get_drvdata(pdev);
> > +	info->variant = pxa3xx_nand_get_variant(pdev);
> >  	probe_success = 0;
> >  	for (cs = 0; cs < pdata->num_cs; cs++) {
> >  		info->cs = cs;
> 
> I would recommend rewriting this patch to remove the #ifdef CONFIG_OF.
> 

Good to hear this! It was my intention in the first place, but refrained
from doing so for I thought someone might complain about the CONFIG_OF removal.

So if I remember correct, I'd say this means a patch split (right?):
one patch to remove CONFIG_OF, one patch to introduce the SoC variant.

Thanks for reviewing!
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH v2 01/14] mtd: nand: pxa3xx: Introduce 'marvell,armada370-nand' compatible string

[Brian Norris]

On Sat, Aug 10, 2013 at 03:41:17PM -0300, Ezequiel Garcia wrote:
> On Sat, Aug 10, 2013 at 11:28:57AM -0700, Brian Norris wrote:
> > On Wed, Aug 07, 2013 at 09:31:06AM -0300, Ezequiel Garcia wrote:
> > > @@ -1221,11 +1239,28 @@ static int pxa3xx_nand_probe_dt(struct platform_device *pdev)
> > >  
> > >  	return 0;
> > >  }
> > > +
> > > +static enum pxa3xx_nand_variant
> > > +pxa3xx_nand_get_variant(struct platform_device *pdev)
> > > +{
> > > +	const struct of_device_id *of_id =
> > > +			of_match_device(pxa3xx_nand_dt_ids, &pdev->dev);
> > > +	if (!of_id)
> > > +		return PXA3XX_NAND_VARIANT_PXA;
> > > +	return (enum pxa3xx_nand_variant) of_id->data;
> > > +}
> > >  #else
> > >  static inline int pxa3xx_nand_probe_dt(struct platform_device *pdev)
> > >  {
> > >  	return 0;
> > >  }
> > 
> > It looks like before this patch, you don't actually need the whole
> > #ifdef/#else CONFIG_OF block. All the of_* helpers have default inline
> > implementations that allow things to compile even without CONFIG_OF. So
> > without CONFIG_OF, of_match_device() will just return NULL and the
> > compiler can easiliy figure out that pxa3xx_nand_probe_dt() always
> > should return 0.
> > 
> > IOW, you only need a single pxa3xx_nand_probe_dt() implementation.
> > 
> > And directly related to this patch: you don't need two
> > pxa3xx_nand_get_variant() implementations either. Again,
> > of_match_device() returns NULL in the !defined(CONFIG_OF) case.
> > 
> > > +
> > > +static enum pxa3xx_nand_variant
> > > +pxa3xx_nand_get_variant(struct platform_device *pdev)
> > > +{
> > > +	/* Default lefacy (non-DT) variant */
> > 
> > s/lefacy/legacy/
> > 
> > > +	return PXA3XX_NAND_VARIANT_PXA;
> > > +}
> > 
> > Given the above comments, you won't need this version of
> > pxa3xx_nand_get_variant().
> > 
> > >  #endif
> > >  
> > >  static int pxa3xx_nand_probe(struct platform_device *pdev)
> > > @@ -1252,6 +1287,7 @@ static int pxa3xx_nand_probe(struct platform_device *pdev)
> > >  	}
> > >  
> > >  	info = platform_get_drvdata(pdev);
> > > +	info->variant = pxa3xx_nand_get_variant(pdev);
> > >  	probe_success = 0;
> > >  	for (cs = 0; cs < pdata->num_cs; cs++) {
> > >  		info->cs = cs;
> > 
> > I would recommend rewriting this patch to remove the #ifdef CONFIG_OF.
> > 
> 
> Good to hear this! It was my intention in the first place, but refrained
> from doing so for I thought someone might complain about the CONFIG_OF removal.

As a general rule: the fewer #ifdef's the better. And in this case, the
emitted code should be identical (but I Am Not A Compiler).

> So if I remember correct, I'd say this means a patch split (right?):
> one patch to remove CONFIG_OF, one patch to introduce the SoC variant.

Sure. I presume you did that in v3, which I will now look at...

Brian

[PATCH v2 02/14] mtd: nand: pxa3xx: Handle ECC and DMA enable/disable properly

[Ezequiel Garcia]

When ECC is not selected, the ECC enable bit must be cleared
in the NAND control register. Same applies to DMA.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 2582e1f..e3cd903 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -314,8 +314,17 @@ static void pxa3xx_nand_start(struct pxa3xx_nand_info *info)
 	uint32_t ndcr;
 
 	ndcr = host->reg_ndcr;
-	ndcr |= info->use_ecc ? NDCR_ECC_EN : 0;
-	ndcr |= info->use_dma ? NDCR_DMA_EN : 0;
+
+	if (info->use_ecc)
+		ndcr |= NDCR_ECC_EN;
+	else
+		ndcr &= ~NDCR_ECC_EN;
+
+	if (info->use_dma)
+		ndcr |= NDCR_DMA_EN;
+	else
+		ndcr &= ~NDCR_DMA_EN;
+
 	ndcr |= NDCR_ND_RUN;
 
 	/* clear status bits and run */
-- 
1.8.1.5

[PATCH v2 03/14] mtd: nand: pxa3xx: Allow to set/clear the 'spare enable' field

[Ezequiel Garcia]

Some commands (such as the ONFI parameter page read) need to
clear the 'spare enable' bit. This commit allows to set/clear
depending on the prepared command, instead of having it always
set.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index e3cd903..a277def 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -185,6 +185,7 @@ struct pxa3xx_nand_info {
 	int			cs;
 	int			use_ecc;	/* use HW ECC ? */
 	int			use_dma;	/* use DMA ? */
+	int			use_spare;	/* use spare ? */
 	int			is_ready;
 
 	unsigned int		page_size;	/* page size of attached chip */
@@ -325,6 +326,11 @@ static void pxa3xx_nand_start(struct pxa3xx_nand_info *info)
 	else
 		ndcr &= ~NDCR_DMA_EN;
 
+	if (info->use_spare)
+		ndcr |= NDCR_SPARE_EN;
+	else
+		ndcr &= ~NDCR_SPARE_EN;
+
 	ndcr |= NDCR_ND_RUN;
 
 	/* clear status bits and run */
@@ -526,6 +532,7 @@ static int prepare_command_pool(struct pxa3xx_nand_info *info, int command,
 	info->buf_count		= 0;
 	info->oob_size		= 0;
 	info->use_ecc		= 0;
+	info->use_spare		= 1;
 	info->use_dma		= (use_dma) ? 1 : 0;
 	info->is_ready		= 0;
 	info->retcode		= ERR_NONE;
-- 
1.8.1.5

[PATCH v2 04/14] mtd: nand: pxa3xx: Support command buffer #3

[Ezequiel Garcia]

Some newer controllers support a fourth command buffer. This additional
command buffer allows to set an arbitrary length count, using the
NDCB3.NDLENCNT field, to perform non-standard length operations
such as the ONFI parameter page read.

In controllers without this register, the operation has no effect.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index a277def..65e3ee9 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -197,6 +197,7 @@ struct pxa3xx_nand_info {
 	uint32_t		ndcb0;
 	uint32_t		ndcb1;
 	uint32_t		ndcb2;
+	uint32_t		ndcb3;
 };
 
 static bool use_dma = 1;
@@ -496,6 +497,10 @@ static irqreturn_t pxa3xx_nand_irq(int irq, void *devid)
 		nand_writel(info, NDCB0, info->ndcb0);
 		nand_writel(info, NDCB0, info->ndcb1);
 		nand_writel(info, NDCB0, info->ndcb2);
+
+		/* NDCB3 register is available in NFCv2 (Armada 370/XP SoC) */
+		if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
+			nand_writel(info, NDCB0, info->ndcb3);
 	}
 
 	/* clear NDSR to let the controller exit the IRQ */
@@ -554,6 +559,7 @@ static int prepare_command_pool(struct pxa3xx_nand_info *info, int command,
 	default:
 		info->ndcb1 = 0;
 		info->ndcb2 = 0;
+		info->ndcb3 = 0;
 		break;
 	}
 
-- 
1.8.1.5

[PATCH v2 05/14] mtd: nand: pxa3xx: Use 'length override' in ONFI paramater page read

[Ezequiel Garcia]

The ONFI command 'parameter page read' needs a non-standard length.
Therefore, we enable the 'length override' field in NDCB0 and set
a non-zero 'length count' in NDCB3.

Additionally, the 'spare enable' bit must be disabled for any command
that sets a non-zero 'length count' in NDCB3.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 65e3ee9..d1e8ddb3 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -80,6 +80,7 @@
 #define NDSR_RDDREQ		(0x1 << 1)
 #define NDSR_WRCMDREQ		(0x1)
 
+#define NDCB0_LEN_OVRD		(0x1 << 28)
 #define NDCB0_ST_ROW_EN         (0x1 << 26)
 #define NDCB0_AUTO_RS		(0x1 << 25)
 #define NDCB0_CSEL		(0x1 << 24)
@@ -553,6 +554,9 @@ static int prepare_command_pool(struct pxa3xx_nand_info *info, int command,
 	case NAND_CMD_READOOB:
 		pxa3xx_set_datasize(info);
 		break;
+	case NAND_CMD_PARAM:
+		info->use_spare = 0;
+		break;
 	case NAND_CMD_SEQIN:
 		exec_cmd = 0;
 		break;
@@ -628,8 +632,10 @@ static int prepare_command_pool(struct pxa3xx_nand_info *info, int command,
 		info->buf_count = 256;
 		info->ndcb0 |= NDCB0_CMD_TYPE(0)
 				| NDCB0_ADDR_CYC(1)
+				| NDCB0_LEN_OVRD
 				| cmd;
 		info->ndcb1 = (column & 0xFF);
+		info->ndcb3 = 256;
 		info->data_size = 256;
 		break;
 
-- 
1.8.1.5

[PATCH v2 06/14] mtd: nand: pxa3xx: Add a local loop variable

[Ezequiel Garcia]

This is just a cosmetic change, to make the code more readable.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index d1e8ddb3..f5626c3 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -1318,8 +1318,9 @@ static int pxa3xx_nand_probe(struct platform_device *pdev)
 	info->variant = pxa3xx_nand_get_variant(pdev);
 	probe_success = 0;
 	for (cs = 0; cs < pdata->num_cs; cs++) {
+		struct mtd_info *mtd = info->host[cs]->mtd;
 		info->cs = cs;
-		ret = pxa3xx_nand_scan(info->host[cs]->mtd);
+		ret = pxa3xx_nand_scan(mtd);
 		if (ret) {
 			dev_warn(&pdev->dev, "failed to scan nand at cs %d\n",
 				cs);
@@ -1327,7 +1328,7 @@ static int pxa3xx_nand_probe(struct platform_device *pdev)
 		}
 
 		ppdata.of_node = pdev->dev.of_node;
-		ret = mtd_device_parse_register(info->host[cs]->mtd, NULL,
+		ret = mtd_device_parse_register(mtd, NULL,
 						&ppdata, pdata->parts[cs],
 						pdata->nr_parts[cs]);
 		if (!ret)
-- 
1.8.1.5

[PATCH v2 07/14] mtd: nand: pxa3xx: Remove hardcoded mtd name

[Ezequiel Garcia]

There's no advantage in using a hardcoded name for the mtd device.
Instead use the provided by the platform_device.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index f5626c3..122e77c 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -244,8 +244,6 @@ static struct pxa3xx_nand_flash builtin_flash_types[] = {
 /* Define a default flash type setting serve as flash detecting only */
 #define DEFAULT_FLASH_TYPE (&builtin_flash_types[0])
 
-const char *mtd_names[] = {"pxa3xx_nand-0", "pxa3xx_nand-1", NULL};
-
 #define NDTR0_tCH(c)	(min((c), 7) << 19)
 #define NDTR0_tCS(c)	(min((c), 7) << 16)
 #define NDTR0_tWH(c)	(min((c), 7) << 11)
@@ -1082,8 +1080,6 @@ KEEP_CONFIG:
 		host->row_addr_cycles = 3;
 	else
 		host->row_addr_cycles = 2;
-
-	mtd->name = mtd_names[0];
 	return nand_scan_tail(mtd);
 }
 
@@ -1319,6 +1315,8 @@ static int pxa3xx_nand_probe(struct platform_device *pdev)
 	probe_success = 0;
 	for (cs = 0; cs < pdata->num_cs; cs++) {
 		struct mtd_info *mtd = info->host[cs]->mtd;
+
+		mtd->name = pdev->name;
 		info->cs = cs;
 		ret = pxa3xx_nand_scan(mtd);
 		if (ret) {
-- 
1.8.1.5

[PATCH v2 08/14] mtd: nand: pxa3xx: Remove uneeded internal cmdset

[Ezequiel Garcia]

Use the defined macros for NAND command instead of using a constant
internal structure. This commit is only a cleanup, there's no
functionality modification.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c                | 63 ++++++++-------------------
 include/linux/platform_data/mtd-nand-pxa3xx.h | 13 ------
 2 files changed, 17 insertions(+), 59 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 122e77c..9f6e21a 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -131,7 +131,6 @@ enum pxa3xx_nand_variant {
 
 struct pxa3xx_nand_host {
 	struct nand_chip	chip;
-	struct pxa3xx_nand_cmdset *cmdset;
 	struct mtd_info         *mtd;
 	void			*info_data;
 
@@ -205,23 +204,6 @@ static bool use_dma = 1;
 module_param(use_dma, bool, 0444);
 MODULE_PARM_DESC(use_dma, "enable DMA for data transferring to/from NAND HW");
 
-/*
- * Default NAND flash controller configuration setup by the
- * bootloader. This configuration is used only when pdata->keep_config is set
- */
-static struct pxa3xx_nand_cmdset default_cmdset = {
-	.read1		= 0x3000,
-	.read2		= 0x0050,
-	.program	= 0x1080,
-	.read_status	= 0x0070,
-	.read_id	= 0x0090,
-	.erase		= 0xD060,
-	.reset		= 0x00FF,
-	.lock		= 0x002A,
-	.unlock		= 0x2423,
-	.lock_status	= 0x007A,
-};
-
 static struct pxa3xx_nand_timing timing[] = {
 	{ 40, 80, 60, 100, 80, 100, 90000, 400, 40, },
 	{ 10,  0, 20,  40, 30,  40, 11123, 110, 10, },
@@ -521,7 +503,6 @@ static inline int is_buf_blank(uint8_t *buf, size_t len)
 static int prepare_command_pool(struct pxa3xx_nand_info *info, int command,
 		uint16_t column, int page_addr)
 {
-	uint16_t cmd;
 	int addr_cycle, exec_cmd;
 	struct pxa3xx_nand_host *host;
 	struct mtd_info *mtd;
@@ -571,21 +552,17 @@ static int prepare_command_pool(struct pxa3xx_nand_info *info, int command,
 	switch (command) {
 	case NAND_CMD_READOOB:
 	case NAND_CMD_READ0:
-		cmd = host->cmdset->read1;
+		info->buf_start = column;
+		info->ndcb0 |= NDCB0_CMD_TYPE(0)
+				| addr_cycle
+				| NAND_CMD_READ0;
+
 		if (command == NAND_CMD_READOOB)
-			info->buf_start = mtd->writesize + column;
-		else
-			info->buf_start = column;
+			info->buf_start += mtd->writesize;
 
-		if (unlikely(host->page_size < PAGE_CHUNK_SIZE))
-			info->ndcb0 |= NDCB0_CMD_TYPE(0)
-					| addr_cycle
-					| (cmd & NDCB0_CMD1_MASK);
-		else
-			info->ndcb0 |= NDCB0_CMD_TYPE(0)
-					| NDCB0_DBC
-					| addr_cycle
-					| cmd;
+		/* Second command setting for large pages */
+		if (host->page_size >= PAGE_CHUNK_SIZE)
+			info->ndcb0 |= NDCB0_DBC | (NAND_CMD_READSTART << 8);
 
 	case NAND_CMD_SEQIN:
 		/* small page addr setting */
@@ -616,62 +593,58 @@ static int prepare_command_pool(struct pxa3xx_nand_info *info, int command,
 			break;
 		}
 
-		cmd = host->cmdset->program;
 		info->ndcb0 |= NDCB0_CMD_TYPE(0x1)
 				| NDCB0_AUTO_RS
 				| NDCB0_ST_ROW_EN
 				| NDCB0_DBC
-				| cmd
+				| (NAND_CMD_PAGEPROG << 8)
+				| NAND_CMD_SEQIN
 				| addr_cycle;
 		break;
 
 	case NAND_CMD_PARAM:
-		cmd = NAND_CMD_PARAM;
 		info->buf_count = 256;
 		info->ndcb0 |= NDCB0_CMD_TYPE(0)
 				| NDCB0_ADDR_CYC(1)
 				| NDCB0_LEN_OVRD
-				| cmd;
+				| command;
 		info->ndcb1 = (column & 0xFF);
 		info->ndcb3 = 256;
 		info->data_size = 256;
 		break;
 
 	case NAND_CMD_READID:
-		cmd = host->cmdset->read_id;
 		info->buf_count = host->read_id_bytes;
 		info->ndcb0 |= NDCB0_CMD_TYPE(3)
 				| NDCB0_ADDR_CYC(1)
-				| cmd;
+				| command;
 		info->ndcb1 = (column & 0xFF);
 
 		info->data_size = 8;
 		break;
 	case NAND_CMD_STATUS:
-		cmd = host->cmdset->read_status;
 		info->buf_count = 1;
 		info->ndcb0 |= NDCB0_CMD_TYPE(4)
 				| NDCB0_ADDR_CYC(1)
-				| cmd;
+				| command;
 
 		info->data_size = 8;
 		break;
 
 	case NAND_CMD_ERASE1:
-		cmd = host->cmdset->erase;
 		info->ndcb0 |= NDCB0_CMD_TYPE(2)
 				| NDCB0_AUTO_RS
 				| NDCB0_ADDR_CYC(3)
 				| NDCB0_DBC
-				| cmd;
+				| (NAND_CMD_ERASE2 << 8)
+				| NAND_CMD_ERASE1;
 		info->ndcb1 = page_addr;
 		info->ndcb2 = 0;
 
 		break;
 	case NAND_CMD_RESET:
-		cmd = host->cmdset->reset;
 		info->ndcb0 |= NDCB0_CMD_TYPE(5)
-				| cmd;
+				| command;
 
 		break;
 
@@ -867,7 +840,6 @@ static int pxa3xx_nand_config_flash(struct pxa3xx_nand_info *info,
 	}
 
 	/* calculate flash information */
-	host->cmdset = &default_cmdset;
 	host->page_size = f->page_size;
 	host->read_id_bytes = (f->page_size == 2048) ? 4 : 2;
 
@@ -913,7 +885,6 @@ static int pxa3xx_nand_detect_config(struct pxa3xx_nand_info *info)
 	}
 
 	host->reg_ndcr = ndcr & ~NDCR_INT_MASK;
-	host->cmdset = &default_cmdset;
 
 	host->ndtr0cs0 = nand_readl(info, NDTR0CS0);
 	host->ndtr1cs0 = nand_readl(info, NDTR1CS0);
diff --git a/include/linux/platform_data/mtd-nand-pxa3xx.h b/include/linux/platform_data/mtd-nand-pxa3xx.h
index c42f39f..ffb8019 100644
--- a/include/linux/platform_data/mtd-nand-pxa3xx.h
+++ b/include/linux/platform_data/mtd-nand-pxa3xx.h
@@ -16,19 +16,6 @@ struct pxa3xx_nand_timing {
 	unsigned int	tAR;  /* ND_ALE low to ND_nRE low delay */
 };
 
-struct pxa3xx_nand_cmdset {
-	uint16_t	read1;
-	uint16_t	read2;
-	uint16_t	program;
-	uint16_t	read_status;
-	uint16_t	read_id;
-	uint16_t	erase;
-	uint16_t	reset;
-	uint16_t	lock;
-	uint16_t	unlock;
-	uint16_t	lock_status;
-};
-
 struct pxa3xx_nand_flash {
 	char		*name;
 	uint32_t	chip_id;
-- 
1.8.1.5

[PATCH v2 09/14] mtd: nand: pxa3xx: Move cached registers to info structure

[Ezequiel Garcia]

This registers are not per-chip (aka host) but controller-wise,
so it's better to store them in the global 'info' structure.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 36 +++++++++++++++++-------------------
 1 file changed, 17 insertions(+), 19 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 9f6e21a..1498fd6 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -144,10 +144,6 @@ struct pxa3xx_nand_host {
 	unsigned int		row_addr_cycles;
 	size_t			read_id_bytes;
 
-	/* cached register value */
-	uint32_t		reg_ndcr;
-	uint32_t		ndtr0cs0;
-	uint32_t		ndtr1cs0;
 };
 
 struct pxa3xx_nand_info {
@@ -193,6 +189,11 @@ struct pxa3xx_nand_info {
 	unsigned int		oob_size;
 	int 			retcode;
 
+	/* cached register value */
+	uint32_t		reg_ndcr;
+	uint32_t		ndtr0cs0;
+	uint32_t		ndtr1cs0;
+
 	/* generated NDCBx register values */
 	uint32_t		ndcb0;
 	uint32_t		ndcb1;
@@ -258,8 +259,8 @@ static void pxa3xx_nand_set_timing(struct pxa3xx_nand_host *host,
 		NDTR1_tWHR(ns2cycle(t->tWHR, nand_clk)) |
 		NDTR1_tAR(ns2cycle(t->tAR, nand_clk));
 
-	host->ndtr0cs0 = ndtr0;
-	host->ndtr1cs0 = ndtr1;
+	info->ndtr0cs0 = ndtr0;
+	info->ndtr1cs0 = ndtr1;
 	nand_writel(info, NDTR0CS0, ndtr0);
 	nand_writel(info, NDTR1CS0, ndtr1);
 }
@@ -267,7 +268,7 @@ static void pxa3xx_nand_set_timing(struct pxa3xx_nand_host *host,
 static void pxa3xx_set_datasize(struct pxa3xx_nand_info *info)
 {
 	struct pxa3xx_nand_host *host = info->host[info->cs];
-	int oob_enable = host->reg_ndcr & NDCR_SPARE_EN;
+	int oob_enable = info->reg_ndcr & NDCR_SPARE_EN;
 
 	info->data_size = host->page_size;
 	if (!oob_enable) {
@@ -293,10 +294,9 @@ static void pxa3xx_set_datasize(struct pxa3xx_nand_info *info)
  */
 static void pxa3xx_nand_start(struct pxa3xx_nand_info *info)
 {
-	struct pxa3xx_nand_host *host = info->host[info->cs];
 	uint32_t ndcr;
 
-	ndcr = host->reg_ndcr;
+	ndcr = info->reg_ndcr;
 
 	if (info->use_ecc)
 		ndcr |= NDCR_ECC_EN;
@@ -674,7 +674,7 @@ static void pxa3xx_nand_cmdfunc(struct mtd_info *mtd, unsigned command,
 	 * "byte" address into a "word" address appropriate
 	 * for indexing a word-oriented device
 	 */
-	if (host->reg_ndcr & NDCR_DWIDTH_M)
+	if (info->reg_ndcr & NDCR_DWIDTH_M)
 		column /= 2;
 
 	/*
@@ -684,8 +684,8 @@ static void pxa3xx_nand_cmdfunc(struct mtd_info *mtd, unsigned command,
 	 */
 	if (info->cs != host->cs) {
 		info->cs = host->cs;
-		nand_writel(info, NDTR0CS0, host->ndtr0cs0);
-		nand_writel(info, NDTR1CS0, host->ndtr1cs0);
+		nand_writel(info, NDTR0CS0, info->ndtr0cs0);
+		nand_writel(info, NDTR1CS0, info->ndtr1cs0);
 	}
 
 	info->state = STATE_PREPARED;
@@ -861,7 +861,7 @@ static int pxa3xx_nand_config_flash(struct pxa3xx_nand_info *info,
 	ndcr |= NDCR_RD_ID_CNT(host->read_id_bytes);
 	ndcr |= NDCR_SPARE_EN; /* enable spare by default */
 
-	host->reg_ndcr = ndcr;
+	info->reg_ndcr = ndcr;
 
 	pxa3xx_nand_set_timing(host, f->timing);
 	return 0;
@@ -884,11 +884,9 @@ static int pxa3xx_nand_detect_config(struct pxa3xx_nand_info *info)
 		host->read_id_bytes = 2;
 	}
 
-	host->reg_ndcr = ndcr & ~NDCR_INT_MASK;
-
-	host->ndtr0cs0 = nand_readl(info, NDTR0CS0);
-	host->ndtr1cs0 = nand_readl(info, NDTR1CS0);
-
+	info->reg_ndcr = ndcr & ~NDCR_INT_MASK;
+	info->ndtr0cs0 = nand_readl(info, NDTR0CS0);
+	info->ndtr1cs0 = nand_readl(info, NDTR1CS0);
 	return 0;
 }
 
@@ -1035,7 +1033,7 @@ KEEP_CONFIG:
 	chip->ecc.size = host->page_size;
 	chip->ecc.strength = 1;
 
-	if (host->reg_ndcr & NDCR_DWIDTH_M)
+	if (info->reg_ndcr & NDCR_DWIDTH_M)
 		chip->options |= NAND_BUSWIDTH_16;
 
 	if (nand_scan_ident(mtd, 1, def))
-- 
1.8.1.5

[PATCH v2 10/14] mtd: nand: pxa3xx: Make dma code dependent on dma capable platforms

[Ezequiel Garcia]

This patch adds a macro ARCH_HAS_DMA to compile-out arch specific
dma code, namely pxa_request_dma() and pxa_free_dma(). These symbols
are available only in pxa, which makes impossible to build the driver in
other platforms than ARCH_PXA.

In order to handle non-dma capable platforms, we implement a fallbacks that
allocate buffers as if 'use_dma=false', putting the dma related code
under the ARCH_HAS_DMA conditional.

Please note that the correct way to handle this is to migrate the
dma code to use of the mmp_pdma dmaengine driver. However, currently
this is not possible because the two dmaengine drivers can't work together.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 1498fd6..523dd35 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -25,7 +25,14 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 
+#if defined(CONFIG_ARCH_PXA) || defined(CONFIG_ARCH_MMP)
+#define ARCH_HAS_DMA
+#endif
+
+#ifdef ARCH_HAS_DMA
 #include <mach/dma.h>
+#endif
+
 #include <linux/platform_data/mtd-nand-pxa3xx.h>
 
 #define	CHIP_DELAY_TIMEOUT	(2 * HZ/10)
@@ -381,6 +388,7 @@ static void handle_data_pio(struct pxa3xx_nand_info *info)
 	}
 }
 
+#ifdef ARCH_HAS_DMA
 static void start_data_dma(struct pxa3xx_nand_info *info)
 {
 	struct pxa_dma_desc *desc = info->data_desc;
@@ -427,6 +435,10 @@ static void pxa3xx_nand_data_dma_irq(int channel, void *data)
 	enable_int(info, NDCR_INT_MASK);
 	nand_writel(info, NDSR, NDSR_WRDREQ | NDSR_RDDREQ);
 }
+#else
+static void start_data_dma(struct pxa3xx_nand_info *info)
+{}
+#endif
 
 static irqreturn_t pxa3xx_nand_irq(int irq, void *devid)
 {
@@ -896,6 +908,7 @@ static int pxa3xx_nand_detect_config(struct pxa3xx_nand_info *info)
  */
 #define MAX_BUFF_SIZE	PAGE_SIZE
 
+#ifdef ARCH_HAS_DMA
 static int pxa3xx_nand_init_buff(struct pxa3xx_nand_info *info)
 {
 	struct platform_device *pdev = info->pdev;
@@ -941,6 +954,20 @@ static void pxa3xx_nand_free_buff(struct pxa3xx_nand_info *info)
 		kfree(info->data_buff);
 	}
 }
+#else
+static int pxa3xx_nand_init_buff(struct pxa3xx_nand_info *info)
+{
+	info->data_buff = kmalloc(MAX_BUFF_SIZE, GFP_KERNEL);
+	if (info->data_buff == NULL)
+		return -ENOMEM;
+	return 0;
+}
+
+static void pxa3xx_nand_free_buff(struct pxa3xx_nand_info *info)
+{
+	kfree(info->data_buff);
+}
+#endif
 
 static int pxa3xx_nand_sensing(struct pxa3xx_nand_info *info)
 {
@@ -1263,6 +1290,13 @@ static int pxa3xx_nand_probe(struct platform_device *pdev)
 	struct pxa3xx_nand_info *info;
 	int ret, cs, probe_success;
 
+#ifndef ARCH_HAS_DMA
+	if (use_dma) {
+		use_dma = 0;
+		dev_warn(&pdev->dev,
+			 "This platform can't do DMA on this device\n");
+	}
+#endif
 	ret = pxa3xx_nand_probe_dt(pdev);
 	if (ret)
 		return ret;
-- 
1.8.1.5

[PATCH v2 11/14] mtd: nand: pxa3xx: Add __maybe_unused keyword to enable_int()

[Ezequiel Garcia]

Now that we have added ARCH_HAS_DMA conditional the function
enable_int() may be unused. Declare it as __maybe_unused,
in order to remove the following warning, when the function is not used:

drivers/mtd/nand//pxa3xx_nand.c:343:24: warning: 'enable_int' defined
but not used [-Wunused-function]

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 523dd35..3952680 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -348,7 +348,7 @@ static void pxa3xx_nand_stop(struct pxa3xx_nand_info *info)
 	nand_writel(info, NDSR, NDSR_MASK);
 }
 
-static void enable_int(struct pxa3xx_nand_info *info, uint32_t int_mask)
+static void __maybe_unused enable_int(struct pxa3xx_nand_info *info, uint32_t int_mask)
 {
 	uint32_t ndcr;
 
-- 
1.8.1.5

[PATCH v2 12/14] mtd: nand: pxa3xx: Allow devices with no dma resources

[Ezequiel Garcia]

When use_dma=0 there's no point in requesting resources for dma,
since they won't be used anyway. Therefore we remove that requirement,
therefore allowing devices without dma to pass the driver probe.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 51 +++++++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 23 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 3952680..5994b88 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -1131,30 +1131,35 @@ static int alloc_nand_resource(struct platform_device *pdev)
 	if (ret < 0)
 		return ret;
 
-	/*
-	 * This is a dirty hack to make this driver work from devicetree
-	 * bindings. It can be removed once we have a prober DMA controller
-	 * framework for DT.
-	 */
-	if (pdev->dev.of_node && of_machine_is_compatible("marvell,pxa3xx")) {
-		info->drcmr_dat = 97;
-		info->drcmr_cmd = 99;
-	} else {
-		r = platform_get_resource(pdev, IORESOURCE_DMA, 0);
-		if (r == NULL) {
-			dev_err(&pdev->dev, "no resource defined for data DMA\n");
-			ret = -ENXIO;
-			goto fail_disable_clk;
-		}
-		info->drcmr_dat = r->start;
-
-		r = platform_get_resource(pdev, IORESOURCE_DMA, 1);
-		if (r == NULL) {
-			dev_err(&pdev->dev, "no resource defined for command DMA\n");
-			ret = -ENXIO;
-			goto fail_disable_clk;
+	if (use_dma) {
+		/*
+		 * This is a dirty hack to make this driver work from
+		 * devicetree bindings. It can be removed once we have
+		 * a prober DMA controller framework for DT.
+		 */
+		if (pdev->dev.of_node &&
+		    of_machine_is_compatible("marvell,pxa3xx")) {
+			info->drcmr_dat = 97;
+			info->drcmr_cmd = 99;
+		} else {
+			r = platform_get_resource(pdev, IORESOURCE_DMA, 0);
+			if (r == NULL) {
+				dev_err(&pdev->dev,
+					"no resource defined for data DMA\n");
+				ret = -ENXIO;
+				goto fail_disable_clk;
+			}
+			info->drcmr_dat = r->start;
+
+			r = platform_get_resource(pdev, IORESOURCE_DMA, 1);
+			if (r == NULL) {
+				dev_err(&pdev->dev,
+					"no resource defined for cmd DMA\n");
+				ret = -ENXIO;
+				goto fail_disable_clk;
+			}
+			info->drcmr_cmd = r->start;
 		}
-		info->drcmr_cmd = r->start;
 	}
 
 	irq = platform_get_irq(pdev, 0);
-- 
1.8.1.5

[PATCH v2 13/14] mtd: nand: pxa3xx: Increase data buffer size

[Ezequiel Garcia]

Devices with 4 KiB pages (plus OOB data) can be detected, so we increase
the data buffer size. A better solution would be to allocate a buffer
depending on the detected page size, but that's not possible given
we need the buffer prior to the device detection.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 5994b88..e3eb961 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -906,7 +906,7 @@ static int pxa3xx_nand_detect_config(struct pxa3xx_nand_info *info)
  * is: 2048 + 64 = 2112 bytes, allocate a page here for both the
  * data buffer and the DMA descriptor
  */
-#define MAX_BUFF_SIZE	PAGE_SIZE
+#define MAX_BUFF_SIZE	(PAGE_SIZE*2)
 
 #ifdef ARCH_HAS_DMA
 static int pxa3xx_nand_init_buff(struct pxa3xx_nand_info *info)
-- 
1.8.1.5

[PATCH v2 14/14] mtd: nand: Allow to build pxa3xx_nand on Orion platforms

[Ezequiel Garcia]

The Armada 370 and Armada XP SoC families, selected by PLAT_ORION,
have a Nand Flash Controller (NFC) IP very similar to the one present
in PXA platforms. Therefore, we want to build this driver on PLAT_ORION.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/mtd/nand/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 5ef8f5e..86c135b 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -354,7 +354,7 @@ config MTD_NAND_ATMEL
 
 config MTD_NAND_PXA3xx
 	tristate "Support for NAND flash devices on PXA3xx"
-	depends on PXA3xx || ARCH_MMP
+	depends on PXA3xx || ARCH_MMP || PLAT_ORION
 	help
 	  This enables the driver for the NAND flash device found on
 	  PXA3xx processors
-- 
1.8.1.5

Re: [PATCH v2 00/14] pxa3xx-nand patches to support mvebu builds

[Daniel Mack]

On 07.08.2013 14:31, Ezequiel Garcia wrote:
> Hello everyone!
> 
> This patchset is part of the work I'm doing to enable the pxa3xx-nand
> driver to support the NAND controller in Armada 370/XP. While this is
> work in progress, here's a subset of patches I consider good enough
> for mainline.

Nice. For the whole series:

  Tested-by: Daniel Mack <zonque@gmail.com>


Thanks,
Daniel

Re: [PATCH v2 00/14] pxa3xx-nand patches to support mvebu builds

[Ezequiel Garcia]

On Wed, Aug 07, 2013 at 02:45:27PM +0200, Daniel Mack wrote:
> On 07.08.2013 14:31, Ezequiel Garcia wrote:
> > Hello everyone!
> > 
> > This patchset is part of the work I'm doing to enable the pxa3xx-nand
> > driver to support the NAND controller in Armada 370/XP. While this is
> > work in progress, here's a subset of patches I consider good enough
> > for mainline.
> 
> Nice. For the whole series:
> 
>   Tested-by: Daniel Mack <zonque@gmail.com>
> 

Thanks Daniel!

FWIW, there's room for another bunch of cleanups.

For instance, the driver does its own device detection which should be
handled by the NAND core instead. I have a few patches to take care of this
but they far more intrusive changes. Therefore I'm postponing them until
I carefully review them to prevent regressions.

-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH v2 00/14] pxa3xx-nand patches to support mvebu builds

[Ezequiel Garcia]

Brian, Artem:

On Wed, Aug 07, 2013 at 09:31:05AM -0300, Ezequiel Garcia wrote:
> Hello everyone!
> 
> This patchset is part of the work I'm doing to enable the pxa3xx-nand
> driver to support the NAND controller in Armada 370/XP. While this is
> work in progress, here's a subset of patches I consider good enough
> for mainline.
> 
> Since pxa3xx has a mach-specific DMA API, I'm including a few patches to
> ifdef such pxa-specific code in non PXA/MMP platforms.
> This is only an ugly workaround and should be removed once the specific
> DMA API is replaced by dmaengine.
> 
> Daniel Mack is already working on that, so it's expected this removal
> will happen soon.
> 
> Other than that, the rest of the patchset is just assorted cleanups
> and minor fixes.
> 
> This patchset is based in Artem's l2-mtd master branch.
> 
> If at all possible and if no regressions are reported I'd like to see
> this queued for v3.12.
> 
> Changes from v1:
> 
>   * Use __maybe_unused instead of inline keyword for a symbol
>     that's maybe unused. Suggested by Brian Norris.
> 
>   * As Daniel Mack reported, the PXA nand controller (NFCv1)
>     lacks an NDBC3 register and it's access is undefined.
>     This means we need a way to distinguish between the two
>     controllers (NFCv1 and NFCv2).
> 
>     We introduce a new 'marvell,armada370-nand' compatible
>     string to distinguish the newer controller (NFCv2).
> 
> Ezequiel Garcia (14):
>   mtd: nand: pxa3xx: Introduce 'marvell,armada370-nand' compatible
>     string
>   mtd: nand: pxa3xx: Handle ECC and DMA enable/disable properly
>   mtd: nand: pxa3xx: Allow to set/clear the 'spare enable' field
>   mtd: nand: pxa3xx: Support command buffer #3
>   mtd: nand: pxa3xx: Use 'length override' in ONFI paramater page read
>   mtd: nand: pxa3xx: Add a local loop variable
>   mtd: nand: pxa3xx: Remove hardcoded mtd name
>   mtd: nand: pxa3xx: Remove uneeded internal cmdset
>   mtd: nand: pxa3xx: Move cached registers to info structure
>   mtd: nand: pxa3xx: Make dma code dependent on dma capable platforms
>   mtd: nand: pxa3xx: Add __maybe_unused keyword to enable_int()
>   mtd: nand: pxa3xx: Allow devices with no dma resources
>   mtd: nand: pxa3xx: Increase data buffer size
>   mtd: nand: Allow to build pxa3xx_nand on Orion platforms
> 
>  drivers/mtd/nand/Kconfig                      |   2 +-
>  drivers/mtd/nand/pxa3xx_nand.c                | 269 ++++++++++++++++----------
>  include/linux/platform_data/mtd-nand-pxa3xx.h |  13 --
>  3 files changed, 171 insertions(+), 113 deletions(-)
> 

Is there anything to review on this series?

Otherwise, I'd like this to be queued for v3.12 -if at all possible.

Thanks!
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH v2 00/14] pxa3xx-nand patches to support mvebu builds

[Brian Norris]

On Sat, Aug 10, 2013 at 5:12 AM, Ezequiel Garcia
<ezequiel.garcia@free-electrons.com> wrote:
> Brian, Artem:
>
> On Wed, Aug 07, 2013 at 09:31:05AM -0300, Ezequiel Garcia wrote:
>> Hello everyone!
>>
>> This patchset is part of the work I'm doing to enable the pxa3xx-nand
>> driver to support the NAND controller in Armada 370/XP. While this is
>> work in progress, here's a subset of patches I consider good enough
>> for mainline.
>>
>> Since pxa3xx has a mach-specific DMA API, I'm including a few patches to
>> ifdef such pxa-specific code in non PXA/MMP platforms.
>> This is only an ugly workaround and should be removed once the specific
>> DMA API is replaced by dmaengine.
>>
>> Daniel Mack is already working on that, so it's expected this removal
>> will happen soon.
>>
>> Other than that, the rest of the patchset is just assorted cleanups
>> and minor fixes.
>>
>> This patchset is based in Artem's l2-mtd master branch.
>>
>> If at all possible and if no regressions are reported I'd like to see
>> this queued for v3.12.
>>
>> Changes from v1:
>>
>>   * Use __maybe_unused instead of inline keyword for a symbol
>>     that's maybe unused. Suggested by Brian Norris.
>>
>>   * As Daniel Mack reported, the PXA nand controller (NFCv1)
>>     lacks an NDBC3 register and it's access is undefined.
>>     This means we need a way to distinguish between the two
>>     controllers (NFCv1 and NFCv2).
>>
>>     We introduce a new 'marvell,armada370-nand' compatible
>>     string to distinguish the newer controller (NFCv2).
>>
>> Ezequiel Garcia (14):
>>   mtd: nand: pxa3xx: Introduce 'marvell,armada370-nand' compatible
>>     string
>>   mtd: nand: pxa3xx: Handle ECC and DMA enable/disable properly
>>   mtd: nand: pxa3xx: Allow to set/clear the 'spare enable' field
>>   mtd: nand: pxa3xx: Support command buffer #3
>>   mtd: nand: pxa3xx: Use 'length override' in ONFI paramater page read
>>   mtd: nand: pxa3xx: Add a local loop variable
>>   mtd: nand: pxa3xx: Remove hardcoded mtd name
>>   mtd: nand: pxa3xx: Remove uneeded internal cmdset
>>   mtd: nand: pxa3xx: Move cached registers to info structure
>>   mtd: nand: pxa3xx: Make dma code dependent on dma capable platforms
>>   mtd: nand: pxa3xx: Add __maybe_unused keyword to enable_int()
>>   mtd: nand: pxa3xx: Allow devices with no dma resources
>>   mtd: nand: pxa3xx: Increase data buffer size
>>   mtd: nand: Allow to build pxa3xx_nand on Orion platforms
>>
>>  drivers/mtd/nand/Kconfig                      |   2 +-
>>  drivers/mtd/nand/pxa3xx_nand.c                | 269 ++++++++++++++++----------
>>  include/linux/platform_data/mtd-nand-pxa3xx.h |  13 --
>>  3 files changed, 171 insertions(+), 113 deletions(-)
>>
>
> Is there anything to review on this series?

Sorry, I'm still getting into the swing of things as a
(sub)maintainer, and I was previously remiss on reviewing your patches
in depth. This series is next on my list though.

> Otherwise, I'd like this to be queued for v3.12 -if at all possible.

If everything looks OK, I'll try to get it queued up soon enough that
it can make it for 3.12.

Brian