Message ID | 1343632944-25971-1-git-send-email-hs@denx.de (mailing list archive) |
---|---|
State | Awaiting Upstream |
Headers | show |
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
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
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!
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
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
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
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.
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
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
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");
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