Changeset View
Standalone View
sys/kern/subr_bus.c
Show First 20 Lines • Show All 2,077 Lines • ▼ Show 20 Lines | device_probe_child(device_t dev, device_t child) | ||||
GIANT_REQUIRED; | GIANT_REQUIRED; | ||||
dc = dev->devclass; | dc = dev->devclass; | ||||
if (!dc) | if (!dc) | ||||
panic("device_probe_child: parent device has no devclass"); | panic("device_probe_child: parent device has no devclass"); | ||||
/* | /* | ||||
* If the state is already probed, then return. However, don't | * If the state is already probed, then return. | ||||
* return if we can rebid this object. | |||||
*/ | */ | ||||
if (child->state == DS_ALIVE && (child->flags & DF_REBID) == 0) | if (child->state == DS_ALIVE) | ||||
mav: Would it better be be >= DS_ATTACHING here? Or what is the logic? | |||||
Done Inline ActionsOriginally, I think that attaching wasn't considered. How can you get here if it's attaching? imp: Originally, I think that attaching wasn't considered. How can you get here if it's attaching? | |||||
Not Done Inline ActionsAs I see, DS_ALIVE means probed. It may not have sense to reprobe again, except one case -- new driver load, in which case you do want reprobe. Though in case of detach device drops into DS_NOTPRESENT, not DS_ALIVE, so it may not be a problem. Not sure how would end up in DS_ALIVE state for a long time. mav: As I see, DS_ALIVE means probed. It may not have sense to reprobe again, except one case -- new… | |||||
Done Inline ActionsYou can get DS_ALIVE for long periods of time for devices that are disabled. We probe those devices to hold the unit, and then never attach them. imp: You can get DS_ALIVE for long periods of time for devices that are disabled. We probe those… | |||||
Done Inline ActionsHmmm, it dates from very early on, but I chased it back to the device/bus split only... Still quite old. imp: Hmmm, it dates from very early on, but I chased it back to the device/bus split only... Still… | |||||
Not Done Inline Actions
jhb: > Hmmm, it dates from very early on, but I chased it back to the device/bus split only... Still… | |||||
Not Done Inline ActionsWe do want to make sure we don't break the device disable case (via hint.foo.0.disable=1 or devctl disable) just because some unrelated driver is kldloaded. Though I think those set a DF_DISABLED flag or the like? jhb: We do want to make sure we don't break the device disable case (via hint.foo.0.disable=1 or… | |||||
return (0); | return (0); | ||||
for (; dc; dc = dc->parent) { | for (; dc; dc = dc->parent) { | ||||
for (dl = first_matching_driver(dc, child); | for (dl = first_matching_driver(dc, child); | ||||
dl; | dl; | ||||
dl = next_matching_driver(dc, child, dl)) { | dl = next_matching_driver(dc, child, dl)) { | ||||
/* If this driver's pass is too high, then ignore it. */ | /* If this driver's pass is too high, then ignore it. */ | ||||
if (dl->pass > bus_current_pass) | if (dl->pass > bus_current_pass) | ||||
Show All 26 Lines | for (dl = first_matching_driver(dc, child); | ||||
resource_int_value(dl->driver->name, child->unit, | resource_int_value(dl->driver->name, child->unit, | ||||
"flags", &child->devflags); | "flags", &child->devflags); | ||||
result = DEVICE_PROBE(child); | result = DEVICE_PROBE(child); | ||||
/* Reset flags and devclass before the next probe. */ | /* Reset flags and devclass before the next probe. */ | ||||
child->devflags = 0; | child->devflags = 0; | ||||
if (!hasclass) | if (!hasclass) | ||||
(void)device_set_devclass(child, NULL); | (void)device_set_devclass(child, NULL); | ||||
Not Done Inline ActionsI think this devclass devflags/removal could be moved after the following if (result == 0) section, so that in case of drivers with absolute priority we have not removed it just to re-set again. mav: I think this devclass devflags/removal could be moved after the following if (result == 0)… | |||||
Done Inline ActionsYou may be right.... I'd like to address that separately though since I don't think this removal changes when we can / should do this. imp: You may be right.... I'd like to address that separately though since I don't think this… | |||||
Not Done Inline ActionsYou'd have to special case the '0' case below to not re-set it though. Not sure device probe is a hot path such that it's worth the complexity to save a few cycles. Hmm, I guess the re-set below already checks child->devclass, so that one doesn't hurt to leave as-is. Moving the devflags inside the pro < 0 check as you suggest would also handle that without real added complexity. I agree that a separate commit would be better for these changes though. jhb: You'd have to special case the '0' case below to not re-set it though. Not sure device probe… | |||||
Not Done Inline ActionsI was thinking not about hot path cycles, but about the fact that device_set_devclass() regenerates unit number. I want to make device_set_unit() possible inside probe methods, where it is much more sane/correct than in attach. But sure I can do it separately. mav: I was thinking not about hot path cycles, but about the fact that device_set_devclass()… | |||||
/* | /* | ||||
* If the driver returns SUCCESS, there can be | * If the driver returns SUCCESS, there can be | ||||
* no higher match for this device. | * no higher match for this device. | ||||
*/ | */ | ||||
if (result == 0) { | if (result == 0) { | ||||
best = dl; | best = dl; | ||||
pri = 0; | pri = 0; | ||||
▲ Show 20 Lines • Show All 42 Lines • ▼ Show 20 Lines | for (; dc; dc = dc->parent) { | ||||
*/ | */ | ||||
if (best && pri == 0) | if (best && pri == 0) | ||||
break; | break; | ||||
} | } | ||||
/* | /* | ||||
* If we found a driver, change state and initialise the devclass. | * If we found a driver, change state and initialise the devclass. | ||||
*/ | */ | ||||
/* XXX What happens if we rebid and got no best? */ | |||||
if (best) { | if (best) { | ||||
/* | |||||
* If this device was attached, and we were asked to | |||||
* rescan, and it is a different driver, then we have | |||||
* to detach the old driver and reattach this new one. | |||||
* Note, we don't have to check for DF_REBID here | |||||
* because if the state is > DS_ALIVE, we know it must | |||||
* be. | |||||
* | |||||
* This assumes that all DF_REBID drivers can have | |||||
* their probe routine called at any time and that | |||||
* they are idempotent as well as completely benign in | |||||
* normal operations. | |||||
* | |||||
* We also have to make sure that the detach | |||||
* succeeded, otherwise we fail the operation (or | |||||
* maybe it should just fail silently? I'm torn). | |||||
*/ | |||||
if (child->state > DS_ALIVE && best->driver != child->driver) | |||||
if ((result = device_detach(dev)) != 0) | |||||
return (result); | |||||
/* Set the winning driver, devclass, and flags. */ | /* Set the winning driver, devclass, and flags. */ | ||||
Not Done Inline ActionsThis section is not needed also. mav: This section is not needed also. | |||||
Done Inline Actionsbecause we'd never re-bid an driver now and need to detach? imp: because we'd never re-bid an driver now and need to detach? | |||||
Not Done Inline ActionsWe should never be attached here without rebid support, so there is nothing to detach. mav: We should never be attached here without rebid support, so there is nothing to detach. | |||||
Done Inline ActionsI see why... yes, removing it. imp: I see why... yes, removing it. | |||||
if (!child->devclass) { | if (!child->devclass) { | ||||
result = device_set_devclass(child, best->driver->name); | result = device_set_devclass(child, best->driver->name); | ||||
if (result != 0) | if (result != 0) | ||||
return (result); | return (result); | ||||
} | } | ||||
result = device_set_driver(child, best->driver); | result = device_set_driver(child, best->driver); | ||||
if (result != 0) | if (result != 0) | ||||
return (result); | return (result); | ||||
resource_int_value(best->driver->name, child->unit, | resource_int_value(best->driver->name, child->unit, | ||||
"flags", &child->devflags); | "flags", &child->devflags); | ||||
Not Done Inline ActionsAnd this section should probably moved inside the following if (pri < 0) mav: And this section should probably moved inside the following if (pri < 0) | |||||
if (pri < 0) { | if (pri < 0) { | ||||
/* | /* | ||||
* A bit bogus. Call the probe method again to make | * A bit bogus. Call the probe method again to make | ||||
* sure that we have the right description. | * sure that we have the right description. | ||||
*/ | */ | ||||
DEVICE_PROBE(child); | DEVICE_PROBE(child); | ||||
#if 0 | } | ||||
child->flags |= DF_REBID; | |||||
#endif | |||||
} else | |||||
child->flags &= ~DF_REBID; | |||||
child->state = DS_ALIVE; | child->state = DS_ALIVE; | ||||
bus_data_generation_update(); | bus_data_generation_update(); | ||||
return (0); | return (0); | ||||
} | } | ||||
return (ENXIO); | return (ENXIO); | ||||
} | } | ||||
▲ Show 20 Lines • Show All 671 Lines • ▼ Show 20 Lines | |||||
*/ | */ | ||||
int | int | ||||
device_probe(device_t dev) | device_probe(device_t dev) | ||||
{ | { | ||||
int error; | int error; | ||||
GIANT_REQUIRED; | GIANT_REQUIRED; | ||||
if (dev->state >= DS_ALIVE && (dev->flags & DF_REBID) == 0) | if (dev->state >= DS_ALIVE) | ||||
return (-1); | return (-1); | ||||
if (!(dev->flags & DF_ENABLED)) { | if (!(dev->flags & DF_ENABLED)) { | ||||
if (bootverbose && device_get_name(dev) != NULL) { | if (bootverbose && device_get_name(dev) != NULL) { | ||||
device_print_prettyname(dev); | device_print_prettyname(dev); | ||||
printf("not probed (disabled)\n"); | printf("not probed (disabled)\n"); | ||||
} | } | ||||
return (-1); | return (-1); | ||||
▲ Show 20 Lines • Show All 1,207 Lines • ▼ Show 20 Lines | |||||
*/ | */ | ||||
void | void | ||||
bus_generic_driver_added(device_t dev, driver_t *driver) | bus_generic_driver_added(device_t dev, driver_t *driver) | ||||
{ | { | ||||
device_t child; | device_t child; | ||||
DEVICE_IDENTIFY(driver, dev); | DEVICE_IDENTIFY(driver, dev); | ||||
TAILQ_FOREACH(child, &dev->children, link) { | TAILQ_FOREACH(child, &dev->children, link) { | ||||
if (child->state == DS_NOTPRESENT || | if (child->state == DS_NOTPRESENT) | ||||
(child->flags & DF_REBID)) | |||||
device_probe_and_attach(child); | device_probe_and_attach(child); | ||||
} | } | ||||
} | } | ||||
/** | /** | ||||
* @brief Helper function for implementing BUS_NEW_PASS(). | * @brief Helper function for implementing BUS_NEW_PASS(). | ||||
* | * | ||||
* This implementing of BUS_NEW_PASS() first calls the identify | * This implementing of BUS_NEW_PASS() first calls the identify | ||||
▲ Show 20 Lines • Show All 1,171 Lines • ▼ Show 20 Lines | print_device_short(device_t dev, int indent) | ||||
indentprintf(("device %d: <%s> %sparent,%schildren,%s%s%s%s%s%s,%sivars,%ssoftc,busy=%d\n", | indentprintf(("device %d: <%s> %sparent,%schildren,%s%s%s%s%s%s,%sivars,%ssoftc,busy=%d\n", | ||||
dev->unit, dev->desc, | dev->unit, dev->desc, | ||||
(dev->parent? "":"no "), | (dev->parent? "":"no "), | ||||
(TAILQ_EMPTY(&dev->children)? "no ":""), | (TAILQ_EMPTY(&dev->children)? "no ":""), | ||||
(dev->flags&DF_ENABLED? "enabled,":"disabled,"), | (dev->flags&DF_ENABLED? "enabled,":"disabled,"), | ||||
(dev->flags&DF_FIXEDCLASS? "fixed,":""), | (dev->flags&DF_FIXEDCLASS? "fixed,":""), | ||||
(dev->flags&DF_WILDCARD? "wildcard,":""), | (dev->flags&DF_WILDCARD? "wildcard,":""), | ||||
(dev->flags&DF_DESCMALLOCED? "descmalloced,":""), | (dev->flags&DF_DESCMALLOCED? "descmalloced,":""), | ||||
(dev->flags&DF_REBID? "rebiddable,":""), | |||||
(dev->flags&DF_SUSPENDED? "suspended,":""), | (dev->flags&DF_SUSPENDED? "suspended,":""), | ||||
(dev->ivars? "":"no "), | (dev->ivars? "":"no "), | ||||
(dev->softc? "":"no "), | (dev->softc? "":"no "), | ||||
dev->busy)); | dev->busy)); | ||||
} | } | ||||
static void | static void | ||||
print_device(device_t dev, int indent) | print_device(device_t dev, int indent) | ||||
▲ Show 20 Lines • Show All 385 Lines • ▼ Show 20 Lines | devctl2_ioctl(struct cdev *cdev, u_long cmd, caddr_t data, int fflag, | ||||
if (error) { | if (error) { | ||||
mtx_unlock(&Giant); | mtx_unlock(&Giant); | ||||
return (error); | return (error); | ||||
} | } | ||||
/* Perform the requested operation. */ | /* Perform the requested operation. */ | ||||
switch (cmd) { | switch (cmd) { | ||||
case DEV_ATTACH: | case DEV_ATTACH: | ||||
if (device_is_attached(dev) && (dev->flags & DF_REBID) == 0) | if (device_is_attached(dev)) | ||||
error = EBUSY; | error = EBUSY; | ||||
else if (!device_is_enabled(dev)) | else if (!device_is_enabled(dev)) | ||||
error = ENXIO; | error = ENXIO; | ||||
else | else | ||||
error = device_probe_and_attach(dev); | error = device_probe_and_attach(dev); | ||||
break; | break; | ||||
case DEV_DETACH: | case DEV_DETACH: | ||||
if (!device_is_attached(dev)) { | if (!device_is_attached(dev)) { | ||||
▲ Show 20 Lines • Show All 298 Lines • Show Last 20 Lines |
Would it better be be >= DS_ATTACHING here? Or what is the logic?