* Re: [PATCH 2/3] staging: android: ion: Add ion driver for Hi6220 SoC platform
2015-10-08 7:55 ` [PATCH 2/3] staging: android: ion: Add ion driver for Hi6220 SoC platform Chen Feng
@ 2015-10-08 8:52 ` Greg KH
2015-10-08 12:03 ` kbuild test robot
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Greg KH @ 2015-10-08 8:52 UTC (permalink / raw)
To: Chen Feng
Cc: arve, riandrews, mitchelh, tranmanphong, gioh.kim, paul.gortmaker,
tapaswenipathak, sumit.semwal, devel, linux-kernel, linuxarm,
dan.zhao, peter.panshilin, qijiwen, xuyiping, w.f, z.liuxinliang,
kong.kongxinwei, weidong2, saberlily.xia, suzhuangluan
On Thu, Oct 08, 2015 at 03:55:12PM +0800, Chen Feng wrote:
> Signed-off-by: Chen Feng <puck.chen@hisilicon.com>
> Signed-off-by: Yu Dongbin <yudongbin@hisilicon.com>
I can't take a patch with no changelog entry at all, sorry. You are
going to have to at least explain what this driver is, what it does, and
what hardware it supports.
thanks,
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 2/3] staging: android: ion: Add ion driver for Hi6220 SoC platform
2015-10-08 7:55 ` [PATCH 2/3] staging: android: ion: Add ion driver for Hi6220 SoC platform Chen Feng
2015-10-08 8:52 ` Greg KH
@ 2015-10-08 12:03 ` kbuild test robot
2015-10-09 8:53 ` Dan Carpenter
2015-10-09 9:08 ` xuyiping
3 siblings, 0 replies; 9+ messages in thread
From: kbuild test robot @ 2015-10-08 12:03 UTC (permalink / raw)
To: Chen Feng
Cc: kbuild-all, puck.chen, gregkh, arve, riandrews, mitchelh,
tranmanphong, gioh.kim, paul.gortmaker, tapaswenipathak,
sumit.semwal, devel, linux-kernel, linuxarm, dan.zhao,
peter.panshilin, qijiwen, xuyiping, w.f, z.liuxinliang,
kong.kongxinwei, weidong2, saberlily.xia, suzhuangluan
[-- Attachment #1: Type: text/plain, Size: 961 bytes --]
Hi Chen,
[auto build test WARNING on v4.3-rc4 -- if it's inappropriate base, please ignore]
config: arm-allyesconfig (attached as .config)
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm
All warnings (new ones prefixed by >>):
>> WARNING: drivers/built-in.o(.data+0x7f8f14): Section mismatch in reference from the variable hi6220_ion_driver to the function .init.text:hi6220_ion_probe()
The variable hi6220_ion_driver references
the function __init hi6220_ion_probe()
If the reference is valid then annotate the
variable with or __refdata (see linux/init.h) or name the variable:
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 53087 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 2/3] staging: android: ion: Add ion driver for Hi6220 SoC platform
2015-10-08 7:55 ` [PATCH 2/3] staging: android: ion: Add ion driver for Hi6220 SoC platform Chen Feng
2015-10-08 8:52 ` Greg KH
2015-10-08 12:03 ` kbuild test robot
@ 2015-10-09 8:53 ` Dan Carpenter
2015-10-09 8:58 ` Dan Carpenter
2015-10-09 9:08 ` xuyiping
3 siblings, 1 reply; 9+ messages in thread
From: Dan Carpenter @ 2015-10-09 8:53 UTC (permalink / raw)
To: Chen Feng
Cc: gregkh, arve, riandrews, mitchelh, tranmanphong, gioh.kim,
paul.gortmaker, tapaswenipathak, sumit.semwal, devel,
linux-kernel, dan.zhao, xuyiping, w.f, peter.panshilin, linuxarm,
z.liuxinliang, kong.kongxinwei, saberlily.xia, suzhuangluan,
weidong2, qijiwen
On Thu, Oct 08, 2015 at 03:55:12PM +0800, Chen Feng wrote:
> Signed-off-by: Chen Feng <puck.chen@hisilicon.com>
> Signed-off-by: Yu Dongbin <yudongbin@hisilicon.com>
> ---
> drivers/staging/android/ion/Kconfig | 7 +
> drivers/staging/android/ion/Makefile | 1 +
> drivers/staging/android/ion/hisilicon/Kconfig | 5 +
> drivers/staging/android/ion/hisilicon/Makefile | 1 +
> drivers/staging/android/ion/hisilicon/hi6220_ion.c | 201 +++++++++++++++++++++
> 5 files changed, 215 insertions(+)
> create mode 100644 drivers/staging/android/ion/hisilicon/Kconfig
> create mode 100644 drivers/staging/android/ion/hisilicon/Makefile
> create mode 100644 drivers/staging/android/ion/hisilicon/hi6220_ion.c
>
> diff --git a/drivers/staging/android/ion/Kconfig b/drivers/staging/android/ion/Kconfig
> index 3452346..19c1572 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_HISI
> + tristate "Ion for Hisilicon"
> + depends on ARCH_HISI && ION
> + help
> + Choose this option if you wish to use ion on Hisilicon Platform.
> +
> +source "drivers/staging/android/ion/hisilicon/Kconfig"
> diff --git a/drivers/staging/android/ion/Makefile b/drivers/staging/android/ion/Makefile
> index b56fd2b..18cc2aa 100644
> --- a/drivers/staging/android/ion/Makefile
> +++ b/drivers/staging/android/ion/Makefile
> @@ -7,4 +7,5 @@ endif
>
> obj-$(CONFIG_ION_DUMMY) += ion_dummy_driver.o
> obj-$(CONFIG_ION_TEGRA) += tegra/
> +obj-$(CONFIG_ION_HISI) += hisilicon/
>
> diff --git a/drivers/staging/android/ion/hisilicon/Kconfig b/drivers/staging/android/ion/hisilicon/Kconfig
> new file mode 100644
> index 0000000..2b4bd07
> --- /dev/null
> +++ b/drivers/staging/android/ion/hisilicon/Kconfig
> @@ -0,0 +1,5 @@
> +config HI6220_ION
> + bool "Hi6220 ION Driver"
> + depends on ARCH_HISI && ION
> + help
> + Build the Hisilicon Hi6220 ion driver.
> diff --git a/drivers/staging/android/ion/hisilicon/Makefile b/drivers/staging/android/ion/hisilicon/Makefile
> new file mode 100644
> index 0000000..2a89414
> --- /dev/null
> +++ b/drivers/staging/android/ion/hisilicon/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_HI6220_ION) += hi6220_ion.o
> diff --git a/drivers/staging/android/ion/hisilicon/hi6220_ion.c b/drivers/staging/android/ion/hisilicon/hi6220_ion.c
> new file mode 100644
> index 0000000..b7d39b8
> --- /dev/null
> +++ b/drivers/staging/android/ion/hisilicon/hi6220_ion.c
> @@ -0,0 +1,201 @@
> +#define pr_fmt(fmt) "Ion: " fmt
> +
> +#include <linux/err.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/of.h>
> +#include <linux/mm.h>
> +#include "../ion_priv.h"
> +#include "../ion.h"
> +
> +struct hi6220_ion_type_table {
> + const char *name;
> + enum ion_heap_type type;
> +};
> +
> +static struct hi6220_ion_type_table ion_type_table[] = {
> + {"ion_system", ION_HEAP_TYPE_SYSTEM},
> + {"ion_system_contig", ION_HEAP_TYPE_SYSTEM_CONTIG},
> + {"ion_carveout", ION_HEAP_TYPE_CARVEOUT},
> + {"ion_chunk", ION_HEAP_TYPE_CHUNK},
> + {"ion_dma", ION_HEAP_TYPE_DMA},
> + {"ion_custom", ION_HEAP_TYPE_CUSTOM},
> +};
> +
> +static struct ion_device *idev;
> +static int num_heaps;
> +static struct ion_heap **heaps;
> +static struct ion_platform_heap **heaps_data;
> +
> +static int get_type_by_name(const char *name, enum ion_heap_type *type)
> +{
> + int i, n;
> +
> + n = ARRAY_SIZE(ion_type_table);
> + for (i = 0; i < n; i++) {
No need for the "n" variable.
> + if (strncmp(name, ion_type_table[i].name, strlen(name)))
> + continue;
> +
> + *type = ion_type_table[i].type;
> + return 0;
> + }
> +
> + return -1;
return -EINVAL;
> +}
> +
> +static int hi6220_set_platform_data(struct platform_device *pdev)
> +{
> + unsigned int base;
> + unsigned int size;
> + unsigned int id;
> + const char *heap_name;
> + const char *type_name;
> + enum ion_heap_type type;
> + int ret;
> + struct device_node *np;
> + struct ion_platform_heap *p_data;
> + const struct device_node *dt_node = pdev->dev.of_node;
> + int index = 0;
> +
> + for_each_child_of_node(dt_node, np)
> + num_heaps++;
Indent this line.
> +
> + heaps_data = devm_kzalloc(&pdev->dev,
> + sizeof(struct ion_platform_heap *) * num_heaps,
> + GFP_KERNEL);
Add a NULL check.
> +
> + for_each_child_of_node(dt_node, np) {
> + p_data = devm_kzalloc(&pdev->dev,
> + sizeof(struct ion_platform_heap),
> + GFP_KERNEL);
I would move this allocation closer to the place where it "p_data" is
used so that it comes after all the validation. Also we need to check
for allocation errors.
> +
> + ret = of_property_read_string(np, "heap-name", &heap_name);
> + if (ret < 0) {
> + pr_err("check the name of node %s\n", np->name);
> + continue;
> + }
> +
> + ret = of_property_read_u32(np, "heap-id", &id);
> + if (ret < 0) {
> + pr_err("check the id %s\n", np->name);
> + continue;
> + }
> +
> + ret = of_property_read_u32(np, "heap-base", &base);
> + if (ret < 0) {
> + pr_err("check the base of node %s\n", np->name);
> + continue;
> + }
> +
> + ret = of_property_read_u32(np, "heap-size", &size);
> + if (ret < 0) {
> + pr_err("check the size of node %s\n", np->name);
> + continue;
> + }
> +
> + ret = of_property_read_string(np, "heap-type", &type_name);
> + if (ret < 0) {
> + pr_err("check the type of node %s\n", np->name);
> + continue;
> + }
> +
> + ret = get_type_by_name(type_name, &type);
> + if (ret < 0) {
> + pr_err("type name error %s!\n", type_name);
> + continue;
> + }
> + pr_info("heap index %d : name %s base 0x%x size 0x%x id %d type %d\n",
> + index, heap_name, base, size, id, type);
> +
> + p_data->name = heap_name;
> + p_data->base = base;
> + p_data->size = size;
> + p_data->id = id;
> + p_data->type = type;
> +
> + heaps_data[index] = p_data;
> + index++;
> + }
> + return 0;
> +}
> +
> +static int __init hi6220_ion_probe(struct platform_device *pdev)
> +{
> + int i;
> + int err = 0;
> + static struct ion_platform_heap *p_heap;
> +
> + idev = ion_device_create(NULL);
> + if (IS_ERR_OR_NULL(idev)) {
> + pr_err("ion_device_create err %lx\n", PTR_ERR(idev));
ion_device_create() can never return NULL. It's important to get these
things correct because passing PTR_ERR(NULL) will introduce a static
checker warning because that means success.
> + return PTR_ERR(idev);
Returning success if NULL here.
> + }
> +
> + hi6220_set_platform_data(pdev);
> +
> + heaps = devm_kzalloc(&pdev->dev,
> + sizeof(struct ion_heap *) * num_heaps,
> + GFP_KERNEL);
Use devm_kcalloc(). Null check.
> +
> + /*
> + * create the heaps as specified in the dts file
> + */
> + for (i = 0; i < num_heaps; i++) {
> + p_heap = heaps_data[i];
> + heaps[i] = ion_heap_create(p_heap);
> + if (IS_ERR_OR_NULL(heaps[i])) {
> + err = PTR_ERR(heaps[i]);
Same IS_ERR_OR_NULL() or NULL issues.
> + goto out;
> + }
> +
> + ion_device_add_heap(idev, heaps[i]);
> +
> + pr_info("%s: adding heap %s of type %d with %lx@%lx\n",
> + __func__, p_heap->name, p_heap->type,
> + p_heap->base, (unsigned long)p_heap->size);
> + }
> + return err;
Hopefully this is equivalent to:
return 0;
> +out:
Labels named "out" are bug prone because handling everything is harder
than using named labels and unwinding one step at a time. The bug here
is that we don't call ion_device_destroy().
> + for (i = 0; i < num_heaps; ++i)
> + ion_heap_destroy(heaps[i]);
> + return err;
Write it like this:
err_free_heaps:
for (i = 0; i < num_heaps; ++i)
ion_heap_destroy(heaps[i]);
err_free_idev:
ion_device_destroy(idev);
return err;
> +}
> +
> +static int hi6220_ion_remove(struct platform_device *pdev)
> +{
> + int i;
> +
> + ion_device_destroy(idev);
> + for (i = 0; i < num_heaps; i++) {
> + if (!heaps[i])
> + continue;
We don't really need this NULL check and it isn't there in the
hi6220_ion_probe() unwind code.
> + ion_heap_destroy(heaps[i]);
> + heaps[i] = NULL;
> + }
> +
> + return 0;
> +}
regards,
dan carpenter
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 2/3] staging: android: ion: Add ion driver for Hi6220 SoC platform
2015-10-09 8:53 ` Dan Carpenter
@ 2015-10-09 8:58 ` Dan Carpenter
2015-10-10 6:07 ` chenfeng
0 siblings, 1 reply; 9+ messages in thread
From: Dan Carpenter @ 2015-10-09 8:58 UTC (permalink / raw)
To: Chen Feng
Cc: dan.zhao, w.f, linuxarm, paul.gortmaker, sumit.semwal, devel,
xuyiping, tapaswenipathak, tranmanphong, z.liuxinliang,
kong.kongxinwei, qijiwen, weidong2, suzhuangluan, riandrews,
gioh.kim, gregkh, peter.panshilin, linux-kernel, arve,
saberlily.xia
On Fri, Oct 09, 2015 at 11:53:32AM +0300, Dan Carpenter wrote:
> > +out:
>
> Labels named "out" are bug prone because handling everything is harder
> than using named labels and unwinding one step at a time. The bug here
> is that we don't call ion_device_destroy().
>
> > + for (i = 0; i < num_heaps; ++i)
> > + ion_heap_destroy(heaps[i]);
> > + return err;
>
> Write it like this:
>
> err_free_heaps:
> for (i = 0; i < num_heaps; ++i)
> ion_heap_destroy(heaps[i]);
> err_free_idev:
> ion_device_destroy(idev);
>
> return err;
>
> > +}
> > +
> > +static int hi6220_ion_remove(struct platform_device *pdev)
> > +{
> > + int i;
> > +
> > + ion_device_destroy(idev);
> > + for (i = 0; i < num_heaps; i++) {
> > + if (!heaps[i])
> > + continue;
>
> We don't really need this NULL check and it isn't there in the
> hi6220_ion_probe() unwind code.
>
> > + ion_heap_destroy(heaps[i]);
> > + heaps[i] = NULL;
> > + }
> > +
Really the unwind from probe() and the remove() function should have
similar code. For example, is it important to set heaps[i] to NULL?
If so then we should do it in the probe function as well. If not then
we could leave it out of the remove function.
Also the ion_device_destroy(idev) should be after freeing heaps in the
remove function.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 2/3] staging: android: ion: Add ion driver for Hi6220 SoC platform
2015-10-09 8:58 ` Dan Carpenter
@ 2015-10-10 6:07 ` chenfeng
0 siblings, 0 replies; 9+ messages in thread
From: chenfeng @ 2015-10-10 6:07 UTC (permalink / raw)
To: Dan Carpenter
Cc: dan.zhao, w.f, linuxarm, paul.gortmaker, sumit.semwal, devel,
xuyiping, tapaswenipathak, tranmanphong, z.liuxinliang,
kong.kongxinwei, qijiwen, weidong2, suzhuangluan, riandrews,
gioh.kim, gregkh, peter.panshilin, linux-kernel, arve,
saberlily.xia
On 2015/10/9 16:58, Dan Carpenter wrote:
> On Fri, Oct 09, 2015 at 11:53:32AM +0300, Dan Carpenter wrote:
>>> +out:
>>
>> Labels named "out" are bug prone because handling everything is harder
>> than using named labels and unwinding one step at a time. The bug here
>> is that we don't call ion_device_destroy().
>>
>>> + for (i = 0; i < num_heaps; ++i)
>>> + ion_heap_destroy(heaps[i]);
>>> + return err;
>>
>> Write it like this:
>>
>> err_free_heaps:
>> for (i = 0; i < num_heaps; ++i)
>> ion_heap_destroy(heaps[i]);
>> err_free_idev:
>> ion_device_destroy(idev);
>>
>> return err;
>>
>>> +}
>>> +
>>> +static int hi6220_ion_remove(struct platform_device *pdev)
>>> +{
>>> + int i;
>>> +
>>> + ion_device_destroy(idev);
>>> + for (i = 0; i < num_heaps; i++) {
>>> + if (!heaps[i])
>>> + continue;
>>
>> We don't really need this NULL check and it isn't there in the
>> hi6220_ion_probe() unwind code.
>>
>>> + ion_heap_destroy(heaps[i]);
>>> + heaps[i] = NULL;
>>> + }
>>> +
>
> Really the unwind from probe() and the remove() function should have
> similar code. For example, is it important to set heaps[i] to NULL?
> If so then we should do it in the probe function as well. If not then
> we could leave it out of the remove function.
>
> Also the ion_device_destroy(idev) should be after freeing heaps in the
> remove function.
>
Thanks.
I will modify this next version.
> regards,
> dan carpenter
>
>
> .
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] staging: android: ion: Add ion driver for Hi6220 SoC platform
2015-10-08 7:55 ` [PATCH 2/3] staging: android: ion: Add ion driver for Hi6220 SoC platform Chen Feng
` (2 preceding siblings ...)
2015-10-09 8:53 ` Dan Carpenter
@ 2015-10-09 9:08 ` xuyiping
3 siblings, 0 replies; 9+ messages in thread
From: xuyiping @ 2015-10-09 9:08 UTC (permalink / raw)
To: Chen Feng, gregkh, arve, riandrews, mitchelh, tranmanphong,
gioh.kim, paul.gortmaker, tapaswenipathak, sumit.semwal, devel,
linux-kernel
Cc: linuxarm, dan.zhao, peter.panshilin, qijiwen, w.f, z.liuxinliang,
kong.kongxinwei, weidong2, saberlily.xia, suzhuangluan
On 2015/10/8 15:55, Chen Feng wrote:
> Signed-off-by: Chen Feng <puck.chen@hisilicon.com>
> Signed-off-by: Yu Dongbin <yudongbin@hisilicon.com>
> ---
> drivers/staging/android/ion/Kconfig | 7 +
> drivers/staging/android/ion/Makefile | 1 +
> drivers/staging/android/ion/hisilicon/Kconfig | 5 +
> drivers/staging/android/ion/hisilicon/Makefile | 1 +
> drivers/staging/android/ion/hisilicon/hi6220_ion.c | 201 +++++++++++++++++++++
> 5 files changed, 215 insertions(+)
> create mode 100644 drivers/staging/android/ion/hisilicon/Kconfig
> create mode 100644 drivers/staging/android/ion/hisilicon/Makefile
> create mode 100644 drivers/staging/android/ion/hisilicon/hi6220_ion.c
>
> diff --git a/drivers/staging/android/ion/Kconfig b/drivers/staging/android/ion/Kconfig
> index 3452346..19c1572 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_HISI
> + tristate "Ion for Hisilicon"
> + depends on ARCH_HISI && ION
> + help
> + Choose this option if you wish to use ion on Hisilicon Platform.
> +
> +source "drivers/staging/android/ion/hisilicon/Kconfig"
> diff --git a/drivers/staging/android/ion/Makefile b/drivers/staging/android/ion/Makefile
> index b56fd2b..18cc2aa 100644
> --- a/drivers/staging/android/ion/Makefile
> +++ b/drivers/staging/android/ion/Makefile
> @@ -7,4 +7,5 @@ endif
>
> obj-$(CONFIG_ION_DUMMY) += ion_dummy_driver.o
> obj-$(CONFIG_ION_TEGRA) += tegra/
> +obj-$(CONFIG_ION_HISI) += hisilicon/
>
> diff --git a/drivers/staging/android/ion/hisilicon/Kconfig b/drivers/staging/android/ion/hisilicon/Kconfig
> new file mode 100644
> index 0000000..2b4bd07
> --- /dev/null
> +++ b/drivers/staging/android/ion/hisilicon/Kconfig
> @@ -0,0 +1,5 @@
> +config HI6220_ION
> + bool "Hi6220 ION Driver"
> + depends on ARCH_HISI && ION
> + help
> + Build the Hisilicon Hi6220 ion driver.
> diff --git a/drivers/staging/android/ion/hisilicon/Makefile b/drivers/staging/android/ion/hisilicon/Makefile
> new file mode 100644
> index 0000000..2a89414
> --- /dev/null
> +++ b/drivers/staging/android/ion/hisilicon/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_HI6220_ION) += hi6220_ion.o
> diff --git a/drivers/staging/android/ion/hisilicon/hi6220_ion.c b/drivers/staging/android/ion/hisilicon/hi6220_ion.c
> new file mode 100644
> index 0000000..b7d39b8
> --- /dev/null
> +++ b/drivers/staging/android/ion/hisilicon/hi6220_ion.c
> @@ -0,0 +1,201 @@
> +#define pr_fmt(fmt) "Ion: " fmt
> +
> +#include <linux/err.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/of.h>
> +#include <linux/mm.h>
> +#include "../ion_priv.h"
> +#include "../ion.h"
> +
> +struct hi6220_ion_type_table {
> + const char *name;
> + enum ion_heap_type type;
> +};
> +
> +static struct hi6220_ion_type_table ion_type_table[] = {
> + {"ion_system", ION_HEAP_TYPE_SYSTEM},
> + {"ion_system_contig", ION_HEAP_TYPE_SYSTEM_CONTIG},
> + {"ion_carveout", ION_HEAP_TYPE_CARVEOUT},
> + {"ion_chunk", ION_HEAP_TYPE_CHUNK},
> + {"ion_dma", ION_HEAP_TYPE_DMA},
> + {"ion_custom", ION_HEAP_TYPE_CUSTOM},
> +};
> +
> +static struct ion_device *idev;
> +static int num_heaps;
> +static struct ion_heap **heaps;
> +static struct ion_platform_heap **heaps_data;
> +
> +static int get_type_by_name(const char *name, enum ion_heap_type *type)
> +{
> + int i, n;
> +
> + n = ARRAY_SIZE(ion_type_table);
> + for (i = 0; i < n; i++) {
> + if (strncmp(name, ion_type_table[i].name, strlen(name)))
> + continue;
> +
> + *type = ion_type_table[i].type;
> + return 0;
> + }
> +
> + return -1;
> +}
> +
> +static int hi6220_set_platform_data(struct platform_device *pdev)
> +{
> + unsigned int base;
> + unsigned int size;
> + unsigned int id;
> + const char *heap_name;
> + const char *type_name;
> + enum ion_heap_type type;
> + int ret;
> + struct device_node *np;
> + struct ion_platform_heap *p_data;
> + const struct device_node *dt_node = pdev->dev.of_node;
> + int index = 0;
> +
> + for_each_child_of_node(dt_node, np)
> + num_heaps++;
indentation
> + heaps_data = devm_kzalloc(&pdev->dev,
> + sizeof(struct ion_platform_heap *) * num_heaps,
> + GFP_KERNEL);
check the result of malloc
> + for_each_child_of_node(dt_node, np) {
> + p_data = devm_kzalloc(&pdev->dev,
> + sizeof(struct ion_platform_heap),
> + GFP_KERNEL);
check the result of malloc
> + ret = of_property_read_string(np, "heap-name", &heap_name);
> + if (ret < 0) {
> + pr_err("check the name of node %s\n", np->name);
> + continue;
> + }
> +
> + ret = of_property_read_u32(np, "heap-id", &id);
> + if (ret < 0) {
> + pr_err("check the id %s\n", np->name);
> + continue;
> + }
> +
> + ret = of_property_read_u32(np, "heap-base", &base);
> + if (ret < 0) {
> + pr_err("check the base of node %s\n", np->name);
> + continue;
> + }
> +
> + ret = of_property_read_u32(np, "heap-size", &size);
> + if (ret < 0) {
> + pr_err("check the size of node %s\n", np->name);
> + continue;
> + }
> +
> + ret = of_property_read_string(np, "heap-type", &type_name);
> + if (ret < 0) {
> + pr_err("check the type of node %s\n", np->name);
> + continue;
> + }
> +
> + ret = get_type_by_name(type_name, &type);
> + if (ret < 0) {
> + pr_err("type name error %s!\n", type_name);
> + continue;
> + }
malloc the heap data here, or, free the ion_platform_heap data in
every (ret < 0) case below.
> + pr_info("heap index %d : name %s base 0x%x size 0x%x id %d type %d\n",
> + index, heap_name, base, size, id, type);
> +
> + p_data->name = heap_name;
> + p_data->base = base;
> + p_data->size = size;
> + p_data->id = id;
> + p_data->type = type;
> +
> + heaps_data[index] = p_data;
> + index++;
> + }
> + return 0;
> +}
> +
> +static int __init hi6220_ion_probe(struct platform_device *pdev)
> +{
> + int i;
> + int err = 0;
> + static struct ion_platform_heap *p_heap;
> +
> + idev = ion_device_create(NULL);
> + if (IS_ERR_OR_NULL(idev)) {
> + pr_err("ion_device_create err %lx\n", PTR_ERR(idev));
> + return PTR_ERR(idev);
> + }
> +
> + hi6220_set_platform_data(pdev);
> +
> + heaps = devm_kzalloc(&pdev->dev,
> + sizeof(struct ion_heap *) * num_heaps,
> + GFP_KERNEL);
> +
> + /*
> + * create the heaps as specified in the dts file
> + */
> + for (i = 0; i < num_heaps; i++) {
> + p_heap = heaps_data[i];
> + heaps[i] = ion_heap_create(p_heap);
> + if (IS_ERR_OR_NULL(heaps[i])) {
> + err = PTR_ERR(heaps[i]);
> + goto out;
> + }
> +
> + ion_device_add_heap(idev, heaps[i]);
> +
> + pr_info("%s: adding heap %s of type %d with %lx@%lx\n",
> + __func__, p_heap->name, p_heap->type,
> + p_heap->base, (unsigned long)p_heap->size);
> + }
> + return err;
> +out:
> + for (i = 0; i < num_heaps; ++i)
> + ion_heap_destroy(heaps[i]);
> + return err;
> +}
> +
> +static int hi6220_ion_remove(struct platform_device *pdev)
> +{
> + int i;
> +
> + ion_device_destroy(idev);
> + for (i = 0; i < num_heaps; i++) {
> + if (!heaps[i])
> + continue;
> + ion_heap_destroy(heaps[i]);
> + heaps[i] = NULL;
> + }
> +
> + return 0;
> +}
> +
> +static const struct of_device_id hi6220_ion_match_table[] = {
> + {.compatible = "hisilicon,hi6220-ion"},
> + {},
> +};
> +
> +static struct platform_driver hi6220_ion_driver = {
> + .probe = hi6220_ion_probe,
> + .remove = hi6220_ion_remove,
> + .driver = {
> + .name = "ion-hi6220",
> + .of_match_table = hi6220_ion_match_table,
> + },
> +};
> +
> +static int __init hi6220_ion_init(void)
> +{
> + int ret;
> +
> + ret = platform_driver_register(&hi6220_ion_driver);
> + return ret;
> +}
> +
> +subsys_initcall(hi6220_ion_init);
>
^ permalink raw reply [flat|nested] 9+ messages in thread