linux-kernel - Re: [PATCH RFC] usb: dwc2: power off during shutdown

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:   Wed, 5 Sep 2018 08:37:03 +0000
From:   Minas Harutyunyan <Minas.Harutyunyan@...opsys.com>
To:     Stefan Wahren <stefan.wahren@...e.com>,
        Minas Harutyunyan <Minas.Harutyunyan@...opsys.com>
CC:     Grigor Tovmasyan <Grigor.Tovmasyan@...opsys.com>,
        Doug Anderson <dianders@...omium.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH RFC] usb: dwc2: power off during shutdown

Hi Stefan,

On 9/1/2018 1:55 PM, Stefan Wahren wrote:
> Currently USB consumes a lot of power after shutting down a Raspberry Pi 3
> (example setup with Ethernet and a keyboard connected). So power off
> USB on shutdown.
> 
> Measured on Raspberry Pi 3B with 4.19rc1/multi_v7_defconfig (HDMI, Ethernet
> and a keyboard connected)
> 
> Before patch: 2.450 W
> After patch:  2.090 W
> 

Could you please elaborate. Power measurements done after shutdown? Why 
your setup continue consume power after shutdown? Maybe its 
sleep/hibernation mode?

dwc2 core in your platform which type of power optimization supports: 
partial power down, hibernation or extended hibernation? Please provide 
me GHWCFG4 register value of your core.

Thanks,
Minas

> Signed-off-by: Stefan Wahren <stefan.wahren@...e.com>
> ---
>   drivers/usb/dwc2/platform.c | 31 +++++++++++--------------------
>   1 file changed, 11 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
> index 9a53a58..eeba40a 100644
> --- a/drivers/usb/dwc2/platform.c
> +++ b/drivers/usb/dwc2/platform.c
> @@ -304,17 +304,11 @@ static int dwc2_lowlevel_hw_init(struct dwc2_hsotg *hsotg)
>   }
>   
>   /**
> - * dwc2_driver_remove() - Called when the DWC_otg core is unregistered with the
> - * DWC_otg driver
> + * dwc2_driver_shutdown() - Called on device shutdown
>    *
>    * @dev: Platform device
> - *
> - * This routine is called, for example, when the rmmod command is executed. The
> - * device may or may not be electrically present. If it is present, the driver
> - * stops device processing. Any resources used on behalf of this device are
> - * freed.
>    */
> -static int dwc2_driver_remove(struct platform_device *dev)
> +static void dwc2_driver_shutdown(struct platform_device *dev)
>   {
>   	struct dwc2_hsotg *hsotg = platform_get_drvdata(dev);
>   
> @@ -329,27 +323,24 @@ static int dwc2_driver_remove(struct platform_device *dev)
>   
>   	reset_control_assert(hsotg->reset);
>   	reset_control_assert(hsotg->reset_ecc);
> -
> -	return 0;
>   }
>   
>   /**
> - * dwc2_driver_shutdown() - Called on device shutdown
> + * dwc2_driver_remove() - Called when the DWC_otg core is unregistered with the
> + * DWC_otg driver
>    *
>    * @dev: Platform device
>    *
> - * In specific conditions (involving usb hubs) dwc2 devices can create a
> - * lot of interrupts, even to the point of overwhelming devices running
> - * at low frequencies. Some devices need to do special clock handling
> - * at shutdown-time which may bring the system clock below the threshold
> - * of being able to handle the dwc2 interrupts. Disabling dwc2-irqs
> - * prevents reboots/poweroffs from getting stuck in such cases.
> + * This routine is called, for example, when the rmmod command is executed. The
> + * device may or may not be electrically present. If it is present, the driver
> + * stops device processing. Any resources used on behalf of this device are
> + * freed.
>    */
> -static void dwc2_driver_shutdown(struct platform_device *dev)
> +static int dwc2_driver_remove(struct platform_device *dev)
>   {
> -	struct dwc2_hsotg *hsotg = platform_get_drvdata(dev);
> +	dwc2_driver_shutdown(dev);
>   
> -	disable_irq(hsotg->irq);
> +	return 0;
>   }
>   
>   /**
>