All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
To: Vinod Koul <vinod.koul-ral2JQCrhuEAvxtiuMwx3w@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: Sun, 12 May 2013 20:28:07 +0200	[thread overview]
Message-ID: <20130512182802.GA2922@katana> (raw)
In-Reply-To: <20130512160440.GA27639-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

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.

WARNING: multiple messages have this Message-ID (diff)
From: Wolfram Sang <wsa@the-dreams.de>
To: Vinod Koul <vinod.koul@intel.com>
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: Sun, 12 May 2013 20:28:07 +0200	[thread overview]
Message-ID: <20130512182802.GA2922@katana> (raw)
In-Reply-To: <20130512160440.GA27639@intel.com>

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.


  parent reply	other threads:[~2013-05-12 18:28 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 [this message]
2013-05-12 18:28                     ` Wolfram Sang
2013-05-13  2:10                     ` Vinod Koul
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=20130512182802.GA2922@katana \
    --to=wsa-z923lk4zbo2bacvfa/9k2g@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=vinod.koul-ral2JQCrhuEAvxtiuMwx3w@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: link
Be 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.