LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v13 10/10] USB/ppc4xx:Synopsys DWC OTG driver enable gadget support
From: tmarri @ 2011-04-03 23:17 UTC (permalink / raw)
  To: linux-usb, linuxppc-dev; +Cc: tmarri, greg, Mark Miesfeld, Fushen Chen

From: Tirumala Marri <tmarri@apm.com>

Enable gadget support

Signed-off-by: Tirumala R Marri <tmarri@apm.com>
Signed-off-by: Fushen Chen <fchen@apm.com>
Signed-off-by: Mark Miesfeld <mmiesfeld@apm.com>
---
 drivers/usb/gadget/Kconfig        |   11 +++++++++++
 drivers/usb/gadget/gadget_chips.h |    9 +++++++++
 2 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index bc5123c..b641c0a 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -365,6 +365,17 @@ config USB_GADGET_MUSB_HDRC
 	  This OTG-capable silicon IP is used in dual designs including
 	  the TI DaVinci, OMAP 243x, OMAP 343x, TUSB 6010, and ADI Blackfin
 
+# dwc_otg builds in ../dwc along with host support
+config USB_GADGET_DWC_HDRC
+	boolean "DesignWare USB Peripheral"
+	depends on DWC_OTG_MODE || DWC_DEVICE_ONLY
+	select USB_GADGET_DUALSPEED
+	select USB_GADGET_SELECTED
+	select USB_GADGET_DWC_OTG
+	help
+	  This OTG-capable Designware USB IP which has host and device
+	  modes.
+
 config USB_GADGET_M66592
 	boolean "Renesas M66592 USB Peripheral Controller"
 	select USB_GADGET_DUALSPEED
diff --git a/drivers/usb/gadget/gadget_chips.h b/drivers/usb/gadget/gadget_chips.h
index e896f63..d1fab43 100644
--- a/drivers/usb/gadget/gadget_chips.h
+++ b/drivers/usb/gadget/gadget_chips.h
@@ -136,6 +136,12 @@
 #define gadget_is_s3c_hsotg(g)    0
 #endif
 
+#if defined(CONFIG_DWC_OTG_MODE) || defined(CONFIG_DWC_DEVICE_ONLY)
+#define gadget_is_dwc_otg_pcd(g)	(!strcmp("dwc_otg_pcd", (g)->name))
+#else
+#define gadget_is_dwc_otg_pcd(g)	0
+#endif
+
 #ifdef CONFIG_USB_GADGET_EG20T
 #define	gadget_is_pch(g)	(!strcmp("pch_udc", (g)->name))
 #else
@@ -207,6 +213,9 @@ static inline int usb_gadget_controller_number(struct usb_gadget *gadget)
 		return 0x27;
 	else if (gadget_is_ci13xxx_msm(gadget))
 		return 0x28;
+	else if (gadget_is_dwc_otg_pcd(gadget))
+		return 0x29;
+
 	return -ENOENT;
 }
 
-- 
1.6.1.rc3

^ permalink raw reply related

* Re: problem PCIe LSI detected at 32 device addresses (ppc460ex)
From: Ayman El-Khashab @ 2011-04-03 23:22 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev list
In-Reply-To: <BANLkTikTeAgc2dg3-M7yM185pg7FAdbAGw@mail.gmail.com>


On Sun, Apr 03, 2011 at 04:09:26PM -0600, Grant Likely wrote:
> On Sun, Apr 3, 2011 at 3:52 PM, Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
> >
> >> Ok, I've narrowed the scope of the problem some. ?I moved forward
> >> to a more recent kernel (2.6.31 to 2.6.36) and that resolved the
> >> problem of the controller showing up as every device on the bus.
> >> However, from 2.6.37 to the current HEAD, I have not been able to
> >> build a kernel to run on the 460EX. ?I tried 2.6.37, 2.6.38, and
> >> the HEAD and all result in the following kernel panic. ?I am not
> >> sure how to proceed here. ?I suppose we can stick with 2.6.36 since
> >> it works, but I'd like to understand what it might take to remedy
> >> this.
> >
> > Smells like somebody changed something with the OF flash code... Josh,
> > Grant, any idea what's up there ?
> 
> Not sure, more information would be helpful.
> 
> Ayman, if you do a 'git log v2.6.36.. drivers/mtd/maps/physmap_of.c',
> then you'll see a list of commits touching the mtd driver.  Would you
> be able to do a 'git checkout <sha1-id>' on each of those are report
> back on at what point things stop working?  Actually, a full bisect
> between 2.6.36 and 2.6.37 would be best, but this is a good start if
> you're limited on time.  Once you find the first commit where it
> fails, do a 'git checkout <sha1>~1' to confirm that it is in fact the
> commit that causes the breakage.

I can try to find the commit tomorrow.  In the interim, i've pasted
the dts below.  The board was originally based on the canyonlands, but
we've made some changes, mostly to the pcie.  we run the 1-l port in 
endpoint mode, we have a chain of plx switches and devices on the 4-l 
port.  One item that I don't think would matter, but is not too common 
is that we are booting these over the pci bus from another PPCs memory.
I only mention this since this failure is during boot, though everything
should by local to the cpu by this time.

> 
> Can you also post your device tree please?

Here is the device tree for our custom board.


/dts-v1/;

/ {
	#address-cells = <2>;
	#size-cells = <1>;
	model = "amcc,canyonlands";
	compatible = "amcc,canyonlands";
	dcr-parent = <&{/cpus/cpu@0}>;

	aliases {
		ethernet0 = &EMAC0;
		ethernet1 = &EMAC1;
		serial0 = &UART0;
		serial1 = &UART1;
	};

	cpus {
		#address-cells = <1>;
		#size-cells = <0>;

		cpu@0 {
			device_type = "cpu";
			model = "PowerPC,460EX";
			reg = <0x00000000>;
			clock-frequency = <0>; /* Filled in by U-Boot */
			timebase-frequency = <0>; /* Filled in by U-Boot */
			i-cache-line-size = <32>;
			d-cache-line-size = <32>;
			i-cache-size = <32768>;
			d-cache-size = <32768>;
			dcr-controller;
			dcr-access-method = "native";
		};
	};

	memory {
		device_type = "memory";
		reg = <0x00000000 0x00000000 0x00000000>; /* Filled in by U-Boot */
	};

	UIC0: interrupt-controller0 {
		compatible = "ibm,uic-460ex","ibm,uic";
		interrupt-controller;
		cell-index = <0>;
		dcr-reg = <0x0c0 0x009>;
		#address-cells = <0>;
		#size-cells = <0>;
		#interrupt-cells = <2>;
	};

	UIC1: interrupt-controller1 {
		compatible = "ibm,uic-460ex","ibm,uic";
		interrupt-controller;
		cell-index = <1>;
		dcr-reg = <0x0d0 0x009>;
		#address-cells = <0>;
		#size-cells = <0>;
		#interrupt-cells = <2>;
		interrupts = <0x1e 0x4 0x1f 0x4>; /* cascade */
		interrupt-parent = <&UIC0>;
	};

	UIC2: interrupt-controller2 {
		compatible = "ibm,uic-460ex","ibm,uic";
		interrupt-controller;
		cell-index = <2>;
		dcr-reg = <0x0e0 0x009>;
		#address-cells = <0>;
		#size-cells = <0>;
		#interrupt-cells = <2>;
		interrupts = <0xa 0x4 0xb 0x4>; /* cascade */
		interrupt-parent = <&UIC0>;
	};

	UIC3: interrupt-controller3 {
		compatible = "ibm,uic-460ex","ibm,uic";
		interrupt-controller;
		cell-index = <3>;
		dcr-reg = <0x0f0 0x009>;
		#address-cells = <0>;
		#size-cells = <0>;
		#interrupt-cells = <2>;
		interrupts = <0x10 0x4 0x11 0x4>; /* cascade */
		interrupt-parent = <&UIC0>;
	};

	SDR0: sdr {
		compatible = "ibm,sdr-460ex";
		dcr-reg = <0x00e 0x002>;
	};

	CPR0: cpr {
		compatible = "ibm,cpr-460ex";
		dcr-reg = <0x00c 0x002>;
	};

	L2C0: l2c {
		compatible = "ibm,l2-cache-460ex", "ibm,l2-cache";
		dcr-reg = <0x020 0x008			/* Internal SRAM DCR's */
			   0x030 0x008>;		/* L2 cache DCR's */
		cache-line-size = <32>;		/* 32 bytes */
		cache-size = <262144>;		/* L2, 256K */
		interrupt-parent = <&UIC2>;
		interrupts = <0x17 0x1>;
	};

	plb {
		compatible = "ibm,plb-460ex", "ibm,plb4";
		#address-cells = <2>;
		#size-cells = <1>;
		ranges;
		clock-frequency = <0>; /* Filled in by U-Boot */

		SDRAM0: sdram {
			compatible = "ibm,sdram-460ex", "ibm,sdram-405gp";
			dcr-reg = <0x010 0x002>;
		};

		MAL0: mcmal {
			compatible = "ibm,mcmal-460ex", "ibm,mcmal2";
			dcr-reg = <0x180 0x062>;
			num-tx-chans = <2>;
			num-rx-chans = <16>;
			#address-cells = <0>;
			#size-cells = <0>;
			interrupt-parent = <&UIC2>;
			interrupts = <	/*TXEOB*/ 0x6 0x4
					/*RXEOB*/ 0x7 0x4
					/*SERR*/  0x3 0x4
					/*TXDE*/  0x4 0x4
					/*RXDE*/  0x5 0x4>;
		};

		SATA0: sata@bffd1000 {
			compatible = "amcc,sata-460ex";
			reg = <0x00000004 0xbffd1000 0x00000800	/* SATA */
			       0x00000004 0xbffd0800 0x00000400>;	/* AHBDMA */
			interrupt-parent = <&UIC3>;
			interrupts = <0x0 0x4	/* SATA */
				      0x5 0x4>;	/* AHBDMA */
		};

		POB0: opb {
			compatible = "ibm,opb-460ex", "ibm,opb";
			#address-cells = <1>;
			#size-cells = <1>;
			ranges = <0xb0000000 0x00000004 0xb0000000 0x50000000>;
			clock-frequency = <0>; /* Filled in by U-Boot */

			EBC0: ebc {
				compatible = "ibm,ebc-460ex", "ibm,ebc";
				dcr-reg = <0x012 0x002>;
				#address-cells = <2>;
				#size-cells = <1>;
				clock-frequency = <0>; /* Filled in by U-Boot */
				/* ranges property is supplied by U-Boot */
				interrupts = <0x6 0x4>;
				interrupt-parent = <&UIC1>;

				flash@1,0 {
					compatible = "cfi-flash";
					reg=<0x00000000 0x00000000 0x01000000>;
					bank-width = <2>;
				};

			};

			UART0: serial@ef600300 {
				device_type = "serial";
				compatible = "ns16550";
				reg = <0xef600300 0x00000008>;
				virtual-reg = <0xef600300>;
				clock-frequency = <0>; /* Filled in by U-Boot */
				current-speed = <0>; /* Filled in by U-Boot */
				interrupt-parent = <&UIC1>;
				interrupts = <0x1 0x4>;
			};

			UART1: serial@ef600400 {
				device_type = "serial";
				compatible = "ns16550";
				reg = <0xef600400 0x00000008>;
				virtual-reg = <0xef600400>;
				clock-frequency = <0>; /* Filled in by U-Boot */
				current-speed = <0>; /* Filled in by U-Boot */
				interrupt-parent = <&UIC0>;
				interrupts = <0x1 0x4>;
			};

                        IIC0: i2c@ef600700 {
                                compatible = "ibm,iic-460ex", "ibm,iic-440ep", "ibm,iic-440gp", "ibm,iic" ;
                                reg = <0xef600700 0x00000014>;
                                interrupt-parent = <&UIC0>;
                                interrupts = <0x2 0x4>;
                                #address-cells = <1>;
                                #size-cells = <0>;

				at24@50 {
					compatible = "at,24c128";
					reg = <0x50>;
				};
				spd@51 {
					compatible = "at,spd";
					reg = <0x51>;
				};
				at24@52 {
					compatible = "at,24c128";
					reg = <0x52>;
				};
				at24@53 {
					compatible = "at,24c128";
					reg = <0x53>;
				};
				at24@54 {
					compatible = "at,24c128";
					reg = <0x54>;
				};
				at24@55 {
					compatible = "at,24c128";
					reg = <0x55>;
				};
				at24@56 {
					compatible = "at,24c128";
					reg = <0x56>;
				};
				tmp423@4c {
					compatible = "ti,tmp423";
					reg = <0x4c>;
				};
				pcf8575@27 {
					compatible = "ph,pcf8575";
					reg = <0x27>;
				};
				pcf8575@21 {
					compatible = "ph,pcf8575";
					reg = <0x21>;
				};
				pcf8575@20 {
					compatible = "ph,pcf8575";
					reg = <0x20>;
				};



                        };

                        GPIO0: gpio@ef600b00 {
                                compatible = "ibm,ppc4xx-gpio", "ibm,440ep-gpio";
                                reg = <0xef600b00 0x00000048>;
				#gpio-cells = <2>;
				gpio-controller;
                        };

                        GPIO1: gpio@ef600c00 {
                                compatible = "ibm,ppc4xx-gpio", "ibm,440ep-gpio";
                                reg = <0xef600c00 0x00000048>;
				#gpio-cells = <2>;
				gpio-controller;
                        };


 

			ZMII0: emac-zmii@ef600d00 {
				compatible = "ibm,zmii-460ex", "ibm,zmii";
				reg = <0xef600d00 0x0000000c>;
			};

			RGMII0: emac-rgmii@ef601500 {
				compatible = "ibm,rgmii-460ex", "ibm,rgmii";
				reg = <0xef601500 0x00000008>;
				has-mdio;
			};

			TAH0: emac-tah@ef601350 {
				compatible = "ibm,tah-460ex", "ibm,tah";
				reg = <0xef601350 0x00000030>;
			};

			TAH1: emac-tah@ef601450 {
				compatible = "ibm,tah-460ex", "ibm,tah";
				reg = <0xef601450 0x00000030>;
			};

			EMAC0: ethernet@ef600e00 {
				device_type = "network";
				compatible = "ibm,emac-460ex", "ibm,emac4";
				interrupt-parent = <&EMAC0>;
				interrupts = <0x0 0x1>;
				#interrupt-cells = <1>;
				#address-cells = <0>;
				#size-cells = <0>;
				interrupt-map = </*Status*/ 0x0 &UIC2 0x10 0x4
						 /*Wake*/   0x1 &UIC2 0x14 0x4>;
				reg = <0xef600e00 0x00000070>;
				local-mac-address = [000000000000]; /* Filled in by U-Boot */
				mal-device = <&MAL0>;
				mal-tx-channel = <0>;
				mal-rx-channel = <0>;
				cell-index = <0>;
				max-frame-size = <9000>;
				rx-fifo-size = <4096>;
				tx-fifo-size = <2048>;
				phy-mode = "rgmii";
				phy-map = <0x00000000>;
				rgmii-device = <&RGMII0>;
				rgmii-channel = <0>;
				tah-device = <&TAH0>;
				tah-channel = <0>;
				has-inverted-stacr-oc;
				has-new-stacr-staopc;
			};

			EMAC1: ethernet@ef600f00 {
				device_type = "network";
				compatible = "ibm,emac-460ex", "ibm,emac4";
				interrupt-parent = <&EMAC1>;
				interrupts = <0x0 0x1>;
				#interrupt-cells = <1>;
				#address-cells = <0>;
				#size-cells = <0>;
				interrupt-map = </*Status*/ 0x0 &UIC2 0x11 0x4
						 /*Wake*/   0x1 &UIC2 0x15 0x4>;
				reg = <0xef600f00 0x00000070>;
				local-mac-address = [000000000000]; /* Filled in by U-Boot */
				mal-device = <&MAL0>;
				mal-tx-channel = <1>;
				mal-rx-channel = <8>;
				cell-index = <1>;
				max-frame-size = <9000>;
				rx-fifo-size = <4096>;
				tx-fifo-size = <2048>;
				phy-mode = "rgmii";
				phy-map = <0x00000000>;
				rgmii-device = <&RGMII0>;
				rgmii-channel = <1>;
				tah-device = <&TAH1>;
				tah-channel = <1>;
				has-inverted-stacr-oc;
				has-new-stacr-staopc;
				mdio-device = <&EMAC0>;
			};
		};

		PCIX0: pci@c0ec00000 {
			device_type = "pci";
			#interrupt-cells = <1>;
			#size-cells = <2>;
			#address-cells = <3>;
			compatible = "ibm,plb-pcix-460ex", "ibm,plb-pcix";
			primary;
			large-inbound-windows;
			enable-msi-hole;
			reg = <0x0000000c 0x0ec00000   0x00000008	/* Config space access */
			       0x00000000 0x00000000 0x00000000		/* no IACK cycles */
			       0x0000000c 0x0ed00000   0x00000004   /* Special cycles */
			       0x0000000c 0x0ec80000 0x00000100	/* Internal registers */
			       0x0000000c 0x0ec80100  0x000000fc>;	/* Internal messaging registers */

			/* Outbound ranges, one memory and one IO,
			 * later cannot be changed
			 */
			ranges = <0x02000000 0x00000000 0x80000000 0x0000000d 0x80000000 0x00000000 0x80000000
				  0x01000000 0x00000000 0x00000000 0x0000000c 0x08000000 0x00000000 0x00010000>;

			/* Inbound 2GB range starting at 0 */
			dma-ranges = <0x42000000 0x0 0x0 0x0 0x0 0x0 0x80000000>;

			/* This drives busses 0 to 0x3f */
			bus-range = <0x0 0x3f>;

			/* All PCI interrupts are routed to ext IRQ 2 -> UIC1-0 */
			interrupt-map-mask = <0x0 0x0 0x0 0x0>;
			interrupt-map = < 0x0 0x0 0x0 0x0 &UIC1 0x0 0x8 >;
		};

		PCIE0: pciex@d00000000 {
			device_type = "pci-endpoint";
                        vendor-id=<0x0222>;
                        device-id=<0xaeae>;
			#interrupt-cells = <1>;
			#size-cells = <2>;
			#address-cells = <3>;
			compatible = "ibm,plb-pciex-460ex", "ibm,plb-pciex";
			primary;
			port = <0x0>; /* port number */
			reg = <0x0000000d 0x00000000 0x20000000	/* Config space access */
			       0x0000000c 0x08010000 0x00001000>;	/* Registers */
			dcr-reg = <0x100 0x020>;
			sdr-base = <0x300>;

			/* Outbound ranges, one memory and one IO,
			 * later cannot be changed
			 */
			ranges = <0x02000000 0x00000000 0x80000000 0x0000000e 0x00000000 0x00000000 0x80000000
				  0x01000000 0x00000000 0x00000000 0x0000000f 0x80000000 0x00000000 0x00010000>;

			/* Inbound 2GB range starting at 0 */
			dma-ranges = <0x42000000 0x0 0x0 0x0 0x0 0x0 0x80000000>;

			/* This drives busses 40 to 0x7f */
			bus-range = <0x40 0x7f>;

			/* Legacy interrupts (note the weird polarity, the bridge seems
			 * to invert PCIe legacy interrupts).
			 * We are de-swizzling here because the numbers are actually for
			 * port of the root complex virtual P2P bridge. But I want
			 * to avoid putting a node for it in the tree, so the numbers
			 * below are basically de-swizzled numbers.
			 * The real slot is on idsel 0, so the swizzling is 1:1
			 */
			interrupt-map-mask = <0x0 0x0 0x0 0x7>;
			interrupt-map = <
				0x0 0x0 0x0 0x1 &UIC3 0xc 0x4 /* swizzled int A */
				0x0 0x0 0x0 0x2 &UIC3 0xd 0x4 /* swizzled int B */
				0x0 0x0 0x0 0x3 &UIC3 0xe 0x4 /* swizzled int C */
				0x0 0x0 0x0 0x4 &UIC3 0xf 0x4 /* swizzled int D */>;
		};

		PCIE1: pciex@d20000000 {
			device_type = "pci";
			#interrupt-cells = <1>;
			#size-cells = <2>;
			#address-cells = <3>;
			compatible = "ibm,plb-pciex-460ex", "ibm,plb-pciex";
			primary;
			port = <0x1>; /* port number */
			reg = <0x0000000d 0x20000000 0x20000000	/* Config space access */
			       0x0000000c 0x08011000 0x00001000>;	/* Registers */
			dcr-reg = <0x120 0x020>;
			sdr-base = <0x340>;

			/* Outbound ranges, one memory and one IO,
			 * later cannot be changed
			 */
			ranges = <0x02000000 0x00000000 0x80000000 0x0000000e 0x80000000 0x00000000 0x80000000
				  0x01000000 0x00000000 0x00000000 0x0000000f 0x80010000 0x00000000 0x00010000>;

			/* Inbound 2GB range starting at 0 */
			dma-ranges = <0x42000000 0x0 0x0 0x0 0x0 0x0 0x80000000>;

			/* This drives busses 80 to 0xbf */
			bus-range = <0x80 0xbf>;

			/* Legacy interrupts (note the weird polarity, the bridge seems
			 * to invert PCIe legacy interrupts).
			 * We are de-swizzling here because the numbers are actually for
			 * port of the root complex virtual P2P bridge. But I want
			 * to avoid putting a node for it in the tree, so the numbers
			 * below are basically de-swizzled numbers.
			 * The real slot is on idsel 0, so the swizzling is 1:1
			 */
			interrupt-map-mask = <0x0 0x0 0x0 0x7>;
			interrupt-map = <
				0x0 0x0 0x0 0x1 &UIC3 0x10 0x4 /* swizzled int A */
				0x0 0x0 0x0 0x2 &UIC3 0x11 0x4 /* swizzled int B */
				0x0 0x0 0x0 0x3 &UIC3 0x12 0x4 /* swizzled int C */
				0x0 0x0 0x0 0x4 &UIC3 0x13 0x4 /* swizzled int D */>;
		};
	};
};

^ permalink raw reply

* [RFC/PATCH] of: Match PCI devices to OF nodes generically
From: Benjamin Herrenschmidt @ 2011-04-04  2:04 UTC (permalink / raw)
  To: linux-arch; +Cc: linuxppc-dev, linux-kernel@vger.kernel.org, David Miller

powerpc has two different ways of matching PCI devices to their
corresponding OF node (if any) for historical reasons. The ppc64 one
does a scan looking for matching bus/dev/fn, while the ppc32 one does a
scan looking only for matching dev/fn on each level in order to be
agnostic to busses being renumbered (which Linux does on some
platforms).

This removes both and instead moves the matching code to the PCI core
itself. It's the most logical place to do it: when a pci_dev is created,
we know the parent and thus can do a single level scan for the matching
device_node (if any).

The benefit is that all archs now get the matching for free. There's one
hook the arch might want to provide to match a PHB bus to its device
node. A default weak implementation is provided that looks for the
parent device device node, but it's not entirely reliable on powerpc for
various reasons so powerpc provides its own.

Dave, I don't think I broke anything in sparc land but I haven't tested
(and have to admit haven't build-tested yet either).

Not-signed-off-yet. Tested on a few ppc's but not the whole range yet
either, mostly posted for comments.

 arch/powerpc/include/asm/pci-bridge.h |   31 +++++-----------------
 arch/powerpc/include/asm/pci.h        |    3 --
 arch/powerpc/include/asm/prom.h       |    1 
 arch/powerpc/kernel/pci-common.c      |   11 +++++--
 arch/powerpc/kernel/pci_32.c          |    6 ----
 arch/powerpc/kernel/pci_dn.c          |   47
----------------------------------
 arch/powerpc/kernel/pci_of_scan.c     |    9 ++----
 arch/sparc/kernel/pci.c               |    2 -
 drivers/pci/Makefile                  |    2 +
 drivers/pci/hotplug/rpadlpar_core.c   |    2 -
 drivers/pci/probe.c                   |    8 +++++
 include/linux/pci.h                   |   17 ++++++++++++
 12 files changed, 46 insertions(+), 93 deletions(-)

diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
index 5e156e0..a4c8e4a 100644
--- a/arch/powerpc/include/asm/pci-bridge.h
+++ b/arch/powerpc/include/asm/pci-bridge.h
@@ -169,18 +169,18 @@ static inline struct pci_controller *pci_bus_to_host(const struct pci_bus *bus)
 	return bus->sysdata;
 }
 
-#ifndef CONFIG_PPC64
+static inline struct device_node *pci_device_to_OF_node(struct pci_dev *dev)
+{
+	return dev->dev.of_node;
+}
 
 static inline struct device_node *pci_bus_to_OF_node(struct pci_bus *bus)
 {
-	struct pci_controller *host;
-
-	if (bus->self)
-		return pci_device_to_OF_node(bus->self);
-	host = pci_bus_to_host(bus);
-	return host ? host->dn : NULL;
+	return bus->dev.of_node;
 }
 
+#ifndef CONFIG_PPC64
+
 static inline int isa_vaddr_is_ioport(void __iomem *address)
 {
 	/* No specific ISA handling on ppc32 at this stage, it
@@ -223,17 +223,8 @@ struct pci_dn {
 /* Get the pointer to a device_node's pci_dn */
 #define PCI_DN(dn)	((struct pci_dn *) (dn)->data)
 
-extern struct device_node *fetch_dev_dn(struct pci_dev *dev);
 extern void * update_dn_pci_info(struct device_node *dn, void *data);
 
-/* Get a device_node from a pci_dev.  This code must be fast except
- * in the case where the sysdata is incorrect and needs to be fixed
- * up (this will only happen once). */
-static inline struct device_node *pci_device_to_OF_node(struct pci_dev *dev)
-{
-	return dev->dev.of_node ? dev->dev.of_node : fetch_dev_dn(dev);
-}
-
 static inline int pci_device_from_OF_node(struct device_node *np,
 					  u8 *bus, u8 *devfn)
 {
@@ -244,14 +235,6 @@ static inline int pci_device_from_OF_node(struct device_node *np,
 	return 0;
 }
 
-static inline struct device_node *pci_bus_to_OF_node(struct pci_bus *bus)
-{
-	if (bus->self)
-		return pci_device_to_OF_node(bus->self);
-	else
-		return bus->dev.of_node; /* Must be root bus (PHB) */
-}
-
 /** Find the bus corresponding to the indicated device node */
 extern struct pci_bus *pcibios_find_pci_bus(struct device_node *dn);
 
diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h
index 7d77909..1f52268 100644
--- a/arch/powerpc/include/asm/pci.h
+++ b/arch/powerpc/include/asm/pci.h
@@ -179,8 +179,7 @@ extern int remove_phb_dynamic(struct pci_controller *phb);
 extern struct pci_dev *of_create_pci_dev(struct device_node *node,
 					struct pci_bus *bus, int devfn);
 
-extern void of_scan_pci_bridge(struct device_node *node,
-				struct pci_dev *dev);
+extern void of_scan_pci_bridge(struct pci_dev *dev);
 
 extern void of_scan_bus(struct device_node *node, struct pci_bus *bus);
 extern void of_rescan_bus(struct device_node *node, struct pci_bus *bus);
diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
index c189aa5..2b888cc 100644
--- a/arch/powerpc/include/asm/prom.h
+++ b/arch/powerpc/include/asm/prom.h
@@ -32,7 +32,6 @@ struct pci_dev;
 extern int pci_device_from_OF_node(struct device_node *node,
 				   u8* bus, u8* devfn);
 extern struct device_node* pci_busdev_to_OF_node(struct pci_bus *, int);
-extern struct device_node* pci_device_to_OF_node(struct pci_dev *);
 extern void pci_create_OF_bus_map(void);
 #endif
 
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 893af2a..47c516b 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1097,9 +1097,6 @@ void __devinit pcibios_setup_bus_devices(struct pci_bus *bus)
 		if (dev->is_added)
 			continue;
 
-		/* Setup OF node pointer in the device */
-		dev->dev.of_node = pci_device_to_OF_node(dev);
-
 		/* Fixup NUMA node as it may not be setup yet by the generic
 		 * code and is needed by the DMA init
 		 */
@@ -1685,6 +1682,13 @@ int early_find_capability(struct pci_controller *hose, int bus, int devfn,
 	return pci_bus_find_capability(fake_pci_bus(hose, bus), devfn, cap);
 }
 
+struct device_node * pcibios_get_phb_of_node(struct pci_bus *bus)
+{
+	struct pci_controller *hose = bus->sysdata;
+
+	return of_node_get(hose->dn);
+}
+
 /**
  * pci_scan_phb - Given a pci_controller, setup and scan the PCI bus
  * @hose: Pointer to the PCI host controller instance structure
@@ -1705,7 +1709,6 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose)
 			hose->global_number);
 		return;
 	}
-	bus->dev.of_node = of_node_get(node);
 	bus->secondary = hose->first_busno;
 	hose->bus = bus;
 
diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c
index bedb370..700a826 100644
--- a/arch/powerpc/kernel/pci_32.c
+++ b/arch/powerpc/kernel/pci_32.c
@@ -276,12 +276,6 @@ pci_busdev_to_OF_node(struct pci_bus *bus, int devfn)
 }
 EXPORT_SYMBOL(pci_busdev_to_OF_node);
 
-struct device_node*
-pci_device_to_OF_node(struct pci_dev *dev)
-{
-	return pci_busdev_to_OF_node(dev->bus, dev->devfn);
-}
-EXPORT_SYMBOL(pci_device_to_OF_node);
 
 static int
 find_OF_pci_device_filter(struct device_node* node, void* data)
diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index d225d99..8cb66a2 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -143,53 +143,6 @@ void __devinit pci_devs_phb_init_dynamic(struct pci_controller *phb)
 	traverse_pci_devices(dn, update_dn_pci_info, phb);
 }
 
-/*
- * Traversal func that looks for a <busno,devfcn> value.
- * If found, the pci_dn is returned (thus terminating the traversal).
- */
-static void *is_devfn_node(struct device_node *dn, void *data)
-{
-	int busno = ((unsigned long)data >> 8) & 0xff;
-	int devfn = ((unsigned long)data) & 0xff;
-	struct pci_dn *pci = dn->data;
-
-	if (pci && (devfn == pci->devfn) && (busno == pci->busno))
-		return dn;
-	return NULL;
-}
-
-/*
- * This is the "slow" path for looking up a device_node from a
- * pci_dev.  It will hunt for the device under its parent's
- * phb and then update of_node pointer.
- *
- * It may also do fixups on the actual device since this happens
- * on the first read/write.
- *
- * Note that it also must deal with devices that don't exist.
- * In this case it may probe for real hardware ("just in case")
- * and add a device_node to the device tree if necessary.
- *
- * Is this function necessary anymore now that dev->dev.of_node is
- * used to store the node pointer?
- *
- */
-struct device_node *fetch_dev_dn(struct pci_dev *dev)
-{
-	struct pci_controller *phb = dev->sysdata;
-	struct device_node *dn;
-	unsigned long searchval = (dev->bus->number << 8) | dev->devfn;
-
-	if (WARN_ON(!phb))
-		return NULL;
-
-	dn = traverse_pci_devices(phb->dn, is_devfn_node, (void *)searchval);
-	if (dn)
-		dev->dev.of_node = dn;
-	return dn;
-}
-EXPORT_SYMBOL(fetch_dev_dn);
-
 /** 
  * pci_devs_phb_init - Initialize phbs and pci devs under them.
  * 
diff --git a/arch/powerpc/kernel/pci_of_scan.c b/arch/powerpc/kernel/pci_of_scan.c
index 1e89a72..fe0a5ad 100644
--- a/arch/powerpc/kernel/pci_of_scan.c
+++ b/arch/powerpc/kernel/pci_of_scan.c
@@ -202,9 +202,9 @@ EXPORT_SYMBOL(of_create_pci_dev);
  * this routine in turn call of_scan_bus() recusively to scan for more child
  * devices.
  */
-void __devinit of_scan_pci_bridge(struct device_node *node,
-				  struct pci_dev *dev)
+void __devinit of_scan_pci_bridge(struct pci_dev *dev)
 {
+	struct device_node *node = dev->dev.of_node;
 	struct pci_bus *bus;
 	const u32 *busrange, *ranges;
 	int len, i, mode;
@@ -238,7 +238,6 @@ void __devinit of_scan_pci_bridge(struct device_node *node,
 	bus->primary = dev->bus->number;
 	bus->subordinate = busrange[1];
 	bus->bridge_ctl = 0;
-	bus->dev.of_node = of_node_get(node);
 
 	/* parse ranges property */
 	/* PCI #address-cells == 3 and #size-cells == 2 always */
@@ -335,9 +334,7 @@ static void __devinit __of_scan_bus(struct device_node *node,
 	list_for_each_entry(dev, &bus->devices, bus_list) {
 		if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||
 		    dev->hdr_type == PCI_HEADER_TYPE_CARDBUS) {
-			struct device_node *child = pci_device_to_OF_node(dev);
-			if (child)
-				of_scan_pci_bridge(child, dev);
+			of_scan_pci_bridge(dev);
 		}
 	}
 }
diff --git a/arch/sparc/kernel/pci.c b/arch/sparc/kernel/pci.c
index 713dc91..e539d23 100644
--- a/arch/sparc/kernel/pci.c
+++ b/arch/sparc/kernel/pci.c
@@ -284,7 +284,7 @@ static struct pci_dev *of_create_pci_dev(struct pci_pbm_info *pbm,
 	dev->sysdata = node;
 	dev->dev.parent = bus->bridge;
 	dev->dev.bus = &pci_bus_type;
-	dev->dev.of_node = node;
+	dev->dev.of_node = of_node_get(node);
 	dev->devfn = devfn;
 	dev->multifunction = 0;		/* maybe a lie? */
 	set_pcie_port_type(dev);
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
index 98d61c8..d5c3cb9 100644
--- a/drivers/pci/Makefile
+++ b/drivers/pci/Makefile
@@ -70,4 +70,6 @@ obj-$(CONFIG_PCI_STUB) += pci-stub.o
 
 obj-$(CONFIG_XEN_PCIDEV_FRONTEND) += xen-pcifront.o
 
+obj-$(CONFIG_OF) += of.o
+
 ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG
diff --git a/drivers/pci/hotplug/rpadlpar_core.c b/drivers/pci/hotplug/rpadlpar_core.c
index 0830347..1d002b1 100644
--- a/drivers/pci/hotplug/rpadlpar_core.c
+++ b/drivers/pci/hotplug/rpadlpar_core.c
@@ -158,7 +158,7 @@ static void dlpar_pci_add_bus(struct device_node *dn)
 	/* Scan below the new bridge */
 	if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||
 	    dev->hdr_type == PCI_HEADER_TYPE_CARDBUS)
-		of_scan_pci_bridge(dn, dev);
+		of_scan_pci_bridge(dev);
 
 	/* Map IO space for child bus, which may or may not succeed */
 	pcibios_map_io_space(dev->subordinate);
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 44cbbba..347349b 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -89,6 +89,7 @@ static void release_pcibus_dev(struct device *dev)
 	if (pci_bus->bridge)
 		put_device(pci_bus->bridge);
 	pci_bus_remove_resources(pci_bus);
+	pci_release_bus_of_node(pci_bus);
 	kfree(pci_bus);
 }
 
@@ -624,7 +625,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
 
 	child->self = bridge;
 	child->bridge = get_device(&bridge->dev);
-
+	pci_set_bus_of_node(child);	
 	pci_set_bus_speed(child);
 
 	/* Set up default resource pointers and names.. */
@@ -1074,6 +1075,7 @@ static void pci_release_dev(struct device *dev)
 
 	pci_dev = to_pci_dev(dev);
 	pci_release_capabilities(pci_dev);
+	pci_release_of_node(pci_dev);
 	kfree(pci_dev);
 }
 
@@ -1150,6 +1152,7 @@ struct pci_dev *alloc_pci_dev(void)
 }
 EXPORT_SYMBOL(alloc_pci_dev);
 
+
 /*
  * Read the config data for a PCI device, sanity-check it
  * and fill in the dev structure...
@@ -1193,6 +1196,8 @@ static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn)
 	dev->vendor = l & 0xffff;
 	dev->device = (l >> 16) & 0xffff;
 
+	pci_set_of_node(dev);
+
 	if (pci_setup_device(dev)) {
 		kfree(dev);
 		return NULL;
@@ -1445,6 +1450,7 @@ struct pci_bus * pci_create_bus(struct device *parent,
 		goto dev_reg_err;
 	b->bridge = get_device(dev);
 	device_enable_async_suspend(b->bridge);
+	pci_set_bus_of_node(b);
 
 	if (!parent)
 		set_dev_node(b->bridge, pcibus_to_node(b));
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 96f70d7..e5111da 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1543,5 +1543,22 @@ int pci_vpd_find_tag(const u8 *buf, unsigned int off, unsigned int len, u8 rdt);
 int pci_vpd_find_info_keyword(const u8 *buf, unsigned int off,
 			      unsigned int len, const char *kw);
 
+/* PCI <-> OF binding helpers */
+#ifdef  CONFIG_OF
+#include <linux/of.h>
+extern void pci_set_of_node(struct pci_dev *dev);
+extern void pci_release_of_node(struct pci_dev *dev);
+extern void pci_set_bus_of_node(struct pci_bus *bus);
+extern void pci_release_bus_of_node(struct pci_bus *bus);
+
+/* Arch may override this (weak) */
+extern struct device_node * __weak pcibios_get_phb_of_node(struct pci_bus *bus);
+
+
+#else /* CONFIG_OF */
+static void pci_locate_of_node(struct pci_dev *dev) { }
+static void pci_release_of_node(struct pci_dev *dev) { }
+#endif  /* CONFIG_OF */
+
 #endif /* __KERNEL__ */
 #endif /* LINUX_PCI_H */

^ permalink raw reply related

* Re: [RFC/PATCH] of: Match PCI devices to OF nodes generically
From: Benjamin Herrenschmidt @ 2011-04-04  3:27 UTC (permalink / raw)
  To: linux-arch
  Cc: linux-pci@vger.kernel.org, linuxppc-dev,
	linux-kernel@vger.kernel.org, David Miller
In-Reply-To: <1301882694.2549.53.camel@pasglop>

On Mon, 2011-04-04 at 12:04 +1000, Benjamin Herrenschmidt wrote:
> powerpc has two different ways of matching PCI devices to their
> corresponding OF node (if any) for historical reasons. The ppc64 one
> does a scan looking for matching bus/dev/fn, while the ppc32 one does a
> scan looking only for matching dev/fn on each level in order to be
> agnostic to busses being renumbered (which Linux does on some
> platforms).
> 
> This removes both and instead moves the matching code to the PCI core
> itself. It's the most logical place to do it: when a pci_dev is created,
> we know the parent and thus can do a single level scan for the matching
> device_node (if any).
> 
> The benefit is that all archs now get the matching for free. There's one
> hook the arch might want to provide to match a PHB bus to its device
> node. A default weak implementation is provided that looks for the
> parent device device node, but it's not entirely reliable on powerpc for
> various reasons so powerpc provides its own.
> 
> Dave, I don't think I broke anything in sparc land but I haven't tested
> (and have to admit haven't build-tested yet either).
> 
> Not-signed-off-yet. Tested on a few ppc's but not the whole range yet
> either, mostly posted for comments.

Nice, looks like I forgot to add the new drivers/pci/of.c file :-)
Here's a new patch. Also added linux-pci to the CC list.

diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
index 5e156e0..a4c8e4a 100644
--- a/arch/powerpc/include/asm/pci-bridge.h
+++ b/arch/powerpc/include/asm/pci-bridge.h
@@ -169,18 +169,18 @@ static inline struct pci_controller *pci_bus_to_host(const struct pci_bus *bus)
 	return bus->sysdata;
 }
 
-#ifndef CONFIG_PPC64
+static inline struct device_node *pci_device_to_OF_node(struct pci_dev *dev)
+{
+	return dev->dev.of_node;
+}
 
 static inline struct device_node *pci_bus_to_OF_node(struct pci_bus *bus)
 {
-	struct pci_controller *host;
-
-	if (bus->self)
-		return pci_device_to_OF_node(bus->self);
-	host = pci_bus_to_host(bus);
-	return host ? host->dn : NULL;
+	return bus->dev.of_node;
 }
 
+#ifndef CONFIG_PPC64
+
 static inline int isa_vaddr_is_ioport(void __iomem *address)
 {
 	/* No specific ISA handling on ppc32 at this stage, it
@@ -223,17 +223,8 @@ struct pci_dn {
 /* Get the pointer to a device_node's pci_dn */
 #define PCI_DN(dn)	((struct pci_dn *) (dn)->data)
 
-extern struct device_node *fetch_dev_dn(struct pci_dev *dev);
 extern void * update_dn_pci_info(struct device_node *dn, void *data);
 
-/* Get a device_node from a pci_dev.  This code must be fast except
- * in the case where the sysdata is incorrect and needs to be fixed
- * up (this will only happen once). */
-static inline struct device_node *pci_device_to_OF_node(struct pci_dev *dev)
-{
-	return dev->dev.of_node ? dev->dev.of_node : fetch_dev_dn(dev);
-}
-
 static inline int pci_device_from_OF_node(struct device_node *np,
 					  u8 *bus, u8 *devfn)
 {
@@ -244,14 +235,6 @@ static inline int pci_device_from_OF_node(struct device_node *np,
 	return 0;
 }
 
-static inline struct device_node *pci_bus_to_OF_node(struct pci_bus *bus)
-{
-	if (bus->self)
-		return pci_device_to_OF_node(bus->self);
-	else
-		return bus->dev.of_node; /* Must be root bus (PHB) */
-}
-
 /** Find the bus corresponding to the indicated device node */
 extern struct pci_bus *pcibios_find_pci_bus(struct device_node *dn);
 
diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h
index 7d77909..1f52268 100644
--- a/arch/powerpc/include/asm/pci.h
+++ b/arch/powerpc/include/asm/pci.h
@@ -179,8 +179,7 @@ extern int remove_phb_dynamic(struct pci_controller *phb);
 extern struct pci_dev *of_create_pci_dev(struct device_node *node,
 					struct pci_bus *bus, int devfn);
 
-extern void of_scan_pci_bridge(struct device_node *node,
-				struct pci_dev *dev);
+extern void of_scan_pci_bridge(struct pci_dev *dev);
 
 extern void of_scan_bus(struct device_node *node, struct pci_bus *bus);
 extern void of_rescan_bus(struct device_node *node, struct pci_bus *bus);
diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
index c189aa5..2b888cc 100644
--- a/arch/powerpc/include/asm/prom.h
+++ b/arch/powerpc/include/asm/prom.h
@@ -32,7 +32,6 @@ struct pci_dev;
 extern int pci_device_from_OF_node(struct device_node *node,
 				   u8* bus, u8* devfn);
 extern struct device_node* pci_busdev_to_OF_node(struct pci_bus *, int);
-extern struct device_node* pci_device_to_OF_node(struct pci_dev *);
 extern void pci_create_OF_bus_map(void);
 #endif
 
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 893af2a..47c516b 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1097,9 +1097,6 @@ void __devinit pcibios_setup_bus_devices(struct pci_bus *bus)
 		if (dev->is_added)
 			continue;
 
-		/* Setup OF node pointer in the device */
-		dev->dev.of_node = pci_device_to_OF_node(dev);
-
 		/* Fixup NUMA node as it may not be setup yet by the generic
 		 * code and is needed by the DMA init
 		 */
@@ -1685,6 +1682,13 @@ int early_find_capability(struct pci_controller *hose, int bus, int devfn,
 	return pci_bus_find_capability(fake_pci_bus(hose, bus), devfn, cap);
 }
 
+struct device_node * pcibios_get_phb_of_node(struct pci_bus *bus)
+{
+	struct pci_controller *hose = bus->sysdata;
+
+	return of_node_get(hose->dn);
+}
+
 /**
  * pci_scan_phb - Given a pci_controller, setup and scan the PCI bus
  * @hose: Pointer to the PCI host controller instance structure
@@ -1705,7 +1709,6 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose)
 			hose->global_number);
 		return;
 	}
-	bus->dev.of_node = of_node_get(node);
 	bus->secondary = hose->first_busno;
 	hose->bus = bus;
 
diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c
index bedb370..700a826 100644
--- a/arch/powerpc/kernel/pci_32.c
+++ b/arch/powerpc/kernel/pci_32.c
@@ -276,12 +276,6 @@ pci_busdev_to_OF_node(struct pci_bus *bus, int devfn)
 }
 EXPORT_SYMBOL(pci_busdev_to_OF_node);
 
-struct device_node*
-pci_device_to_OF_node(struct pci_dev *dev)
-{
-	return pci_busdev_to_OF_node(dev->bus, dev->devfn);
-}
-EXPORT_SYMBOL(pci_device_to_OF_node);
 
 static int
 find_OF_pci_device_filter(struct device_node* node, void* data)
diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index d225d99..8cb66a2 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -143,53 +143,6 @@ void __devinit pci_devs_phb_init_dynamic(struct pci_controller *phb)
 	traverse_pci_devices(dn, update_dn_pci_info, phb);
 }
 
-/*
- * Traversal func that looks for a <busno,devfcn> value.
- * If found, the pci_dn is returned (thus terminating the traversal).
- */
-static void *is_devfn_node(struct device_node *dn, void *data)
-{
-	int busno = ((unsigned long)data >> 8) & 0xff;
-	int devfn = ((unsigned long)data) & 0xff;
-	struct pci_dn *pci = dn->data;
-
-	if (pci && (devfn == pci->devfn) && (busno == pci->busno))
-		return dn;
-	return NULL;
-}
-
-/*
- * This is the "slow" path for looking up a device_node from a
- * pci_dev.  It will hunt for the device under its parent's
- * phb and then update of_node pointer.
- *
- * It may also do fixups on the actual device since this happens
- * on the first read/write.
- *
- * Note that it also must deal with devices that don't exist.
- * In this case it may probe for real hardware ("just in case")
- * and add a device_node to the device tree if necessary.
- *
- * Is this function necessary anymore now that dev->dev.of_node is
- * used to store the node pointer?
- *
- */
-struct device_node *fetch_dev_dn(struct pci_dev *dev)
-{
-	struct pci_controller *phb = dev->sysdata;
-	struct device_node *dn;
-	unsigned long searchval = (dev->bus->number << 8) | dev->devfn;
-
-	if (WARN_ON(!phb))
-		return NULL;
-
-	dn = traverse_pci_devices(phb->dn, is_devfn_node, (void *)searchval);
-	if (dn)
-		dev->dev.of_node = dn;
-	return dn;
-}
-EXPORT_SYMBOL(fetch_dev_dn);
-
 /** 
  * pci_devs_phb_init - Initialize phbs and pci devs under them.
  * 
diff --git a/arch/powerpc/kernel/pci_of_scan.c b/arch/powerpc/kernel/pci_of_scan.c
index 1e89a72..fe0a5ad 100644
--- a/arch/powerpc/kernel/pci_of_scan.c
+++ b/arch/powerpc/kernel/pci_of_scan.c
@@ -202,9 +202,9 @@ EXPORT_SYMBOL(of_create_pci_dev);
  * this routine in turn call of_scan_bus() recusively to scan for more child
  * devices.
  */
-void __devinit of_scan_pci_bridge(struct device_node *node,
-				  struct pci_dev *dev)
+void __devinit of_scan_pci_bridge(struct pci_dev *dev)
 {
+	struct device_node *node = dev->dev.of_node;
 	struct pci_bus *bus;
 	const u32 *busrange, *ranges;
 	int len, i, mode;
@@ -238,7 +238,6 @@ void __devinit of_scan_pci_bridge(struct device_node *node,
 	bus->primary = dev->bus->number;
 	bus->subordinate = busrange[1];
 	bus->bridge_ctl = 0;
-	bus->dev.of_node = of_node_get(node);
 
 	/* parse ranges property */
 	/* PCI #address-cells == 3 and #size-cells == 2 always */
@@ -335,9 +334,7 @@ static void __devinit __of_scan_bus(struct device_node *node,
 	list_for_each_entry(dev, &bus->devices, bus_list) {
 		if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||
 		    dev->hdr_type == PCI_HEADER_TYPE_CARDBUS) {
-			struct device_node *child = pci_device_to_OF_node(dev);
-			if (child)
-				of_scan_pci_bridge(child, dev);
+			of_scan_pci_bridge(dev);
 		}
 	}
 }
diff --git a/arch/sparc/kernel/pci.c b/arch/sparc/kernel/pci.c
index 713dc91..e539d23 100644
--- a/arch/sparc/kernel/pci.c
+++ b/arch/sparc/kernel/pci.c
@@ -284,7 +284,7 @@ static struct pci_dev *of_create_pci_dev(struct pci_pbm_info *pbm,
 	dev->sysdata = node;
 	dev->dev.parent = bus->bridge;
 	dev->dev.bus = &pci_bus_type;
-	dev->dev.of_node = node;
+	dev->dev.of_node = of_node_get(node);
 	dev->devfn = devfn;
 	dev->multifunction = 0;		/* maybe a lie? */
 	set_pcie_port_type(dev);
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
index 98d61c8..d5c3cb9 100644
--- a/drivers/pci/Makefile
+++ b/drivers/pci/Makefile
@@ -70,4 +70,6 @@ obj-$(CONFIG_PCI_STUB) += pci-stub.o
 
 obj-$(CONFIG_XEN_PCIDEV_FRONTEND) += xen-pcifront.o
 
+obj-$(CONFIG_OF) += of.o
+
 ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG
diff --git a/drivers/pci/hotplug/rpadlpar_core.c b/drivers/pci/hotplug/rpadlpar_core.c
index 0830347..1d002b1 100644
--- a/drivers/pci/hotplug/rpadlpar_core.c
+++ b/drivers/pci/hotplug/rpadlpar_core.c
@@ -158,7 +158,7 @@ static void dlpar_pci_add_bus(struct device_node *dn)
 	/* Scan below the new bridge */
 	if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||
 	    dev->hdr_type == PCI_HEADER_TYPE_CARDBUS)
-		of_scan_pci_bridge(dn, dev);
+		of_scan_pci_bridge(dev);
 
 	/* Map IO space for child bus, which may or may not succeed */
 	pcibios_map_io_space(dev->subordinate);
diff --git a/drivers/pci/of.c b/drivers/pci/of.c
new file mode 100644
index 0000000..306d852
--- /dev/null
+++ b/drivers/pci/of.c
@@ -0,0 +1,87 @@
+/*
+ * PCI <-> OF mapping helpers
+ *
+ * Copyright 2011 IBM Corp.
+ * 
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include <linux/kernel.h>
+#include <linux/pci.h>
+#include "pci.h"
+
+static int pci_check_one_of_node(struct pci_dev *dev, struct device_node *node)
+{
+	unsigned int size;
+	const __be32 *reg = of_get_property(node, "reg", &size);
+
+	if (reg && size >= 4 && ((reg[0] >> 8) & 0xff) == dev->devfn) {
+		dev->dev.of_node = of_node_get(node);
+		return 1;
+	}
+	return 0;
+}
+
+void pci_set_of_node(struct pci_dev *dev)
+{
+	struct device_node *parent, *node, *node2;
+
+	parent = dev->bus->dev.of_node;
+	if (!parent)
+		return;
+	for_each_child_of_node(parent, node) {
+		if (pci_check_one_of_node(dev, node))
+			return;
+		/*
+		 * Some OFs create a parent node "multifunc-device" as
+		 * a fake root for all functions of a multi-function
+		 * device we go down them as well.
+		 */
+                if (!strcmp(node->name, "multifunc-device")) {
+			for_each_child_of_node(node, node2) {
+				if (pci_check_one_of_node(dev, node2))
+					return;
+			}
+                }
+	}
+}
+
+void pci_release_of_node(struct pci_dev *dev)
+{
+	of_node_put(dev->dev.of_node);
+	dev->dev.of_node = NULL;
+}
+
+void pci_set_bus_of_node(struct pci_bus *bus)
+{
+	if (bus->self == NULL)
+		bus->dev.of_node = pcibios_get_phb_of_node(bus);
+	else
+		bus->dev.of_node = of_node_get(bus->self->dev.of_node);
+}
+
+void pci_release_bus_of_node(struct pci_bus *bus)
+{
+	of_node_put(bus->dev.of_node);
+	bus->dev.of_node = NULL;
+}
+
+struct device_node * __weak pcibios_get_phb_of_node(struct pci_bus *bus)
+{
+	/* This should only be called for PHBs */
+	if (WARN_ON(bus->self || bus->parent))
+		return NULL;
+
+	/* Look for a node pointer in either the intermediary device we
+	 * create above the root bus or it's own parent. Normally only
+	 * the later is populated.
+	 */
+	if (bus->bridge->of_node)
+		return of_node_get(bus->bridge->of_node);
+	if (bus->bridge->parent->of_node)
+		return of_node_get(bus->bridge->parent->of_node);
+	return NULL;
+}
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 44cbbba..347349b 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -89,6 +89,7 @@ static void release_pcibus_dev(struct device *dev)
 	if (pci_bus->bridge)
 		put_device(pci_bus->bridge);
 	pci_bus_remove_resources(pci_bus);
+	pci_release_bus_of_node(pci_bus);
 	kfree(pci_bus);
 }
 
@@ -624,7 +625,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
 
 	child->self = bridge;
 	child->bridge = get_device(&bridge->dev);
-
+	pci_set_bus_of_node(child);	
 	pci_set_bus_speed(child);
 
 	/* Set up default resource pointers and names.. */
@@ -1074,6 +1075,7 @@ static void pci_release_dev(struct device *dev)
 
 	pci_dev = to_pci_dev(dev);
 	pci_release_capabilities(pci_dev);
+	pci_release_of_node(pci_dev);
 	kfree(pci_dev);
 }
 
@@ -1150,6 +1152,7 @@ struct pci_dev *alloc_pci_dev(void)
 }
 EXPORT_SYMBOL(alloc_pci_dev);
 
+
 /*
  * Read the config data for a PCI device, sanity-check it
  * and fill in the dev structure...
@@ -1193,6 +1196,8 @@ static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn)
 	dev->vendor = l & 0xffff;
 	dev->device = (l >> 16) & 0xffff;
 
+	pci_set_of_node(dev);
+
 	if (pci_setup_device(dev)) {
 		kfree(dev);
 		return NULL;
@@ -1445,6 +1450,7 @@ struct pci_bus * pci_create_bus(struct device *parent,
 		goto dev_reg_err;
 	b->bridge = get_device(dev);
 	device_enable_async_suspend(b->bridge);
+	pci_set_bus_of_node(b);
 
 	if (!parent)
 		set_dev_node(b->bridge, pcibus_to_node(b));
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 96f70d7..e5111da 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1543,5 +1543,22 @@ int pci_vpd_find_tag(const u8 *buf, unsigned int off, unsigned int len, u8 rdt);
 int pci_vpd_find_info_keyword(const u8 *buf, unsigned int off,
 			      unsigned int len, const char *kw);
 
+/* PCI <-> OF binding helpers */
+#ifdef  CONFIG_OF
+#include <linux/of.h>
+extern void pci_set_of_node(struct pci_dev *dev);
+extern void pci_release_of_node(struct pci_dev *dev);
+extern void pci_set_bus_of_node(struct pci_bus *bus);
+extern void pci_release_bus_of_node(struct pci_bus *bus);
+
+/* Arch may override this (weak) */
+extern struct device_node * __weak pcibios_get_phb_of_node(struct pci_bus *bus);
+
+
+#else /* CONFIG_OF */
+static void pci_locate_of_node(struct pci_dev *dev) { }
+static void pci_release_of_node(struct pci_dev *dev) { }
+#endif  /* CONFIG_OF */
+
 #endif /* __KERNEL__ */
 #endif /* LINUX_PCI_H */

^ permalink raw reply related

* Re: [RFC/PATCH] of: Match PCI devices to OF nodes generically
From: Benjamin Herrenschmidt @ 2011-04-04  7:37 UTC (permalink / raw)
  To: linux-arch
  Cc: linux-pci@vger.kernel.org, linuxppc-dev,
	linux-kernel@vger.kernel.org, David Miller
In-Reply-To: <1301887630.2549.71.camel@pasglop>


> Nice, looks like I forgot to add the new drivers/pci/of.c file :-)
> Here's a new patch. Also added linux-pci to the CC list.

And this one removes a lot more cruft from the powermac code while at
it, and moves the core matching logic to drivers/of/of_pci.c...

>From 917ea61d6afcf443ca467d0a6ffa00d5c6e21bb3 Mon Sep 17 00:00:00 2001
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Mon, 4 Apr 2011 14:38:13 +1000
Subject: [PATCH] of: Match PCI devices to OF nodes dynamically

powerpc has two different ways of matching PCI devices to their
corresponding OF node (if any) for historical reasons. The ppc64 one
does a scan looking for matching bus/dev/fn, while the ppc32 one does a
scan looking only for matching dev/fn on each level in order to be
agnostic to busses being renumbered (which Linux does on some
platforms).

This removes both and instead moves the matching code to the PCI core
itself. It's the most logical place to do it: when a pci_dev is created,
we know the parent and thus can do a single level scan for the matching
device_node (if any).

The benefit is that all archs now get the matching for free. There's one
hook the arch might want to provide to match a PHB bus to its device
node. A default weak implementation is provided that looks for the
parent device device node, but it's not entirely reliable on powerpc for
various reasons so powerpc provides its own.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 arch/powerpc/include/asm/pci-bridge.h |   35 +++------
 arch/powerpc/include/asm/pci.h        |    3 +-
 arch/powerpc/include/asm/prom.h       |   14 ---
 arch/powerpc/kernel/pci-common.c      |   11 ++-
 arch/powerpc/kernel/pci_32.c          |  148 +++------------------------------
 arch/powerpc/kernel/pci_dn.c          |   47 -----------
 arch/powerpc/kernel/pci_of_scan.c     |    9 +--
 arch/powerpc/platforms/powermac/pci.c |    3 +-
 arch/sparc/kernel/pci.c               |    2 +-
 drivers/of/of_pci.c                   |   36 ++++++++
 drivers/pci/Makefile                  |    2 +
 drivers/pci/hotplug/rpadlpar_core.c   |    2 +-
 drivers/pci/of.c                      |   60 +++++++++++++
 drivers/pci/probe.c                   |    8 ++-
 include/linux/of_pci.h                |    5 +
 include/linux/pci.h                   |   17 ++++
 16 files changed, 165 insertions(+), 237 deletions(-)
 create mode 100644 drivers/pci/of.c

diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
index 5e156e0..6912c45 100644
--- a/arch/powerpc/include/asm/pci-bridge.h
+++ b/arch/powerpc/include/asm/pci-bridge.h
@@ -169,18 +169,22 @@ static inline struct pci_controller *pci_bus_to_host(const struct pci_bus *bus)
 	return bus->sysdata;
 }
 
-#ifndef CONFIG_PPC64
+static inline struct device_node *pci_device_to_OF_node(struct pci_dev *dev)
+{
+	return dev->dev.of_node;
+}
 
 static inline struct device_node *pci_bus_to_OF_node(struct pci_bus *bus)
 {
-	struct pci_controller *host;
-
-	if (bus->self)
-		return pci_device_to_OF_node(bus->self);
-	host = pci_bus_to_host(bus);
-	return host ? host->dn : NULL;
+	return bus->dev.of_node;
 }
 
+#ifndef CONFIG_PPC64
+
+extern int pci_device_from_OF_node(struct device_node *node,
+				   u8* bus, u8* devfn);
+extern void pci_create_OF_bus_map(void);
+
 static inline int isa_vaddr_is_ioport(void __iomem *address)
 {
 	/* No specific ISA handling on ppc32 at this stage, it
@@ -223,17 +227,8 @@ struct pci_dn {
 /* Get the pointer to a device_node's pci_dn */
 #define PCI_DN(dn)	((struct pci_dn *) (dn)->data)
 
-extern struct device_node *fetch_dev_dn(struct pci_dev *dev);
 extern void * update_dn_pci_info(struct device_node *dn, void *data);
 
-/* Get a device_node from a pci_dev.  This code must be fast except
- * in the case where the sysdata is incorrect and needs to be fixed
- * up (this will only happen once). */
-static inline struct device_node *pci_device_to_OF_node(struct pci_dev *dev)
-{
-	return dev->dev.of_node ? dev->dev.of_node : fetch_dev_dn(dev);
-}
-
 static inline int pci_device_from_OF_node(struct device_node *np,
 					  u8 *bus, u8 *devfn)
 {
@@ -244,14 +239,6 @@ static inline int pci_device_from_OF_node(struct device_node *np,
 	return 0;
 }
 
-static inline struct device_node *pci_bus_to_OF_node(struct pci_bus *bus)
-{
-	if (bus->self)
-		return pci_device_to_OF_node(bus->self);
-	else
-		return bus->dev.of_node; /* Must be root bus (PHB) */
-}
-
 /** Find the bus corresponding to the indicated device node */
 extern struct pci_bus *pcibios_find_pci_bus(struct device_node *dn);
 
diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h
index 7d77909..1f52268 100644
--- a/arch/powerpc/include/asm/pci.h
+++ b/arch/powerpc/include/asm/pci.h
@@ -179,8 +179,7 @@ extern int remove_phb_dynamic(struct pci_controller *phb);
 extern struct pci_dev *of_create_pci_dev(struct device_node *node,
 					struct pci_bus *bus, int devfn);
 
-extern void of_scan_pci_bridge(struct device_node *node,
-				struct pci_dev *dev);
+extern void of_scan_pci_bridge(struct pci_dev *dev);
 
 extern void of_scan_bus(struct device_node *node, struct pci_bus *bus);
 extern void of_rescan_bus(struct device_node *node, struct pci_bus *bus);
diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
index c189aa5..b823536 100644
--- a/arch/powerpc/include/asm/prom.h
+++ b/arch/powerpc/include/asm/prom.h
@@ -22,20 +22,6 @@
 
 #define HAVE_ARCH_DEVTREE_FIXUPS
 
-#ifdef CONFIG_PPC32
-/*
- * PCI <-> OF matching functions
- * (XXX should these be here?)
- */
-struct pci_bus;
-struct pci_dev;
-extern int pci_device_from_OF_node(struct device_node *node,
-				   u8* bus, u8* devfn);
-extern struct device_node* pci_busdev_to_OF_node(struct pci_bus *, int);
-extern struct device_node* pci_device_to_OF_node(struct pci_dev *);
-extern void pci_create_OF_bus_map(void);
-#endif
-
 /*
  * OF address retreival & translation
  */
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 893af2a..47c516b 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1097,9 +1097,6 @@ void __devinit pcibios_setup_bus_devices(struct pci_bus *bus)
 		if (dev->is_added)
 			continue;
 
-		/* Setup OF node pointer in the device */
-		dev->dev.of_node = pci_device_to_OF_node(dev);
-
 		/* Fixup NUMA node as it may not be setup yet by the generic
 		 * code and is needed by the DMA init
 		 */
@@ -1685,6 +1682,13 @@ int early_find_capability(struct pci_controller *hose, int bus, int devfn,
 	return pci_bus_find_capability(fake_pci_bus(hose, bus), devfn, cap);
 }
 
+struct device_node * pcibios_get_phb_of_node(struct pci_bus *bus)
+{
+	struct pci_controller *hose = bus->sysdata;
+
+	return of_node_get(hose->dn);
+}
+
 /**
  * pci_scan_phb - Given a pci_controller, setup and scan the PCI bus
  * @hose: Pointer to the PCI host controller instance structure
@@ -1705,7 +1709,6 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose)
 			hose->global_number);
 		return;
 	}
-	bus->dev.of_node = of_node_get(node);
 	bus->secondary = hose->first_busno;
 	hose->bus = bus;
 
diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c
index bedb370..ca6bcb7 100644
--- a/arch/powerpc/kernel/pci_32.c
+++ b/arch/powerpc/kernel/pci_32.c
@@ -167,150 +167,26 @@ pcibios_make_OF_bus_map(void)
 #endif
 }
 
-typedef int (*pci_OF_scan_iterator)(struct device_node* node, void* data);
-
-static struct device_node*
-scan_OF_pci_childs(struct device_node *parent, pci_OF_scan_iterator filter, void* data)
-{
-	struct device_node *node;
-	struct device_node* sub_node;
-
-	for_each_child_of_node(parent, node) {
-		const unsigned int *class_code;
-	
-		if (filter(node, data)) {
-			of_node_put(node);
-			return node;
-		}
-
-		/* For PCI<->PCI bridges or CardBus bridges, we go down
-		 * Note: some OFs create a parent node "multifunc-device" as
-		 * a fake root for all functions of a multi-function device,
-		 * we go down them as well.
-		 */
-		class_code = of_get_property(node, "class-code", NULL);
-		if ((!class_code || ((*class_code >> 8) != PCI_CLASS_BRIDGE_PCI &&
-			(*class_code >> 8) != PCI_CLASS_BRIDGE_CARDBUS)) &&
-			strcmp(node->name, "multifunc-device"))
-			continue;
-		sub_node = scan_OF_pci_childs(node, filter, data);
-		if (sub_node) {
-			of_node_put(node);
-			return sub_node;
-		}
-	}
-	return NULL;
-}
-
-static struct device_node *scan_OF_for_pci_dev(struct device_node *parent,
-					       unsigned int devfn)
-{
-	struct device_node *np, *cnp;
-	const u32 *reg;
-	unsigned int psize;
-
-	for_each_child_of_node(parent, np) {
-		reg = of_get_property(np, "reg", &psize);
-                if (reg && psize >= 4 && ((reg[0] >> 8) & 0xff) == devfn)
-			return np;
-
-		/* Note: some OFs create a parent node "multifunc-device" as
-		 * a fake root for all functions of a multi-function device,
-		 * we go down them as well. */
-                if (!strcmp(np->name, "multifunc-device")) {
-                        cnp = scan_OF_for_pci_dev(np, devfn);
-                        if (cnp)
-                                return cnp;
-                }
-	}
-	return NULL;
-}
-
-
-static struct device_node *scan_OF_for_pci_bus(struct pci_bus *bus)
-{
-	struct device_node *parent, *np;
-
-	/* Are we a root bus ? */
-	if (bus->self == NULL || bus->parent == NULL) {
-		struct pci_controller *hose = pci_bus_to_host(bus);
-		if (hose == NULL)
-			return NULL;
-		return of_node_get(hose->dn);
-	}
-
-	/* not a root bus, we need to get our parent */
-	parent = scan_OF_for_pci_bus(bus->parent);
-	if (parent == NULL)
-		return NULL;
-
-	/* now iterate for children for a match */
-	np = scan_OF_for_pci_dev(parent, bus->self->devfn);
-	of_node_put(parent);
-
-	return np;
-}
-
-/*
- * Scans the OF tree for a device node matching a PCI device
- */
-struct device_node *
-pci_busdev_to_OF_node(struct pci_bus *bus, int devfn)
-{
-	struct device_node *parent, *np;
-
-	pr_debug("pci_busdev_to_OF_node(%d,0x%x)\n", bus->number, devfn);
-	parent = scan_OF_for_pci_bus(bus);
-	if (parent == NULL)
-		return NULL;
-	pr_debug(" parent is %s\n", parent ? parent->full_name : "<NULL>");
-	np = scan_OF_for_pci_dev(parent, devfn);
-	of_node_put(parent);
-	pr_debug(" result is %s\n", np ? np->full_name : "<NULL>");
-
-	/* XXX most callers don't release the returned node
-	 * mostly because ppc64 doesn't increase the refcount,
-	 * we need to fix that.
-	 */
-	return np;
-}
-EXPORT_SYMBOL(pci_busdev_to_OF_node);
-
-struct device_node*
-pci_device_to_OF_node(struct pci_dev *dev)
-{
-	return pci_busdev_to_OF_node(dev->bus, dev->devfn);
-}
-EXPORT_SYMBOL(pci_device_to_OF_node);
-
-static int
-find_OF_pci_device_filter(struct device_node* node, void* data)
-{
-	return ((void *)node == data);
-}
 
 /*
  * Returns the PCI device matching a given OF node
  */
-int
-pci_device_from_OF_node(struct device_node* node, u8* bus, u8* devfn)
+int pci_device_from_OF_node(struct device_node* node, u8* bus, u8* devfn)
 {
-	const unsigned int *reg;
-	struct pci_controller* hose;
 	struct pci_dev* dev = NULL;
-	
-	/* Make sure it's really a PCI device */
-	hose = pci_find_hose_for_OF_device(node);
-	if (!hose || !hose->dn)
-		return -ENODEV;
-	if (!scan_OF_pci_childs(hose->dn,
-			find_OF_pci_device_filter, (void *)node))
+	const __be32 *reg;
+	int size;
+
+	/* Check if it might have a chance to be a PCI device */
+	if (!pci_find_hose_for_OF_device(node))
 		return -ENODEV;
-	reg = of_get_property(node, "reg", NULL);
-	if (!reg)
+
+	reg = of_get_property(node, "reg", &size);
+	if (!reg || size < 5 * sizeof(u32))
 		return -ENODEV;
-	*bus = (reg[0] >> 16) & 0xff;
-	*devfn = ((reg[0] >> 8) & 0xff);
+	
+	*bus = (be32_to_cpup(&reg[0]) >> 16) & 0xff;
+	*devfn = (be32_to_cpup(&reg[0]) >> 8) & 0xff;
 
 	/* Ok, here we need some tweak. If we have already renumbered
 	 * all busses, we can't rely on the OF bus number any more.
diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index d225d99..8cb66a2 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -143,53 +143,6 @@ void __devinit pci_devs_phb_init_dynamic(struct pci_controller *phb)
 	traverse_pci_devices(dn, update_dn_pci_info, phb);
 }
 
-/*
- * Traversal func that looks for a <busno,devfcn> value.
- * If found, the pci_dn is returned (thus terminating the traversal).
- */
-static void *is_devfn_node(struct device_node *dn, void *data)
-{
-	int busno = ((unsigned long)data >> 8) & 0xff;
-	int devfn = ((unsigned long)data) & 0xff;
-	struct pci_dn *pci = dn->data;
-
-	if (pci && (devfn == pci->devfn) && (busno == pci->busno))
-		return dn;
-	return NULL;
-}
-
-/*
- * This is the "slow" path for looking up a device_node from a
- * pci_dev.  It will hunt for the device under its parent's
- * phb and then update of_node pointer.
- *
- * It may also do fixups on the actual device since this happens
- * on the first read/write.
- *
- * Note that it also must deal with devices that don't exist.
- * In this case it may probe for real hardware ("just in case")
- * and add a device_node to the device tree if necessary.
- *
- * Is this function necessary anymore now that dev->dev.of_node is
- * used to store the node pointer?
- *
- */
-struct device_node *fetch_dev_dn(struct pci_dev *dev)
-{
-	struct pci_controller *phb = dev->sysdata;
-	struct device_node *dn;
-	unsigned long searchval = (dev->bus->number << 8) | dev->devfn;
-
-	if (WARN_ON(!phb))
-		return NULL;
-
-	dn = traverse_pci_devices(phb->dn, is_devfn_node, (void *)searchval);
-	if (dn)
-		dev->dev.of_node = dn;
-	return dn;
-}
-EXPORT_SYMBOL(fetch_dev_dn);
-
 /** 
  * pci_devs_phb_init - Initialize phbs and pci devs under them.
  * 
diff --git a/arch/powerpc/kernel/pci_of_scan.c b/arch/powerpc/kernel/pci_of_scan.c
index 1e89a72..fe0a5ad 100644
--- a/arch/powerpc/kernel/pci_of_scan.c
+++ b/arch/powerpc/kernel/pci_of_scan.c
@@ -202,9 +202,9 @@ EXPORT_SYMBOL(of_create_pci_dev);
  * this routine in turn call of_scan_bus() recusively to scan for more child
  * devices.
  */
-void __devinit of_scan_pci_bridge(struct device_node *node,
-				  struct pci_dev *dev)
+void __devinit of_scan_pci_bridge(struct pci_dev *dev)
 {
+	struct device_node *node = dev->dev.of_node;
 	struct pci_bus *bus;
 	const u32 *busrange, *ranges;
 	int len, i, mode;
@@ -238,7 +238,6 @@ void __devinit of_scan_pci_bridge(struct device_node *node,
 	bus->primary = dev->bus->number;
 	bus->subordinate = busrange[1];
 	bus->bridge_ctl = 0;
-	bus->dev.of_node = of_node_get(node);
 
 	/* parse ranges property */
 	/* PCI #address-cells == 3 and #size-cells == 2 always */
@@ -335,9 +334,7 @@ static void __devinit __of_scan_bus(struct device_node *node,
 	list_for_each_entry(dev, &bus->devices, bus_list) {
 		if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||
 		    dev->hdr_type == PCI_HEADER_TYPE_CARDBUS) {
-			struct device_node *child = pci_device_to_OF_node(dev);
-			if (child)
-				of_scan_pci_bridge(child, dev);
+			of_scan_pci_bridge(dev);
 		}
 	}
 }
diff --git a/arch/powerpc/platforms/powermac/pci.c b/arch/powerpc/platforms/powermac/pci.c
index ab68989..68a18da 100644
--- a/arch/powerpc/platforms/powermac/pci.c
+++ b/arch/powerpc/platforms/powermac/pci.c
@@ -17,6 +17,7 @@
 #include <linux/init.h>
 #include <linux/bootmem.h>
 #include <linux/irq.h>
+#include <linux/of_pci.h>
 
 #include <asm/sections.h>
 #include <asm/io.h>
@@ -235,7 +236,7 @@ static int chaos_validate_dev(struct pci_bus *bus, int devfn, int offset)
 
 	if (offset >= 0x100)
 		return  PCIBIOS_BAD_REGISTER_NUMBER;
-	np = pci_busdev_to_OF_node(bus, devfn);
+	np = of_pci_find_child_device(bus->dev.of_node, devfn);
 	if (np == NULL)
 		return PCIBIOS_DEVICE_NOT_FOUND;
 
diff --git a/arch/sparc/kernel/pci.c b/arch/sparc/kernel/pci.c
index 713dc91..e539d23 100644
--- a/arch/sparc/kernel/pci.c
+++ b/arch/sparc/kernel/pci.c
@@ -284,7 +284,7 @@ static struct pci_dev *of_create_pci_dev(struct pci_pbm_info *pbm,
 	dev->sysdata = node;
 	dev->dev.parent = bus->bridge;
 	dev->dev.bus = &pci_bus_type;
-	dev->dev.of_node = node;
+	dev->dev.of_node = of_node_get(node);
 	dev->devfn = devfn;
 	dev->multifunction = 0;		/* maybe a lie? */
 	set_pcie_port_type(dev);
diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
index ac1ec54..9d179c4 100644
--- a/drivers/of/of_pci.c
+++ b/drivers/of/of_pci.c
@@ -90,3 +90,39 @@ int of_irq_map_pci(struct pci_dev *pdev, struct of_irq *out_irq)
 	return of_irq_map_raw(ppnode, &lspec_be, 1, laddr, out_irq);
 }
 EXPORT_SYMBOL_GPL(of_irq_map_pci);
+
+
+static inline int __of_pci_pci_compare(struct device_node *node, unsigned int devfn)
+{
+	unsigned int size;
+	const __be32 *reg = of_get_property(node, "reg", &size);
+
+	if (!reg || size < 5 * sizeof(__be32))
+		return 0;
+	return ((be32_to_cpup(&reg[0]) >> 8) & 0xff) == devfn;
+}
+
+struct device_node *of_pci_find_child_device(struct device_node *parent,
+					     unsigned int devfn)
+{
+	struct device_node *node, *node2;
+	
+	for_each_child_of_node(parent, node) {
+		if (__of_pci_pci_compare(node, devfn))
+			return node;
+		/*
+		 * Some OFs create a parent node "multifunc-device" as
+		 * a fake root for all functions of a multi-function
+		 * device we go down them as well.
+		 */
+                if (!strcmp(node->name, "multifunc-device")) {
+			for_each_child_of_node(node, node2) {
+				if (__of_pci_pci_compare(node2, devfn)) {
+					of_node_put(node);
+					return node2;
+				}
+			}
+                }
+	}
+	return NULL;
+}
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
index 98d61c8..d5c3cb9 100644
--- a/drivers/pci/Makefile
+++ b/drivers/pci/Makefile
@@ -70,4 +70,6 @@ obj-$(CONFIG_PCI_STUB) += pci-stub.o
 
 obj-$(CONFIG_XEN_PCIDEV_FRONTEND) += xen-pcifront.o
 
+obj-$(CONFIG_OF) += of.o
+
 ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG
diff --git a/drivers/pci/hotplug/rpadlpar_core.c b/drivers/pci/hotplug/rpadlpar_core.c
index 0830347..1d002b1 100644
--- a/drivers/pci/hotplug/rpadlpar_core.c
+++ b/drivers/pci/hotplug/rpadlpar_core.c
@@ -158,7 +158,7 @@ static void dlpar_pci_add_bus(struct device_node *dn)
 	/* Scan below the new bridge */
 	if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||
 	    dev->hdr_type == PCI_HEADER_TYPE_CARDBUS)
-		of_scan_pci_bridge(dn, dev);
+		of_scan_pci_bridge(dev);
 
 	/* Map IO space for child bus, which may or may not succeed */
 	pcibios_map_io_space(dev->subordinate);
diff --git a/drivers/pci/of.c b/drivers/pci/of.c
new file mode 100644
index 0000000..fff7270
--- /dev/null
+++ b/drivers/pci/of.c
@@ -0,0 +1,60 @@
+/*
+ * PCI <-> OF mapping helpers
+ *
+ * Copyright 2011 IBM Corp.
+ * 
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include <linux/kernel.h>
+#include <linux/pci.h>
+#include <linux/of_pci.h>
+#include "pci.h"
+
+void pci_set_of_node(struct pci_dev *dev)
+{
+	if (!dev->bus->dev.of_node)
+		return;
+	dev->dev.of_node = of_pci_find_child_device(dev->bus->dev.of_node,
+						    dev->devfn);
+}
+
+void pci_release_of_node(struct pci_dev *dev)
+{
+	of_node_put(dev->dev.of_node);
+	dev->dev.of_node = NULL;
+}
+
+void pci_set_bus_of_node(struct pci_bus *bus)
+{
+	if (bus->self == NULL)
+		bus->dev.of_node = pcibios_get_phb_of_node(bus);
+	else
+		bus->dev.of_node = of_node_get(bus->self->dev.of_node);
+}
+
+void pci_release_bus_of_node(struct pci_bus *bus)
+{
+	of_node_put(bus->dev.of_node);
+	bus->dev.of_node = NULL;
+}
+
+struct device_node * __weak pcibios_get_phb_of_node(struct pci_bus *bus)
+{
+	/* This should only be called for PHBs */
+	if (WARN_ON(bus->self || bus->parent))
+		return NULL;
+
+	/* Look for a node pointer in either the intermediary device we
+	 * create above the root bus or it's own parent. Normally only
+	 * the later is populated.
+	 */
+	if (bus->bridge->of_node)
+		return of_node_get(bus->bridge->of_node);
+	if (bus->bridge->parent->of_node)
+		return of_node_get(bus->bridge->parent->of_node);
+	return NULL;
+}
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 44cbbba..347349b 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -89,6 +89,7 @@ static void release_pcibus_dev(struct device *dev)
 	if (pci_bus->bridge)
 		put_device(pci_bus->bridge);
 	pci_bus_remove_resources(pci_bus);
+	pci_release_bus_of_node(pci_bus);
 	kfree(pci_bus);
 }
 
@@ -624,7 +625,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
 
 	child->self = bridge;
 	child->bridge = get_device(&bridge->dev);
-
+	pci_set_bus_of_node(child);	
 	pci_set_bus_speed(child);
 
 	/* Set up default resource pointers and names.. */
@@ -1074,6 +1075,7 @@ static void pci_release_dev(struct device *dev)
 
 	pci_dev = to_pci_dev(dev);
 	pci_release_capabilities(pci_dev);
+	pci_release_of_node(pci_dev);
 	kfree(pci_dev);
 }
 
@@ -1150,6 +1152,7 @@ struct pci_dev *alloc_pci_dev(void)
 }
 EXPORT_SYMBOL(alloc_pci_dev);
 
+
 /*
  * Read the config data for a PCI device, sanity-check it
  * and fill in the dev structure...
@@ -1193,6 +1196,8 @@ static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn)
 	dev->vendor = l & 0xffff;
 	dev->device = (l >> 16) & 0xffff;
 
+	pci_set_of_node(dev);
+
 	if (pci_setup_device(dev)) {
 		kfree(dev);
 		return NULL;
@@ -1445,6 +1450,7 @@ struct pci_bus * pci_create_bus(struct device *parent,
 		goto dev_reg_err;
 	b->bridge = get_device(dev);
 	device_enable_async_suspend(b->bridge);
+	pci_set_bus_of_node(b);
 
 	if (!parent)
 		set_dev_node(b->bridge, pcibus_to_node(b));
diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
index 85a27b6..f93e217 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -6,4 +6,9 @@
 struct pci_dev;
 struct of_irq;
 int of_irq_map_pci(struct pci_dev *pdev, struct of_irq *out_irq);
+
+struct device_node;
+struct device_node *of_pci_find_child_device(struct device_node *parent,
+					     unsigned int devfn);
+
 #endif
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 96f70d7..e5111da 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1543,5 +1543,22 @@ int pci_vpd_find_tag(const u8 *buf, unsigned int off, unsigned int len, u8 rdt);
 int pci_vpd_find_info_keyword(const u8 *buf, unsigned int off,
 			      unsigned int len, const char *kw);
 
+/* PCI <-> OF binding helpers */
+#ifdef  CONFIG_OF
+#include <linux/of.h>
+extern void pci_set_of_node(struct pci_dev *dev);
+extern void pci_release_of_node(struct pci_dev *dev);
+extern void pci_set_bus_of_node(struct pci_bus *bus);
+extern void pci_release_bus_of_node(struct pci_bus *bus);
+
+/* Arch may override this (weak) */
+extern struct device_node * __weak pcibios_get_phb_of_node(struct pci_bus *bus);
+
+
+#else /* CONFIG_OF */
+static void pci_locate_of_node(struct pci_dev *dev) { }
+static void pci_release_of_node(struct pci_dev *dev) { }
+#endif  /* CONFIG_OF */
+
 #endif /* __KERNEL__ */
 #endif /* LINUX_PCI_H */

^ permalink raw reply related

* Re: [RFC/PATCH] of: Match PCI devices to OF nodes generically
From: David Miller @ 2011-04-04  7:48 UTC (permalink / raw)
  To: benh; +Cc: linux-arch, linux-pci, linuxppc-dev, linux-kernel
In-Reply-To: <1301887630.2549.71.camel@pasglop>

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Mon, 04 Apr 2011 13:27:10 +1000

> +struct device_node * __weak pcibios_get_phb_of_node(struct pci_bus *bus)
> +{
> +	/* This should only be called for PHBs */
> +	if (WARN_ON(bus->self || bus->parent))
> +		return NULL;

This WARN_ON() will always trigger on sparc, because we use the OF
device tree object at the "parent" of the PCI bus devices we create
for the PCI controller domains.

I'm really surprised you don't link the PCI bus roots into the rest of
the global device hierarchy on powerpc.

^ permalink raw reply

* Re: [PATCH v13 02/10] USB/ppc4xx: Add Synopsys DWC OTG driver framework
From: Felipe Balbi @ 2011-04-04 12:19 UTC (permalink / raw)
  To: tmarri; +Cc: Mark Miesfeld, greg, linux-usb, linuxppc-dev, Fushen Chen
In-Reply-To: <1301872610-2785-1-git-send-email-tmarri@apm.com>

On Sun, Apr 03, 2011 at 04:16:50PM -0700, tmarri@apm.com wrote:
> From: Tirumala Marri <tmarri@apm.com>
> 
> Platform probing is in dwc_otg_apmppc.c.
> Driver parameter and parameter checking are in dwc_otg_param.c.
> 
> Signed-off-by: Tirumala R Marri <tmarri@apm.com>
> Signed-off-by: Fushen Chen <fchen@apm.com>
> Signed-off-by: Mark Miesfeld <mmiesfeld@apm.com>
> ---
>  drivers/usb/dwc/apmppc.c |  414 ++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/usb/dwc/driver.h |   76 +++++++++
>  drivers/usb/dwc/param.c  |  180 ++++++++++++++++++++
>  3 files changed, 670 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/usb/dwc/apmppc.c
>  create mode 100644 drivers/usb/dwc/driver.h
>  create mode 100644 drivers/usb/dwc/param.c
> 
> diff --git a/drivers/usb/dwc/apmppc.c b/drivers/usb/dwc/apmppc.c
> new file mode 100644
> index 0000000..ffbe6dd
> --- /dev/null
> +++ b/drivers/usb/dwc/apmppc.c
> @@ -0,0 +1,414 @@
> +/*
> + * DesignWare HS OTG controller driver
> + * Copyright (C) 2006 Synopsys, Inc.
> + * Portions Copyright (C) 2010 Applied Micro Circuits Corporation.
> + *
> + * This program is free software: you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License version 2 for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see http://www.gnu.org/licenses
> + * or write to the Free Software Foundation, Inc., 51 Franklin Street,
> + * Suite 500, Boston, MA 02110-1335 USA.
> + *
> + * Based on Synopsys driver version 2.60a
> + * Modified by Mark Miesfeld <mmiesfeld@apm.com>
> + * Modified by Stefan Roese <sr@denx.de>, DENX Software Engineering
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING BUT NOT LIMITED TO THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL SYNOPSYS, INC. BE LIABLE FOR ANY DIRECT,
> + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY OR CONSEQUENTIAL DAMAGES
> + * (INCLUDING BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
> + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + */
> +
> +/*
> + * The dwc_otg module provides the initialization and cleanup entry
> + * points for the dwcotg driver. This module will be dynamically installed
> + * after Linux is booted using the insmod command. When the module is
> + * installed, the dwc_otg_driver_init function is called. When the module is
> + * removed (using rmmod), the dwc_otg_driver_cleanup function is called.
> + *
> + * This module also defines a data structure for the dwc_otg driver, which is
> + * used in conjunction with the standard device structure. These
> + * structures allow the OTG driver to comply with the standard Linux driver
> + * model in which devices and drivers are registered with a bus driver. This
> + * has the benefit that Linux can expose attributes of the driver and device
> + * in its special sysfs file system. Users can then read or write files in
> + * this file system to perform diagnostics on the driver components or the
> + * device.
> + */
> +
> +#include <linux/of_platform.h>
> +
> +#include "driver.h"
> +
> +#define DWC_DRIVER_VERSION		"1.05"
> +#define DWC_DRIVER_DESC			"HS OTG USB Controller driver"
> +static const char dwc_driver_name[] = "dwc_otg";
> +
> +/**
> + * This function is the top level interrupt handler for the Common
> + * (Device and host modes) interrupts.
> + */
> +static irqreturn_t dwc_otg_common_irq(int _irq, void *dev)
> +{
> +	struct dwc_otg_device *dwc_dev = dev;
> +	int retval;
> +
> +	retval = dwc_otg_handle_common_intr(dwc_dev->core_if);

no lock needed ? What if you get another IRQ while you're calling
dwc_otg_handle_common_intr() ??

> +	return IRQ_RETVAL(retval);
> +}
> +
> +/**
> + * This function is the interrupt handler for the OverCurrent condition
> + * from the external charge pump (if enabled)
> + */

this comment is completely unnecessary. Same goes for most all other
functions. The ones which would be nice to keep, should be converted to
kerneldoc so we can extract documentation from source code if we
need/want.

> +static irqreturn_t dwc_otg_externalchgpump_irq(int _irq, void *dev)
> +{
> +	struct dwc_otg_device *dwc_dev = dev;
> +
> +	if (dwc_otg_is_host_mode(dwc_dev->core_if)) {
> +		struct dwc_hcd *dwc_hcd;
> +		u32 hprt0 = 0;
> +
> +		dwc_hcd = dwc_dev->hcd;
> +		spin_lock(&dwc_hcd->lock);
> +		dwc_hcd->flags.b.port_over_current_change = 1;
> +
> +		hprt0 = DWC_HPRT0_PRT_PWR_RW(hprt0, 0);
> +		dwc_write32(dwc_dev->core_if->host_if->hprt0, hprt0);
> +		spin_unlock(&dwc_hcd->lock);
> +	} else {
> +		/* Device mode - This int is n/a for device mode */
> +		dev_dbg(dev, "DeviceMode: OTG OverCurrent Detected\n");
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +/**
> + * This function is called when a device is unregistered with the
> + * dwc_otg_driver. This happens, for example, when the rmmod command is
> + * executed. The device may or may not be electrically present. If it is
> + * present, the driver stops device processing. Any resources used on behalf
> + * of this device are freed.
> + */
> +static int __devexit dwc_otg_driver_remove(struct platform_device *ofdev)
> +{
> +	struct device *dev = &ofdev->dev;
> +	struct dwc_otg_device *dwc_dev = dev_get_drvdata(dev);
> +
> +	/* Memory allocation for dwc_otg_device may have failed. */
> +	if (!dwc_dev)
> +		return 0;

this should never happen. Remove this check. If memory allocation
failed, you didn't probe() anyway.

> +	/* Free the IRQ */
> +	if (dwc_dev->common_irq_installed)
> +		free_irq(dwc_dev->irq, dwc_dev);

you know you allocate the IRQ on probe()

> +	if (!dwc_has_feature(dwc_dev->core_if, DWC_DEVICE_ONLY)) {
> +		if (dwc_dev->hcd) {
> +			if (dwc_dev->hcd->cp_irq_installed)
> +				free_irq(dwc_dev->hcd->cp_irq, dwc_dev);
> +			dwc_otg_hcd_remove(dev);
> +		}
> +	}

the feature check could probably be dropped. I'm sure you only
initialize hcd to something other than NULL if DWC_DEVICE_ONLY is false.
Also, you know that IRQ will be requested, so drop the cp_irq_installed
flag and check.

> +	if (!dwc_has_feature(dwc_dev->core_if, DWC_HOST_ONLY)) {
> +		if (dwc_dev->pcd)
> +			dwc_otg_pcd_remove(dev);
> +	}

same here.

> +	if (dwc_dev->core_if)
> +		dwc_otg_cil_remove(dwc_dev->core_if);
> +
> +	/* Return the memory. */
> +	if (dwc_dev->base)
> +		iounmap(dwc_dev->base);

this should be valid pointer otherwise your driver doesn't work, right ?
Drop the check and ensure base is valid.

> +	if (dwc_dev->phys_addr)
> +		release_mem_region(dwc_dev->phys_addr, dwc_dev->base_len);

same here.

> +	if (dwc_dev->core_if->xceiv) {
> +		otg_put_transceiver(dwc_dev->core_if->xceiv);
> +		dwc_dev->core_if->xceiv = NULL;
> +		usb_nop_xceiv_unregister();
> +	}
> +
> +	kfree(dwc_dev);
> +
> +	/* Clear the drvdata pointer. */
> +	dev_set_drvdata(dev, NULL);
> +	return 0;
> +}
> +
> +/**
> + * This function is called when an device is bound to a
> + * dwc_otg_driver. It creates the driver components required to
> + * control the device (CIL, HCD, and PCD) and it initializes the
> + * device. The driver components are stored in a dwc_otg_device
> + * structure. A reference to the dwc_otg_device is saved in the
> + * device. This allows the driver to access the dwc_otg_device
> + * structure on subsequent calls to driver methods for this device.
> + */
> +static int __devinit dwc_otg_driver_probe(struct platform_device *ofdev)
> +{
> +	int retval;
> +	struct dwc_otg_device *dwc_dev;
> +	struct device *dev = &ofdev->dev;
> +	struct resource res;
> +	ulong gusbcfg_addr;
> +	u32 usbcfg = 0;
> +
> +	dev_dbg(dev, "dwc_otg_driver_probe(%p)\n", dev);

not needed. Either dropo or move to dev_vdbg().

> +	dwc_dev = kzalloc(sizeof(*dwc_dev), GFP_KERNEL);
> +	if (!dwc_dev) {
> +		dev_err(dev, "kmalloc of dwc_otg_device failed\n");
> +		retval = -ENOMEM;
> +		goto fail_dwc_dev;
> +	}
> +
> +	/* Retrieve the memory and IRQ resources. */
> +	dwc_dev->irq = irq_of_parse_and_map(ofdev->dev.of_node, 0);
> +	if (dwc_dev->irq == NO_IRQ) {
> +		dev_err(dev, "no device irq\n");
> +		retval = -ENODEV;
> +		goto fail_of_irq;
> +	}
> +	dev_dbg(dev, "OTG - device irq: %d\n", dwc_dev->irq);

unneeded print.

> +	if (of_address_to_resource(ofdev->dev.of_node, 0, &res)) {
> +		dev_err(dev, "%s: Can't get USB-OTG register address\n",
> +			__func__);
> +		retval = -ENOMEM;
> +		goto fail_of_irq;
> +	}
> +	dev_dbg(dev, "OTG - ioresource_mem start0x%llx: end:0x%llx\n",
> +		(unsigned long long)res.start, (unsigned long long)res.end);

unneeded print.

> +	dwc_dev->phys_addr = res.start;
> +	dwc_dev->base_len = res.end - res.start + 1;
> +	if (!request_mem_region(dwc_dev->phys_addr,
> +				dwc_dev->base_len, dwc_driver_name)) {
> +		dev_err(dev, "request_mem_region failed\n");
> +		retval = -EBUSY;
> +		goto fail_of_irq;
> +	}
> +
> +	/* Map the DWC_otg Core memory into virtual address space. */
> +	dwc_dev->base = ioremap(dwc_dev->phys_addr, dwc_dev->base_len);
> +	if (!dwc_dev->base) {
> +		dev_err(dev, "ioremap() failed\n");
> +		retval = -ENOMEM;
> +		goto fail_ioremap;
> +	}
> +	dev_dbg(dev, "mapped base=0x%08x\n", (__force u32)dwc_dev->base);

unneeded print.

> +	/*
> +	 * Initialize driver data to point to the global DWC_otg
> +	 * Device structure.
> +	 */
> +	dev_set_drvdata(dev, dwc_dev);
> +
> +	dwc_dev->core_if =
> +	    dwc_otg_cil_init(dwc_dev->base, &dwc_otg_module_params);
> +	if (!dwc_dev->core_if) {
> +		dev_err(dev, "CIL initialization failed!\n");
> +		retval = -ENOMEM;
> +		goto fail_cil_init;
> +	}
> +
> +	/*
> +	 * Validate parameter values after dwc_otg_cil_init.
> +	 */
> +	if (check_parameters(dwc_dev->core_if)) {
> +		retval = -EINVAL;
> +		goto fail_check_param;
> +	}
> +
> +	usb_nop_xceiv_register();

not here. Other boards might use a different transceiver, and you simply
can't couple this driver to the nop xceiv.

> +	dwc_dev->core_if->xceiv = otg_get_transceiver();
> +	if (!dwc_dev->core_if->xceiv) {
> +		retval = -ENODEV;
> +		goto fail_xceiv;
> +	}
> +	dwc_set_feature(dwc_dev->core_if);
> +
> +	/* Initialize the DWC_otg core. */
> +	dwc_otg_core_init(dwc_dev->core_if);
> +
> +	/*
> +	 * Disable the global interrupt until all the interrupt
> +	 * handlers are installed.
> +	 */
> +	dwc_otg_disable_global_interrupts(dwc_dev->core_if);
> +
> +	/*
> +	 * Install the interrupt handler for the common interrupts before
> +	 * enabling common interrupts in core_init below.
> +	 */
> +	retval = request_irq(dwc_dev->irq, dwc_otg_common_irq,
> +			     IRQF_SHARED, "dwc_otg", dwc_dev);

do you always want shared ? What about other boards which don't want
shared ? How will they re-use this driver ?

> +	if (retval) {
> +		dev_err(dev, "request of irq%d failed retval: %d\n",
> +			dwc_dev->irq, retval);
> +		retval = -EBUSY;
> +		goto fail_req_irq;
> +	} else {
> +		dwc_dev->common_irq_installed = 1;

drop this flag.

> +	}
> +
> +	if (!dwc_has_feature(dwc_dev->core_if, DWC_HOST_ONLY)) {

I don't like the inversion of the features here. Why didn't you make:

if (dwc_has_feature(dwc_dev->core_if, DWC_FEATURE_DEVICE) {
	....

the way you did is always inverted.

> +		/* Initialize the PCD */
> +		retval = dwc_otg_pcd_init(dev);
> +		if (retval) {
> +			dev_err(dev, "dwc_otg_pcd_init failed\n");
> +			dwc_dev->pcd = NULL;
> +			goto fail_req_irq;
> +		}
> +	}
> +
> +	gusbcfg_addr = (ulong) (dwc_dev->core_if->core_global_regs)
> +		+ DWC_GUSBCFG;

why didn't you use void __iomem * ??

> +	if (!dwc_has_feature(dwc_dev->core_if, DWC_DEVICE_ONLY)) {
> +		/* Initialize the HCD and force_host_mode */
> +		usbcfg = dwc_read32(gusbcfg_addr);

the way this is generally handled is to pass two parameters to
dwc_read32(), the base and the offset...

> +		usbcfg |= DWC_USBCFG_FRC_HST_MODE;
> +		dwc_write32(gusbcfg_addr, usbcfg);

... and three parameters here: base, offset and value to write.

> +		retval = dwc_otg_hcd_init(dev, dwc_dev);
> +		if (retval) {
> +			dev_err(dev, "dwc_otg_hcd_init failed\n");
> +			dwc_dev->hcd = NULL;
> +			goto fail_hcd;
> +		}
> +		/* configure chargepump interrupt */
> +		dwc_dev->hcd->cp_irq = irq_of_parse_and_map(ofdev->dev.of_node,
> +							    3);
> +		if (dwc_dev->hcd->cp_irq) {
> +			retval = request_irq(dwc_dev->hcd->cp_irq,
> +					     dwc_otg_externalchgpump_irq,
> +					     IRQF_SHARED,
> +					     "dwc_otg_ext_chg_pump", dwc_dev);
> +			if (retval) {
> +				dev_err(dev,
> +					"request of irq failed retval: %d\n",
> +					retval);
> +				retval = -EBUSY;
> +				goto fail_hcd;
> +			} else {
> +				dev_dbg(dev, "%s: ExtChgPump Detection "
> +					"IRQ registered\n", dwc_driver_name);

unneded print.

> +				dwc_dev->hcd->cp_irq_installed = 1;

unneeded flag.

> +			}
> +		}
> +	}
> +	/*
> +	 * Enable the global interrupt after all the interrupt
> +	 * handlers are installed.
> +	 */
> +	dwc_otg_enable_global_interrupts(dwc_dev->core_if);
> +
> +	usbcfg = dwc_read32(gusbcfg_addr);
> +	usbcfg &= ~DWC_USBCFG_FRC_HST_MODE;
> +	dwc_write32(gusbcfg_addr, usbcfg);
> +
> +	return 0;
> +fail_hcd:
> +	free_irq(dwc_dev->irq, dwc_dev);
> +	if (!dwc_has_feature(dwc_dev->core_if, DWC_HOST_ONLY)) {
> +		if (dwc_dev->pcd)
> +			dwc_otg_pcd_remove(dev);
> +	}
> +fail_req_irq:
> +	otg_put_transceiver(dwc_dev->core_if->xceiv);
> +fail_xceiv:
> +	usb_nop_xceiv_unregister();
> +fail_check_param:
> +	dwc_otg_cil_remove(dwc_dev->core_if);
> +fail_cil_init:
> +	dev_set_drvdata(dev, NULL);
> +	iounmap(dwc_dev->base);
> +fail_ioremap:
> +	release_mem_region(dwc_dev->phys_addr, dwc_dev->base_len);
> +fail_of_irq:
> +	kfree(dwc_dev);
> +fail_dwc_dev:
> +	return retval;
> +}
> +
> +/*
> + * This structure defines the methods to be called by a bus driver
> + * during the lifecycle of a device on that bus. Both drivers and
> + * devices are registered with a bus driver. The bus driver matches
> + * devices to drivers based on information in the device and driver
> + * structures.
> + *
> + * The probe function is called when the bus driver matches a device
> + * to this driver. The remove function is called when a device is
> + * unregistered with the bus driver.
> + */

unneeded comment.

> +static const struct of_device_id dwc_otg_match[] = {
> +	{.compatible = "amcc,dwc-otg",},
> +	{}
> +};
> +
> +MODULE_DEVICE_TABLE(of, dwc_otg_match);
> +
> +static struct platform_driver dwc_otg_driver = {
> +	.probe = dwc_otg_driver_probe,
> +	.remove = __devexit_p(dwc_otg_driver_remove),
> +	.driver = {
> +		   .name = "dwc_otg",
> +		   .owner = THIS_MODULE,
> +		   .of_match_table = dwc_otg_match,
> +		   },

wrong indentation.

> +};
> +
> +/**
> + * This function is called when the dwc_otg_driver is installed with the
> + * insmod command. It registers the dwc_otg_driver structure with the
> + * appropriate bus driver. This will cause the dwc_otg_driver_probe function
> + * to be called. In addition, the bus driver will automatically expose
> + * attributes defined for the device and driver in the special sysfs file
> + * system.
> + */
> +static int __init dwc_otg_driver_init(void)
> +{
> +
> +	pr_info("%s: version %s\n", dwc_driver_name, DWC_DRIVER_VERSION);

drop this pr_info(), it just makes things noisier.

-- 
balbi

^ permalink raw reply

* Re: [PATCH v13 03/10] USB/ppc4xx: Add Synopsys DWC OTG Core Interface Layer (CIL)
From: Felipe Balbi @ 2011-04-04 13:43 UTC (permalink / raw)
  To: tmarri; +Cc: Mark Miesfeld, greg, linux-usb, linuxppc-dev, Fushen Chen
In-Reply-To: <1301872613-2825-1-git-send-email-tmarri@apm.com>

Hi,

On Sun, Apr 03, 2011 at 04:16:53PM -0700, tmarri@apm.com wrote:
> +const char *op_state_str(enum usb_otg_state state)

please don't introduce yet another version of this. Make the ones
available generic. Place a static inline const char
*otg_state_string(struct otg_transceiver *x) on include/linux/usb/otg.h

> +/**
> + * This function enables the controller's Global Interrupt in the AHB Config
> + * register.
> + */

useless comments :-) or convert to kerneldoc.

-- 
balbi

^ permalink raw reply

* Re: [PATCH v13 07/10] USB/ppc4xx: Add Synopsys DWC OTG PCD function
From: Felipe Balbi @ 2011-04-04 13:54 UTC (permalink / raw)
  To: tmarri; +Cc: Mark Miesfeld, greg, linux-usb, linuxppc-dev, Fushen Chen
In-Reply-To: <1301872630-2980-1-git-send-email-tmarri@apm.com>

Hi,

On Sun, Apr 03, 2011 at 04:17:10PM -0700, tmarri@apm.com wrote:
> +static const struct usb_gadget_ops dwc_otg_pcd_ops = {
> +	.get_frame = dwc_otg_pcd_get_frame,
> +	.wakeup = dwc_otg_pcd_wakeup,
> +	/* not selfpowered */

will you really have a board running this top operating system which is
Bus powered ? Unless you're forcing all your boards to come with USB
charging I guess you should implement set_selfpowered.


-- 
balbi

^ permalink raw reply

* Re: [PATCH v13 09/10] USB ppc4xx: Add Synopsys DWC OTG driver kernel configuration and Makefile
From: Felipe Balbi @ 2011-04-04 13:59 UTC (permalink / raw)
  To: tmarri; +Cc: Mark Miesfeld, greg, linux-usb, linuxppc-dev, Fushen Chen
In-Reply-To: <1301872644-3074-1-git-send-email-tmarri@apm.com>

On Sun, Apr 03, 2011 at 04:17:24PM -0700, tmarri@apm.com wrote:
> From: Tirumala Marri <tmarri@apm.com>
> 
> Add Synopsys DesignWare HS USB OTG driver kernel configuration.
> Synopsys OTG driver may operate in  host only, device only, or OTG mode.
> The driver also allows user configure the core to use its internal DMA
> or Slave (PIO) mode.
> 
> Signed-off-by: Tirumala R Marri <tmarri@apm.com>
> Signed-off-by: Fushen Chen <fchen@apm.com>
> Signed-off-by: Mark Miesfeld <mmiesfeld@apm.com>
> ---
>  drivers/Makefile         |    1 +
>  drivers/usb/Kconfig      |    2 +
>  drivers/usb/dwc/Kconfig  |   84 ++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/usb/dwc/Makefile |   19 ++++++++++
>  4 files changed, 106 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/usb/dwc/Kconfig
>  create mode 100644 drivers/usb/dwc/Makefile
> 
> diff --git a/drivers/Makefile b/drivers/Makefile
> index a125e0b..d4133a6 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -65,6 +65,7 @@ obj-$(CONFIG_PARIDE) 		+= block/paride/
>  obj-$(CONFIG_TC)		+= tc/
>  obj-$(CONFIG_UWB)		+= uwb/
>  obj-$(CONFIG_USB_OTG_UTILS)	+= usb/otg/
> +obj-$(CONFIG_USB_DWC_OTG)	+= usb/dwc/

another one ? Greg, I know MUSB is also wrong, but can we avoid adding
more guys to drivers/Makefile and use drivers/usb/Makefile instead ?

Promise I'll send a patch to MUSB for next merge window :-)

-- 
balbi

^ permalink raw reply

* Re: [PATCH v13 08/10] USB ppc4xx: Add Synopsys DWC OTG PCD interrupt function
From: Felipe Balbi @ 2011-04-04 13:58 UTC (permalink / raw)
  To: tmarri; +Cc: Mark Miesfeld, greg, linux-usb, linuxppc-dev, Fushen Chen
In-Reply-To: <1301872633-3021-1-git-send-email-tmarri@apm.com>

On Sun, Apr 03, 2011 at 04:17:13PM -0700, tmarri@apm.com wrote:
> +void start_next_request(struct pcd_ep *ep)

this will probably cause namespace clashes at some point. prepent with
dwc_ or make it static.

-- 
balbi

^ permalink raw reply

* Re: [RFC/PATCH] of: Match PCI devices to OF nodes generically
From: Bjorn Helgaas @ 2011-04-04 15:25 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: linux-arch, linux-pci@vger.kernel.org, linuxppc-dev,
	linux-kernel@vger.kernel.org, David Miller
In-Reply-To: <1301902664.2549.76.camel@pasglop>

On Mon, Apr 4, 2011 at 1:37 AM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:

> powerpc has two different ways of matching PCI devices to their
> corresponding OF node (if any) for historical reasons. The ppc64 one
> does a scan looking for matching bus/dev/fn, while the ppc32 one does a
> scan looking only for matching dev/fn on each level in order to be
> agnostic to busses being renumbered (which Linux does on some
> platforms).
>
> This removes both and instead moves the matching code to the PCI core
> itself. It's the most logical place to do it: when a pci_dev is created,
> we know the parent and thus can do a single level scan for the matching
> device_node (if any).

Some of this is reminiscent of the ACPI/PCI binding we do on x86/ia64,
e.g., acpi_get_pci_dev() and the stuff in drivers/acpi/glue.c.  Have
you looked at that to see if there's any hope of covering both OF and
ACPI with something more generic?

Bjorn

^ permalink raw reply

* Re: problem PCIe LSI detected at 32 device addresses (ppc460ex)
From: Grant Likely @ 2011-04-04 20:54 UTC (permalink / raw)
  To: Ayman El-Khashab; +Cc: linuxppc-dev list
In-Reply-To: <20110403232213.GA28042@crust.elkhashab.com>

On Sun, Apr 03, 2011 at 06:22:13PM -0500, Ayman El-Khashab wrote:
> 
> On Sun, Apr 03, 2011 at 04:09:26PM -0600, Grant Likely wrote:
> > On Sun, Apr 3, 2011 at 3:52 PM, Benjamin Herrenschmidt
> > <benh@kernel.crashing.org> wrote:
> > >
> > >> Ok, I've narrowed the scope of the problem some. ?I moved forward
> > >> to a more recent kernel (2.6.31 to 2.6.36) and that resolved the
> > >> problem of the controller showing up as every device on the bus.
> > >> However, from 2.6.37 to the current HEAD, I have not been able to
> > >> build a kernel to run on the 460EX. ?I tried 2.6.37, 2.6.38, and
> > >> the HEAD and all result in the following kernel panic. ?I am not
> > >> sure how to proceed here. ?I suppose we can stick with 2.6.36 since
> > >> it works, but I'd like to understand what it might take to remedy
> > >> this.
> > >
> > > Smells like somebody changed something with the OF flash code... Josh,
> > > Grant, any idea what's up there ?
> > 
> > Not sure, more information would be helpful.
> > 
> > Ayman, if you do a 'git log v2.6.36.. drivers/mtd/maps/physmap_of.c',
> > then you'll see a list of commits touching the mtd driver.  Would you
> > be able to do a 'git checkout <sha1-id>' on each of those are report
> > back on at what point things stop working?  Actually, a full bisect
> > between 2.6.36 and 2.6.37 would be best, but this is a good start if
> > you're limited on time.  Once you find the first commit where it
> > fails, do a 'git checkout <sha1>~1' to confirm that it is in fact the
> > commit that causes the breakage.
> 
> I can try to find the commit tomorrow.  In the interim, i've pasted
> the dts below.  The board was originally based on the canyonlands, but
> we've made some changes, mostly to the pcie.  we run the 1-l port in 
> endpoint mode, we have a chain of plx switches and devices on the 4-l 
> port.  One item that I don't think would matter, but is not too common 
> is that we are booting these over the pci bus from another PPCs memory.
> I only mention this since this failure is during boot, though everything
> should by local to the cpu by this time.
> 
> > 
> > Can you also post your device tree please?
> 
> Here is the device tree for our custom board.

Hmmm, considering that there is no device node for NAND in this tree,
something is definitely wrong.  The NAND driver should not be getting
probed.  If you can do a git bisect on the kernel it will go a long
way to figuring out what is wrong.

I suspect that it is related to merging of_platform_bus_type into the
platform_bus_type.  It looks like a device is getting incorrectly
matched to the driver, but I don't know why.

It would also help to add this code to 2.6.38 and send me the log
output:

g.


diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index c01cd1a..e9ac215 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -56,6 +56,8 @@ static int platform_driver_probe_shim(struct platform_device *pdev)
 	 * come up empty.  Return -EINVAL in this case so other drivers get
 	 * the chance to bind. */
 	match = of_match_device(pdev->dev.driver->of_match_table, &pdev->dev);
+	dev_info(&pdev->dev, "match to of_platform_driver, node:%s\n",
+		pdev->dev.of_node ? pdev->dev.of_node->full_name : "!none!");
 	return match ? ofpdrv->probe(pdev, match) : -EINVAL;
 }
 

^ permalink raw reply related

* Re: [RFC/PATCH] of: Match PCI devices to OF nodes generically
From: Benjamin Herrenschmidt @ 2011-04-04 21:03 UTC (permalink / raw)
  To: David Miller; +Cc: linux-arch, linux-pci, linuxppc-dev, linux-kernel
In-Reply-To: <20110404.004856.55863268.davem@davemloft.net>

On Mon, 2011-04-04 at 00:48 -0700, David Miller wrote:
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Date: Mon, 04 Apr 2011 13:27:10 +1000
> 
> > +struct device_node * __weak pcibios_get_phb_of_node(struct pci_bus *bus)
> > +{
> > +	/* This should only be called for PHBs */
> > +	if (WARN_ON(bus->self || bus->parent))
> > +		return NULL;
> 
> This WARN_ON() will always trigger on sparc, because we use the OF
> device tree object at the "parent" of the PCI bus devices we create
> for the PCI controller domains.
> 
> I'm really surprised you don't link the PCI bus roots into the rest of
> the global device hierarchy on powerpc.

But in the above test bus->parent is the "struct pci_bus *" parent, not
the "struct device *" nor "struct device_node *" parent... That
shouldn't be linked to anything on a PHB.

To answer your other point, we do link PHBs on some platforms, not
others. Historical stuff here. Most of our platforms discover PCI
bridges very early from setup_arch() before we have any struct device
around. Mostly because that's how we always did and there's some
subtle/nasty corner cases to deal with if we change that.

Cheers,
Ben.

^ permalink raw reply

* Re: [RFC/PATCH] of: Match PCI devices to OF nodes generically
From: David Miller @ 2011-04-04 21:09 UTC (permalink / raw)
  To: benh; +Cc: linux-arch, linux-pci, linuxppc-dev, linux-kernel
In-Reply-To: <1301950997.2549.84.camel@pasglop>

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Tue, 05 Apr 2011 07:03:17 +1000

> On Mon, 2011-04-04 at 00:48 -0700, David Miller wrote:
>> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Date: Mon, 04 Apr 2011 13:27:10 +1000
>> 
>> > +struct device_node * __weak pcibios_get_phb_of_node(struct pci_bus *bus)
>> > +{
>> > +	/* This should only be called for PHBs */
>> > +	if (WARN_ON(bus->self || bus->parent))
>> > +		return NULL;
>> 
>> This WARN_ON() will always trigger on sparc, because we use the OF
>> device tree object at the "parent" of the PCI bus devices we create
>> for the PCI controller domains.
>> 
>> I'm really surprised you don't link the PCI bus roots into the rest of
>> the global device hierarchy on powerpc.
> 
> But in the above test bus->parent is the "struct pci_bus *" parent, not
> the "struct device *" nor "struct device_node *" parent... That
> shouldn't be linked to anything on a PHB.

Aha, ok, then it shouldn't trigger on sparc, thanks for explaining.

^ permalink raw reply

* Re: [RFC/PATCH] of: Match PCI devices to OF nodes generically
From: Benjamin Herrenschmidt @ 2011-04-04 21:03 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-arch, linux-pci@vger.kernel.org, linuxppc-dev,
	linux-kernel@vger.kernel.org, David Miller
In-Reply-To: <BANLkTimBD=NfFKGpy0ZTnT9Dcu=dkTHxZg@mail.gmail.com>

On Mon, 2011-04-04 at 09:25 -0600, Bjorn Helgaas wrote:
> 
> Some of this is reminiscent of the ACPI/PCI binding we do on x86/ia64,
> e.g., acpi_get_pci_dev() and the stuff in drivers/acpi/glue.c.  Have
> you looked at that to see if there's any hope of covering both OF and
> ACPI with something more generic?

I haven't but I will have a look. Thanks.

Cheers,
Ben.

^ permalink raw reply

* Re: Pegasos i8042 broken again
From: Benjamin Herrenschmidt @ 2011-04-04 22:28 UTC (permalink / raw)
  To: Gabriel Paubert; +Cc: pacman, linuxppc-dev
In-Reply-To: <20110404222637.GA4633@iram.es>


> Ok, I got fed up about it. The patch referred above is obviously wrong since
> it leaves interrupts at 0 when a device_type or name of 8042 is found,
> so what about the following? 
> 
> I can ship it with a signed-off-by and proper comments a bit later if people agree.
> 
> Compiled and tested, otherwise I couldn't even type this message :-)

Shouldn't that be a pegasos specific quirk in chrp/setup.c ?

Cheers,
Ben.

> 	Regards,
> 	Gabriel
> 
> diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
> index 9d4882a..06865ac 100644
> --- a/arch/powerpc/kernel/setup-common.c
> +++ b/arch/powerpc/kernel/setup-common.c
> @@ -599,6 +599,10 @@ int check_legacy_ioport(unsigned long base_port)
>  		 * name instead */
>  		if (!np)
>  			np = of_find_node_by_name(NULL, "8042");
> +		if (np) {
> +			of_i8042_kbd_irq = 1;
> +			of_i8042_aux_irq = 12;
> +		}
>  		break;
>  	case FDC_BASE: /* FDC1 */
>  		np = of_find_node_by_type(NULL, "fdc");

^ permalink raw reply

* Re: Pegasos i8042 broken again
From: Gabriel Paubert @ 2011-04-04 22:26 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: pacman, linuxppc-dev
In-Reply-To: <1286696147.2463.499.camel@pasglop>

On Sun, Oct 10, 2010 at 06:35:47PM +1100, Benjamin Herrenschmidt wrote:
> On Sat, 2010-10-09 at 20:37 -0500, pacman@kosh.dhis.org wrote:
> > Pegasos has no keyboard again. I blame commit
> > 540c6c392f01887dcc96bef0a41e63e6c1334f01, which tries to find i8042 IRQs in
> > the device-tree but doesn't fall back to the old hardcoded 1 and 12 in all
> > failure cases.
> > 
> > Specifically, the case where the device-tree contains nothing matching
> > pnpPNP,303 or pnpPNP,f03 doesn't seem to be handled well. It sort of falls
> > through to the old code, but leaves the IRQs set to 0.
> > 
> > The last time something like this happened, I submitted a patch:
> > http://lists.ozlabs.org/pipermail/linuxppc-dev/2007-July/039988.html
> > which got committed, but afterward I was scolded for working around a bug
> > instead of fixing it in nvramrc.
> > 
> > This time I just won't send my workaround patch, at least until it's decided
> > that the kernel should be made to understand the device-tree as is.
> > 
> > If it's decided instead that the firmware should be patched... well I just
> > don't feel comfortable inventing my own patch for nvramrc, since it's written
> > in a language I don't know and presumably could brick the machine if I get it
> > wrong. Also I'm not even sure what the kernel is expecting to find there. 
> 
> Those things really suck. They absolutely refuse to fix their FW for
> reasons I never quite managed to figure out.
> 
> At this stage, I'd say the best is to add yet another pegasos workaround
> in prom_init that adds the missing compatible property.

Ok, I got fed up about it. The patch referred above is obviously wrong since
it leaves interrupts at 0 when a device_type or name of 8042 is found,
so what about the following? 

I can ship it with a signed-off-by and proper comments a bit later if people agree.

Compiled and tested, otherwise I couldn't even type this message :-)

	Regards,
	Gabriel

diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
index 9d4882a..06865ac 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -599,6 +599,10 @@ int check_legacy_ioport(unsigned long base_port)
 		 * name instead */
 		if (!np)
 			np = of_find_node_by_name(NULL, "8042");
+		if (np) {
+			of_i8042_kbd_irq = 1;
+			of_i8042_aux_irq = 12;
+		}
 		break;
 	case FDC_BASE: /* FDC1 */
 		np = of_find_node_by_type(NULL, "fdc");

^ permalink raw reply related

* Re: Pegasos i8042 broken again
From: Gabriel Paubert @ 2011-04-04 22:49 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: pacman, linuxppc-dev
In-Reply-To: <1301956130.2549.93.camel@pasglop>

On Tue, Apr 05, 2011 at 08:28:50AM +1000, Benjamin Herrenschmidt wrote:
> 
> > Ok, I got fed up about it. The patch referred above is obviously wrong since
> > it leaves interrupts at 0 when a device_type or name of 8042 is found,
> > so what about the following? 
> > 
> > I can ship it with a signed-off-by and proper comments a bit later if people agree.
> > 
> > Compiled and tested, otherwise I couldn't even type this message :-)
> 
> Shouldn't that be a pegasos specific quirk in chrp/setup.c ?

In this case I don't think so: 
1) The code looks for the pnp id for the 8042 controllers and tries
  to fill the interrupt fields from OF/DT, but falls back to defaults 
  1 and 12 if it does not get them.

2) Then it tries to find device_type of 8042 or node name of 8042
  and returns success if it finds them, but in this case leaves
  the interrupt numbers at zero. Note that in this case the code does 
  not even attempt to get interrupt information from OF/DT, this looks
  like a fallback case where filling with defaults seems reasonable.
  Actually, it is not filling with defaults which seems wrong since
  the other case always provides interrupt information.

	Regards,
	Gabriel
> > 
> > diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
> > index 9d4882a..06865ac 100644
> > --- a/arch/powerpc/kernel/setup-common.c
> > +++ b/arch/powerpc/kernel/setup-common.c
> > @@ -599,6 +599,10 @@ int check_legacy_ioport(unsigned long base_port)
> >  		 * name instead */
> >  		if (!np)
> >  			np = of_find_node_by_name(NULL, "8042");
> > +		if (np) {
> > +			of_i8042_kbd_irq = 1;
> > +			of_i8042_aux_irq = 12;
> > +		}
> >  		break;
> >  	case FDC_BASE: /* FDC1 */
> >  		np = of_find_node_by_type(NULL, "fdc");
> 

^ permalink raw reply

* Re: Pegasos i8042 broken again
From: pacman @ 2011-04-04 23:02 UTC (permalink / raw)
  To: Gabriel Paubert; +Cc: linuxppc-dev
In-Reply-To: <20110404222637.GA4633@iram.es>

Gabriel Paubert writes:
> 
> Ok, I got fed up about it. The patch referred above is obviously wrong since
> it leaves interrupts at 0 when a device_type or name of 8042 is found,
> so what about the following? 

Looks like the workaround I was using for a while.

In the original report I said I wasn't sending my kernel workaround patch
because of the previous disagreements about whether the kernel should work
around this type of bug. (In fact the current difficulty is the result of
changes being made without considering the special case that was created by
my first workaround... what a mess.) I also said I wasn't comfortable hacking
the Forth-based part of the boot sequence because I didn't know the language.

As it turned out, learning Forth was much easier than getting any guidance
from the kernel people on how to proceed with a workaround, so I wrote this:

=== CUT HERE ===
" /isa/8042" find-device
: open true ;
: close ;
: decode-unit ( addr len -- phys )
  1 <> if
    abort" invalid unit address"
  then
  c@
  dup ascii 0 = if
    drop
    0 exit
  then
  ascii 1 = if
    1 exit
  then
  abort" invalid unit address"
;
: encode-unit ( phys -- addr len )
  dup 0 = if
    " 0" exit
  then
  1 = if
    " 1" exit
  then
  abort" invalid unit address"
;

1 encode-int
3 encode-int encode+
d# 12 encode-int encode+
3 encode-int encode+
" interrupts" property

0 0 " 0" " /isa/8042" begin-package
 " keyboard" device-name
 " keyboard" device-type
 " pnpPNP,303" encode-string " compatible" property
 0 encode-int " reg" property
end-package

0 0 " 1" " /isa/8042" begin-package
 " mouse" device-name
 " mouse" device-type
 " pnpPNP,f03" encode-string " compatible" property
 1 encode-int " reg" property
end-package
=== CUT HERE ===

Along with the previous device tree patch (pegasos-dts-20071018), this should
present the kernel with a properly filled-out 8042 device-tree node,
preventing the need for any more patching the next time the kernel changes
its mind about how to initialize the keyboard driver.

-- 
Alan Curry

^ permalink raw reply

* Revert 737a3bb9416ce2a7c7a4170852473a4fcc9c67e8 ?
From: Gabriel Paubert @ 2011-04-04 23:52 UTC (permalink / raw)
  To: Greg KH; +Cc: linuxppc-dev, LKML, Uwe Kleine-König

	Hi,

I've had the following funny crashes on PPC machines, with
cataleptic X server as a consequence:

kernel: [drm] Setting GART location based on new memory map
kernel: Oops: Exception in kernel mode, sig: 4 [#1]
kernel: CHRP
kernel: last sysfs file: /sys/devices/pci0001:01/0001:01:08.0/resource
kernel: NIP: c05648fc LR: c0226f58 CTR: 00000008
kernel: REGS: ddb53d20 TRAP: 0700   Not tainted  (2.6.38)
kernel: MSR: 00089032 <EE,ME,IR,DR>  CR: 48044482  XER: 00000000
kernel: TASK = ddab12b0[3040] 'Xorg' THREAD: ddb52000
kernel: GPR00: c0226f34 ddb53dd0 ddab12b0 00000000 c0509e6c 00000000 00000000 00000000 
kernel: GPR08: 00000000 00000000 00000000 00000000 28044488 101f3d8c bf8166b4 00002c00 
kernel: GPR16: 101b9458 1006f1a0 101ebe0c 00000001 101ebe08 00000000 df9efc20 df9efc00 
kernel: GPR24: c0591e54 80546440 ddacf660 df9efc00 c0506048 c0480210 00a00000 df9ef800 
kernel: NIP [c05648fc] platform_device_register_resndata+0x4/0xa4
kernel: LR [c0226f58] radeon_cp_init+0xd08/0x10c4
kernel: Call Trace:
kernel: [ddb53dd0] [c0226f34] radeon_cp_init+0xce4/0x10c4 (unreliable)
kernel: [ddb53df0] [c020801c] drm_ioctl+0x2c0/0x3e4
kernel: [ddb53eb0] [c0091264] do_vfs_ioctl+0x674/0x710
kernel: [ddb53f10] [c0091340] sys_ioctl+0x40/0x70
kernel: [ddb53f40] [c00111a8] ret_from_syscall+0x0/0x38
kernel: --- Exception: c01 at 0xfc54a78
kernel:     LR = 0xfc549dc
kernel: Instruction dump:
kernel: 736f2e31 32002f75 73722f6c 69622f6c 6962786b 6c617669 65722e73 6f2e3132 
kernel: 006c6962 786b6266 696c652e 736f2e31 <002f7573> 722f6c69 622f6c69 62786b62 
kernel: ---[ end trace ed79daba161e31d9 ]---

As you can see, the processor is trying to execute ASCII strings like
"/usr/lib/libxkb" and has trouble digesting them :-) 

The backtrace is actually missing radeon_cp_init_microcode and radeon_do_init_cp
which are inlined inside radeon_cp_init.

The trouble is that radeon_cp_init_microcode calls platform_device_register_simple
which is a simple inline wrapper around platform_device_register_resndata, which
happens to be already freed and overwritten with something looking like a list
of filenames, since I have a non modular kernel.

For now I have locally reverted 737a3bb9416ce2a7c7a4170852473a4fcc9c67e8
which simply added an _init_or_module section attribute to 
platform_device_register_resndata, and X is up again... 

Now it may be that it is the ioctl that does not have the right to do
this. Actually I thought that the name radeon_cp that is registered there
would appear somwhere under /sys (or /proc) but failed to find it...

	Regards,
	Gabriel

^ permalink raw reply

* Re: [RFC/PATCH] of: Match PCI devices to OF nodes generically
From: Benjamin Herrenschmidt @ 2011-04-04 23:58 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-arch, linux-pci@vger.kernel.org, linuxppc-dev,
	linux-kernel@vger.kernel.org, David Miller
In-Reply-To: <BANLkTimBD=NfFKGpy0ZTnT9Dcu=dkTHxZg@mail.gmail.com>

On Mon, 2011-04-04 at 09:25 -0600, Bjorn Helgaas wrote:
> Some of this is reminiscent of the ACPI/PCI binding we do on x86/ia64,
> e.g., acpi_get_pci_dev() and the stuff in drivers/acpi/glue.c.  Have
> you looked at that to see if there's any hope of covering both OF and
> ACPI with something more generic?

Hrm, the ACPI stuff is quite different (to some extent akin to what
power used to do) form what I can tell. You basically traverse the ACPI
tree do perform a matching after the fact.

The idea with my patch is really to populate things as they get
discovered, which makes the code much simpler. Since we have the pointer
to the OF device node in the generic struct device nowadays, if we
populate things that way, by the time we discover a device we already
have at hand the OF device node of the parent bus, so it's a
single/simpler one level search to locate the device itself and populate
it's node as well, done once for all.

I suppose you could do something similar for ACPI, but I wouldn't try to
make a "common infrastructure" at that point, especially since there is
the possibility on x86 to have both OF device-trees and ACPI :-)

Note that I don't really provide a direct/good equivalent of your
acpi_get_pci_dev() ... the matching is mostly used the other way around,
ie a driver for a pci_dev wanting to peek a some properties in the
corresponding OF device_node. ppc32 has some reverse mapping stuff but
it's pretty crappy (and on my to-fixup list for after that patch goes
in) and in fact it would be reasonably easy from now on to implement it
as well, but so far there is no real demand.

Cheers,
Ben.

^ permalink raw reply

* Re: [RFC/PATCH] of: Match PCI devices to OF nodes generically
From: Grant Likely @ 2011-04-05  2:32 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: linux-arch, linux-pci@vger.kernel.org, linuxppc-dev,
	linux-kernel@vger.kernel.org, David Miller
In-Reply-To: <1301902664.2549.76.camel@pasglop>

On Mon, Apr 04, 2011 at 05:37:44PM +1000, Benjamin Herrenschmidt wrote:
> 
> > Nice, looks like I forgot to add the new drivers/pci/of.c file :-)
> > Here's a new patch. Also added linux-pci to the CC list.
> 
> And this one removes a lot more cruft from the powermac code while at
> it, and moves the core matching logic to drivers/of/of_pci.c...
> 
> From 917ea61d6afcf443ca467d0a6ffa00d5c6e21bb3 Mon Sep 17 00:00:00 2001
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Date: Mon, 4 Apr 2011 14:38:13 +1000
> Subject: [PATCH] of: Match PCI devices to OF nodes dynamically
> 
> powerpc has two different ways of matching PCI devices to their
> corresponding OF node (if any) for historical reasons. The ppc64 one
> does a scan looking for matching bus/dev/fn, while the ppc32 one does a
> scan looking only for matching dev/fn on each level in order to be
> agnostic to busses being renumbered (which Linux does on some
> platforms).
> 
> This removes both and instead moves the matching code to the PCI core
> itself. It's the most logical place to do it: when a pci_dev is created,
> we know the parent and thus can do a single level scan for the matching
> device_node (if any).
> 
> The benefit is that all archs now get the matching for free. There's one
> hook the arch might want to provide to match a PHB bus to its device
> node. A default weak implementation is provided that looks for the
> parent device device node, but it's not entirely reliable on powerpc for
> various reasons so powerpc provides its own.

Awesome.  I'm looking at doing pretty much exactly the same thing for
USB and platform_devices on SoCs.  I'm glad to see this for pci.

> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>  arch/powerpc/include/asm/pci-bridge.h |   35 +++------
>  arch/powerpc/include/asm/pci.h        |    3 +-
>  arch/powerpc/include/asm/prom.h       |   14 ---
>  arch/powerpc/kernel/pci-common.c      |   11 ++-
>  arch/powerpc/kernel/pci_32.c          |  148 +++------------------------------
>  arch/powerpc/kernel/pci_dn.c          |   47 -----------
>  arch/powerpc/kernel/pci_of_scan.c     |    9 +--
>  arch/powerpc/platforms/powermac/pci.c |    3 +-
>  arch/sparc/kernel/pci.c               |    2 +-
>  drivers/of/of_pci.c                   |   36 ++++++++
>  drivers/pci/Makefile                  |    2 +
>  drivers/pci/hotplug/rpadlpar_core.c   |    2 +-
>  drivers/pci/of.c                      |   60 +++++++++++++
>  drivers/pci/probe.c                   |    8 ++-
>  include/linux/of_pci.h                |    5 +
>  include/linux/pci.h                   |   17 ++++
>  16 files changed, 165 insertions(+), 237 deletions(-)
>  create mode 100644 drivers/pci/of.c
> 
> diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
> index 5e156e0..6912c45 100644
> --- a/arch/powerpc/include/asm/pci-bridge.h
> +++ b/arch/powerpc/include/asm/pci-bridge.h
> @@ -169,18 +169,22 @@ static inline struct pci_controller *pci_bus_to_host(const struct pci_bus *bus)
>  	return bus->sysdata;
>  }
>  
> -#ifndef CONFIG_PPC64
> +static inline struct device_node *pci_device_to_OF_node(struct pci_dev *dev)
> +{
> +	return dev->dev.of_node;
> +}
>  
>  static inline struct device_node *pci_bus_to_OF_node(struct pci_bus *bus)
>  {
> -	struct pci_controller *host;
> -
> -	if (bus->self)
> -		return pci_device_to_OF_node(bus->self);
> -	host = pci_bus_to_host(bus);
> -	return host ? host->dn : NULL;
> +	return bus->dev.of_node;
>  }

Should these two inlines move to include/linux/of_pci.h?  Microblaze
defines it differently, but I don't think it should be.  Sparc also
has a different variant in arch/sparc/kernel/pci.c, but not in
arch/sparc/kernel/pcic.c.  It looks to me like this should be the
common case unless a specific platform needs otherwise.

>  
> +#ifndef CONFIG_PPC64
> +
> +extern int pci_device_from_OF_node(struct device_node *node,
> +				   u8* bus, u8* devfn);
> +extern void pci_create_OF_bus_map(void);
> +
>  static inline int isa_vaddr_is_ioport(void __iomem *address)
>  {
>  	/* No specific ISA handling on ppc32 at this stage, it
> @@ -223,17 +227,8 @@ struct pci_dn {
>  /* Get the pointer to a device_node's pci_dn */
>  #define PCI_DN(dn)	((struct pci_dn *) (dn)->data)
>  
> -extern struct device_node *fetch_dev_dn(struct pci_dev *dev);
>  extern void * update_dn_pci_info(struct device_node *dn, void *data);
>  
> -/* Get a device_node from a pci_dev.  This code must be fast except
> - * in the case where the sysdata is incorrect and needs to be fixed
> - * up (this will only happen once). */
> -static inline struct device_node *pci_device_to_OF_node(struct pci_dev *dev)
> -{
> -	return dev->dev.of_node ? dev->dev.of_node : fetch_dev_dn(dev);
> -}
> -
>  static inline int pci_device_from_OF_node(struct device_node *np,
>  					  u8 *bus, u8 *devfn)
>  {
> @@ -244,14 +239,6 @@ static inline int pci_device_from_OF_node(struct device_node *np,
>  	return 0;
>  }
>  
> -static inline struct device_node *pci_bus_to_OF_node(struct pci_bus *bus)
> -{
> -	if (bus->self)
> -		return pci_device_to_OF_node(bus->self);
> -	else
> -		return bus->dev.of_node; /* Must be root bus (PHB) */
> -}
> -

Yay!

>  /** Find the bus corresponding to the indicated device node */
>  extern struct pci_bus *pcibios_find_pci_bus(struct device_node *dn);
>  
> diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h
> index 7d77909..1f52268 100644
> --- a/arch/powerpc/include/asm/pci.h
> +++ b/arch/powerpc/include/asm/pci.h
> @@ -179,8 +179,7 @@ extern int remove_phb_dynamic(struct pci_controller *phb);
>  extern struct pci_dev *of_create_pci_dev(struct device_node *node,
>  					struct pci_bus *bus, int devfn);
>  
> -extern void of_scan_pci_bridge(struct device_node *node,
> -				struct pci_dev *dev);
> +extern void of_scan_pci_bridge(struct pci_dev *dev);
>  
>  extern void of_scan_bus(struct device_node *node, struct pci_bus *bus);
>  extern void of_rescan_bus(struct device_node *node, struct pci_bus *bus);
> diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
> index c189aa5..b823536 100644
> --- a/arch/powerpc/include/asm/prom.h
> +++ b/arch/powerpc/include/asm/prom.h
> @@ -22,20 +22,6 @@
>  
>  #define HAVE_ARCH_DEVTREE_FIXUPS
>  
> -#ifdef CONFIG_PPC32
> -/*
> - * PCI <-> OF matching functions
> - * (XXX should these be here?)
> - */
> -struct pci_bus;
> -struct pci_dev;
> -extern int pci_device_from_OF_node(struct device_node *node,
> -				   u8* bus, u8* devfn);
> -extern struct device_node* pci_busdev_to_OF_node(struct pci_bus *, int);
> -extern struct device_node* pci_device_to_OF_node(struct pci_dev *);
> -extern void pci_create_OF_bus_map(void);
> -#endif
> -
>  /*
>   * OF address retreival & translation
>   */
> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> index 893af2a..47c516b 100644
> --- a/arch/powerpc/kernel/pci-common.c
> +++ b/arch/powerpc/kernel/pci-common.c
> @@ -1097,9 +1097,6 @@ void __devinit pcibios_setup_bus_devices(struct pci_bus *bus)
>  		if (dev->is_added)
>  			continue;
>  
> -		/* Setup OF node pointer in the device */
> -		dev->dev.of_node = pci_device_to_OF_node(dev);
> -
>  		/* Fixup NUMA node as it may not be setup yet by the generic
>  		 * code and is needed by the DMA init
>  		 */
> @@ -1685,6 +1682,13 @@ int early_find_capability(struct pci_controller *hose, int bus, int devfn,
>  	return pci_bus_find_capability(fake_pci_bus(hose, bus), devfn, cap);
>  }
>  
> +struct device_node * pcibios_get_phb_of_node(struct pci_bus *bus)
> +{
> +	struct pci_controller *hose = bus->sysdata;
> +
> +	return of_node_get(hose->dn);
> +}
> +
>  /**
>   * pci_scan_phb - Given a pci_controller, setup and scan the PCI bus
>   * @hose: Pointer to the PCI host controller instance structure
> @@ -1705,7 +1709,6 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose)
>  			hose->global_number);
>  		return;
>  	}
> -	bus->dev.of_node = of_node_get(node);
>  	bus->secondary = hose->first_busno;
>  	hose->bus = bus;
>  
> diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c
> index bedb370..ca6bcb7 100644
> --- a/arch/powerpc/kernel/pci_32.c
> +++ b/arch/powerpc/kernel/pci_32.c
> @@ -167,150 +167,26 @@ pcibios_make_OF_bus_map(void)
>  #endif
>  }
>  
> -typedef int (*pci_OF_scan_iterator)(struct device_node* node, void* data);
> -
> -static struct device_node*
> -scan_OF_pci_childs(struct device_node *parent, pci_OF_scan_iterator filter, void* data)
> -{
> -	struct device_node *node;
> -	struct device_node* sub_node;
> -
> -	for_each_child_of_node(parent, node) {
> -		const unsigned int *class_code;
> -	
> -		if (filter(node, data)) {
> -			of_node_put(node);
> -			return node;
> -		}
> -
> -		/* For PCI<->PCI bridges or CardBus bridges, we go down
> -		 * Note: some OFs create a parent node "multifunc-device" as
> -		 * a fake root for all functions of a multi-function device,
> -		 * we go down them as well.
> -		 */
> -		class_code = of_get_property(node, "class-code", NULL);
> -		if ((!class_code || ((*class_code >> 8) != PCI_CLASS_BRIDGE_PCI &&
> -			(*class_code >> 8) != PCI_CLASS_BRIDGE_CARDBUS)) &&
> -			strcmp(node->name, "multifunc-device"))
> -			continue;
> -		sub_node = scan_OF_pci_childs(node, filter, data);
> -		if (sub_node) {
> -			of_node_put(node);
> -			return sub_node;
> -		}
> -	}
> -	return NULL;
> -}
> -
> -static struct device_node *scan_OF_for_pci_dev(struct device_node *parent,
> -					       unsigned int devfn)
> -{
> -	struct device_node *np, *cnp;
> -	const u32 *reg;
> -	unsigned int psize;
> -
> -	for_each_child_of_node(parent, np) {
> -		reg = of_get_property(np, "reg", &psize);
> -                if (reg && psize >= 4 && ((reg[0] >> 8) & 0xff) == devfn)
> -			return np;
> -
> -		/* Note: some OFs create a parent node "multifunc-device" as
> -		 * a fake root for all functions of a multi-function device,
> -		 * we go down them as well. */
> -                if (!strcmp(np->name, "multifunc-device")) {
> -                        cnp = scan_OF_for_pci_dev(np, devfn);
> -                        if (cnp)
> -                                return cnp;
> -                }
> -	}
> -	return NULL;
> -}
> -
> -
> -static struct device_node *scan_OF_for_pci_bus(struct pci_bus *bus)
> -{
> -	struct device_node *parent, *np;
> -
> -	/* Are we a root bus ? */
> -	if (bus->self == NULL || bus->parent == NULL) {
> -		struct pci_controller *hose = pci_bus_to_host(bus);
> -		if (hose == NULL)
> -			return NULL;
> -		return of_node_get(hose->dn);
> -	}
> -
> -	/* not a root bus, we need to get our parent */
> -	parent = scan_OF_for_pci_bus(bus->parent);
> -	if (parent == NULL)
> -		return NULL;
> -
> -	/* now iterate for children for a match */
> -	np = scan_OF_for_pci_dev(parent, bus->self->devfn);
> -	of_node_put(parent);
> -
> -	return np;
> -}
> -
> -/*
> - * Scans the OF tree for a device node matching a PCI device
> - */
> -struct device_node *
> -pci_busdev_to_OF_node(struct pci_bus *bus, int devfn)
> -{
> -	struct device_node *parent, *np;
> -
> -	pr_debug("pci_busdev_to_OF_node(%d,0x%x)\n", bus->number, devfn);
> -	parent = scan_OF_for_pci_bus(bus);
> -	if (parent == NULL)
> -		return NULL;
> -	pr_debug(" parent is %s\n", parent ? parent->full_name : "<NULL>");
> -	np = scan_OF_for_pci_dev(parent, devfn);
> -	of_node_put(parent);
> -	pr_debug(" result is %s\n", np ? np->full_name : "<NULL>");
> -
> -	/* XXX most callers don't release the returned node
> -	 * mostly because ppc64 doesn't increase the refcount,
> -	 * we need to fix that.
> -	 */
> -	return np;
> -}
> -EXPORT_SYMBOL(pci_busdev_to_OF_node);
> -
> -struct device_node*
> -pci_device_to_OF_node(struct pci_dev *dev)
> -{
> -	return pci_busdev_to_OF_node(dev->bus, dev->devfn);
> -}
> -EXPORT_SYMBOL(pci_device_to_OF_node);
> -
> -static int
> -find_OF_pci_device_filter(struct device_node* node, void* data)
> -{
> -	return ((void *)node == data);
> -}
>  
>  /*
>   * Returns the PCI device matching a given OF node
>   */
> -int
> -pci_device_from_OF_node(struct device_node* node, u8* bus, u8* devfn)
> +int pci_device_from_OF_node(struct device_node* node, u8* bus, u8* devfn)
>  {
> -	const unsigned int *reg;
> -	struct pci_controller* hose;
>  	struct pci_dev* dev = NULL;
> -	
> -	/* Make sure it's really a PCI device */
> -	hose = pci_find_hose_for_OF_device(node);
> -	if (!hose || !hose->dn)
> -		return -ENODEV;
> -	if (!scan_OF_pci_childs(hose->dn,
> -			find_OF_pci_device_filter, (void *)node))
> +	const __be32 *reg;
> +	int size;
> +
> +	/* Check if it might have a chance to be a PCI device */
> +	if (!pci_find_hose_for_OF_device(node))
>  		return -ENODEV;
> -	reg = of_get_property(node, "reg", NULL);
> -	if (!reg)
> +
> +	reg = of_get_property(node, "reg", &size);
> +	if (!reg || size < 5 * sizeof(u32))
>  		return -ENODEV;
> -	*bus = (reg[0] >> 16) & 0xff;
> -	*devfn = ((reg[0] >> 8) & 0xff);
> +	
> +	*bus = (be32_to_cpup(&reg[0]) >> 16) & 0xff;
> +	*devfn = (be32_to_cpup(&reg[0]) >> 8) & 0xff;
>  
>  	/* Ok, here we need some tweak. If we have already renumbered
>  	 * all busses, we can't rely on the OF bus number any more.
> diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
> index d225d99..8cb66a2 100644
> --- a/arch/powerpc/kernel/pci_dn.c
> +++ b/arch/powerpc/kernel/pci_dn.c
> @@ -143,53 +143,6 @@ void __devinit pci_devs_phb_init_dynamic(struct pci_controller *phb)
>  	traverse_pci_devices(dn, update_dn_pci_info, phb);
>  }
>  
> -/*
> - * Traversal func that looks for a <busno,devfcn> value.
> - * If found, the pci_dn is returned (thus terminating the traversal).
> - */
> -static void *is_devfn_node(struct device_node *dn, void *data)
> -{
> -	int busno = ((unsigned long)data >> 8) & 0xff;
> -	int devfn = ((unsigned long)data) & 0xff;
> -	struct pci_dn *pci = dn->data;
> -
> -	if (pci && (devfn == pci->devfn) && (busno == pci->busno))
> -		return dn;
> -	return NULL;
> -}
> -
> -/*
> - * This is the "slow" path for looking up a device_node from a
> - * pci_dev.  It will hunt for the device under its parent's
> - * phb and then update of_node pointer.
> - *
> - * It may also do fixups on the actual device since this happens
> - * on the first read/write.
> - *
> - * Note that it also must deal with devices that don't exist.
> - * In this case it may probe for real hardware ("just in case")
> - * and add a device_node to the device tree if necessary.
> - *
> - * Is this function necessary anymore now that dev->dev.of_node is
> - * used to store the node pointer?
> - *
> - */
> -struct device_node *fetch_dev_dn(struct pci_dev *dev)
> -{
> -	struct pci_controller *phb = dev->sysdata;
> -	struct device_node *dn;
> -	unsigned long searchval = (dev->bus->number << 8) | dev->devfn;
> -
> -	if (WARN_ON(!phb))
> -		return NULL;
> -
> -	dn = traverse_pci_devices(phb->dn, is_devfn_node, (void *)searchval);
> -	if (dn)
> -		dev->dev.of_node = dn;
> -	return dn;
> -}
> -EXPORT_SYMBOL(fetch_dev_dn);
> -
>  /** 
>   * pci_devs_phb_init - Initialize phbs and pci devs under them.
>   * 
> diff --git a/arch/powerpc/kernel/pci_of_scan.c b/arch/powerpc/kernel/pci_of_scan.c
> index 1e89a72..fe0a5ad 100644
> --- a/arch/powerpc/kernel/pci_of_scan.c
> +++ b/arch/powerpc/kernel/pci_of_scan.c
> @@ -202,9 +202,9 @@ EXPORT_SYMBOL(of_create_pci_dev);
>   * this routine in turn call of_scan_bus() recusively to scan for more child
>   * devices.
>   */
> -void __devinit of_scan_pci_bridge(struct device_node *node,
> -				  struct pci_dev *dev)
> +void __devinit of_scan_pci_bridge(struct pci_dev *dev)
>  {
> +	struct device_node *node = dev->dev.of_node;
>  	struct pci_bus *bus;
>  	const u32 *busrange, *ranges;
>  	int len, i, mode;
> @@ -238,7 +238,6 @@ void __devinit of_scan_pci_bridge(struct device_node *node,
>  	bus->primary = dev->bus->number;
>  	bus->subordinate = busrange[1];
>  	bus->bridge_ctl = 0;
> -	bus->dev.of_node = of_node_get(node);
>  
>  	/* parse ranges property */
>  	/* PCI #address-cells == 3 and #size-cells == 2 always */
> @@ -335,9 +334,7 @@ static void __devinit __of_scan_bus(struct device_node *node,
>  	list_for_each_entry(dev, &bus->devices, bus_list) {
>  		if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||
>  		    dev->hdr_type == PCI_HEADER_TYPE_CARDBUS) {
> -			struct device_node *child = pci_device_to_OF_node(dev);
> -			if (child)
> -				of_scan_pci_bridge(child, dev);
> +			of_scan_pci_bridge(dev);
>  		}
>  	}
>  }
> diff --git a/arch/powerpc/platforms/powermac/pci.c b/arch/powerpc/platforms/powermac/pci.c
> index ab68989..68a18da 100644
> --- a/arch/powerpc/platforms/powermac/pci.c
> +++ b/arch/powerpc/platforms/powermac/pci.c
> @@ -17,6 +17,7 @@
>  #include <linux/init.h>
>  #include <linux/bootmem.h>
>  #include <linux/irq.h>
> +#include <linux/of_pci.h>
>  
>  #include <asm/sections.h>
>  #include <asm/io.h>
> @@ -235,7 +236,7 @@ static int chaos_validate_dev(struct pci_bus *bus, int devfn, int offset)
>  
>  	if (offset >= 0x100)
>  		return  PCIBIOS_BAD_REGISTER_NUMBER;
> -	np = pci_busdev_to_OF_node(bus, devfn);
> +	np = of_pci_find_child_device(bus->dev.of_node, devfn);
>  	if (np == NULL)
>  		return PCIBIOS_DEVICE_NOT_FOUND;
>  
> diff --git a/arch/sparc/kernel/pci.c b/arch/sparc/kernel/pci.c
> index 713dc91..e539d23 100644
> --- a/arch/sparc/kernel/pci.c
> +++ b/arch/sparc/kernel/pci.c
> @@ -284,7 +284,7 @@ static struct pci_dev *of_create_pci_dev(struct pci_pbm_info *pbm,
>  	dev->sysdata = node;
>  	dev->dev.parent = bus->bridge;
>  	dev->dev.bus = &pci_bus_type;
> -	dev->dev.of_node = node;
> +	dev->dev.of_node = of_node_get(node);
>  	dev->devfn = devfn;
>  	dev->multifunction = 0;		/* maybe a lie? */
>  	set_pcie_port_type(dev);
> diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
> index ac1ec54..9d179c4 100644
> --- a/drivers/of/of_pci.c
> +++ b/drivers/of/of_pci.c
> @@ -90,3 +90,39 @@ int of_irq_map_pci(struct pci_dev *pdev, struct of_irq *out_irq)
>  	return of_irq_map_raw(ppnode, &lspec_be, 1, laddr, out_irq);
>  }
>  EXPORT_SYMBOL_GPL(of_irq_map_pci);
> +
> +
> +static inline int __of_pci_pci_compare(struct device_node *node, unsigned int devfn)
> +{
> +	unsigned int size;
> +	const __be32 *reg = of_get_property(node, "reg", &size);
> +
> +	if (!reg || size < 5 * sizeof(__be32))
> +		return 0;
> +	return ((be32_to_cpup(&reg[0]) >> 8) & 0xff) == devfn;
> +}
> +
> +struct device_node *of_pci_find_child_device(struct device_node *parent,
> +					     unsigned int devfn)
> +{
> +	struct device_node *node, *node2;
> +	
> +	for_each_child_of_node(parent, node) {
> +		if (__of_pci_pci_compare(node, devfn))
> +			return node;
> +		/*
> +		 * Some OFs create a parent node "multifunc-device" as
> +		 * a fake root for all functions of a multi-function
> +		 * device we go down them as well.
> +		 */
> +                if (!strcmp(node->name, "multifunc-device")) {
> +			for_each_child_of_node(node, node2) {
> +				if (__of_pci_pci_compare(node2, devfn)) {
> +					of_node_put(node);
> +					return node2;
> +				}
> +			}
> +                }
> +	}
> +	return NULL;
> +}
> diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
> index 98d61c8..d5c3cb9 100644
> --- a/drivers/pci/Makefile
> +++ b/drivers/pci/Makefile
> @@ -70,4 +70,6 @@ obj-$(CONFIG_PCI_STUB) += pci-stub.o
>  
>  obj-$(CONFIG_XEN_PCIDEV_FRONTEND) += xen-pcifront.o
>  
> +obj-$(CONFIG_OF) += of.o
> +
>  ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG
> diff --git a/drivers/pci/hotplug/rpadlpar_core.c b/drivers/pci/hotplug/rpadlpar_core.c
> index 0830347..1d002b1 100644
> --- a/drivers/pci/hotplug/rpadlpar_core.c
> +++ b/drivers/pci/hotplug/rpadlpar_core.c
> @@ -158,7 +158,7 @@ static void dlpar_pci_add_bus(struct device_node *dn)
>  	/* Scan below the new bridge */
>  	if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||
>  	    dev->hdr_type == PCI_HEADER_TYPE_CARDBUS)
> -		of_scan_pci_bridge(dn, dev);
> +		of_scan_pci_bridge(dev);
>  
>  	/* Map IO space for child bus, which may or may not succeed */
>  	pcibios_map_io_space(dev->subordinate);
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> new file mode 100644
> index 0000000..fff7270
> --- /dev/null
> +++ b/drivers/pci/of.c

Should this be consolidated with drivers/of/of_pci.c?  I don't have
strong opinions about where the result lives, but I don't think they
should be split up.

> @@ -0,0 +1,60 @@
> +/*
> + * PCI <-> OF mapping helpers
> + *
> + * Copyright 2011 IBM Corp.
> + * 
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/pci.h>
> +#include <linux/of_pci.h>
> +#include "pci.h"
> +
> +void pci_set_of_node(struct pci_dev *dev)
> +{
> +	if (!dev->bus->dev.of_node)
> +		return;
> +	dev->dev.of_node = of_pci_find_child_device(dev->bus->dev.of_node,
> +						    dev->devfn);
> +}
> +
> +void pci_release_of_node(struct pci_dev *dev)
> +{
> +	of_node_put(dev->dev.of_node);
> +	dev->dev.of_node = NULL;
> +}
> +
> +void pci_set_bus_of_node(struct pci_bus *bus)
> +{
> +	if (bus->self == NULL)
> +		bus->dev.of_node = pcibios_get_phb_of_node(bus);
> +	else
> +		bus->dev.of_node = of_node_get(bus->self->dev.of_node);
> +}
> +
> +void pci_release_bus_of_node(struct pci_bus *bus)
> +{
> +	of_node_put(bus->dev.of_node);
> +	bus->dev.of_node = NULL;
> +}
> +
> +struct device_node * __weak pcibios_get_phb_of_node(struct pci_bus *bus)
> +{
> +	/* This should only be called for PHBs */
> +	if (WARN_ON(bus->self || bus->parent))
> +		return NULL;
> +
> +	/* Look for a node pointer in either the intermediary device we
> +	 * create above the root bus or it's own parent. Normally only
> +	 * the later is populated.
> +	 */
> +	if (bus->bridge->of_node)
> +		return of_node_get(bus->bridge->of_node);
> +	if (bus->bridge->parent->of_node)
> +		return of_node_get(bus->bridge->parent->of_node);
> +	return NULL;
> +}
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 44cbbba..347349b 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -89,6 +89,7 @@ static void release_pcibus_dev(struct device *dev)
>  	if (pci_bus->bridge)
>  		put_device(pci_bus->bridge);
>  	pci_bus_remove_resources(pci_bus);
> +	pci_release_bus_of_node(pci_bus);
>  	kfree(pci_bus);
>  }
>  
> @@ -624,7 +625,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
>  
>  	child->self = bridge;
>  	child->bridge = get_device(&bridge->dev);
> -
> +	pci_set_bus_of_node(child);	

If I'm reading this correctly, the only time an of_node can be
associated with a pci_bus is here and pci_alloc_bus.  In both cases
that makes it safe for release_pcibus_dev() to remove it.  Am I
correct?

oh, and trailing whitespace.  :-)

>  	pci_set_bus_speed(child);
>  
>  	/* Set up default resource pointers and names.. */
> @@ -1074,6 +1075,7 @@ static void pci_release_dev(struct device *dev)
>  
>  	pci_dev = to_pci_dev(dev);
>  	pci_release_capabilities(pci_dev);
> +	pci_release_of_node(pci_dev);
>  	kfree(pci_dev);
>  }
>  
> @@ -1150,6 +1152,7 @@ struct pci_dev *alloc_pci_dev(void)
>  }
>  EXPORT_SYMBOL(alloc_pci_dev);
>  
> +
>  /*
>   * Read the config data for a PCI device, sanity-check it
>   * and fill in the dev structure...

Unrelated whitespace change

> @@ -1193,6 +1196,8 @@ static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn)
>  	dev->vendor = l & 0xffff;
>  	dev->device = (l >> 16) & 0xffff;
>  
> +	pci_set_of_node(dev);
> +
>  	if (pci_setup_device(dev)) {
>  		kfree(dev);
>  		return NULL;
> @@ -1445,6 +1450,7 @@ struct pci_bus * pci_create_bus(struct device *parent,
>  		goto dev_reg_err;
>  	b->bridge = get_device(dev);
>  	device_enable_async_suspend(b->bridge);
> +	pci_set_bus_of_node(b);

Similarly here; so device node linkage to pci devices will always use
the same mechanism except for in of_create_pci_dev() for sparc, right?

>  
>  	if (!parent)
>  		set_dev_node(b->bridge, pcibus_to_node(b));
> diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
> index 85a27b6..f93e217 100644
> --- a/include/linux/of_pci.h
> +++ b/include/linux/of_pci.h
> @@ -6,4 +6,9 @@
>  struct pci_dev;
>  struct of_irq;
>  int of_irq_map_pci(struct pci_dev *pdev, struct of_irq *out_irq);
> +
> +struct device_node;
> +struct device_node *of_pci_find_child_device(struct device_node *parent,
> +					     unsigned int devfn);
> +
>  #endif
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 96f70d7..e5111da 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1543,5 +1543,22 @@ int pci_vpd_find_tag(const u8 *buf, unsigned int off, unsigned int len, u8 rdt);
>  int pci_vpd_find_info_keyword(const u8 *buf, unsigned int off,
>  			      unsigned int len, const char *kw);
>  
> +/* PCI <-> OF binding helpers */
> +#ifdef  CONFIG_OF
> +#include <linux/of.h>

linux/of.h is safe to include when CONFIG_OF is not selected; it can
live at the top of the file with the rest of the includes.

> +extern void pci_set_of_node(struct pci_dev *dev);
> +extern void pci_release_of_node(struct pci_dev *dev);
> +extern void pci_set_bus_of_node(struct pci_bus *bus);
> +extern void pci_release_bus_of_node(struct pci_bus *bus);

Looks to me like these functions still get called when
!defined(CONFIG_OF).

> +
> +/* Arch may override this (weak) */
> +extern struct device_node * __weak pcibios_get_phb_of_node(struct pci_bus *bus);
> +
> +
> +#else /* CONFIG_OF */
> +static void pci_locate_of_node(struct pci_dev *dev) { }
> +static void pci_release_of_node(struct pci_dev *dev) { }
> +#endif  /* CONFIG_OF */
> +
>  #endif /* __KERNEL__ */
>  #endif /* LINUX_PCI_H */
> 
> 
> --
> 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/

^ permalink raw reply

* [git pull] Please pull powerpc.git merge branch
From: Kumar Gala @ 2011-04-05  4:30 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev

The following changes since commit c0bb9e45f3a7f67fc358946727bc3d5f23d0f55d:

  kdump: Allow shrinking of kdump region to be overridden (2011-04-01 16:14:30 +1100)

are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/galak/powerpc.git merge

Kumar Gala (1):
      edac/mpc85xx: Limit setting/clearing of HID1[RFXE] to e500v1/v2 cores

Prabhakar Kushwaha (1):
      powerpc/85xx: Update dts for PCIe memory maps to match u-boot of Px020RDB

 arch/powerpc/boot/dts/p1020rdb.dts            |   12 +++++-----
 arch/powerpc/boot/dts/p2020rdb.dts            |   12 +++++-----
 arch/powerpc/boot/dts/p2020rdb_camp_core0.dts |    4 +-
 arch/powerpc/boot/dts/p2020rdb_camp_core1.dts |   10 ++++----
 drivers/edac/mpc85xx_edac.c                   |   27 +++++++++++++++++-------
 5 files changed, 38 insertions(+), 27 deletions(-)

^ permalink raw reply

* [PATCHES] Bits and pieces for Power7 support in HV mode
From: Benjamin Herrenschmidt @ 2011-04-05  6:13 UTC (permalink / raw)
  To: linuxppc-dev

Here's a patch series which adds bits and pieces (and in some case
not directly related cleanups that will be useful for the upcoming
platform that uses that stuff) to allow running a recent CPU in
hypervisor mode.

It applies on top of the few cleanups and the XICS rewrite I've
already posted and is a pre-requisite for the upcoming KVM support
and the "non-virtualized" platform (still brewing).

I've also put them in my "test" branch

Cheers,
Ben.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox