linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Felipe Balbi <balbi@kernel.org>
To: Roger Quadros <rogerq@ti.com>
Cc: vigneshr@ti.com, gregkh@linuxfoundation.org,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	"linux-stable # = v4 . 13" <stable@vger.kernel.org>,
	Heikki Krogerus <heikki.krogerus@linux.intel.com>
Subject: usb: dwc3: core: Don't try to get PHYs during suspend/resume
Date: Thu, 11 Jan 2018 11:31:45 +0200	[thread overview]
Message-ID: <877esoyc5a.fsf@linux.intel.com> (raw)

Hi,

Roger Quadros <rogerq@ti.com> writes:
>> Roger Quadros <rogerq@ti.com> writes:
>>>>>> -	ret = dwc3_core_soft_reset(dwc);
>>>>>> +	ret = dwc3_core_get_phy(dwc);
>>>>>
>>>>> we can get_phy in dwc3_core_init() as it will get called on resume().
>>>>> This was the $subject of this patch.
>>>>
>>>> indeed. thanks :-)
>>>>
>>>
>>> oops sorry. I meant we can't call dwc3_core_get_phy() in dwc3_core_init(). :P
>> 
>> bit of a chicken-and-egg problem. We need to setup the PHY interface
>> before getting the PHYs, but can't get PHY during resume. Maybe the best
>> way here would be to check for the pointers being valid. Something like:
>> 
>> if (!phy)
>> 	get_phy();
>> 
>
> OK that should take care of not calling get_phy() on suspend.
> However there is one more issue with the approach
>
>> @@ -754,15 +754,15 @@ static int dwc3_core_init(struct dwc3 *dwc)
>>  			dwc->maximum_speed = USB_SPEED_HIGH;
>>  	}
>>  
>> -	ret = dwc3_core_get_phy(dwc);
>> +	ret = dwc3_phy_setup(dwc);
>>  	if (ret)
>>  		goto err0;
>
> here we configure PHY related bits and register the ulpi interface.
>
>>  
>> -	ret = dwc3_core_soft_reset(dwc);
>> +	ret = dwc3_core_get_phy(dwc);
>>  	if (ret)
>>  		goto err0;
>>  
>
> we got the PHYs. all OK here.
>
>> -	ret = dwc3_phy_setup(dwc);
>> +	ret = dwc3_core_soft_reset(dwc);
>>  	if (ret)
>>  		goto err0;
>
> Now we do a soft reset. This means we loose the PHY configuration bits that we did
> in dwc3_phy_setup. So we need to call dwc3_phy_setup again but not re-register the ulpi interface.
> I can use a flag there so that dwc3_ulpi_init() is done only once.

sounds like it's better to extract out a smaller function that just
checks if we need ULPI bus and registers it, something akin to:

@@ -482,6 +482,21 @@ static void dwc3_cache_hwparams(struct dwc3 *dwc)
 	parms->hwparams8 = dwc3_readl(dwc->regs, DWC3_GHWPARAMS8);
 }
 
+static int dwc3_ulpi_init(struct dwc3 *dwc)
+{
+	int intf;
+
+	intf = DWC3_GHWPARAMS3_HSPHY_IFC(dwc->hwparams.hwparams3);
+
+	if (intf == DWC3_GHWPARAMS3_HSPHY_IFC_ULPI ||
+			(intf == DWC3_GHWPARAMS3_HSPHY_IFC_UTMI_ULPI &&
+					dwc->hsphy_interface &&
+					!strncmp(dwc->hsphy_interface, "ulpi", 4)))
+		return dwc3_ulpi_init(dwc);
+
+	return 0;
+}
+
 /**
  * dwc3_phy_setup - Configure USB PHY Interface of DWC3 Core
  * @dwc: Pointer to our controller context structure
@@ -563,11 +578,6 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
 				break;
 		}
 		/* FALLTHROUGH */
-	case DWC3_GHWPARAMS3_HSPHY_IFC_ULPI:
-		ret = dwc3_ulpi_init(dwc);
-		if (ret)
-			return ret;
-		/* FALLTHROUGH */
 	default:
 		break;
 	}

Then we just call that outside of any functions that get called during PM.

             reply	other threads:[~2018-01-11  9:31 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-11  9:31 Felipe Balbi [this message]
  -- strict thread matches above, loose matches on Subject: below --
2018-01-11  9:46 usb: dwc3: core: Don't try to get PHYs during suspend/resume Roger Quadros
2018-01-11  9:23 Roger Quadros
2018-01-11  9:09 Roger Quadros
2018-01-11  8:25 Felipe Balbi
2018-01-10 14:13 Roger Quadros
2018-01-10 14:04 Felipe Balbi
2018-01-10 13:56 Roger Quadros
2018-01-10 13:33 Felipe Balbi
2018-01-10 13:24 Roger Quadros
2018-01-10 13:11 Roger Quadros

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=877esoyc5a.fsf@linux.intel.com \
    --to=balbi@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=heikki.krogerus@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=rogerq@ti.com \
    --cc=stable@vger.kernel.org \
    --cc=vigneshr@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).