| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20130121102638.GA13784@core.coreip.homeip.net>
Date: Mon, 21 Jan 2013 02:26:38 -0800
From: Dmitry Torokhov <dmitry.torokhov@...il.com>
To: Thierry Reding <thierry.reding@...onic-design.de>
Cc: linux-kernel@...r.kernel.org,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Arnd Bergmann <arnd@...db.de>,
Wolfram Sang <w.sang@...gutronix.de>
Subject: Re: [PATCH 01/33] lib: devres: Introduce devm_ioremap_resource()
On Mon, Jan 21, 2013 at 11:08:54AM +0100, Thierry Reding wrote:
> The devm_request_and_ioremap() function is very useful and helps avoid a
> whole lot of boilerplate. However, one issue that keeps popping up is
> its lack of a specific error code to determine which of the steps that
> it performs failed. Furthermore, while the function gives an example and
> suggests what error code to return on failure, a wide variety of error
> codes are used throughout the tree.
>
> In an attempt to fix these problems, this patch adds a new function that
> drivers can transition to. The devm_ioremap_resource() returns a pointer
> to the remapped I/O memory on success or an ERR_PTR() encoded error code
> on failure. Callers can check for failure using IS_ERR() and determine
> its cause by extracting the error code using PTR_ERR().
>
> devm_request_and_ioremap() is implemented as a wrapper around the new
> API and return NULL on failure as before. This ensures that backwards
> compatibility is maintained until all users have been converted to the
> new API, at which point the old devm_request_and_ioremap() function
> should be removed. To help prevent new users from being added in the
> meantime, devm_request_and_ioremap() is marked __deprecated.
>
> A semantic patch is included which can be used to convert from the old
> devm_request_and_ioremap() API to the new devm_ioremap_resource() API.
> Some non-trivial cases may require manual intervention, though.
>
> Signed-off-by: Thierry Reding <thierry.reding@...onic-design.de>
> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Cc: Dmitry Torokhov <dmitry.torokhov@...il.com>
This looks good to me.
Acked-by: Dmitry Torokhov <dmitry.torokhov@...il.com>
> Cc: Arnd Bergmann <arnd@...db.de>
> ---
> Documentation/driver-model/devres.txt | 3 +-
> include/linux/device.h | 3 +-
> lib/devres.c | 57 ++++++++++----
> scripts/coccinelle/api/devm_ioremap_resource.cocci | 90 ++++++++++++++++++++++
> 4 files changed, 138 insertions(+), 15 deletions(-)
> create mode 100644 scripts/coccinelle/api/devm_ioremap_resource.cocci
>
> diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
> index 43cff70..25240fc 100644
> --- a/Documentation/driver-model/devres.txt
> +++ b/Documentation/driver-model/devres.txt
> @@ -266,7 +266,8 @@ IOMAP
> devm_ioremap()
> devm_ioremap_nocache()
> devm_iounmap()
> - devm_request_and_ioremap() : checks resource, requests region, ioremaps
> + devm_ioremap_resource() : checks resource, requests memory region, ioremaps
> + devm_request_and_ioremap() : obsoleted by devm_ioremap_resource()
> pcim_iomap()
> pcim_iounmap()
> pcim_iomap_table() : array of mapped addresses indexed by BAR
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 43dcda9..558c405 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -573,8 +573,9 @@ extern int devres_release_group(struct device *dev, void *id);
> extern void *devm_kzalloc(struct device *dev, size_t size, gfp_t gfp);
> extern void devm_kfree(struct device *dev, void *p);
>
> +void __iomem *devm_ioremap_resource(struct device *dev, struct resource *res);
> void __iomem *devm_request_and_ioremap(struct device *dev,
> - struct resource *res);
> + struct resource *res) __deprecated;
>
> struct device_dma_parameters {
> /*
> diff --git a/lib/devres.c b/lib/devres.c
> index 80b9c76..9c76b3a 100644
> --- a/lib/devres.c
> +++ b/lib/devres.c
> @@ -86,22 +86,24 @@ void devm_iounmap(struct device *dev, void __iomem *addr)
> EXPORT_SYMBOL(devm_iounmap);
>
> /**
> - * devm_request_and_ioremap() - Check, request region, and ioremap resource
> - * @dev: Generic device to handle the resource for
> + * devm_ioremap_resource() - check, request region, and ioremap resource
> + * @dev: generic device to handle the resource for
> * @res: resource to be handled
> *
> - * Takes all necessary steps to ioremap a mem resource. Uses managed device, so
> - * everything is undone on driver detach. Checks arguments, so you can feed
> - * it the result from e.g. platform_get_resource() directly. Returns the
> - * remapped pointer or NULL on error. Usage example:
> + * Checks that a resource is a valid memory region, requests the memory region
> + * and ioremaps it either as cacheable or as non-cacheable memory depending on
> + * the resource's flags. All operations are managed and will be undone on
> + * driver detach.
> + *
> + * Returns a pointer to the remapped memory or an ERR_PTR() encoded error code
> + * on failure. Usage example:
> *
> * res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - * base = devm_request_and_ioremap(&pdev->dev, res);
> - * if (!base)
> - * return -EADDRNOTAVAIL;
> + * base = devm_ioremap_resource(&pdev->dev, res);
> + * if (IS_ERR(base))
> + * return PTR_ERR(base);
> */
> -void __iomem *devm_request_and_ioremap(struct device *dev,
> - struct resource *res)
> +void __iomem *devm_ioremap_resource(struct device *dev, struct resource *res)
> {
> resource_size_t size;
> const char *name;
> @@ -111,7 +113,7 @@ void __iomem *devm_request_and_ioremap(struct device *dev,
>
> if (!res || resource_type(res) != IORESOURCE_MEM) {
> dev_err(dev, "invalid resource\n");
> - return NULL;
> + return ERR_PTR(-EINVAL);
> }
>
> size = resource_size(res);
> @@ -119,7 +121,7 @@ void __iomem *devm_request_and_ioremap(struct device *dev,
>
> if (!devm_request_mem_region(dev, res->start, size, name)) {
> dev_err(dev, "can't request region for resource %pR\n", res);
> - return NULL;
> + return ERR_PTR(-EBUSY);
> }
>
> if (res->flags & IORESOURCE_CACHEABLE)
> @@ -130,10 +132,39 @@ void __iomem *devm_request_and_ioremap(struct device *dev,
> if (!dest_ptr) {
> dev_err(dev, "ioremap failed for resource %pR\n", res);
> devm_release_mem_region(dev, res->start, size);
> + dest_ptr = ERR_PTR(-ENOMEM);
> }
>
> return dest_ptr;
> }
> +EXPORT_SYMBOL(devm_ioremap_resource);
> +
> +/**
> + * devm_request_and_ioremap() - Check, request region, and ioremap resource
> + * @dev: Generic device to handle the resource for
> + * @res: resource to be handled
> + *
> + * Takes all necessary steps to ioremap a mem resource. Uses managed device, so
> + * everything is undone on driver detach. Checks arguments, so you can feed
> + * it the result from e.g. platform_get_resource() directly. Returns the
> + * remapped pointer or NULL on error. Usage example:
> + *
> + * res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + * base = devm_request_and_ioremap(&pdev->dev, res);
> + * if (!base)
> + * return -EADDRNOTAVAIL;
> + */
> +void __iomem *devm_request_and_ioremap(struct device *device,
> + struct resource *res)
> +{
> + void __iomem *dest_ptr;
> +
> + dest_ptr = devm_ioremap_resource(device, res);
> + if (IS_ERR(dest_ptr))
> + return NULL;
> +
> + return dest_ptr;
> +}
> EXPORT_SYMBOL(devm_request_and_ioremap);
>
> #ifdef CONFIG_HAS_IOPORT
> diff --git a/scripts/coccinelle/api/devm_ioremap_resource.cocci b/scripts/coccinelle/api/devm_ioremap_resource.cocci
> new file mode 100644
> index 0000000..495daa3
> --- /dev/null
> +++ b/scripts/coccinelle/api/devm_ioremap_resource.cocci
> @@ -0,0 +1,90 @@
> +virtual patch
> +virtual report
> +
> +@...ends on patch@
> +expression base, dev, res;
> +@@
> +
> +-base = devm_request_and_ioremap(dev, res);
> ++base = devm_ioremap_resource(dev, res);
> + ...
> + if (
> +-base == NULL
> ++IS_ERR(base)
> + || ...) {
> +<...
> +- return ...;
> ++ return PTR_ERR(base);
> +...>
> + }
> +
> +@...ends on patch@
> +expression e, E, ret;
> +identifier l;
> +@@
> +
> + e = devm_ioremap_resource(...);
> + ...
> + if (IS_ERR(e) || ...) {
> + ... when any
> +- ret = E;
> ++ ret = PTR_ERR(e);
> + ...
> +(
> + return ret;
> +|
> + goto l;
> +)
> + }
> +
> +@...ends on patch@
> +expression e;
> +@@
> +
> + e = devm_ioremap_resource(...);
> + ...
> + if (IS_ERR(e) || ...) {
> + ...
> +- \(dev_dbg\|dev_err\|pr_debug\|pr_err\|DRM_ERROR\)(...);
> + ...
> + }
> +
> +@...ends on patch@
> +expression e;
> +identifier l;
> +@@
> +
> + e = devm_ioremap_resource(...);
> + ...
> + if (IS_ERR(e) || ...)
> +-{
> +(
> + return ...;
> +|
> + goto l;
> +)
> +-}
> +
> +@r depends on report@
> +expression e;
> +identifier l;
> +position p1;
> +@@
> +
> +*e = devm_request_and_ioremap@p1(...);
> + ...
> + if (e == NULL || ...) {
> + ...
> +(
> + return ...;
> +|
> + goto l;
> +)
> + }
> +
> +@...ipt:python depends on r@
> +p1 << r.p1;
> +@@
> +
> +msg = "ERROR: deprecated devm_request_and_ioremap() API used on line %s" % (p1[0].line)
> +coccilib.report.print_report(p1[0], msg)
> --
> 1.8.1.1
>
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists