On Tuesday 26 December 2006 02:29, Pyun YongHyeon wrote: > On Mon, Dec 25, 2006 at 11:30:45PM -0500, Scott Long wrote: > > Pyun YongHyeon wrote: > > >On Sat, Dec 23, 2006 at 09:54:58PM +0000, Bruce M. Simpson wrote: > > > > Hi, > > > > > > > > It looks like MSI was detected, but not used by the msk(4) driver on > > > the > Asus Vintage AH-1. > > > > > > > > This is a uniprocessor Athlon64 system. The PCI bridges on this system > > > > aren't in the MSI blacklist, however, there are several odd messages > > > > regarding a non-default MSI window. Looking at the code suggests it > > > > expects to see the MSI window at 0xfee00000. > > > > > > > > BTW: This system's on-board SATA controller stopped working with > > > 6.2-RC, > so I'm using an add-on PCI-e card for SATA to connect the root > > > disk. > > > > > > > > pcib0: <ACPI Host-PCI bridge> port 0xcf8-0xcff on acpi0 > > > > pci0: <ACPI PCI bus> on pcib0 > > > > pci0: physical bus=0 > > > > > >[...] > > > > > > > pcib2: <ACPI PCI-PCI bridge> at device 6.0 on pci0 > > > > pcib2: secondary bus 2 > > > > pcib2: subordinate bus 2 > > > > pcib2: I/O decode 0xc000-0xcfff > > > > pcib2: memory decode 0xfe400000-0xfe4fffff > > > > pcib2: no prefetched decode > > > > pci2: <ACPI PCI bus> on pcib2 > > > > pci2: physical bus=2 > > > > found-> vendor=0x11ab, dev=0x4362, revid=0x19 > > > > bus=2, slot=0, func=0 > > > > class=02-00-00, hdrtype=0x00, mfdev=0 > > > > cmdreg=0x0107, statreg=0x4010, cachelnsz=16 (dwords) > > > > lattimer=0x00 (0 ns), mingnt=0x00 (0 ns), maxlat=0x00 (0 ns) > > > > intpin=a, irq=5 > > > > powerspec 2 supports D0 D1 D2 D3 current D0 > > > > VPD Ident: Marvell Yukon 88E8053 Gigabit Ethernet Controller > > > > PN: Yukon 88E8053 > > > > EC: Rev. 1.9 > > > > MN: Marvell > > > > SN: AbCdEfG32a88a > > > > CP: id 1, BAR16, off 0x3cc > > > > RV: 0x24 > > > > MSI supports 2 messages, 64 bit > > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > >I think Yukon II supports one MSI message. But all systems I know > > >reported that it supports two MSI messages. This is main reason why > > >msk(4) doesn't use MSI ATM. I don't know why Yukon II claims to > > >support two MSI messages.(for dual port MAC configuraiton?) > > > > My guess is that it advertises 2 messages for the dual-port > > configuration. The intention is probably that the driver would > > run a loop where it allocates a message as it configures each port. > > So if you only have a single port, then you only allocate a single > > message and ignore the other one. This looks like it will be hard to > > fit into the msk+mskc code that is there right now. > > > > > > > >You can force to use MSI by assigning 'msic = 1' before calling > > >pci_alloc_msi(9) in mskc_attach(). However it wouldn't work if you > > >reload the msk(4) again. Other than that it works well with MSI. > > > > I don't understand why there would be a failure here. Can you explain? > > > > Me too. > > I don't remember what message it was but it's somthing like > a message with two irq numbers for the device on second loading > and then it failed to allocate interrupt handler. > > mskc0: <Marvell Yukon 88E8053 Gigabit Ethernet> port 0xXXXX-0xYYYY mem > 0xXXXXXXXX-0xYYYYYYYY irq 19, 256 at device X.Y on pciX > ^^^^^^^^^^^ You need to do the pci_release_msi() after releasing the SYS_RES_IRQ resource. Also, to support MSI-X parts, you need to alloc your SYS_RES_MEMORY resource before you call pci_alloc_msi(). The patch below fixes much of this, but disables MSI for the dual port cards for now since I didn't want to untangle all the interrupt code to create separate MSI handlers for each port. You also have a bug in your task where it seems that if you ifconfig the first port on a dual port card down, you won't ever handle interrupts for the second port: if (sc_if0 != NULL) { ifp0 = sc_if0->msk_ifp; if ((ifp0->if_drv_flags & IFF_DRV_RUNNING) == 0) goto done; } if (sc_if1 != NULL) { ifp1 = sc_if1->msk_ifp; if ((ifp1->if_drv_flags & IFF_DRV_RUNNING) == 0) goto done; } Since RUNNING would be clear for ifp0, you would goto done and not handle any of the events for ifp1. Anyways, here is the patch I came up with so far: Index: if_msk.c =================================================================== RCS file: /host/cvs/usr/cvs/src/sys/dev/msk/if_msk.c,v retrieving revision 1.1 diff -u -r1.1 if_msk.c --- if_msk.c 13 Dec 2006 02:30:11 -0000 1.1 +++ if_msk.c 28 Dec 2006 20:45:57 -0000 _at__at_ -339,16 +339,25 _at__at_ static struct resource_spec msk_res_spec_io[] = { { SYS_RES_IOPORT, PCIR_BAR(1), RF_ACTIVE }, - { SYS_RES_IRQ, 0, RF_ACTIVE | RF_SHAREABLE }, { -1, 0, 0 } }; static struct resource_spec msk_res_spec_mem[] = { { SYS_RES_MEMORY, PCIR_BAR(0), RF_ACTIVE }, + { -1, 0, 0 } +}; + +static struct resource_spec msk_irq_spec_legacy[] = { { SYS_RES_IRQ, 0, RF_ACTIVE | RF_SHAREABLE }, { -1, 0, 0 } }; +static struct resource_spec msk_irq_spec_msi[] = { + { SYS_RES_IRQ, 1, RF_ACTIVE }, + { SYS_RES_IRQ, 2, RF_ACTIVE }, + { -1, 0, 0 } +}; + static int msk_miibus_readreg(device_t dev, int phy, int reg) { _at__at_ -1546,25 +1555,7 _at__at_ */ pci_enable_busmaster(dev); - /* Allocate resources */ - sc->msk_msi = 0; - msic = pci_msi_count(dev); - if (bootverbose) - device_printf(dev, "MSI count : %d\n", msic); - /* - * Due to a unknown reason Yukon II reports it can handle two - * messages even if it can handle just one message. Forcing - * to allocate 1 message seems to work but reloading kernel - * module after unloading the driver fails. Only use MSI when - * it reports 1 message until we have better understanding - * for the hardware. - */ - if (msic == 1 && msi_disable == 0 && pci_alloc_msi(dev, &msic) == 0) { - sc->msk_msi = 1; - /* Set rid to 1 for SYS_RES_IRQ to use MSI. */ - msk_res_spec_io[1].rid = 1; - msk_res_spec_mem[1].rid = 1; - } + /* Allocate I/O resource */ #ifdef MSK_USEIOSPACE sc->msk_res_spec = msk_res_spec_io; #else _at__at_ -1710,6 +1701,36 _at__at_ sc->msk_hw_feature = 0; } + /* Allocate IRQ resources. */ + msic = pci_msi_count(dev); + if (bootverbose) + device_printf(dev, "MSI count : %d\n", msic); + /* + * The Yukon II reports it can handle two messages, one for each + * possible port. We go ahead and allocate two messages and only + * setup a handler for both if we have a dual port card. + * + * XXX: I haven't untangled the interrupt handler to handle dual + * port cards with separate MSI messages, so for now I disable MSI + * on dual port cards. + */ + if (msic == 2 && msi_disable == 0 && sc->msk_num_port == 1 && + pci_alloc_msi(dev, &msic) == 0) { + if (msic == 2) { + sc->msk_msi = 1; + sc->msk_irq_spec = msk_irq_spec_msi; + } else { + pci_release_msi(dev); + sc->msk_irq_spec = msk_irq_spec_legacy; + } + } + + error = bus_alloc_resources(dev, sc->msk_irq_spec, sc->msk_irq); + if (error) { + device_printf(dev, "couldn't allocate IRQ resources\n"); + goto fail; + } + if ((error = msk_status_dma_alloc(sc)) != 0) goto fail; _at__at_ -1770,8 +1791,8 _at__at_ taskqueue_start_threads(&sc->msk_tq, 1, PI_NET, "%s taskq", device_get_nameunit(sc->msk_dev)); /* Hook interrupt last to avoid having to lock softc. */ - error = bus_setup_intr(dev, sc->msk_res[1], INTR_TYPE_NET | - INTR_MPSAFE | INTR_FAST, msk_intr, sc, &sc->msk_intrhand); + error = bus_setup_intr(dev, sc->msk_irq[0], INTR_TYPE_NET | + INTR_MPSAFE | INTR_FAST, msk_intr, sc, &sc->msk_intrhand[0]); if (error != 0) { device_printf(dev, "couldn't set up interrupt handler\n"); _at__at_ -1884,10 +1905,15 _at__at_ taskqueue_free(sc->msk_tq); sc->msk_tq = NULL; } - if (sc->msk_intrhand) { - bus_teardown_intr(dev, sc->msk_res[1], sc->msk_intrhand); - sc->msk_intrhand = NULL; + if (sc->msk_intrhand[0]) { + bus_teardown_intr(dev, sc->msk_irq[0], sc->msk_intrhand[0]); + sc->msk_intrhand[0] = NULL; + } + if (sc->msk_intrhand[1]) { + bus_teardown_intr(dev, sc->msk_irq[0], sc->msk_intrhand[0]); + sc->msk_intrhand[1] = NULL; } + bus_release_resources(dev, sc->msk_irq_spec, sc->msk_irq); if (sc->msk_msi) pci_release_msi(dev); bus_release_resources(dev, sc->msk_res_spec, sc->msk_res); Index: if_mskreg.h =================================================================== RCS file: /host/cvs/usr/cvs/src/sys/dev/msk/if_mskreg.h,v retrieving revision 1.1 diff -u -r1.1 if_mskreg.h --- if_mskreg.h 13 Dec 2006 02:30:11 -0000 1.1 +++ if_mskreg.h 28 Dec 2006 20:45:37 -0000 _at__at_ -2314,9 +2314,11 _at__at_ /* Softc for the Marvell Yukon II controller. */ struct msk_softc { - struct resource *msk_res[2]; /* I/O and IRQ resources */ + struct resource *msk_res[1]; /* I/O resource */ struct resource_spec *msk_res_spec; - void *msk_intrhand; /* irq handler handle */ + struct resource *msk_irq[2]; /* IRQ resources */ + struct resource_spec *msk_irq_spec; + void *msk_intrhand[2]; /* irq handler handle */ device_t msk_dev; uint8_t msk_hw_id; uint8_t msk_hw_rev; -- John BaldwinReceived on Thu Dec 28 2006 - 20:22:18 UTC
This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:39:04 UTC