lkml.org 
[lkml]   [2013]   [Aug]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRE: [PATCH] usb: xhci-plat: Enable USB 2.0 hardware LPM support for platform xHCs
    Date
    > From: Sarah Sharp [mailto:sarah.a.sharp@linux.intel.com]
    > Sent: Thursday, August 29, 2013 10:42 AM
    >
    > On Wed, Aug 28, 2013 at 10:15:34PM +0000, Paul Zimmerman wrote:
    > > > From: Sarah Sharp [mailto:sarah.a.sharp@linux.intel.com]
    > > > Sent: Wednesday, August 28, 2013 2:52 PM
    > > >
    > > > On Mon, Aug 26, 2013 at 07:37:56PM +0000, Paul Zimmerman wrote:
    > > > > > From: Sarah Sharp [mailto:sarah.a.sharp@linux.intel.com]
    > > > > > Sent: Monday, August 26, 2013 12:14 PM
    > > > > >
    > > > > > On Thu, Aug 22, 2013 at 05:11:49AM +0000, Paul Zimmerman wrote:
    > > > > > > Just to set the record straight :)
    > > > > > >
    > > > > > > The PORTHLPMC registers do exist on DWC3, starting with the 1.90a version
    > > > > > > of the core or thereabouts. They only supported the HIRD flavor of LPM,
    > > > > > > though. Only fairly recently has support for BESL been added, around
    > > > > > > version 2.41a or so.
    > > > > >
    > > > > > And the 2.41a version that supports BESL properly sets the BLC flag in
    > > > > > the USB 2.0 Protocol extended capabilities for all the USB 2.0 ports?
    > > > > > Has this functionality been well-tested?
    > > > >
    > > > > In 2.41a it is described as an "early adopter" feature in the databook,
    > > > > and no mention is made of the BLC flag. So the answer there is "maybe".
    > > > > Starting with 2.50a it is a full-fledged feature and the databook does
    > > > > describe the BLC flag.
    > > >
    > > > So the 2.41a has BESL support, but may not set the BLC flag. What
    > > > happens if we use the HIRD encoding instead? Will things break? It
    > > > seems like we would need to disable USB 2.0 LPM on that host all
    > > > together, if it expects BESL encoding, but advertises HIRD encoding.
    > >
    > > I imagine things would break, but I don't know for sure. I don't have a
    > > 2.41a version of the core to test this with.
    > >
    > > Maybe the LPM support should be disabled by default, and enabled with a
    > > quirk? That seems safer to me.
    >
    > I don't think that's a sustainable option.
    >
    > I expect that the majority of hosts that support USB 2.0 Link PM in the
    > future will have BESL support. It makes no sense to maintain an
    > ever-growing list of hosts that support BESL.
    >
    > We did something similar for the Intel EHCI to xHCI port switchover.
    > Every time someone added a new skew with a different PCI device ID, we
    > had to add that to the list of hosts that had the port switchover. The
    > list grew and grew, and backporting and notifying distros of new list
    > entries was a pain. It just wasn't sustainable, and we ended up ripping
    > out the list and dynamically looking for the Intel EHCI companion host
    > instead.

    Yes, but that was a case of things not working at all, correct? The worst
    that can happen with LPM disabled is that a new host will consume a little
    more power than necessary, until someone gets around to adding a quirk
    for it. Whereas if you enable it by default, things could be broken until
    the quirk is added.

    > So, no, I don't want to go there. I would rather have an xHCI quirk
    > that disables USB 2.0 LPM instead.

    ...

    > I think DT attributes would be the best way to go. I think the patch
    > should be changed to set those USB 2.0 LPM function pointers for the
    > platform devices, but add a new xHCI host quirk, XHCI_DISABLE_USB2_LPM.
    > Make the LPM functions return immediately if that quirk is set. Then
    > set the quirk based on DT attributes for the Synopsis 2.41a host.
    >
    > > > > In 2.51a it has been well-tested in simulation. In actual hardware, it
    > > > > has only been briefly tested in an ad-hoc sort of way, since none of the
    > > > > standard drivers in the market support the feature yet, as far as we know.
    > > > >
    > > > > Once the support is fully working in the Linux driver we can try testing
    > > > > it there.
    > > >
    > > > Can you please test Julius' patch on the 2.41a, 2.50a, and 2.51a hosts?
    > > > Please test against usb-next, since that includes a fix for the BESL
    > > > patches.
    > >
    > > As I said, I don't have the 2.41a version available to test. I do have
    > > 2.50a and 2.60a available, so I can try those.
    >
    > Ok, let me know if those work. In the meantime, can you help Julius
    > create a patch to add DT attributes to distinguish between the different
    > versions?

    I don't think there's a need to distinguish between different versions,
    is there? Don't we need just a single DT attribute that means "does not
    support BESL"?

    --
    Paul



    \
     
     \ /
      Last update: 2013-08-29 21:02    [W:2.361 / U:0.892 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site