| Message ID | 20131202193459.GL26766@atomide.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 12/02/2013 07:34 PM, Tony Lindgren wrote: > Commit 31ee9181eb92: (mmc: omap: Fix DMA configuration to not rely > on device id) fixed getting of the DMA resources when booted with > device tree. This patch however changed the handling of the > free_mem_region() error path by reusing the struct resource *res > for the DMA resources. > > Fix the error the same way omap_hsmmc.c driver handles it, which > is to restore the resource before using the values to call > release_mem_region(). > > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > Signed-off-by: Tony Lindgren <tony@atomide.com> > I guess my devm_ioremap conversion would have fixed this? http://www.spinics.net/lists/linux-mmc/msg23317.html Does not help now but I'm thinking if queuing some of those my patches (3-5) on top of yours as a fix? Especially fix to NULL pointer dereference: http://www.spinics.net/lists/linux-mmc/msg23320.html
* Jarkko Nikula <jarkko.nikula@bitmer.com> [131203 08:18]: > On 12/02/2013 07:34 PM, Tony Lindgren wrote: > > Commit 31ee9181eb92: (mmc: omap: Fix DMA configuration to not rely > > on device id) fixed getting of the DMA resources when booted with > > device tree. This patch however changed the handling of the > > free_mem_region() error path by reusing the struct resource *res > > for the DMA resources. > > > > Fix the error the same way omap_hsmmc.c driver handles it, which > > is to restore the resource before using the values to call > > release_mem_region(). > > > > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > > Signed-off-by: Tony Lindgren <tony@atomide.com> > > > I guess my devm_ioremap conversion would have fixed this? > > http://www.spinics.net/lists/linux-mmc/msg23317.html Probably.. > Does not help now but I'm thinking if queuing some of those my patches > (3-5) on top of yours as a fix? Especially fix to NULL pointer dereference: > > http://www.spinics.net/lists/linux-mmc/msg23320.html Yeah please do, how about also adding the error log to the description? Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Tony Lindgren <tony@atomide.com> [131202 11:36]: > Commit 31ee9181eb92: (mmc: omap: Fix DMA configuration to not rely > on device id) fixed getting of the DMA resources when booted with > device tree. This patch however changed the handling of the > free_mem_region() error path by reusing the struct resource *res > for the DMA resources. > > Fix the error the same way omap_hsmmc.c driver handles it, which > is to restore the resource before using the values to call > release_mem_region(). > > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > Signed-off-by: Tony Lindgren <tony@atomide.com> > > --- > > Chris, looks like the patch introducing this error is queued in > arm-soc fixes branch.. Care to ack this one too and I'll merge it > via arm-soc as well? > > After that, no need for me to patch this driver for my mach-omap2 > device tree conversion I hope :) OK the previous fixes just hit the mainline, no need for me to queue this any longer. Please feel free to pick this one up for the -rc cycle. Regards, Tony > --- a/drivers/mmc/host/omap.c > +++ b/drivers/mmc/host/omap.c > @@ -1465,7 +1465,10 @@ err_free_mmc_host: > err_ioremap: > kfree(host); > err_free_mem_region: > - release_mem_region(res->start, resource_size(res)); > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (res) > + release_mem_region(res->start, resource_size(res)); > + > return ret; > } > > -- > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi
I've sent these patches earlier to linux-mmc list. This time set is one
patch smaller since commit 31ee9181eb92
("mmc: omap: Fix DMA configuration to not rely on device id") does
practically same what one of my patch did.
Patch 1 is an obvious fix. 2-4 are preparing for 5 which also should fix
the error introduced by 31ee9181eb92:
http://www.spinics.net/lists/linux-omap/msg100790.html
Patch 6 continue cleanup and 7 adds the ERASE capability to the driver so
one could utilize discards with mkfs, mount option or with the fstrim.
Set goes on top of v3.13-rc2-208-g8ecffd791448.
Jarkko Nikula (7):
mmc: omap: Fix NULL pointer dereference due uninitialized
cover_tasklet
mmc: omap: Convert to devm_kzalloc
mmc: omap: Remove duplicate host->irq assignment
mmc: omap: Remove mem_res field from struct mmc_omap_host
mmc: omap: Convert to devm_ioremap_resource
mmc: omap: Remove always set use_dma flag from struct mmc_omap_host
mmc: omap: Add erase capability
drivers/mmc/host/omap.c | 93 +++++++++++++++++++------------------------------
1 file changed, 36 insertions(+), 57 deletions(-)
* Jarkko Nikula <jarkko.nikula@bitmer.com> [131204 11:15]: > Hi > > I've sent these patches earlier to linux-mmc list. This time set is one > patch smaller since commit 31ee9181eb92 > ("mmc: omap: Fix DMA configuration to not rely on device id") does > practically same what one of my patch did. > > Patch 1 is an obvious fix. 2-4 are preparing for 5 which also should fix > the error introduced by 31ee9181eb92: > http://www.spinics.net/lists/linux-omap/msg100790.html > > Patch 6 continue cleanup and 7 adds the ERASE capability to the driver so > one could utilize discards with mkfs, mount option or with the fstrim. > > Set goes on top of v3.13-rc2-208-g8ecffd791448. Great they look good to me: Acked-by: Tony Lindgren <tony@atomide.com> > Jarkko Nikula (7): > mmc: omap: Fix NULL pointer dereference due uninitialized > cover_tasklet > mmc: omap: Convert to devm_kzalloc > mmc: omap: Remove duplicate host->irq assignment > mmc: omap: Remove mem_res field from struct mmc_omap_host > mmc: omap: Convert to devm_ioremap_resource > mmc: omap: Remove always set use_dma flag from struct mmc_omap_host > mmc: omap: Add erase capability > > drivers/mmc/host/omap.c | 93 +++++++++++++++++++------------------------------ > 1 file changed, 36 insertions(+), 57 deletions(-) > > -- > 1.8.4.3 > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Wed, Dec 04, 2013 at 09:14:07PM +0000, Jarkko Nikula wrote: > I've sent these patches earlier to linux-mmc list. This time set is one > patch smaller since commit 31ee9181eb92 > ("mmc: omap: Fix DMA configuration to not rely on device id") does > practically same what one of my patch did. > > Patch 1 is an obvious fix. 2-4 are preparing for 5 which also should fix > the error introduced by 31ee9181eb92: > http://www.spinics.net/lists/linux-omap/msg100790.html > > Patch 6 continue cleanup and 7 adds the ERASE capability to the driver so > one could utilize discards with mkfs, mount option or with the fstrim. > > Set goes on top of v3.13-rc2-208-g8ecffd791448. Patch 5 had some fuzz with plain 3.13-rc2. Otherwise these look fine. I tested these on 770 and N800, so for all patches: Tested-by: Aaro Koskinen <aaro.koskinen@iki.fi> A. -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/06/2013 01:41 PM, Aaro Koskinen wrote: >> Set goes on top of v3.13-rc2-208-g8ecffd791448. > > Patch 5 had some fuzz with plain 3.13-rc2. Otherwise these look fine. > I tested these on 770 and N800, so for all patches: > > Tested-by: Aaro Koskinen <aaro.koskinen@iki.fi> > Yes, reason for your fuzz observation is that 3.13-rc2 don't have those two patches from Tony but 8ecffd791448 had.
--- a/drivers/mmc/host/omap.c +++ b/drivers/mmc/host/omap.c @@ -1465,7 +1465,10 @@ err_free_mmc_host: err_ioremap: kfree(host); err_free_mem_region: - release_mem_region(res->start, resource_size(res)); + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (res) + release_mem_region(res->start, resource_size(res)); + return ret; }
Commit 31ee9181eb92: (mmc: omap: Fix DMA configuration to not rely on device id) fixed getting of the DMA resources when booted with device tree. This patch however changed the handling of the free_mem_region() error path by reusing the struct resource *res for the DMA resources. Fix the error the same way omap_hsmmc.c driver handles it, which is to restore the resource before using the values to call release_mem_region(). Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: Tony Lindgren <tony@atomide.com> --- Chris, looks like the patch introducing this error is queued in arm-soc fixes branch.. Care to ack this one too and I'll merge it via arm-soc as well? After that, no need for me to patch this driver for my mach-omap2 device tree conversion I hope :) -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html