[Fwd: [Bug 7431] New: ohci1394 Oops after a rmmod/modprobe cycle]

[Fwd: [Bug 7431] New: ohci1394 Oops after a rmmod/modprobe cycle]

[Stefan Richter]

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

Hi list,
there was a bug report on ohci1394 which seems to be related to the
platform-specific code in the driver. Additional details were posted on
linux1394-user:
http://thread.gmane.org/gmane.linux.kernel.firewire.user/focus=2120
Subject: "strange interaction: ohci1394 and backlight on iBook"
Any ideas? Thanks,
-- 
Stefan Richter
-=====-=-==- =-== --=-=
http://arcgraph.de/sr/

[-- Attachment #2: [Bug 7431] New: ohci1394 Oops after a rmmod/modprobe cycle --]
[-- Type: message/rfc822, Size: 4665 bytes --]

From: bugme-daemon@bugzilla.kernel.org
To: stefan-r-bz@s5r6.in-berlin.de
Subject: [Bug 7431] New: ohci1394 Oops after a rmmod/modprobe cycle
Date: Sun, 29 Oct 2006 17:24:53 -0800
Message-ID: <200610300124.k9U1Orae030173@fire-2.osdl.org>

http://bugzilla.kernel.org/show_bug.cgi?id=7431

           Summary: ohci1394 Oops after a rmmod/modprobe cycle
    Kernel Version: 2.6.18 (gentoo-sources-2.6.18)
            Status: NEW
          Severity: normal
             Owner: drivers_ieee1394@kernel-bugs.osdl.org
         Submitter: dev@gioelebarabucci.com


Distribution: Gentoo
Hardware Environment: PPC iBook G3 (PowerBook4,3)
Problem Description:

When I 'rmmod ohci1394' and then 'modprobe ohci1394', I get a "Bus error" 
message from modprobe and my backlight is set to full brightness.
dmesg reveals that an oops occurred. Here is the dmesg output after 'modprobe 
ohci1394':

| Machine check in kernel mode.
| Caused by (from SRR1=49030): Transfer error ack signal
| Oops: Machine check, sig: 7 [#1]
|
| Modules linked in: ohci1394 ipv6 af_packet radeon drm snd_seq snd_seq_device 
snd_powermac ide_cd cdrom snd_aoa_i2sbus snd_pcm airport snd_timer 
snd_page_alloc snd orinoco soundcore hermes snd_aoa_soundbus sungem ieee1394 
ohci_hcd usbcore uninorth_agp sungem_phy agpgart evdev unix
| NIP: EA08D29C LR: EA091618 CTR: C002CCB0
| REGS: cd533c80 TRAP: 0200   Not tainted  (2.6.18-gentoo)
| MSR: 00049030 <EE,ME,IR,DR>  CR: 24004482  XER: 00000000
| TASK = cd947850[8127] 'modprobe' THREAD: cd532000
| GPR00: FFFFFFFF CD533D30 CD947850 CB2B9E68 EA08E094 CB2B9F44 CC1BD7FC 
00000000
| GPR08: 0000001F EA084000 000007C0 EA084000 84004488 1001F288 0000001D 
EA08B13C
| GPR16: E7454AE0 EA0886D4 00000124 00000000 C0045298 EA088148 EA088724 
EA090000
| GPR24: CB2B8000 CB2B9EE8 CB2B9FA0 CB2B9E8C CB2B9F44 C79BA000 CB2B9E68 
00000000
| NIP [EA08D29C] ohci_soft_reset+0x50/0x78 [ohci1394]
| LR [EA091618] ohci1394_pci_probe+0x2f4/0xb90 [ohci1394]
| Call Trace:
| [CD533D30] [C79BA000] 0xc79ba000 (unreliable)
| [CD533D40] [EA091618] ohci1394_pci_probe+0x2f4/0xb90 [ohci1394]
| [CD533DA0] [C00F755C] pci_device_probe+0x84/0xa4
| [CD533DC0] [C014EE88] driver_probe_device+0x60/0x118
| [CD533DE0] [C014F0C0] __driver_attach+0xcc/0xf8
| [CD533E00] [C014E764] bus_for_each_dev+0x58/0x94
| [CD533E30] [C014ED9C] driver_attach+0x24/0x34
| [CD533E40] [C014E2CC] bus_add_driver+0x88/0x164
| [CD533E60] [C014F36C] driver_register+0x70/0xb8
| [CD533E70] [C00F7378] __pci_register_driver+0x4c/0x8c
| [CD533E80] [E900D020] ohci1394_init+0x20/0x60 [ohci1394]
| [CD533E90] [C0046214] sys_init_module+0x170/0x1544
| [CD533F40] [C000F2FC] ret_from_syscall+0x0/0x38
| --- Exception: c01 at 0xff3caac
|    LR = 0x10002e84
| Instruction dump:
| 7c0004ac 7d20052c 3be00000 48000014 4800570d 2f9f0063 3bff0001 419e0028
| 813e0008 38090050 7c0004ac 7c00042c <0c000000> 4c00012c 74090001 386003e8

Steps to reproduce:
* rmmod ohci1394
* modprobe ohci1394

In addition, it is impossible now to remove the ohci1394 module:
| # rmmod ohci1394
| ERROR: Module ohci1394 is in use

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

Re: [Fwd: [Bug 7431] New: ohci1394 Oops after a rmmod/modprobe cycle]

[Benjamin Herrenschmidt]

On Sun, 2006-11-05 at 11:41 +0100, Stefan Richter wrote:
> Hi list,
> there was a bug report on ohci1394 which seems to be related to the
> platform-specific code in the driver. Additional details were posted on
> linux1394-user:
> http://thread.gmane.org/gmane.linux.kernel.firewire.user/focus=2120
> Subject: "strange interaction: ohci1394 and backlight on iBook"
> Any ideas? Thanks,

The machine check means basically that the chip didn't respond on the
PCI bus. The most probable cause is that something is wrong with the
platform code that switches the chip clock on/off or with the PCI D
state change.

One thing you can check is wether that's always called properly,
especially when starting the chip. Another possibly might be that the
chip needs some time after the clocks are restored to be back online,
thus you might need a delay after the platform code and/or the PCI D
state change before you start poking at registers.

A couple of thing to make sure of:

 - On init, call platform code first to bring clocks back up, then only
do the PCI D state transition to D0 (maybe with a delay)

 - On rmmod or suspend, call the platform code last, after the D3 state
transition (if any), and make sure the chip's been properly stopped
first.

It might also be useful if there isn't some sort of bad interaction with
sungem which on the same PCI bus and has similar clock control as I've
heard about possible issues on some older chips. Thus, the user could
verify that sungem is allways up and running (link on) during the test
and check if that makes any difference.

Cheers,
Ben.

Re: [Fwd: [Bug 7431] New: ohci1394 Oops after a rmmod/modprobe cycle]

[Stefan Richter]

Benjamin Herrenschmidt wrote:
> The machine check means basically that the chip didn't respond on the
> PCI bus. The most probable cause is that something is wrong with the
> platform code that switches the chip clock on/off or with the PCI D
> state change.
> 
> One thing you can check is wether that's always called properly,
> especially when starting the chip.

Actually, we have platform code in ohci1394's pci_driver.suspend(),
.resume(), and .remove(), but not in .probe().

.suspend() has:
	pmac_call_feature(PMAC_FTR_1394_ENABLE, of_node, 0, 0);
.resume() has:
	pmac_call_feature(PMAC_FTR_1394_ENABLE, of_node, 0, 1);
.remove() has:
	pmac_call_feature(PMAC_FTR_1394_ENABLE, of_node, 0, 0);
	pmac_call_feature(PMAC_FTR_1394_CABLE_POWER, of_node, 0, 0);

How about this patch:

--- linux.orig/drivers/ieee1394/ohci1394.c	2006-11-02 22:35:16.000000000 +0100
+++ linux/drivers/ieee1394/ohci1394.c	2006-11-05 12:19:52.000000000 +0100
@@ -3215,6 +3215,18 @@ static int __devinit ohci1394_pci_probe(
 	struct ti_ohci *ohci;	/* shortcut to currently handled device */
 	resource_size_t ohci_base;

+#ifdef CONFIG_PPC_PMAC
+	/* Necessary if ohci1394 was loaded and unloaded before */
+	if (machine_is(powermac)) {
+		struct device_node *of_node = pci_device_to_OF_node(dev);
+
+		if (of_node)
+			pmac_call_feature(PMAC_FTR_1394_CABLE_POWER, of_node,
+					  0, 1);
+			pmac_call_feature(PMAC_FTR_1394_ENABLE, of_node, 0, 1);
+	}
+#endif /* CONFIG_PPC_PMAC */
+
         if (pci_enable_device(dev))
 		FAIL(-ENXIO, "Failed to enable OHCI hardware");
         pci_set_master(dev);


(Diff is against linux1394-2.6.git or the patchsets at
http://me.in-berlin.de/~s5r6/linux1394/updates/. But perhaps it applies
to stock ohci1394 of recent kernels too.)

> Another possibly might be that the
> chip needs some time after the clocks are restored to be back online,
> thus you might need a delay after the platform code and/or the PCI D
> state change before you start poking at registers.
> 
> A couple of thing to make sure of:
> 
>  - On init, call platform code first to bring clocks back up, then only
> do the PCI D state transition to D0 (maybe with a delay)
> 
>  - On rmmod or suspend, call the platform code last, after the D3 state
> transition (if any), and make sure the chip's been properly stopped
> first.
> 
> It might also be useful if there isn't some sort of bad interaction with
> sungem which on the same PCI bus and has similar clock control as I've
> heard about possible issues on some older chips. Thus, the user could
> verify that sungem is allways up and running (link on) during the test
> and check if that makes any difference.

Yes, PowerBook G3 Pismo seems affected.
https://bugzilla.novell.com/show_bug.cgi?id=115228

Thanks for the help.
-- 
Stefan Richter
-=====-=-==- =-== --=-=
http://arcgraph.de/sr/

Re: [Fwd: [Bug 7431] New: ohci1394 Oops after a rmmod/modprobe cycle]

[Stefan Richter]

I wrote:
> Benjamin Herrenschmidt wrote:
>> The machine check means basically that the chip didn't respond on the
>> PCI bus. The most probable cause is that something is wrong with the
>> platform code that switches the chip clock on/off or with the PCI D
>> state change.
>>
>> One thing you can check is wether that's always called properly,
>> especially when starting the chip.
[...]
> How about this patch:
> 
> --- linux.orig/drivers/ieee1394/ohci1394.c	2006-11-02 22:35:16.000000000 +0100
> +++ linux/drivers/ieee1394/ohci1394.c	2006-11-05 12:19:52.000000000 +0100
> @@ -3215,6 +3215,18 @@ static int __devinit ohci1394_pci_probe(
>  	struct ti_ohci *ohci;	/* shortcut to currently handled device */
>  	resource_size_t ohci_base;
> 
> +#ifdef CONFIG_PPC_PMAC
> +	/* Necessary if ohci1394 was loaded and unloaded before */
> +	if (machine_is(powermac)) {
> +		struct device_node *of_node = pci_device_to_OF_node(dev);
> +
> +		if (of_node)
> +			pmac_call_feature(PMAC_FTR_1394_CABLE_POWER, of_node,
> +					  0, 1);
> +			pmac_call_feature(PMAC_FTR_1394_ENABLE, of_node, 0, 1);
> +	}
> +#endif /* CONFIG_PPC_PMAC */
> +
>          if (pci_enable_device(dev))
>  		FAIL(-ENXIO, "Failed to enable OHCI hardware");
>          pci_set_master(dev);
[...]

Hm. There is the following comment in ohci1394_pci_remove():

	/* On UniNorth, power down the cable and turn off the chip
	 * clock when the module is removed to save power on
	 * laptops. Turning it back ON is done by the arch code when
	 * pci_enable_device() is called */

Is this true or wrong in current kernels?
-- 
Stefan Richter
-=====-=-==- =-== --=-=
http://arcgraph.de/sr/

Re: [Fwd: [Bug 7431] New: ohci1394 Oops after a rmmod/modprobe cycle]

[Benjamin Herrenschmidt]

> Hm. There is the following comment in ohci1394_pci_remove():
> 
> 	/* On UniNorth, power down the cable and turn off the chip
> 	 * clock when the module is removed to save power on
> 	 * laptops. Turning it back ON is done by the arch code when
> 	 * pci_enable_device() is called */
> 
> Is this true or wrong in current kernels?

Might have bitrotted.... I'll have a look.

Ben.

Re: [Fwd: [Bug 7431] New: ohci1394 Oops after a rmmod/modprobe cycle]

[Olaf Hering]

On Sun, Nov 05, Stefan Richter wrote:

> Yes, PowerBook G3 Pismo seems affected.
> https://bugzilla.novell.com/show_bug.cgi?id=115228

This patch does not help. Doing an ip link set dev eth0 up makes no
difference.

Re: [Fwd: [Bug 7431] New: ohci1394 Oops after a rmmod/modprobe cycle]

[Benjamin Herrenschmidt]

On Mon, 2006-11-06 at 17:58 +0100, Olaf Hering wrote:
> On Sun, Nov 05, Stefan Richter wrote:
> 
> > Yes, PowerBook G3 Pismo seems affected.
> > https://bugzilla.novell.com/show_bug.cgi?id=115228
> 
> This patch does not help. Doing an ip link set dev eth0 up makes no
> difference.

What about trying to add a delay between the pmac clock code and the
init of the controller ? does that help ?

(You need to have a cable plug and the interface up for the power
management bits not to get in the way btw)

Ben.

Re: [Fwd: [Bug 7431] New: ohci1394 Oops after a rmmod/modprobe cycle]

[Olaf Hering]

On Tue, Nov 07, Benjamin Herrenschmidt wrote:

> On Mon, 2006-11-06 at 17:58 +0100, Olaf Hering wrote:
> > On Sun, Nov 05, Stefan Richter wrote:
> > 
> > > Yes, PowerBook G3 Pismo seems affected.
> > > https://bugzilla.novell.com/show_bug.cgi?id=115228
> > 
> > This patch does not help. Doing an ip link set dev eth0 up makes no
> > difference.
> 
> What about trying to add a delay between the pmac clock code and the
> init of the controller ? does that help ?

no. and it is the rmmod, not the insmod, that hangs the pismo.
Adding some mdelay to the remove function doesnt help either.

Re: [Fwd: [Bug 7431] New: ohci1394 Oops after a rmmod/modprobe cycle]

[Benjamin Herrenschmidt]

On Tue, 2006-11-07 at 08:08 +0100, Olaf Hering wrote:
> On Tue, Nov 07, Benjamin Herrenschmidt wrote:
> 
> > On Mon, 2006-11-06 at 17:58 +0100, Olaf Hering wrote:
> > > On Sun, Nov 05, Stefan Richter wrote:
> > > 
> > > > Yes, PowerBook G3 Pismo seems affected.
> > > > https://bugzilla.novell.com/show_bug.cgi?id=115228
> > > 
> > > This patch does not help. Doing an ip link set dev eth0 up makes no
> > > difference.
> > 
> > What about trying to add a delay between the pmac clock code and the
> > init of the controller ? does that help ?
> 
> no. and it is the rmmod, not the insmod, that hangs the pismo.
> Adding some mdelay to the remove function doesnt help either.

Tried not calling the pmac feature stuff at all in the rmmod ?

I reckon there might be a problem with the cable power control on some
old models like... the pismo :-)

Ben.

Re: [Fwd: [Bug 7431] New: ohci1394 Oops after a rmmod/modprobe cycle]

[Stefan Richter]

On  7 Nov, Benjamin Herrenschmidt wrote:
> On Tue, 2006-11-07 at 08:08 +0100, Olaf Hering wrote:
[...]
>> and it is the rmmod, not the insmod, that hangs the pismo.
>> Adding some mdelay to the remove function doesnt help either.
> 
> Tried not calling the pmac feature stuff at all in the rmmod ?

Olaf?

> I reckon there might be a problem with the cable power control on some
> old models like... the pismo :-)

Is there a way to detect affected machines or at least to blacklist
known broken machines?
-- 
Stefan Richter
-=====-=-==- =-== -=-=-
http://arcgraph.de/sr/

[PATCH 1/2] ieee1394: ohci1394: add PPC_PMAC platform code to driver probe

[Stefan Richter]

Fixes http://bugzilla.kernel.org/show_bug.cgi?id=7431 :
iBook G3 threw a machine check exception and put the display backlight
to full brightness after ohci1394 was unloaded and reloaded.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/ieee1394/ohci1394.c |   18 ++++++++++++++----
 1 files changed, 14 insertions(+), 4 deletions(-)

Index: linux/drivers/ieee1394/ohci1394.c
===================================================================
--- linux.orig/drivers/ieee1394/ohci1394.c	2006-11-08 18:19:56.000000000 +0100
+++ linux/drivers/ieee1394/ohci1394.c	2006-11-10 23:25:04.000000000 +0100
@@ -3215,6 +3215,18 @@ static int __devinit ohci1394_pci_probe(
 	struct ti_ohci *ohci;	/* shortcut to currently handled device */
 	resource_size_t ohci_base;
 
+#ifdef CONFIG_PPC_PMAC
+	/* Necessary on some machines if ohci1394 was loaded/ unloaded before */
+	if (machine_is(powermac)) {
+		struct device_node *of_node = pci_device_to_OF_node(dev);
+
+		if (of_node)
+			pmac_call_feature(PMAC_FTR_1394_CABLE_POWER, of_node,
+					  0, 1);
+			pmac_call_feature(PMAC_FTR_1394_ENABLE, of_node, 0, 1);
+	}
+#endif /* CONFIG_PPC_PMAC */
+
         if (pci_enable_device(dev))
 		FAIL(-ENXIO, "Failed to enable OHCI hardware");
         pci_set_master(dev);
@@ -3503,10 +3515,8 @@ static void ohci1394_pci_remove(struct p
 #endif
 
 #ifdef CONFIG_PPC_PMAC
-	/* On UniNorth, power down the cable and turn off the chip
-	 * clock when the module is removed to save power on
-	 * laptops. Turning it back ON is done by the arch code when
-	 * pci_enable_device() is called */
+	/* On UniNorth, power down the cable and turn off the chip clock
+	 * to save power on laptops */
 	{
 		struct device_node* of_node;
 

[PATCH 2/2] ieee1394: ohci1394: reformat PPC_PMAC platform code

[Stefan Richter]

Adjust whitespace and line lengths

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/ieee1394/ohci1394.c |   34 ++++++++++++++--------------------
 1 files changed, 14 insertions(+), 20 deletions(-)

Index: linux/drivers/ieee1394/ohci1394.c
===================================================================
--- linux.orig/drivers/ieee1394/ohci1394.c	2006-11-10 23:25:04.000000000 +0100
+++ linux/drivers/ieee1394/ohci1394.c	2006-11-10 23:26:12.000000000 +0100
@@ -3218,12 +3218,11 @@ static int __devinit ohci1394_pci_probe(
 #ifdef CONFIG_PPC_PMAC
 	/* Necessary on some machines if ohci1394 was loaded/ unloaded before */
 	if (machine_is(powermac)) {
-		struct device_node *of_node = pci_device_to_OF_node(dev);
+		struct device_node *ofn = pci_device_to_OF_node(dev);
 
-		if (of_node)
-			pmac_call_feature(PMAC_FTR_1394_CABLE_POWER, of_node,
-					  0, 1);
-			pmac_call_feature(PMAC_FTR_1394_ENABLE, of_node, 0, 1);
+		if (ofn)
+			pmac_call_feature(PMAC_FTR_1394_CABLE_POWER, ofn, 0, 1);
+			pmac_call_feature(PMAC_FTR_1394_ENABLE, ofn, 0, 1);
 	}
 #endif /* CONFIG_PPC_PMAC */
 
@@ -3518,12 +3517,11 @@ static void ohci1394_pci_remove(struct p
 	/* On UniNorth, power down the cable and turn off the chip clock
 	 * to save power on laptops */
 	{
-		struct device_node* of_node;
+		struct device_node* ofn = pci_device_to_OF_node(ohci->dev);
 
-		of_node = pci_device_to_OF_node(ohci->dev);
-		if (of_node) {
-			pmac_call_feature(PMAC_FTR_1394_ENABLE, of_node, 0, 0);
-			pmac_call_feature(PMAC_FTR_1394_CABLE_POWER, of_node, 0, 0);
+		if (ofn) {
+			pmac_call_feature(PMAC_FTR_1394_ENABLE, ofn, 0, 0);
+			pmac_call_feature(PMAC_FTR_1394_CABLE_POWER, ofn, 0, 0);
 		}
 	}
 #endif /* CONFIG_PPC_PMAC */
@@ -3583,12 +3581,10 @@ static int ohci1394_pci_suspend(struct p
 /* PowerMac suspend code comes last */
 #ifdef CONFIG_PPC_PMAC
 	if (machine_is(powermac)) {
-		struct device_node *of_node;
+		struct device_node *ofn = pci_device_to_OF_node(pdev);
 
-		/* Disable 1394 */
-		of_node = pci_device_to_OF_node (pdev);
-		if (of_node)
-			pmac_call_feature(PMAC_FTR_1394_ENABLE, of_node, 0, 0);
+		if (ofn)
+			pmac_call_feature(PMAC_FTR_1394_ENABLE, ofn, 0, 0);
 	}
 #endif /* CONFIG_PPC_PMAC */
 
@@ -3610,12 +3606,10 @@ static int ohci1394_pci_resume(struct pc
 /* PowerMac resume code comes first */
 #ifdef CONFIG_PPC_PMAC
 	if (machine_is(powermac)) {
-		struct device_node *of_node;
+		struct device_node *ofn = pci_device_to_OF_node(pdev);
 
-		/* Re-enable 1394 */
-		of_node = pci_device_to_OF_node (pdev);
-		if (of_node)
-			pmac_call_feature (PMAC_FTR_1394_ENABLE, of_node, 0, 1);
+		if (ofn)
+			pmac_call_feature(PMAC_FTR_1394_ENABLE, ofn, 0, 1);
 	}
 #endif /* CONFIG_PPC_PMAC */
 

Re: [Fwd: [Bug 7431] New: ohci1394 Oops after a rmmod/modprobe cycle]

[Stefan Richter]

I wrote:
> On  7 Nov, Benjamin Herrenschmidt wrote:
>> On Tue, 2006-11-07 at 08:08 +0100, Olaf Hering wrote:
> [...]
>>> and it is the rmmod, not the insmod, that hangs the pismo.
>>> Adding some mdelay to the remove function doesnt help either.
>> 
>> Tried not calling the pmac feature stuff at all in the rmmod ?
> 
> Olaf?
> 
>> I reckon there might be a problem with the cable power control on some
>> old models like... the pismo :-)
> 
> Is there a way to detect affected machines or at least to blacklist
> known broken machines?

Actually, shouldn't ohci1394_pci_remove enlcose the platform code into

	if (machine_is(powermac)) {
		...
	}

like ohci1394_pci_suspend and ohci1394_pci_resume do?
-- 
Stefan Richter
-=====-=-==- =-== -=-==
http://arcgraph.de/sr/

Re: [Fwd: [Bug 7431] New: ohci1394 Oops after a rmmod/modprobe cycle]

[Benjamin Herrenschmidt]

On Sat, 2006-11-11 at 00:07 +0100, Stefan Richter wrote:
> I wrote:
> > On  7 Nov, Benjamin Herrenschmidt wrote:
> >> On Tue, 2006-11-07 at 08:08 +0100, Olaf Hering wrote:
> > [...]
> >>> and it is the rmmod, not the insmod, that hangs the pismo.
> >>> Adding some mdelay to the remove function doesnt help either.
> >> 
> >> Tried not calling the pmac feature stuff at all in the rmmod ?
> > 
> > Olaf?
> > 
> >> I reckon there might be a problem with the cable power control on some
> >> old models like... the pismo :-)
> > 
> > Is there a way to detect affected machines or at least to blacklist
> > known broken machines?
> 
> Actually, shouldn't ohci1394_pci_remove enlcose the platform code into
> 
> 	if (machine_is(powermac)) {
> 		...
> 	}
> 
> like ohci1394_pci_suspend and ohci1394_pci_resume do?

Yes.

Ben.

[PATCH update 1/3] ieee1394: ohci1394: add PPC_PMAC platform code to driver probe

[Stefan Richter]

Fixes http://bugzilla.kernel.org/show_bug.cgi?id=7431
iBook G3 threw a machine check exception and put the display backlight
to full brightness after ohci1394 was unloaded and reloaded.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
Patch update:  There were braces missing.  Thanks to Jens Maurer.

 drivers/ieee1394/ohci1394.c |   19 +++++++++++++++----
 1 files changed, 15 insertions(+), 4 deletions(-)

Index: linux/drivers/ieee1394/ohci1394.c
===================================================================
--- linux.orig/drivers/ieee1394/ohci1394.c	2006-11-08 18:19:56.000000000 +0100
+++ linux/drivers/ieee1394/ohci1394.c	2006-11-11 00:12:31.000000000 +0100
@@ -3215,6 +3215,19 @@ static int __devinit ohci1394_pci_probe(
 	struct ti_ohci *ohci;	/* shortcut to currently handled device */
 	resource_size_t ohci_base;
 
+#ifdef CONFIG_PPC_PMAC
+	/* Necessary on some machines if ohci1394 was loaded/ unloaded before */
+	if (machine_is(powermac)) {
+		struct device_node *of_node = pci_device_to_OF_node(dev);
+
+		if (of_node) {
+			pmac_call_feature(PMAC_FTR_1394_CABLE_POWER, of_node,
+					  0, 1);
+			pmac_call_feature(PMAC_FTR_1394_ENABLE, of_node, 0, 1);
+		}
+	}
+#endif /* CONFIG_PPC_PMAC */
+
         if (pci_enable_device(dev))
 		FAIL(-ENXIO, "Failed to enable OHCI hardware");
         pci_set_master(dev);
@@ -3503,10 +3516,8 @@ static void ohci1394_pci_remove(struct p
 #endif
 
 #ifdef CONFIG_PPC_PMAC
-	/* On UniNorth, power down the cable and turn off the chip
-	 * clock when the module is removed to save power on
-	 * laptops. Turning it back ON is done by the arch code when
-	 * pci_enable_device() is called */
+	/* On UniNorth, power down the cable and turn off the chip clock
+	 * to save power on laptops */
 	{
 		struct device_node* of_node;
 

[PATCH update 2/3] ieee1394: ohci1394: reformat PPC_PMAC platform code

[Stefan Richter]

Adjust whitespace and line lengths

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/ieee1394/ohci1394.c |   34 ++++++++++++++--------------------
 1 files changed, 14 insertions(+), 20 deletions(-)

Index: linux/drivers/ieee1394/ohci1394.c
===================================================================
--- linux.orig/drivers/ieee1394/ohci1394.c	2006-11-11 00:12:31.000000000 +0100
+++ linux/drivers/ieee1394/ohci1394.c	2006-11-11 00:13:45.000000000 +0100
@@ -3218,12 +3218,11 @@ static int __devinit ohci1394_pci_probe(
 #ifdef CONFIG_PPC_PMAC
 	/* Necessary on some machines if ohci1394 was loaded/ unloaded before */
 	if (machine_is(powermac)) {
-		struct device_node *of_node = pci_device_to_OF_node(dev);
+		struct device_node *ofn = pci_device_to_OF_node(dev);
 
-		if (of_node) {
-			pmac_call_feature(PMAC_FTR_1394_CABLE_POWER, of_node,
-					  0, 1);
-			pmac_call_feature(PMAC_FTR_1394_ENABLE, of_node, 0, 1);
+		if (ofn) {
+			pmac_call_feature(PMAC_FTR_1394_CABLE_POWER, ofn, 0, 1);
+			pmac_call_feature(PMAC_FTR_1394_ENABLE, ofn, 0, 1);
 		}
 	}
 #endif /* CONFIG_PPC_PMAC */
@@ -3519,12 +3518,11 @@ static void ohci1394_pci_remove(struct p
 	/* On UniNorth, power down the cable and turn off the chip clock
 	 * to save power on laptops */
 	{
-		struct device_node* of_node;
+		struct device_node* ofn = pci_device_to_OF_node(ohci->dev);
 
-		of_node = pci_device_to_OF_node(ohci->dev);
-		if (of_node) {
-			pmac_call_feature(PMAC_FTR_1394_ENABLE, of_node, 0, 0);
-			pmac_call_feature(PMAC_FTR_1394_CABLE_POWER, of_node, 0, 0);
+		if (ofn) {
+			pmac_call_feature(PMAC_FTR_1394_ENABLE, ofn, 0, 0);
+			pmac_call_feature(PMAC_FTR_1394_CABLE_POWER, ofn, 0, 0);
 		}
 	}
 #endif /* CONFIG_PPC_PMAC */
@@ -3584,12 +3582,10 @@ static int ohci1394_pci_suspend(struct p
 /* PowerMac suspend code comes last */
 #ifdef CONFIG_PPC_PMAC
 	if (machine_is(powermac)) {
-		struct device_node *of_node;
+		struct device_node *ofn = pci_device_to_OF_node(pdev);
 
-		/* Disable 1394 */
-		of_node = pci_device_to_OF_node (pdev);
-		if (of_node)
-			pmac_call_feature(PMAC_FTR_1394_ENABLE, of_node, 0, 0);
+		if (ofn)
+			pmac_call_feature(PMAC_FTR_1394_ENABLE, ofn, 0, 0);
 	}
 #endif /* CONFIG_PPC_PMAC */
 
@@ -3611,12 +3607,10 @@ static int ohci1394_pci_resume(struct pc
 /* PowerMac resume code comes first */
 #ifdef CONFIG_PPC_PMAC
 	if (machine_is(powermac)) {
-		struct device_node *of_node;
+		struct device_node *ofn = pci_device_to_OF_node(pdev);
 
-		/* Re-enable 1394 */
-		of_node = pci_device_to_OF_node (pdev);
-		if (of_node)
-			pmac_call_feature (PMAC_FTR_1394_ENABLE, of_node, 0, 1);
+		if (ofn)
+			pmac_call_feature(PMAC_FTR_1394_ENABLE, ofn, 0, 1);
 	}
 #endif /* CONFIG_PPC_PMAC */
 

[PATCH update 3/3] ieee1394: ohci1394: call PMac code in shutdown only for proper machines

[Stefan Richter]

There has been an if(...) missing, for ages.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/ieee1394/ohci1394.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

Index: linux/drivers/ieee1394/ohci1394.c
===================================================================
--- linux.orig/drivers/ieee1394/ohci1394.c	2006-11-11 00:13:45.000000000 +0100
+++ linux/drivers/ieee1394/ohci1394.c	2006-11-11 00:16:37.000000000 +0100
@@ -3517,7 +3517,7 @@ static void ohci1394_pci_remove(struct p
 #ifdef CONFIG_PPC_PMAC
 	/* On UniNorth, power down the cable and turn off the chip clock
 	 * to save power on laptops */
-	{
+	if (machine_is(powermac)) {
 		struct device_node* ofn = pci_device_to_OF_node(ohci->dev);
 
 		if (ofn) {

Re: [Fwd: [Bug 7431] New: ohci1394 Oops after a rmmod/modprobe cycle]

[Stefan Richter]

Benjamin Herrenschmidt wrote:
> On Sat, 2006-11-11 at 00:07 +0100, Stefan Richter wrote:
>> Actually, shouldn't ohci1394_pci_remove enlcose the platform code into
>>
>> 	if (machine_is(powermac)) {

> Yes.

And how does this expression evaluate on PowerBook G3 "Pismo"?
-- 
Stefan Richter
-=====-=-==- =-== -=-==
http://arcgraph.de/sr/

Re: [Fwd: [Bug 7431] New: ohci1394 Oops after a rmmod/modprobe cycle]

[Benjamin Herrenschmidt]

On Sat, 2006-11-11 at 00:55 +0100, Stefan Richter wrote:
> Benjamin Herrenschmidt wrote:
> > On Sat, 2006-11-11 at 00:07 +0100, Stefan Richter wrote:
> >> Actually, shouldn't ohci1394_pci_remove enlcose the platform code into
> >>
> >> 	if (machine_is(powermac)) {
> 
> > Yes.
> 
> And how does this expression evaluate on PowerBook G3 "Pismo"?

This will be true on all powerpc based apple machines, so it will be
true there too.

Ben.

Re: [Fwd: [Bug 7431] New: ohci1394 Oops after a rmmod/modprobe cycle]

[Olaf Hering]

On Fri, Nov 10, Stefan Richter wrote:

> On  7 Nov, Benjamin Herrenschmidt wrote:
> > On Tue, 2006-11-07 at 08:08 +0100, Olaf Hering wrote:
> [...]
> >> and it is the rmmod, not the insmod, that hangs the pismo.
> >> Adding some mdelay to the remove function doesnt help either.
> > 
> > Tried not calling the pmac feature stuff at all in the rmmod ?
> 
> Olaf?

The pismo usually locks up after the fist rmmod. With a few delays
around the feature calls, a few more modprobe/rmmod cycles are possible.
But it still locks up after a while.

Re: [Fwd: [Bug 7431] New: ohci1394 Oops after a rmmod/modprobe cycle]

[Stefan Richter]

Olaf Hering wrote:
>> On  7 Nov, Benjamin Herrenschmidt wrote:
>> > Tried not calling the pmac feature stuff at all in the rmmod ?

> The pismo usually locks up after the fist rmmod. With a few delays
> around the feature calls, a few more modprobe/rmmod cycles are possible.
> But it still locks up after a while.

But what if you delete the feature calls from ohci1394_pci_remove?
-- 
Stefan Richter
-=====-=-==- =-== -==-=
http://arcgraph.de/sr/

Re: [Fwd: [Bug 7431] New: ohci1394 Oops after a rmmod/modprobe cycle]

[Benjamin Herrenschmidt]

> The pismo usually locks up after the fist rmmod. With a few delays
> around the feature calls, a few more modprobe/rmmod cycles are possible.
> But it still locks up after a while.

What if you just remove all the feature calls ?

Ben.