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
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 <[email protected]>
---
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
Allows to obtain a reference to the global /dev/ion backing
struct ion_device via devicetree.
Signed-off-by: Moritz Fischer <[email protected]>
---
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
Derp,
On Tue, Mar 22, 2016 at 3:33 PM, Moritz Fischer
<[email protected]> wrote:
> Allows to obtain a reference to the global /dev/ion backing
> struct ion_device via devicetree.
>
> Signed-off-by: Moritz Fischer <[email protected]>
> ---
> 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
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
Meh,
On Tue, Mar 22, 2016 at 3:33 PM, Moritz Fischer
<[email protected]> 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
Hi Laura,
On Tue, Mar 22, 2016 at 3:51 PM, Laura Abbott <[email protected]> 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
On 03/22/2016 04:08 PM, Moritz Fischer wrote:
> Hi Laura,
>
> On Tue, Mar 22, 2016 at 3:51 PM, Laura Abbott <[email protected]> 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
On Tue, Mar 22, 2016 at 4:20 PM, Laura Abbott <[email protected]> wrote:
> On 03/22/2016 04:08 PM, Moritz Fischer wrote:
>>
>> Hi Laura,
>>
>> On Tue, Mar 22, 2016 at 3:51 PM, Laura Abbott <[email protected]> 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
On Tue, Mar 22, 2016 at 4:20 PM, Laura Abbott <[email protected]> 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 ...
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