usb: dwc3: qcom: Add shutdown handler

[PATCH] usb: dwc3: qcom: Add shutdown handler

Posted by Prashanth K 2 months, 1 week ago

Currently during system reboot, SMMU disables its translations
while devices like USB may still be actively using DMA buffers.
This can lead to NOC errors and system crashes due to invalid
memory access.

Address this by adding a shutdown callback to dwc3-qcom, which
ensures proper teardown of UDC stack and prevents DWC3 controller
from accessing memory after SMMU translation is disabled. Reuse
the existing remove callback for this purpose.

Signed-off-by: Prashanth K <prashanth.k@oss.qualcomm.com>
---
 drivers/usb/dwc3/dwc3-qcom.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index ca7e1c02773a..308360a32c93 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -764,11 +764,14 @@ static void dwc3_qcom_remove(struct platform_device *pdev)
 	struct dwc3 *dwc = platform_get_drvdata(pdev);
 	struct dwc3_qcom *qcom = to_dwc3_qcom(dwc);
 
-	dwc3_core_remove(&qcom->dwc);
+	if (pm_runtime_resume_and_get(qcom->dev) < 0)
+		return;
 
+	dwc3_core_remove(&qcom->dwc);
 	clk_bulk_disable_unprepare(qcom->num_clocks, qcom->clks);
-
 	dwc3_qcom_interconnect_exit(qcom);
+
+	pm_runtime_put_noidle(qcom->dev);
 }
 
 static int dwc3_qcom_pm_suspend(struct device *dev)
@@ -873,6 +876,7 @@ MODULE_DEVICE_TABLE(of, dwc3_qcom_of_match);
 static struct platform_driver dwc3_qcom_driver = {
 	.probe		= dwc3_qcom_probe,
 	.remove		= dwc3_qcom_remove,
+	.shutdown	= dwc3_qcom_remove,
 	.driver		= {
 		.name	= "dwc3-qcom",
 		.pm	= pm_ptr(&dwc3_qcom_dev_pm_ops),
-- 
2.25.1

Re: [PATCH] usb: dwc3: qcom: Add shutdown handler

Posted by Thinh Nguyen 2 months, 1 week ago

On Fri, Jul 25, 2025, Prashanth K wrote:
> Currently during system reboot, SMMU disables its translations
> while devices like USB may still be actively using DMA buffers.
> This can lead to NOC errors and system crashes due to invalid
> memory access.
> 
> Address this by adding a shutdown callback to dwc3-qcom, which
> ensures proper teardown of UDC stack and prevents DWC3 controller
> from accessing memory after SMMU translation is disabled. Reuse
> the existing remove callback for this purpose.
> 
> Signed-off-by: Prashanth K <prashanth.k@oss.qualcomm.com>
> ---
>  drivers/usb/dwc3/dwc3-qcom.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index ca7e1c02773a..308360a32c93 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -764,11 +764,14 @@ static void dwc3_qcom_remove(struct platform_device *pdev)
>  	struct dwc3 *dwc = platform_get_drvdata(pdev);
>  	struct dwc3_qcom *qcom = to_dwc3_qcom(dwc);
>  
> -	dwc3_core_remove(&qcom->dwc);
> +	if (pm_runtime_resume_and_get(qcom->dev) < 0)
> +		return;
>  
> +	dwc3_core_remove(&qcom->dwc);
>  	clk_bulk_disable_unprepare(qcom->num_clocks, qcom->clks);
> -
>  	dwc3_qcom_interconnect_exit(qcom);
> +
> +	pm_runtime_put_noidle(qcom->dev);
>  }
>  
>  static int dwc3_qcom_pm_suspend(struct device *dev)
> @@ -873,6 +876,7 @@ MODULE_DEVICE_TABLE(of, dwc3_qcom_of_match);
>  static struct platform_driver dwc3_qcom_driver = {
>  	.probe		= dwc3_qcom_probe,
>  	.remove		= dwc3_qcom_remove,
> +	.shutdown	= dwc3_qcom_remove,
>  	.driver		= {
>  		.name	= "dwc3-qcom",
>  		.pm	= pm_ptr(&dwc3_qcom_dev_pm_ops),
> -- 
> 2.25.1
> 

Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

BR,
Thinh

Re: [PATCH] usb: dwc3: qcom: Add shutdown handler

Posted by Greg Kroah-Hartman 2 months, 1 week ago

On Fri, Jul 25, 2025 at 11:51:58AM +0530, Prashanth K wrote:
> Currently during system reboot, SMMU disables its translations
> while devices like USB may still be actively using DMA buffers.
> This can lead to NOC errors and system crashes due to invalid
> memory access.
> 
> Address this by adding a shutdown callback to dwc3-qcom, which
> ensures proper teardown of UDC stack and prevents DWC3 controller
> from accessing memory after SMMU translation is disabled. Reuse
> the existing remove callback for this purpose.
> 
> Signed-off-by: Prashanth K <prashanth.k@oss.qualcomm.com>
> ---
>  drivers/usb/dwc3/dwc3-qcom.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)

What commit id does this fix?  Or is this just a new feature?

thanks,

greg k-h

Re: [PATCH] usb: dwc3: qcom: Add shutdown handler

Posted by Prashanth K 2 months, 1 week ago


On 7/25/2025 2:18 PM, Greg Kroah-Hartman wrote:
> On Fri, Jul 25, 2025 at 11:51:58AM +0530, Prashanth K wrote:
>> Currently during system reboot, SMMU disables its translations
>> while devices like USB may still be actively using DMA buffers.
>> This can lead to NOC errors and system crashes due to invalid
>> memory access.
>>
>> Address this by adding a shutdown callback to dwc3-qcom, which
>> ensures proper teardown of UDC stack and prevents DWC3 controller
>> from accessing memory after SMMU translation is disabled. Reuse
>> the existing remove callback for this purpose.
>>
>> Signed-off-by: Prashanth K <prashanth.k@oss.qualcomm.com>
>> ---
>>  drivers/usb/dwc3/dwc3-qcom.c | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> What commit id does this fix?  Or is this just a new feature?
> 
> thanks,
> 
> greg k-h

I didn't Fixes tag because a small dilemma. This patch is rebased on top
of the flattening series, specifically commit 1881a32fe14d ("usb: dwc3:
qcom: Transition to flattened model").
However, I don't think that this commit caused this issue.

Let me know if you want the Fixes tag.

Thanks in advance,
Prashanth K

Re: [PATCH] usb: dwc3: qcom: Add shutdown handler

Posted by Greg Kroah-Hartman 2 months, 1 week ago

On Fri, Jul 25, 2025 at 03:28:12PM +0530, Prashanth K wrote:
> 
> 
> On 7/25/2025 2:18 PM, Greg Kroah-Hartman wrote:
> > On Fri, Jul 25, 2025 at 11:51:58AM +0530, Prashanth K wrote:
> >> Currently during system reboot, SMMU disables its translations
> >> while devices like USB may still be actively using DMA buffers.
> >> This can lead to NOC errors and system crashes due to invalid
> >> memory access.
> >>
> >> Address this by adding a shutdown callback to dwc3-qcom, which
> >> ensures proper teardown of UDC stack and prevents DWC3 controller
> >> from accessing memory after SMMU translation is disabled. Reuse
> >> the existing remove callback for this purpose.
> >>
> >> Signed-off-by: Prashanth K <prashanth.k@oss.qualcomm.com>
> >> ---
> >>  drivers/usb/dwc3/dwc3-qcom.c | 8 ++++++--
> >>  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > What commit id does this fix?  Or is this just a new feature?
> > 
> > thanks,
> > 
> > greg k-h
> 
> I didn't Fixes tag because a small dilemma. This patch is rebased on top
> of the flattening series, specifically commit 1881a32fe14d ("usb: dwc3:
> qcom: Transition to flattened model").
> However, I don't think that this commit caused this issue.
> 
> Let me know if you want the Fixes tag.

If you don't need/want this backported to any stable tree, then leave it
as-is.  Your choice.

Re: [PATCH] usb: dwc3: qcom: Add shutdown handler

Posted by Prashanth K 2 months, 1 week ago


On 7/25/2025 5:29 PM, Greg Kroah-Hartman wrote:
> On Fri, Jul 25, 2025 at 03:28:12PM +0530, Prashanth K wrote:
>>
>>
>> On 7/25/2025 2:18 PM, Greg Kroah-Hartman wrote:
>>> On Fri, Jul 25, 2025 at 11:51:58AM +0530, Prashanth K wrote:
>>>> Currently during system reboot, SMMU disables its translations
>>>> while devices like USB may still be actively using DMA buffers.
>>>> This can lead to NOC errors and system crashes due to invalid
>>>> memory access.
>>>>
>>>> Address this by adding a shutdown callback to dwc3-qcom, which
>>>> ensures proper teardown of UDC stack and prevents DWC3 controller
>>>> from accessing memory after SMMU translation is disabled. Reuse
>>>> the existing remove callback for this purpose.
>>>>
>>>> Signed-off-by: Prashanth K <prashanth.k@oss.qualcomm.com>
>>>> ---
>>>>  drivers/usb/dwc3/dwc3-qcom.c | 8 ++++++--
>>>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> What commit id does this fix?  Or is this just a new feature?
>>>
>>> thanks,
>>>
>>> greg k-h
>>
>> I didn't Fixes tag because a small dilemma. This patch is rebased on top
>> of the flattening series, specifically commit 1881a32fe14d ("usb: dwc3:
>> qcom: Transition to flattened model").
>> However, I don't think that this commit caused this issue.
>>
>> Let me know if you want the Fixes tag.
> 
> If you don't need/want this backported to any stable tree, then leave it
> as-is.  Your choice.
> 

Lets not take it to stable now, because it will cause conflicts.
If we encounter any issue in future, we can manually resolve the
conflict and backport to stable.

Regards,
Prashanth K