[alsa-devel] [PATCH 1/2] ASoC: atmel-ssc: use devm_xxx() managed function
Using the devm_xxx() managed function to stripdown the error and remove code.
Signed-off-by: Bo Shen voice.shen@atmel.com --- drivers/misc/atmel-ssc.c | 47 +++++++++++++++------------------------------- 1 file changed, 15 insertions(+), 32 deletions(-)
diff --git a/drivers/misc/atmel-ssc.c b/drivers/misc/atmel-ssc.c index 5bb1877..23dcb76 100644 --- a/drivers/misc/atmel-ssc.c +++ b/drivers/misc/atmel-ssc.c @@ -70,37 +70,33 @@ EXPORT_SYMBOL(ssc_free);
static int __init ssc_probe(struct platform_device *pdev) { - int retval = 0; struct resource *regs; struct ssc_device *ssc;
- ssc = kzalloc(sizeof(struct ssc_device), GFP_KERNEL); + ssc = devm_kzalloc(&pdev->dev, sizeof(struct ssc_device), GFP_KERNEL); if (!ssc) { dev_dbg(&pdev->dev, "out of memory\n"); - retval = -ENOMEM; - goto out; + return -ENOMEM; }
+ ssc->pdev = pdev; + regs = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (!regs) { dev_dbg(&pdev->dev, "no mmio resource defined\n"); - retval = -ENXIO; - goto out_free; + return -ENXIO; }
- ssc->clk = clk_get(&pdev->dev, "pclk"); - if (IS_ERR(ssc->clk)) { - dev_dbg(&pdev->dev, "no pclk clock defined\n"); - retval = -ENXIO; - goto out_free; - } - - ssc->pdev = pdev; - ssc->regs = ioremap(regs->start, resource_size(regs)); + ssc->regs = devm_request_and_ioremap(&pdev->dev, regs); if (!ssc->regs) { dev_dbg(&pdev->dev, "ioremap failed\n"); - retval = -EINVAL; - goto out_clk; + return -EINVAL; + } + + ssc->clk = devm_clk_get(&pdev->dev, "pclk"); + if (IS_ERR(ssc->clk)) { + dev_dbg(&pdev->dev, "no pclk clock defined\n"); + return -ENXIO; }
/* disable all interrupts */ @@ -112,8 +108,7 @@ static int __init ssc_probe(struct platform_device *pdev) ssc->irq = platform_get_irq(pdev, 0); if (!ssc->irq) { dev_dbg(&pdev->dev, "could not get irq\n"); - retval = -ENXIO; - goto out_unmap; + return -ENXIO; }
spin_lock(&user_lock); @@ -125,16 +120,7 @@ static int __init ssc_probe(struct platform_device *pdev) dev_info(&pdev->dev, "Atmel SSC device at 0x%p (irq %d)\n", ssc->regs, ssc->irq);
- goto out; - -out_unmap: - iounmap(ssc->regs); -out_clk: - clk_put(ssc->clk); -out_free: - kfree(ssc); -out: - return retval; + return 0; }
static int __devexit ssc_remove(struct platform_device *pdev) @@ -142,10 +128,7 @@ static int __devexit ssc_remove(struct platform_device *pdev) struct ssc_device *ssc = platform_get_drvdata(pdev);
spin_lock(&user_lock); - iounmap(ssc->regs); - clk_put(ssc->clk); list_del(&ssc->list); - kfree(ssc); spin_unlock(&user_lock);
return 0;
This patch removes some code duplication by using module_platform_driver
Signed-off-by: Bo Shen voice.shen@atmel.com --- drivers/misc/atmel-ssc.c | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-)
diff --git a/drivers/misc/atmel-ssc.c b/drivers/misc/atmel-ssc.c index 23dcb76..ac00f83 100644 --- a/drivers/misc/atmel-ssc.c +++ b/drivers/misc/atmel-ssc.c @@ -68,7 +68,7 @@ void ssc_free(struct ssc_device *ssc) } EXPORT_SYMBOL(ssc_free);
-static int __init ssc_probe(struct platform_device *pdev) +static int ssc_probe(struct platform_device *pdev) { struct resource *regs; struct ssc_device *ssc; @@ -135,24 +135,14 @@ static int __devexit ssc_remove(struct platform_device *pdev) }
static struct platform_driver ssc_driver = { - .remove = __devexit_p(ssc_remove), .driver = { .name = "ssc", .owner = THIS_MODULE, }, + .probe = ssc_probe, + .remove = __devexit_p(ssc_remove), }; - -static int __init ssc_init(void) -{ - return platform_driver_probe(&ssc_driver, ssc_probe); -} -module_init(ssc_init); - -static void __exit ssc_exit(void) -{ - platform_driver_unregister(&ssc_driver); -} -module_exit(ssc_exit); +module_platform_driver(ssc_driver);
MODULE_AUTHOR("Hans-Christian Egtvedt hcegtvedt@atmel.com"); MODULE_DESCRIPTION("SSC driver for Atmel AVR32 and AT91");
On 10/16/2012 05:56 AM, Bo Shen :
This patch removes some code duplication by using module_platform_driver
Signed-off-by: Bo Shen voice.shen@atmel.com
Very good simplification but...
drivers/misc/atmel-ssc.c | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-)
diff --git a/drivers/misc/atmel-ssc.c b/drivers/misc/atmel-ssc.c index 23dcb76..ac00f83 100644 --- a/drivers/misc/atmel-ssc.c +++ b/drivers/misc/atmel-ssc.c @@ -68,7 +68,7 @@ void ssc_free(struct ssc_device *ssc) } EXPORT_SYMBOL(ssc_free);
-static int __init ssc_probe(struct platform_device *pdev) +static int ssc_probe(struct platform_device *pdev)
Here you remove the __init altogether, maybe converting to __devinit is the proper replacement for this? I do not know myself but if anybody knows?
Bye,
{ struct resource *regs; struct ssc_device *ssc; @@ -135,24 +135,14 @@ static int __devexit ssc_remove(struct platform_device *pdev) }
static struct platform_driver ssc_driver = {
- .remove = __devexit_p(ssc_remove), .driver = { .name = "ssc", .owner = THIS_MODULE, },
- .probe = ssc_probe,
- .remove = __devexit_p(ssc_remove),
};
-static int __init ssc_init(void) -{
- return platform_driver_probe(&ssc_driver, ssc_probe);
-} -module_init(ssc_init);
-static void __exit ssc_exit(void) -{
- platform_driver_unregister(&ssc_driver);
-} -module_exit(ssc_exit); +module_platform_driver(ssc_driver);
MODULE_AUTHOR("Hans-Christian Egtvedt hcegtvedt@atmel.com"); MODULE_DESCRIPTION("SSC driver for Atmel AVR32 and AT91");
On 10:07 Tue 16 Oct , Nicolas Ferre wrote:
On 10/16/2012 05:56 AM, Bo Shen :
This patch removes some code duplication by using module_platform_driver
Signed-off-by: Bo Shen voice.shen@atmel.com
Very good simplification but...
drivers/misc/atmel-ssc.c | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-)
diff --git a/drivers/misc/atmel-ssc.c b/drivers/misc/atmel-ssc.c index 23dcb76..ac00f83 100644 --- a/drivers/misc/atmel-ssc.c +++ b/drivers/misc/atmel-ssc.c @@ -68,7 +68,7 @@ void ssc_free(struct ssc_device *ssc) } EXPORT_SYMBOL(ssc_free);
-static int __init ssc_probe(struct platform_device *pdev) +static int ssc_probe(struct platform_device *pdev)
Here you remove the __init altogether, maybe converting to __devinit is the proper replacement for this? I do not know myself but if anybody knows?
yes __devinit is mandatory
Best Regards, J.
On 10/16/2012 16:49, Jean-Christophe PLAGNIOL-VILLARD wrote:
On 10:07 Tue 16 Oct , Nicolas Ferre wrote:
On 10/16/2012 05:56 AM, Bo Shen :
This patch removes some code duplication by using module_platform_driver
Signed-off-by: Bo Shen voice.shen@atmel.com
Very good simplification but...
drivers/misc/atmel-ssc.c | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-)
diff --git a/drivers/misc/atmel-ssc.c b/drivers/misc/atmel-ssc.c index 23dcb76..ac00f83 100644 --- a/drivers/misc/atmel-ssc.c +++ b/drivers/misc/atmel-ssc.c @@ -68,7 +68,7 @@ void ssc_free(struct ssc_device *ssc) } EXPORT_SYMBOL(ssc_free);
-static int __init ssc_probe(struct platform_device *pdev) +static int ssc_probe(struct platform_device *pdev)
Here you remove the __init altogether, maybe converting to __devinit is the proper replacement for this? I do not know myself but if anybody knows?
yes __devinit is mandatory
I will add a small patch to fix this.
Thanks.
BRs, Bo Shen
Best Regards, J.
On Tue, Oct 16, 2012 at 10:49:47AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
On 10:07 Tue 16 Oct , Nicolas Ferre wrote:
On 10/16/2012 05:56 AM, Bo Shen :
-static int __init ssc_probe(struct platform_device *pdev) +static int ssc_probe(struct platform_device *pdev)
Here you remove the __init altogether, maybe converting to __devinit is the proper replacement for this? I do not know myself but if anybody knows?
yes __devinit is mandatory
No it isn't. __init is plain buggy. __devinit is better, but there's plans to remove all __devinit from the kernel. Having nothing there will make GregKH's life a lot easier when he does come to remove __devinit.
On Tue, Oct 16, 2012 at 5:56 AM, Bo Shen voice.shen@atmel.com wrote:
This patch removes some code duplication by using module_platform_driver
Signed-off-by: Bo Shen voice.shen@atmel.com
drivers/misc/atmel-ssc.c | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-)
diff --git a/drivers/misc/atmel-ssc.c b/drivers/misc/atmel-ssc.c index 23dcb76..ac00f83 100644 --- a/drivers/misc/atmel-ssc.c +++ b/drivers/misc/atmel-ssc.c @@ -68,7 +68,7 @@ void ssc_free(struct ssc_device *ssc) } EXPORT_SYMBOL(ssc_free);
-static int __init ssc_probe(struct platform_device *pdev) +static int ssc_probe(struct platform_device *pdev) { struct resource *regs; struct ssc_device *ssc; @@ -135,24 +135,14 @@ static int __devexit ssc_remove(struct platform_device *pdev) }
static struct platform_driver ssc_driver = {
.remove = __devexit_p(ssc_remove), .driver = { .name = "ssc", .owner = THIS_MODULE, },
.probe = ssc_probe,.remove = __devexit_p(ssc_remove),};
-static int __init ssc_init(void) -{
return platform_driver_probe(&ssc_driver, ssc_probe);-} -module_init(ssc_init);
-static void __exit ssc_exit(void) -{
platform_driver_unregister(&ssc_driver);-} -module_exit(ssc_exit); +module_platform_driver(ssc_driver);
MODULE_AUTHOR("Hans-Christian Egtvedt hcegtvedt@atmel.com"); MODULE_DESCRIPTION("SSC driver for Atmel AVR32 and AT91"); -- 1.7.9.5
Maybe the module_platform_driver isn't a substitute of platform_driver_probe, because module_platform_driver use platform_driver_register/unregister. Using that macro we lose the advantage of platform_driver_probe, as stated in:
https://lkml.org/lkml/2012/1/10/335 On Tue, Jan 10, 2012 at 21:47, Geert Uytterhoeven geert@linux-m68k.org wrote:
[...] Still, setting up platform_driver.probe and removing __init from all probe functions is not the right thing to do, as this make (non-__init) kernel code size bigger, while none of these devices are hotpluggable and thus cannot appear after bootup. That's why we have platform_driver_probe() in the first place. So I think all of this should be reverted for non-hotpluggable drivers. [...]
What do you think?
Best regards
On 10/16/2012 05:56 AM, Bo Shen :
Using the devm_xxx() managed function to stripdown the error and remove code.
Signed-off-by: Bo Shen voice.shen@atmel.com
Maybe too late, but:
Acked-by: Nicolas Ferre nicolas.ferre@atmel.com
drivers/misc/atmel-ssc.c | 47 +++++++++++++++------------------------------- 1 file changed, 15 insertions(+), 32 deletions(-)
diff --git a/drivers/misc/atmel-ssc.c b/drivers/misc/atmel-ssc.c index 5bb1877..23dcb76 100644 --- a/drivers/misc/atmel-ssc.c +++ b/drivers/misc/atmel-ssc.c @@ -70,37 +70,33 @@ EXPORT_SYMBOL(ssc_free);
static int __init ssc_probe(struct platform_device *pdev) {
int retval = 0; struct resource *regs; struct ssc_device *ssc;
ssc = kzalloc(sizeof(struct ssc_device), GFP_KERNEL);
- ssc = devm_kzalloc(&pdev->dev, sizeof(struct ssc_device), GFP_KERNEL); if (!ssc) { dev_dbg(&pdev->dev, "out of memory\n");
retval = -ENOMEM;goto out;
return -ENOMEM;}
ssc->pdev = pdev;
regs = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (!regs) { dev_dbg(&pdev->dev, "no mmio resource defined\n");
retval = -ENXIO;goto out_free;
return -ENXIO;
- ssc->clk = clk_get(&pdev->dev, "pclk");
- if (IS_ERR(ssc->clk)) {
dev_dbg(&pdev->dev, "no pclk clock defined\n");retval = -ENXIO;goto out_free;- }
- ssc->pdev = pdev;
- ssc->regs = ioremap(regs->start, resource_size(regs));
- ssc->regs = devm_request_and_ioremap(&pdev->dev, regs); if (!ssc->regs) { dev_dbg(&pdev->dev, "ioremap failed\n");
retval = -EINVAL;goto out_clk;
return -EINVAL;}
ssc->clk = devm_clk_get(&pdev->dev, "pclk");
if (IS_ERR(ssc->clk)) {
dev_dbg(&pdev->dev, "no pclk clock defined\n");return -ENXIO;}
/* disable all interrupts */
@@ -112,8 +108,7 @@ static int __init ssc_probe(struct platform_device *pdev) ssc->irq = platform_get_irq(pdev, 0); if (!ssc->irq) { dev_dbg(&pdev->dev, "could not get irq\n");
retval = -ENXIO;goto out_unmap;
return -ENXIO;}
spin_lock(&user_lock);
@@ -125,16 +120,7 @@ static int __init ssc_probe(struct platform_device *pdev) dev_info(&pdev->dev, "Atmel SSC device at 0x%p (irq %d)\n", ssc->regs, ssc->irq);
- goto out;
-out_unmap:
- iounmap(ssc->regs);
-out_clk:
- clk_put(ssc->clk);
-out_free:
- kfree(ssc);
-out:
- return retval;
- return 0;
}
static int __devexit ssc_remove(struct platform_device *pdev) @@ -142,10 +128,7 @@ static int __devexit ssc_remove(struct platform_device *pdev) struct ssc_device *ssc = platform_get_drvdata(pdev);
spin_lock(&user_lock);
iounmap(ssc->regs);
clk_put(ssc->clk); list_del(&ssc->list);
kfree(ssc); spin_unlock(&user_lock);
return 0;
participants (6)
-
Bo Shen -
Fabio Porcedda -
Jean-Christophe PLAGNIOL-VILLARD -
Mark Brown -
Nicolas Ferre -
Russell King - ARM Linux