Re: amd64/179376: xhci ehci irq storm

2013-06-13 Thread Konstantin Belousov 
On Thu, Jun 13, 2013 at 10:18:30AM -0400, John Baldwin wrote:
> On Tuesday, June 11, 2013 1:48:22 pm Konstantin Belousov wrote:
> > +   count = pci_msi_count(self);
> > +   if (count >= 1) {
> > +   if (count > 1)
> > +   count = 1;
> 
> You can just drop the if and always do the assignment here, yes?
> (Minor nit I know, and the original patch is fine for testing)

The comparision is useless, I removed it in my repo, thank you.

I remember somebody complained that xhci(4) cannot attach on the
Macs with the diagnostic about missing PRT entry for routing the
legacy interrupt.  As if Apple does not care about proper legacy
routing.  Hopefully, use of MSI-X would help there too.

Here is it:
http://lists.freebsd.org/pipermail/freebsd-current/2013-February/039595.html
http://people.freebsd.org/~dchagin/macbookpro/dmesg.generic.HEAD.txt


pgpgXj2sYflS1.pgp
Description: PGP signature

Re: amd64/179376: xhci ehci irq storm

2013-06-13 Thread John Baldwin 
On Tuesday, June 11, 2013 1:48:22 pm Konstantin Belousov wrote:
> --- a/sys/dev/usb/controller/xhci_pci.c
> +++ b/sys/dev/usb/controller/xhci_pci.c
> @@ -171,11 +170,22 @@ xhci_pci_attach(device_t self)
>  
>   usb_callout_init_mtx(&sc->sc_callout, &sc->sc_bus.bus_mtx, 0);
>  
> - rid = 0;
> - sc->sc_irq_res = bus_alloc_resource_any(self, SYS_RES_IRQ, &rid,
> - RF_SHAREABLE | RF_ACTIVE);
> + sc->sc_irq_rid = 0;
> + count = pci_msi_count(self);
> + if (count >= 1) {
> + if (count > 1)
> + count = 1;

You can just drop the if and always do the assignment here, yes?  (Minor nit I 
know, and the original patch is fine for testing)

-- 
John Baldwin
___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to "freebsd-usb-unsubscr...@freebsd.org"

Re: amd64/179376: xhci ehci irq storm

2013-06-11 Thread Konstantin Belousov 
On Tue, Jun 11, 2013 at 06:54:21PM +0200, Michal Sviba wrote:
> xhci0@pci0:0:20:0:  class=0x0c0330 card=0x3397103c chip=0x1e318086 
> rev=0x04 hdr=0x00
>  vendor = 'Intel Corporation'
>  device = 'Panther Point USB xHCI Host Controller'
>  class  = serial bus
>  subclass   = USB
>  cap 01[70] = powerspec 2  supports D0 D3  current D0
>  cap 05[80] = MSI supports 8 messages, 64 bit

Please try this.  I cannot test since I do not have any usb 3.0 controller,
not to mention controllers which support MSI.

diff --git a/sys/dev/usb/controller/xhci.h b/sys/dev/usb/controller/xhci.h
index 0872f40..3d436fd 100644
--- a/sys/dev/usb/controller/xhci.h
+++ b/sys/dev/usb/controller/xhci.h
@@ -447,6 +447,7 @@ struct xhci_softc {
 
struct usb_device   *sc_devices[XHCI_MAX_DEVICES];
struct resource *sc_io_res;
+   int sc_irq_rid;
struct resource *sc_irq_res;
 
void*sc_intr_hdl;
diff --git a/sys/dev/usb/controller/xhci_pci.c 
b/sys/dev/usb/controller/xhci_pci.c
index 8695dac..d714e0f 100644
--- a/sys/dev/usb/controller/xhci_pci.c
+++ b/sys/dev/usb/controller/xhci_pci.c
@@ -146,8 +146,7 @@ static int
 xhci_pci_attach(device_t self)
 {
struct xhci_softc *sc = device_get_softc(self);
-   int err;
-   int rid;
+   int count, err, rid;
 
/* XXX check for 64-bit capability */
 
@@ -171,11 +170,22 @@ xhci_pci_attach(device_t self)
 
usb_callout_init_mtx(&sc->sc_callout, &sc->sc_bus.bus_mtx, 0);
 
-   rid = 0;
-   sc->sc_irq_res = bus_alloc_resource_any(self, SYS_RES_IRQ, &rid,
-   RF_SHAREABLE | RF_ACTIVE);
+   sc->sc_irq_rid = 0;
+   count = pci_msi_count(self);
+   if (count >= 1) {
+   if (count > 1)
+   count = 1;
+   if (pci_alloc_msi(self, &count) == 0) {
+   if (bootverbose)
+   device_printf(self, "MSI enabled\n");
+   sc->sc_irq_rid = 1;
+   }
+   }
+   sc->sc_irq_res = bus_alloc_resource_any(self, SYS_RES_IRQ,
+   &sc->sc_irq_rid, RF_SHAREABLE | RF_ACTIVE);
if (sc->sc_irq_res == NULL) {
device_printf(self, "Could not allocate IRQ\n");
+   goto error;
}
sc->sc_bus.bdev = device_add_child(self, "usbus", -1);
if (sc->sc_bus.bdev == NULL) {
@@ -249,7 +259,10 @@ xhci_pci_detach(device_t self)
sc->sc_intr_hdl = NULL;
}
if (sc->sc_irq_res) {
-   bus_release_resource(self, SYS_RES_IRQ, 0, sc->sc_irq_res);
+   if (sc->sc_irq_rid == 1)
+   pci_release_msi(self);
+   bus_release_resource(self, SYS_RES_IRQ, sc->sc_irq_rid,
+   sc->sc_irq_res);
sc->sc_irq_res = NULL;
}
if (sc->sc_io_res) {


pgps3VF39ureS.pgp
Description: PGP signature