* [PATCH 0/2] staging: ion: Add ion-physmem driver
@ 2015-04-06 15:49 Andrew Andrianov
2015-04-06 15:58 ` [PATCH 1/2] " Andrew Andrianov
2015-04-06 15:59 ` [PATCH 2/2] staging: ion: Add ion-physmem documentation Andrew Andrianov
0 siblings, 2 replies; 6+ messages in thread
From: Andrew Andrianov @ 2015-04-06 15:49 UTC (permalink / raw)
To: Greg Kroah-Hartman, Arve Hj�nnev�g, Riley Andrews,
Chen Gang, Fabian Frederick
Cc: Andrew Andrianov, linux-kernel
Please do not merge the following patchset yet, it's only a very early
proof-of-concept and it needs cleanup, discussion and advice.
Since I'm quite new to ION, so I may be missing a few vital details.
The following ion driver implements a simple way to define ION heaps from a
devicetree description. e.g.
>From on-chip SRAM:
ion_im0: ion@00100000 {
compatible = "ion,physmem";
reg = <0x00100000 0x40000>;
reg-names = "memory";
ion-heap-id = <2>;
ion-heap-type = <ION_HEAP_TYPE_DMA>;
ion-heap-align = <0x10>;
ion-heap-name = "IM0";
};
or
ion_crv: ion@deadbeef {
compatible = "ion,physmem";
reg = <0x00000000 0x100000>;
reg-names = "memory";
ion-heap-id = <3>;
ion-heap-type = <ION_HEAP_TYPE_CARVEOUT>;
ion-heap-align = <0x10>;
ion-heap-name = "carveout";
};
This way you can define any ION heap, so it pretty much supersedes the dummy
driver that has everything hardcoded and is good for nothing but tests.
For ION_HEAP_TYPE_DMA, if 'reg' range is present in devicetree it does a
proper declare_dma_coherent_memory() and parses reg field as a physical address
range. E.g. this way you can pass on-chip SRAM or dedicated RAM banks
to ION to be used as a heap.
If reg is not present - behavior is identical to a DMA heap of ion_dummy driver.
For carveout and chunk heaps it behaves just like the dummy driver, allocating
some free pages as a heap and freeing them when unloaded. reg range is used
for size calculations via resource_size() only.
For system/system_contig things stay pretty much the same, except for it
doesn't do any page allocations by itself and just passes all
parameters down to ION
My use case:
The SoC I'm making this for is K1879XB1YA (К1879ХБ1Я) / MB77.07:
Product page: http://www.module.ru/en/catalog/micro/micro_pc/
Hopefully I'll get basic support for this SoC ready for submission by
the next merge window.
It has dedicated 128MiB 'multimedia' memory that ARM core can't execute
code from, but can write/read to and several huge (256KiB) banks of
on-chip SRAM. All mapped into physical address space.
ION's job will be pretty much allocating from those banks, each making up it's
own heap and sharing the buffers between DSP, ARM and a few multimedia devices.
Remaining questions about this driver:
* Should this ION driver take care to optionally enable/disable clocks the way
drivers/misc/sram.c does?
* Not sure if try_module_get/module_put magic is really needed there and used
properly. Building as a module and unloading has NOT yet been tested, is on my
TODO list.
Regards,
Andrew
Andrew Andrianov (2):
staging: ion: Add ion-physmem driver
staging: ion: Add ion-physmem documentation
Documentation/devicetree/bindings/ion,physmem.txt | 80 +++++++
drivers/staging/android/ion/Kconfig | 7 +
drivers/staging/android/ion/Makefile | 5 +-
drivers/staging/android/ion/ion_physmem.c | 237 +++++++++++++++++++++
4 files changed, 327 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/ion,physmem.txt
create mode 100644 drivers/staging/android/ion/ion_physmem.c
--
1.7.10.4
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/2] staging: ion: Add ion-physmem driver 2015-04-06 15:49 [PATCH 0/2] staging: ion: Add ion-physmem driver Andrew Andrianov @ 2015-04-06 15:58 ` Andrew Andrianov 2015-04-07 8:35 ` Paul Bolle 2015-04-06 15:59 ` [PATCH 2/2] staging: ion: Add ion-physmem documentation Andrew Andrianov 1 sibling, 1 reply; 6+ messages in thread From: Andrew Andrianov @ 2015-04-06 15:58 UTC (permalink / raw) To: Greg Kroah-Hartman, Arve Hj�nnev�g, Riley Andrews, Chen Gang, Fabian Frederick Cc: Andrew Andrianov, linux-kernel Signed-off-by: Andrew Andrianov <andrew@ncrmnt.org> --- drivers/staging/android/ion/Kconfig | 7 + drivers/staging/android/ion/Makefile | 5 +- drivers/staging/android/ion/ion_physmem.c | 237 +++++++++++++++++++++++++++++ 3 files changed, 247 insertions(+), 2 deletions(-) create mode 100644 drivers/staging/android/ion/ion_physmem.c diff --git a/drivers/staging/android/ion/Kconfig b/drivers/staging/android/ion/Kconfig index 3452346..c558cf8 100644 --- a/drivers/staging/android/ion/Kconfig +++ b/drivers/staging/android/ion/Kconfig @@ -33,3 +33,10 @@ config ION_TEGRA help Choose this option if you wish to use ion on an nVidia Tegra. +config ION_PHYSMEM + bool "Generic PhysMem ION driver" + depends on ION + help + Provides a generic ION driver that registers the + /dev/ion device with heaps from devicetree entries. + This heaps are made of chunks of physical memory diff --git a/drivers/staging/android/ion/Makefile b/drivers/staging/android/ion/Makefile index b56fd2b..ac9856d 100644 --- a/drivers/staging/android/ion/Makefile +++ b/drivers/staging/android/ion/Makefile @@ -5,6 +5,7 @@ ifdef CONFIG_COMPAT obj-$(CONFIG_ION) += compat_ion.o endif -obj-$(CONFIG_ION_DUMMY) += ion_dummy_driver.o -obj-$(CONFIG_ION_TEGRA) += tegra/ +obj-$(CONFIG_ION_DUMMY) += ion_dummy_driver.o +obj-$(CONFIG_ION_PHYSMEM) += ion_physmem.o +obj-$(CONFIG_ION_TEGRA) += tegra/ diff --git a/drivers/staging/android/ion/ion_physmem.c b/drivers/staging/android/ion/ion_physmem.c new file mode 100644 index 0000000..e922d74 --- /dev/null +++ b/drivers/staging/android/ion/ion_physmem.c @@ -0,0 +1,237 @@ +/* + * Copyright (C) 2015 RC Module + * Andrew Andrianov <andrew@ncrmnt.org> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * Generic devicetree physmem driver for ION Memory Manager + * + */ + +#include <linux/init.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/slab.h> +#include <linux/of.h> +#include <linux/of_platform.h> +#include <linux/of_address.h> +#include <linux/dma-mapping.h> +#include <linux/io.h> +#include "ion.h" +#include "ion_priv.h" + +#define DRVNAME "ion-physmem" + +static struct ion_device *idev; + +static const struct of_device_id of_match_table[] = { + { .compatible = "ion,physmem", }, + { /* end of list */ } +}; +MODULE_DEVICE_TABLE(of, of_match_table); + +/* +#define PHYSMAP_ION_DEBUG y +*/ + +struct physmem_ion_dev { + struct ion_platform_heap data; + struct ion_heap *heap; + int need_free_coherent; + void *freepage_ptr; +}; + +static long ion_physmem_custom_ioctl(struct ion_client *client, + unsigned int cmd, + unsigned long arg) +{ +/* Just an ugly debugging hack. */ +#ifdef PHYSMAP_ION_DEBUG + if ((cmd == 0xff)) { + ion_phys_addr_t addr; + size_t len; + struct ion_handle *hndl; + + printk(KERN_INFO "=== ION PHYSMEM DEBUG PRINTOUT ===\n"); + printk(KERN_INFO "N.B. DO disable CONFIG_ION_PHYSMEM_DEBUG in production\n"); + printk(KERN_INFO "Shared fd is %d\n", (int) arg); + hndl = ion_import_dma_buf(client, arg); + if (!hndl) { + printk(KERN_INFO "Failed to import shared descriptor.\n"); + return 0; + } + ion_phys(client, hndl, &addr, &len); + printk(KERN_INFO "shared buffer: phys 0x%lx len %ul\n", + addr, len); + printk(KERN_INFO "=== ION PHYSMEM DEBUG PRINTOUT ===\n"); + } +#endif + return 0; +} + +static int ion_physmem_probe(struct platform_device *pdev) +{ + int ret; + u32 ion_heap_id, ion_heap_align, ion_heap_type; + ion_phys_addr_t addr; + size_t size = 0; + const char *ion_heap_name; + struct resource *res; + struct physmem_ion_dev *ipdev; + + /* + Looks like we can only have one ION device in our system. + Therefore we call ion_device_create on first probe and only + add heaps to it on subsequent probe calls. + FixMe: Do we need to hold a spinlock here once device probing + becomes async? + */ + + if (!idev) { + idev = ion_device_create(ion_physmem_custom_ioctl); + dev_info(&pdev->dev, "ion-physmem: ION PhysMem Driver. (c) RC Module 2015\n"); + if (!idev) + return -ENOMEM; + } + + ipdev = kzalloc(sizeof(struct physmem_ion_dev), GFP_KERNEL); + if (!ipdev) + return -ENOMEM; + + platform_set_drvdata(pdev, ipdev); + + ret = of_property_read_u32(pdev->dev.of_node, "ion-heap-id", + &ion_heap_id); + if (ret != 0) + goto errfreeipdev; + + ret = of_property_read_u32(pdev->dev.of_node, "ion-heap-type", + &ion_heap_type); + if (ret != 0) + goto errfreeipdev; + + ret = of_property_read_u32(pdev->dev.of_node, "ion-heap-align", + &ion_heap_align); + if (ret != 0) + goto errfreeipdev; + + ret = of_property_read_string(pdev->dev.of_node, "ion-heap-name", + &ion_heap_name); + if (ret != 0) + goto errfreeipdev; + + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "memory"); + /* Not always needed, throw no error if missing */ + if (res) { + /* Fill in some defaults */ + addr = res->start; + size = resource_size(res); + } + + switch (ion_heap_type) { + case ION_HEAP_TYPE_DMA: + if (res) { + ret = dma_declare_coherent_memory(&pdev->dev, + res->start, + res->start, + resource_size(res), + DMA_MEMORY_MAP | + DMA_MEMORY_EXCLUSIVE); + if (ret == 0) { + ret = -ENODEV; + goto errfreeipdev; + } + } + /* + * If no memory region declared in dt we assume that + * the user is okay with plain dma_alloc_coherent. + */ + break; + case ION_HEAP_TYPE_CARVEOUT: + case ION_HEAP_TYPE_CHUNK: + { + if (size == 0) { + ret = -EIO; + goto errfreeipdev; + } + ipdev->freepage_ptr = alloc_pages_exact(size, GFP_KERNEL); + if (ipdev->freepage_ptr) { + addr = virt_to_phys(ipdev->freepage_ptr); + } else { + ret = -ENOMEM; + goto errfreeipdev; + } + break; + } + } + + ipdev->data.id = ion_heap_id; + ipdev->data.type = ion_heap_type; + ipdev->data.name = ion_heap_name; + ipdev->data.align = ion_heap_align; + ipdev->data.base = addr; + ipdev->data.size = size; + /* This one make dma_declare_coherent_memory actually work */ + ipdev->data.priv = &pdev->dev; + + + ipdev->heap = ion_heap_create(&ipdev->data); + if (!ipdev->heap) + goto errfreeipdev; + + if (!try_module_get(THIS_MODULE)) + goto errfreeheap; + + ion_device_add_heap(idev, ipdev->heap); + + dev_info(&pdev->dev, "ion-physmem: heap %s id %d type %d align 0x%x at phys 0x%lx len %lu KiB\n", + ion_heap_name, ion_heap_id, ion_heap_type, ion_heap_align, + (unsigned long int) addr, ((unsigned long int) size / 1024)); + + return 0; + +errfreeheap: + kfree(ipdev->heap); +errfreeipdev: + kfree(ipdev); + return -ENOMEM; +} + +static int ion_physmem_remove(struct platform_device *pdev) +{ + struct physmem_ion_dev *ipdev = platform_get_drvdata(pdev); + + ion_heap_destroy(ipdev->heap); + if (ipdev->need_free_coherent) + dma_release_declared_memory(&pdev->dev); + if (ipdev->freepage_ptr) + free_pages_exact(ipdev->freepage_ptr, ipdev->data.size); + kfree(ipdev->heap); + kfree(ipdev); + module_put(THIS_MODULE); + return 0; +} + +static void __exit ion_physmem_exit(void) +{ + if (idev) + ion_device_destroy(idev); +} +__exitcall(ion_physmem_exit); + +static struct platform_driver ion_physmem_driver = { + .probe = ion_physmem_probe, + .remove = ion_physmem_remove, + .driver = { + .owner = THIS_MODULE, + .name = "ion-physmem", + .of_match_table = of_match_ptr(of_match_table), + }, +}; +module_platform_driver(ion_physmem_driver); + +MODULE_DESCRIPTION("Generic physmem driver for ION"); +MODULE_AUTHOR("Andrew Andrianov <andrew@ncrmnt.org>"); +MODULE_LICENSE("GPL"); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] staging: ion: Add ion-physmem driver 2015-04-06 15:58 ` [PATCH 1/2] " Andrew Andrianov @ 2015-04-07 8:35 ` Paul Bolle 2015-04-07 8:57 ` Greg Kroah-Hartman 2015-04-07 11:35 ` Andrew 0 siblings, 2 replies; 6+ messages in thread From: Paul Bolle @ 2015-04-07 8:35 UTC (permalink / raw) To: Andrew Andrianov Cc: Greg Kroah-Hartman, Arve Hjønnevåg, Riley Andrews, Chen Gang, Fabian Frederick, linux-kernel This patch adds a mismatch between the Kconfig symbol (a bool) and the code (which suggests it could be built modular too). On Mon, 2015-04-06 at 18:58 +0300, Andrew Andrianov wrote: > +config ION_PHYSMEM > + bool "Generic PhysMem ION driver" > + depends on ION > + help > + Provides a generic ION driver that registers the > + /dev/ion device with heaps from devicetree entries. > + This heaps are made of chunks of physical memory > --- a/drivers/staging/android/ion/Makefile > +++ b/drivers/staging/android/ion/Makefile > -obj-$(CONFIG_ION_DUMMY) += ion_dummy_driver.o > -obj-$(CONFIG_ION_TEGRA) += tegra/ > +obj-$(CONFIG_ION_DUMMY) += ion_dummy_driver.o > +obj-$(CONFIG_ION_PHYSMEM) += ion_physmem.o > +obj-$(CONFIG_ION_TEGRA) += tegra/ To make absolutely sure I'm reading this correctly: there's no way ion_physmem.o can ever be part of a module, right? (If I'm not reading this correctly you can stop reading here.) > --- /dev/null > +++ b/drivers/staging/android/ion/ion_physmem.c > @@ -0,0 +1,237 @@ > +/* > + * Copyright (C) 2015 RC Module > + * Andrew Andrianov <andrew@ncrmnt.org> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * Generic devicetree physmem driver for ION Memory Manager > + * > + */ > +#include <linux/module.h> If this file can only be built-in this include might not be needed. > +MODULE_DEVICE_TABLE(of, of_match_table); MODULE_DEVICE_TABLE will always be preprocessed away for built-in code (see include/linux/module.h). > +static int ion_physmem_probe(struct platform_device *pdev) > +{ > + int ret; > + u32 ion_heap_id, ion_heap_align, ion_heap_type; > + ion_phys_addr_t addr; > + size_t size = 0; > + const char *ion_heap_name; > + struct resource *res; > + struct physmem_ion_dev *ipdev; > + > + /* > + Looks like we can only have one ION device in our system. > + Therefore we call ion_device_create on first probe and only > + add heaps to it on subsequent probe calls. > + FixMe: Do we need to hold a spinlock here once device probing > + becomes async? > + */ > + > + if (!idev) { > + idev = ion_device_create(ion_physmem_custom_ioctl); > + dev_info(&pdev->dev, "ion-physmem: ION PhysMem Driver. (c) RC Module 2015\n"); > + if (!idev) > + return -ENOMEM; > + } > + > + ipdev = kzalloc(sizeof(struct physmem_ion_dev), GFP_KERNEL); > + if (!ipdev) > + return -ENOMEM; > + > + platform_set_drvdata(pdev, ipdev); > + > + ret = of_property_read_u32(pdev->dev.of_node, "ion-heap-id", > + &ion_heap_id); > + if (ret != 0) > + goto errfreeipdev; > + > + ret = of_property_read_u32(pdev->dev.of_node, "ion-heap-type", > + &ion_heap_type); > + if (ret != 0) > + goto errfreeipdev; > + > + ret = of_property_read_u32(pdev->dev.of_node, "ion-heap-align", > + &ion_heap_align); > + if (ret != 0) > + goto errfreeipdev; > + > + ret = of_property_read_string(pdev->dev.of_node, "ion-heap-name", > + &ion_heap_name); > + if (ret != 0) > + goto errfreeipdev; > + > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "memory"); > + /* Not always needed, throw no error if missing */ > + if (res) { > + /* Fill in some defaults */ > + addr = res->start; > + size = resource_size(res); > + } > + > + switch (ion_heap_type) { > + case ION_HEAP_TYPE_DMA: > + if (res) { > + ret = dma_declare_coherent_memory(&pdev->dev, > + res->start, > + res->start, > + resource_size(res), > + DMA_MEMORY_MAP | > + DMA_MEMORY_EXCLUSIVE); > + if (ret == 0) { > + ret = -ENODEV; > + goto errfreeipdev; > + } > + } > + /* > + * If no memory region declared in dt we assume that > + * the user is okay with plain dma_alloc_coherent. > + */ > + break; > + case ION_HEAP_TYPE_CARVEOUT: > + case ION_HEAP_TYPE_CHUNK: > + { > + if (size == 0) { > + ret = -EIO; > + goto errfreeipdev; > + } > + ipdev->freepage_ptr = alloc_pages_exact(size, GFP_KERNEL); > + if (ipdev->freepage_ptr) { > + addr = virt_to_phys(ipdev->freepage_ptr); > + } else { > + ret = -ENOMEM; > + goto errfreeipdev; > + } > + break; > + } > + } > + > + ipdev->data.id = ion_heap_id; > + ipdev->data.type = ion_heap_type; > + ipdev->data.name = ion_heap_name; > + ipdev->data.align = ion_heap_align; > + ipdev->data.base = addr; > + ipdev->data.size = size; > + /* This one make dma_declare_coherent_memory actually work */ > + ipdev->data.priv = &pdev->dev; > + > + > + ipdev->heap = ion_heap_create(&ipdev->data); > + if (!ipdev->heap) > + goto errfreeipdev; > + > + if (!try_module_get(THIS_MODULE)) > + goto errfreeheap; For built-in only code THIS_MODULE will be, basically, always NULL. So, I think try_module_get() will always return true. > + ion_device_add_heap(idev, ipdev->heap); > + > + dev_info(&pdev->dev, "ion-physmem: heap %s id %d type %d align 0x%x at phys 0x%lx len %lu KiB\n", > + ion_heap_name, ion_heap_id, ion_heap_type, ion_heap_align, > + (unsigned long int) addr, ((unsigned long int) size / 1024)); > + > + return 0; > + > +errfreeheap: > + kfree(ipdev->heap); > +errfreeipdev: > + kfree(ipdev); > + return -ENOMEM; > +} > + > +static int ion_physmem_remove(struct platform_device *pdev) > +{ > + struct physmem_ion_dev *ipdev = platform_get_drvdata(pdev); > + > + ion_heap_destroy(ipdev->heap); > + if (ipdev->need_free_coherent) > + dma_release_declared_memory(&pdev->dev); > + if (ipdev->freepage_ptr) > + free_pages_exact(ipdev->freepage_ptr, ipdev->data.size); > + kfree(ipdev->heap); > + kfree(ipdev); > + module_put(THIS_MODULE); Again, THIS_MODULE will be, basically, always NULL. > + return 0; > +} > + > +static struct platform_driver ion_physmem_driver = { > + .probe = ion_physmem_probe, > + .remove = ion_physmem_remove, > + .driver = { > + .owner = THIS_MODULE, Ditto. > + .name = "ion-physmem", > + .of_match_table = of_match_ptr(of_match_table), > + }, > +}; > +module_platform_driver(ion_physmem_driver); For built-in only code this is equivalent to, if I remember correctly, having a small wrapper call platform_driver_register(&ion_physmem_driver); and mark that wrapper with device_initcall(). > +MODULE_DESCRIPTION("Generic physmem driver for ION"); > +MODULE_AUTHOR("Andrew Andrianov <andrew@ncrmnt.org>"); > +MODULE_LICENSE("GPL"); These macros will always be effectively preprocessed away for built-in only code. (If you plan to make ION_PHYSMEM tristate, you should note that "GPL" doesn't match the license stated in the comment at the top of this file, see include/linux/module.h.) Paul Bolle ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] staging: ion: Add ion-physmem driver 2015-04-07 8:35 ` Paul Bolle @ 2015-04-07 8:57 ` Greg Kroah-Hartman 2015-04-07 11:35 ` Andrew 1 sibling, 0 replies; 6+ messages in thread From: Greg Kroah-Hartman @ 2015-04-07 8:57 UTC (permalink / raw) To: Paul Bolle Cc: Andrew Andrianov, Arve Hjønnevåg, Riley Andrews, Chen Gang, Fabian Frederick, linux-kernel On Tue, Apr 07, 2015 at 10:35:45AM +0200, Paul Bolle wrote: > > + ipdev->heap = ion_heap_create(&ipdev->data); > > + if (!ipdev->heap) > > + goto errfreeipdev; > > + > > + if (!try_module_get(THIS_MODULE)) > > + goto errfreeheap; > > For built-in only code THIS_MODULE will be, basically, always NULL. So, > I think try_module_get() will always return true. Even if this is a module, this code is racy and broken, no module should _ever_ call try_module_get(THIS_MODULE). That's a sign of broken code, and/or a broken API. thanks, greg k-h ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] staging: ion: Add ion-physmem driver 2015-04-07 8:35 ` Paul Bolle 2015-04-07 8:57 ` Greg Kroah-Hartman @ 2015-04-07 11:35 ` Andrew 1 sibling, 0 replies; 6+ messages in thread From: Andrew @ 2015-04-07 11:35 UTC (permalink / raw) To: Paul Bolle, Greg Kroah-Hartman Cc: Arve Hjønnevåg, Riley Andrews, Chen Gang, Fabian Frederick, linux-kernel Paul Bolle писал 07.04.2015 11:35: > This patch adds a mismatch between the Kconfig symbol (a bool) and the > code (which suggests it could be built modular too). > > On Mon, 2015-04-06 at 18:58 +0300, Andrew Andrianov wrote: >> +config ION_PHYSMEM >> + bool "Generic PhysMem ION driver" >> + depends on ION >> + help >> + Provides a generic ION driver that registers the >> + /dev/ion device with heaps from devicetree entries. >> + This heaps are made of chunks of physical memory > >> --- a/drivers/staging/android/ion/Makefile >> +++ b/drivers/staging/android/ion/Makefile > >> -obj-$(CONFIG_ION_DUMMY) += ion_dummy_driver.o >> -obj-$(CONFIG_ION_TEGRA) += tegra/ >> +obj-$(CONFIG_ION_DUMMY) += ion_dummy_driver.o >> +obj-$(CONFIG_ION_PHYSMEM) += ion_physmem.o >> +obj-$(CONFIG_ION_TEGRA) += tegra/ > > To make absolutely sure I'm reading this correctly: there's no way > ion_physmem.o can ever be part of a module, right? > > (If I'm not reading this correctly you can stop reading here.) > >> --- /dev/null >> +++ b/drivers/staging/android/ion/ion_physmem.c >> @@ -0,0 +1,237 @@ >> +/* >> + * Copyright (C) 2015 RC Module >> + * Andrew Andrianov <andrew@ncrmnt.org> >> + * >> + * This program is free software; you can redistribute it and/or >> modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + * >> + * Generic devicetree physmem driver for ION Memory Manager >> + * >> + */ > >> +#include <linux/module.h> > > If this file can only be built-in this include might not be needed. > >> +MODULE_DEVICE_TABLE(of, of_match_table); > > MODULE_DEVICE_TABLE will always be preprocessed away for built-in code > (see include/linux/module.h). > >> +static int ion_physmem_probe(struct platform_device *pdev) >> +{ >> + int ret; >> + u32 ion_heap_id, ion_heap_align, ion_heap_type; >> + ion_phys_addr_t addr; >> + size_t size = 0; >> + const char *ion_heap_name; >> + struct resource *res; >> + struct physmem_ion_dev *ipdev; >> + >> + /* >> + Looks like we can only have one ION device in our system. >> + Therefore we call ion_device_create on first probe and only >> + add heaps to it on subsequent probe calls. >> + FixMe: Do we need to hold a spinlock here once device probing >> + becomes async? >> + */ >> + >> + if (!idev) { >> + idev = ion_device_create(ion_physmem_custom_ioctl); >> + dev_info(&pdev->dev, "ion-physmem: ION PhysMem Driver. (c) RC >> Module 2015\n"); >> + if (!idev) >> + return -ENOMEM; >> + } >> + >> + ipdev = kzalloc(sizeof(struct physmem_ion_dev), GFP_KERNEL); >> + if (!ipdev) >> + return -ENOMEM; >> + >> + platform_set_drvdata(pdev, ipdev); >> + >> + ret = of_property_read_u32(pdev->dev.of_node, "ion-heap-id", >> + &ion_heap_id); >> + if (ret != 0) >> + goto errfreeipdev; >> + >> + ret = of_property_read_u32(pdev->dev.of_node, "ion-heap-type", >> + &ion_heap_type); >> + if (ret != 0) >> + goto errfreeipdev; >> + >> + ret = of_property_read_u32(pdev->dev.of_node, "ion-heap-align", >> + &ion_heap_align); >> + if (ret != 0) >> + goto errfreeipdev; >> + >> + ret = of_property_read_string(pdev->dev.of_node, "ion-heap-name", >> + &ion_heap_name); >> + if (ret != 0) >> + goto errfreeipdev; >> + >> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "memory"); >> + /* Not always needed, throw no error if missing */ >> + if (res) { >> + /* Fill in some defaults */ >> + addr = res->start; >> + size = resource_size(res); >> + } >> + >> + switch (ion_heap_type) { >> + case ION_HEAP_TYPE_DMA: >> + if (res) { >> + ret = dma_declare_coherent_memory(&pdev->dev, >> + res->start, >> + res->start, >> + resource_size(res), >> + DMA_MEMORY_MAP | >> + DMA_MEMORY_EXCLUSIVE); >> + if (ret == 0) { >> + ret = -ENODEV; >> + goto errfreeipdev; >> + } >> + } >> + /* >> + * If no memory region declared in dt we assume that >> + * the user is okay with plain dma_alloc_coherent. >> + */ >> + break; >> + case ION_HEAP_TYPE_CARVEOUT: >> + case ION_HEAP_TYPE_CHUNK: >> + { >> + if (size == 0) { >> + ret = -EIO; >> + goto errfreeipdev; >> + } >> + ipdev->freepage_ptr = alloc_pages_exact(size, GFP_KERNEL); >> + if (ipdev->freepage_ptr) { >> + addr = virt_to_phys(ipdev->freepage_ptr); >> + } else { >> + ret = -ENOMEM; >> + goto errfreeipdev; >> + } >> + break; >> + } >> + } >> + >> + ipdev->data.id = ion_heap_id; >> + ipdev->data.type = ion_heap_type; >> + ipdev->data.name = ion_heap_name; >> + ipdev->data.align = ion_heap_align; >> + ipdev->data.base = addr; >> + ipdev->data.size = size; >> + /* This one make dma_declare_coherent_memory actually work */ >> + ipdev->data.priv = &pdev->dev; >> + >> + >> + ipdev->heap = ion_heap_create(&ipdev->data); >> + if (!ipdev->heap) >> + goto errfreeipdev; >> + >> + if (!try_module_get(THIS_MODULE)) >> + goto errfreeheap; > > For built-in only code THIS_MODULE will be, basically, always NULL. So, > I think try_module_get() will always return true. > >> + ion_device_add_heap(idev, ipdev->heap); >> + >> + dev_info(&pdev->dev, "ion-physmem: heap %s id %d type %d align 0x%x >> at phys 0x%lx len %lu KiB\n", >> + ion_heap_name, ion_heap_id, ion_heap_type, ion_heap_align, >> + (unsigned long int) addr, ((unsigned long int) size / 1024)); >> + >> + return 0; >> + >> +errfreeheap: >> + kfree(ipdev->heap); >> +errfreeipdev: >> + kfree(ipdev); >> + return -ENOMEM; >> +} >> + >> +static int ion_physmem_remove(struct platform_device *pdev) >> +{ >> + struct physmem_ion_dev *ipdev = platform_get_drvdata(pdev); >> + >> + ion_heap_destroy(ipdev->heap); >> + if (ipdev->need_free_coherent) >> + dma_release_declared_memory(&pdev->dev); >> + if (ipdev->freepage_ptr) >> + free_pages_exact(ipdev->freepage_ptr, ipdev->data.size); >> + kfree(ipdev->heap); >> + kfree(ipdev); >> + module_put(THIS_MODULE); > > Again, THIS_MODULE will be, basically, always NULL. > >> + return 0; >> +} >> + >> +static struct platform_driver ion_physmem_driver = { >> + .probe = ion_physmem_probe, >> + .remove = ion_physmem_remove, >> + .driver = { >> + .owner = THIS_MODULE, > > Ditto. > >> + .name = "ion-physmem", >> + .of_match_table = of_match_ptr(of_match_table), >> + }, >> +}; >> +module_platform_driver(ion_physmem_driver); > > For built-in only code this is equivalent to, if I remember correctly, > having a small wrapper call > platform_driver_register(&ion_physmem_driver); > > and mark that wrapper with device_initcall(). > >> +MODULE_DESCRIPTION("Generic physmem driver for ION"); >> +MODULE_AUTHOR("Andrew Andrianov <andrew@ncrmnt.org>"); >> +MODULE_LICENSE("GPL"); > > These macros will always be effectively preprocessed away for built-in > only code. > > (If you plan to make ION_PHYSMEM tristate, you should note that "GPL" > doesn't match the license stated in the comment at the top of this > file, > see include/linux/module.h.) > > > Paul Bolle Thanks for the detailed review, I'll clean things up and resubmit. I've marked Kconfig option as bool for now, since I haven't tested module unloading yet and didn't have a chance to look how ION handles it. Right now it is unclean for me if ION device drivers _should_ be built as modules (e.g. ion-dummy-driver can't be ever unloaded and it's the only one upstream). If we go the module way - I'm not (yet) sure what steps are necessary to take to ensure that ION device driver will NOT get unloaded when any of the heaps it provides are in use by anyone. (module_get/module_put came from initial playing with that use case) So far building as a module looks like the only sane way for generic arm kernels supporting multiple SoCs, since we can only have ONE /dev/ion registered in the system. -- Regards, Andrew ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] staging: ion: Add ion-physmem documentation 2015-04-06 15:49 [PATCH 0/2] staging: ion: Add ion-physmem driver Andrew Andrianov 2015-04-06 15:58 ` [PATCH 1/2] " Andrew Andrianov @ 2015-04-06 15:59 ` Andrew Andrianov 1 sibling, 0 replies; 6+ messages in thread From: Andrew Andrianov @ 2015-04-06 15:59 UTC (permalink / raw) To: Greg Kroah-Hartman, Arve Hj�nnev�g, Riley Andrews, Chen Gang, Fabian Frederick Cc: Andrew Andrianov, linux-kernel Signed-off-by: Andrew Andrianov <andrew@ncrmnt.org> --- Documentation/devicetree/bindings/ion,physmem.txt | 80 +++++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 Documentation/devicetree/bindings/ion,physmem.txt diff --git a/Documentation/devicetree/bindings/ion,physmem.txt b/Documentation/devicetree/bindings/ion,physmem.txt new file mode 100644 index 0000000..df18b5f --- /dev/null +++ b/Documentation/devicetree/bindings/ion,physmem.txt @@ -0,0 +1,80 @@ +ION PhysMem Driver +#include <dt-bindings/ion,physmem.h> + + +ION PhysMem is a generic driver for ION Memory Manager that allows you to +define ION Memory Manager heaps using device tree. This is mostly useful if +your SoC has several 'special' regions (e.g. SRAM, dedicated memory banks, +etc) that are present in the physical memory map and you want to add them to +ION as heaps of memory. + + +Examples: + +1. 256KiB On-chip SRAM used as ION DMA heap. reg range is treated as a physical + address range + + ion_im0: ion@0x00100000 { + compatible = "ion,physmem"; + reg = <0x00100000 0x40000>; + reg-names = "memory"; + ion-heap-id = <2>; + ion-heap-type = <ION_HEAP_TYPE_DMA>; + ion-heap-align = <0x10>; + ion-heap-name = "IM0"; + }; + +2. The same, but using system DMA memory. + + ion_dma: ion@0xdeadbeef { + compatible = "ion,physmem"; + ion-heap-id = <2>; + ion-heap-type = <ION_HEAP_TYPE_DMA>; + ion-heap-align = <0x10>; + ion-heap-name = "SYSDMA"; + }; + +2. Carveout heap, 1MiB size, ion-physmem will alloc pages for it using + alloc_pages_exact(). reg range is used for specifying size only. + + ion_crv: ion@deadbeef { + compatible = "ion,physmem"; + reg = <0x00000000 0x100000>; + reg-names = "memory"; + ion-heap-id = <3>; + ion-heap-type = <ION_HEAP_TYPE_CARVEOUT>; + ion-heap-align = <0x10>; + ion-heap-name = "carveout"; + }; + +3. Chunk heap. 1MiB size, ion-physmem will alloc pages for it using + alloc_pages_exact(). reg range is used for specifying size only. + + ion_chunk: ion@0xdeadbeef { + compatible = "ion,physmem"; + ion-heap-id = <2>; + ion-heap-type = <ION_HEAP_TYPE_CHUNK>; + ion-heap-align = <0x10>; + ion-heap-name = "chunky"; + }; + + +4. vmalloc(); + + ion_chunk: ion@0xdeadbeef { + compatible = "ion,physmem"; + ion-heap-id = <2>; + ion-heap-type = <ION_HEAP_TYPE_SYSTEM>; + ion-heap-align = <0x10>; + ion-heap-name = "sys"; + }; + +5. kmalloc(); + + ion_chunk: ion@0xdeadbeef { + compatible = "ion,physmem"; + ion-heap-id = <2>; + ion-heap-type = <ION_HEAP_TYPE_SYSTEM_CONTIG>; + ion-heap-align = <0x10>; + ion-heap-name = "syscont"; + }; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-04-07 11:35 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-04-06 15:49 [PATCH 0/2] staging: ion: Add ion-physmem driver Andrew Andrianov 2015-04-06 15:58 ` [PATCH 1/2] " Andrew Andrianov 2015-04-07 8:35 ` Paul Bolle 2015-04-07 8:57 ` Greg Kroah-Hartman 2015-04-07 11:35 ` Andrew 2015-04-06 15:59 ` [PATCH 2/2] staging: ion: Add ion-physmem documentation Andrew Andrianov
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.