LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] gianfar:don't add FCB length to hard_header_len
From: Jan Ceuleers @ 2012-05-24 18:03 UTC (permalink / raw)
  To: Joe Perches; +Cc: netdev, b06378, linuxppc-dev, David Miller
In-Reply-To: <1337876210.5070.4.camel@joe2Laptop>

On 05/24/2012 06:16 PM, Joe Perches wrote:
> On Thu, 2012-05-24 at 17:04 +0200, Jan Ceuleers wrote:
>> On 05/22/2012 09:18 PM, David Miller wrote:
>>> Someone needs to go through this driver when net-next opens up
>>> and fix all of the indentation in this driver.
>>
>> May I give that a go?
> 
> I have scripts that automate most of this.
> I don't have the card though.
> 
> Maybe this is a starting point?
> It doesn't fix most 80 column warnings.

Thanks Joe.

I also don't have the card; the kinds of changes I was proposing to make
are code-neutral, whose correctness I was intending to prove by a binary
compare of the before-and-after .o.

Still happy to do that taking your contribution as a starting point.

Thanks, Jan

^ permalink raw reply

* Re: [PATCH v6 0/3] netdev/of/phy: MDIO bus multiplexer support.
From: Timur Tabi @ 2012-05-24 18:28 UTC (permalink / raw)
  To: David Daney
  Cc: netdev@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <4FB6CBF1.40300@gmail.com>

David Daney wrote:
> Yes.  You may note in the DTS file I attached in the parent (sorry for 
> the fubar mime types), that there are two, almost identical, MDIO 
> masters.  smi0 has two directly attached PHYs.  smi1 goes to the mux, 
> and each child of the mux has four attached PHYs.

I'm till have trouble understanding all this.  I'm just hacking things up
in order to help me understand it, but it's a slow and painful process.

This call in mdio_mux_init() is failing:

	parent_bus = of_mdio_find_bus(parent_bus_node);

It returns NULL.  Here is my MDIO node:

		fman0: fman@400000 {
			enet0: ethernet@e0000 {
				tbi-handle = <&tbi0>;
				phy-handle = <&phy0>;
				phy-connection-type = "sgmii";
			};

			mdio0: mdio@e1120 {
				gpios = <&gpio0 0 0
					 &gpio0 1 0>;

				tbi0: tbi-phy@8 {
					reg = <0x8>;
					device_type = "tbi-phy";
				};

				phy0: ethernet-phy@1c {
					reg = <0x1c>;
				};
			};
		};

What am I missing?  What do I need to do in order to get
of_mdio_find_bus() to return a real value?

-- 
Timur Tabi
Linux kernel developer at Freescale

^ permalink raw reply

* Re: [PATCH v6 0/3] netdev/of/phy: MDIO bus multiplexer support.
From: David Daney @ 2012-05-24 18:50 UTC (permalink / raw)
  To: Timur Tabi
  Cc: netdev@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <4FBE7DD8.509@freescale.com>

On 05/24/2012 11:28 AM, Timur Tabi wrote:
> David Daney wrote:
>> Yes.  You may note in the DTS file I attached in the parent (sorry for
>> the fubar mime types), that there are two, almost identical, MDIO
>> masters.  smi0 has two directly attached PHYs.  smi1 goes to the mux,
>> and each child of the mux has four attached PHYs.
>
> I'm till have trouble understanding all this.  I'm just hacking things up
> in order to help me understand it, but it's a slow and painful process.
>
> This call in mdio_mux_init() is failing:
>
> 	parent_bus = of_mdio_find_bus(parent_bus_node);
>

Well, the MDIO bus must have an associated device tree node.

For my OCTEON code, the MDIO bus device is created as a result of the 
call to of_platform_bus_probe(), which takes care of filling in all the 
device tree nodes of the devices it finds and creates.

> It returns NULL.  Here is my MDIO node:
>
> 		fman0: fman@400000 {
> 			enet0: ethernet@e0000 {
> 				tbi-handle =<&tbi0>;
> 				phy-handle =<&phy0>;
> 				phy-connection-type = "sgmii";
> 			};
>
> 			mdio0: mdio@e1120 {
> 				gpios =<&gpio0 0 0
> 					&gpio0 1 0>;
>
> 				tbi0: tbi-phy@8 {
> 					reg =<0x8>;
> 					device_type = "tbi-phy";
> 				};
>
> 				phy0: ethernet-phy@1c {
> 					reg =<0x1c>;
> 				};
> 			};
> 		};
>
> What am I missing?

For starters, I do not see any compatible properties that would allow 
the proper drivers to be bound to anything.

Also I see no MDIO mux node there, so it is unclear why you are even 
asking these questions.

David Daney

^ permalink raw reply

* Re: [PATCH v6 0/3] netdev/of/phy: MDIO bus multiplexer support.
From: Timur Tabi @ 2012-05-24 19:03 UTC (permalink / raw)
  To: David Daney
  Cc: netdev@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <4FBE82F2.6080100@gmail.com>

David Daney wrote:

> Well, the MDIO bus must have an associated device tree node.
> 
> For my OCTEON code, the MDIO bus device is created as a result of the 
> call to of_platform_bus_probe(), which takes care of filling in all the 
> device tree nodes of the devices it finds and creates.

Ok, let me give you some background.  We actually already have MDIO muxing
code in-house, but it's different from yours.  So now I'm rewriting it to
use your design instead.

So our current code looks for "virtual MDIO nodes", and we call
mdiobus_alloc() and then of_mdiobus_register().  I think this is what I'm
missing now.

I just don't know what to do next.  Part of the problem is that I don't
have much experience with MDIO drivers.

>> It returns NULL.  Here is my MDIO node:
>>
>> 		fman0: fman@400000 {
>> 			enet0: ethernet@e0000 {
>> 				tbi-handle =<&tbi0>;
>> 				phy-handle =<&phy0>;
>> 				phy-connection-type = "sgmii";
>> 			};
>>
>> 			mdio0: mdio@e1120 {
>> 				gpios =<&gpio0 0 0
>> 					&gpio0 1 0>;
>>
>> 				tbi0: tbi-phy@8 {
>> 					reg =<0x8>;
>> 					device_type = "tbi-phy";
>> 				};
>>
>> 				phy0: ethernet-phy@1c {
>> 					reg =<0x1c>;
>> 				};
>> 			};
>> 		};
>>
>> What am I missing?
> 
> For starters, I do not see any compatible properties that would allow 
> the proper drivers to be bound to anything.

Ok, that makes sense.

> Also I see no MDIO mux node there, so it is unclear why you are even 
> asking these questions.

I only gave you part of the device tree.  Here's my mdio mux node:

	mdio-mux {
		compatible = "mdio-mux-gpio";
		gpios = <&gpio0 0 0>, <&gpio0 1 0>;
		mdio-parent-bus = <&mdio0>;
		#address-cells = <1>;
		#size-cells = <0>;

		mdio@2 {
			reg = <2>;
			#address-cells = <1>;
			#size-cells = <0>;

			phy21: ethernet-phy@1 {
				reg = <1>;
//				compatible = "marvell,88e1149r", "ethernet-phy-ieee802.3-c22";
				marvell,reg-init = <3 0x10 0 0x5777>,
					<3 0x11 0 0x00aa>,
					<3 0x12 0 0x4105>,
					<3 0x13 0 0x0a60>;
				interrupt-parent = <&gpio0>;
//				interrupts = <10 8>; /* Pin 10, active low */
			};
		};
	};
};




> 
> David Daney
> 


-- 
Timur Tabi
Linux kernel developer at Freescale

^ permalink raw reply

* Re: [PATCH v6 0/3] netdev/of/phy: MDIO bus multiplexer support.
From: David Daney @ 2012-05-24 19:19 UTC (permalink / raw)
  To: Timur Tabi
  Cc: netdev@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <4FBE8605.2020507@freescale.com>

On 05/24/2012 12:03 PM, Timur Tabi wrote:
> David Daney wrote:
>
>> Well, the MDIO bus must have an associated device tree node.
>>
>> For my OCTEON code, the MDIO bus device is created as a result of the
>> call to of_platform_bus_probe(), which takes care of filling in all the
>> device tree nodes of the devices it finds and creates.
>
> Ok, let me give you some background.  We actually already have MDIO muxing
> code in-house, but it's different from yours.  So now I'm rewriting it to
> use your design instead.
>
> So our current code looks for "virtual MDIO nodes", and we call
> mdiobus_alloc() and then of_mdiobus_register().  I think this is what I'm
> missing now.
>
> I just don't know what to do next.

You will have to debug it and find out why the device match is failing, 
then fix it.

David Daney

^ permalink raw reply

* module loading issue/flaw in busy memory situation?
From: Wrobel Heinz-R39252 @ 2012-05-24 20:05 UTC (permalink / raw)
  To: linuxppc-dev@lists.ozlabs.org

[-- Attachment #1: Type: text/plain, Size: 1905 bytes --]

Hi,

let's assume a module gets loaded into an already busy system, and the ".init.text" section with the __init function gets loaded into one memory region, and the normal ".text" section gets loaded into a totally different memory region.
Now assume that both regions are >32MB apart in addressing, so that a call from the __init .init.text function to any function in .text requires a trampoline as set up by the do_plt_call() function in arch/kernel/module*.c
So far so good for user code.

Now assume that the __init function is not trivial and will require register save/restore functions in prologue/epilogue with such calls generated by gcc, e.g., the __init function calls _rest32gpr_28_x() in the epilogue. This restore functions however is in the .text section due to the static link of the normal libs.

Now we have the __init function calling the trampoline, which is destroying r11. The trampoline is then jumping to the register restore function which relies on r11 still being intact, which it now isn't anymore.
Net result is a crash because the trampoline ABI conflicts with the register restore ABI and you get a case of garbage in leading to garbage out.

This situation has apparently occurred based on the debug results I have here.

In the general case of module development it seems unpredictable if gcc will actually call a register restore function from the __init function, or if the sections get loaded to require a trampoline, so for any non-trivial function in non-trivial memory setups, a crash would have to be expected, depending on the time of day and moon phase when the module gets loaded.

Is this a fundamental flaw in the interaction of the module section use specification and the module load mechanism with the ABI definition?

Or am I missing some incorrect setup or requirement for __init functions that I should deal with?

Thanks,

Heinz

[-- Attachment #2: Type: text/html, Size: 4382 bytes --]

^ permalink raw reply

* Re: Oops with Radeon/Uninorth on Maple
From: Benjamin Herrenschmidt @ 2012-05-25  0:59 UTC (permalink / raw)
  To: Dmitry Eremin-Solenikov
  Cc: David Airlie, Paul Mackerras, linuxppc-dev, Michel Dänzer,
	dri-devel
In-Reply-To: <4FBDD2CF.5060008@mentor.com>

On Thu, 2012-05-24 at 10:18 +0400, Dmitry Eremin-Solenikov wrote:
> Hello, colleagues,
> 
> I'm trying to enable an AGP slot (again) on my Maple board (dual 
> PPC970FX board, with CPC925 (U3H) north bridge).
> 
>   For now I'm stuck with a problem: I use radeon card, drm-radeon driver 
> with KMS.
> 
> If I force drm-radeon to think about a card as about PCI card (by 
> commenting corresponding lines in drm_radeon_kms.c), everything works, I 
> get framebuffer, working X11, etc. If I enable agpgart-uninorth driver
> and RADEON_IS_AGP flag in drm driver, I get an Oops early during the 
> bootstrap. Relevant part of the log (I can send full dmesg of normal 
> bootstrap or this oops on request, if that would help).

Machine Check probably means that there's a HW configuration issue,
possibly something missing in the initialization of the AGP hardware to
make it actually work.

Cheers,
Ben.

> [    2.820647] Linux agpgart interface v0.103
> [    2.824909] agpgart-uninorth 0000:f0:0b.0: Apple U3H chipset
> [    2.830668] agpgart-uninorth 0000:f0:0b.0: Found device u3, rev 35
> [    2.843611] agpgart-uninorth 0000:f0:0b.0: configuring for size idx: 64
> [    2.850638] agpgart-uninorth 0000:f0:0b.0: AGP aperture is 256M @ 0x0
> [    2.857646] [drm] Initialized drm 1.1.0 20060810
> [    2.862567] [drm] radeon defaulting to kernel modesetting.
> [    2.868091] [drm] radeon kernel modesetting enabled.
> [    2.873222] radeon 0000:f0:10.0: enabling device (0000 -> 0003)
> [    2.880311] radeon 0000:f0:10.0: enabling bus mastering
> [    2.885591] [drm] initializing kernel modesetting (RV350 
> 0x1002:0x4152 0x18BC:0x0416).
> [    2.893629] [drm] register mmio base: 0xD0020000
> [    2.898260] [drm] register mmio size: 65536
> [    2.947112] [drm] GPU not posted. posting now...
> [    3.051033] agpgart-uninorth 0000:f0:0b.0: putting AGP V3 device into 
> 8x mode
> [    3.058197] radeon 0000:f0:10.0: putting AGP V3 device into 8x mode
> [    3.064666] radeon 0000:f0:10.0: GTT: 256M 0x00000000 - 0x0FFFFFFF
> [    3.070864] [drm] Generation 2 PCI interface, using max accessible memory
> [    3.077672] radeon 0000:f0:10.0: VRAM: 128M 0x00000000C0000000 - 
> 0x00000000C7FFFFFF (128M used)
> [    3.086487] [drm] Supports vblank timestamp caching Rev 1 (10.10.2010).
> [    3.093126] [drm] Driver supports precise vblank timestamp query.
> [    3.099291] [drm] radeon: irq initialized.
> [    3.103404] [drm] Detected VRAM RAM=128M, BAR=128M
> [    3.108214] [drm] RAM width 128bits DDR
> [    3.112263] [TTM] Zone  kernel: Available graphics memory: 496682 kiB
> [    3.118732] [TTM] Initializing pool allocator
> [    3.123346] [drm] radeon: 128M of VRAM memory ready
> [    3.128256] [drm] radeon: 256M of GTT memory ready.
> [    3.133295] [drm] radeon: ib pool ready.
> [    3.137708] [drm] radeon: 1 quad pipes, 1 Z pipes initialized.
> [    3.144018] radeon 0000:f0:10.0: WB disabled
> [    3.148326] [drm] fence driver on ring 0 use gpu addr 0x00000000 and 
> cpu addr 0xd000000000066000
> [    3.157474] [drm] Loading R300 Microcode
> [    3.162480] [drm] radeon: ring at 0x0000000000001000
> [    3.167569] [drm] ring test succeeded in 0 usecs
> cpu 0x0: Vector: 200 (Machine Check) at [c000000000d63aa0]
>      pc: c0000000000cc07c: .trace_hardirqs_on_caller+0x6c/0x190
>      lr: c0000000000152f4: .cpu_idle+0x1a4/0x220
>      sp: c000000000d63d20
>     msr: 9000000000009032
>    current = 0xc000000000c4db30
>    paca    = 0xc00000000ffff000   softe: 0        irq_happened: 0x01
>      pid   = 0, comm = swapper/0
> enter ? for help
> [c000000000d63db0] c0000000000152f4 .cpu_idle+0x1a4/0x220
> [c000000000d63e50] c000000000008fb8 .rest_init+0xe8/0x110
> [c000000000d63ee0] c000000000ba2998 .start_kernel+0x3e4/0x408
> [c000000000d63f90] c000000000007558 .start_here_common+0x20/0x48
> 0:mon> x
> [  843.783295] Oops: Machine check, sig: 7 [#1]
> [  843.787589] SMP NR_CPUS=4 Maple
> [  843.790768] Modules linked in:
> [  843.793855] NIP: c0000000000cc07c LR: c0000000000152f4 CTR: 
> c000000000023eac
> [  843.800920] REGS: c000000000d63aa0 TRAP: 0200   Not tainted  (3.4.0+)
> [  843.807376] MSR: 9000000000009032 <SF,HV,EE,ME,IR,DR,RI>  CR: 
> 24222222  XER: 00000006
> [  843.815412] SOFTE: 0
> [  843.817607] TASK = c000000000c4db30[0] 'swapper/0' THREAD: 
> c000000000d60000 CPU: 0
> [  843.825035] GPR00: 0000000000000000 c000000000d63d20 c000000000d63280 
> c0000000000152f4
> [  843.833169] GPR04: 0000000000000000 c000000000099d10 0000000000000001 
> 0000000000000002
> [  843.841302] GPR08: 0100000000000000 c000000000e828e8 0140000000000000 
> 0000000000000000
> [  843.849436] GPR12: 0000000044222282 c00000000ffff000 0000000000000000 
> 0000000000000000
> [  843.857570] GPR16: 0000000000ff8750 0000000000cdc890 00000000010001e0 
> 0000000000000000
> [  843.865702] GPR20: 0000000000000000 0000000000000000 000000001dcd6500 
> 0000000000000000
> [  843.873835] GPR24: 0000000000000000 0000000000ec7b00 9000000000009032 
> c000000000d7b178
> [  843.881979] GPR28: c000000000d7b278 0000000000000008 c000000000c970f8 
> c0000000000152f4
> [  843.890314] NIP [c0000000000cc07c] .trace_hardirqs_on_caller+0x6c/0x190
> [  843.896942] LR [c0000000000152f4] .cpu_idle+0x1a4/0x220
> [  843.902181] Call Trace:
> [  843.904640] [c000000000d63d20] [c000000000d63db0] 
> init_thread_union+0x3db0/0x4000 (unreliable)
> [  843.913317] [c000000000d63db0] [c0000000000152f4] .cpu_idle+0x1a4/0x220
> [  843.919964] [c000000000d63e50] [c000000000008fb8] .rest_init+0xe8/0x110
> [  843.926615] [c000000000d63ee0] [c000000000ba2998] 
> .start_kernel+0x3e4/0x408
> [  843.933611] [c000000000d63f90] [c000000000007558] 
> .start_here_common+0x20/0x48
> [  843.940866] Instruction dump:
> [  843.943868] 40de00a4 e92d01c8 800908e8 2f800000 40de0094 e93e8098 
> 80090000 2f800000
> [  843.951829] 409e0030 880d01f2 2fa00000 40de00b0 <e93e84e8> 88090000 
> 2f800000 40de00c8
> 
> 
> Looking for any suggestions on this.
> 

^ permalink raw reply

* Re: [PATCH v3 1/2] powerpc/PCI: move DMA & IRQ init to device_add() notification path
From: Benjamin Herrenschmidt @ 2012-05-25  3:00 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Michal Simek, Hiroo Matsumoto, microblaze-uclinux,
	Kenji Kaneshige, Jesse Larrew, jbarnes, Dominik Brodowski,
	linux-pci, linuxppc-dev
In-Reply-To: <20120523223700.24276.76804.stgit@bhelgaas.mtv.corp.google.com>

On Wed, 2012-05-23 at 16:37 -0600, Bjorn Helgaas wrote:
> From: Hiroo Matsumoto <matsumoto.hiroo@jp.fujitsu.com>
> 
> PowerPC initialized DMA and IRQ information in the pci_scan_child_bus()
> -> pcibios_fixup_bus() path.  Some hotplug drivers use that path, but
> others don't, e.g., pciehp, so sometimes hot-added devices are only
> partly initialized.
> 
> This patch moves that initialization from pcibios_fixup_bus() to a new
> pci_bus_notify() called in the pci_bus_add_device() -> device_add() path.
> That means the initialization happens the same way for all PCI devices,
> whether they are present at boot or hot-added later.
> 
> Note that powerpc was the only user of pci_fixup_cardbus(), which was
> used to do this same setup for cardbus devices.  That's no longer
> needed because this setup will now be done in the same device_add()
> notification path as all other PCI devices.

Hrm. That will require a good deal of testing... Unfortunately I'm out
for a few weeks getting some surgery and then recovering...

Our PCI code has ancient roots and I wouldn't be surprised if that
change breaks subtle assumptions made here or there, we'd need to test
at least on a good range of macs and ibm hotplug stuff.

Cheers,
Ben.

> Typical failure of a hot-added e1000e device prior to this change:
> 
>     # echo 1 > /sys/bus/pci/slots/1/power
>     <snip>
>     e1000e 0000:03:00.0: enabling device (0000 -> 0002)
>     e1000e 0000:03:00.0: No usable DMA configuration, aborting
>     e1000e: probe of 0000:03:00.0 failed with error -5
> 
> Successful initialization after this change:
> 
>     # echo 1 > /sys/bus/pci/slots/1/power
>     <snip>
>     e1000e 0000:03:00.0: enabling device (0000 -> 0002)
>     irq: irq 6 on host /soc@ffe00000/msi@41600 mapped to virtual irq 27
>     e1000e 0000:03:00.0: eth0: (PCI Express:2.5GT/s:Width x1) 00:15:17:bf:c0:c9
>     e1000e 0000:03:00.0: eth0: Intel(R) PRO/1000 Network Connection
>     e1000e 0000:03:00.0: eth0: MAC: 1, PHY: 4, PBA No: D50861-003
> 
> [bhelgaas: changelog, notifier name, registration can be __init]
> CC: Dominik Brodowski <linux@dominikbrodowski.net>
> Signed-off-by: Hiroo MATSUMOTO <matsumoto.hiroo@jp.fujitsu.com>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
>  arch/powerpc/include/asm/pci.h             |    2 -
>  arch/powerpc/kernel/pci-common.c           |   87 ++++++++++++++--------------
>  arch/powerpc/kernel/pci_32.c               |    2 +
>  arch/powerpc/kernel/pci_64.c               |    2 +
>  arch/powerpc/kernel/pci_of_scan.c          |    1 
>  arch/powerpc/platforms/pseries/pci_dlpar.c |    1 
>  drivers/pci/pci.c                          |    5 --
>  drivers/pcmcia/cardbus.c                   |    3 -
>  include/linux/pci.h                        |    3 -
>  9 files changed, 49 insertions(+), 57 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h
> index 6653f27..d6a36a4 100644
> --- a/arch/powerpc/include/asm/pci.h
> +++ b/arch/powerpc/include/asm/pci.h
> @@ -183,10 +183,10 @@ extern void pci_resource_to_user(const struct pci_dev *dev, int bar,
>  				 resource_size_t *start, resource_size_t *end);
>  
>  extern resource_size_t pcibios_io_space_offset(struct pci_controller *hose);
> -extern void pcibios_setup_bus_devices(struct pci_bus *bus);
>  extern void pcibios_setup_bus_self(struct pci_bus *bus);
>  extern void pcibios_setup_phb_io_space(struct pci_controller *hose);
>  extern void pcibios_scan_phb(struct pci_controller *hose);
> +extern void pcibios_setup_bus_notifier(void);
>  
>  #endif	/* __KERNEL__ */
>  #endif /* __ASM_POWERPC_PCI_H */
> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> index 7320f36..41b39ba 100644
> --- a/arch/powerpc/kernel/pci-common.c
> +++ b/arch/powerpc/kernel/pci-common.c
> @@ -1009,40 +1009,6 @@ void __devinit pcibios_setup_bus_self(struct pci_bus *bus)
>  		ppc_md.pci_dma_bus_setup(bus);
>  }
>  
> -void __devinit pcibios_setup_bus_devices(struct pci_bus *bus)
> -{
> -	struct pci_dev *dev;
> -
> -	pr_debug("PCI: Fixup bus devices %d (%s)\n",
> -		 bus->number, bus->self ? pci_name(bus->self) : "PHB");
> -
> -	list_for_each_entry(dev, &bus->devices, bus_list) {
> -		/* Cardbus can call us to add new devices to a bus, so ignore
> -		 * those who are already fully discovered
> -		 */
> -		if (dev->is_added)
> -			continue;
> -
> -		/* Fixup NUMA node as it may not be setup yet by the generic
> -		 * code and is needed by the DMA init
> -		 */
> -		set_dev_node(&dev->dev, pcibus_to_node(dev->bus));
> -
> -		/* Hook up default DMA ops */
> -		set_dma_ops(&dev->dev, pci_dma_ops);
> -		set_dma_offset(&dev->dev, PCI_DRAM_OFFSET);
> -
> -		/* Additional platform DMA/iommu setup */
> -		if (ppc_md.pci_dma_dev_setup)
> -			ppc_md.pci_dma_dev_setup(dev);
> -
> -		/* Read default IRQs and fixup if necessary */
> -		pci_read_irq_line(dev);
> -		if (ppc_md.pci_irq_fixup)
> -			ppc_md.pci_irq_fixup(dev);
> -	}
> -}
> -
>  void pcibios_set_master(struct pci_dev *dev)
>  {
>  	/* No special bus mastering setup handling */
> @@ -1059,19 +1025,9 @@ void __devinit pcibios_fixup_bus(struct pci_bus *bus)
>  
>  	/* Now fixup the bus bus */
>  	pcibios_setup_bus_self(bus);
> -
> -	/* Now fixup devices on that bus */
> -	pcibios_setup_bus_devices(bus);
>  }
>  EXPORT_SYMBOL(pcibios_fixup_bus);
>  
> -void __devinit pci_fixup_cardbus(struct pci_bus *bus)
> -{
> -	/* Now fixup devices on that bus */
> -	pcibios_setup_bus_devices(bus);
> -}
> -
> -
>  static int skip_isa_ioresource_align(struct pci_dev *dev)
>  {
>  	if (pci_has_flag(PCI_CAN_SKIP_ISA_ALIGN) &&
> @@ -1685,6 +1641,49 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose)
>  	}
>  }
>  
> +static int pci_bus_notify(struct notifier_block *nb, unsigned long action,
> +			  void *data)
> +{
> +	struct pci_dev *dev = to_pci_dev(data);
> +
> +	switch (action) {
> +	case BUS_NOTIFY_ADD_DEVICE:
> +		/* 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
> +		 */
> +		set_dev_node(&dev->dev, pcibus_to_node(dev->bus));
> +
> +		/* Hook up default DMA ops */
> +		set_dma_ops(&dev->dev, pci_dma_ops);
> +		set_dma_offset(&dev->dev, PCI_DRAM_OFFSET);
> +
> +		/* Additional platform DMA/iommu setup */
> +		if (ppc_md.pci_dma_dev_setup)
> +			ppc_md.pci_dma_dev_setup(dev);
> +
> +		/* Read default IRQs and fixup if necessary */
> +		pci_read_irq_line(dev);
> +		if (ppc_md.pci_irq_fixup)
> +			ppc_md.pci_irq_fixup(dev);
> +
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +static struct notifier_block device_nb = {
> +	.notifier_call = pci_bus_notify,
> +};
> +
> +void __init pcibios_setup_bus_notifier(void)
> +{
> +	bus_register_notifier(&pci_bus_type, &device_nb);
> +}
> +
>  static void fixup_hide_host_resource_fsl(struct pci_dev *dev)
>  {
>  	int i, class = dev->class >> 8;
> diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c
> index 4b06ec5..640cc35 100644
> --- a/arch/powerpc/kernel/pci_32.c
> +++ b/arch/powerpc/kernel/pci_32.c
> @@ -231,6 +231,8 @@ static int __init pcibios_init(void)
>  
>  	printk(KERN_INFO "PCI: Probing PCI hardware\n");
>  
> +	pcibios_setup_bus_notifier();
> +
>  	if (pci_has_flag(PCI_REASSIGN_ALL_BUS))
>  		pci_assign_all_buses = 1;
>  
> diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
> index 4ff190f..8b212d3 100644
> --- a/arch/powerpc/kernel/pci_64.c
> +++ b/arch/powerpc/kernel/pci_64.c
> @@ -48,6 +48,8 @@ static int __init pcibios_init(void)
>  
>  	printk(KERN_INFO "PCI: Probing PCI hardware\n");
>  
> +	pcibios_setup_bus_notifier();
> +
>  	/* For now, override phys_mem_access_prot. If we need it,g
>  	 * later, we may move that initialization to each ppc_md
>  	 */
> diff --git a/arch/powerpc/kernel/pci_of_scan.c b/arch/powerpc/kernel/pci_of_scan.c
> index ae5ea5e..eb09eca 100644
> --- a/arch/powerpc/kernel/pci_of_scan.c
> +++ b/arch/powerpc/kernel/pci_of_scan.c
> @@ -333,7 +333,6 @@ static void __devinit __of_scan_bus(struct device_node *node,
>  	 */
>  	if (!rescan_existing)
>  		pcibios_setup_bus_self(bus);
> -	pcibios_setup_bus_devices(bus);
>  
>  	/* Now scan child busses */
>  	list_for_each_entry(dev, &bus->devices, bus_list) {
> diff --git a/arch/powerpc/platforms/pseries/pci_dlpar.c b/arch/powerpc/platforms/pseries/pci_dlpar.c
> index 3ccebc8..0b1b6b3 100644
> --- a/arch/powerpc/platforms/pseries/pci_dlpar.c
> +++ b/arch/powerpc/platforms/pseries/pci_dlpar.c
> @@ -120,7 +120,6 @@ void pcibios_add_pci_devices(struct pci_bus * bus)
>  		num = pci_scan_slot(bus, PCI_DEVFN(slotno, 0));
>  		if (!num)
>  			return;
> -		pcibios_setup_bus_devices(bus);
>  		max = bus->busn_res.start;
>  		for (pass=0; pass < 2; pass++)
>  			list_for_each_entry(dev, &bus->devices, bus_list) {
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 15d442a..43e0a4f 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -3789,11 +3789,6 @@ int __attribute__ ((weak)) pci_ext_cfg_avail(struct pci_dev *dev)
>  	return 1;
>  }
>  
> -void __weak pci_fixup_cardbus(struct pci_bus *bus)
> -{
> -}
> -EXPORT_SYMBOL(pci_fixup_cardbus);
> -
>  static int __init pci_setup(char *str)
>  {
>  	while (str) {
> diff --git a/drivers/pcmcia/cardbus.c b/drivers/pcmcia/cardbus.c
> index 24caeaf..a980691 100644
> --- a/drivers/pcmcia/cardbus.c
> +++ b/drivers/pcmcia/cardbus.c
> @@ -71,7 +71,6 @@ int __ref cb_alloc(struct pcmcia_socket *s)
>  	unsigned int max, pass;
>  
>  	s->functions = pci_scan_slot(bus, PCI_DEVFN(0, 0));
> -	pci_fixup_cardbus(bus);
>  
>  	max = bus->busn_res.start;
>  	for (pass = 0; pass < 2; pass++)
> @@ -85,7 +84,6 @@ int __ref cb_alloc(struct pcmcia_socket *s)
>  	 */
>  	pci_bus_size_bridges(bus);
>  	pci_bus_assign_resources(bus);
> -	cardbus_config_irq_and_cls(bus, s->pci_irq);
>  
>  	/* socket specific tune function */
>  	if (s->tune_bridge)
> @@ -93,6 +91,7 @@ int __ref cb_alloc(struct pcmcia_socket *s)
>  
>  	pci_enable_bridges(bus);
>  	pci_bus_add_devices(bus);
> +	cardbus_config_irq_and_cls(bus, s->pci_irq);
>  
>  	return 0;
>  }
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index a0e2d7f..3924c02 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -651,9 +651,6 @@ resource_size_t pcibios_align_resource(void *, const struct resource *,
>  				resource_size_t);
>  void pcibios_update_irq(struct pci_dev *, int irq);
>  
> -/* Weak but can be overriden by arch */
> -void pci_fixup_cardbus(struct pci_bus *);
> -
>  /* Generic PCI functions used internally */
>  
>  void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region,

^ permalink raw reply

* Re: [PATCH v3 1/2] powerpc/PCI: move DMA & IRQ init to device_add() notification path
From: Bjorn Helgaas @ 2012-05-25  3:08 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Michal Simek, Hiroo Matsumoto, microblaze-uclinux,
	Kenji Kaneshige, Jesse Larrew, jbarnes, Dominik Brodowski,
	linux-pci, linuxppc-dev
In-Reply-To: <1337914811.16119.7.camel@pasglop>

On Thu, May 24, 2012 at 9:00 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Wed, 2012-05-23 at 16:37 -0600, Bjorn Helgaas wrote:
>> From: Hiroo Matsumoto <matsumoto.hiroo@jp.fujitsu.com>
>>
>> PowerPC initialized DMA and IRQ information in the pci_scan_child_bus()
>> -> pcibios_fixup_bus() path. =A0Some hotplug drivers use that path, but
>> others don't, e.g., pciehp, so sometimes hot-added devices are only
>> partly initialized.
>>
>> This patch moves that initialization from pcibios_fixup_bus() to a new
>> pci_bus_notify() called in the pci_bus_add_device() -> device_add() path=
.
>> That means the initialization happens the same way for all PCI devices,
>> whether they are present at boot or hot-added later.
>>
>> Note that powerpc was the only user of pci_fixup_cardbus(), which was
>> used to do this same setup for cardbus devices. =A0That's no longer
>> needed because this setup will now be done in the same device_add()
>> notification path as all other PCI devices.
>
> Hrm. That will require a good deal of testing... Unfortunately I'm out
> for a few weeks getting some surgery and then recovering...
>
> Our PCI code has ancient roots and I wouldn't be surprised if that
> change breaks subtle assumptions made here or there, we'd need to test
> at least on a good range of macs and ibm hotplug stuff.

OK.  Are you worried about cardbus in particular?

This is headed for the 3.6, not 3.5, so we should have plenty of time.
 As soon as everything for the current merge window gets merged and
-next is ready for the next batch, I'll put this in there.

>> Typical failure of a hot-added e1000e device prior to this change:
>>
>> =A0 =A0 # echo 1 > /sys/bus/pci/slots/1/power
>> =A0 =A0 <snip>
>> =A0 =A0 e1000e 0000:03:00.0: enabling device (0000 -> 0002)
>> =A0 =A0 e1000e 0000:03:00.0: No usable DMA configuration, aborting
>> =A0 =A0 e1000e: probe of 0000:03:00.0 failed with error -5
>>
>> Successful initialization after this change:
>>
>> =A0 =A0 # echo 1 > /sys/bus/pci/slots/1/power
>> =A0 =A0 <snip>
>> =A0 =A0 e1000e 0000:03:00.0: enabling device (0000 -> 0002)
>> =A0 =A0 irq: irq 6 on host /soc@ffe00000/msi@41600 mapped to virtual irq=
 27
>> =A0 =A0 e1000e 0000:03:00.0: eth0: (PCI Express:2.5GT/s:Width x1) 00:15:=
17:bf:c0:c9
>> =A0 =A0 e1000e 0000:03:00.0: eth0: Intel(R) PRO/1000 Network Connection
>> =A0 =A0 e1000e 0000:03:00.0: eth0: MAC: 1, PHY: 4, PBA No: D50861-003
>>
>> [bhelgaas: changelog, notifier name, registration can be __init]
>> CC: Dominik Brodowski <linux@dominikbrodowski.net>
>> Signed-off-by: Hiroo MATSUMOTO <matsumoto.hiroo@jp.fujitsu.com>
>> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>> ---
>> =A0arch/powerpc/include/asm/pci.h =A0 =A0 =A0 =A0 =A0 =A0 | =A0 =A02 -
>> =A0arch/powerpc/kernel/pci-common.c =A0 =A0 =A0 =A0 =A0 | =A0 87 +++++++=
+++++++--------------
>> =A0arch/powerpc/kernel/pci_32.c =A0 =A0 =A0 =A0 =A0 =A0 =A0 | =A0 =A02 +
>> =A0arch/powerpc/kernel/pci_64.c =A0 =A0 =A0 =A0 =A0 =A0 =A0 | =A0 =A02 +
>> =A0arch/powerpc/kernel/pci_of_scan.c =A0 =A0 =A0 =A0 =A0| =A0 =A01
>> =A0arch/powerpc/platforms/pseries/pci_dlpar.c | =A0 =A01
>> =A0drivers/pci/pci.c =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
| =A0 =A05 --
>> =A0drivers/pcmcia/cardbus.c =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 | =A0 =
=A03 -
>> =A0include/linux/pci.h =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0| =
=A0 =A03 -
>> =A09 files changed, 49 insertions(+), 57 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/p=
ci.h
>> index 6653f27..d6a36a4 100644
>> --- a/arch/powerpc/include/asm/pci.h
>> +++ b/arch/powerpc/include/asm/pci.h
>> @@ -183,10 +183,10 @@ extern void pci_resource_to_user(const struct pci_=
dev *dev, int bar,
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0resource_=
size_t *start, resource_size_t *end);
>>
>> =A0extern resource_size_t pcibios_io_space_offset(struct pci_controller =
*hose);
>> -extern void pcibios_setup_bus_devices(struct pci_bus *bus);
>> =A0extern void pcibios_setup_bus_self(struct pci_bus *bus);
>> =A0extern void pcibios_setup_phb_io_space(struct pci_controller *hose);
>> =A0extern void pcibios_scan_phb(struct pci_controller *hose);
>> +extern void pcibios_setup_bus_notifier(void);
>>
>> =A0#endif =A0 =A0 =A0 /* __KERNEL__ */
>> =A0#endif /* __ASM_POWERPC_PCI_H */
>> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-=
common.c
>> index 7320f36..41b39ba 100644
>> --- a/arch/powerpc/kernel/pci-common.c
>> +++ b/arch/powerpc/kernel/pci-common.c
>> @@ -1009,40 +1009,6 @@ void __devinit pcibios_setup_bus_self(struct pci_=
bus *bus)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 ppc_md.pci_dma_bus_setup(bus);
>> =A0}
>>
>> -void __devinit pcibios_setup_bus_devices(struct pci_bus *bus)
>> -{
>> - =A0 =A0 struct pci_dev *dev;
>> -
>> - =A0 =A0 pr_debug("PCI: Fixup bus devices %d (%s)\n",
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0bus->number, bus->self ? pci_name(bus->self=
) : "PHB");
>> -
>> - =A0 =A0 list_for_each_entry(dev, &bus->devices, bus_list) {
>> - =A0 =A0 =A0 =A0 =A0 =A0 /* Cardbus can call us to add new devices to a=
 bus, so ignore
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0* those who are already fully discovered
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0*/
>> - =A0 =A0 =A0 =A0 =A0 =A0 if (dev->is_added)
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 continue;
>> -
>> - =A0 =A0 =A0 =A0 =A0 =A0 /* Fixup NUMA node as it may not be setup yet =
by the generic
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0* code and is needed by the DMA init
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0*/
>> - =A0 =A0 =A0 =A0 =A0 =A0 set_dev_node(&dev->dev, pcibus_to_node(dev->bu=
s));
>> -
>> - =A0 =A0 =A0 =A0 =A0 =A0 /* Hook up default DMA ops */
>> - =A0 =A0 =A0 =A0 =A0 =A0 set_dma_ops(&dev->dev, pci_dma_ops);
>> - =A0 =A0 =A0 =A0 =A0 =A0 set_dma_offset(&dev->dev, PCI_DRAM_OFFSET);
>> -
>> - =A0 =A0 =A0 =A0 =A0 =A0 /* Additional platform DMA/iommu setup */
>> - =A0 =A0 =A0 =A0 =A0 =A0 if (ppc_md.pci_dma_dev_setup)
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ppc_md.pci_dma_dev_setup(dev);
>> -
>> - =A0 =A0 =A0 =A0 =A0 =A0 /* Read default IRQs and fixup if necessary */
>> - =A0 =A0 =A0 =A0 =A0 =A0 pci_read_irq_line(dev);
>> - =A0 =A0 =A0 =A0 =A0 =A0 if (ppc_md.pci_irq_fixup)
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ppc_md.pci_irq_fixup(dev);
>> - =A0 =A0 }
>> -}
>> -
>> =A0void pcibios_set_master(struct pci_dev *dev)
>> =A0{
>> =A0 =A0 =A0 /* No special bus mastering setup handling */
>> @@ -1059,19 +1025,9 @@ void __devinit pcibios_fixup_bus(struct pci_bus *=
bus)
>>
>> =A0 =A0 =A0 /* Now fixup the bus bus */
>> =A0 =A0 =A0 pcibios_setup_bus_self(bus);
>> -
>> - =A0 =A0 /* Now fixup devices on that bus */
>> - =A0 =A0 pcibios_setup_bus_devices(bus);
>> =A0}
>> =A0EXPORT_SYMBOL(pcibios_fixup_bus);
>>
>> -void __devinit pci_fixup_cardbus(struct pci_bus *bus)
>> -{
>> - =A0 =A0 /* Now fixup devices on that bus */
>> - =A0 =A0 pcibios_setup_bus_devices(bus);
>> -}
>> -
>> -
>> =A0static int skip_isa_ioresource_align(struct pci_dev *dev)
>> =A0{
>> =A0 =A0 =A0 if (pci_has_flag(PCI_CAN_SKIP_ISA_ALIGN) &&
>> @@ -1685,6 +1641,49 @@ void __devinit pcibios_scan_phb(struct pci_contro=
ller *hose)
>> =A0 =A0 =A0 }
>> =A0}
>>
>> +static int pci_bus_notify(struct notifier_block *nb, unsigned long acti=
on,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 void *data)
>> +{
>> + =A0 =A0 struct pci_dev *dev =3D to_pci_dev(data);
>> +
>> + =A0 =A0 switch (action) {
>> + =A0 =A0 case BUS_NOTIFY_ADD_DEVICE:
>> + =A0 =A0 =A0 =A0 =A0 =A0 /* Setup OF node pointer in the device */
>> + =A0 =A0 =A0 =A0 =A0 =A0 dev->dev.of_node =3D pci_device_to_OF_node(dev=
);
>> +
>> + =A0 =A0 =A0 =A0 =A0 =A0 /* Fixup NUMA node as it may not be setup yet =
by the generic
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0* code and is needed by the DMA init
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0*/
>> + =A0 =A0 =A0 =A0 =A0 =A0 set_dev_node(&dev->dev, pcibus_to_node(dev->bu=
s));
>> +
>> + =A0 =A0 =A0 =A0 =A0 =A0 /* Hook up default DMA ops */
>> + =A0 =A0 =A0 =A0 =A0 =A0 set_dma_ops(&dev->dev, pci_dma_ops);
>> + =A0 =A0 =A0 =A0 =A0 =A0 set_dma_offset(&dev->dev, PCI_DRAM_OFFSET);
>> +
>> + =A0 =A0 =A0 =A0 =A0 =A0 /* Additional platform DMA/iommu setup */
>> + =A0 =A0 =A0 =A0 =A0 =A0 if (ppc_md.pci_dma_dev_setup)
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ppc_md.pci_dma_dev_setup(dev);
>> +
>> + =A0 =A0 =A0 =A0 =A0 =A0 /* Read default IRQs and fixup if necessary */
>> + =A0 =A0 =A0 =A0 =A0 =A0 pci_read_irq_line(dev);
>> + =A0 =A0 =A0 =A0 =A0 =A0 if (ppc_md.pci_irq_fixup)
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ppc_md.pci_irq_fixup(dev);
>> +
>> + =A0 =A0 =A0 =A0 =A0 =A0 break;
>> + =A0 =A0 }
>> +
>> + =A0 =A0 return 0;
>> +}
>> +
>> +static struct notifier_block device_nb =3D {
>> + =A0 =A0 .notifier_call =3D pci_bus_notify,
>> +};
>> +
>> +void __init pcibios_setup_bus_notifier(void)
>> +{
>> + =A0 =A0 bus_register_notifier(&pci_bus_type, &device_nb);
>> +}
>> +
>> =A0static void fixup_hide_host_resource_fsl(struct pci_dev *dev)
>> =A0{
>> =A0 =A0 =A0 int i, class =3D dev->class >> 8;
>> diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c
>> index 4b06ec5..640cc35 100644
>> --- a/arch/powerpc/kernel/pci_32.c
>> +++ b/arch/powerpc/kernel/pci_32.c
>> @@ -231,6 +231,8 @@ static int __init pcibios_init(void)
>>
>> =A0 =A0 =A0 printk(KERN_INFO "PCI: Probing PCI hardware\n");
>>
>> + =A0 =A0 pcibios_setup_bus_notifier();
>> +
>> =A0 =A0 =A0 if (pci_has_flag(PCI_REASSIGN_ALL_BUS))
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 pci_assign_all_buses =3D 1;
>>
>> diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
>> index 4ff190f..8b212d3 100644
>> --- a/arch/powerpc/kernel/pci_64.c
>> +++ b/arch/powerpc/kernel/pci_64.c
>> @@ -48,6 +48,8 @@ static int __init pcibios_init(void)
>>
>> =A0 =A0 =A0 printk(KERN_INFO "PCI: Probing PCI hardware\n");
>>
>> + =A0 =A0 pcibios_setup_bus_notifier();
>> +
>> =A0 =A0 =A0 /* For now, override phys_mem_access_prot. If we need it,g
>> =A0 =A0 =A0 =A0* later, we may move that initialization to each ppc_md
>> =A0 =A0 =A0 =A0*/
>> diff --git a/arch/powerpc/kernel/pci_of_scan.c b/arch/powerpc/kernel/pci=
_of_scan.c
>> index ae5ea5e..eb09eca 100644
>> --- a/arch/powerpc/kernel/pci_of_scan.c
>> +++ b/arch/powerpc/kernel/pci_of_scan.c
>> @@ -333,7 +333,6 @@ static void __devinit __of_scan_bus(struct device_no=
de *node,
>> =A0 =A0 =A0 =A0*/
>> =A0 =A0 =A0 if (!rescan_existing)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 pcibios_setup_bus_self(bus);
>> - =A0 =A0 pcibios_setup_bus_devices(bus);
>>
>> =A0 =A0 =A0 /* Now scan child busses */
>> =A0 =A0 =A0 list_for_each_entry(dev, &bus->devices, bus_list) {
>> diff --git a/arch/powerpc/platforms/pseries/pci_dlpar.c b/arch/powerpc/p=
latforms/pseries/pci_dlpar.c
>> index 3ccebc8..0b1b6b3 100644
>> --- a/arch/powerpc/platforms/pseries/pci_dlpar.c
>> +++ b/arch/powerpc/platforms/pseries/pci_dlpar.c
>> @@ -120,7 +120,6 @@ void pcibios_add_pci_devices(struct pci_bus * bus)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 num =3D pci_scan_slot(bus, PCI_DEVFN(slotno,=
 0));
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!num)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return;
>> - =A0 =A0 =A0 =A0 =A0 =A0 pcibios_setup_bus_devices(bus);
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 max =3D bus->busn_res.start;
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 for (pass=3D0; pass < 2; pass++)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 list_for_each_entry(dev, &bu=
s->devices, bus_list) {
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index 15d442a..43e0a4f 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -3789,11 +3789,6 @@ int __attribute__ ((weak)) pci_ext_cfg_avail(stru=
ct pci_dev *dev)
>> =A0 =A0 =A0 return 1;
>> =A0}
>>
>> -void __weak pci_fixup_cardbus(struct pci_bus *bus)
>> -{
>> -}
>> -EXPORT_SYMBOL(pci_fixup_cardbus);
>> -
>> =A0static int __init pci_setup(char *str)
>> =A0{
>> =A0 =A0 =A0 while (str) {
>> diff --git a/drivers/pcmcia/cardbus.c b/drivers/pcmcia/cardbus.c
>> index 24caeaf..a980691 100644
>> --- a/drivers/pcmcia/cardbus.c
>> +++ b/drivers/pcmcia/cardbus.c
>> @@ -71,7 +71,6 @@ int __ref cb_alloc(struct pcmcia_socket *s)
>> =A0 =A0 =A0 unsigned int max, pass;
>>
>> =A0 =A0 =A0 s->functions =3D pci_scan_slot(bus, PCI_DEVFN(0, 0));
>> - =A0 =A0 pci_fixup_cardbus(bus);
>>
>> =A0 =A0 =A0 max =3D bus->busn_res.start;
>> =A0 =A0 =A0 for (pass =3D 0; pass < 2; pass++)
>> @@ -85,7 +84,6 @@ int __ref cb_alloc(struct pcmcia_socket *s)
>> =A0 =A0 =A0 =A0*/
>> =A0 =A0 =A0 pci_bus_size_bridges(bus);
>> =A0 =A0 =A0 pci_bus_assign_resources(bus);
>> - =A0 =A0 cardbus_config_irq_and_cls(bus, s->pci_irq);
>>
>> =A0 =A0 =A0 /* socket specific tune function */
>> =A0 =A0 =A0 if (s->tune_bridge)
>> @@ -93,6 +91,7 @@ int __ref cb_alloc(struct pcmcia_socket *s)
>>
>> =A0 =A0 =A0 pci_enable_bridges(bus);
>> =A0 =A0 =A0 pci_bus_add_devices(bus);
>> + =A0 =A0 cardbus_config_irq_and_cls(bus, s->pci_irq);
>>
>> =A0 =A0 =A0 return 0;
>> =A0}
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index a0e2d7f..3924c02 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -651,9 +651,6 @@ resource_size_t pcibios_align_resource(void *, const=
 struct resource *,
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 resource_siz=
e_t);
>> =A0void pcibios_update_irq(struct pci_dev *, int irq);
>>
>> -/* Weak but can be overriden by arch */
>> -void pci_fixup_cardbus(struct pci_bus *);
>> -
>> =A0/* Generic PCI functions used internally */
>>
>> =A0void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_regi=
on *region,
>
>

^ permalink raw reply

* Re: [PATCH v3 1/2] powerpc/PCI: move DMA & IRQ init to device_add() notification path
From: Benjamin Herrenschmidt @ 2012-05-25  3:38 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Michal Simek, Hiroo Matsumoto, microblaze-uclinux,
	Kenji Kaneshige, Jesse Larrew, jbarnes, Dominik Brodowski,
	linux-pci, linuxppc-dev
In-Reply-To: <CAErSpo6W28r4f5pJmZr=ofUAaffiUtOqgXWeabGpr+DmKw+Z+g@mail.gmail.com>

On Thu, 2012-05-24 at 21:08 -0600, Bjorn Helgaas wrote:
> 
> OK.  Are you worried about cardbus in particular?
> 
> This is headed for the 3.6, not 3.5, so we should have plenty of time.
>  As soon as everything for the current merge window gets merged and
> -next is ready for the next batch, I'll put this in there.

I'm not that worried about cardbus ... in fact I wouldn't be surprised
if it's already somewhat broken, I haven't tested it in a long time...

I'm more worried about basic functionality and expectations about when
things happen during boot vs. platform hacks, that and pseries specific
kind of hotplug which can be a bit odd.

Cheers,
Ben.

^ permalink raw reply

* Re: module loading issue/flaw in busy memory situation?
From: Steffen Rumler @ 2012-05-25  7:56 UTC (permalink / raw)
  To: linuxppc-dev

Hi,

The basic question is, has the GPR r11 a dedicated function (to point to the previous stack frame)
or is it still a volatile GPR, according to EABI definition ?
In the case of a dedicated function was assigned, the trampoline code generation in

     arch/powerpc/kernel/module_32.c

must be corrected. I'd suggest to use r12 instead of r11, in this case.

Best Regards
Steffen

^ permalink raw reply

* pread() and pwrite() system calls
From: David Laight @ 2012-05-25 13:29 UTC (permalink / raw)
  To: linuxppc-dev

We have a system with linux 2.6.32 and the somewhat archaic
uClibc 0.9.27 (but I'm not sure the current version is
any better, and I think there are binary compatibility
if we update).

I've just discovered that pread() is 'implemented'
by using 3 lseek() system calls and read().
(the same is true for the 64bit versions).

I thought that pread() was supposed to be atomic
(so usable concurrently by multiple threads) which
means that this implementation is completely broken.

I've not looked to see what glibc does.

I can see that part of the problem is the alignment
of the 64bit value on the argument list of syscall()
(when the register save area is cast to a sycall
argument structure).
But it also looks as though the uClibc syscall()
stub can only pass 5 arguments in registers, and
pread() (with an aligned 64bit offset) requires 6.

The ucLibc source seems to be predicated by __NR_pread,
but if that were defined it would try to call
__syscall_pread() and I can't find that anywhere.

A special pread/pwrite asm stub that just copies
r7 to r0 could be used.

Would it be enough to do:
syscall_pread_pwrite:
	mov 0,7
	sc
	blr
and handle the -ve -> errno in C?

I've seen other parts of linux pass structures to
avoid issues with excessive number of arguments.
Unlike things like NetBSD amd64 which will read
additional args off the user stack (and has per-system
call stubs in libc so doesn't lose one argument).
(I rearranged the NetBSD amd64 kernel trap frame
layout so all the args ended up adjacent...)

	David

^ permalink raw reply

* Re: Oops with Radeon/Uninorth on Maple
From: Alex Deucher @ 2012-05-25 15:06 UTC (permalink / raw)
  To: Dmitry Eremin-Solenikov
  Cc: Paul, Mackerras, linuxppc-dev, dri-devel, Michel Dänzer
In-Reply-To: <4FBDD2CF.5060008@mentor.com>

On Thu, May 24, 2012 at 2:18 AM, Dmitry Eremin-Solenikov
<dmitry_eremin@mentor.com> wrote:
> Hello, colleagues,
>
> I'm trying to enable an AGP slot (again) on my Maple board (dual PPC970FX
> board, with CPC925 (U3H) north bridge).
>
> =A0For now I'm stuck with a problem: I use radeon card, drm-radeon driver=
 with
> KMS.
>
> If I force drm-radeon to think about a card as about PCI card (by comment=
ing
> corresponding lines in drm_radeon_kms.c), everything works, I get
> framebuffer, working X11, etc. If I enable agpgart-uninorth driver
> and RADEON_IS_AGP flag in drm driver, I get an Oops early during the
> bootstrap. Relevant part of the log (I can send full dmesg of normal
> bootstrap or this oops on request, if that would help).

For future reference, you can disable AGP by loading radeon with the
agpmode parameter set to -1, e.g., radeon.agpmode=3D-1

No need to edit the source.

Alex

^ permalink raw reply

* Re: pread() and pwrite() system calls
From: Gabriel Paubert @ 2012-05-25 16:45 UTC (permalink / raw)
  To: David Laight; +Cc: linuxppc-dev
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B6F26@saturn3.aculab.com>

On Fri, May 25, 2012 at 02:29:06PM +0100, David Laight wrote:
> We have a system with linux 2.6.32 and the somewhat archaic
> uClibc 0.9.27 (but I'm not sure the current version is
> any better, and I think there are binary compatibility
> if we update).
> 
> I've just discovered that pread() is 'implemented'
> by using 3 lseek() system calls and read().
> (the same is true for the 64bit versions).
> 
> I thought that pread() was supposed to be atomic
> (so usable concurrently by multiple threads) which
> means that this implementation is completely broken.
> 
> I've not looked to see what glibc does.
> 
> I can see that part of the problem is the alignment
> of the 64bit value on the argument list of syscall()
> (when the register save area is cast to a sycall
> argument structure).
> But it also looks as though the uClibc syscall()
> stub can only pass 5 arguments in registers, and
> pread() (with an aligned 64bit offset) requires 6.
> 
> The ucLibc source seems to be predicated by __NR_pread,
> but if that were defined it would try to call
> __syscall_pread() and I can't find that anywhere.
> 
> A special pread/pwrite asm stub that just copies
> r7 to r0 could be used.
> 
> Would it be enough to do:
> syscall_pread_pwrite:
> 	mov 0,7
> 	sc
> 	blr
> and handle the -ve -> errno in C?

Huh? Won't fly, r0 is used for the system call number!

On the other hand, I believed PPC had no problems passing
up to 8 32 bit arguments in registers (r3 to r10), but
I may be confusing with the standard ABI for function calls.

Hmm, a quick look at kernel/entry_32.s shows that it should 
be able to use at least r3 to r8, which should be sufficient.

I think that it is an uClibc problem.

	Gabriel

^ permalink raw reply

* Re: [PATCH] gianfar:don't add FCB length to hard_header_len
From: Paul Gortmaker @ 2012-05-25 15:58 UTC (permalink / raw)
  To: Joe Perches; +Cc: netdev, b06378, linuxppc-dev, David Miller, Jan Ceuleers
In-Reply-To: <1337876210.5070.4.camel@joe2Laptop>

[Re: [PATCH] gianfar:don't add FCB length to hard_header_len] On 24/05/2012 (Thu 09:16) Joe Perches wrote:

> On Thu, 2012-05-24 at 17:04 +0200, Jan Ceuleers wrote:
> > On 05/22/2012 09:18 PM, David Miller wrote:
> > > From: Jiajun Wu <b06378@freescale.com>
> > > Date: Tue, 22 May 2012 17:00:48 +0800
> > > 
> > >> FCB(Frame Control Block) isn't the part of netdev hard header.
> > >> Add FCB to hard_header_len will make GRO fail at MAC comparision stage.
> > >>
> > >> Signed-off-by: Jiajun Wu <b06378@freescale.com>
> > > 
> > > Applied, thanks.
> > > 
> > > Someone needs to go through this driver when net-next opens up
> > > and fix all of the indentation in this driver.
> > 
> > May I give that a go?
> 
> I have scripts that automate most of this.
> I don't have the card though.

There is no card.  The gianfar is a SOC for freescale 83xx, 85xx, 86xx
CPUs.  If need be, I can test just as I did for your name overrun fix
in commit 0015e551e.

But you really shouldn't need the hardware to validate this kind of
patch anyways -- aside from your code flow change in the irq routine of
gianfar_ptp, you should have been simply able to check for object file
equivalence before and after your change.

Paul.

> 
> Maybe this is a starting point?
> It doesn't fix most 80 column warnings.
> 
>  drivers/net/ethernet/freescale/gianfar.c         |  299 +++++++++++-----------
>  drivers/net/ethernet/freescale/gianfar_ethtool.c |  131 +++++-----
>  drivers/net/ethernet/freescale/gianfar_ptp.c     |    8 +-
>  drivers/net/ethernet/freescale/gianfar_sysfs.c   |    2 +-
>  4 files changed, 225 insertions(+), 215 deletions(-)
> 

[...]

> diff --git a/drivers/net/ethernet/freescale/gianfar_ptp.c b/drivers/net/ethernet/freescale/gianfar_ptp.c
> index c08e5d4..3f7b81d 100644
> --- a/drivers/net/ethernet/freescale/gianfar_ptp.c
> +++ b/drivers/net/ethernet/freescale/gianfar_ptp.c
> @@ -268,11 +268,11 @@ static irqreturn_t isr(int irq, void *priv)
>  		ptp_clock_event(etsects->clock, &event);
>  	}
>  
> -	if (ack) {
> -		gfar_write(&etsects->regs->tmr_tevent, ack);
> -		return IRQ_HANDLED;
> -	} else
> +	if (!ack)
>  		return IRQ_NONE;
> +
> +	gfar_write(&etsects->regs->tmr_tevent, ack);
> +	return IRQ_HANDLED;
>  }
>  
>  /*

^ permalink raw reply

* Re: [PATCH] gianfar:don't add FCB length to hard_header_len
From: Joe Perches @ 2012-05-25 19:51 UTC (permalink / raw)
  To: Paul Gortmaker; +Cc: netdev, b06378, linuxppc-dev, David Miller, Jan Ceuleers
In-Reply-To: <20120525155820.GA25102@windriver.com>

On Fri, 2012-05-25 at 11:58 -0400, Paul Gortmaker wrote:
> But you really shouldn't need the hardware to validate this kind of
> patch anyways -- aside from your code flow change in the irq routine of
> gianfar_ptp, you should have been simply able to check for object file
> equivalence before and after your change.

No cross compiler either, and I'm lazy 'bout that...

cheers, Joe

^ permalink raw reply

* Re: [PATCH] gianfar:don't add FCB length to hard_header_len
From: Paul Gortmaker @ 2012-05-25 20:04 UTC (permalink / raw)
  To: Joe Perches; +Cc: netdev, b06378, linuxppc-dev, David Miller, Jan Ceuleers
In-Reply-To: <1337975465.30100.4.camel@joe2Laptop>

On 12-05-25 03:51 PM, Joe Perches wrote:
> On Fri, 2012-05-25 at 11:58 -0400, Paul Gortmaker wrote:
>> But you really shouldn't need the hardware to validate this kind of
>> patch anyways -- aside from your code flow change in the irq routine of
>> gianfar_ptp, you should have been simply able to check for object file
>> equivalence before and after your change.
> 
> No cross compiler either, and I'm lazy 'bout that...

Can't get much easier than using one of these:

http://www.kernel.org/pub/tools/crosstool/

Just untar, export PATH ARCH CROSS_COMPILE and go.

Can't get much lazier than that. Great to have around.

Paul.

> 
> cheers, Joe
> 

^ permalink raw reply

* Re: [git pull] signals, the first series
From: Al Viro @ 2012-05-25 21:48 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: linux-arch, linuxppc-dev, Andrew Morton, Linus Torvalds,
	linux-kernel
In-Reply-To: <20120523151405.f264a88ca4edf8d22c08c86c@canb.auug.org.au>

On Wed, May 23, 2012 at 03:14:05PM +1000, Stephen Rothwell wrote:
> OK, it seem that most of this has been in Andrew's tree for a while,
> sorry about that.

Grr...  *Another* missing prereq for task_work_add() series, this time on
ppc64.  Could somebody familiar with that beast take a look at this and
tell if the change is sane?  What we want is
	r0 = r3 & MSR_PR ?
		_TIF_NEED_RESCHED | _TIF_NOTIFY_RESUME | _TIF_SIGPENDING :
		_TIF_NEED_RESCHED;
and when Roland re-added NOTIFY_RESUME he'd missed that hack (non-PREEMPT
variant and 32bit code all just check _TIF_USER_WORK_MASK, so updating
it had been enough in those cases).  I don't have the hardware in
question; the same instructions in userland on ppc32 box produce the
right value.  Unless NAKed I'm going to throw that one into the second
pull request from signal.git, so if anyone has objections, please yell.

I'll gladly replace that with better solution if one shows up (or, better
yet, goes via ppc tree).  AFAICS the diff below should work, but whether
it's the best variant or not... No idea.

Back to massaging VFS queue for pull...

diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index ed1718f..e4fd9bb 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -563,9 +563,10 @@ _GLOBAL(ret_from_except_lite)
 	li	r0,_TIF_NEED_RESCHED	/* bits to check */
 	ld	r3,_MSR(r1)
 	ld	r4,TI_FLAGS(r9)
-	/* Move MSR_PR bit in r3 to _TIF_SIGPENDING position in r0 */
+	/* Add MSR_PR bit in r3 in _TIF_SIGPENDING and _TIF_NOTIFY_RESUME positions in r0 */
 	rlwimi	r0,r3,32+TIF_SIGPENDING-MSR_PR_LG,_TIF_SIGPENDING
-	and.	r0,r4,r0	/* check NEED_RESCHED and maybe SIGPENDING */
+	rlwimi	r0,r3,32+TIF_NOTIFY_RESUME-MSR_PR_LG,_TIF_NOTIFY_RESUME
+	and.	r0,r4,r0	/* check NEED_RESCHED and maybe SIGPENDING/NOTIFY_RESUME */
 	bne	do_work
 
 #else /* !CONFIG_PREEMPT */

^ permalink raw reply related

* Re: ppc/sata-fsl: orphan config value: CONFIG_MPC8315_DS
From: Anthony Foiani @ 2012-05-26  6:53 UTC (permalink / raw)
  To: Li Yang-R58472
  Cc: Robert P.J.Day, linuxppc-dev@lists.ozlabs.org, ashish kalra,
	Jeff Garzik, Adrian Bunk
In-Reply-To: <94F013E7935FF44C83EBE7784D62AD3F0930B855@039-SN2MPN1-022.039d.mgd.msft.net>

Li Yang-R58472 <r58472@freescale.com> writes:

> Thanks for bringing [CONFIG_MPC8315_DS] up again.  Looks like we do
> have a problem here.

My impression is that the simplest fix is Adrian's patch, which simply
keys off CONFIG_MPC831x_RDB.  It's not very satisfying, but I'll take
"working" vs. "rare lockups at boot".

If there is some other defining characteristic of boards that require
this patch, then a simple KConfig snippit with a description would be
even better.  Without any KConfig support for this variable, I lost it
even after using an oldconfig from my vendor.

(Or, if it was preserved, I might have removed it when trying to
optimize the kernel for support for our hardware, and I had no way of
knowing that the MPC8315_DS had any impact on my system at all...)

If it's actually a CPU/SOC-level problem, then an ideal situation
would conditionalize the fix by examining CPU version or stepping.
That would allow us to get rid of the config variable entirely.

> Btw, did it help with your problem by enabling it?

Possibly.  :)

I only saw the problem (failure to SATA handshake at 3Gbps?) very
rarely, maybe one in 100 warm boot cycles.

I've added the patch to my current project, and have not seen the
problem since then, but until I'm problem free for another few weeks,
I can't sign off on it.

It certainly does look like a reasonable band-aid fix.  In our case,
we don't need anywhere near the higher bandwidth, so it's acceptable
from that point of view.

A clear statement or reference to a CPU / SOC errata would be
preferred, though.  It's a 4-year-old design, so even a brown paper
bag bug isn't all that embarrassing anymore.  :)

Thanks,
Tony

p.s. This board also seems to suffer from occasionaly USB lockups on
     boot; if you end up digging through errata on 8315-based boards,
     please keep an eye out for that as well.  Thanks!  Link:

        http://patchwork.ozlabs.org/patch/152755/

     I'm currently using that patch as well as a 10ms delay to try to
     avoid the hang.  Successfully, so far, but a "blessed" solution
     from FSL would be awesome.

^ permalink raw reply

* [PATCH 1/2] lib: Fix generic strnlen_user for 32-bit big-endian machines
From: Paul Mackerras @ 2012-05-28  2:59 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linuxppc-dev, David Miller

The aligned_byte_mask() definition is wrong for 32-bit big-endian
machines: the "7-(n)" part of the definition assumes a long is 8
bytes.  This fixes it by using BITS_PER_LONG - 8 instead of 8*7.
Tested on 32-bit and 64-bit PowerPC.

Signed-off-by: Paul Mackerras <paulus@samba.org>
---
 lib/strnlen_user.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/strnlen_user.c b/lib/strnlen_user.c
index 90900ec..a28df52 100644
--- a/lib/strnlen_user.c
+++ b/lib/strnlen_user.c
@@ -8,7 +8,7 @@
 #ifdef __LITTLE_ENDIAN
 #  define aligned_byte_mask(n) ((1ul << 8*(n))-1)
 #else
-#  define aligned_byte_mask(n) (~0xfful << 8*(7-(n)))
+#  define aligned_byte_mask(n) (~0xfful << (BITS_PER_LONG - 8 - 8*(n)))
 #endif
 
 /*
-- 
1.7.10.rc3.219.g53414

^ permalink raw reply related

* [PATCH 2/2] powerpc: Use the new generic strncpy_from_user() and strnlen_user()
From: Paul Mackerras @ 2012-05-28  3:03 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linuxppc-dev, David Miller
In-Reply-To: <20120528025956.GA6822@bloggs.ozlabs.ibm.com>

This is much the same as for SPARC except that we can do the find_zero()
function more efficiently using the count-leading-zeroes instructions.
Tested on 32-bit and 64-bit PowerPC.

Signed-off-by: Paul Mackerras <paulus@samba.org>
---
I'm sending this in without Ben H's ack as he is away for a few weeks for
medical reasons.

 arch/powerpc/Kconfig                      |    2 ++
 arch/powerpc/include/asm/uaccess.h        |   41 ++++----------------------
 arch/powerpc/include/asm/word-at-a-time.h |   41 ++++++++++++++++++++++++++
 arch/powerpc/kernel/ppc_ksyms.c           |    2 --
 arch/powerpc/lib/string.S                 |   45 -----------------------------
 5 files changed, 48 insertions(+), 83 deletions(-)
 create mode 100644 arch/powerpc/include/asm/word-at-a-time.h

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 00b9874..050cb37 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -135,6 +135,8 @@ config PPC
 	select GENERIC_CMOS_UPDATE
 	select GENERIC_TIME_VSYSCALL
 	select GENERIC_CLOCKEVENTS
+	select GENERIC_STRNCPY_FROM_USER
+	select GENERIC_STRNLEN_USER
 
 config EARLY_PRINTK
 	bool
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index bd0fb84..17bb40c 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -40,6 +40,8 @@
 
 #define segment_eq(a, b)	((a).seg == (b).seg)
 
+#define user_addr_max()	(get_fs().seg)
+
 #ifdef __powerpc64__
 /*
  * This check is sufficient because there is a large enough
@@ -453,42 +455,9 @@ static inline unsigned long clear_user(void __user *addr, unsigned long size)
 	return size;
 }
 
-extern int __strncpy_from_user(char *dst, const char __user *src, long count);
-
-static inline long strncpy_from_user(char *dst, const char __user *src,
-		long count)
-{
-	might_sleep();
-	if (likely(access_ok(VERIFY_READ, src, 1)))
-		return __strncpy_from_user(dst, src, count);
-	return -EFAULT;
-}
-
-/*
- * Return the size of a string (including the ending 0)
- *
- * Return 0 for error
- */
-extern int __strnlen_user(const char __user *str, long len, unsigned long top);
-
-/*
- * Returns the length of the string at str (including the null byte),
- * or 0 if we hit a page we can't access,
- * or something > len if we didn't find a null byte.
- *
- * The `top' parameter to __strnlen_user is to make sure that
- * we can never overflow from the user area into kernel space.
- */
-static inline int strnlen_user(const char __user *str, long len)
-{
-	unsigned long top = current->thread.fs.seg;
-
-	if ((unsigned long)str > top)
-		return 0;
-	return __strnlen_user(str, len, top);
-}
-
-#define strlen_user(str)	strnlen_user((str), 0x7ffffffe)
+extern long strncpy_from_user(char *dst, const char __user *src, long count);
+extern __must_check long strlen_user(const char __user *str);
+extern __must_check long strnlen_user(const char __user *str, long n);
 
 #endif  /* __ASSEMBLY__ */
 #endif /* __KERNEL__ */
diff --git a/arch/powerpc/include/asm/word-at-a-time.h b/arch/powerpc/include/asm/word-at-a-time.h
new file mode 100644
index 0000000..d0b6d4a
--- /dev/null
+++ b/arch/powerpc/include/asm/word-at-a-time.h
@@ -0,0 +1,41 @@
+#ifndef _ASM_WORD_AT_A_TIME_H
+#define _ASM_WORD_AT_A_TIME_H
+
+/*
+ * Word-at-a-time interfaces for PowerPC.
+ */
+
+#include <linux/kernel.h>
+#include <asm/asm-compat.h>
+
+struct word_at_a_time {
+	const unsigned long high_bits, low_bits;
+};
+
+#define WORD_AT_A_TIME_CONSTANTS { REPEAT_BYTE(0xfe) + 1, REPEAT_BYTE(0x7f) }
+
+/* Bit set in the bytes that have a zero */
+static inline long prep_zero_mask(unsigned long val, unsigned long rhs, const struct word_at_a_time *c)
+{
+	unsigned long mask = (val & c->low_bits) + c->low_bits;
+	return ~(mask | rhs);
+}
+
+#define create_zero_mask(mask) (mask)
+
+static inline long find_zero(unsigned long mask)
+{
+	long leading_zero_bits;
+
+	asm (PPC_CNTLZL "%0,%1" : "=r" (leading_zero_bits) : "r" (mask));
+	return leading_zero_bits >> 3;
+}
+
+static inline bool has_zero(unsigned long val, unsigned long *data, const struct word_at_a_time *c)
+{
+	unsigned long rhs = val | c->low_bits;
+	*data = rhs;
+	return (val + c->high_bits) & ~rhs;
+}
+
+#endif /* _ASM_WORD_AT_A_TIME_H */
diff --git a/arch/powerpc/kernel/ppc_ksyms.c b/arch/powerpc/kernel/ppc_ksyms.c
index d1f2aaf..3e40315 100644
--- a/arch/powerpc/kernel/ppc_ksyms.c
+++ b/arch/powerpc/kernel/ppc_ksyms.c
@@ -85,8 +85,6 @@ EXPORT_SYMBOL(csum_tcpudp_magic);
 
 EXPORT_SYMBOL(__copy_tofrom_user);
 EXPORT_SYMBOL(__clear_user);
-EXPORT_SYMBOL(__strncpy_from_user);
-EXPORT_SYMBOL(__strnlen_user);
 EXPORT_SYMBOL(copy_page);
 
 #if defined(CONFIG_PCI) && defined(CONFIG_PPC32)
diff --git a/arch/powerpc/lib/string.S b/arch/powerpc/lib/string.S
index 455881a..093d631 100644
--- a/arch/powerpc/lib/string.S
+++ b/arch/powerpc/lib/string.S
@@ -160,48 +160,3 @@ _GLOBAL(__clear_user)
 	PPC_LONG	1b,91b
 	PPC_LONG	8b,92b
 	.text
-
-_GLOBAL(__strncpy_from_user)
-	addi	r6,r3,-1
-	addi	r4,r4,-1
-	cmpwi	0,r5,0
-	beq	2f
-	mtctr	r5
-1:	lbzu	r0,1(r4)
-	cmpwi	0,r0,0
-	stbu	r0,1(r6)
-	bdnzf	2,1b		/* dec ctr, branch if ctr != 0 && !cr0.eq */
-	beq	3f
-2:	addi	r6,r6,1
-3:	subf	r3,r3,r6
-	blr
-99:	li	r3,-EFAULT
-	blr
-
-	.section __ex_table,"a"
-	PPC_LONG	1b,99b
-	.text
-
-/* r3 = str, r4 = len (> 0), r5 = top (highest addr) */
-_GLOBAL(__strnlen_user)
-	addi	r7,r3,-1
-	subf	r6,r7,r5	/* top+1 - str */
-	cmplw	0,r4,r6
-	bge	0f
-	mr	r6,r4
-0:	mtctr	r6		/* ctr = min(len, top - str) */
-1:	lbzu	r0,1(r7)	/* get next byte */
-	cmpwi	0,r0,0
-	bdnzf	2,1b		/* loop if --ctr != 0 && byte != 0 */
-	addi	r7,r7,1
-	subf	r3,r3,r7	/* number of bytes we have looked at */
-	beqlr			/* return if we found a 0 byte */
-	cmpw	0,r3,r4		/* did we look at all len bytes? */
-	blt	99f		/* if not, must have hit top */
-	addi	r3,r4,1		/* return len + 1 to indicate no null found */
-	blr
-99:	li	r3,0		/* bad address, return 0 */
-	blr
-
-	.section __ex_table,"a"
-	PPC_LONG	1b,99b
-- 
1.7.10.rc3.219.g53414

^ permalink raw reply related

* Re: [PATCH 1/2] lib: Fix generic strnlen_user for 32-bit big-endian machines
From: David Miller @ 2012-05-28  3:55 UTC (permalink / raw)
  To: paulus; +Cc: linuxppc-dev, torvalds
In-Reply-To: <20120528025956.GA6822@bloggs.ozlabs.ibm.com>

From: Paul Mackerras <paulus@samba.org>
Date: Mon, 28 May 2012 12:59:56 +1000

> The aligned_byte_mask() definition is wrong for 32-bit big-endian
> machines: the "7-(n)" part of the definition assumes a long is 8
> bytes.  This fixes it by using BITS_PER_LONG - 8 instead of 8*7.
> Tested on 32-bit and 64-bit PowerPC.
> 
> Signed-off-by: Paul Mackerras <paulus@samba.org>

Acked-by: David S. Miller <davem@davemloft.net>

^ permalink raw reply

* Re: [PATCH 2/2] powerpc: Use the new generic strncpy_from_user() and strnlen_user()
From: David Miller @ 2012-05-28  3:56 UTC (permalink / raw)
  To: paulus; +Cc: linuxppc-dev, torvalds
In-Reply-To: <20120528030347.GB6822@bloggs.ozlabs.ibm.com>

From: Paul Mackerras <paulus@samba.org>
Date: Mon, 28 May 2012 13:03:47 +1000

> This is much the same as for SPARC except that we can do the find_zero()
> function more efficiently using the count-leading-zeroes instructions.
> Tested on 32-bit and 64-bit PowerPC.
> 
> Signed-off-by: Paul Mackerras <paulus@samba.org>

Looks great.

Acked-by: David S. Miller <davem@davemloft.net>

I might hack sparc64 to use 'lzd' or 'popc' on newer chips via code
patching at some point.

^ permalink raw reply

* [PATCH] powerpc: 64bit optimised __clear_user
From: Anton Blanchard @ 2012-05-28  5:54 UTC (permalink / raw)
  To: benh, paulus, michael, linuxppc-dev


I noticed __clear_user high up in a profile of one of my RAID stress
tests. The testcase was doing a dd from /dev/zero which ends up
calling __clear_user.

__clear_user is basically a loop with a single 4 byte store which
is horribly slow. We can do much better by aligning the desination
and doing 32 bytes of 8 byte stores in a loop.

The following testcase was used to verify the patch:

http://ozlabs.org/~anton/junkcode/stress_clear_user.c

To show the improvement in performance I ran a dd from /dev/zero
to /dev/null on a POWER7 box:

Before:

# dd if=/dev/zero of=/dev/null bs=1M count=10000
10485760000 bytes (10 GB) copied, 3.72379 s, 2.8 GB/s

After:

# time dd if=/dev/zero of=/dev/null bs=1M count=10000
10485760000 bytes (10 GB) copied, 0.728318 s, 14.4 GB/s

Over 5x faster.

Signed-off-by: Anton Blanchard <anton@samba.org>
---

Interestingly, it picked up an issue with the old clear_user which
fails when we are less than 4 bytes to the end of a page and the
next page is unmapped:

offset 4094 length 526 expected 2 got -1
expected 0x00 at offset 4094, got 0xff
expected 0x00 at offset 4095, got 0xff

We should fix that.

Index: linux-build/arch/powerpc/lib/Makefile
===================================================================
--- linux-build.orig/arch/powerpc/lib/Makefile	2012-05-28 10:59:09.281806751 +1000
+++ linux-build/arch/powerpc/lib/Makefile	2012-05-28 11:02:35.017452778 +1000
@@ -17,7 +17,7 @@ obj-$(CONFIG_HAS_IOMEM)	+= devres.o
 obj-$(CONFIG_PPC64)	+= copypage_64.o copyuser_64.o \
 			   memcpy_64.o usercopy_64.o mem_64.o string.o \
 			   checksum_wrappers_64.o hweight_64.o \
-			   copyuser_power7.o
+			   copyuser_power7.o string_64.o
 obj-$(CONFIG_XMON)	+= sstep.o ldstfp.o
 obj-$(CONFIG_KPROBES)	+= sstep.o ldstfp.o
 obj-$(CONFIG_HAVE_HW_BREAKPOINT)	+= sstep.o ldstfp.o
Index: linux-build/arch/powerpc/lib/string_64.S
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-build/arch/powerpc/lib/string_64.S	2012-05-28 14:56:03.937833406 +1000
@@ -0,0 +1,141 @@
+/*
+ * 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.
+ *
+ * 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 for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ * Copyright (C) IBM Corporation, 2012
+ *
+ * Author: Anton Blanchard <anton@au.ibm.com>
+ */
+
+#include <asm/ppc_asm.h>
+
+/**
+ * __clear_user: - Zero a block of memory in user space, with less checking.
+ * @to:   Destination address, in user space.
+ * @n:    Number of bytes to zero.
+ *
+ * Zero a block of memory in user space.  Caller must check
+ * the specified block with access_ok() before calling this function.
+ *
+ * Returns number of bytes that could not be cleared.
+ * On success, this will be zero.
+ */
+
+	.macro err1
+100:
+	.section __ex_table,"a"
+	.align 3
+	.llong 100b,.Ldo_err1
+	.previous
+	.endm
+
+	.macro err2
+200:
+	.section __ex_table,"a"
+	.align 3
+	.llong 200b,.Ldo_err2
+	.previous
+	.endm
+
+	.macro err3
+300:
+	.section __ex_table,"a"
+	.align 3
+	.llong 300b,.Ldo_err3
+	.previous
+	.endm
+
+.Ldo_err1:
+	mr	r3,r8
+
+.Ldo_err2:
+	mtctr	r4
+1:
+err3;	stb	r0,0(r3)
+	addi	r3,r3,1
+	addi	r4,r4,-1
+	bdnz	1b
+
+.Ldo_err3:
+	mr	r3,r4
+	blr
+
+_GLOBAL(__clear_user)
+	cmpdi	r4,32
+	neg	r6,r3
+	li	r0,0
+	blt	.Lshort_clear
+	mr	r8,r3
+	mtocrf	0x01,r6
+	clrldi	r6,r6,(64-3)
+
+	/* Get the destination 8 byte aligned */
+	bf	cr7*4+3,1f
+err1;	stb	r0,0(r3)
+	addi	r3,r3,1
+
+1:	bf	cr7*4+2,2f
+err1;	sth	r0,0(r3)
+	addi	r3,r3,2
+
+2:	bf	cr7*4+1,3f
+err1;	stw	r0,0(r3)
+	addi	r3,r3,4
+
+3:	sub	r4,r4,r6
+	srdi	r6,r4,5
+	cmpdi	r4,32
+	blt	.Lshort_clear
+	mtctr	r6
+
+	/* Do 32 byte chunks */
+4:
+err2;	std	r0,0(r3)
+err2;	std	r0,8(r3)
+err2;	std	r0,16(r3)
+err2;	std	r0,24(r3)
+	addi	r3,r3,32
+	addi	r4,r4,-32
+	bdnz	4b
+
+.Lshort_clear:
+	/* up to 31 bytes to go */
+	cmpdi	r4,16
+	blt	6f
+err2;	std	r0,0(r3)
+err2;	std	r0,8(r3)
+	addi	r3,r3,16
+	addi	r4,r4,-16
+
+	/* Up to 15 bytes to go */
+6:	mr	r8,r3
+	clrldi	r4,r4,(64-4)
+	mtocrf	0x01,r4
+	bf	cr7*4+0,7f
+err1;	std	r0,0(r3)
+	addi	r3,r3,8
+
+7:	bf	cr7*4+1,8f
+err1;	stw	r0,0(r3)
+	addi	r3,r3,4
+
+8:	bf	cr7*4+2,9f
+err1;	sth	r0,0(r3)
+	addi	r3,r3,2
+
+9:	bf	cr7*4+3,10f
+err1;	stb	r0,0(r3)
+
+10:	li	r3,0
+	blr
Index: linux-build/arch/powerpc/lib/string.S
===================================================================
--- linux-build.orig/arch/powerpc/lib/string.S	2011-09-07 15:15:49.146459439 +1000
+++ linux-build/arch/powerpc/lib/string.S	2012-05-28 11:01:28.728249934 +1000
@@ -119,6 +119,7 @@ _GLOBAL(memchr)
 2:	li	r3,0
 	blr
 
+#ifdef CONFIG_PPC32
 _GLOBAL(__clear_user)
 	addi	r6,r3,-4
 	li	r3,0
@@ -160,6 +161,7 @@ _GLOBAL(__clear_user)
 	PPC_LONG	1b,91b
 	PPC_LONG	8b,92b
 	.text
+#endif
 
 _GLOBAL(__strncpy_from_user)
 	addi	r6,r3,-1

^ permalink raw reply

* [RFC PATCH 09/10] POWERPC: smp: remove call to ipi_call_lock()/ipi_call_unlock()
From: Yong Zhang @ 2012-05-29  7:16 UTC (permalink / raw)
  To: linux-kernel
  Cc: axboe, sshtylyov, nikunj, david.daney, peterz, akpm, ralf,
	Paul Mackerras, srivatsa.bhat, tglx, paulmck, linuxppc-dev, mingo
In-Reply-To: <1338275765-3217-1-git-send-email-yong.zhang0@gmail.com>

From: Yong Zhang <yong.zhang@windriver.com>

1) call_function.lock used in smp_call_function_many() is just to protect
   call_function.queue and &data->refs, cpu_online_mask is outside of the
   lock. And it's not necessary to protect cpu_online_mask,
   because data->cpumask is pre-calculate and even if a cpu is brougt up
   when calling arch_send_call_function_ipi_mask(), it's harmless because
   validation test in generic_smp_call_function_interrupt() will take care
   of it.

2) For cpu down issue, stop_machine() will guarantee that no concurrent
   smp_call_fuction() is processing.

Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@lists.ozlabs.org
---
 arch/powerpc/kernel/smp.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index e4cb343..e1417c4 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -571,7 +571,6 @@ void __devinit start_secondary(void *unused)
 	if (system_state == SYSTEM_RUNNING)
 		vdso_data->processorCount++;
 #endif
-	ipi_call_lock();
 	notify_cpu_starting(cpu);
 	set_cpu_online(cpu, true);
 	/* Update sibling maps */
@@ -601,7 +600,6 @@ void __devinit start_secondary(void *unused)
 		of_node_put(np);
 	}
 	of_node_put(l2_cache);
-	ipi_call_unlock();
 
 	local_irq_enable();
 
-- 
1.7.5.4

^ permalink raw reply related


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