lkml.org 
[lkml]   [2016]   [Oct]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RESEND PATCH v2 1/4] usb: dbc: early driver for xhci debug capability
On Wed, Oct 19, 2016 at 08:18:22AM +0800, Lu Baolu wrote:
> +++ b/drivers/usb/early/xhci-dbc.c

> +static int xdbc_bulk_write(const char *bytes, int size)
> +{
> + unsigned long flags;
> + int ret, timeout = 0;
> +
> + spin_lock_irqsave(&xdbc.lock, flags);

Yikes!!

So how is this supposed to work from NMI context and the like?

(also, at the very least, that should be a raw_spinlock_t)

What do you need the spinlock for? Afaict this is a 'simple' polling
event handling loop on MMIO, right?

All we really need to guarantee is that there's only a single CPU trying
to do that at any one time.

Wouldn't something like:

https://marc.info/?l=linux-kernel&m=147681099108509&w=2

already take care of that? Then you can drop the lock and things will
work 'nested'.

> +
> + xdbc_handle_events();
> +
> + /* Check completion of the previous request. */
> + while (xdbc.flags & XDBC_FLAGS_OUT_PROCESS) {
> + if (timeout > 1000000)
> + break;
> +
> + spin_unlock_irqrestore(&xdbc.lock, flags);
> + xdbc_delay(100);
> + spin_lock_irqsave(&xdbc.lock, flags);
> + timeout += 100;
> +
> + xdbc_handle_events();
> + }
> +
> + if (xdbc.flags & XDBC_FLAGS_OUT_PROCESS) {
> + spin_unlock_irqrestore(&xdbc.lock, flags);
> +
> + /*
> + * Oops, hardware wasn't able to complete the
> + * previous transfer.
> + */
> + xdbc_trace("oops: previous transfer not completed yet\n");
> +
> + return -ETIMEDOUT;
> + }
> +
> + ret = xdbc_bulk_transfer((void *)bytes, size, false);
> +
> + spin_unlock_irqrestore(&xdbc.lock, flags);
> +
> + return ret;
> +}
> +
> +static void early_xdbc_write(struct console *con, const char *str, u32 n)
> +{
> + int chunk, ret;
> + static char buf[XDBC_MAX_PACKET];
> + int use_cr = 0;
> +
> + if (!xdbc.xdbc_reg)
> + return;
> + memset(buf, 0, XDBC_MAX_PACKET);
> + while (n > 0) {
> + for (chunk = 0; chunk < XDBC_MAX_PACKET && n > 0;
> + str++, chunk++, n--) {
> + if (!use_cr && *str == '\n') {
> + use_cr = 1;
> + buf[chunk] = '\r';
> + str--;
> + n++;
> + continue;
> + }
> + if (use_cr)
> + use_cr = 0;
> + buf[chunk] = *str;
> + }
> + if (chunk > 0) {
> + ret = xdbc_bulk_write(buf, chunk);
> + if (ret < 0)
> + break;
> + }
> + }
> +}
> +
> +static struct console early_xdbc_console = {
> + .name = "earlyxdbc",
> + .write = early_xdbc_write,
> + .flags = CON_PRINTBUFFER,
> + .index = -1,
> +};
> +
> +void __init early_xdbc_register_console(void)
> +{
> + if (early_console)
> + return;
> +
> + early_console = &early_xdbc_console;
> + if (early_console_keep)
> + early_console->flags &= ~CON_BOOT;
> + else
> + early_console->flags |= CON_BOOT;
> + register_console(early_console);
> +}
> +
> +static void xdbc_scrub_function(struct work_struct *work)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&xdbc.lock, flags);
> +
> + /*
> + * DbC is running, check the event ring and
> + * handle the events.
> + */
> + if (readl(&xdbc.xdbc_reg->control) & CTRL_DRC)
> + xdbc_handle_events();
> +
> + /*
> + * External reset happened. Need to restart the
> + * debugging hardware.
> + */
> + if (unlikely(!(readl(&xdbc.xdbc_reg->control) & CTRL_DCE)))
> + xdbc_handle_external_reset();
> +
> + spin_unlock_irqrestore(&xdbc.lock, flags);
> +
> + queue_delayed_work(xdbc_wq, &xdbc.scrub, usecs_to_jiffies(100));
> +}

Excuse my total lack of USB knowledge, but WTH does this do and what do
we need it for?

\
 
 \ /
  Last update: 2016-10-19 18:12    [W:1.327 / U:0.060 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site