On Sat, Mar 12, 2016 at 11:35 AM, Andy Lutomirski <luto@xxxxxxxxxxxxxx> wrote:
> On Wed, Mar 9, 2016 at 5:34 PM, Mario Limonciello
> <mario_limonciello@xxxxxxxx> wrote:
>>
>>
>> On 03/07/2016 02:42 PM, Andy Lutomirski wrote:
>>> On Mon, Mar 7, 2016 at 12:13 PM, Greg Kroah-Hartman
>>> <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>>> I'm adding a couple Dell people.
>>>
>>> Hi, Dell people-
>>>
>>> Your latest DSDT has this blatantly buggy method:
>>>
>>> Method (_Q66, 0, NotSerialized) // _Qxx: EC Query
>>> {
>>> Acquire (PATM, 0x0064)
>>> If ((ECRD != One))
>>> {
>>> Return (Zero)
>>> }
>>>
>>> NEVT ()
>>> Release (PATM)
>>> Return (Zero)
>>> }
>>>
>>> At some point during boot (presumably), this runs with ECRD == 0,
>>> causing PATM to be acquired and never released. Later on, something
>>> involved in USB-C hotplug (I think -- I can occasionally trigger
>>> errors during hotplug) breaks when it can't acquire PATM.
>>>
>>> Could you ask your BIOS team to please add the obviously missing Release (PATM)?
>>>
>>> I don't know if fixing this bug will solve all the USB-C issues, but
>>> it seems unlikely to hurt. I'm going to try to get it to work with a
>>> custom method, but I may or may not succeed.
>>>
>>> --Andy
>>>
>>>
>> Andy,
>>
>> Sorry I haven't gotten back. I was hoping to have an answer when I
>> responded but not yet so just wanted to let you know the message wasn't
>> missed.
>> I'm inquiring to my team about this. I'm unsure the circumstances that
>> the EC isn't ready (race condition?) so it might take some time to get
>> answer.
>
> It looks like ECRD is managed entirely in AML like this:
>
> Method (_REG, 2, NotSerialized) // _REG: Region Availability
> {
> If (((Arg1 == One) == (Arg0 == 0x03)))
> {
> ECRD = One
> ECIN ()
> }
>
> If (((Arg1 == Zero) && (Arg0 == 0x03)))
> {
> ECRD = Zero
> }
> }
>
> That is also buggy. (((Arg1 == One) == (Arg0 == 0x03))) should be
> (((Arg1 == One) && (Arg0 == 0x03))). But that bug goes in the other
> direction, so I don't think it explains how this gets triggered.
>
> I've tried to instrument the code, but I haven't found it yet.
Got it. I was barking up the wrong tree.
Q: What happens if _Q66 runs concurrently with itself:
A:
Method (_Q66, 0, NotSerialized) // _Qxx: EC Query
{
Acquire (PATM, 0x0064)
If ((ECRD != One))
{
Return (Zero)
}
NEVT ()
Release (PATM)
Return (Zero)
}
The first one acquires PATM. The second one fails to acquire PATM due
to the timeout, does something potentially harmful when it reenters
NEVT (not sure -- maybe it's fine), then blows up when it tries to
release PATM, which it doesn't hold.
--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
