[RFC PATCH 0/3] OMAP: omap_device: Add a method to build an omap_device from a DT node

[RFC PATCH 0/3] OMAP: omap_device: Add a method to build an omap_device from a DT node

[Benoit Cousson]

Hi Kevin,

This is an initial attempt to use a notifier in order to create an
omap_device from a platform_device bound to DT node as suggested by Grant.
For the moment, the informations are all extracted from the hwmod data.
The idea is to focus first on the devices / board static init removal.
The other issue is that some bindings, like dma, are still not present
in the DT core code. The reg and irq bindings are there, but cannot
be used by some drivers due to the lack of named ressources in DT.
Since the DT core cannot change, the drivers will have to be modified
first.
In a second phase these informations will be moved from hwmod to DT.

Patches are based on my for_3.2/1_omap_device_cleanup branch and are
available here:
git://gitorious.org/omap-pm/linux.git for_3.2/2_omap_device_dt

It is tested on OMAP4 SDP and Panda.

Regards,
Benoit


Benoit Cousson (3):
  OMAP: omap_device: Add omap_device_[alloc|delete] for DT integration
  OMAP: omap_device: Add a DT parser for multiple strings
  OMAP: omap_device: Add a method to build an omap_device from a DT node

 arch/arm/plat-omap/omap_device.c |  309 +++++++++++++++++++++++++++++++-------
 1 files changed, 255 insertions(+), 54 deletions(-)

[RFC PATCH 1/3] OMAP: omap_device: Add omap_device_[alloc|delete] for DT integration

[Benoit Cousson]

Split the omap_device_build_ss into two smaller functions
that will allow to populate a platform_device already alocated by
device-tree.
The functionality of the omap_device_build_ss is still the same, but
the omap_device_alloc will be usable with devices already built by
device-tree.

Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Kevin Hilman <khilman@ti.com>
---
 arch/arm/plat-omap/omap_device.c |  168 ++++++++++++++++++++++++++------------
 1 files changed, 114 insertions(+), 54 deletions(-)

diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index f2149be..752d72a 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -96,6 +96,11 @@
 
 static int omap_device_register(struct platform_device *pdev);
 static int omap_early_device_register(struct platform_device *pdev);
+static struct omap_device *omap_device_alloc(struct platform_device *pdev,
+				      struct omap_hwmod **ohs, int oh_cnt,
+				      struct omap_device_pm_latency *pm_lats,
+				      int pm_lats_cnt);
+
 
 static struct omap_device_pm_latency omap_default_latency[] = {
 	{
@@ -397,6 +402,105 @@ static int omap_device_fill_resources(struct omap_device *od,
 }
 
 /**
+ * omap_device_alloc - allocate an omap_device
+ * @pdev: platform_device that will be represent this omap_device
+ * @oh: ptr to the single omap_hwmod that backs this omap_device
+ * @pdata: platform_data ptr to associate with the platform_device
+ * @pdata_len: amount of memory pointed to by @pdata
+ * @pm_lats: pointer to a omap_device_pm_latency array for this device
+ * @pm_lats_cnt: ARRAY_SIZE() of @pm_lats
+ *
+ * Convenience function for allocating an omap_device structure and filling
+ * hwmods, resources and pm_latency attributes.
+ *
+ * Returns an struct omap_device pointer or ERR_PTR() on error;
+ */
+static struct omap_device *omap_device_alloc(struct platform_device *pdev,
+					struct omap_hwmod **ohs, int oh_cnt,
+					struct omap_device_pm_latency *pm_lats,
+					int pm_lats_cnt)
+{
+	int ret = -ENOMEM;
+	struct omap_device *od;
+	struct resource *res = NULL;
+	int i, res_count;
+	struct omap_hwmod **hwmods;
+
+	od = kzalloc(sizeof(struct omap_device), GFP_KERNEL);
+	if (!od) {
+		ret = -ENOMEM;
+		goto oda_exit1;
+	}
+	od->hwmods_cnt = oh_cnt;
+
+	hwmods = kzalloc(sizeof(struct omap_hwmod *) * oh_cnt,
+			 GFP_KERNEL);
+	if (!hwmods)
+		goto oda_exit2;
+
+	memcpy(hwmods, ohs, sizeof(struct omap_hwmod *) * oh_cnt);
+	od->hwmods = hwmods;
+	od->pdev = pdev;
+
+	if (pdev->num_resources && pdev->resource)
+		dev_warn(&pdev->dev, "%s(): ressources already allocated %d\n",
+			__func__, pdev->num_resources);
+	/*
+	 * HACK: Idealy the resources from DT should match, and hwmod
+	 * should just add the missing ones. Since the name is not
+	 * properly populated by DT, stick to hwmod resources only.
+	 */
+	res_count = omap_device_count_resources(od);
+	if (res_count > 0) {
+		dev_dbg(&pdev->dev, "%s(): ressources allocated from hwmod %d\n",
+			__func__, res_count);
+		res = kzalloc(sizeof(struct resource) * res_count, GFP_KERNEL);
+		if (!res)
+			goto oda_exit3;
+
+		omap_device_fill_resources(od, res);
+
+		ret = platform_device_add_resources(pdev, res, res_count);
+		kfree(res);
+
+		if (ret)
+			goto oda_exit3;
+	}
+
+	if (pm_lats) {
+		od->pm_lats = pm_lats;
+		od->pm_lats_cnt = pm_lats_cnt;
+	} else {
+		od->pm_lats = omap_default_latency;
+		od->pm_lats_cnt = ARRAY_SIZE(omap_default_latency);
+	}
+
+	pdev->archdata.od = od;
+
+	for (i = 0; i < oh_cnt; i++) {
+		hwmods[i]->od = od;
+		_add_hwmod_clocks_clkdev(od, hwmods[i]);
+	}
+
+	return od;
+
+oda_exit3:
+	kfree(hwmods);
+oda_exit2:
+	kfree(od);
+oda_exit1:
+	dev_err(&pdev->dev, "omap_device: build failed (%d)\n", ret);
+
+	return ERR_PTR(ret);
+}
+
+void omap_device_delete(struct omap_device *od)
+{
+	kfree(od->hwmods);
+	kfree(od);
+}
+
+/**
  * omap_device_build - build and register an omap_device with one omap_hwmod
  * @pdev_name: name of the platform_device driver to use
  * @pdev_id: this platform_device's connection ID
@@ -455,9 +559,6 @@ struct platform_device *omap_device_build_ss(const char *pdev_name, int pdev_id,
 	int ret = -ENOMEM;
 	struct platform_device *pdev;
 	struct omap_device *od;
-	struct resource *res = NULL;
-	int i, res_count;
-	struct omap_hwmod **hwmods;
 
 	if (!ohs || oh_cnt == 0 || !pdev_name)
 		return ERR_PTR(-EINVAL);
@@ -471,72 +572,31 @@ struct platform_device *omap_device_build_ss(const char *pdev_name, int pdev_id,
 		goto odbs_exit;
 	}
 
-	pr_debug("omap_device: %s: building with %d hwmods\n", pdev_name,
-		 oh_cnt);
+	/* Set the dev_name early to allow dev_xxx in omap_device_alloc */
+	if (pdev->id != -1)
+		dev_set_name(&pdev->dev, "%s.%d", pdev->name,  pdev->id);
+	else
+		dev_set_name(&pdev->dev, "%s", pdev->name);
 
-	od = kzalloc(sizeof(struct omap_device), GFP_KERNEL);
-	if (!od) {
-		ret = -ENOMEM;
+	od = omap_device_alloc(pdev, ohs, oh_cnt, pm_lats, pm_lats_cnt);
+	if (!od)
 		goto odbs_exit1;
-	}
-	od->hwmods_cnt = oh_cnt;
-
-	hwmods = kzalloc(sizeof(struct omap_hwmod *) * oh_cnt,
-			 GFP_KERNEL);
-	if (!hwmods)
-		goto odbs_exit2;
-
-	memcpy(hwmods, ohs, sizeof(struct omap_hwmod *) * oh_cnt);
-	od->hwmods = hwmods;
-	od->pdev = pdev;
-
-	res_count = omap_device_count_resources(od);
-	if (res_count > 0) {
-		res = kzalloc(sizeof(struct resource) * res_count, GFP_KERNEL);
-		if (!res)
-			goto odbs_exit3;
-
-		omap_device_fill_resources(od, res);
-
-		ret = platform_device_add_resources(pdev, res, res_count);
-		kfree(res);
-
-		if (ret)
-			goto odbs_exit3;
-	}
 
 	ret = platform_device_add_data(pdev, pdata, pdata_len);
 	if (ret)
-		goto odbs_exit3;
-
-	pdev->archdata.od = od;
+		goto odbs_exit2;
 
 	if (is_early_device)
 		ret = omap_early_device_register(pdev);
 	else
 		ret = omap_device_register(pdev);
 	if (ret)
-		goto odbs_exit3;
-
-	if (pm_lats) {
-		od->pm_lats = pm_lats;
-		od->pm_lats_cnt = pm_lats_cnt;
-	} else {
-		od->pm_lats = omap_default_latency;
-		od->pm_lats_cnt = ARRAY_SIZE(omap_default_latency);
-	}
-
-	for (i = 0; i < oh_cnt; i++) {
-		hwmods[i]->od = od;
-		_add_hwmod_clocks_clkdev(od, hwmods[i]);
-	}
+		goto odbs_exit2;
 
 	return pdev;
 
-odbs_exit3:
-	kfree(hwmods);
 odbs_exit2:
-	kfree(od);
+	omap_device_delete(od);
 odbs_exit1:
 	platform_device_put(pdev);
 odbs_exit:
-- 
1.7.0.4

[RFC PATCH 1/3] OMAP: omap_device: Add omap_device_[alloc|delete] for DT integration

[Kevin Hilman]

Benoit Cousson <b-cousson@ti.com> writes:

> Split the omap_device_build_ss into two smaller functions
> that will allow to populate a platform_device already alocated by
> device-tree.
> The functionality of the omap_device_build_ss is still the same, but
> the omap_device_alloc will be usable with devices already built by
> device-tree.

Very nice, I started down this path in the original series, but didn't
finish.

Some minor comments below...

> Signed-off-by: Benoit Cousson <b-cousson@ti.com>
> Cc: Kevin Hilman <khilman@ti.com>
> ---
>  arch/arm/plat-omap/omap_device.c |  168 ++++++++++++++++++++++++++------------
>  1 files changed, 114 insertions(+), 54 deletions(-)
>
> diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
> index f2149be..752d72a 100644
> --- a/arch/arm/plat-omap/omap_device.c
> +++ b/arch/arm/plat-omap/omap_device.c
> @@ -96,6 +96,11 @@
>  
>  static int omap_device_register(struct platform_device *pdev);
>  static int omap_early_device_register(struct platform_device *pdev);
> +static struct omap_device *omap_device_alloc(struct platform_device *pdev,
> +				      struct omap_hwmod **ohs, int oh_cnt,
> +				      struct omap_device_pm_latency *pm_lats,
> +				      int pm_lats_cnt);
> +
>  
>  static struct omap_device_pm_latency omap_default_latency[] = {
>  	{
> @@ -397,6 +402,105 @@ static int omap_device_fill_resources(struct omap_device *od,
>  }
>  
>  /**
> + * omap_device_alloc - allocate an omap_device
> + * @pdev: platform_device that will be represent this omap_device
> + * @oh: ptr to the single omap_hwmod that backs this omap_device
> + * @pdata: platform_data ptr to associate with the platform_device
> + * @pdata_len: amount of memory pointed to by @pdata
> + * @pm_lats: pointer to a omap_device_pm_latency array for this device
> + * @pm_lats_cnt: ARRAY_SIZE() of @pm_lats
> + *
> + * Convenience function for allocating an omap_device structure and filling
> + * hwmods, resources and pm_latency attributes.
> + *
> + * Returns an struct omap_device pointer or ERR_PTR() on error;
> + */
> +static struct omap_device *omap_device_alloc(struct platform_device *pdev,
> +					struct omap_hwmod **ohs, int oh_cnt,
> +					struct omap_device_pm_latency *pm_lats,
> +					int pm_lats_cnt)
> +{
> +	int ret = -ENOMEM;
> +	struct omap_device *od;
> +	struct resource *res = NULL;
> +	int i, res_count;
> +	struct omap_hwmod **hwmods;
> +
> +	od = kzalloc(sizeof(struct omap_device), GFP_KERNEL);
> +	if (!od) {
> +		ret = -ENOMEM;
> +		goto oda_exit1;
> +	}
> +	od->hwmods_cnt = oh_cnt;
> +
> +	hwmods = kzalloc(sizeof(struct omap_hwmod *) * oh_cnt,
> +			 GFP_KERNEL);
> +	if (!hwmods)
> +		goto oda_exit2;
> +
> +	memcpy(hwmods, ohs, sizeof(struct omap_hwmod *) * oh_cnt);
> +	od->hwmods = hwmods;
> +	od->pdev = pdev;
> +
> +	if (pdev->num_resources && pdev->resource)
> +		dev_warn(&pdev->dev, "%s(): ressources already allocated %d\n",

typo: ressources (occurs again below)

also, this check/warn should probably go after the 'HACK' comment below.

> +			__func__, pdev->num_resources);
> +	/*
> +	 * HACK: Idealy the resources from DT should match, and hwmod

s/Idealy/Ideally/

> +	 * should just add the missing ones. Since the name is not
> +	 * properly populated by DT, stick to hwmod resources only.

I think this limitation is fine until we come to resolution in the named
resources debate.

> +	 */
> +	res_count = omap_device_count_resources(od);
> +	if (res_count > 0) {
> +		dev_dbg(&pdev->dev, "%s(): ressources allocated from hwmod %d\n",
> +			__func__, res_count);
> +		res = kzalloc(sizeof(struct resource) * res_count, GFP_KERNEL);
> +		if (!res)
> +			goto oda_exit3;
> +
> +		omap_device_fill_resources(od, res);
> +
> +		ret = platform_device_add_resources(pdev, res, res_count);
> +		kfree(res);
> +
> +		if (ret)
> +			goto oda_exit3;
> +	}
> +
> +	if (pm_lats) {
> +		od->pm_lats = pm_lats;
> +		od->pm_lats_cnt = pm_lats_cnt;
> +	} else {
> +		od->pm_lats = omap_default_latency;
> +		od->pm_lats_cnt = ARRAY_SIZE(omap_default_latency);
> +	}
> +
> +	pdev->archdata.od = od;
> +
> +	for (i = 0; i < oh_cnt; i++) {
> +		hwmods[i]->od = od;
> +		_add_hwmod_clocks_clkdev(od, hwmods[i]);
> +	}
> +
> +	return od;
> +
> +oda_exit3:
> +	kfree(hwmods);
> +oda_exit2:
> +	kfree(od);
> +oda_exit1:
> +	dev_err(&pdev->dev, "omap_device: build failed (%d)\n", ret);
> +
> +	return ERR_PTR(ret);
> +}
> +
> +void omap_device_delete(struct omap_device *od)
> +{
> +	kfree(od->hwmods);
> +	kfree(od);

maybe set pdev->archdata.od = NULL too.

> +}
> +
> +/**
>   * omap_device_build - build and register an omap_device with one omap_hwmod
>   * @pdev_name: name of the platform_device driver to use
>   * @pdev_id: this platform_device's connection ID
> @@ -455,9 +559,6 @@ struct platform_device *omap_device_build_ss(const char *pdev_name, int pdev_id,
>  	int ret = -ENOMEM;
>  	struct platform_device *pdev;
>  	struct omap_device *od;
> -	struct resource *res = NULL;
> -	int i, res_count;
> -	struct omap_hwmod **hwmods;
>  
>  	if (!ohs || oh_cnt == 0 || !pdev_name)
>  		return ERR_PTR(-EINVAL);
> @@ -471,72 +572,31 @@ struct platform_device *omap_device_build_ss(const char *pdev_name, int pdev_id,
>  		goto odbs_exit;
>  	}
>  
> -	pr_debug("omap_device: %s: building with %d hwmods\n", pdev_name,
> -		 oh_cnt);
> +	/* Set the dev_name early to allow dev_xxx in omap_device_alloc */
> +	if (pdev->id != -1)
> +		dev_set_name(&pdev->dev, "%s.%d", pdev->name,  pdev->id);
> +	else
> +		dev_set_name(&pdev->dev, "%s", pdev->name);

Minor: I think setting dev->init_name is more appropriate here, and
should have the same effect.

>  
> -	od = kzalloc(sizeof(struct omap_device), GFP_KERNEL);
> -	if (!od) {
> -		ret = -ENOMEM;
> +	od = omap_device_alloc(pdev, ohs, oh_cnt, pm_lats, pm_lats_cnt);
> +	if (!od)
>  		goto odbs_exit1;
> -	}
> -	od->hwmods_cnt = oh_cnt;
> -
> -	hwmods = kzalloc(sizeof(struct omap_hwmod *) * oh_cnt,
> -			 GFP_KERNEL);
> -	if (!hwmods)
> -		goto odbs_exit2;
> -
> -	memcpy(hwmods, ohs, sizeof(struct omap_hwmod *) * oh_cnt);
> -	od->hwmods = hwmods;
> -	od->pdev = pdev;
> -
> -	res_count = omap_device_count_resources(od);
> -	if (res_count > 0) {
> -		res = kzalloc(sizeof(struct resource) * res_count, GFP_KERNEL);
> -		if (!res)
> -			goto odbs_exit3;
> -
> -		omap_device_fill_resources(od, res);
> -
> -		ret = platform_device_add_resources(pdev, res, res_count);
> -		kfree(res);
> -
> -		if (ret)
> -			goto odbs_exit3;
> -	}
>  
>  	ret = platform_device_add_data(pdev, pdata, pdata_len);
>  	if (ret)
> -		goto odbs_exit3;
> -
> -	pdev->archdata.od = od;
> +		goto odbs_exit2;
>  
>  	if (is_early_device)
>  		ret = omap_early_device_register(pdev);
>  	else
>  		ret = omap_device_register(pdev);
>  	if (ret)
> -		goto odbs_exit3;
> -
> -	if (pm_lats) {
> -		od->pm_lats = pm_lats;
> -		od->pm_lats_cnt = pm_lats_cnt;
> -	} else {
> -		od->pm_lats = omap_default_latency;
> -		od->pm_lats_cnt = ARRAY_SIZE(omap_default_latency);
> -	}
> -
> -	for (i = 0; i < oh_cnt; i++) {
> -		hwmods[i]->od = od;
> -		_add_hwmod_clocks_clkdev(od, hwmods[i]);
> -	}
> +		goto odbs_exit2;
>  
>  	return pdev;
>  
> -odbs_exit3:
> -	kfree(hwmods);
>  odbs_exit2:
> -	kfree(od);
> +	omap_device_delete(od);
>  odbs_exit1:
>  	platform_device_put(pdev);
>  odbs_exit:


Kevin

[RFC PATCH 1/3] OMAP: omap_device: Add omap_device_[alloc|delete] for DT integration

[Cousson, Benoit]

On 9/1/2011 8:30 PM, Hilman, Kevin wrote:
> Benoit Cousson<b-cousson@ti.com>  writes:
> 
>> Split the omap_device_build_ss into two smaller functions
>> that will allow to populate a platform_device already alocated by
>> device-tree.
>> The functionality of the omap_device_build_ss is still the same, but
>> the omap_device_alloc will be usable with devices already built by
>> device-tree.
> 
> Very nice, I started down this path in the original series, but didn't
> finish.
> Some minor comments below...
> 
>> Signed-off-by: Benoit Cousson<b-cousson@ti.com>
>> Cc: Kevin Hilman<khilman@ti.com>
>> ---
>>   arch/arm/plat-omap/omap_device.c |  168 ++++++++++++++++++++++++++------------
>>   1 files changed, 114 insertions(+), 54 deletions(-)
>>
>> diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
>> index f2149be..752d72a 100644
>> --- a/arch/arm/plat-omap/omap_device.c
>> +++ b/arch/arm/plat-omap/omap_device.c
>> @@ -96,6 +96,11 @@
>>
>>   static int omap_device_register(struct platform_device *pdev);
>>   static int omap_early_device_register(struct platform_device *pdev);
>> +static struct omap_device *omap_device_alloc(struct platform_device *pdev,
>> +				      struct omap_hwmod **ohs, int oh_cnt,
>> +				      struct omap_device_pm_latency *pm_lats,
>> +				      int pm_lats_cnt);
>> +
>>
>>   static struct omap_device_pm_latency omap_default_latency[] = {
>>   	{
>> @@ -397,6 +402,105 @@ static int omap_device_fill_resources(struct omap_device *od,
>>   }
>>
>>   /**
>> + * omap_device_alloc - allocate an omap_device
>> + * @pdev: platform_device that will be represent this omap_device
>> + * @oh: ptr to the single omap_hwmod that backs this omap_device
>> + * @pdata: platform_data ptr to associate with the platform_device
>> + * @pdata_len: amount of memory pointed to by @pdata
>> + * @pm_lats: pointer to a omap_device_pm_latency array for this device
>> + * @pm_lats_cnt: ARRAY_SIZE() of @pm_lats
>> + *
>> + * Convenience function for allocating an omap_device structure and filling
>> + * hwmods, resources and pm_latency attributes.
>> + *
>> + * Returns an struct omap_device pointer or ERR_PTR() on error;
>> + */
>> +static struct omap_device *omap_device_alloc(struct platform_device *pdev,
>> +					struct omap_hwmod **ohs, int oh_cnt,
>> +					struct omap_device_pm_latency *pm_lats,
>> +					int pm_lats_cnt)
>> +{
>> +	int ret = -ENOMEM;
>> +	struct omap_device *od;
>> +	struct resource *res = NULL;
>> +	int i, res_count;
>> +	struct omap_hwmod **hwmods;
>> +
>> +	od = kzalloc(sizeof(struct omap_device), GFP_KERNEL);
>> +	if (!od) {
>> +		ret = -ENOMEM;
>> +		goto oda_exit1;
>> +	}
>> +	od->hwmods_cnt = oh_cnt;
>> +
>> +	hwmods = kzalloc(sizeof(struct omap_hwmod *) * oh_cnt,
>> +			 GFP_KERNEL);
>> +	if (!hwmods)
>> +		goto oda_exit2;
>> +
>> +	memcpy(hwmods, ohs, sizeof(struct omap_hwmod *) * oh_cnt);
>> +	od->hwmods = hwmods;
>> +	od->pdev = pdev;
>> +
>> +	if (pdev->num_resources&&  pdev->resource)
>> +		dev_warn(&pdev->dev, "%s(): ressources already allocated %d\n",
> 
> typo: ressources (occurs again below)

It is not a real typo, it is a French word :-)
 
> also, this check/warn should probably go after the 'HACK' comment below.

yep.

> 
>> +			__func__, pdev->num_resources);
>> +	/*
>> +	 * HACK: Idealy the resources from DT should match, and hwmod
> 
> s/Idealy/Ideally/

OK, for that one, I do not have the French word excuse...

> 
>> +	 * should just add the missing ones. Since the name is not
>> +	 * properly populated by DT, stick to hwmod resources only.
> 
> I think this limitation is fine until we come to resolution in the named
> resources debate.

Indeed :-)

>> +	 */
>> +	res_count = omap_device_count_resources(od);
>> +	if (res_count>  0) {
>> +		dev_dbg(&pdev->dev, "%s(): ressources allocated from hwmod %d\n",
>> +			__func__, res_count);
>> +		res = kzalloc(sizeof(struct resource) * res_count, GFP_KERNEL);
>> +		if (!res)
>> +			goto oda_exit3;
>> +
>> +		omap_device_fill_resources(od, res);
>> +
>> +		ret = platform_device_add_resources(pdev, res, res_count);
>> +		kfree(res);
>> +
>> +		if (ret)
>> +			goto oda_exit3;
>> +	}
>> +
>> +	if (pm_lats) {
>> +		od->pm_lats = pm_lats;
>> +		od->pm_lats_cnt = pm_lats_cnt;
>> +	} else {
>> +		od->pm_lats = omap_default_latency;
>> +		od->pm_lats_cnt = ARRAY_SIZE(omap_default_latency);
>> +	}
>> +
>> +	pdev->archdata.od = od;
>> +
>> +	for (i = 0; i<  oh_cnt; i++) {
>> +		hwmods[i]->od = od;
>> +		_add_hwmod_clocks_clkdev(od, hwmods[i]);
>> +	}
>> +
>> +	return od;
>> +
>> +oda_exit3:
>> +	kfree(hwmods);
>> +oda_exit2:
>> +	kfree(od);
>> +oda_exit1:
>> +	dev_err(&pdev->dev, "omap_device: build failed (%d)\n", ret);
>> +
>> +	return ERR_PTR(ret);
>> +}
>> +
>> +void omap_device_delete(struct omap_device *od)
>> +{
>> +	kfree(od->hwmods);
>> +	kfree(od);
> 
> maybe set pdev->archdata.od = NULL too.

OK.

>> +}
>> +
>> +/**
>>    * omap_device_build - build and register an omap_device with one omap_hwmod
>>    * @pdev_name: name of the platform_device driver to use
>>    * @pdev_id: this platform_device's connection ID
>> @@ -455,9 +559,6 @@ struct platform_device *omap_device_build_ss(const char *pdev_name, int pdev_id,
>>   	int ret = -ENOMEM;
>>   	struct platform_device *pdev;
>>   	struct omap_device *od;
>> -	struct resource *res = NULL;
>> -	int i, res_count;
>> -	struct omap_hwmod **hwmods;
>>
>>   	if (!ohs || oh_cnt == 0 || !pdev_name)
>>   		return ERR_PTR(-EINVAL);
>> @@ -471,72 +572,31 @@ struct platform_device *omap_device_build_ss(const char *pdev_name, int pdev_id,
>>   		goto odbs_exit;
>>   	}
>>
>> -	pr_debug("omap_device: %s: building with %d hwmods\n", pdev_name,
>> -		 oh_cnt);
>> +	/* Set the dev_name early to allow dev_xxx in omap_device_alloc */
>> +	if (pdev->id != -1)
>> +		dev_set_name(&pdev->dev, "%s.%d", pdev->name,  pdev->id);
>> +	else
>> +		dev_set_name(&pdev->dev, "%s", pdev->name);
> 
> Minor: I think setting dev->init_name is more appropriate here, and
> should have the same effect.

The slight difference is that if I do that:
pdev->dev.init_name = kasprintf(GFP_KERNEL, "%s.%d", pdev->name, pdev->id);

I will have to free it myself, because device_add is doing only that:

        if (dev->init_name) {
                dev_set_name(dev, "%s", dev->init_name);
                dev->init_name = NULL;
        }

Whereas dev_set_name is doing it for me.

So it will add one more line later. Does it worth it?

The init_name seems to be used only to provide a static const name most of the time, hence the lack of kfree.


Thanks for the comments,
Benoit

[RFC PATCH 1/3] OMAP: omap_device: Add omap_device_[alloc|delete] for DT integration

[Kevin Hilman]

"Cousson, Benoit" <b-cousson@ti.com> writes:

[...]

>>> @@ -471,72 +572,31 @@ struct platform_device *omap_device_build_ss(const char *pdev_name, int pdev_id,
>>>   		goto odbs_exit;
>>>   	}
>>>
>>> -	pr_debug("omap_device: %s: building with %d hwmods\n", pdev_name,
>>> -		 oh_cnt);
>>> +	/* Set the dev_name early to allow dev_xxx in omap_device_alloc */
>>> +	if (pdev->id != -1)
>>> +		dev_set_name(&pdev->dev, "%s.%d", pdev->name,  pdev->id);
>>> +	else
>>> +		dev_set_name(&pdev->dev, "%s", pdev->name);
>> 
>> Minor: I think setting dev->init_name is more appropriate here, and
>> should have the same effect.
>
> The slight difference is that if I do that:
> pdev->dev.init_name = kasprintf(GFP_KERNEL, "%s.%d", pdev->name, pdev->id);
>
> I will have to free it myself, because device_add is doing only that:
>
>         if (dev->init_name) {
>                 dev_set_name(dev, "%s", dev->init_name);
>                 dev->init_name = NULL;
>         }
>
> Whereas dev_set_name is doing it for me.
>
> So it will add one more line later. Does it worth it?

OK, keeping dev_set_name() is fine with me then.

Kevin

[RFC PATCH 2/3] OMAP: omap_device: Add a DT parser for multiple strings

[Benoit Cousson]

Add two helpers function to parse a property that contains multiple
strings.

These functions might be exported and moved to a common place if they
can to be useful elsewhere.

Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Kevin Hilman <khilman@ti.com>
---
 arch/arm/plat-omap/omap_device.c |   39 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index 752d72a..70361f8 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -315,6 +315,45 @@ static void _add_hwmod_clocks_clkdev(struct omap_device *od,
 		_add_clkdev(od, oh->opt_clks[i].role, oh->opt_clks[i].clk);
 }
 
+/*
+ * XXX: DT helper functions that should probably move elsewhere if
+ * they become usefull for other needs.
+ */
+static int _dt_count_property_string(const char *prop, int len)
+{
+	int i = 0;
+	size_t l = 0, total = 0;
+
+	if (!prop || !len)
+		return -EINVAL;
+
+	for (i = 0; len >= total; total += l, prop += l) {
+		l = strlen(prop) + 1;
+		if (*prop != 0)
+			i++;
+	}
+	return i;
+}
+
+static int _dt_get_property(const char *prop, int len, int index, char *output,
+			    int size)
+{
+	int i = 0;
+	size_t l = 0, total = 0;
+
+	if (!prop || !len)
+		return -EINVAL;
+
+	for (i = 0; len >= total; total += l, prop += l) {
+		l = strlcpy(output, prop, size) + 1;
+		if (*prop != 0) {
+			if (i++ == index)
+				return 0;
+		}
+	}
+	return -ENODEV;
+}
+
 
 /* Public functions for use by core code */
 
-- 
1.7.0.4

[RFC PATCH 2/3] OMAP: omap_device: Add a DT parser for multiple strings

[Kevin Hilman]

Benoit Cousson <b-cousson@ti.com> writes:

> Add two helpers function to parse a property that contains multiple
> strings.
>
> These functions might be exported and moved to a common place if they
> can to be useful elsewhere.
>
> Signed-off-by: Benoit Cousson <b-cousson@ti.com>
> Cc: Kevin Hilman <khilman@ti.com>

These should just be folded into 3/3 where they are used (with a comment
changelog of course.)

Kevin

> ---
>  arch/arm/plat-omap/omap_device.c |   39 ++++++++++++++++++++++++++++++++++++++
>  1 files changed, 39 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
> index 752d72a..70361f8 100644
> --- a/arch/arm/plat-omap/omap_device.c
> +++ b/arch/arm/plat-omap/omap_device.c
> @@ -315,6 +315,45 @@ static void _add_hwmod_clocks_clkdev(struct omap_device *od,
>  		_add_clkdev(od, oh->opt_clks[i].role, oh->opt_clks[i].clk);
>  }
>  
> +/*
> + * XXX: DT helper functions that should probably move elsewhere if
> + * they become usefull for other needs.
> + */
> +static int _dt_count_property_string(const char *prop, int len)
> +{
> +	int i = 0;
> +	size_t l = 0, total = 0;
> +
> +	if (!prop || !len)
> +		return -EINVAL;
> +
> +	for (i = 0; len >= total; total += l, prop += l) {
> +		l = strlen(prop) + 1;
> +		if (*prop != 0)
> +			i++;
> +	}
> +	return i;
> +}
> +
> +static int _dt_get_property(const char *prop, int len, int index, char *output,
> +			    int size)
> +{
> +	int i = 0;
> +	size_t l = 0, total = 0;
> +
> +	if (!prop || !len)
> +		return -EINVAL;
> +
> +	for (i = 0; len >= total; total += l, prop += l) {
> +		l = strlcpy(output, prop, size) + 1;
> +		if (*prop != 0) {
> +			if (i++ == index)
> +				return 0;
> +		}
> +	}
> +	return -ENODEV;
> +}
> +
>  
>  /* Public functions for use by core code */

[RFC PATCH 2/3] OMAP: omap_device: Add a DT parser for multiple strings

[Cousson, Benoit]

On 9/1/2011 8:38 PM, Hilman, Kevin wrote:
> Benoit Cousson<b-cousson@ti.com>  writes:
>
>> Add two helpers function to parse a property that contains multiple
>> strings.
>>
>> These functions might be exported and moved to a common place if they
>> can to be useful elsewhere.
>>
>> Signed-off-by: Benoit Cousson<b-cousson@ti.com>
>> Cc: Kevin Hilman<khilman@ti.com>
>
> These should just be folded into 3/3 where they are used (with a comment
> changelog of course.)

I did that because I was hoping to generalize them or to remove them if 
something better become available in DT core. And it seems that Stephen 
Warren from nvidia introduced a generic iterator for properties:
http://www.spinics.net/lists/arm-kernel/msg138013.html

But since it is not in mainline or even in devicetree/test, I still have 
to add mine for the moment.

Regards,
Benoit


>
> Kevin
>
>> ---
>>   arch/arm/plat-omap/omap_device.c |   39 ++++++++++++++++++++++++++++++++++++++
>>   1 files changed, 39 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
>> index 752d72a..70361f8 100644
>> --- a/arch/arm/plat-omap/omap_device.c
>> +++ b/arch/arm/plat-omap/omap_device.c
>> @@ -315,6 +315,45 @@ static void _add_hwmod_clocks_clkdev(struct omap_device *od,
>>   		_add_clkdev(od, oh->opt_clks[i].role, oh->opt_clks[i].clk);
>>   }
>>
>> +/*
>> + * XXX: DT helper functions that should probably move elsewhere if
>> + * they become usefull for other needs.
>> + */
>> +static int _dt_count_property_string(const char *prop, int len)
>> +{
>> +	int i = 0;
>> +	size_t l = 0, total = 0;
>> +
>> +	if (!prop || !len)
>> +		return -EINVAL;
>> +
>> +	for (i = 0; len>= total; total += l, prop += l) {
>> +		l = strlen(prop) + 1;
>> +		if (*prop != 0)
>> +			i++;
>> +	}
>> +	return i;
>> +}
>> +
>> +static int _dt_get_property(const char *prop, int len, int index, char *output,
>> +			    int size)
>> +{
>> +	int i = 0;
>> +	size_t l = 0, total = 0;
>> +
>> +	if (!prop || !len)
>> +		return -EINVAL;
>> +
>> +	for (i = 0; len>= total; total += l, prop += l) {
>> +		l = strlcpy(output, prop, size) + 1;
>> +		if (*prop != 0) {
>> +			if (i++ == index)
>> +				return 0;
>> +		}
>> +	}
>> +	return -ENODEV;
>> +}
>> +
>>
>>   /* Public functions for use by core code */

[RFC PATCH 3/3] OMAP: omap_device: Add a method to build an omap_device from a DT node

[Benoit Cousson]

Add a notifier called during device_add phase. If a of_node is present,
retrieve the hwmod entry in order to populate propely the omap_device
structure.
For the moment the resource from the device-tree are overloaded.
DT does not support named resource yet, and thus, most driver
will not work without that information.

Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Kevin Hilman <khilman@ti.com>
---
 arch/arm/plat-omap/omap_device.c |  102 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 102 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index 70361f8..0ae9e7f 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -85,6 +85,8 @@
 #include <linux/clk.h>
 #include <linux/clkdev.h>
 #include <linux/pm_runtime.h>
+#include <linux/of_platform.h>
+#include <linux/notifier.h>
 
 #include <plat/omap_device.h>
 #include <plat/omap_hwmod.h>
@@ -94,6 +96,8 @@
 #define USE_WAKEUP_LAT			0
 #define IGNORE_WAKEUP_LAT		1
 
+#define MAX_HWMOD_NAME_SIZE		32
+
 static int omap_device_register(struct platform_device *pdev);
 static int omap_early_device_register(struct platform_device *pdev);
 static struct omap_device *omap_device_alloc(struct platform_device *pdev,
@@ -354,6 +358,100 @@ static int _dt_get_property(const char *prop, int len, int index, char *output,
 	return -ENODEV;
 }
 
+static struct dev_pm_domain omap_device_pm_domain;
+
+/**
+ * omap_device_build_from_dt - build an omap_device with multiple hwmods
+ * @pdev_name: name of the platform_device driver to use
+ * @pdev_id: this platform_device's connection ID
+ * @oh: ptr to the single omap_hwmod that backs this omap_device
+ * @pdata: platform_data ptr to associate with the platform_device
+ * @pdata_len: amount of memory pointed to by @pdata
+ * @pm_lats: pointer to a omap_device_pm_latency array for this device
+ * @pm_lats_cnt: ARRAY_SIZE() of @pm_lats
+ * @is_early_device: should the device be registered as an early device or not
+ *
+ * Function for building an omap_device already registered from device-tree
+ *
+ * Returns 0 or PTR_ERR() on error.
+ */
+static int omap_device_build_from_dt(struct platform_device *pdev)
+{
+	struct omap_hwmod **hwmods;
+	struct omap_device *od;
+	struct omap_hwmod *oh;
+	char oh_name[MAX_HWMOD_NAME_SIZE];
+	const char *prop;
+	int oh_cnt, i, prop_len;
+	int ret = 0;
+
+	prop = of_get_property(pdev->dev.of_node, "hwmods", &prop_len);
+	oh_cnt = _dt_count_property_string(prop, prop_len);
+	if (!oh_cnt || IS_ERR_VALUE(oh_cnt)) {
+		dev_warn(&pdev->dev, "No 'hwmods' to build omap_device\n");
+		return -ENODEV;
+	}
+
+	hwmods = kzalloc(sizeof(struct omap_hwmod *) * oh_cnt, GFP_KERNEL);
+	if (!hwmods) {
+		ret = -ENOMEM;
+		goto odbfd_exit;
+	}
+
+	for (i = 0; i < oh_cnt; i++) {
+		_dt_get_property(prop, prop_len, i, oh_name,
+				 MAX_HWMOD_NAME_SIZE);
+
+		oh = omap_hwmod_lookup(oh_name);
+		if (!oh) {
+			dev_err(&pdev->dev, "Cannot lookup hwmod '%s'\n",
+				oh_name);
+			ret = -EINVAL;
+			goto odbfd_exit1;
+		}
+		hwmods[i] = oh;
+	}
+
+	od = omap_device_alloc(pdev, hwmods, oh_cnt, NULL, 0);
+	if (!od) {
+		dev_err(&pdev->dev, "Cannot allocate omap_device for :%s\n",
+			oh_name);
+		ret = PTR_ERR(od);
+		goto odbfd_exit1;
+	}
+
+	if (of_get_property(pdev->dev.of_node, "no_idle_on_suspend", NULL))
+		omap_device_disable_idle_on_suspend(pdev);
+
+	pdev->dev.pm_domain = &omap_device_pm_domain;
+
+odbfd_exit1:
+	kfree(hwmods);
+odbfd_exit:
+	return ret;
+}
+
+static int _omap_device_notifier_call(struct notifier_block *nb,
+				      unsigned long event, void *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+
+	switch (event) {
+	case BUS_NOTIFY_ADD_DEVICE:
+		if (pdev->dev.of_node) {
+			dev_dbg(&pdev->dev, "add_device with DT bindings\n");
+			omap_device_build_from_dt(pdev);
+		}
+		break;
+
+	case BUS_NOTIFY_DEL_DEVICE:
+		dev_dbg(&pdev->dev, "del_device\n");
+		break;
+	}
+
+	return NOTIFY_DONE;
+}
+
 
 /* Public functions for use by core code */
 
@@ -1064,8 +1162,12 @@ struct device omap_device_parent = {
 	.parent         = &platform_bus,
 };
 
+static struct notifier_block platform_nb;
+
 static int __init omap_device_init(void)
 {
+	platform_nb.notifier_call = _omap_device_notifier_call;
+	bus_register_notifier(&platform_bus_type, &platform_nb);
 	return device_register(&omap_device_parent);
 }
 core_initcall(omap_device_init);
-- 
1.7.0.4

[RFC PATCH 3/3] OMAP: omap_device: Add a method to build an omap_device from a DT node

[Cousson, Benoit]

Kevin,

Some self-review...

On 8/22/2011 8:22 PM, Cousson, Benoit wrote:
> Add a notifier called during device_add phase. If a of_node is present,
> retrieve the hwmod entry in order to populate propely the omap_device

typo

> structure.
> For the moment the resource from the device-tree are overloaded.
> DT does not support named resource yet, and thus, most driver
> will not work without that information.
>
> Signed-off-by: Benoit Cousson<b-cousson@ti.com>
> Cc: Kevin Hilman<khilman@ti.com>
> ---
>   arch/arm/plat-omap/omap_device.c |  102 ++++++++++++++++++++++++++++++++++++++
>   1 files changed, 102 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
> index 70361f8..0ae9e7f 100644
> --- a/arch/arm/plat-omap/omap_device.c
> +++ b/arch/arm/plat-omap/omap_device.c
> @@ -85,6 +85,8 @@
>   #include<linux/clk.h>
>   #include<linux/clkdev.h>
>   #include<linux/pm_runtime.h>
> +#include<linux/of_platform.h>

This is not the proper include and will break non-DT build.
It should be that one:

+#include<linux/of.h>

I'll fix the series and re-send.

Benoit


> +#include<linux/notifier.h>
>
>   #include<plat/omap_device.h>
>   #include<plat/omap_hwmod.h>
> @@ -94,6 +96,8 @@
>   #define USE_WAKEUP_LAT			0
>   #define IGNORE_WAKEUP_LAT		1
>
> +#define MAX_HWMOD_NAME_SIZE		32
> +
>   static int omap_device_register(struct platform_device *pdev);
>   static int omap_early_device_register(struct platform_device *pdev);
>   static struct omap_device *omap_device_alloc(struct platform_device *pdev,
> @@ -354,6 +358,100 @@ static int _dt_get_property(const char *prop, int len, int index, char *output,
>   	return -ENODEV;
>   }
>
> +static struct dev_pm_domain omap_device_pm_domain;
> +
> +/**
> + * omap_device_build_from_dt - build an omap_device with multiple hwmods
> + * @pdev_name: name of the platform_device driver to use
> + * @pdev_id: this platform_device's connection ID
> + * @oh: ptr to the single omap_hwmod that backs this omap_device
> + * @pdata: platform_data ptr to associate with the platform_device
> + * @pdata_len: amount of memory pointed to by @pdata
> + * @pm_lats: pointer to a omap_device_pm_latency array for this device
> + * @pm_lats_cnt: ARRAY_SIZE() of @pm_lats
> + * @is_early_device: should the device be registered as an early device or not
> + *
> + * Function for building an omap_device already registered from device-tree
> + *
> + * Returns 0 or PTR_ERR() on error.
> + */
> +static int omap_device_build_from_dt(struct platform_device *pdev)
> +{
> +	struct omap_hwmod **hwmods;
> +	struct omap_device *od;
> +	struct omap_hwmod *oh;
> +	char oh_name[MAX_HWMOD_NAME_SIZE];
> +	const char *prop;
> +	int oh_cnt, i, prop_len;
> +	int ret = 0;
> +
> +	prop = of_get_property(pdev->dev.of_node, "hwmods",&prop_len);
> +	oh_cnt = _dt_count_property_string(prop, prop_len);
> +	if (!oh_cnt || IS_ERR_VALUE(oh_cnt)) {
> +		dev_warn(&pdev->dev, "No 'hwmods' to build omap_device\n");
> +		return -ENODEV;
> +	}
> +
> +	hwmods = kzalloc(sizeof(struct omap_hwmod *) * oh_cnt, GFP_KERNEL);
> +	if (!hwmods) {
> +		ret = -ENOMEM;
> +		goto odbfd_exit;
> +	}
> +
> +	for (i = 0; i<  oh_cnt; i++) {
> +		_dt_get_property(prop, prop_len, i, oh_name,
> +				 MAX_HWMOD_NAME_SIZE);
> +
> +		oh = omap_hwmod_lookup(oh_name);
> +		if (!oh) {
> +			dev_err(&pdev->dev, "Cannot lookup hwmod '%s'\n",
> +				oh_name);
> +			ret = -EINVAL;
> +			goto odbfd_exit1;
> +		}
> +		hwmods[i] = oh;
> +	}
> +
> +	od = omap_device_alloc(pdev, hwmods, oh_cnt, NULL, 0);
> +	if (!od) {
> +		dev_err(&pdev->dev, "Cannot allocate omap_device for :%s\n",
> +			oh_name);
> +		ret = PTR_ERR(od);
> +		goto odbfd_exit1;
> +	}
> +
> +	if (of_get_property(pdev->dev.of_node, "no_idle_on_suspend", NULL))
> +		omap_device_disable_idle_on_suspend(pdev);
> +
> +	pdev->dev.pm_domain =&omap_device_pm_domain;
> +
> +odbfd_exit1:
> +	kfree(hwmods);
> +odbfd_exit:
> +	return ret;
> +}
> +
> +static int _omap_device_notifier_call(struct notifier_block *nb,
> +				      unsigned long event, void *dev)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +
> +	switch (event) {
> +	case BUS_NOTIFY_ADD_DEVICE:
> +		if (pdev->dev.of_node) {
> +			dev_dbg(&pdev->dev, "add_device with DT bindings\n");
> +			omap_device_build_from_dt(pdev);
> +		}
> +		break;
> +
> +	case BUS_NOTIFY_DEL_DEVICE:
> +		dev_dbg(&pdev->dev, "del_device\n");
> +		break;
> +	}
> +
> +	return NOTIFY_DONE;
> +}
> +
>
>   /* Public functions for use by core code */
>
> @@ -1064,8 +1162,12 @@ struct device omap_device_parent = {
>   	.parent         =&platform_bus,
>   };
>
> +static struct notifier_block platform_nb;
> +
>   static int __init omap_device_init(void)
>   {
> +	platform_nb.notifier_call = _omap_device_notifier_call;
> +	bus_register_notifier(&platform_bus_type,&platform_nb);
>   	return device_register(&omap_device_parent);
>   }
>   core_initcall(omap_device_init);

[RFC PATCH 3/3] OMAP: omap_device: Add a method to build an omap_device from a DT node

[Kevin Hilman]

Benoit Cousson <b-cousson@ti.com> writes:

> Add a notifier called during device_add phase. If a of_node is present,
> retrieve the hwmod entry in order to populate propely the omap_device
> structure.
> For the moment the resource from the device-tree are overloaded.
> DT does not support named resource yet, and thus, most driver
> will not work without that information.
>
> Signed-off-by: Benoit Cousson <b-cousson@ti.com>
> Cc: Kevin Hilman <khilman@ti.com>

Nice, minor comment below...

> ---
>  arch/arm/plat-omap/omap_device.c |  102 ++++++++++++++++++++++++++++++++++++++
>  1 files changed, 102 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
> index 70361f8..0ae9e7f 100644
> --- a/arch/arm/plat-omap/omap_device.c
> +++ b/arch/arm/plat-omap/omap_device.c
> @@ -85,6 +85,8 @@
>  #include <linux/clk.h>
>  #include <linux/clkdev.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/of_platform.h>
> +#include <linux/notifier.h>
>  
>  #include <plat/omap_device.h>
>  #include <plat/omap_hwmod.h>
> @@ -94,6 +96,8 @@
>  #define USE_WAKEUP_LAT			0
>  #define IGNORE_WAKEUP_LAT		1
>  
> +#define MAX_HWMOD_NAME_SIZE		32
> +
>  static int omap_device_register(struct platform_device *pdev);
>  static int omap_early_device_register(struct platform_device *pdev);
>  static struct omap_device *omap_device_alloc(struct platform_device *pdev,
> @@ -354,6 +358,100 @@ static int _dt_get_property(const char *prop, int len, int index, char *output,
>  	return -ENODEV;
>  }
>  
> +static struct dev_pm_domain omap_device_pm_domain;
> +
> +/**
> + * omap_device_build_from_dt - build an omap_device with multiple hwmods
> + * @pdev_name: name of the platform_device driver to use
> + * @pdev_id: this platform_device's connection ID
> + * @oh: ptr to the single omap_hwmod that backs this omap_device
> + * @pdata: platform_data ptr to associate with the platform_device
> + * @pdata_len: amount of memory pointed to by @pdata
> + * @pm_lats: pointer to a omap_device_pm_latency array for this device
> + * @pm_lats_cnt: ARRAY_SIZE() of @pm_lats
> + * @is_early_device: should the device be registered as an early device or not
> + *
> + * Function for building an omap_device already registered from device-tree
> + *
> + * Returns 0 or PTR_ERR() on error.
> + */
> +static int omap_device_build_from_dt(struct platform_device *pdev)
> +{
> +	struct omap_hwmod **hwmods;
> +	struct omap_device *od;
> +	struct omap_hwmod *oh;
> +	char oh_name[MAX_HWMOD_NAME_SIZE];
> +	const char *prop;
> +	int oh_cnt, i, prop_len;
> +	int ret = 0;
> +
> +	prop = of_get_property(pdev->dev.of_node, "hwmods", &prop_len);
> +	oh_cnt = _dt_count_property_string(prop, prop_len);
> +	if (!oh_cnt || IS_ERR_VALUE(oh_cnt)) {
> +		dev_warn(&pdev->dev, "No 'hwmods' to build omap_device\n");
> +		return -ENODEV;
> +	}
> +
> +	hwmods = kzalloc(sizeof(struct omap_hwmod *) * oh_cnt, GFP_KERNEL);
> +	if (!hwmods) {
> +		ret = -ENOMEM;
> +		goto odbfd_exit;
> +	}
> +
> +	for (i = 0; i < oh_cnt; i++) {
> +		_dt_get_property(prop, prop_len, i, oh_name,
> +				 MAX_HWMOD_NAME_SIZE);
> +
> +		oh = omap_hwmod_lookup(oh_name);
> +		if (!oh) {
> +			dev_err(&pdev->dev, "Cannot lookup hwmod '%s'\n",
> +				oh_name);
> +			ret = -EINVAL;
> +			goto odbfd_exit1;
> +		}
> +		hwmods[i] = oh;
> +	}
> +
> +	od = omap_device_alloc(pdev, hwmods, oh_cnt, NULL, 0);
> +	if (!od) {
> +		dev_err(&pdev->dev, "Cannot allocate omap_device for :%s\n",
> +			oh_name);
> +		ret = PTR_ERR(od);
> +		goto odbfd_exit1;
> +	}
> +
> +	if (of_get_property(pdev->dev.of_node, "no_idle_on_suspend", NULL))
> +		omap_device_disable_idle_on_suspend(pdev);
> +
> +	pdev->dev.pm_domain = &omap_device_pm_domain;
> +
> +odbfd_exit1:
> +	kfree(hwmods);
> +odbfd_exit:
> +	return ret;
> +}
> +
> +static int _omap_device_notifier_call(struct notifier_block *nb,
> +				      unsigned long event, void *dev)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +
> +	switch (event) {
> +	case BUS_NOTIFY_ADD_DEVICE:
> +		if (pdev->dev.of_node) {
> +			dev_dbg(&pdev->dev, "add_device with DT bindings\n");
> +			omap_device_build_from_dt(pdev);
> +		}
> +		break;
> +
> +	case BUS_NOTIFY_DEL_DEVICE:
> +		dev_dbg(&pdev->dev, "del_device\n");

Need a delete/cleanup here.  

Looks like all all it needs to do is call omap_device_delete()?

Kevin

> +		break;
> +	}
> +
> +	return NOTIFY_DONE;
> +}
> +
>  
>  /* Public functions for use by core code */
>  
> @@ -1064,8 +1162,12 @@ struct device omap_device_parent = {
>  	.parent         = &platform_bus,
>  };
>  
> +static struct notifier_block platform_nb;
> +
>  static int __init omap_device_init(void)
>  {
> +	platform_nb.notifier_call = _omap_device_notifier_call;
> +	bus_register_notifier(&platform_bus_type, &platform_nb);
>  	return device_register(&omap_device_parent);
>  }
>  core_initcall(omap_device_init);

[RFC PATCH 3/3] OMAP: omap_device: Add a method to build an omap_device from a DT node

[Cousson, Benoit]

On 9/2/2011 1:40 AM, Hilman, Kevin wrote:
> Benoit Cousson<b-cousson@ti.com>  writes:
>
>> Add a notifier called during device_add phase. If a of_node is present,
>> retrieve the hwmod entry in order to populate propely the omap_device
>> structure.
>> For the moment the resource from the device-tree are overloaded.
>> DT does not support named resource yet, and thus, most driver
>> will not work without that information.
>>
>> Signed-off-by: Benoit Cousson<b-cousson@ti.com>
>> Cc: Kevin Hilman<khilman@ti.com>
>
> Nice, minor comment below...

[...]

>> +	case BUS_NOTIFY_DEL_DEVICE:
>> +		dev_dbg(&pdev->dev, "del_device\n");
>
> Need a delete/cleanup here.
>
> Looks like all all it needs to do is call omap_device_delete()?

Indeed, I was too lazy to add it.

Thanks,
Benoit