[PATCH 1/2] ASoC: atmel-ssc: use devm_xxx() managed function

[PATCH 1/2] ASoC: atmel-ssc: use devm_xxx() managed function

[Bo Shen]

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

[PATCH 2/2] ASoC: atmel-ssc: use module_platform_driver macro

[Bo Shen]

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

[PATCH 2/2] ASoC: atmel-ssc: use module_platform_driver macro

[Nicolas Ferre]

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

[PATCH 2/2] ASoC: atmel-ssc: use module_platform_driver macro

[Jean-Christophe PLAGNIOL-VILLARD]

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.

[PATCH 2/2] ASoC: atmel-ssc: use module_platform_driver macro

[Bo Shen]

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.
>

[PATCH 2/2] ASoC: atmel-ssc: use module_platform_driver macro

[Russell King - ARM Linux]

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.

[PATCH 2/2] ASoC: atmel-ssc: use module_platform_driver macro

[Fabio Porcedda]

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

[PATCH 1/2] ASoC: atmel-ssc: use devm_xxx() managed function

[Mark Brown]

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.

[PATCH 1/2] ASoC: atmel-ssc: use devm_xxx() managed function

[Nicolas Ferre]

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