[PATCH 0/3] firewire: PPC PMac updates

[PATCH 0/3] firewire: PPC PMac updates

[Stefan Richter]

Next up:
1/3 firewire: fw-ohci: PPC PMac platform code
2/3 firewire: fw-ohci: Apple UniNorth 1st generation support
3/3 firewire: fw-ohci: shut up false compiler warning on PPC32
-- 
Stefan Richter
-=====-==--- --== ----=
http://arcgraph.de/sr/

[PATCH 1/3] firewire: fw-ohci: PPC PMac platform code

[Stefan Richter]

Copied from ohci1394.c.  This code is necessary to prevent machine check
exceptions when reloading or resuming the driver.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/fw-ohci.c |   50 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

Index: linux-2.6.25-rc3/drivers/firewire/fw-ohci.c
===================================================================
--- linux-2.6.25-rc3.orig/drivers/firewire/fw-ohci.c
+++ linux-2.6.25-rc3/drivers/firewire/fw-ohci.c
@@ -33,6 +33,10 @@
 #include <asm/page.h>
 #include <asm/system.h>
 
+#ifdef CONFIG_PPC_PMAC
+#include <asm/pmac_feature.h>
+#endif
+
 #include "fw-ohci.h"
 #include "fw-transaction.h"
 
@@ -2057,6 +2061,18 @@ pci_probe(struct pci_dev *dev, const str
 	int err;
 	size_t size;
 
+#ifdef CONFIG_PPC_PMAC
+	/* Necessary on some machines if fw-ohci was loaded/ unloaded before */
+	if (machine_is(powermac)) {
+		struct device_node *ofn = pci_device_to_OF_node(dev);
+
+		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 */
+
 	ohci = kzalloc(sizeof(*ohci), GFP_KERNEL);
 	if (ohci == NULL) {
 		fw_error("Could not malloc fw_ohci data.\n");
@@ -2189,6 +2205,20 @@ static void pci_remove(struct pci_dev *d
 	pci_iounmap(dev, ohci->registers);
 	pci_release_region(dev, 0);
 	pci_disable_device(dev);
+
+#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(dev);
+
+		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 */
+
 	kfree(&ohci->card);
 
 	fw_notify("Removed fw-ohci device.\n");
@@ -2211,6 +2241,16 @@ static int pci_suspend(struct pci_dev *p
 	if (err)
 		fw_error("pci_set_power_state failed with %d\n", err);
 
+/* PowerMac suspend code comes last */
+#ifdef CONFIG_PPC_PMAC
+	if (machine_is(powermac)) {
+		struct device_node *ofn = pci_device_to_OF_node(pdev);
+
+		if (ofn)
+			pmac_call_feature(PMAC_FTR_1394_ENABLE, ofn, 0, 0);
+	}
+#endif /* CONFIG_PPC_PMAC */
+
 	return 0;
 }
 
@@ -2219,6 +2259,16 @@ static int pci_resume(struct pci_dev *pd
 	struct fw_ohci *ohci = pci_get_drvdata(pdev);
 	int err;
 
+/* PowerMac resume code comes first */
+#ifdef CONFIG_PPC_PMAC
+	if (machine_is(powermac)) {
+		struct device_node *ofn = pci_device_to_OF_node(pdev);
+
+		if (ofn)
+			pmac_call_feature(PMAC_FTR_1394_ENABLE, ofn, 0, 1);
+	}
+#endif /* CONFIG_PPC_PMAC */
+
 	pci_set_power_state(pdev, PCI_D0);
 	pci_restore_state(pdev);
 	err = pci_enable_device(pdev);

-- 
Stefan Richter
-=====-==--- --== ----=
http://arcgraph.de/sr/

[PATCH 2/3] firewire: fw-ohci: Apple UniNorth 1st generation support

[Stefan Richter]

Mostly copied from ohci1394.c.  Necessary for some older Macs, e.g.
PowerBook G3 Pismo and early PowerBook G4 Titanium.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---

Since my TiBook has a broken FireWire PHY I was only able to test
the byte order of self ID packets but not of the headers of any
other packets.  The code in handle_ar_packet() is only guesswork
from ohci1394.c's respective code.  Also, I wonder what's up with
isochronous packets...

 drivers/firewire/fw-ohci.c |   29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

Index: linux-2.6.25-rc3/drivers/firewire/fw-ohci.c
===================================================================
--- linux-2.6.25-rc3.orig/drivers/firewire/fw-ohci.c
+++ linux-2.6.25-rc3/drivers/firewire/fw-ohci.c
@@ -179,6 +179,7 @@ struct fw_ohci {
 	int generation;
 	int request_generation;
 	u32 bus_seconds;
+	bool old_uninorth;
 
 	/*
 	 * Spinlock for accessing fw_ohci data.  Never call out of
@@ -315,15 +316,22 @@ static int ar_context_add_page(struct ar
 	return 0;
 }
 
+#if defined(CONFIG_PPC_PMAC) && defined(CONFIG_PPC32)
+#define cond_le32_to_cpu(v) \
+	(ohci->old_uninorth ? (__force __u32)(v) : le32_to_cpu(v))
+#else
+#define cond_le32_to_cpu(v) le32_to_cpu(v)
+#endif
+
 static __le32 *handle_ar_packet(struct ar_context *ctx, __le32 *buffer)
 {
 	struct fw_ohci *ohci = ctx->ohci;
 	struct fw_packet p;
 	u32 status, length, tcode;
 
-	p.header[0] = le32_to_cpu(buffer[0]);
-	p.header[1] = le32_to_cpu(buffer[1]);
-	p.header[2] = le32_to_cpu(buffer[2]);
+	p.header[0] = cond_le32_to_cpu(buffer[0]);
+	p.header[1] = cond_le32_to_cpu(buffer[1]);
+	p.header[2] = cond_le32_to_cpu(buffer[2]);
 
 	tcode = (p.header[0] >> 4) & 0x0f;
 	switch (tcode) {
@@ -335,7 +343,7 @@ static __le32 *handle_ar_packet(struct a
 		break;
 
 	case TCODE_READ_BLOCK_REQUEST :
-		p.header[3] = le32_to_cpu(buffer[3]);
+		p.header[3] = cond_le32_to_cpu(buffer[3]);
 		p.header_length = 16;
 		p.payload_length = 0;
 		break;
@@ -344,7 +352,7 @@ static __le32 *handle_ar_packet(struct a
 	case TCODE_READ_BLOCK_RESPONSE:
 	case TCODE_LOCK_REQUEST:
 	case TCODE_LOCK_RESPONSE:
-		p.header[3] = le32_to_cpu(buffer[3]);
+		p.header[3] = cond_le32_to_cpu(buffer[3]);
 		p.header_length = 16;
 		p.payload_length = p.header[3] >> 16;
 		break;
@@ -361,7 +369,7 @@ static __le32 *handle_ar_packet(struct a
 
 	/* FIXME: What to do about evt_* errors? */
 	length = (p.header_length + p.payload_length + 3) / 4;
-	status = le32_to_cpu(buffer[length]);
+	status = cond_le32_to_cpu(buffer[length]);
 
 	p.ack        = ((status >> 16) & 0x1f) - 16;
 	p.speed      = (status >> 21) & 0x7;
@@ -1026,13 +1034,14 @@ static void bus_reset_tasklet(unsigned l
 	 */
 
 	self_id_count = (reg_read(ohci, OHCI1394_SelfIDCount) >> 3) & 0x3ff;
-	generation = (le32_to_cpu(ohci->self_id_cpu[0]) >> 16) & 0xff;
+	generation = (cond_le32_to_cpu(ohci->self_id_cpu[0]) >> 16) & 0xff;
 	rmb();
 
 	for (i = 1, j = 0; j < self_id_count; i += 2, j++) {
 		if (ohci->self_id_cpu[i] != ~ohci->self_id_cpu[i + 1])
 			fw_error("inconsistent self IDs\n");
-		ohci->self_id_buffer[j] = le32_to_cpu(ohci->self_id_cpu[i]);
+		ohci->self_id_buffer[j] =
+				cond_le32_to_cpu(ohci->self_id_cpu[i]);
 	}
 	rmb();
 
@@ -2091,6 +2100,10 @@ pci_probe(struct pci_dev *dev, const str
 	pci_write_config_dword(dev, OHCI1394_PCI_HCI_Control, 0);
 	pci_set_drvdata(dev, ohci);
 
+#if defined(CONFIG_PPC_PMAC) && defined(CONFIG_PPC32)
+	ohci->old_uninorth = dev->vendor == PCI_VENDOR_ID_APPLE &&
+			     dev->device == PCI_DEVICE_ID_APPLE_UNI_N_FW;
+#endif
 	spin_lock_init(&ohci->lock);
 
 	tasklet_init(&ohci->bus_reset_tasklet,

-- 
Stefan Richter
-=====-==--- --== ----=
http://arcgraph.de/sr/

[PATCH 3/3] firewire: fw-ohci: shut up false compiler warning on PPC32

[Stefan Richter]

Shut up two "may be used uninitialised in this function" warnings due to
PPC32's implementation of dma_alloc_coherent().

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---

Wasn't there a macro somewhere for this?

Also, could this be better done in PPC32's dma_alloc_coherent()?
At least this kind of workaround here doesn't add anything to the
compiled result.

 drivers/firewire/fw-ohci.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux-2.6.25-rc3/drivers/firewire/fw-ohci.c
===================================================================
--- linux-2.6.25-rc3.orig/drivers/firewire/fw-ohci.c
+++ linux-2.6.25-rc3/drivers/firewire/fw-ohci.c
@@ -544,7 +544,7 @@ static int
 context_add_buffer(struct context *ctx)
 {
 	struct descriptor_buffer *desc;
-	dma_addr_t bus_addr;
+	dma_addr_t bus_addr = bus_addr;
 	int offset;
 
 	/*
@@ -1334,7 +1334,7 @@ ohci_set_config_rom(struct fw_card *card
 	unsigned long flags;
 	int retval = -EBUSY;
 	__be32 *next_config_rom;
-	dma_addr_t next_config_rom_bus;
+	dma_addr_t next_config_rom_bus = next_config_rom_bus;
 
 	ohci = fw_ohci(card);
 

-- 
Stefan Richter
-=====-==--- --== ----=
http://arcgraph.de/sr/

Re: [PATCH 1/3] firewire: fw-ohci: PPC PMac platform code

[Stefan Richter]

I wrote:
> +		struct device_node* ofn = pci_device_to_OF_node(dev);

-ECOPYANDWASTE:	struct device_node *ofn

-- 
Stefan Richter
-=====-==--- --== ----=
http://arcgraph.de/sr/

Re: [PATCH 1/3] firewire: fw-ohci: PPC PMac platform code

[Benjamin Herrenschmidt]

> +#ifdef CONFIG_PPC_PMAC
> +	/* Necessary on some machines if fw-ohci was loaded/ unloaded before */
> +	if (machine_is(powermac)) {
> +		struct device_node *ofn = pci_device_to_OF_node(dev);
> +
> +		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 */

I -think- that the hunk above can be removed if you just call
pci_enable_device(). I have a hook in the arch pcibios_enable_device()
that detects the internal firewire and does the calls. Can you verify it
works ?

> +#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(dev);
> +
> +		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 */

Unfortunately, I don't have the pending hook here for disable so you
need to keep that one for now.

>  	kfree(&ohci->card);
>  
>  	fw_notify("Removed fw-ohci device.\n");
> @@ -2211,6 +2241,16 @@ static int pci_suspend(struct pci_dev *p
>  	if (err)
>  		fw_error("pci_set_power_state failed with %d\n", err);
>  
> +/* PowerMac suspend code comes last */
> +#ifdef CONFIG_PPC_PMAC
> +	if (machine_is(powermac)) {
> +		struct device_node *ofn = pci_device_to_OF_node(pdev);
> +
> +		if (ofn)
> +			pmac_call_feature(PMAC_FTR_1394_ENABLE, ofn, 0, 0);
> +	}
> +#endif /* CONFIG_PPC_PMAC */

I wonder why we don't do the cable power thingy here...

Cheers,
Ben.

Re: [PATCH 3/3] firewire: fw-ohci: shut up false compiler warning on PPC32

[Stephen Rothwell]

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

On Sat, 1 Mar 2008 02:50:44 +0100 (CET) Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
>
> Shut up two "may be used uninitialised in this function" warnings due to
> PPC32's implementation of dma_alloc_coherent().
> 
> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
> ---
> 
> Wasn't there a macro somewhere for this?

Yes, uninitialized_var() in linux/compiler*.h

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 1/3] firewire: fw-ohci: PPC PMac platform code

[Stefan Richter]

Benjamin Herrenschmidt wrote:
>> +#ifdef CONFIG_PPC_PMAC
>> +	/* Necessary on some machines if fw-ohci was loaded/ unloaded before */
>> +	if (machine_is(powermac)) {
>> +		struct device_node *ofn = pci_device_to_OF_node(dev);
>> +
>> +		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 */
> 
> I -think- that the hunk above can be removed if you just call
> pci_enable_device(). I have a hook in the arch pcibios_enable_device()
> that detects the internal firewire and does the calls. Can you verify it
> works ?

The first test I did was
# modprobe ohci1394    # this has the same four feature call blocks
# modprobe -r ohci1394
# modprobe firewire-ohci
<boom>

Tried the same now again (actually modprobe -r firewire-ohci (patched); 
modprobe firewire-ohci (unpatched)):

Machine check in kernel mode.
Caused by (from SRR1=49030): Transfer error ack signal
Oops: Machine check, sig: 7 [#1]
PowerMac
Modules linked in: firewire_ohci(+) firewire_core crc_itu_t af_packet 
apm_emu apm_emulation usbhid sungem sungem_phy yenta_socket 
rsrc_nonstatic ohci_hcd pcmcia_core usbcore ide_cd_mod cdrom evdev [last 
unloaded: crc_itu_t]
NIP: e18ee3dc LR: e18ee32c CTR: c0065178
REGS: d7c01c40 TRAP: 0200   Not tainted  (2.6.25-rc3)
MSR: 00049030 <EE,ME,IR,DR>  CR: 84004428  XER: 00000000
TASK = dedae6d0[5376] 'modprobe' THREAD: d7c00000
GPR00: e18e61c0 d7c01cf0 dedae6d0 defcf000 00000000 00000385 c06c09f8 
00001000
GPR08: ffffec0f 00000000 e18e6000 dfafd000 24004428 1001e354 00000000 
00000000
GPR16: 0000007a 0000007a 00000000 e18e9bf4 00000124 00000000 e18e6000 
c0050280
GPR24: 00000019 e18e9cc4 c0290630 dfb61420 df85b800 dfafdcf8 00000000 
dfafdcf8
NIP [e18ee3dc] ar_context_add_page+0xd0/0xf4 [firewire_ohci]
LR [e18ee32c] ar_context_add_page+0x20/0xf4 [firewire_ohci]
Call Trace:
[d7c01cf0] [e18ee32c] ar_context_add_page+0x20/0xf4 [firewire_ohci] 
(unreliable)
[d7c01d00] [e18ee440] ar_context_init+0x40/0x70 [firewire_ohci]
[d7c01d50] [e18f0004] pci_probe+0x118/0x70c [firewire_ohci]
[d7c01d70] [c010e0ac] pci_device_probe+0x80/0xa0
[d7c01d90] [c0146230] driver_probe_device+0xb8/0x1cc
[d7c01db0] [c0146528] __driver_attach+0xd4/0x104
[d7c01dd0] [c0144d78] bus_for_each_dev+0x58/0x94
[d7c01e00] [c0145f60] driver_attach+0x24/0x34
[d7c01e10] [c0145d48] bus_add_driver+0xb0/0x250
[d7c01e30] [c0146a18] driver_register+0x48/0x130
[d7c01e50] [c010dcb0] __pci_register_driver+0x48/0x94
[d7c01e70] [e101e028] fw_ohci_init+0x28/0x48 [firewire_ohci]
[d7c01e80] [c00516e8] sys_init_module+0xd4/0x1754
[d7c01f40] [c0013d98] ret_from_syscall+0x0/0x38
--- Exception: c01 at 0xff7325c
     LR = 0x10003b50
Instruction dump:
90690010 817f0000 907f0008 814b04b4 813f0010 7c0a4a14 7c0004ac 7ce0052c
817f0000 812b04b4 7c0004ac 7d204c2c <0c090000> 4c00012c 38000000 7c030378
---[ end trace a3cf91a5f7d45355 ]---


BTW, ohci1394 had traditionally only 3 of the blocks; I added the block 
by commit 48cfae44b4d6c7ca843d611a93ed2f94b59bcb38 in November 2006. 
This fixed http://bugzilla.kernel.org/show_bug.cgi?id=7431 .

[...]
>> @@ -2211,6 +2241,16 @@ static int pci_suspend(struct pci_dev *p
>>  	if (err)
>>  		fw_error("pci_set_power_state failed with %d\n", err);
>>  
>> +/* PowerMac suspend code comes last */
>> +#ifdef CONFIG_PPC_PMAC
>> +	if (machine_is(powermac)) {
>> +		struct device_node *ofn = pci_device_to_OF_node(pdev);
>> +
>> +		if (ofn)
>> +			pmac_call_feature(PMAC_FTR_1394_ENABLE, ofn, 0, 0);
>> +	}
>> +#endif /* CONFIG_PPC_PMAC */
> 
> I wonder why we don't do the cable power thingy here...

I don't know.

I tested with a bus powered device now.  Actually, suspend to RAM 
switches off cable power nevertheless, but resume does not switch it on 
anymore...

Of course the same is true with ohci1394.  Why did nobody ever complain? 
I suppose the last two users of FireWire + Linux + UniNorth rev01 don't 
use cable power or don't suspend and resume.
-- 
Stefan Richter
-=====-==--- --== ----=
http://arcgraph.de/sr/

Re: [PATCH 3/3] firewire: fw-ohci: shut up false compiler warning on PPC32

[Stefan Richter]

Stephen Rothwell wrote:
> On Sat, 1 Mar 2008 02:50:44 +0100 (CET) Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
>> Wasn't there a macro somewhere for this?
> 
> Yes, uninitialized_var() in linux/compiler*.h

Right.  I actually did look into compiler.h and compiler-gcc.h but 
somehow didn't make it down into -gcc?.h.  Thanks,
-- 
Stefan Richter
-=====-==--- --== ----=
http://arcgraph.de/sr/

[PATCH 3/3 update] firewire: fw-ohci: shut up false compiler warning on PPC32

[Stefan Richter]

Shut up two "may be used uninitialised in this function" warnings due to
PPC32's implementation of dma_alloc_coherent().

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/fw-ohci.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux-2.6.25-rc3/drivers/firewire/fw-ohci.c
===================================================================
--- linux-2.6.25-rc3.orig/drivers/firewire/fw-ohci.c
+++ linux-2.6.25-rc3/drivers/firewire/fw-ohci.c
@@ -544,7 +544,7 @@ static int
 context_add_buffer(struct context *ctx)
 {
 	struct descriptor_buffer *desc;
-	dma_addr_t bus_addr;
+	dma_addr_t uninitialized_var(bus_addr);
 	int offset;
 
 	/*
@@ -1334,7 +1334,7 @@ ohci_set_config_rom(struct fw_card *card
 	unsigned long flags;
 	int retval = -EBUSY;
 	__be32 *next_config_rom;
-	dma_addr_t next_config_rom_bus;
+	dma_addr_t uninitialized_var(next_config_rom_bus);
 
 	ohci = fw_ohci(card);
 

-- 
Stefan Richter
-=====-==--- --== ----=
http://arcgraph.de/sr/

[PATCH 0/4] firewire, ieee1394: bus power lost after resume (PPC PMac)

[Stefan Richter]

I wrote:
> Benjamin Herrenschmidt wrote:
> [...]
>>> @@ -2211,6 +2241,16 @@ static int pci_suspend(struct pci_dev *p
>>>  	if (err)
>>>  		fw_error("pci_set_power_state failed with %d\n", err);
>>>  
>>> +/* PowerMac suspend code comes last */
>>> +#ifdef CONFIG_PPC_PMAC
>>> +	if (machine_is(powermac)) {
>>> +		struct device_node *ofn = pci_device_to_OF_node(pdev);
>>> +
>>> +		if (ofn)
>>> +			pmac_call_feature(PMAC_FTR_1394_ENABLE, ofn, 0, 0);
>>> +	}
>>> +#endif /* CONFIG_PPC_PMAC */
>> 
>> I wonder why we don't do the cable power thingy here...
> 
> I don't know.
> 
> I tested with a bus powered device now.  Actually, suspend to RAM 
> switches off cable power nevertheless, but resume does not switch it on 
> anymore...
> 
> Of course the same is true with ohci1394.  Why did nobody ever complain? 
> I suppose the last two users of FireWire + Linux + UniNorth rev01 don't 
> use cable power or don't suspend and resume.

At least this is what the 1st generation PowerBook G4 does.  I don't
know if other PPC_PMAC are affected.

I suggest we explicitly switch cable power on in .resume() but also
explicitly switch cable power off in .suspend(), so that we don't get
any surprises. (Hardware bugs related to these platform features
excepted.)  Patches follow:

 1/4 firewire: fw-ohci: switch on bus power after resume on PPC PMac
 2/4 firewire: fw-ohci: refactor probe, remove, suspend, resume
 3/4 ieee1394: ohci1394: switch on bus power after resume on PPC PMac
 4/4 ieee1394: ohci1394: refactor probe, remove, suspend, resume

 drivers/firewire/fw-ohci.c  |   96 +++++++++++++------------------
 drivers/ieee1394/ohci1394.c |  109 ++++++++++++++++--------------------
 2 files changed, 91 insertions(+), 114 deletions(-)
-- 
Stefan Richter
-=====-==--- --== ----=
http://arcgraph.de/sr/

[PATCH 1/4] firewire: fw-ohci: switch on bus power after resume on PPC PMac

[Stefan Richter]

The platform feature calls in the suspend method switched off cable
power, but the calls in the resume method did not switch it back on.

Add the necessary feature call to .resume.  Also add the corresponding
call to .suspend to make .suspend's behavior explicitly the same on all
PMacs.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/fw-ohci.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Index: linux/drivers/firewire/fw-ohci.c
===================================================================
--- linux.orig/drivers/firewire/fw-ohci.c
+++ linux/drivers/firewire/fw-ohci.c
@@ -2259,8 +2259,10 @@ static int pci_suspend(struct pci_dev *p
 	if (machine_is(powermac)) {
 		struct device_node *ofn = pci_device_to_OF_node(pdev);
 
-		if (ofn)
+		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 */
 
@@ -2277,8 +2279,10 @@ static int pci_resume(struct pci_dev *pd
 	if (machine_is(powermac)) {
 		struct device_node *ofn = pci_device_to_OF_node(pdev);
 
-		if (ofn)
+		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 */
 

-- 
Stefan Richter
-=====-==--- --== ----=
http://arcgraph.de/sr/

[PATCH 2/4] firewire: fw-ohci: refactor probe, remove, suspend, resume

[Stefan Richter]

Clean up shared code and variable names.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/fw-ohci.c |  100 ++++++++++++++++++---------------------------
 1 file changed, 42 insertions(+), 58 deletions(-)

Index: linux/drivers/firewire/fw-ohci.c
===================================================================
--- linux.orig/drivers/firewire/fw-ohci.c
+++ linux/drivers/firewire/fw-ohci.c
@@ -2061,17 +2061,9 @@ static const struct fw_card_driver ohci_
 	.stop_iso		= ohci_stop_iso,
 };
 
-static int __devinit
-pci_probe(struct pci_dev *dev, const struct pci_device_id *ent)
-{
-	struct fw_ohci *ohci;
-	u32 bus_options, max_receive, link_speed;
-	u64 guid;
-	int err;
-	size_t size;
-
 #ifdef CONFIG_PPC_PMAC
-	/* Necessary on some machines if fw-ohci was loaded/ unloaded before */
+static void ohci_pmac_on(struct pci_dev *dev)
+{
 	if (machine_is(powermac)) {
 		struct device_node *ofn = pci_device_to_OF_node(dev);
 
@@ -2080,8 +2072,35 @@ pci_probe(struct pci_dev *dev, const str
 			pmac_call_feature(PMAC_FTR_1394_ENABLE, ofn, 0, 1);
 		}
 	}
+}
+
+static void ohci_pmac_off(struct pci_dev *dev)
+{
+	if (machine_is(powermac)) {
+		struct device_node *ofn = pci_device_to_OF_node(dev);
+
+		if (ofn) {
+			pmac_call_feature(PMAC_FTR_1394_ENABLE, ofn, 0, 0);
+			pmac_call_feature(PMAC_FTR_1394_CABLE_POWER, ofn, 0, 0);
+		}
+	}
+}
+#else
+#define ohci_pmac_on(dev)
+#define ohci_pmac_off(dev)
 #endif /* CONFIG_PPC_PMAC */
 
+static int __devinit
+pci_probe(struct pci_dev *dev, const struct pci_device_id *ent)
+{
+	struct fw_ohci *ohci;
+	u32 bus_options, max_receive, link_speed;
+	u64 guid;
+	int err;
+	size_t size;
+
+	ohci_pmac_on(dev);
+
 	ohci = kzalloc(sizeof(*ohci), GFP_KERNEL);
 	if (ohci == NULL) {
 		fw_error("Could not malloc fw_ohci data.\n");
@@ -2218,77 +2237,42 @@ static void pci_remove(struct pci_dev *d
 	pci_iounmap(dev, ohci->registers);
 	pci_release_region(dev, 0);
 	pci_disable_device(dev);
-
-#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(dev);
-
-		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 */
-
+	ohci_pmac_off(dev);
 	kfree(&ohci->card);
 
 	fw_notify("Removed fw-ohci device.\n");
 }
 
 #ifdef CONFIG_PM
-static int pci_suspend(struct pci_dev *pdev, pm_message_t state)
+static int pci_suspend(struct pci_dev *dev, pm_message_t state)
 {
-	struct fw_ohci *ohci = pci_get_drvdata(pdev);
+	struct fw_ohci *ohci = pci_get_drvdata(dev);
 	int err;
 
 	software_reset(ohci);
-	free_irq(pdev->irq, ohci);
-	err = pci_save_state(pdev);
+	free_irq(dev->irq, ohci);
+	err = pci_save_state(dev);
 	if (err) {
 		fw_error("pci_save_state failed\n");
 		return err;
 	}
-	err = pci_set_power_state(pdev, pci_choose_state(pdev, state));
+	err = pci_set_power_state(dev, pci_choose_state(dev, state));
 	if (err)
 		fw_error("pci_set_power_state failed with %d\n", err);
-
-/* PowerMac suspend code comes last */
-#ifdef CONFIG_PPC_PMAC
-	if (machine_is(powermac)) {
-		struct device_node *ofn = pci_device_to_OF_node(pdev);
-
-		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 */
+	ohci_pmac_off(dev);
 
 	return 0;
 }
 
-static int pci_resume(struct pci_dev *pdev)
+static int pci_resume(struct pci_dev *dev)
 {
-	struct fw_ohci *ohci = pci_get_drvdata(pdev);
+	struct fw_ohci *ohci = pci_get_drvdata(dev);
 	int err;
 
-/* PowerMac resume code comes first */
-#ifdef CONFIG_PPC_PMAC
-	if (machine_is(powermac)) {
-		struct device_node *ofn = pci_device_to_OF_node(pdev);
-
-		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 */
-
-	pci_set_power_state(pdev, PCI_D0);
-	pci_restore_state(pdev);
-	err = pci_enable_device(pdev);
+	ohci_pmac_on(dev);
+	pci_set_power_state(dev, PCI_D0);
+	pci_restore_state(dev);
+	err = pci_enable_device(dev);
 	if (err) {
 		fw_error("pci_enable_device failed\n");
 		return err;

-- 
Stefan Richter
-=====-==--- --== ----=
http://arcgraph.de/sr/

[PATCH 3/4] ieee1394: ohci1394: switch on bus power after resume on PPC PMac

[Stefan Richter]

The platform feature calls in the suspend method switched off cable
power, but the calls in the resume method did not switch it back on.

Add the necessary feature call to .resume.  Also add the corresponding
call to .suspend to make .suspend's behavior explicitly the same on all
PMacs.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/ieee1394/ohci1394.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Index: linux/drivers/ieee1394/ohci1394.c
===================================================================
--- linux.orig/drivers/ieee1394/ohci1394.c
+++ linux/drivers/ieee1394/ohci1394.c
@@ -3340,8 +3340,10 @@ static int ohci1394_pci_suspend(struct p
 	if (machine_is(powermac)) {
 		struct device_node *ofn = pci_device_to_OF_node(pdev);
 
-		if (ofn)
+		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 */
 
@@ -3365,8 +3367,10 @@ static int ohci1394_pci_resume(struct pc
 	if (machine_is(powermac)) {
 		struct device_node *ofn = pci_device_to_OF_node(pdev);
 
-		if (ofn)
+		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 */
 

-- 
Stefan Richter
-=====-==--- --== ----=
http://arcgraph.de/sr/

[PATCH 4/4] ieee1394: ohci1394: refactor probe, remove, suspend, resume

[Stefan Richter]

Clean up shared code and variable names.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/ieee1394/ohci1394.c |  113 +++++++++++++++++++-------------------------
 1 file changed, 49 insertions(+), 64 deletions(-)

Index: linux/drivers/ieee1394/ohci1394.c
===================================================================
--- linux.orig/drivers/ieee1394/ohci1394.c
+++ linux/drivers/ieee1394/ohci1394.c
@@ -2993,15 +2993,9 @@ do {						\
 	return err;				\
 } while (0)
 
-static int __devinit ohci1394_pci_probe(struct pci_dev *dev,
-					const struct pci_device_id *ent)
-{
-	struct hpsb_host *host;
-	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 */
+static void ohci1394_pmac_on(struct pci_dev *dev)
+{
 	if (machine_is(powermac)) {
 		struct device_node *ofn = pci_device_to_OF_node(dev);
 
@@ -3010,8 +3004,32 @@ static int __devinit ohci1394_pci_probe(
 			pmac_call_feature(PMAC_FTR_1394_ENABLE, ofn, 0, 1);
 		}
 	}
+}
+
+static void ohci1394_pmac_off(struct pci_dev *dev)
+{
+	if (machine_is(powermac)) {
+		struct device_node *ofn = pci_device_to_OF_node(dev);
+
+		if (ofn) {
+			pmac_call_feature(PMAC_FTR_1394_ENABLE, ofn, 0, 0);
+			pmac_call_feature(PMAC_FTR_1394_CABLE_POWER, ofn, 0, 0);
+		}
+	}
+}
+#else
+#define ohci1394_pmac_on(dev)
+#define ohci1394_pmac_off(dev)
 #endif /* CONFIG_PPC_PMAC */
 
+static int __devinit ohci1394_pci_probe(struct pci_dev *dev,
+					const struct pci_device_id *ent)
+{
+	struct hpsb_host *host;
+	struct ti_ohci *ohci;	/* shortcut to currently handled device */
+	resource_size_t ohci_base;
+
+	ohci1394_pmac_on(dev);
         if (pci_enable_device(dev))
 		FAIL(-ENXIO, "Failed to enable OHCI hardware");
         pci_set_master(dev);
@@ -3203,16 +3221,16 @@ static int __devinit ohci1394_pci_probe(
 #undef FAIL
 }
 
-static void ohci1394_pci_remove(struct pci_dev *pdev)
+static void ohci1394_pci_remove(struct pci_dev *dev)
 {
 	struct ti_ohci *ohci;
-	struct device *dev;
+	struct device *device;
 
-	ohci = pci_get_drvdata(pdev);
+	ohci = pci_get_drvdata(dev);
 	if (!ohci)
 		return;
 
-	dev = get_device(&ohci->host->device);
+	device = get_device(&ohci->host->device);
 
 	switch (ohci->init_state) {
 	case OHCI_INIT_DONE:
@@ -3246,7 +3264,7 @@ static void ohci1394_pci_remove(struct p
 		/* Soft reset before we start - this disables
 		 * interrupts and clears linkEnable and LPS. */
 		ohci_soft_reset(ohci);
-		free_irq(ohci->dev->irq, ohci);
+		free_irq(dev->irq, ohci);
 
 	case OHCI_INIT_HAVE_TXRX_BUFFERS__MAYBE:
 		/* The ohci_soft_reset() stops all DMA contexts, so we
@@ -3257,12 +3275,12 @@ static void ohci1394_pci_remove(struct p
 		free_dma_trm_ctx(&ohci->at_resp_context);
 
 	case OHCI_INIT_HAVE_SELFID_BUFFER:
-		pci_free_consistent(ohci->dev, OHCI1394_SI_DMA_BUF_SIZE,
+		pci_free_consistent(dev, OHCI1394_SI_DMA_BUF_SIZE,
 				    ohci->selfid_buf_cpu,
 				    ohci->selfid_buf_bus);
 
 	case OHCI_INIT_HAVE_CONFIG_ROM_BUFFER:
-		pci_free_consistent(ohci->dev, OHCI_CONFIG_ROM_LEN,
+		pci_free_consistent(dev, OHCI_CONFIG_ROM_LEN,
 				    ohci->csr_config_rom_cpu,
 				    ohci->csr_config_rom_bus);
 
@@ -3270,35 +3288,24 @@ static void ohci1394_pci_remove(struct p
 		iounmap(ohci->registers);
 
 	case OHCI_INIT_HAVE_MEM_REGION:
-		release_mem_region(pci_resource_start(ohci->dev, 0),
+		release_mem_region(pci_resource_start(dev, 0),
 				   OHCI1394_REGISTER_SIZE);
 
-#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) {
-			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 */
+		ohci1394_pmac_off(dev);
 
 	case OHCI_INIT_ALLOC_HOST:
-		pci_set_drvdata(ohci->dev, NULL);
+		pci_set_drvdata(dev, NULL);
 	}
 
-	if (dev)
-		put_device(dev);
+	if (device)
+		put_device(device);
 }
 
 #ifdef CONFIG_PM
-static int ohci1394_pci_suspend(struct pci_dev *pdev, pm_message_t state)
+static int ohci1394_pci_suspend(struct pci_dev *dev, pm_message_t state)
 {
 	int err;
-	struct ti_ohci *ohci = pci_get_drvdata(pdev);
+	struct ti_ohci *ohci = pci_get_drvdata(dev);
 
 	if (!ohci) {
 		printk(KERN_ERR "%s: tried to suspend nonexisting host\n",
@@ -3326,34 +3333,23 @@ static int ohci1394_pci_suspend(struct p
 	ohci_devctl(ohci->host, RESET_BUS, LONG_RESET_NO_FORCE_ROOT);
 	ohci_soft_reset(ohci);
 
-	err = pci_save_state(pdev);
+	err = pci_save_state(dev);
 	if (err) {
 		PRINT(KERN_ERR, "pci_save_state failed with %d", err);
 		return err;
 	}
-	err = pci_set_power_state(pdev, pci_choose_state(pdev, state));
+	err = pci_set_power_state(dev, pci_choose_state(dev, state));
 	if (err)
 		DBGMSG("pci_set_power_state failed with %d", err);
-
-/* PowerMac suspend code comes last */
-#ifdef CONFIG_PPC_PMAC
-	if (machine_is(powermac)) {
-		struct device_node *ofn = pci_device_to_OF_node(pdev);
-
-		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 */
+	ohci1394_pmac_off(dev);
 
 	return 0;
 }
 
-static int ohci1394_pci_resume(struct pci_dev *pdev)
+static int ohci1394_pci_resume(struct pci_dev *dev)
 {
 	int err;
-	struct ti_ohci *ohci = pci_get_drvdata(pdev);
+	struct ti_ohci *ohci = pci_get_drvdata(dev);
 
 	if (!ohci) {
 		printk(KERN_ERR "%s: tried to resume nonexisting host\n",
@@ -3362,21 +3358,10 @@ static int ohci1394_pci_resume(struct pc
 	}
 	DBGMSG("resume called");
 
-/* PowerMac resume code comes first */
-#ifdef CONFIG_PPC_PMAC
-	if (machine_is(powermac)) {
-		struct device_node *ofn = pci_device_to_OF_node(pdev);
-
-		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 */
-
-	pci_set_power_state(pdev, PCI_D0);
-	pci_restore_state(pdev);
-	err = pci_enable_device(pdev);
+	ohci1394_pmac_on(dev);
+	pci_set_power_state(dev, PCI_D0);
+	pci_restore_state(dev);
+	err = pci_enable_device(dev);
 	if (err) {
 		PRINT(KERN_ERR, "pci_enable_device failed with %d", err);
 		return err;

-- 
Stefan Richter
-=====-==--- --== ----=
http://arcgraph.de/sr/

Re: [PATCH 1/3] firewire: fw-ohci: PPC PMac platform code

[Jarod Wilson]

Stefan Richter wrote:
> Copied from ohci1394.c.  This code is necessary to prevent machine check
> exceptions when reloading or resuming the driver.
> 
> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>

I was able to reproduce the system exception on resume with a 3rd-gen 
Titanium PowerBook G4 667, and this patch does let the system resume 
successfully now.

Not quite clear if there was possibly an updated version coming using 
pci_enable_device() instead of the pair of pmac_call_feature() calls, 
but either way, this is a definite must-have, at least for older ppc 
macs -- my Aluminum PowerBook G4/1.67 suspends and resumes without this 
patch just fine.

Signed-off-by: Jarod Wilson <jwilson@redhat.com>

-- 
Jarod Wilson
jwilson@redhat.com

Re: [PATCH 1/3] firewire: fw-ohci: PPC PMac platform code

[Benjamin Herrenschmidt]

On Sat, 2008-03-01 at 14:43 -0500, Jarod Wilson wrote:
> Stefan Richter wrote:
> > Copied from ohci1394.c.  This code is necessary to prevent machine check
> > exceptions when reloading or resuming the driver.
> > 
> > Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
> 
> I was able to reproduce the system exception on resume with a 3rd-gen 
> Titanium PowerBook G4 667, and this patch does let the system resume 
> successfully now.
> 
> Not quite clear if there was possibly an updated version coming using 
> pci_enable_device() instead of the pair of pmac_call_feature() calls, 
> but either way, this is a definite must-have, at least for older ppc 
> macs -- my Aluminum PowerBook G4/1.67 suspends and resumes without this 
> patch just fine.

Let's go with Stefan patch for now. I'll find out what's up with
pci_enable_device() later.

Ben.