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

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

 



Hello Bjorn,

> -----Original Message-----
> From: linux-hotplug-owner@xxxxxxxxxxxxxxx [mailto:linux-hotplug-
> owner@xxxxxxxxxxxxxxx] On Behalf Of Bjorn Helgaas
> Sent: Tuesday, February 11, 2014 4:35 PM
> To: Rajat Jain
> Cc: Rajat Jain; linux-pci@xxxxxxxxxxxxxxx; linux hotplug mailing; Kenji
> Kaneshige; Yijing Wang; Greg KH; Tom Nguyen; Kristen Accardi; Rajat
> Jain; Guenter Roeck
> Subject: Re: [PATCH] pciehp: Acknowledge the spurious "cmd completed"
> event.
> 
> 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.

Thanks,

Rajat



 


> 
> 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
> 

��.n��������+%������w��{.n�����{���"�)��jg��������ݢj����G�������j:+v���w�m������w�������h�����٥





[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux