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.
Agreed.
: 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.
OK.
: 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.
OK.
: 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.
OK.
: 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.
OK.
: 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
code.
: 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...
Warner
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
