* [PATCH 1/2] ASoC: atmel-ssc: use devm_xxx() managed function
@ 2012-10-16 3:56 Bo Shen
2012-10-16 3:56 ` [PATCH 2/2] ASoC: atmel-ssc: use module_platform_driver macro Bo Shen
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Bo Shen @ 2012-10-16 3:56 UTC (permalink / raw)
To: linux-arm-kernel
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;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] ASoC: atmel-ssc: use module_platform_driver macro
2012-10-16 3:56 [PATCH 1/2] ASoC: atmel-ssc: use devm_xxx() managed function Bo Shen
@ 2012-10-16 3:56 ` Bo Shen
2012-10-16 8:07 ` Nicolas Ferre
2012-10-16 9:32 ` Fabio Porcedda
2012-10-16 5:12 ` [PATCH 1/2] ASoC: atmel-ssc: use devm_xxx() managed function Mark Brown
2012-10-16 8:05 ` Nicolas Ferre
2 siblings, 2 replies; 9+ messages in thread
From: Bo Shen @ 2012-10-16 3:56 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 1/2] ASoC: atmel-ssc: use devm_xxx() managed function
2012-10-16 3:56 [PATCH 1/2] ASoC: atmel-ssc: use devm_xxx() managed function Bo Shen
2012-10-16 3:56 ` [PATCH 2/2] ASoC: atmel-ssc: use module_platform_driver macro Bo Shen
@ 2012-10-16 5:12 ` Mark Brown
2012-10-16 8:05 ` Nicolas Ferre
2 siblings, 0 replies; 9+ messages in thread
From: Mark Brown @ 2012-10-16 5:12 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Oct 16, 2012 at 11:56:58AM +0800, Bo Shen wrote:
> Using the devm_xxx() managed function to stripdown the error
> and remove code.
Applied both, thanks.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] ASoC: atmel-ssc: use devm_xxx() managed function
2012-10-16 3:56 [PATCH 1/2] ASoC: atmel-ssc: use devm_xxx() managed function Bo Shen
2012-10-16 3:56 ` [PATCH 2/2] ASoC: atmel-ssc: use module_platform_driver macro Bo Shen
2012-10-16 5:12 ` [PATCH 1/2] ASoC: atmel-ssc: use devm_xxx() managed function Mark Brown
@ 2012-10-16 8:05 ` Nicolas Ferre
2 siblings, 0 replies; 9+ messages in thread
From: Nicolas Ferre @ 2012-10-16 8:05 UTC (permalink / raw)
To: linux-arm-kernel
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;
>
--
Nicolas Ferre
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] ASoC: atmel-ssc: use module_platform_driver macro
2012-10-16 3:56 ` [PATCH 2/2] ASoC: atmel-ssc: use module_platform_driver macro Bo Shen
@ 2012-10-16 8:07 ` Nicolas Ferre
2012-10-16 8:49 ` Jean-Christophe PLAGNIOL-VILLARD
2012-10-16 9:32 ` Fabio Porcedda
1 sibling, 1 reply; 9+ messages in thread
From: Nicolas Ferre @ 2012-10-16 8:07 UTC (permalink / raw)
To: linux-arm-kernel
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");
>
--
Nicolas Ferre
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] ASoC: atmel-ssc: use module_platform_driver macro
2012-10-16 8:07 ` Nicolas Ferre
@ 2012-10-16 8:49 ` Jean-Christophe PLAGNIOL-VILLARD
2012-10-16 8:59 ` Bo Shen
2012-10-16 9:07 ` Russell King - ARM Linux
0 siblings, 2 replies; 9+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-10-16 8:49 UTC (permalink / raw)
To: linux-arm-kernel
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] ASoC: atmel-ssc: use module_platform_driver macro
2012-10-16 8:49 ` Jean-Christophe PLAGNIOL-VILLARD
@ 2012-10-16 8:59 ` Bo Shen
2012-10-16 9:07 ` Russell King - ARM Linux
1 sibling, 0 replies; 9+ messages in thread
From: Bo Shen @ 2012-10-16 8:59 UTC (permalink / raw)
To: linux-arm-kernel
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.
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] ASoC: atmel-ssc: use module_platform_driver macro
2012-10-16 8:49 ` Jean-Christophe PLAGNIOL-VILLARD
2012-10-16 8:59 ` Bo Shen
@ 2012-10-16 9:07 ` Russell King - ARM Linux
1 sibling, 0 replies; 9+ messages in thread
From: Russell King - ARM Linux @ 2012-10-16 9:07 UTC (permalink / raw)
To: linux-arm-kernel
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] ASoC: atmel-ssc: use module_platform_driver macro
2012-10-16 3:56 ` [PATCH 2/2] ASoC: atmel-ssc: use module_platform_driver macro Bo Shen
2012-10-16 8:07 ` Nicolas Ferre
@ 2012-10-16 9:32 ` Fabio Porcedda
1 sibling, 0 replies; 9+ messages in thread
From: Fabio Porcedda @ 2012-10-16 9:32 UTC (permalink / raw)
To: linux-arm-kernel
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
--
Fabio Porcedda
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2012-10-16 9:32 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-16 3:56 [PATCH 1/2] ASoC: atmel-ssc: use devm_xxx() managed function Bo Shen
2012-10-16 3:56 ` [PATCH 2/2] ASoC: atmel-ssc: use module_platform_driver macro Bo Shen
2012-10-16 8:07 ` Nicolas Ferre
2012-10-16 8:49 ` Jean-Christophe PLAGNIOL-VILLARD
2012-10-16 8:59 ` Bo Shen
2012-10-16 9:07 ` Russell King - ARM Linux
2012-10-16 9:32 ` Fabio Porcedda
2012-10-16 5:12 ` [PATCH 1/2] ASoC: atmel-ssc: use devm_xxx() managed function Mark Brown
2012-10-16 8:05 ` Nicolas Ferre
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).