Re: [PATCH RFC] ehci-omap: simple suspend implementation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Andreas,

On 16/02/18 20:35, Andreas Kemnade wrote:
> On Fri, 16 Feb 2018 13:13:11 -0500 (EST)
> Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> 
>> On Fri, 16 Feb 2018, Andreas Kemnade wrote:
>>
>>> This powers down the phy and on a gta04 it reduces
>>> suspend current by 13 mA.
>>> For unknown reasons usb does not power on properly.
>>> Also calling usb_phy_shutdown() here feels wrong
>>> apparently the reset line has to be activated.
>>> usb_phy_set_suspend is not enough here. The power
>>> consumption still stays approximately the same as
>>> without any patch.
>>>
>>> With a device connected the device does not enumerate
>>> after resume. A rmmod ehci-omap ; modprobe ehci-omap
>>> does not make it reenumerade.
>>> So there is still something wrong here.
>>>
>>> Signed-off-by: Andreas Kemnade <andreas@xxxxxxxxxxxx>
>>> ---
>>>  drivers/usb/host/ehci-omap.c | 59 ++++++++++++++++++++++++++++++++++++++++++--
>>>  1 file changed, 57 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
>>> index 8d8bafc70c1f..0be2ccf8182a 100644
>>> --- a/drivers/usb/host/ehci-omap.c
>>> +++ b/drivers/usb/host/ehci-omap.c
>>> @@ -266,6 +266,58 @@ static int ehci_hcd_omap_remove(struct platform_device *pdev)
>>>  	return 0;
>>>  }
>>>  
>>> +
>>> +static int __maybe_unused ehci_omap_suspend(struct device *dev)
>>> +{
>>> +	struct usb_hcd *hcd = dev_get_drvdata(dev);
>>> +	struct omap_hcd *omap = (struct omap_hcd *)hcd_to_ehci(hcd)->priv;
>>> +	int ret;
>>> +	int i;
>>> +
>>> +	ret = ehci_suspend(hcd, false);
>>> +	if (ret) {
>>> +		dev_err(dev, "ehci suspend failed: %d\n", ret);
>>> +		return ret;
>>> +	}
>>> +	for (i = 0; i < omap->nports; i++) {
>>> +		if (omap->phy[i])
>>> +			usb_phy_shutdown(omap->phy[i]);
>>> +	}
>>> +	pm_runtime_put_sync(dev);  
>>
>> Why do you include a runtime PM call here, given that the driver
>> doesn't support runtime suspend or resume?
>>
> Well, the parent (drivers/mfd/omap-usb-host.c) has, and there are runtime PM
> calls here in the _probe/_remove functions, so it seems to be sane to do it
> here, too.
> 
> 
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int __maybe_unused ehci_omap_resume(struct device *dev)
>>> +{
>>> +	struct usb_hcd *hcd = dev_get_drvdata(dev);
>>> +	struct omap_hcd *omap = (struct omap_hcd *)hcd_to_ehci(hcd)->priv;
>>> +	int i;
>>> +
>>> +	pm_runtime_get_sync(dev);
>>> +	/*
>>> +	 * An undocumented "feature" in the OMAP3 EHCI controller,
>>> +	 * causes suspended ports to be taken out of suspend when
>>> +	 * the USBCMD.Run/Stop bit is cleared (for example when
>>> +	 * we do ehci_bus_suspend).
>>> +	 * This breaks suspend-resume if the root-hub is allowed
>>> +	 * to suspend. Writing 1 to this undocumented register bit
>>> +	 * disables this feature and restores normal behavior.
>>> +	 */
>>> +	ehci_write(hcd->regs, EHCI_INSNREG04,
>>> +		   EHCI_INSNREG04_DISABLE_UNSUSPEND);  
>>
>> Doesn't this code belong in ehci_hcd_omap_probe()?  I assume you only
>> need to set this undocumented bit once, not every time the controller
>> is suspended.  And in any case, according to the comment, you would
>> need to take care of this before the root hub is suspended -- by the
>> time the controller is suspended, it is already too late.
>>
>  
> I am not really sure about this one. It is set in ehci_hcd_omap_probe()
> so it is set before suspend. I set it here to ensure it is re-set when
> the controller is powered down too much to keep register contents. I
> do not know if that is the case. But I thought it would not harm.
> 

If the Hardware SAR (Save and restore) functionality is enabled then
everything will be restored by hardware after a sleep to wake transition.

But you will need this patch to enable SAR for the USB power domain.
https://lkml.org/lkml/2013/7/10/356

Missing this might be the reason why things break for you after a system
suspend/resume.

-- 
cheers,
-roger

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux