On Wed, Jan 09, 2013 at 08:07:45AM +0100, Thierry Reding wrote:

On Sun, Jan 06, 2013 at 11:57:48AM -0800, Dmitry Torokhov wrote:

quoted

On Sun, Jan 06, 2013 at 08:27:39PM +0100, Thierry Reding wrote:

quoted

On Sat, Jan 05, 2013 at 12:06:58AM -0800, Dmitry Torokhov wrote:

quoted

On Sat, Jan 05, 2013 at 01:15:08PM +0530, Laxman Dewangan wrote:

[...]

quoted

quoted

@@ -735,25 +738,16 @@ static int tegra_kbc_probe(struct platform_device *pdev)
 	spin_lock_init(&kbc->lock);
 	setup_timer(&kbc->timer, tegra_kbc_keypress_timer, (unsigned long)kbc);
 
-	res = request_mem_region(res->start, resource_size(res), pdev->name);
-	if (!res) {
-		dev_err(&pdev->dev, "failed to request I/O memory\n");
-		err = -EBUSY;
-		goto err_free_mem;
-	}
-
-	kbc->mmio = ioremap(res->start, resource_size(res));
+	kbc->mmio = devm_request_and_ioremap(&pdev->dev, res);
 	if (!kbc->mmio) {
-		dev_err(&pdev->dev, "failed to remap I/O memory\n");
-		err = -ENXIO;
-		goto err_free_mem_region;
+		dev_err(&pdev->dev, "Cannot request memregion/iomap address\n");
+		return -EADDRNOTAVAIL;

Erm, no, -EBUSY please.

EADDRNOTAVAIL is the canonical error for devm_request_and_ioremap()
failure. The kerneldoc comment in lib/devres.c even gives a short
example that uses this error code.

I am sorry, but I do not consider a function that was added a little
over a year ago as a canon. If you look at the uses of EADDRNOTAVAIL it
is used predominantly in networking code to indicate that attempted
_network_ address is not available.

EBUSY might be misleading, though. devm_request_and_ioremap() can fail
in both the request_mem_region() and ioremap() calls. Furthermore it'd
be good to settle on a consistent error-code instead of doing it
differently depending on subsystem and/or driver. Currently the various
error codes used are:

	EBUSY, EADDRNOTAVAIL, ENXIO, ENOMEM, ENODEV, ENOENT, EINVAL,
	EIO, EFAULT, EADDRINUSE

Also if we can settle on one error code we should follow up with a patch
to make it consistent across the tree and also update that kerneldoc
comment. I volunteer to do that if nobody else steps up. I'm also Cc'ing
Wolfram (the original author), maybe he has some thoughts on this.

If you going to change all drivers make devm_request_and_ioremap()
return ERR_PTR()-encoded errors and then we can differentiate what
part of it failed.

Thanks.

-- 
Dmitry