In message <[EMAIL PROTECTED]> "Justin T. Gibbs" writes:
:  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.


: 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.


: 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.

Makes sense.

: 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.

I'll have to look up the CIS_PTR spec.  I'm not sure I like hardwiring
things like this.

: 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?

I'd think so, but I'm not sure.

: 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.

I'll have look this one up as well.

: 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.


: 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.


: 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).

I'd love to see that as well.  There's a lot of duplicated code that
I'd like to see factored.  This is especially true with the 16bit

: 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.

That might be interesting.

Now, I'm off to try these patches...


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

Reply via email to