OMAP: omap_device: Add a method to build an omap_device from a DT node
Subsystems (2)
cover OMAP: omap_device: Add a method to build an omap_device from a DT node
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
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.
--- arch/arm/plat-omap/omap_device.c | 168 ++++++++++++++++++++++++++------------ 1 files changed, 114 insertions(+), 54 deletions(-)
arch/arm/plat-omap/omap_device.c +114 -54
@@ -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:
Signature · 1 line
1.7.0.4
RFC PATCH 1/3 OMAP: omap_device: Add omap_device_[alloc|delete] for DT integration
Quoted 6 lines · Benoit Cousson
Split the
omap_device_build_ssinto two smaller functions that will allow to populate aplatform_devicealready alocated by device-tree.
The functionality of theomap_device_build_ssis still the same, but theomap_device_allocwill 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...
Quoted 58 lines
Signed-off-by Benoit Cousson <[email protected]>Cc Kevin Hilman <[email protected]>--- arch/arm/plat-omap/omap_device.c | 168 ++++++++++++++++++++++++++------------ 1 files changed, 114 insertions(+), 54 deletions(-)arch/arm/plat-omap/omap_device.c +37
-0@@ -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,
arch/arm/plat-omap/omap_device.c static int omap_device_fill_resources(struct omap_device *od, · line 446
Quoted 10 lines
arch/arm/plat-omap/omap_device.c +10
-0+ 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.
arch/arm/plat-omap/omap_device.c static int omap_device_fill_resources(struct omap_device *od, · line 449
Quoted 3 lines
arch/arm/plat-omap/omap_device.c +3
-0+ __func__, pdev->num_resources); +/* +* HACK: Idealy the resources from DT should match, and hwmod
s/Idealy/Ideally/
arch/arm/plat-omap/omap_device.c static int omap_device_fill_resources(struct omap_device *od, · line 451
Quoted 2 lines
arch/arm/plat-omap/omap_device.c +2
-0+* 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.
Quoted 39 lines
arch/arm/plat-omap/omap_device.c +39
-0+*/ + 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);
arch/arm/plat-omap/omap_device.c static int omap_device_fill_resources(struct omap_device *od, · line 500
Quoted 10 lines
arch/arm/plat-omap/omap_device.c +10
-0+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.
Quoted 17 lines
arch/arm/plat-omap/omap_device.c +3
-3+} + +/** * 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 );
arch/arm/plat-omap/omap_device.c struct platform_device *omap_device_build_ss(const char *pdev_name, int pdev_id, · line 579
Quoted 10 lines
arch/arm/plat-omap/omap_device.c +5
-2@@ -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.
Quoted 62 lines
arch/arm/plat-omap/omap_device.c +4
-46- 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;
arch/arm/plat-omap/omap_device.c struct platform_device *omap_device_build_ss(const char *pdev_name, int pdev_id, · line 602
Quoted 10 lines
arch/arm/plat-omap/omap_device.c +1
-3return 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
Quoted 75 lines · Kevin
Quoted 6 lines · Benoit Cousson
Split the
omap_device_build_ssinto two smaller functions that will allow to populate aplatform_devicealready alocated by device-tree.
The functionality of theomap_device_build_ssis still the same, but theomap_device_allocwill 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...Quoted 62 lines
Signed-off-by Benoit Cousson<[email protected]>Cc Kevin Hilman<[email protected]>--- arch/arm/plat-omap/omap_device.c | 168 ++++++++++++++++++++++++++------------ 1 files changed, 114 insertions(+), 54 deletions(-)arch/arm/plat-omap/omap_device.c +41
-0@@ -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; +
Quoted 8 lines
arch/arm/plat-omap/omap_device.c static int omap_device_fill_resources(struct omap_device *od, · line 446
Quoted 6 lines
arch/arm/plat-omap/omap_device.c +6
-0+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 :-)
Quoted 1 line
also, this check/warn should probably go after the 'HACK' comment below.
yep.
Quoted 6 lines
arch/arm/plat-omap/omap_device.c static int omap_device_fill_resources(struct omap_device *od, · line 449
Quoted 4 lines
arch/arm/plat-omap/omap_device.c +3
-0+ __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...
Quoted 6 lines
arch/arm/plat-omap/omap_device.c static int omap_device_fill_resources(struct omap_device *od, · line 451
Quoted 3 lines
arch/arm/plat-omap/omap_device.c +2
-0+* 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 :-)
Quoted 43 lines
Quoted 43 lines
arch/arm/plat-omap/omap_device.c +43
-0+*/ + 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);
Quoted 8 lines
arch/arm/plat-omap/omap_device.c static int omap_device_fill_resources(struct omap_device *od, · line 500
Quoted 6 lines
arch/arm/plat-omap/omap_device.c +6
-0+} + +void omap_device_delete (struct omap_device *od) +{ +kfree (od->hwmods); +kfree (od);maybe set
pdev->archdata.od= NULL too.
OK.
Quoted 20 lines
Quoted 20 lines
arch/arm/plat-omap/omap_device.c +3
-3+} + +/** * 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; }
Quoted 10 lines
arch/arm/plat-omap/omap_device.c struct platform_device *omap_device_build_ss(const char *pdev_name, int pdev_id, · line 579
Quoted 7 lines
arch/arm/plat-omap/omap_device.c +5
-2-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_nameis 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
"Cousson, Benoit" <[email protected]> writes:
[...]
Quoted 20 lines
Quoted 4 lines
Quoted 4 lines
arch/arm/plat-omap/omap_device.c +0
-0@@ -471,72 +572,31 @@ struct platform_device *omap_device_build_ss(const char *pdev_name, int pdev_id, goto odbs_exit; }Quoted 10 lines
arch/arm/plat-omap/omap_device.c struct platform_device *omap_device_build_ss(const char *pdev_name, int pdev_id, · line 579
Quoted 7 lines
arch/arm/plat-omap/omap_device.c +5
-2-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_nameis 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_addis doing only that:
Quoted 8 lines
if (dev->init_name) { dev_set_name(dev, "%s", dev->init_name); dev->init_name = NULL; }Whereas
dev_set_nameis 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
In reply to Benoit CoussonAdd 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.
--- arch/arm/plat-omap/omap_device.c | 39 ++++++++++++++++++++++++++++++++++++++ 1 files changed, 39 insertions(+), 0 deletions(-)
arch/arm/plat-omap/omap_device.c +39 -0
@@ -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 */
Signature · 1 line
1.7.0.4
RFC PATCH 2/3 OMAP: omap_device: Add a DT parser for multiple strings
Quoted 8 lines · 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 <[email protected]>Cc Kevin Hilman <[email protected]>
These should just be folded into 3/3 where they are used (with a comment changelog of course.)
Kevin
Quoted 53 lines
--- arch/arm/plat-omap/omap_device.c | 39 ++++++++++++++++++++++++++++++++++++++ 1 files changed, 39 insertions(+), 0 deletions(-)arch/arm/plat-omap/omap_device.c +39
-0@@ -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
Quoted 5 lines · Kevin
Quoted 3 lines · Benoit Cousson
Add two helpers function to parse a property that contains multiple strings.
Quoted 8 lines
Quoted 5 lines
These functions might be exported and moved to a common place if they can to be useful elsewhere.
Signed-off-by Benoit Cousson<[email protected]>Cc Kevin Hilman<[email protected]>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
Quoted 56 lines
Kevin
Quoted 53 lines
--- arch/arm/plat-omap/omap_device.c | 39 ++++++++++++++++++++++++++++++++++++++ 1 files changed, 39 insertions(+), 0 deletions(-)arch/arm/plat-omap/omap_device.c +39
-0@@ -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
In reply to Benoit CoussonAdd 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.
--- arch/arm/plat-omap/omap_device.c | 102 ++++++++++++++++++++++++++++++++++++++ 1 files changed, 102 insertions(+), 0 deletions(-)
arch/arm/plat-omap/omap_device.c +102 -0
@@ -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);
Signature · 1 line
1.7.0.4
RFC PATCH 3/3 OMAP: omap_device: Add a method to build an omap_device from a DT node
Kevin,
Some self-review...
Quoted 2 lines · Benoit
Add a notifier called during
device_addphase. If aof_nodeis present, retrieve the hwmod entry in order to populate propely theomap_device
typo
Quoted 16 lines
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<[email protected]>Cc Kevin Hilman<[email protected]>--- arch/arm/plat-omap/omap_device.c | 102 ++++++++++++++++++++++++++++++++++++++ 1 files changed, 102 insertions(+), 0 deletions(-)arch/arm/plat-omap/omap_device.c +0
-0
arch/arm/plat-omap/omap_device.c line 88
Quoted 4 lines
arch/arm/plat-omap/omap_device.c +1
-0@@ -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
Quoted 127 lines
arch/arm/plat-omap/omap_device.c +101
-0+#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
In reply to Benoit CoussonQuoted 9 lines · Benoit Cousson
Add a notifier called during
device_addphase. If aof_nodeis present, retrieve the hwmod entry in order to populate propely theomap_devicestructure.
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 <[email protected]>Cc Kevin Hilman <[email protected]>
Nice, minor comment below...
Quoted 108 lines
--- arch/arm/plat-omap/omap_device.c | 102 ++++++++++++++++++++++++++++++++++++++ 1 files changed, 102 insertions(+), 0 deletions(-)arch/arm/plat-omap/omap_device.c +82
-0@@ -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); +
arch/arm/plat-omap/omap_device.c static int _dt_get_property(const char *prop, int len, int index, char *output, · line 448
Quoted 10 lines
arch/arm/plat-omap/omap_device.c +10
-0+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
Quoted 22 lines
arch/arm/plat-omap/omap_device.c +10
-0+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
Quoted 13 lines · Kevin
Quoted 9 lines · Benoit Cousson
Add a notifier called during
device_addphase. If aof_nodeis present, retrieve the hwmod entry in order to populate propely theomap_devicestructure.
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<[email protected]>Cc Kevin Hilman<[email protected]>Nice, minor comment below...
[...]
Quoted 6 lines
arch/arm/plat-omap/omap_device.c static int _dt_get_property(const char *prop, int len, int index, char *output, · line 448
Quoted 2 lines
arch/arm/plat-omap/omap_device.c +2
-0+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