On Wed, Mar 19, 2014 at 12:18 PM, Yinghai Lu <yinghai@xxxxxxxxxx> wrote:
> On Tue, Feb 25, 2014 at 9:45 AM, Bjorn Helgaas <bhelgaas@xxxxxxxxxx> wrote:
>> On Mon, Feb 24, 2014 at 6:55 PM, Yinghai Lu <yinghai@xxxxxxxxxx> wrote:
>>
>> It's quite likely that I'm mistaken, and this is not a silicon
>> problem, especially since two vendors apparently did it the same way.
>> But I'd like to see the explanation from Intel about how this complies
>> with the spec. I provided a spec reference for why I think we should
>> wait for a command completed event. The next step would be for you or
>> Intel to respond with "here's the section in the spec that you missed,
>> and it says the OS should not wait for an event unless it is changing
>> the EIC, PCC, PIC, or AIC bits."
>
> Hi, Bjorn,
>
> Intel admits that it is silicon problem.
> CC will be set iff EIC || PCC || PIC || AIC is set.
>
> so please me know if you want
> 1. just use the patch in current form. it will do that for ...
> 2. or modify the patch to check vendor to set flag for every controller, and use
> that to decide if need to wait CC.
>
> BTW, other OS does not check CC for other than the four bits.
I think the current pciehp design is a bit broken. Today we perform
commands very synchronously:
pcie_write_cmd() {
write Slot Control
if (Command Complete supported) {
wait for Command Complete
}
}
The typical driver pattern would be more asynchronous, like this:
pcie_write_cmd() {
read Slot Status
if (!Command Completed) {
wait for Command Complete
}
write Slot Control
}
With the asynchronous pattern, we would probably never wait at all
unless we performed two commands in immediate succession. I wouldn't
be surprised if doing this asynchronously would effectively cover up
these hardware differences.
But as long as we have the current synchronous design, I think we have
to do *something* so we handle both the Intel/AMD/Nvidia-style chips
and the IDT-style chips correctly. I suspect that means an explicit
quirk, or maybe some sort of test, e.g., set one of the other bits
(not EIC, PCC, PIC, AIC) and see whether we get an interrupt.
In any case, it needs to be clear why we expect CC for some bits but not others.
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
