This is a brief introduction into the top-level newbus
driver methods and net80211 driver registration.
Details
- Reviewers
- None
- Group Reviewers
wireless - Commits
- rG2409e2436a53: net80211: add driver registration documentation
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
3rd party quick look remarks noted, thank you @adrian very nice documentation, my minor remarks / questions noted :-)
There may be some diagrams handy to have directly in the plaintext docs that could be then rendered into a vector image. There is a classic PlantUML [1,2,3] solution (Java based) that can be easily integrated with Sphinx and produce html and pdf output, there are some js integrations.. or newer more fancy pancy Mermaid [4] diagrams system but JS based and not compatible with PlantUML of coulse :D
[1] https://plantuml.com/
[2] https://github.com/plantuml
[3] https://github.com/plantuml
[4] https://mermaid.js.org/
| sys/net80211/DRIVER_REGISTRATION.md | ||
|---|---|---|
| 16–19 | There currently isn't a call to allocate or initialise | |
| 51 | why not "listing out the NIC hardware, firmware and feature support" is not done in Probe stage? :-P Is it possible to obtain these information as Probe stage or the driver needs to be attached to get them? | |
| 77 | ). | |
| 78 | ). | |
| 81 | ). | |
| 109 | detach pass -> phase? | |
| 135 | hardware is no longer available -> turned off? inactive state? ..but can be used again with another driver attach? :-) | |
| 160 | starting or started / operational already? wake up completed? | |
| 172 | What is the difference between Detach and Shutdown then? :-) Shall Shutdown be followed by Detach? | |
| 238 | Wouldn't be safer to recommend struct ieee80211com X = {0} or any other way to make sure zero fill? | |
| 248 | maybe state struct member would be simple and handy solution? state -> [off, probing, probed, attaching, attached, suspending, suspended, resuming, resumed, detaching, detached, error, ...]. | |
Thanks for your review!
| sys/net80211/DRIVER_REGISTRATION.md | ||
|---|---|---|
| 51 | the probe stage of a driver isn't for that. It's for doing the minimum matching of the bus info and device info to see if the driver wants to claim the device. It needs to be super light weight as it's currently called by every new device on that bus type coming and going. | |
| 77 | Is the use of ")." documented anywhere? typical english grammar rules put the sentence finish period inside the final paranthesis like this. Do we have a policy saying we should put it on the outside? | |
| 135 | The point here is that you don't know the state of the hardware, nor whether it's there or not. newbus takes care of telling you if it shows up again through calls to probe/attach. | |
| 160 | Oh crap, it should be ieee80211_resume_all() . I'll fix that in a sec. It may or may not be started or operational. ignoring suspend/resume, imagine the cases of having no vaps, then creating a few, starting them, stoppign them, then destroying them. net80211 doesn't know about the hardware state - it's just calling ic_parent() each time to let the driver know it should go see what's going on. If it's going from 0 active -> 1 active, it'll likely need to wake up the hardware, load firwmare, setup the VAP state and program it in. Then the net80211 machinery does its thing. suspend/resume wraps the above behaviour. For example, entering suspend the same machinery to shut all vaps down, put the NIC into some firmware sleep state or whatnot needs to happen. Then the driver call here can do stuff like release the bus, reconfigure it for lower power behaviour, etc. Resume can do the opposite - it may need to do some bus work to configure some stuff first before it tries talking to the hardware, and then the ic_parent state machine to re-init the device, load firmware, create VAP firmware state, etc all has to happen. The only real thing to keep in mind here is an entry into suspend() needs to leave the hardware in a state that doesn't block suspend (either by hanging it, or keeping a bus configured in a way that blocks that, etc) and resume needs to assume the hardware was put in a suspend-compatible state and undo that. | |
| 172 | no, shutdown is "we're shutting down the kernel, you can skip any state you need to here". | |
| 238 | I'm leaving it as future work rather than making suggestions as I'd like to document what we're currently doing, not what we should be doing. I / others can then tackle the future work sections to figure out what we should do, and then update the documentation about what to do and fix the stack/drivers. (eg once this lands, if you want to go audit all of the wifi drivers and write a net80211_com_init(), net80211_com_postattach(), net80211_com_preshutdonw() methods - even if they're just TODO items for now - then we can document that here. :-) | |
| 248 | in general, yeah. we still have fun side things like "ok we're shutting down, what do we do about drivers being in ioctls still?" since newbus doesn't track the driver entry/exit point or the stack entry/exit point. But it'd be good to figure this out a bit moer. (same as above though - I'm documenting what it currently does and what we should think about cleaning up in the future, but not saying HOW to clean it up in the future.) | |