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
