Message-ID: <20150905181559.GA6978@laptop.cereza>
Date:	Sat, 5 Sep 2015 15:15:59 -0300
From:	Ezequiel Garcia <ezequiel@...guardiasur.com.ar>
To:	Robert Jarzmik <robert.jarzmik@...e.fr>
Cc:	Ezequiel Garcia <ezequiel.garcia@...e-electrons.com>,
	David Woodhouse <dwmw2@...radead.org>,
	Brian Norris <computersforpeace@...il.com>,
	linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mtd: nand: pxa3xx-nand: switch to dmaengine

On 29 Aug 01:01 PM, Robert Jarzmik wrote:
> Ezequiel Garcia <ezequiel@...guardiasur.com.ar> writes:
> 
> > Robert,
> >
> > On 24 Aug 08:41 PM, Robert Jarzmik wrote:
> >> Now pxa architecture has a dmaengine driver, remove the access to direct
> >> dma registers in favor of the more generic dmaengine code.
> >> 
> >> This should be also applicable for mmp and orion, provided they work in
> >> device-tree environment.
> >> 
> >> This patch also removes the previous hack which was necessary to make
> >> the driver work in a devicetree environment.
> >>
> >
> > Oh, this is nice.
> >
> > I haven't been following the PXA dmaengine effort,
> > should I expect this to Just Work (TM) on a CM-X300 (PXA310) in linux-next?
> No, because linux-next without it gives :
> Unable to handle kernel paging request at virtual address e36208e4
> pgd = c0004000
> [e36208e4] *pgd=c360044e(bad)
> Internal error: Oops: 823 [#1] ARM
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper Not tainted 4.2.0-rc8-next-20150828-cm-x300+ #864
> Hardware name: CM-X300 module
> task: e3436000 ti: e3444000 task.ti: e3444000
> PC is at nand_scan_ident+0xe00/0x1330
> LR is at nand_scan_ident+0xd28/0x1330
> pc : [<c02a7250>]    lr : [<c02a7178>]    psr: 68000053
> sp : e3445d70  ip : 68000053  fp : c0ba0c80
> r10: 00000000  r9 : 000000dc  r8 : 000000ec
> r7 : 00000001  r6 : e36208dc  r5 : 00000000  r4 : 20000000
> r3 : 00000000  r2 : 00000000  r1 : 00000000  r0 : 00000000
> Flags: nZCv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment none
> Control: 0000397f  Table: a0004018  DAC: 00000071
> Process swapper (pid: 1, stack limit = 0xe3444198)
> Stack: (0xe3445d70 to 0xe3446000)
> 5d60:                                     00000800 00000040 00000000 00000001
> 5d80: 00000095 00000001 e22e0500 00002000 9510dcec ffffffff e3620810 e3620810
> 5da0: e3620b24 e36208dc c0b8bd34 00000000 c02ab1a8 c02ab1e0 c0b8bd34 c02ac228
> 5dc0: c04823d8 e3620810 c0b885c0 c0b8bd4c c0b885d0 e3620878 00000007 e3403888
> 5de0: e3620810 00000000 c0b8bd34 80000000 001000d0 28000053 00000000 00000000
> 5e00: 00000000 c0206fc8 e22e5b40 000019b9 00000000 c00f8f94 00000002 00000000
> 5e20: e22ed190 e3529460 e3529460 c00f9148 e22ed0f0 e3529460 e3529460 00000001
> 5e40: e3529460 e22ed0f0 e22ed190 c04bde88 e3529460 ffffffed c0b885d0 fffffdfb
> 5e60: c0ba1a2c 00000000 c0533830 c051559c 00000000 c0271bfc c0271bcc c0b885d0
> 5e80: c0c02b6c c0ba1a2c c0b9bb48 c0270300 c0b885d0 c0ba1a2c c0b88604 c0b9bb48
> 5ea0: 00000000 c0270440 00000000 c0ba1a2c c02703b4 c026e890 e3432b8c e3477750
> 5ec0: c0ba1a2c e22ee000 00000000 c026f9e8 c04823d8 00000006 c0ba1a2c c0ba1a2c
> 5ee0: c0b85b80 e22e5b00 c052b2a0 c0270c04 00000000 c0b85b80 c0b85b80 c00096d0
> 5f00: c0bf6b6c e3407600 e3463980 c03d60f8 00000000 00000000 00000000 c00f0a4c
> 5f20: 00000000 00000000 e3ffce2e c002f694 00000000 c04f2d68 e3ffce49 00000006
> 5f40: 00000006 00000000 00000000 c053ee4c c053ef9c 00000006 c0bb1080 00000075
> 5f60: c0bb1080 c051559c 00000000 c0515d6c 00000006 00000006 00000000 c051559c
> 5f80: c0018bcc 00000000 c03d121c 00000000 00000000 00000000 00000000 00000000
> 5fa0: 00000000 c03d1224 00000000 c000a4d0 00000000 00000000 00000000 00000000
> 5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000 50001052 31082120
> [<c02a7250>] (nand_scan_ident) from [<c02ac228>] (pxa3xx_nand_probe+0x344/0xc6c)
> [<c02ac228>] (pxa3xx_nand_probe) from [<c0271bfc>] (platform_drv_probe+0x30/0x94)
> [<c0271bfc>] (platform_drv_probe) from [<c0270300>] (driver_probe_device+0x240/0x2f4)
> [<c0270300>] (driver_probe_device) from [<c0270440>] (__driver_attach+0x8c/0x90)
> [<c0270440>] (__driver_attach) from [<c026e890>] (bus_for_each_dev+0x70/0xa0)
> [<c026e890>] (bus_for_each_dev) from [<c026f9e8>] (bus_add_driver+0x18c/0x210)
> [<c026f9e8>] (bus_add_driver) from [<c0270c04>] (driver_register+0x78/0xf8)
> [<c0270c04>] (driver_register) from [<c00096d0>] (do_one_initcall+0x80/0x1e8)
> [<c00096d0>] (do_one_initcall) from [<c0515d6c>] (kernel_init_freeable+0x100/0x1c8)
> [<c0515d6c>] (kernel_init_freeable) from [<c03d1224>] (kernel_init+0x8/0xec)
> [<c03d1224>] (kernel_init) from [<c000a4d0>] (ret_from_fork+0x14/0x24)
> Code: e0854093 e3a00000 e0232391 e0835005 (e1c640f8) 
> ---[ end trace b0bd534a8da9246f ]---
> Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
> 
> ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
> 
> >> -#ifndef ARCH_HAS_DMA
> >> -	if (use_dma) {
> >> +	dma_available = IS_ENABLED(CONFIG_ARM) &&
> >> +		(IS_ENABLED(CONFIG_ARCH_PXA) || IS_ENABLED(CONFIG_ARCH_MMP));
> >> +	if (use_dma && !dma_available) {
> >
> > Isn't it just simpler to always fallback to PIO if the
> > devicetree doesn't provide any DMA channels?
> Most certainly, but that deserves another patch as it changes the current
> behavior.
> 

Right.

> > Platforms without DMA would fall naturally to PIO.
> What about -EPROBE_DEFER handling ?
> 

Hmm.. dunno.

> > Also: do we need to update the devicetree binding?
> Yes, the example and an optional dma node property.
> 

OK, so you'll be sending a v2?

Have a few more comments on this, so I'll reply to the original patch.
-- 
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists