[PATCH] drivers: of: Export of_reserved_mem_device_{init,release}

[PATCH] drivers: of: Export of_reserved_mem_device_{init,release}

[George G. Davis]

Export of_reserved_mem_device_{init,release} so that modules
can initialize and release their assigned per-device cma_area.

Signed-off-by: George G. Davis <george_davis@mentor.com>
---
 drivers/of/of_reserved_mem.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index dc566b3..5eeb5b2 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -265,6 +265,7 @@ int of_reserved_mem_device_init(struct device *dev)
 
 	return ret;
 }
+EXPORT_SYMBOL(of_reserved_mem_device_init);
 
 /**
  * of_reserved_mem_device_release() - release reserved memory device structures
@@ -289,3 +290,4 @@ void of_reserved_mem_device_release(struct device *dev)
 
 	rmem->ops->device_release(rmem, dev);
 }
+EXPORT_SYMBOL(of_reserved_mem_device_release);
-- 
1.9.3

Re: [PATCH] drivers: of: Export of_reserved_mem_device_{init,release}

[Rob Herring]

On Fri, Jan 9, 2015 at 8:29 AM, George G. Davis <ggdavisiv@gmail.com> wrote:
> Export of_reserved_mem_device_{init,release} so that modules
> can initialize and release their assigned per-device cma_area.

I believe the original intent was for the core bus code to call these
functions. While the commit adding them says "automated assignment"
that part did not go in. As it stands, there are no in tree users of
these functions.

Marek, comments?

Rob

>
> Signed-off-by: George G. Davis <george_davis@mentor.com>
> ---
>  drivers/of/of_reserved_mem.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index dc566b3..5eeb5b2 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -265,6 +265,7 @@ int of_reserved_mem_device_init(struct device *dev)
>
>         return ret;
>  }
> +EXPORT_SYMBOL(of_reserved_mem_device_init);
>
>  /**
>   * of_reserved_mem_device_release() - release reserved memory device structures
> @@ -289,3 +290,4 @@ void of_reserved_mem_device_release(struct device *dev)
>
>         rmem->ops->device_release(rmem, dev);
>  }
> +EXPORT_SYMBOL(of_reserved_mem_device_release);
> --
> 1.9.3
>

Re: [PATCH] drivers: of: Export of_reserved_mem_device_{init,release}

[George G. Davis]

On Fri, Jan 09, 2015 at 11:53:01AM -0600, Rob Herring wrote:
> On Fri, Jan 9, 2015 at 8:29 AM, George G. Davis <ggdavisiv@gmail.com> wrote:
> > Export of_reserved_mem_device_{init,release} so that modules
> > can initialize and release their assigned per-device cma_area.
> 
> I believe the original intent was for the core bus code to call these
> functions. While the commit adding them says "automated assignment"
> that part did not go in.

I suspected that there may have been a bit of missing magic.  The only
use of these functions that I was able to find was in an earlier out of
tree patch series where these functions were called from the s5p-mfc
driver:

http://lists.linaro.org/pipermail/linaro-mm-sig/2014-August/004140.html

I agree that "automated assignment" via core bus code would be a better
solution but I don't know where to add the calls.

> As it stands, there are no in tree users of
> these functions.
> 
> Marek, comments?

Thanks!

--
Regards,
George

> 
> Rob
> 
> >
> > Signed-off-by: George G. Davis <george_davis@mentor.com>
> > ---
> >  drivers/of/of_reserved_mem.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> > index dc566b3..5eeb5b2 100644
> > --- a/drivers/of/of_reserved_mem.c
> > +++ b/drivers/of/of_reserved_mem.c
> > @@ -265,6 +265,7 @@ int of_reserved_mem_device_init(struct device *dev)
> >
> >         return ret;
> >  }
> > +EXPORT_SYMBOL(of_reserved_mem_device_init);
> >
> >  /**
> >   * of_reserved_mem_device_release() - release reserved memory device structures
> > @@ -289,3 +290,4 @@ void of_reserved_mem_device_release(struct device *dev)
> >
> >         rmem->ops->device_release(rmem, dev);
> >  }
> > +EXPORT_SYMBOL(of_reserved_mem_device_release);
> > --
> > 1.9.3
> >

Re: [PATCH] drivers: of: Export of_reserved_mem_device_{init,release}

[George G. Davis]

On Fri, Jan 09, 2015 at 01:33:10PM -0500, George G. Davis wrote:
> On Fri, Jan 09, 2015 at 11:53:01AM -0600, Rob Herring wrote:
> > On Fri, Jan 9, 2015 at 8:29 AM, George G. Davis <ggdavisiv@gmail.com> wrote:
> > > Export of_reserved_mem_device_{init,release} so that modules
> > > can initialize and release their assigned per-device cma_area.
> > 
> > I believe the original intent was for the core bus code to call these
> > functions. While the commit adding them says "automated assignment"
> > that part did not go in.
> 
> I suspected that there may have been a bit of missing magic.  The only
> use of these functions that I was able to find was in an earlier out of
> tree patch series where these functions were called from the s5p-mfc
> driver:
> 
> http://lists.linaro.org/pipermail/linaro-mm-sig/2014-August/004140.html
> 
> I agree that "automated assignment" via core bus code would be a better
> solution but I don't know where to add the calls.

How about something like the following for automated assignment?:

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 9421fed..231b427 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -14,6 +14,7 @@
 #include <linux/platform_device.h>
 #include <linux/of_device.h>
 #include <linux/of_irq.h>
+#include <linux/of_reserved_mem.h>
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/dma-mapping.h>
@@ -509,9 +510,12 @@ static int platform_drv_probe(struct device *_dev)
 
 	ret = dev_pm_domain_attach(_dev, true);
 	if (ret != -EPROBE_DEFER) {
+		of_reserved_mem_device_init(_dev);
 		ret = drv->probe(dev);
-		if (ret)
+		if (ret) {
+			of_reserved_mem_device_release(_dev);
 			dev_pm_domain_detach(_dev, true);
+		}
 	}
 
 	if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) {
@@ -534,6 +538,7 @@ static int platform_drv_remove(struct device *_dev)
 	int ret;
 
 	ret = drv->remove(dev);
+	of_reserved_mem_device_release(_dev);
 	dev_pm_domain_detach(_dev, true);
 
 	return ret;


--
Regards,
George
> 
> > As it stands, there are no in tree users of
> > these functions.
> > 
> > Marek, comments?
> 
> Thanks!
> 
> --
> Regards,
> George
> 
> > 
> > Rob
> > 
> > >
> > > Signed-off-by: George G. Davis <george_davis@mentor.com>
> > > ---
> > >  drivers/of/of_reserved_mem.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> > > index dc566b3..5eeb5b2 100644
> > > --- a/drivers/of/of_reserved_mem.c
> > > +++ b/drivers/of/of_reserved_mem.c
> > > @@ -265,6 +265,7 @@ int of_reserved_mem_device_init(struct device *dev)
> > >
> > >         return ret;
> > >  }
> > > +EXPORT_SYMBOL(of_reserved_mem_device_init);
> > >
> > >  /**
> > >   * of_reserved_mem_device_release() - release reserved memory device structures
> > > @@ -289,3 +290,4 @@ void of_reserved_mem_device_release(struct device *dev)
> > >
> > >         rmem->ops->device_release(rmem, dev);
> > >  }
> > > +EXPORT_SYMBOL(of_reserved_mem_device_release);
> > > --
> > > 1.9.3
> > >

Re: [PATCH] drivers: of: Export of_reserved_mem_device_{init,release}

[Rob Herring]

On Fri, Jan 9, 2015 at 4:05 PM, George G. Davis <ggdavisiv@gmail.com> wrote:
> On Fri, Jan 09, 2015 at 01:33:10PM -0500, George G. Davis wrote:
>> On Fri, Jan 09, 2015 at 11:53:01AM -0600, Rob Herring wrote:
>> > On Fri, Jan 9, 2015 at 8:29 AM, George G. Davis <ggdavisiv@gmail.com> wrote:
>> > > Export of_reserved_mem_device_{init,release} so that modules
>> > > can initialize and release their assigned per-device cma_area.
>> >
>> > I believe the original intent was for the core bus code to call these
>> > functions. While the commit adding them says "automated assignment"
>> > that part did not go in.
>>
>> I suspected that there may have been a bit of missing magic.  The only
>> use of these functions that I was able to find was in an earlier out of
>> tree patch series where these functions were called from the s5p-mfc
>> driver:
>>
>> http://lists.linaro.org/pipermail/linaro-mm-sig/2014-August/004140.html
>>
>> I agree that "automated assignment" via core bus code would be a better
>> solution but I don't know where to add the calls.
>
> How about something like the following for automated assignment?:

It seems Grant wasn't happy about the core code doing this[1]. So I
guess your original patch was okay except you should use
EXPORT_SYMBOL_GPL. I can fix that up if you agree.

Rob


[1] https://lkml.org/lkml/2014/3/2/30

>
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index 9421fed..231b427 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -14,6 +14,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/of_device.h>
>  #include <linux/of_irq.h>
> +#include <linux/of_reserved_mem.h>
>  #include <linux/module.h>
>  #include <linux/init.h>
>  #include <linux/dma-mapping.h>
> @@ -509,9 +510,12 @@ static int platform_drv_probe(struct device *_dev)
>
>         ret = dev_pm_domain_attach(_dev, true);
>         if (ret != -EPROBE_DEFER) {
> +               of_reserved_mem_device_init(_dev);
>                 ret = drv->probe(dev);
> -               if (ret)
> +               if (ret) {
> +                       of_reserved_mem_device_release(_dev);
>                         dev_pm_domain_detach(_dev, true);
> +               }
>         }
>
>         if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) {
> @@ -534,6 +538,7 @@ static int platform_drv_remove(struct device *_dev)
>         int ret;
>
>         ret = drv->remove(dev);
> +       of_reserved_mem_device_release(_dev);
>         dev_pm_domain_detach(_dev, true);
>
>         return ret;
>
>
> --
> Regards,
> George
>>
>> > As it stands, there are no in tree users of
>> > these functions.
>> >
>> > Marek, comments?
>>
>> Thanks!
>>
>> --
>> Regards,
>> George
>>
>> >
>> > Rob
>> >
>> > >
>> > > Signed-off-by: George G. Davis <george_davis@mentor.com>
>> > > ---
>> > >  drivers/of/of_reserved_mem.c | 2 ++
>> > >  1 file changed, 2 insertions(+)
>> > >
>> > > diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
>> > > index dc566b3..5eeb5b2 100644
>> > > --- a/drivers/of/of_reserved_mem.c
>> > > +++ b/drivers/of/of_reserved_mem.c
>> > > @@ -265,6 +265,7 @@ int of_reserved_mem_device_init(struct device *dev)
>> > >
>> > >         return ret;
>> > >  }
>> > > +EXPORT_SYMBOL(of_reserved_mem_device_init);
>> > >
>> > >  /**
>> > >   * of_reserved_mem_device_release() - release reserved memory device structures
>> > > @@ -289,3 +290,4 @@ void of_reserved_mem_device_release(struct device *dev)
>> > >
>> > >         rmem->ops->device_release(rmem, dev);
>> > >  }
>> > > +EXPORT_SYMBOL(of_reserved_mem_device_release);
>> > > --
>> > > 1.9.3
>> > >

Re: [PATCH] drivers: of: Export of_reserved_mem_device_{init,release}

[George G. Davis]

On Fri, Jan 09, 2015 at 04:29:43PM -0600, Rob Herring wrote:
> On Fri, Jan 9, 2015 at 4:05 PM, George G. Davis <ggdavisiv@gmail.com> wrote:
> > On Fri, Jan 09, 2015 at 01:33:10PM -0500, George G. Davis wrote:
> >> On Fri, Jan 09, 2015 at 11:53:01AM -0600, Rob Herring wrote:
> >> > On Fri, Jan 9, 2015 at 8:29 AM, George G. Davis <ggdavisiv@gmail.com> wrote:
> >> > > Export of_reserved_mem_device_{init,release} so that modules
> >> > > can initialize and release their assigned per-device cma_area.
> >> >
> >> > I believe the original intent was for the core bus code to call these
> >> > functions. While the commit adding them says "automated assignment"
> >> > that part did not go in.
> >>
> >> I suspected that there may have been a bit of missing magic.  The only
> >> use of these functions that I was able to find was in an earlier out of
> >> tree patch series where these functions were called from the s5p-mfc
> >> driver:
> >>
> >> http://lists.linaro.org/pipermail/linaro-mm-sig/2014-August/004140.html
> >>
> >> I agree that "automated assignment" via core bus code would be a better
> >> solution but I don't know where to add the calls.
> >
> > How about something like the following for automated assignment?:
> 
> It seems Grant wasn't happy about the core code doing this[1].

For what it's worth, I was rather liking the automagic assignments since
no driver changes were necessary for that approach but I do agree with
the rationale for not adding it to core bus code.

> So I
> guess your original patch was okay except you should use
> EXPORT_SYMBOL_GPL. I can fix that up if you agree.

Yes, please go ahead.


Thanks!

--
Regards,
George

> 
> Rob
> 
> 
> [1] https://lkml.org/lkml/2014/3/2/30
> 
> >
> > diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> > index 9421fed..231b427 100644
> > --- a/drivers/base/platform.c
> > +++ b/drivers/base/platform.c
> > @@ -14,6 +14,7 @@
> >  #include <linux/platform_device.h>
> >  #include <linux/of_device.h>
> >  #include <linux/of_irq.h>
> > +#include <linux/of_reserved_mem.h>
> >  #include <linux/module.h>
> >  #include <linux/init.h>
> >  #include <linux/dma-mapping.h>
> > @@ -509,9 +510,12 @@ static int platform_drv_probe(struct device *_dev)
> >
> >         ret = dev_pm_domain_attach(_dev, true);
> >         if (ret != -EPROBE_DEFER) {
> > +               of_reserved_mem_device_init(_dev);
> >                 ret = drv->probe(dev);
> > -               if (ret)
> > +               if (ret) {
> > +                       of_reserved_mem_device_release(_dev);
> >                         dev_pm_domain_detach(_dev, true);
> > +               }
> >         }
> >
> >         if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) {
> > @@ -534,6 +538,7 @@ static int platform_drv_remove(struct device *_dev)
> >         int ret;
> >
> >         ret = drv->remove(dev);
> > +       of_reserved_mem_device_release(_dev);
> >         dev_pm_domain_detach(_dev, true);
> >
> >         return ret;
> >
> >
> > --
> > Regards,
> > George
> >>
> >> > As it stands, there are no in tree users of
> >> > these functions.
> >> >
> >> > Marek, comments?
> >>
> >> Thanks!
> >>
> >> --
> >> Regards,
> >> George
> >>
> >> >
> >> > Rob
> >> >
> >> > >
> >> > > Signed-off-by: George G. Davis <george_davis@mentor.com>
> >> > > ---
> >> > >  drivers/of/of_reserved_mem.c | 2 ++
> >> > >  1 file changed, 2 insertions(+)
> >> > >
> >> > > diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> >> > > index dc566b3..5eeb5b2 100644
> >> > > --- a/drivers/of/of_reserved_mem.c
> >> > > +++ b/drivers/of/of_reserved_mem.c
> >> > > @@ -265,6 +265,7 @@ int of_reserved_mem_device_init(struct device *dev)
> >> > >
> >> > >         return ret;
> >> > >  }
> >> > > +EXPORT_SYMBOL(of_reserved_mem_device_init);
> >> > >
> >> > >  /**
> >> > >   * of_reserved_mem_device_release() - release reserved memory device structures
> >> > > @@ -289,3 +290,4 @@ void of_reserved_mem_device_release(struct device *dev)
> >> > >
> >> > >         rmem->ops->device_release(rmem, dev);
> >> > >  }
> >> > > +EXPORT_SYMBOL(of_reserved_mem_device_release);
> >> > > --
> >> > > 1.9.3
> >> > >

Re: [PATCH] drivers: of: Export of_reserved_mem_device_{init,release}

[Marek Szyprowski]

Hello,

On 2015-01-09 18:53, Rob Herring wrote:
> On Fri, Jan 9, 2015 at 8:29 AM, George G. Davis <ggdavisiv@gmail.com> wrote:
>> Export of_reserved_mem_device_{init,release} so that modules
>> can initialize and release their assigned per-device cma_area.
> I believe the original intent was for the core bus code to call these
> functions. While the commit adding them says "automated assignment"
> that part did not go in. As it stands, there are no in tree users of
> these functions.
>
> Marek, comments?

Yes, my idea was to call it directly from platform bus code, but Grant
opposed, so right now the drivers need to do it on their own. Assuming
that those functions need to be called from drivers, then EXPORT_SYMPOL_GPL
is the correct approach.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland