Changeset View
Standalone View
sys/dev/aacraid/aacraid.c
| Show First 20 Lines • Show All 218 Lines • ▼ Show 20 Lines | static struct cdevsw aacraid_cdevsw = { | ||||
| .d_open = aac_open, | .d_open = aac_open, | ||||
| .d_ioctl = aac_ioctl, | .d_ioctl = aac_ioctl, | ||||
| .d_poll = aac_poll, | .d_poll = aac_poll, | ||||
| .d_name = "aacraid", | .d_name = "aacraid", | ||||
| }; | }; | ||||
| MALLOC_DEFINE(M_AACRAIDBUF, "aacraid_buf", "Buffers for the AACRAID driver"); | MALLOC_DEFINE(M_AACRAIDBUF, "aacraid_buf", "Buffers for the AACRAID driver"); | ||||
| /* sysctl node */ | /* sysctl node */ | ||||
| SYSCTL_NODE(_hw, OID_AUTO, aacraid, CTLFLAG_RD | CTLFLAG_MPSAFE, 0, | SYSCTL_NODE(_hw, OID_AUTO, aacraid, CTLFLAG_RD | CTLFLAG_MPSAFE, 0, | ||||
| "AACRAID driver parameters"); | "AACRAID driver parameters"); | ||||
emaste: it looks like this was never used before | |||||
luporlAuthorUnsubmitted Done Inline ActionsIt's really not used. Do you think it's better to remove it, or just ignore it? luporl: It's really not used. Do you think it's better to remove it, or just ignore it? | |||||
emasteUnsubmitted Done Inline ActionsI would delete it in a separate commit, either before or after this change. emaste: I would delete it in a separate commit, either before or after this change. | |||||
luporlAuthorUnsubmitted Done Inline ActionsOk, I'll delete it after this change. luporl: Ok, I'll delete it after this change. | |||||
| static void | |||||
| aacraid_setup_sysctl(struct aac_softc *sc) | |||||
| { | |||||
| struct sysctl_ctx_list *sysctl_ctx = NULL; | |||||
| struct sysctl_oid *sysctl_tree = NULL; | |||||
| char tmpstr[80], tmpstr2[80]; | |||||
| /* | /* | ||||
| * Get existing sysctl node for this device or create a new one if it | |||||
| * doesn't exist yet. | |||||
| */ | |||||
| sysctl_ctx = device_get_sysctl_ctx(sc->aac_dev); | |||||
| if (sysctl_ctx != NULL) | |||||
| sysctl_tree = device_get_sysctl_tree(sc->aac_dev); | |||||
| if (sysctl_tree == NULL) { | |||||
| snprintf(tmpstr, sizeof(tmpstr), "aacraid controller %d", | |||||
| device_get_unit(sc->aac_dev)); | |||||
| snprintf(tmpstr2, sizeof(tmpstr2), "%d", | |||||
| device_get_unit(sc->aac_dev)); | |||||
| sysctl_ctx_init(&sc->sysctl_ctx); | |||||
| sc->sysctl_tree = SYSCTL_ADD_NODE(&sc->sysctl_ctx, | |||||
| SYSCTL_STATIC_CHILDREN(_hw_aacraid), OID_AUTO, tmpstr2, | |||||
emasteUnsubmitted Done Inline Actionsdoes it make sense to put this under dev.aacraid instead? I think the sysctl setup is simplified by doing so emaste: does it make sense to put this under dev.aacraid instead? I think the sysctl setup is… | |||||
scottlUnsubmitted Done Inline ActionsCorrect, it should be dev.aacraid. scottl: Correct, it should be dev.aacraid. | |||||
luporlAuthorUnsubmitted Done Inline ActionsOk, it's now always added to dev.aacraid and setup is skipped if for some reason sysctl_ctx or sysctl_tree is NULL. luporl: Ok, it's now always added to dev.aacraid and setup is skipped if for some reason sysctl_ctx or… | |||||
| CTLFLAG_RD | CTLFLAG_MPSAFE, 0, tmpstr); | |||||
| if (sc->sysctl_tree == NULL) | |||||
| return; | |||||
| sysctl_ctx = &sc->sysctl_ctx; | |||||
| sysctl_tree = sc->sysctl_tree; | |||||
| } | |||||
| SYSCTL_ADD_ULONG(sysctl_ctx, SYSCTL_CHILDREN(sysctl_tree), | |||||
| OID_AUTO, "dma_load_fail", CTLFLAG_RD, | |||||
| &sc->dma_load_fail, "DMA load buffer failures"); | |||||
| } | |||||
| /* | |||||
| * Device Interface | * Device Interface | ||||
| */ | */ | ||||
| /* | /* | ||||
| * Initialize the controller and softc | * Initialize the controller and softc | ||||
| */ | */ | ||||
| int | int | ||||
| aacraid_attach(struct aac_softc *sc) | aacraid_attach(struct aac_softc *sc) | ||||
| ▲ Show 20 Lines • Show All 94 Lines • ▼ Show 20 Lines | aacraid_attach(struct aac_softc *sc) | ||||
| aac_container_bus(sc); | aac_container_bus(sc); | ||||
| /* Register with CAM for the non-DASD devices */ | /* Register with CAM for the non-DASD devices */ | ||||
| if ((sc->flags & AAC_FLAGS_ENABLE_CAM) != 0) | if ((sc->flags & AAC_FLAGS_ENABLE_CAM) != 0) | ||||
| aac_get_bus_info(sc); | aac_get_bus_info(sc); | ||||
| /* poke the bus to actually attach the child devices */ | /* poke the bus to actually attach the child devices */ | ||||
| bus_generic_attach(sc->aac_dev); | bus_generic_attach(sc->aac_dev); | ||||
| aacraid_setup_sysctl(sc); | |||||
| /* mark the controller up */ | /* mark the controller up */ | ||||
| sc->aac_state &= ~AAC_STATE_SUSPEND; | sc->aac_state &= ~AAC_STATE_SUSPEND; | ||||
| /* enable interrupts now */ | /* enable interrupts now */ | ||||
| AAC_ACCESS_DEVREG(sc, AAC_ENABLE_INTERRUPT); | AAC_ACCESS_DEVREG(sc, AAC_ENABLE_INTERRUPT); | ||||
| mtx_lock(&sc->aac_io_lock); | mtx_lock(&sc->aac_io_lock); | ||||
| callout_reset(&sc->aac_daemontime, 60 * hz, aac_daemon, sc); | callout_reset(&sc->aac_daemontime, 60 * hz, aac_daemon, sc); | ||||
| ▲ Show 20 Lines • Show All 942 Lines • ▼ Show 20 Lines | aacraid_map_command_sg(void *arg, bus_dma_segment_t *segs, int nseg, int error) | ||||
| struct aac_fib *fib; | struct aac_fib *fib; | ||||
| int i; | int i; | ||||
| cm = (struct aac_command *)arg; | cm = (struct aac_command *)arg; | ||||
| sc = cm->cm_sc; | sc = cm->cm_sc; | ||||
| fib = cm->cm_fib; | fib = cm->cm_fib; | ||||
| fwprintf(sc, HBA_FLAGS_DBG_FUNCTION_ENTRY_B, "nseg %d", nseg); | fwprintf(sc, HBA_FLAGS_DBG_FUNCTION_ENTRY_B, "nseg %d", nseg); | ||||
| mtx_assert(&sc->aac_io_lock, MA_OWNED); | mtx_assert(&sc->aac_io_lock, MA_OWNED); | ||||
| if (error != 0) { | |||||
| if (sc->dma_load_fail == 0) | |||||
Done Inline ActionsDoes this need a rate limiter? imp: Does this need a rate limiter? | |||||
Done Inline ActionsYes, it could generate a lot of messages. luporl: Yes, it could generate a lot of messages.
Limiting it to 1 message in each 10 seconds (mostly… | |||||
| device_printf(sc->aac_dev, | |||||
| "ERROR: failed to load buffer to DMA area, error %d\n", | |||||
| error); | |||||
| sc->dma_load_fail++; | |||||
| /* | |||||
| * Let the command run (and fail) to be able to clean up and | |||||
| * wake up waiters in the interrupt handler. | |||||
| */ | |||||
Not Done Inline ActionsIsn't it possible that a bogus command will trigger controller firmware bugs and command timeouts? IMHO it is better to abort the command at this stage, though I'm not sure yet how to do that in this driver. markj: Isn't it possible that a bogus command will trigger controller firmware bugs and command… | |||||
Done Inline ActionsYes, I think it's possible. I agree that it would be better to abort the command here. That was my plan, until I realized this wasn't a simple task. I guess we would need to replicate part of what is being done in the interrupt handler, to clean up and wake up who is waiting on this command to complete. But I'm afraid I won't have cycles available to implement proper command abort. In any case, we already allow bogus commands with current code and this change at least warns about DMA errors. luporl: Yes, I think it's possible. I agree that it would be better to abort the command here. That was… | |||||
| } | |||||
| if ((sc->flags & AAC_FLAGS_SYNC_MODE) && sc->aac_sync_cm) | if ((sc->flags & AAC_FLAGS_SYNC_MODE) && sc->aac_sync_cm) | ||||
| return; | return; | ||||
| /* copy into the FIB */ | /* copy into the FIB */ | ||||
| if (cm->cm_sgtable != NULL) { | if (cm->cm_sgtable != NULL) { | ||||
| if (fib->Header.Command == RawIo2) { | if (fib->Header.Command == RawIo2) { | ||||
| struct aac_raw_io2 *raw; | struct aac_raw_io2 *raw; | ||||
| ▲ Show 20 Lines • Show All 2,621 Lines • Show Last 20 Lines | |||||
it looks like this was never used before