21 Jun, 2010

2 commits

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

    Clemens Ladisch
     
  • 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
     

20 Jun, 2010

1 commit


19 Jun, 2010

8 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
     
  • which caused gcc 4.6 to warn about
    variable 'XYZ' set but not used.

    sbp2.c, unit_characteristics:

    The underlying problem which was spotted here --- an incomplete
    implementation --- is already 50% fixed in drivers/firewire/sbp2.c which
    observes mgt_ORB_timeout but not yet ORB_size.

    raw1394.c, length_conflict; dv1394.c, ts_off:

    Impossible to tell why these variables are there. We can safely remove
    them though because we don't need a compiler warning to realize that we
    are dealing with (at least stylistically) flawed code here.

    dv1394.c, packet_time:

    This was used in debug macro that is only compiled in with
    DV1394_DEBUG_LEVEL >= 2 defined at compile-time. Just drop it since
    nobody debugs dv1394 anymore. Avoids noise in regular kernel builds.

    dv1394.c, ohci; eth1394.c, priv:

    These variables clearly can go away. Somebody wanted to use them but
    then didn't (or not anymore).

    Note, all of this code is considered to be at its end of life and is
    thus not really meant to receive janitorial updates anymore. But if we
    can easily remove noisy warnings from kernel builds, we should.

    Reported-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
     
  • 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

    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

20 commits

  • 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
     
  • 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
     
  • 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
     
  • 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
     
  • 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
     
  • 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
     
  • Implement the BUSY_TIMEOUT register, which is required for nodes that
    support retries.

    Signed-off-by: Clemens Ladisch

    Clemens Ladisch
     
  • 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
     
  • 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
     
  • 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
     
  • This implements the RESET_START register (as a dummy) to make the Base
    1394 Test Suite happy.

    Signed-off-by: Clemens Ladisch

    Clemens Ladisch
     
  • 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
     
  • 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
     
  • 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
     
  • 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
     
  • 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
     
  • WARN's format string argument should not carry a printk level prefix.

    Signed-off-by: Stefan Richter

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

    Clemens Ladisch
     
  • 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

    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

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

    Stefan Richter
     

01 Jun, 2010

1 commit


26 May, 2010

1 commit

  • All application domains that are supported by the old ieee1394 driver
    stack are supported by the newer firewire driver stack too. There is
    now good and extensive experience with the newer stack from deployment
    in Fedora since F7 as well as by enthusiast users of other
    distributions.

    The new drivers have consequently been recommended as the default ones
    since 2.6.33, in order to fix some severe usability problems of FireWire
    on Linux due to limitations of the old stack. It is now high time to
    announce when the obsolete drivers will be removed.

    Signed-off-by: Stefan Richter
    Acked-by: Jarod Wilson

    Stefan Richter
     

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

2 commits

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

    Stefan Richter
     

10 Apr, 2010

3 commits

  • The
    - raw1394 (/dev/raw1394),
    - video1394 (/dev/video1394/*),
    - dv1394 (/dev/dv1394/*)
    character device file ABIs do not make any use of lseek(), pread(), or
    pwrite(). Therefore 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.

    Although all this is legacy code which should be left in peace until it
    is eventually removed (as it is superseded by firewire-core's
    ABI), this change seems still worth doing to
    further minimize the presence of BKL usage in the kernel.

    Signed-off-by: Stefan Richter

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

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

    Stefan Richter