Re: [PATCH] usb: dwc3: gadget: Avoid race between dwc3 interrupt handler and irq thread handler

From: Felipe Balbi <balbi@kernel.org>
To: Baolin Wang <baolin.wang@linaro.org>
Cc: Janusz Dziedzic <janusz.dziedzic@gmail.com>,
	Greg KH <gregkh@linuxfoundation.org>,
	USB <linux-usb@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Linaro Kernel Mailman List <linaro-kernel@lists.linaro.org>,
	Mark Brown <broonie@kernel.org>
Subject: Re: [PATCH] usb: dwc3: gadget: Avoid race between dwc3 interrupt handler and irq thread handler
Date: Thu, 05 Jan 2017 13:19:39 +0200	[thread overview]
Message-ID: <8760ltwy7o.fsf@linux.intel.com> (raw)
In-Reply-To: <CAMz4kuKn9sfUH2Oe9JPiZDktgRrKcS7cFyU2zpb3k8JYYZwjSA@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2731 bytes --]

Hi,

Baolin Wang <baolin.wang@linaro.org> writes:

[...]

>>>> and you have triggered this with mailine? How? We don't write to GEVNT*
>>>> registers from PM code and we only allow runtime_suspend with cable
>>>> dettached.
>>>
>>> Sorry for late reply since I am busy on other things. I just agreed
>>> with the possible races pointed by Janusz. I need to look at if these
>>> are happened on my platform and also I found some out of tree code
>>> which will clean the GEVNTCOUNT register when stop the gadget. I will
>>> check the mainline kernel and resend new patch if I make this problem
>>> clearly. Anyway thanks for your help and suggestion.
>>
>> IOW, you sent me a patch to be integrated in the tree which everybody in
>> the whole world uses and you didn't even test anything in that very
>> tree? How am I supposed to trust you're sending me tested patches from
>> now on?
>>
>> Clearly you have no empathy for those working countless hours to keep
>> this stable and working. If you're ready to send me a completely
>> untested patch and claim that it's fixing a race condition you have
>> never seen for yourself, it becomes difficult to trust any patches
>> you're sending me.
>
> I am sure I send you every patch was tested on my vendor platform and
> I saw the problem on my platform. But like my said I missed something
> that we have masked all interrupts in the dwc3 interrupt handler, so
> the real reason maybe caused by some out of tree code on my vendor
> platform which will clean the GEVNTCOUNT register when stop the
> gadget. Moreover I did not only do this one thing, and some other

and this is the very problem I'm referring to. If you have changes on
DWC3 on your "vendor tree" you're testing *mainline* DWC3. Which kernel
is your tree even based on? 

> problem I also need time to test and investigate. So I think I need
> some time to make things clear, then I can send you one better patch
> with the correct explanation, am I wrong here?

you're wrong to assume your vendor tree *with changes on DWC3 driver* is
equivalent to testing *mainline*. That just doesn't add up.

If you were adding just platform init code (something under your mach-*
directory, some DTS, etc) that's fine. But you have changes on the USB
peripheral controller driver. This makes me rather uneasy about your
patches. I mean, if you have changes to DWC3, what other changes do you
have there? Also, if your changes are in PM code, which we have support
in upstream, this suggests that you're using older kernel from the time
when we didn't have PM support upstream. This means you're using
something pre-v4.8. Which kernel are you using?

cheers

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]