RE: [PATCH 3/4] [SCSI] ips: PCI API cleanups

2007-10-25 Thread Salyzyn, Mark
ACK. Inspected only. Looks ok.

Sincerely -- Mark Salyzyn

> -Original Message-
> From: [EMAIL PROTECTED] 
> [mailto:[EMAIL PROTECTED] On Behalf Of Jeff Garzik
> Sent: Wednesday, October 24, 2007 7:49 PM
> To: LKML; linux-scsi@vger.kernel.org
> Cc: [EMAIL PROTECTED]
> Subject: [PATCH 3/4] [SCSI] ips: PCI API cleanups
> 
> * pass Scsi_Host to ips_remove_device() via pci_set_drvdata(),
>   allowing us to eliminate the ips_ha[] search loop and call
>   ips_release() directly.
> 
> * call pci_{request,release}_regions() and eliminate individual
>   request/release_[mem_]region() calls
> 
> * call pci_disable_device(), paired with pci_enable_device()
> 
> * s/0/NULL/ in a few places
> 
> * check ioremap() return value
> 
> Signed-off-by: Jeff Garzik <[EMAIL PROTECTED]>
> ---
>  drivers/scsi/ips.c |   72 
> ++-
>  1 files changed, 31 insertions(+), 41 deletions(-)
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/4] [SCSI] ips: PCI API cleanups

2007-10-24 Thread Rolf Eike Beer
Jeff Garzik wrote:
> * pass Scsi_Host to ips_remove_device() via pci_set_drvdata(),
>   allowing us to eliminate the ips_ha[] search loop and call
>   ips_release() directly.
>
> * call pci_{request,release}_regions() and eliminate individual
>   request/release_[mem_]region() calls
>
> * call pci_disable_device(), paired with pci_enable_device()
>
> * s/0/NULL/ in a few places
>
> * check ioremap() return value

> @@ -7036,32 +7042,17 @@ ips_init_phase1(struct pci_dev *pci_dev, int
> *indexPtr) uint32_t base;
>   uint32_t offs;
>
> - if (!request_mem_region(mem_addr, mem_len, "ips")) {
> - IPS_PRINTK(KERN_WARNING, pci_dev,
> -"Couldn't allocate IO Memory space %x len 
> %d.\n",
> -mem_addr, mem_len);
> - return -1;
> - }
> -
>   base = mem_addr & PAGE_MASK;
>   offs = mem_addr - base;
>   ioremap_ptr = ioremap(base, PAGE_SIZE);

This looks odd. What are we actually doing here?

It seems that we want to map that PCI BAR. Since we're playing with PAGE_MASK 
I assume the BAR always has a length 

signature.asc
Description: This is a digitally signed message part.