lkml.org 
[lkml]   [2016]   [Feb]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v7 05/10] usb: dbc: add bulk out and bulk in interfaces
    On 26.01.2016 14:58, Lu Baolu wrote:
    > This patch adds interfaces for bulk out and bulk in ops. These
    > interfaces could be used to implement early printk bootconsole
    > or hook to various system debuggers.
    >
    > Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
    > ---
    > drivers/usb/early/xhci-dbc.c | 373 +++++++++++++++++++++++++++++++++++++++++++
    > include/linux/usb/xhci-dbc.h | 30 ++++
    > 2 files changed, 403 insertions(+)
    >

    ...

    > +
    > +/*
    > + * Check and dispatch events in event ring. It also checks status
    > + * of hardware. This function will be called from multiple threads.
    > + * An atomic lock is applied to protect the access of event ring.
    > + */
    > +static int xdbc_check_event(void)
    > +{
    > + /* event ring is under checking by other thread? */
    > + if (!test_bit(XDBC_ATOMIC_EVENT, &xdbcp->atomic_flags) &&
    > + !test_and_set_bit(XDBC_ATOMIC_EVENT,
    > + &xdbcp->atomic_flags))
    > + return 0;

    homemade trylock, can't the real ones be used?

    > +
    > + xdbc_handle_events();
    > +
    > + test_and_clear_bit(XDBC_ATOMIC_EVENT, &xdbcp->atomic_flags);
    > +
    > + return 0;
    > +}
    > +
    > +#define BULK_IN_COMPLETED(p) ((xdbcp->in_pending == (p)) && \
    > + xdbcp->in_complete)
    > +#define BULK_OUT_COMPLETED(p) ((xdbcp->out_pending == (p)) && \
    > + xdbcp->out_complete)
    > +

    ...

    > +}
    > +
    > +int xdbc_bulk_read(void *data, int size, int loops)
    > +{
    > + int ret;
    > +
    > + do {
    > + if (!test_bit(XDBC_ATOMIC_BULKIN, &xdbcp->atomic_flags) &&
    > + !test_and_set_bit(XDBC_ATOMIC_BULKIN,
    > + &xdbcp->atomic_flags))
    > + break;
    > + } while (1);

    homemeade spin_lock, can't the real one be used?

    If the xdbc_bulk_write() can be accessed from interrupt context (handler, soft, timer) it
    may deadlock

    > +
    > + ret = xdbc_bulk_transfer(data, size, loops, true);
    > +
    > + test_and_clear_bit(XDBC_ATOMIC_BULKIN, &xdbcp->atomic_flags);
    > +
    > + return ret;
    > +}
    > +
    > +int xdbc_bulk_write(const char *bytes, int size)
    > +{
    > + int ret;
    > +
    > + do {
    > + if (!test_bit(XDBC_ATOMIC_BULKOUT, &xdbcp->atomic_flags) &&
    > + !test_and_set_bit(XDBC_ATOMIC_BULKOUT,
    > + &xdbcp->atomic_flags))
    > + break;
    > + } while (1);

    Another homemeade spin_lock, can't the real one be used?

    same issue here, deadlock if accessible from interrupt context.


    Would it make sense to have only one spinlock, and start one separate thread for
    reading the event ring. The thread would, lock, handle pending events, unlock,
    then call shedule, in a loop. ehci early debug code has some variant of this.

    So the lock would be taken while events are being handled.

    The same lock would be used for bulk_read and bulk_write. Yes this would prevent read and
    write at the same time, and the read and writes need to be modified to not block until
    the reansfer is finished, just to write the TRBs on the ring, update ring pointers,
    and ring the doorbell.

    Or is all this impossibe due to the earlyness of the code?

    -Mathias

    \
     
     \ /
      Last update: 2016-02-18 14:41    [W:2.900 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site