Re: patch "usb: dwc2: gadget: Do not read GINTMSK2" added to usb-testing

Re: patch "usb: dwc2: gadget: Do not read GINTMSK2" added to usb-testing

[Minas Harutyunyan]

Added linux-usb@vger.kernel.org to recipient list.

On 7/3/2020 11:43 AM, Minas Harutyunyan wrote:
> Hi,
> 
> On 7/3/2020 11:34 AM, gregkh@linuxfoundation.org wrote:
>>
>> This is a note to let you know that I've just added the patch titled
>>
>>       usb: dwc2: gadget: Do not read GINTMSK2
>>
>> to my usb git tree which can be found at
>>       git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
>> in the usb-testing branch.
>>
>> The patch will show up in the next release of the linux-next tree
>> (usually sometime within the next 24 hours during the week.)
>>
>> The patch will be merged to the usb-next branch sometime soon,
>> after it passes testing, and the merge window is open.
>>
>> If you have any questions about this process, please let me know.
>>
> 

This patch is not correct. Please read below.

>>
>>   From 4f1ae9d60adb3d4f9b8ea94f02bfc883f954bed6 Mon Sep 17 00:00:00 2001
>> From: Lee Jones <lee.jones@linaro.org>
>> Date: Thu, 2 Jul 2020 15:46:04 +0100
>> Subject: usb: dwc2: gadget: Do not read GINTMSK2
>> MIME-Version: 1.0
>> Content-Type: text/plain; charset=UTF-8
>> Content-Transfer-Encoding: 8bit
>>
>> The value of GINTMSK2 hasn't been checked since the inception of
>> dwc2_gadget_wkup_alert_handler() in 2018.
>>
>> Fixes the following W=1 warning:
>>
>>    drivers/usb/dwc2/gadget.c: In function ‘dwc2_gadget_wkup_alert_handler’:
>>    drivers/usb/dwc2/gadget.c:259:6: warning: variable ‘gintmsk2’ set but not used [-Wunused-but-set-variable]
>>    259 | u32 gintmsk2;
>>    | ^~~~~~~~
>>
>> Cc: Minas Harutyunyan <hminas@synopsys.com>
>> Cc: Ben Dooks <ben@simtec.co.uk>
>> Signed-off-by: Lee Jones <lee.jones@linaro.org>
>> Link: https://urldefense.com/v3/__https://lore.kernel.org/r/20200702144625.2533530-10-lee.jones@linaro.org__;!!A4F2R9G_pg!MlXvn0tSkTvsGZZoWD5zq_1Owq-U-DIHNaHoyU1OfG8TwGGQBEfo9g0n9r2TxvQ$
>> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> ---
>>    drivers/usb/dwc2/gadget.c | 2 --
>>    1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
>> index 1def9000f936..ea7c9c73b4ff 100644
>> --- a/drivers/usb/dwc2/gadget.c
>> +++ b/drivers/usb/dwc2/gadget.c
>> @@ -256,10 +256,8 @@ int dwc2_hsotg_tx_fifo_total_depth(struct dwc2_hsotg *hsotg)
>>    static void dwc2_gadget_wkup_alert_handler(struct dwc2_hsotg *hsotg)
>>    {
>>    	u32 gintsts2;
>> -	u32 gintmsk2;
>>    
>>    	gintsts2 = dwc2_readl(hsotg, GINTSTS2);
>> -	gintmsk2 = dwc2_readl(hsotg, GINTMSK2);
>>    
>>    	if (gintsts2 & GINTSTS2_WKUP_ALERT_INT) {
>>    		dev_dbg(hsotg->dev, "%s: Wkup_Alert_Int\n", __func__);


Lee, 'gintsts2' can't be removed. Just, please add follow line after
reading gintmsk2:

  	gintsts2 &= gintmsk2;


Please add also "Fixes:" tag and submit v2.

Thanks,
Minas

>>

Re: patch "usb: dwc2: gadget: Do not read GINTMSK2" added to usb-testing

[Minas Harutyunyan]

Hi,

On 7/3/2020 12:19 PM, gregkh@linuxfoundation.org wrote:
> On Fri, Jul 03, 2020 at 07:43:10AM +0000, Minas Harutyunyan wrote:
>> Hi,
>>
>> On 7/3/2020 11:34 AM, gregkh@linuxfoundation.org wrote:
>>>
>>> This is a note to let you know that I've just added the patch titled
>>>
>>>       usb: dwc2: gadget: Do not read GINTMSK2
>>>
>>> to my usb git tree which can be found at
>>>       git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
>>> in the usb-testing branch.
>>>
>>> The patch will show up in the next release of the linux-next tree
>>> (usually sometime within the next 24 hours during the week.)
>>>
>>> The patch will be merged to the usb-next branch sometime soon,
>>> after it passes testing, and the merge window is open.
>>>
>>> If you have any questions about this process, please let me know.
>>>
>>
>> This patch is not correct. Please read below.
>>>
>>>   From 4f1ae9d60adb3d4f9b8ea94f02bfc883f954bed6 Mon Sep 17 00:00:00 2001
>>> From: Lee Jones <lee.jones@linaro.org>
>>> Date: Thu, 2 Jul 2020 15:46:04 +0100
>>> Subject: usb: dwc2: gadget: Do not read GINTMSK2
>>> MIME-Version: 1.0
>>> Content-Type: text/plain; charset=UTF-8
>>> Content-Transfer-Encoding: 8bit
>>>
>>> The value of GINTMSK2 hasn't been checked since the inception of
>>> dwc2_gadget_wkup_alert_handler() in 2018.
>>>
>>> Fixes the following W=1 warning:
>>>
>>>    drivers/usb/dwc2/gadget.c: In function ‘dwc2_gadget_wkup_alert_handler’:
>>>    drivers/usb/dwc2/gadget.c:259:6: warning: variable ‘gintmsk2’ set but not used [-Wunused-but-set-variable]
>>>    259 | u32 gintmsk2;
>>>    | ^~~~~~~~
>>>
>>> Cc: Minas Harutyunyan <hminas@synopsys.com>
>>> Cc: Ben Dooks <ben@simtec.co.uk>
>>> Signed-off-by: Lee Jones <lee.jones@linaro.org>
>>> Link: https://urldefense.com/v3/__https://lore.kernel.org/r/20200702144625.2533530-10-lee.jones@linaro.org__;!!A4F2R9G_pg!MlXvn0tSkTvsGZZoWD5zq_1Owq-U-DIHNaHoyU1OfG8TwGGQBEfo9g0n9r2TxvQ$
>>> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> ---
>>>    drivers/usb/dwc2/gadget.c | 2 --
>>>    1 file changed, 2 deletions(-)
>>>
>>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
>>> index 1def9000f936..ea7c9c73b4ff 100644
>>> --- a/drivers/usb/dwc2/gadget.c
>>> +++ b/drivers/usb/dwc2/gadget.c
>>> @@ -256,10 +256,8 @@ int dwc2_hsotg_tx_fifo_total_depth(struct dwc2_hsotg *hsotg)
>>>    static void dwc2_gadget_wkup_alert_handler(struct dwc2_hsotg *hsotg)
>>>    {
>>>    	u32 gintsts2;
>>> -	u32 gintmsk2;
>>>    
>>>    	gintsts2 = dwc2_readl(hsotg, GINTSTS2);
>>> -	gintmsk2 = dwc2_readl(hsotg, GINTMSK2);
>>>    
>>>    	if (gintsts2 & GINTSTS2_WKUP_ALERT_INT) {
>>>    		dev_dbg(hsotg->dev, "%s: Wkup_Alert_Int\n", __func__);
>>
>> Lee, 'gintsts2' can't be removed. Just, please add follow line after
>> reading gintmsk2:
>>
>> 	gintsts2 &= gintmsk2;
> 
> So that's a real bugfix needed?
> 

Yes, here real bugfix need, otherwise spurious interrupt 'wkup_alert' 
can be handled.

Thanks,
Minas

> Ok, I'll drop this patch from the tree now, thanks.
> 
> greg k-h
>