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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
Control of more than one AV/C device at once --- e.g. camcorders, tape
decks, audio devices, TV tuners --- failed or worked only unreliably,
depending on driver implementation. This affected kernelspace and
userspace drivers alike and was caused by firewire-core's inability to
accept multiple registrations of FCP listeners.
The fix allows multiple address handlers to be registered for the FCP
command and response registers. When a request for these registers is
received, all handlers are invoked, and the Firewire response is
generated by the core and not by any handler.
The cdev API does not change, i.e., userspace is still expected to send
a response for FCP requests; this response is silently ignored.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> (changelog, rebased, whitespace)
I was told that there are obscure architectures with non-coherent DMA
which may DMA-map to bus address 0. We shall not use 0 as a magic
number of uninitialized bus address variables.
The packet->payload_length > 0 test cannot be used either (except in
at_context_queue_packet) because local requests are not DMA-mapped
regardless of payload_length. Hence add a state flag to struct
fw_packet.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
In the code path which creates request packets, clearly mark a switch
branch which must never be reached with a WARN.
In the code path which creates response packets, replace a BUG by a
friendlier to debug WARN.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
The Topology Map of the local node was created in CPU byte order,
then a temporary big endian copy was created to compute the CRC,
and when a read request to the Topology Map arrived it had to be
converted to big endian byte order again.
We now generate it in big endian byte order in the first place.
This also rids us of 1000 bytes stack usage in tasklet context.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
This register is 1 kBytes large. Adjust topology_map.length to prevent
registration of other response handlers in this region and to make sure
that we respond to requests to the upper half of the register.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Tlabel is a 6 bits wide datum. Wrap it after 63 rather than 31 for more
safety against transaction label exhaustion and potential responders'
transaction layer bugs. (As noted by Guus Sliepen, this change requires
an expansion of tlabel_mask to 64 bits.)
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
The source files of firewire-core, firewire-ohci, firewire-sbp2, i.e.
"drivers/firewire/fw-*.c"
are renamed to
"drivers/firewire/core-*.c",
"drivers/firewire/ohci.c",
"drivers/firewire/sbp2.c".
The old fw- prefix was redundant to the directory name. The new core-
prefix distinguishes the files according to which driver they belong to.
This change comes a little late, but still before further firewire
drivers are added as anticipated RSN.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Clemens Ladisch