[PATCH update 2] firewire: fw-ohci: don't append to AT context when its not active - Kernel

This is a discussion on [PATCH update 2] firewire: fw-ohci: don't append to AT context when its not active - Kernel ; From: Jarod Wilson I finally tracked down the issues with this JMicron PCI-e card in my possession to a failure to comply with section 7.2.3.2 of the OHCI 1.1 specification (thanks to Kristian for the pointer to illustrate that it ...

+ Reply to Thread
Results 1 to 2 of 2

Thread: [PATCH update 2] firewire: fw-ohci: don't append to AT context when its not active

  1. [PATCH update 2] firewire: fw-ohci: don't append to AT context when its not active

    From: Jarod Wilson

    I finally tracked down the issues with this JMicron PCI-e card in my
    possession to a failure to comply with section 7.2.3.2 of the OHCI 1.1
    specification (thanks to Kristian for the pointer to illustrate that it
    is indeed a flaw in this card, not the driver). The controller should
    simply flush the packets we've appended to its AT queue if a bus reset
    occurs before they've been transmitted and we'll try again, but
    something goes wrong and the controller winds up hung.

    However, we can avoid the problem by simply checking if the
    IntEvent.busReset register had been set before we try appending to the
    AT context. When busReset is set, the AT context is completely halted
    until busReset is cleared, so there's no point in appending AT packets
    until the register is cleared. So at_context_queue_packet() now checks
    for busReset being set, and bails with an RCODE_GENERATION packet ack,
    which results in us trying to append the packet again after recognizing
    the fact there has been a bus reset, and clearing busReset.

    Signed-off-by: Jarod Wilson

    Split out busReset event logging to do it safer as a separate patch.

    Signed-off-by: Stefan Richter
    ---
    drivers/firewire/fw-ohci.c | 15 +++++++++++++--
    1 file changed, 13 insertions(+), 2 deletions(-)

    Index: linux/drivers/firewire/fw-ohci.c
    ================================================== =================
    --- linux.orig/drivers/firewire/fw-ohci.c
    +++ linux/drivers/firewire/fw-ohci.c
    @@ -950,8 +950,19 @@ at_context_queue_packet(struct context *
    DESCRIPTOR_IRQ_ALWAYS |
    DESCRIPTOR_BRANCH_ALWAYS);

    - /* FIXME: Document how the locking works. */
    - if (ohci->generation != packet->generation) {
    + /*
    + * If the controller and packet generations don't match, we need to
    + * bail out and try again. If IntEvent.busReset is set, the AT context
    + * is halted, so appending to the context and trying to run it is
    + * futile. Most controllers do the right thing and just flush the AT
    + * queue (per section 7.2.3.2 of the OHCI 1.1 specification), but
    + * some controllers (like a JMicron JMB381 PCI-e) misbehave and wind
    + * up stalling out. So we just bail out in software and try again
    + * later, and everyone is happy.
    + * FIXME: Document how the locking works.
    + */
    + if (ohci->generation != packet->generation ||
    + reg_read(ohci, OHCI1394_IntEventSet) & OHCI1394_busReset) {
    if (packet->payload_length > 0)
    dma_unmap_single(ohci->card.device, payload_bus,
    packet->payload_length, DMA_TO_DEVICE);

    --
    Stefan Richter
    -=====-==--- -=-- --===
    http://arcgraph.de/sr/

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [PATCH] firewire: fw-ohci: conditionally log busReset interrupts

    On Monday 07 April 2008 04:33:35 pm Stefan Richter wrote:
    > Add a debug option to watch bus reset interrupt events. Half of this
    > patch is taken from Jarod Wilson's first version of the JMicron fix.
    >
    > BusReset interrupts are only generated if the respective module
    > parameter flag was set before the controller is being initialized. We
    > keep this event masked otherwise to reduce IRQ load in normal operation
    > and to avoid potential problems with buggy chips.
    >
    > Note, this is unlike the other IRQ events whose logging can be enabled
    > any time after chip initialization. This and the influence on what
    > interrupts the chip generates is why I added an extra flag for it.
    >
    > Also, reorder the debug parameter flags according to their perceived
    > usefulness.
    >
    > Signed-off-by: Stefan Richter
    > ---


    Indeed, after discussing, this looks much safer.

    Signed-off-by: Jarod Wilson


    --
    Jarod Wilson
    jwilson@redhat.com
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread