Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH] usb: dwc2: hcd: call dwc2_is_controller_alive under
 spinlock
To:     Minas Harutyunyan <minas.harutyunyan@synopsys.com>,
        "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>
Cc:     "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "m.szyprowski@samsung.com" <m.szyprowski@samsung.com>
From:   Jaehoon Chung <jh80.chung@samsung.com>
Message-ID: <4f1362aa-f544-4857-b16f-2d0202277ddd@samsung.com>
Date:   Fri, 15 Feb 2019 14:51:43 +0900
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
        Thunderbird/60.4.0
MIME-Version: 1.0
In-Reply-To: <410670D7E743164D87FA6160E7907A56013A7DC6C8@am04wembxa.internal.synopsys.com>
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 101P
DLP-Filter: Pass
References: <CGME20190214100344epcas1p316a40d88f3b52a10745c95cc3d16a0ed@epcas1p3.samsung.com>
        <1550138622-10761-1-git-send-email-jh80.chung@samsung.com>
        <410670D7E743164D87FA6160E7907A56013A7DC6C8@am04wembxa.internal.synopsys.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Minas,

On 2/14/19 8:43 PM, Minas Harutyunyan wrote:
> Hi Jaehoon Chung,
> 
> On 2/14/2019 2:04 PM, Jaehoon Chung wrote:
>> This patch is referred to Robert's patch
>> commit cf54772b913b ("usb: dwc2: call dwc2_is_controller_alive() under spinlock")
>>
>> During running sdb with otg mode, the usb is hung sometime.
>>
>> The one of SDB hang issues should be fixed with this patch.
>> After hang, it doesn't never trigger the usb interrupt.
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> ---
>>   drivers/usb/dwc2/hcd_intr.c | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/usb/dwc2/hcd_intr.c b/drivers/usb/dwc2/hcd_intr.c
>> index 88b5dcf..8d3b155 100644
>> --- a/drivers/usb/dwc2/hcd_intr.c
>> +++ b/drivers/usb/dwc2/hcd_intr.c
>> @@ -2222,13 +2222,13 @@ irqreturn_t dwc2_handle_hcd_intr(struct dwc2_hsotg *hsotg)
>>   	u32 gintsts, dbg_gintsts;
>>   	irqreturn_t retval = IRQ_NONE;
>>   
>> +	spin_lock(&hsotg->lock);
>> +
>>   	if (!dwc2_is_controller_alive(hsotg)) {
>>   		dev_warn(hsotg->dev, "Controller is dead\n");
>> -		return retval;
>> +		goto out;
>>   	}
>>   
>> -	spin_lock(&hsotg->lock);
>> -
>>   	/* Check if HOST Mode */
>>   	if (dwc2_is_host_mode(hsotg)) {
>>   		gintsts = dwc2_read_core_intr(hsotg);
>> @@ -2276,6 +2276,7 @@ irqreturn_t dwc2_handle_hcd_intr(struct dwc2_hsotg *hsotg)
>>   		}
>>   	}
>>   
>> +out:
>>   	spin_unlock(&hsotg->lock);
>>   
>>   	return retval;
>>
> 
> 1. Checking core alive or not was introduced in our internal reference 
> driver for our setups, because sometime AHB-PCI bridge can hung, not 
> core itself. If it happen then only way to overcome it - reboot core 
> setup and PC.
> 2. Actually this issue was fixed on our setups and currently no need to 
> check access via bridge to core.
> 3. Any case if it still happen on some platforms then first need to 
> check and fix HW

I didn't check with latest kernel. we're using v4.4.172.
This issue is occurred on only ARTIK boards.
- During running test about 10~15 hours, suddenly hang.
- It's impossible to fix HW side.

After applied this patch, passed the test case as 100%. (Before applied, it didn't pass the test.)

> 
> On asserted any interrupt for dwc2, handlers call sequence is follow: 
> common->gadget->host handlers. If core alive checked in common handler 
> then checking same stuff in host handler again not needed at all.

Right, it's the duplicated checking according to sequence.

> 
> So, core alive checking in host handler create issue for your test, I 
> suggest to not fix alive checking in host interrupt handler as you 
> suggested and done in common handler, but just remove it.

Will remove it.

Best Regards,
Jaehoon Chung

> 
> Thanks,
> Minas
> 
> 
> 
> 
>