10 Jun, 2010

16 commits

  • 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

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

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

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

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

    Clemens Ladisch
     
  • Move the register reading code from ohci_update_phy_reg() into
    a function which can be used separately.

    Signed-off-by: Clemens Ladisch
    Signed-off-by: Stefan Richter

    Clemens Ladisch
     
  • Signed-off-by: Stefan Richter

    Stefan Richter
     
  • Add the missing documentation for iso packets.

    Signed-off-by: Clemens Ladisch
    Signed-off-by: Stefan Richter

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

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

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

    Clemens Ladisch
     

25 Mar, 2010

2 commits

  • 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

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

    Stefan Richter
     

18 Mar, 2010

1 commit

  • 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 (untested)
    Signed-off-by: Stefan Richter (added #define)

    Clemens Ladisch
     

15 Mar, 2010

1 commit

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

    Clemens Ladisch
     

25 Feb, 2010

1 commit