Patchwork [v6] ARM: mtd: nand: davinci: add OF support for davinci nand controller

login
register
mail settings
Submitter Heiko Schocher
Date July 30, 2012, 7:22 a.m.
Message ID <1343632944-25971-1-git-send-email-hs@denx.de>
Download mbox | patch
Permalink /patch/173978/
State New
Headers show

Comments

Heiko Schocher - July 30, 2012, 7:22 a.m.
add OF support for the davinci nand controller.

Signed-off-by: Heiko Schocher <hs@denx.de>
Cc: davinci-linux-open-source@linux.davincidsp.com
Cc: linux-arm-kernel@lists.infradead.org
Cc: devicetree-discuss@lists.ozlabs.org
Cc: linux-mtd@lists.infradead.org
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Scott Wood <scottwood@freescale.com>

---
- changes for v2:
  - add comments from Scott Wood:
    - add "ti,davinci-" prefix
    - Dashes are preferred to underscores
    - rename "nandflash" to "nand"
    - introduce new "ti,davinci" specific properties for setting
      up ecc_mode, ecc_bits, options and bbt options, instead
      using linux defines
    - readme fixes
- no changes for v3
- changes for v4
  remove "pinmux-handle" property as discussed here:
  http://www.spinics.net/lists/arm-kernel/msg175701.html
  with Nori Sekhar
- no changes for v5

- changes for v6:
  add comments from Sekhar Nori:
  - remove unnecessary includes
  - use devm_kzalloc instead of kzalloc
  - remove AEMIF timing specific bindings, as
    this has to be done later.

 .../devicetree/bindings/arm/davinci/nand.txt       |   51 ++++++++++++++
 drivers/mtd/nand/davinci_nand.c                    |   72 +++++++++++++++++++-
 2 files changed, 122 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/davinci/nand.txt
Heiko Schocher - Aug. 7, 2012, 2:34 p.m.
Hello,

On 30.07.2012 09:22, Heiko Schocher wrote:
> add OF support for the davinci nand controller.
>
> Signed-off-by: Heiko Schocher<hs@denx.de>
> Cc: davinci-linux-open-source@linux.davincidsp.com
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: devicetree-discuss@lists.ozlabs.org
> Cc: linux-mtd@lists.infradead.org
> Cc: David Woodhouse<dwmw2@infradead.org>
> Cc: Grant Likely<grant.likely@secretlab.ca>
> Cc: Sekhar Nori<nsekhar@ti.com>
> Cc: Wolfgang Denk<wd@denx.de>
> Cc: Scott Wood<scottwood@freescale.com>
>
> ---
> - changes for v2:
>    - add comments from Scott Wood:
>      - add "ti,davinci-" prefix
>      - Dashes are preferred to underscores
>      - rename "nandflash" to "nand"
>      - introduce new "ti,davinci" specific properties for setting
>        up ecc_mode, ecc_bits, options and bbt options, instead
>        using linux defines
>      - readme fixes
> - no changes for v3
> - changes for v4
>    remove "pinmux-handle" property as discussed here:
>    http://www.spinics.net/lists/arm-kernel/msg175701.html
>    with Nori Sekhar
> - no changes for v5
>
> - changes for v6:
>    add comments from Sekhar Nori:
>    - remove unnecessary includes
>    - use devm_kzalloc instead of kzalloc
>    - remove AEMIF timing specific bindings, as
>      this has to be done later.
>
>   .../devicetree/bindings/arm/davinci/nand.txt       |   51 ++++++++++++++
>   drivers/mtd/nand/davinci_nand.c                    |   72 +++++++++++++++++++-
>   2 files changed, 122 insertions(+), 1 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/arm/davinci/nand.txt

ping ... any comments?

bye,
Heiko
Sekhar Nori - Aug. 7, 2012, 5:36 p.m.
On 7/30/2012 12:52 PM, Heiko Schocher wrote:
> add OF support for the davinci nand controller.
> 
> Signed-off-by: Heiko Schocher <hs@denx.de>

Acked-by: Sekhar Nori <nsekhar@ti.com>

Thanks,
Sekhar
Artem Bityutskiy - Aug. 23, 2012, 3:34 p.m.
On Mon, 2012-07-30 at 09:22 +0200, Heiko Schocher wrote:
> add OF support for the davinci nand controller.
> 
> Signed-off-by: Heiko Schocher <hs@denx.de>

Pushed to l2-mtd.git, thanks!
Sergei Shtylyov - Jan. 2, 2013, 7:43 p.m.
Hello.

On 07/30/2012 11:22 AM, Heiko Schocher wrote:

> add OF support for the davinci nand controller.

> Signed-off-by: Heiko Schocher <hs@denx.de>
> Cc: davinci-linux-open-source@linux.davincidsp.com
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: devicetree-discuss@lists.ozlabs.org
> Cc: linux-mtd@lists.infradead.org
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Scott Wood <scottwood@freescale.com>

[...]

> diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
> index d94b03c..f386b3c 100644
> --- a/drivers/mtd/nand/davinci_nand.c
> +++ b/drivers/mtd/nand/davinci_nand.c
[...]
> @@ -518,9 +519,75 @@ static struct nand_ecclayout hwecc4_2048 __initconst = {
>  	},
>  };
>  
> +#if defined(CONFIG_OF)
> +static const struct of_device_id davinci_nand_of_match[] = {
> +	{.compatible = "ti,davinci-nand", },
> +	{},
> +}

   I have only one question: have you ever try to compile this patch with
CONFIG_OF enabled? If you have, you would have noticed:

drivers/mtd/nand/davinci_nand.c:527: error: expected ‘,’ or ‘;’ before ‘extern’

> +MODULE_DEVICE_TABLE(of, davinci_nand_of_match);

   No need to worry now, I'll send out the trivial patch...

WBR, Sergei
Heiko Schocher - Jan. 3, 2013, 7:03 a.m.
Hello Sergei,

On 02.01.2013 20:43, Sergei Shtylyov wrote:
> Hello.
>
> On 07/30/2012 11:22 AM, Heiko Schocher wrote:
>
>> add OF support for the davinci nand controller.
>
>> Signed-off-by: Heiko Schocher<hs@denx.de>
>> Cc: davinci-linux-open-source@linux.davincidsp.com
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: devicetree-discuss@lists.ozlabs.org
>> Cc: linux-mtd@lists.infradead.org
>> Cc: David Woodhouse<dwmw2@infradead.org>
>> Cc: Grant Likely<grant.likely@secretlab.ca>
>> Cc: Sekhar Nori<nsekhar@ti.com>
>> Cc: Wolfgang Denk<wd@denx.de>
>> Cc: Scott Wood<scottwood@freescale.com>
>
> [...]
>
>> diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
>> index d94b03c..f386b3c 100644
>> --- a/drivers/mtd/nand/davinci_nand.c
>> +++ b/drivers/mtd/nand/davinci_nand.c
> [...]
>> @@ -518,9 +519,75 @@ static struct nand_ecclayout hwecc4_2048 __initconst = {
>>   	},
>>   };
>>
>> +#if defined(CONFIG_OF)
>> +static const struct of_device_id davinci_nand_of_match[] = {
>> +	{.compatible = "ti,davinci-nand", },
>> +	{},
>> +}
>
>     I have only one question: have you ever try to compile this patch with
> CONFIG_OF enabled? If you have, you would have noticed:
>
> drivers/mtd/nand/davinci_nand.c:527: error: expected ‘,’ or ‘;’ before ‘extern’
>
>> +MODULE_DEVICE_TABLE(of, davinci_nand_of_match);

Hmm.. maybe this crept in later after I sent the patches? They were
pending for a while ... I compiled it just yet again (based on my
tree when I posted this patch based on commit:

commit 3bf671af14d591ede9251acb0085e8017f3705e7
Merge: 1c212c6 4a5a418
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date:   Mon Aug 13 09:59:04 2012 +0300

     Merge branch 'fixes-for-3.6' of git://git.kernel.org/pub/scm/linux/kernel/git/cooloney/linux-leds

and it compiled fine:

$ eldk-switch -r 5.2 armv7a
Setup for armv7a (using ELDK 5.2)
$ make -s uImage
arch/arm/kernel/return_address.c:62:2: warning: #warning "TODO: return_address should use unwind tables" [-Wcpp]
arch/arm/kernel/return_address.c:62:2: warning: #warning "TODO: return_address should use unwind tables" [-Wcpp]
arch/arm/mm/alignment.c: In function 'do_alignment':
arch/arm/mm/alignment.c:327:15: warning: 'offset.un' may be used uninitialized in this function [-Wuninitialized]
arch/arm/mm/alignment.c:749:21: note: 'offset.un' was declared here
drivers/mtd/chips/cfi_cmdset_0002.c: In function 'cfi_amdstd_panic_write':
include/linux/mtd/map.h:331:11: warning: 'r.x[0]' may be used uninitialized in this function [-Wuninitialized]
drivers/mtd/chips/cfi_cmdset_0002.c: In function 'cfi_amdstd_write_words':
include/linux/mtd/map.h:331:11: warning: 'r.x[0]' may be used uninitialized in this function [-Wuninitialized]
drivers/mtd/chips/cfi_cmdset_0001.c: In function 'cfi_intelext_write_words':
include/linux/mtd/map.h:331:11: warning: 'r.x[0]' may be used uninitialized in this function [-Wuninitialized]
   Kernel: arch/arm/boot/Image is ready
   Kernel: arch/arm/boot/zImage is ready
Image Name:   Linux-3.6.0-rc1-01880-g0227e8b
Created:      Thu Jan  3 06:55:26 2013
Image Type:   ARM Linux Kernel Image (uncompressed)
Data Size:    2432040 Bytes = 2375.04 kB = 2.32 MB
Load Address: c0008000
Entry Point:  c0008000
   Image arch/arm/boot/uImage is ready
$
$ ${CROSS_COMPILE}gcc --version
arm-linux-gnueabi-gcc (GCC) 4.6.4 20120303 (prerelease)
Copyright (C) 2011 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$ grep -n CONFIG_OF .config
707:CONFIG_OF=y
713:# CONFIG_OF_SELFTEST is not set
714:CONFIG_OF_FLATTREE=y
715:CONFIG_OF_EARLY_FLATTREE=y
716:CONFIG_OF_ADDRESS=y
717:CONFIG_OF_IRQ=y
718:CONFIG_OF_DEVICE=y
719:CONFIG_OF_I2C=y
720:CONFIG_OF_NET=y
721:CONFIG_OF_MDIO=y
722:CONFIG_OF_MTD=y
1067:CONFIG_OF_GPIO=y
1629:CONFIG_OF_IOMMU=y
$

>     No need to worry now, I'll send out the trivial patch...

Thanks!

bye,
Heiko
Sergei Shtylyov - Jan. 3, 2013, 11:26 a.m.
Hello.

On 03-01-2013 11:03, Heiko Schocher wrote:

>>> add OF support for the davinci nand controller.

>>> Signed-off-by: Heiko Schocher<hs@denx.de>
>>> Cc: davinci-linux-open-source@linux.davincidsp.com
>>> Cc: linux-arm-kernel@lists.infradead.org
>>> Cc: devicetree-discuss@lists.ozlabs.org
>>> Cc: linux-mtd@lists.infradead.org
>>> Cc: David Woodhouse<dwmw2@infradead.org>
>>> Cc: Grant Likely<grant.likely@secretlab.ca>
>>> Cc: Sekhar Nori<nsekhar@ti.com>
>>> Cc: Wolfgang Denk<wd@denx.de>
>>> Cc: Scott Wood<scottwood@freescale.com>

>> [...]

>>> diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
>>> index d94b03c..f386b3c 100644
>>> --- a/drivers/mtd/nand/davinci_nand.c
>>> +++ b/drivers/mtd/nand/davinci_nand.c
>> [...]
>>> @@ -518,9 +519,75 @@ static struct nand_ecclayout hwecc4_2048 __initconst = {
>>>       },
>>>   };

>>> +#if defined(CONFIG_OF)
>>> +static const struct of_device_id davinci_nand_of_match[] = {
>>> +    {.compatible = "ti,davinci-nand", },
>>> +    {},
>>> +}

>>     I have only one question: have you ever try to compile this patch with
>> CONFIG_OF enabled? If you have, you would have noticed:

>> drivers/mtd/nand/davinci_nand.c:527: error: expected ‘,’ or ‘;’ before ‘extern’

>>> +MODULE_DEVICE_TABLE(of, davinci_nand_of_match);

> Hmm.. maybe this crept in later after I sent the patches? They were
> pending for a while ... I compiled it just yet again (based on my
> tree when I posted this patch based on commit:

    I've just checked the archives: every patch version you posted had ';' 
after '}' missing.

WBR, Sergei
David Woodhouse - Jan. 3, 2013, 11:32 a.m.
On Thu, 2013-01-03 at 15:26 +0400, Sergei Shtylyov wrote:
> >>> +#if defined(CONFIG_OF)
> >>> +static const struct of_device_id davinci_nand_of_match[] = {
> >>> +    {.compatible = "ti,davinci-nand", },
> >>> +    {},
> >>> +}
> >>> +MODULE_DEVICE_TABLE(of, davinci_nand_of_match);
> 
> > Hmm.. maybe this crept in later after I sent the patches? They were
> > pending for a while ... I compiled it just yet again (based on my
> > tree when I posted this patch based on commit:
> 
>     I've just checked the archives: every patch version you posted had ';' 
> after '}' missing.

If it was built-in, rather than as a module, then MODULE_DEVICE_TABLE(…)
expands to nothing, and the structure gets to use the semicolon from the
end of that line.
Sergei Shtylyov - Jan. 3, 2013, 11:39 a.m.
Hello.

On 03-01-2013 15:32, David Woodhouse wrote:

>>>>> +#if defined(CONFIG_OF)
>>>>> +static const struct of_device_id davinci_nand_of_match[] = {
>>>>> +    {.compatible = "ti,davinci-nand", },
>>>>> +    {},
>>>>> +}
>>>>> +MODULE_DEVICE_TABLE(of, davinci_nand_of_match);

>>> Hmm.. maybe this crept in later after I sent the patches? They were
>>> pending for a while ... I compiled it just yet again (based on my
>>> tree when I posted this patch based on commit:

>>      I've just checked the archives: every patch version you posted had ';'
>> after '}' missing.

> If it was built-in, rather than as a module, then MODULE_DEVICE_TABLE(…)
> expands to nothing, and the structure gets to use the semicolon from the
> end of that line.

    Ah, that explains it: 'davinci_all_defconfig' has 
CONFIG_MTD_NAND_DAVINCI=m, so that's how the error got triggered at last. 
Probably worth adding this explanation to the changelog, how do you think?

WBR, Sergei
Sergei Shtylyov - Jan. 3, 2013, 5:56 p.m.
Hello.

On 01/03/2013 02:39 PM, Sergei Shtylyov wrote:

>>>>>> +#if defined(CONFIG_OF)
>>>>>> +static const struct of_device_id davinci_nand_of_match[] = {
>>>>>> +    {.compatible = "ti,davinci-nand", },
>>>>>> +    {},
>>>>>> +}
>>>>>> +MODULE_DEVICE_TABLE(of, davinci_nand_of_match);

>>>> Hmm.. maybe this crept in later after I sent the patches? They were
>>>> pending for a while ... I compiled it just yet again (based on my
>>>> tree when I posted this patch based on commit:

>>>      I've just checked the archives: every patch version you posted had ';'
>>> after '}' missing.

>> If it was built-in, rather than as a module, then MODULE_DEVICE_TABLE(…)
>> expands to nothing, and the structure gets to use the semicolon from the
>> end of that line.

>    Ah, that explains it: 'davinci_all_defconfig' has CONFIG_MTD_NAND_DAVINCI=m,
> so that's how the error got triggered at last. Probably worth adding this
> explanation to the changelog, how do you think?

   OK, I take your silence as a sign of consent. Changelog indeed needs to be
rewritten a bit now.

WBR, Sergei

Patch

diff --git a/Documentation/devicetree/bindings/arm/davinci/nand.txt b/Documentation/devicetree/bindings/arm/davinci/nand.txt
new file mode 100644
index 0000000..e37241f
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/davinci/nand.txt
@@ -0,0 +1,51 @@ 
+* Texas Instruments Davinci NAND
+
+This file provides information, what the device node for the
+davinci nand interface contain.
+
+Required properties:
+- compatible: "ti,davinci-nand";
+- reg : contain 2 offset/length values:
+        - offset and length for the access window
+        - offset and length for accessing the aemif control registers
+- ti,davinci-chipselect: Indicates on the davinci_nand driver which
+                         chipselect is used for accessing the nand.
+
+Recommended properties :
+- ti,davinci-mask-ale: mask for ale
+- ti,davinci-mask-cle: mask for cle
+- ti,davinci-mask-chipsel: mask for chipselect
+- ti,davinci-ecc-mode: ECC mode valid values for davinci driver:
+		- "none"
+		- "soft"
+		- "hw"
+- ti,davinci-ecc-bits: used ECC bits, currently supported 1 or 4.
+- ti,davinci-nand-buswidth: buswidth 8 or 16
+- ti,davinci-nand-use-bbt: use flash based bad block table support.
+
+Example (enbw_cmc board):
+aemif@60000000 {
+	compatible = "ti,davinci-aemif";
+	#address-cells = <2>;
+	#size-cells = <1>;
+	reg = <0x68000000 0x80000>;
+	ranges = <2 0 0x60000000 0x02000000
+		  3 0 0x62000000 0x02000000
+		  4 0 0x64000000 0x02000000
+		  5 0 0x66000000 0x02000000
+		  6 0 0x68000000 0x02000000>;
+	nand@3,0 {
+		compatible = "ti,davinci-nand";
+		reg = <3 0x0 0x807ff
+			6 0x0 0x8000>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ti,davinci-chipselect = <1>;
+		ti,davinci-mask-ale = <0>;
+		ti,davinci-mask-cle = <0>;
+		ti,davinci-mask-chipsel = <0>;
+		ti,davinci-ecc-mode = "hw";
+		ti,davinci-ecc-bits = <4>;
+		ti,davinci-nand-use-bbt;
+	};
+};
diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
index d94b03c..f386b3c 100644
--- a/drivers/mtd/nand/davinci_nand.c
+++ b/drivers/mtd/nand/davinci_nand.c
@@ -33,6 +33,7 @@ 
 #include <linux/mtd/nand.h>
 #include <linux/mtd/partitions.h>
 #include <linux/slab.h>
+#include <linux/of_device.h>
 
 #include <mach/nand.h>
 #include <mach/aemif.h>
@@ -518,9 +519,75 @@  static struct nand_ecclayout hwecc4_2048 __initconst = {
 	},
 };
 
+#if defined(CONFIG_OF)
+static const struct of_device_id davinci_nand_of_match[] = {
+	{.compatible = "ti,davinci-nand", },
+	{},
+}
+MODULE_DEVICE_TABLE(of, davinci_nand_of_match);
+
+static struct davinci_nand_pdata
+	*nand_davinci_get_pdata(struct platform_device *pdev)
+{
+	if (!pdev->dev.platform_data && pdev->dev.of_node) {
+		struct davinci_nand_pdata *pdata;
+		const char *mode;
+		u32 prop;
+		int len;
+
+		pdata =  devm_kzalloc(&pdev->dev,
+				sizeof(struct davinci_nand_pdata),
+				GFP_KERNEL);
+		pdev->dev.platform_data = pdata;
+		if (!pdata)
+			return NULL;
+		if (!of_property_read_u32(pdev->dev.of_node,
+			"ti,davinci-chipselect", &prop))
+			pdev->id = prop;
+		if (!of_property_read_u32(pdev->dev.of_node,
+			"ti,davinci-mask-ale", &prop))
+			pdata->mask_ale = prop;
+		if (!of_property_read_u32(pdev->dev.of_node,
+			"ti,davinci-mask-cle", &prop))
+			pdata->mask_cle = prop;
+		if (!of_property_read_u32(pdev->dev.of_node,
+			"ti,davinci-mask-chipsel", &prop))
+			pdata->mask_chipsel = prop;
+		if (!of_property_read_string(pdev->dev.of_node,
+			"ti,davinci-ecc-mode", &mode)) {
+			if (!strncmp("none", mode, 4))
+				pdata->ecc_mode = NAND_ECC_NONE;
+			if (!strncmp("soft", mode, 4))
+				pdata->ecc_mode = NAND_ECC_SOFT;
+			if (!strncmp("hw", mode, 2))
+				pdata->ecc_mode = NAND_ECC_HW;
+		}
+		if (!of_property_read_u32(pdev->dev.of_node,
+			"ti,davinci-ecc-bits", &prop))
+			pdata->ecc_bits = prop;
+		if (!of_property_read_u32(pdev->dev.of_node,
+			"ti,davinci-nand-buswidth", &prop))
+			if (prop == 16)
+				pdata->options |= NAND_BUSWIDTH_16;
+		if (of_find_property(pdev->dev.of_node,
+			"ti,davinci-nand-use-bbt", &len))
+			pdata->bbt_options = NAND_BBT_USE_FLASH;
+	}
+
+	return pdev->dev.platform_data;
+}
+#else
+#define davinci_nand_of_match NULL
+static struct davinci_nand_pdata
+	*nand_davinci_get_pdata(struct platform_device *pdev)
+{
+	return pdev->dev.platform_data;
+}
+#endif
+
 static int __init nand_davinci_probe(struct platform_device *pdev)
 {
-	struct davinci_nand_pdata	*pdata = pdev->dev.platform_data;
+	struct davinci_nand_pdata	*pdata;
 	struct davinci_nand_info	*info;
 	struct resource			*res1;
 	struct resource			*res2;
@@ -530,6 +597,7 @@  static int __init nand_davinci_probe(struct platform_device *pdev)
 	uint32_t			val;
 	nand_ecc_modes_t		ecc_mode;
 
+	pdata = nand_davinci_get_pdata(pdev);
 	/* insist on board-specific configuration */
 	if (!pdata)
 		return -ENODEV;
@@ -816,6 +884,8 @@  static struct platform_driver nand_davinci_driver = {
 	.remove		= __exit_p(nand_davinci_remove),
 	.driver		= {
 		.name	= "davinci_nand",
+		.owner	= THIS_MODULE,
+		.of_match_table = davinci_nand_of_match,
 	},
 };
 MODULE_ALIAS("platform:davinci_nand");