[PATCH] pci: use pci_ioremap_bar() in drivers/ide

[PATCH] pci: use pci_ioremap_bar() in drivers/ide

[Arjan van de Ven]

>From 9233d7e38207749586c47410b400afcf3e7e59fb Mon Sep 17 00:00:00 2001
From: Arjan van de Ven <arjan@linux.intel.com>
Date: Sun, 28 Sep 2008 16:14:10 -0700
Subject: [PATCH] pci: use pci_ioremap_bar() in drivers/ide

Use the newly introduced pci_ioremap_bar() function in drivers/ide.
pci_ioremap_bar() just takes a pci device and a bar number, with the goal
of making it really hard to get wrong, while also having a central place
to stick sanity checks.

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
 drivers/ide/pci/sgiioc4.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/ide/pci/sgiioc4.c b/drivers/ide/pci/sgiioc4.c
index 8af9b23..9233217 100644
--- a/drivers/ide/pci/sgiioc4.c
+++ b/drivers/ide/pci/sgiioc4.c
@@ -574,7 +574,7 @@ sgiioc4_ide_setup_pci_device(struct pci_dev *dev)
 
 	/*  Get the CmdBlk and CtrlBlk Base Registers */
 	bar0 = pci_resource_start(dev, 0);
-	virt_base = ioremap(bar0, pci_resource_len(dev, 0));
+	virt_base = pci_ioremap_bar(dev, 0);
 	if (virt_base == NULL) {
 		printk(KERN_ERR "%s: Unable to remap BAR 0 address: 0x%lx\n",
 				DRV_NAME, bar0);
-- 
1.5.5.1



-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [PATCH] pci: use pci_ioremap_bar() in drivers/ide

[Sergei Shtylyov]

Hello.

Arjan van de Ven wrote:

> Use the newly introduced pci_ioremap_bar() function in drivers/ide.
> pci_ioremap_bar() just takes a pci device and a bar number, with the goal
> of making it really hard to get wrong, while also having a central place
> to stick sanity checks.
>
> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>

Acked-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>

MBR, Sergei

Re: [PATCH] pci: use pci_ioremap_bar() in drivers/ide

[Sergei Shtylyov]

Hello, I wrote:

>> Use the newly introduced pci_ioremap_bar() function in drivers/ide.
>> pci_ioremap_bar() just takes a pci device and a bar number, with the goal
>> of making it really hard to get wrong, while also having a central place
>> to stick sanity checks.

>> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>

> Acked-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>

    No, I'm takign back my ACK.
    Since the patch intends to address all drivers/ide/, it's clearly no 
compolete -- siimage.c and scc_pata.c beg for the alike change (it will even 
permit to kill 4 local variables in the latter driver).

MBR, Sergei

Re: [PATCH] pci: use pci_ioremap_bar() in drivers/ide

[Arjan van de Ven]

On Wed, 22 Oct 2008 21:40:37 +0400
Sergei Shtylyov <sshtylyov@ru.mvista.com> wrote:

> Hello, I wrote:
> 
> >> Use the newly introduced pci_ioremap_bar() function in drivers/ide.
> >> pci_ioremap_bar() just takes a pci device and a bar number, with
> >> the goal of making it really hard to get wrong, while also having
> >> a central place to stick sanity checks.
> 
> >> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
> 
> > Acked-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
> 
>     No, I'm takign back my ACK.
>     Since the patch intends to address all drivers/ide/, it's clearly
> no compolete -- siimage.c and scc_pata.c beg for the alike change (it
> will even permit to kill 4 local variables in the latter driver).
> 
> MBR, Sergei

I'll be happy to take a second pass, but "doesn't convert everyone"
shouldn't really be a reason to do the first pass ;-)

-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [PATCH] pci: use pci_ioremap_bar() in drivers/ide

[Sergei Shtylyov]

Arjan van de Ven wrote:

>>>>Use the newly introduced pci_ioremap_bar() function in drivers/ide.
>>>>pci_ioremap_bar() just takes a pci device and a bar number, with
>>>>the goal of making it really hard to get wrong, while also having
>>>>a central place to stick sanity checks.

>>>>Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>

>>>Acked-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>

>>    No, I'm takign back my ACK.
>>    Since the patch intends to address all drivers/ide/, it's clearly
>>no compolete -- siimage.c and scc_pata.c beg for the alike change (it
>>will even permit to kill 4 local variables in the latter driver).

> I'll be happy to take a second pass, but "doesn't convert everyone"
> shouldn't really be a reason to do the first pass ;-)

    To not do, you mean?
    If you're going to leave this patch as it, it's worth to change the 
subject/header reflecting a modest goal of doing this only for one driver. ;-)

MBR, Sergei

Re: [PATCH] pci: use pci_ioremap_bar() in drivers/ide

[Arjan van de Ven]

On Wed, 22 Oct 2008 21:40:37 +0400
Sergei Shtylyov <sshtylyov@ru.mvista.com> wrote:

> Hello, I wrote:
> 
> >> Use the newly introduced pci_ioremap_bar() function in drivers/ide.
> >> pci_ioremap_bar() just takes a pci device and a bar number, with
> >> the goal of making it really hard to get wrong, while also having
> >> a central place to stick sanity checks.
> 
> >> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
> 
> > Acked-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
> 
>     No, I'm takign back my ACK.
>     Since the patch intends to address all drivers/ide/, it's clearly
> no compolete -- siimage.c and scc_pata.c beg for the alike change (it
> will even permit to kill 4 local variables in the latter driver).

like this ?

>From 3f88a6936f7802a5d4af6c54a1ef353d14ff1ab2 Mon Sep 17 00:00:00 2001
From: Arjan van de Ven <arjan@linux.intel.com>
Date: Wed, 22 Oct 2008 13:21:07 -0700
Subject: [PATCH] ide: two more pci_ioremap_bar() conversions

based on suggestion from Sergei Shtylyov, there are two more places
where using pci_ioremap_bar() makes sense.

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
 drivers/ide/pci/scc_pata.c |   10 ++++------
 drivers/ide/pci/siimage.c  |    2 +-
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/ide/pci/scc_pata.c b/drivers/ide/pci/scc_pata.c
index 9ce1d80..b4be295 100644
--- a/drivers/ide/pci/scc_pata.c
+++ b/drivers/ide/pci/scc_pata.c
@@ -536,10 +536,6 @@ static u8 scc_udma_filter(ide_drive_t *drive)
 
 static int setup_mmio_scc (struct pci_dev *dev, const char *name)
 {
-	unsigned long ctl_base = pci_resource_start(dev, 0);
-	unsigned long dma_base = pci_resource_start(dev, 1);
-	unsigned long ctl_size = pci_resource_len(dev, 0);
-	unsigned long dma_size = pci_resource_len(dev, 1);
 	void __iomem *ctl_addr;
 	void __iomem *dma_addr;
 	int i, ret;
@@ -557,10 +553,12 @@ static int setup_mmio_scc (struct pci_dev *dev, const char *name)
 		return ret;
 	}
 
-	if ((ctl_addr = ioremap(ctl_base, ctl_size)) == NULL)
+	ctl_addr = pci_ioremap_bar(dev, 0);
+	if (!ctl_addr)
 		goto fail_0;
 
-	if ((dma_addr = ioremap(dma_base, dma_size)) == NULL)
+	dma_addr = pci_ioremap_bar(dev, 1);
+	if (!dma_addr)
 		goto fail_1;
 
 	pci_set_master(dev);
diff --git a/drivers/ide/pci/siimage.c b/drivers/ide/pci/siimage.c
index eb4faf9..c3107df 100644
--- a/drivers/ide/pci/siimage.c
+++ b/drivers/ide/pci/siimage.c
@@ -784,7 +784,7 @@ static int __devinit siimage_init_one(struct pci_dev *dev,
 			printk(KERN_WARNING DRV_NAME " %s: MMIO ports not "
 				"available\n", pci_name(dev));
 		} else {
-			ioaddr = ioremap(bar5, barsize);
+			ioaddr = pci_ioremap_bar(dev, 5);
 			if (ioaddr == NULL)
 				release_mem_region(bar5, barsize);
 		}
-- 
1.5.5.1



-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [PATCH] pci: use pci_ioremap_bar() in drivers/ide

[Sergei Shtylyov]

Arjan van de Ven wrote:

>>>>Use the newly introduced pci_ioremap_bar() function in drivers/ide.
>>>>pci_ioremap_bar() just takes a pci device and a bar number, with
>>>>the goal of making it really hard to get wrong, while also having
>>>>a central place to stick sanity checks.

>>>>Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>

>>>Acked-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>

>>    No, I'm takign back my ACK.
>>    Since the patch intends to address all drivers/ide/, it's clearly
>>no compolete -- siimage.c and scc_pata.c beg for the alike change (it
>>will even permit to kill 4 local variables in the latter driver).

> like this ?

    Exactly.

> From 3f88a6936f7802a5d4af6c54a1ef353d14ff1ab2 Mon Sep 17 00:00:00 2001
> From: Arjan van de Ven <arjan@linux.intel.com>
> Date: Wed, 22 Oct 2008 13:21:07 -0700
> Subject: [PATCH] ide: two more pci_ioremap_bar() conversions

> based on suggestion from Sergei Shtylyov, there are two more places
> where using pci_ioremap_bar() makes sense.

> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>

Acked-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>

> diff --git a/drivers/ide/pci/scc_pata.c b/drivers/ide/pci/scc_pata.c
> index 9ce1d80..b4be295 100644
> --- a/drivers/ide/pci/scc_pata.c
> +++ b/drivers/ide/pci/scc_pata.c
[...]
> @@ -557,10 +553,12 @@ static int setup_mmio_scc (struct pci_dev *dev, const char *name)
>  		return ret;
>  	}
>  
> -	if ((ctl_addr = ioremap(ctl_base, ctl_size)) == NULL)
> +	ctl_addr = pci_ioremap_bar(dev, 0);
> +	if (!ctl_addr)
>  		goto fail_0;
>  
> -	if ((dma_addr = ioremap(dma_base, dma_size)) == NULL)
> +	dma_addr = pci_ioremap_bar(dev, 1);
> +	if (!dma_addr)
>  		goto fail_1;
>  
>  	pci_set_master(dev);

    An added bonus here is fixing couple checkpatch.pl's warnings.
    There were 22 errors and 18 warnings total before this patch. :-)

MBR, Sergei

Re: [PATCH] pci: use pci_ioremap_bar() in drivers/ide

[Sergei Shtylyov]

I just wrote:

>> diff --git a/drivers/ide/pci/scc_pata.c b/drivers/ide/pci/scc_pata.c
>> index 9ce1d80..b4be295 100644
>> --- a/drivers/ide/pci/scc_pata.c
>> +++ b/drivers/ide/pci/scc_pata.c

> [...]

>> @@ -557,10 +553,12 @@ static int setup_mmio_scc (struct pci_dev *dev, 
>> const char *name)
>>          return ret;
>>      }
>>  
>> -    if ((ctl_addr = ioremap(ctl_base, ctl_size)) == NULL)
>> +    ctl_addr = pci_ioremap_bar(dev, 0);
>> +    if (!ctl_addr)
>>          goto fail_0;
>>  
>> -    if ((dma_addr = ioremap(dma_base, dma_size)) == NULL)
>> +    dma_addr = pci_ioremap_bar(dev, 1);
>> +    if (!dma_addr)
>>          goto fail_1;
>>  
>>      pci_set_master(dev);

>    An added bonus here is fixing couple checkpatch.pl's warnings.

    Wait, those even were errors, so even better. :-)

MBR, Sergei

Re: [PATCH] pci: use pci_ioremap_bar() in drivers/ide

[Bartlomiej Zolnierkiewicz]

On Wednesday 22 October 2008, Sergei Shtylyov wrote:
> Arjan van de Ven wrote:
> 
> >>>>Use the newly introduced pci_ioremap_bar() function in drivers/ide.
> >>>>pci_ioremap_bar() just takes a pci device and a bar number, with
> >>>>the goal of making it really hard to get wrong, while also having
> >>>>a central place to stick sanity checks.
> 
> >>>>Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
> 
> >>>Acked-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
> 
> >>    No, I'm takign back my ACK.
> >>    Since the patch intends to address all drivers/ide/, it's clearly
> >>no compolete -- siimage.c and scc_pata.c beg for the alike change (it
> >>will even permit to kill 4 local variables in the latter driver).
> 
> > like this ?
> 
>     Exactly.
> 
> > From 3f88a6936f7802a5d4af6c54a1ef353d14ff1ab2 Mon Sep 17 00:00:00 2001
> > From: Arjan van de Ven <arjan@linux.intel.com>
> > Date: Wed, 22 Oct 2008 13:21:07 -0700
> > Subject: [PATCH] ide: two more pci_ioremap_bar() conversions
> 
> > based on suggestion from Sergei Shtylyov, there are two more places
> > where using pci_ioremap_bar() makes sense.
> 
> > Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
> 
> Acked-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>

applied

Re: [PATCH] pci: use pci_ioremap_bar() in drivers/ide

[Bartlomiej Zolnierkiewicz]

On Tuesday 21 October 2008, Sergei Shtylyov wrote:
> Hello.
> 
> Arjan van de Ven wrote:
> 
> > Use the newly introduced pci_ioremap_bar() function in drivers/ide.
> > pci_ioremap_bar() just takes a pci device and a bar number, with the goal
> > of making it really hard to get wrong, while also having a central place
> > to stick sanity checks.
> >
> > Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
> 
> Acked-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>

applied