On Thu, Feb 20, 2014 at 2:20 PM, Bjorn Helgaas <bhelgaas@xxxxxxxxxx> wrote: > On Thu, Feb 20, 2014 at 12:42 AM, Rajat Jain <rajatxjain@xxxxxxxxx> wrote: >> Hello Bjorn, >> >>>> >>>> On Mon, Nov 25, 2013 at 07:03:11PM +0000, Rajat Jain wrote: >>>> > Hello, >>>> > >>>> > > > 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: >>>> > >>>> > OK, I'll attempt to fix it that way when I get time. >>>> > >>>> > > >>>> > > - 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. >>>> > >>>> > Ok. >>>> >>>> This patch is still in patchwork, but I've lost track of where we are. >>>> Did you resolve this in the series that I just applied, or is it still >>>> an outstanding issue? >>> >>> No, I did not solve it. It is still an outstanding issue. So far I am using your patch to overcome this: >>> >>> http://www.spinics.net/lists/hotplug/msg05830.html >>> >>> I'll just attempt to conclude the status on this issue so that you can make the decision on the course of action. IMHO there are 2 independent issues that we discussed in this thread: >>> >>> 1) PCIe compliant HW (that generates cmd completed interrupts at every write of Slot_ctrl register) being penalized with 1 second delay during the boot up. Your patch solves this. >>> >>> 2) If there is a genuine spurious interrupt, it does not get acknowledged. I had originally posted a patch for THIS problem. >>> http://www.spinics.net/lists/hotplug/msg05815.html >>> >>> You had indicated that you would rather want a bigger restructuring of the driver to solve (2). >>> >>> My observation: MY problem (in my setup) is not seen if I use either of the patches (yours or mine). >>> >>> My opinion: I think my patch solves (2) but might not solve (1) for all corner cases. Also your patch solves (1) but may not solve (2) for all corner cases -Thus we should probably solve both of these problems individually. >>> >> >> Just wondering if you decided on how to solve this problem? >> >> Are you planning this for 3.15? > > Sorry, I haven't had a chance to work on this, so I don't think *I* > will get anything done for v3.15. To make forward progress, maybe we > should merge your original patch? Would you mind posting it again so > it gets into patchwork again? Just sent it again, Thanks! -- 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
