[PATCH] cardbus: Add a fixup hook and fix powerpc

[PATCH] cardbus: Add a fixup hook and fix powerpc

[Benjamin Herrenschmidt]

The cardbus code creates PCI devices without ever going through the
necessary fixup bits and pieces that normal PCI devices go through.

There's in fact a commented out call to pcibios_fixup_bus() in there,
it's commented because ... it doesn't work.

I could make pcibios_fixup_bus() do the right thing on powerpc easily
but I felt it cleaner instead to provide a specific hook pci_fixup_cardbus
for which a weak empty implementation is provided by the PCI core.

This fixes cardbus on powerbooks and probably all other PowerPC
platforms which was broken completely for ever on some platforms and
since 2.6.31 on others such as PowerBooks when we made the DMA ops
mandatory (since those are setup by the fixups).

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

Note: In the long run we might want to streamline the hooks for fixing
up new devices vs. new busses and make some stuff common between cardbus
and PCI hotplug which is actually a mess.

This is a quick fix that makes it work again in the meantime, and that
I would like to see in 2.6.31 and 2.6.32 stable at least. 

Another option if you prefer is I can make my pcibios_fixup_bus() do the
right thing in all cases I believe (by testing bus->is_added to skip
stuff that must not be done twice) and we can remove the comment and
stick the call in #ifdef CONFIG_PPC but I felt that solution was a tad
cleaner.

Olof, once that's in you should be able to remove the hack you have in
the PA-Semi code to work around this.

Cheers,
Ben.

Index: linux-work/drivers/pcmcia/cardbus.c
===================================================================
--- linux-work.orig/drivers/pcmcia/cardbus.c	2009-12-09 17:16:10.000000000 +1100
+++ linux-work/drivers/pcmcia/cardbus.c	2009-12-09 17:34:58.000000000 +1100
@@ -214,7 +214,7 @@ int __ref cb_alloc(struct pcmcia_socket 
 	unsigned int max, pass;
 
 	s->functions = pci_scan_slot(bus, PCI_DEVFN(0, 0));
-//	pcibios_fixup_bus(bus);
+	pci_fixup_cardbus(bus);
 
 	max = bus->secondary;
 	for (pass = 0; pass < 2; pass++)
Index: linux-work/include/linux/pci.h
===================================================================
--- linux-work.orig/include/linux/pci.h	2009-12-09 17:16:10.000000000 +1100
+++ linux-work/include/linux/pci.h	2009-12-09 17:34:47.000000000 +1100
@@ -564,6 +564,9 @@ void pcibios_align_resource(void *, stru
 				resource_size_t);
 void pcibios_update_irq(struct pci_dev *, int irq);
 
+/* Weak but can be overriden by arch */
+void pci_fixup_cardbus(struct pci_bus *);
+
 /* Generic PCI functions used internally */
 
 extern struct pci_bus *pci_find_bus(int domain, int busnr);
Index: linux-work/arch/powerpc/kernel/pci-common.c
===================================================================
--- linux-work.orig/arch/powerpc/kernel/pci-common.c	2009-12-09 17:16:10.000000000 +1100
+++ linux-work/arch/powerpc/kernel/pci-common.c	2009-12-09 17:35:19.000000000 +1100
@@ -1107,6 +1107,12 @@ void __devinit pcibios_setup_bus_devices
 	list_for_each_entry(dev, &bus->devices, bus_list) {
 		struct dev_archdata *sd = &dev->dev.archdata;
 
+		/* Cardbus can call us to add new devices to a bus, so ignore
+		 * those who are already fully discovered
+		 */
+		if (dev->is_added)
+			continue;
+
 		/* Setup OF node pointer in archdata */
 		sd->of_node = pci_device_to_OF_node(dev);
 
@@ -1147,6 +1153,13 @@ void __devinit pcibios_fixup_bus(struct 
 }
 EXPORT_SYMBOL(pcibios_fixup_bus);
 
+void __devinit pci_fixup_cardbus(struct pci_bus *bus)
+{
+	/* Now fixup devices on that bus */
+	pcibios_setup_bus_devices(bus);
+}
+
+
 static int skip_isa_ioresource_align(struct pci_dev *dev)
 {
 	if ((ppc_pci_flags & PPC_PCI_CAN_SKIP_ISA_ALIGN) &&
Index: linux-work/drivers/pci/pci.c
===================================================================
--- linux-work.orig/drivers/pci/pci.c	2009-12-09 17:33:24.000000000 +1100
+++ linux-work/drivers/pci/pci.c	2009-12-09 17:34:16.000000000 +1100
@@ -2723,6 +2723,11 @@ int __attribute__ ((weak)) pci_ext_cfg_a
 	return 1;
 }
 
+void __weak pci_fixup_cardbus(struct pci_bus *bus)
+{
+}
+EXPORT_SYMBOL(pci_fixup_cardbus);
+
 static int __init pci_setup(char *str)
 {
 	while (str) {

Re: [PATCH] cardbus: Add a fixup hook and fix powerpc

[Dominik Brodowski]

Hey Benjamin,

On Wed, Dec 09, 2009 at 05:52:13PM +1100, Benjamin Herrenschmidt wrote:
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Acked-by: Dominik Brodowski <linux@dominikbrodowski.net> (for PCMCIA)

> Note: In the long run we might want to streamline the hooks for fixing
> up new devices vs. new busses and make some stuff common between cardbus
> and PCI hotplug which is actually a mess.

... though I'd prefer such a generic approach.

Best,
	Dominik

Re: [PATCH] cardbus: Add a fixup hook and fix powerpc

[Benjamin Herrenschmidt]

On Wed, 2009-12-09 at 22:37 +0100, Dominik Brodowski wrote:
> Acked-by: Dominik Brodowski <linux@dominikbrodowski.net> (for PCMCIA)
> 
> > Note: In the long run we might want to streamline the hooks for fixing
> > up new devices vs. new busses and make some stuff common between cardbus
> > and PCI hotplug which is actually a mess.
> 
> ... though I'd prefer such a generic approach. 

Thanks. Yes that would be better. I don't have the bandwidth right now
to look at that though, maybe early next year.

Cheers,
Ben.

Re: [PATCH] cardbus: Add a fixup hook and fix powerpc

[Jesse Barnes]

On Wed, 09 Dec 2009 17:52:13 +1100
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> The cardbus code creates PCI devices without ever going through the
> necessary fixup bits and pieces that normal PCI devices go through.
> 
> There's in fact a commented out call to pcibios_fixup_bus() in there,
> it's commented because ... it doesn't work.
> 
> I could make pcibios_fixup_bus() do the right thing on powerpc easily
> but I felt it cleaner instead to provide a specific hook
> pci_fixup_cardbus for which a weak empty implementation is provided
> by the PCI core.
> 
> This fixes cardbus on powerbooks and probably all other PowerPC
> platforms which was broken completely for ever on some platforms and
> since 2.6.31 on others such as PowerBooks when we made the DMA ops
> mandatory (since those are setup by the fixups).
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Applied, thanks.  Had to fix up a conflict since someone had "helpfully"
changed the comment from a // to a /* .. */ in cardbus.c.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] cardbus: Add a fixup hook and fix powerpc

[Jesse Barnes]

On Wed, 09 Dec 2009 17:52:13 +1100
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> The cardbus code creates PCI devices without ever going through the
> necessary fixup bits and pieces that normal PCI devices go through.
> 
> There's in fact a commented out call to pcibios_fixup_bus() in there,
> it's commented because ... it doesn't work.
> 
> I could make pcibios_fixup_bus() do the right thing on powerpc easily
> but I felt it cleaner instead to provide a specific hook
> pci_fixup_cardbus for which a weak empty implementation is provided
> by the PCI core.
> 
> This fixes cardbus on powerbooks and probably all other PowerPC
> platforms which was broken completely for ever on some platforms and
> since 2.6.31 on others such as PowerBooks when we made the DMA ops
> mandatory (since those are setup by the fixups).
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> 
> Note: In the long run we might want to streamline the hooks for fixing
> up new devices vs. new busses and make some stuff common between
> cardbus and PCI hotplug which is actually a mess.
> 
> This is a quick fix that makes it work again in the meantime, and that
> I would like to see in 2.6.31 and 2.6.32 stable at least. 
> 
> Another option if you prefer is I can make my pcibios_fixup_bus() do
> the right thing in all cases I believe (by testing bus->is_added to
> skip stuff that must not be done twice) and we can remove the comment
> and stick the call in #ifdef CONFIG_PPC but I felt that solution was
> a tad cleaner.
> 
> Olof, once that's in you should be able to remove the hack you have in
> the PA-Semi code to work around this.
> 

Oops, looks like this fails for the modular case?  I get an unresolved
symbol error when building this with my default config...

Care to resend with the fix against my for-linus branch?

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] cardbus: Add a fixup hook and fix powerpc

[Benjamin Herrenschmidt]

On Wed, 2009-12-16 at 14:01 -0800, Jesse Barnes wrote:

> > Olof, once that's in you should be able to remove the hack you have in
> > the PA-Semi code to work around this.
> > 
> 
> Oops, looks like this fails for the modular case?  I get an unresolved
> symbol error when building this with my default config...
> 
> Care to resend with the fix against my for-linus branch?

Care to send a log ? I fail to see where the problem is since the patch
has:

--- linux-work.orig/drivers/pci/pci.c   2009-12-09 17:33:24.000000000 +1100
+++ linux-work/drivers/pci/pci.c        2009-12-09 17:34:16.000000000 +1100
@@ -2723,6 +2723,11 @@ int __attribute__ ((weak)) pci_ext_cfg_a
        return 1;
 }
 
+void __weak pci_fixup_cardbus(struct pci_bus *bus)
+{
+}
+EXPORT_SYMBOL(pci_fixup_cardbus);

Or is that broken in some way ?

I'm starting to regret trying to use weak stuff :-) I may turn it into a good
old:

#ifndef pcibios_fixup_cardbus
static inline void pcibios_fixup_cardbus(struct pci_bus *bus) { }
#define pcibios_fixup_cardbus
#endif

In a header and have ppc define it.

(And call it pcibios_ instead of pci_ which matches better what other arch
fixups are called :-)

Cheers,
Ben.

Re: [PATCH] cardbus: Add a fixup hook and fix powerpc

[Jesse Barnes]

On Wed, 09 Dec 2009 17:52:13 +1100
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> The cardbus code creates PCI devices without ever going through the
> necessary fixup bits and pieces that normal PCI devices go through.
> 
> There's in fact a commented out call to pcibios_fixup_bus() in there,
> it's commented because ... it doesn't work.
> 
> I could make pcibios_fixup_bus() do the right thing on powerpc easily
> but I felt it cleaner instead to provide a specific hook
> pci_fixup_cardbus for which a weak empty implementation is provided
> by the PCI core.
> 
> This fixes cardbus on powerbooks and probably all other PowerPC
> platforms which was broken completely for ever on some platforms and
> since 2.6.31 on others such as PowerBooks when we made the DMA ops
> mandatory (since those are setup by the fixups).
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Ah the link failure was my fault.  I had fixed up the conflict
incorrectly.  It's applied now.

-- 
Jesse Barnes, Intel Open Source Technology Center