| Message ID | 1322011285-4002-8-git-send-email-swarren@nvidia.com |
|---|---|
| State | Accepted, archived |
| Headers | show |
* Stephen Warren wrote: > module_platform_drive saves some boiler-plate code. > > The devm_ APIs remove the need to manually clean up allocations, > thus removing some code. > > Signed-off-by: Stephen Warren <swarren@nvidia.com> > --- > sound/soc/tegra/tegra_das.c | 45 +++++++++--------------------------------- > 1 files changed, 10 insertions(+), 35 deletions(-) > > diff --git a/sound/soc/tegra/tegra_das.c b/sound/soc/tegra/tegra_das.c [...] > @@ -208,32 +208,18 @@ static int __devinit tegra_das_probe(struct platform_device *pdev) > > return 0; > > -err_release: > - release_mem_region(res->start, resource_size(res)); > -err_free: > - kfree(das); > +err: > das = NULL; > -exit: > return ret; > } > > static int __devexit tegra_das_remove(struct platform_device *pdev) > { > - struct resource *res; > - > if (!das) > return -ENODEV; > > - platform_set_drvdata(pdev, NULL); > - [...] Setting the driver data to NULL may still be a good idea. Thierry
On Wed, Nov 23, 2011 at 07:58:08AM +0100, Thierry Reding wrote: > * Stephen Warren wrote: > > - platform_set_drvdata(pdev, NULL); > > - > Setting the driver data to NULL may still be a good idea. It's always been a waste of time. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Nov 22, 2011 at 06:21:15PM -0700, Stephen Warren wrote: > module_platform_drive saves some boiler-plate code. > > The devm_ APIs remove the need to manually clean up allocations, > thus removing some code. Applied but this is two separate changes and should've been split. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Thierry Reding wrote at Tuesday, November 22, 2011 11:58 PM: > * Stephen Warren wrote: > > module_platform_drive saves some boiler-plate code. > > > > The devm_ APIs remove the need to manually clean up allocations, > > thus removing some code. ... > > static int __devexit tegra_das_remove(struct platform_device *pdev) > > { > > - struct resource *res; > > - > > if (!das) > > return -ENODEV; > > > > - platform_set_drvdata(pdev, NULL); > > - > [...] > > Setting the driver data to NULL may still be a good idea. When Mark Brown reviewed the TrimSlice machine driver, he mentioned that clearing the drvdata was pointless; nothing should be using it when the device is not created. As background, soon after that, I modified the tegra_wm8903.c machine driver along the same lines, but evidently didn't update tegra_das.c in a similar way, but would have if I'd been paying attention...
* Stephen Warren wrote: > Thierry Reding wrote at Tuesday, November 22, 2011 11:58 PM: > > * Stephen Warren wrote: > > > module_platform_drive saves some boiler-plate code. > > > > > > The devm_ APIs remove the need to manually clean up allocations, > > > thus removing some code. > ... > > > static int __devexit tegra_das_remove(struct platform_device *pdev) > > > { > > > - struct resource *res; > > > - > > > if (!das) > > > return -ENODEV; > > > > > > - platform_set_drvdata(pdev, NULL); > > > - > > [...] > > > > Setting the driver data to NULL may still be a good idea. > > When Mark Brown reviewed the TrimSlice machine driver, he mentioned that > clearing the drvdata was pointless; nothing should be using it when the > device is not created. > > As background, soon after that, I modified the tegra_wm8903.c machine > driver along the same lines, but evidently didn't update tegra_das.c > in a similar way, but would have if I'd been paying attention... Thinking about this some more I have to agree. The only one using the driver data would be the driver, which in turn shouldn't be doing anything with it after remove is called. I was just commenting on it because I've seen it done in a lot of drivers and thought it was common (best) practice. Thierry
diff --git a/sound/soc/tegra/tegra_das.c b/sound/soc/tegra/tegra_das.c index 3b55a44..fa3a442 100644 --- a/sound/soc/tegra/tegra_das.c +++ b/sound/soc/tegra/tegra_das.c @@ -172,11 +172,11 @@ static int __devinit tegra_das_probe(struct platform_device *pdev) if (das) return -ENODEV; - das = kzalloc(sizeof(struct tegra_das), GFP_KERNEL); + das = devm_kzalloc(&pdev->dev, sizeof(struct tegra_das), GFP_KERNEL); if (!das) { dev_err(&pdev->dev, "Can't allocate tegra_das\n"); ret = -ENOMEM; - goto exit; + goto err; } das->dev = &pdev->dev; @@ -184,22 +184,22 @@ static int __devinit tegra_das_probe(struct platform_device *pdev) if (!res) { dev_err(&pdev->dev, "No memory resource\n"); ret = -ENODEV; - goto err_free; + goto err; } - region = request_mem_region(res->start, resource_size(res), - pdev->name); + region = devm_request_mem_region(&pdev->dev, res->start, + resource_size(res), pdev->name); if (!region) { dev_err(&pdev->dev, "Memory region already claimed\n"); ret = -EBUSY; - goto err_free; + goto err; } - das->regs = ioremap(res->start, resource_size(res)); + das->regs = devm_ioremap(&pdev->dev, res->start, resource_size(res)); if (!das->regs) { dev_err(&pdev->dev, "ioremap failed\n"); ret = -ENOMEM; - goto err_release; + goto err; } tegra_das_debug_add(das); @@ -208,32 +208,18 @@ static int __devinit tegra_das_probe(struct platform_device *pdev) return 0; -err_release: - release_mem_region(res->start, resource_size(res)); -err_free: - kfree(das); +err: das = NULL; -exit: return ret; } static int __devexit tegra_das_remove(struct platform_device *pdev) { - struct resource *res; - if (!das) return -ENODEV; - platform_set_drvdata(pdev, NULL); - tegra_das_debug_remove(das); - iounmap(das->regs); - - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - release_mem_region(res->start, resource_size(res)); - - kfree(das); das = NULL; return 0; @@ -246,18 +232,7 @@ static struct platform_driver tegra_das_driver = { .name = DRV_NAME, }, }; - -static int __init tegra_das_modinit(void) -{ - return platform_driver_register(&tegra_das_driver); -} -module_init(tegra_das_modinit); - -static void __exit tegra_das_modexit(void) -{ - platform_driver_unregister(&tegra_das_driver); -} -module_exit(tegra_das_modexit); +module_platform_driver(tegra_das_driver); MODULE_AUTHOR("Stephen Warren <swarren@nvidia.com>"); MODULE_DESCRIPTION("Tegra DAS driver");
module_platform_drive saves some boiler-plate code. The devm_ APIs remove the need to manually clean up allocations, thus removing some code. Signed-off-by: Stephen Warren <swarren@nvidia.com> --- sound/soc/tegra/tegra_das.c | 45 +++++++++--------------------------------- 1 files changed, 10 insertions(+), 35 deletions(-)