DT GPMC SRAM and NOR flash support ?

DT GPMC SRAM and NOR flash support ?

[Mark Jackson]

There's plenty of DT support going in for NAND flash, but is there any work going on to support NOR
flash ?

And how about SRAM chips or other memory mapped devices ?

Regards
Mark J.

Re: DT GPMC SRAM and NOR flash support ?

[Jon Hunter]

Hi Mark,

On 02/01/2013 10:56 AM, Mark Jackson wrote:
> There's plenty of DT support going in for NAND flash, but is there any work going on to support NOR
> flash ?

What board and device are you working that is using NOR? I have a
OMAP2420 H4 with NOR that I have been doing a bit of maintenance for but
I don't spend much time on it. Eventually it will have to be done but it
is always good to know if there is a pressing need.

> And how about SRAM chips or other memory mapped devices ?

Not sure about SRAM (trying to think if I have a board with SRAM even),
but definitely, boards using the GPMC to interface to ethernet chips
need to be added.

Cheers
Jon

Re: DT GPMC SRAM and NOR flash support ?

[Mark Jackson]

On 01/02/13 17:12, Jon Hunter wrote:
> Hi Mark,
>
> On 02/01/2013 10:56 AM, Mark Jackson wrote:
>> There's plenty of DT support going in for NAND flash, but is there any work going on to support NOR
>> flash ?
>
> What board and device are you working that is using NOR? I have a
> OMAP2420 H4 with NOR that I have been doing a bit of maintenance for but
> I don't spend much time on it. Eventually it will have to be done but it
> is always good to know if there is a pressing need.

We have a custom AM335x CPU board (arriving on my desk within the next 
few weeks) that has both NAND and NOR Flash.

>> And how about SRAM chips or other memory mapped devices ?
>
> Not sure about SRAM (trying to think if I have a board with SRAM even),
> but definitely, boards using the GPMC to interface to ethernet chips
> need to be added.

The board also contains an FRAM chip (which is just treated as SRAM).

If you'd anything in the pipeline, I'm glad to help in any testing. 
I've created a custom cape board for the BeagleBone which contains the 
NOR flash and FRAM chips, so I'm not waiting for the new hardware to arrive.

Cheers
Mark J.

Re: DT GPMC SRAM and NOR flash support ?

[Mark Jackson]

On 01/02/13 19:39, Mark Jackson wrote:
> On 01/02/13 17:12, Jon Hunter wrote:
>> Hi Mark,
>>
>> On 02/01/2013 10:56 AM, Mark Jackson wrote:
>>> There's plenty of DT support going in for NAND flash, but is there any work going on to support NOR
>>> flash ?
>>
>> What board and device are you working that is using NOR? I have a
>> OMAP2420 H4 with NOR that I have been doing a bit of maintenance for but
>> I don't spend much time on it. Eventually it will have to be done but it
>> is always good to know if there is a pressing need.
> 
> We have a custom AM335x CPU board (arriving on my desk within the next few weeks) that has both NAND
> and NOR Flash.
> 
>>> And how about SRAM chips or other memory mapped devices ?
>>
>> Not sure about SRAM (trying to think if I have a board with SRAM even),
>> but definitely, boards using the GPMC to interface to ethernet chips
>> need to be added.
> 
> The board also contains an FRAM chip (which is just treated as SRAM).
> 
> If you'd anything in the pipeline, I'm glad to help in any testing. I've created a custom cape board
> for the BeagleBone which contains the NOR flash and FRAM chips, so I'm not waiting for the new
> hardware to arrive.

I've experimented with trying to add a mtd-physmap device, but no joy.

I'm guessing that the current mtd-physmap code is (in some way) currently incompatible with the GPMC
device ?

Mark J.

Re: DT GPMC SRAM and NOR flash support ?

[Jon Hunter]

On 02/05/2013 10:16 AM, Mark Jackson wrote:
> On 01/02/13 19:39, Mark Jackson wrote:
>> On 01/02/13 17:12, Jon Hunter wrote:
>>> Hi Mark,
>>>
>>> On 02/01/2013 10:56 AM, Mark Jackson wrote:
>>>> There's plenty of DT support going in for NAND flash, but is there any work going on to support NOR
>>>> flash ?
>>>
>>> What board and device are you working that is using NOR? I have a
>>> OMAP2420 H4 with NOR that I have been doing a bit of maintenance for but
>>> I don't spend much time on it. Eventually it will have to be done but it
>>> is always good to know if there is a pressing need.
>>
>> We have a custom AM335x CPU board (arriving on my desk within the next few weeks) that has both NAND
>> and NOR Flash.
>>
>>>> And how about SRAM chips or other memory mapped devices ?
>>>
>>> Not sure about SRAM (trying to think if I have a board with SRAM even),
>>> but definitely, boards using the GPMC to interface to ethernet chips
>>> need to be added.
>>
>> The board also contains an FRAM chip (which is just treated as SRAM).
>>
>> If you'd anything in the pipeline, I'm glad to help in any testing. I've created a custom cape board
>> for the BeagleBone which contains the NOR flash and FRAM chips, so I'm not waiting for the new
>> hardware to arrive.

Sorry for the delay. I personally don't have anything in the pipe.

Afzal, do you know of anyone looking at this?

> I've experimented with trying to add a mtd-physmap device, but no joy.
> 
> I'm guessing that the current mtd-physmap code is (in some way) currently incompatible with the GPMC
> device ?

I am not familiar with mtd-physmap to comment. Is that DT specific?

Cheers
Jon

Re: DT GPMC SRAM and NOR flash support ?

[Mark Jackson]

On 05/02/13 16:35, Jon Hunter wrote:
> 
> On 02/05/2013 10:16 AM, Mark Jackson wrote:
>> On 01/02/13 19:39, Mark Jackson wrote:
>>> On 01/02/13 17:12, Jon Hunter wrote:
>>>> Hi Mark,
>>>>
>>>> On 02/01/2013 10:56 AM, Mark Jackson wrote:
>>>>> There's plenty of DT support going in for NAND flash, but is there any work going on to support NOR
>>>>> flash ?
>>>>
>>>> What board and device are you working that is using NOR? I have a
>>>> OMAP2420 H4 with NOR that I have been doing a bit of maintenance for but
>>>> I don't spend much time on it. Eventually it will have to be done but it
>>>> is always good to know if there is a pressing need.
>>>
>>> We have a custom AM335x CPU board (arriving on my desk within the next few weeks) that has both NAND
>>> and NOR Flash.
>>>
>>>>> And how about SRAM chips or other memory mapped devices ?
>>>>
>>>> Not sure about SRAM (trying to think if I have a board with SRAM even),
>>>> but definitely, boards using the GPMC to interface to ethernet chips
>>>> need to be added.
>>>
>>> The board also contains an FRAM chip (which is just treated as SRAM).
>>>
>>> If you'd anything in the pipeline, I'm glad to help in any testing. I've created a custom cape board
>>> for the BeagleBone which contains the NOR flash and FRAM chips, so I'm not waiting for the new
>>> hardware to arrive.
> 
> Sorry for the delay. I personally don't have anything in the pipe.
> 
> Afzal, do you know of anyone looking at this?
> 
>> I've experimented with trying to add a mtd-physmap device, but no joy.
>>
>> I'm guessing that the current mtd-physmap code is (in some way) currently incompatible with the GPMC
>> device ?
> 
> I am not familiar with mtd-physmap to comment. Is that DT specific?

Well, it's documented in Documentation/devicetree/bindings/mtd/mtd-physmap.txt, showing you can
specify "cfi-flash" or "mtd-ram" devices, eg (taken from mtd-physmap.txt):-

	flash@ff000000 {
		compatible = "amd,am29lv128ml", "cfi-flash";
		reg = <ff000000 01000000>;
		bank-width = <4>;
		device-width = <1>;
		#address-cells = <1>;
		#size-cells = <1>;
		fs@0 {
			label = "fs";
			reg = <0 f80000>;
		};
		firmware@f80000 {
			label ="firmware";
			reg = <f80000 80000>;
			read-only;
		};
	};

	sram@2,0 {
		compatible = "samsung,k6f1616u6a", "mtd-ram";
		reg = <2 0 0x00200000>;
		bank-width = <2>;
	};

But I guess the GPMC needs to be configured to enable chip selects, data widths, etc.

I did get *something* on the address / data bus by adding the patches below, but I'm getting bus
contention, so something else needs setting up in the GPMC.

I'm no great device driver writer, but if anyone can give me some pointers, I'm happy to keep digging.

Cheers
Mark J.

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index a37e1c9..8f45d18 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -1265,10 +1265,13 @@ static int gpmc_probe_dt(struct platform_device *pdev)
        struct device_node *child;
        const struct of_device_id *of_id =
                of_match_device(gpmc_dt_ids, &pdev->dev);
+       unsigned long base;

        if (!of_id)
                return 0;

+       gpmc_cs_request(0, SZ_16M, &base);
+
        for_each_node_by_name(child, "nand") {
                ret = gpmc_probe_nand_child(pdev, child);
                of_node_put(child);
diff --git a/arch/arm/boot/dts/am335x-bone.dts b/arch/arm/boot/dts/am335x-bone.dts
index 11b240c..ad89446 100644
--- a/arch/arm/boot/dts/am335x-bone.dts
+++ b/arch/arm/boot/dts/am335x-bone.dts
@@ -26,7 +26,7 @@

        am33xx_pinmux: pinmux@44e10800 {
                pinctrl-names = "default";
-               pinctrl-0 = <&user_leds_s0>;
+               pinctrl-0 = <&user_leds_s0 &gpmc_pins_s0>;

                user_leds_s0: user_leds_s0 {
                        pinctrl-single,pins = <
@@ -36,6 +36,49 @@
                                0x60 0x17       /* gpmc_a8.gpio1_24, OUTPUT_PULLUP | MODE7 */
                        >;
                };
+
+               gpmc_pins_s0: gpmc_pins_s0 {
+                       pinctrl-single,pins = <
+                               0x0 0x30        /* gpmc_ad0.gpmc_ad0, INPUT | PULLUP | MODE0 */
+                               0x4 0x30        /* gpmc_ad1.gpmc_ad1, INPUT | PULLUP | MODE0 */
+                               0x8 0x30        /* gpmc_ad2.gpmc_ad2, INPUT | PULLUP | MODE0 */
+                               0xc 0x30        /* gpmc_ad3.gpmc_ad3, INPUT | PULLUP | MODE0 */
+                               0x10 0x30       /* gpmc_ad4.gpmc_ad4, INPUT | PULLUP | MODE0 */
+                               0x14 0x30       /* gpmc_ad5.gpmc_ad5, INPUT | PULLUP | MODE0 */
+                               0x18 0x30       /* gpmc_ad6.gpmc_ad6, INPUT | PULLUP | MODE0 */
+                               0x1c 0x30       /* gpmc_ad7.gpmc_ad7, INPUT | PULLUP | MODE0 */
+                               0x20 0x30       /* gpmc_ad8.gpmc_ad8, INPUT | PULLUP | MODE0 */
+                               0x24 0x30       /* gpmc_ad9.gpmc_ad9, INPUT | PULLUP | MODE0 */
+                               0x28 0x30       /* gpmc_ad10.gpmc_ad10, INPUT | PULLUP | MODE0 */
+                               0x2c 0x30       /* gpmc_ad11.gpmc_ad11, INPUT | PULLUP | MODE0 */
+                               0x30 0x30       /* gpmc_ad12.gpmc_ad12, INPUT | PULLUP | MODE0 */
+                               0x34 0x30       /* gpmc_ad13.gpmc_ad13, INPUT | PULLUP | MODE0 */
+                               0x38 0x30       /* gpmc_ad14.gpmc_ad14, INPUT | PULLUP | MODE0 */
+                               0x3c 0x30       /* gpmc_ad15.gpmc_ad15, INPUT | PULLUP | MODE0 */
+
+                               0x7c 0x10       /* gpmc_csn0.gpmc_csn0, OUTPUT_PULLUP | MODE0 */
+                               0x80 0x10       /* gpmc_csn1.gpmc_csn1, OUTPUT_PULLUP | MODE0 */
+                               0x84 0x10       /* gpmc_csn2.gpmc_csn2, OUTPUT_PULLUP | MODE0 */
+                               0x88 0x10       /* gpmc_csn3.gpmc_csn3, OUTPUT_PULLUP | MODE0 */
+
+                               0x90 0x10       /* gpmc_advn_ale.gpmc_advn_ale, OUTPUT_PULLUP | MODE0 */
+                               0x94 0x10       /* gpmc_oen_ren.gpmc_oen_ren, OUTPUT_PULLUP | MODE0 */
+                               0x98 0x10       /* gpmc_wen.gpmc_wen, OUTPUT_PULLUP | MODE0 */
+                               0x9c 0x10       /* gpmc_ben0_cle.gpmc_ben0_cle, OUTPUT_PULLUP | MODE0 */
+
+                               0xa4 0x11       /* lcd_data1.gpmc_a1, OUTPUT_PULLUP | MODE1 */
+                               0xa8 0x11       /* lcd_data2.gpmc_a2, OUTPUT_PULLUP | MODE1 */
+                               0xac 0x11       /* lcd_data3.gpmc_a3, OUTPUT_PULLUP | MODE1 */
+                               0xb0 0x11       /* lcd_data4.gpmc_a4, OUTPUT_PULLUP | MODE1 */
+                               0xb4 0x11       /* lcd_data5.gpmc_a5, OUTPUT_PULLUP | MODE1 */
+                               0xb8 0x11       /* lcd_data6.gpmc_a6, OUTPUT_PULLUP | MODE1 */
+                               0xbc 0x11       /* lcd_data7.gpmc_a7, OUTPUT_PULLUP | MODE1 */
+
+                               0xe0 0x11       /* lcd_vsync.gpmc_a8, OUTPUT_PULLUP | MODE1 */
+                               0xe4 0x11       /* lcd_hsync.gpmc_a9, OUTPUT_PULLUP | MODE1 */
+                               0xe8 0x11       /* lcd_pclk.gpmc_a10, OUTPUT_PULLUP | MODE1 */
+                       >;
+               };
        };

        ocp {
@@ -50,7 +93,15 @@
                        tps: tps@24 {
                                reg = <0x24>;
                        };
+               };

+               elm: elm@48080000 {
+                       status = "okay";
+               };
+
+               gpmc: gpmc@50000000 {
+                       status = "okay";
+                       ranges = <0 0 0x08000000 0x08000000>;   /* CS0: NOR 16M */
                };
        };

Re: DT GPMC SRAM and NOR flash support ?

[Jon Hunter]

On 02/05/2013 10:48 AM, Mark Jackson wrote:
> On 05/02/13 16:35, Jon Hunter wrote:
>>
>> On 02/05/2013 10:16 AM, Mark Jackson wrote:
>>> On 01/02/13 19:39, Mark Jackson wrote:
>>>> On 01/02/13 17:12, Jon Hunter wrote:
>>>>> Hi Mark,
>>>>>
>>>>> On 02/01/2013 10:56 AM, Mark Jackson wrote:
>>>>>> There's plenty of DT support going in for NAND flash, but is there any work going on to support NOR
>>>>>> flash ?
>>>>>
>>>>> What board and device are you working that is using NOR? I have a
>>>>> OMAP2420 H4 with NOR that I have been doing a bit of maintenance for but
>>>>> I don't spend much time on it. Eventually it will have to be done but it
>>>>> is always good to know if there is a pressing need.
>>>>
>>>> We have a custom AM335x CPU board (arriving on my desk within the next few weeks) that has both NAND
>>>> and NOR Flash.
>>>>
>>>>>> And how about SRAM chips or other memory mapped devices ?
>>>>>
>>>>> Not sure about SRAM (trying to think if I have a board with SRAM even),
>>>>> but definitely, boards using the GPMC to interface to ethernet chips
>>>>> need to be added.
>>>>
>>>> The board also contains an FRAM chip (which is just treated as SRAM).
>>>>
>>>> If you'd anything in the pipeline, I'm glad to help in any testing. I've created a custom cape board
>>>> for the BeagleBone which contains the NOR flash and FRAM chips, so I'm not waiting for the new
>>>> hardware to arrive.
>>
>> Sorry for the delay. I personally don't have anything in the pipe.
>>
>> Afzal, do you know of anyone looking at this?
>>
>>> I've experimented with trying to add a mtd-physmap device, but no joy.
>>>
>>> I'm guessing that the current mtd-physmap code is (in some way) currently incompatible with the GPMC
>>> device ?
>>
>> I am not familiar with mtd-physmap to comment. Is that DT specific?
> 
> Well, it's documented in Documentation/devicetree/bindings/mtd/mtd-physmap.txt, showing you can
> specify "cfi-flash" or "mtd-ram" devices, eg (taken from mtd-physmap.txt):-
> 
> 	flash@ff000000 {
> 		compatible = "amd,am29lv128ml", "cfi-flash";
> 		reg = <ff000000 01000000>;
> 		bank-width = <4>;
> 		device-width = <1>;
> 		#address-cells = <1>;
> 		#size-cells = <1>;
> 		fs@0 {
> 			label = "fs";
> 			reg = <0 f80000>;
> 		};
> 		firmware@f80000 {
> 			label ="firmware";
> 			reg = <f80000 80000>;
> 			read-only;
> 		};
> 	};
> 
> 	sram@2,0 {
> 		compatible = "samsung,k6f1616u6a", "mtd-ram";
> 		reg = <2 0 0x00200000>;
> 		bank-width = <2>;
> 	};

Ok, I see that the NOR flash on my omap2420 H4 is named "physmap-flash"
(I have not ported to DT yet). So the GPMC should work with it.

> But I guess the GPMC needs to be configured to enable chip selects, data widths, etc.

Exactly.

> I did get *something* on the address / data bus by adding the patches below, but I'm getting bus
> contention, so something else needs setting up in the GPMC.
> 
> I'm no great device driver writer, but if anyone can give me some pointers, I'm happy to keep digging.
> 
> Cheers
> Mark J.
> 
> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
> index a37e1c9..8f45d18 100644
> --- a/arch/arm/mach-omap2/gpmc.c
> +++ b/arch/arm/mach-omap2/gpmc.c
> @@ -1265,10 +1265,13 @@ static int gpmc_probe_dt(struct platform_device *pdev)
>         struct device_node *child;
>         const struct of_device_id *of_id =
>                 of_match_device(gpmc_dt_ids, &pdev->dev);
> +       unsigned long base;
> 
>         if (!of_id)
>                 return 0;
> 
> +       gpmc_cs_request(0, SZ_16M, &base);
> +

You should check what gpmc_cs_request returns.

Where are the NOR flash timings setup? Or are they already configured by
the bootloader? If so then that is probably ok for now.

>         for_each_node_by_name(child, "nand") {
>                 ret = gpmc_probe_nand_child(pdev, child);
>                 of_node_put(child);

If you look at the gpmc_probe_nand_child(), this function is setting up
the NAND timings for the GPMC.

Ideally, we would have a similar function for nor (ie.
gpmc_probe_nor_child()), that would read the NOR information
(chip-select, size, timings, etc) from DT. However, I understand that
the above is just for test purposes.

Cheers
Jon

RE: DT GPMC SRAM and NOR flash support ?

[Mohammed, Afzal]

Hi Jon,

On Tue, Feb 05, 2013 at 22:05:20, Hunter, Jon wrote:
> On 02/05/2013 10:16 AM, Mark Jackson wrote:

> >>>> There's plenty of DT support going in for NAND flash, but is there any work going on to support NOR
> >>>> flash ?

> >> If you'd anything in the pipeline, I'm glad to help in any testing. I've created a custom cape board
> >> for the BeagleBone which contains the NOR flash and FRAM chips, so I'm not waiting for the new
> >> hardware to arrive.

> Sorry for the delay. I personally don't have anything in the pipe.
> 
> Afzal, do you know of anyone looking at this?

I am not aware of anyone working on it.

Regards
Afzal

Re: DT GPMC SRAM and NOR flash support ?

[Mark Jackson]

Okay ... I have made some progress, but it's not ideal.

Currently I've hacked the GPMC DT driver (gpmc_probe_dt(), etc) so it now handles setting up the
chip selects and timings for NOR devices, e.g.

		gpmc: gpmc@50000000 {
			status = "okay";
			ranges = <0 0 0x08000000 0x08000000>;	/* CS0: NOR 16M */
			
			nor@0,0 {
				compatible = "spansion,s29gl064n90t", "cfi-flash";
				reg = <0 0 0>;
				bank-width = <2>;

				gpmc,sync-clk = <0>;
				gpmc,cs-on = <10>;
				gpmc,cs-rd-off = <150>;
				gpmc,cs-wr-off = <150>;
				gpmc,adv-on = <10>;
				gpmc,adv-rd-off = <10>;
				gpmc,adv-wr-off = <10>;
				gpmc,oe-on = <30>;
				gpmc,oe-off = <150>;
				gpmc,we-on = <30>;
				gpmc,we-off = <150>;
				gpmc,rd-cycle = <150>;
				gpmc,wr-cycle = <150>;
				gpmc,access = <130>;
				gpmc,page-burst-access = <10>;
				gpmc,cycle2cycle-diff = <1>;
				gpmc,cycle2cycle-same = <1>;
				gpmc,cycle2cycle-delay = <10>;
				gpmc,wr-data-mux-bus = <60>;
			};
		};

But the physmap driver (of_flash_probe()) is unable to use this information.  It seems that although
I can call of_flash_probe() from my NOR setup code, the platform_device being reference is wrong.

The platform_device passed to my gpmc_probe_nor_child() routine from gpmc_probe_dt() points to my
gpmc entry (above), but the physmap probe requires its own DT entry (rather than a node child such
as my NOR entry with the GPMC device entry).

So I need to have any extra entry in the DT file as follows:-

	nor-flash@08000000 {
		compatible = "spansion,s29gl064n90t", "cfi-flash";
		reg = <0x08000000 0x00800000>;
		bank-width = <2>;
	};

So the GPMC entry handles all the chip select and timing setup, but the 2nd entry is the only one
the physmap driver can see.

Would it be acceptable to re-code of_flash_probe() to allow either a child device_node to be passed
or a platform_device ?

Cheers
Mark J.

Re: DT GPMC SRAM and NOR flash support ?

[Mark Jackson]

On 07/02/13 09:51, Mark Jackson wrote:
> Okay ... I have made some progress, but it's not ideal.

<snip>

> But the physmap driver (of_flash_probe()) is unable to use this information.  It seems that although
> I can call of_flash_probe() from my NOR setup code, the platform_device being reference is wrong.
> 
> The platform_device passed to my gpmc_probe_nor_child() routine from gpmc_probe_dt() points to my
> gpmc entry (above), but the physmap probe requires its own DT entry (rather than a node child such
> as my NOR entry with the GPMC device entry).
> 
> So I need to have any extra entry in the DT file as follows:-
> 
> 	nor-flash@08000000 {
> 		compatible = "spansion,s29gl064n90t", "cfi-flash";
> 		reg = <0x08000000 0x00800000>;
> 		bank-width = <2>;
> 	};
> 
> So the GPMC entry handles all the chip select and timing setup, but the 2nd entry is the only one
> the physmap driver can see.
> 
> Would it be acceptable to re-code of_flash_probe() to allow either a child device_node to be passed
> or a platform_device ?

Or is it acceptable to simply state the gpmc portion is for setting up the chip access, and you *do*
need a separate physmap section ?

That's certainly easier, and it works without any changes to the physmap driver.

Regards
Mark J.

Re: DT GPMC SRAM and NOR flash support ?

[Jon Hunter]

On 02/07/2013 03:51 AM, Mark Jackson wrote:
> Okay ... I have made some progress, but it's not ideal.
> 
> Currently I've hacked the GPMC DT driver (gpmc_probe_dt(), etc) so it now handles setting up the
> chip selects and timings for NOR devices, e.g.
> 
> 		gpmc: gpmc@50000000 {
> 			status = "okay";
> 			ranges = <0 0 0x08000000 0x08000000>;	/* CS0: NOR 16M */
> 			
> 			nor@0,0 {
> 				compatible = "spansion,s29gl064n90t", "cfi-flash";
> 				reg = <0 0 0>;
> 				bank-width = <2>;
> 
> 				gpmc,sync-clk = <0>;
> 				gpmc,cs-on = <10>;
> 				gpmc,cs-rd-off = <150>;
> 				gpmc,cs-wr-off = <150>;
> 				gpmc,adv-on = <10>;
> 				gpmc,adv-rd-off = <10>;
> 				gpmc,adv-wr-off = <10>;
> 				gpmc,oe-on = <30>;
> 				gpmc,oe-off = <150>;
> 				gpmc,we-on = <30>;
> 				gpmc,we-off = <150>;
> 				gpmc,rd-cycle = <150>;
> 				gpmc,wr-cycle = <150>;
> 				gpmc,access = <130>;
> 				gpmc,page-burst-access = <10>;
> 				gpmc,cycle2cycle-diff = <1>;
> 				gpmc,cycle2cycle-same = <1>;
> 				gpmc,cycle2cycle-delay = <10>;
> 				gpmc,wr-data-mux-bus = <60>;
> 			};
> 		};
> 
> But the physmap driver (of_flash_probe()) is unable to use this information.  It seems that although
> I can call of_flash_probe() from my NOR setup code, the platform_device being reference is wrong.
> 
> The platform_device passed to my gpmc_probe_nor_child() routine from gpmc_probe_dt() points to my
> gpmc entry (above), but the physmap probe requires its own DT entry (rather than a node child such
> as my NOR entry with the GPMC device entry).
> 
> So I need to have any extra entry in the DT file as follows:-
> 
> 	nor-flash@08000000 {
> 		compatible = "spansion,s29gl064n90t", "cfi-flash";
> 		reg = <0x08000000 0x00800000>;
> 		bank-width = <2>;
> 	};
> 
> So the GPMC entry handles all the chip select and timing setup, but the 2nd entry is the only one
> the physmap driver can see.
> 
> Would it be acceptable to re-code of_flash_probe() to allow either a child device_node to be passed
> or a platform_device ?

Ideally, it should be a child device of the gpmc so that if the gpmc
device fails to init correctly, the child is not registered.

I need to think about this some more to figure out the best way to
handle nor.

Cheers
Jon

Re: DT GPMC SRAM and NOR flash support ?

[Ezequiel Garcia]

Hi Mark,

On Thu, Feb 7, 2013 at 6:51 AM, Mark Jackson <mpfj-list@mimc.co.uk> wrote:
> Okay ... I have made some progress, but it's not ideal.
>
> Currently I've hacked the GPMC DT driver (gpmc_probe_dt(), etc) so it now handles setting up the
> chip selects and timings for NOR devices, e.g.
>
>                 gpmc: gpmc@50000000 {
>                         status = "okay";
>                         ranges = <0 0 0x08000000 0x08000000>;   /* CS0: NOR 16M */
>
>                         nor@0,0 {
>                                 compatible = "spansion,s29gl064n90t", "cfi-flash";
>                                 reg = <0 0 0>;
>                                 bank-width = <2>;
>
>                                 gpmc,sync-clk = <0>;
>                                 gpmc,cs-on = <10>;
>                                 gpmc,cs-rd-off = <150>;
>                                 gpmc,cs-wr-off = <150>;
>                                 gpmc,adv-on = <10>;
>                                 gpmc,adv-rd-off = <10>;
>                                 gpmc,adv-wr-off = <10>;
>                                 gpmc,oe-on = <30>;
>                                 gpmc,oe-off = <150>;
>                                 gpmc,we-on = <30>;
>                                 gpmc,we-off = <150>;
>                                 gpmc,rd-cycle = <150>;
>                                 gpmc,wr-cycle = <150>;
>                                 gpmc,access = <130>;
>                                 gpmc,page-burst-access = <10>;
>                                 gpmc,cycle2cycle-diff = <1>;
>                                 gpmc,cycle2cycle-same = <1>;
>                                 gpmc,cycle2cycle-delay = <10>;
>                                 gpmc,wr-data-mux-bus = <60>;
>                         };
>                 };
>
> But the physmap driver (of_flash_probe()) is unable to use this information.  It seems that although
> I can call of_flash_probe() from my NOR setup code, the platform_device being reference is wrong.
>
> The platform_device passed to my gpmc_probe_nor_child() routine from gpmc_probe_dt() points to my
> gpmc entry (above), but the physmap probe requires its own DT entry (rather than a node child such
> as my NOR entry with the GPMC device entry).
>

I think you can call something like:

of_platform_populate();

On your GPMC node and have the child initialize.
This way you don't need to have separate cfi-flash compatible nodes,
you can have them as childs.

Although I'm not sure this is a sane approach, I'm almost sure this should work.

-- 
    Ezequiel

Re: DT GPMC SRAM and NOR flash support ?

[Jon Hunter]

On 02/09/2013 07:27 AM, Ezequiel Garcia wrote:
> Hi Mark,
> 
> On Thu, Feb 7, 2013 at 6:51 AM, Mark Jackson <mpfj-list@mimc.co.uk> wrote:
>> Okay ... I have made some progress, but it's not ideal.
>>
>> Currently I've hacked the GPMC DT driver (gpmc_probe_dt(), etc) so it now handles setting up the
>> chip selects and timings for NOR devices, e.g.
>>
>>                 gpmc: gpmc@50000000 {
>>                         status = "okay";
>>                         ranges = <0 0 0x08000000 0x08000000>;   /* CS0: NOR 16M */
>>
>>                         nor@0,0 {
>>                                 compatible = "spansion,s29gl064n90t", "cfi-flash";
>>                                 reg = <0 0 0>;
>>                                 bank-width = <2>;
>>
>>                                 gpmc,sync-clk = <0>;
>>                                 gpmc,cs-on = <10>;
>>                                 gpmc,cs-rd-off = <150>;
>>                                 gpmc,cs-wr-off = <150>;
>>                                 gpmc,adv-on = <10>;
>>                                 gpmc,adv-rd-off = <10>;
>>                                 gpmc,adv-wr-off = <10>;
>>                                 gpmc,oe-on = <30>;
>>                                 gpmc,oe-off = <150>;
>>                                 gpmc,we-on = <30>;
>>                                 gpmc,we-off = <150>;
>>                                 gpmc,rd-cycle = <150>;
>>                                 gpmc,wr-cycle = <150>;
>>                                 gpmc,access = <130>;
>>                                 gpmc,page-burst-access = <10>;
>>                                 gpmc,cycle2cycle-diff = <1>;
>>                                 gpmc,cycle2cycle-same = <1>;
>>                                 gpmc,cycle2cycle-delay = <10>;
>>                                 gpmc,wr-data-mux-bus = <60>;
>>                         };
>>                 };
>>
>> But the physmap driver (of_flash_probe()) is unable to use this information.  It seems that although
>> I can call of_flash_probe() from my NOR setup code, the platform_device being reference is wrong.

By the way, you shouldn't be calling of_flash_probe directly. If we
register the device correctly this should be called for us. See below on
how to fix the registering of the device.

>> The platform_device passed to my gpmc_probe_nor_child() routine from gpmc_probe_dt() points to my
>> gpmc entry (above), but the physmap probe requires its own DT entry (rather than a node child such
>> as my NOR entry with the GPMC device entry).
>>
> 
> I think you can call something like:
> 
> of_platform_populate();

This is being call from the mach-omap2/board-generic.c file on boot.
Where are you suggesting this is called from?

> On your GPMC node and have the child initialize.
> This way you don't need to have separate cfi-flash compatible nodes,
> you can have them as childs.
> 
> Although I'm not sure this is a sane approach, I'm almost sure this should work.

>From looking at the DT code and playing around with my omap2 board, if
we add "simple-bus" to the compatible string for the gpmc controller,
then DT will register the cfi-flash device as implemented above and you
don't need the extra node. For example ...

                gpmc: gpmc@6800a000 {
-                       compatible = "ti,omap2420-gpmc";
+                       compatible = "ti,omap2420-gpmc", "simple-bus";
                        ti,hwmods = "gpmc";
                        reg = <0x6800a000 0x1000>;
                        interrupts = <20>;

I am also concerned about how gpmc_cs_request() is implemented when we
are booting with device-tree. This function is mapping the CS where ever
it has space available and is not looking at the DTS to see where you
want the flash to be. So this needs to be addressed too.

Cheers
Jon

Re: DT GPMC SRAM and NOR flash support ?

[Ezequiel Garcia]

Hi Jon

On Mon, Feb 11, 2013 at 7:21 PM, Jon Hunter <jon-hunter@ti.com> wrote:
>
> This is being call from the mach-omap2/board-generic.c file on boot.
> Where are you suggesting this is called from?
>

I was suggesting this could be called in gpmc_probe_dt() in gpmc.c.
Instead of using for_each_node_by_name() and initialize manually each node,
it should be possibly (perhaps with some ugly hack) to use
of_platform_populate()
to initialize gpmc child devices.

But I'm not exactly sure how this would work.

-- 
    Ezequiel

Re: DT GPMC SRAM and NOR flash support ?

[Jon Hunter]

On 02/07/2013 03:51 AM, Mark Jackson wrote:
> Okay ... I have made some progress, but it's not ideal.
> 
> Currently I've hacked the GPMC DT driver (gpmc_probe_dt(), etc) so it now handles setting up the
> chip selects and timings for NOR devices, e.g.
> 
> 		gpmc: gpmc@50000000 {
> 			status = "okay";
> 			ranges = <0 0 0x08000000 0x08000000>;	/* CS0: NOR 16M */

Is that really 16M? Looks more like 128M :-)

> 			nor@0,0 {
> 				compatible = "spansion,s29gl064n90t", "cfi-flash";
> 				reg = <0 0 0>;

I think that cfi-flash is expecting a length here. Here is
what I have ...

+&gpmc {
+       ranges = <0 0 0x08000000 0x04000000>;
+
+       nor@0,0 {
+               compatible = "cfi-flash";
+               linux,mtd-name= "intel,ge28f256l18b85";
+               #address-cells = <1>;
+               #size-cells = <1>;
+               reg = <0 0 0x04000000>;
+               bank-width = <2>;
+
+               partition@0 {
+                       label = "bootloader";
+                       reg = <0 0x20000>;
+               };
+               partition@0x20000 {
+                       label = "params";
+                       reg = <0x20000 0x20000>;
+               };
+               partition@0x40000 {
+                       label = "kernel";
+                       reg = <0x40000 0x200000>;
+               };
+               partition@0x240000 {
+                       label = "file-system";
+                       reg = <0x240000 0x3dc0000>;
+               };
+       };

> 	nor-flash@08000000 {
> 		compatible = "spansion,s29gl064n90t", "cfi-flash";
> 		reg = <0x08000000 0x00800000>;
> 		bank-width = <2>;
> 	};

You don't need this extra entry if you add "simple-bus" to
the gpmc node compatible string.

+               gpmc: gpmc@6800a000 {
+                       compatible = "ti,omap2420-gpmc", "simple-bus";
+                       ti,hwmods = "gpmc";
+                       reg = <0x6800a000 0x1000>;
+                       interrupts = <20>;
+
+                       gpmc,num-cs = <8>;
+                       gpmc,num-waitpins = <4>;
+                       #address-cells = <2>;
+                       #size-cells = <1>;
+               };

Currently I have the following and this is working for me. 
Please note that on my board the bootloader is configuring
the timings for me and so this is missing from the below
implementation and still needs to be added.

Cheers
Jon

>From c0ede833fad704ab452b116154177e3a59166c7e Mon Sep 17 00:00:00 2001
From: Jon Hunter <jon-hunter@ti.com>
Date: Fri, 8 Feb 2013 16:46:13 -0600
Subject: [PATCH] ARM: OMAP2+: Add device-tree support for NOR flash

---
 arch/arm/mach-omap2/gpmc.c |   69 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index bc90155..421320b 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -26,6 +26,7 @@
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
 #include <linux/of.h>
+#include <linux/of_address.h>
 #include <linux/of_mtd.h>
 #include <linux/of_device.h>
 #include <linux/mtd/nand.h>
@@ -517,6 +518,26 @@ static int gpmc_cs_delete_mem(int cs)
 	return r;
 }
 
+static int gpmc_cs_remap(int cs, u32 base)
+{
+	int r;
+	u32 old_base, size;
+
+	gpmc_cs_get_memconf(cs, &old_base, &size);
+	if (base == old_base)
+		return 0;
+	gpmc_cs_disable_mem(cs);
+	r = gpmc_cs_delete_mem(cs);
+	if (IS_ERR_VALUE(r))
+		return r;
+	r = gpmc_cs_insert_mem(cs, base, size);
+	if (IS_ERR_VALUE(r))
+		return r;
+	gpmc_cs_enable_mem(cs, base, size);
+
+	return 0;
+}
+
 int gpmc_cs_request(int cs, unsigned long size, unsigned long *base)
 {
 	struct resource *res = &gpmc_cs_mem[cs];
@@ -1305,6 +1326,45 @@ static int gpmc_probe_onenand_child(struct platform_device *pdev,
 }
 #endif
 
+static int gpmc_probe_nor_child(struct platform_device *pdev,
+				struct device_node *child)
+{
+	struct resource res;
+	unsigned long base;
+	int rc, cs;
+
+	if (of_property_read_u32(child, "reg", &cs) < 0) {
+		dev_err(&pdev->dev, "%s has no 'reg' property\n",
+			child->full_name);
+		return -ENODEV;
+	}
+
+	if (of_address_to_resource(child, 0, &res)) {
+		dev_err(&pdev->dev, "%s has malformed 'reg' property\n",
+			child->full_name);
+		return -ENODEV;
+	}
+
+	rc = gpmc_cs_request(cs, res.end - res.start, &base);
+	if (rc < 0)
+		pr_err("%s: cannot request GPMC CS %d\n", __func__, cs);
+
+	/*
+	 * gpmc_cs_request() will map the CS to an arbitary location
+	 * in the gpmc address space. When booting with device-tree we
+	 * want the NOR flash to be mapped to the location specified
+	 * in the device-tree blob. So remap the CS to this location.
+	 */
+	rc = gpmc_cs_remap(cs, res.start);
+	if (rc < 0) {
+		pr_err("%s: cannot remap GPMC CS %d to 0x%x\n",
+		       __func__, cs, res.start);
+		gpmc_cs_free(cs);
+	}
+
+	return rc;
+}
+
 static int gpmc_probe_dt(struct platform_device *pdev)
 {
 	int ret;
@@ -1330,6 +1390,15 @@ static int gpmc_probe_dt(struct platform_device *pdev)
 			return ret;
 		}
 	}
+
+	for_each_node_by_name(child, "nor") {
+		ret = gpmc_probe_nor_child(pdev, child);
+		if (ret < 0) {
+			of_node_put(child);
+			return ret;
+		}
+	}
+
 	return 0;
 }
 #else
-- 
1.7.10.4

Re: DT GPMC SRAM and NOR flash support ?

[Ezequiel Garcia]

Hi Jon,

I'm working on a similar memory controller for plat-orion.
I have a few questions about your approach.

On Wed, Feb 13, 2013 at 03:07:06PM -0600, Jon Hunter wrote:
> 
> On 02/07/2013 03:51 AM, Mark Jackson wrote:
> > Okay ... I have made some progress, but it's not ideal.
> > 
> > Currently I've hacked the GPMC DT driver (gpmc_probe_dt(), etc) so it now handles setting up the
> > chip selects and timings for NOR devices, e.g.
> > 
> > 		gpmc: gpmc@50000000 {
> > 			status = "okay";
> > 			ranges = <0 0 0x08000000 0x08000000>;	/* CS0: NOR 16M */
> 
> Is that really 16M? Looks more like 128M :-)
> 
> > 			nor@0,0 {
> > 				compatible = "spansion,s29gl064n90t", "cfi-flash";
> > 				reg = <0 0 0>;
> 
> I think that cfi-flash is expecting a length here. Here is
> what I have ...
> 
> +&gpmc {
> +       ranges = <0 0 0x08000000 0x04000000>;
> +
> +       nor@0,0 {
> +               compatible = "cfi-flash";
> +               linux,mtd-name= "intel,ge28f256l18b85";
> +               #address-cells = <1>;
> +               #size-cells = <1>;
> +               reg = <0 0 0x04000000>;
> +               bank-width = <2>;
> +
> +               partition@0 {
> +                       label = "bootloader";
> +                       reg = <0 0x20000>;
> +               };
> +               partition@0x20000 {
> +                       label = "params";
> +                       reg = <0x20000 0x20000>;
> +               };
> +               partition@0x40000 {
> +                       label = "kernel";
> +                       reg = <0x40000 0x200000>;
> +               };
> +               partition@0x240000 {
> +                       label = "file-system";
> +                       reg = <0x240000 0x3dc0000>;
> +               };
> +       };
> 
> > 	nor-flash@08000000 {
> > 		compatible = "spansion,s29gl064n90t", "cfi-flash";
> > 		reg = <0x08000000 0x00800000>;
> > 		bank-width = <2>;
> > 	};
> 
> You don't need this extra entry if you add "simple-bus" to
> the gpmc node compatible string.
> 

That would be nice. However, I wonder what happens if cfi-flash probing
fails: will the gpmc cs request by undone? See below...

> +               gpmc: gpmc@6800a000 {
> +                       compatible = "ti,omap2420-gpmc", "simple-bus";
> +                       ti,hwmods = "gpmc";
> +                       reg = <0x6800a000 0x1000>;
> +                       interrupts = <20>;
> +
> +                       gpmc,num-cs = <8>;
> +                       gpmc,num-waitpins = <4>;
> +                       #address-cells = <2>;
> +                       #size-cells = <1>;
> +               };
> 
> Currently I have the following and this is working for me. 
> Please note that on my board the bootloader is configuring
> the timings for me and so this is missing from the below
> implementation and still needs to be added.
> 
> Cheers
> Jon
> 
> From c0ede833fad704ab452b116154177e3a59166c7e Mon Sep 17 00:00:00 2001
> From: Jon Hunter <jon-hunter@ti.com>
> Date: Fri, 8 Feb 2013 16:46:13 -0600
> Subject: [PATCH] ARM: OMAP2+: Add device-tree support for NOR flash
> 
> ---
>  arch/arm/mach-omap2/gpmc.c |   69 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 69 insertions(+)
> 
> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
> index bc90155..421320b 100644
> --- a/arch/arm/mach-omap2/gpmc.c
> +++ b/arch/arm/mach-omap2/gpmc.c
> @@ -26,6 +26,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/platform_device.h>
>  #include <linux/of.h>
> +#include <linux/of_address.h>
>  #include <linux/of_mtd.h>
>  #include <linux/of_device.h>
>  #include <linux/mtd/nand.h>
> @@ -517,6 +518,26 @@ static int gpmc_cs_delete_mem(int cs)
>  	return r;
>  }
>  
> +static int gpmc_cs_remap(int cs, u32 base)
> +{
> +	int r;
> +	u32 old_base, size;
> +
> +	gpmc_cs_get_memconf(cs, &old_base, &size);
> +	if (base == old_base)
> +		return 0;
> +	gpmc_cs_disable_mem(cs);
> +	r = gpmc_cs_delete_mem(cs);
> +	if (IS_ERR_VALUE(r))
> +		return r;
> +	r = gpmc_cs_insert_mem(cs, base, size);
> +	if (IS_ERR_VALUE(r))
> +		return r;
> +	gpmc_cs_enable_mem(cs, base, size);
> +
> +	return 0;
> +}
> +
>  int gpmc_cs_request(int cs, unsigned long size, unsigned long *base)
>  {
>  	struct resource *res = &gpmc_cs_mem[cs];
> @@ -1305,6 +1326,45 @@ static int gpmc_probe_onenand_child(struct platform_device *pdev,
>  }
>  #endif
>  
> +static int gpmc_probe_nor_child(struct platform_device *pdev,
> +				struct device_node *child)
> +{
> +	struct resource res;
> +	unsigned long base;
> +	int rc, cs;
> +
> +	if (of_property_read_u32(child, "reg", &cs) < 0) {
> +		dev_err(&pdev->dev, "%s has no 'reg' property\n",
> +			child->full_name);
> +		return -ENODEV;
> +	}
> +
> +	if (of_address_to_resource(child, 0, &res)) {
> +		dev_err(&pdev->dev, "%s has malformed 'reg' property\n",
> +			child->full_name);
> +		return -ENODEV;
> +	}
> +
> +	rc = gpmc_cs_request(cs, res.end - res.start, &base);
> +	if (rc < 0)
> +		pr_err("%s: cannot request GPMC CS %d\n", __func__, cs);

gpmc_cs_request will request a memory region from the resource allocator
using allocate_resource().
How will this be released in case cfi-flash probe fails?

IMHO, this works fine currently for NAND and OneNAND because we register
the platform device explicitly and, thus, know if that fails.

> +
> +	/*
> +	 * gpmc_cs_request() will map the CS to an arbitary location
> +	 * in the gpmc address space. When booting with device-tree we
> +	 * want the NOR flash to be mapped to the location specified
> +	 * in the device-tree blob. So remap the CS to this location.
> +	 */
> +	rc = gpmc_cs_remap(cs, res.start);
> +	if (rc < 0) {
> +		pr_err("%s: cannot remap GPMC CS %d to 0x%x\n",
> +		       __func__, cs, res.start);
> +		gpmc_cs_free(cs);
> +	}
> +
> +	return rc;
> +}
> +
>  static int gpmc_probe_dt(struct platform_device *pdev)
>  {
>  	int ret;
> @@ -1330,6 +1390,15 @@ static int gpmc_probe_dt(struct platform_device *pdev)
>  			return ret;
>  		}
>  	}
> +
> +	for_each_node_by_name(child, "nor") {
> +		ret = gpmc_probe_nor_child(pdev, child);
> +		if (ret < 0) {
> +			of_node_put(child);
> +			return ret;
> +		}
> +	}
> +
>  	return 0;
>  }
>  #else
> -- 


Thanks,

-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: DT GPMC SRAM and NOR flash support ?

[Jon Hunter]

On 02/14/2013 04:18 AM, Ezequiel Garcia wrote:
> Hi Jon,
> 
> I'm working on a similar memory controller for plat-orion.
> I have a few questions about your approach.
> 
> On Wed, Feb 13, 2013 at 03:07:06PM -0600, Jon Hunter wrote:
>>
>> On 02/07/2013 03:51 AM, Mark Jackson wrote:
>>> Okay ... I have made some progress, but it's not ideal.
>>>
>>> Currently I've hacked the GPMC DT driver (gpmc_probe_dt(), etc) so it now handles setting up the
>>> chip selects and timings for NOR devices, e.g.
>>>
>>> 		gpmc: gpmc@50000000 {
>>> 			status = "okay";
>>> 			ranges = <0 0 0x08000000 0x08000000>;	/* CS0: NOR 16M */
>>
>> Is that really 16M? Looks more like 128M :-)
>>
>>> 			nor@0,0 {
>>> 				compatible = "spansion,s29gl064n90t", "cfi-flash";
>>> 				reg = <0 0 0>;
>>
>> I think that cfi-flash is expecting a length here. Here is
>> what I have ...
>>
>> +&gpmc {
>> +       ranges = <0 0 0x08000000 0x04000000>;
>> +
>> +       nor@0,0 {
>> +               compatible = "cfi-flash";
>> +               linux,mtd-name= "intel,ge28f256l18b85";
>> +               #address-cells = <1>;
>> +               #size-cells = <1>;
>> +               reg = <0 0 0x04000000>;
>> +               bank-width = <2>;
>> +
>> +               partition@0 {
>> +                       label = "bootloader";
>> +                       reg = <0 0x20000>;
>> +               };
>> +               partition@0x20000 {
>> +                       label = "params";
>> +                       reg = <0x20000 0x20000>;
>> +               };
>> +               partition@0x40000 {
>> +                       label = "kernel";
>> +                       reg = <0x40000 0x200000>;
>> +               };
>> +               partition@0x240000 {
>> +                       label = "file-system";
>> +                       reg = <0x240000 0x3dc0000>;
>> +               };
>> +       };
>>
>>> 	nor-flash@08000000 {
>>> 		compatible = "spansion,s29gl064n90t", "cfi-flash";
>>> 		reg = <0x08000000 0x00800000>;
>>> 		bank-width = <2>;
>>> 	};
>>
>> You don't need this extra entry if you add "simple-bus" to
>> the gpmc node compatible string.
>>
> 
> That would be nice. However, I wonder what happens if cfi-flash probing
> fails: will the gpmc cs request by undone? See below...
> 
>> +               gpmc: gpmc@6800a000 {
>> +                       compatible = "ti,omap2420-gpmc", "simple-bus";
>> +                       ti,hwmods = "gpmc";
>> +                       reg = <0x6800a000 0x1000>;
>> +                       interrupts = <20>;
>> +
>> +                       gpmc,num-cs = <8>;
>> +                       gpmc,num-waitpins = <4>;
>> +                       #address-cells = <2>;
>> +                       #size-cells = <1>;
>> +               };
>>
>> Currently I have the following and this is working for me. 
>> Please note that on my board the bootloader is configuring
>> the timings for me and so this is missing from the below
>> implementation and still needs to be added.
>>
>> Cheers
>> Jon
>>
>> From c0ede833fad704ab452b116154177e3a59166c7e Mon Sep 17 00:00:00 2001
>> From: Jon Hunter <jon-hunter@ti.com>
>> Date: Fri, 8 Feb 2013 16:46:13 -0600
>> Subject: [PATCH] ARM: OMAP2+: Add device-tree support for NOR flash
>>
>> ---
>>  arch/arm/mach-omap2/gpmc.c |   69 ++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 69 insertions(+)
>>
>> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
>> index bc90155..421320b 100644
>> --- a/arch/arm/mach-omap2/gpmc.c
>> +++ b/arch/arm/mach-omap2/gpmc.c
>> @@ -26,6 +26,7 @@
>>  #include <linux/interrupt.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/of.h>
>> +#include <linux/of_address.h>
>>  #include <linux/of_mtd.h>
>>  #include <linux/of_device.h>
>>  #include <linux/mtd/nand.h>
>> @@ -517,6 +518,26 @@ static int gpmc_cs_delete_mem(int cs)
>>  	return r;
>>  }
>>  
>> +static int gpmc_cs_remap(int cs, u32 base)
>> +{
>> +	int r;
>> +	u32 old_base, size;
>> +
>> +	gpmc_cs_get_memconf(cs, &old_base, &size);
>> +	if (base == old_base)
>> +		return 0;
>> +	gpmc_cs_disable_mem(cs);
>> +	r = gpmc_cs_delete_mem(cs);
>> +	if (IS_ERR_VALUE(r))
>> +		return r;
>> +	r = gpmc_cs_insert_mem(cs, base, size);
>> +	if (IS_ERR_VALUE(r))
>> +		return r;
>> +	gpmc_cs_enable_mem(cs, base, size);
>> +
>> +	return 0;
>> +}
>> +
>>  int gpmc_cs_request(int cs, unsigned long size, unsigned long *base)
>>  {
>>  	struct resource *res = &gpmc_cs_mem[cs];
>> @@ -1305,6 +1326,45 @@ static int gpmc_probe_onenand_child(struct platform_device *pdev,
>>  }
>>  #endif
>>  
>> +static int gpmc_probe_nor_child(struct platform_device *pdev,si
>> +				struct device_node *child)
>> +{
>> +	struct resource res;
>> +	unsigned long base;
>> +	int rc, cs;
>> +
>> +	if (of_property_read_u32(child, "reg", &cs) < 0) {
>> +		dev_err(&pdev->dev, "%s has no 'reg' property\n",
>> +			child->full_name);
>> +		return -ENODEV;
>> +	}
>> +
>> +	if (of_address_to_resource(child, 0, &res)) {
>> +		dev_err(&pdev->dev, "%s has malformed 'reg' property\n",
>> +			child->full_name);
>> +		return -ENODEV;
>> +	}
>> +
>> +	rc = gpmc_cs_request(cs, res.end - res.start, &base);
>> +	if (rc < 0)
>> +		pr_err("%s: cannot request GPMC CS %d\n", __func__, cs);
> 
> gpmc_cs_request will request a memory region from the resource allocator
> using allocate_resource().
> How will this be released in case cfi-flash probe fails?
> 
> IMHO, this works fine currently for NAND and OneNAND because we register
> the platform device explicitly and, thus, know if that fails.

You are correct it would not be freed. However, the nice thing about
this approach is that all the nor info (name, bus-width, etc) is kept in
the dts file and we don't need to keep this in some platform code.

For argument sake, although I agree that if the cfi-flash probe fails
ideally we should free the CS, is this really a problem? This is not a
hot-pluggable bus and so even if we do not free the CS, there is no one
else that would be using it. Ok there is also memory space reserved too
but that should not impact other devices either.

I don't really have strong feelings on this either way, but it does seem
nice to minimise the amount of platform data in the mach-omap2 directory.

Cheers
Jon

Re: DT GPMC SRAM and NOR flash support ?

[Ezequiel Garcia]

Hi Jon,

Yet more questions :-) See below...

On Wed, Feb 13, 2013 at 7:07 PM, Jon Hunter <jon-hunter@ti.com> wrote:
>
>
> You don't need this extra entry if you add "simple-bus" to
> the gpmc node compatible string.
>
> +               gpmc: gpmc@6800a000 {
> +                       compatible = "ti,omap2420-gpmc", "simple-bus";
> +                       ti,hwmods = "gpmc";
> +                       reg = <0x6800a000 0x1000>;
> +                       interrupts = <20>;
> +

This "simple-bus" trick is great and very clean!

I'm working on a new driver, similar to gpmc, called device-bus for Marvell SoC.
One remarkable difference is that I need to *setup* the address space
(sort of allocate the address region) for a child device, before I can
access it.

Using simple-bus is a very clean solution. However I'm facing a strange issue:
the child (physmap driver) probe() is being called *before* the parent
(device-bus driver)
probe(), and so the flash device address space cannot get accessed because
it hasn't been setup yet.

Am I doing something wrong? Or is this the expected behavior?

In the latter case, I won't be able to use a compatible "simple-bus"
at all, right?
-- 
    Ezequiel

Re: DT GPMC SRAM and NOR flash support ?

[Ezequiel Garcia]

On Fri, Mar 1, 2013 at 6:08 PM, Ezequiel Garcia <elezegarcia@gmail.com> wrote:
> Hi Jon,
>
> Yet more questions :-) See below...
>
> On Wed, Feb 13, 2013 at 7:07 PM, Jon Hunter <jon-hunter@ti.com> wrote:
>>
>>
>> You don't need this extra entry if you add "simple-bus" to
>> the gpmc node compatible string.
>>
>> +               gpmc: gpmc@6800a000 {
>> +                       compatible = "ti,omap2420-gpmc", "simple-bus";
>> +                       ti,hwmods = "gpmc";
>> +                       reg = <0x6800a000 0x1000>;
>> +                       interrupts = <20>;
>> +
>
> This "simple-bus" trick is great and very clean!
>
> I'm working on a new driver, similar to gpmc, called device-bus for Marvell SoC.
> One remarkable difference is that I need to *setup* the address space
> (sort of allocate the address region) for a child device, before I can
> access it.
>
> Using simple-bus is a very clean solution. However I'm facing a strange issue:
> the child (physmap driver) probe() is being called *before* the parent
> (device-bus driver)
> probe(), and so the flash device address space cannot get accessed because
> it hasn't been setup yet.
>

Ok, now I think I understand that my problem has nothing to do with simple-bus,
but instead related to the driver probing order.

The physmap driver is just being probed before the device bus driver, and so it
won't work.

Now: is there any clean solution?
-- 
    Ezequiel

Re: DT GPMC SRAM and NOR flash support ?

[Jon Hunter]

On 03/01/2013 03:42 PM, Ezequiel Garcia wrote:
> On Fri, Mar 1, 2013 at 6:08 PM, Ezequiel Garcia <elezegarcia@gmail.com> wrote:
>> Hi Jon,
>>
>> Yet more questions :-) See below...
>>
>> On Wed, Feb 13, 2013 at 7:07 PM, Jon Hunter <jon-hunter@ti.com> wrote:
>>>
>>>
>>> You don't need this extra entry if you add "simple-bus" to
>>> the gpmc node compatible string.
>>>
>>> +               gpmc: gpmc@6800a000 {
>>> +                       compatible = "ti,omap2420-gpmc", "simple-bus";
>>> +                       ti,hwmods = "gpmc";
>>> +                       reg = <0x6800a000 0x1000>;
>>> +                       interrupts = <20>;
>>> +
>>
>> This "simple-bus" trick is great and very clean!
>>
>> I'm working on a new driver, similar to gpmc, called device-bus for Marvell SoC.
>> One remarkable difference is that I need to *setup* the address space
>> (sort of allocate the address region) for a child device, before I can
>> access it.
>>
>> Using simple-bus is a very clean solution. However I'm facing a strange issue:
>> the child (physmap driver) probe() is being called *before* the parent
>> (device-bus driver)
>> probe(), and so the flash device address space cannot get accessed because
>> it hasn't been setup yet.
>>
> 
> Ok, now I think I understand that my problem has nothing to do with simple-bus,
> but instead related to the driver probing order.
> 
> The physmap driver is just being probed before the device bus driver, and so it
> won't work.
> 
> Now: is there any clean solution?

I can't say I understand why if it is a child device. I have just posted
a series this week to enable NOR support for OMAP with DT [1].

Cheers
Jon

[1] http://permalink.gmane.org/gmane.linux.ports.arm.omap/94378