Re: [PATCH] ehea: Fix: Remove adapter from adapter list in error path

2009-02-11 Thread Stefan Richter
Hannes Hering wrote:
 Remove adapter from adapter list before freeing data structure in error path.
...
 --- linux-2.6.29-rc4/drivers/net/ehea/ehea_main.c 2009-02-11 
 13:13:47.812542928 +0100
 +++ patched_kernel/drivers/net/ehea/ehea_main.c   2009-02-11 
 13:14:04.197540184 +0100
 @@ -3448,6 +3448,7 @@ out_kill_eq:
   ehea_destroy_eq(adapter-neq);
  
  out_free_ad:
 + list_del(adapter-list);
   kfree(adapter);
  
  out:

On a related note, ehea_set_mac_addr()/ehea_update_bcmc_registrations()
accesses adapter_list without serialization by ehea_fw_handles.lock and
thus apparently unsafely.

There may be other unsafe accesses of adapter_list; that's just the
first one which I spotted.
-- 
Stefan Richter
-=-==--= --=- -=-==
http://arcgraph.de/sr/
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


dma_alloc_coherent() on PPC32: physical addresses above 2G possible?

2008-07-20 Thread Stefan Richter

Hi all,

I have to implement a workaround for a PCI device which gets into 
trouble if descriptors are located at 32bit addresses, while 31bit 
addresses are fine.  I would like to avoid this workaround on machines 
on which dma_alloc_coherent() won't ever go at memory above 2 GB.


Is defined(CONFIG_PPC32) a safe test for this?  I'm under the impression 
that defined(CONFIG_X86_32) is safe.


Are there any other means to detect when the workaround can be omitted, 
at compile time or at runtime?


PS:  I don't want to set the DMA mask of this device to DMA_31BIT_MASK 
because that would be detrimental to other functions of the device. It's 
a TI TSB43AB22A FireWire controller.

--
Stefan Richter
-=-==--- -=== =-=--
http://arcgraph.de/sr/
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: dma_alloc_coherent() on PPC32: physical addresses above 2G possible?

2008-07-20 Thread Stefan Richter
Arjan van de Ven wrote:
 On Sun, 20 Jul 2008 20:36:23 +0200
 Stefan Richter [EMAIL PROTECTED] wrote:
 
 PS:  I don't want to set the DMA mask of this device to
 DMA_31BIT_MASK because that would be detrimental to other functions
 of the device. It's a TI TSB43AB22A FireWire controller.
 
 Hi,
 
 just want to mention that you can set the coherent mask separately from
 the generic mask... is that sufficient for your load?
 (you can even set it just for this allocation..)

Hmm.  Would that be done this way?
During probe:

if (chip_is_tsb43ab22a) {
if (dma_supported(dev, DMA_31BIT_MASK))
chip-needs_dma_mask_workaround = 1;
else
chip-needs_some_other_workaround = 1;
}

Later on:

if (dev-needs_dma_mask_workaround)
pci_set_consistent_dma_mask(pdev, DMA_31BIT_MASK);
allocate_something_special;
if (dev-needs_dma_mask_workaround)
pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);

Or is there a variant of dma_alloc_coherent() which directly accepts a
mask argument?
-- 
Stefan Richter
-=-==--- -=== =-=--
http://arcgraph.de/sr/
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: dma_alloc_coherent() on PPC32: physical addresses above 2G possible?

2008-07-20 Thread Stefan Richter
Arjan van de Ven wrote:
 On Sun, 20 Jul 2008 21:25:51 +0200
 Stefan Richter [EMAIL PROTECTED] wrote:
  if (dev-needs_dma_mask_workaround)
  pci_set_consistent_dma_mask(pdev, DMA_31BIT_MASK);
  allocate_something_special;
  if (dev-needs_dma_mask_workaround)
  pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
...
 something like this.
 But realistically, how many consistent/coherent allocations do you have?
 some ring buffers and other one time stuff surely... but not after that?

It's for DMA programs (i.e. lists of descriptors), not for the data DMA
buffers themselves.  FireWire controllers have several DMA contexts for
isochronous and asynchronous reception and transmission, and some
others.  Only context programs for isochronous reception need the
workaround.

We are dynamically appending new descriptors during runtime.  I.e. the
affected allocations happen during setup of the DMA context and
sometimes while the DMA context is active.  Particularly, in
drivers/firewire/fw-ohci.c:
ohci_allocate_iso_context
context_init
context_add_buffer  --
and
ohci_queue_iso
ohci_queue_iso_receive_dualbuffer
context_get_descriptors
context_add_buffer  --

The other unaffected context types use context_add_buffer too.
-- 
Stefan Richter
-=-==--- -=== =-=--
http://arcgraph.de/sr/
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 1/2] firewire: endianess fix

2008-03-05 Thread Stefan Richter
Gabriel Paubert wrote:
 I have a Pismo which I use on a virtually
 daily basis (and about to remove the last remnants of MacOS on it). 
 However I have disabled Firewire because it would not sleep and wake 
 up properly. 
...
 For now I have only tested the new stack with a 6 year old 1.8 disk
 and everything works, including suspend to RAM. The kernel is 2.6.25-rc4
 plus additional pull from linux1394-2.6.git: 2.6.25-rc4-00032-g8d36ba4.

That's great.  Thanks for testing.
-- 
Stefan Richter
-=-==--- --== --==-
http://arcgraph.de/sr/
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 1/2] firewire: endianess fix

2008-03-03 Thread Stefan Richter
Gabriel Paubert wrote:
 I have a Pismo which I use on a virtually
 daily basis (and about to remove the last remnants of MacOS on it). 
 However I have disabled Firewire because it would not sleep and wake 
 up properly. 
 
 I can test it on Wednesday with a 5GB fireflly disk from 2001.
 
 Please tell me which configuration options I need to set for
 Firewire (which stack, etc...).

Config options of the new stack:
FIREWIRE=m
FIREWIRE_OHCI=m
FIREWIRE_SBP2=m

Config options of the old stack:
IEEE1394=m
IEEE1394_OHCI1394=m
IEEE1394_SBP2=m
and if desired also the other drivers for raw userspace access,
isochronous I/O (alias video1394 even though it can also be used for
other purposes), DV I/O, and IPv4 over 1394.

The two SBP2 drivers also need SCSI and BLK_DEV_SD a.k.a. SCSI disk
support or/and BLK_DEV_SR a.k.a. SCSI CDROM support.

You can also set the options to Y but I find loadable and hence
unloadable modules more practical... 'cause I unload and reload them all
the time.  :-)

Regarding which of the two stacks to build:  If you are going to test
FireWire with a storage device only, then you can choose either stack.
Caveats:
  - You could build and install both stacks but should then blacklist
at least one of ohci1394 or firewire-ohci.  Better keep it simple
and install only one of the stacks at a time.
  - We still have a serious use-after-free bug in the new stack.  This
may lead to kernel panic if the kernel was build with (slab? or
page allocation?) debugging enabled.
Users of IP over 1394 and pro/semipro audio still need the old stack.
Users of video should stick with the stack which their distribution has
enabled because our support in the lowlevel libraries libraw1394 and
libdc1394 to switch between the stacks is not quite comfortable yet.

Suspend (to RAM) and resume worked for me [TM] when I last tested them
with each stack.  I tested i586/APM, x86-64/ACPI, and last weekend ppc32
on 1st generation PowerBook G4.  I haven't tested hibernate (to disk)
and restore yet.

For successful resume on the Pismo with the new stack, you will most
certainly need the brand new patches which add PPC_PMAC platform support
to firewire-ohci.  From what I saw with the PowerBook, you will also
need the UniNorth rev1 related patch.  I wasn't able to fully test that
patch though because the electrical interface of my PowerBook's onboard
FireWire is dead.  You can get the patches from kernel.org's
linux1394-2.6.git (master branch, close to Linus's latest linux-2.6.git)
or http://me.in-berlin.de/~s5r6/linux1394/updates/ (for a few kernel.org
kernels).

The old stack as found in any remotely recent 2.6 kernel should be OK
out of the box without patches... in theory.  I wouldn't be surprised of
until now unreported bugs or old reported but forgotten bugs though.
-- 
Stefan Richter
-=-==--- --== ---==
http://arcgraph.de/sr/
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


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

2008-03-01 Thread 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: 
TASK = dedae6d0[5376] 'modprobe' THREAD: d7c0
GPR00: e18e61c0 d7c01cf0 dedae6d0 defcf000  0385 c06c09f8 
1000
GPR08: ec0f  e18e6000 dfafd000 24004428 1001e354  

GPR16: 007a 007a  e18e9bf4 0124  e18e6000 
c0050280
GPR24: 0019 e18e9cc4 c0290630 dfb61420 df85b800 dfafdcf8  
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 817f 907f0008 814b04b4 813f0010 7c0a4a14 7c0004ac 7ce0052c
817f 812b04b4 7c0004ac 7d204c2c 0c09 4c00012c 3800 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/
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


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

2008-03-01 Thread Stefan Richter
Stephen Rothwell wrote:
 On Sat, 1 Mar 2008 02:50:44 +0100 (CET) Stefan Richter [EMAIL PROTECTED] 
 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/
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


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

2008-03-01 Thread 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 [EMAIL PROTECTED]
---
 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/

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


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

2008-03-01 Thread Stefan Richter
Clean up shared code and variable names.

Signed-off-by: Stefan Richter [EMAIL PROTECTED]
---
 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

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

2008-03-01 Thread 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 [EMAIL PROTECTED]
---
 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/

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


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

2008-03-01 Thread Stefan Richter
Clean up shared code and variable names.

Signed-off-by: Stefan Richter [EMAIL PROTECTED]
---
 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

Re: [PATCH 1/2] firewire: endianess fix

2008-03-01 Thread Stefan Richter
On 23 Feb, I wrote:
 This needs to be tested on different big endian PCs, if possible with
 the Apple Uninorth FireWire controller and other types of controllers.

I tested it myself now with VT6306 on PPC32.

 it should be triggered by replacing
   fw_high_memory_region
 by
   fw_private_region
 in drivers/firewire/fw-sbp2.c and testing with any SBP-2 device

This indeed demonstrates the fix.  Any IO to SBP-2 devices fails with
timeouts.  Just removing the posted write enable bit in fw-ohci wasn't
sufficient to catch it though.  Maybe the controller has write posting
enabled by default.

However, this endianess bug was low-profile because there are currently
no kernelspace or userspace drivers for the firewire stack which need to
respond in split transactions.
-- 
Stefan Richter
-=-==--- --== =
http://arcgraph.de/sr/

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 1/2] firewire: endianess fix

2008-02-29 Thread Stefan Richter
Paul Mackerras wrote:
 Jarod Wilson writes:
 I wonder how many people still actually 1) have a machine 
 with this controller, 2) are running Linux on it and 3) use firewire 
 devices with it. Both of you, please speak up, we're trying to help you! 
 (if only out of morbid curiosity to see this mythical goofy controller).
 
 I have a first-generation titanium powerbook that has this controller
 (assuming we're talking about vendor/device id = 0x106b / 0x18), and
 yes I run Linux (only) on it and use firewire disks. :)

I actually have a TiBook 400 myself, but so far without Linux, and its 
FireWire PHY is dead.  But I can use CardBus FireWire cards on it to do 
basic testing on a big endian PC, and I can test the selfID 
byte-swapping by the PHY-less onboard controller.

I now started a Fedora 8 live CD (self-test says the medium is 
corrupt... need to burn another one) and dmesg says:
firewire_ohci: Added fw-ohci device 0002:24:0e.0, OHCI version 1.0
firewire_ohci: recursive bus reset detected, discarding self ids
[...]

The second line looks like this is indeed one of those which needs the 
header byte-swap workaround which ohci1394 has but firewire-ohci hasn't yet.

On the weekend I'm going to attempt to put Linux on this PowerBook, at last.
-- 
Stefan Richter
-=-==--- --=- ===-=
http://arcgraph.de/sr/
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


[PATCH 0/3] firewire: PPC PMac updates

2008-02-29 Thread 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/

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


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

2008-02-29 Thread 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 [EMAIL PROTECTED]
---
 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/

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


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

2008-02-29 Thread 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 [EMAIL PROTECTED]
---

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/


___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


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

2008-02-29 Thread 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 [EMAIL PROTECTED]
---

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/

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


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

2008-02-29 Thread 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/

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 1/2] firewire: endianess fix

2008-02-27 Thread Stefan Richter
Jarod Wilson wrote:
 Works just fine with the Apple UniNorth controller in my powerbook in cursory 
 testing.

Could you remove the OHCI1394_HCControl_postedWriteEnable flag from 
fw-ohci.c::ohci_enable() and test without and with the endianess patch?
-- 
Stefan Richter
-=-==--- --=- ==-==
http://arcgraph.de/sr/
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


[PATCH 2/2] firewire: endianess annotations

2008-02-23 Thread Stefan Richter
Kills warnings from 'make C=1 CHECKFLAGS=-D__CHECK_ENDIAN__ modules':

drivers/firewire/fw-transaction.c:771:10: warning: incorrect type in assignment 
(different base types)
drivers/firewire/fw-transaction.c:771:10:expected unsigned int [unsigned] 
[usertype] noident
drivers/firewire/fw-transaction.c:771:10:got restricted unsigned int 
[usertype] noident
drivers/firewire/fw-transaction.h:93:10: warning: incorrect type in assignment 
(different base types)
drivers/firewire/fw-transaction.h:93:10:expected unsigned int [unsigned] 
[usertype] noident
drivers/firewire/fw-transaction.h:93:10:got restricted unsigned int 
[usertype] noident
drivers/firewire/fw-ohci.c:1490:8: warning: restricted degrades to integer
drivers/firewire/fw-ohci.c:1490:35: warning: restricted degrades to integer
drivers/firewire/fw-ohci.c:1516:5: warning: cast to restricted type

Signed-off-by: Stefan Richter [EMAIL PROTECTED]
Cc: [EMAIL PROTECTED]
Cc: linuxppc-dev@ozlabs.org
---
 drivers/firewire/fw-ohci.c|4 ++--
 drivers/firewire/fw-transaction.c |2 +-
 drivers/firewire/fw-transaction.h |6 +++---
 3 files changed, 6 insertions(+), 6 deletions(-)

Index: linux/drivers/firewire/fw-ohci.c
===
--- linux.orig/drivers/firewire/fw-ohci.c
+++ linux/drivers/firewire/fw-ohci.c
@@ -1487,7 +1487,7 @@ static int handle_ir_dualbuffer_packet(s
void *p, *end;
int i;
 
-   if (db-first_res_count  0  db-second_res_count  0) {
+   if (db-first_res_count != 0  db-second_res_count != 0) {
if (ctx-excess_bytes = le16_to_cpu(db-second_req_count)) {
/* This descriptor isn't done yet, stop iteration. */
return 0;
@@ -1513,7 +1513,7 @@ static int handle_ir_dualbuffer_packet(s
memcpy(ctx-header + i + 4, p + 8, ctx-base.header_size - 4);
i += ctx-base.header_size;
ctx-excess_bytes +=
-   (le32_to_cpu(*(u32 *)(p + 4))  16)  0x;
+   (le32_to_cpu(*(__le32 *)(p + 4))  16)  0x;
p += ctx-base.header_size + 4;
}
ctx-header_length = i;
Index: linux/drivers/firewire/fw-transaction.c
===
--- linux.orig/drivers/firewire/fw-transaction.c
+++ linux/drivers/firewire/fw-transaction.c
@@ -751,7 +751,7 @@ handle_topology_map(struct fw_card *card
void *payload, size_t length, void *callback_data)
 {
int i, start, end;
-   u32 *map;
+   __be32 *map;
 
if (!TCODE_IS_READ_REQUEST(tcode)) {
fw_send_response(card, request, RCODE_TYPE_ERROR);
Index: linux/drivers/firewire/fw-transaction.h
===
--- linux.orig/drivers/firewire/fw-transaction.h
+++ linux/drivers/firewire/fw-transaction.h
@@ -85,12 +85,12 @@
 static inline void
 fw_memcpy_from_be32(void *_dst, void *_src, size_t size)
 {
-   u32 *dst = _dst;
-   u32 *src = _src;
+   u32*dst = _dst;
+   __be32 *src = _src;
int i;
 
for (i = 0; i  size / 4; i++)
-   dst[i] = cpu_to_be32(src[i]);
+   dst[i] = be32_to_cpu(src[i]);
 }
 
 static inline void

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

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


[PATCH 1/2] firewire: endianess fix

2008-02-23 Thread Stefan Richter
The generation of incoming requests was filled in in wrong byte order on
machines with big endian CPU.

Signed-off-by: Stefan Richter [EMAIL PROTECTED]
Cc: [EMAIL PROTECTED]
Cc: linuxppc-dev@ozlabs.org
---

This patch is a shot in the dark, based on a warning when building with
C=1 CHECKFLAGS=-D__CHECK_ENDIAN__.  Is it really a fix, or was the
previous code accidentally correct?

This needs to be tested on different big endian PCs, if possible with
the Apple Uninorth FireWire controller and other types of controllers.
One test which involves ohci-request_generation is simply with an SBP-2
device (harddisk, CD-ROM...).  Does SBP-2 login etc. work?

If possible, also test whether the device remains accessible after
forcing a bus reset, e.g. by echo br short  firecontrol.  You need
the easy to build utility firecontrol and a libraw1394 with juju
backend.  See wiki.linux1394.org for directions.


 drivers/firewire/fw-ohci.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux/drivers/firewire/fw-ohci.c
===
--- linux.orig/drivers/firewire/fw-ohci.c
+++ linux/drivers/firewire/fw-ohci.c
@@ -375,7 +375,7 @@ static __le32 *handle_ar_packet(struct a
 */
 
if (p.ack + 16 == 0x09)
-   ohci-request_generation = (buffer[2]  16)  0xff;
+   ohci-request_generation = (p.header[2]  16)  0xff;
else if (ctx == ohci-ar_request_ctx)
fw_core_handle_request(ohci-card, p);
else

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

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 1/2] firewire: endianess fix

2008-02-23 Thread Stefan Richter
I wrote:
 This needs to be tested on different big endian PCs, if possible with
 the Apple Uninorth FireWire controller and other types of controllers.
 One test which involves ohci-request_generation is simply with an SBP-2
 device (harddisk, CD-ROM...).  Does SBP-2 login etc. work?

Hmm, no, tests with SBP-2 devices won't trigger that problem at all.
All of the requests from the device will be:
   - read requests to the host's config ROM, handled by the controller's
 physical response unit instead of the driver stack's response
 handlers,
   - read requests to ORBs and read and write requests to SCSI data
 buffers which are DMA-mapped at bus addresses below 4G, hence be
 handled by the physical response unit as well,
   - write requests to firewire-sbp2's status FIFO which is mapped into
 a FireWire address range for which PCI write posting is enabled,
 hence no response subaction will be generated (unified transaction)
 and therefore ohci-request_generation remain unused.

Alas I have no idea how to create a simple test setup which really 
triggers the questionable code.  Or wait, it should be triggered by 
replacing
fw_high_memory_region
by
fw_private_region
in drivers/firewire/fw-sbp2.c and testing with any SBP-2 device which is 
_not_ based on the PL3507 bridge chip.  This moves the status FIFO into 
an area outside of PCI write posting and forces the driver stack to 
generate response packets.  (Some PL-3507 only accept unified 
transactions, hence this test needs to be performed with other bridge 
chips.)
-- 
Stefan Richter
-=-==--- --=- =-===
http://arcgraph.de/sr/
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: Sleep problems with kernels = 2.6.21 on powerpc

2007-09-05 Thread Stefan Richter
Randy Dunlap wrote:
 On Wed, 5 Sep 2007 10:07:54 -0700 Andrew Morton wrote:
 So here is the output from dmesg that suggested to me that firewire 
 might be a problem:
 Straightforward regression, two reporters, nothing happening.
 
 (material for ksummit discussion, e.g.)

It's a simple thing in this incident:  I don't read all posts on LKML.
But I typically catch everything where I am in the CC list thanks to the
wonders of sieve.  Andrew, thanks for putting me in the CCs.
-- 
Stefan Richter
-=-=-=== =--= --=-=
http://arcgraph.de/sr/
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: Sleep problems with kernels = 2.6.21 on powerpc

2007-09-05 Thread Stefan Richter
Andrew Morton wrote:
 On 30 Aug 2007 22:42:46 +0200 Tim Teulings [EMAIL PROTECTED] wrote:
 Hello!

 I don't have traces at hand and due to lack of time cannot reproduce it 
 up to tomorrow. However this hint may speed up your analysis!
 Sorry for the delay, but my desktop PC had an urgent hard disk problem I 
 had to fix ASASP.

 So here is the output from dmesg that suggested to me that firewire 
 might be a problem:
 
 Straightforward regression, two reporters, nothing happening.

Thanks for CCing me.  I'm also adding the CC of Kristian, author and
other maintainer of the code in question.

 usb_endpoint usbdev2.2_ep81: PM: suspend 0-2, parent 2-1:1.0 already 2
 usb_device usbdev1.1: PM: suspend 0-2, parent usb1 already 2
 usb_endpoint usbdev1.1_ep81: PM: suspend 0-2, parent 1-0:1.0 already 2
 hub 1-0:1.0: PM: suspend 2-2, parent usb1 already 2
 usb_endpoint usbdev1.1_ep00: PM: suspend 0-2, parent usb1 already 2
 eth2: Airport entering sleep mode
 eth0: suspending, WakeOnLan disabled
 pci_set_power_state(): 0002:20:0e.0: state=3, current state=5
 firewire_ohci: pci_set_power_state failed with -223pci_device_suspend(): 
 pci_suspend+0x0/0x9c [firewire_ohci]() returns -22
 suspend_device(): pci_device_suspend+0x0/0x98() returns -22
 Could not suspend device 0002:20:0e.0: error -22
 eth0: resuming
 PHY ID: 4061e4, addr: 0
 eth2: Airport waking up
 eth2: New link status: Connected (0001)
 hda: Enabling Ultra DMA 2
 eth0: Link is up at 100 Mbps, full-duplex.
 eth0: Pause is disabled
 ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
 hdb: Enabling Ultra DMA 2
 usb_endpoint usbdev1.1_ep00: PM: resume from 0, parent usb1 still 2
 usb_endpoint usbdev1.1_ep81: PM: resume from 0, parent 1-0:1.0 still 2
 usb_device usbdev1.1: PM: resume from 0, parent usb1 still 2
 Driver sleep failed
 Sleep rejected by devices
 adb: starting probe task...
 adb devices: [2]: 2 c4 [3]: 3 1 [7]: 7 1f
 ADB keyboard at 2, handler 1
 ADB mouse at 3, handler set to 4 (trackpad)
 adb: finished probe task...
 usb_device usbdev1.1: PM: suspend 0-2, parent usb1 already 2
 usb_endpoint usbdev1.1_ep81: PM: suspend 0-2, parent 1-0:1.0 already 2
 hub 1-0:1.0: PM: suspend 2-2, parent usb1 already 2
 usb_endpoint usbdev1.1_ep00: PM: suspend 0-2, parent usb1 already 2
 eth2: Airport entering sleep mode
 eth0: suspending, WakeOnLan disabled
 Trying to free already-free IRQ 40
 pci_set_power_state(): 0002:20:0e.0: state=3, current state=5
 firewire_ohci: pci_set_power_state failed with -223pci_device_suspend(): 
 pci_suspend+0x0/0x9c [firewire_ohci]() returns -22
 
 I grepped the whole tree for firewire_ohci and came up blank.  What is it?

drivers/firewire/fw-ohci.c - fw-ohci.o - firewire-ohci.o -
firewire-ohci.ko

 But yes, a failed pci_set_power_state() will hurt.  Perhaps this is
 a result of some recently-added return-value checking fix but as I
 cannot find the dang code I cannot tell.

The old ohci1394.c used to ignore pci_set_power_state's return value.
In the pre 2.6.19-rc1 commit ea6104c22468239083857fa07425c312b1ecb424, I
added a fail-on-error.  This was toned down to a printk-on-err by pre
2.6.19-rc4 commit 346f5c7ee7fa4ebee0e4c96415a7e59716bfa1d0.

This was because of Benjamin Herrenschmidt's regression report:
http://lkml.org/lkml/2006/10/24/13

So, we went into the same trap again with fw-ohci.  I should have
noticed how fw-ohci treats pci_set_power_state when I signed off the
commit which added the suspend/resume methods to fw-ohci --- but somehow
I didn't.

 suspend_device(): pci_device_suspend+0x0/0x98() returns -22
 Could not suspend device 0002:20:0e.0: error -22
 eth0: resuming
 PHY ID: 4061e4, addr: 0
 eth2: Airport waking up
 eth2: New link status: Connected (0001)
 hda: Enabling Ultra DMA 2
 hdb: Enabling Ultra DMA 2
 eth0: Link is up at 100 Mbps, full-duplex.
 eth0: Pause is disabled
 The problem wa sinitiated by closing the lid. The iBook then seems to 
 permanetly try to go to sleep (I can hear the cd-rom drive get 
 periodically initialized). So above contains more than one iteration.

 The kernel is:
 Linux kismet 2.6.22-1-powerpc #1 Sun Jul 29 13:58:06 CEST 2007 ppc GNU/Linux
 The relveant debian package:
 linux-image-2.6.22-1-powerpc_2.6.22-3_powerpc.deb
 I'm running a mixture of debian testing/unstable.

 The firewire and the USB connector were unused, the network connector 
 was used.

 If there are questions regarding other packages, or somebody wants me to 
 test a fix (I would prever a debian package), don't hesitate - I would 
 like to get the bug fixed :-)


A trivial post -rc1 compatible fix is coming in a minute.
-- 
Stefan Richter
-=-=-=== =--= --=-=
http://arcgraph.de/sr/
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


[PATCH] Re: Sleep problems with kernels = 2.6.21 on powerpc

2007-09-05 Thread Stefan Richter
From: Stefan Richter [EMAIL PROTECTED]
Subject: firewire: fw-ohci: ignore failure of pci_set_power_state (fix suspend 
regression)

Fixes Sleep problems with kernels = 2.6.21 on powerpc,
http://lkml.org/lkml/2007/8/25/155.

Like it was suggested earlier in http://lkml.org/lkml/2006/10/24/13,
we do *not* fail .suspend anymore if pci_set_power_state failed.

Signed-off-by: Stefan Richter [EMAIL PROTECTED]
---
 drivers/firewire/fw-ohci.c |5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

Index: linux-2.6.23-rc4/drivers/firewire/fw-ohci.c
===
--- linux-2.6.23-rc4.orig/drivers/firewire/fw-ohci.c
+++ linux-2.6.23-rc4/drivers/firewire/fw-ohci.c
@@ -1946,8 +1946,11 @@ static int pci_suspend(struct pci_dev *p
}
err = pci_set_power_state(pdev, pci_choose_state(pdev, state));
if (err) {
+   /*
+* Some Apple onboard controllers are known to fail here.
+* Ignore it and let the machine proceed to suspend.
+*/
fw_error(pci_set_power_state failed\n);
-   return err;
}
 
return 0;

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

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] Re: Sleep problems with kernels = 2.6.21 on powerpc

2007-09-05 Thread Stefan Richter
Rogério Brito wrote on 2007-08-27:
 If things progress well, I will incrementally include features on the
 kernel that I need (I left out, for instance, the Firewire subsystem, so
 that compilation wouldn't take more than an hour here, despite the fact
 that I do need Firewire support on the kernel) and see the point where
 things are not normal.

The trivial pci_set_power_state issue aside, resume is currently
defective with the new firewire-ohci driver:
  - The version in Linus' tree doesn't restore a certain detail of
IEEE 1394 state during resume, hence some protocols like SBP-2 don't
work after resume unless you unload and reload the drivers.
  - The version in -mm restores everything but panics soon after resume
on an APM notebook in combination with some SBP-2 targets.  I have
yet to try netconsole or so to gather more information.
-- 
Stefan Richter
-=-=-=== =--= --=-=
http://arcgraph.de/sr/
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


firewire in prebuilt kernel packages (was Re: Sleep problems with kernels = 2.6.21 on powerpc)

2007-09-05 Thread Stefan Richter
 On 30 Aug 2007 22:42:46 +0200 Tim Teulings [EMAIL PROTECTED] wrote:
 The kernel is:
 Linux kismet 2.6.22-1-powerpc #1 Sun Jul 29 13:58:06 CEST 2007 ppc 
 GNU/Linux
 The relveant debian package:
 linux-image-2.6.22-1-powerpc_2.6.22-3_powerpc.deb
 I'm running a mixture of debian testing/unstable.

At the moment, distributors should not provide the experimental
firewire-* drivers as the only or primary FireWire drivers, unless they
know exactly what the gotchas are.  As a hint, CONFIG_FIREWIRE currently
depends on EXPERIMENTAL, CONFIG_IEEE1394 does not.  Some basic
information can be found at http://wiki.linux1394.org/JujuMigration.
-- 
Stefan Richter
-=-=-=== =--= --=-=
http://arcgraph.de/sr/
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: Sleep problems with kernels = 2.6.21 on powerpc

2007-09-05 Thread Stefan Richter
On  5 Sep, Andrew Morton wrote:
 On Wed, 05 Sep 2007 19:47:42 +0200 Stefan Richter [EMAIL PROTECTED] wrote:
  Trying to free already-free IRQ 40
  pci_set_power_state(): 0002:20:0e.0: state=3, current state=5
  firewire_ohci: pci_set_power_state failed with 
  -223pci_device_suspend(): pci_suspend+0x0/0x9c [firewire_ohci]() 
  returns -22
...
 The old ohci1394.c used to ignore pci_set_power_state's return value.
 In the pre 2.6.19-rc1 commit ea6104c22468239083857fa07425c312b1ecb424, I
 added a fail-on-error.  This was toned down to a printk-on-err by pre
 2.6.19-rc4 commit 346f5c7ee7fa4ebee0e4c96415a7e59716bfa1d0.
 
 OK.
 
 This was because of Benjamin Herrenschmidt's regression report:
 http://lkml.org/lkml/2006/10/24/13
 
 It's not clear _why_ pci_set_power_state() is failing.

The only -22 error path in pci_set_power_state is this:

/* Validate current state:
 * Can enter D0 from any state, but if we can only go deeper 
 * to sleep if we're already in a low power state
 */
if (state != PCI_D0  dev-current_state  state) {
printk(KERN_ERR %s(): %s: state=%d, current state=%d\n,
__FUNCTION__, pci_name(dev), state, dev-current_state);
return -EINVAL;
} [...else...]

(There seems to be one if too many in the comment.)


dev-current_state was PCI_UNKNOWN, and this is not properly handled by
pci_set_power_state.  Some checks later, there is

switch (dev-current_state) {
case PCI_D0:
case PCI_D1:
case PCI_D2:
pmcsr = ~PCI_PM_CTRL_STATE_MASK;
pmcsr |= state;
break;
case PCI_UNKNOWN: /* Boot-up */
if ((pmcsr  PCI_PM_CTRL_STATE_MASK) == PCI_D3hot
  !(pmcsr  PCI_PM_CTRL_NO_SOFT_RESET))
need_restore = 1;
/* Fall-through: force to D0 */
default:
pmcsr = 0;
break;
}

But the PCI_UNKNOWN branch will never be reached because of the if ()
clause quoted above.

Also, this FireWire controller here surely had left the boot-up phase
already long ago when the reporter tried to suspend the machine.
-- 
Stefan Richter
-=-=-=== =--= --=-=
http://arcgraph.de/sr/


___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 2.6.22.y] ieee1394: revert sbp2: enforce 32bit DMA mapping

2007-08-06 Thread Stefan Richter
Benjamin Herrenschmidt wrote:
 Oh and, don't do the set_dma_mask() in sbp2, it has nothing to do there.
 It should be in the ohci1394 driver.

That's not quite right.  OHCI-1394 implementations can go beyond 4GB bus
address space.  (Although I don't know if there are such implementations
available.  At least there are two implementations which can set the
so-called Physical Range bigger than 4GB.)

Sbp2 however requires that everything which it DMA-maps resides in the
Physical Range of the controller.  This way the CPU is not involved in
most of the data transfers.  The OHCI-1394 controller acts as bus bridge
between IEEE 1394 bus and local bus, with a 1:1 mapping of IEEE 1394 bus
addresses to and from local bus addresses --- but not in the whole 48
bits white IEEE 1394 bus address range, only in the
implementation-dependent Physical Range.  The minimum Physical Range
that all OHCI-1394 implementations guarantee is 4GB.  I could actually
have set a bigger mask in sbp2 when the controller supports a
programmable bigger range.

So that's the story why that dma_set_mask went into sbp2:  Sbp2 wants
mappings in a _subset_ of the OHCI-1394 controllers DMA range.

Anyway.  For now I will simply go with what 2.6.23-rc has and what
2.6.21 had:  No dma_set_mask anywhere in the 1394 subsystem.  We can
revisit this whenever an actual need arises.
-- 
Stefan Richter
-=-=-=== =--- --==-
http://arcgraph.de/sr/
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 2.6.22.y] ieee1394: revert sbp2: enforce 32bit DMA mapping

2007-08-06 Thread Stefan Richter
Robert Hancock wrote:
 Stefan Richter wrote:
 So that's the story why that dma_set_mask went into sbp2:  Sbp2 wants
 mappings in a _subset_ of the OHCI-1394 controllers DMA range.

 Anyway.  For now I will simply go with what 2.6.23-rc has and what
 2.6.21 had:  No dma_set_mask anywhere in the 1394 subsystem.  We can
 revisit this whenever an actual need arises.
 
 Not sure this is a very good idea. This seems rather likely to fail on
 x86_64 machines with 4GB of RAM for example..

I'll deal with it when a bug report comes in.

  1.) The ieee1394 subsystem is known to work on x86_64 with more than 4
GB RAM, so I gather that architecture code already sets a proper DMA
mask for all those 32bit PCI OHCI-1394 implementations out there.

  2.) I haven't heard of an OHCI-1394 chip whose overall DMA range was
bigger than the Physical Range.  There are only two chips, from ALi and
from Fujitsu, whose Physical Range is curiously bigger than the actual
DMA range.
-- 
Stefan Richter
-=-=-=== =--- --===
http://arcgraph.de/sr/
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 2.6.22.y] ieee1394: revert sbp2: enforce 32bit DMA mapping

2007-08-06 Thread Stefan Richter
Robert Hancock wrote:
 I would agree, though, that sbp2 isn't really the place for setting
 this, since the DMA mask is presently a property of the device, not of
 the user..

The mask that sbp2 set was because sbp2 has (in theory, not yet in
practice) a _narrower requirement on address ranges than the chip_ ---
hence it has (in theory) a narrower requirement on DMA mappings than the
ohci1394 driver has.

That's because sbp2 uses the controller in a special mode, as a bus
bridge.  It is the only user of that feature among the higher-level IEEE
1394 drivers.  No other IEEE 1394 application-layer software has this
requirement.

(Well, debugging and forensic tools rely on that mode too, notably
BenH's firescope, but this software runs remote, hence it's a different
beast from sbp2.)
-- 
Stefan Richter
-=-=-=== =--- --===
http://arcgraph.de/sr/
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 2.6.22.y] ieee1394: revert sbp2: enforce 32bit DMA mapping

2007-08-04 Thread Stefan Richter
(Adding Cc: linuxppc-dev, olh)

Robert Hancock wrote:
 Stefan Richter wrote:
 Date: Wed, 1 Aug 2007 20:30:36 +0200 (CEST)
 From: Stefan Richter [EMAIL PROTECTED]
 Subject: ieee1394: revert sbp2: enforce 32bit DMA mapping

 Revert commit 0555659d63c285ceb7ead3115532e1b71b0f27a7 from 2.6.22-rc1.
 The dma_set_mask call somehow failed on a PowerMac G5, PPC64:
 http://lkml.org/lkml/2007/8/1/344

 Should there ever occur a DMA mapping beyond the physical DMA range, a
 proper SBP-2 firmware will report transport errors.  So let's leave it
 at that.
 
 Isn't this a rather poor workaround? All this means is that if we fail
 to set a 32-bit DMA mask, we're likely to blow up at runtime instead of
 at initialization time, when we get a DMA mapping over 4GB.

I generally agree with you.  But since I actually never heard of
problems that could directly be related to sbp2's DMA areas exceeding
the OHCI-1394 physical DMA range (4GB in most OHCI-1394
implementations), I consider this simple reversion good enough for post
2.6.23-rc1 and especially for 2.6.22.y.

My original commit 0555659.. was a violation of If it ain't (known)
broken, don't fix it.

 If setting 32-bit DMA mask fails on ppc64, that sounds like a problem
 with the DMA implementation on that architecture. There are enough cards
 out there that only support 32-bit DMA that this really needs to work..

Yes, could the PPC folks please have a look at it?  Thanks.

 Signed-off-by: Stefan Richter [EMAIL PROTECTED]
 Tested-by: Olaf Hering [EMAIL PROTECTED]
 ---
 Same as commit a9c2f18800753c82c45fc13b27bdc148849bdbb2.

  drivers/ieee1394/sbp2.c |5 -
  1 file changed, 5 deletions(-)

 Index: linux-2.6.22/drivers/ieee1394/sbp2.c
 ===
 --- linux-2.6.22.orig/drivers/ieee1394/sbp2.c
 +++ linux-2.6.22/drivers/ieee1394/sbp2.c
 @@ -774,11 +774,6 @@ static struct sbp2_lu *sbp2_alloc_device
  SBP2_ERR(failed to register lower 4GB address range);
  goto failed_alloc;
  }
 -#else
 -if (dma_set_mask(hi-host-device.parent, DMA_32BIT_MASK)) {
 -SBP2_ERR(failed to set 4GB DMA mask);
 -goto failed_alloc;
 -}
  #endif
  }
  

-- 
Stefan Richter
-=-=-=== =--- --=--
http://arcgraph.de/sr/
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev