[PATCH v2 0/2] PM / AVS: SmartReflex: optimization series

[PATCH v2 0/2] PM / AVS: SmartReflex: optimization series

[Andrii Tseglytskyi]

The following patch series introduces a few optimizations for SmartReflex driver.

1. devm_* API usage for SmartReflex.
Benefits:
- auto handling of resources - allocation/auto free, map/auto unmap.
- lot of error checks can be dropped.

2. Another small optimization resolves order of initialization between SmartReflex driver
and SmartReflex class. Class is initialized before driver.

Patches are based on:
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
tag: v3.10-rc4

Verified on OMAP4430. Boot - OK. SmartReflex registers debug dump - OK

Series v2 contains:
- spurious #define removed from "PM / AVS: SmartReflex: use devm_* API to initialize SmartReflex"
- rebased to v3.10-rc4

Andrii Tseglytskyi (2):
  PM / AVS: SmartReflex: use devm_* API to initialize SmartReflex
  PM / AVS: SmartReflex/class3: Fix order of initialization of SR class
    and SR driver

 arch/arm/mach-omap2/smartreflex-class3.c |    2 +-
 drivers/power/avs/smartreflex.c          |   85 ++++++++----------------------
 2 files changed, 22 insertions(+), 65 deletions(-)

-- 
1.7.9.5

[PATCH v2 0/2] PM / AVS: SmartReflex: optimization series

[Andrii Tseglytskyi]

The following patch series introduces a few optimizations for SmartReflex driver.

1. devm_* API usage for SmartReflex.
Benefits:
- auto handling of resources - allocation/auto free, map/auto unmap.
- lot of error checks can be dropped.

2. Another small optimization resolves order of initialization between SmartReflex driver
and SmartReflex class. Class is initialized before driver.

Patches are based on:
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
tag: v3.10-rc4

Verified on OMAP4430. Boot - OK. SmartReflex registers debug dump - OK

Series v2 contains:
- spurious #define removed from "PM / AVS: SmartReflex: use devm_* API to initialize SmartReflex"
- rebased to v3.10-rc4

Andrii Tseglytskyi (2):
  PM / AVS: SmartReflex: use devm_* API to initialize SmartReflex
  PM / AVS: SmartReflex/class3: Fix order of initialization of SR class
    and SR driver

 arch/arm/mach-omap2/smartreflex-class3.c |    2 +-
 drivers/power/avs/smartreflex.c          |   85 ++++++++----------------------
 2 files changed, 22 insertions(+), 65 deletions(-)

-- 
1.7.9.5

[PATCH v2 1/2] PM / AVS: SmartReflex: use devm_* API to initialize SmartReflex

[Andrii Tseglytskyi]

Use of of devm_* API for resource allocation provides benefits such
as auto handling of resource free. This reduces possibility have
memory leaks in case of wrong error handling. All direct release
calls should be removed to avoid races.

Reported-by: Grygorii Strashko <grygorii.strashko@ti.com>
Signed-off-by: Andrii Tseglytskyi <andrii.tseglytskyi@ti.com>
---
 drivers/power/avs/smartreflex.c |   76 +++++++++++----------------------------
 1 file changed, 21 insertions(+), 55 deletions(-)

diff --git a/drivers/power/avs/smartreflex.c b/drivers/power/avs/smartreflex.c
index 6b2238b..fd71d5a 100644
--- a/drivers/power/avs/smartreflex.c
+++ b/drivers/power/avs/smartreflex.c
@@ -27,7 +27,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/power/smartreflex.h>
 
-#define SMARTREFLEX_NAME_LEN	16
+#define SMARTREFLEX_NAME_LEN	32
 #define NVALUE_NAME_LEN		40
 #define SR_DISABLE_TIMEOUT	200
 
@@ -207,12 +207,11 @@ static void sr_stop_vddautocomp(struct omap_sr *sr)
 static int sr_late_init(struct omap_sr *sr_info)
 {
 	struct omap_sr_data *pdata = sr_info->pdev->dev.platform_data;
-	struct resource *mem;
 	int ret = 0;
 
 	if (sr_class->notify && sr_class->notify_flags && sr_info->irq) {
-		ret = request_irq(sr_info->irq, sr_interrupt,
-				  0, sr_info->name, sr_info);
+		ret = devm_request_irq(&sr_info->pdev->dev, sr_info->irq,
+				       sr_interrupt, 0, sr_info->name, sr_info);
 		if (ret)
 			goto error;
 		disable_irq(sr_info->irq);
@@ -224,14 +223,10 @@ static int sr_late_init(struct omap_sr *sr_info)
 	return ret;
 
 error:
-	iounmap(sr_info->base);
-	mem = platform_get_resource(sr_info->pdev, IORESOURCE_MEM, 0);
-	release_mem_region(mem->start, resource_size(mem));
 	list_del(&sr_info->node);
 	dev_err(&sr_info->pdev->dev, "%s: ERROR in registering"
 		"interrupt handler. Smartreflex will"
 		"not function as desired\n", __func__);
-	kfree(sr_info);
 
 	return ret;
 }
@@ -847,34 +842,33 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 	struct dentry *nvalue_dir;
 	int i, ret = 0;
 
-	sr_info = kzalloc(sizeof(struct omap_sr), GFP_KERNEL);
+	sr_info = devm_kzalloc(&pdev->dev, sizeof(struct omap_sr), GFP_KERNEL);
 	if (!sr_info) {
 		dev_err(&pdev->dev, "%s: unable to allocate sr_info\n",
 			__func__);
 		return -ENOMEM;
 	}
 
+	sr_info->name = devm_kzalloc(&pdev->dev,
+				     SMARTREFLEX_NAME_LEN, GFP_KERNEL);
+	if (!sr_info->name) {
+		dev_err(&pdev->dev, "%s: unable to allocate SR instance name\n",
+			__func__);
+		return -ENOMEM;
+	}
+
 	platform_set_drvdata(pdev, sr_info);
 
 	if (!pdata) {
 		dev_err(&pdev->dev, "%s: platform data missing\n", __func__);
-		ret = -EINVAL;
-		goto err_free_devinfo;
+		return -EINVAL;
 	}
 
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!mem) {
-		dev_err(&pdev->dev, "%s: no mem resource\n", __func__);
-		ret = -ENODEV;
-		goto err_free_devinfo;
-	}
-
-	mem = request_mem_region(mem->start, resource_size(mem),
-					dev_name(&pdev->dev));
-	if (!mem) {
-		dev_err(&pdev->dev, "%s: no mem region\n", __func__);
-		ret = -EBUSY;
-		goto err_free_devinfo;
+	sr_info->base = devm_ioremap_resource(&pdev->dev, mem);
+	if (IS_ERR(sr_info->base)) {
+		dev_err(&pdev->dev, "%s: ioremap fail\n", __func__);
+		return PTR_ERR(sr_info->base);
 	}
 
 	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
@@ -882,13 +876,7 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 	pm_runtime_enable(&pdev->dev);
 	pm_runtime_irq_safe(&pdev->dev);
 
-	sr_info->name = kasprintf(GFP_KERNEL, "%s", pdata->name);
-	if (!sr_info->name) {
-		dev_err(&pdev->dev, "%s: Unable to alloc SR instance name\n",
-			__func__);
-		ret = -ENOMEM;
-		goto err_release_region;
-	}
+	snprintf(sr_info->name, SMARTREFLEX_NAME_LEN, "%s", pdata->name);
 
 	sr_info->pdev = pdev;
 	sr_info->srid = pdev->id;
@@ -905,13 +893,6 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 	sr_info->autocomp_active = false;
 	sr_info->ip_type = pdata->ip_type;
 
-	sr_info->base = ioremap(mem->start, resource_size(mem));
-	if (!sr_info->base) {
-		dev_err(&pdev->dev, "%s: ioremap fail\n", __func__);
-		ret = -ENOMEM;
-		goto err_free_name;
-	}
-
 	if (irq)
 		sr_info->irq = irq->start;
 
@@ -927,7 +908,7 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 		ret = sr_late_init(sr_info);
 		if (ret) {
 			pr_warning("%s: Error in SR late init\n", __func__);
-			goto err_iounmap;
+			goto err_list_del;
 		}
 	}
 
@@ -938,7 +919,7 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 			ret = PTR_ERR(sr_dbg_dir);
 			pr_err("%s:sr debugfs dir creation failed(%d)\n",
 				__func__, ret);
-			goto err_iounmap;
+			goto err_list_del;
 		}
 	}
 
@@ -991,16 +972,8 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 
 err_debugfs:
 	debugfs_remove_recursive(sr_info->dbg_dir);
-err_iounmap:
+err_list_del:
 	list_del(&sr_info->node);
-	iounmap(sr_info->base);
-err_free_name:
-	kfree(sr_info->name);
-err_release_region:
-	release_mem_region(mem->start, resource_size(mem));
-err_free_devinfo:
-	kfree(sr_info);
-
 	return ret;
 }
 
@@ -1008,7 +981,6 @@ static int omap_sr_remove(struct platform_device *pdev)
 {
 	struct omap_sr_data *pdata = pdev->dev.platform_data;
 	struct omap_sr *sr_info;
-	struct resource *mem;
 
 	if (!pdata) {
 		dev_err(&pdev->dev, "%s: platform data missing\n", __func__);
@@ -1028,12 +1000,6 @@ static int omap_sr_remove(struct platform_device *pdev)
 		debugfs_remove_recursive(sr_info->dbg_dir);
 
 	list_del(&sr_info->node);
-	iounmap(sr_info->base);
-	kfree(sr_info->name);
-	kfree(sr_info);
-	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	release_mem_region(mem->start, resource_size(mem));
-
 	return 0;
 }
 
-- 
1.7.9.5

[PATCH v2 1/2] PM / AVS: SmartReflex: use devm_* API to initialize SmartReflex

[Andrii Tseglytskyi]

Use of of devm_* API for resource allocation provides benefits such
as auto handling of resource free. This reduces possibility have
memory leaks in case of wrong error handling. All direct release
calls should be removed to avoid races.

Reported-by: Grygorii Strashko <grygorii.strashko@ti.com>
Signed-off-by: Andrii Tseglytskyi <andrii.tseglytskyi@ti.com>
---
 drivers/power/avs/smartreflex.c |   76 +++++++++++----------------------------
 1 file changed, 21 insertions(+), 55 deletions(-)

diff --git a/drivers/power/avs/smartreflex.c b/drivers/power/avs/smartreflex.c
index 6b2238b..fd71d5a 100644
--- a/drivers/power/avs/smartreflex.c
+++ b/drivers/power/avs/smartreflex.c
@@ -27,7 +27,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/power/smartreflex.h>
 
-#define SMARTREFLEX_NAME_LEN	16
+#define SMARTREFLEX_NAME_LEN	32
 #define NVALUE_NAME_LEN		40
 #define SR_DISABLE_TIMEOUT	200
 
@@ -207,12 +207,11 @@ static void sr_stop_vddautocomp(struct omap_sr *sr)
 static int sr_late_init(struct omap_sr *sr_info)
 {
 	struct omap_sr_data *pdata = sr_info->pdev->dev.platform_data;
-	struct resource *mem;
 	int ret = 0;
 
 	if (sr_class->notify && sr_class->notify_flags && sr_info->irq) {
-		ret = request_irq(sr_info->irq, sr_interrupt,
-				  0, sr_info->name, sr_info);
+		ret = devm_request_irq(&sr_info->pdev->dev, sr_info->irq,
+				       sr_interrupt, 0, sr_info->name, sr_info);
 		if (ret)
 			goto error;
 		disable_irq(sr_info->irq);
@@ -224,14 +223,10 @@ static int sr_late_init(struct omap_sr *sr_info)
 	return ret;
 
 error:
-	iounmap(sr_info->base);
-	mem = platform_get_resource(sr_info->pdev, IORESOURCE_MEM, 0);
-	release_mem_region(mem->start, resource_size(mem));
 	list_del(&sr_info->node);
 	dev_err(&sr_info->pdev->dev, "%s: ERROR in registering"
 		"interrupt handler. Smartreflex will"
 		"not function as desired\n", __func__);
-	kfree(sr_info);
 
 	return ret;
 }
@@ -847,34 +842,33 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 	struct dentry *nvalue_dir;
 	int i, ret = 0;
 
-	sr_info = kzalloc(sizeof(struct omap_sr), GFP_KERNEL);
+	sr_info = devm_kzalloc(&pdev->dev, sizeof(struct omap_sr), GFP_KERNEL);
 	if (!sr_info) {
 		dev_err(&pdev->dev, "%s: unable to allocate sr_info\n",
 			__func__);
 		return -ENOMEM;
 	}
 
+	sr_info->name = devm_kzalloc(&pdev->dev,
+				     SMARTREFLEX_NAME_LEN, GFP_KERNEL);
+	if (!sr_info->name) {
+		dev_err(&pdev->dev, "%s: unable to allocate SR instance name\n",
+			__func__);
+		return -ENOMEM;
+	}
+
 	platform_set_drvdata(pdev, sr_info);
 
 	if (!pdata) {
 		dev_err(&pdev->dev, "%s: platform data missing\n", __func__);
-		ret = -EINVAL;
-		goto err_free_devinfo;
+		return -EINVAL;
 	}
 
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!mem) {
-		dev_err(&pdev->dev, "%s: no mem resource\n", __func__);
-		ret = -ENODEV;
-		goto err_free_devinfo;
-	}
-
-	mem = request_mem_region(mem->start, resource_size(mem),
-					dev_name(&pdev->dev));
-	if (!mem) {
-		dev_err(&pdev->dev, "%s: no mem region\n", __func__);
-		ret = -EBUSY;
-		goto err_free_devinfo;
+	sr_info->base = devm_ioremap_resource(&pdev->dev, mem);
+	if (IS_ERR(sr_info->base)) {
+		dev_err(&pdev->dev, "%s: ioremap fail\n", __func__);
+		return PTR_ERR(sr_info->base);
 	}
 
 	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
@@ -882,13 +876,7 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 	pm_runtime_enable(&pdev->dev);
 	pm_runtime_irq_safe(&pdev->dev);
 
-	sr_info->name = kasprintf(GFP_KERNEL, "%s", pdata->name);
-	if (!sr_info->name) {
-		dev_err(&pdev->dev, "%s: Unable to alloc SR instance name\n",
-			__func__);
-		ret = -ENOMEM;
-		goto err_release_region;
-	}
+	snprintf(sr_info->name, SMARTREFLEX_NAME_LEN, "%s", pdata->name);
 
 	sr_info->pdev = pdev;
 	sr_info->srid = pdev->id;
@@ -905,13 +893,6 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 	sr_info->autocomp_active = false;
 	sr_info->ip_type = pdata->ip_type;
 
-	sr_info->base = ioremap(mem->start, resource_size(mem));
-	if (!sr_info->base) {
-		dev_err(&pdev->dev, "%s: ioremap fail\n", __func__);
-		ret = -ENOMEM;
-		goto err_free_name;
-	}
-
 	if (irq)
 		sr_info->irq = irq->start;
 
@@ -927,7 +908,7 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 		ret = sr_late_init(sr_info);
 		if (ret) {
 			pr_warning("%s: Error in SR late init\n", __func__);
-			goto err_iounmap;
+			goto err_list_del;
 		}
 	}
 
@@ -938,7 +919,7 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 			ret = PTR_ERR(sr_dbg_dir);
 			pr_err("%s:sr debugfs dir creation failed(%d)\n",
 				__func__, ret);
-			goto err_iounmap;
+			goto err_list_del;
 		}
 	}
 
@@ -991,16 +972,8 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 
 err_debugfs:
 	debugfs_remove_recursive(sr_info->dbg_dir);
-err_iounmap:
+err_list_del:
 	list_del(&sr_info->node);
-	iounmap(sr_info->base);
-err_free_name:
-	kfree(sr_info->name);
-err_release_region:
-	release_mem_region(mem->start, resource_size(mem));
-err_free_devinfo:
-	kfree(sr_info);
-
 	return ret;
 }
 
@@ -1008,7 +981,6 @@ static int omap_sr_remove(struct platform_device *pdev)
 {
 	struct omap_sr_data *pdata = pdev->dev.platform_data;
 	struct omap_sr *sr_info;
-	struct resource *mem;
 
 	if (!pdata) {
 		dev_err(&pdev->dev, "%s: platform data missing\n", __func__);
@@ -1028,12 +1000,6 @@ static int omap_sr_remove(struct platform_device *pdev)
 		debugfs_remove_recursive(sr_info->dbg_dir);
 
 	list_del(&sr_info->node);
-	iounmap(sr_info->base);
-	kfree(sr_info->name);
-	kfree(sr_info);
-	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	release_mem_region(mem->start, resource_size(mem));
-
 	return 0;
 }
 
-- 
1.7.9.5

Re: [PATCH v2 1/2] PM / AVS: SmartReflex: use devm_* API to initialize SmartReflex

[Kevin Hilman]

Andrii Tseglytskyi <andrii.tseglytskyi@ti.com> writes:

> Use of of devm_* API for resource allocation provides benefits such
> as auto handling of resource free. This reduces possibility have
> memory leaks in case of wrong error handling. All direct release
> calls should be removed to avoid races.
>
> Reported-by: Grygorii Strashko <grygorii.strashko@ti.com>
> Signed-off-by: Andrii Tseglytskyi <andrii.tseglytskyi@ti.com>

Thanks, queuing this for v3.11.

Kevin

Re: [PATCH v2 1/2] PM / AVS: SmartReflex: use devm_* API to initialize SmartReflex

[Kevin Hilman]

Andrii Tseglytskyi <andrii.tseglytskyi@ti.com> writes:

> Use of of devm_* API for resource allocation provides benefits such
> as auto handling of resource free. This reduces possibility have
> memory leaks in case of wrong error handling. All direct release
> calls should be removed to avoid races.
>
> Reported-by: Grygorii Strashko <grygorii.strashko@ti.com>
> Signed-off-by: Andrii Tseglytskyi <andrii.tseglytskyi@ti.com>

Thanks, queuing this for v3.11.

Kevin

[PATCH v2 2/2] PM / AVS: SmartReflex/class3: Fix order of initialization of SR class and SR driver

[Andrii Tseglytskyi]

SmartReflex consists of three entities: SR device, SR class and
SR driver. SmartReflex driver depends on SmartReflex class, but
order of their initialization is not clear. They both use
late_initcall(), and order depends on Makefile calls.
Patch moves initialization of SR class to device_initcall(),
and removes redundant call of sr_late_init().

This provides predictable order of SmartReflex initcalls:
1. device_initcall() -> SmartReflex class init
2. late_initcall() -> SmartReflex driver init

Signed-off-by: Andrii Tseglytskyi <andrii.tseglytskyi@ti.com>
---
 arch/arm/mach-omap2/smartreflex-class3.c |    2 +-
 drivers/power/avs/smartreflex.c          |    9 ---------
 2 files changed, 1 insertion(+), 10 deletions(-)

diff --git a/arch/arm/mach-omap2/smartreflex-class3.c b/arch/arm/mach-omap2/smartreflex-class3.c
index aee3c89..50523b8 100644
--- a/arch/arm/mach-omap2/smartreflex-class3.c
+++ b/arch/arm/mach-omap2/smartreflex-class3.c
@@ -59,4 +59,4 @@ static int __init sr_class3_init(void)
 	pr_info("SmartReflex Class3 initialized\n");
 	return sr_register_class(&class3_data);
 }
-omap_late_initcall(sr_class3_init);
+omap_device_initcall(sr_class3_init);
diff --git a/drivers/power/avs/smartreflex.c b/drivers/power/avs/smartreflex.c
index fd71d5a..42eed34 100644
--- a/drivers/power/avs/smartreflex.c
+++ b/drivers/power/avs/smartreflex.c
@@ -650,8 +650,6 @@ void sr_disable(struct voltagedomain *voltdm)
  */
 int sr_register_class(struct omap_sr_class_data *class_data)
 {
-	struct omap_sr *sr_info;
-
 	if (!class_data) {
 		pr_warning("%s:, Smartreflex class data passed is NULL\n",
 			__func__);
@@ -666,13 +664,6 @@ int sr_register_class(struct omap_sr_class_data *class_data)
 
 	sr_class = class_data;
 
-	/*
-	 * Call into late init to do intializations that require
-	 * both sr driver and sr class driver to be initiallized.
-	 */
-	list_for_each_entry(sr_info, &sr_list, node)
-		sr_late_init(sr_info);
-
 	return 0;
 }
 
-- 
1.7.9.5

[PATCH v2 2/2] PM / AVS: SmartReflex/class3: Fix order of initialization of SR class and SR driver

[Andrii Tseglytskyi]

SmartReflex consists of three entities: SR device, SR class and
SR driver. SmartReflex driver depends on SmartReflex class, but
order of their initialization is not clear. They both use
late_initcall(), and order depends on Makefile calls.
Patch moves initialization of SR class to device_initcall(),
and removes redundant call of sr_late_init().

This provides predictable order of SmartReflex initcalls:
1. device_initcall() -> SmartReflex class init
2. late_initcall() -> SmartReflex driver init

Signed-off-by: Andrii Tseglytskyi <andrii.tseglytskyi@ti.com>
---
 arch/arm/mach-omap2/smartreflex-class3.c |    2 +-
 drivers/power/avs/smartreflex.c          |    9 ---------
 2 files changed, 1 insertion(+), 10 deletions(-)

diff --git a/arch/arm/mach-omap2/smartreflex-class3.c b/arch/arm/mach-omap2/smartreflex-class3.c
index aee3c89..50523b8 100644
--- a/arch/arm/mach-omap2/smartreflex-class3.c
+++ b/arch/arm/mach-omap2/smartreflex-class3.c
@@ -59,4 +59,4 @@ static int __init sr_class3_init(void)
 	pr_info("SmartReflex Class3 initialized\n");
 	return sr_register_class(&class3_data);
 }
-omap_late_initcall(sr_class3_init);
+omap_device_initcall(sr_class3_init);
diff --git a/drivers/power/avs/smartreflex.c b/drivers/power/avs/smartreflex.c
index fd71d5a..42eed34 100644
--- a/drivers/power/avs/smartreflex.c
+++ b/drivers/power/avs/smartreflex.c
@@ -650,8 +650,6 @@ void sr_disable(struct voltagedomain *voltdm)
  */
 int sr_register_class(struct omap_sr_class_data *class_data)
 {
-	struct omap_sr *sr_info;
-
 	if (!class_data) {
 		pr_warning("%s:, Smartreflex class data passed is NULL\n",
 			__func__);
@@ -666,13 +664,6 @@ int sr_register_class(struct omap_sr_class_data *class_data)
 
 	sr_class = class_data;
 
-	/*
-	 * Call into late init to do intializations that require
-	 * both sr driver and sr class driver to be initiallized.
-	 */
-	list_for_each_entry(sr_info, &sr_list, node)
-		sr_late_init(sr_info);
-
 	return 0;
 }
 
-- 
1.7.9.5

Re: [PATCH v2 2/2] PM / AVS: SmartReflex/class3: Fix order of initialization of SR class and SR driver

[Kevin Hilman]

Andrii Tseglytskyi <andrii.tseglytskyi@ti.com> writes:

> SmartReflex consists of three entities: SR device, SR class and
> SR driver. SmartReflex driver depends on SmartReflex class, but
> order of their initialization is not clear. They both use
> late_initcall(), and order depends on Makefile calls.
> Patch moves initialization of SR class to device_initcall(),
> and removes redundant call of sr_late_init().
>
> This provides predictable order of SmartReflex initcalls:
> 1. device_initcall() -> SmartReflex class init
> 2. late_initcall() -> SmartReflex driver init
>
> Signed-off-by: Andrii Tseglytskyi <andrii.tseglytskyi@ti.com>

Tony will have to decide on whether he's OK with the initcall changes.

I can queue this with the rest of the AVS changes with Tony's ack.

Kevin

> ---
>  arch/arm/mach-omap2/smartreflex-class3.c |    2 +-
>  drivers/power/avs/smartreflex.c          |    9 ---------
>  2 files changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/smartreflex-class3.c b/arch/arm/mach-omap2/smartreflex-class3.c
> index aee3c89..50523b8 100644
> --- a/arch/arm/mach-omap2/smartreflex-class3.c
> +++ b/arch/arm/mach-omap2/smartreflex-class3.c
> @@ -59,4 +59,4 @@ static int __init sr_class3_init(void)
>  	pr_info("SmartReflex Class3 initialized\n");
>  	return sr_register_class(&class3_data);
>  }
> -omap_late_initcall(sr_class3_init);
> +omap_device_initcall(sr_class3_init);
> diff --git a/drivers/power/avs/smartreflex.c b/drivers/power/avs/smartreflex.c
> index fd71d5a..42eed34 100644
> --- a/drivers/power/avs/smartreflex.c
> +++ b/drivers/power/avs/smartreflex.c
> @@ -650,8 +650,6 @@ void sr_disable(struct voltagedomain *voltdm)
>   */
>  int sr_register_class(struct omap_sr_class_data *class_data)
>  {
> -	struct omap_sr *sr_info;
> -
>  	if (!class_data) {
>  		pr_warning("%s:, Smartreflex class data passed is NULL\n",
>  			__func__);
> @@ -666,13 +664,6 @@ int sr_register_class(struct omap_sr_class_data *class_data)
>  
>  	sr_class = class_data;
>  
> -	/*
> -	 * Call into late init to do intializations that require
> -	 * both sr driver and sr class driver to be initiallized.
> -	 */
> -	list_for_each_entry(sr_info, &sr_list, node)
> -		sr_late_init(sr_info);
> -
>  	return 0;
>  }

Re: [PATCH v2 2/2] PM / AVS: SmartReflex/class3: Fix order of initialization of SR class and SR driver

[Kevin Hilman]

Andrii Tseglytskyi <andrii.tseglytskyi@ti.com> writes:

> SmartReflex consists of three entities: SR device, SR class and
> SR driver. SmartReflex driver depends on SmartReflex class, but
> order of their initialization is not clear. They both use
> late_initcall(), and order depends on Makefile calls.
> Patch moves initialization of SR class to device_initcall(),
> and removes redundant call of sr_late_init().
>
> This provides predictable order of SmartReflex initcalls:
> 1. device_initcall() -> SmartReflex class init
> 2. late_initcall() -> SmartReflex driver init
>
> Signed-off-by: Andrii Tseglytskyi <andrii.tseglytskyi@ti.com>

Tony will have to decide on whether he's OK with the initcall changes.

I can queue this with the rest of the AVS changes with Tony's ack.

Kevin

> ---
>  arch/arm/mach-omap2/smartreflex-class3.c |    2 +-
>  drivers/power/avs/smartreflex.c          |    9 ---------
>  2 files changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/smartreflex-class3.c b/arch/arm/mach-omap2/smartreflex-class3.c
> index aee3c89..50523b8 100644
> --- a/arch/arm/mach-omap2/smartreflex-class3.c
> +++ b/arch/arm/mach-omap2/smartreflex-class3.c
> @@ -59,4 +59,4 @@ static int __init sr_class3_init(void)
>  	pr_info("SmartReflex Class3 initialized\n");
>  	return sr_register_class(&class3_data);
>  }
> -omap_late_initcall(sr_class3_init);
> +omap_device_initcall(sr_class3_init);
> diff --git a/drivers/power/avs/smartreflex.c b/drivers/power/avs/smartreflex.c
> index fd71d5a..42eed34 100644
> --- a/drivers/power/avs/smartreflex.c
> +++ b/drivers/power/avs/smartreflex.c
> @@ -650,8 +650,6 @@ void sr_disable(struct voltagedomain *voltdm)
>   */
>  int sr_register_class(struct omap_sr_class_data *class_data)
>  {
> -	struct omap_sr *sr_info;
> -
>  	if (!class_data) {
>  		pr_warning("%s:, Smartreflex class data passed is NULL\n",
>  			__func__);
> @@ -666,13 +664,6 @@ int sr_register_class(struct omap_sr_class_data *class_data)
>  
>  	sr_class = class_data;
>  
> -	/*
> -	 * Call into late init to do intializations that require
> -	 * both sr driver and sr class driver to be initiallized.
> -	 */
> -	list_for_each_entry(sr_info, &sr_list, node)
> -		sr_late_init(sr_info);
> -
>  	return 0;
>  }

Re: [PATCH v2 2/2] PM / AVS: SmartReflex/class3: Fix order of initialization of SR class and SR driver

[Tony Lindgren]

* Kevin Hilman <khilman@linaro.org> [130610 10:58]:
> Andrii Tseglytskyi <andrii.tseglytskyi@ti.com> writes:
> 
> > SmartReflex consists of three entities: SR device, SR class and
> > SR driver. SmartReflex driver depends on SmartReflex class, but
> > order of their initialization is not clear. They both use
> > late_initcall(), and order depends on Makefile calls.
> > Patch moves initialization of SR class to device_initcall(),
> > and removes redundant call of sr_late_init().
> >
> > This provides predictable order of SmartReflex initcalls:
> > 1. device_initcall() -> SmartReflex class init
> > 2. late_initcall() -> SmartReflex driver init
> >
> > Signed-off-by: Andrii Tseglytskyi <andrii.tseglytskyi@ti.com>
> 
> Tony will have to decide on whether he's OK with the initcall changes.
> 
> I can queue this with the rest of the AVS changes with Tony's ack.

I'd rather not make anything earlier, relying on the Makefile is just
fine here. These pieces are always compiled in too. The reason why
we should only minimal things initialized earlier than module_init
is that this way we have a proper console initialized and see real
error messages without having to have DEBUG_LL + earlyprintk enabled.

If anything else is needed, you have have just one late_initcall
that checks the return values of the various SR related init functions
to make sure all the dependencies are met.

Regards,

Tony
 
> >  arch/arm/mach-omap2/smartreflex-class3.c |    2 +-
> >  drivers/power/avs/smartreflex.c          |    9 ---------
> >  2 files changed, 1 insertion(+), 10 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/smartreflex-class3.c b/arch/arm/mach-omap2/smartreflex-class3.c
> > index aee3c89..50523b8 100644
> > --- a/arch/arm/mach-omap2/smartreflex-class3.c
> > +++ b/arch/arm/mach-omap2/smartreflex-class3.c
> > @@ -59,4 +59,4 @@ static int __init sr_class3_init(void)
> >  	pr_info("SmartReflex Class3 initialized\n");
> >  	return sr_register_class(&class3_data);
> >  }
> > -omap_late_initcall(sr_class3_init);
> > +omap_device_initcall(sr_class3_init);
> > diff --git a/drivers/power/avs/smartreflex.c b/drivers/power/avs/smartreflex.c
> > index fd71d5a..42eed34 100644
> > --- a/drivers/power/avs/smartreflex.c
> > +++ b/drivers/power/avs/smartreflex.c
> > @@ -650,8 +650,6 @@ void sr_disable(struct voltagedomain *voltdm)
> >   */
> >  int sr_register_class(struct omap_sr_class_data *class_data)
> >  {
> > -	struct omap_sr *sr_info;
> > -
> >  	if (!class_data) {
> >  		pr_warning("%s:, Smartreflex class data passed is NULL\n",
> >  			__func__);
> > @@ -666,13 +664,6 @@ int sr_register_class(struct omap_sr_class_data *class_data)
> >  
> >  	sr_class = class_data;
> >  
> > -	/*
> > -	 * Call into late init to do intializations that require
> > -	 * both sr driver and sr class driver to be initiallized.
> > -	 */
> > -	list_for_each_entry(sr_info, &sr_list, node)
> > -		sr_late_init(sr_info);
> > -
> >  	return 0;
> >  }

Re: [PATCH v2 2/2] PM / AVS: SmartReflex/class3: Fix order of initialization of SR class and SR driver

[Andrii Tseglytskyi]

On 06/12/2013 04:32 PM, Tony Lindgren wrote:
> * Kevin Hilman <khilman@linaro.org> [130610 10:58]:
>> Andrii Tseglytskyi <andrii.tseglytskyi@ti.com> writes:
>>
>>> SmartReflex consists of three entities: SR device, SR class and
>>> SR driver. SmartReflex driver depends on SmartReflex class, but
>>> order of their initialization is not clear. They both use
>>> late_initcall(), and order depends on Makefile calls.
>>> Patch moves initialization of SR class to device_initcall(),
>>> and removes redundant call of sr_late_init().
>>>
>>> This provides predictable order of SmartReflex initcalls:
>>> 1. device_initcall() -> SmartReflex class init
>>> 2. late_initcall() -> SmartReflex driver init
>>>
>>> Signed-off-by: Andrii Tseglytskyi <andrii.tseglytskyi@ti.com>
>> Tony will have to decide on whether he's OK with the initcall changes.
>>
>> I can queue this with the rest of the AVS changes with Tony's ack.
> I'd rather not make anything earlier, relying on the Makefile is just
> fine here. These pieces are always compiled in too. The reason why
> we should only minimal things initialized earlier than module_init
> is that this way we have a proper console initialized and see real
> error messages without having to have DEBUG_LL + earlyprintk enabled.
>
> If anything else is needed, you have have just one late_initcall
> that checks the return values of the various SR related init functions
> to make sure all the dependencies are met.
>
> Regards,
>
> Tony
>   
>
Hi Tony,

Thank you for your comment - it sounds reasonable.
Patch can be dropped from this patch series.

Regards,
Andrii

Re: [PATCH v2 2/2] PM / AVS: SmartReflex/class3: Fix order of initialization of SR class and SR driver

[Andrii Tseglytskyi]

On 06/12/2013 04:32 PM, Tony Lindgren wrote:
> * Kevin Hilman <khilman@linaro.org> [130610 10:58]:
>> Andrii Tseglytskyi <andrii.tseglytskyi@ti.com> writes:
>>
>>> SmartReflex consists of three entities: SR device, SR class and
>>> SR driver. SmartReflex driver depends on SmartReflex class, but
>>> order of their initialization is not clear. They both use
>>> late_initcall(), and order depends on Makefile calls.
>>> Patch moves initialization of SR class to device_initcall(),
>>> and removes redundant call of sr_late_init().
>>>
>>> This provides predictable order of SmartReflex initcalls:
>>> 1. device_initcall() -> SmartReflex class init
>>> 2. late_initcall() -> SmartReflex driver init
>>>
>>> Signed-off-by: Andrii Tseglytskyi <andrii.tseglytskyi@ti.com>
>> Tony will have to decide on whether he's OK with the initcall changes.
>>
>> I can queue this with the rest of the AVS changes with Tony's ack.
> I'd rather not make anything earlier, relying on the Makefile is just
> fine here. These pieces are always compiled in too. The reason why
> we should only minimal things initialized earlier than module_init
> is that this way we have a proper console initialized and see real
> error messages without having to have DEBUG_LL + earlyprintk enabled.
>
> If anything else is needed, you have have just one late_initcall
> that checks the return values of the various SR related init functions
> to make sure all the dependencies are met.
>
> Regards,
>
> Tony
>   
>
Hi Tony,

Thank you for your comment - it sounds reasonable.
Patch can be dropped from this patch series.

Regards,
Andrii