On Wed, Mar 31, 2010 at 12:13:46PM -0600, Bjorn Helgaas wrote:
> On Wednesday 31 March 2010 10:38:42 am Ira W. Snyder wrote:
> > Read the memory ranges behind the Broadcom CNB20LE host bridge out of the
> > hardware. This allows PCI hotplugging to work, since we know which memory
> > range to allocate PCI BAR's from.
> >
> > Signed-off-by: Ira W. Snyder <iws@xxxxxxxxxxxxxxxx>
> > ---
> >
> > With this patch, I can successfully use the fakephp driver on a Trenton
> > CPLE SBC. The BIOS on this computer does not use ACPI, and therefore we
> > cannot use the ACPI _CRS section to get the memory ranges behind the host
> > bridge.
>
> The CNB20LE might also be used in systems that *do* have ACPI, and in
> that case, I think we should use ACPI rather than read the info out of
> the hardware. I expect that's what Windows will do, and Linux should
> do the same as Windows when it's practical. Also, that's what the BIOS
> writers expect the OS to do, and _CRS is the logical place for them to
> put platform-specific workarounds.
>
Is there any way to detect if we do have ACPI and shouldn't run this
quirk? How?
> > arch/x86/pci/Makefile | 2 +-
> > arch/x86/pci/broadcom_bus.c | 80 +++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 81 insertions(+), 1 deletions(-)
> > create mode 100644 arch/x86/pci/broadcom_bus.c
> >
> > diff --git a/arch/x86/pci/Makefile b/arch/x86/pci/Makefile
> > index b110d97..19fc7ce 100644
> > --- a/arch/x86/pci/Makefile
> > +++ b/arch/x86/pci/Makefile
> > @@ -16,7 +16,7 @@ obj-$(CONFIG_X86_NUMAQ) += numaq_32.o
> > obj-$(CONFIG_X86_MRST) += mrst.o
> >
> > obj-y += common.o early.o
> > -obj-y += amd_bus.o bus_numa.o
> > +obj-y += amd_bus.o bus_numa.o broadcom_bus.o
> >
> > ifeq ($(CONFIG_PCI_DEBUG),y)
> > EXTRA_CFLAGS += -DDEBUG
> > diff --git a/arch/x86/pci/broadcom_bus.c b/arch/x86/pci/broadcom_bus.c
> > new file mode 100644
> > index 0000000..b50d112
> > --- /dev/null
> > +++ b/arch/x86/pci/broadcom_bus.c
> > @@ -0,0 +1,80 @@
> > +/*
> > + * Read address ranges from a Broadcom CNB20LE Host Bridge
> > + *
> > + * Copyright (c) 2009 Ira W. Snyder <iws@xxxxxxxxxxxxxxxx>
> > + *
> > + * This file is licensed under the terms of the GNU General Public License
> > + * version 2. This program is licensed "as is" without any warranty of any
> > + * kind, whether express or implied.
> > + */
> > +
> > +#include <linux/delay.h>
> > +#include <linux/dmi.h>
> > +#include <linux/pci.h>
> > +#include <linux/init.h>
> > +#include <asm/pci_x86.h>
> > +
> > +#include "bus_numa.h"
> > +
> > +static void __devinit cnb20le_res(struct pci_dev *dev)
> > +{
> > + struct pci_root_info *info;
> > + unsigned long flags;
> > + u16 word1, word2;
> > + u32 start, end;
> > + u8 fbus, lbus;
> > +
> > + info = &pci_root_info[pci_root_num];
> > + pci_root_num++;
> > +
> > + /* add legacy IDE ports on bus 0 */
> > + if (pci_root_num == 1) {
> > + dev_dbg(&dev->dev, "CNB20LE: adding legacy IDE ports\n");
> > + update_res(info, 0x01f0, 0x01f7, IORESOURCE_IO, 0);
> > + update_res(info, 0x03f6, 0x03f6, IORESOURCE_IO, 0);
> > + update_res(info, 0x0170, 0x0177, IORESOURCE_IO, 0);
> > + update_res(info, 0x0376, 0x0376, IORESOURCE_IO, 0);
> > + update_res(info, 0xffa0, 0xffaf, IORESOURCE_IO, 0);
> > + }
>
> I don't like the fact that this depends on the PCI enumeration order.
> Can you do this when "fbus == 0" instead?
>
Sure, not a problem.
> > +
> > + pci_read_config_byte(dev, 0x44, &fbus);
> > + pci_read_config_byte(dev, 0x45, &lbus);
> > + dev_dbg(&dev->dev, "CNB20LE: busses: %d to %d\n", fbus, lbus);
>
> Please print the bus number, I/O port, and memory ranges in the same
> format used by %pR, e.g., "[bus 00-ff]", etc. You might as well use
> text similar to that in arch/x86/pci/acpi.c, e.g., "host bridge window".
>
Should these have the "window" text that IORESOURCE_WINDOW would print,
too? Just for IORESOURCE_(MEM|IO), or for IORESOURCE_BUS too?
> > +
> > + info->bus_min = fbus;
> > + info->bus_max = lbus;
> > +
> > + pci_read_config_word(dev, 0xc0, &word1);
> > + pci_read_config_word(dev, 0xc2, &word2);
> > + dev_dbg(&dev->dev, "CNB20LE: noPF 0x%.4x 0x%.4x\n", word1, word2);
> > + if (word1 != word2) {
> > + start = (word1 << 16) | 0x0000;
> > + end = (word2 << 16) | 0xffff;
> > + flags = IORESOURCE_MEM;
> > + update_res(info, start, end, flags, 0);
> > + }
> > +
> > + pci_read_config_word(dev, 0xc4, &word1);
> > + pci_read_config_word(dev, 0xc6, &word2);
> > + dev_dbg(&dev->dev, "CNB20LE: PF 0x%.4x 0x%.4x\n", word1, word2);
> > + if (word1 != word2) {
> > + start = (word1 << 16) | 0x0000;
> > + end = (word2 << 16) | 0xffff;
> > + flags = IORESOURCE_MEM | IORESOURCE_PREFETCH;
> > + update_res(info, start, end, flags, 0);
> > + }
> > +
> > + pci_read_config_word(dev, 0xd0, &word1);
> > + pci_read_config_word(dev, 0xd2, &word2);
> > + dev_dbg(&dev->dev, "CNB20LE: IO 0x%.4x 0x%.4x\n", word1, word2);
> > + if (word1 != word2) {
> > + start = word1;
> > + end = word2;
> > + flags = IORESOURCE_IO;
> > + update_res(info, start, end, flags, 0);
> > + }
> > +}
> > +
> > +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SERVERWORKS, PCI_DEVICE_ID_SERVERWORKS_LE,
> > + cnb20le_res);
> > +
Thanks for the review.
Ira
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
