Re: [PATCH] pciehp: Acknowledge the spurious "cmd completed" event.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Nov 23, 2013 at 7:56 AM, Rajat Jain <rajatjain@xxxxxxxxxxx> wrote:
> Hello Bjorn,
>
>> >
>> > https://bugzilla.kernel.org/show_bug.cgi?id=64821
>> >
>> > Please let me know if you need any help in trying something out - I'd
>> > be more than keen to help - write code or test.
>>
>> What do you think of the patch below?  I'm afraid we'll trip over a few
>> other old parts similar to the 82801H, but I'd rather do that than
>> penalize the parts that work correctly.
>
> Tested and works fine. Looks good (on a side note, we are introducing device specific check in a common code, but I do not think there is any other cleaner way). I think this patch should be applied as it solves the problem of slow probing.

Thanks for testing.  I don't like the device-specific code either, but
it does seem like a device-specific defect, and I didn't have any
better ideas.

> On a different note, I feel there is still a need to apply my original patch. There is still an open problem in case of spurious interrupts (or in any case where the condition "if (slot_status & PCI_EXP_SLTSTA_CC)" becomes true in pcie_write_cmd()). That is because once that happens, we never clear that interrupt, and no further hotplug interrupts shall be received unless we do that.

I agree this is an issue and we should address it somehow.  My
hesitation is just that I'd prefer to do some more aggressive
restructuring rather than apply a point fix.  For example:

- We currently look at PCI_EXP_SLTSTA_CC in pcie_isr(),
pcie_poll_cmd(), and pcie_write_cmd().  I think it would be better to
look at it only in pcie_isr().

- I don't think pcie_poll_cmd() should exist at all; we should poll by
calling pcie_isr() instead.

- We need pcie_write_cmd(), but I think the way it waits is backwards.
 Currently we issue the command, then wait for it to complete.  I
think we should issue the command, note the current time, and return
without waiting.  The *next* time we need to issue a command, we can
wait for completion of the previous one (or timeout) if necessary.

But maybe we need the point fix in the interim, especially if anybody
can actually produce the scenario you mention.

Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel]     [Linux DVB]     [Asterisk Internet PBX]     [DCCP]     [Netdev]     [X.org]     [Util Linux NG]     [Fedora Women]     [ALSA Devel]     [Linux USB]

  Powered by Linux