diff mbox series

usb: dwc3: gadget: remove warning during kernel boot

Message ID	20240220081205.135063-1-raychi@google.com
State	New
Headers	Received-SPF: pass (google.com: domain of linux-kernel+bounces-72517-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3; Date: Tue, 20 Feb 2024 16:12:04 +0800 Precedence: bulk Mime-Version: 1.0 Message-ID: <20240220081205.135063-1-raychi@google.com> Subject: [PATCH] usb: dwc3: gadget: remove warning during kernel boot From: Ray Chi <raychi@google.com> To: Thinh.Nguyen@synopsys.com, gregkh@linuxfoundation.org, quic_uaggarwa@quicinc.com Cc: albertccwang@google.com, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, Ray Chi <raychi@google.com>, stable@vger.kernel.org Content-Type: text/plain; charset="UTF-8" X-getmail-retrieved-from-mailbox: INBOX
Series	usb: dwc3: gadget: remove warning during kernel boot \| usb: dwc3: gadget: remove warning during kernel boot

Commit Message

Ray Chi Feb. 20, 2024, 8:12 a.m. UTC

  The dwc3->gadget_driver is not initialized during the dwc3 probe
process. This leads to a warning when the runtime power management (PM)
attempts to suspend the gadget using dwc3_gadget_suspend().

This patch adds a check to prevent the warning.

Cc: stable@vger.kernel.org
Fixes: 61a348857e86 ("usb: dwc3: gadget: Fix NULL pointer dereference in dwc3_gadget_suspend")
Signed-off-by: Ray Chi <raychi@google.com>
---
 drivers/usb/dwc3/gadget.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Ray Chi Feb. 20, 2024, 8:20 a.m. UTC | #1

On Tue, Feb 20, 2024 at 4:12 PM Ray Chi <raychi@google.com> wrote:
>
> The dwc3->gadget_driver is not initialized during the dwc3 probe
> process. This leads to a warning when the runtime power management (PM)
> attempts to suspend the gadget using dwc3_gadget_suspend().
>
> This patch adds a check to prevent the warning.
>
> Cc: stable@vger.kernel.org
> Fixes: 61a348857e86 ("usb: dwc3: gadget: Fix NULL pointer dereference in dwc3_gadget_suspend")
> Signed-off-by: Ray Chi <raychi@google.com>
> ---
>  drivers/usb/dwc3/gadget.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 28f49400f3e8..de987cffe1ec 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -4708,6 +4708,9 @@ int dwc3_gadget_suspend(struct dwc3 *dwc)
>         unsigned long flags;
>         int ret;
>
> +       if (!dwc->gadget_driver)
> +               return 0;
> +
>         ret = dwc3_gadget_soft_disconnect(dwc);
>         if (ret)
>                 goto err;
> --
> 2.44.0.rc0.258.g7320e95886-goog
>

Attached the warning message.

02-20 06:01:24.227725  root    86    86 W Call trace:
02-20 06:01:24.227730  root    86    86 W         :
dwc3_ep0_out_start+0xec/0x22c
02-20 06:01:24.227738  root    86    86 W         :
dwc3_ep0_stall_and_restart+0xb8/0xd4
02-20 06:01:24.227747  root    86    86 W         :
dwc3_gadget_soft_disconnect+0x1a8/0x284
02-20 06:01:24.227756  root    86    86 W         :
dwc3_gadget_suspend+0x18/0xe4
02-20 06:01:24.227764  root    86    86 W         :
dwc3_suspend_common+0x5c/0x320
02-20 06:01:24.227774  root    86    86 W         :
dwc3_runtime_suspend+0x30/0x4c
02-20 06:01:24.227783  root    86    86 W         :
pm_generic_runtime_suspend+0x3c/0x54
02-20 06:01:24.227793  root    86    86 W         : __rpm_callback+0xfc/0x76c
02-20 06:01:24.227802  root    86    86 W         : rpm_suspend+0x534/0xd78
02-20 06:01:24.227811  root    86    86 W         : dwc3_runtime_idle+0xcc/0x104
02-20 06:01:24.227819  root    86    86 W         : rpm_idle+0x228/0x4e0
02-20 06:01:24.227827  root    86    86 W         : update_autosuspend+0x50/0xb8
02-20 06:01:24.227836  root    86    86 W         :
__pm_runtime_use_autosuspend+0x50/0x6c

Greg KH Feb. 20, 2024, 8:34 a.m. UTC | #2

On Tue, Feb 20, 2024 at 04:12:04PM +0800, Ray Chi wrote:
> The dwc3->gadget_driver is not initialized during the dwc3 probe
> process. This leads to a warning when the runtime power management (PM)
> attempts to suspend the gadget using dwc3_gadget_suspend().

What type of warning happens?

> This patch adds a check to prevent the warning.
> 
> Cc: stable@vger.kernel.org
> Fixes: 61a348857e86 ("usb: dwc3: gadget: Fix NULL pointer dereference in dwc3_gadget_suspend")
> Signed-off-by: Ray Chi <raychi@google.com>
> ---
>  drivers/usb/dwc3/gadget.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 28f49400f3e8..de987cffe1ec 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -4708,6 +4708,9 @@ int dwc3_gadget_suspend(struct dwc3 *dwc)
>  	unsigned long flags;
>  	int ret;
>  
> +	if (!dwc->gadget_driver)
> +		return 0;
> +

This directly reverts part of the commit you say this fixes, are you
SURE about this?  Why?

thanks,

greg k-h

Greg KH Feb. 20, 2024, 8:39 a.m. UTC | #3

On Tue, Feb 20, 2024 at 04:20:50PM +0800, Ray Chi wrote:
> On Tue, Feb 20, 2024 at 4:12 PM Ray Chi <raychi@google.com> wrote:
> >
> > The dwc3->gadget_driver is not initialized during the dwc3 probe
> > process. This leads to a warning when the runtime power management (PM)
> > attempts to suspend the gadget using dwc3_gadget_suspend().
> >
> > This patch adds a check to prevent the warning.
> >
> > Cc: stable@vger.kernel.org
> > Fixes: 61a348857e86 ("usb: dwc3: gadget: Fix NULL pointer dereference in dwc3_gadget_suspend")
> > Signed-off-by: Ray Chi <raychi@google.com>
> > ---
> >  drivers/usb/dwc3/gadget.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > index 28f49400f3e8..de987cffe1ec 100644
> > --- a/drivers/usb/dwc3/gadget.c
> > +++ b/drivers/usb/dwc3/gadget.c
> > @@ -4708,6 +4708,9 @@ int dwc3_gadget_suspend(struct dwc3 *dwc)
> >         unsigned long flags;
> >         int ret;
> >
> > +       if (!dwc->gadget_driver)
> > +               return 0;
> > +
> >         ret = dwc3_gadget_soft_disconnect(dwc);
> >         if (ret)
> >                 goto err;
> > --
> > 2.44.0.rc0.258.g7320e95886-goog
> >
> 
> Attached the warning message.
> 
> 02-20 06:01:24.227725  root    86    86 W Call trace:
> 02-20 06:01:24.227730  root    86    86 W         :
> dwc3_ep0_out_start+0xec/0x22c
> 02-20 06:01:24.227738  root    86    86 W         :
> dwc3_ep0_stall_and_restart+0xb8/0xd4
> 02-20 06:01:24.227747  root    86    86 W         :
> dwc3_gadget_soft_disconnect+0x1a8/0x284
> 02-20 06:01:24.227756  root    86    86 W         :
> dwc3_gadget_suspend+0x18/0xe4
> 02-20 06:01:24.227764  root    86    86 W         :
> dwc3_suspend_common+0x5c/0x320
> 02-20 06:01:24.227774  root    86    86 W         :
> dwc3_runtime_suspend+0x30/0x4c
> 02-20 06:01:24.227783  root    86    86 W         :
> pm_generic_runtime_suspend+0x3c/0x54
> 02-20 06:01:24.227793  root    86    86 W         : __rpm_callback+0xfc/0x76c
> 02-20 06:01:24.227802  root    86    86 W         : rpm_suspend+0x534/0xd78
> 02-20 06:01:24.227811  root    86    86 W         : dwc3_runtime_idle+0xcc/0x104
> 02-20 06:01:24.227819  root    86    86 W         : rpm_idle+0x228/0x4e0
> 02-20 06:01:24.227827  root    86    86 W         : update_autosuspend+0x50/0xb8
> 02-20 06:01:24.227836  root    86    86 W         :
> __pm_runtime_use_autosuspend+0x50/0x6c

That's a kernel warning?  Odd wrapping :(

Can you clean it up a bit and put it in the changelog?  There should be
more info before the "Call trace" portion, right?  What is causing the
warning exactly?

thanks,

greg k-h

Krishna Kurapati Feb. 20, 2024, 8:40 a.m. UTC | #4

On 2/20/2024 2:04 PM, Greg KH wrote:
> On Tue, Feb 20, 2024 at 04:12:04PM +0800, Ray Chi wrote:
>> The dwc3->gadget_driver is not initialized during the dwc3 probe
>> process. This leads to a warning when the runtime power management (PM)
>> attempts to suspend the gadget using dwc3_gadget_suspend().
> 
> What type of warning happens?
> 
>> This patch adds a check to prevent the warning.
>>
>> Cc: stable@vger.kernel.org
>> Fixes: 61a348857e86 ("usb: dwc3: gadget: Fix NULL pointer dereference in dwc3_gadget_suspend")
>> Signed-off-by: Ray Chi <raychi@google.com>
>> ---
>>   drivers/usb/dwc3/gadget.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index 28f49400f3e8..de987cffe1ec 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -4708,6 +4708,9 @@ int dwc3_gadget_suspend(struct dwc3 *dwc)
>>   	unsigned long flags;
>>   	int ret;
>>   
>> +	if (!dwc->gadget_driver)
>> +		return 0;
>> +
> 
> This directly reverts part of the commit you say this fixes, are you
> SURE about this?  Why?
> 

Hi Ray,

Thinh sent a patch recently addressing the issue in soft disconnect.
Can you check if it helps:

https://lore.kernel.org/all/e3be9b929934e0680a6f4b8f6eb11b18ae9c7e07.1708043922.git.Thinh.Nguyen@synopsys.com/

Regards,
Krishna,

Ray Chi Feb. 20, 2024, 9:42 a.m. UTC | #5

Hi Krishna,

I verified the Thinh's patch and the warning could be
fixed. Thanks for the information.

Regards,
Ray

On Tue, Feb 20, 2024 at 4:40 PM Krishna Kurapati PSSNV
<quic_kriskura@quicinc.com> wrote:
>
>
>
> On 2/20/2024 2:04 PM, Greg KH wrote:
> > On Tue, Feb 20, 2024 at 04:12:04PM +0800, Ray Chi wrote:
> >> The dwc3->gadget_driver is not initialized during the dwc3 probe
> >> process. This leads to a warning when the runtime power management (PM)
> >> attempts to suspend the gadget using dwc3_gadget_suspend().
> >
> > What type of warning happens?
> >
> >> This patch adds a check to prevent the warning.
> >>
> >> Cc: stable@vger.kernel.org
> >> Fixes: 61a348857e86 ("usb: dwc3: gadget: Fix NULL pointer dereference in dwc3_gadget_suspend")
> >> Signed-off-by: Ray Chi <raychi@google.com>
> >> ---
> >>   drivers/usb/dwc3/gadget.c | 3 +++
> >>   1 file changed, 3 insertions(+)
> >>
> >> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> >> index 28f49400f3e8..de987cffe1ec 100644
> >> --- a/drivers/usb/dwc3/gadget.c
> >> +++ b/drivers/usb/dwc3/gadget.c
> >> @@ -4708,6 +4708,9 @@ int dwc3_gadget_suspend(struct dwc3 *dwc)
> >>      unsigned long flags;
> >>      int ret;
> >>
> >> +    if (!dwc->gadget_driver)
> >> +            return 0;
> >> +
> >
> > This directly reverts part of the commit you say this fixes, are you
> > SURE about this?  Why?
> >
>
> Hi Ray,
>
> Thinh sent a patch recently addressing the issue in soft disconnect.
> Can you check if it helps:
>
> https://lore.kernel.org/all/e3be9b929934e0680a6f4b8f6eb11b18ae9c7e07.1708043922.git.Thinh.Nguyen@synopsys.com/
>
> Regards,
> Krishna,

Greg KH Feb. 20, 2024, 1:56 p.m. UTC | #6

On Tue, Feb 20, 2024 at 05:42:56PM +0800, Ray Chi wrote:
> Hi Krishna,
> 
> I verified the Thinh's patch and the warning could be
> fixed. Thanks for the information.

Can you provide a tested-by for that one?

And please do not top post :(

thanks,

greg k-h

Ray Chi Feb. 23, 2024, 10:36 a.m. UTC | #7

On Tue, Feb 20, 2024 at 9:56 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Tue, Feb 20, 2024 at 05:42:56PM +0800, Ray Chi wrote:
> > Hi Krishna,
> >
> > I verified the Thinh's patch and the warning could be
> > fixed. Thanks for the information.
>
> Can you provide a tested-by for that one?

Since the solution has been merged, do I still need to provide tested-by?
If tested-by is required, should I reply to the email thread for the
merged patch or the reported patch?

> And please do not top post :(

Thanks for the tips and your patience.

Regards,
Ray

diff mbox series

Patch

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 28f49400f3e8..de987cffe1ec 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -4708,6 +4708,9 @@  int dwc3_gadget_suspend(struct dwc3 *dwc)
 	unsigned long flags;
 	int ret;
 
+	if (!dwc->gadget_driver)
+		return 0;
+
 	ret = dwc3_gadget_soft_disconnect(dwc);
 	if (ret)
 		goto err;