| Message ID | 4A1D40FD.5050102@kernel.org (mailing list archive) |
|---|---|
| State | RFC, archived |
| Headers | show |
On Wed, May 27, 2009 at 10:32:45PM +0900, Tejun Heo wrote: > THIS IS A RFC PATCH, SO NO SOB. PLEASE DON'T APPLY YET. This breaks CONFIG_PPC64, fwiw. We'll want to stub out pci_set_cacheline_size() for the PCI_DISABLE_MWI case too. I don't know what PPC machines have Cardbus slots, presumably some Macs do. I don't know whether firmware takes care of configuring the Cacheline Size register for Cardbus hotplug or not. So we may want to include pci_set_cacheline_size() in the !MWI build, or not. Ben, Paul? > towerlexa, can you please test this patch? > > Cc: Daniel Ritz <daniel.ritz@gmx.ch> > Cc: Dominik Brodowski <linux@dominikbrodowski.net> > Cc: Greg KH <greg@kroah.com> > Cc: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com> > Cc: towerlexa@gmx.de > --- > drivers/pci/pci.c | 3 +-- > drivers/pcmcia/cardbus.c | 23 +++++++++++++++-------- > include/linux/pci.h | 1 + > 3 files changed, 17 insertions(+), 10 deletions(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index 1a91bf9..eafbe01 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -1860,8 +1860,7 @@ u8 pci_cache_line_size = PCI_CACHE_LINE_BYTES / 4; > * > * RETURNS: An appropriate -ERRNO error value on error, or zero for success. > */ > -static int > -pci_set_cacheline_size(struct pci_dev *dev) > +int pci_set_cacheline_size(struct pci_dev *dev) > { > u8 cacheline_size; > > diff --git a/drivers/pcmcia/cardbus.c b/drivers/pcmcia/cardbus.c > index db77e1f..98789c0 100644 > --- a/drivers/pcmcia/cardbus.c > +++ b/drivers/pcmcia/cardbus.c > @@ -184,26 +184,33 @@ fail: > > =====================================================================*/ > > -/* > - * Since there is only one interrupt available to CardBus > - * devices, all devices downstream of this device must > - * be using this IRQ. > - */ > -static void cardbus_assign_irqs(struct pci_bus *bus, int irq) > +static void cardbus_config_irq_and_cls(struct pci_bus *bus, int irq) > { > struct pci_dev *dev; > > list_for_each_entry(dev, &bus->devices, bus_list) { > u8 irq_pin; > > + /* > + * Since there is only one interrupt available to > + * CardBus devices, all devices downstream of this > + * device must be using this IRQ. > + */ > pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &irq_pin); > if (irq_pin) { > dev->irq = irq; > pci_write_config_byte(dev, PCI_INTERRUPT_LINE, dev->irq); > } > > + /* > + * Some controllers transfer very slowly with 0 CLS. > + * Configure it. This may fail as CLS configuration > + * is mandatory only for MWI. > + */ > + pci_set_cacheline_size(dev); > + > if (dev->subordinate) > - cardbus_assign_irqs(dev->subordinate, irq); > + cardbus_config_irq_and_cls(dev->subordinate, irq); > } > } > > @@ -228,7 +235,7 @@ int __ref cb_alloc(struct pcmcia_socket * s) > */ > pci_bus_size_bridges(bus); > pci_bus_assign_resources(bus); > - cardbus_assign_irqs(bus, s->pci_irq); > + cardbus_config_irq_and_cls(bus, s->pci_irq); > > /* socket specific tune function */ > if (s->tune_bridge) > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 72698d8..e1a1aa6 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -688,6 +688,7 @@ void pci_disable_device(struct pci_dev *dev); > void pci_set_master(struct pci_dev *dev); > void pci_clear_master(struct pci_dev *dev); > int pci_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state); > +int pci_set_cacheline_size(struct pci_dev *dev); > #define HAVE_PCI_SET_MWI > int __must_check pci_set_mwi(struct pci_dev *dev); > int pci_try_set_mwi(struct pci_dev *dev); > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, Matthew Wilcox wrote: > On Wed, May 27, 2009 at 10:32:45PM +0900, Tejun Heo wrote: >> THIS IS A RFC PATCH, SO NO SOB. PLEASE DON'T APPLY YET. > > This breaks CONFIG_PPC64, fwiw. We'll want to stub out > pci_set_cacheline_size() for the PCI_DISABLE_MWI case too. Right, thanks for spotting it. > I don't know what PPC machines have Cardbus slots, presumably some > Macs do. I don't know whether firmware takes care of configuring the > Cacheline Size register for Cardbus hotplug or not. So we may want to > include pci_set_cacheline_size() in the !MWI build, or not. Ben, Paul? ppc64 is also missing PCI_CACHE_LINE_SIZE so pci_set_cacheline_size() can't be built as-is. BTW, on x86, pci_cache_line_size isn't configured like other pci devices on many machines, which doesn't harm correctness but still... CLS being the same for all devices coming down from the same root bridge, maybe we can do away with the current logic and just take it from the upstream pci bridge? Thanks.
On Thu, 2009-05-28 at 08:11 +0900, Tejun Heo wrote: > Hello, > > Matthew Wilcox wrote: > > On Wed, May 27, 2009 at 10:32:45PM +0900, Tejun Heo wrote: > >> THIS IS A RFC PATCH, SO NO SOB. PLEASE DON'T APPLY YET. > > > > This breaks CONFIG_PPC64, fwiw. We'll want to stub out > > pci_set_cacheline_size() for the PCI_DISABLE_MWI case too. > > Right, thanks for spotting it. > > > I don't know what PPC machines have Cardbus slots, presumably some > > Macs do. I don't know whether firmware takes care of configuring the > > Cacheline Size register for Cardbus hotplug or not. So we may want to > > include pci_set_cacheline_size() in the !MWI build, or not. Ben, Paul? Right, 32-bit Mac laptops mostly, maybe embedded stuff too. On these we definitely want to configure stuff properly from the kernel. > ppc64 is also missing PCI_CACHE_LINE_SIZE so pci_set_cacheline_size() > can't be built as-is. Well, the PCI cache line size would be a runtime thing. There are some "issues" though on some HT platforms that I don't completely remember, it really all depends on what the machine actually is. So I'll need to have a look at the actual patch set to figure out how we want to deal with it. > BTW, on x86, pci_cache_line_size isn't > configured like other pci devices on many machines, which doesn't harm > correctness but still... CLS being the same for all devices coming > down from the same root bridge, maybe we can do away with the current > logic and just take it from the upstream pci bridge? Cheers, Ben. -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Tejun, i'm sorry, but i don't know how to apply this patch. I'am not a developer, and so i don't know what i should do. If it make sense (in your opinion) i would try this, but maybe someone could me help please? (of course its not a problem to do the three-step: configure, make, make install... but after this i don't know to remove the installed files) So, Maybe someone could build me an deb-package for my Kubuntu-System??? --------------- Linux UBUNTUNB 2.6.24-24-generic #1 SMP Wed Apr 15 15:54:25 UTC 2009 i686 GNU/Linux .... cat /etc/lsb-release DISTRIB_ID=Ubuntu DISTRIB_RELEASE=8.04 DISTRIB_CODENAME=hardy DISTRIB_DESCRIPTION="Ubuntu 8.04.2" ------------------ Thanks and regards Axel Tejun Heo schrieb: > > THIS IS A RFC PATCH, SO NO SOB. PLEASE DON'T APPLY YET. > > towerlexa, can you please test this patch? > --- > drivers/pci/pci.c | 3 +-- > drivers/pcmcia/cardbus.c | 23 +++++++++++++++-------- > include/linux/pci.h | 1 + > 3 files changed, 17 insertions(+), 10 deletions(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index 1a91bf9..eafbe01 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -1860,8 +1860,7 @@ u8 pci_cache_line_size = PCI_CACHE_LINE_BYTES / 4; > * > * RETURNS: An appropriate -ERRNO error value on error, or zero for success. > */ > -static int > -pci_set_cacheline_size(struct pci_dev *dev) > +int pci_set_cacheline_size(struct pci_dev *dev) > { > u8 cacheline_size; > > diff --git a/drivers/pcmcia/cardbus.c b/drivers/pcmcia/cardbus.c > index db77e1f..98789c0 100644 > --- a/drivers/pcmcia/cardbus.c > +++ b/drivers/pcmcia/cardbus.c > @@ -184,26 +184,33 @@ fail: > > =====================================================================*/ > > -/* > - * Since there is only one interrupt available to CardBus > - * devices, all devices downstream of this device must > - * be using this IRQ. > - */ > -static void cardbus_assign_irqs(struct pci_bus *bus, int irq) > +static void cardbus_config_irq_and_cls(struct pci_bus *bus, int irq) > { > struct pci_dev *dev; > > list_for_each_entry(dev, &bus->devices, bus_list) { > u8 irq_pin; > > + /* > + * Since there is only one interrupt available to > + * CardBus devices, all devices downstream of this > + * device must be using this IRQ. > + */ > pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &irq_pin); > if (irq_pin) { > dev->irq = irq; > pci_write_config_byte(dev, PCI_INTERRUPT_LINE, dev->irq); > } > > + /* > + * Some controllers transfer very slowly with 0 CLS. > + * Configure it. This may fail as CLS configuration > + * is mandatory only for MWI. > + */ > + pci_set_cacheline_size(dev); > + > if (dev->subordinate) > - cardbus_assign_irqs(dev->subordinate, irq); > + cardbus_config_irq_and_cls(dev->subordinate, irq); > } > } > > @@ -228,7 +235,7 @@ int __ref cb_alloc(struct pcmcia_socket * s) > */ > pci_bus_size_bridges(bus); > pci_bus_assign_resources(bus); > - cardbus_assign_irqs(bus, s->pci_irq); > + cardbus_config_irq_and_cls(bus, s->pci_irq); > > /* socket specific tune function */ > if (s->tune_bridge) > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 72698d8..e1a1aa6 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -688,6 +688,7 @@ void pci_disable_device(struct pci_dev *dev); > void pci_set_master(struct pci_dev *dev); > void pci_clear_master(struct pci_dev *dev); > int pci_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state); > +int pci_set_cacheline_size(struct pci_dev *dev); > #define HAVE_PCI_SET_MWI > int __must_check pci_set_mwi(struct pci_dev *dev); > int pci_try_set_mwi(struct pci_dev *dev); > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 1a91bf9..eafbe01 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1860,8 +1860,7 @@ u8 pci_cache_line_size = PCI_CACHE_LINE_BYTES / 4; * * RETURNS: An appropriate -ERRNO error value on error, or zero for success. */ -static int -pci_set_cacheline_size(struct pci_dev *dev) +int pci_set_cacheline_size(struct pci_dev *dev) { u8 cacheline_size; diff --git a/drivers/pcmcia/cardbus.c b/drivers/pcmcia/cardbus.c index db77e1f..98789c0 100644 --- a/drivers/pcmcia/cardbus.c +++ b/drivers/pcmcia/cardbus.c @@ -184,26 +184,33 @@ fail: =====================================================================*/ -/* - * Since there is only one interrupt available to CardBus - * devices, all devices downstream of this device must - * be using this IRQ. - */ -static void cardbus_assign_irqs(struct pci_bus *bus, int irq) +static void cardbus_config_irq_and_cls(struct pci_bus *bus, int irq) { struct pci_dev *dev; list_for_each_entry(dev, &bus->devices, bus_list) { u8 irq_pin; + /* + * Since there is only one interrupt available to + * CardBus devices, all devices downstream of this + * device must be using this IRQ. + */ pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &irq_pin); if (irq_pin) { dev->irq = irq; pci_write_config_byte(dev, PCI_INTERRUPT_LINE, dev->irq); } + /* + * Some controllers transfer very slowly with 0 CLS. + * Configure it. This may fail as CLS configuration + * is mandatory only for MWI. + */ + pci_set_cacheline_size(dev); + if (dev->subordinate) - cardbus_assign_irqs(dev->subordinate, irq); + cardbus_config_irq_and_cls(dev->subordinate, irq); } } @@ -228,7 +235,7 @@ int __ref cb_alloc(struct pcmcia_socket * s) */ pci_bus_size_bridges(bus); pci_bus_assign_resources(bus); - cardbus_assign_irqs(bus, s->pci_irq); + cardbus_config_irq_and_cls(bus, s->pci_irq); /* socket specific tune function */ if (s->tune_bridge) diff --git a/include/linux/pci.h b/include/linux/pci.h index 72698d8..e1a1aa6 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -688,6 +688,7 @@ void pci_disable_device(struct pci_dev *dev); void pci_set_master(struct pci_dev *dev); void pci_clear_master(struct pci_dev *dev); int pci_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state); +int pci_set_cacheline_size(struct pci_dev *dev); #define HAVE_PCI_SET_MWI int __must_check pci_set_mwi(struct pci_dev *dev); int pci_try_set_mwi(struct pci_dev *dev);