Commit Graph

29 Commits

Author SHA1 Message Date
Clemens Ladisch e7014dada0 firewire: ohci: do not clear PHY interrupt status inadvertently
The interrupt status bits in PHY register 5 are cleared by writing a one
bit.  To avoid clearing them unadvertently, do not write them back when
they were read as set, but only when they have been explicitly requested
to be set.

Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-04-10 16:51:14 +02:00
Clemens Ladisch 4a96b4fcd6 firewire: ohci: add a function for reading PHY registers
Move the register reading code from ohci_update_phy_reg() into
a function which can be used separately.

Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-04-10 16:51:14 +02:00
Clemens Ladisch 8301b91ba0 firewire: ohci: add cycle timer quirk for the TI TSB12LV22
Among the many entries in the TSB12LV22 errata list (TI literature
number SLLS312) is the following:

  PCI Slave reads of the Cycle Timer register may occasionally get an
  incorrect value.
  Software may be able to validate value by reading the register
  multiple times rapidly and evaluating for a reasonable difference.

Signed-off-by: Clemens Ladisch <clemens@ladisch.de> (untested)
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> (added #define)
2010-03-17 23:24:42 +01:00
Stefan Richter 6fdb2ee243 firewire: ohci: extend initialization log message
by the number of available isochronous DMA contexts and active quirks
which is occasionally useful information.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-02-24 20:36:55 +01:00
Stefan Richter 4802f16d51 firewire: ohci: fix IR/IT context mask mixup
This bug was present in firewire-ohci since day one:  The number of
available isochronous receive DMA contexts was mixed up with that of
available isochronous transmit DMA contexts.

This is harmless on a few chips which offer the same number of contexts
in both directions, but most chips nowadays implement only the standard
minimum of 4 IR contexts, but 8 IT contexts.  If a user attempted to run
a lot of IR contexts at once, results with more than four were therefore
unpredictable.  I suppose the controller would simply refuse to start
DMA of any unimplemented context.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-02-24 20:36:55 +01:00
Stefan Richter 3e9cc2f3b7 firewire: ohci: add module parameter to activate quirk fixes
This way, we can advise users of precompiled kernel packages to test
existing quirk fixes on chips which have not been listed yet, without
them having to build a kernel from source.

Note, to use this feature on a machine with more than one controller,
steps like these are necessary:
# lspci | grep 1394
# ls /sys/bus/pci/drivers/firewire_ohci/
# echo -n "0000:03:02.0" > /sys/bus/pci/drivers/firewire_ohci/unbind
# echo 2 > /sys/module/firewire_ohci/parameters/quirks
# echo -n "0000:03:02.0" > /sys/bus/pci/drivers/firewire_ohci/bind
# echo 0 > /sys/module/firewire_ohci/parameters/quirks

The parameter can also be used to switch off quirk flags that were
hardwired into firewire-ohci's quirks table.  Simply specify a non-zero
quirks value but without any known flags, e.g. 0x100.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-02-24 20:36:55 +01:00
Stefan Richter 4a635593f4 firewire: ohci: use an ID table for quirks detection
We don't have a lot of quirks to take into account (especially since
dual-buffer IR is out of the picture), but still, a table-based approach
is more organized than a series of if () clauses.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-02-24 20:36:55 +01:00
Stefan Richter ecb1cf9c44 firewire: ohci: reorder struct fw_ohci for better cache efficiency
The config_rom struct members are only accessed during relatively
infrequent self-ID-complete interrupts and only if the local config ROM
was changed, while the ar_, at_, ir_, it_ members are used very
frequently during I/O.  Hence move the config_rom members further down.

More importantly, make the huge self_id_buffer member the last one; this
is only accessed in self-ID-complete interrupts.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-02-24 20:36:55 +01:00
Stefan Richter 6498ba04ae firewire: ohci: remove unused dualbuffer IR code
This code was no longer used since 2.6.33, "firewire: ohci: always use
packet-per-buffer mode for isochronous reception" commit 090699c0.  If
anybody needs this code in the future for special purposes, it can be
brought back in.  But it must not be re-enabled by default; drivers
(kernelspace or userspace drivers) should only get this mode if they
explicitly request it.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-02-24 20:36:55 +01:00
Stefan Richter 109d28152b Merge tag 'v2.6.33' for its firewire changes since last branch point
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-02-24 20:33:45 +01:00
Stefan Richter 168cf9af69 firewire: remove incomplete Bus_Time CSR support
The current implementation of Bus_Time read access was buggy since it
did not ensure that Bus_Time.second_count_hi and second_count_lo came
from the same 128 seconds period.

Reported-by: Håkan Johansson <f96hajo@chalmers.se>

Instead of a fix, remove Bus_Time register support altogether.  The spec
requires all cycle master capable nodes to implement this (all Linux
nodes are cycle master capable) while it also says that it "may" be
initialized by the bus manager or by the IRM standing in for a bus
manager.  (Neither Linux' firewire-core nor ieee1394 nodemgr implement
this.)

Since we cannot rely on Bus_Time having been initialized by a bus
manager, it is better to return an error instead of a nonsensical value
on a read request to Bus_Time.

Alternatively, we could fix the Bus_Time read integrity bug _and_
implement (a) cycle master's write support of the register as well as
(b) bus manager's Bus_Time initialization service, i.e. preservation of
the Bus_Time when the cycle master node of a bus changes.  However, that
would be quite some code for a feature that is unreliable to begin with
and very likely unused in practice.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-02-20 22:33:14 +01:00
Stefan Richter 4a9bde9b8a firewire: get_cycle_timer optimization and cleanup
ohci:  Break out of the retry loop if too many attempts were necessary.
This may theoretically happen if the chip is fatally defective or if the
get_cycle_timer ioctl was performed after a CardBus controller was
ejected.

Also micro-optimize the loop by re-using the last two register reads in
the next iteration, remove a questionable inline keyword, and shuffle a
comment around.

core:  ioctl_get_cycle_timer() is always called with interrupts on,
therefore local_irq_save() can be replaced by local_irq_disable().
Disabled local IRQs imply disabled preemption, hence preempt_disable()
can be removed.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-02-20 22:33:13 +01:00
Stefan Richter 1c1517efe1 firewire: ohci: enable cycle timer fix on ALi and NEC controllers
Discussed in "read_cycle_timer backwards for sub-cycle 0000, 0001",
http://thread.gmane.org/gmane.linux.kernel.firewire.devel/13704

Known bad controllers:
  ALi M5271, listed by lspci as M5253 [10b9:5253]
  NEC OrangeLink [1033:00cd] (rev 03)
  NEC uPD72874 [1033:00f2] (rev 01)
  VIA VT6306 [1106:3044] (rev 46)
  VIA VT6308P, listed by lspci as rev c0

Reported-by: Pieter Palmers <pieterp@joow.be>
Reported-by: Håkan Johansson <f96hajo@chalmers.se>
Reported-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-02-19 20:51:10 +01:00
Clemens Ladisch b677532b97 firewire: ohci: work around cycle timer bugs on VIA controllers
VIA controllers sometimes return an inconsistent value when reading the
isochronous cycle timer register.  To work around this, read the
register multiple times and add consistency checks.

Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Reported-by: Pieter Palmers <pieterp@joow.be>
Reported-by: Håkan Johansson <f96hajo@chalmers.se>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-02-19 20:51:10 +01:00
Clemens Ladisch 7f51a100bb firewire: ohci: retransmit isochronous transmit packets on cycle loss
In isochronous transmit DMA descriptors, link the skip address pointer
back to the descriptor itself.  When a cycle is lost, the controller
will send the packet in the next cycle, instead of terminating the
entire DMA program.

There are two reasons for this:

* This behaviour is compatible with the old IEEE1394 stack.  Old
  applications would not expect the DMA program to stop in this case.

* Since the OHCI driver does not report any uncompleted packets, the
  context would stop silently; clients would not have any chance to
  detect and handle this error without a watchdog timer.

Signed-off-by: Clemens Ladisch <clemens@ladisch.de>

Pieter Palmers notes:

"The reason I added this retry behavior to the old stack is because some
cards now and then fail to send a packet (e.g. the o2micro card in my
dell laptop).  I couldn't figure out why exactly this happens, my best
guess is that the card cannot fetch the payload data on time.  This
happens much more frequently when sending large packets, which leads me
to suspect that there are some contention issues with the DMA that fills
the transmit FIFO.

In the old stack it was a pretty critical issue as it resulted in a
freeze of the userspace application.

The omission of a packet doesn't necessarily have to be an issue.  E.g.
in IEC61883 streams the DBC field can be used to detect discontinuities
in the stream.  So as long as the other side doesn't bail when no
[packet] is present in a cycle, there is not really a problem.

I'm not convinced though that retrying is the proper solution, but it is
simple and effective for what it had to do.  And I think there are no
reasons not to do it this way.  Userspace can still detect this by
checking the cycle the descriptor was sent in."

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> (changelog, comment)
2010-02-14 15:10:41 +01:00
Stefan Richter 7a48143678 firewire: ohci: fix crashes with TSB43AB23 on 64bit systems
Unsurprisingly, Texas Instruments TSB43AB23 exhibits the same behaviour
as TSB43AB22/A in dual buffer IR DMA mode:  If descriptors are located
at physical addresses above the 31 bit address range (2 GB), the
controller will overwrite random memory.  With luck, this merely
prevents video reception.  With only a little less luck, the machine
crashes.

We use the same workaround here as with TSB43AB22/A:  Switch off the
dual buffer capability flag and use packet-per-buffer IR DMA instead.
Another possible workaround would be to limit the coherent DMA mask to
31 bits.

In Linux 2.6.33, this change serves effectively only as documentation
since dual buffer mode is not used for any controller anymore.  But
somebody might want to re-enable it in the future to make use of
features of dual buffer DMA that are not available in packet-per-buffer
mode.

In Linux 2.6.32 and older, this update is vital for anyone with this
controller, more than 2 GB RAM, a 64 bit kernel, and FireWire video or
audio applications.

We have at least four reports:
http://bugzilla.kernel.org/show_bug.cgi?id=13808
http://marc.info/?l=linux1394-user&m=126154279004083
https://bugzilla.redhat.com/show_bug.cgi?id=552142
http://marc.info/?l=linux1394-user&m=126432246128386

Reported-by: Paul Johnson
Reported-by: Ronneil Camara
Reported-by: G Zornetzer
Reported-by: Mark Thompson
Cc: stable@kernel.org
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-01-27 18:24:53 +01:00
Németh Márton a67483d2be firewire: make PCI device id constant
The id_table field of the struct pci_driver is constant in <linux/pci.h>
so it is worth to make pci_table also constant.  Found with Coccinelle.

Signed-off-by: Márton Németh <nm127@freemail.hu>
Cc: Julia Lawall <julia@diku.dk>
Cc: cocci@diku.dk
Signed-off-by: Stefan Richter stefanr@s5r6.in-berlin.de> (changelog)
2010-01-10 17:04:19 +01:00
Stefan Richter 090699c053 firewire: ohci: always use packet-per-buffer mode for isochronous reception
This is a minimal change meant for the short term:  Never set the
ohci->use_dualbuffer flag to true.

There are two reasons to do so:

  - Packet-per-buffer mode and dual-buffer mode do not behave the same
    under certain circumstances, notably if several packets are covered
    by a single fw_cdev_iso_packet descriptor.
    http://marc.info/?l=linux1394-devel&m=124965653718313
    Therefore the driver stack should not silently choose one or the
    other mode but should leave the choice to the high-level driver
    (regardless if kernel driver or userspace driver).  Or simply always
    only offer packet-per-buffer mode, since a considerable number of
    controllers, even current ones, does not offer dual-buffer support.

  - Even under circumstances where packet-per-buffer mode and
    dual-buffer mode behave exactly the same --- notably when used
    through libraw1394, libdc1394, as well as the current two kernel
    drivers which use isochronous reception (firewire-net and firedtv)
    --- we are still faced with the problem that several OHCI 1.1
    controllers have bugs in dual-buffer mode.  Although it looks like
    we have identified most of those buggy controllers by now, we
    cannot be quite sure about that.

So, use packet-per-buffer by default from now on.  This change should
be followed up by a more complete solution:  Either extend the
in-kernel API and the userspace ABI by a choice between the two IR modes
or remove all dual-buffer related code from firewire-ohci.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-12-29 19:58:17 +01:00
Linus Torvalds 5f1141eb35 Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6
* 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6:
  firewire: ohci: handle receive packets with a data length of zero
2009-12-11 15:22:27 -08:00
Jay Fenlason 8c0c0cc2d9 firewire: ohci: handle receive packets with a data length of zero
Queueing to receive an ISO packet with a payload length of zero
silently does nothing in dualbuffer mode, and crashes the kernel in
packet-per-buffer mode.  Return an error in dualbuffer mode, because
the DMA controller won't let us do what we want, and work correctly in
packet-per-buffer mode.

Signed-off-by: Jay Fenlason <fenlason@redhat.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Cc: stable@kernel.org
2009-12-11 21:43:45 +01:00
Linus Torvalds bb592cf474 Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6
* 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6:
  ieee1394: Use hweight32
  firewire: cdev: reduce stack usage by ioctl_dispatch
  firewire: ohci: 0 may be a valid DMA address
  firewire: core: WARN on wrong usage of core transaction functions
  firewire: core: optimize Topology Map creation
  firewire: core: clarify generate_config_rom usage
  firewire: optimize config ROM creation
  firewire: cdev: normalize variable names
  firewire: normalize style of queue_work wrappers
  firewire: cdev: fix memory leak in an error path
2009-12-08 08:13:10 -08:00
Jay Fenlason 31769cef2e firewire: ohci: pass correct iso xmit timestamps to core
Here is the final set of patches I used to get ffado to work with the
new firewire stack.  With these patches, I was able to start ardour
and record from and playback to my PreSonus Inspire1394 from a
(mostly) Fedora 12 system.

Signed-off-by: Jay Fenlason <fenlason@redhat.com>

Until now, firewire-ohci exposed only the transmit cycle of the last
transmitted packet at each isochronous transmit complete event.  This
made it impossible for FFADO (FireWire audio drivers in userspace) to
synchronize audio-out streams.  The fix is to store the timestamp of
each packet in the iso xmit event.  As a bonus, the transfer status is
stored too.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-11-21 00:56:47 +01:00
Jay Fenlason 5ed1f321a7 firewire: ohci: Make cycleMatch ISO transmission work
Calling the START_ISO ioctl with a nonnegative cycle paramater has
never worked.  Last night I got around to figuring out why.  Most of
this patch is a big comment explaining why we enable an interrupt
source then don't actually do anything when we get one.  As the
comment says, we should do more, but we don't have a way to tell
userspace what happened. . .

Signed-off-by: Jay Fenlason <fenlason@redhat.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> (edited comment)
2009-11-18 20:31:17 +01:00
Stefan Richter 19593ffdb6 firewire: ohci: 0 may be a valid DMA address
I was told that there are obscure architectures with non-coherent DMA
which may DMA-map to bus address 0.  We shall not use 0 as a magic
number of uninitialized bus address variables.

The packet->payload_length > 0 test cannot be used either (except in
at_context_queue_packet) because local requests are not DMA-mapped
regardless of payload_length.  Hence add a state flag to struct
fw_packet.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-10-31 11:40:51 +01:00
Stefan Richter 8e85973efc firewire: optimize config ROM creation
The config ROM image of the local node was created in CPU byte order,
then a temporary big endian copy was created to compute the CRC, and
finally the card driver created its own big endian copy.

We now generate it in big endian byte order in the first place to avoid
one byte order conversion and the temporary on-stack copy of the ROM
image (1000 bytes stack usage in process context).  Furthermore, two
1000 bytes memset()s are replaced by one 1000 bytes - ROM length sized
memset.

The trivial fw_memcpy_{from,to}_be32() helpers are now superfluous and
removed.  The newly added __compute_block_crc() function will be folded
into fw_compute_block_crc() in a subsequent change.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-10-14 23:10:48 +02:00
Stefan Richter 928ec5f148 firewire: ohci: fix Self ID Count register mask (safeguard against buffer overflow)
The selfIDSize field of Self ID Count is 9 bits wide, and we are only
interested in the high 8 bits.  Fix the mask accordingly.  The
previously too large mask didn't do damage though because the next few
bits in the register are reserved and therefore zero with presently
existing hardware.

Also, check for the maximum possible self ID count of 252 (according to
OHCI 1.1 clause 11.2 and IEEE 1394a-2000 clause 4.3.4.1, i.e. up to four
self IDs of up to 63 nodes, even though IEEE 1394 up to edition 2008
defines only up to three self IDs per node).  More than 252 self IDs
would only happen if the self ID receive DMA unit malfunctioned, which
would likely be caught by other self ID buffer checks.  However, check
it early to be sure.  More than 253 quadlets would overflow the Topology
Map CSR.

Reported-By: PaX Team
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-09-12 14:48:40 +02:00
Stefan Richter 4fe0badd58 firewire: ohci: fix Ricoh R5C832, video reception
In dual-buffer DMA mode, no video frames are ever received from R5C832
by libdc1394.  Fallback to packet-per-buffer DMA works reliably.
http://thread.gmane.org/gmane.linux.kernel.firewire.devel/13393/focus=13476

Reported-by: Jonathan Cameron <jic23@cam.ac.uk>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-09-05 15:59:34 +02:00
Stefan Richter fc383796a8 firewire: ohci: fix Agere FW643 and multiple cameras
An Agere FW643 OHCI 1.1 card works fine for video reception from one
camera but fails early if receiving from two cameras.  After a short
while, no IR IRQ events occur and the context control register does not
react anymore.  This happens regardless whether both IR DMA contexts are
dual-buffer or one is dual-buffer and the other packet-per-buffer.

This can be worked around by disabling dual buffer DMA mode entirely.
http://sourceforge.net/mailarchive/message.php?msg_name=4A7C0594.2020208%40gmail.com
(Reported by Samuel Audet.)

In another report (by Jonathan Cameron), an FW643 works OK with two
cameras in dual buffer mode.  Whether this is due to different chip
revisions or different usage patterns (different video formats) is not
yet clear.  However, as far as the current capabilities of
firewire-core's isochronous I/O interface are concerned, simply
switching off dual-buffer on non-working and working FW643s alike is not
a problem in practice.  We only need to revisit this issue if we are
going to enhance the interface, e.g. so that applications can explicitly
choose modes.

Reported-by: Samuel Audet <samuel.audet@gmail.com>
Reported-by: Jonathan Cameron <jic23@cam.ac.uk>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-09-05 15:59:34 +02:00
Stefan Richter e71d31da06 firewire: rename source files
The source files of firewire-core, firewire-ohci, firewire-sbp2, i.e.
 "drivers/firewire/fw-*.c"
are renamed to
 "drivers/firewire/core-*.c",
 "drivers/firewire/ohci.c",
 "drivers/firewire/sbp2.c".

The old fw- prefix was redundant to the directory name.  The new core-
prefix distinguishes the files according to which driver they belong to.

This change comes a little late, but still before further firewire
drivers are added as anticipated RSN.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-06-05 16:26:18 +02:00