* [PATCH 1/3] docs: dts: Add documentation for hi6220 SoC ION node
@ 2015-10-08 7:55 Chen Feng
2015-10-08 7:55 ` [PATCH 2/3] staging: android: ion: Add ion driver for Hi6220 SoC platform Chen Feng
2015-10-08 7:55 ` [PATCH 3/3] arm64: dts: Add dts files to enable ION on Hi6220 SoC Chen Feng
0 siblings, 2 replies; 9+ messages in thread
From: Chen Feng @ 2015-10-08 7:55 UTC (permalink / raw)
To: puck.chen, gregkh, arve, riandrews, mitchelh, tranmanphong,
gioh.kim, paul.gortmaker, tapaswenipathak, sumit.semwal, devel,
linux-kernel
Cc: linuxarm, dan.zhao, peter.panshilin, qijiwen, xuyiping, w.f,
z.liuxinliang, kong.kongxinwei, weidong2, saberlily.xia,
suzhuangluan
Documentation for hi6220 SoC ION node
Signed-off-by: Chen Feng <puck.chen@hisilicon.com>
Signed-off-by: Yu Dongbin <yudongbin@hisilicon.com>
---
.../devicetree/bindings/staging/ion/hi6220-ion.txt | 27 ++++++++++++++++++++++
1 file changed, 27 insertions(+)
create mode 100644 Documentation/devicetree/bindings/staging/ion/hi6220-ion.txt
diff --git a/Documentation/devicetree/bindings/staging/ion/hi6220-ion.txt b/Documentation/devicetree/bindings/staging/ion/hi6220-ion.txt
new file mode 100644
index 0000000..5f969d7
--- /dev/null
+++ b/Documentation/devicetree/bindings/staging/ion/hi6220-ion.txt
@@ -0,0 +1,27 @@
+Hi6220 SoC ION
+
+Required properties:
+- compatible : "hisilicon,hi6220-ion"
+- list of the ION heaps
+
+Example:
+ hi6220-ion {
+ compatible = "hisilicon,hi6220-ion";
+
+ heap_sys_user@0 {
+ heap-name = "sys_user";
+ heap-id = <0x0>;
+ heap-base = <0x0>;
+ heap-size = <0x0>;
+ heap-type = "ion_system";
+ };
+
+ heap_sys_contig@0 {
+ heap-name = "sys_contig";
+ heap-id = <0x1>;
+ heap-base = <0x0>;
+ heap-size = <0x0>;
+ heap-type = "ion_system_contig";
+ };
+ };
+
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/3] staging: android: ion: Add ion driver for Hi6220 SoC platform
2015-10-08 7:55 [PATCH 1/3] docs: dts: Add documentation for hi6220 SoC ION node Chen Feng
@ 2015-10-08 7:55 ` Chen Feng
2015-10-08 8:52 ` Greg KH
` (3 more replies)
2015-10-08 7:55 ` [PATCH 3/3] arm64: dts: Add dts files to enable ION on Hi6220 SoC Chen Feng
1 sibling, 4 replies; 9+ messages in thread
From: Chen Feng @ 2015-10-08 7:55 UTC (permalink / raw)
To: puck.chen, gregkh, arve, riandrews, mitchelh, tranmanphong,
gioh.kim, paul.gortmaker, tapaswenipathak, sumit.semwal, devel,
linux-kernel
Cc: linuxarm, dan.zhao, peter.panshilin, qijiwen, xuyiping, w.f,
z.liuxinliang, kong.kongxinwei, weidong2, saberlily.xia,
suzhuangluan
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++;
+
+ heaps_data = devm_kzalloc(&pdev->dev,
+ sizeof(struct ion_platform_heap *) * num_heaps,
+ GFP_KERNEL);
+
+ for_each_child_of_node(dt_node, np) {
+ p_data = devm_kzalloc(&pdev->dev,
+ sizeof(struct ion_platform_heap),
+ GFP_KERNEL);
+
+ 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));
+ 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);
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/3] arm64: dts: Add dts files to enable ION on Hi6220 SoC.
2015-10-08 7:55 [PATCH 1/3] docs: dts: Add documentation for hi6220 SoC ION node Chen Feng
2015-10-08 7:55 ` [PATCH 2/3] staging: android: ion: Add ion driver for Hi6220 SoC platform Chen Feng
@ 2015-10-08 7:55 ` Chen Feng
1 sibling, 0 replies; 9+ messages in thread
From: Chen Feng @ 2015-10-08 7:55 UTC (permalink / raw)
To: puck.chen, gregkh, arve, riandrews, mitchelh, tranmanphong,
gioh.kim, paul.gortmaker, tapaswenipathak, sumit.semwal, devel,
linux-kernel
Cc: linuxarm, dan.zhao, peter.panshilin, qijiwen, xuyiping, w.f,
z.liuxinliang, kong.kongxinwei, weidong2, saberlily.xia,
suzhuangluan
Add ION node to enable ION on hi6220 SoC platform
Signed-off-by: Chen Feng <puck.chen@hisilicon.com>
Signed-off-by: Yu Dongbin <yudongbin@hisilicon.com>
---
arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 1 +
arch/arm64/boot/dts/hisilicon/hi6220-ion.dtsi | 23 +++++++++++++++++++++++
2 files changed, 24 insertions(+)
create mode 100644 arch/arm64/boot/dts/hisilicon/hi6220-ion.dtsi
diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
index e36a539..44b75d2 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
+++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
@@ -11,6 +11,7 @@
/memreserve/ 0x05e00000 0x00100000;
#include "hi6220.dtsi"
+#include "hi6220-ion.dtsi"
/ {
model = "HiKey Development Board";
diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-ion.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220-ion.dtsi
new file mode 100644
index 0000000..24d3722
--- /dev/null
+++ b/arch/arm64/boot/dts/hisilicon/hi6220-ion.dtsi
@@ -0,0 +1,23 @@
+/ {
+ hi6220-ion {
+ compatible = "hisilicon,hi6220-ion";
+
+ heap_sys_user@0 {
+ heap-name = "sys_user";
+ heap-id = <0x0>;
+ heap-base = <0x0>;
+ heap-size = <0x0>;
+ heap-type = "ion_system";
+ };
+
+ heap_sys_contig@0 {
+ heap-name = "sys_contig";
+ heap-id = <0x1>;
+ heap-base = <0x0>;
+ heap-size = <0x0>;
+ heap-type = "ion_system_contig";
+ };
+ };
+
+};
+
--
1.9.1
^ permalink raw reply related [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
` (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-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
* 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
end of thread, other threads:[~2015-10-10 6:11 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-08 7:55 [PATCH 1/3] docs: dts: Add documentation for hi6220 SoC ION node Chen Feng
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-10 6:07 ` chenfeng
2015-10-09 9:08 ` xuyiping
2015-10-08 7:55 ` [PATCH 3/3] arm64: dts: Add dts files to enable ION on Hi6220 SoC Chen Feng
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.