[PATCH 0/2] ARM: dts: OMAP3: Add GPMC controller and NAND memory to Overo

[PATCH 0/2] ARM: dts: OMAP3: Add GPMC controller and NAND memory to Overo

[Florian Vaussard]

Hello,

This is more an RFC serie, as an issue is still unclear to me.
Building on the work of Daniel Mack for the GPMC controller (staged
in Tony's tree [1]), it was easy to add the GPMC controller to OMAP3.

The issue comes from the Overo on-board NAND, as the amount of flash
depends on the revision. Currently, partitions are handled in the board
file using MTDPART_SIZ_FULL, but looking at the ofpart parser, the size
given to the parser must be fixed.

So how should we handle such case? Having several dtsi depending
on the Overo's revision would be a mess to my sense, considering
the non-conditional include inside the expansion boards' dts.
Or would it make sense to extend the DT binding for partitions?

This serie was tested on an Overo with 512MB of NAND.

Best regards,

Florian

[1] git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git omap-for-v3.9/gpmc

Florian Vaussard (2):
  ARM: dts: OMAP3: Add GPMC controller
  ARM: dts: OMAP3: Add NAND memory for Overo products

 arch/arm/boot/dts/omap3-overo.dtsi |   49 ++++++++++++++++++++++++++++++++++++
 arch/arm/boot/dts/omap3.dtsi       |   11 ++++++++
 2 files changed, 60 insertions(+), 0 deletions(-)

-- 
1.7.5.4

[PATCH 1/2] ARM: dts: OMAP3: Add GPMC controller

[Florian Vaussard]

Add device-tree support for the GPMC controller on the OMAP3.

Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
---
 arch/arm/boot/dts/omap3.dtsi |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
index 6c63118..2ddae38 100644
--- a/arch/arm/boot/dts/omap3.dtsi
+++ b/arch/arm/boot/dts/omap3.dtsi
@@ -403,5 +403,16 @@
 			ti,timer-alwon;
 			ti,timer-secure;
 		};
+
+		gpmc: gpmc at 6e000000 {
+			compatible = "ti,omap3430-gpmc";
+			ti,hwmods = "gpmc";
+			reg = <0x6e000000 0x1000000>;
+			interrupts = <20>;
+			gpmc,num-cs = <8>;
+			gpmc,num-waitpins = <4>;
+			#address-cells = <2>;
+			#size-cells = <1>;
+		};
 	};
 };
-- 
1.7.5.4

[PATCH 2/2] ARM: dts: OMAP3: Add NAND memory for Overo products

[Florian Vaussard]

Add device-tree support for the on-board NAND memory,
with corresponding partitions.

Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
---
 arch/arm/boot/dts/omap3-overo.dtsi |   49 ++++++++++++++++++++++++++++++++++++
 1 files changed, 49 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/omap3-overo.dtsi b/arch/arm/boot/dts/omap3-overo.dtsi
index 81341fa..0efd6f3 100644
--- a/arch/arm/boot/dts/omap3-overo.dtsi
+++ b/arch/arm/boot/dts/omap3-overo.dtsi
@@ -33,6 +33,55 @@
 	};
 };
 
+&gpmc {
+	ranges = <0 0 0x30000000 0x00000004>;       /* CS0: NAND */
+
+	nand at 0 {
+		reg = <0 0 0>; /* CS0, offset 0 */
+
+		nand-bus-width = <16>;
+		ti,nand-ecc-opt = "sw";
+
+		gpmc,sync-clk = <0>;
+		gpmc,cs-on = <0>;
+		gpmc,cs-rd-off = <36>;
+		gpmc,cs-wr-off = <36>;
+		gpmc,adv-on = <6>;
+		gpmc,adv-rd-off = <24>;
+		gpmc,adv-wr-off = <36>;
+		gpmc,we-off = <30>;
+		gpmc,oe-off = <48>;
+		gpmc,access = <54>;
+		gpmc,rd-cycle = <72>;
+		gpmc,wr-cycle = <72>;
+		gpmc,wr-access = <30>;
+		gpmc,wr-data-mux-bus = <0>;
+
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		xloader at 0 {
+			reg = <0x00000000 0x00080000>;
+		};
+
+		uboot at 80000 {
+			reg = <0x00080000 0x001c0000>;
+		};
+
+		ubootenv at 240000 {
+			reg = <0x00240000 0x00040000>;
+		};
+
+		linux at 280000 {
+			reg = <0x00280000 0x00400000>;
+		};
+
+		rootfs at 680000 {
+			reg = <0x00680000 0x1f980000>; /* 500 MB */
+		};
+	};
+};
+
 &i2c1 {
 	clock-frequency = <2600000>;
 
-- 
1.7.5.4

[PATCH 0/2] ARM: dts: OMAP3: Add GPMC controller and NAND memory to Overo

[Tony Lindgren]

* Florian Vaussard <florian.vaussard@epfl.ch> [130128 09:57]:
> Hello,
> 
> This is more an RFC serie, as an issue is still unclear to me.
> Building on the work of Daniel Mack for the GPMC controller (staged
> in Tony's tree [1]), it was easy to add the GPMC controller to OMAP3.
> 
> The issue comes from the Overo on-board NAND, as the amount of flash
> depends on the revision. Currently, partitions are handled in the board
> file using MTDPART_SIZ_FULL, but looking at the ofpart parser, the size
> given to the parser must be fixed.
> 
> So how should we handle such case? Having several dtsi depending
> on the Overo's revision would be a mess to my sense, considering
> the non-conditional include inside the expansion boards' dts.
> Or would it make sense to extend the DT binding for partitions?

Yes makes sense to extend the binding to use the full flash memory
if the size can be probed.

Regards,

Tony

[PATCH 0/2] ARM: dts: OMAP3: Add GPMC controller and NAND memory to Overo

[Florian Vaussard]

>>
>> So how should we handle such case? Having several dtsi depending
>> on the Overo's revision would be a mess to my sense, considering
>> the non-conditional include inside the expansion boards' dts.
>> Or would it make sense to extend the DT binding for partitions?
>
> Yes makes sense to extend the binding to use the full flash memory
> if the size can be probed.
>

Ok, I will see for a patch soon (i.e. in the coming weeks).

Regards,

Florian

[PATCH 1/2] ARM: dts: OMAP3: Add GPMC controller

[Javier Martinez Canillas]

Hi Florian,

On Mon, Jan 28, 2013 at 6:54 PM, Florian Vaussard
<florian.vaussard@epfl.ch> wrote:
> Add device-tree support for the GPMC controller on the OMAP3.
>
> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
> ---
>  arch/arm/boot/dts/omap3.dtsi |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
> index 6c63118..2ddae38 100644
> --- a/arch/arm/boot/dts/omap3.dtsi
> +++ b/arch/arm/boot/dts/omap3.dtsi
> @@ -403,5 +403,16 @@
>                         ti,timer-alwon;
>                         ti,timer-secure;
>                 };
> +
> +               gpmc: gpmc at 6e000000 {
> +                       compatible = "ti,omap3430-gpmc";
> +                       ti,hwmods = "gpmc";
> +                       reg = <0x6e000000 0x1000000>;
> +                       interrupts = <20>;
> +                       gpmc,num-cs = <8>;
> +                       gpmc,num-waitpins = <4>;
> +                       #address-cells = <2>;
> +                       #size-cells = <1>;
> +               };
>         };
>  };

I had the same patch on a tree I was working on to add DT support for
gpmc-smsc911x on an OMAP3 board but I was waiting for Daniel's patches
to hit mainline before sending the RFC. So please feel free to add:

Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>

Best regards,
Javier

[PATCH 1/2] ARM: dts: OMAP3: Add GPMC controller

[Florian Vaussard]

Hi Javier,

On 02/04/2013 10:27 AM, Javier Martinez Canillas wrote:
> Hi Florian,
>
> On Mon, Jan 28, 2013 at 6:54 PM, Florian Vaussard
> <florian.vaussard@epfl.ch> wrote:
>> Add device-tree support for the GPMC controller on the OMAP3.
>>
>> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
>> ---
>>   arch/arm/boot/dts/omap3.dtsi |   11 +++++++++++
>>   1 files changed, 11 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
>> index 6c63118..2ddae38 100644
>> --- a/arch/arm/boot/dts/omap3.dtsi
>> +++ b/arch/arm/boot/dts/omap3.dtsi
>> @@ -403,5 +403,16 @@
>>                          ti,timer-alwon;
>>                          ti,timer-secure;
>>                  };
>> +
>> +               gpmc: gpmc at 6e000000 {
>> +                       compatible = "ti,omap3430-gpmc";
>> +                       ti,hwmods = "gpmc";
>> +                       reg = <0x6e000000 0x1000000>;
>> +                       interrupts = <20>;
>> +                       gpmc,num-cs = <8>;
>> +                       gpmc,num-waitpins = <4>;
>> +                       #address-cells = <2>;
>> +                       #size-cells = <1>;
>> +               };
>>          };
>>   };
>
> I had the same patch on a tree I was working on to add DT support for
> gpmc-smsc911x on an OMAP3 board but I was waiting for Daniel's patches
> to hit mainline before sending the RFC. So please feel free to add:
>
> Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>

Great, the smsc911x was on my TODO list, I can cross it out :) BTW,
do you have a public git for this, so I can test your work on my setup?

For the GPMC support, I will have to make a few more more as discussed with
Tony, and as I will be away for more than 2 weeks, feel free to go ahead
before me. This patchset was only at an RFC stage.

Regards,

Florian

[PATCH 1/2] ARM: dts: OMAP3: Add GPMC controller

[Javier Martinez Canillas]

On Mon, Feb 4, 2013 at 11:36 AM, Florian Vaussard
<florian.vaussard@epfl.ch> wrote:
> Hi Javier,
>
>

Hi Florian,

> On 02/04/2013 10:27 AM, Javier Martinez Canillas wrote:
>>
>> Hi Florian,
>>
>> On Mon, Jan 28, 2013 at 6:54 PM, Florian Vaussard
>> <florian.vaussard@epfl.ch> wrote:
>>>
>>> Add device-tree support for the GPMC controller on the OMAP3.
>>>
>>> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
>>> ---
>>>   arch/arm/boot/dts/omap3.dtsi |   11 +++++++++++
>>>   1 files changed, 11 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
>>> index 6c63118..2ddae38 100644
>>> --- a/arch/arm/boot/dts/omap3.dtsi
>>> +++ b/arch/arm/boot/dts/omap3.dtsi
>>> @@ -403,5 +403,16 @@
>>>                          ti,timer-alwon;
>>>                          ti,timer-secure;
>>>                  };
>>> +
>>> +               gpmc: gpmc at 6e000000 {
>>> +                       compatible = "ti,omap3430-gpmc";
>>> +                       ti,hwmods = "gpmc";
>>> +                       reg = <0x6e000000 0x1000000>;
>>> +                       interrupts = <20>;
>>> +                       gpmc,num-cs = <8>;
>>> +                       gpmc,num-waitpins = <4>;
>>> +                       #address-cells = <2>;
>>> +                       #size-cells = <1>;
>>> +               };
>>>          };
>>>   };
>>
>>
>> I had the same patch on a tree I was working on to add DT support for
>> gpmc-smsc911x on an OMAP3 board but I was waiting for Daniel's patches
>> to hit mainline before sending the RFC. So please feel free to add:
>>
>> Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>>
>
> Great, the smsc911x was on my TODO list, I can cross it out :) BTW,
> do you have a public git for this, so I can test your work on my setup?
>

Yes, it is the gpmc-smsc911x-wip branch on my github linux tree [1]

That branch is Linus master tree + linux-omap/omap-for-v3.9/gpmc +
linux-omap-dt/for_3.9/dts

plus these patches:

Javier Martinez Canillas (4):
      ARM: OMAP: gpmc-smsc911x: add DT dev node init function
      ARM: OMAP: gpmc: add support for gpmc-smsc911x child nodes
      ARM: dts: OMAP: Add an GPMC node for OMAP3
      ARM: dts: omap3-igep0020: Add SMSC911x LAN chip support

You can browse these patches here [2].

With these patches I have ethernet working on my IGEPv2 but this board
uses an OMAP GPIO pin as the smsc911x IRQ line, so it needs to set a
mux pin (mcspi1_cs2.gpio_176 INPUT | MODE4) or it will fallback on
poll mode.

For some reasons I still couldn't figure out (I'm still learning about
Device Trees) adding:

       pinctrl = devm_pinctrl_get_select_default(&pdev->dev);

to the gpmc-smsc911x child node parse function (gpmc_smsc911x_init_dt)
didn't have a functional effect and I had to initialize the defined
pinctrl-single pins for the smsc911x in the omap3_pmx_core device node
instead of pmc_smsc911x at 0 as I do for other devices (mmc, uarts, etc).

So I just removed the devm_pinctrl_get_select_default() call in
gpmc_smsc911x_init_dt() in for this RFC.

I don't know if this is because arch/arm/mach-omap2/gpmc-smsc911x.c is
not a real platform driver (is just a wrapper/helper function) and
doesn't have a probe function.

Which brings me to the question if my approach of adding a binding for
gpmc-smsc911x is correct or if we should extend/use the binding that
already exist for smsc911x
(Documentation/devicetree/bindings/net/smsc911x.txt).

> For the GPMC support, I will have to make a few more more as discussed with
> Tony, and as I will be away for more than 2 weeks, feel free to go ahead
> before me. This patchset was only at an RFC stage.
>
> Regards,
>
> Florian
>

Yes, I saw on the list that Tony asked you too extend the GPMC DT
support. Flash support is on my TODO list too but I don't know if I'm
going to have time to work on this in the next few weeks.

Since you are thinking to change the binding, there is something I
want to discuss with you.

We have two different version for each IGEP board, one that uses NAND
memory and another that has OneNAND.

With board files this is easy because the flash memory type is
hardcoded (in hardware) using sysboot pins [3]. So we check the
sysboot value and call board_nand_init() or board_onenand_init()
accordingly and pass the same static struct mtd_partition
igep_flash_partitions[] in both cases.

But with DT this is a little bit trickier since you have to define
either a nand at 0 or onenand at 0 child node for the gpmc device node. So,
we will have to create lots of dts and dtsi to handle each combination
(IGEPv2 + NAND, IGEPv2 + OneNAND, IGEP COM + NAND, etc).

I wonder if we could just have a generic gpmc-flash binding and maybe
use a "gpmc, flash_type" property or something like that to decide if
gpmc_onenand_init() or gpmc_nand_init() has to be called.

Thanks a lot and best regards,
Javier

[1]: git://github.com/martinezjavier/linux.git
[2]: https://github.com/martinezjavier/linux/commits/gpmc-smsc911x-wip
[3]: http://omappedia.org/wiki/Bootloader_Project#SYSBOOT_Pins

[PATCH 1/2] ARM: dts: OMAP3: Add GPMC controller

[Tony Lindgren]

* Javier Martinez Canillas <javier@dowhile0.org> [130204 04:00]:
> On Mon, Feb 4, 2013 at 11:36 AM, Florian Vaussard
> > Great, the smsc911x was on my TODO list, I can cross it out :) BTW,
> > do you have a public git for this, so I can test your work on my setup?
> >
> 
> Yes, it is the gpmc-smsc911x-wip branch on my github linux tree [1]
> 
> That branch is Linus master tree + linux-omap/omap-for-v3.9/gpmc +
> linux-omap-dt/for_3.9/dts
> 
> plus these patches:
> 
> Javier Martinez Canillas (4):
>       ARM: OMAP: gpmc-smsc911x: add DT dev node init function
>       ARM: OMAP: gpmc: add support for gpmc-smsc911x child nodes
>       ARM: dts: OMAP: Add an GPMC node for OMAP3
>       ARM: dts: omap3-igep0020: Add SMSC911x LAN chip support
> 
> You can browse these patches here [2].
> 
> With these patches I have ethernet working on my IGEPv2 but this board
> uses an OMAP GPIO pin as the smsc911x IRQ line, so it needs to set a
> mux pin (mcspi1_cs2.gpio_176 INPUT | MODE4) or it will fallback on
> poll mode.

Great, that's good news :)

> For some reasons I still couldn't figure out (I'm still learning about
> Device Trees) adding:
> 
>        pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
> 
> to the gpmc-smsc911x child node parse function (gpmc_smsc911x_init_dt)
> didn't have a functional effect and I had to initialize the defined
> pinctrl-single pins for the smsc911x in the omap3_pmx_core device node
> instead of pmc_smsc911x at 0 as I do for other devices (mmc, uarts, etc).

Maybe this is related to the initcall ordering..
 
> So I just removed the devm_pinctrl_get_select_default() call in
> gpmc_smsc911x_init_dt() in for this RFC.
> 
> I don't know if this is because arch/arm/mach-omap2/gpmc-smsc911x.c is
> not a real platform driver (is just a wrapper/helper function) and
> doesn't have a probe function.

..that function just initializes the pdata for smsc911x driver, and
should not be a driver. You need to add devm_pinctrl_get_select_default()
to the probe of smsc911x or the GPMC probe.
 
> Which brings me to the question if my approach of adding a binding for
> gpmc-smsc911x is correct or if we should extend/use the binding that
> already exist for smsc911x
> (Documentation/devicetree/bindings/net/smsc911x.txt).

We should use the existing smsc911x binding, then have a separate GPMC
binding for the GPMC driver.

Regards,

Tony 
 
> [1]: git://github.com/martinezjavier/linux.git
> [2]: https://github.com/martinezjavier/linux/commits/gpmc-smsc911x-wip
> [3]: http://omappedia.org/wiki/Bootloader_Project#SYSBOOT_Pins

[PATCH 1/2] ARM: dts: OMAP3: Add GPMC controller

[Javier Martinez Canillas]

On Mon, Feb 4, 2013 at 6:32 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Javier Martinez Canillas <javier@dowhile0.org> [130204 04:00]:
>> On Mon, Feb 4, 2013 at 11:36 AM, Florian Vaussard
>> > Great, the smsc911x was on my TODO list, I can cross it out :) BTW,
>> > do you have a public git for this, so I can test your work on my setup?
>> >
>>
>> Yes, it is the gpmc-smsc911x-wip branch on my github linux tree [1]
>>
>> That branch is Linus master tree + linux-omap/omap-for-v3.9/gpmc +
>> linux-omap-dt/for_3.9/dts
>>
>> plus these patches:
>>
>> Javier Martinez Canillas (4):
>>       ARM: OMAP: gpmc-smsc911x: add DT dev node init function
>>       ARM: OMAP: gpmc: add support for gpmc-smsc911x child nodes
>>       ARM: dts: OMAP: Add an GPMC node for OMAP3
>>       ARM: dts: omap3-igep0020: Add SMSC911x LAN chip support
>>
>> You can browse these patches here [2].
>>
>> With these patches I have ethernet working on my IGEPv2 but this board
>> uses an OMAP GPIO pin as the smsc911x IRQ line, so it needs to set a
>> mux pin (mcspi1_cs2.gpio_176 INPUT | MODE4) or it will fallback on
>> poll mode.
>
> Great, that's good news :)
>

it's a start at least :-)

>> For some reasons I still couldn't figure out (I'm still learning about
>> Device Trees) adding:
>>
>>        pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
>>
>> to the gpmc-smsc911x child node parse function (gpmc_smsc911x_init_dt)
>> didn't have a functional effect and I had to initialize the defined
>> pinctrl-single pins for the smsc911x in the omap3_pmx_core device node
>> instead of pmc_smsc911x at 0 as I do for other devices (mmc, uarts, etc).
>
> Maybe this is related to the initcall ordering..
>

I don't think so since I added the gpmc at 6e000000 device node as the
last child node for ocp in omap3.dtsi. So, is way after omap3_pmx_core
child node is defined.

Btw, there is a way to specify the initialization order or the
dependencies between device nodes (e.g: gpmc depends on
omap3_pmx_core) or is just because of the position inside the DT?

>> So I just removed the devm_pinctrl_get_select_default() call in
>> gpmc_smsc911x_init_dt() in for this RFC.
>>
>> I don't know if this is because arch/arm/mach-omap2/gpmc-smsc911x.c is
>> not a real platform driver (is just a wrapper/helper function) and
>> doesn't have a probe function.
>
> ..that function just initializes the pdata for smsc911x driver, and
> should not be a driver. You need to add devm_pinctrl_get_select_default()
> to the probe of smsc911x or the GPMC probe.
>

Yes, that's what I meant. In that case I would have something like
this on my board dts:

&gpmc {
	pinctrl-names = "default";
	pinctrl-0 = <&smsc911x_pins>;
	gpmc_smsc911x at 0 {
		gpmc,cs = <5>; /* IGEP2_SMSC911X_CS */
		gpmc,gpio_irq = <176>; /* IGEP2_SMSC911X_GPIO */
		gpmc,flags = <18>; /* SMSC911X_USE_32BIT | SMSC911X_SAVE_MAC_ADDRESS */
		vmmc-supply = <&vddvario>;
		vmmc_aux-supply = <&vdd33a>;
      };
};

Are you ok with tha approach?

Certainly is better than initializing in omap3_pmx_core node but still
is not consistent with other devices where the pinctrl pins are
defined inside the node of the device that requires them.

>> Which brings me to the question if my approach of adding a binding for
>> gpmc-smsc911x is correct or if we should extend/use the binding that
>> already exist for smsc911x
>> (Documentation/devicetree/bindings/net/smsc911x.txt).
>
> We should use the existing smsc911x binding, then have a separate GPMC
> binding for the GPMC driver.
>

You mean a binding for GPMC or a binding for gpmc-smsc911x?

I don't think that we can use the smsc911x binding directly since
gpmc_smsc911x_init() not only initializes the platform data but also
does some setup such as requesting a memory range for a specific chip
select (gpmc_cs_request), claiming a GPIO and configuring as input
(gpio_request_one), etc.

You can't just calculate the I/O space address for the GPMC cs your
peripheral is connected to and set it on the reg property of the
current smsc911x binding, right?. The same can be said for the nic
IRQ.

Or did I get wrong?

> Regards,
>
> Tony
>

Thanks a lot and best regards,
Javier

[PATCH 1/2] ARM: dts: OMAP3: Add GPMC controller

[Florian Vaussard]

Hi Javier,

On 02/04/2013 12:57 PM, Javier Martinez Canillas wrote:

[...]

>
> Yes, I saw on the list that Tony asked you too extend the GPMC DT
> support. Flash support is on my TODO list too but I don't know if I'm
> going to have time to work on this in the next few weeks.
>
> Since you are thinking to change the binding, there is something I
> want to discuss with you.
>
> We have two different version for each IGEP board, one that uses NAND
> memory and another that has OneNAND.
>
> With board files this is easy because the flash memory type is
> hardcoded (in hardware) using sysboot pins [3]. So we check the
> sysboot value and call board_nand_init() or board_onenand_init()
> accordingly and pass the same static struct mtd_partition
> igep_flash_partitions[] in both cases.
>
> But with DT this is a little bit trickier since you have to define
> either a nand at 0 or onenand at 0 child node for the gpmc device node. So,
> we will have to create lots of dts and dtsi to handle each combination
> (IGEPv2 + NAND, IGEPv2 + OneNAND, IGEP COM + NAND, etc).
>
> I wonder if we could just have a generic gpmc-flash binding and maybe
> use a "gpmc, flash_type" property or something like that to decide if
> gpmc_onenand_init() or gpmc_nand_init() has to be called.
>

This boils down to the problem where you have an "if" statement in
your board file, and I think no general solution exists.

In your suggestion, having a property will force you anyway to write
distinct DT files to account for the two versions, so it won't really
help if I understand correctly. We would need conditional
section inside device trees, at least to test for simple parameters.
Or a way to generate several DT blobs based on a single DT source file.

Another hackish solution would be to write a meta component, whose
probe section would do your test. But this is just a stripped down version
of the board file, far from being generic...

I will think on it during my holidays :)

Regards,

Florian

[PATCH 1/2] ARM: dts: OMAP3: Add GPMC controller

[Javier Martinez Canillas]

On Tue, Feb 5, 2013 at 6:23 PM, Florian Vaussard
<florian.vaussard@epfl.ch> wrote:
> Hi Javier,
>

Hi Florian,

> On 02/04/2013 12:57 PM, Javier Martinez Canillas wrote:
>
> [...]
>
>
>>
>> Yes, I saw on the list that Tony asked you too extend the GPMC DT
>> support. Flash support is on my TODO list too but I don't know if I'm
>> going to have time to work on this in the next few weeks.
>>
>> Since you are thinking to change the binding, there is something I
>> want to discuss with you.
>>
>> We have two different version for each IGEP board, one that uses NAND
>> memory and another that has OneNAND.
>>
>> With board files this is easy because the flash memory type is
>> hardcoded (in hardware) using sysboot pins [3]. So we check the
>> sysboot value and call board_nand_init() or board_onenand_init()
>> accordingly and pass the same static struct mtd_partition
>> igep_flash_partitions[] in both cases.
>>
>> But with DT this is a little bit trickier since you have to define
>> either a nand at 0 or onenand at 0 child node for the gpmc device node. So,
>> we will have to create lots of dts and dtsi to handle each combination
>> (IGEPv2 + NAND, IGEPv2 + OneNAND, IGEP COM + NAND, etc).
>>
>> I wonder if we could just have a generic gpmc-flash binding and maybe
>> use a "gpmc, flash_type" property or something like that to decide if
>> gpmc_onenand_init() or gpmc_nand_init() has to be called.
>>
>
> This boils down to the problem where you have an "if" statement in
> your board file, and I think no general solution exists.
>

Yes, this is a general issue since DT uses a declarative paradigm.

I was just pointing out that I found that particular issue with the flash
memory type on IGEP boards.

> In your suggestion, having a property will force you anyway to write
> distinct DT files to account for the two versions, so it won't really
> help if I understand correctly. We would need conditional
> section inside device trees, at least to test for simple parameters.
> Or a way to generate several DT blobs based on a single DT source file.
>

Well you will still need a different dtb for each <board,flash type> combination
but it could make your device trees smaller and simpler since you can define
your GPMC flash child node on a dtsi where you set all the partition and sizes
and export that device node to your boards dts that only set the
"gpmc, flash_type"
property to nand or onenand (assuming both your NAND and OneNAND version
have the same partition layout and sizes).

Otherwise you would have to define the complete onenand or nand child node
on your board dts having duplicated definition for your flash partition layouts.

> Another hackish solution would be to write a meta component, whose
> probe section would do your test. But this is just a stripped down version
> of the board file, far from being generic...
>

Yes, and that doesn't fit on the DT model too well. I still don't know what's
the best way to do it.

> I will think on it during my holidays :)
>

great, I hope you enjoy your holidays :-)

> Regards,
>
> Florian
>

Thanks a lot and best regards,
Javier

[PATCH 1/2] ARM: dts: OMAP3: Add GPMC controller

[Anil Kumar]

Hi Florian,

On Mon, Jan 28, 2013 at 11:24 PM, Florian Vaussard
<florian.vaussard@epfl.ch> wrote:
> Add device-tree support for the GPMC controller on the OMAP3.
>
> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
> ---
>  arch/arm/boot/dts/omap3.dtsi |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
> index 6c63118..2ddae38 100644
> --- a/arch/arm/boot/dts/omap3.dtsi
> +++ b/arch/arm/boot/dts/omap3.dtsi
> @@ -403,5 +403,16 @@
>                         ti,timer-alwon;
>                         ti,timer-secure;
>                 };
> +
> +               gpmc: gpmc at 6e000000 {
> +                       compatible = "ti,omap3430-gpmc";
> +                       ti,hwmods = "gpmc";
> +                       reg = <0x6e000000 0x1000000>;
> +                       interrupts = <20>;
> +                       gpmc,num-cs = <8>;
> +                       gpmc,num-waitpins = <4>;
> +                       #address-cells = <2>;
> +                       #size-cells = <1>;
> +               };
>         };
>  };

Tested on devkit8000 (omap3 based board)

Please take my Tested-by: Anil Kumar <anilk4.v@gmail.com>

Thanks,
Anil

[PATCH 1/2] ARM: dts: OMAP3: Add GPMC controller

[Javier Martinez Canillas]

On Sat, Feb 16, 2013 at 2:09 PM, Anil Kumar <anilkumar880@gmail.com> wrote:
> Hi Florian,
>
> On Mon, Jan 28, 2013 at 11:24 PM, Florian Vaussard
> <florian.vaussard@epfl.ch> wrote:
>> Add device-tree support for the GPMC controller on the OMAP3.
>>
>> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
>> ---
>>  arch/arm/boot/dts/omap3.dtsi |   11 +++++++++++
>>  1 files changed, 11 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
>> index 6c63118..2ddae38 100644
>> --- a/arch/arm/boot/dts/omap3.dtsi
>> +++ b/arch/arm/boot/dts/omap3.dtsi
>> @@ -403,5 +403,16 @@
>>                         ti,timer-alwon;
>>                         ti,timer-secure;
>>                 };
>> +
>> +               gpmc: gpmc at 6e000000 {
>> +                       compatible = "ti,omap3430-gpmc";
>> +                       ti,hwmods = "gpmc";
>> +                       reg = <0x6e000000 0x1000000>;
>> +                       interrupts = <20>;
>> +                       gpmc,num-cs = <8>;
>> +                       gpmc,num-waitpins = <4>;
>> +                       #address-cells = <2>;
>> +                       #size-cells = <1>;
>> +               };
>>         };
>>  };
>
> Tested on devkit8000 (omap3 based board)
>
> Please take my Tested-by: Anil Kumar <anilk4.v@gmail.com>
>
> Thanks,
> Anil
>

Hello Tony and Benoit,

Could this patch be merged to one of your branches?

It has already my Reviewed-by and now Anil's Tested-by.

Now that Daniel's OMAP GPMC binding were merged, there seems to be
many people working on adding support on their boards DT for
peripherals connected through the GPMC (NAND, OneNAND, SMSC LAN, etc).

I think it will be easier if people can base their work on top of this
patch instead of duplicating the work and sending the same patch to
add a GPMC device node to omap3.dtsi on each patch-set.

Thanks a lot and best regards,
Javier

[PATCH 1/2] ARM: dts: OMAP3: Add GPMC controller

[Cousson, Benoit]

Hi Javier,

On 2/16/2013 5:44 PM, Javier Martinez Canillas wrote:
> On Sat, Feb 16, 2013 at 2:09 PM, Anil Kumar <anilkumar880@gmail.com> wrote:
>> Hi Florian,
>>
>> On Mon, Jan 28, 2013 at 11:24 PM, Florian Vaussard
>> <florian.vaussard@epfl.ch> wrote:
>>> Add device-tree support for the GPMC controller on the OMAP3.
>>>
>>> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
>>> ---
>>>   arch/arm/boot/dts/omap3.dtsi |   11 +++++++++++
>>>   1 files changed, 11 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
>>> index 6c63118..2ddae38 100644
>>> --- a/arch/arm/boot/dts/omap3.dtsi
>>> +++ b/arch/arm/boot/dts/omap3.dtsi
>>> @@ -403,5 +403,16 @@
>>>                          ti,timer-alwon;
>>>                          ti,timer-secure;
>>>                  };
>>> +
>>> +               gpmc: gpmc at 6e000000 {
>>> +                       compatible = "ti,omap3430-gpmc";
>>> +                       ti,hwmods = "gpmc";
>>> +                       reg = <0x6e000000 0x1000000>;
>>> +                       interrupts = <20>;
>>> +                       gpmc,num-cs = <8>;
>>> +                       gpmc,num-waitpins = <4>;
>>> +                       #address-cells = <2>;
>>> +                       #size-cells = <1>;
>>> +               };
>>>          };
>>>   };
>>
>> Tested on devkit8000 (omap3 based board)
>>
>> Please take my Tested-by: Anil Kumar <anilk4.v@gmail.com>
>>
>> Thanks,
>> Anil
>>
>
> Hello Tony and Benoit,
>
> Could this patch be merged to one of your branches?

I'll take it.

> It has already my Reviewed-by and now Anil's Tested-by.
>
> Now that Daniel's OMAP GPMC binding were merged, there seems to be
> many people working on adding support on their boards DT for
> peripherals connected through the GPMC (NAND, OneNAND, SMSC LAN, etc).
>
> I think it will be easier if people can base their work on top of this
> patch instead of duplicating the work and sending the same patch to
> add a GPMC device node to omap3.dtsi on each patch-set.

Regards,
Benoit

[PATCH 1/2] ARM: dts: OMAP3: Add GPMC controller

[Jon Hunter]

On 01/28/2013 11:54 AM, Florian Vaussard wrote:
> Add device-tree support for the GPMC controller on the OMAP3.
> 
> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
> ---
>  arch/arm/boot/dts/omap3.dtsi |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
> index 6c63118..2ddae38 100644
> --- a/arch/arm/boot/dts/omap3.dtsi 
> +++ b/arch/arm/boot/dts/omap3.dtsi
> @@ -403,5 +403,16 @@
>  			ti,timer-alwon;
>  			ti,timer-secure;
>  		};
> +
> +		gpmc: gpmc at 6e000000 {
> +			compatible = "ti,omap3430-gpmc";
> +			ti,hwmods = "gpmc";
> +			reg = <0x6e000000 0x1000000>;

Can you make this size smaller? Although the reference manual states
16MB, the registers use less than 1KB of address space. Hence, it is
pointless mapping all this address space for the gpmc registers.

> +			interrupts = <20>;
> +			gpmc,num-cs = <8>;
> +			gpmc,num-waitpins = <4>;
> +			#address-cells = <2>;
> +			#size-cells = <1>;
> +		};
>  	};
>  };

Otherwise ...

Reviewed-by: Jon Hunter <jon-hunter@ti.com>

Cheers
Jon

[PATCH 1/2] ARM: dts: OMAP3: Add GPMC controller

[Benoit Cousson]

Hi Florian,

On 01/28/2013 06:54 PM, Florian Vaussard wrote:
> Add device-tree support for the GPMC controller on the OMAP3.
> 
> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>

Applied and pushed.
git://git.kernel.org/pub/scm/linux/kernel/git/bcousson/linux-omap-dt.git for_3.10/dts

Regards,
Benoit