linux-kernel - Re: [PATCH v1] USB: dwc3: only call usb_phy_set

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:   Fri, 3 Nov 2023 12:05:15 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Francesco Dolcini <francesco@...cini.it>
Cc:     Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
        Felipe Balbi <balbi@...com>,
        Kishon Vijay Abraham I <kishon@...com>,
        Stefan Eichenberger <stefan.eichenberger@...adex.com>,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
        Francesco Dolcini <francesco.dolcini@...adex.com>
Subject: Re: [PATCH v1] USB: dwc3: only call usb_phy_set_suspend in
 suspend/resume

On Fri, Nov 03, 2023 at 11:22:36AM +0100, Francesco Dolcini wrote:
> From: Stefan Eichenberger <stefan.eichenberger@...adex.com>
> 
> Currently we have the following two call chains:
> dwc3_probe -> dwc3_core_init -> dwc3_phy_init -> usb_phy_init
> dwc3_probe -> dwc3_core_init -> dwc3_phy_power_on -> usb_phy_set_suspend
> 
> If we look at phy-generic we see the following calls:
> usb_gen_phy_init -> regulator_enable
> usb_gen_phy_init -> clk_prepare_enable
> 
> If we call usb_phy_set_suspend we call the following in phy-generic:
> nop_set_suspend -> clk_prepare_enable
> and we sent a patch to also call:
> nop_set_suspend -> regulator_enable
> 
> Because clk_prepare_enable and regulator_enable do reference counting we
> increased the reference counter of the clock and regulator to two. If we
> want to put the system into suspend we only decrease the reference
> counters by one and therefore the clock and regulator stay on.
> 
> This change fixes it by not calling usb_phy_set_suspend in
> dwc3_phy_power_on but only in dwc3_suspend_common.
> 
> Fixes: 8ba007a971bb ("usb: dwc3: core: enable the USB2 and USB3 phy in probe")
> Signed-off-by: Stefan Eichenberger <stefan.eichenberger@...adex.com>
> Signed-off-by: Francesco Dolcini <francesco.dolcini@...adex.com>
> ---
>  drivers/usb/dwc3/core.c | 17 +++++++----------
>  1 file changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 9c6bf054f15d..fae24a9c480d 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -770,12 +770,9 @@ static int dwc3_phy_power_on(struct dwc3 *dwc)
>  {
>  	int ret;
>  
> -	usb_phy_set_suspend(dwc->usb2_phy, 0);
> -	usb_phy_set_suspend(dwc->usb3_phy, 0);
> -
>  	ret = phy_power_on(dwc->usb2_generic_phy);
>  	if (ret < 0)
> -		goto err_suspend_usb3_phy;
> +		return ret;
>  
>  	ret = phy_power_on(dwc->usb3_generic_phy);
>  	if (ret < 0)
> @@ -785,9 +782,6 @@ static int dwc3_phy_power_on(struct dwc3 *dwc)
>  
>  err_power_off_usb2_phy:
>  	phy_power_off(dwc->usb2_generic_phy);
> -err_suspend_usb3_phy:
> -	usb_phy_set_suspend(dwc->usb3_phy, 1);
> -	usb_phy_set_suspend(dwc->usb2_phy, 1);
>  
>  	return ret;
>  }
> @@ -796,9 +790,6 @@ static void dwc3_phy_power_off(struct dwc3 *dwc)
>  {
>  	phy_power_off(dwc->usb3_generic_phy);
>  	phy_power_off(dwc->usb2_generic_phy);
> -
> -	usb_phy_set_suspend(dwc->usb3_phy, 1);
> -	usb_phy_set_suspend(dwc->usb2_phy, 1);
>  }
>  
>  static int dwc3_clk_enable(struct dwc3 *dwc)
> @@ -2018,6 +2009,9 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
>  		break;
>  	}
>  
> +	usb_phy_set_suspend(dwc->usb2_phy, 1);
> +	usb_phy_set_suspend(dwc->usb3_phy, 1);
> +
>  	return 0;
>  }
>  
> @@ -2027,6 +2021,9 @@ static int dwc3_resume_common(struct dwc3 *dwc, pm_message_t msg)
>  	int		ret;
>  	u32		reg;
>  
> +	usb_phy_set_suspend(dwc->usb2_phy, 0);
> +	usb_phy_set_suspend(dwc->usb3_phy, 0);
> +
>  	switch (dwc->current_dr_role) {
>  	case DWC3_GCTL_PRTCAP_DEVICE:
>  		ret = dwc3_core_init_for_resume(dwc);
> -- 
> 2.25.1
> 
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- You have marked a patch with a "Fixes:" tag for a commit that is in an
  older released kernel, yet you do not have a cc: stable line in the
  signed-off-by area at all, which means that the patch will not be
  applied to any older kernel releases.  To properly fix this, please
  follow the documented rules in the
  Documentation/process/stable-kernel-rules.rst file for how to resolve
  this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot