On Sun, 22 Oct 2023 17:34:04 +0200
Andrew Lunn [off-list ref] wrote:

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

quoted

On Sun, Oct 22, 2023 at 11:47 AM FUJITA Tomonori
[off-list ref] wrote:

quoted

Agreed that the first three paragraphs at the top of the file are
implementation comments. Are there any other comments in the file,
which look implementation comments to you? To me, the rest look the
docs for Rust API users.

I think some should be improved with that in mind, yeah. For instance,
this one seems good to me:

    /// An instance of a PHY driver.

But this one is not:

    /// Creates the kernel's `phy_driver` instance.

It is especially bad because the first line of the docs is the "short
description" used for lists by `rustdoc`.

For similar reasons, this one is bad (and in this case it is the only line!):

    /// Corresponds to the kernel's `enum phy_state`.

That line could be part of the documentation if you think it is
helpful for a reader as a practical note explaining what it is
supposed to map in the C side. But it should really not be the very
first line / short description.

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
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

I added links to the kdoc like:

/// Corresponds to the kernel's [`enum phy_state`](https://docs.kernel.org/networking/kapi.html#c.phy_state).

But the first line needs to a short description so I copy the C
description:

/// PHY state machine states.

I revised all the comments.