[PATCH] Revert "xhci: plat: Register shutdown for xhci_plat"

[PATCH] Revert "xhci: plat: Register shutdown for xhci_plat"

[Greg Hackmann]

Pixel 2 field testers reported that when they tried to reboot their
phones with some USB devices plugged in, the reboot would get wedged and
eventually trigger watchdog reset.  Once the Pixel kernel team found a
reliable repro case, they narrowed it down to this commit's 4.4.y
backport.  Reverting the change made the issue go away.

This reverts commit b07c12517f2aed0add8ce18146bb426b14099392.

Cc: stable@vger.kernel.org
Signed-off-by: Greg Hackmann <ghackmann@google.com>
---
 drivers/usb/host/xhci-plat.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index df327dcc2bac..ea089fdda611 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -420,7 +420,6 @@ MODULE_DEVICE_TABLE(acpi, usb_xhci_acpi_match);
 static struct platform_driver usb_xhci_driver = {
 	.probe	= xhci_plat_probe,
 	.remove	= xhci_plat_remove,
-	.shutdown	= usb_hcd_platform_shutdown,
 	.driver	= {
 		.name = "xhci-hcd",
 		.pm = &xhci_plat_pm_ops,
-- 
2.17.0.484.g0c8726318c-goog

Re: [PATCH] Revert "xhci: plat: Register shutdown for xhci_plat"

[Harsh Shandilya]

On 13 April 2018 5:59:51 AM IST, Greg Hackmann <ghackmann@google.com> wrote:
>Pixel 2 field testers reported that when they tried to reboot their
>phones with some USB devices plugged in, the reboot would get wedged
>and
>eventually trigger watchdog reset.  Once the Pixel kernel team found a
>reliable repro case, they narrowed it down to this commit's 4.4.y
>backport.  Reverting the change made the issue go away.

Are you allowed to make the repro steps public? I'm writing this from a walleye and would be grateful if I could test for this in the modifed tree I'm running atm. 
-- 
Harsh Shandilya, PRJKT Development LLC

Re: [PATCH] Revert "xhci: plat: Register shutdown for xhci_plat"

[Greg Kroah-Hartman]

On Fri, Apr 13, 2018 at 08:12:31AM +0530, Harsh Shandilya wrote:
> On 13 April 2018 5:59:51 AM IST, Greg Hackmann <ghackmann@google.com> wrote:
> >Pixel 2 field testers reported that when they tried to reboot their
> >phones with some USB devices plugged in, the reboot would get wedged
> >and
> >eventually trigger watchdog reset.  Once the Pixel kernel team found a
> >reliable repro case, they narrowed it down to this commit's 4.4.y
> >backport.  Reverting the change made the issue go away.
> 
> Are you allowed to make the repro steps public? I'm writing this from
> a walleye and would be grateful if I could test for this in the
> modifed tree I'm running atm.  -- 

I was told the steps are pretty simple:
	- reboot the phone a lot
eventually it will hang.  There's a fix in the code aurora kernel tree
for this that they never sent upstream for some odd reason (they sent
the first patch, why not the second?)

I'll go revert this for now, thanks for the patch!

greg k-h

Re: [PATCH] Revert "xhci: plat: Register shutdown for xhci_plat"

[Harsh Shandilya]

On 13 April 2018 11:51:28 AM IST, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
>On Fri, Apr 13, 2018 at 08:12:31AM +0530, Harsh Shandilya wrote:
>> On 13 April 2018 5:59:51 AM IST, Greg Hackmann <ghackmann@google.com>
>wrote:
>> >Pixel 2 field testers reported that when they tried to reboot their
>> >phones with some USB devices plugged in, the reboot would get wedged
>> >and
>> >eventually trigger watchdog reset.  Once the Pixel kernel team found
>a
>> >reliable repro case, they narrowed it down to this commit's 4.4.y
>> >backport.  Reverting the change made the issue go away.
>> 
>> Are you allowed to make the repro steps public? I'm writing this from
>> a walleye and would be grateful if I could test for this in the
>> modifed tree I'm running atm.  -- 
>
>I was told the steps are pretty simple:
>	- reboot the phone a lot
>eventually it will hang.  There's a fix in the code aurora kernel tree
>for this that they never sent upstream for some odd reason (they sent
>the first patch, why not the second?)
>
>I'll go revert this for now, thanks for the patch!
>
>greg k-h

That'd make sense, I only tried rebooting like five times before I had to run for a class.

As far as CAF is concerned, I feel the not submitting upstream, working extra to write patches which have usually better variants already upstream, seems to be common. All USB changes were dropped when they merged kernel-common into msm-3.18 with no real explanation which has been an annoyance more than once during merging -stable in my fork of msm-3.18. While I understand their situation of maintaining upwards of 5 million lines of code not upstream, it still feels sloppy to not merge stable updates and do extra work instead. /* End rant */
-- 
Harsh Shandilya, PRJKT Development LLC

Re: [PATCH] Revert "xhci: plat: Register shutdown for xhci_plat"

[Greg Kroah-Hartman]

On Fri, Apr 13, 2018 at 12:34:00PM +0530, Harsh Shandilya wrote:
> On 13 April 2018 11:51:28 AM IST, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> >On Fri, Apr 13, 2018 at 08:12:31AM +0530, Harsh Shandilya wrote:
> >> On 13 April 2018 5:59:51 AM IST, Greg Hackmann <ghackmann@google.com>
> >wrote:
> >> >Pixel 2 field testers reported that when they tried to reboot their
> >> >phones with some USB devices plugged in, the reboot would get wedged
> >> >and
> >> >eventually trigger watchdog reset.  Once the Pixel kernel team found
> >a
> >> >reliable repro case, they narrowed it down to this commit's 4.4.y
> >> >backport.  Reverting the change made the issue go away.
> >> 
> >> Are you allowed to make the repro steps public? I'm writing this from
> >> a walleye and would be grateful if I could test for this in the
> >> modifed tree I'm running atm.  -- 
> >
> >I was told the steps are pretty simple:
> >	- reboot the phone a lot
> >eventually it will hang.  There's a fix in the code aurora kernel tree
> >for this that they never sent upstream for some odd reason (they sent
> >the first patch, why not the second?)
> >
> >I'll go revert this for now, thanks for the patch!
> >
> >greg k-h
> 
> That'd make sense, I only tried rebooting like five times before I had to run for a class.
> 
> As far as CAF is concerned, I feel the not submitting upstream,
> working extra to write patches which have usually better variants
> already upstream, seems to be common. All USB changes were dropped
> when they merged kernel-common into msm-3.18 with no real explanation
> which has been an annoyance more than once during merging -stable in
> my fork of msm-3.18. While I understand their situation of maintaining
> upwards of 5 million lines of code not upstream, it still feels sloppy
> to not merge stable updates and do extra work instead. /* End rant */

CAF fixed this back on Feb 1 in their tree, yet did not send that
upstream, or to anyone else:
	https://source.codeaurora.org/quic/la/kernel/msm-4.4/commit/?h=LV.HB.1.1.5-03810-8x96.0&id=a7a5307ee04ad349d365ad50f304605a9cd9bd0a

Feel free to rant some more, I'm going to go revert the original
upstream patch as that is half-completed, and obviously broken :(

thanks,

greg k-h