13 Jul, 2010

1 commit


19 Jun, 2010

1 commit

  • 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
     

10 Apr, 2010

1 commit

  • 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
     

12 Oct, 2009

1 commit


12 Sep, 2009

1 commit

  • Do not leak the allocated memory in case the mutex_trylock() failed
    to acquire the lock.

    Signed-off-by: Michael Buesch

    This bug does not happen in practice: All raw1394 clients use
    libraw1394, and accesses to a libraw1394 handle need to be serialized
    by the client. This is documented in libraw1394's API reference.

    Signed-off-by: Stefan Richter

    Michael Buesch
     

25 Mar, 2009

3 commits

  • Signed-off-by: Stefan Richter

    Stefan Richter
     
  • Eliminate the following warnings in raw1394_compat_write()'s error
    return path, seen on x86-64 with CONFIG_COMPAT=y:

    drivers/ieee1394/raw1394.c:381:17: warning: incorrect type in return expression (different address spaces)
    drivers/ieee1394/raw1394.c:381:17: expected char const [noderef] *
    drivers/ieee1394/raw1394.c:381:17: got void *
    drivers/ieee1394/raw1394.c:2252:14: warning: incorrect type in argument 1 (different address spaces)
    drivers/ieee1394/raw1394.c:2252:14: expected void const *ptr
    drivers/ieee1394/raw1394.c:2252:14: got char const [noderef] *[assigned] buffer
    drivers/ieee1394/raw1394.c:2253:19: warning: incorrect type in argument 1 (different address spaces)
    drivers/ieee1394/raw1394.c:2253:19: expected void const *ptr
    drivers/ieee1394/raw1394.c:2253:19: got char const [noderef] *[assigned] buffer

    Signed-off-by: Stefan Richter

    Stefan Richter
     
  • The C99 specification states in section 6.11.5:

    The placement of a storage-class specifier other than at the beginning
    of the declaration specifiers in a declaration is an obsolescent
    feature.

    Signed-off-by: Tobias Klauser
    Signed-off-by: Stefan Richter

    Tobias Klauser
     

05 Jan, 2009

1 commit


07 Nov, 2008

1 commit


31 Oct, 2008

1 commit

  • Regression in 2.6.28-rc1: When I added the new state_mutex which
    prevents corruption of raw1394's internal state when accessed by
    multithreaded client applications, the following possible though
    highly unlikely deadlock slipped in:

    Thread A: Thread B:
    - acquire mmap_sem - raw1394_write() or raw1394_ioctl()
    - raw1394_mmap() - acquire state_mutex
    - acquire state_mutex - copy_to/from_user(), possible page fault:
    acquire mmap_sem

    The simplest fix is to use mutex_trylock() instead of mutex_lock() in
    raw1394_mmap(). This changes the behavior under contention in a way
    which is visible to userspace clients. However, since multithreaded
    access was entirely buggy before state_mutex was added and libraw1394's
    documentation advised application programmers to use a handle only in a
    single thread, this change in behaviour should not be an issue in
    practice at all.

    Since we have to use mutex_trylock() in raw1394_mmap() regardless
    whether /dev/raw1394 was opened with O_NONBLOCK or not, we now use
    mutex_trylock() unconditionally everywhere for state_mutex, just to have
    consistent behavior.

    Reported-by: Johannes Weiner
    Signed-off-by: Stefan Richter

    Stefan Richter
     

17 Oct, 2008

2 commits

  • * 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6:
    firewire: Add more documentation to firewire-cdev.h
    firewire: fix ioctl() return code
    firewire: fix setting tag and sy in iso transmission
    firewire: fw-sbp2: fix another small generation access bug
    firewire: fw-sbp2: enforce s/g segment size limit
    firewire: fw_send_request_sync()
    ieee1394: survive a few seconds connection loss
    ieee1394: nodemgr clean up class iterators
    ieee1394: dv1394, video1394: remove unnecessary expressions
    ieee1394: raw1394: make write() thread-safe
    ieee1394: raw1394: narrow down the state_mutex protected region
    ieee1394: raw1394: replace BKL by local mutex, make ioctl() and mmap() thread-safe
    ieee1394: sbp2: enforce s/g segment size limit
    ieee1394: sbp2: check for DMA mapping failures
    ieee1394: sbp2: stricter dma_sync
    ieee1394: Use DIV_ROUND_UP

    Linus Torvalds
     
  • Now that device_create() has been audited, rename things back to the
    original call to be sane.

    Cc: Ben Collins
    Acked-by: Stefan Richter
    Signed-off-by: Greg Kroah-Hartman

    Greg Kroah-Hartman
     

16 Oct, 2008

3 commits

  • Application programs should use a libraw1394 handle only in a single
    thread. The raw1394 driver was apparently relying on this, because it
    did nothing to protect its fi->state variable from corruption due to
    concurrent accesses.

    We now serialize the fi->state accesses. This affects the write() path.
    We re-use the state_mutex which was introduced to protect fi->iso_state
    accesses in the ioctl() path. These paths and accesses are independent
    of each other, hence separate mutexes could be used. But I don't see
    much benefit in that.

    Signed-off-by: Stefan Richter

    Stefan Richter
     
  • Refactor the ioctl dispatcher in order to move a fraction of it out of
    the section which is serialized by fi->state_mutex. This is not so much
    about performance but more about self-documentation: The mutex_lock()/
    mutex_unlock() calls are now closer to the data accesses which the mutex
    protects, i.e. to the iso_state switch.

    Signed-off-by: Stefan Richter

    Stefan Richter
     
  • This removes the last usage of the Big Kernel Lock from the ieee1394
    stack, i.e. from raw1394's (unlocked_)ioctl and compat_ioctl.

    The ioctl()s don't need to take the BKL, but they need to be serialized
    per struct file *. In particular, accesses to ->iso_state need to be
    serial. We simply use a blocking mutex for this purpose because
    libraw1394 does not use O_NONBLOCK. In practice, there is no lock
    contention anyway because most if not all libraw1394 clients use a
    libraw1394 handle only in a single thread.

    mmap() also accesses ->iso_state. Until now this was unprotected
    against concurrent changes by ioctls. Fix this bug while we are at it.

    Signed-off-by: Stefan Richter

    Stefan Richter
     

22 Jul, 2008

1 commit


14 Jul, 2008

1 commit


26 Apr, 2008

2 commits

  • Currently the kernel will issue the following warning:
    drivers/ieee1394/raw1394.c:2938: warning: 'raw1394_id_table' defined but not used
    Add #ifdef MODULE guards around the declaration.

    Signed-off-by: Tony Breeds

    Ditto with dv1394_id_table and video1394_id_table.

    Signed-off-by: Stefan Richter

    Tony Breeds
     
  • As it seems, some host controllers have issues that can cause them to
    skip cycles now and then when using large packets. I suspect that this
    is due to DMA not succeeding in time. If the transmit fifo can't contain
    more than one packet (big packets), the DMA should provide a new packet
    each cycle (125us). I am under the impression that my current PCI
    express test system can't guarantee this.

    In any case, the patch tries to provide a workaround as follows:
    The DMA program descriptors are modified such that when an error occurs,
    the DMA engine retries the descriptor the next cycle instead of
    stalling. This way no data is lost. The side effect of this is that
    packets are sent with one cycle delay. This however might not be that
    much of a problem for certain protocols (e.g. AM824). If they use
    padding packets for e.g. rate matching they can drop one of those to
    resync the streams.

    The amount of skips between two userspace wakeups is counted. This
    number is then propagated to userspace through the upper 16 bits of the
    'dropped' parameter. This allows unmodified userspace applications due
    to the following:
    1) libraw simply passes this dropped parameter to the user application
    2) the meaning of the dropped parameter is: if it's nonzero, something
    bad has happened. The actual value of the parameter at this moment does
    not have a specific meaning.

    A libraw client can then retrieve the number of skipped cycles and
    account for them if needed.

    Signed-off-by: Pieter Palmers
    Signed-off-by: Stefan Richter

    Pieter Palmers
     

18 Apr, 2008

2 commits


31 Jan, 2008

1 commit


27 Jul, 2007

1 commit


10 Jul, 2007

7 commits

  • Based on patch "the scheduled removal of RAW1394_REQ_ISO_{SEND,LISTEN}"
    from Adrian Bunk, November 20 2006.

    This patch also removes the underlying facilities in ohci1394 and
    disables them in pcilynx. That is, hpsb_host_driver.devctl() and
    hpsb_host_driver.transmit_packet() are no longer used for iso reception
    and transmission.

    Since video1394 and dv1394 only work with ohci1394 and raw1394's rawiso
    interface has never been implemented in pcilynx, pcilynx is now no
    longer useful for isochronous applications.

    raw1394 will still handle the request types but will complete the
    requests with errors that indicate API version conflicts.

    Signed-off-by: Stefan Richter

    Stefan Richter
     
  • Here is a straightforward conversion to "struct device". The "struct
    class_device" will be removed from the kernel.

    It seems to work fine for me with and without CONFIG_SYSFS_DEPRECATED
    set.

    Signed-off-by: Stefan Richter

    Kay Sievers
     
  • I was told that only i386 aligns 64 bit integers at 4 bytes boundaries
    while all other architectures (32 bit architectures with 64 bit
    siblings) align it on 8 bytes boundaries.

    Signed-off-by: Stefan Richter

    Stefan Richter
     
  • Pointed out by Arnd Bergmann: PPC32 aligns this at 64bit, IA32 packs
    it. A kernel-wide available __compat_u64 which is 4-byte aligned on
    AMD64 and IA64 would be nicer though.

    Signed-off-by: Stefan Richter

    Stefan Richter
     
  • Add compat_ioctl. Although all structures are more or less same,
    raw1394_iso_packets got pointer inside, and raw1394_cycle_timer got unwanted
    padding in the middle. I did not add any translation for ioctls passing array
    of integers around as integers seem to have same size (32 bits) on all
    architectures supported by Linux.

    Signed-off-by: Petr Vandrovec
    Acked-by: Dan Dennedy
    Signed-off-by: Stefan Richter (split into 3 patches)

    Petr Vandrovec
     
  • * write(fd, buf, 52) from 32bit app was returning 56. Most of callers did not
    care, but some (arm registration) did, and anyway it looks bad if request for
    writing 52 bytes returns 56. And returning sizeof anything in 'int' is not
    good as well. So all functions now return '0' instead of
    sizeof(struct raw1394_request) on success, and write() itself provides correct
    return value (it just returns value it was asked to write on success as raw1394
    does not do any partial writes at all).

    * Related to this was problem that write() could have returned 0 when kernel
    state would become corrupted and moved to different state than
    opened/initialized/connected. Now it returns -EBADFD which seemed appropriate.

    Signed-off-by: Petr Vandrovec
    Acked-by: Dan Dennedy
    Signed-off-by: Stefan Richter (split into 3 patches)

    Petr Vandrovec
     
  • read() always failed with -EFAULT. This was happening due to
    raw1394_compat_read copying data to wrong location - access_ok always
    failed as 'r' is kernel address, not user. Whole function just tried to
    copy data from 'r' to 'r', which is not good.

    Signed-off-by: Petr Vandrovec
    Acked-by: Dan Dennedy
    Signed-off-by: Stefan Richter (split into 3 patches)

    Petr Vandrovec
     

28 May, 2007

1 commit

  • While playing with libiec61883 I've noticed that async_send is broken
    because it was doing copy_from_user(...., packet->data_size) before
    packet->data_size was set to any useful value. It got broken when
    packet->allocated_data_size got introduced, as hpsb_alloc_packet does
    not set packet->data_size anymore. (Regression in 2.6.22-rc1)

    Signed-off-by: Petr Vandrovec
    Signed-off-by: Stefan Richter

    Petr Vandrovec
     

09 May, 2007

1 commit


30 Apr, 2007

1 commit


20 Feb, 2007

1 commit


17 Feb, 2007

1 commit

  • This implements the simultaneous read of the isochronous cycle timer and
    the system clock (in usecs). This allows to express the exact receive
    time of an ISO packet as a system time with microsecond accuracy.
    http://bugzilla.kernel.org/show_bug.cgi?id=7773

    The counterpart patch for libraw1394 can be found at
    http://thread.gmane.org/gmane.linux.kernel.firewire.devel/8934

    Patch update (Stefan R.): Disable preemption and local interrupts.
    Prevent integer overflow. Add paranoid error checks and kerneldoc to
    hpsb_read_cycle_timer. Move it to other ieee1394_core high-level API
    functions. Change comments. Adjust whitespace. Rename struct
    _raw1394_cycle_timer.

    Signed-off-by: Stefan Richter
    Acked-by: Pieter Palmers
    Acked-by: Dan Dennedy

    Pieter Palmers
     

13 Feb, 2007

1 commit

  • Many struct file_operations in the kernel can be "const". Marking them const
    moves these to the .rodata section, which avoids false sharing with potential
    dirty data. In addition it'll catch accidental writes at compile time to
    these shared resources.

    Signed-off-by: Arjan van de Ven
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Arjan van de Ven
     

09 Feb, 2007

1 commit

  • Unloading the low-level driver module of a FireWire host can lead to
    all sorts of trouble if a raw1394 userspace client is using the host.
    Just disallow it by incrementing the LLD's module reference count on
    a RAW1394_REQ_SET_CARD write operation. Decrement it when the file
    is closed.

    This feature wouldn't be relevant if "modprobe -r video1394" or
    "modprobe -r dv1394" didn't automatically unload ohci1394 too.
    http://bugzilla.kernel.org/show_bug.cgi?id=7701

    Signed-off-by: Stefan Richter
    Signed-off-by: Dan Dennedy

    Stefan Richter
     

08 Dec, 2006

2 commits

  • This patch consolidates some bookkeeping for driver registering. It
    closely models what pci_register_driver() does. The main addition is
    that the owner of the driver is set, so we get a proper symlink
    for /sys/bus/ieee1394/driver/*/module.

    Also moves setting of name and bus type into nodemgr. Because of this,
    we can remove the EXPORT_SYMBOL for ieee1394_bus_type, since it's now
    only used in ieee1394.ko.

    Signed-off-by: Stefan Richter

    Ben Collins
     
  • Known to be affected:
    - libdc1394: prefers video1394 for now, old-style raw1394 support might
    be dropped eventually
    - OpenH323 PWLib, AVC video input module: uses libraw1394's old API

    Signed-off-by: Stefan Richter

    Stefan Richter