On Sun, Oct 22, 2023 at 5:34 PM Andrew Lunn [off-list ref] wrote:

On Sun, Oct 22, 2023 at 01:37:33PM +0200, Miguel Ojeda wrote:

quoted

Instead, the documentation should answer the question "What is this?".
And the answer should be something like "The state of the PHY ......"

Its the state of the state machine, not the state of the PHY. It is

I didn't say it was the state of the PHY -- please note the dots above.

already documented in kernel doc, so we don't really want to duplicate
it. So maybe just cross reference to the kdoc:

https://docs.kernel.org/networking/kapi.html#c.phy_state

Yes, that can be worth it for simple 1:1 cases like the `enum`
(assuming they are properly documented in the C side), but we still
want a suitable short description (e.g. "PHY state machine states"),
like Tomonori did in the version he just sent (v6).

I wondered about the docs of each variant, too, but those should be OK
too, because `rustdoc` does not create an individual page for them, so
one can always see the link to the C docs at the top from the `enum`
description.

quoted

Yes, documenting that something wraps/relies on/maps a particular C
functionality is something we do for clarity and practicality (we also
link the related C headers). This is, I assume, the kind of clarity
Andrew was asking for, i.e. to be practical and let the user know what
they are dealing with on the C side, especially early on.

I don't think 'early on' is relevant. In the kernel, you pretty much
always need the bigger picture, how a pieces of the puzzle fits in
with what is above it and what is below it. Sometimes you need to
extend what is above and below. Or a reviewer will tell you to move
code into the core, so others can share it, etc.

"Early on" in my reply is referring to what you said earlier, i.e.
that initially abstractions are minimal.

In any case, yes, using complex systems typically requires knowing a
bit of what is going on in different parts, but that does not mean we
cannot make self-contained documentation as much as reasonably
possible. We want that using a particular Rust abstraction does not
require reading its source code or the code in the C side.

In the example that you mention, if the reviewer wants to share code,
then it should be extracted into a new Rust abstraction (and possibly
the C core depending on what it is, of course) and using it from the
driver, but also documenting the Rust abstraction.

Cheers,
Miguel