The 1394 OHCI driver allocates several non-coherent DMA buffers for AR
request and response contexts. The buffers are mapped to kernel virtual
address (VMA) so that the first page locates after the last page. Even
when large payload of packet is handled crossing the boundary of buffers,
the driver operates continuously on VMA.
No kernel API is provided for this kind of mapping, while it is possible
to release the buffer when PCI device is going to be released.
This commit moves the call of release helper function to the callback
function of release resources.
Link: https://lore.kernel.org/r/20230604054451.161076-10-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
The 1394 OHCI driver allocates DMA coherent buffer to transfer content
of configuration ROM.
This commit utilizes managed device resource to maintain the lifetime of
buffer.
Link: https://lore.kernel.org/r/20230604054451.161076-9-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
The 1394 OHCI driver allocates DMA coherent buffer for descriptors of IT,
IR, AT receive, and AT request contexts by the same way.
This commit utilizes managed device resource to maintain the lifetime of
buffers.
Link: https://lore.kernel.org/r/20230604054451.161076-8-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
The 1394 OHCI driver allocates the list of isochronous contexts as much
as the hardware supports.
This commit utilizes managed device resource to maintain the lifetime of
list.
Link: https://lore.kernel.org/r/20230604054451.161076-7-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
The 1394 OHCI driver allocates a DMA coherent buffer for multi-purposes.
The buffer is split into three region for specific purposes; i.e. 1/4 for
context descriptors of AR request and response as well as 1/2 for self
ID handling.
This commit uses managed device resource to maintain the lifetime of
buffer.
Link: https://lore.kernel.org/r/20230604054451.161076-5-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
The PCI framework has the convenient helper function to check and map MMIO
region with managed device resource.
This commit elaborates 1394 OHCI driver to use the function.
Link: https://lore.kernel.org/r/20230604054451.161076-4-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
The managed device resource (devres) framework is convenient to maintain
lifetime of allocated memory object for device.
This commit utilizes the framework for the object of ohci structure. The
extra operation for power management is required in Apple PowerMac based
machines, thus release callback is assigned to the object to call the
operation.
Link: https://lore.kernel.org/r/20230604054451.161076-2-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
In 1394 OHCI, the OUTPUT_LAST descriptor of Asynchronous Transmit (AT)
request context has timeStamp field, in which 1394 OHCI controller
record the isochronous cycle when the packet was sent for the request
subaction. Additionally, for the case of split transaction in IEEE 1394,
Asynchronous Receive (AT) request context is used for response subaction
to finish the transaction. The trailer quadlet of descriptor in the
context has timeStamp field, in which 1394 OHCI controller records the
isochronous cycle when the packet arrived.
Current implementation of 1394 OHCI controller driver stores values of
both fields to internal structure as time stamp, while Linux FireWire
subsystem provides no way to access to it. When using asynchronous
transaction service provided by the subsystem, callback function is passed
to kernel API. The prototype of callback function has the lack of argument
for the values.
This commit adds a new callback function for the purpose. It has an
additional argument to point to the constant array with two elements. For
backward compatibility to kernel space, a new union is also adds to wrap
two different prototype of callback function. The fw_transaction structure
has the union as a member and a boolean flag to express which function
callback is available.
The core function is changed to handle the two cases; with or without
time stamp. For the error path to process transaction, the isochronous
cycle is computed by current value of CYCLE_TIMER register in 1394 OHCI
controller. Especially for the case of timeout of split transaction, the
expected isochronous cycle is computed.
Link: https://lore.kernel.org/r/20230529113406.986289-6-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
tasklet_disable() is invoked in several places. Some of them are in atomic
context which prevents a conversion of tasklet_disable() to a sleepable
function.
The atomic callchains are:
ar_context_tasklet()
ohci_cancel_packet()
tasklet_disable()
...
ohci_flush_iso_completions()
tasklet_disable()
The invocation of tasklet_disable() from at_context_flush() is always in
preemptible context.
Use tasklet_disable_in_atomic() for the two invocations in
ohci_cancel_packet() and ohci_flush_iso_completions().
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lore.kernel.org/r/20210309084242.616379058@linutronix.de
Use dma_alloc_pages to allocate DMAable pages instead of hoping that
the architecture either has GFP_DMA32 or not more than 4G of memory.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Using uninitialized_var() is dangerous as it papers over real bugs[1]
(or can in the future), and suppresses unrelated compiler warnings
(e.g. "unused variable"). If the compiler thinks it is uninitialized,
either simply initialize the variable or make compiler changes.
In preparation for removing[2] the[3] macro[4], remove all remaining
needless uses with the following script:
git grep '\buninitialized_var\b' | cut -d: -f1 | sort -u | \
xargs perl -pi -e \
's/\buninitialized_var\(([^\)]+)\)/\1/g;
s:\s*/\* (GCC be quiet|to make compiler happy) \*/$::g;'
drivers/video/fbdev/riva/riva_hw.c was manually tweaked to avoid
pathological white-space.
No outstanding warnings were found building allmodconfig with GCC 9.3.0
for x86_64, i386, arm64, arm, powerpc, powerpc64le, s390x, mips, sparc64,
alpha, and m68k.
[1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
[2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
[3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
[4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
Reviewed-by: Leon Romanovsky <leonro@mellanox.com> # drivers/infiniband and mlx4/mlx5
Acked-by: Jason Gunthorpe <jgg@mellanox.com> # IB
Acked-by: Kalle Valo <kvalo@codeaurora.org> # wireless drivers
Reviewed-by: Chao Yu <yuchao0@huawei.com> # erofs
Signed-off-by: Kees Cook <keescook@chromium.org>
There is a regular need in the kernel to provide a way to declare having a
dynamically sized set of trailing elements in a structure. Kernel code should
always use “flexible array members”[1] for these cases. The older style of
one-element or zero-length arrays should no longer be used[2].
[1] https://en.wikipedia.org/wiki/Flexible_array_member
[2] https://github.com/KSPP/linux/issues/21
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
The ohci driver uses the get_seconds() function to implement the 32-bit
CSR_BUS_TIME register. This was added in 2010 commit a48777e03a
("firewire: add CSR BUS_TIME support").
As get_seconds() returns a 32-bit value (on 32-bit architectures), it
seems like a good fit for that register, but it is also deprecated because
of the y2038/y2106 overflow problem, and should be replaced throughout
the kernel with either ktime_get_real_seconds() or ktime_get_seconds().
I'm using the latter here, which uses monotonic time. This has the
advantage of behaving better during concurrent settimeofday() updates
or leap second adjustments and won't overflow a 32-bit integer, but
the downside of using CLOCK_MONOTONIC instead of CLOCK_REALTIME is
that the observed values are not related to external clocks.
If we instead need UTC but can live with clock jumps or overflows,
then we should use ktime_get_real_seconds() instead, retaining the
existing behavior.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Link: https://lore.kernel.org/lkml/20180711124923.1205200-1-arnd@arndb.de/
Reviewed-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Based on 1 normalized pattern(s):
this program is free software you can redistribute it and or modify
it under the terms of the gnu general public license as published by
the free software foundation either version 2 of the license or at
your option any later version this program is distributed in the
hope that it will be useful but without any warranty without even
the implied warranty of merchantability or fitness for a particular
purpose see the gnu general public license for more details you
should have received a copy of the gnu general public license along
with this program if not write to the free software foundation inc
59 temple place suite 330 boston ma 02111 1307 usa
extracted by the scancode license scanner the SPDX license identifier
GPL-2.0-or-later
has been chosen to replace the boilerplate/reference in 1334 file(s).
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Allison Randal <allison@lohutok.net>
Reviewed-by: Richard Fontana <rfontana@redhat.com>
Cc: linux-spdx@vger.kernel.org
Link: https://lkml.kernel.org/r/20190527070033.113240726@linutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
mmiowb() is now implied by spin_unlock() on architectures that require
it, so there is no reason to call it from driver code. This patch was
generated using coccinelle:
@mmiowb@
@@
- mmiowb();
and invoked as:
$ for d in drivers include/linux/qed sound; do \
spatch --include-headers --sp-file mmiowb.cocci --dir $d --in-place; done
NOTE: mmiowb() has only ever guaranteed ordering in conjunction with
spin_unlock(). However, pairing each mmiowb() removal in this patch with
the corresponding call to spin_unlock() is not at all trivial, so there
is a small chance that this change may regress any drivers incorrectly
relying on mmiowb() to order MMIO writes between CPUs using lock-free
synchronisation. If you've ended up bisecting to this commit, you can
reintroduce the mmiowb() calls using wmb() instead, which should restore
the old behaviour on all architectures other than some esoteric ia64
systems.
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Will Deacon <will.deacon@arm.com>
- make JMicron JMB38x controllers work with IOMMU-equipped systems
- IP-over-1394: allow user-configured MTU of up to 4096 bytes
-----BEGIN PGP SIGNATURE-----
iQJOBAABCAA4FiEElVwAmOXEbvmhUkgUefNvslRdedAFAlp0aGYaHHN0ZWZhbnJA
czVyNi5pbi1iZXJsaW4uZGUACgkQefNvslRdedDOHg/+NBs+6uv7SDZ2HXfQ6EQs
ytoAaPMjrSWztHWrz3JM/Q1CECgwYJhq3pWqTCIho2CXE/PFJF0YvqSgg45buzW1
sBOsJ6Sn5yIFTcHw9IDcTjI73lNLjEyBuft7KzHswCYbNbftOaebVE+19D24wOCb
MspCcrqqQehmlHLrmhJmkv9n4kUEx7v/ZT5lXy4gXxO3/NvQ02Vq7AblbNX1/mcy
Lf7ONqWi4cjY5sp49KuR0tZJZmclYodUnqkCSSRsLGtJSj/oacMufqk9/LU80Oc4
0J9Xzf4kRGWxmdO7TqLRu4632oZeXjX/q57IsKJOcjIqPX/FDAWOdBadaEEVOSLy
AOwpXQ3GOl9SC8HmSptTSzMedRoxszwZ7NdbLvkuydoPa2mpTyxHYhOf1gy6639g
fa0iQ8LdzDIjI1R8bka9SWVmUlxaOFKbYMqhuWk+ugoG4QtZKpCdXs4MFOmSyWGX
kAtanLXKijA1iVrr1Kn2Cf2MJlvjuYfG00X4dzKIsIEvZHgwXs2mYua5wNHy4eLe
KrKC3WBGDkW81Zokk9FFvdsECvCc1W4z/bLIOWrGsMa3OLvA0jiZTVydQXMdZyHd
GL2LtpwfCbuRUZkt/ScJ3ELyXDULviaBu/mlX4Ppc2c15JW/RSmwr3S/W/n0HL6j
pe9TTDYRTnPpScJ2eI7NlYY=
=FQUh
-----END PGP SIGNATURE-----
Merge tag 'firewire-updates' of git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394
Pull firewire updates from Stefan Richter
- make JMicron JMB38x controllers work with IOMMU-equipped systems
- IP-over-1394: allow user-configured MTU of up to 4096 bytes
* tag 'firewire-updates' of git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394:
firewire-ohci: work around oversized DMA reads on JMicron controllers
firewire: net: max MTU off by one
At least some JMicron controllers issue buggy oversized DMA reads when
fetching context descriptors, always fetching 0x20 bytes at once for
descriptors which are only 0x10 bytes long. This is often harmless, but
can cause page faults on modern systems with IOMMUs:
DMAR: [DMA Read] Request device [05:00.0] fault addr fff56000 [fault reason 06] PTE Read access is not set
firewire_ohci 0000:05:00.0: DMA context IT0 has stopped, error code: evt_descriptor_read
This works around the problem by always leaving 0x10 padding bytes at
the end of descriptor buffer pages, which should be harmless to do
unconditionally for controllers in case others have the same behavior.
Signed-off-by: Hector Martin <marcan@marcan.st>
Reviewed-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Please do not apply this to mainline directly, instead please re-run the
coccinelle script shown below and apply its output.
For several reasons, it is desirable to use {READ,WRITE}_ONCE() in
preference to ACCESS_ONCE(), and new code is expected to use one of the
former. So far, there's been no reason to change most existing uses of
ACCESS_ONCE(), as these aren't harmful, and changing them results in
churn.
However, for some features, the read/write distinction is critical to
correct operation. To distinguish these cases, separate read/write
accessors must be used. This patch migrates (most) remaining
ACCESS_ONCE() instances to {READ,WRITE}_ONCE(), using the following
coccinelle script:
----
// Convert trivial ACCESS_ONCE() uses to equivalent READ_ONCE() and
// WRITE_ONCE()
// $ make coccicheck COCCI=/home/mark/once.cocci SPFLAGS="--include-headers" MODE=patch
virtual patch
@ depends on patch @
expression E1, E2;
@@
- ACCESS_ONCE(E1) = E2
+ WRITE_ONCE(E1, E2)
@ depends on patch @
expression E;
@@
- ACCESS_ONCE(E)
+ READ_ONCE(E)
----
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: davem@davemloft.net
Cc: linux-arch@vger.kernel.org
Cc: mpe@ellerman.id.au
Cc: shuah@kernel.org
Cc: snitzer@redhat.com
Cc: thor.thayer@linux.intel.com
Cc: tj@kernel.org
Cc: viro@zeniv.linux.org.uk
Cc: will.deacon@arm.com
Link: http://lkml.kernel.org/r/1508792849-3115-19-git-send-email-paulmck@linux.vnet.ibm.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
software_reset() may fail
- due to unresponsive chip with -EBUSY (-16), or
- due to ejected or unseated card with -ENODEV (-19).
Let the PCI probe and resume routines log the actual error code instead
of hardwired -EBUSY.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Reported by Clifford and Craig for JMicron OHCI-1394 + SDHCI combo
controllers: Often or even most of the time, the controller is
initialized with the message "added OHCI v1.10 device as card 0, 4 IR +
0 IT contexts, quirks 0x10". With 0 isochronous transmit DMA contexts
(IT contexts), applications like audio output are impossible.
However, OHCI-1394 demands that at least 4 IT contexts are implemented
by the link layer controller, and indeed JMicron JMB38x do implement
four of them. Only their IsoXmitIntMask register is unreliable at early
access.
With my own JMB381 single function controller I found:
- I can reproduce the problem with a lower probability than Craig's.
- If I put a loop around the section which clears and reads
IsoXmitIntMask, then either the first or the second attempt will
return the correct initial mask of 0x0000000f. I never encountered
a case of needing more than a second attempt.
- Consequently, if I put a dummy reg_read(...IsoXmitIntMaskSet)
before the first write, the subsequent read will return the correct
result.
- If I merely ignore a wrong read result and force the known real
result, later isochronous transmit DMA usage works just fine.
So let's just fix this chip bug up by the latter method. Tested with
JMB381 on kernel 3.13 and 4.3.
Since OHCI-1394 generally requires 4 IT contexts at a minium, this
workaround is simply applied whenever the initial read of IsoXmitIntMask
returns 0, regardless whether it's a JMicron chip or not. I never heard
of this issue together with any other chip though.
I am not 100% sure that this fix works on the OHCI-1394 part of JMB380
and JMB388 combo controllers exactly the same as on the JMB381 single-
function controller, but so far I haven't had a chance to let an owner
of a combo chip run a patched kernel.
Strangely enough, IsoRecvIntMask is always reported correctly, even
though it is probed right before IsoXmitIntMask.
Reported-by: Clifford Dunn
Reported-by: Craig Moore <craig.moore@qenos.com>
Cc: stable@vger.kernel.org
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Remove the function ar_prev_buffer_index() that is not used anywhere.
This was partially found by using a static code analysis program called cppcheck.
Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
vm_map_ram() is intended for short-lived objects, so using it for the AR
buffers could fragment address space, especially on a 32-bit machine.
For an allocation that lives as long as the device, vmap() is the better
choice.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
with some isochronous workloads (regression since v3.16-rc1).
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQIcBAABAgAGBQJT02gRAAoJEHnzb7JUXXnQJDcQAOQ7w0+/QNR9xQUuj4Kmn8Kw
paUJ0ccGa5LdfTZIThbVPdn3R5mdKgCYVookRbSktmAXo8C7IzLsGk8hw35SdA7g
Ap638MKIfY1+tNajG4fRZGptIRf04SzJpH3qYtApZQIaXZ3NWS9zHhobyOX/wFIf
HyiorPp6PEy1BBGdnMavS71v3L04oQ84eg2bUyWvi2sKT1RHO1pcHTl25FgXARsX
k1a35waw2fiP7USlYWdq+dNFnKxirlij9P1Ma72stLY16/NoxDiLRHs+hQTWBfM2
BsHtx739THyTYA0rpsO7U7YypsxM+SihnwJAC0ZRk760FsCrWBvOIcjHuqP3Wh1j
vzydxLYTpOCtdrRJ1epZpC96BSBgq2+N8vf4dNC/MOxX4/O8im80jGf5QpMuARxu
MqLkTfIeFPirvXv2PNtWvomeAzDTvyaH/OYxh6G8389yIRD9hs9kxc0bjyfXbPlh
I/Yl7XhhQq3Gbq09th3rMy6LZUW9IW50w9HHDeGVJWwsltMV0cR2Wm9R4nWcRmtB
TxGuGAxiwFI13eEcJvtHf/CB2JvR24t51kmje/0s+ag4KOw7ByDGZH60K3y14tb0
9vVisqfputElv0rz7heW+gYFi9+3Jo0NGGRDTUoJ4wTTJvCUyoUsiXTXl3ryc+ux
tayMjniMBsPKLA/iKbuX
=xNu7
-----END PGP SIGNATURE-----
Merge tag 'firewire-fix-vt6315' of git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394
Pull firewire regression fix from Stefan Richter:
"IEEE 1394 (FireWire) subsystem fix: MSI don't work on VIA PCIe
controllers with some isochronous workloads (regression since
v3.16-rc1)"
* tag 'firewire-fix-vt6315' of git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394:
firewire: ohci: disable MSI for VIA VT6315 again
Revert half of commit d151f9854f21: If isochronous I/O is attempted with
packets larget than 1 kByte, VIA VT6315 rev 01 immediately stops to generate
any interrupts if MSI are used. Fix this by going back to legacy interrupts.
[Thread "Isochronous streaming with VT6315 OHCI",
http://marc.info/?t=139049641500003]
With smaller packets, the loss of IRQs happens too but only very rarely ---
rarely eneough that it was not yet possible for me to determine whether
QUIRK_NO_MSI is an actual fix for this rare variation of this chip bug.
I am keeping QUIRK_CYCLE_TIMER off of VT6315 rev >= 1 because this has been
verified by myself with certainty. On the other hand, I am also keeping
QUIRK_CYCLE_TIMER on for VT6315 rev 0 because I don't know at this time
whether this revision accesses Cycle Timer non-atomically like most of the
other VIA OHCIs are known to do.
Reported-by: Rémy Bruno <remy-fw@remy.trinnov.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
One optimization for some VIA controllers, one fix, one kconfig brushup.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)
iQIcBAABAgAGBQJTjjgrAAoJEHnzb7JUXXnQK7wP/0SaXBk/sWSaxn0JM50xRv2e
90rt2V2QFrlEuk/kNOJHJIgMdIhl+z14caCMxNhXnlHVD6gPnLUAXPCLrf1Kth0R
kcpQ+nILuigMWm8GnyA1o/pr62J/gyF47xvceHqadkPEsESRT3allWe5kCtRLasT
FRBd/tAhcdtZlsbjZS8qsCbZeg6T38P2w3kHlvx7gTt87UTD1NkqJOwKhxcTAd3C
YK+T6XG9RsHJDv6mfQNp29RTQbkrkUewjoD62sqoDQkb1VgGnNanr1YlBvtwrrvl
MYAR6tsaH2fV9fb3ztM6iERuG3zLBsB6t5IImGL5R7AnAxQx4rDby0qmUVismrMA
AkDI1XDBoyhAcfF+1qPlCAvQOmbIoVo3BH428WREf33Nq4RV4lkTwKb+jq+RKOJ4
2Z5DdRrbiaimwChkbznk93tLAF4FiqzX8NI5BQnGHWB7ZD1LsteJRgcDRYu90Fxr
dR16y4mnbWTwPcxZprQcDMF5NH8KhAGkL7A1K0nbz5ZblDtBkbqwRdwZ6xhs/RSc
NBcnR+eMXp3KRLqN7qnD8lIon8EezW0tn46EBTkI+iGWd+mLR4FMu6spF2gcwOjM
tXF+YotZ08H5NHCVSKe2GudDedPjf/gPKJ6z2+bXrwtaXvtfLS8cNYK7jpTBfrq5
zR/2fhPPf4iCy5vuSwQY
=27Wn
-----END PGP SIGNATURE-----
Merge tag 'firewire-updates' of git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394 into next
Pull firewire updates from Stefan Richter:
"IEEE 1394 (FireWire) subsystem changes: One optimization for some VIA
controllers, one fix, one kconfig brushup"
* tag 'firewire-updates' of git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394:
firewire: ohci: enable MSI for VIA VT6315 rev 1, drop cycle timer quirk
firewire: Use COMPILE_TEST for build testing
firewire: net: fix NULL derefencing in fwnet_probe()
Commit af0cdf4947 "firewire: ohci: fix regression with VIA VT6315,
disable MSI" acted upon a report against VT6315 rev 0:
http://linux.derkeiler.com/Mailing-Lists/Kernel/2010-12/msg02301.html
$ lspci -nn
VIA Technologies, Inc. VT6315 Series Firewire Controller [1106:3403]
I now got a card with
$ lspci -nn
VIA Technologies, Inc. VT6315 Series Firewire Controller [1106:3403] (rev 01)
and this works fine with MSI enabled.
Second, I tested this VT6315 rev 1 without CYCLE_TIMER quirk flag using
http://me.in-berlin.de/~s5r6/linux1394/utils/test_cycle_time_v20100125.c
and found that this chip does in fact access the cycle timer atomically.
Things I can't test because I don't have the hardware:
- whether VT6315 rev 0 really needs QUIRK_CYCLE_TIMER,
- whether the VT6320 PCI device needs QUIRK_CYCLE_TIMER,
- whether the VT6325 and VT6330 PCIe devices need QUIRK_CYCLE_TIMER
and QUIRK_NO_MSI.
Hence, just add a whitelist entry specifically for VT6315 rev >= 1
without any quirk flags. Before this entry we need an extra entry to
catch VT6315 rev <= 0 due to how our ID matching logic works.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Undo a feature introduced in v3.14 by commit fcd46b3442
"firewire: Enable remote DMA above 4 GB". That change raised the
minimum address at which protocol drivers and user programs can register
for request reception from 0x0001'0000'0000 to 0x8000'0000'0000.
It turned out that at least one vendor-specific protocol exists which
uses lower addresses: https://bugzilla.kernel.org/show_bug.cgi?id=76921
For the time being, revert most of commit fcd46b3442 so that affected
protocols work like with kernel v3.13 and before. Just keep the valid
documentation parts from the regressing commit, and the ability to
identify controllers which could be programmed to accept >32 bit
physical DMA addresses. The rest of fcd46b3442 should probably be
brought back as an optional instead of default feature.
Reported-by: Fabien Spindler <fabien.spindler@inria.fr>
Cc: <stable@vger.kernel.org> # 3.14+
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Since commit bd972688eb
"firewire: ohci: Fix 'failed to read phy reg' on FW643 rev8",
there is a high chance that firewire-ohci fails to initialize LSI née
Agere controllers.
https://bugzilla.kernel.org/show_bug.cgi?id=65151
Peter Hurley points out the reason: IEEE 1394a:2000 clause 5A.1 (or
IEEE 1394:2008 clause 17.2.1) say: "The PHY shall insure that no more
than 10 ms elapse from the reassertion of LPS until the interface is
reset. The link shall not assert LReq until the reset is complete."
In other words, the link needs to give the PHY at least 10 ms to get
the interface operational.
With just the msleep(1) in bd972688eb, the first read_phy_reg()
during ohci_enable() may happen before the phy-link interface reset was
finished, and fail. Due to the high variability of msleep(n) with small
n, this failure was not fully reproducible, and not apparent at all with
low CONFIG_HZ setting.
On the other hand, Peter can no longer reproduce the issue with FW643
rev8. The read phy reg failures that happened back then may have had an
unrelated cause. So, just revert bd972688eb, except for the valid
comment on TSB82AA2 cards.
Reported-by: Mikhail Gavrilov
Reported-by: Jay Fenlason <fenlason@redhat.com>
Reported-by: Clemens Ladisch <clemens@ladisch.de>
Reported-by: Peter Hurley <peter@hurleysoftware.com>
Cc: stable@vger.kernel.org # v3.10+
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
This makes all of a machine's memory accessible to remote debugging via
FireWire, using the physical response unit (i.e. RDMA) of OHCI-1394 link
layer controllers.
This requires actual support by the controller. The only ones currently
known to support it are Agere/LSI FW643. Most if not all other OHCI-1394
controllers do not implement the optional Physical Upper Bound register.
With them, RDMA will continue to be limited to the lowermost 4 GB.
firewire-ohci's startup message in the kernel log is augmented to tell
whether the controller does expose more than 4 GB to RDMA.
While OHCI-1394 allows for a maximum Physical Upper Bound of
0xffff'0000'0000 (near 256 TB), this implementation sets it to
0x8000'0000'0000 (128 TB) in order to avoid interference with applications
that require interrupt-served asynchronous request reception at
respectively low addresses.
Note, this change does not switch remote DMA on. It only increases the
range of remote access to all memory (instead of just 4 GB) whenever
remote DMA was switched on by other means. The latter is achieved by
setting firewire-ohci's remote_dma parameter, or if the physical DMA
filter is opened through firewire-sbp2.
Derived from patch "firewire: Enable physical DMA above 4GB" by
Peter Hurley <peter@hurleysoftware.com> from March 27, 2013.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
This makes it possible to debug kernel over FireWire without the need to
recompile it.
[Stefan R: changed description from "...0" to "...N"]
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Put bus_reset_work into its own workqueue. By doing this, forward
progress of bus_reset_work() is guaranteed if the work is switched over
to a rescuer thread.
Switching work to a rescuer thread happens if a new worker thread could
not be allocated in certain time (MAYDAY_INITIAL_TIMEOUT, typically 10
ms). This might not be possible under high memory pressure or even on a
heavily loaded embedded system running a slow serial console.
The former deadlock occured in the following situation:
The rescuer thread ran
fw_device_init->read_config_rom->read_rom->fw_run_transaction.
fw_run_transaction blocked waiting for the completion object.
This completion object would have been completed in bus_reset_work,
but this work was never executed in the rescuer thread due to its
strictly sequential behaviour.
[Stefan R.: Removed WQ_NON_REENTRANT flag from allocation because
it is no longer needed in current kernels. Add it back if you backport
to kernels older than 3.7, i.e. one which does not contain dbf2576e37
"workqueue: make all workqueues non-reentrant". Swapped order of
destroy_workqueue and pci_unregister_driver.]
Signed-off-by: Stephan Gatzka <stephan.gatzka@gmail.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
This is a prerequisite to allocate a per driver self_id workqueue.
This reverts the ohci.c part of patch
fe2af11c22.
Signed-off-by: Stephan Gatzka <stephan.gatzka@gmail.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
a) Sort device IDs by vendor -- device -- revision.
b) Write quirk flags in hexadecimal. This affects the user-visible
output of "modinfo firewire-ohci". Since more flags have been added
recently, it is now easier to cope with them in hexadecimal represen-
tation. Besides, the device-specific combination of quirk flags is
shown in hexadecimal in the kernel log too. (And firewire-sbp2
presents its own quirk flags in modinfo as hexadecimals as well.)
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
We have got
struct descriptor *descriptors;
dma_addr_t descriptors_bus;
dma_addr_t buffer_bus;
struct descriptor buffer[0];
void *misc_buffer;
dma_addr_t misc_buffer_bus;
__be32 *config_rom;
dma_addr_t config_rom_bus;
__be32 *next_config_rom;
dma_addr_t next_config_rom_bus;
But then we have got
__le32 *self_id_cpu;
dma_addr_t self_id_bus;
Better apply the pattern of xyz vs. xyz_bus to self_id vs. self_id_bus
as well. The _cpu suffix looks particularly weird in conversions from
little endian to CPU endian.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Commit 18d627113b (firewire: prevent dropping of completed iso packet
header data) was intended to be an obvious bug fix, but libdc1394 and
FlyCap2 depend on the old behaviour by ignoring all returned information
and thus not noticing that not all packets have been received yet. The
result was that the video frame buffers would be saved before they
contained the correct data.
Reintroduce the old behaviour for old clients.
Tested-by: Stepan Salenikovich <stepan.salenikovich@gmail.com>
Tested-by: Josep Bosch <jep250@gmail.com>
Cc: <stable@vger.kernel.org> # 3.4+
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
A stack trace is an invaluable tool in determining the basis
and cause of PHY regs read/write failures.
Include PHY reg addr (and value for writes) in the diagnostic.
[Stefan R: changed whitespace]
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Many of the error messages possible from bus_reset_work() do not
contain enough information to distinguish which error condition
occurred nor enough information to evaluate the error afterwards.
Differentiate all error conditions in bus_reset_work(); add
additional information to make error diagnosis possible.
[Stefan R: fixed self-ID endian conversion]
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Convert dev_xxxx(ohci->card.device, ...) log functions to
ohci_xxxx(ohci, ...).
[Stefan R: Peter argues that this increases readability of the code.]
[Stefan R: changed whitespace]
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Add quirk for VT6306 wake bit behavior.
VT6306 seems to reread the wrong descriptor when the wake bit is
written. work around by putting a copy of the branch address in the
first descriptor of the block.
[Stefan R: This fixes the known broken video reception via gstreamer
on VIA VT6306. 100% repeatable testcase:
$ gst-launch-0.10 dv1394src \! dvdemux \! dvdec \! xvimagesink
with a camcorder or other DV source connected. Likewise for MPEG2-TS
reception via gstreamer, e.g. from TV settop boxes.
Perhaps this also fixes dv4l on VT6306, but this is as yet untested.
Kino, dvgrab or FFADO had not been affected by this chip quirk.
Additional comments from Andy:]
I've looked into some problems with the wake bit on a vt6306 family
chip (1106:3044, rev 46).
I used this firewire card in a mythtv setup (ISO receive MPEG2 stream)
with Debian 2.6.32 kernels for ~2 years without problems.
Since upgrading to 3.2, I've been having problems with the input stream
freezing -- input data stops until I restart mythtv (I expect closing
and reopening the device would be sufficient). This happens
infrequently, maybe one out of 20 recordings. I eventually determined
that the problem is more likely to occur if the system is loaded.
I isolated the kernel version as the triggering SW factor and then
specifically the change from dualbuffer back to packet-per-buffer DMA
mode.
The possibility that the controller does not properly respond to the
wake bit was suggested in
https://bugzilla.redhat.com/show_bug.cgi?id=415841, but not proven.
Based on the fact that dualbuffer mode worked while packet-per-buffer
has trouble, I guessed that upon seeing the wake bit written, the vt6306
controller only checks the branch address in the first descriptor of the
block, even if that is not the correct place to look (because the block
has multiple descriptors).
This theory seems to be correct. When the ISO reception is hung, I am
able to resume it by manually writing the branch address to the first
descriptor in the block, and then writing the wake bit.
I've had luck so far with the attached patch, so I'm including it. It's
probably not a complete solution -- I haven't tested transmit modes to
see whether they have a similar issue.
I doubt that the quirk test is any cheaper than just writing the extra
branch address in all cases, but it does reduce the risk of breaking
other hardware.
[Stefan R: omitted QUIRK_NO_MSI from VT6306 quirks table entry,
changed whitespace]
Signed-off-by: Andy Leiserson <andy@leiserson.org>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
A pci device can be removed while in its suspended state. If the ohci
host controller is suspended, the PHY is also in low-power mode and
LPS is disabled. If LPS is disabled, most of the host registers aren't
accessible, including IntMaskClear. Furthermore, access to these registers
when LPS is disabled can cause hard lockups on some hardware. Since
interrupts are already disabled in this mode, further action is
unnecessary.
Test LPS before attempting to write IntMaskClear to disable interrupts.
[Stefan R: whitespace changes]
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
A pci device can be removed while in its suspended state.
Because the ohci driver freed the irq to suspend, free_irq() is
called twice; once from pci_remove() and again from pci_suspend(),
which issues the warning below [1].
Rather than allocate the irq in the .enable() path, move the
allocation to .probe(). Consequently, the irq is not reallocated
upon pci_resume() and thus is not freed upon pci_suspend().
[1] Warning reported by Mark Einon <mark.einon@gmail.com> when
suspending an MSI MS-1727 GT740 laptop on Ubuntu 3.5.0-22-generic
WARNING: at ./kernel/irq/manage.c:1198 __free_irq+0xa3/0x1e0()
Hardware name: MS-1727
Trying to free already-free IRQ 16
Modules linked in: ip6table_filter ip6_tables ebtable_nat ebtables <...snip...>
Pid: 4, comm: kworker/0:0 Tainted: P O 3.5.0-22-generic #34-Ubuntu
Call Trace:
[<ffffffff81051c1f>] warn_slowpath_common+0x7f/0xc0
[<ffffffff81051d16>] warn_slowpath_fmt+0x46/0x50
[<ffffffff8103fa39>] ? default_spin_lock_flags+0x9/0x10
[<ffffffff810df6b3>] __free_irq+0xa3/0x1e0
[<ffffffff810df844>] free_irq+0x54/0xc0
[<ffffffffa005a27e>] pci_remove+0x6e/0x210 [firewire_ohci]
[<ffffffff8135ae7f>] pci_device_remove+0x3f/0x110
[<ffffffff8141fdbc>] __device_release_driver+0x7c/0xe0
[<ffffffff8141fe4c>] device_release_driver+0x2c/0x40
[<ffffffff8141f5f1>] bus_remove_device+0xe1/0x120
[<ffffffff8141cd1a>] device_del+0x12a/0x1c0
[<ffffffff8141cdc6>] device_unregister+0x16/0x30
[<ffffffff81354784>] pci_stop_bus_device+0x94/0xa0
[<ffffffffa0091c67>] acpiphp_disable_slot+0xb7/0x1a0 [acpiphp]
[<ffffffffa0090716>] ? get_slot_status+0x46/0xc0 [acpiphp]
[<ffffffffa0091d7d>] acpiphp_check_bridge.isra.15+0x2d/0xf0 [acpiphp]
[<ffffffffa0092442>] _handle_hotplug_event_bridge+0x372/0x4d0 [acpiphp]
[<ffffffff81390f8c>] ? acpi_os_execute_deferred+0x2f/0x34
[<ffffffff8116e22d>] ? kfree+0xed/0x110
[<ffffffff8107086a>] process_one_work+0x12a/0x420
[<ffffffffa00920d0>] ? _handle_hotplug_event_func+0x1d0/0x1d0 [acpiphp]
[<ffffffff8107141e>] worker_thread+0x12e/0x2f0
[<ffffffff810712f0>] ? manage_workers.isra.26+0x200/0x200
[<ffffffff81075f13>] kthread+0x93/0xa0
[<ffffffff8168d024>] kernel_thread_helper+0x4/0x10
[<ffffffff81075e80>] ? kthread_freezable_should_stop+0x70/0x70
[<ffffffff8168d020>] ? gs_change+0x13/0x13
Reported-by: Mark Einon <mark.einon@gmail.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
The old IEEE 1394 driver stack was removed in v2.6.37. That made the
checks for two Kconfig (module) macros unneeded, since they will now
always evaluate to true. Remove these two checks.
Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Takashi Sakamoto