| Message ID | 1405326487-15346-3-git-send-email-m.szyprowski@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 14 Jul 2014 10:28:05 +0200, Marek Szyprowski <m.szyprowski@samsung.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@samsung.com> I'm still not okay with this patch. I don't think it is appropriate to add the hook into the generic path that gets used for every platform device. The devices that need reserved memory should explicitly request it, and any setup be done at that time. g. > --- > drivers/of/platform.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/of/platform.c b/drivers/of/platform.c > index 500436f9be7f..a9055d3da5c2 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[] = { > @@ -233,12 +234,15 @@ 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 > */ > > if (of_device_add(dev) != 0) { > + of_reserved_mem_device_release(&dev->dev); > platform_device_put(dev); > goto err_clear_flag; > } > @@ -300,6 +304,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) > @@ -326,6 +332,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); > err_clear_flag: > of_node_clear_flag(node, OF_POPULATED); > -- > 1.9.2 >
Hello, On 2014-07-28 16:17, Grant Likely wrote: > On Mon, 14 Jul 2014 10:28:05 +0200, Marek Szyprowski <m.szyprowski@samsung.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@samsung.com> > I'm still not okay with this patch. I don't think it is appropriate to > add the hook into the generic path that gets used for every platform > device. The devices that need reserved memory should explicitly request > it, and any setup be done at that time. Okay... I thought that it will be easier to have it done in generic code, if You don't think so, then I give up and we will add it in all drivers requiring such memory regions. What about patch 3/4 and 4/4? Would it be possible to have your ack to get them merged? Right now patch 4/4 depends on changes from akpm tree, so it will be best to merge them to akpm tree. Best regards
On Tue, 29 Jul 2014 16:47:04 +0200, Marek Szyprowski <m.szyprowski@samsung.com> wrote: > Hello, > > On 2014-07-28 16:17, Grant Likely wrote: > > On Mon, 14 Jul 2014 10:28:05 +0200, Marek Szyprowski <m.szyprowski@samsung.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@samsung.com> > > I'm still not okay with this patch. I don't think it is appropriate to > > add the hook into the generic path that gets used for every platform > > device. The devices that need reserved memory should explicitly request > > it, and any setup be done at that time. > > Okay... I thought that it will be easier to have it done in generic > code, if You don't > think so, then I give up and we will add it in all drivers requiring > such memory regions. The problem is that it makes the big assumption that every device is going to conform to the pattern without any recourse if something special needs to be done (such as to handle quirks). It also puts this code into the probe path of every device, when the vast majority of device drivers just don't care. When code like this is run unconditionally from generic code, it becomes really difficult to override when needed. I've made the same comment on the PM clock domain patches. > What about patch 3/4 and 4/4? Would it be possible to have your ack to > get them merged? > Right now patch 4/4 depends on changes from akpm tree, so it will be > best to merge them > to akpm tree. I'll take a look. g.
diff --git a/drivers/of/platform.c b/drivers/of/platform.c index 500436f9be7f..a9055d3da5c2 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[] = { @@ -233,12 +234,15 @@ 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 */ if (of_device_add(dev) != 0) { + of_reserved_mem_device_release(&dev->dev); platform_device_put(dev); goto err_clear_flag; } @@ -300,6 +304,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) @@ -326,6 +332,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); err_clear_flag: of_node_clear_flag(node, OF_POPULATED);
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@samsung.com> --- drivers/of/platform.c | 7 +++++++ 1 file changed, 7 insertions(+)