I've been side tracked for a long time since after the con with various
more urgant things to do, and have left the cardbus stuff along for a
while, but now that thanksgiving is near, I might have some more time to
work on this...

On Sat, Nov 18, 2000 at 10:05:01PM -0700, Justin T. Gibbs wrote:
> While working on getting the APA-1480 to work under FreeBSD's new
> cardbus support, I ran into several issues.
> 
>  1) When mucking with mapping registers, it is best to *not* have
>     io or memory space access enabled.  This patch defers the setting
>     of these bits until after all of the mapping registers are probed.
>     It might be even better to defer this until a particular mapping
>     is activated and to disable that type of access when a new
>     register is activated.

Good...

> 2) The PCI spec is very explicit about how mapping registers and
>    the expansion ROM mapping register should be probed.  This patch
>    makes cardbus_add_map() follow the spec.

Gook...

> 3) The PCI spec allows a device to use the same address decoder for
>    expansion ROM access as is used for memory mapped register access.
>    This patch carefully enables and disables ROM access along with
>    resource (de)activiation.

Hmm... I didn't think of this...

> 4) The cardbus CIS code treats the CIS_PTR as a mapping register if
>    it is mentioned in the CIS.  I don't have a spec handy to understand
>    why the CIS_PTR is mentioned in the CIS, but allocating a memory range
>    for it is certainly bogus.  My patch ignores bar #6 to prevent the
>    mapping.
> 
> 5) The CIS code allocated duplicate resources to those already found
>    by cardbus_add_resources().  The fix is to pass in the bar computed
>    from the CIS instead of the particular resource ID for that bar,
>    so bus_generic_alloc_resource succeeds in finding the old resource.
>    It seems somewhat strange that we have to have two methods for
>    finding and activating the mapping registers.  Isn't one method
>    sufficient?
> 
> 6) cardbus_read_exrom_cis() failed to advance correctly to higer rom
>    images.  To effect the fix, the cis_ptr value must be provided to
>    the different CIS reading methods, unaltered.
> 
> 7) The CIS code seems to use the wrong bit to determine rather a particular
>    register mapping is for I/O or memory space.  From looking at the
>    two cards I have, it seems TPL_BAR_REG_AS should be 0x10 instead
>    of 0x08.  Otherwise, all registers that should be I/O mapped gain
>    a second mapping in memory space.

Okay... CIS stuff is nasty in the current code.  Basically -

First, the whole CIS reading needs to be rewritten.  The current practice
of reading the whole rom then passing it to various functions is bad
because it would fail for CIS tuples that should make the reader jump
around, say, to another rom image or whatever.

Second, the BAR mapping stuff, yes, it is wrong.  I don't know where I
got the idea of doing it wrong in the first place... too much coding
late at night without proper documentation I guess... I've had a fix
sitting on my local tree for a while, and will commit that soon-ish.

And lastly, as for why we need to read the BAR at all, I thought about this
briefly when I first wrote the code and came up with the logic that there
might be some random cardbus device that stuck a BAR outside of the normal
registers but still had the proper mapping in the CIS.  For "normal" cards
this additional mapping doesn't do anything as they are already
mapped.  This attempt at mapping BARs using both the PCI and the CIS
methods has made the resource allocation functions more ugly than it needs
to be.  I'm wondering if perhaps one of the two should just be left out.

> 8) The cardbus bridge code leaves memory space prefetching enabled.
>    Prefetching is only allowed if the target device indicates (through
>    its mapping register) that prefetching is allowable.  My patch
>    simply disables prefetching and includes code to detect this capability
>    and pass an RF flag to enable it, but nothing more.

Good... I was wondering whether I should leave that enabled or not.

> 9) The pccbb code was impoperly handling the I/O and mem range limit
>    registers.  The limit register indicates the highest valid address
>    in the window, not the first invalid address outside the window.

Oops...

> One last thing that is started here is an attempt to rely more heavily
> on PCI register definitions and eventually functions, to get things
> done.  The cardbus code duplicates a lot of functionality that is
> already available in the pci code (mapping register size/type detection).

Good.

> One other thing that struck me while I was looking at this was that
> the resource manager should be providing the "resource pooling"
> that pccbb_cardbus_auto_open() emulates.  Although the cardbus
> bridges we support only provide 4K granularity for memory mapped
> windows, things like external rom access often can be mapped on
> 2K boundaries.  This could allow the resource manager to allocate
> a range that doesn't appear to overlap with another allocation but
> does due to the bridges constraints.  I might look into adding the
> concept of hierarchical resource pools to the resource manager so
> that, for example, the cardbus bridges pool will always grow in
> 4K increments from its parent resource pool.  The parent would then
> grow according to its own requirements, etc.

This will be very cool...

-- 
    (o_ 1-2-1-2-1-2-1-2-1-2-1-2-1-2-1-2-1-2-1-2-1-2-1-2-1-2-1-2-1-2 _o)
 \\\_\            Jonathan Chen              [EMAIL PROTECTED]           /_///
 <____)  No electrons were harmed during production of this message (____>
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message

Reply via email to