Re: [alsa-devel] [PATCH] AsoC/s6000: use resource_size for {request/release}_mem_region and ioremap
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);
- 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);@@ -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);@@ -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);@@ -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
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
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?
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
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
On Wed, Jun 02, 2010 at 03:45:02PM +0100, Liam Girdwood wrote:
Acked-by: Liam Girdwood lrg@slimlogic.co.uk
Applied, thanks.
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.
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.
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.:)
On Wed, Jun 02, 2010 at 02:35:18PM +0200, Daniel Glöckner wrote:
Acked-by: Daniel Glöckner dg@emlix.com
Applied, thanks.
participants (4)
-
Daniel Glöckner -
Liam Girdwood -
Mark Brown -
Wan ZongShun