Re: [RFC PATCH 3/3] usb: phy: msm: Do not do runtime pm if the phy is not idle

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

 



On Mon, Jun 30, 2014 at 11:23:43AM +0100, Srinivas Kandagatla wrote:
> Hi Felipe,
> 
> On 27/06/14 16:54, Felipe Balbi wrote:
> >Hi,
> >
> >On Wed, Jun 18, 2014 at 06:01:08PM +0100, Srinivas Kandagatla wrote:
> >>Use case is when the phy is configured in host mode and a usb device is
> >>attached to board before bootup. On bootup, with the existing code and
> >>runtime pm enabled, the driver would decrement the pm usage count
> >>without checking the current state of the phy. This pm usage count
> >>decrement would trigger the runtime pm which than would abort the
> >>usb enumeration which was in progress. In my case a usb stick gets
> >>detected and then immediatly the driver goes to low power mode which is
> >>not correct.
> >>
> >>log:
> >>[    1.631412] msm_hsusb_host 12520000.usb: EHCI Host Controller
> >>[    1.636556] msm_hsusb_host 12520000.usb: new USB bus registered, assigned bus number 1
> >>[    1.642563] msm_hsusb_host 12520000.usb: irq 220, io mem 0x12520000
> >>[    1.658197] msm_hsusb_host 12520000.usb: USB 2.0 started, EHCI 1.00
> >>[    1.659473] hub 1-0:1.0: USB hub found
> >>[    1.663415] hub 1-0:1.0: 1 port detected
> >>...
> >>[    1.973352] usb 1-1: new high-speed USB device number 2 using msm_hsusb_host
> >>[    2.107707] usb-storage 1-1:1.0: USB Mass Storage device detected
> >>[    2.108993] scsi0 : usb-storage 1-1:1.0
> >>[    2.678341] msm_otg 12520000.phy: USB in low power mode
> >>[    3.168977] usb 1-1: USB disconnect, device number 2
> >>
> >>This issue was detected on IFC6410 board.
> >>
> >>This patch fixes the intial runtime pm trigger by checking the phy
> >>state and decrementing the pm use count only when the phy state is IDLE.
> >>
> >>Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>
> >>---
> >>  drivers/usb/phy/phy-msm-usb.c | 4 +++-
> >>  1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
> >>index 3bb559d..78cc870 100644
> >>--- a/drivers/usb/phy/phy-msm-usb.c
> >>+++ b/drivers/usb/phy/phy-msm-usb.c
> >>@@ -1229,7 +1229,9 @@ static void msm_otg_sm_work(struct work_struct *w)
> >>  			motg->chg_state = USB_CHG_STATE_UNDEFINED;
> >>  			motg->chg_type = USB_INVALID_CHARGER;
> >>  		}
> >>-		pm_runtime_put_sync(otg->phy->dev);
> >>+
> >>+		if (otg->phy->state == OTG_STATE_B_IDLE)
> >>+			pm_runtime_put_sync(otg->phy->dev);
> >
> >instead, you might as well just use autosuspend.
> 
> autosuspend is a good idea and will provide a delay before rumtime suspend,
> however the bug which is fixed here still needs to be fixed anyway.
> 
> runtime PM in this driver is not that great, the driver just increments the
> pm use count if the phy is configured in host or deivce mode. This patch
> fixes a bug in its state-machine which decrements pm use-count without
> checking the state.
> 
> As this is a PHY driver, am not sure moving to autosuspend means we would
> end up just adding some static inactivity delay? Is it really going to add
> any value to this PHY driver?

yeah, perhaps not... I guess we can apply this as a fix; you're right.

-- 
balbi

Attachment: signature.asc
Description: Digital signature


[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux