Re: [PATCH 1/1] USB: ehci-omap: Reset dma_mask pointer on probe
On 06/03/2013 08:51 PM, Greg KH wrote: On Thu, May 23, 2013 at 05:46:44PM +0300, Roger Quadros wrote: On 05/23/2013 05:11 PM, Alan Stern wrote: On Thu, 23 May 2013, Roger Quadros wrote: Device tree probed devices don't get dma_mask set. Previously we were setting the dma_mask pointer only if it was NULL. However, the address of 'omap_ehci_dma_mask' would change each time the module is unloaded and loaded back thus causing the devices dma_mask pointer to be invalid on the next load. This will cause page faults if any driver tries to access the old dma_mask pointer. Unconditionally re-setting the dma_mask pointer fixes this problem. diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c index 3d1491b..b33e306 100644 --- a/drivers/usb/host/ehci-omap.c +++ b/drivers/usb/host/ehci-omap.c @@ -146,8 +146,7 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev) * Since shared usb code relies on it, set it here for now. * Once we have dma capability bindings this can go away. */ - if (!pdev-dev.dma_mask) - pdev-dev.dma_mask = omap_ehci_dma_mask; + pdev-dev.dma_mask = omap_ehci_dma_mask; Is this the solution that people have agreed on? There has been a lot of discussion on this topic. In particular, there has been talk about fixing it in the DT core. Fixing it in DT core would be best. Then please do that. It seems [1] went into 3.10-rc3 so this issue is fixed for 3.10. [1] https://patchwork.kernel.org/patch/2537021/ cheers, -roger -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] USB: ehci-omap: Reset dma_mask pointer on probe
On Thu, May 23, 2013 at 05:46:44PM +0300, Roger Quadros wrote: On 05/23/2013 05:11 PM, Alan Stern wrote: On Thu, 23 May 2013, Roger Quadros wrote: Device tree probed devices don't get dma_mask set. Previously we were setting the dma_mask pointer only if it was NULL. However, the address of 'omap_ehci_dma_mask' would change each time the module is unloaded and loaded back thus causing the devices dma_mask pointer to be invalid on the next load. This will cause page faults if any driver tries to access the old dma_mask pointer. Unconditionally re-setting the dma_mask pointer fixes this problem. diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c index 3d1491b..b33e306 100644 --- a/drivers/usb/host/ehci-omap.c +++ b/drivers/usb/host/ehci-omap.c @@ -146,8 +146,7 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev) * Since shared usb code relies on it, set it here for now. * Once we have dma capability bindings this can go away. */ - if (!pdev-dev.dma_mask) - pdev-dev.dma_mask = omap_ehci_dma_mask; + pdev-dev.dma_mask = omap_ehci_dma_mask; Is this the solution that people have agreed on? There has been a lot of discussion on this topic. In particular, there has been talk about fixing it in the DT core. Fixing it in DT core would be best. Then please do that. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] USB: ehci-omap: Reset dma_mask pointer on probe
Device tree probed devices don't get dma_mask set. Previously we were setting the dma_mask pointer only if it was NULL. However, the address of 'omap_ehci_dma_mask' would change each time the module is unloaded and loaded back thus causing the devices dma_mask pointer to be invalid on the next load. This will cause page faults if any driver tries to access the old dma_mask pointer. Unconditionally re-setting the dma_mask pointer fixes this problem. e.g. of backtrace when mass storage device is plugged in after ehci-omap module is unloaded and loaded back when booted with device tree. [ 646.398223] Unable to handle kernel paging request at virtual address bf08f678 [ 646.406005] pgd = c0004000 [ 646.408966] [bf08f678] *pgd=8c9ec811, *pte=, *ppte= [ 646.415649] Internal error: Oops: 7 [#1] SMP ARM [ 646.420532] Modules linked in: ehci_omap ehci_hcd snd_soc_omap snd_soc_omap_mcbsp snd_soc_core regmap_spi snd_compress snd_pcm snd_timer snd soundcore snd_page_alloc [last unloaded: ehci_hcd] [ 646.438629] CPU: 0 PID: 691 Comm: kworker/u2:2 Not tainted 3.10.0-rc2-00047-g519fe2e #490 [ 646.447265] Workqueue: events_unbound async_run_entry_fn [ 646.452880] task: ce12cb80 ti: ce194000 task.ti: ce194000 [ 646.458618] PC is at scsi_calculate_bounce_limit+0x34/0x48 [ 646.464385] LR is at __scsi_alloc_queue+0x80/0x108 [ 646.469451] pc : [c032a680]lr : [c032a714]psr: a013 [ 646.469451] sp : ce195c88 ip : 000c fp : 0001 [ 646.481536] r10: cca4d018 r9 : cca42808 r8 : [ 646.487060] r7 : 012a r6 : ce527420 r5 : ce3561b0 r4 : ce415000 [ 646.493927] r3 : bf08f678 r2 : ce424cd0 r1 : 00f0 r0 : ce415000 [ 646.500823] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel [ 646.508544] Control: 10c5387d Table: 8c9bc019 DAC: 0015 [ 646.514587] Process kworker/u2:2 (pid: 691, stack limit = 0xce194240) [ 646.521392] Stack: (0xce195c88 to 0xce196000) [ 646.526000] 5c80: cca42800 cca4d000 ce415000 ffe0 c032b978 [ 646.534606] 5ca0: cca42800 c032c114 ce12cb80 cca4d000 ce195d64 ce415000 [ 646.543243] 5cc0: ce194000 c07b5574 c032c654 c008c2bc 6093 ce194000 [ 646.551879] 5ce0: ce12cb80 c04cc890 0001 ce415000 ce194000 c07b5574 c008c2bc [ 646.560485] 5d00: 6013 cca4d0a8 0004 6013 c04cc890 c07b5880 cca4d000 [ 646.569122] 5d20: ce415180 ce194000 c07b5574 c032cf40 [ 646.577758] 5d40: ce195d8c c08a8208 0002 [ 646.586364] 5d60: ce415220 0002 ce12d010 ce12cb80 0006 [ 646.595001] 5d80: 0004 c07b932c 0006 c008c0fc ce12cb80 ce415180 ce194000 [ 646.603637] 5da0: ce12cb80 c04cc890 0001 ce194000 ce415068 c07b5574 c008c2bc [ 646.612243] 5dc0: 6013 0001 ce415000 ce194000 ce415180 [ 646.620880] 5de0: c07b5574 c032d400 ce415000 ce415180 ce415000 [ 646.629516] 5e00: ce415180 ce415068 c032d708 cc9759c0 c00690bc 005d ce415000 [ 646.638122] 5e20: ce329940 ccae4300 ce195ec0 ccb26dd0 ccb26dc0 c032d824 [ 646.646759] 5e40: c07b6f1c ce329940 ccae4300 c032d9c4 c07b6f1c ce329940 ce03f000 [ 646.655395] 5e60: ce195ec0 c0065384 6013 ce047f00 ce03f000 ccb26dd0 [ 646.664001] 5e80: ce329940 ccb26dd0 ce329940 ce03f000 ce195ec0 ce047f00 c005842c [ 646.672637] 5ea0: 0002 c00583a0 c04ca4d4 ce12cb80 0002 [ 646.681274] 5ec0: c07cb97c c08a8128 c06187b0 ce03f000 ce329940 ce03f030 ce329958 [ 646.689910] 5ee0: ce194000 ce194000 c07b5208 0001 ce03f000 c0058b10 [ 646.698516] 5f00: ce194000 6113 ce0a1e34 ce329940 c00589d8 [ 646.707153] 5f20: c005ebec ce12cb80 0001 ce329940 [ 646.715789] 5f40: dead4ead c07cb8f4 [ 646.724395] 5f60: c0615efc ce195f64 ce195f64 dead4ead [ 646.733032] 5f80: c07cb8f4 c0615efc ce195f90 ce195f90 ce195fac ce0a1e34 [ 646.741668] 5fa0: c005eb48 c0013688 [ 646.750274] 5fc0: [ 646.758911] 5fe0: 0013 2a7fef02 00b3ff00 [ 646.767547] [c032a680] (scsi_calculate_bounce_limit+0x34/0x48) from [c032a714] (__scsi_alloc_queue+0x80/0x108) [ 646.778472] [c032a714] (__scsi_alloc_queue+0x80/0x108) from [c032b978] (scsi_alloc_queue+0x10/0x60) [ 646.788391] [c032b978] (scsi_alloc_queue+0x10/0x60) from [c032c114] (scsi_alloc_sdev+0x170/0x270) [ 646.798126] [c032c114] (scsi_alloc_sdev+0x170/0x270) from [c032c654]
Re: [PATCH 1/1] USB: ehci-omap: Reset dma_mask pointer on probe
On Thu, 23 May 2013, Roger Quadros wrote: Device tree probed devices don't get dma_mask set. Previously we were setting the dma_mask pointer only if it was NULL. However, the address of 'omap_ehci_dma_mask' would change each time the module is unloaded and loaded back thus causing the devices dma_mask pointer to be invalid on the next load. This will cause page faults if any driver tries to access the old dma_mask pointer. Unconditionally re-setting the dma_mask pointer fixes this problem. diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c index 3d1491b..b33e306 100644 --- a/drivers/usb/host/ehci-omap.c +++ b/drivers/usb/host/ehci-omap.c @@ -146,8 +146,7 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev) * Since shared usb code relies on it, set it here for now. * Once we have dma capability bindings this can go away. */ - if (!pdev-dev.dma_mask) - pdev-dev.dma_mask = omap_ehci_dma_mask; + pdev-dev.dma_mask = omap_ehci_dma_mask; Is this the solution that people have agreed on? There has been a lot of discussion on this topic. In particular, there has been talk about fixing it in the DT core. This particular approach doesn't seem very robust. What if pdev-dev.dma_mask is already set to a different value for some good reason? Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] USB: ehci-omap: Reset dma_mask pointer on probe
On 05/23/2013 05:11 PM, Alan Stern wrote: On Thu, 23 May 2013, Roger Quadros wrote: Device tree probed devices don't get dma_mask set. Previously we were setting the dma_mask pointer only if it was NULL. However, the address of 'omap_ehci_dma_mask' would change each time the module is unloaded and loaded back thus causing the devices dma_mask pointer to be invalid on the next load. This will cause page faults if any driver tries to access the old dma_mask pointer. Unconditionally re-setting the dma_mask pointer fixes this problem. diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c index 3d1491b..b33e306 100644 --- a/drivers/usb/host/ehci-omap.c +++ b/drivers/usb/host/ehci-omap.c @@ -146,8 +146,7 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev) * Since shared usb code relies on it, set it here for now. * Once we have dma capability bindings this can go away. */ -if (!pdev-dev.dma_mask) -pdev-dev.dma_mask = omap_ehci_dma_mask; +pdev-dev.dma_mask = omap_ehci_dma_mask; Is this the solution that people have agreed on? There has been a lot of discussion on this topic. In particular, there has been talk about fixing it in the DT core. Fixing it in DT core would be best. This particular approach doesn't seem very robust. What if pdev-dev.dma_mask is already set to a different value for some good reason? Then it breaks. But for OMAP, that situation seems unlikely. cheers, -roger -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
