Re: [PATCH] AsoC/s6000: use resource_size for {request/release}_mem_region and ioremap

Re: [PATCH] AsoC/s6000: use resource_size for {request/release}_mem_region and ioremap

[Mark Brown]

On Wed, Jun 02, 2010 at 06:49:53PM +0800, Wan ZongShun wrote:
> The size calculation is end - start + 1. But,sometimes, the '1' can
> be forgotten carelessly, witch will have potential risk, so use resource_size
> for {request/release}_mem_region and ioremap here should be good habit.
> 
> Signed-off-by: Wan ZongShun <mcuos.com@gmail.com>

CCing in Daniel who wrote the S6000 support.  Daniel, do you want to add
an entry to MAINTAINERS for this stuff or are you happy without having
stuff run past you?

> ---
>  sound/soc/s6000/s6000-i2s.c |   38 +++++++++++++++++---------------------
>  1 files changed, 17 insertions(+), 21 deletions(-)
> 
> diff --git a/sound/soc/s6000/s6000-i2s.c b/sound/soc/s6000/s6000-i2s.c
> index 5b9ac17..59e3fa7 100644
> --- a/sound/soc/s6000/s6000-i2s.c
> +++ b/sound/soc/s6000/s6000-i2s.c
> @@ -451,16 +451,15 @@ static int __devinit s6000_i2s_probe(struct platform_device *pdev)
>  		goto err_release_none;
>  	}
> 
> -	region = request_mem_region(scbmem->start,
> -				    scbmem->end - scbmem->start + 1,
> -				    pdev->name);
> +	region = request_mem_region(scbmem->start, resource_size(scbmem),
> +								pdev->name);
>  	if (!region) {
>  		dev_err(&pdev->dev, "I2S SCB region already claimed\n");
>  		ret = -EBUSY;
>  		goto err_release_none;
>  	}
> 
> -	mmio = ioremap(scbmem->start, scbmem->end - scbmem->start + 1);
> +	mmio = ioremap(scbmem->start, resource_size(scbmem));
>  	if (!mmio) {
>  		dev_err(&pdev->dev, "can't ioremap SCB region\n");
>  		ret = -ENOMEM;
> @@ -474,9 +473,8 @@ static int __devinit s6000_i2s_probe(struct platform_device *pdev)
>  		goto err_release_map;
>  	}
> 
> -	region = request_mem_region(sifmem->start,
> -				    sifmem->end - sifmem->start + 1,
> -				    pdev->name);
> +	region = request_mem_region(sifmem->start, resource_size(sifmem),
> +								pdev->name);
>  	if (!region) {
>  		dev_err(&pdev->dev, "I2S SIF region already claimed\n");
>  		ret = -EBUSY;
> @@ -490,8 +488,8 @@ static int __devinit s6000_i2s_probe(struct platform_device *pdev)
>  		goto err_release_sif;
>  	}
> 
> -	region = request_mem_region(dma1->start, dma1->end - dma1->start + 1,
> -				    pdev->name);
> +	region = request_mem_region(dma1->start, resource_size(dma1),
> +								pdev->name);
>  	if (!region) {
>  		dev_err(&pdev->dev, "I2S DMA region already claimed\n");
>  		ret = -EBUSY;
> @@ -500,9 +498,8 @@ static int __devinit s6000_i2s_probe(struct platform_device *pdev)
> 
>  	dma2 = platform_get_resource(pdev, IORESOURCE_DMA, 1);
>  	if (dma2) {
> -		region = request_mem_region(dma2->start,
> -					    dma2->end - dma2->start + 1,
> -					    pdev->name);
> +		region = request_mem_region(dma2->start, resource_size(dma2),
> +								pdev->name);
>  		if (!region) {
>  			dev_err(&pdev->dev,
>  				"I2S DMA region already claimed\n");
> @@ -561,15 +558,15 @@ err_release_dev:
>  	kfree(dev);
>  err_release_dma2:
>  	if (dma2)
> -		release_mem_region(dma2->start, dma2->end - dma2->start + 1);
> +		release_mem_region(dma2->start, resource_size(dma2));
>  err_release_dma1:
> -	release_mem_region(dma1->start, dma1->end - dma1->start + 1);
> +	release_mem_region(dma1->start, resource_size(dma1));
>  err_release_sif:
> -	release_mem_region(sifmem->start, (sifmem->end - sifmem->start) + 1);
> +	release_mem_region(sifmem->start, resource_size(sifmem));
>  err_release_map:
>  	iounmap(mmio);
>  err_release_scb:
> -	release_mem_region(scbmem->start, (scbmem->end - scbmem->start) + 1);
> +	release_mem_region(scbmem->start, resource_size(scbmem));
>  err_release_none:
>  	return ret;
>  }
> @@ -590,19 +587,18 @@ static void __devexit s6000_i2s_remove(struct platform_device *pdev)
>  	kfree(dev);
> 
>  	region = platform_get_resource(pdev, IORESOURCE_DMA, 0);
> -	release_mem_region(region->start, region->end - region->start + 1);
> +	release_mem_region(region->start, resource_size(region));
> 
>  	region = platform_get_resource(pdev, IORESOURCE_DMA, 1);
>  	if (region)
> -		release_mem_region(region->start,
> -				   region->end - region->start + 1);
> +		release_mem_region(region->start, resource_size(region));
> 
>  	region = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	release_mem_region(region->start, (region->end - region->start) + 1);
> +	release_mem_region(region->start, resource_size(region));
> 
>  	iounmap(mmio);
>  	region = platform_get_resource(pdev, IORESOURCE_IO, 0);
> -	release_mem_region(region->start, (region->end - region->start) + 1);
> +	release_mem_region(region->start, resource_size(region));
>  }
> 
>  static struct platform_driver s6000_i2s_driver = {
> -- 
> 1.6.3.3

Re: [PATCH] AsoC/s6000: use resource_size for {request/release}_mem_region and ioremap

[Daniel Glöckner]

Acked-by: Daniel Glöckner <dg@emlix.com>

On 06/02/2010 12:51 PM, Mark Brown wrote:
> CCing in Daniel who wrote the S6000 support.  Daniel, do you want to add
> an entry to MAINTAINERS for this stuff or are you happy without having
> stuff run past you?

I don't think it is necessary to add an entry to MAINTAINERS.
My name and address listed at the top of the file should be enough.
It's not like I'm still working on it, but if needed I can take the
device from the shelf to perform some tests, should anyone make more
serious changes to the driver.

  Daniel


-- 
Dipl.-Math. Daniel Glöckner, emlix GmbH, http://www.emlix.com
Fon +49 551 30664-0, Fax -11, Bahnhofsallee 1b, 37081 Göttingen, Germany
Sitz der Gesellschaft: Göttingen, Amtsgericht Göttingen HR B 3160
Geschäftsführer: Dr. Uwe Kracke, Ust-IdNr.: DE 205 198 055

emlix - your embedded linux partner

Re: [PATCH] AsoC/s6000: use resource_size for {request/release}_mem_region and ioremap

[Mark Brown]

On Wed, Jun 02, 2010 at 02:35:18PM +0200, Daniel Gl?ckner wrote:

> I don't think it is necessary to add an entry to MAINTAINERS.
> My name and address listed at the top of the file should be enough.
> It's not like I'm still working on it, but if needed I can take the
> device from the shelf to perform some tests, should anyone make more
> serious changes to the driver.

MAINTAINERS is used by automatic tools like get_maintainer to find who
to CC so entries there are still useful - original authorship is often
not a good indicator that someone is willing to take responsibility for
the code.  From the sounds of it though it was for a particular project
and you've moved on from it so you're not particularly concerned about
now so it's not worth it anyway?

[PATCH] MAINTAINERS: add entry for s6000 sound driver

[Daniel Glöckner]

Signed-off-by: Daniel Glöckner <dg@emlix.com>
---
 MAINTAINERS |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2652ebc..c650d48 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4893,6 +4893,12 @@ L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
 S:	Supported
 F:	drivers/mmc/host/s3cmci.*
 
+S6000 SOUND DRIVER
+M:	Daniel Glöckner <dg@emlix.com>
+L:	alsa-devel@alsa-project.org (moderated for non-subscribers)
+S:	Odd Fixes
+F:	sound/soc/s6000/
+
 SAA7146 VIDEO4LINUX-2 DRIVER
 M:	Michael Hunold <michael@mihu.de>
 L:	linux-media@vger.kernel.org
-- 
1.6.1.3

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] MAINTAINERS: add entry for s6000 sound driver

[Liam Girdwood]

On Wed, 2010-06-02 at 15:35 +0200, Daniel Glöckner wrote:
> Signed-off-by: Daniel Glöckner <dg@emlix.com>
> ---
>  MAINTAINERS |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2652ebc..c650d48 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4893,6 +4893,12 @@ L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
>  S:	Supported
>  F:	drivers/mmc/host/s3cmci.*
>  
> +S6000 SOUND DRIVER
> +M:	Daniel Glöckner <dg@emlix.com>
> +L:	alsa-devel@alsa-project.org (moderated for non-subscribers)
> +S:	Odd Fixes
> +F:	sound/soc/s6000/
> +
>  SAA7146 VIDEO4LINUX-2 DRIVER
>  M:	Michael Hunold <michael@mihu.de>
>  L:	linux-media@vger.kernel.org

Acked-by: Liam Girdwood <lrg@slimlogic.co.uk>
-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] MAINTAINERS: add entry for s6000 sound driver

[Mark Brown]

On Wed, Jun 02, 2010 at 03:45:02PM +0100, Liam Girdwood wrote:

> Acked-by: Liam Girdwood <lrg@slimlogic.co.uk>

Applied, thanks.

Re: [PATCH] AsoC/s6000: use resource_size for {request/release}_mem_region and ioremap

[Wan ZongShun]

2010/6/2 Mark Brown <broonie@opensource.wolfsonmicro.com>:
> On Wed, Jun 02, 2010 at 02:35:18PM +0200, Daniel Gl?ckner wrote:
>
>> I don't think it is necessary to add an entry to MAINTAINERS.
>> My name and address listed at the top of the file should be enough.
>> It's not like I'm still working on it, but if needed I can take the
>> device from the shelf to perform some tests, should anyone make more
>> serious changes to the driver.
>
> MAINTAINERS is used by automatic tools like get_maintainer to find who
> to CC so entries there are still useful - original authorship is often
> not a good indicator that someone is willing to take responsibility for
> the code.  From the sounds of it though it was for a particular project
> and you've moved on from it so you're not particularly concerned about
> now so it's not worth it anyway?

Oh, it just be my worry.
If some codes were no longer useful or valuable, what should we do for them?
delete them or keep them be there yet?

If I my patch to fix a worthless codes, everything I do will be nothing.

>



-- 
*linux-arm-kernel mailing list
mail addr:linux-arm-kernel@lists.infradead.org
you can subscribe by:
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

* linux-arm-NUC900 mailing list
mail addr:NUC900@googlegroups.com
main web: https://groups.google.com/group/NUC900
you can subscribe it by sending me mail:
mcuos.com@gmail.com
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] AsoC/s6000: use resource_size for {request/release}_mem_region and ioremap

[Mark Brown]

On Wed, Jun 02, 2010 at 09:40:13PM +0800, Wan ZongShun wrote:

> Oh, it just be my worry.
> If some codes were no longer useful or valuable, what should we do for them?
> delete them or keep them be there yet?

> If I my patch to fix a worthless codes, everything I do will be nothing.

The hardware is still very much live, it's just that Daniel isn't
working on it.

Re: [PATCH] AsoC/s6000: use resource_size for {request/release}_mem_region and ioremap

[Wan ZongShun]

2010/6/2 Mark Brown <broonie@opensource.wolfsonmicro.com>:
> On Wed, Jun 02, 2010 at 09:40:13PM +0800, Wan ZongShun wrote:
>
>> Oh, it just be my worry.
>> If some codes were no longer useful or valuable, what should we do for them?
>> delete them or keep them be there yet?
>
>> If I my patch to fix a worthless codes, everything I do will be nothing.
>
> The hardware is still very much live, it's just that Daniel isn't
> working on it.

Of course, the s6000 is live, but we cannot indentify whether other
hardwares still live or not in all Linux tree.:)

>



-- 
*linux-arm-kernel mailing list
mail addr:linux-arm-kernel@lists.infradead.org
you can subscribe by:
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

* linux-arm-NUC900 mailing list
mail addr:NUC900@googlegroups.com
main web: https://groups.google.com/group/NUC900
you can subscribe it by sending me mail:
mcuos.com@gmail.com

Re: [PATCH] AsoC/s6000: use resource_size for {request/release}_mem_region and ioremap

[Mark Brown]

On Wed, Jun 02, 2010 at 02:35:18PM +0200, Daniel Glöckner wrote:
> Acked-by: Daniel Glöckner <dg@emlix.com>

Applied, thanks.