Thread (22 messages) 22 messages, 6 authors, 2018-11-13

Re: [PATCH V3 4/6] usb: ohci-platform: Add support for Broadcom STB SoC's

From: Alan Stern <stern@rowland.harvard.edu>
Date: 2018-11-07 16:27:29
Also in: linux-usb, lkml

On Wed, 7 Nov 2018, Al Cooper wrote:
On 11/7/18 10:23 AM, Alan Stern wrote:
quoted
On Tue, 6 Nov 2018, Florian Fainelli wrote:
quoted
On 11/6/18 1:40 PM, Al Cooper wrote:
quoted
On 11/6/18 11:08 AM, Alan Stern wrote:
quoted
On Mon, 5 Nov 2018, Al Cooper wrote:
quoted
Add support for Broadcom STB SoC's to the ohci platform driver.

Signed-off-by: Al Cooper <alcooperx@gmail.com>
---
quoted
@@ -177,6 +189,8 @@ static int ohci_platform_probe(struct
platform_device *dev)
           ohci->flags |= OHCI_QUIRK_FRAME_NO;
       if (pdata->num_ports)
           ohci->num_ports = pdata->num_ports;
+    if (pdata->suspend_without_phy_exit)
+        hcd->suspend_without_phy_exit = 1;
Sorry if I missed this in the earlier discussions...  Is there any
possibility of adding a DT binding that could express this requirement,
instead of putting it in the platform data?

Alan Stern
Alan,

That was my original approach but internal review suggested that I use
pdata instead. Below is my original patch for:
And the reason for that suggestion was really because it was percevied
as encoding a driver behavior as a Device Tree property as opposed to
describing something that was inherently and strictly a hardware
behavior (therefore suitable for Device Tree).
Right.  The best way to approach this problem is to identify and
characterize the hardware behavior which makes this override necessary.
Then _that_ can be added to DT, since it will be a property of the
hardware rather than of the driver.
quoted
quoted
Add the ability to skip calling the PHY's exit routine on suspend
and the PHY's init routine on resume. This is to handle a USB PHY
that should have it's power_off function called on suspend but cannot
have it's exit function called because on exit it will disable the
PHY to the point where register accesses to the Host Controllers
using the PHY will be disabled and the host drivers will crash.
What's special about this PHY?  Why does the exit function mess the PHY
up?  Or to put it another way, why doesn't the exit function mess up
other PHYs in the same way?

For that matter, can we change the code so that suspend doesn't call
the exit function for _any_ PHY?  Will just calling the power_off
function be good enough?  If not, then why not?

Alan Stern
In our USB hardware the USB PHY supplies a clock for the EHCI/OHCI and 
XHCI host controllers and if the PHY is totally shut down the EHCI, OHCI 
and XHCI registers will cause an exception if accessed and cause the 
EHCI, OHCI and XHCI drivers to crash. There is always talk of fixing 
this in the hardware by adding an aux clock that will takeover when the 
PHY clock is shut down, but this hasn't happened yet. It seems like 
"exit on suspend" still makes sense on systems that don't have this 
problem (additional power savings?) so removing the exit on suspend for 
all systems is not a good idea.
Then in theory you should be able to add a Device Tree property which
says that the PHY provides a clock for the USB host controller.  That
is strictly a property of the hardware; it has nothing to do with the
driver.  Therefore it is appropriate for DT.

Wouldn't this solve your issue?

Alan Stern
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help