Change the log line prefix from "firewire_net: " to "net firewire0: "
etc. for the case that several RFC 2734 interfaces are being used in the
same machine.
Note, the netdev_printk API is not very useful to firewire-net.
netdev_notice(net, "abc\n") would result in irritating messages like
"firewire_ohci 0000:0a:00.0: firewire0: abc". Nor would a dev_printk on
the fw_unit.device to which firewire-net is being bound be useful,
because there are generally multiple ones of those per interface (from
all RFC 2734 peers on the bus, the local node being only one of them).
In the initialization message of each interface, log the PCI device
name of the card which is parent of the netdevice instead of the GUID
of the peer which was semi-randomly used to establish the netdevice.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
On second thought, there is little reason to have driver name differ
from module name. Therefore, change
/sys/bus/firewire/drivers/net
/sys/bus/firewire/devices/fw0.0/driver -> [...]/net
/sys/module/firewire_net/drivers/firewire:net
to
/sys/bus/firewire/drivers/firewire_net
/sys/bus/firewire/devices/fw0.0/driver -> [...]/firewire_net
/sys/module/firewire_net/drivers/firewire:firewire_net
It is redundant but consistent with firewire-sbp2's recently changed
driver name.
I don't see this anywhere used, so it should not matter either way.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Commit eba9ebaaa2 "firewire: sbp2: use dev_printk API" changed
messages from e.g.
firewire_sbp2: fw3.0: logged in to LUN 0000 (0 retries)
to
sbp2 fw3.0: logged in to LUN 0000 (0 retries)
because the driver calls itself as "sbp2" when registering with driver
core and with SCSI core. This is of course confusing, so switch to the
name "firewire_sbp2" for driver core in order to match what lsmod and
/sys/module/ show. So we are back to
firewire_sbp2 fw3.0: logged in to LUN 0000 (0 retries)
in the kernel log.
This also changes
/sys/bus/firewire/drivers/sbp2
/sys/bus/firewire/devices/fw3.0/driver -> [...]/sbp2
/sys/module/firewire_sbp2/drivers/firewire:sbp2
to
/sys/bus/firewire/drivers/firewire_sbp2
/sys/bus/firewire/devices/fw3.0/driver -> [...]/firewire_sbp2
/sys/module/firewire_sbp2/drivers/firewire:firewire_sbp2
but "cat /sys/class/scsi_host/host27/proc_name" stays "sbp2" just in
case that proc_name is used by any userland.
The transport detection in lsscsi is not affected. (Tested with lsscsi
version 0.25.) Udev's /dev/disk/by-id and by-path symlinks are not
affected either.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
The PCIe device
FireWire (IEEE 1394) [0c00]: Ricoh Co Ltd FireWire Host Controller
[1180:e832] (prog-if 10 [OHCI])
is unable to access attached FireWire devices when MSI is enabled but
works if MSI is disabled.
http://www.mail-archive.com/alsa-user@lists.sourceforge.net/msg28251.html
Hence add the "disable MSI" quirks flag for this device, or in fact for
safety and simplicity for all current (R5U230, R5U231, R5U240) and
future Ricoh PCIe 1394 controllers.
Reported-by: Stefan Thomas <kontrapunktstefan@googlemail.com>
Cc: 2.6.36+ <stable@vger.kernel.org>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
The Audigy's SB1394 controller is actually from Texas Instruments
and has the same bus reset packet generation bug, so it needs the
same quirk entry.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Cc: 2.6.36+ <stable@vger.kernel.org>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
fw_unit device drivers invariably need to talk to the fw_unit's parent
(an fw_device) and grandparent (an fw_card). firewire-core already
maintains an fw_card reference for the entire lifetime of an fw_device.
Likewise, let firewire-core maintain an fw_device reference for the
entire lifetime of an fw_unit so that fw_unit drivers don't have to.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
module_param(bool) used to counter-intuitively take an int. In
fddd5201 (mid-2009) we allowed bool or int/unsigned int using a messy
trick.
It's time to remove the int/unsigned int option. For this version
it'll simply give a warning, but it'll break next kernel version.
Acked-by: Mauro Carvalho Chehab <mchehab@redhat.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
* 'modsplit-Oct31_2011' of git://git.kernel.org/pub/scm/linux/kernel/git/paulg/linux: (230 commits)
Revert "tracing: Include module.h in define_trace.h"
irq: don't put module.h into irq.h for tracking irqgen modules.
bluetooth: macroize two small inlines to avoid module.h
ip_vs.h: fix implicit use of module_get/module_put from module.h
nf_conntrack.h: fix up fallout from implicit moduleparam.h presence
include: replace linux/module.h with "struct module" wherever possible
include: convert various register fcns to macros to avoid include chaining
crypto.h: remove unused crypto_tfm_alg_modname() inline
uwb.h: fix implicit use of asm/page.h for PAGE_SIZE
pm_runtime.h: explicitly requires notifier.h
linux/dmaengine.h: fix implicit use of bitmap.h and asm/page.h
miscdevice.h: fix up implicit use of lists and types
stop_machine.h: fix implicit use of smp.h for smp_processor_id
of: fix implicit use of errno.h in include/linux/of.h
of_platform.h: delete needless include <linux/module.h>
acpi: remove module.h include from platform/aclinux.h
miscdevice.h: delete unnecessary inclusion of module.h
device_cgroup.h: delete needless include <linux/module.h>
net: sch_generic remove redundant use of <linux/module.h>
net: inet_timewait_sock doesnt need <linux/module.h>
...
Fix up trivial conflicts (other header files, and removal of the ab3550 mfd driver) in
- drivers/media/dvb/frontends/dibx000_common.c
- drivers/media/video/{mt9m111.c,ov6650.c}
- drivers/mfd/ab3550-core.c
- include/linux/dmaengine.h
Ensure that the EXPORT_SYMBOL macros are present for when we clean up
the "module.h" is everywhere situation, to prevent build failures.
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Add the dma_sync_single_* calls necessary to ensure proper cache
synchronization for isochronous data buffers on non-coherent
architectures.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
If a device's firmware initiates a bus reset by setting the IBR bit in
PHY register 1 without resetting the gap count field to 63 (and without
having sent a PHY configuration packet beforehand), the gap count of
this node will remain at the old value after the bus reset and thus be
inconsistent with the gap count on all other nodes.
The bus manager is supposed to detect the inconsistent gap count values
in the self ID packets and correct them by issuing another bus reset.
However, if the buggy device happens to be the cycle master, and if it
sends a cycle start packet immediately after the bus reset (which is
likely after a long bus reset), then the time between the end of the
selfID phase and the start of the cycle start packet will be based on
the too-small gap count value, so this gap will be too short to be
detected as a subaction gap by the other nodes. This means that the
cycle start packet will be assumed to be self ID data, and will be
stored after the actual self ID quadlets in the self ID buffer.
This garbage in the self ID buffer made firewire-core ignore all of the
self ID data, and thus prevented the Linux bus manager from correcting
the problem. Furthermore, because the bus reset handling was aborted
completely, asynchronous transfers would be no longer handled correctly,
and fw_run_transaction() would hang until the next bus reset.
To fix this, make the detection of inconsistent self IDs more
discriminating: If the invalid data in the self ID buffer looks like
a cycle start packet, we can assume that the previous data in the buffer
is correctly received self ID information, and process it normally.
(We inspect only the first quadlet of the cycle start packet, because
this value is different enough from any valid self ID quadlet, and many
controllers do not store the cycle start packet in five quadlets because
they expect self ID data to have an even number of quadlets.)
This bug has been observed when a bus-powered DesktopKonnekt6 is
switched off with its power button.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Change memory region to ohci "middle address space". This effectively
reduces the number of packets by 50%.
[Stefan R.:] This eliminates 1394 ack packets and improved throughput
by a few percent in some tests with an S400a connection with and without
gap count optimization. Since firewire-net taxes the AR-req DMA unit of
a FireWire controller much more than firewire-sbp2 (which uses the
middle address space with PCI posted writes too), this commit also
changes a related error printk into a ratelimited one as a precaution.
Side note: The IPv4-over-1394 drivers of Mac OS X 10.4, Windows XP SP3,
and the Thesycon 1394 bus driver for Windows all use the middle address
space too.
Signed-off-by: Stephan Gatzka <stephan@gatzka.org>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Use kernel.h's convenience macros. Also omit a printk that should never
happen and won't matter much if it ever happened.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Takes less source code and machine code, and less runtime with PHYs
other than TSB41BA3D (e.g. TSB81BA3 with device ID 0x831304 which takes
one instead of six read_paged_phy_reg now).
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Fix: phy_reg_mutex must be held over the write/read_phy_reg pair which
gets PHY port status.
Only print to the log when a TSB41BA3D was found. By far most TSB82AA2
cards have a TSB81BA3, and firewire-ohci can keep quiet about that.
Shorten some strings and comments. Change some whitespace.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
This patch implements a work around for the Texas Instruments PHY
TSB41BA3D. This phy has a bug at least in combination with the TI LLCs
TSB82AA2B and TSB12LV26. The selfid coming from the locally connected
phy is not propagated into the selfid buffer of the OHCI (see
http://www.ti.com/litv/pdf/sllz059 for details). The main idea is to
construct the selfid ourselves.
Signed-off-by: Stephan Gatzka <stephan@gatzka.org>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Code inside bus_reset_work may now sleep. This is a prerequisite to
support a phy from Texas Instruments cleanly. The patch to support this
phy will be submitted later.
Signed-off-by: Stephan Gatzka <stephan@gatzka.org>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
sbp2_release_target() is folded into its primary user, sbp2_remove().
The only other caller, a failure path in sbp2_probe(), now uses
sbp2_remove(). This adds unnecessary cancel_delayed_work_sync() calls
to that failure path but results in less code and text.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Implement sbp2_queue_work(), which is now a very simple accessor to one
of the struct sbp2_logical_unit members, right after the definition of
struct sbp2_logical_unit.
Put the sbp2_reconnect() implementation right after the sbp2_login()
implementation. They are both part of the SBP-2 access protocol.
Implement the driver methods sbp2_probe(), spp2_update(), sbp2_remove()
in this order, reflecting the lifetime of an SBP-2 target.
Place the sbp2_release_target() implementation right next to
sbp2_remove() which is its primary user, and after sbp2_probe() which is
the counterpart to sbp2_release_target().
There are no changes to the implementations here, or at least not meant
to be.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Since commit 0278ccd9d5 "firewire: sbp2:
fix panic after rmmod with slow targets", the lifetime of an sbp2_target
instance does no longer extent past the return of sbp2_remove().
Therefore it is no longer necessary to call fw_unit_get/put() and
fw_device_get/put() in sbp2_probe/remove().
Furthermore, said commit also ensures that lu->work is not going to be
executed or requeued at a time when the sbp2_target is no longer in use.
Hence there is no need for sbp2_target reference counting for lu->work.
Other concurrent contexts:
- Processes which access the sysfs of the SCSI host device or of one
of its subdevices are safe because these interfaces are all removed
by scsi_remove_device/host() in sbp2_release_target().
- SBP-2 command block ORB transactions are finished when
scsi_remove_device() in sbp2_release_target() returns.
- SBP-2 management ORB transactions are finished when
cancel_delayed_work_sync(&lu->work) before sbp2_release_target()
returns.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
This fixes https://bugs.launchpad.net/ubuntu/+source/linux/+bug/801719 .
An O2Micro PCI Express FireWire controller,
"FireWire (IEEE 1394) [0c00]: O2 Micro, Inc. Device [1217:11f7] (rev 05)"
which is a combination device together with an SDHCI controller and some
sort of storage controller, misses SBP-2 status writes from an attached
FireWire HDD. This problem goes away if MSI is disabled for this
FireWire controller.
The device reportedly does not require QUIRK_CYCLE_TIMER.
Signed-off-by: Ming Lei <ming.lei@canonical.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> (amended changelog)
Cc: <stable@kernel.org>
If firewire-sbp2 starts a login to a target that doesn't complete ORBs
in a timely manner (and has to retry the login), and the module is
removed before the operation times out, you end up with a null-pointer
dereference and a kernel panic.
[SR: This happens because sbp2_target_get/put() do not maintain
module references. scsi_device_get/put() do, but at occasions like
Chris describes one, nobody holds a reference to an SBP-2 sdev.]
This patch cancels pending work for each unit in sbp2_remove(), which
hopefully means there are no extra references around that prevent us
from unloading. This fixes my crash.
Signed-off-by: Chris Boot <bootc@bootc.net>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
* 'fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6:
firewire: ohci: fix DMA unmapping in an error path
firewire: cdev: fix 32 bit userland on 64 bit kernel compat corner cases
Some older Panasonic made camcorders (Panasonic AG-EZ30 and NV-DX110,
Grundig Scenos DLC 2000) reject requests with ack_busy_X if a request is
sent immediately after they sent a response to a prior transaction.
This causes firewire-core to fail probing of the camcorder with "giving
up on config rom for node id ...". Consequently, programs like kino or
dvgrab are unaware of the presence of a camcorder.
Such transaction failures happen also with the ieee1394 driver stack
(of the 2.4...2.6 kernel series until 2.6.36 inclusive) but with a lower
likelihood, such that kino or dvgrab are generally able to use these
camcorders via the older driver stack. The cause for firewire-ohci's or
firewire-core's worse behavior is not yet known. Gap count optimization
in firewire-core is not the cause. Perhaps the slightly higher latency
of transaction completion in the older stack plays a role. (ieee1394:
AR-resp DMA context tasklet -> packet completion ktread -> user process;
firewire-core: tasklet -> user process.)
This change introduces retries and delays after ack_busy_X into
firewire-core's Config ROM reader, such that at least firewire-core's
probing and /dev/fw* creation are successful. This still leaves the
problem that userland processes are facing transaction failures.
gscanbus's built-in retry routines deal with them successfully, but
neither kino's nor dvgrab's do ever succeed.
But at least DV capture with "dvgrab -noavc -card 0" works now. Live
video preview in kino works too, but not actual capture.
One way to prevent Configuration ROM reading failures in application
programs is to modify libraw1394 to synthesize read responses by means
of firewire-core's Configuration ROM cache. This would only leave
CMP and FCP transaction failures as a potential problem source for
applications.
Reported-and-tested-by: Thomas Seilund <tps@netmaster.dk>
Reported-and-tested-by: René Fritz <rene@colorcube.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
If request_irq failed, we would pass wrong arguments to
dma_free_coherent. https://bugzilla.redhat.com/show_bug.cgi?id=728185
Reported-by: Mads Kiilerich
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Clemens points out that we need to use compat_ptr() in order to safely
cast from u64 to addresses of a 32-bit usermode client.
Before, our conversion went wrong
- in practice if the client cast from pointer to integer such that
sign-extension happened, (libraw1394 and libdc1394 at least were not
doing that, IOW were not affected)
or
- in theory on s390 (which doesn't have FireWire though) and on the
tile architecture, regardless of what the client does.
The bug would usually be observed as the initial get_info ioctl failing
with "Bad address" (EFAULT).
Reported-by: Carl Karsten <carl@personnelware.com>
Reported-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
This allows us to move duplicated code in <asm/atomic.h>
(atomic_inc_not_zero() for now) to <linux/atomic.h>
Signed-off-by: Arun Sharma <asharma@fb.com>
Reviewed-by: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: David Miller <davem@davemloft.net>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Acked-by: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Between open(2) of a /dev/fw* and the first FW_CDEV_IOC_GET_INFO
ioctl(2) on it, the kernel already queues FW_CDEV_EVENT_BUS_RESET events
to be read(2) by the client. The get_info ioctl is practically always
issued right away after open, hence this condition only occurs if the
client opens during a bus reset, especially during a rapid series of bus
resets.
The problem with this condition is twofold:
- These bus reset events carry the (as yet undocumented) @closure
value of 0. But it is not the kernel's place to choose closures;
they are privat to the client. E.g., this 0 value forced from the
kernel makes it unsafe for clients to dereference it as a pointer to
a closure object without NULL pointer check.
- It is impossible for clients to determine the relative order of bus
reset events from get_info ioctl(2) versus those from read(2),
except in one way: By comparison of closure values. Again, such a
procedure imposes complexity on clients and reduces freedom in use
of the bus reset closure.
So, change the ABI to suppress queuing of bus reset events before the
first FW_CDEV_IOC_GET_INFO ioctl was issued by the client.
Note, this ABI change cannot be version-controlled. The kernel cannot
distinguish old from new clients before the first FW_CDEV_IOC_GET_INFO
ioctl.
We will try to back-merge this change into currently maintained stable/
longterm series, and we only document the new behaviour. The old
behavior is now considered a kernel bug, which it basically is.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Cc: <stable@kernel.org>
On Jun 27 Linus Torvalds wrote:
> The correct error code for "I don't understand this ioctl" is ENOTTY.
> The naming may be odd, but you should think of that error value as a
> "unrecognized ioctl number, you're feeding me random numbers that I
> don't understand and I assume for historical reasons that you tried to
> do some tty operation on me".
[...]
> The EINVAL thing goes way back, and is a disaster. It predates Linux
> itself, as far as I can tell. You'll find lots of man-pages that have
> this line in it:
>
> EINVAL Request or argp is not valid.
>
> and it shows up in POSIX etc. And sadly, it generally shows up
> _before_ the line that says
>
> ENOTTY The specified request does not apply to the kind of object
> that the descriptor d references.
>
> so a lot of people get to the EINVAL, and never even notice the ENOTTY.
[...]
> At least glibc (and hopefully other C libraries) use a _string_ that
> makes much more sense: strerror(ENOTTY) is "Inappropriate ioctl for
> device"
So let's correct this in the <linux/firewire-cdev.h> ABI while it is
still young, relative to distributor adoption.
Side note: We return -ENOTTY not only on _IOC_TYPE or _IOC_NR mismatch,
but also on _IOC_SIZE mismatch. An ioctl with an unsupported size of
argument structure can be seen as an unsupported version of that ioctl.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Cc: <stable@kernel.org>
When firewire-ohci is bound to a Pinnacle MovieBoard, eventually a
"Register access failure" is logged and an interrupt storm or a kernel
panic happens. https://bugzilla.kernel.org/show_bug.cgi?id=36622
Until this is sorted out (if that is going to succeed at all), let's
just prevent firewire-ohci from touching these devices.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Cc: <stable@kernel.org>
The software reset in firewire-ohci's pci_remove does not have a great
prospect of success if the card was already physically removed at this
point. So let's skip the 500 ms that were spent in retries here.
Also, replace a defined constant by its open-coded value. This is not a
constant from a specification but an arbitrarily chosen retry limit. It
was only used in this single place.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Detect and handle ejection of FireWire CardBus cards in PHY register
accesses:
- The last attempt of firewire-core to reset the bus during shutdown
caused a spurious "firewire_ohci: failed to write phy reg" error
message in the log. Skip this message as well as the prior retry
loop that needlessly took 100 milliseconds.
- In the unlikely case that a PHY register was read right after card
ejection, a bogus value was obtained and possibly acted upon.
Instead, fail the read attempt.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Stopping an isochronous reception DMA context takes two loop iterations
in context_stop on several controllers (JMicron, NEC, VIA). But there
is no extra delay necessary between these two reg_read trials; the MMIO
reads themselves are slow enough. Hence bring back the behavior from
before commit dd6254e5c0 "firewire: ohci:
remove superfluous posted write flushes" on these controllers by means
of an "if (i)" condition.
Isochronous context stop is performed in preemptible contexts (and only
rarely), hence this change is of little impact. (Besides, Agere and TI
controllers always, or almost always, have the context stopped already
at the first ContextControl read.)
More important is asynchronous transmit context stop, which is performed
while local interrupts are disabled (on the two AT DMAs in
bus_reset_tasklet, i.e. after a self-ID-complete event). In my
experience with several controllers, tested with a usermode AT-request
transmitter as well as with FTP transmission over firewire-net, the AT
contexts were luckily already stopped at the first ContextControl read,
i.e. never required another MMIO read let alone mdelay. A possible
explanation for this is that the controllers which I tested perhaps stop
AT DMA before they perform the self-ID reception DMA.
But we cannot be sure about that and should keep the interrupts-disabled
busy loop as short as possible. Hence, query the ContextControl
register in 1000 udelay(10) intervals instead of 10 udelay(1000)
intervals. I understand from an estimation by Clemens Ladisch that
stopping a busy DMA context should take microseconds or at worst tens of
microseconds, not milliseconds.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
The call to flush_writes() in context_stop() is superfluous because
another register read is done immediately afterwards.
The call to flush_writes() in ar_context_run() does not need to be done
individually for each AR context, so move it to ohci_enable(). This
also makes ohci_enable() clearer because it no longer depends on a side
effect of ar_context_run() to flush its own register writes.
Finally, the setting of a context's wake bit does not need to be flushed
because neither the driver logic nor the API require the CPU to wait for
this action. This removes the last MMIO reads from the packet queueing
code paths.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Fixing a deprecation, replacing __attribute__((packed)) with __packed.
It was deprecated for portability, specifically to avoid GCC specific
code. See commit 82ddcb0405.
Signed-off-by: August Lilleaas <august@augustl.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> (added include compiler.h)
The struct sbp2_logical_unit.work items can all be executed in parallel
but are not reentrant. Furthermore, reconnect or re-login work must be
executed in a WQ_MEM_RECLAIM workqueue.
Hence replace the old single-threaded firewire-sbp2 workqueue by a
concurrency-managed but non-reentrant workqueue with rescuer.
firewire-core already maintains one, hence use this one.
In earlier versions of this change, I observed occasional failures of
parallel INQUIRY to an Initio INIC-2430 FireWire 800 to dual IDE bridge.
More testing indicates that parallel INQUIRY is not actually a problem,
but too quick successions of logout and login + INQUIRY, e.g. a quick
sequence of cable plugout and plugin, can result in failed INQUIRY.
This does not seem to be something that should or could be addressed by
serialization.
Another dual-LU device to which I currently have access to, an
OXUF924DSB FireWire 800 to dual SATA bridge with firmware from MacPower,
has been successfully tested with this too.
This change is beneficial to environments with two or more FireWire
storage devices, especially if they are located on the same bus.
Management tasks that should be performed as soon and as quickly as
possible, especially reconnect, are no longer held up by tasks on other
devices that may take a long time, especially login with INQUIRY and sd
or sr driver probe.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
We do not need slab allocations for ORB pointer write transactions
anymore in order to satisfy streaming DMA mapping constraints, thanks to
commit da28947e7e "firewire: ohci: avoid separate DMA mapping for
small AT payloads".
(Besides, the slab-allocated buffers that firewire-sbp2 used to provide
for 8-byte write requests were still not fully portable since they
shared a cacheline with unrelated CPU-accessed data.)
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
firewire-sbp2 already takes care for internal serialization where
required (ORB list accesses), and it does not use cmd->serial_number
internally. Hence it is safe to not grab the shost lock around
queuecommand.
While we are at housekeeping, drop a redundant struct member:
sbp2_command_orb.done is set once in a hot path and dereferenced once in
a hot path. We can as well dereference sbp2_command_orb.cmd->scsi_done
instead.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
firewire-core manages the following types of work items:
fw_card.br_work:
- resets the bus on a card and possibly sends a PHY packet before that
- does not sleep for long or not at all
- is scheduled via fw_schedule_bus_reset() by
- firewire-ohci's pci_probe method
- firewire-ohci's set_config_rom method, called by kernelspace
protocol drivers and userspace drivers which add/remove
Configuration ROM descriptors
- userspace drivers which use the bus reset ioctl
- itself if the last reset happened less than 2 seconds ago
fw_card.bm_work:
- performs bus management duties
- usually does not (but may in corner cases) sleep for long
- is scheduled via fw_schedule_bm_work() by
- firewire-ohci's self-ID-complete IRQ handler tasklet
- firewire-core's fw_device.work instances whenever the root node
device was (successfully or unsuccessfully) discovered,
refreshed, or rediscovered
- itself in case of resource allocation failures or in order to
obey the 125ms bus manager arbitration interval
fw_device.work:
- performs node probe, update, shutdown, revival, removal; including
kernel driver probe, update, shutdown and bus reset notification to
userspace drivers
- usually sleeps moderately long, in corner cases very long
- is scheduled by
- firewire-ohci's self-ID-complete IRQ handler tasklet via the
core's fw_node_event
- firewire-ohci's pci_remove method via core's fw_destroy_nodes/
fw_node_event
- itself during retries, e.g. while a node is powering up
iso_resource.work:
- accesses registers at the Isochronous Resource Manager node
- usually does not (but may in corner cases) sleep for long
- is scheduled via schedule_iso_resource() by
- the owning userspace driver at addition and removal of the
resource
- firewire-core's fw_device.work instances after bus reset
- itself in case of resource allocation if necessary to obey the
1000ms reallocation period after bus reset
fw_card.br_work instances should not, and instances of the others must
not, be executed in parallel by multiple CPUs -- but were not protected
against that. Hence allocate a non-reentrant workqueue for them.
fw_device.work may be used in the memory reclaim path in case of SBP-2
device updates. Hence we need a workqueue with rescuer and cannot use
system_nrt_wq.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Reviewed-by: Tejun Heo <tj@kernel.org>
When queueing iso packets, the run time is dominated by the two
MMIO accesses that set the DMA context's wake bit. Because most
drivers submit packets in batches, we can save much time by
removing all but the last wakeup.
The internal kernel API is changed to require a call to
fw_iso_context_queue_flush() after a batch of queued packets.
The user space API does not change, so one call to
FW_CDEV_IOC_QUEUE_ISO must specify multiple packets to take
advantage of this optimization.
In my measurements, this patch reduces the time needed to queue
fifty skip packets from userspace to one sixth on a 2.5 GHz CPU,
or to one third at 800 MHz.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
We do not need slab allocations anymore in order to satisfy
streaming DMA mapping constraints, thanks to commit da28947e7e
"firewire: ohci: avoid separate DMA mapping for small AT payloads".
(Besides, the slab-allocated buffers that firewire-core, firewire-sbp2,
and firedtv used to provide for 8-byte write and lock requests were
still not fully portable since they crossed cacheline boundaries or
shared a cacheline with unrelated CPU-accessed data. snd-firewire-lib
got this aspect right by using an extra kmalloc/ kfree just for the
8-byte transaction buffer.)
This change replaces kmalloc'ed lock transaction scratch buffers in
firewire-core, firedtv, and snd-firewire-lib by local stack allocations.
Perhaps the most notable result of the change is simpler locking because
there is no need to serialize usages of preallocated per-device buffers
anymore. Also, allocations and deallocations are simpler.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Acked-by: Clemens Ladisch <clemens@ladisch.de>
Current implementation of ohci_set_config_rom() uses a deferred
bus reset via fw_schedule_bus_reset(). If clients add multiple
unit descriptors to the config_rom in quick succession, the
deferred bus reset may not have fired before succeeding update
requests have come in. This can lead to an incorrect partial
update of the config_rom for both addition and removal of
config_rom descriptors, as the ohci_set_config_rom() routine
will return -EBUSY if a previous pending update has not been
completed yet; the requested update just gets dropped on the floor.
This patch recognizes that the "in-flight" update can be modified
until it has been processed by the bus-reset, and the locking
in the bus_reset_tasklet ensures that the update is done atomically
with respect to modifications made by ohci_set_config_rom(). The
-EBUSY error case is simply removed.
[Stefan R: The bug always existed at least theoretically. But it
became easy to trigger since 2.6.36 commit 02d37bed18 "firewire: core:
integrate software-forced bus resets with bus management" which
introduced long mandatory delays between janitorial bus resets.]
Signed-off-by: Benjamin Buchalter <bj@mhlabs.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> (trivial style changes)
Cc: <stable@kernel.org> # 2.6.36.y and newer
When z==2, the condition "key == 2" is superfluous because it cannot
occur without "b == 3", as a descriptor with b!=3 and key==2 would be
an OUTPUT_MORE_IMMEDIATE descriptor which cannot be used alone.
Also remove magic numbers and needless computations on the b field.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
For AT packet payloads of up to eight bytes, we have enough unused space
in the DMA descriptors list so that we can put a copy of the payload
there and thus avoid having to create a separate streaming DMA mapping
for the payload buffer.
In a CPU-bound microbenchmark that just sends 8-byte packets, bandwidth
was measured to increase by 5.7 %, from 1009 KB/s to 1067 KB/s. In
practice, the only performance-sensitive usage of small asynchronous
packets is the SBP-2 driver's write to the ORB_POINTER register during
SCSI command submission.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
OHCI 1.1 5.7.3 not only forbids enabling or starting any DMA contexts
before the linkEnable bit is set, but also explicitly warns of undefined
behaviour if this order is violated.
Don't violate it then.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
* 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6:
firewire: core: ignore link-active bit of new nodes, fix device recognition
firewire: sbp2: revert obsolete 'fix stall with "Unsolicited response"'
firewire: core: increase default SPLIT_TIMEOUT value
firewire: ohci: Misleading kfree in ohci.c::pci_probe/remove
firewire: ohci: omit IntEvent.busReset check rom AT queueing
firewire: ohci: prevent starting of iso contexts with empty queue
firewire: ohci: prevent iso completion callbacks after context stop
firewire: core: rename some variables
firewire: nosy: should work on Power Mac G4 PCI too
firewire: core: fix card->reset_jiffies overflow
firewire: cdev: remove unneeded reference
firewire: cdev: always wait for outbound transactions to complete
firewire: cdev: remove unneeded idr_find() from complete_transaction()
firewire: ohci: log dead DMA contexts
Like the older ieee1394 core driver, firewire-core skipped scanning of
any new node whose PHY sent a self ID without "link active" bit. If a
device had this bit off mistakenly, it meant that it was inaccessible to
kernel drivers with the old IEEE 1394 driver stack but could still be
accessed by userspace drivers through the raw1394 interface.
But with firewire-core, userspace drivers don't get to see such buggy
devices anymore. This is effectively a driver regression since this
device bug is otherwise harmless.
We now attempt to scan all devices, even repeaters that don't have a
link or powered-down devices that have everything but their PHY shut
down when plugged in. This results in futile repeated scanning attempts
in case of such devices that really don't have an active link, but this
doesn't hurt since recent workqueue infrastructure lets us run more
concurrent scanning jobs than we can shake a stick at.
This should fix accessibility of Focusrite Saffire PRO 26 I/O:
http://sourceforge.net/mailarchive/forum.php?thread_name=20110314215622.5c751bb0%40stein&forum_name=ffado-user
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Now that firewire-core sets the local node's SPLIT_TIMEOUT to 2 seconds
per default, commit a481e97d3c is no
longer required.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
The SPLIT_TIMEOUT mechanism is intended to detect requests that somehow
got lost. However, when the timeout value is too low, transactions that
could have been completed successfully will be cancelled. Furthermore,
there are chips whose firmwares ignore the configured split timeout and
send late split response; known examples are the DM1x00 (BeBoB), TCD22x0
(DICE), and some OXUF936QSE firmwares.
This patch changes the default timeout to two seconds, which happens to
be the default on other OSes, too.
Actual lost requests are extremely rare, so there should be no practical
downside to increasing the split timeout even on devices that work
correctly.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Add a driver for two playback-only FireWire devices based on the OXFW970
chip.
v2: better AMDTP API abstraction; fix fw_unit leak; small fixes
v3: cache the iPCR value
v4: FireWave constraints; fix fw_device reference counting;
fix PCR caching; small changes and fixes
v5: volume/mute support; fix crashing due to pcm stop races
v6: fix build; one-channel volume for LaCie
v7: use signed values to make volume (range checks) work; fix function
block IDs for volume/mute; always use channel 0 for LaCie volume
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Acked-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Tested-by: Jay Fenlason <fenlason@redhat.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
It seems drivers/firewire/ohci.c is making some optimistic assumptions
about struct fw_ohci and that member "card" will always remain the first
member of the struct.
Plus it's probably going to confuse a lot of static code analyzers too.
So I wonder if there is a good reason not to free the ohci struct just
like it was allocated instead of the tricky &ohci->card way?
Signed-off-by: Oleg Drokin <green@linuxhacker.ru>
It is perhaps just a rudiment from before mainline submission of the
driver.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Since commit 82b662dc41 "flush AT contexts after bus reset for OHCI 1.2",
the driver takes care of any AT packets that were enqueued during a bus
reset phase. The check from commit 76f73ca1b2 is therefore no longer
necessary and the MMIO read can be avoided.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
If a misguided program tried to start an isochronous context before it
has queued any packets, the call would appear to succeed, but the
context would not actually go into the running state, and the OHCI
controller would then raise an unrecoverableError interrupt because the
first Z value is zero and thus invalid. The driver logs such errors,
but there is no mechanism to report this back to the program.
Add an explicit check so that this error can be returned synchronously.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
To prevent the iso packet callback from being called after
fw_iso_context_stop() has returned, make sure that the
context's tasklet has finished executing before that.
This fixes access-after-free bugs that have so far been
observed only in the upcoming snd-firewire-speakers driver,
but can theoretically also happen in the firedtv driver.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
In manage_channel(), rename the variables "c" and "i" to the more
expressive "bit" and "channel".
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
The first board generation of Power Mac G4 ("Yikes!", those with PCI
graphics) still had a PCILynx controller like their G3 predecessors,
but not the later AGP models. (Jonathan Woithe recalls to have heard
of it, and some web sources reinforce it.)
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
On a 32-bit machine with, e.g., HZ=1000, jiffies will overflow after
about 50 days, so if there are between 25 and 50 days between bus
resets, the card->reset_jiffies comparisons can get wrong results.
To fix this, ensure that this timestamp always uses 64 bits.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: "Stefan Richter" <stefanr@s5r6.in-berlin.de>
For outbound transactions, the IDR's and the callback's references now
have exactly the same lifetime, so we do not need both of them.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: "Stefan Richter" <stefanr@s5r6.in-berlin.de>
We must not use fw_cancel_transaction() because it cannot correctly
abort still-active transactions. The only place in core-cdev where this
matters is when the file is released. Instead of trying to abort the
transactions, we wait for them to complete normally, i.e., until all
outbound transaction resources have been removed from the IDR tree.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: "Stefan Richter" <stefanr@s5r6.in-berlin.de>
Outbound transactions are never aborted with release_client_resource(),
so it is not necessary for complete_transaction() to check whether the
resource is still registered. Only shutdown_resource() can abort such
an transaction, and this is already handled with the in_shutdown check.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: "Stefan Richter" <stefanr@s5r6.in-berlin.de>
When a DMA context goes into the dead state (and the controller thus
stops working correctly), logging this error and the controller's error
code might be helpful for debugging.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
* 'fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6:
firewire: core: fix unstable I/O with Canon camcorder
* 'fwnet' of git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6:
firewire: net: is not experimental anymore
firewire: net: invalidate ARP entries of removed nodes
thanks to Clemens' and Maxim's fixes to firewire-ohci and -net in the
last two kernel releases.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
This makes it possible to resume communication with a node that dropped
off the bus for a brief period. Otherwise communication will only be
possible after ARP cache entry timeouts.
Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> (rebased)
Regression since commit 1038953674, "firewire: core: check for 1394a
compliant IRM, fix inaccessibility of Sony camcorder":
The camcorder Canon MV5i generates lots of bus resets when asynchronous
requests are sent to it (e.g. Config ROM read requests or FCP Command
write requests) if the camcorder is not root node. This causes drop-
outs in videos or makes the camcorder entirely inaccessible.
https://bugzilla.redhat.com/show_bug.cgi?id=633260
Fix this by allowing any Canon device, even if it is a pre-1394a IRM
like MV5i are, to remain root node (if it is at least Cycle Master
capable). With the FireWire controller cards that I tested, MV5i always
becomes root node when plugged in and left to its own devices.
Reported-by: Ralf Lange
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Cc: <stable@kernel.org> # 2.6.32.y and newer
PAGE_KERNEL_RO is not available on all architectures, so its use
in the new AR code broke compilation on sparc64.
Because the read-only mapping was just a debugging aid, just use
PAGE_KERNEL instead.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
James Bottomley wrote:
> On Thu, 2011-01-13 at 08:27 +0100, Clemens Ladisch wrote:
>> firewire: ohci: fix compilation on arches without PAGE_KERNEL_RO, e.g. sparc
>>
>> PAGE_KERNEL_RO is not available on all architectures, so its use in the
>> new AR code broke compilation on sparc64.
>>
>> Because the R/O mapping is only used to catch drivers that try to write
>> to the reception buffer and not actually required for correct operation,
>> we can just use a normal PAGE_KERNEL mapping where _RO is not available.
[...]
>> +/*
>> + * For archs where PAGE_KERNEL_RO is not supported;
>> + * mapping the AR buffers readonly for the CPU is just a debugging aid.
>> + */
>> +#ifndef PAGE_KERNEL_RO
>> +#define PAGE_KERNEL_RO PAGE_KERNEL
>> +#endif
>
> This might cause interesting issues on sparc64 if it ever acquired a
> PAGE_KERNEL_RO. Sparc64 has extern pgprot_t for it's PAGE_KERNEL types
> rather than #defines, so the #ifdef check wouldn't see this.
>
> I think either PAGE_PROT_RO becomes part of our arch API (so all
> architectures are forced to add it), or, if it's not part of the API,
> ohci isn't entitled to use it. The latter seems simplest since you have
> no real use for write protection anyway.
Reported-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Instead of starting the split transaction timeout timer when any request
is submitted, start it only when the destination's ACK_PENDING has been
received. This prevents us from using a timeout that is too short, and,
if the controller's AT queue is emptying very slowly, from cancelling
a packet that has not yet been sent.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
"firewire: ohci: restart iso DMA contexts on resume from low power mode"
added the flag struct context.active and "firewire: ohci: cache the
context run bit" added struct context.running.
These flags contain the same information; combine them.
Also, normalize whitespace in pci_resume().
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
The DMA context run control bit is entirely controlled by software, so
it is safe to cache it. This allows the driver to avoid doing an
additional MMIO read when queueing an AT packet.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Add comments
- on why bus_reset_tasklet flushes AT queues,
- that commit 76f73ca1b2 can possibly be reverted now.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Acked-by: Jarod Wilson <jarod@redhat.com>
The OHCI 1.2 (draft) specification, clause 7.2.3.3, allows and
recommends that, after a bus reset, the controller does not flush all
the packets in the AT queues. Therefore, the driver has to do this
itself.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
At ifup, carrier status would be shown on even if it actually was off.
Also add an include for ethtool_ops rather than to rely on the one from
netdevice.h.
Note, we can alas not use fwnet_device_mutex to serialize access to
dev->peer_count (as I originally wanted). This would cause a lock
inversion:
- fwnet_probe | takes fwnet_device_mutex
+ register_netdev | takes rtnl_mutex
- devinet_ioctl | takes rtnl_mutex
+ fwnet_open | ...must not take fwnet_device_mutex
Hence use the dev->lock spinlock for serialization.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
To make userland, e.g. NetworkManager work with firewire, we need to
detect whether cable is plugged or not. Simple and correct way of doing
that is just counting number of peers. No peers - no link and vice
versa.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Unfortunately its easy to trigger such error messages by removing the
cable while sending streams of data over the link.
Such errors are normal, and therefore this patch stops firewire-net from
flooding the kernel log with these errors, by combining series of same
errors together.
Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
(Stefan R:) Eventually we should remove this logging when firewire-net
and related firewire-ohci facilities have been stabilized.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Restore iso channels DMA so that iso channels could continue to work
after resume from RAM/disk.
Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Some lousy BIOSes, e.g. my Aspire 5720 BIOS forget to restore the GUID
register on resume from RAM.
Fix that by setting it to the last value that was read from it.
Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
The buffers used for the selfIDs packets and the AR request and response
descriptors end up using three pages because dma_alloc_coherent()
allocates at least one page per call. However, these data structures
would all fit into 4 KB, so we can save space by using a common buffer
for them.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
When the isochRx/isochTx bit is clear, we do not need to read the
corresponding iso interrupt event register.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Change the header of PHY packets to be sent to include a pseudo
transaction code. This makes the header consistent with that of
received PHY packets, and allows at_context_queue_packet() and
log_ar_at_event() to see the packet type directly instead of having
to deduce it from the header length or even from the header contents.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
To remove the error information from the controller's queue and to allow
more posted writes, the driver has to read the failed posted write
address before clearing the postedWriteErr interrupt bit.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
(Stefan R:) The spec is somewhat fuzzy about the actual requirements.
To err on the safe side, let's do these two read accesses.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Make sure that interrupt event clear bit writes are executed before the
interrupt handler returns.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Add proper error handling for the context_init() calls.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Move the AR DMA descriptors out of the buffer pages, and map the buffer
pages linearly into the kernel's address space. This allows the driver
to ignore any page boundaries in the DMA data and thus to avoid any
copying around of packet payloads.
This fixes the bug where S800 packets that are so big (> 4080 bytes)
that they can be split over three pages were not handled correctly.
Due to the changed algorithm, we can now use arbitrarily many buffer
pages, which improves performance because the controller can more easily
unload its DMA FIFO.
Furthermore, using streaming DMA mappings should improve perfomance on
architectures where coherent DMA mappings are not cacheable. Even on
other architectures, the caching behaviour should be improved slightly
because the CPU no longer writes to the buffer pages.
v2: Detect the last filled buffer page by searching the descriptor's
residual count value fields in order (like in the old code), instead
of going backwards through the transfer status fields; it looks as
if some controllers do not set the latter correctly.
v3: Fix an old resume bug that would now make the handler run into
a BUG_ON, and replace that check with more useful error handling.
Increase the buffer size for better performance with non-TI chips.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Maxim Levitsky writes:
Works almost perfectly. I can still see RCODE_BUSY errors
sometimes, not very often though. 64K here eliminates these errors
completely. This is most likely due to nouveau drivers and lowest
perf level I use to lower card temperature. That increases
latencies too much I think. Besides that the IO is just perfect.
Tested-by: Maxim Levitsky <maximlevitsky@gmail.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Amend .open handler accordingly and remove the .llseek handler.
.llseek = NULL means no_llseek (return error) since commit 776c163b1b.
The only client that uses this interface is nosy-dump in linux/tools/firewire
and it knows not to seek in this char dev.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Agere FW643 rev 06, listed as "11c1:5901 (rev 06) (prog-if 10 [OHCI])",
produced SBP-2 I/O errors since kernel 2.6.36. Disabling MSI fixes it.
Since MSI work on Agere FW643-E (same vendor and device ID, but rev 07),
introduce a device revision field into firewire-ohci's quirks list so
that different quirks can be defined for older and newer revisions.
Reported-by: Jonathan Isom <jeisom@gmail.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Cc: <stable@kernel.org> # 2.6.36.y
"VIA Technologies, Inc. VT6315 Series Firewire Controller [1106:3403]"
does not generate any interrupts if Message Signaled Interrupts were
enabled. This is a regression since kernel 2.6.36 in which MSI support
was added to firewire-ohci. Hence blacklist MSI on all VIA controllers.
Reported-by: Robin Cook <rcook@wyrms.net>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Cc: <stable@kernel.org> # 2.6.36.y
This prevents firewire-net from submitting write requests in fast
succession until failure due to all 64 transaction labels were used up
for unfinished split transactions. The netif_stop/wake_queue API is
used for this purpose.
Without this stop/wake mechanism, datagrams were simply lost whenever
the tlabel pool was exhausted. Plus, tlabel exhaustion by firewire-net
also prevented other unrelated outbound transactions to be initiated.
The chosen queue depth was checked by me to hit the maximum possible
throughput with an OS X peer whose receive DMA is good enough to never
reject requests due to busy inbound request FIFO. Current Linux peers
show a mixed picture of -5%...+15% change in bandwidth; their current
bottleneck are RCODE_BUSY situations (fewer or more, depending on TX
queue depth) due to too small AR buffer in firewire-ohci.
Maxim Levitsky tested this change with similar watermarks with a Linux
peer and some pending firewire-ohci improvements that address the
RCODE_BUSY problem and confirmed that these TX queue limits are good.
Note: This removes some netif_wake_queue from reception code paths.
They were apparently copy&paste artefacts from a nonsensical
netif_wake_queue use in the older eth1394 driver. This belongs only
into the transmit path.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Tested-by: Maxim Levitsky <maximlevitsky@gmail.com>
The current transmit code does not at all make use of
- fwnet_device.packet_list
and only very limited use of
- fwnet_device.broadcasted_list,
- fwnet_device.queued_packets.
Their current function is to track whether the TX soft-IRQ finished
dealing with an skb when the AT-req tasklet takes over, and to discard
pending tx datagrams (if there are any) when the local node is removed.
The latter does actually contain a race condition bug with TX soft-IRQ
and AT-req tasklet.
Instead of these lists and the corresponding link in fwnet_packet_task,
- a flag in fwnet_packet_task to track whether fwnet_tx is done,
- a counter of queued datagrams in fwnet_device
do the job as well.
The above mentioned theoretic race condition is resolved by letting
fwnet_remove sleep until all datagrams were flushed. It may sleep
almost arbitrarily long since fwnet_remove is executed in the context of
a multithreaded (concurrency managed) workqueue.
The type of max_payload is changed to u16 here to avoid waste in struct
fwnet_packet_task. This value cannot exceed 4096 per IEEE 1394:2008
table 16-18 (or 32678 per specification of packet headers, if there is
ever going to be something else than beta mode).
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
a) fwnet_transmit_packet_done used to poison ptask->pt_link by list_del.
If fwnet_send_packet checked later whether it was responsible to clean
up (in the border case that the TX soft IRQ was outpaced by the AT-req
tasklet on another CPU), it missed this because ptask->pt_link was no
longer shown as empty.
b) If fwnet_write_complete got an rcode other than RCODE_COMPLETE, we
missed to free the skb and ptask entirely.
Also, count stats.tx_dropped and stats.tx_errors when rcode != 0.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Move the mid-layer's ->queuecommand() invocation from being locked
with the host lock to being unlocked to facilitate speeding up the
critical path for drivers who don't need this lock taken anyway.
The patch below presents a simple SCSI host lock push-down as an
equivalent transformation. No locking or other behavior should change
with this patch. All existing bugs and locking orders are preserved.
Additionally, add one parameter to queuecommand,
struct Scsi_Host *
and remove one parameter from queuecommand,
void (*done)(struct scsi_cmnd *)
Scsi_Host* is a convenient pointer that most host drivers need anyway,
and 'done' is redundant to struct scsi_cmnd->scsi_done.
Minimal code disturbance was attempted with this change. Most drivers
needed only two one-line modifications for their host lock push-down.
Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
Acked-by: James Bottomley <James.Bottomley@suse.de>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
* 'fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6:
firewire: ohci: fix race when reading count in AR descriptor
firewire: ohci: avoid reallocation of AR buffers
firewire: ohci: fix race in AR split packet handling
firewire: ohci: fix buffer overflow in AR split packet handling
If the controller is storing a split packet and therefore changing
d->res_count to zero between the two reads by the driver, we end up with
an end pointer that is not at a packet boundary, and therefore overflow
the buffer when handling the split packet.
To fix this, read the field once, atomically. The compiler usually
merges the two reads anyway, but for correctness, we have to enforce it.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Tested-by: Maxim Levitsky <maximlevitsky@gmail.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Freeing an AR buffer page just to allocate a new page immediately
afterwards is not only a pointless effort but also dangerous because
the allocation can fail, which would result in an oops later.
Split ar_context_add_page() into two functions so that we can reuse
the old page directly.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Tested-by: Maxim Levitsky <maximlevitsky@gmail.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
When handling an AR buffer that has been completely filled, we assumed
that its descriptor will not be read by the controller and can be
overwritten. However, when the last received packet happens to end at
the end of the buffer, the controller might not yet have moved on to the
next buffer and might read the branch address later. If we overwrite
and free the page before that, the DMA context will either go dead
because of an invalid Z value, or go off into some random memory.
To fix this, ensure that the descriptor does not get overwritten by
using only the actual buffer instead of the entire page for reassembling
the split packet. Furthermore, to avoid freeing the page too early,
move on to the next buffer only when some data in it guarantees that the
controller has moved on.
This should eliminate the remaining firewire-net problems.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Cc: 2.6.22-2.6.36 <stable@kernel.org>
Tested-by: Maxim Levitsky <maximlevitsky@gmail.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
When the controller had to split a received asynchronous packet into two
buffers, the driver tries to reassemble it by copying both parts into
the first page. However, if size + rest > PAGE_SIZE, i.e., if the yet
unhandled packets before the split packet, the split packet itself, and
any received packets after the split packet are together larger than one
page, then the memory after the first page would get overwritten.
To fix this, do not try to copy the data of all unhandled packets at
once, but copy the possibly needed data every time when handling
a packet.
This gets rid of most of the infamous crashes and data corruptions when
using firewire-net.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Cc: 2.6.22-2.6.36 <stable@kernel.org>
Tested-by: Maxim Levitsky <maximlevitsky@gmail.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> (cast PAGE_SIZE to size_t)
* 'ieee1394-removal' of git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6:
ieee1394: remove the old IEEE 1394 driver stack
ieee1394: move init_ohci1394_dma to drivers/firewire/
Fix trivial change/delete conflict: drivers/ieee1394/eth1394.c is
getting removed, but was modified by the networking merge.
* git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next-2.6: (1699 commits)
bnx2/bnx2x: Unsupported Ethtool operations should return -EINVAL.
vlan: Calling vlan_hwaccel_do_receive() is always valid.
tproxy: use the interface primary IP address as a default value for --on-ip
tproxy: added IPv6 support to the socket match
cxgb3: function namespace cleanup
tproxy: added IPv6 support to the TPROXY target
tproxy: added IPv6 socket lookup function to nf_tproxy_core
be2net: Changes to use only priority codes allowed by f/w
tproxy: allow non-local binds of IPv6 sockets if IP_TRANSPARENT is enabled
tproxy: added tproxy sockopt interface in the IPV6 layer
tproxy: added udp6_lib_lookup function
tproxy: added const specifiers to udp lookup functions
tproxy: split off ipv6 defragmentation to a separate module
l2tp: small cleanup
nf_nat: restrict ICMP translation for embedded header
can: mcp251x: fix generation of error frames
can: mcp251x: fix endless loop in interrupt handler if CANINTF_MERRF is set
can-raw: add msg_flags to distinguish local traffic
9p: client code cleanup
rds: make local functions/variables static
...
Fix up conflicts in net/core/dev.c, drivers/net/pcmcia/smc91c92_cs.c and
drivers/net/wireless/ath/ath9k/debug.c as per David
* 'llseek' of git://git.kernel.org/pub/scm/linux/kernel/git/arnd/bkl:
vfs: make no_llseek the default
vfs: don't use BKL in default_llseek
llseek: automatically add .llseek fop
libfs: use generic_file_llseek for simple_attr
mac80211: disallow seeks in minstrel debug code
lirc: make chardev nonseekable
viotape: use noop_llseek
raw: use explicit llseek file operations
ibmasmfs: use generic_file_llseek
spufs: use llseek in all file operations
arm/omap: use generic_file_llseek in iommu_debug
lkdtm: use generic_file_llseek in debugfs
net/wireless: use generic_file_llseek in debugfs
drm: use noop_llseek
Revert commit 54672386cc
"firewire: ohci: fix up configuration of TI chips".
It caused massive slow-down and data corruption with a TSB82AA2 based
StarTech EC1394B2 ExpressCard and FireWire 800 harddisks.
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/657081http://thread.gmane.org/gmane.linux.kernel.firewire.user/4013
The fact that some card EEPROMs do not program these enhancements may be
related to TSB81BA3 phy chip errata, if not to bugs of TSB82AA2 itself.
We could re-add these configuration steps, but only conditional on a
whitelist of cards on which these enhancements bring a proven positive
effect.
Reported-and-tested-by: Eric Shattow <lucent@gmail.com>
Cc: Clemens Ladisch <clemens@ladisch.de>
Cc: <stable@kernel.org> 2.6.35
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
All file_operations should get a .llseek operation so we can make
nonseekable_open the default for future file operations without a
.llseek pointer.
The three cases that we can automatically detect are no_llseek, seq_lseek
and default_llseek. For cases where we can we can automatically prove that
the file offset is always ignored, we use noop_llseek, which maintains
the current behavior of not returning an error from a seek.
New drivers should normally not use noop_llseek but instead use no_llseek
and call nonseekable_open at open time. Existing drivers can be converted
to do the same when the maintainer knows for certain that no user code
relies on calling seek on the device file.
The generated code is often incorrectly indented and right now contains
comments that clarify for each added line why a specific variant was
chosen. In the version that gets submitted upstream, the comments will
be gone and I will manually fix the indentation, because there does not
seem to be a way to do that using coccinelle.
Some amount of new code is currently sitting in linux-next that should get
the same modifications, which I will do at the end of the merge window.
Many thanks to Julia Lawall for helping me learn to write a semantic
patch that does all this.
===== begin semantic patch =====
// This adds an llseek= method to all file operations,
// as a preparation for making no_llseek the default.
//
// The rules are
// - use no_llseek explicitly if we do nonseekable_open
// - use seq_lseek for sequential files
// - use default_llseek if we know we access f_pos
// - use noop_llseek if we know we don't access f_pos,
// but we still want to allow users to call lseek
//
@ open1 exists @
identifier nested_open;
@@
nested_open(...)
{
<+...
nonseekable_open(...)
...+>
}
@ open exists@
identifier open_f;
identifier i, f;
identifier open1.nested_open;
@@
int open_f(struct inode *i, struct file *f)
{
<+...
(
nonseekable_open(...)
|
nested_open(...)
)
...+>
}
@ read disable optional_qualifier exists @
identifier read_f;
identifier f, p, s, off;
type ssize_t, size_t, loff_t;
expression E;
identifier func;
@@
ssize_t read_f(struct file *f, char *p, size_t s, loff_t *off)
{
<+...
(
*off = E
|
*off += E
|
func(..., off, ...)
|
E = *off
)
...+>
}
@ read_no_fpos disable optional_qualifier exists @
identifier read_f;
identifier f, p, s, off;
type ssize_t, size_t, loff_t;
@@
ssize_t read_f(struct file *f, char *p, size_t s, loff_t *off)
{
... when != off
}
@ write @
identifier write_f;
identifier f, p, s, off;
type ssize_t, size_t, loff_t;
expression E;
identifier func;
@@
ssize_t write_f(struct file *f, const char *p, size_t s, loff_t *off)
{
<+...
(
*off = E
|
*off += E
|
func(..., off, ...)
|
E = *off
)
...+>
}
@ write_no_fpos @
identifier write_f;
identifier f, p, s, off;
type ssize_t, size_t, loff_t;
@@
ssize_t write_f(struct file *f, const char *p, size_t s, loff_t *off)
{
... when != off
}
@ fops0 @
identifier fops;
@@
struct file_operations fops = {
...
};
@ has_llseek depends on fops0 @
identifier fops0.fops;
identifier llseek_f;
@@
struct file_operations fops = {
...
.llseek = llseek_f,
...
};
@ has_read depends on fops0 @
identifier fops0.fops;
identifier read_f;
@@
struct file_operations fops = {
...
.read = read_f,
...
};
@ has_write depends on fops0 @
identifier fops0.fops;
identifier write_f;
@@
struct file_operations fops = {
...
.write = write_f,
...
};
@ has_open depends on fops0 @
identifier fops0.fops;
identifier open_f;
@@
struct file_operations fops = {
...
.open = open_f,
...
};
// use no_llseek if we call nonseekable_open
////////////////////////////////////////////
@ nonseekable1 depends on !has_llseek && has_open @
identifier fops0.fops;
identifier nso ~= "nonseekable_open";
@@
struct file_operations fops = {
... .open = nso, ...
+.llseek = no_llseek, /* nonseekable */
};
@ nonseekable2 depends on !has_llseek @
identifier fops0.fops;
identifier open.open_f;
@@
struct file_operations fops = {
... .open = open_f, ...
+.llseek = no_llseek, /* open uses nonseekable */
};
// use seq_lseek for sequential files
/////////////////////////////////////
@ seq depends on !has_llseek @
identifier fops0.fops;
identifier sr ~= "seq_read";
@@
struct file_operations fops = {
... .read = sr, ...
+.llseek = seq_lseek, /* we have seq_read */
};
// use default_llseek if there is a readdir
///////////////////////////////////////////
@ fops1 depends on !has_llseek && !nonseekable1 && !nonseekable2 && !seq @
identifier fops0.fops;
identifier readdir_e;
@@
// any other fop is used that changes pos
struct file_operations fops = {
... .readdir = readdir_e, ...
+.llseek = default_llseek, /* readdir is present */
};
// use default_llseek if at least one of read/write touches f_pos
/////////////////////////////////////////////////////////////////
@ fops2 depends on !fops1 && !has_llseek && !nonseekable1 && !nonseekable2 && !seq @
identifier fops0.fops;
identifier read.read_f;
@@
// read fops use offset
struct file_operations fops = {
... .read = read_f, ...
+.llseek = default_llseek, /* read accesses f_pos */
};
@ fops3 depends on !fops1 && !fops2 && !has_llseek && !nonseekable1 && !nonseekable2 && !seq @
identifier fops0.fops;
identifier write.write_f;
@@
// write fops use offset
struct file_operations fops = {
... .write = write_f, ...
+ .llseek = default_llseek, /* write accesses f_pos */
};
// Use noop_llseek if neither read nor write accesses f_pos
///////////////////////////////////////////////////////////
@ fops4 depends on !fops1 && !fops2 && !fops3 && !has_llseek && !nonseekable1 && !nonseekable2 && !seq @
identifier fops0.fops;
identifier read_no_fpos.read_f;
identifier write_no_fpos.write_f;
@@
// write fops use offset
struct file_operations fops = {
...
.write = write_f,
.read = read_f,
...
+.llseek = noop_llseek, /* read and write both use no f_pos */
};
@ depends on has_write && !has_read && !fops1 && !fops2 && !has_llseek && !nonseekable1 && !nonseekable2 && !seq @
identifier fops0.fops;
identifier write_no_fpos.write_f;
@@
struct file_operations fops = {
... .write = write_f, ...
+.llseek = noop_llseek, /* write uses no f_pos */
};
@ depends on has_read && !has_write && !fops1 && !fops2 && !has_llseek && !nonseekable1 && !nonseekable2 && !seq @
identifier fops0.fops;
identifier read_no_fpos.read_f;
@@
struct file_operations fops = {
... .read = read_f, ...
+.llseek = noop_llseek, /* read uses no f_pos */
};
@ depends on !has_read && !has_write && !fops1 && !fops2 && !has_llseek && !nonseekable1 && !nonseekable2 && !seq @
identifier fops0.fops;
@@
struct file_operations fops = {
...
+.llseek = noop_llseek, /* no read or write fn */
};
===== End semantic patch =====
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Julia Lawall <julia@diku.dk>
Cc: Christoph Hellwig <hch@infradead.org>
The drivers
- ohci1394 (controller driver)
- ieee1394 (core)
- dv1394, raw1394, video1394 (userspace ABI)
- eth1394, sbp2 (protocol drivers)
are replaced by
- firewire-ohci (controller driver)
- firewire-core (core and userspace ABI)
- firewire-net, firewire-sbp2 (protocol drivers)
which are more featureful, better performing, and more secure than the older
drivers; all with a smaller and more modern code base.
The driver firedtv in drivers/media/dvb/firewire/ contains backends to both
ieee1394 and firewire-core. Its ieee1394 backend code can be removed in an
independent commit; firedtv as-is builds and works fine without ieee1394.
The driver pcilynx (an incomplete controller driver) is deleted without
replacement since PCILynx cards are extremely rare. Owners of these cards
use them with the stand-alone bus sniffer driver nosy instead.
The drivers nosy and init_ohci1394_dma which do not interact with either of
the two IEEE 1394 stacks are not affected by the ieee1394 subsystem removal.
There are still some issues with the newer firewire subsystem compared to
the older one:
- The rare and quirky controllers ALi M52xx, Apple UniNorth v1, NVIDIA
NForce2 are even less well supported by firewire-ohci than by ohci1394.
I am looking into the M52xx issue.
- The experimental firewire-net is reportedly less stable than its
experimental cousin eth1394.
- Audio playback of a certain group of audio devices (ones based on DICE
chipset with EAP; supported by prerelease FFADO code) does not work yet.
This issue is still under investigation.
- There were some ieee1394 based out-of-the-mainline drivers. Of them,
only lisight, an audio driver for iSight webcams, seems still useful.
Work is underway to reimplement it on top of firewire-core.
All these remainig issues are minor; they should not stand in the way of
overall better user experience of IEEE 1394 on Linux, together with a
reduction in support efforts and maintenance burden. The coexistence of two
IEEE 1394 kernel driver stacks in the mainline since 2.6.22 shall end now,
as announced earlier this year.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
because drivers/ieee1394/ will be deleted.
Additional changes:
- add some #include directives
- adjust to use firewire/ohci.h instead of ieee1394/ohci1394.h,
replace struct ti_ohci by a minimal struct ohci,
replace quadlet_t from ieee1394_types.h by u32
- two or three trivial stylistic changes
- __iomem annotation
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
The Ricoh FireWire controllers appear to have the non-atomic cycle
timer register access bug, so, activate the driver workaround by
default.
The behaviour was observed on:
Ricoh Co Ltd R5C552 IEEE 1394 Controller [1180:0552] and
Ricoh Co Ltd R5C832 IEEE 1394 Controller [1180:0832] (rev 04).
Signed-off-by: Heikki Lindholm <holin@iki.fi>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
VIA VT6306, VIA VT6308, and NEC OrangeLink controllers do not write
packet event codes for received PHY packets (or perhaps write
evt_no_status, hard to tell). Work around it by overwriting the
packet's ACK by ack_complete, so that upper layers that listen to PHY
packet reception get to see these packets.
(Also tested: TI TSB82AA2, TI TSB43AB22/A, TI XIO2213A, Agere FW643,
JMicron JMB381 --- these do not exhibit this bug.)
Clemens proposed a quirks flag for that, IOW whitelist known misbehaving
controllers for this workaround. Though to me it seems harmless enough
to enable for all controllers.
The log_ar_at_event() debug log will continue to show the original
status from the DMA unit.
Reported-by: Clemens Ladisch <clemens@ladisch.de> (VT6308)
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Because we might be in interrupt context, replace del_timer_sync() with
del_timer(). If the timer is already running, we know that it will
clean up the transaction, so we do not need to do any further processing
in the normal transaction handler.
Many thanks to Yong Zhang for diagnosing this.
Reported-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
The incoming request hander fwnet_receive_packet() expects subsequent
datagram handling code to return non-zero on errors. However, almost
none of the failure paths did so. Fix them all.
(This error reporting is used to send and RCODE_CONFLICT_ERROR to the
sender node in such failure cases. Two modes of failure exist: Out of
memory, or firewire-net is unaware of any peer node to which a fragment
or an ARP packet belongs. However, it is unclear whether a sender can
actually make use of such information. A Linux peer apparently can't.
Maybe it should all be simplified to void functions.)
Reported-by: Julia Lawall <julia@diku.dk>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Fix I/O stalls with some 4-bay RAID enclosures which are based on
OXUF936QSE:
- Onnto dataTale RSM4QO, old firmware (not anymore with current
firmware),
- inXtron Hydra Super-S LCM, old as well as current firmware
when used in RAID-5 mode, perhaps also in other RAID modes.
The stalls happen during heavy or moderate disk traffic in periods that
are a multiple of 5 minutes, roughly twice per hour. They are caused
by the target responding too late to an ORB_Pointer register write:
The target responds after Split_Timeout, hence firewire-core cancels
the transaction, and firewire-sbp2 fails the SCSI request. The SCSI
core retries the request, that fails again (and again), hence SCSI core
calls firewire-sbp2's abort handler (and even the Management_Agent
register write in the abort handler has the transaction timeout
problem).
During all that, the process which issued the I/O is stalled in I/O
wait state.
Meanwhile, the target actually acts on the first failed SCSI request:
It responds to the ORB_Pointer write later (seen in the kernel log as
"firewire_core: Unsolicited response") and also finishes the SCSI
request with proper status (seen in the kernel log as "firewire_sbp2:
status write for unknown orb").
So let's just ignore RCODE_CANCELLED in the transaction callback and
wait for the target to complete the ORB nevertheless. This requires
a small modification is sbp2_cancel_orbs(); it now needs to call
orb->callback() regardless whether fw_cancel_transaction() found the
transaction unfinished or finished.
A different solution is to increase Split_Timeout on the local node.
(Tested: 2000ms timeout; maybe 1000ms or something like that works too.
200ms is insufficient. Standard is 100ms.) However, I rather not do
this because any software on any node could change the Split_Timeout to
something unsuitable. Or such a large Split_Timeout may be undesirable
for other purposes.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
When an ORB was canceled (Command ORB i.e. SCSI request timed out, or
Management ORB timed out), or there was a send error in the initial
transaction, we missed to drop one of the ORB's references and thus
leaked memory.
Background:
In total, we hold 3 references to each Operation Request Block:
- 1 during sbp2_scsi_queuecommand() or sbp2_send_management_orb()
respectively,
- 1 for the duration of the write transaction to the ORB_Pointer or
Management_Agent register of the target,
- 1 for as long as the ORB stays within the lu->orb_list, until
the ORB is unlinked from the list and the orb->callback was
executed.
The latter one of these 3 references is finished
- normally by sbp2_status_write() when the target wrote status
for a pending ORB,
- or by sbp2_cancel_orbs() in case of an ORB time-out,
- or by complete_transaction() in case of a send error.
Of them, the latter two lacked the kref_put.
Add the missing kref_put()s. Add comments to the gets and puts of
references for transaction callbacks and ORB callbacks so that it is
easier to see what is supposed to happen.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
The driver name and bus address for a net_device can normally be found
through the driver model now. Instead of requiring drivers to provide
this information redundantly through the ethtool_ops::get_drvinfo
operation, use the driver model to do so if the driver does not define
the operation. Since ETHTOOL_GDRVINFO no longer requires the driver
to implement any operations, do not require net_device::ethtool_ops to
be set either.
Remove implementations of get_drvinfo and ethtool_ops that provide
only this information.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Conflicts:
drivers/firewire/core-card.c
drivers/firewire/core-cdev.c
and forgotten #include <linux/time.h> in drivers/firewire/ohci.c
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
There is an at least theoretic race condition in which .start_iso etc.
could still be called between when the dummy driver is bound to the card
and when the children devices are being shut down. Add dummy_start_iso
and friends.
On the other hand, .enable, .set_config_rom, .read_csr, write_csr do not
need to be implemented by the dummy driver, as commented.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
This adds the DMA context programming and userspace ABI for multichannel
reception, i.e. for listening on multiple channel numbers by means of a
single DMA context.
The use case is reception of more streams than there are IR DMA units
offered by the link layer. This is already implemented by the older
ohci1394 + ieee1394 + raw1394 stack. And as discussed recently on
linux1394-devel, this feature is occasionally used in practice.
The big drawbacks of this mode are that buffer layout and interrupt
generation necessarily differ from single-channel reception: Headers
and trailers are not stripped from packets, packets are not aligned with
buffer chunks, interrupts are per buffer chunk, not per packet.
These drawbacks also cause a rather hefty code footprint to support this
rarely used OHCI-1394 feature. (367 lines added, among them 94 lines of
added userspace ABI documentation.)
This implementation enforces that a multichannel reception context may
only listen to channels to which no single-channel context on the same
link layer is presently listening to. OHCI-1394 would allow to overlay
single-channel contexts by the multi-channel context, but this would be
a departure from the present first-come-first-served policy of IR
context creation.
The implementation is heavily based on an earlier one by Jay Fenlason.
Thanks Jay.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
firewire-ohci keeps book of which isochronous channels are occupied by
IR DMA contexts, so that there cannot be more than one context listening
to a certain channel.
If IR context creation failed due to an out-of-memory condition, this
bookkeeping leaked a channel.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
When we append to a DMA program, we need to ensure that the order in
which initialization of the new descriptors and update of the
branch_address of the old tail descriptor, as seen by the PCI device,
happen as intended.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
This adds nosy-dump, the userspace part of nosy, the IEEE 1394 traffic
sniffer for Texas Instruments PCILynx/ PCILynx2 based cards. Author is
Kristian Høgsberg.
The files added here are taken from
git://anongit.freedesktop.org/~krh/nosy commit ee29be97 (2009-11-10)
with the following changes by Stefan Richter:
- Parts pertaining to the kernel module removed from Makefile.
- dist target removed from the Makefile.
- Mentioned nosy-dump in the Kconfig help to nosy's kernel component.
- Add copyright notice to nosy-dump.c. This is a duplicate of the
respective notice in the kernel component nosy.c except for a time
span of 2002 - 2006, according to Kristian's git log.
"git shortlog decode-fcp.c list.h nosy-dump.[ch]" from nosy's git
repository:
Jonathan Woithe (1):
Save logs on Ctrl-C
Kristian Høgsberg (11):
Pull over nosy from mercurial repo.
Remove some fields from default view, add logging feature.
Use infinite time out for poll(), mark more detail fields.
Fix byte ordering macro.
Add decoding of iso data and lock packets.
Add flag to indicate data length field.
Add cycle start packet decoding, add --iso and --cycle-start flags.
Distinguish between phy-packets and 0-length iso data.
Fix transaction and stats view.
Add simple AV/C decoder.
Don't break down on big payloads.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Acked-by: Kristian Høgsberg <krh@bitplanet.net>
Replace home-grown printk wrapper macros by ones from kernel.h and
device.h.
Also raise the log level in set_phy_reg() from debug to error because
these are really error conditions. Could even be WARN_ON. Lower the
log level in the device probe and driver shutdown from notice to info.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
1.) The DMA programs (struct pcl) are PCI-endian = little endian data
(except for the 3rd quadlet in a PCL which the controller does not
touch). Annotate them as such.
Fix all accesses of the PCL to work with big endian CPUs also. Not
actually tested, I only have a little endian PC to test with. This
includes replacement of a bitfield struct pcl_status by open-coded
shift and mask operations.
2.) The two __attribute__ ((packed)) at struct pcl are not really
required since it consists of u32/__le32 only, i.e. there will be no
padding with or without the attribute.
3.) The received IEEE 1394 data are byteswapped by the controller from
IEEE 1394 endian = big endian to PCI endian = little endian because the
PCL_BIGENDIAN control bit is set. Therefore annotate the DMA buffer as
a __le32 array.
Fix the one access of the DMA buffer (the check of the transaction code
of link packets) to work with big endian CPUs. Also fix the two
accesses of the client bounce buffer (the reading of packet length).
4.) Add a comment to the userspace ABI header that all of the data gets
out as little endian data, except for the timestamp which is CPU endian.
(We could make it little endian too, but why? Vice versa, an ioctl
could be added to dump packet data in big endian byte order...)
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Fix race between nosy_open() and remove_card() by replacing the
unprotected array of card pointers by a mutex-protected list of cards.
Make card instances reference-counted and let each client hold a
reference.
Notify clients about card removal via POLLHUP in poll()'s events
bitmap; also let read() fail with errno=ENODEV if the card was removed
and everything in the buffer was read.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
nosy_start/stop_snoop() and nosy_add/remove_client() are simple enough
to be inlined into their callers.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
nosy_start/stop_snoop() are always only called by the ioctl method, i.e.
with IRQs enabled. packet_handler() and bus_reset_handler() are always
only called by the IRQ handler. Hence neither one needs to track IRQ
flags.
To underline the call context of packet_handler() and
bus_reset_handler(), rename these functions to *_irq_handler().
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
nosy_stop_snoop() would blow up the second time it was called without
nosy_start_snoop() in between.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
The required serialization of NOSY_IOC_START and NOSY_IOC_STOP is
already provided by the client_list_lock.
NOSY_IOC_FILTER does not really require serialization since accesses
to tcode_mask are atomic on any sane CPU architecture. Nevertheless,
make it explicit that we want this to be atomic by means of
client_list_lock (which also surrounds the other tcode_mask access in
the IRQ handler). While we are at it, change the type of tcode_mask to
u32 for consistency with the user API.
NOSY_IOC_GET_STATS does not require serialization against itself. But
there is a bug here regarding concurrent updates of the two counters
by the IRQ handler. Fix it by taking the client_list_lock in this ioctl
too.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Extend copyright note to 2007, c.f. Kristian's git log.
Includes:
- replace some <asm/*.h> by <linux/*.h>
- add required indirectly included <linux/spinlock.h>
- order alphabetically
Coding style related changes:
- change to utf8
- normalize whitespace
- normalize comment style
- remove usages of __FUNCTION__
- remove an unnecessary cast from void *
Const and static declarations:
- driver_name is not const in pci_driver.name, drop const qualifier
- driver_name can be taken from KBUILD_MODNAME
- the global variable minors[] can and should be static
- constify struct file_operations instance
Data types:
- Remove unused struct member struct packet.code. struct packet is
only used for driver-internal bookkeeping; it does not appear on the
wire or in DMA programs or the userspace ABI. Hence the unused
member .code can be removed without worries.
Preprocessor macros:
- unroll a preprocessor macro that containd a return
- use list_for_each_entry
Printk:
- add missing terminating \n in some format strings
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
This adds the traffic sniffer driver for Texas Instruments PCILynx/
PCILynx2 based cards. The use cases for nosy are analysis of
nonstandard protocols and as an aid in development of drivers,
applications, or firmwares.
Author of the driver is Kristian Høgsberg. Known contributers are
Jody McIntyre and Jonathan Woithe.
Nosy programs PCILynx chips to operate in promiscuous mode, which is a
feature that is not found in OHCI-1394 controllers. Hence, only special
hardware as mentioned in the Kconfig help text is suitable for nosy.
This is only the kernelspace part of nosy. There is a userspace
interface to it, called nosy-dump, proposed to be added into the tools/
subdirectory of the kernel sources in a subsequent change. Kernelspace
and userspave component of nosy communicate via a 'misc' character
device file called /dev/nosy with a simple ioctl() and read() based
protocol, as described by nosy-user.h.
The files added here are taken from
git://anongit.freedesktop.org/~krh/nosy commit ee29be97 (2009-11-10)
with the following changes by Stefan Richter:
- Kconfig and Makefile hunks are written from scratch.
- Commented out version printk in nosy.c.
- Included missing <linux/sched.h>, reported by Stephen Rothwell.
"git shortlog nosy{-user.h,.c,.h}" from nosy's git repository:
Jonathan Woithe (2):
Nosy updates for recent kernels
Fix uninitialised memory (needed for 2.6.31 kernel)
Kristian Høgsberg (5):
Pull over nosy from mercurial repo.
Use a misc device instead.
Add simple AV/C decoder.
Don't break down on big payloads.
Set parent device for misc device.
As a low-level IEEE 1394 driver, its files are placed into
drivers/firewire/ although nosy is not part of the firewire driver
stack.
I am aware of the following literature from Texas Instruments about
PCILynx programming:
SCPA020A - PCILynx 1394 to PCI Bus Interface TSB12LV21BPGF
Functional Specification
SLLA023 - Initialization and Asynchronous Programming of the
TSB12LV21A 1394 Device
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Acked-by: Kristian Høgsberg <krh@bitplanet.net>
In both the ieee1394 stack and the firewire stack, the core treats
kernelspace drivers better than userspace drivers when it comes to
CSR address range allocation: The former may request a register to be
placed automatically at a free spot anywhere inside a specified address
range. The latter may only request a register at a fixed offset.
Hence, userspace drivers which do not require a fixed offset potentially
need to implement a retry loop with incremented offset in each retry
until the kernel does not fail allocation with EBUSY. This awkward
procedure is not fundamentally necessary as the core already provides a
superior allocation API to kernelspace drivers.
Therefore change the ioctl() ABI by addition of a region_end member in
the existing struct fw_cdev_allocate. Userspace and kernelspace APIs
work the same way now.
There is a small cost to pay by clients though: If client source code
is required to compile with older kernel headers too, then any use of
the new member fw_cdev_allocate.region_end needs to be enclosed by
#ifdef/#endif directives. However, any client program that seriously
wants to use address range allocations will require a kernel of cdev ABI
version >= 4 at runtime and a linux/firewire-cdev.h header of >= 4
anyway. This is because v4 brings FW_CDEV_EVENT_REQUEST2. The only
client program in which build-time compatibility with struct
fw_cdev_allocate as found in older kernel headers makes sense is
libraw1394.
(libraw1394 uses the older broken FW_CDEV_EVENT_REQUEST to implement a
makeshift, incorrect transaction responder that does at least work
somewhat in many simple scenarios, relying on guesswork by libraw1394
and by libraw1394 based applications. Plus, address range allocation
and transaction responder is only one of many features that libraw1394
needs to provide, and these other features need to work with kernel and
kernel-headers as old as possible. Any new linux/firewire-cdev.h based
client that implements a transaction responder should never attempt to
do it like libraw1394; instead it should make a header and kernel of v4
or later a hard requirement.)
While we are at it, update the struct fw_cdev_allocate documentation to
better reflect the recent fw_cdev_event_request2 ABI addition.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
region->end is defined as an upper bound of the requested address range,
exclusive --- i.e. as an address outside of the range in which the
requested CSR is to be placed.
Hence 0x0001,0000,0000,0000 is the biggest valid region->end, not
0x0000,ffff,ffff,fffc like the current check asserted.
For simplicity, the fix drops the region->end & 3 test because there is
no actual problem with these bits set in region->end. The allocated
address range will be quadlet aligned and of a size of multiple quadlets
due to the checks for region->start & 3 and handler->length & 3 alone.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
This extends the FW_CDEV_IOC_SEND_PHY_PACKET ioctl() for /dev/fw* to be
useful for ping time measurements. One application for it would be gap
count optimization in userspace that is based on ping times rather than
hop count. (The latter is implemented in firewire-core itself but is
not applicable to beta PHYs that act as repeater.)
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Add an FW_CDEV_IOC_RECEIVE_PHY_PACKETS ioctl() and
FW_CDEV_EVENT_PHY_PACKET_RECEIVED poll()/read() event for /dev/fw*.
This can be used to get information from remote PHYs by remote access
PHY packets.
This is also the 2nd half of the functionality (the receive part) to
support a userspace implementation of a VersaPHY transaction layer.
Safety considerations:
- PHY packets are generally broadcasts, hence some kind of elevated
privileges should be required of a process to be able to listen in
on PHY packets. This implementation assumes that a process that is
allowed to open the /dev/fw* of a local node does have this
privilege.
There was an inconclusive discussion about introducing POSIX
capabilities as a means to check for user privileges for these
kinds of operations.
Other limitations:
- PHY packet reception may be switched on by ioctl() but cannot be
switched off again. It would be trivial to provide an off switch,
but this is not worth the code. The client should simply close()
the fd then, or just ignore further events.
- For sake of simplicity of API and kernel-side implementation, no
filter per packet content is provided.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Add an FW_CDEV_IOC_SEND_PHY_PACKET ioctl() for /dev/fw* which can be
used to implement bus management related functionality in userspace.
This is also half of the functionality (the transmit part) that is
needed to support a userspace implementation of a VersaPHY transaction
layer.
Safety considerations:
- PHY packets are generally broadcasts and may have interesting
effects on PHYs and the bus, e.g. make asynchronous arbitration
impossible due to too low gap count. Hence some kind of elevated
privileges should be required of a process to be able to send
PHY packets. This implementation assumes that a process that is
allowed to open the /dev/fw* of a local node does have this
privilege.
There was an inconclusive discussion about introducing POSIX
capabilities as a means to check for user privileges for these
kinds of operations.
- The kernel does not check integrity of the supplied packet data.
That would be far too much code, considering the many kinds of
PHY packets. A process which got the privilege to send these
packets is trusted to do it correctly.
Just like with the other "send packet" ioctls, a non-blocking API is
chosen; i.e. the ioctl may return even before AT DMA started. After
transmission, an event for poll()/read() is enqueued. Most users are
going to need a blocking API, but a blocking userspace wrapper is easy
to implement, and the second of the two existing libraw1394 calls
raw1394_phy_packet_write() and raw1394_start_phy_packet_write() can be
better supported that way.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
core-transaction.c transmit_complete_callback() and close_transaction()
expect packet callback status to be an ACK or RCODE, and ACKs get
translated to RCODEs for transaction callbacks.
An old comment on the packet callback API (been there from the initial
submission of the stack) and the dummy_driver implementation of
send_request/send_response deviated from this as they also included
-ERRNO in the range of status values.
Let's narrow status values down to ACK and RCODE to prevent surprises.
RCODE_CANCELLED is chosen as the dummy_driver's RCODE as its meaning of
"transaction timed out" comes closest to what happens when a transaction
coincides with card removal.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Bus resets which are triggered
- by the kernel drivers after updates of the local nodes' config ROM,
- by userspace software via ioctl
shall be deferred until after >=2 seconds after the last bus reset.
If multiple modifications of the local nodes' config ROM happen in a row,
only a single bus reset should happen after them.
When the local node's link goes from inactive to active or vice versa,
and at the two occasions of bus resets mentioned above --- and if the
current gap count differs from 63 --- the bus reset should be preceded
by a PHY configuration packet that reaffirms the gap count. Otherwise a
bus manager would have to reset the bus again right after that.
This is necessary to promote bus stability, e.g. leave grace periods for
allocations and reallocations of isochronous channels and bandwidth,
SBP-2 reconnections etc.; see IEEE 1394 clause 8.2.1.
This change implements all of the above by moving bus reset initiation
into a delayed work (except for bus resets which are triggered by the
bus manager workqueue job and are performed there immediately). It
comes with a necessary addition to the card driver methods that allows
to get the current gap count from PHY registers.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
When a descriptor was added or removed to the local node's config ROM,
userspace clients which had a local node's /dev/fw* open did not receive
any fw_cdev_event_bus_reset for poll()/read() consumption.
The cause was that the core-device.c facility which re-reads the config
ROM of the bus reset initiator node missed to call the fw_device update
function. The fw_units are destroyed and newly added, but their parent
stays and needs to be updated.
Reported-by: Jay Fenlason <fenlason@redhat.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
The FW_ISO_ constants of the in-kernel API of firewire-core and
FW_CDEV_ISO_ constants of the userspace API of firewire-core have
nothing to do with each other --- except that the core-cdev.c
implementation relies on them having the same values.
Hence put some compile-time assertions into core-cdev.c. It's lame but
I prefer it over including the userspace API header into the kernelspace
API header and defining kernelspace API constants from userspace API
constants. Nor do I want to expose the kernelspace constants in one of
the two firewire headers that are exported to userland since this only
concerns the core-cdev.c implementation.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
The present inline documentation of the fw_send_request() in-kernel API
refers to userland code that is not applicable to kernel drivers at all.
Reported-by: Ben Gamari <bgamari.foss@gmail.com>
While we are at fixing the whole documentation of fw_send_request(),
also improve the rest of firewire-core's kerneldoc comments:
- Add a bit of text concerning fw_run_transaction()'s call parameters.
- Append () to function names and tab-align parameter descriptions as
suggested by the example in Documentation/kernel-doc-nano-HOWTO.txt.
- Remove kerneldoc markers from comments on static functions.
- Remove outdated parameter descriptions at build_tree().
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Check that the data length of a write quadlet request actually is large
enough for a quadlet. Otherwise, fw_fill_request could access the four
bytes after the end of the outbound_transaction_event structure.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Modification of Clemens' change: Consolidate the check into
init_request() which is used by the affected ioctl_send_request() and
ioctl_send_broadcast_request() and the unaffected
ioctl_send_stream_packet(), to save a few lines of code.
Note, since struct outbound_transaction_event *e is slab-allocated, such
an out-of-bounds access won't hit unallocated memory but may result in a
(virtually impossible to exploit) information disclosure.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Fix an obscure ABI feature that is a bit of a hassle to implement.
However, somebody put it into the ABI, so let's fill in a sensible
value there.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
The problem:
A target-like userspace driver, e.g. AV/C target or SBP-2/3 target,
needs to be able to act as responder and requester. In the latter role,
it needs to send requests to nods from which it received requests. This
is currently impossible because fw_cdev_event_request lacks information
about sender node ID.
Reported-by: Jay Fenlason <fenlason@redhat.com>
Libffado + libraw1394 + firewire-core is currently unable to drive two
or more audio devices on the same bus.
Reported-by: Arnold Krille <arnold@arnoldarts.de>
This is because libffado requires destination node ID of FCP requests
and sender node ID of FCP responses to match. It even prohibits
libffado from working with a bus on which libraw1394 opens a /dev/fw* as
default ioctl device that does not correspond with the audio device.
This is because libraw1394 does not receive the sender node ID from the
kernel.
Moreover, fw_cdev_event_request makes it impossible to tell unicast and
broadcast write requests apart.
The fix:
Add a replacement of struct fw_cdev_event_request request, boringly
called struct fw_cdev_event_request2. The new event will be sent to a
userspace client instead of the old one if the client claims
compatibility with <linux/firewire-cdev.h> ABI version 4 or later.
libraw1394 needs to be extended to make use of the new event, in order
to properly support libffado and other FCP or address range mapping
users who require correct sender node IDs.
Further notes:
While we are at it, change back the range of possible values of
fw_cdev_event_request.tcode to 0x0...0xb like in ABI version <= 3.
The preceding change "firewire: expose extended tcode of incoming lock
requests to (userspace) drivers" expanded it to 0x0...0x17 which could
catch sloppily coded clients by surprise. The extended range of codes
is only used in the new fw_cdev_event_request2.tcode.
Jay and I also suggested an alternative approach to fix the ABI for
incoming requests: Add an FW_CDEV_IOC_GET_REQUEST_INFO ioctl which can
be called after reception of an fw_cdev_event_request, before issuing of
the closing FW_CDEV_IOC_SEND_RESPONSE ioctl. The new ioctl would reveal
the vital information about a request that fw_cdev_event_request lacks.
Jay showed an implementation of this approach.
The former event approach adds 27 LOC of rather trivial code to
core-cdev.c, the ioctl approach 34 LOC, some of which is nontrivial.
The ioctl approach would certainly also add more LOC to userspace
programs which require the expanded information on inbound requests.
This approach is probably only on the lighter-weight side in case of
clients that want to be compatible with kernels that lack the new
capability, like libraw1394. However, the code to be added to such
libraw1394-like clients in case of the event approach is a straight-
forward additional switch () case in its event handler.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
When a remote device does a LOCK_REQUEST, the core does not pass
the extended tcode to userspace. This patch makes it use the
juju-specific tcodes listed in firewire-constants.h for incoming
requests.
Signed-off-by: Jay Fenlason <fenlason@redhat.com>
This matches how tcode in the API for outbound requests is treated.
Affects kernelspace and userspace drivers alike, but at the moment there
are no kernespace drivers that receive lock requests.
Split out from a combo patch, slightly reordered, changelog reworded.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
libraw1394 v2.0.0...v2.0.5 takes FW_CDEV_VERSION from an externally
installed header file and uses it to declare its own implementation
level in FW_CDEV_IOC_GET_INFO. This is wrong; it should set the real
version for which it was actually written.
If we add features to the kernel ABI that require the kernel to check
a client's implementation level, we can not trust the client version if
it was set from FW_CDEV_VERSION.
Hence freeze FW_CDEV_VERSION at the current value (no damage has been
done yet), clearly document FW_CDEV_VERSION as a dummy version and what
clients are expected to do with fw_cdev_get_info.version, and use a new
defined constant (which is not placed into the exported header file) as
kernel implementation level.
Note, in order to check in client program source code which features are
present in an externally installed linux/firewire-cdev.h, use
preprocessor directives like
#ifdef FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE
or
#ifdef FW_CDEV_EVENT_ISO_RESOURCE_ALLOCATED
instead of a check of FW_CDEV_VERSION.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
If a request comes in to an address range managed by a userspace driver
i.e. <linux/firewire-cdev.h> client, the card instance of request and
response may differ from the card instance of the client device.
Therefore we need to take a reference of the card until the response was
sent.
I thought about putting the reference counting into core-transaction.c,
but the various high-level drivers besides cdev clients (firewire-net,
firewire-sbp2, firedtv) use the card pointer in their fw_address_handler
address_callback method only to look up devices of which they already
hold the necessary references. So this seems to be a specific
firewire-cdev issue which is better addressed locally.
We do not need the reference
- in case of FCP_REQUEST or FCP_RESPONSE requests because then the
firewire-core will send the split transaction response for us
already in the context of the request handler,
- if it is the same card as the client device's because we hold a
card reference indirectly via teh client->device reference.
To keep things simple, we take the reference nevertheless.
Jay Fenlason wrote:
> there's no way for the core to tell cdev "this card is gone,
> kill any inbound transactions on it", while cdev holds the transaction
> open until userspace issues a SEND_RESPONSE ioctl, which may be a very,
> very long time. But when it does, it calls fw_send_response(), which
> will dereference the card...
>
> So how unhappy are we about userspace potentially holding a fw_card
> open forever?
While termination of inbound transcations at card removal could be
implemented, it is IMO not worth the effort. Currently, the effect of
holding a reference of a card that has been removed is to block the
process that called the pci_remove of the card. This is
- either a user process ran by root. Root can find and kill processes
that have /dev/fw* open, if desired.
- a kernel thread (which one?) in case of hot removal of a PCCard or
ExpressCard.
The latter case could be a problem indeed. firewire-core's card
shutdown and card release should probably be improved not to block in
shutdown, just to defer freeing of memory until release.
This is not a new problem though; the same already always happens with
the client->device->card without the need of inbound transactions or
other special conditions involved, other than the client not closing the
file.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
My box has two firewire cards in it: card0 and card1.
My application opens /dev/fw0 (card 0) and allocates an address space.
The core makes the address space available on both cards.
Along comes the remote device, which sends a READ_QUADLET_REQUEST to
card1. The request gets passed up to my application, which calls
ioctl_send_response().
ioctl_send_response() then calls fw_send_response() with card0,
because that's the card it's bound to.
Card0's driver drops the response, because it isn't part of
a transaction that it has outstanding.
So in core-cdev: handle_request(), we need to stash the
card of the inbound request in the struct inbound_transaction_resource and
use that card to send the response to.
The hard part will be refcounting the card correctly
so it can't get deallocated while we hold a pointer to it.
Here's a trivial patch, which does not do the card refcounting, but at
least demonstrates what the problem is.
Note that we can't depend on the fact that the core-cdev:client
structure holds a card open, because in this case the card it holds
open is not the card the request came in on.
..and there's no way for the core to tell cdev "this card is gone,
kill any inbound transactions on it", while cdev holds the transaction
open until userspace issues a SEND_RESPONSE ioctl, which may be a very,
very long time. But when it does, it calls fw_send_response(), which
will dereference the card...
So how unhappy are we about userspace potentially holding a fw_card
open forever?
Signed-off-by: Jay Fenlason <fenlason@redhat.com>
Reference counting to be addressed in a separate change.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> (whitespace)
Protect the client's iso context pointer against a race that can happen
when more than one creation call is executed at the same time.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
void (*fw_address_callback_t)(..., int speed, ...) is the speed that a
remote node chose to transmit a request to us. In case of split
transactions, firewire-core will transmit the response at that speed.
Upper layer drivers on the other hand (firewire-net, -sbp2, firedtv, and
userspace drivers) cannot do anything useful with that speed datum,
except log it for debug purposes. But data that is merely potentially
(not even actually) used for debug purposes does not belong into the API.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
All of the fields of the iso_interrupt_event instance are overwritten
right after it was allocated.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
which caused gcc 4.6 to warn about
variable 'destination' set but not used.
Since the hardware ensures that we receive only response packets with
proper destination node ID (in a given bus generation), we have no use
for destination here in the core as well as in upper layers.
(This is different with request packets. There we pass destination node
ID to upper layers because they may for example need to check whether
this was an unicast or broadcast request.)
Reported-and-Tested-By: Justin P. Mattock <justinmattock@gmail.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Rather than "read a Control and Status Registers (CSR) Architecture
register" I prefer to say "read a Control and Status Register".
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
All of these CSRs have the same read/ write/ aynthing-else handling,
except for CSR_PRIORITY_BUDGET which might not be implemented.
The CSR_CYCLE_TIME read handler implementation accepted 4-byte-sized
block write requests before this change but this is just silly; the
register is only required to support quadlet read and write requests
like the other r/w CSR core and Serial-Bus-dependent registers.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Push the maintenance of STATE_CLEAR/SET.abdicate down into the card
driver. This way, the read/write_csr_reg driver method works uniformly
across all CSR offsets.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
by feature variables in the fw_card struct. The hook appeared to be an
unnecessary abstraction in the card driver interface.
Cleaner would be to pass those feature flags as arguments to
fw_card_initialize() or fw_card_add(), but the FairnessControl register
is in the SCLK domain and may therefore not be accessible while Link
Power Status is off, i.e. before the card->driver->enable call from
fw_card_add().
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
In case of fw_card_bm_work()'s lock request, the present sizeof
expression is going to be wrong if somebody changes the fw_card's DMA
scratch buffer's size in the future.
In case of quadlet write requests, sizeof(u32) is just silly; it's 4.
In case of SBP-2 ORB pointer write requests, 8 is arguably quicker to
understand as the correct and only possible value than
sizeof(some_datum).
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Add a comment on which of the conflicting NODE_IDS specifications we
implement. Reduce a comment on rather irrelevant register bits that can
all be looked up in the spec (or from now on in the code history).
Directly include the required indirectly included bug.h.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
As part of the bus manager responsibilities, make sure that the cycle
master sends cycle start packets. This is needed when the old bus
manager disabled the cycle master's cmstr bit and there are iso-capable
nodes on the new bus.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
On OHCI 1.1 controllers, let the hardware allocate the broadcast channel
automatically. This removes a theoretical race condition directly after
a bus reset where it could be possible to read the channel allocation
register with channel 31 still being unallocated.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Implement the abdicate bit, which is required for bus manager
capable nodes and tested by the Base 1394 Test Suite.
Finally, something to do at a command reset! :-)
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Implement the cmstr bit, which is required for cycle master capable
nodes and tested for by the Base 1394 Test Suite.
This bit allows the bus master to disable cycle start packets; there are
bus master implementations that actually do this.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Implement the MAIN_UTILITY register, which is utterly optional
but useful as a safe target for diagnostic read/write/broadcast
transactions.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
If supported by the OHCI controller, implement the PRIORITY_BUDGET
register, which is required for nodes that can use asynchronous
priority arbitration.
To allow the core to determine what features the lowlevel device
supports, add a new card driver callback.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Implement the BUS_TIME register, which is required for cycle master
capable nodes and tested for by the Base 1393 Test Suite. Even when
there is not yet bus master initialization support, this register allows
us to work together with other bus masters.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
The specification requires that CYCLE_TIME is writable so that it can be
initialized, so we better implement it.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Implement the SPLIT_TIMEOUT registers. Besides being required by the
spec, this is desirable for some IIDC devices and necessary for many
audio devices to be able to increase the timeout from userspace.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
The NODE_IDS register, and especially its bus_id field, is quite
useless because 1394.1 requires that the bus_id field always stays
0x3ff. However, the 1394 specification requires this register on all
transaction capable nodes, and the Base 1394 Test Suite tests for it,
so we better implement it.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
To prepare for the following additions of more OHCI-implemented CSR
registers, replace the get_cycle_time driver callback with a generic
CSR register callback.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
The state registers are zero and read-only in this implementation, so
they are not of much use. However, the specification requires that they
are present for transaction capable nodes, and the Base 1394 Test Suite
tests for them, so we better implement them.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
When the candidate bus manager fails to do the lock request with which
it tries to become bus manager, it assumes that the current IRM is not
actually IRM capable and forces itself to become root. However, if that
lock request failed because the local node itself was not able to send
it, then we cannot blame the current IRM and should not steal its
rootness.
In this case, RCODE_SEND_ERROR is likely to indicate a temporary error
condition such as exhausted tlabels or low memory, so we better try
again later.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Most PHY chips, when idle, can complete a register access in the time
needed for two or three PCI read transactions; bigger delays occur only
when data is currently being moved over the link/PHY interface. So if
we busy-wait a few times when waiting for the register access to finish,
it is likely that we can finish without having to sleep.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Add a check that the data length in the SEND_RESPONSE ioctl is correct.
Incidentally, this also fixes the previously wrong response length of
software-handled lock requests.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
This patch adds support for message-signaled interrupts.
Any native PCI-Express OHCI controller should support MSI, but most are
just PCI cores behind a PCI-E/PCI bridge. The only chips that are known
to claim to support MSI are the Lucent/Agere/LSI FW643 and the VIA
VT6315, none of which I have been able to test.
Due to the high level of trust I have in the competence of these and any
future chip makers, I thought it a good idea to add a disable-MSI quirk.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Tested Agere FW643 rev 07 [11c1:5901] and JMicron JMB381 [197b:2380].
Added a quirks list entry for JMB38X since it kept its count of MSI
events consistently at zero.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
On 26 Apr 2010, Clemens Ladisch wrote:
> In theory, none of the interrupts should occur before the link is
> enabled. In practice, I'd rather make sure to not set the master
> interrupt enable bit until we have installed the interrupt handler.
and proposed to move OHCI1394_masterIntEnable out of the present
reg_write() into a new one before the HCControl.linkEnable reg_write().
Why not defer setting /all/ of the bits until right before linkEnable?
Reviewed-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Per IEEE 1394 clause 8.4.2.3, a contender for the IRM role shall check
whether the current IRM complies to 1394a-2000 or later. If not force a
compliant node (e.g. itself) to become IRM. This was implemented in the
older ieee1394 driver but not yet in firewire-core.
An older Sony camcorder (Sony DCR-TRV25) which implements 1394-1995 IRM
but neither 1394a-2000 IRM nor BM was now found to cause an
interoperability bug:
- Camcorder becomes root node when plugged in, hence gets IRM role.
- firewire-core successfully contends for BM role, proceeds to perform
gap count optimization and resets the bus.
- Sony camcorder ignores presence of a BM (against the spec, this is
a firmware bug), performs its idea of gap count optimization and
resets the bus.
- Preceding two steps are repeated endlessly, bus never settles,
regular I/O is practically impossible.
http://thread.gmane.org/gmane.linux.kernel.firewire.user/3913
This is an interoperability regression from the old to the new drivers.
Fix it indirectly by adding the 1394a IRM check. The spec suggests
three and a half methods to determine 1394a compliance of a remote IRM;
we choose the method of testing the Config_ROM.Bus_Info.generation
field. This is data that firewire-core should have readily available at
this point, i.e. does not require extra I/O.
Reported-by: Clemens Ladisch <clemens@ladisch.de> (missing 1394a check)
Reported-by: H. S. <hs.samix@gmail.com> (issue with Sony DCR-TRV25)
Tested-by: H. S. <hs.samix@gmail.com>
Cc: <stable@kernel.org> # .32.x and newer
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
* 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6:
ieee1394: schedule for removal
firewire: core: use separate timeout for each transaction
firewire: core: Fix tlabel exhaustion problem
firewire: core: make transaction label allocation more robust
firewire: core: clean up config ROM related defined constants
ieee1394: mark char device files as not seekable
firewire: cdev: mark char device files as not seekable
firewire: ohci: cleanups and fix for nonstandard build without debug facility
firewire: ohci: wait for PHY register accesses to complete
firewire: ohci: fix up configuration of TI chips
firewire: ohci: enable 1394a enhancements
firewire: ohci: do not clear PHY interrupt status inadvertently
firewire: ohci: add a function for reading PHY registers
Trivial conflicts in Documentation/feature-removal-schedule.txt
* 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jikos/trivial: (44 commits)
vlynq: make whole Kconfig-menu dependant on architecture
add descriptive comment for TIF_MEMDIE task flag declaration.
EEPROM: max6875: Header file cleanup
EEPROM: 93cx6: Header file cleanup
EEPROM: Header file cleanup
agp: use NULL instead of 0 when pointer is needed
rtc-v3020: make bitfield unsigned
PCI: make bitfield unsigned
jbd2: use NULL instead of 0 when pointer is needed
cciss: fix shadows sparse warning
doc: inode uses a mutex instead of a semaphore.
uml: i386: Avoid redefinition of NR_syscalls
fix "seperate" typos in comments
cocbalt_lcdfb: correct sections
doc: Change urls for sparse
Powerpc: wii: Fix typo in comment
i2o: cleanup some exit paths
Documentation/: it's -> its where appropriate
UML: Fix compiler warning due to missing task_struct declaration
UML: add kernel.h include to signal.c
...
Using a single timeout for all transaction that need to be flushed does
not work if the submission of new transactions can defer the timeout
indefinitely into the future. We need to have timeouts that do not
change due to other transactions; the simplest way to do this is with a
separate timer for each transaction.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> (+ one lockdep annotation)
fw_core_handle_response() was not properly clearing tlabel_mask. This
was resulting in premature tlabel exhaustion.
Signed-off-by: Peter Hurley <phurley@charter.net>
This fixes an omission in 2.6.31-rc1 commit 1e626fdc "firewire: core:
use more outbound tlabels" which prevented to really use 64 instead of
32 transaction labels, as soon as split transactions occurred that had
their AR-resp tasklet run after the AT-req tasklet.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
* 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6:
firewire: ohci: wait for local CSR lock access to finish
firewire: ohci: prevent aliasing of locally handled register addresses
firewire: core: fw_iso_resource_manage: return -EBUSY when out of resources
firewire: core: fix retries calculation in iso manage_channel()
firewire: cdev: fix cut+paste mistake in disclaimer
If one request is so long-lived that it does not get a response before
the following 63 requests, its bit in tlabel_mask is still set when the
next request tries to allocate a transaction label for that number. In
this state, while the first request is not completed or timed out, no
new requests can be submitted.
To fix this, skip over any label still in use, and do not error out
unless we have entirely run out of labels.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Clemens Ladisch pointed out that
- BIB_IMC is not named like the field is called in the standard,
- readers of the code may get worried about the magic 0x0c0083c0,
- a CSR_NODE_CAPABILITIES key is there in the header but not put to
good use.
So let's rename BIB_IMC, add a defined constant for Node_Capabilities
and a comment which reassures people that somebody thought about it and
they don't have to (or if they still do, tell them where they have to
look for confirmation), and prune our incomplete and arbitrary set of
defined constants of CSR key IDs. And there is a nother magic number,
that of Bus_Information_Block.Bus_Name, to be defined and commented.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Add a loop to wait for the controller to finish a locally-initiated CSR
lock operation. Google shows some occurrences of the "swap not done
yet" message which might indicate that some OHCI controllers are not
fast enough to do the lock/swap in the time needed for one PCI access.
This also correctly handles the case where the lock operation did not
finish, instead of silently returning an uninitialized value.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
We must compute the offset from the CSR register base with the
full 48 address bits to prevent matching with addresses whose
lower 32 bits happen to be equal with one of the specially
handled registers.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Returning -EIO for all errors would not allow clients to determine if
the resource allocation process itself failed, or if the resources are
not available. (The latter information is needed by CMP to synchronize
restoring of overlayed connections after a bus reset.)
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
If there is a permanent error condition when communicating with the IRM,
after the sixth error, the retry variable will be decremented to -1.
If, in this case, the bits in channels_mask are not yet exhausted, the
next channel is retried 2^32 times.
To fix this, check that retry is never decremented beyond zero.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
The <linux/firewire-cdev.h> character device file ABI (i.e. /dev/fw*
character device file interface) does not make any use of lseek(),
pread(), pwrite() (or any kind of write() at all).
Use nonseekable_open() and, redundantly, set file_operations.llseek to
no_llseek to remove any doubt whether the BKL-grabbing default_llseek
handler is used. (Also shuffle file_operations initialization according
to the order of handler definitions.)
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
1) Clean up two function names: The ohci_ prefix is only used in names
of fw_card_driver hooks. There were two unnecessary exceptions.
2) Replace empty macros by empty inline functions so that call parameter
type checking is available in #ifndef'd builds.
3) CONFIG_FIREWIRE_OHCI_DEBUG is currently a hidden kconfig variable,
hence is not going to be switched off by anybody. Still, it can be
switched off but then compilation will fail in ohci_enable() at the
expression param_debug & OHCI_PARAM_DEBUG_BUSRESETS. Add the necessary
definitions in the nonstandard case.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Rather than having the arbitrary msleep(2) pause, let read_phy_reg()
loop until the link--phy access was finished.
Factor write_phy_reg() out of ohci_update_phy_reg() and of
read_paged_phy_reg() and let it loop too until the link--phy access was
finished.
Like in the older ohci1394 driver, a timeout of 100 milliseconds is
chosen. Unlike the old driver, we sleep instead of busy-wait in each
waiting loop iteration. Instead of a loop, the waiting could probably
also be implemented interrupt driven, but why bother. It would require
up and running interrupt handling before the link was fully configured
and enabled.
Also modify functions a bit: Error return and value return can be
combined in read_phy_reg() since the domain of values is only u8.
Likewise in read_paged_phy_reg().
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
On TI chips (OHCI-Lynx and later), enable link enhancements features
that TI recommends to be used. None of these are required for proper
operation, but they are safe and nice to have.
In theory, these bits should have been set by default, but in practice,
some BIOS/EEPROM writers apparently do not read the datasheet, or get
spooked by names like "unfair".
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
The OHCI spec says that, if the programPhyEnable bit is set, the driver
is responsible for configuring the IEEE1394a enhancements within the PHY
and the link consistently. So do this.
Also add a quirk to allow disabling these enhancements; this is needed
for the TSB12LV22 where ack accelerations are buggy (erratum b).
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
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>
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>
A userspace client got to see uninitialized stack-allocated memory if it
specified an _IOC_READ type of ioctl and an argument size larger than
expected by firewire-core's ioctl handlers (but not larger than the
core's union ioctl_arg).
Fix this by clearing the requested buffer size to zero, but only at _IOR
ioctls. This way, there is almost no runtime penalty to legitimate
ioctls. The only legitimate _IOR is FW_CDEV_IOC_GET_CYCLE_TIMER with 12
or 16 bytes to memset.
[Another way to fix this would be strict checking of argument size (and
possibly direction) vs. command number. However, we then need a lookup
table, and we need to allow for slight size deviations in case of 32bit
userland on 64bit kernel.]
Reported-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
The definition of struct fw_cdev_iso_packet seems to imply that the
header_length must be quadlet-aligned, and in fact, specifying an
unaligned header has never really worked when using multiple packet
structures, because the position of the next control word is computed by
rounding the header_length _down_, so the last one to three bytes of the
header would overlap the next control word.
To avoid this problem, check that the header length is properly aligned.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
In receive contexts, reject packets with header_length==0. This would
be an instruction to queue zero packets which would not make sense.
This prevents a division by zero in the OHCI driver.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
percpu.h is included by sched.h and module.h and thus ends up being
included when building most .c files. percpu.h includes slab.h which
in turn includes gfp.h making everything defined by the two files
universally available and complicating inclusion dependencies.
percpu.h -> slab.h dependency is about to be removed. Prepare for
this change by updating users of gfp and slab facilities include those
headers directly instead of assuming availability. As this conversion
needs to touch large number of source files, the following script is
used as the basis of conversion.
http://userweb.kernel.org/~tj/misc/slabh-sweep.py
The script does the followings.
* Scan files for gfp and slab usages and update includes such that
only the necessary includes are there. ie. if only gfp is used,
gfp.h, if slab is used, slab.h.
* When the script inserts a new include, it looks at the include
blocks and try to put the new include such that its order conforms
to its surrounding. It's put in the include block which contains
core kernel includes, in the same order that the rest are ordered -
alphabetical, Christmas tree, rev-Xmas-tree or at the end if there
doesn't seem to be any matching order.
* If the script can't find a place to put a new include (mostly
because the file doesn't have fitting include block), it prints out
an error message indicating which .h file needs to be added to the
file.
The conversion was done in the following steps.
1. The initial automatic conversion of all .c files updated slightly
over 4000 files, deleting around 700 includes and adding ~480 gfp.h
and ~3000 slab.h inclusions. The script emitted errors for ~400
files.
2. Each error was manually checked. Some didn't need the inclusion,
some needed manual addition while adding it to implementation .h or
embedding .c file was more appropriate for others. This step added
inclusions to around 150 files.
3. The script was run again and the output was compared to the edits
from #2 to make sure no file was left behind.
4. Several build tests were done and a couple of problems were fixed.
e.g. lib/decompress_*.c used malloc/free() wrappers around slab
APIs requiring slab.h to be added manually.
5. The script was run on all .h files but without automatically
editing them as sprinkling gfp.h and slab.h inclusions around .h
files could easily lead to inclusion dependency hell. Most gfp.h
inclusion directives were ignored as stuff from gfp.h was usually
wildly available and often used in preprocessor macros. Each
slab.h inclusion directive was examined and added manually as
necessary.
6. percpu.h was updated not to include slab.h.
7. Build test were done on the following configurations and failures
were fixed. CONFIG_GCOV_KERNEL was turned off for all tests (as my
distributed build env didn't work with gcov compiles) and a few
more options had to be turned off depending on archs to make things
build (like ipr on powerpc/64 which failed due to missing writeq).
* x86 and x86_64 UP and SMP allmodconfig and a custom test config.
* powerpc and powerpc64 SMP allmodconfig
* sparc and sparc64 SMP allmodconfig
* ia64 SMP allmodconfig
* s390 SMP allmodconfig
* alpha SMP allmodconfig
* um on x86_64 SMP allmodconfig
8. percpu.h modifications were reverted so that it could be applied as
a separate patch and serve as bisection point.
Given the fact that I had only a couple of failures from tests on step
6, I'm fairly confident about the coverage of this conversion patch.
If there is a breakage, it's likely to be something in one of the arch
headers which should be easily discoverable easily on most builds of
the specific arch.
Signed-off-by: Tejun Heo <tj@kernel.org>
Guess-its-ok-by: Christoph Lameter <cl@linux-foundation.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Lee Schermerhorn <Lee.Schermerhorn@hp.com>
* 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6:
firewire: core: align driver match with modalias
firewire: core: fix Model_ID in modalias
firewire: ohci: add cycle timer quirk for the TI TSB12LV22
firewire: core: fw_iso_resource_manage: fix error handling
The driver match strategy was:
- Match vendor/model/specifier/version of the unit directory.
- If that was a miss, match vendor from the root directory and
model/specifier/version of the unit directory.
This was inconsistent with how the modalias string was constructed
until recently (take vendor/model from root directory and specifier/
version from unit directory). It was also inconsistent with how it is
done since the parent commit:
- Use vendor/model/specifier/version of the unit directory if possible,
- fall back to one or more of vendor/model/specifier/version from the
root directory depending on which ones are not present at the unit
directory.
Fix this inconsistency by sharing the ROM scanner function between
modalias printer function and driver match function.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
The modalias string of devices that represent units on a FireWire node
did not show Module_ID entries within unit directories. This was
because firewire-core searched only the root directory of the
configuration ROM for a Model_ID entry.
We now search first the root directory, then the unit directory. IOW
honor a unit directory's Model_ID if present, otherwise fall back to the
root directory's model ID (if present).
Furthermore, apply the same change to Vendor_ID. This had the same
issue but it was less apparent because most devices provide Vendor_ID
only in the root directory.
And finally, also use this strategy for the remaining two IDs in the
modalias, Specifier_ID and Version. It does not actually make sense to
look for them elsewhere than in the unit directory because they are
mandatory there. However, a uniform search order simplifies the
implementation and has no adverse affect in practice.
Side notes:
- The older counterpart of this, nodemgr.c of ieee1394, looked for
Vendor_ID first in the root directory, then in the unit directory,
and for Model_ID only in the unit directory.
- There is a single mainline driver which requires Vendor_ID and
Model_ID --- the firedtv driver. This one worked because FireDTVs
provide Vendor_ID in the root directory and Model_ID identically in
root directory and unit directory.
- Apart from firedtv, there are currently no drivers known to me
(including userspace drivers) that look at the Vendor_ID or Model_ID
of the modalias.
Reported-by: Maciej Żenczykowski <zenczykowski@gmail.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
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)
If the bandwidth allocation fails, the error must be returned in
*channel regardless of whether the channel allocation succeeded.
Checking for c >= 0 is not correct if no channel allocation was
requested, in which case this part of the code is reached with
c == -EINVAL.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
In the future, we are going to be changing the lock type for struct
device (once we get the lockdep infrastructure properly worked out) To
make that changeover easier, and to possibly burry the lock in a
different part of struct device, let's create some functions to lock and
unlock a device so that no out-of-core code needs to be changed in the
future.
This patch creates the device_lock/unlock/trylock() functions, and
converts all in-tree users to them.
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Jean Delvare <khali@linux-fr.org>
Cc: Dave Young <hidave.darkstar@gmail.com>
Cc: Ming Lei <tom.leiming@gmail.com>
Cc: Jiri Kosina <jkosina@suse.cz>
Cc: Phil Carmody <ext-phil.2.carmody@nokia.com>
Cc: Arjan van de Ven <arjan@linux.intel.com>
Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
Cc: Rafael J. Wysocki <rjw@sisk.pl>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Len Brown <len.brown@intel.com>
Cc: Magnus Damm <damm@igel.co.jp>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Randy Dunlap <randy.dunlap@oracle.com>
Cc: Stefan Richter <stefanr@s5r6.in-berlin.de>
Cc: David Brownell <dbrownell@users.sourceforge.net>
Cc: Vegard Nossum <vegard.nossum@gmail.com>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Alex Chiang <achiang@hp.com>
Cc: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andrew Patterson <andrew.patterson@hp.com>
Cc: Yu Zhao <yu.zhao@intel.com>
Cc: Dominik Brodowski <linux@dominikbrodowski.net>
Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: Wolfram Sang <w.sang@pengutronix.de>
Cc: CHENG Renquan <rqcheng@smu.edu.sg>
Cc: Oliver Neukum <oliver@neukum.org>
Cc: Frans Pop <elendil@planet.nl>
Cc: David Vrabel <david.vrabel@csr.com>
Cc: Kay Sievers <kay.sievers@vrfy.org>
Cc: Sarah Sharp <sarah.a.sharp@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
* 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6: (23 commits)
firewire: ohci: extend initialization log message
firewire: ohci: fix IR/IT context mask mixup
firewire: ohci: add module parameter to activate quirk fixes
firewire: ohci: use an ID table for quirks detection
firewire: ohci: reorder struct fw_ohci for better cache efficiency
firewire: ohci: remove unused dualbuffer IR code
firewire: core: combine a bit of repeated code
firewire: core: change type of a data buffer
firewire: cdev: increment ABI version number
firewire: cdev: add more flexible cycle timer ioctl
firewire: core: rename an internal function
firewire: core: fix an information leak
firewire: core: increase stack size of config ROM reader
firewire: core: don't fail device creation in case of too large config ROM blocks
firewire: core: fix "giving up on config rom" with Panasonic AG-DV2500
firewire: remove incomplete Bus_Time CSR support
firewire: get_cycle_timer optimization and cleanup
firewire: ohci: enable cycle timer fix on ALi and NEC controllers
firewire: ohci: work around cycle timer bugs on VIA controllers
firewire: make PCI device id constant
...
The block layer calling convention is blk_queue_<limit name>.
blk_queue_max_sectors predates this practice, leading to some confusion.
Rename the function to appropriately reflect that its intended use is to
set max_hw_sectors.
Also introduce a temporary wrapper for backwards compability. This can
be removed after the merge window is closed.
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Jens Axboe <jens.axboe@oracle.com>
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>
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>
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>
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>
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>
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>
from array of char to union of structs. I already used a union to size
the buffer which holds ioctl arguments; more consequent is to define it
as an instance of this union in the first place.
Also rename several local variables from "request" to "a"(rgument) since
the term request can be mistaken to mean a transaction subaction, e.g.
an instance of struct fw_request.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
The system time from CLOCK_REALTIME is not monotonic, hence problematic
for the main user of the FW_CDEV_IOC_GET_CYCLE_TIMER ioctl. This issue
exists in its successor ABI, i.e. raw1394, too.
http://subversion.ffado.org/ticket/242
We now offer an alternative ioctl which lets the caller choose between
CLOCK_REALTIME, CLOCK_MONOTONIC, and CLOCK_MONOTONIC_RAW as source of
the local time, very similar to the clock_gettime libc function. The
format of the local time return value matches that of clock_gettime
(seconds and nanoseconds, instead of a single microseconds value from
the existing ioctl).
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
If a device exposes a sparsely populated configuration ROM,
firewire-core's sysfs interface and character device file interface
showed random data in the gaps between config ROM blocks. Fix this by
zero-initialization of the config ROM reader's scratch buffer.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
The stack size of 16 was artificially chosen and may be too small in
extreme cases. A device won't be accessible then.
Since it doesn't really matter to the slab allocator whether we ask for
1088 bytes or 2048 bytes of scratch memory, just allocate 2048 bytes for
the sum of temporary config ROM image and stack, and we will never ever
overflow the stack (because there simply can't be more stack items than
ROM entries).
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
It never happened yet, but better safe than sorry: If a device's config
ROM contains a block which overlaps the boundary at 0xfffff00007ff, just
ignore that one block instead of refusing to add the device
representation. That way, upper layers (kernelspace or userspace
drivers) might still be able to use the device to some degree.
That's better than total inaccessibility of the device. Worse, the core
would have logged only a generic "giving up on config rom" message which
could only be debugged by feeding a firewire-ohci debug logging session
through a config ROM interpreter, IOW would likely remain undiagnosed.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
The Panasonic AG-DV2500 tape deck contains an invalid entry in its
configuration ROM root directory: A leaf pointer with the undefined key
ID 0 and an offset that points way out of the standard config ROM area.
This caused firewire-core to dismiss the device with the generic log
message "giving up on config rom for node id...", after which it was of
course impossible to access the tape deck with dvgrab or any other
program. https://bugzilla.redhat.com/show_bug.cgi?id=449252#c29
The fix is to simply ignore this invalid ROM entry and proceed to read
the valid rest of the ROM. There is a catch though: When the kernel
later iterates over the ROM, it would be nasty having to check again for
such too large ROM offsets. Therefore we manipulate the defective or
unsupported ROM entry to become a harmless immediate entry that won't
have any side effects later (an entry with the value 0x00000000).
Reported-by: George Chriss
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
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>
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>
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>
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>
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)
In the transmit path of firewire-net (IPv4 over 1394), the following
race condition may occur:
- The networking soft IRQ inserts a datagram into the 1394 async
request transmit DMA.
- The 1394 async transmit completion tasklet runs to finish cleaning
up (unlink datagram from list of pending ones, release skb and
outbound 1394 transaction object) --- before the networking soft IRQ
had a chance to proceed and add the datagram to the list of pending
datagrams.
This caused a panic in the 1394 async transmit completion tasklet when
it dereferenced unitialized list heads:
http://bugzilla.kernel.org/show_bug.cgi?id=15077
The fix is to add checks in the tx soft IRQ and in the tasklet to
determine which of these two is the last referrer to the transaction
object. Then handle the cleanup of the object by the last referrer
rather than assuming that the tasklet is always the last one.
There is another similar race: Between said tasklet and fwnet_close,
i.e. at ifdown. However, that race is much less likely to occur in
practice and shall be fixed in a separate update.
Reported-by: Илья Басин <basinilya@gmail.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
