Page MenuHomeFreeBSD

bus: Cleanup device_probe_child()

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



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

rG FreeBSD src repository
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; 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...


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?


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.


We should add a comment here that says we try to preserve the devclass of an object that the bus has set maybe?


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 added inline comments.

Unindenting all of this with the early failure is a nice cleanup.


Why did you move this earlier? I guess it matches the main probe loop above? Either order is probably fine though.


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.


Yes, just to match the above code. Also the previous code missed error handling, so would need to be touched any way.

This revision was automatically updated to reflect the committed changes.

I still think this should be >= DS_ALIVE... But I don't know if it is possible to get here for >.


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.