LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Sneeker Yeh <sneeker.yeh@gmail.com>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Mathias Nyman <mathias.nyman@intel.com>,
	Rob Herring <robh+dt@kernel.org>, Pawel Moll <pawel.moll@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	Kumar Gala <galak@codeaurora.org>, Felipe Balbi <balbi@ti.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Grant Likely <grant.likely@linaro.org>,
	Huang Rui <ray.huang@amd.com>,
	Kishon Vijay Abraham I <kishon@ti.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	"linux-omap@vger.kernel.org" <linux-omap@vger.kernel.org>,
	Andy Green <andy.green@linaro.org>,
	Jassi Brar <jaswinder.singh@linaro.org>,
	Sneeker Yeh <Sneeker.Yeh@tw.fujitsu.com>
Subject: Re: [PATCH v3 1/5] xhci: add a quirk for device disconnection errata for Synopsis Designware USB3 core
Date: Sun, 15 Feb 2015 23:09:10 +0800	[thread overview]
Message-ID: <CAJ1gpc2FKe06gWMoMHXriO52_AnKdDSa1G46mvrzAFTEQUY99w@mail.gmail.com> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1502121013390.1209-100000@iolanthe.rowland.org>

Hi Alan:

thanks for comment it,
and sorry that a little bit late for replying,

2015-02-12 23:18 GMT+08:00 Alan Stern <stern@rowland.harvard.edu>:
> On Thu, 12 Feb 2015, Mathias Nyman wrote:
>
>> On 25.01.2015 10:13, Sneeker Yeh wrote:
>> > This issue is defined by a three-way race at disconnect, between
>> > 1) Class driver interrupt endpoint resheduling attempts if the ISR gave an ep
>> >    error event due to device detach (it would try 3 times)
>> > 2) Disconnect interrupt on PORTSC_CSC, which is cleared by hub thread
>> >    asynchronously
>> > 3) The hardware IP was configured in silicon with
>> >    - DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1
>> >    - Synopsys IP version is < 3.00a
>> > The IP will auto-suspend itself on device detach with some phy-specific interval
>> > after CSC is cleared by 2)
>> >
>> > If 2) and 3) complete before 1), the interrupts it expects will not be generated
>> > by the autosuspended IP, leading to a deadlock. Even later disconnection
>> > procedure would detect that corresponding urb is still in-progress and issue a
>> > ep stop command, auto-suspended IP still won't respond to that command.
>
> If the Synopsys IP provides a way to do it, it would be better to turn
> off the autosuspend feature entirely.  Doesn't autosuspend violate the
> xHCI specification?

it's an IP parameter that can't be turn off via any register ><.

I guess Synopsis can insisted that xHCI specification doesn't
explicitly state that hardware designer shouldn't do auto-suspend when
device is attached, especially that definition of the auto-suspend is
their own specific low power state.
IIRC, this is point of view from Synopsis.

I wonder maybe they just didn't have a good assumption when
implementing auto-suspend, e.g. they shouldn't take PORTCSC clearing
as a commitment that software has done all the device detach task, so
that IP can go into suspend. it seems more reasonable to take
slot-disabling as a commitment to a start of auto-suspend.

anyway the "errata" would be disclosed recently, and Synopsis plan to
fix it in their new silicon version IP.
Old version IP has to live happily with some patch that can workaround
this monster i think.

Much appreciate,
Sneeker

>
>> So did I understand correctly that the class driver submits a new urb which
>> is enqueued by xhci_urb_enqueue() before the hub thread notices the device is disconnected.
>> Then hub thread clears CSC bit, controller suspends and the new urb is never given back?
>>
>> Doesn't the CSC bit and PORT_CONNECT bit show the device is disconnected when we enter
>> xhci_enqueue_urb(), even it the hub thread doesn't know this yet?
>
> What if the device disconnects _after_ the new URB is enqueued?
>
>> Would it make sense to check those bits in xhci_enqueue_urb, and just return error
>> in the xhci_urb_enqueue() if device is not connected? Then there wouldn't be a need for any quirk
>> at all.
>
> That wouldn't help URBs that were already enqueued when the disconnect
> occurred.
>
> Alan Stern
>

  parent reply	other threads:[~2015-02-15 15:09 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-25  8:13 [PATCH v3 0/5] Add support for Fujitsu USB host controller Sneeker Yeh
2015-01-25  8:13 ` [PATCH v3 1/5] xhci: add a quirk for device disconnection errata for Synopsis Designware USB3 core Sneeker Yeh
2015-02-12 13:50   ` Mathias Nyman
2015-02-12 15:18     ` Alan Stern
2015-02-13  8:54       ` Mathias Nyman
2015-02-15 15:09       ` Sneeker Yeh [this message]
2015-02-15 14:29     ` Sneeker Yeh
2015-02-16  9:26       ` Mathias Nyman
2015-02-13  9:08   ` Mathias Nyman
2015-01-25  8:13 ` [PATCH v3 2/5] xhci: Platform: Set Synopsis device disconnection quirk based on platform data Sneeker Yeh
2015-01-25  8:13 ` [PATCH v3 3/5] usb: dwc3: add revision number DWC3_REVISION_290A and DWC3_REVISION_300A Sneeker Yeh
2015-01-25  8:13 ` [PATCH v3 4/5] usb: dwc3: Add quirk for Synopsis device disconnection errata Sneeker Yeh
2015-01-25  8:13 ` [PATCH v3 5/5] usb: dwc3: add Fujitsu Specific Glue layer Sneeker Yeh
2015-01-27 15:22 ` [PATCH v3 0/5] Add support for Fujitsu USB host controller Felipe Balbi
2015-01-29 16:23   ` Felipe Balbi
2015-01-30 16:38     ` Felipe Balbi
2015-02-10 15:43       ` Sneeker Yeh
2015-02-11 11:57         ` Mathias Nyman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAJ1gpc2FKe06gWMoMHXriO52_AnKdDSa1G46mvrzAFTEQUY99w@mail.gmail.com \
    --to=sneeker.yeh@gmail.com \
    --cc=Sneeker.Yeh@tw.fujitsu.com \
    --cc=andy.green@linaro.org \
    --cc=balbi@ti.com \
    --cc=devicetree@vger.kernel.org \
    --cc=galak@codeaurora.org \
    --cc=grant.likely@linaro.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=ijc+devicetree@hellion.org.uk \
    --cc=jaswinder.singh@linaro.org \
    --cc=kishon@ti.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mathias.nyman@intel.com \
    --cc=pawel.moll@arm.com \
    --cc=ray.huang@amd.com \
    --cc=robh+dt@kernel.org \
    --cc=stern@rowland.harvard.edu \
    --subject='Re: [PATCH v3 1/5] xhci: add a quirk for device disconnection errata for Synopsis Designware USB3 core' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).