| Message ID | 1311292338-11830-9-git-send-email-khilman@ti.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Thu, Jul 21, 2011 at 5:52 PM, Kevin Hilman <khilman@ti.com> wrote: > Rather than embedding a struct platform_device inside a struct > omap_device, decouple them, leaving only a pointer to the > platform_device inside the omap_device. > > This patch uses devres to allocate and attach the omap_device to the > struct device, so finding an omap_device from a struct device means > using devres_find(), and the to_omap_device() helper function was > modified accordingly. > > RFC/Hack alert: > > Currently the driver core (drivers/base/dd.c) doesn't expect any > devres resources to exist before the driver's ->probe() is called. In > this patch, I just comment out the warning, but we'll need to > understand why that limitation exists, and if it's a real limitation. > A first glance suggests that it's not really needed. If this is a > true limitation, we'll need to find some way other than devres to > attach an omap_device to a struct device. The devres lifetime is scoped to binding a driver; it is added at probe time and removed at release. For this use-case, it needs to be scoped to the lifetime of the struct device. To make this work, you'll need to add a flag to devres so that the driver core can differentiate between driver-scoped and device-scoped lifetimes (which I do think is the right thing to do). Without fixing this, the driver core will remove all the devres when a driver gets unbound, or if a .probe() hook fails, which completely breaks the driver model. g. > > On OMAP, we will need an omap_device attached to a struct device > before probe because device HW may be disabled in probe and drivers > are expected to use runtime PM in ->probe() to activate the hardware > before access. Because the runtime PM API calls use omap_device (via > our PM domain layer), we need omap_device attached to a > platform_device before probe. > --- > arch/arm/mach-omap2/opp.c | 2 +- > arch/arm/plat-omap/include/plat/omap_device.h | 4 +- > arch/arm/plat-omap/omap_device.c | 78 +++++++++++++++---------- > drivers/base/dd.c | 2 +- > 4 files changed, 50 insertions(+), 36 deletions(-) > > diff --git a/arch/arm/mach-omap2/opp.c b/arch/arm/mach-omap2/opp.c > index ab8b35b..9262a6b 100644 > --- a/arch/arm/mach-omap2/opp.c > +++ b/arch/arm/mach-omap2/opp.c > @@ -69,7 +69,7 @@ int __init omap_init_opp_table(struct omap_opp_def *opp_def, > opp_def->hwmod_name, i); > return -EINVAL; > } > - dev = &oh->od->pdev.dev; > + dev = &oh->od->pdev->dev; > > r = opp_add(dev, opp_def->freq, opp_def->u_volt); > if (r) { > diff --git a/arch/arm/plat-omap/include/plat/omap_device.h b/arch/arm/plat-omap/include/plat/omap_device.h > index 7a3ec4f..6bd4f6f 100644 > --- a/arch/arm/plat-omap/include/plat/omap_device.h > +++ b/arch/arm/plat-omap/include/plat/omap_device.h > @@ -64,7 +64,7 @@ extern struct device omap_device_parent; > * > */ > struct omap_device { > - struct platform_device pdev; > + struct platform_device *pdev; > struct omap_hwmod **hwmods; > struct omap_device_pm_latency *pm_lats; > u32 dev_wakeup_lat; > @@ -142,6 +142,6 @@ struct omap_device_pm_latency { > #define OMAP_DEVICE_LATENCY_AUTO_ADJUST BIT(1) > > /* Get omap_device pointer from platform_device pointer */ > -#define to_omap_device(x) container_of((x), struct omap_device, pdev) > +struct omap_device *to_omap_device(struct platform_device *pdev); > > #endif > diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c > index c420b94..52ce013 100644 > --- a/arch/arm/plat-omap/omap_device.c > +++ b/arch/arm/plat-omap/omap_device.c > @@ -117,7 +117,7 @@ static int _omap_device_activate(struct omap_device *od, u8 ignore_lat) > { > struct timespec a, b, c; > > - dev_dbg(&od->pdev.dev, "omap_device: activating\n"); > + dev_dbg(&od->pdev->dev, "omap_device: activating\n"); > > while (od->pm_lat_level > 0) { > struct omap_device_pm_latency *odpl; > @@ -141,7 +141,7 @@ static int _omap_device_activate(struct omap_device *od, u8 ignore_lat) > c = timespec_sub(b, a); > act_lat = timespec_to_ns(&c); > > - dev_dbg(&od->pdev.dev, > + dev_dbg(&od->pdev->dev, > "omap_device: pm_lat %d: activate: elapsed time " > "%llu nsec\n", od->pm_lat_level, act_lat); > > @@ -149,12 +149,12 @@ static int _omap_device_activate(struct omap_device *od, u8 ignore_lat) > odpl->activate_lat_worst = act_lat; > if (odpl->flags & OMAP_DEVICE_LATENCY_AUTO_ADJUST) { > odpl->activate_lat = act_lat; > - dev_warn(&od->pdev.dev, > + dev_warn(&od->pdev->dev, > "new worst case activate latency " > "%d: %llu\n", > od->pm_lat_level, act_lat); > } else > - dev_warn(&od->pdev.dev, > + dev_warn(&od->pdev->dev, > "activate latency %d " > "higher than exptected. (%llu > %d)\n", > od->pm_lat_level, act_lat, > @@ -185,7 +185,7 @@ static int _omap_device_deactivate(struct omap_device *od, u8 ignore_lat) > { > struct timespec a, b, c; > > - dev_dbg(&od->pdev.dev, "omap_device: deactivating\n"); > + dev_dbg(&od->pdev->dev, "omap_device: deactivating\n"); > > while (od->pm_lat_level < od->pm_lats_cnt) { > struct omap_device_pm_latency *odpl; > @@ -208,7 +208,7 @@ static int _omap_device_deactivate(struct omap_device *od, u8 ignore_lat) > c = timespec_sub(b, a); > deact_lat = timespec_to_ns(&c); > > - dev_dbg(&od->pdev.dev, > + dev_dbg(&od->pdev->dev, > "omap_device: pm_lat %d: deactivate: elapsed time " > "%llu nsec\n", od->pm_lat_level, deact_lat); > > @@ -216,12 +216,12 @@ static int _omap_device_deactivate(struct omap_device *od, u8 ignore_lat) > odpl->deactivate_lat_worst = deact_lat; > if (odpl->flags & OMAP_DEVICE_LATENCY_AUTO_ADJUST) { > odpl->deactivate_lat = deact_lat; > - dev_warn(&od->pdev.dev, > + dev_warn(&od->pdev->dev, > "new worst case deactivate latency " > "%d: %llu\n", > od->pm_lat_level, deact_lat); > } else > - dev_warn(&od->pdev.dev, > + dev_warn(&od->pdev->dev, > "deactivate latency %d " > "higher than exptected. (%llu > %d)\n", > od->pm_lat_level, deact_lat, > @@ -245,11 +245,11 @@ static void _add_clkdev(struct omap_device *od, const char *clk_alias, > if (!clk_alias || !clk_name) > return; > > - dev_dbg(&od->pdev.dev, "Creating %s -> %s\n", clk_alias, clk_name); > + dev_dbg(&od->pdev->dev, "Creating %s -> %s\n", clk_alias, clk_name); > > - r = clk_get_sys(dev_name(&od->pdev.dev), clk_alias); > + r = clk_get_sys(dev_name(&od->pdev->dev), clk_alias); > if (!IS_ERR(r)) { > - dev_warn(&od->pdev.dev, > + dev_warn(&od->pdev->dev, > "alias %s already exists\n", clk_alias); > clk_put(r); > return; > @@ -257,14 +257,14 @@ static void _add_clkdev(struct omap_device *od, const char *clk_alias, > > r = omap_clk_get_by_name(clk_name); > if (IS_ERR(r)) { > - dev_err(&od->pdev.dev, > + dev_err(&od->pdev->dev, > "omap_clk_get_by_name for %s failed\n", clk_name); > return; > } > > - l = clkdev_alloc(r, clk_alias, dev_name(&od->pdev.dev)); > + l = clkdev_alloc(r, clk_alias, dev_name(&od->pdev->dev)); > if (!l) { > - dev_err(&od->pdev.dev, > + dev_err(&od->pdev->dev, > "clkdev_alloc for %s failed\n", clk_alias); > return; > } > @@ -351,7 +351,7 @@ static int omap_device_count_resources(struct omap_device *od) > c += omap_hwmod_count_resources(od->hwmods[i]); > > pr_debug("omap_device: %s: counted %d total resources across %d " > - "hwmods\n", od->pdev.name, c, od->hwmods_cnt); > + "hwmods\n", od->pdev->name, c, od->hwmods_cnt); > > return c; > } > @@ -388,6 +388,21 @@ static int omap_device_fill_resources(struct omap_device *od, > return 0; > } > > +static void _od_dr_release(struct device *dev, void *res) > +{ > + kfree(res); > +} > + > +struct omap_device *to_omap_device(struct platform_device *pdev) > +{ > + void *res; > + > + res = devres_find(&pdev->dev, _od_dr_release, NULL, NULL); > + WARN_ON(!res); > + > + return (struct omap_device *)res; > +} > + > /** > * omap_device_build - build and register an omap_device with one omap_hwmod > * @pdev_name: name of the platform_device driver to use > @@ -445,8 +460,8 @@ struct platform_device *omap_device_build_ss(const char *pdev_name, int pdev_id, > int pm_lats_cnt, int is_early_device) > { > int ret = -ENOMEM; > + struct platform_device *pdev; > struct omap_device *od; > - char *pdev_name2; > struct resource *res = NULL; > int i, res_count; > struct omap_hwmod **hwmods; > @@ -460,7 +475,8 @@ struct platform_device *omap_device_build_ss(const char *pdev_name, int pdev_id, > pr_debug("omap_device: %s: building with %d hwmods\n", pdev_name, > oh_cnt); > > - od = kzalloc(sizeof(struct omap_device), GFP_KERNEL); > + od = devres_alloc(_od_dr_release, sizeof(struct omap_device), > + GFP_KERNEL); > if (!od) > return ERR_PTR(-ENOMEM); > > @@ -474,13 +490,9 @@ struct platform_device *omap_device_build_ss(const char *pdev_name, int pdev_id, > memcpy(hwmods, ohs, sizeof(struct omap_hwmod *) * oh_cnt); > od->hwmods = hwmods; > > - pdev_name2 = kzalloc(strlen(pdev_name) + 1, GFP_KERNEL); > - if (!pdev_name2) > + pdev = platform_device_alloc(pdev_name, pdev_id); > + if (!pdev) > goto odbs_exit2; > - strcpy(pdev_name2, pdev_name); > - > - od->pdev.name = pdev_name2; > - od->pdev.id = pdev_id; > > res_count = omap_device_count_resources(od); > if (res_count > 0) { > @@ -490,35 +502,37 @@ struct platform_device *omap_device_build_ss(const char *pdev_name, int pdev_id, > } > omap_device_fill_resources(od, res); > > - od->pdev.num_resources = res_count; > - od->pdev.resource = res; > + pdev->num_resources = res_count; > + pdev->resource = res; > > - ret = platform_device_add_data(&od->pdev, pdata, pdata_len); > + ret = platform_device_add_data(pdev, pdata, pdata_len); > if (ret) > goto odbs_exit4; > > od->pm_lats = pm_lats; > od->pm_lats_cnt = pm_lats_cnt; > > - if (is_early_device) > - ret = omap_early_device_register(&od->pdev); > - else > - ret = omap_device_register(&od->pdev); > + od->pdev = pdev; > + devres_add(&pdev->dev, od); > > for (i = 0; i < oh_cnt; i++) { > hwmods[i]->od = od; > _add_hwmod_clocks_clkdev(od, hwmods[i]); > } > > + if (is_early_device) > + ret = omap_early_device_register(pdev); > + else > + ret = omap_device_register(pdev); > + > if (ret) > goto odbs_exit4; > > - return &od->pdev; > + return pdev; > > odbs_exit4: > kfree(res); > odbs_exit3: > - kfree(pdev_name2); > odbs_exit2: > kfree(hwmods); > odbs_exit1: > diff --git a/drivers/base/dd.c b/drivers/base/dd.c > index 6658da7..9289dac 100644 > --- a/drivers/base/dd.c > +++ b/drivers/base/dd.c > @@ -112,7 +112,7 @@ static int really_probe(struct device *dev, struct device_driver *drv) > atomic_inc(&probe_count); > pr_debug("bus: '%s': %s: probing driver %s with device %s\n", > drv->bus->name, __func__, drv->name, dev_name(dev)); > - WARN_ON(!list_empty(&dev->devres_head)); > + /* WARN_ON(!list_empty(&dev->devres_head)); */ > > dev->driver = drv; > if (driver_sysfs_add(dev)) { > -- > 1.7.6 > >
On Thu, Jul 21, 2011 at 04:52:18PM -0700, Kevin Hilman wrote: > Rather than embedding a struct platform_device inside a struct > omap_device, decouple them, leaving only a pointer to the > platform_device inside the omap_device. > > This patch uses devres to allocate and attach the omap_device to the > struct device, so finding an omap_device from a struct device means > using devres_find(), and the to_omap_device() helper function was > modified accordingly. > > RFC/Hack alert: > > Currently the driver core (drivers/base/dd.c) doesn't expect any > devres resources to exist before the driver's ->probe() is called. In > this patch, I just comment out the warning, but we'll need to > understand why that limitation exists, and if it's a real limitation. > A first glance suggests that it's not really needed. If this is a > true limitation, we'll need to find some way other than devres to > attach an omap_device to a struct device. > > On OMAP, we will need an omap_device attached to a struct device > before probe because device HW may be disabled in probe and drivers > are expected to use runtime PM in ->probe() to activate the hardware > before access. Because the runtime PM API calls use omap_device (via > our PM domain layer), we need omap_device attached to a > platform_device before probe. This feels really wrong to overload devres with this. devres purpose is to provide the device's _drivers_ with a way to allocate and free resources in such a way to avoid leaks on .remove or probe failure. So I think that overloading it with something that has a different lifetime is completely wrong. We could add a new member to struct dev_archdata or pdev_archdata to carry a pointer to this data, which I think would be a far cleaner (and saner) way to deal with this. In much the same way as we already have an of_node member in struct device. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Jul 30, 2011 at 01:03:32PM +0100, Russell King - ARM Linux wrote: > On Thu, Jul 21, 2011 at 04:52:18PM -0700, Kevin Hilman wrote: > > Rather than embedding a struct platform_device inside a struct > > omap_device, decouple them, leaving only a pointer to the > > platform_device inside the omap_device. > > > > This patch uses devres to allocate and attach the omap_device to the > > struct device, so finding an omap_device from a struct device means > > using devres_find(), and the to_omap_device() helper function was > > modified accordingly. > > > > RFC/Hack alert: > > > > Currently the driver core (drivers/base/dd.c) doesn't expect any > > devres resources to exist before the driver's ->probe() is called. In > > this patch, I just comment out the warning, but we'll need to > > understand why that limitation exists, and if it's a real limitation. > > A first glance suggests that it's not really needed. If this is a > > true limitation, we'll need to find some way other than devres to > > attach an omap_device to a struct device. > > > > On OMAP, we will need an omap_device attached to a struct device > > before probe because device HW may be disabled in probe and drivers > > are expected to use runtime PM in ->probe() to activate the hardware > > before access. Because the runtime PM API calls use omap_device (via > > our PM domain layer), we need omap_device attached to a > > platform_device before probe. > > This feels really wrong to overload devres with this. devres purpose is > to provide the device's _drivers_ with a way to allocate and free resources > in such a way to avoid leaks on .remove or probe failure. So I think that > overloading it with something that has a different lifetime is completely > wrong. I disagree; extending devres to also handle device lifetime scoped resources makes perfect sense. It is a clean extension, and struct device is really getting rather huge. I certainly prefer re-scoping devres to adding more elements to struct device. > We could add a new member to struct dev_archdata or pdev_archdata to carry > a pointer to this data, which I think would be a far cleaner (and saner) > way to deal with this. In much the same way as we already have an of_node > member in struct device. Since this is just for omap_device, which by definition is arm-only, I don't have any strong objection to using pdev_archdata. If it was cross-architecture, then I'd argue strongly for the devres approach. g. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Jul 30, 2011 at 08:58:07PM -0600, Grant Likely wrote: > On Sat, Jul 30, 2011 at 01:03:32PM +0100, Russell King - ARM Linux wrote: > > On Thu, Jul 21, 2011 at 04:52:18PM -0700, Kevin Hilman wrote: > > > Rather than embedding a struct platform_device inside a struct > > > omap_device, decouple them, leaving only a pointer to the > > > platform_device inside the omap_device. > > > > > > This patch uses devres to allocate and attach the omap_device to the > > > struct device, so finding an omap_device from a struct device means > > > using devres_find(), and the to_omap_device() helper function was > > > modified accordingly. > > > > > > RFC/Hack alert: > > > > > > Currently the driver core (drivers/base/dd.c) doesn't expect any > > > devres resources to exist before the driver's ->probe() is called. In > > > this patch, I just comment out the warning, but we'll need to > > > understand why that limitation exists, and if it's a real limitation. > > > A first glance suggests that it's not really needed. If this is a > > > true limitation, we'll need to find some way other than devres to > > > attach an omap_device to a struct device. > > > > > > On OMAP, we will need an omap_device attached to a struct device > > > before probe because device HW may be disabled in probe and drivers > > > are expected to use runtime PM in ->probe() to activate the hardware > > > before access. Because the runtime PM API calls use omap_device (via > > > our PM domain layer), we need omap_device attached to a > > > platform_device before probe. > > > > This feels really wrong to overload devres with this. devres purpose is > > to provide the device's _drivers_ with a way to allocate and free resources > > in such a way to avoid leaks on .remove or probe failure. So I think that > > overloading it with something that has a different lifetime is completely > > wrong. > > I disagree; extending devres to also handle device lifetime scoped > resources makes perfect sense. It is a clean extension, and struct device > is really getting rather huge. I certainly prefer re-scoping devres > to adding more elements to struct device. The point is you're asking something which is designed to have a well defined lifetime of driver-bind...driver-unbind to do a lot more than that - extending its lifetime conditional on some flag to the entire device lifetime instead. Such extensions tend to be a disaster - and a recipe for confusion as people will no longer have a clear idea of the lifetime issues. We already know that people absolutely struggle to understand lifed objects - we see it time and time again where people directly kfree stuff like platform devices without thinking about whether they're still in use. So no, extending devres is a _big_ mistake and is far from clean. Not only that, but if most of the platform devices are omap devices, (as it would be on OMAP), you'll consume more memory through having to have the additional management structs for the omap_device stuff than a simple pointer. As for struct device getting rather huge, last time I looked it contained a load of stuff which was there whether or not a platform used it. Eg, you get 4 bytes wasted for an of_node whether you have DT support or not. If sizeof(struct device) really is a problem, then it needs the unused stuff ifdef'd away. So I don't buy the "its getting rather huge" argument. > > We could add a new member to struct dev_archdata or pdev_archdata to carry > > a pointer to this data, which I think would be a far cleaner (and saner) > > way to deal with this. In much the same way as we already have an of_node > > member in struct device. > > Since this is just for omap_device, which by definition is arm-only, I > don't have any strong objection to using pdev_archdata. If it was > cross-architecture, then I'd argue strongly for the devres approach. Indeed, which is why I think putting it in the platform device archdata makes total sense, more sense than buggering up the clean devres lifetime rules that we have today. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/mach-omap2/opp.c b/arch/arm/mach-omap2/opp.c index ab8b35b..9262a6b 100644 --- a/arch/arm/mach-omap2/opp.c +++ b/arch/arm/mach-omap2/opp.c @@ -69,7 +69,7 @@ int __init omap_init_opp_table(struct omap_opp_def *opp_def, opp_def->hwmod_name, i); return -EINVAL; } - dev = &oh->od->pdev.dev; + dev = &oh->od->pdev->dev; r = opp_add(dev, opp_def->freq, opp_def->u_volt); if (r) { diff --git a/arch/arm/plat-omap/include/plat/omap_device.h b/arch/arm/plat-omap/include/plat/omap_device.h index 7a3ec4f..6bd4f6f 100644 --- a/arch/arm/plat-omap/include/plat/omap_device.h +++ b/arch/arm/plat-omap/include/plat/omap_device.h @@ -64,7 +64,7 @@ extern struct device omap_device_parent; * */ struct omap_device { - struct platform_device pdev; + struct platform_device *pdev; struct omap_hwmod **hwmods; struct omap_device_pm_latency *pm_lats; u32 dev_wakeup_lat; @@ -142,6 +142,6 @@ struct omap_device_pm_latency { #define OMAP_DEVICE_LATENCY_AUTO_ADJUST BIT(1) /* Get omap_device pointer from platform_device pointer */ -#define to_omap_device(x) container_of((x), struct omap_device, pdev) +struct omap_device *to_omap_device(struct platform_device *pdev); #endif diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c index c420b94..52ce013 100644 --- a/arch/arm/plat-omap/omap_device.c +++ b/arch/arm/plat-omap/omap_device.c @@ -117,7 +117,7 @@ static int _omap_device_activate(struct omap_device *od, u8 ignore_lat) { struct timespec a, b, c; - dev_dbg(&od->pdev.dev, "omap_device: activating\n"); + dev_dbg(&od->pdev->dev, "omap_device: activating\n"); while (od->pm_lat_level > 0) { struct omap_device_pm_latency *odpl; @@ -141,7 +141,7 @@ static int _omap_device_activate(struct omap_device *od, u8 ignore_lat) c = timespec_sub(b, a); act_lat = timespec_to_ns(&c); - dev_dbg(&od->pdev.dev, + dev_dbg(&od->pdev->dev, "omap_device: pm_lat %d: activate: elapsed time " "%llu nsec\n", od->pm_lat_level, act_lat); @@ -149,12 +149,12 @@ static int _omap_device_activate(struct omap_device *od, u8 ignore_lat) odpl->activate_lat_worst = act_lat; if (odpl->flags & OMAP_DEVICE_LATENCY_AUTO_ADJUST) { odpl->activate_lat = act_lat; - dev_warn(&od->pdev.dev, + dev_warn(&od->pdev->dev, "new worst case activate latency " "%d: %llu\n", od->pm_lat_level, act_lat); } else - dev_warn(&od->pdev.dev, + dev_warn(&od->pdev->dev, "activate latency %d " "higher than exptected. (%llu > %d)\n", od->pm_lat_level, act_lat, @@ -185,7 +185,7 @@ static int _omap_device_deactivate(struct omap_device *od, u8 ignore_lat) { struct timespec a, b, c; - dev_dbg(&od->pdev.dev, "omap_device: deactivating\n"); + dev_dbg(&od->pdev->dev, "omap_device: deactivating\n"); while (od->pm_lat_level < od->pm_lats_cnt) { struct omap_device_pm_latency *odpl; @@ -208,7 +208,7 @@ static int _omap_device_deactivate(struct omap_device *od, u8 ignore_lat) c = timespec_sub(b, a); deact_lat = timespec_to_ns(&c); - dev_dbg(&od->pdev.dev, + dev_dbg(&od->pdev->dev, "omap_device: pm_lat %d: deactivate: elapsed time " "%llu nsec\n", od->pm_lat_level, deact_lat); @@ -216,12 +216,12 @@ static int _omap_device_deactivate(struct omap_device *od, u8 ignore_lat) odpl->deactivate_lat_worst = deact_lat; if (odpl->flags & OMAP_DEVICE_LATENCY_AUTO_ADJUST) { odpl->deactivate_lat = deact_lat; - dev_warn(&od->pdev.dev, + dev_warn(&od->pdev->dev, "new worst case deactivate latency " "%d: %llu\n", od->pm_lat_level, deact_lat); } else - dev_warn(&od->pdev.dev, + dev_warn(&od->pdev->dev, "deactivate latency %d " "higher than exptected. (%llu > %d)\n", od->pm_lat_level, deact_lat, @@ -245,11 +245,11 @@ static void _add_clkdev(struct omap_device *od, const char *clk_alias, if (!clk_alias || !clk_name) return; - dev_dbg(&od->pdev.dev, "Creating %s -> %s\n", clk_alias, clk_name); + dev_dbg(&od->pdev->dev, "Creating %s -> %s\n", clk_alias, clk_name); - r = clk_get_sys(dev_name(&od->pdev.dev), clk_alias); + r = clk_get_sys(dev_name(&od->pdev->dev), clk_alias); if (!IS_ERR(r)) { - dev_warn(&od->pdev.dev, + dev_warn(&od->pdev->dev, "alias %s already exists\n", clk_alias); clk_put(r); return; @@ -257,14 +257,14 @@ static void _add_clkdev(struct omap_device *od, const char *clk_alias, r = omap_clk_get_by_name(clk_name); if (IS_ERR(r)) { - dev_err(&od->pdev.dev, + dev_err(&od->pdev->dev, "omap_clk_get_by_name for %s failed\n", clk_name); return; } - l = clkdev_alloc(r, clk_alias, dev_name(&od->pdev.dev)); + l = clkdev_alloc(r, clk_alias, dev_name(&od->pdev->dev)); if (!l) { - dev_err(&od->pdev.dev, + dev_err(&od->pdev->dev, "clkdev_alloc for %s failed\n", clk_alias); return; } @@ -351,7 +351,7 @@ static int omap_device_count_resources(struct omap_device *od) c += omap_hwmod_count_resources(od->hwmods[i]); pr_debug("omap_device: %s: counted %d total resources across %d " - "hwmods\n", od->pdev.name, c, od->hwmods_cnt); + "hwmods\n", od->pdev->name, c, od->hwmods_cnt); return c; } @@ -388,6 +388,21 @@ static int omap_device_fill_resources(struct omap_device *od, return 0; } +static void _od_dr_release(struct device *dev, void *res) +{ + kfree(res); +} + +struct omap_device *to_omap_device(struct platform_device *pdev) +{ + void *res; + + res = devres_find(&pdev->dev, _od_dr_release, NULL, NULL); + WARN_ON(!res); + + return (struct omap_device *)res; +} + /** * omap_device_build - build and register an omap_device with one omap_hwmod * @pdev_name: name of the platform_device driver to use @@ -445,8 +460,8 @@ struct platform_device *omap_device_build_ss(const char *pdev_name, int pdev_id, int pm_lats_cnt, int is_early_device) { int ret = -ENOMEM; + struct platform_device *pdev; struct omap_device *od; - char *pdev_name2; struct resource *res = NULL; int i, res_count; struct omap_hwmod **hwmods; @@ -460,7 +475,8 @@ struct platform_device *omap_device_build_ss(const char *pdev_name, int pdev_id, pr_debug("omap_device: %s: building with %d hwmods\n", pdev_name, oh_cnt); - od = kzalloc(sizeof(struct omap_device), GFP_KERNEL); + od = devres_alloc(_od_dr_release, sizeof(struct omap_device), + GFP_KERNEL); if (!od) return ERR_PTR(-ENOMEM); @@ -474,13 +490,9 @@ struct platform_device *omap_device_build_ss(const char *pdev_name, int pdev_id, memcpy(hwmods, ohs, sizeof(struct omap_hwmod *) * oh_cnt); od->hwmods = hwmods; - pdev_name2 = kzalloc(strlen(pdev_name) + 1, GFP_KERNEL); - if (!pdev_name2) + pdev = platform_device_alloc(pdev_name, pdev_id); + if (!pdev) goto odbs_exit2; - strcpy(pdev_name2, pdev_name); - - od->pdev.name = pdev_name2; - od->pdev.id = pdev_id; res_count = omap_device_count_resources(od); if (res_count > 0) { @@ -490,35 +502,37 @@ struct platform_device *omap_device_build_ss(const char *pdev_name, int pdev_id, } omap_device_fill_resources(od, res); - od->pdev.num_resources = res_count; - od->pdev.resource = res; + pdev->num_resources = res_count; + pdev->resource = res; - ret = platform_device_add_data(&od->pdev, pdata, pdata_len); + ret = platform_device_add_data(pdev, pdata, pdata_len); if (ret) goto odbs_exit4; od->pm_lats = pm_lats; od->pm_lats_cnt = pm_lats_cnt; - if (is_early_device) - ret = omap_early_device_register(&od->pdev); - else - ret = omap_device_register(&od->pdev); + od->pdev = pdev; + devres_add(&pdev->dev, od); for (i = 0; i < oh_cnt; i++) { hwmods[i]->od = od; _add_hwmod_clocks_clkdev(od, hwmods[i]); } + if (is_early_device) + ret = omap_early_device_register(pdev); + else + ret = omap_device_register(pdev); + if (ret) goto odbs_exit4; - return &od->pdev; + return pdev; odbs_exit4: kfree(res); odbs_exit3: - kfree(pdev_name2); odbs_exit2: kfree(hwmods); odbs_exit1: diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 6658da7..9289dac 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -112,7 +112,7 @@ static int really_probe(struct device *dev, struct device_driver *drv) atomic_inc(&probe_count); pr_debug("bus: '%s': %s: probing driver %s with device %s\n", drv->bus->name, __func__, drv->name, dev_name(dev)); - WARN_ON(!list_empty(&dev->devres_head)); + /* WARN_ON(!list_empty(&dev->devres_head)); */ dev->driver = drv; if (driver_sysfs_add(dev)) {