[PATCH] fix for firewire patch added in 2.6.17.2 that breaks things on ppc

[PATCH] fix for firewire patch added in 2.6.17.2 that breaks things on ppc

[danny]

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

Hello,
I sent this to Ben earlier but forgot to cc this list:

> 
> Sleep broke on my ibook G3 with 2.6.17. After some tests it seemed it only broke when I had used my 
> iSight. Plugging it after a sleep/resume cycle would sometimes instantly hang the machine.
> 
> The problem appeared to result from a patch added in 2.6.17.2, where a
> pci_save_state(pdev);
> is called after
>  pmac_call_feature(PMAC_FTR_1394_ENABLE, of_node, 0, 0);
> 
> resuming shows that the corresponding pci_restore_state is writing all ffffffff to the config space. 
> Which is probably not a good idea.
> 

Patch is attached

Danny



[-- Attachment #2: ohci1394_save_state.patch --]
[-- Type: text/plain, Size: 987 bytes --]


    2.6.17.2 contained a patch for preliminary suspend/resume 
    handling on !PPC_PMAC. However, this broke suspend and firewire
    on powerpc because it saves the state after the device has already
    been disabled.

    Firewire works perfectly through suspend on my ibook, so save/restore
    state is not needed there.
    
    Signed-off-by: Danny Tholen <obiwan@mailmij.org>

--- linux-2.6.17.7/drivers/ieee1394/ohci1394.c~	2006-08-03 10:00:01.875855084 -0400
+++ linux-2.6.17.7/drivers/ieee1394/ohci1394.c	2006-08-03 10:08:24.274059577 -0400
@@ -3537,9 +3537,9 @@
 		if (of_node)
 			pmac_call_feature (PMAC_FTR_1394_ENABLE, of_node, 0, 1);
 	}
-#endif /* CONFIG_PPC_PMAC */
-
+#else
 	pci_restore_state(pdev);
+#endif /* CONFIG_PPC_PMAC */
 	pci_enable_device(pdev);
 
 	return 0;
@@ -3557,10 +3557,9 @@
 		if (of_node)
 			pmac_call_feature(PMAC_FTR_1394_ENABLE, of_node, 0, 0);
 	}
-#endif
-
+#else
 	pci_save_state(pdev);
-
+#endif /* CONFIG_PPC_PMAC */
 	return 0;
 }
 

Re: [PATCH] fix for firewire patch added in 2.6.17.2 that breaks things on ppc

[Paul Mackerras]

danny@mailmij.org writes:

> -#endif
> -
> +#else
>  	pci_save_state(pdev);
> -
> +#endif /* CONFIG_PPC_PMAC */

Shouldn't we just move the pci_save_state call up before the
pmac_call_feature() call?

Paul.

Re: [PATCH] fix for firewire patch added in 2.6.17.2 that breaks things on ppc

[danny]

On Tue, Aug 08, 2006 at 03:05:15PM +1000, Paul Mackerras wrote:
> danny@mailmij.org writes:
> 
> > -#endif
> > -
> > +#else
> >  	pci_save_state(pdev);
> > -
> > +#endif /* CONFIG_PPC_PMAC */
> 
> Shouldn't we just move the pci_save_state call up before the
> pmac_call_feature() call?
> 
I can do a rebuild tonight or so to test this. But since it was working perfectly before I did not see 
any reason to keep it. 

Danny

Re: [PATCH] fix for firewire patch added in 2.6.17.2 that breaks things on ppc

[danny]

On Tue, Aug 08, 2006 at 03:05:15PM +1000, Paul Mackerras wrote:
> danny@mailmij.org writes:
> 
> > -#endif
> > -
> > +#else
> >  	pci_save_state(pdev);
> > -
> > +#endif /* CONFIG_PPC_PMAC */
> 
> Shouldn't we just move the pci_save_state call up before the
> pmac_call_feature() call?
> 
> Paul.
> 
I tested this. It works, but it has no effect (pci_restore_state does not report that anything changed,
thus this means that the config space already contained the correct values).

I discovered one additional problem with leaving a device running through a sleep/resume cycle:
suspend:
ohci1394: fw-host0: SelfID received outside of bus reset sequence
resume:
ohci1394: fw-host0: SelfID is inconsistent [0x7e80773f/0x7e80773f]
The last leads to the node not being able to resume when the device is actually plugged.

Anyway, it's a separate issue, I will investigate.

Danny