lkml.org 
[lkml]   [2005]   [Jul]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [patch 2.6.12 (repost w/ corrected subject)] pci: restore BAR values in pci_enable_device_bars
    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
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2005-07-02 10:14    [W:0.024 / U:0.448 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site