[RFC 0/2] staging: ion: of_ion_device_get

[RFC 0/2] staging: ion: of_ion_device_get

[Moritz Fischer]

Hi all,

probably I'm doing this all wrong. I'm playing around with a bunch of accelerators
and I need to share buffers between them. I have my heaps implemented as a
platform device and from what I understand I'll need a reference to the
struct ion_device in each of them.
I couldn't find a way to get to that via devicetree, though. I'll admit that hacking
up miscdevice is quite hacky, maybe I should add a foo_get_ion_device() to my
heap implementing platform device?
If someone can explain me how to correctly do that with what we currently have,
even better.

Cheers,

   Moritz

Moritz Fischer (2):
  misc: Add of_get_misc get a reference from devicetree
  staging: android: ion: Add of_ion_device_get function

 drivers/char/misc.c               | 38 ++++++++++++++++++++++++++++++++++++++
 drivers/staging/android/ion/ion.c | 10 ++++++++++
 include/linux/miscdevice.h        |  3 +++
 3 files changed, 51 insertions(+)

-- 
2.7.4

[RFC 1/2] misc: Add of_get_misc get a reference from devicetree

[Moritz Fischer]

This commit enables access to a miscdevice via a reference
obtained from devicetree.
This allows to implement a of_ion_device_get() in the next step.

Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
---
 drivers/char/misc.c        | 38 ++++++++++++++++++++++++++++++++++++++
 include/linux/miscdevice.h |  3 +++
 2 files changed, 41 insertions(+)

diff --git a/drivers/char/misc.c b/drivers/char/misc.c
index 8069b36..0623834 100644
--- a/drivers/char/misc.c
+++ b/drivers/char/misc.c
@@ -275,6 +275,44 @@ static char *misc_devnode(struct device *dev, umode_t *mode)
 	return NULL;
 }
 
+#ifdef CONFIG_OF
+static int misc_of_node_match(struct device *dev, const void *data)
+{
+	return dev->of_node == data;
+}
+
+struct miscdevice *of_misc_get(struct device_node *node)
+{
+	struct miscdevice *m;
+	struct device *dev;
+	int ret = -ENODEV;
+
+	dev = class_find_device(misc_class, NULL, node,
+				misc_of_node_match);
+	if (!dev)
+		return ERR_PTR(-ENODEV);
+
+	m = dev_get_drvdata(dev);
+	if (!m)
+		goto err_dev;
+
+	if (!try_module_get(dev->parent->driver->owner))
+		goto err_dev;
+
+	return m;
+
+err_dev:
+	put_device(dev);
+	return ERR_PTR(ret);
+}
+#else
+struct misc_device *of_misc_get(struct device_node *)
+{
+	return ERR_PTR(-ENODEV);
+}
+#endif
+EXPORT_SYMBOL(of_misc_get);
+
 static int __init misc_init(void)
 {
 	int err;
diff --git a/include/linux/miscdevice.h b/include/linux/miscdevice.h
index 5430374..a4b605c 100644
--- a/include/linux/miscdevice.h
+++ b/include/linux/miscdevice.h
@@ -53,6 +53,7 @@
 #define MISC_DYNAMIC_MINOR	255
 
 struct device;
+struct device_node;
 struct attribute_group;
 
 struct miscdevice  {
@@ -70,6 +71,8 @@ struct miscdevice  {
 extern int misc_register(struct miscdevice *misc);
 extern void misc_deregister(struct miscdevice *misc);
 
+extern struct miscdevice *of_misc_get(struct device_node *node);
+
 #define MODULE_ALIAS_MISCDEV(minor)				\
 	MODULE_ALIAS("char-major-" __stringify(MISC_MAJOR)	\
 	"-" __stringify(minor))
-- 
2.7.4

Re: [RFC 1/2] misc: Add of_get_misc get a reference from devicetree

[Moritz Fischer]

Meh,

On Tue, Mar 22, 2016 at 3:33 PM, Moritz Fischer
<moritz.fischer@ettus.com> wrote:

>
> +
> +err_dev:
> +       put_device(dev);
> +       return ERR_PTR(ret);
> +}
> +#else
> +struct misc_device *of_misc_get(struct device_node *)

Ok, that one is broken ....

Sorry,

Moritz

[RFC 2/2] staging: android: ion: Add of_ion_device_get function

[Moritz Fischer]

Allows to obtain a reference to the global /dev/ion backing
struct ion_device via devicetree.

Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
---
 drivers/staging/android/ion/ion.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index e237e9f..cea264e0 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -64,6 +64,16 @@ struct ion_device {
 	struct dentry *clients_debug_root;
 };
 
+struct ion_device *of_ion_device_get(struct device_node *node)
+{
+	struct miscdevice *mdev = of_misc_get(node);
+
+	if (IS_ERR(mdev))
+		return ERR_PTR(PTR_ERR(mdev));
+
+	return container_of(mdev, struct ion_device, dev);
+}
+
 /**
  * struct ion_client - a process/hw block local address space
  * @node:		node in the tree of all clients
-- 
2.7.4

Re: [RFC 2/2] staging: android: ion: Add of_ion_device_get function

[Moritz Fischer]

Derp,

On Tue, Mar 22, 2016 at 3:33 PM, Moritz Fischer
<moritz.fischer@ettus.com> wrote:
> Allows to obtain a reference to the global /dev/ion backing
> struct ion_device via devicetree.
>
> Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
> ---
>  drivers/staging/android/ion/ion.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
> index e237e9f..cea264e0 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -64,6 +64,16 @@ struct ion_device {
>         struct dentry *clients_debug_root;
>  };
>
> +struct ion_device *of_ion_device_get(struct device_node *node)
> +{
> +       struct miscdevice *mdev = of_misc_get(node);
> +
> +       if (IS_ERR(mdev))
> +               return ERR_PTR(PTR_ERR(mdev));
> +
> +       return container_of(mdev, struct ion_device, dev);
> +}
> +
>  /**
>   * struct ion_client - a process/hw block local address space
>   * @node:              node in the tree of all clients
> --
> 2.7.4
>

It's missing the header file ... if it's deemed useful at all I'll
clean up and resubmit.

Moritz

Re: [RFC 2/2] staging: android: ion: Add of_ion_device_get function

[Dan Carpenter]

On Tue, Mar 22, 2016 at 03:33:51PM -0700, Moritz Fischer wrote:
> +struct ion_device *of_ion_device_get(struct device_node *node)
> +{
> +	struct miscdevice *mdev = of_misc_get(node);
> +
> +	if (IS_ERR(mdev))
> +		return ERR_PTR(PTR_ERR(mdev));

Use ERR_CAST() for this.

regarda,
dan carpenter

Re: [RFC 0/2] staging: ion: of_ion_device_get

[Laura Abbott]

On 03/22/2016 03:33 PM, Moritz Fischer wrote:
> Hi all,
>
> probably I'm doing this all wrong. I'm playing around with a bunch of accelerators
> and I need to share buffers between them. I have my heaps implemented as a
> platform device and from what I understand I'll need a reference to the
> struct ion_device in each of them.
> I couldn't find a way to get to that via devicetree, though. I'll admit that hacking
> up miscdevice is quite hacky, maybe I should add a foo_get_ion_device() to my
> heap implementing platform device?
> If someone can explain me how to correctly do that with what we currently have,
> even better.
>
> Cheers,
>
>     Moritz
>
> Moritz Fischer (2):
>    misc: Add of_get_misc get a reference from devicetree
>    staging: android: ion: Add of_ion_device_get function
>
>   drivers/char/misc.c               | 38 ++++++++++++++++++++++++++++++++++++++
>   drivers/staging/android/ion/ion.c | 10 ++++++++++
>   include/linux/miscdevice.h        |  3 +++
>   3 files changed, 51 insertions(+)
>

In the past what drivers have done is a foo_ion_client_create which has the reference
to the ion_device created from ion_device_create. Drivers then call the
foo_ion_client_create function.

Can you elaborate more on your sharing and allocation flow? This might suggest
another idea.

Thanks,
Laura

Re: [RFC 0/2] staging: ion: of_ion_device_get

[Moritz Fischer]

Hi Laura,

On Tue, Mar 22, 2016 at 3:51 PM, Laura Abbott <labbott@redhat.com> wrote:

> In the past what drivers have done is a foo_ion_client_create which has the
> reference
> to the ion_device created from ion_device_create. Drivers then call the
> foo_ion_client_create function.

Oh, so you mean you add a function to create a client to the platform
device implementing
the heap and the export this function?

heap implements:

foo_create_client();

driver calls:

foo_create_client() ?

> Can you elaborate more on your sharing and allocation flow? This might
> suggest
> another idea.

Well I'll have a bunch of DMA streams to / from an FPGA that contains
DMA engines & accelerators. To that end
my userland software would allocate say 64 buffers, hand them over to
driver A to queue/deque them.

In future I might want to share these buffers between streams and with
other peripherals on the same bus,
which is why I looked at ION.

Thanks,

Moritz

Re: [RFC 0/2] staging: ion: of_ion_device_get

[Laura Abbott]

On 03/22/2016 04:08 PM, Moritz Fischer wrote:
> Hi Laura,
>
> On Tue, Mar 22, 2016 at 3:51 PM, Laura Abbott <labbott@redhat.com> wrote:
>
>> In the past what drivers have done is a foo_ion_client_create which has the
>> reference
>> to the ion_device created from ion_device_create. Drivers then call the
>> foo_ion_client_create function.
>
> Oh, so you mean you add a function to create a client to the platform
> device implementing
> the heap and the export this function?
>
> heap implements:
>
> foo_create_client();
>
> driver calls:
>
> foo_create_client() ?
>

Yes, exactly

>> Can you elaborate more on your sharing and allocation flow? This might
>> suggest
>> another idea.
>
> Well I'll have a bunch of DMA streams to / from an FPGA that contains
> DMA engines & accelerators. To that end
> my userland software would allocate say 64 buffers, hand them over to
> driver A to queue/deque them.
>
> In future I might want to share these buffers between streams and with
> other peripherals on the same bus,
> which is why I looked at ION.
>

If allocation is coming from userspace and drivers are only importing
you should be using the dma_buf APIs instead of Ion APIs directly.
Ion is a dma_buf exporter and dma_buf APIs are the preferred API.

> Thanks,
>
> Moritz
>

Thanks,
Laura

Re: [RFC 0/2] staging: ion: of_ion_device_get

[Moritz Fischer]

On Tue, Mar 22, 2016 at 4:20 PM, Laura Abbott <labbott@redhat.com> wrote:
> On 03/22/2016 04:08 PM, Moritz Fischer wrote:
>>
>> Hi Laura,
>>
>> On Tue, Mar 22, 2016 at 3:51 PM, Laura Abbott <labbott@redhat.com> wrote:
>>
>>> In the past what drivers have done is a foo_ion_client_create which has
>>> the
>>> reference
>>> to the ion_device created from ion_device_create. Drivers then call the
>>> foo_ion_client_create function.
>>
>>
>> Oh, so you mean you add a function to create a client to the platform
>> device implementing
>> the heap and the export this function?
>>
>> heap implements:
>>
>> foo_create_client();
>>
>> driver calls:
>>
>> foo_create_client() ?
>>
>
> Yes, exactly
>
>>> Can you elaborate more on your sharing and allocation flow? This might
>>> suggest
>>> another idea.
>>
>>
>> Well I'll have a bunch of DMA streams to / from an FPGA that contains
>> DMA engines & accelerators. To that end
>> my userland software would allocate say 64 buffers, hand them over to
>> driver A to queue/deque them.
>>
>> In future I might want to share these buffers between streams and with
>> other peripherals on the same bus,
>> which is why I looked at ION.
>>
>
> If allocation is coming from userspace and drivers are only importing
> you should be using the dma_buf APIs instead of Ion APIs directly.
> Ion is a dma_buf exporter and dma_buf APIs are the preferred API.
>
>> Thanks,
>>
>> Moritz
>>
>
> Thanks,
> Laura

Re: [RFC 0/2] staging: ion: of_ion_device_get

[Moritz Fischer]

On Tue, Mar 22, 2016 at 4:20 PM, Laura Abbott <labbott@redhat.com> wrote:

> If allocation is coming from userspace and drivers are only importing
> you should be using the dma_buf APIs instead of Ion APIs directly.
> Ion is a dma_buf exporter and dma_buf APIs are the preferred API.

Ok, thanks. Sounds reasonable.

Thanks,

Moritz

PS: My email skills today suck ... sorry about that ... I should go home ...