Page MenuHomeFreeBSD

LinuxKPI: add device_reprobe() and device_release_driver()
ClosedPublic

Authored by bz on May 28 2021, 11:22 AM.

Details

Summary

Add two new (though untested) functions to linux/device.h which are
dealing with manually managing the device/driver and are used by
at least one wireless driver.

Sponsored by: The FreeBSD Foundation
MFC after: 10 days

Please note the "untested part". I am not entirely sure this will work
like this if called from within the driver itself. I kept wondering if
a task calling the devctl2 ioctl would make sense instead on FreeBSD or
if we should try to fully manage this through the Linux device compat
code. This gets used in case the NIC needs a "restart" or is gone
after a "HotPlug" event; so think of it as "damage control". Hence
my comment about "untested".
I could also leave the implementation "blank" and simply return an error
in case of reprobe if we think that might be better for the LinuxKPI
code?

Test Plan

?

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

bz requested review of this revision.May 28 2021, 11:22 AM
sys/compat/linuxkpi/common/include/linux/device.h
505

What's Linux say this should do?

510

This takes the device's parent bus to rescan... If you call it on a normal device nothing will happen.

519

what's linux say this should do?

524

not really error here. Why not call it directly:

if (devicce_is_attached()) {
    error = device_detach();
    if (error)
       goto unlock;
}

makes more sense to me.

530

Shouldn't devclass already be NULL here?
device_detach always sets it to NULL in the non-error case.
You can rmeove it.

531

device_detach() leaves the device detached and unprobed/bound

532

So this should be all you need...

sys/compat/linuxkpi/common/include/linux/device.h
505

I dunno. I don't read generic Linux code.

Random doc on the net say https://docs.huihoo.com/linux/kernel/2.6.26/kernel-api/re636.html

510

Hmm.. so this is a NOP; based on the text this also needs a if (device_is_attached()) { block.

530

Hmm does this mean devctl2:DEV_CLEAR_DRIVER can be improved as well?

532

And I am not sure anymore if I do need this.

The other real problem is that we'd call this in the driver context which is releasing itself. Would that ever work?

sys/compat/linuxkpi/common/include/linux/device.h
505

There is no harm in reading Linux code. The fear of infection is unjustified in the last 30 odd years of worry.

510

Actually, it just needs a device_detach followed by a device_probe_and_attach.

530

Likely

532

Why wouldn't it work? The device_t isn't destroying so there are non of the use after free issues.

bz edited the test plan for this revision. (Show Details)

Try II of these functions base don feedback.

Please not functions have been swapped so previous review comments may be mis-leading.

bz marked 11 inline comments as done.Jun 4 2021, 4:55 PM

Move the devres declaration further up so they can be used
earlier in the file (with one of the new functions).
This avoids build errros.

Do we think the new version could work (enough for now)?

Yea, this is good enough for now, I think... I suspect we'll need to tweak based on what Linux does vs the docs...

This revision is now accepted and ready to land.Jun 11 2021, 3:49 PM