Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1503685

Re: [RESEND PATCH v2 1/4] usb: dbc: early driver for xhci debug capability

From Peter Zijlstra <peterz@infradead.org>
Newsgroups linux.kernel
Subject Re: [RESEND PATCH v2 1/4] usb: dbc: early driver for xhci debug capability
Date 2016-10-19 16:30 +0200
Message-ID <su02v-2rf-115@gated-at.bofh.it> (permalink)
References <stMLT-1jl-9@gated-at.bofh.it> <stMLT-1jl-7@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


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?

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[RESEND PATCH v2 0/4] usb: early: add support for early printk through USB3 debug port Lu Baolu <baolu.lu@linux.intel.com> - 2016-10-19 02:20 +0200
  [RESEND PATCH v2 1/4] usb: dbc: early driver for xhci debug capability Lu Baolu <baolu.lu@linux.intel.com> - 2016-10-19 02:20 +0200
    Re: [RESEND PATCH v2 1/4] usb: dbc: early driver for xhci debug  capability Peter Zijlstra <peterz@infradead.org> - 2016-10-19 16:30 +0200
      Re: [RESEND PATCH v2 1/4] usb: dbc: early driver for xhci debug  capability Lu Baolu <baolu.lu@linux.intel.com> - 2016-10-20 10:20 +0200
        Re: [RESEND PATCH v2 1/4] usb: dbc: early driver for xhci debug  capability Peter Zijlstra <peterz@infradead.org> - 2016-10-20 10:50 +0200
          Re: [RESEND PATCH v2 1/4] usb: dbc: early driver for xhci debug  capability Peter Zijlstra <peterz@infradead.org> - 2016-10-20 11:10 +0200
            Re: [RESEND PATCH v2 1/4] usb: dbc: early driver for xhci debug  capability Lu Baolu <baolu.lu@linux.intel.com> - 2016-10-21 02:40 +0200
  [RESEND PATCH v2 3/4] usb: serial: usb_debug: add support for dbc debug device Lu Baolu <baolu.lu@linux.intel.com> - 2016-10-19 02:20 +0200
  [RESEND PATCH v2 2/4] x86: add support for earlyprintk via USB3 debug port Lu Baolu <baolu.lu@linux.intel.com> - 2016-10-19 02:30 +0200
  [RESEND PATCH v2 4/4] usb: doc: add document for USB3 debug port usage Lu Baolu <baolu.lu@linux.intel.com> - 2016-10-19 02:30 +0200
    Re: [RESEND PATCH v2 4/4] usb: doc: add document for USB3 debug port usage Jani Nikula <jani.nikula@linux.intel.com> - 2016-10-19 18:30 +0200
      Re: [RESEND PATCH v2 4/4] usb: doc: add document for USB3 debug port  usage Lu Baolu <baolu.lu@linux.intel.com> - 2016-10-20 07:50 +0200
        Re: [RESEND PATCH v2 4/4] usb: doc: add document for USB3 debug port usage Jani Nikula <jani.nikula@linux.intel.com> - 2016-10-20 10:40 +0200
          Re: [RESEND PATCH v2 4/4] usb: doc: add document for USB3 debug port  usage Lu Baolu <baolu.lu@linux.intel.com> - 2016-10-21 02:40 +0200

csiph-web