diff mbox

[RFC,6/7] ARM: mtd: nand: davinci: add OF support for davinci nand controller

Message ID 1327308967-8092-7-git-send-email-hs@denx.de
State RFC
Headers show

Commit Message

Heiko Schocher Jan. 23, 2012, 8:56 a.m. UTC
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>
---
 .../devicetree/bindings/arm/davinci/nand.txt       |   72 ++++++++++++++++++
 drivers/mtd/nand/davinci_nand.c                    |   78 +++++++++++++++++++-
 2 files changed, 148 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/davinci/nand.txt

Comments

Scott Wood Jan. 23, 2012, 11:59 p.m. UTC | #1
On 01/23/2012 02:56 AM, Heiko Schocher wrote:
> diff --git a/Documentation/devicetree/bindings/arm/davinci/nand.txt b/Documentation/devicetree/bindings/arm/davinci/nand.txt
> new file mode 100644
> index 0000000..7e8d6db
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/davinci/nand.txt
> @@ -0,0 +1,72 @@
> +* 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
> +- id: id of the controller

What does "id of the controller" mean, specfically?  From this I can't
even tell if it's a number or a string, much less how to use it
semantically.  If it's just a "match what's in the manual" thing,
perhaps an alias would be better here.  Or, if it's a value with a
specific meaning (e.g. that you need to program into a register) use a
more specific name.

> +Recommended properties :
> +- mask_ale: mask for ale
> +- mask_cle: mask for cle
> +- mask_chipsel: mask for chipselect
> +- ecc_mode: ECC mode, see NAND_ECC_* defines
> +- ecc_bits: used ECC bits
> +- options: nand options, defined in
> +           include/linux/mtd/nand.h, grep for NAND_NO_AUTOINCR
> +- bbt_options: NAND_BBT_* defines

Binding-specific properties should have a vendor prefix.  Dashes are
preferred to underscores.

Don't specify Linux internals by reference -- they could change and
invalidate existing device trees and non-Linux code that accepts them
(e.g. U-Boot).  If you want them to line up, copy the definition here,
and if Linux changes, write glue code to convert.  It would probably be
better to define specific properties for anything that must be specified
here (neither deteted dynamically nor defined by compatible =
"ti,davinci-nand").

Do all of these properties really belong here?  I can see providing some
information about ECC or BBT mode, if there are multiple conventions
already in use (or that are reasonably justifiable for different
situations), as that must agree with how the flash has already been
programmed.  How much of the rest can vary from one "ti,davinci-nand" to
another?  Maybe it's obvious to someone more familiar with davinci, but
what does "mask for ale/cle/chipselect" mean?

> +	nandflash@3,0 {

PowerPC's ePAPR document -- not directly relevant to ARM, but contains a
more recently updated list of generic names than the IEEE1275
recommended pratice -- specifies "flash@".  If you don't want to do
that, "nand@" is used in many existing trees.  Why introduce a new name?

-Scott
Heiko Schocher Jan. 24, 2012, 7:23 a.m. UTC | #2
Hello Scott,

Scott Wood wrote:
> On 01/23/2012 02:56 AM, Heiko Schocher wrote:
>> diff --git a/Documentation/devicetree/bindings/arm/davinci/nand.txt b/Documentation/devicetree/bindings/arm/davinci/nand.txt
>> new file mode 100644
>> index 0000000..7e8d6db
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/davinci/nand.txt
>> @@ -0,0 +1,72 @@
>> +* 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
>> +- id: id of the controller
> 
> What does "id of the controller" mean, specfically?  From this I can't
> even tell if it's a number or a string, much less how to use it
> semantically.  If it's just a "match what's in the manual" thing,
> perhaps an alias would be better here.  Or, if it's a value with a
> specific meaning (e.g. that you need to program into a register) use a
> more specific name.

Ok, fix this. Id means here, which chipselect the controller uses.
Maybe it is better to rename it to "chipselect" ?

>> +Recommended properties :
>> +- mask_ale: mask for ale
>> +- mask_cle: mask for cle
>> +- mask_chipsel: mask for chipselect
>> +- ecc_mode: ECC mode, see NAND_ECC_* defines
>> +- ecc_bits: used ECC bits
>> +- options: nand options, defined in
>> +           include/linux/mtd/nand.h, grep for NAND_NO_AUTOINCR
>> +- bbt_options: NAND_BBT_* defines
> 
> Binding-specific properties should have a vendor prefix.  Dashes are
> preferred to underscores.

You think something like that:

davinci-mask-ale
davinci-mask-cle
...

?

> Don't specify Linux internals by reference -- they could change and
> invalidate existing device trees and non-Linux code that accepts them
> (e.g. U-Boot).  If you want them to line up, copy the definition here,
> and if Linux changes, write glue code to convert.  It would probably be
> better to define specific properties for anything that must be specified
> here (neither deteted dynamically nor defined by compatible =
> "ti,davinci-nand").

Ok, I add the defines here, and add also a comment in the dts.

> Do all of these properties really belong here?  I can see providing some

I think so, because this values come from existing platform code
(grep for struct davinci_nand_pdata)

> information about ECC or BBT mode, if there are multiple conventions
> already in use (or that are reasonably justifiable for different
> situations), as that must agree with how the flash has already been
> programmed.  How much of the rest can vary from one "ti,davinci-nand" to
> another?  Maybe it's obvious to someone more familiar with davinci, but
> what does "mask for ale/cle/chipselect" mean?

I mapped here just the existing platform code (=struct davinci_nand_pdata)
to OF, so existing boards can easy switch to OF.

Comment in arch/arm/mach-davinci/include/mach/nand.h says for
mask_ale and mask_cle:

/* NOTE:  boards don't need to use these address bits
 * for ALE/CLE unless they support booting from NAND.
 * They're used unless platform data overrides them.
 */

It is used for addressing the ALE/CLE Signals through the address,
used on the arch/arm/mach-davinci/board-dm646x-evm.c and
arch/arm/mach-davinci/board-tnetv107x-evm.c board ... so I think,
this should be also be setupable through OF ...

>> +	nandflash@3,0 {
> 
> PowerPC's ePAPR document -- not directly relevant to ARM, but contains a
> more recently updated list of generic names than the IEEE1275
> recommended pratice -- specifies "flash@".  If you don't want to do
> that, "nand@" is used in many existing trees.  Why introduce a new name?

There is no reason for that, change this to "nand@".

Thanks for the review.

bye,
Heiko
Scott Wood Jan. 24, 2012, 7:45 p.m. UTC | #3
On 01/24/2012 01:23 AM, Heiko Schocher wrote:
> Hello Scott,
> 
> Scott Wood wrote:
>> On 01/23/2012 02:56 AM, Heiko Schocher wrote:
>>> diff --git a/Documentation/devicetree/bindings/arm/davinci/nand.txt b/Documentation/devicetree/bindings/arm/davinci/nand.txt
>>> new file mode 100644
>>> index 0000000..7e8d6db
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/arm/davinci/nand.txt
>>> @@ -0,0 +1,72 @@
>>> +* 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
>>> +- id: id of the controller
>>
>> What does "id of the controller" mean, specfically?  From this I can't
>> even tell if it's a number or a string, much less how to use it
>> semantically.  If it's just a "match what's in the manual" thing,
>> perhaps an alias would be better here.  Or, if it's a value with a
>> specific meaning (e.g. that you need to program into a register) use a
>> more specific name.
> 
> Ok, fix this. Id means here, which chipselect the controller uses.
> Maybe it is better to rename it to "chipselect" ?

Yes, or better "ti,chipselect" or "ti,davinci-chipselect".

>>> +Recommended properties :
>>> +- mask_ale: mask for ale
>>> +- mask_cle: mask for cle
>>> +- mask_chipsel: mask for chipselect
>>> +- ecc_mode: ECC mode, see NAND_ECC_* defines
>>> +- ecc_bits: used ECC bits
>>> +- options: nand options, defined in
>>> +           include/linux/mtd/nand.h, grep for NAND_NO_AUTOINCR
>>> +- bbt_options: NAND_BBT_* defines
>>
>> Binding-specific properties should have a vendor prefix.  Dashes are
>> preferred to underscores.
> 
> You think something like that:
> 
> davinci-mask-ale
> davinci-mask-cle
> ...

"ti,davinci-mask-ale", etc.

>> Don't specify Linux internals by reference -- they could change and
>> invalidate existing device trees and non-Linux code that accepts them
>> (e.g. U-Boot).  If you want them to line up, copy the definition here,
>> and if Linux changes, write glue code to convert.  It would probably be
>> better to define specific properties for anything that must be specified
>> here (neither deteted dynamically nor defined by compatible =
>> "ti,davinci-nand").
> 
> Ok, I add the defines here, and add also a comment in the dts.

Which options actually need to come from the device tree?

>> Do all of these properties really belong here?  I can see providing some
> 
> I think so, because this values come from existing platform code
> (grep for struct davinci_nand_pdata)

The standards are a bit stricter for the device tree, since it's a more
stable interface across components -- at least that's how we've used it
on a lot of powerpc targets.  I'm not sure if that's the intent here,
but I have seen U-Boot patches for ARM hardware using them as well.

> Comment in arch/arm/mach-davinci/include/mach/nand.h says for
> mask_ale and mask_cle:
> 
> /* NOTE:  boards don't need to use these address bits
>  * for ALE/CLE unless they support booting from NAND.
>  * They're used unless platform data overrides them.
>  */
> 
> It is used for addressing the ALE/CLE Signals through the address,
> used on the arch/arm/mach-davinci/board-dm646x-evm.c and
> arch/arm/mach-davinci/board-tnetv107x-evm.c board ... so I think,
> this should be also be setupable through OF ...

OK, if it's board logic that does the decoding, and the compatible is
not board-specific, they belong here.

-Scott
Heiko Schocher Jan. 25, 2012, 7:09 a.m. UTC | #4
Hello Scott,

Scott Wood wrote:
> On 01/24/2012 01:23 AM, Heiko Schocher wrote:
>> Hello Scott,
>>
>> Scott Wood wrote:
>>> On 01/23/2012 02:56 AM, Heiko Schocher wrote:
>>>> diff --git a/Documentation/devicetree/bindings/arm/davinci/nand.txt b/Documentation/devicetree/bindings/arm/davinci/nand.txt
>>>> new file mode 100644
>>>> index 0000000..7e8d6db
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/arm/davinci/nand.txt
>>>> @@ -0,0 +1,72 @@
>>>> +* 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
>>>> +- id: id of the controller
>>> What does "id of the controller" mean, specfically?  From this I can't
>>> even tell if it's a number or a string, much less how to use it
>>> semantically.  If it's just a "match what's in the manual" thing,
>>> perhaps an alias would be better here.  Or, if it's a value with a
>>> specific meaning (e.g. that you need to program into a register) use a
>>> more specific name.
>> Ok, fix this. Id means here, which chipselect the controller uses.
>> Maybe it is better to rename it to "chipselect" ?
> 
> Yes, or better "ti,chipselect" or "ti,davinci-chipselect".

Ok. fixed this to "ti,davinci-chipselect"

>>>> +Recommended properties :
>>>> +- mask_ale: mask for ale
>>>> +- mask_cle: mask for cle
>>>> +- mask_chipsel: mask for chipselect
>>>> +- ecc_mode: ECC mode, see NAND_ECC_* defines
>>>> +- ecc_bits: used ECC bits
>>>> +- options: nand options, defined in
>>>> +           include/linux/mtd/nand.h, grep for NAND_NO_AUTOINCR
>>>> +- bbt_options: NAND_BBT_* defines
>>> Binding-specific properties should have a vendor prefix.  Dashes are
>>> preferred to underscores.
>> You think something like that:
>>
>> davinci-mask-ale
>> davinci-mask-cle
>> ...
> 
> "ti,davinci-mask-ale", etc.

Ok, fixed.

>>> Don't specify Linux internals by reference -- they could change and
>>> invalidate existing device trees and non-Linux code that accepts them
>>> (e.g. U-Boot).  If you want them to line up, copy the definition here,
>>> and if Linux changes, write glue code to convert.  It would probably be
>>> better to define specific properties for anything that must be specified
>>> here (neither deteted dynamically nor defined by compatible =
>>> "ti,davinci-nand").
>> Ok, I add the defines here, and add also a comment in the dts.
> 
> Which options actually need to come from the device tree?

I found the following used options:

ecc_mode:
NAND_ECC_NONE
NAND_ECC_SOFT
NAND_ECC_HW
NAND_ECC_HW_SYNDROME

bbt_options:
NAND_BBT_USE_FLASH

ecc_bits:
1
4

options:
NAND_BUSWIDTH_16

>>> Do all of these properties really belong here?  I can see providing some
>> I think so, because this values come from existing platform code
>> (grep for struct davinci_nand_pdata)
> 
> The standards are a bit stricter for the device tree, since it's a more
> stable interface across components -- at least that's how we've used it
> on a lot of powerpc targets.  I'm not sure if that's the intent here,
> but I have seen U-Boot patches for ARM hardware using them as well.

Ok, so, should I introduce instead properties for the above
needed parameters? (As this are not davinci specific parameters,
are there somewhere such definitions for them?)

>> Comment in arch/arm/mach-davinci/include/mach/nand.h says for
>> mask_ale and mask_cle:
>>
>> /* NOTE:  boards don't need to use these address bits
>>  * for ALE/CLE unless they support booting from NAND.
>>  * They're used unless platform data overrides them.
>>  */
>>
>> It is used for addressing the ALE/CLE Signals through the address,
>> used on the arch/arm/mach-davinci/board-dm646x-evm.c and
>> arch/arm/mach-davinci/board-tnetv107x-evm.c board ... so I think,
>> this should be also be setupable through OF ...
> 
> OK, if it's board logic that does the decoding, and the compatible is
> not board-specific, they belong here.

Ok.

bye,
Heiko
Scott Wood Jan. 26, 2012, 8:33 p.m. UTC | #5
On 01/25/2012 01:09 AM, Heiko Schocher wrote:
> Scott Wood wrote:
> I found the following used options:
> 
> ecc_mode:
> NAND_ECC_NONE
> NAND_ECC_SOFT
> NAND_ECC_HW
> NAND_ECC_HW_SYNDROME
> 
> bbt_options:
> NAND_BBT_USE_FLASH
> 
> ecc_bits:
> 1
> 4
> 
> options:
> NAND_BUSWIDTH_16
> 
>>>> Do all of these properties really belong here?  I can see providing some
>>> I think so, because this values come from existing platform code
>>> (grep for struct davinci_nand_pdata)
>>
>> The standards are a bit stricter for the device tree, since it's a more
>> stable interface across components -- at least that's how we've used it
>> on a lot of powerpc targets.  I'm not sure if that's the intent here,
>> but I have seen U-Boot patches for ARM hardware using them as well.
> 
> Ok, so, should I introduce instead properties for the above
> needed parameters? 

Yes.

> (As this are not davinci specific parameters, are there somewhere such definitions for them?)

It's controller-specific which options are changeable, and whether
there's a better source of information.  Most controllers don't seem to
need this.  I'd keep the definitions davinci specific for now.  If
there's enough of a common need, a common definition could be considered.

-Scott
Heiko Schocher Jan. 27, 2012, 6:40 a.m. UTC | #6
Hello Scott,

Scott Wood wrote:
> On 01/25/2012 01:09 AM, Heiko Schocher wrote:
>> Scott Wood wrote:
>> I found the following used options:
>>
>> ecc_mode:
>> NAND_ECC_NONE
>> NAND_ECC_SOFT
>> NAND_ECC_HW
>> NAND_ECC_HW_SYNDROME
>>
>> bbt_options:
>> NAND_BBT_USE_FLASH
>>
>> ecc_bits:
>> 1
>> 4
>>
>> options:
>> NAND_BUSWIDTH_16
>>
>>>>> Do all of these properties really belong here?  I can see providing some
>>>> I think so, because this values come from existing platform code
>>>> (grep for struct davinci_nand_pdata)
>>> The standards are a bit stricter for the device tree, since it's a more
>>> stable interface across components -- at least that's how we've used it
>>> on a lot of powerpc targets.  I'm not sure if that's the intent here,
>>> but I have seen U-Boot patches for ARM hardware using them as well.
>> Ok, so, should I introduce instead properties for the above
>> needed parameters? 
> 
> Yes.

Ok, I change this.

>> (As this are not davinci specific parameters, are there somewhere such definitions for them?)
> 
> It's controller-specific which options are changeable, and whether
> there's a better source of information.  Most controllers don't seem to
> need this.  I'd keep the definitions davinci specific for now.  If
> there's enough of a common need, a common definition could be considered.

Ok, a proposal for the properties names and values:

>> ecc_mode:
>> NAND_ECC_NONE
>> NAND_ECC_SOFT
>> NAND_ECC_HW
>> NAND_ECC_HW_SYNDROME

"ti,davinci-nand-ecc-mode" = "none", "soft", "hw" or "hw_syndrome"

>> bbt_options:
>> NAND_BBT_USE_FLASH

"ti,davinci-nand-bbt-options" = "use_flash"

>> ecc_bits:
>> 1
>> 4

"ti,davinci-nand-ecc-bits" = "1" or "4"

>> options:
>> NAND_BUSWIDTH_16

"ti,davinci-nand-options" = "buswidth-16"

bye,
Heiko
Scott Wood Jan. 27, 2012, 5:02 p.m. UTC | #7
On 01/27/2012 12:40 AM, Heiko Schocher wrote:
> Hello Scott,
> 
> Scott Wood wrote:
>> On 01/25/2012 01:09 AM, Heiko Schocher wrote:
>>> ecc_mode:
>>> NAND_ECC_NONE
>>> NAND_ECC_SOFT
>>> NAND_ECC_HW
>>> NAND_ECC_HW_SYNDROME
> 
> "ti,davinci-nand-ecc-mode" = "none", "soft", "hw" or "hw_syndrome"

OK.

>>> bbt_options:
>>> NAND_BBT_USE_FLASH
> 
> "ti,davinci-nand-bbt-options" = "use_flash"

Just make it a boolean ti,davinci-nand-use-bbt

>>> ecc_bits:
>>> 1
>>> 4
> 
> "ti,davinci-nand-ecc-bits" = "1" or "4"

<1> or <4>

>>> options:
>>> NAND_BUSWIDTH_16
> 
> "ti,davinci-nand-options" = "buswidth-16"

ti,davinci-nand-buswidth = <16>;

-Scott
diff mbox

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..7e8d6db
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/davinci/nand.txt
@@ -0,0 +1,72 @@ 
+* 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
+- id: id of the controller
+
+Recommended properties :
+- mask_ale: mask for ale
+- mask_cle: mask for cle
+- mask_chipsel: mask for chipselect
+- ecc_mode: ECC mode, see NAND_ECC_* defines
+- ecc_bits: used ECC bits
+- options: nand options, defined in
+           include/linux/mtd/nand.h, grep for NAND_NO_AUTOINCR
+- bbt_options: NAND_BBT_* defines
+
+Optional properties:
+- pinmux-handle: contains a handle to configure the pinmux settings.
+- timing-handle: contains a handle to setup aemif timing.
+
+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_cs: cs3@68000000 {
+		compatible = "ti,davinci-cs";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		/* all timings in nanoseconds */
+		cs = <3>;
+		asize = <0>;
+		ta = <0>;
+		rhold = <7>;
+		rstrobe = <42>;
+		rsetup = <7>;
+		whold = <7>;
+		wstrobe = <14>;
+		wsetup = <7>;
+		ew = <0>;
+		ss = <0>;
+	};
+	nandflash@3,0 {
+		compatible = "ti,davinci-nand";
+		reg = <3 0x0 0x807ff
+			6 0x0 0x8000>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		id = <1>;
+		mask_ale = <0>;
+		mask_cle = <0>;
+		mask_chipsel = <0>;
+		ecc_mode = <2>;
+		ecc_bits = <4>;
+		options = <0>;
+		bbt_options = <0x20000>;
+		pinmux-handle = <&nand_pins>;
+		timing-handle = <&nand_cs>;
+	};
+};
diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
index 6e56615..a1946c3 100644
--- a/drivers/mtd/nand/davinci_nand.c
+++ b/drivers/mtd/nand/davinci_nand.c
@@ -33,9 +33,12 @@ 
 #include <linux/mtd/nand.h>
 #include <linux/mtd/partitions.h>
 #include <linux/slab.h>
+#include <linux/of_i2c.h>
+#include <linux/of_device.h>
 
 #include <mach/nand.h>
 #include <mach/aemif.h>
+#include <mach/mux.h>
 
 /*
  * This is a device driver for the NAND flash controller found on the
@@ -518,9 +521,76 @@  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 == NULL) &&
+		(pdev->dev.of_node)) {
+		struct device_node *tmp_np;
+		struct davinci_nand_pdata *pdata;
+		u32 prop;
+
+		pdata =  kzalloc(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, "id",
+			&prop))
+			pdev->id = prop;
+		if (!of_property_read_u32(pdev->dev.of_node, "mask_ale",
+			&prop))
+			pdata->mask_ale = prop;
+		if (!of_property_read_u32(pdev->dev.of_node, "mask_cle",
+			&prop))
+			pdata->mask_cle = prop;
+		if (!of_property_read_u32(pdev->dev.of_node, "mask_chipsel",
+			&prop))
+			pdata->mask_chipsel = prop;
+		if (!of_property_read_u32(pdev->dev.of_node, "ecc_mode",
+			&prop))
+			pdata->ecc_mode = prop;
+		if (!of_property_read_u32(pdev->dev.of_node, "ecc_bits",
+			&prop))
+			pdata->ecc_bits = prop;
+		if (!of_property_read_u32(pdev->dev.of_node, "options",
+			&prop))
+			pdata->options = prop;
+		if (!of_property_read_u32(pdev->dev.of_node, "bbt_options",
+			&prop))
+			pdata->bbt_options = prop;
+		tmp_np = of_parse_phandle(pdev->dev.of_node,
+				"pinmux-handle", 0);
+		if (tmp_np)
+			davinci_cfg_reg_of(tmp_np);
+
+		tmp_np = of_parse_phandle(pdev->dev.of_node,
+				"timing-handle", 0);
+		if (tmp_np)
+			davinci_aemif_setup_timing_of(tmp_np);
+	}
+
+	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 +600,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;
@@ -812,16 +883,19 @@  static int __exit nand_davinci_remove(struct platform_device *pdev)
 }
 
 static struct platform_driver nand_davinci_driver = {
+	.probe		= nand_davinci_probe,
 	.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");
 
 static int __init nand_davinci_init(void)
 {
-	return platform_driver_probe(&nand_davinci_driver, nand_davinci_probe);
+	return platform_driver_register(&nand_davinci_driver);
 }
 module_init(nand_davinci_init);