Page MenuHomeFreeBSD

LinuxKPI: add device_reprobe() and device_release_driver()
ClosedPublic

Authored by bz on May 28 2021, 11:22 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 1, 8:32 AM
Unknown Object (File)
Mon, Oct 21, 9:07 AM
Unknown Object (File)
Oct 1 2024, 5:01 AM
Unknown Object (File)
Sep 29 2024, 2:19 AM
Unknown Object (File)
Sep 27 2024, 11:31 PM
Unknown Object (File)
Sep 23 2024, 6:25 AM
Unknown Object (File)
Sep 11 2024, 4:00 PM
Unknown Object (File)
Sep 8 2024, 9:49 PM
Subscribers

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
rG FreeBSD src repository
Lint
Lint Not Applicable
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
506

What's Linux say this should do?

511

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

520

what's linux say this should do?

525

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.

531

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

532

device_detach() leaves the device detached and unprobed/bound

533

So this should be all you need...

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

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

511

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

531

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

533

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
506

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

511

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

531

Likely

533

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