From: Vinod Koul <vinod.koul-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> To: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> Cc: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Dan Williams <djbw-b10kYP2dOMg@public.gmane.org>, linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Subject: Re: [RFC 04/42] drivers/dma: don't check resource with devm_ioremap_resource Date: Mon, 13 May 2013 07:40:00 +0530 [thread overview] Message-ID: <20130513021000.GB27639@intel.com> (raw) In-Reply-To: <20130512182802.GA2922@katana> On Sun, May 12, 2013 at 08:28:07PM +0200, Wolfram Sang wrote: > On Sun, May 12, 2013 at 09:34:40PM +0530, Vinod Koul wrote: > > On Fri, May 10, 2013 at 01:25:36PM -0600, Stephen Warren wrote: > > > On 05/10/2013 11:57 AM, Wolfram Sang wrote: > > > > On Fri, May 10, 2013 at 10:35:32AM -0600, Stephen Warren wrote: > > > >> On 05/10/2013 02:16 AM, Wolfram Sang wrote: > > > >>> devm_ioremap_resource does sanity checks on the given resource. No need to > > > >>> duplicate this in the driver. > > > >> > > > >>> diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c > > > >> > > > >>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > > >>> - if (!res) { > > > >>> - dev_err(&pdev->dev, "No mem resource for DMA\n"); > > > >>> - return -EINVAL; > > > >>> - } > > > >>> - > > > >>> tdma->base_addr = devm_ioremap_resource(&pdev->dev, res); > > > >> > > > >> One issue here is that it's not obvious just from reading the code > > > >> that's left behind that the "missing" error-checking of the > > > >> platform_get_resource() return value is OK because > > > >> devm_ioremap_resource() will check it "for us". Everyone now has to > > > >> mentally maintain a list of exceptions where it's OK not to error-check. > > > > > > > > My goal is to make not-checking the standard case with devm. > > > > > > OK, if no parameters passed to any devm function every need to be > > > error-checked, that'll certainly be a bit easier to remember. > > Okay to remove the log message and move to devm_ but I dont agree with this > > patch not returning error above. We shouldnt supress the error.. > > The error will be reported because devm_ioremap_resource will return an > ERR_PTR. And this patch removed the check on 'res' pointer, so on failure we continue... -- ~Vinod >
WARNING: multiple messages have this Message-ID (diff)
From: Vinod Koul <vinod.koul@intel.com> To: Wolfram Sang <wsa@the-dreams.de> Cc: Stephen Warren <swarren@wwwdotorg.org>, linux-kernel@vger.kernel.org, Dan Williams <djbw@fb.com>, linux-tegra@vger.kernel.org Subject: Re: [RFC 04/42] drivers/dma: don't check resource with devm_ioremap_resource Date: Mon, 13 May 2013 07:40:00 +0530 [thread overview] Message-ID: <20130513021000.GB27639@intel.com> (raw) In-Reply-To: <20130512182802.GA2922@katana> On Sun, May 12, 2013 at 08:28:07PM +0200, Wolfram Sang wrote: > On Sun, May 12, 2013 at 09:34:40PM +0530, Vinod Koul wrote: > > On Fri, May 10, 2013 at 01:25:36PM -0600, Stephen Warren wrote: > > > On 05/10/2013 11:57 AM, Wolfram Sang wrote: > > > > On Fri, May 10, 2013 at 10:35:32AM -0600, Stephen Warren wrote: > > > >> On 05/10/2013 02:16 AM, Wolfram Sang wrote: > > > >>> devm_ioremap_resource does sanity checks on the given resource. No need to > > > >>> duplicate this in the driver. > > > >> > > > >>> diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c > > > >> > > > >>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > > >>> - if (!res) { > > > >>> - dev_err(&pdev->dev, "No mem resource for DMA\n"); > > > >>> - return -EINVAL; > > > >>> - } > > > >>> - > > > >>> tdma->base_addr = devm_ioremap_resource(&pdev->dev, res); > > > >> > > > >> One issue here is that it's not obvious just from reading the code > > > >> that's left behind that the "missing" error-checking of the > > > >> platform_get_resource() return value is OK because > > > >> devm_ioremap_resource() will check it "for us". Everyone now has to > > > >> mentally maintain a list of exceptions where it's OK not to error-check. > > > > > > > > My goal is to make not-checking the standard case with devm. > > > > > > OK, if no parameters passed to any devm function every need to be > > > error-checked, that'll certainly be a bit easier to remember. > > Okay to remove the log message and move to devm_ but I dont agree with this > > patch not returning error above. We shouldnt supress the error.. > > The error will be reported because devm_ioremap_resource will return an > ERR_PTR. And this patch removed the check on 'res' pointer, so on failure we continue... -- ~Vinod >
next prev parent reply other threads:[~2013-05-13 2:10 UTC|newest] Thread overview: 136+ messages / expand[flat|nested] mbox.gz Atom feed top 2013-05-10 8:16 [RFC 00/42] devm improvement series, part #1 Wolfram Sang 2013-05-10 8:16 ` Wolfram Sang 2013-05-10 8:16 ` Wolfram Sang 2013-05-10 8:16 ` [RFC 01/42] drivers/ata: don't check resource with devm_ioremap_resource Wolfram Sang 2013-05-10 8:16 ` [RFC 02/42] drivers/char/hw_random: " Wolfram Sang 2013-05-10 8:16 ` [RFC 03/42] drivers/cpufreq: " Wolfram Sang 2013-05-10 11:09 ` Rafael J. Wysocki 2013-05-10 8:16 ` [RFC 04/42] drivers/dma: " Wolfram Sang [not found] ` <1368173847-5661-5-git-send-email-wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> 2013-05-10 16:35 ` Stephen Warren 2013-05-10 16:35 ` Stephen Warren [not found] ` <518D21D4.6000609-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> 2013-05-10 17:57 ` Wolfram Sang 2013-05-10 17:57 ` Wolfram Sang 2013-05-10 19:25 ` Stephen Warren 2013-05-10 19:25 ` Stephen Warren [not found] ` <518D49B0.4050901-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> 2013-05-12 16:04 ` Vinod Koul 2013-05-12 16:04 ` Vinod Koul [not found] ` <20130512160440.GA27639-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2013-05-12 18:28 ` Wolfram Sang 2013-05-12 18:28 ` Wolfram Sang 2013-05-13 2:10 ` Vinod Koul [this message] 2013-05-13 2:10 ` Vinod Koul [not found] ` <20130513021000.GB27639-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2013-05-13 6:08 ` Wolfram Sang 2013-05-13 6:08 ` Wolfram Sang [not found] ` <1368173847-5661-1-git-send-email-wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> 2013-05-10 8:16 ` [RFC 05/42] drivers/gpio: " Wolfram Sang 2013-05-10 8:16 ` Wolfram Sang [not found] ` <1368173847-5661-6-git-send-email-wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> 2013-05-12 15:11 ` Viresh Kumar 2013-05-12 15:11 ` Viresh Kumar 2013-05-10 8:16 ` [RFC 06/42] drivers/gpu/drm/exynos: " Wolfram Sang 2013-05-10 8:16 ` [RFC 07/42] drivers/gpu/host1x: " Wolfram Sang 2013-05-10 8:16 ` [RFC 08/42] drivers/gpu/host1x/drm: " Wolfram Sang 2013-05-10 8:16 ` [RFC 09/42] drivers/i2c/busses: " Wolfram Sang 2013-05-11 5:18 ` Jingoo Han 2013-05-11 5:18 ` Jingoo Han 2013-06-04 18:34 ` Kevin Hilman 2013-06-04 18:34 ` Kevin Hilman 2013-05-10 8:16 ` [RFC 10/42] drivers/input/keyboard: " Wolfram Sang 2013-05-12 15:12 ` Viresh Kumar 2013-05-10 8:16 ` [RFC 11/42] drivers/input/serio: " Wolfram Sang 2013-05-10 8:16 ` [RFC 12/42] drivers/memory: " Wolfram Sang 2013-05-10 8:16 ` [RFC 13/42] drivers/mfd: " Wolfram Sang 2013-05-16 22:15 ` Samuel Ortiz 2013-05-10 8:16 ` [RFC 14/42] drivers/misc: " Wolfram Sang 2013-05-10 15:24 ` Greg Kroah-Hartman 2013-05-10 8:17 ` [RFC 15/42] drivers/mtd/maps: " Wolfram Sang 2013-05-10 8:17 ` [RFC 16/42] drivers/mtd/nand: " Wolfram Sang 2013-05-10 8:17 ` [RFC 17/42] drivers/net/ethernet/renesas: " Wolfram Sang 2013-05-10 11:07 ` Sergei Shtylyov 2013-05-11 20:31 ` Sergei Shtylyov 2013-05-11 20:35 ` Sergei Shtylyov 2013-05-12 9:42 ` Wolfram Sang 2013-05-12 15:26 ` Sergei Shtylyov 2013-05-12 16:32 ` Wolfram Sang 2013-05-12 18:33 ` Sergei Shtylyov 2013-05-10 8:17 ` [RFC 18/42] drivers/pinctrl: " Wolfram Sang 2013-05-10 14:30 ` Jean-Christophe PLAGNIOL-VILLARD 2013-05-10 14:30 ` Jean-Christophe PLAGNIOL-VILLARD 2013-05-14 12:19 ` Linus Walleij 2013-05-14 12:19 ` Linus Walleij 2013-06-06 8:43 ` Jean-Christophe PLAGNIOL-VILLARD 2013-06-06 8:43 ` Jean-Christophe PLAGNIOL-VILLARD 2013-05-10 8:17 ` [RFC 19/42] drivers/pinctrl/spear: " Wolfram Sang 2013-05-12 15:10 ` Viresh Kumar 2013-05-12 15:10 ` Viresh Kumar 2013-05-14 12:56 ` Linus Walleij 2013-05-14 12:56 ` Linus Walleij 2013-05-10 8:17 ` [RFC 20/42] drivers/pwm: " Wolfram Sang [not found] ` <1368173847-5661-21-git-send-email-wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> 2013-05-12 15:12 ` Viresh Kumar 2013-05-12 15:12 ` Viresh Kumar 2013-05-12 15:12 ` Viresh Kumar 2013-05-10 8:17 ` [RFC 21/42] drivers/remoteproc: " Wolfram Sang 2013-05-10 8:17 ` [RFC 22/42] drivers/rtc: " Wolfram Sang [not found] ` <1368173847-5661-23-git-send-email-wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> 2013-05-12 15:11 ` Viresh Kumar 2013-05-12 15:11 ` Viresh Kumar 2013-05-12 15:11 ` Viresh Kumar 2013-05-10 8:17 ` [RFC 23/42] drivers/spi: " Wolfram Sang [not found] ` <1368173847-5661-24-git-send-email-wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> 2013-05-10 10:55 ` Mark Brown 2013-05-10 10:55 ` Mark Brown 2013-05-10 10:55 ` Mark Brown 2013-05-10 16:37 ` Stephen Warren 2013-05-10 16:37 ` Stephen Warren 2013-05-11 5:24 ` Jingoo Han 2013-05-11 5:24 ` Jingoo Han 2013-05-10 8:17 ` [RFC 24/42] drivers/staging/dwc2: " Wolfram Sang 2013-05-10 15:24 ` Greg Kroah-Hartman 2013-05-10 8:17 ` [RFC 25/42] drivers/staging/nvec: " Wolfram Sang 2013-05-10 8:17 ` Wolfram Sang 2013-05-10 15:24 ` Greg Kroah-Hartman [not found] ` <1368173847-5661-26-git-send-email-wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> 2013-05-10 16:07 ` Marc Dietrich 2013-05-10 16:07 ` Marc Dietrich 2013-05-10 8:17 ` [RFC 26/42] drivers/thermal: " Wolfram Sang 2013-05-10 13:50 ` Eduardo Valentin 2013-05-10 13:50 ` Eduardo Valentin 2013-05-16 7:22 ` Zhang Rui 2013-05-10 8:17 ` [RFC 27/42] drivers/tty/serial: " Wolfram Sang 2013-05-10 15:26 ` Greg Kroah-Hartman 2013-05-10 17:58 ` Wolfram Sang 2013-05-10 8:17 ` [RFC 28/42] drivers/usb/chipidea: " Wolfram Sang 2013-05-10 15:25 ` Greg Kroah-Hartman 2013-05-10 8:17 ` [RFC 29/42] drivers/usb/gadget: " Wolfram Sang 2013-05-10 15:25 ` Greg Kroah-Hartman 2013-05-10 8:17 ` [RFC 30/42] drivers/usb/host: " Wolfram Sang 2013-05-10 14:03 ` Alan Stern 2013-05-10 18:00 ` Wolfram Sang 2013-05-10 15:25 ` Greg Kroah-Hartman 2013-05-10 8:17 ` [RFC 31/42] drivers/usb/phy: " Wolfram Sang 2013-05-10 15:24 ` Greg Kroah-Hartman 2013-05-10 8:17 ` [RFC 32/42] drivers/video: " Wolfram Sang 2013-05-10 8:17 ` Wolfram Sang 2013-05-11 5:33 ` Jingoo Han 2013-05-11 5:33 ` Jingoo Han 2013-05-13 5:39 ` Shawn Guo 2013-05-13 5:39 ` Shawn Guo 2013-05-10 8:17 ` [RFC 33/42] drivers/video/omap2: " Wolfram Sang 2013-05-10 8:17 ` Wolfram Sang 2013-05-11 5:37 ` Jingoo Han 2013-05-11 5:37 ` Jingoo Han 2013-05-10 8:17 ` [RFC 34/42] drivers/video/omap2/dss: " Wolfram Sang 2013-05-10 8:17 ` Wolfram Sang 2013-05-11 5:36 ` Jingoo Han 2013-05-11 5:36 ` Jingoo Han 2013-05-10 8:17 ` [RFC 35/42] drivers/w1/masters: " Wolfram Sang 2013-05-10 8:17 ` [RFC 36/42] drivers/watchdog: " Wolfram Sang 2013-05-12 17:23 ` Wim Van Sebroeck 2013-05-10 8:17 ` [RFC 37/42] arch/arm/mach-tegra: " Wolfram Sang 2013-05-10 8:17 ` [RFC 38/42] arch/arm/plat-omap: " Wolfram Sang 2013-05-10 16:46 ` Tony Lindgren 2013-05-10 16:46 ` Tony Lindgren 2013-05-10 8:17 ` [RFC 39/42] arch/arm/plat-samsung: " Wolfram Sang 2013-05-11 5:15 ` Jingoo Han 2013-05-11 5:15 ` Jingoo Han 2013-05-10 8:17 ` [RFC 40/42] arch/mips/lantiq/xway: " Wolfram Sang 2013-05-10 8:57 ` John Crispin 2013-05-10 8:17 ` [RFC 41/42] sound/soc/fsl: " Wolfram Sang 2013-05-10 10:56 ` Mark Brown 2013-05-10 10:56 ` Mark Brown 2013-05-10 8:17 ` [RFC 42/42] sound/soc/kirkwood: " Wolfram Sang 2013-05-10 10:56 ` Mark Brown
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20130513021000.GB27639@intel.com \ --to=vinod.koul-ral2jqcrhueavxtiumwx3w@public.gmane.org \ --cc=djbw-b10kYP2dOMg@public.gmane.org \ --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org \ --cc=wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.