linux-kernel - Re: [RESEND PATCH 1/3] usb: dwc3: Don't reinitialize core during host bus-suspend/resume

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 11 Jan 2018 11:04:24 +0200
From:   Felipe Balbi <balbi@...nel.org>
To:     Manu Gautam <mgautam@...eaurora.org>, Roger Quadros <rogerq@...com>
Cc:     linux-arm-msm@...r.kernel.org, linux-usb@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        open list <linux-kernel@...r.kernel.org>,
        linux-omap <linux-omap@...r.kernel.org>
Subject: Re: [RESEND PATCH 1/3] usb: dwc3: Don't reinitialize core during host bus-suspend/resume


Hi,

Manu Gautam <mgautam@...eaurora.org> writes:
>>>> On 27/09/17 14:19, Manu Gautam wrote:
>>>>> Driver powers-off PHYs and reinitializes DWC3 core and gadget on
>>>>> resume. While this works fine for gadget mode but in host
>>>>> mode there is not re-initialization of host stack. Also, resetting
>>>>> bus as part of bus_suspend/resume is not correct which could affect
>>>>> (or disconnect) connected devices.
>>>>> Fix this by not reinitializing core on suspend/resume in host mode
>>>>> for HOST only and OTG/drd configurations.
>>>>>
>>>> All this seems correct but we (TI) were relying on dwc3_core_exit() to be called
>>>> during dwc3_suspend() to have the lowest power state for our platforms.
>>>>
>>>> After this patch, DWC3 controller and PHYs won't be turned off thus
>>>> preventing our platform from reaching low power levels.
>>>>
>>>> So this is a regression for us (TI) in v4.15-rc.
>>>>
>>>> Felipe, do you agree?
>>>>
>>>> If yes I can send a patch which fixes the regression
>>>> and also makes USB host work after suspend/resume.
>>>>
>>> I think it will be better to separate runtime_suspend and pm_suspend handling for
>>> host mode in dwc3. Powering offf/on PHYs and dwc3_core_exit/init across system
>>> suspend-resume should be ok but doing that for runtime suspend-resume is not
>>> correct.
>> it sure is. It's part of hibernation-while-disconnected programming sequence
>>
>>> Let me know if that sounds ok, I can provide a patch for same instead of
>>> reverting this which affects runtime PM with dwc3 host.
>> nope, that would break platforms using hibernation
>
> Please don't mind me asking this if it is very basic, I am probably
> missing something there
>
> We should be able to distinguish between runtime_pm vs
> system_suspend/hibernation and then process accordingly.

I'm not talking about Linux suspend to disk; I'm talking about Synopsys'
Hibernation feature (open up your databook and have a read ;-)

> In host mode runtime suspend/resume could happen very often with
> device connected, and resetting h/w on every runtime_resume might not
> be desired. And PHYs drivers can also support runtime_suspend which
> would be preferred instead of shutting down phy.

We don't do anything when dwc3 is working as a host, we simply assume if
we reach dwc3.ko, xhci has done its part. Here's what our
suspend_common looks like:

static int dwc3_suspend_common(struct dwc3 *dwc)
{
	unsigned long	flags;

	switch (dwc->current_dr_role) {
	case DWC3_GCTL_PRTCAP_DEVICE:
		spin_lock_irqsave(&dwc->lock, flags);
		dwc3_gadget_suspend(dwc);
		spin_unlock_irqrestore(&dwc->lock, flags);
		dwc3_core_exit(dwc);
		break;
	case DWC3_GCTL_PRTCAP_HOST:
	default:
		/* do nothing */
		break;
	}

	return 0;
}

We're not resetting anything, not tearing down anything. No idea why
you're saying that in host mode we're breaking things apart. If you have
out-of-tree patches on top of v4.15-rc7, fix them instead of claiming
mainline is at fault.

-- 
balbi

Download attachment "signature.asc" of type "application/pgp-signature" (833 bytes)