[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-scsi
Subject:    Re: [PATCH 2.6.7-mm3] ipr: minor fixes and assorted nit
From:       James Bottomley <James.Bottomley () steeleye ! com>
Date:       2004-06-30 15:52:40
Message-ID: 1088610767.1904.13.camel () mulgrave
[Download RAW message or body]

On Wed, 2004-06-30 at 10:04, Brian King wrote:
> Is there any downside to requesting all the memory regions on the 
> adapter? Some of these adapters have very large BAR windows that are not 
> needed by the device driver. This is the reason I wrote the code the way 
> I did, in the thought that I was conserving pci address space.

Yes, there is.  There's a particular quad tulip card that seems to have
256k of boot rom for each of it's IO processors, but which only seems to
get 1MB of BAR space for its bridge, so obviously, when it allocates ROM
bars, they don't fit.

However, check out what happens for this adapter.  I believe
pci_request_regions() is now more selective about which BARs it maps.

> > - ipr_alloc_mem() can not simply issue a call to ipr_free_mem() when
> >   something goes wrong as it would lead to pci_free_consistent() of
> >   unset data. Let ipr_alloc_mem() carefully release whatever it has
> >   allocated instead;
> 
> pci_free_consistent specifically checks for NULL, just like kfree. I 
> figured it was cleaner code to rely on the ability to call kfree on NULL 
> and pci_free_consistent on NULL. Is this frowned upon?

The API doesn't say that.  It works on x86 because dma_free_coherent()
calls free_pages(), which checks for NULLs, but I think you'll find it
will produce a panic or warning on other platforms (arm seems to dump
stack at least).

James


-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic