On Sat, Jul 02, 2005 at 01:29:54AM -0600, Grant Grundler wrote:
> On Thu, Jun 30, 2005 at 10:26:37PM -0400, John W. Linville wrote:
> ...
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -378,9 +378,56 @@ pci_restore_state(struct pci_dev *dev)
> > int
> > pci_enable_device_bars(struct pci_dev *dev, int bars)
> > {
> > - int err;
> > + int i, numres, err;
> >
> > pci_set_power_state(dev, PCI_D0);
> > +
> > + /* Some devices lose PCI config header data during D3hot->D0
>
> Can you name some of those devices here?
> I just want to know what sort of devices need to be tested
> if this code changes in the future.
>
> > + transition. Since some firmware leaves devices in D3hot
> > + state at boot, this information needs to be restored.
>
> Again, which firmware?
> Examples are good since it makes it possible to track down
> the offending devices for testing.
>
> The following chunk looks like it will have issues with 64-bit BARs:
> ...
> > + for (i = 0; i < numres; i ++) {
> > + struct pci_bus_region region;
> > + u32 val;
> > + int reg;
> ...
> > + val = region.start
> > + | (dev->resource[i].flags & PCI_REGION_FLAG_MASK);
> > +
> > + reg = PCI_BASE_ADDRESS_0 + (i * 4);
>
> ISTR dev->resource[i] doesn't necessarily correspond directly BAR[i].
> If BAR0 is a 64-bit BAR, then dev->resource[1] will point at BAR2.
That's contary to the assumptions made by setup-res.c. BAR0 is
dev->resource[0]. If that resource is 64-bit, dev->resource[1]
is unused and the next resource is dev->resource[2].
> > + pci_write_config_dword(dev, reg, val);
> > +
> > + if ((val & (PCI_BASE_ADDRESS_SPACE
> > + | PCI_BASE_ADDRESS_MEM_TYPE_MASK))
> > + == (PCI_BASE_ADDRESS_SPACE_MEMORY
> > + | PCI_BASE_ADDRESS_MEM_TYPE_64)) {
> > + pci_write_config_dword(dev, reg + 4, 0);
>
> 64-bit BARs need the upper half of dev->resource[] written.
> I expect something like:
> val = region.start >> 4;
Are you sure you mean >> 4 ? Don't you mean >> 32 ? Note again that
setup-res.c writes zero unconditionally here.
The PCI subsystem is incomplete for 64-bit BAR support. What it does
do though is ensure that 64-bit BARs will work correctly in a 32-bit
system. Therefore, I think that folk who want 64-bit BAR support to
work need to do some code reviews on the PCI subsystem to resolve the
remaining issues.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
