09 Oct, 2011

1 commit


11 May, 2011

3 commits

  • 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

    Stefan Richter
     
  • 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
    Reviewed-by: Tejun Heo

    Stefan Richter
     
  • We do not need slab allocations anymore in order to satisfy
    streaming DMA mapping constraints, thanks to commit da28947e7e36
    "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
    Acked-by: Clemens Ladisch

    Stefan Richter
     

04 Jan, 2011

1 commit

  • 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
    Signed-off-by: Stefan Richter

    Clemens Ladisch
     

14 Dec, 2010

1 commit

  • 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
    Signed-off-by: Stefan Richter

    Clemens Ladisch
     

12 Dec, 2010

1 commit


20 Aug, 2010

1 commit

  • 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
    Signed-off-by: Clemens Ladisch
    Signed-off-by: Stefan Richter

    Clemens Ladisch
     

23 Jul, 2010

3 commits

  • 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

    Stefan Richter
     
  • 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

    Stefan Richter
     
  • 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

    Stefan Richter
     

13 Jul, 2010

2 commits

  • 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

    Stefan Richter
     
  • 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

    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

    Stefan Richter
     

21 Jun, 2010

2 commits

  • 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

    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

    Jay Fenlason
     
  • 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

    Stefan Richter
     

19 Jun, 2010

6 commits

  • 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
    Signed-off-by: Stefan Richter

    Stefan Richter
     
  • 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

    Stefan Richter
     
  • 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

    Stefan Richter
     
  • 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

    Stefan Richter
     
  • 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

    Stefan Richter
     
  • 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

    Stefan Richter
     

10 Jun, 2010

14 commits


19 May, 2010

2 commits

  • 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
    Signed-off-by: Stefan Richter (+ one lockdep annotation)

    Clemens Ladisch
     
  • fw_core_handle_response() was not properly clearing tlabel_mask. This
    was resulting in premature tlabel exhaustion.

    Signed-off-by: Peter Hurley

    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

    Peter Hurley
     

20 Apr, 2010

1 commit

  • 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
    Signed-off-by: Stefan Richter

    Clemens Ladisch
     

21 Feb, 2010

1 commit

  • 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

    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

    Stefan Richter
     

30 Dec, 2009

1 commit

  • 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
    Signed-off-by: Stefan Richter (changelog, rebased, whitespace)

    Clemens Ladisch