linux-kernel - Re: [PATCH v5 06/11] drivers: of: initialize and assign reserved memory to newly created devices

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 26 Feb 2014 12:14:45 +0000
From:	Grant Likely <grant.likely@...aro.org>
To:	Marek Szyprowski <m.szyprowski@...sung.com>,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linaro-mm-sig@...ts.linaro.org, devicetree@...r.kernel.org,
	linux-doc@...r.kernel.org
Cc:	Marek Szyprowski <m.szyprowski@...sung.com>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Arnd Bergmann <arnd@...db.de>,
	Michal Nazarewicz <mina86@...a86.com>,
	Tomasz Figa <t.figa@...sung.com>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	Laura Abbott <lauraa@...eaurora.org>,
	Rob Herring <robh+dt@...nel.org>,
	Olof Johansson <olof@...om.net>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Stephen Warren <swarren@...dotorg.org>,
	Ian Campbell <ian.campbell@...rix.com>,
	Tomasz Figa <tomasz.figa@...il.com>,
	Kumar Gala <galak@...eaurora.org>,
	Nishanth Peethambaran <nishanth.p@...il.com>,
	Marc <marc.ceeeee@...il.com>,
	Josh Cartwright <joshc@...eaurora.org>,
	Catalin Marinas <catalin.marinas@....com>,
	Will Deacon <will.deacon@....com>,
	Paul Mackerras <paulus@...ba.org>
Subject: Re: [PATCH v5 06/11] drivers: of: initialize and assign reserved memory to newly created devices

On Fri, 21 Feb 2014 13:25:22 +0100, Marek Szyprowski <m.szyprowski@...sung.com> wrote:
> Use recently introduced of_reserved_mem_device_init() function to
> automatically assign respective reserved memory region to the newly created
> platform and amba device.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@...sung.com>

I'm wary on this patch. It hides the assignment of regions into the core
code and I worry that it is the wrong level of abstraction. I would
think that drivers should know that they need a reserved memory region
and should be calling the API to obtain the region directly rather than
doing it for them. The reason being is that there may be some situations
where the common code isn't quite right and the driver needs to override
the behaviour. If it is called automatically then the driver cannot do
that.

Is it really a big burden to have the driver call the reserved memory
init function?

g.

> ---
>  drivers/of/platform.c |    7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 404d1daebefa..3df0b1826e8b 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -21,6 +21,7 @@
>  #include <linux/of_device.h>
>  #include <linux/of_irq.h>
>  #include <linux/of_platform.h>
> +#include <linux/of_reserved_mem.h>
>  #include <linux/platform_device.h>
>  
>  const struct of_device_id of_default_bus_match_table[] = {
> @@ -220,6 +221,8 @@ static struct platform_device *of_platform_device_create_pdata(
>  	dev->dev.bus = &platform_bus_type;
>  	dev->dev.platform_data = platform_data;
>  
> +	of_reserved_mem_device_init(&dev->dev);
> +
>  	/* We do not fill the DMA ops for platform devices by default.
>  	 * This is currently the responsibility of the platform code
>  	 * to do such, possibly using a device notifier
> @@ -227,6 +230,7 @@ static struct platform_device *of_platform_device_create_pdata(
>  
>  	if (of_device_add(dev) != 0) {
>  		platform_device_put(dev);
> +		of_reserved_mem_device_release(&dev->dev);
>  		return NULL;
>  	}
>  
> @@ -282,6 +286,8 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
>  	else
>  		of_device_make_bus_id(&dev->dev);
>  
> +	of_reserved_mem_device_init(&dev->dev);
> +
>  	/* Allow the HW Peripheral ID to be overridden */
>  	prop = of_get_property(node, "arm,primecell-periphid", NULL);
>  	if (prop)
> @@ -308,6 +314,7 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
>  	return dev;
>  
>  err_free:
> +	of_reserved_mem_device_release(&dev->dev);
>  	amba_device_put(dev);
>  	return NULL;
>  }
> -- 
> 1.7.9.5
> 

--
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/