diff mbox

[11/14] ARM: OMAP2+: Add device-tree support for NOR flash

Message ID 1361899842-30303-12-git-send-email-jon-hunter@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Hunter, Jon Feb. 26, 2013, 5:30 p.m. UTC
NOR flash is not currently supported when booting with device-tree
on OMAP2+ devices. Add support to detect and configure NOR devices
when booting with device-tree.

Add documentation for the TI GPMC NOR binding.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 Documentation/devicetree/bindings/mtd/gpmc-nor.txt |   98 ++++++++++++++++
 arch/arm/mach-omap2/gpmc.c                         |  120 ++++++++++++++++++++
 2 files changed, 218 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nor.txt

Comments

Ezequiel Garcia March 1, 2013, 9:25 p.m. UTC | #1
Hi Jon,

On Tue, Feb 26, 2013 at 2:30 PM, Jon Hunter <jon-hunter@ti.com> wrote:
[...]
> +static int gpmc_probe_nor_child(struct platform_device *pdev,
> +                               struct device_node *child)
> +{
> +       struct gpmc_settings gpmc_s;
> +       struct gpmc_timings gpmc_t;
> +       struct resource res;
> +       unsigned long base;
> +       int ret, 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;
> +       }
> +
> +       ret = gpmc_cs_request(cs, res.end - res.start, &base);

How about using resource_size() above?

BTW, I believe it's size = end - start + 1.
Hunter, Jon March 1, 2013, 10:24 p.m. UTC | #2
On 03/01/2013 03:25 PM, Ezequiel Garcia wrote:
> Hi Jon,
> 
> On Tue, Feb 26, 2013 at 2:30 PM, Jon Hunter <jon-hunter@ti.com> wrote:
> [...]
>> +static int gpmc_probe_nor_child(struct platform_device *pdev,
>> +                               struct device_node *child)
>> +{
>> +       struct gpmc_settings gpmc_s;
>> +       struct gpmc_timings gpmc_t;
>> +       struct resource res;
>> +       unsigned long base;
>> +       int ret, 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;
>> +       }
>> +
>> +       ret = gpmc_cs_request(cs, res.end - res.start, &base);
> 
> How about using resource_size() above?
> 
> BTW, I believe it's size = end - start + 1.

Thanks, yes I can update.

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
Ezequiel Garcia March 4, 2013, 11:57 a.m. UTC | #3
Hi Jon,

On Tue, Feb 26, 2013 at 2:30 PM, Jon Hunter <jon-hunter@ti.com> wrote:
> +
> +Example:
> +
> +gpmc: gpmc@6e000000 {
> +       compatible = "ti,omap3430-gpmc", "simple-bus";

I'm concern about using simple-bus, and I'm not entirely sure this will work.

AFAIK, you can't correlate a parent-child relationship in the device tree
to the order in which drivers will be probed,
so it's only a matter of coincidence if this is working for you right now.
The GPMC code is in "arch/arm/mach-omap2", is located *before* the mtd code
in the Makefile, and thus the GPMC driver loads *before* the MTD code.

Morevore, I believe that when we move GPMC from arch/arm/mach-omap2
to drivers/memory where it should be, this 'simple-bus' will stop
working properly.

Of course, I can be wrong, but I think you will have to find some other way
to initialize the GPMC's childs.
Hunter, Jon March 4, 2013, 5:51 p.m. UTC | #4
On 03/04/2013 05:57 AM, Ezequiel Garcia wrote:
> Hi Jon,
> 
> On Tue, Feb 26, 2013 at 2:30 PM, Jon Hunter <jon-hunter@ti.com> wrote:
>> +
>> +Example:
>> +
>> +gpmc: gpmc@6e000000 {
>> +       compatible = "ti,omap3430-gpmc", "simple-bus";
> 
> I'm concern about using simple-bus, and I'm not entirely sure this will work.
> 
> AFAIK, you can't correlate a parent-child relationship in the device tree
> to the order in which drivers will be probed,
> so it's only a matter of coincidence if this is working for you right now.

Ummm, I am not sure I am convinced. Have you looked at
of_platform_bus_create() which creates the devices? It is clearly
creating the child devices after creating the parent device.

> The GPMC code is in "arch/arm/mach-omap2", is located *before* the mtd code
> in the Makefile, and thus the GPMC driver loads *before* the MTD code.

But shouldn't it only matter in the order in which devices are
registered? Unless you have a situation in which the devices are
registered in order (parent then child) but only the driver for the
child is registered before the parent and child devices are registered.
I could see that being a problem.

The only other thing that I can think of is if you happen to be creating
a device both via device-tree and may be you are creating a duplicate
device and calling platform_device_register() somewhere else in your code.

> Morevore, I believe that when we move GPMC from arch/arm/mach-omap2
> to drivers/memory where it should be, this 'simple-bus' will stop
> working properly.

Why? I don't see how the location of files would change this.

> Of course, I can be wrong, but I think you will have to find some other way
> to initialize the GPMC's childs.

Well isn't this how device-tree is meant to work?

It would be great if you could debug your problem and fully understand
why this is not working for you, so we can see if there is a real
problem here. Otherwise this appears to be just guess work.

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
Hunter, Jon March 4, 2013, 6:19 p.m. UTC | #5
On 03/04/2013 11:51 AM, Jon Hunter wrote:
> 
> On 03/04/2013 05:57 AM, Ezequiel Garcia wrote:
>> Hi Jon,
>>
>> On Tue, Feb 26, 2013 at 2:30 PM, Jon Hunter <jon-hunter@ti.com> wrote:
>>> +
>>> +Example:
>>> +
>>> +gpmc: gpmc@6e000000 {
>>> +       compatible = "ti,omap3430-gpmc", "simple-bus";
>>
>> I'm concern about using simple-bus, and I'm not entirely sure this will work.
>>
>> AFAIK, you can't correlate a parent-child relationship in the device tree
>> to the order in which drivers will be probed,
>> so it's only a matter of coincidence if this is working for you right now.
> 
> Ummm, I am not sure I am convinced. Have you looked at
> of_platform_bus_create() which creates the devices? It is clearly
> creating the child devices after creating the parent device.

Sorry you said order by which drivers are probed. Yes I would agree that
you cannot control the order drivers are probed, but just the order
devices are registered.

However, I don't see this as being a device-tree specific issue. Even
when not using device-tree you would need to ensure that probing the
parent happens before the child for the GPMC.

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
Mark Jackson March 5, 2013, 2:34 p.m. UTC | #6
On 26/02/13 17:30, Jon Hunter wrote:
> NOR flash is not currently supported when booting with device-tree
> on OMAP2+ devices. Add support to detect and configure NOR devices
> when booting with device-tree.
> 
> Add documentation for the TI GPMC NOR binding.
> 
> Signed-off-by: Jon Hunter <jon-hunter@ti.com>

I'm trying to test this series, and am unable get my NOR device recognised.

If I remove all reference to NOR (and keep only my NAND device definition), then
everything seems to boot fine (so I'm assuming I've got at least the basics of
the patch set working).

My GPMC tree looks like this:-

gpmc: gpmc@50000000 {
	compatible = "ti,am3352-gpmc", "simple-bus";
	ti,hwmods = "gpmc";
	status = "okay";
	gpmc,num-waitpins = <2>;
	pinctrl-names = "default";
	pinctrl-0 = <&gpmc_pins>;

	#address-cells = <2>;
	#size-cells = <1>;
	ranges = <0 0 0x08000000 0x10000000>,	/* CS0: NAND 256M */
		 <3 0 0x1a000000 0x04000000>;	/* CS3: NOR 64M */

	nand@0,0 {
		linux,mtd-name= "micron,mt29f2g08abaea";
		reg = <0 0x00000000 0x10000000>; /* CS0, offset 0 */
		nand-bus-width = <8>;
		ti,nand-ecc-opt = "bch8";

		gpmc,device-nand;
		gpmc,device-width = <1>;
		gpmc,wait-pin = <0>;

		gpmc,sync-clk = <0>;
		gpmc,cs-on = <0>;
		gpmc,cs-rd-off = <44>;
		gpmc,cs-wr-off = <44>;
		gpmc,adv-on = <6>;
		gpmc,adv-rd-off = <34>;
		gpmc,adv-wr-off = <44>;
		gpmc,we-off = <40>;
		gpmc,oe-off = <54>;
		gpmc,access = <64>;
		gpmc,rd-cycle = <82>;
		gpmc,wr-cycle = <82>;
		gpmc,wr-access = <40>;
		gpmc,wr-data-mux-bus = <0>;

		#address-cells = <1>;
		#size-cells = <1>;
		elm_id = <&elm>;

		/*
		MTD partition table
		===================
		+------------+-->0x00000000-> SPL start         (SPL copy on 1st block)
		|            |
		|            |-->0x0001FFFF-> SPL end
		|            |-->0x00020000-> SPL.backup1 start (SPL copy on 2nd block)
		|            |
		|            |-->0x0003FFFF-> SPL.backup1 end
		|            |-->0x00040000-> SPL.backup2 start (SPL copy on 3rd block)
		|            |
		|            |-->0x0005FFFF-> SPL.backup2 end
		|            |-->0x00060000-> SPL.backup3 start (SPL copy on 4th block)
		|            |
		|            |-->0x0007FFFF-> SPL.backup3 end
		|            |-->0x00080000-> U-Boot start
		|            |                                    
		|            |-->0x001DFFFF-> U-Boot end
		|            |-->0x001E0000-> ENV start
		|            |
		|            |-->0x001FFFFF-> ENV end
		|            |-->0x00200000-> File system start
		|            |
		|            |-->0x041FFFFF-> File system end
		|            |-->0x04200000-> Data storage start
		|            |
		+------------+-->0x10000000-> NAND end (Free end)
		*/
		partition@0 {
			label = "spl1";
			reg = <0x00000000 0x00020000>; /* 128KB */
		};

		partition@1 {
			label = "spl2";
			reg = <0x00020000 0x00020000>; /* 128KB */
		};

		partition@2 {
			label = "spl3";
			reg = <0x00040000 0x00020000>; /* 128KB */
		};

		partition@3 {
			label = "spl4";
			reg = <0x00060000 0x00020000>; /* 128KB */
		};

		partition@4 {
			label = "boot";
			reg = <0x00080000 0x00160000>; /* 1408KB */
		};

		partition@5 {
			label = "env";
			reg = <0x001e0000 0x00020000>; /* 128KB */
		};

		partition@6 {
			label = "rootfs";
			reg = <0x00200000 0x04000000>; /* 64MB */
		};

		partition@7 {
			label = "data";
			reg = <0x04200000 0x0be00000>; /* 190MB */
		};
	};

	nor@1,0 {
		reg = <3 0x00000000 0x04000000>;
		compatible = "cfi-flash";
		linux,mtd-name = "spansion,s29gl064n90t";
		bank-width = <2>;

		gpmc,device-width = <1>;
		gpmc,mux-add-data;

		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>;

		#address-cells = <1>;
		#size-cells = <1>;
		elm_id = <&elm>;

		partition@0 {
			label = "data";
			reg = <0x00000000 0x04000000>; /* 64MB */
		};
	};
};

Booting with this NOR device produces the following oops:-

[    0.000000] Booting Linux on physical CPU 0x0
[    0.000000] Linux version 3.9.0-rc1-12191-ga00d6d1-dirty (mpfj@mpfj-nanobone) (gcc version 4.5.4 (Buildroot 2012.11) ) #33 Tue Mar 5 13:08:25 GMT 2013
[    0.000000] CPU: ARMv7 Processor [413fc082] revision 2 (ARMv7), cr=10c53c7d
[    0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache
[    0.000000] Machine: Generic AM33XX (Flattened Device Tree), model: TI AM335x BeagleBone
[    0.000000] Memory policy: ECC disabled, Data cache writeback
[    0.000000] CPU: All CPU(s) started in SVC mode.
[    0.000000] AM335X ES1.0 (neon )
[    0.000000] Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 64768
[    0.000000] Kernel command line: console=ttyO0,115200n8 noinitrd ip=off mem=256M rootwait=1 ubi.mtd=6,2048 rootfstype=ubifs root=ubi0:rootfs
[    0.000000] PID hash table entries: 1024 (order: 0, 4096 bytes)
[    0.000000] Dentry cache hash table entries: 32768 (order: 5, 131072 bytes)
[    0.000000] Inode-cache hash table entries: 16384 (order: 4, 65536 bytes)
[    0.000000] __ex_table already sorted, skipping sort
[    0.000000] Memory: 255MB = 255MB total
[    0.000000] Memory: 248068k/248068k available, 14076k reserved, 0K highmem
[    0.000000] Virtual kernel memory layout:
[    0.000000]     vector  : 0xffff0000 - 0xffff1000   (   4 kB)
[    0.000000]     fixmap  : 0xfff00000 - 0xfffe0000   ( 896 kB)
[    0.000000]     vmalloc : 0xd0800000 - 0xff000000   ( 744 MB)
[    0.000000]     lowmem  : 0xc0000000 - 0xd0000000   ( 256 MB)
[    0.000000]     pkmap   : 0xbfe00000 - 0xc0000000   (   2 MB)
[    0.000000]       .text : 0xc0008000 - 0xc04d407c   (4913 kB)
[    0.000000]       .init : 0xc04d5000 - 0xc050593c   ( 195 kB)
[    0.000000]       .data : 0xc0506000 - 0xc0554e80   ( 316 kB)
[    0.000000]        .bss : 0xc0554e80 - 0xc0a76b60   (5256 kB)
[    0.000000] NR_IRQS:16 nr_irqs:16 16
[    0.000000] IRQ: Found an INTC at 0xfa200000 (revision 5.0) with 128 interrupts
[    0.000000] Total of 128 interrupts on 1 active controller
[    0.000000] OMAP clockevent source: GPTIMER1 at 26000000 Hz
[    0.000000] sched_clock: 32 bits at 26MHz, resolution 38ns, wraps every 165191ms
[    0.000000] OMAP clocksource: GPTIMER2 at 26000000 Hz
[    0.000000] Console: colour dummy device 80x30
[    0.000000] Lock dependency validator: Copyright (c) 2006 Red Hat, Inc., Ingo Molnar
[    0.000000] ... MAX_LOCKDEP_SUBCLASSES:  8
[    0.000000] ... MAX_LOCK_DEPTH:          48
[    0.000000] ... MAX_LOCKDEP_KEYS:        8191
[    0.000000] ... CLASSHASH_SIZE:          4096
[    0.000000] ... MAX_LOCKDEP_ENTRIES:     16384
[    0.000000] ... MAX_LOCKDEP_CHAINS:      32768
[    0.000000] ... CHAINHASH_SIZE:          16384
[    0.000000]  memory used by lock dependency info: 3695 kB
[    0.000000]  per task-struct memory footprint: 1152 bytes
[    0.000845] Calibrating delay loop... 479.23 BogoMIPS (lpj=2396160)
[    0.109860] pid_max: default: 32768 minimum: 301
[    0.110128] Security Framework initialized
[    0.110234] Mount-cache hash table entries: 512
[    0.120373] CPU: Testing write buffer coherency: ok
[    0.121628] Setting up static identity map for 0xc03caf60 - 0xc03cafb8
[    0.125001] devtmpfs: initialized
[    0.186744] pinctrl core: initialized pinctrl subsystem
[    0.191942] regulator-dummy: no parameters
[    0.194142] NET: Registered protocol family 16
[    0.194891] DMA: preallocated 256 KiB pool for atomic coherent allocations
[    0.213783] OMAP GPIO hardware version 0.1
[    0.236730] omap-gpmc 50000000.gpmc: could not find pctldev for node /pinmux@44e10800/gpmc_pins, deferring probe
[    0.236781] platform 50000000.gpmc: Driver omap-gpmc requests probe deferral
[    0.237476] No ATAGs?
[    0.237494] hw-breakpoint: debug architecture 0x4 unsupported.
[    0.269694] bio: create slab <bio-0> at 0
[    0.335502] omap-dma-engine omap-dma-engine: OMAP DMA engine driver
[    0.342078] usbcore: registered new interface driver usbfs
[    0.342491] usbcore: registered new interface driver hub
[    0.343136] usbcore: registered new device driver usb
[    0.344082] omap_i2c 44e0b000.i2c: could not find pctldev for node /pinmux@44e10800/i2c1_pins, deferring probe
[    0.344128] platform 44e0b000.i2c: Driver omap_i2c requests probe deferral
[    0.351421] cfg80211: Calling CRDA to update world regulatory domain
[    0.353089] Switching to clocksource gp_timer
[    0.401911] NET: Registered protocol family 2
[    0.403612] TCP established hash table entries: 2048 (order: 2, 16384 bytes)
[    0.403822] TCP bind hash table entries: 2048 (order: 4, 73728 bytes)
[    0.404761] TCP: Hash tables configured (established 2048 bind 2048)
[    0.404941] TCP: reno registered
[    0.404969] UDP hash table entries: 256 (order: 2, 20480 bytes)
[    0.405226] UDP-Lite hash table entries: 256 (order: 2, 20480 bytes)
[    0.406010] NET: Registered protocol family 1
[    0.406917] NetWinder Floating Point Emulator V0.97 (double precision)
[    0.407357] CPU PMU: probing PMU on CPU 0
[    0.407384] hw perfevents: enabled with ARMv7 Cortex-A8 PMU driver, 5 counters available
[    0.418794] msgmni has been set to 484
[    0.423083] io scheduler noop registered
[    0.423107] io scheduler deadline registered
[    0.423300] io scheduler cfq registered (default)
[    0.424353] pinctrl-single 44e10800.pinmux: 142 pins at pa f9e10800 size 568
[    0.425948] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
[    0.433116] 44e09000.serial: ttyO0 at MMIO 0x44e09000 (irq = 88) is a OMAP UART0
[    0.954564] console [ttyO0] enabled
[    0.960651] 48022000.serial: ttyO1 at MMIO 0x48022000 (irq = 89) is a OMAP UART1
[    0.970290] 48024000.serial: ttyO2 at MMIO 0x48024000 (irq = 90) is a OMAP UART2
[    0.979840] 481a6000.serial: ttyO3 at MMIO 0x481a6000 (irq = 60) is a OMAP UART3
[    0.989314] 481a8000.serial: ttyO4 at MMIO 0x481a8000 (irq = 61) is a OMAP UART4
[    0.998713] 481aa000.serial: ttyO5 at MMIO 0x481aa000 (irq = 62) is a OMAP UART5
[    1.033299] brd: module loaded
[    1.052135] loop: module loaded
[    1.060341] Unhandled fault: external abort on non-linefetch (0x1028) at 0xd3000020
[    1.068463] Internal error: : 1028 [#1] ARM
[    1.072895] CPU: 0    Not tainted  (3.9.0-rc1-12191-ga00d6d1-dirty #33)
[    1.079903] PC is at cfi_qry_present+0x294/0x2c0
[    1.084785] LR is at cfi_qry_present+0x60/0x2c0
[    1.089576] pc : [<c0230c94>]    lr : [<c0230a60>]    psr: 60000113
[    1.089576] sp : cf04dd28  ip : 00000006  fp : 00000001
[    1.101716] r10: 00005151  r9 : 00000000  r8 : 00005959
[    1.107238] r7 : 00000002  r6 : 00000002  r5 : 00000008  r4 : cf28d95c
[    1.114140] r3 : 00005252  r2 : d3000020  r1 : 00000020  r0 : d3000000
[    1.121044] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
[    1.128772] Control: 10c5387d  Table: 80004019  DAC: 00000015
[    1.134849] Process swapper (pid: 1, stack limit = 0xcf04c238)
[    1.141018] Stack: (0xcf04dd28 to 0xcf04e000)
[    1.145627] dd20:                   00000000 cf28d95c cf04ddc4 00000000 c050e0a8 00000002
[    1.154277] dd40: 000000aa 00000008 00009898 c0230e38 cf04c000 00000001 cf04c000 00000000
[    1.162928] dd60: 00000000 cf04ddc4 cf28d95c 00000002 cf0bcc00 00000001 c04920a8 c022fe80
[    1.171576] dd80: cf04c000 cf04b7b0 cf04b340 00000000 00000001 000000d0 c06223ac 00000001
[    1.180228] dda0: cf28d95c c05440a0 cf04ddc4 00000002 cf0bcc00 00000001 c04920a8 c023a97c
[    1.188880] ddc0: cf04c000 00000000 00000000 00000002 00000001 00000000 00000000 00000000
[    1.197530] dde0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    1.206182] de00: c054405c c0544094 c0544084 c054407c c04920a8 cf28d95c cf0bcc00 00000000
[    1.214833] de20: c04920a8 c022fde8 cf28d950 00000000 c0c81f60 cf28d950 04000000 c023b180
[    1.223483] de40: 00000000 22222222 cf0bcc10 cf294500 00000000 cf04de80 1a000000 1dffffff
[    1.232134] de60: c0c81f9c 00000200 00000000 00000000 00000000 c005a7ac 00000000 c00c3c50
[    1.240784] de80: c0a79aa4 00000001 c0a716b8 cf0bcc10 c0a716b8 c0a716c8 00000000 00000000
[    1.249434] dea0: c0505608 c0544210 00000000 c0213bb0 c0213b98 c0212b08 00000000 cf0bcc10
[    1.258085] dec0: c0544210 cf0bcc44 00000000 c04f16e4 0000005a c0212d30 c0544210 c0212c9c
[    1.266735] dee0: cf04dee8 c02112a8 cf0444a8 cf0af590 c04f16e4 c0544210 c0542430 cf28d9c0
[    1.275386] df00: 00000000 c0211b00 c04928b0 c0544210 c0544210 00000006 c04fcff4 00000000
[    1.284037] df20: c04f16e4 c02132fc 00000000 c04fd014 00000006 c04fcff4 00000000 c04f16e4
[    1.292688] df40: 0000005a c04d581c 00000006 00000006 60000113 c04fd010 c04fd014 00000006
[    1.301341] df60: c04fcff4 c0554e80 c04d5168 0000005a 00000000 c04d59d8 00000006 00000006
[    1.309990] df80: c04d5168 00000000 00000000 c03c6494 00000000 00000000 00000000 00000000
[    1.318640] dfa0: 00000000 c03c649c 00000000 c0013870 00000000 00000000 00000000 00000000
[    1.327291] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    1.335943] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000 ff99f7ff f7b97f3f
[    1.344601] [<c0230c94>] (cfi_qry_present+0x294/0x2c0) from [<c0230e38>] (cfi_qry_mode_on+0x178/0xfd8)
[    1.354449] [<c0230e38>] (cfi_qry_mode_on+0x178/0xfd8) from [<c022fe80>] (cfi_probe_chip+0x40/0xbc0)
[    1.364116] [<c022fe80>] (cfi_probe_chip+0x40/0xbc0) from [<c023a97c>] (mtd_do_chip_probe+0xa8/0x388)
[    1.373873] [<c023a97c>] (mtd_do_chip_probe+0xa8/0x388) from [<c022fde8>] (do_map_probe+0x6c/0x8c)
[    1.383354] [<c022fde8>] (do_map_probe+0x6c/0x8c) from [<c023b180>] (of_flash_probe+0x218/0x5a4)
[    1.392658] [<c023b180>] (of_flash_probe+0x218/0x5a4) from [<c0213bb0>] (platform_drv_probe+0x18/0x1c)
[    1.402506] [<c0213bb0>] (platform_drv_probe+0x18/0x1c) from [<c0212b08>] (driver_probe_device+0x94/0x228)
[    1.412720] [<c0212b08>] (driver_probe_device+0x94/0x228) from [<c0212d30>] (__driver_attach+0x94/0x98)
[    1.422671] [<c0212d30>] (__driver_attach+0x94/0x98) from [<c02112a8>] (bus_for_each_dev+0x68/0x8c)
[    1.432244] [<c02112a8>] (bus_for_each_dev+0x68/0x8c) from [<c0211b00>] (bus_add_driver+0x1b0/0x22c)
[    1.441908] [<c0211b00>] (bus_add_driver+0x1b0/0x22c) from [<c02132fc>] (driver_register+0x78/0x144)
[    1.451580] [<c02132fc>] (driver_register+0x78/0x144) from [<c04d581c>] (do_one_initcall+0x98/0x16c)
[    1.461245] [<c04d581c>] (do_one_initcall+0x98/0x16c) from [<c04d59d8>] (kernel_init_freeable+0xe8/0x1ac)
[    1.471369] [<c04d59d8>] (kernel_init_freeable+0xe8/0x1ac) from [<c03c649c>] (kernel_init+0x8/0xe4)
[    1.480956] [<c03c649c>] (kernel_init+0x8/0xe4) from [<c0013870>] (ret_from_fork+0x14/0x24)
[    1.489792] Code: e7d05001 e6ef5075 eaffffcb e0802002 (e1d2c0b0)
[    1.496294] ---[ end trace a9ca4747ade8eff8 ]---
[    1.501297] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
[    1.501297]


Can anyone point out what I've got wrong ?
--
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
Hunter, Jon March 5, 2013, 2:46 p.m. UTC | #7
On 03/05/2013 08:34 AM, Mark Jackson wrote:
> On 26/02/13 17:30, Jon Hunter wrote:
>> NOR flash is not currently supported when booting with device-tree
>> on OMAP2+ devices. Add support to detect and configure NOR devices
>> when booting with device-tree.
>>
>> Add documentation for the TI GPMC NOR binding.
>>
>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
> 
> I'm trying to test this series, and am unable get my NOR device recognised.
> 
> If I remove all reference to NOR (and keep only my NAND device definition), then
> everything seems to boot fine (so I'm assuming I've got at least the basics of
> the patch set working).
> 
> My GPMC tree looks like this:-
> 
> gpmc: gpmc@50000000 {
> 	compatible = "ti,am3352-gpmc", "simple-bus";
> 	ti,hwmods = "gpmc";
> 	status = "okay";
> 	gpmc,num-waitpins = <2>;
> 	pinctrl-names = "default";
> 	pinctrl-0 = <&gpmc_pins>;
> 
> 	#address-cells = <2>;
> 	#size-cells = <1>;
> 	ranges = <0 0 0x08000000 0x10000000>,	/* CS0: NAND 256M */
> 		 <3 0 0x1a000000 0x04000000>;	/* CS3: NOR 64M */
> 
> 	nand@0,0 {
> 		linux,mtd-name= "micron,mt29f2g08abaea";
> 		reg = <0 0x00000000 0x10000000>; /* CS0, offset 0 */
> 		nand-bus-width = <8>;
> 		ti,nand-ecc-opt = "bch8";
> 
> 		gpmc,device-nand;
> 		gpmc,device-width = <1>;
> 		gpmc,wait-pin = <0>;
> 
> 		gpmc,sync-clk = <0>;
> 		gpmc,cs-on = <0>;
> 		gpmc,cs-rd-off = <44>;
> 		gpmc,cs-wr-off = <44>;
> 		gpmc,adv-on = <6>;
> 		gpmc,adv-rd-off = <34>;
> 		gpmc,adv-wr-off = <44>;
> 		gpmc,we-off = <40>;
> 		gpmc,oe-off = <54>;
> 		gpmc,access = <64>;
> 		gpmc,rd-cycle = <82>;
> 		gpmc,wr-cycle = <82>;
> 		gpmc,wr-access = <40>;
> 		gpmc,wr-data-mux-bus = <0>;
> 
> 		#address-cells = <1>;
> 		#size-cells = <1>;
> 		elm_id = <&elm>;
> 
> 		/*
> 		MTD partition table
> 		===================
> 		+------------+-->0x00000000-> SPL start         (SPL copy on 1st block)
> 		|            |
> 		|            |-->0x0001FFFF-> SPL end
> 		|            |-->0x00020000-> SPL.backup1 start (SPL copy on 2nd block)
> 		|            |
> 		|            |-->0x0003FFFF-> SPL.backup1 end
> 		|            |-->0x00040000-> SPL.backup2 start (SPL copy on 3rd block)
> 		|            |
> 		|            |-->0x0005FFFF-> SPL.backup2 end
> 		|            |-->0x00060000-> SPL.backup3 start (SPL copy on 4th block)
> 		|            |
> 		|            |-->0x0007FFFF-> SPL.backup3 end
> 		|            |-->0x00080000-> U-Boot start
> 		|            |                                    
> 		|            |-->0x001DFFFF-> U-Boot end
> 		|            |-->0x001E0000-> ENV start
> 		|            |
> 		|            |-->0x001FFFFF-> ENV end
> 		|            |-->0x00200000-> File system start
> 		|            |
> 		|            |-->0x041FFFFF-> File system end
> 		|            |-->0x04200000-> Data storage start
> 		|            |
> 		+------------+-->0x10000000-> NAND end (Free end)
> 		*/
> 		partition@0 {
> 			label = "spl1";
> 			reg = <0x00000000 0x00020000>; /* 128KB */
> 		};
> 
> 		partition@1 {
> 			label = "spl2";
> 			reg = <0x00020000 0x00020000>; /* 128KB */
> 		};
> 
> 		partition@2 {
> 			label = "spl3";
> 			reg = <0x00040000 0x00020000>; /* 128KB */
> 		};
> 
> 		partition@3 {
> 			label = "spl4";
> 			reg = <0x00060000 0x00020000>; /* 128KB */
> 		};
> 
> 		partition@4 {
> 			label = "boot";
> 			reg = <0x00080000 0x00160000>; /* 1408KB */
> 		};
> 
> 		partition@5 {
> 			label = "env";
> 			reg = <0x001e0000 0x00020000>; /* 128KB */
> 		};
> 
> 		partition@6 {
> 			label = "rootfs";
> 			reg = <0x00200000 0x04000000>; /* 64MB */
> 		};
> 
> 		partition@7 {
> 			label = "data";
> 			reg = <0x04200000 0x0be00000>; /* 190MB */
> 		};
> 	};
> 
> 	nor@1,0 {
> 		reg = <3 0x00000000 0x04000000>;
> 		compatible = "cfi-flash";
> 		linux,mtd-name = "spansion,s29gl064n90t";
> 		bank-width = <2>;
> 
> 		gpmc,device-width = <1>;

Only bank-width should be necessary for NOR (per the binding
documentation). However, if you do specify both, then they should match.
Do you have two 8-bits devices? If so may be I need to update the
documentation to make it clear this is the total width of all devices
for a given chip-select.

> 		gpmc,mux-add-data;
> 
> 		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>;
> 
> 		#address-cells = <1>;
> 		#size-cells = <1>;
> 		elm_id = <&elm>;
> 
> 		partition@0 {
> 			label = "data";
> 			reg = <0x00000000 0x04000000>; /* 64MB */
> 		};
> 	};
> };
> 
> Booting with this NOR device produces the following oops:-
> 
> [    0.000000] Booting Linux on physical CPU 0x0
> [    0.000000] Linux version 3.9.0-rc1-12191-ga00d6d1-dirty (mpfj@mpfj-nanobone) (gcc version 4.5.4 (Buildroot 2012.11) ) #33 Tue Mar 5 13:08:25 GMT 2013
> [    0.000000] CPU: ARMv7 Processor [413fc082] revision 2 (ARMv7), cr=10c53c7d
> [    0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache
> [    0.000000] Machine: Generic AM33XX (Flattened Device Tree), model: TI AM335x BeagleBone
> [    0.000000] Memory policy: ECC disabled, Data cache writeback
> [    0.000000] CPU: All CPU(s) started in SVC mode.
> [    0.000000] AM335X ES1.0 (neon )
> [    0.000000] Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 64768
> [    0.000000] Kernel command line: console=ttyO0,115200n8 noinitrd ip=off mem=256M rootwait=1 ubi.mtd=6,2048 rootfstype=ubifs root=ubi0:rootfs
> [    0.000000] PID hash table entries: 1024 (order: 0, 4096 bytes)
> [    0.000000] Dentry cache hash table entries: 32768 (order: 5, 131072 bytes)
> [    0.000000] Inode-cache hash table entries: 16384 (order: 4, 65536 bytes)
> [    0.000000] __ex_table already sorted, skipping sort
> [    0.000000] Memory: 255MB = 255MB total
> [    0.000000] Memory: 248068k/248068k available, 14076k reserved, 0K highmem
> [    0.000000] Virtual kernel memory layout:
> [    0.000000]     vector  : 0xffff0000 - 0xffff1000   (   4 kB)
> [    0.000000]     fixmap  : 0xfff00000 - 0xfffe0000   ( 896 kB)
> [    0.000000]     vmalloc : 0xd0800000 - 0xff000000   ( 744 MB)
> [    0.000000]     lowmem  : 0xc0000000 - 0xd0000000   ( 256 MB)
> [    0.000000]     pkmap   : 0xbfe00000 - 0xc0000000   (   2 MB)
> [    0.000000]       .text : 0xc0008000 - 0xc04d407c   (4913 kB)
> [    0.000000]       .init : 0xc04d5000 - 0xc050593c   ( 195 kB)
> [    0.000000]       .data : 0xc0506000 - 0xc0554e80   ( 316 kB)
> [    0.000000]        .bss : 0xc0554e80 - 0xc0a76b60   (5256 kB)
> [    0.000000] NR_IRQS:16 nr_irqs:16 16
> [    0.000000] IRQ: Found an INTC at 0xfa200000 (revision 5.0) with 128 interrupts
> [    0.000000] Total of 128 interrupts on 1 active controller
> [    0.000000] OMAP clockevent source: GPTIMER1 at 26000000 Hz
> [    0.000000] sched_clock: 32 bits at 26MHz, resolution 38ns, wraps every 165191ms
> [    0.000000] OMAP clocksource: GPTIMER2 at 26000000 Hz
> [    0.000000] Console: colour dummy device 80x30
> [    0.000000] Lock dependency validator: Copyright (c) 2006 Red Hat, Inc., Ingo Molnar
> [    0.000000] ... MAX_LOCKDEP_SUBCLASSES:  8
> [    0.000000] ... MAX_LOCK_DEPTH:          48
> [    0.000000] ... MAX_LOCKDEP_KEYS:        8191
> [    0.000000] ... CLASSHASH_SIZE:          4096
> [    0.000000] ... MAX_LOCKDEP_ENTRIES:     16384
> [    0.000000] ... MAX_LOCKDEP_CHAINS:      32768
> [    0.000000] ... CHAINHASH_SIZE:          16384
> [    0.000000]  memory used by lock dependency info: 3695 kB
> [    0.000000]  per task-struct memory footprint: 1152 bytes
> [    0.000845] Calibrating delay loop... 479.23 BogoMIPS (lpj=2396160)
> [    0.109860] pid_max: default: 32768 minimum: 301
> [    0.110128] Security Framework initialized
> [    0.110234] Mount-cache hash table entries: 512
> [    0.120373] CPU: Testing write buffer coherency: ok
> [    0.121628] Setting up static identity map for 0xc03caf60 - 0xc03cafb8
> [    0.125001] devtmpfs: initialized
> [    0.186744] pinctrl core: initialized pinctrl subsystem
> [    0.191942] regulator-dummy: no parameters
> [    0.194142] NET: Registered protocol family 16
> [    0.194891] DMA: preallocated 256 KiB pool for atomic coherent allocations
> [    0.213783] OMAP GPIO hardware version 0.1
> [    0.236730] omap-gpmc 50000000.gpmc: could not find pctldev for node /pinmux@44e10800/gpmc_pins, deferring probe
> [    0.236781] platform 50000000.gpmc: Driver omap-gpmc requests probe deferral

This look like your problem. I would figure out why this is failing and
try again.

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
Mark Jackson March 5, 2013, 4:20 p.m. UTC | #8
On 05/03/13 14:46, Jon Hunter wrote:
> 
> On 03/05/2013 08:34 AM, Mark Jackson wrote:
>> On 26/02/13 17:30, Jon Hunter wrote:
>>> NOR flash is not currently supported when booting with device-tree
>>> on OMAP2+ devices. Add support to detect and configure NOR devices
>>> when booting with device-tree.
>>>
>>> Add documentation for the TI GPMC NOR binding.
>>>
>>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
>>
>> I'm trying to test this series, and am unable get my NOR device recognised.
>>
>> If I remove all reference to NOR (and keep only my NAND device definition), then
>> everything seems to boot fine (so I'm assuming I've got at least the basics of
>> the patch set working).
>>
>> My GPMC tree looks like this:-

<snip>

>> 	nor@1,0 {
>> 		reg = <3 0x00000000 0x04000000>;
>> 		compatible = "cfi-flash";
>> 		linux,mtd-name = "spansion,s29gl064n90t";
>> 		bank-width = <2>;
>>
>> 		gpmc,device-width = <1>;
> 
> Only bank-width should be necessary for NOR (per the binding
> documentation). However, if you do specify both, then they should match.
> Do you have two 8-bits devices? If so may be I need to update the
> documentation to make it clear this is the total width of all devices
> for a given chip-select.

No ... that was wrong, so I've fixed that.

<snip>

>> Booting with this NOR device produces the following oops:-
>>
>> [    0.000000] Booting Linux on physical CPU 0x0
>> [    0.000000] Linux version 3.9.0-rc1-12191-ga00d6d1-dirty (mpfj@mpfj-nanobone) (gcc version 4.5.4 (Buildroot 2012.11) ) #33 Tue Mar 5 13:08:25 GMT 2013
>> [    0.000000] CPU: ARMv7 Processor [413fc082] revision 2 (ARMv7), cr=10c53c7d

<snip>

>> [    0.236730] omap-gpmc 50000000.gpmc: could not find pctldev for node /pinmux@44e10800/gpmc_pins, deferring probe
>> [    0.236781] platform 50000000.gpmc: Driver omap-gpmc requests probe deferral
> 
> This look like your problem. I would figure out why this is failing and
> try again.

Hmmmm ... I get this even when I've no NOR device defined and the board boots up fine.

But I can see in physmap_of.c that the device gets registered without any call to
devm_pinctrl_get_select_default() and hence no probe deferring takes place is the
pinctrl device hasn't yet been started (which it hasn't).

Does probe deferral need adding to physmap_of.c, or should the pinctrl device really
be registered sooner ?

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
Hunter, Jon March 5, 2013, 5:30 p.m. UTC | #9
On 03/05/2013 10:20 AM, Mark Jackson wrote:
> On 05/03/13 14:46, Jon Hunter wrote:
>>
>> On 03/05/2013 08:34 AM, Mark Jackson wrote:
>>> On 26/02/13 17:30, Jon Hunter wrote:
>>>> NOR flash is not currently supported when booting with device-tree
>>>> on OMAP2+ devices. Add support to detect and configure NOR devices
>>>> when booting with device-tree.
>>>>
>>>> Add documentation for the TI GPMC NOR binding.
>>>>
>>>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
>>>
>>> I'm trying to test this series, and am unable get my NOR device recognised.
>>>
>>> If I remove all reference to NOR (and keep only my NAND device definition), then
>>> everything seems to boot fine (so I'm assuming I've got at least the basics of
>>> the patch set working).
>>>
>>> My GPMC tree looks like this:-
> 
> <snip>
> 
>>> 	nor@1,0 {
>>> 		reg = <3 0x00000000 0x04000000>;
>>> 		compatible = "cfi-flash";
>>> 		linux,mtd-name = "spansion,s29gl064n90t";
>>> 		bank-width = <2>;
>>>
>>> 		gpmc,device-width = <1>;
>>
>> Only bank-width should be necessary for NOR (per the binding
>> documentation). However, if you do specify both, then they should match.
>> Do you have two 8-bits devices? If so may be I need to update the
>> documentation to make it clear this is the total width of all devices
>> for a given chip-select.
> 
> No ... that was wrong, so I've fixed that.

Ok, I may update the documentation to make this clearer anyway.

> <snip>
> 
>>> Booting with this NOR device produces the following oops:-
>>>
>>> [    0.000000] Booting Linux on physical CPU 0x0
>>> [    0.000000] Linux version 3.9.0-rc1-12191-ga00d6d1-dirty (mpfj@mpfj-nanobone) (gcc version 4.5.4 (Buildroot 2012.11) ) #33 Tue Mar 5 13:08:25 GMT 2013
>>> [    0.000000] CPU: ARMv7 Processor [413fc082] revision 2 (ARMv7), cr=10c53c7d
> 
> <snip>
> 
>>> [    0.236730] omap-gpmc 50000000.gpmc: could not find pctldev for node /pinmux@44e10800/gpmc_pins, deferring probe
>>> [    0.236781] platform 50000000.gpmc: Driver omap-gpmc requests probe deferral
>>
>> This look like your problem. I would figure out why this is failing and
>> try again.
> 
> Hmmmm ... I get this even when I've no NOR device defined and the board boots up fine.

So for NAND, the device does not get registered until the GPMC probe
completes. However, in the case of NOR, DT is registering the GPMC
and its child devices and so it is working slightly different in
this case.

> But I can see in physmap_of.c that the device gets registered without any call to
> devm_pinctrl_get_select_default() and hence no probe deferring takes place is the
> pinctrl device hasn't yet been started (which it hasn't).
>
> Does probe deferral need adding to physmap_of.c, or should the pinctrl device really
> be registered sooner ?

I see, so the pinctrl driver is not getting probed until later.

This really highlights a weakness in the GPMC driver, particularly
for NOR, where the child device can only be probed once the parent
is probed. I don't see this as being DT specific issue, because
even on older OMAP boards we always registered the NOR flash device
independently of the GPMC device. So we have always been susceptible to
this problem AFAICT.

This is admittedly a hack, but I am curious if you add the pinctrl
properties to the NOR node, if this would also defer the probe on the
NOR device?

Ideally it would be great to defer the probing of the child until the
parent has been probed successfully.

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
Ezequiel Garcia March 5, 2013, 5:43 p.m. UTC | #10
Hi Jon,

On Tue, Mar 5, 2013 at 2:30 PM, Jon Hunter <jon-hunter@ti.com> wrote:
>
> This really highlights a weakness in the GPMC driver, particularly
> for NOR, where the child device can only be probed once the parent
> is probed. I don't see this as being DT specific issue, because
> even on older OMAP boards we always registered the NOR flash device
> independently of the GPMC device. So we have always been susceptible to
> this problem AFAICT.
>
> This is admittedly a hack, but I am curious if you add the pinctrl
> properties to the NOR node, if this would also defer the probe on the
> NOR device?
>
> Ideally it would be great to defer the probing of the child until the
> parent has been probed successfully.
>

This seems related to the probing parent-child ordering probing
I pointed out in a previous mail, isn't it?
(which by the way, I'll answer when I can gather some more convincing
feedback).

I don't have OMAP NOR boards, so I'm not entirely familiar to the
way GPMC registers the NOR device. However, by looking
into board-flash.c:board_nor_init() function, it seems to me that:

1. first we request the CS with gpmc_cs_request() and
2. later we register the NOR device explicitly with
    platform_device_register().

So, unless I'm missing something, we force the NOR device
to be probed *after* the GPMC device.

This is definitely the way NAND, OneNAND is handled.

On the otherside, by using 'simple-bus' you create your devices
first, when the whole device-tree is parsed and later the drivers
are probed at module_initcall time.

So, AFAIK, this problem is DT specific.

On the other side, when you say we should defer the probing
of the child. Do you mean changing something in physmap/NAND/etc. drivers?

Please keep in mind, I have nothing against using simple-bus,
since it's a very clean solution. It's just that it seems to me it will
be problematic.
Moreover, the fix shouldn't be too complicated (still working on this).

I'll try to post my Device Bus driver soon (similar to GPMC)
and also I'll try to answer the previous discussion with some
information on why I think this won't work.

(I hope I'm not making too much noise with this)
Hunter, Jon March 5, 2013, 6:41 p.m. UTC | #11
On 03/05/2013 11:43 AM, Ezequiel Garcia wrote:
> Hi Jon,
> 
> On Tue, Mar 5, 2013 at 2:30 PM, Jon Hunter <jon-hunter@ti.com> wrote:
>>
>> This really highlights a weakness in the GPMC driver, particularly
>> for NOR, where the child device can only be probed once the parent
>> is probed. I don't see this as being DT specific issue, because
>> even on older OMAP boards we always registered the NOR flash device
>> independently of the GPMC device. So we have always been susceptible to
>> this problem AFAICT.
>>
>> This is admittedly a hack, but I am curious if you add the pinctrl
>> properties to the NOR node, if this would also defer the probe on the
>> NOR device?
>>
>> Ideally it would be great to defer the probing of the child until the
>> parent has been probed successfully.
>>
> 
> This seems related to the probing parent-child ordering probing
> I pointed out in a previous mail, isn't it?

Yes in this case. However, I have no idea what the problem you are
having is. So I cannot comment if your problem represents a real
scenario or not.

> (which by the way, I'll answer when I can gather some more convincing
> feedback).

Thanks.

> I don't have OMAP NOR boards, so I'm not entirely familiar to the
> way GPMC registers the NOR device. However, by looking
> into board-flash.c:board_nor_init() function, it seems to me that:
> 
> 1. first we request the CS with gpmc_cs_request() and
> 2. later we register the NOR device explicitly with
>     platform_device_register().
> 
> So, unless I'm missing something, we force the NOR device
> to be probed *after* the GPMC device.
>
> This is definitely the way NAND, OneNAND is handled.
> 
> On the otherside, by using 'simple-bus' you create your devices
> first, when the whole device-tree is parsed and later the drivers
> are probed at module_initcall time.
> 
> So, AFAIK, this problem is DT specific.

This assumes that the GPMC driver is registered (and probed) before 1
and 2 above occur. What is DT specific about when the driver is registered?

What happens in the non-DT case when the GPMC probe is deferred or fails?

We would never register the GPMC devices, right? For the deferred case
that is not so good. So the current implementation is flawed if the
probe is deferred. However, it would not crash which is a plus.

So my point is in Mark's example, if someone were to add pinctrl support
to the existing GPMC driver which caused the GPMC probe to be deferred
then none of the GPMC child devices would ever be registered. I don't
see that being related to DT either.

What you are also missing is that in the probe_nor() function, if we
fail to allocate the required resources, then I disable the child node
and the child is not registered and hence not probed. I have tested this
and for me this works. I was hoping this would be sufficient to handle
failure cases and avoid crashes.

> On the other side, when you say we should defer the probing
> of the child. Do you mean changing something in physmap/NAND/etc. drivers?

Not necessarily.

> Please keep in mind, I have nothing against using simple-bus,
> since it's a very clean solution. It's just that it seems to me it will
> be problematic.
> Moreover, the fix shouldn't be too complicated (still working on this).

I am interested to know what your approach will be. One alternative is
to call of_platform_device_create() from the GPMC driver. I could see
this as being a safe option.

> I'll try to post my Device Bus driver soon (similar to GPMC)
> and also I'll try to answer the previous discussion with some
> information on why I think this won't work.
> 
> (I hope I'm not making too much noise with this)

No it is all good inputs. I am just frustrated to be told that this
implementation is flawed without being able to tell me exactly why or
the scenario where it would not work.

The probe deferred case is a good example where this implementation will
have problems. I see that as legitimate. However, I have no idea if what
you are reporting is also legitimate or not. May be it is, I just don't
know.

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
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/mtd/gpmc-nor.txt b/Documentation/devicetree/bindings/mtd/gpmc-nor.txt
new file mode 100644
index 0000000..8c638fc
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/gpmc-nor.txt
@@ -0,0 +1,98 @@ 
+Device tree bindings for NOR flash connect to TI GPMC
+
+NOR flash connected to the TI GPMC (found on OMAP boards) are represented as
+child nodes of the GPMC controller with a name of "nor".
+
+All timing relevant properties as well as generic GPMC child properties are
+explained in a separate documents. Please refer to
+Documentation/devicetree/bindings/bus/ti-gpmc.txt
+
+Required properties:
+- bank-width: 		Width of NOR flash in bytes. GPMC supports 8-bit and
+			16-bit devices and so must be either 1 or 2 bytes.
+- compatible:		Documentation/devicetree/bindings/mtd/mtd-physmap.txt
+- gpmc,cs-on:		Chip-select assertion time
+- gpmc,cs-rd-off:	Chip-select de-assertion time for reads
+- gpmc,cs-wr-off:	Chip-select de-assertion time for writes
+- gpmc,oe-on:		Output-enable assertion time
+- gpmc,oe-off		Output-enable de-assertion time
+- gpmc,we-on:		Write-enable assertion time
+- gpmc,we-off:		Write-enable de-assertion time
+- gpmc,access:		Start cycle to first data capture (read access)
+- gpmc,rd-cycle:	Total read cycle time
+- gpmc,wr-cycle:	Total write cycle time
+- linux,mtd-name:	Documentation/devicetree/bindings/mtd/mtd-physmap.txt
+- reg:			Chip-select, base address (relative to chip-select)
+			and size of NOR flash. Note that base address will be
+			typically 0 as this is the start of the chip-select.
+
+Optional properties:
+- gpmc,XXX		Additional GPMC timings and settings parameters. See
+			Documentation/devicetree/bindings/bus/ti-gpmc.txt
+
+Optional properties for partiton table parsing:
+- #address-cells: should be set to 1
+- #size-cells: should be set to 1
+
+Example:
+
+gpmc: gpmc@6e000000 {
+	compatible = "ti,omap3430-gpmc", "simple-bus";
+	ti,hwmods = "gpmc";
+	reg = <0x6e000000 0x1000>;
+	interrupts = <20>;
+	gpmc,num-cs = <8>;
+	gpmc,num-waitpins = <4>;
+	#address-cells = <2>;
+	#size-cells = <1>;
+
+	ranges = <0 0 0x10000000 0x08000000>;
+
+	nor@0,0 {
+		compatible = "cfi-flash";
+		linux,mtd-name= "intel,pf48f6000m0y1be";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		reg = <0 0 0x08000000>;
+		bank-width = <2>;
+
+		gpmc,mux-add-data;
+		gpmc,cs-on = <0>;
+		gpmc,cs-rd-off = <186>;
+		gpmc,cs-wr-off = <186>;
+		gpmc,adv-on = <12>;
+		gpmc,adv-rd-off = <48>;
+		gpmc,adv-wr-off = <48>;
+		gpmc,oe-on = <54>;
+		gpmc,oe-off = <168>;
+		gpmc,we-on = <54>;
+		gpmc,we-off = <168>;
+		gpmc,rd-cycle = <186>;
+		gpmc,wr-cycle = <186>;
+		gpmc,access = <114>;
+		gpmc,page-burst-access = <6>;
+		gpmc,bus-turnaround = <12>;
+		gpmc,cycle2cycle-delay = <18>;
+		gpmc,wr-data-mux-bus = <90>;
+		gpmc,wr-access = <186>;
+		gpmc,cycle2cycle-samecsen;
+		gpmc,cycle2cycle-diffcsen;
+
+		partition@0 {
+			label = "bootloader-nor";
+			reg = <0 0x40000>;
+		};
+		partition@0x40000 {
+			label = "params-nor";
+			reg = <0x40000 0x40000>;
+		};
+		partition@0x80000 {
+			label = "kernel-nor";
+			reg = <0x80000 0x200000>;
+		};
+		partition@0x280000 {
+			label = "filesystem-nor";
+			reg = <0x240000 0x7d80000>;
+		};
+	};
+};
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index e822d2b..eb167b3 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>
@@ -513,6 +514,37 @@  static int gpmc_cs_delete_mem(int cs)
 	return r;
 }
 
+/**
+ * gpmc_cs_remap - remaps a chip-select physical base address
+ * @cs:		chip-select to remap
+ * @base:	physical base address to re-map chip-select to
+ *
+ * Re-maps a chip-select to a new physical base address specified by
+ * "base". Returns 0 on success and appropriate negative error code
+ * on failure.
+ */
+static int gpmc_cs_remap(int cs, u32 base)
+{
+	int ret;
+	u32 old_base, size;
+
+	if (cs > GPMC_CS_NUM)
+		return -ENODEV;
+	gpmc_cs_get_memconf(cs, &old_base, &size);
+	if (base == old_base)
+		return 0;
+	gpmc_cs_disable_mem(cs);
+	ret = gpmc_cs_delete_mem(cs);
+	if (IS_ERR_VALUE(ret))
+		return ret;
+	ret = gpmc_cs_insert_mem(cs, base, size);
+	if (IS_ERR_VALUE(ret))
+		return ret;
+	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];
@@ -1172,6 +1204,12 @@  int gpmc_cs_program_settings(int cs, struct gpmc_settings *p)
 }
 
 #ifdef CONFIG_OF
+static struct property device_disabled = {
+	.name = "status",
+	.length = sizeof("disabled"),
+	.value = "disabled",
+};
+
 static struct of_device_id gpmc_dt_ids[] = {
 	{ .compatible = "ti,omap2420-gpmc" },
 	{ .compatible = "ti,omap2430-gpmc" },
@@ -1391,6 +1429,79 @@  static int gpmc_probe_onenand_child(struct platform_device *pdev,
 }
 #endif
 
+/**
+ * gpmc_probe_nor_child - configures the gpmc for a nor device
+ * @pdev:	pointer to gpmc platform device
+ * @child:	pointer to device-tree node for nor device
+ *
+ * Allocates and configures a GPMC chip-select for a NOR flash device.
+ * Returns 0 on success and appropriate negative error code on failure.
+ */
+static int gpmc_probe_nor_child(struct platform_device *pdev,
+				struct device_node *child)
+{
+	struct gpmc_settings gpmc_s;
+	struct gpmc_timings gpmc_t;
+	struct resource res;
+	unsigned long base;
+	int ret, 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;
+	}
+
+	ret = gpmc_cs_request(cs, res.end - res.start, &base);
+	if (IS_ERR_VALUE(ret)) {
+		dev_err(&pdev->dev, "cannot request GPMC CS %d\n", cs);
+		goto err1;
+	}
+
+	/*
+	 * FIXME: 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. Once DT migration is complete should
+	 * just make gpmc_cs_request() map a specific address.
+	 */
+	ret = gpmc_cs_remap(cs, res.start);
+	if (IS_ERR_VALUE(ret)) {
+		dev_err(&pdev->dev, "cannot remap GPMC CS %d to 0x%x\n",
+			cs, res.start);
+		goto err2;
+	}
+
+	gpmc_read_settings_dt(child, &gpmc_s);
+
+	ret = of_property_read_u32(child, "bank-width", &gpmc_s.device_width);
+	if (IS_ERR_VALUE(ret))
+		goto err2;
+
+	ret = gpmc_cs_program_settings(cs, &gpmc_s);
+	if (IS_ERR_VALUE(ret))
+		goto err2;
+
+	gpmc_read_timings_dt(child, &gpmc_t);
+	gpmc_cs_set_timings(cs, &gpmc_t);
+
+	return 0;
+err2:
+	gpmc_cs_free(cs);
+err1:
+	/* Prevent registering child device on error */
+	of_add_property(child, &device_disabled);
+
+	return ret;
+}
+
 static int gpmc_probe_dt(struct platform_device *pdev)
 {
 	int ret;
@@ -1424,6 +1535,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