Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <5ccff4a9-1073-ea82-8360-445a956e9266@kernel.org>
Date:   Wed, 12 Jul 2023 15:16:05 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: [PATCH v3 0/3] Support dwc3 runtime suspend during bus suspend
Content-Language: en-US
To:     Elson Roy Serrao <quic_eserrao@quicinc.com>,
        Thinh.Nguyen@synopsys.com, stern@rowland.harvard.edu
Cc:     gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
        linux-usb@vger.kernel.org, quic_wcheng@quicinc.com,
        quic_jackp@quicinc.com
References: <20230711174320.24058-1-quic_eserrao@quicinc.com>
From:   Roger Quadros <rogerq@kernel.org>
In-Reply-To: <20230711174320.24058-1-quic_eserrao@quicinc.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi Elson,

On 11/07/2023 20:43, Elson Roy Serrao wrote:
> Changes in v3
>  - Added a dt property 'snps,allow-rtsusp-on-u3' to make this feature platform
>    dependent as per the feedback from Thinh N.
>  - Changed the RT idle/suspend/resume handling to device mode specific and dt
>    property dependent.
>  - Modified the cover letter to document how resume is handled on qcom platforms.
>  
> Changes in v2
>  - Used pm_runtime_resume_and_get() API instead of pm_runtime_get_sync()
>    as suggested by Dan.
>  - Handled the return value in ether_wakeup_host to print error message.
> 
> When a USB link is idle, the host sends a bus suspend event to the device
> so that the device can save power. But true power savings during bus
> suspend can be seen only if we let the USB controller enter low power
> mode and turn off the clocks. Vendor drivers may have their own runtime
> power management framework to power up/down the controller. But since
> vendor drivers' runtime suspend/resume routines depend on the dwc3 child
> node we would need a framework to trigger dwc3 runtime pm ops whenever a
> bus suspend is received. If the device wants to exit from bus suspend
> state it can send a wakeup signal to the host by first bringing out the
> controller from low power mode. This series implements the needed
> framework to achieve this functionality when a bus suspend interupt is
> received. The assumption here is that the dwc3 hibernation feature is not
> enabled and the platform is responsible in detecting the resume events to
> bring the controller out of suspend.
> 
> On Qualcomm platforms the bus resume is handled through Phy and informed to
> software through wakeup capable phy interrupts.
> usb2 PHY is configured to detect the Resume K event and sends an interrupt
> when this event is detected. This would trigger the runtime resume of the
> glue driver which would intrinsically wakeup the dwc3 child. In case of usb3 PHY,
> it is configured to detect the LFPS wake signal during bus resume and the
> corresponding interrupt triggers the runtime resume of the glue driver.

Subject says runtime suspend. But are you testing system sleep/wakeup as well
while USB is suspended?

> 
> The series is organized in below fashion:
> Patch 1: This includes the modification needed from function drivers to let
> UDC enter low power mode with u_ether as an example.
> Patch 2: New dt property to allow dwc3 runtime suspedn during bus suspend scenario. 
> Patch 3: This has the modification needed in the UDC driver to trigger runtime
> suspend whene a bus suspend interrupt is received. Since this is a platform
> dependent change it is made applicable through a dt property. This also handles
> resume and remote wakeup modifications from power management perspective.
> 
> Elson Roy Serrao (3):
>   usb: function: u_ether: Handle rx requests during suspend/resume
>   dt-bindings: usb: snps,dwc3: Add allow-rtsusp-on-u3 property
>   usb: dwc3: Modify runtime pm ops to handle bus suspend
> 
>  .../devicetree/bindings/usb/snps,dwc3.yaml    |  5 ++
>  drivers/usb/dwc3/core.c                       | 26 ++++++++--
>  drivers/usb/dwc3/core.h                       |  3 ++
>  drivers/usb/dwc3/gadget.c                     | 40 +++++++++++++---
>  drivers/usb/gadget/function/u_ether.c         | 47 +++++++++++++++----
>  5 files changed, 102 insertions(+), 19 deletions(-)
> 

-- 
cheers,
-roger