| Message ID | 511137CF.1090706@mimc.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 -- 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
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.
--
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
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. -- 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
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 -- 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
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.
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 -- 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
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.
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 */ }; };