bus: Cleanup device_probe_child()
ClosedPublic
Actions

Authored by mav on Sep 24 2021, 7:34 PM.

Details

Reviewers

imp
jhb

Commits

rGd97bfe3ff840: bus: Cleanup device_probe_child()
rGf73c2bbf811b: bus: Cleanup device_probe_child()

Summary

When device driver probe method returns 0, i.e. absolute priority, do not remove its class from the device just to set it back few lines later, that may change the device unit number, etc. and after which we'd better call the probe again.

If during search we found some driver with absolute priority, we do not need to set device driver and class since we haven't removed them before.

It should not happen, but if second probe method call failed, remove the driver and possibly the class from the device as it was when we started.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

mav requested review of this revision.Sep 24 2021, 7:34 PM

mav created this revision.

This looks good... I have one question though...

sys/kern/subr_bus.c
2216	Why is the hasclass test needed here? If we've probed the device and this second probe fails, shouldn't we always set devclass to NULL?

mav added inline comments.Sep 24 2021, 8:31 PM

sys/kern/subr_bus.c
2216	There can be other drivers of the same class. And if we haven't set the class for the device (it can be set by the bus) we should not remove it.

OK I'm happy. A suggested place to add a comment, but if you don't want to, I can do it as a followup.

sys/kern/subr_bus.c
2078	We should add a comment here that says we try to preserve the devclass of an object that the bus has set maybe?
2216	Makes sense... I'd forgotten that was why this was here...

This revision is now accepted and ready to land.Sep 24 2021, 8:46 PM

jhb accepted this revision.Sep 25 2021, 12:17 AM

jhb added inline comments.

sys/kern/subr_bus.c
2188	Unindenting all of this with the early failure is a nice cleanup.
2196	Why did you move this earlier? I guess it matches the main probe loop above? Either order is probably fine though.
2216	This is what lets hinted devices work for example. I think that's the main case of pre-set device classes. devctl set driver is another instance.

mav added inline comments.Sep 25 2021, 12:32 AM

sys/kern/subr_bus.c
2196	Yes, just to match the above code. Also the previous code missed error handling, so would need to be touched any way.

Closed by commit rGf73c2bbf811b: bus: Cleanup device_probe_child() (authored by mav). · Explain WhySep 25 2021, 12:36 AM

This revision was automatically updated to reflect the committed changes.

mav added a commit: rGf73c2bbf811b: bus: Cleanup device_probe_child().

imp added inline comments.Sep 25 2021, 12:37 AM

sys/kern/subr_bus.c
2089	I still think this should be >= DS_ALIVE... But I don't know if it is possible to get here for >.
2216	Yea, hinted devices are set by the bus and probe just approves / rejects the device. Disabled devices will also be similar, but will be in DS_ALIVE.

mav added a commit: rGd97bfe3ff840: bus: Cleanup device_probe_child().Jan 4 2022, 5:18 PM

Revision Contents
Changeset List

Path

Size

sys/

kern/

subr_bus.c

50 lines

Diff 95687

View Options

bus: Cleanup device_probe_child()ClosedPublicActions