[PATCH V1 1/3] docs: dts: Add documentation for hi6220 SoC ION node

[PATCH V1 1/3] docs: dts: Add documentation for hi6220 SoC ION node

[Chen Feng]

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 | 31 ++++++++++++++++++++++
 1 file changed, 31 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..97ba233
--- /dev/null
+++ b/Documentation/devicetree/bindings/staging/ion/hi6220-ion.txt
@@ -0,0 +1,31 @@
+Hi6220 SoC ION
+============================================================
+Required properties:
+- compatible : "hisilicon,hi6220-ion"
+- list of the ION heaps
+	- heap name : maybe heap_sys_user@0
+	- heap id   : id should be unique in the system.
+	- heap base : base ddr address of the heap,0 means that
+	it is dynamic.
+	- heap size : memory size and 0 means it is dynamic.
+	- heap type : the heap type of the heap, please also
+see the define in ion.h(drivers/staging/android/uapi/ion.h)
+------------------------------------------------------------
+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

[PATCH V1 2/3] taging: android: ion: Add ion driver for Hi6220 SoC platform

[Chen Feng]

Add ion support for hi6220 SoC platform.

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 | 204 +++++++++++++++++++++
 5 files changed, 218 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..0e8632a
--- /dev/null
+++ b/drivers/staging/android/ion/hisilicon/hi6220_ion.c
@@ -0,0 +1,204 @@
+#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;
+
+	for (i = 0; i < ARRAY_SIZE(ion_type_table); i++) {
+		if (strncmp(name, ion_type_table[i].name, strlen(name)))
+			continue;
+
+		*type = ion_type_table[i].type;
+		return 0;
+	}
+
+	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++;
+
+	heaps_data = devm_kzalloc(&pdev->dev,
+				  sizeof(struct ion_platform_heap *) *
+				  num_heaps,
+				  GFP_KERNEL);
+	if (!heaps_data)
+		return -ENOMEM;
+
+	for_each_child_of_node(dt_node, np) {
+		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 = devm_kzalloc(&pdev->dev,
+				      sizeof(struct ion_platform_heap),
+				      GFP_KERNEL);
+		if (!p_data)
+			return -ENOMEM;
+
+		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 hi6220_ion_probe(struct platform_device *pdev)
+{
+	int i;
+	int err = 0;
+	static struct ion_platform_heap *p_heap;
+
+	idev = ion_device_create(NULL);
+	hi6220_set_platform_data(pdev);
+	heaps = devm_kzalloc(&pdev->dev,
+			     sizeof(struct ion_heap *) * num_heaps,
+			     GFP_KERNEL);
+	if (!heaps)
+		return -ENOMEM;
+
+	/*
+	 * 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 err_free_heaps;
+		}
+
+		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;
+
+err_free_heaps:
+	ion_device_destroy(idev);
+	for (i = 0; i < num_heaps; ++i) {
+		ion_heap_destroy(heaps[i]);
+		heaps[i] = NULL;
+	}
+
+	return err;
+}
+
+static int hi6220_ion_remove(struct platform_device *pdev)
+{
+	int i;
+
+	ion_device_destroy(idev);
+	for (i = 0; i < num_heaps; i++) {
+		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

[PATCH V1 3/3] arm64: dts: Add dts files to enable ION on Hi6220 SoC.

[Chen Feng]

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  | 20 ++++++++++++++++++++
 2 files changed, 21 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..c79f2ce
--- /dev/null
+++ b/arch/arm64/boot/dts/hisilicon/hi6220-ion.dtsi
@@ -0,0 +1,20 @@
+/ {
+	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

Re: [PATCH V1 2/3] taging: android: ion: Add ion driver for Hi6220 SoC platform

[Dan Carpenter]

On Sat, Oct 10, 2015 at 02:48:22PM +0800, Chen Feng wrote:
> +static int hi6220_ion_probe(struct platform_device *pdev)
> +{
> +	int i;
> +	int err = 0;
> +	static struct ion_platform_heap *p_heap;
> +
> +	idev = ion_device_create(NULL);
> +	hi6220_set_platform_data(pdev);
> +	heaps = devm_kzalloc(&pdev->dev,
> +			     sizeof(struct ion_heap *) * num_heaps,
> +			     GFP_KERNEL);
> +	if (!heaps)
> +		return -ENOMEM;
> +
> +	/*
> +	 * 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 err_free_heaps;
> +		}
> +
> +		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;
> +
> +err_free_heaps:
> +	ion_device_destroy(idev);
> +	for (i = 0; i < num_heaps; ++i) {
> +		ion_heap_destroy(heaps[i]);
> +		heaps[i] = NULL;
> +	}
> +
> +	return err;
> +}

Thanks this is better but still not quite right.  You have to unwind in
the reverse order from how you allocated things.

err_free_heaps:
	for (i = 0; i < num_heaps; ++i) {
		ion_heap_destroy(heaps[i]);
		heaps[i] = NULL;
	}
err_destroy_idev:
	ion_device_destroy(idev);

	return err;

And earlier it should be:

 	idev = ion_device_create(NULL);
 	hi6220_set_platform_data(pdev);
 	heaps = devm_kzalloc(&pdev->dev,
 			     sizeof(struct ion_heap *) * num_heaps,
 			     GFP_KERNEL);
-	if (!heaps)
-		return -ENOMEM;
+	if (!heaps) {
+		err = -ENOMEM;
+		goto err_destroy_idev;
+	}

Otherwise we leak some resources if we can't allocate "heaps".

regards,
dan carpenter

Re: [PATCH V1 2/3] taging: android: ion: Add ion driver for Hi6220 SoC platform

[chenfeng]

On 2015/10/10 23:00, Dan Carpenter wrote:
> On Sat, Oct 10, 2015 at 02:48:22PM +0800, Chen Feng wrote:
>> +static int hi6220_ion_probe(struct platform_device *pdev)
>> +{
>> +	int i;
>> +	int err = 0;
>> +	static struct ion_platform_heap *p_heap;
>> +
>> +	idev = ion_device_create(NULL);
>> +	hi6220_set_platform_data(pdev);
>> +	heaps = devm_kzalloc(&pdev->dev,
>> +			     sizeof(struct ion_heap *) * num_heaps,
>> +			     GFP_KERNEL);
>> +	if (!heaps)
>> +		return -ENOMEM;
>> +
>> +	/*
>> +	 * 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 err_free_heaps;
>> +		}
>> +
>> +		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;
>> +
>> +err_free_heaps:
>> +	ion_device_destroy(idev);
>> +	for (i = 0; i < num_heaps; ++i) {
>> +		ion_heap_destroy(heaps[i]);
>> +		heaps[i] = NULL;
>> +	}
>> +
>> +	return err;
>> +}
> 
> Thanks this is better but still not quite right.  You have to unwind in
> the reverse order from how you allocated things.
> 
> err_free_heaps:
> 	for (i = 0; i < num_heaps; ++i) {
> 		ion_heap_destroy(heaps[i]);
> 		heaps[i] = NULL;
> 	}
> err_destroy_idev:
> 	ion_device_destroy(idev);
> 
> 	return err;
> 
> And earlier it should be:
> 
>  	idev = ion_device_create(NULL);
>  	hi6220_set_platform_data(pdev);
>  	heaps = devm_kzalloc(&pdev->dev,
>  			     sizeof(struct ion_heap *) * num_heaps,
>  			     GFP_KERNEL);
> -	if (!heaps)
> -		return -ENOMEM;
> +	if (!heaps) {
> +		err = -ENOMEM;
> +		goto err_destroy_idev;
> +	}
> 
> Otherwise we leak some resources if we can't allocate "heaps".
> 
Yeah,it's right. I will fix this.

> regards,
> dan carpenter
> 
> 
> .
>