Linux-USB Archive on lore.kernel.org
 help / color / Atom feed
From: Kai-Heng Feng <kai.heng.feng@canonical.com>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: mathias.nyman@intel.com, gregkh@linuxfoundation.org,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Revert "usb: Avoid unnecessary LPM enabling and disabling during suspend and resume"
Date: Thu, 3 Oct 2019 19:55:06 +0800
Message-ID: <123BCB7F-5ABA-4DDD-9599-46D3240903F6@canonical.com> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1910021146260.1552-100000@iolanthe.rowland.org>



> On Oct 2, 2019, at 23:47, Alan Stern <stern@rowland.harvard.edu> wrote:
> 
> On Wed, 2 Oct 2019, Kai-Heng Feng wrote:
> 
>> This reverts commit d590c23111505635e1beb01006612971e5ede8aa.
>> 
>> Dell WD15 dock has a topology like this:
>> /:  Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 10000M
>>    |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/7p, 5000M
>>            |__ Port 2: Dev 3, If 0, Class=Vendor Specific Class, Driver=r8152, 5000M
>> 
>> Their IDs:
>> Bus 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
>> Bus 004 Device 002: ID 0424:5537 Standard Microsystems Corp.
>> Bus 004 Device 004: ID 0bda:8153 Realtek Semiconductor Corp.
>> 
>> Ethernet cannot be detected after plugging ethernet cable to the dock,
>> the hub and roothub get runtime resumed and runtime suspended
>> immediately:
>> ...
> 
>> After some trial and errors, the issue goes away if LPM on the SMSC hub
>> is disabled. Digging further, enabling and disabling LPM during runtime
>> resume and runtime suspend respectively can solve the issue.
>> 
>> So bring back the old LPM behavior, which the SMSC hub inside Dell WD15
>> depends on.
>> 
>> Fixes: d590c2311150 ("usb: Avoid unnecessary LPM enabling and disabling during suspend and resume")
>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> 
> Maybe it would be better to have a VID/PID-specific quirk for this?

Re-reading the spec, I think we need some clarification:
"If the value is 3, then host software wants to selectively suspend the
device connected to this port. The hub shall transition the link to U3
from any of the other U states using allowed link state transitions.
If the port is not already in the U0 state, then it shall transition the
port to the U0 state and then initiate the transition to U3."

The phrase "then it shall transition the port to the U0 state" what does "it" here refer to?
Is it the hub or the software?
If it's the former then it's indeed a buggy hub, but if it's the latter I think reverting the commit is the right thing to do.

Kai-Heng

> 
> Alan Stern
> 


  reply index

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-02 15:15 Kai-Heng Feng
2019-10-02 15:47 ` Alan Stern
2019-10-03 11:55   ` Kai-Heng Feng [this message]
2019-10-03 14:26     ` Alan Stern
2019-10-03 14:57       ` Kai-Heng Feng
2019-10-03 15:56 ` [PATCH v2] usb: Add a new quirk to let buggy hub enable and disable LPM during suspend and resume Kai-Heng Feng
2019-10-03 16:02   ` Greg KH
2019-10-03 17:24   ` [PATCH v3] " Kai-Heng Feng
2019-10-03 19:04     ` Alan Stern
2019-10-17  6:33       ` Kai-Heng Feng
2019-10-18 18:59         ` Greg Kroah-Hartman
2019-10-21 13:59           ` Mathias Nyman
2019-10-21 18:40             ` Kai-Heng Feng

Reply instructions:

You may reply publically 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=123BCB7F-5ABA-4DDD-9599-46D3240903F6@canonical.com \
    --to=kai.heng.feng@canonical.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mathias.nyman@intel.com \
    --cc=stern@rowland.harvard.edu \
    /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

Linux-USB Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-usb/0 linux-usb/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-usb linux-usb/ https://lore.kernel.org/linux-usb \
		linux-usb@vger.kernel.org
	public-inbox-index linux-usb

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-usb


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git