netdev - Re: [PATCH net-next 4/5] net: phy: Document core PHY structures

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3b0df238-bedc-3030-f81d-2c50d3e99be2@gmail.com>
Date:   Sun, 20 Sep 2020 14:29:44 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Andrew Lunn <andrew@...n.ch>, David Miller <davem@...emloft.net>
Cc:     netdev <netdev@...r.kernel.org>,
        Heiner Kallweit <hkallweit1@...il.com>
Subject: Re: [PATCH net-next 4/5] net: phy: Document core PHY structures



On 9/20/2020 10:17 AM, Andrew Lunn wrote:
> Add kerneldoc for the core PHY data structures, a few inline functions
> and exported functions which are not already documented.
> 
> Signed-off-by: Andrew Lunn <andrew@...n.ch>

Looks good for the most part, if you can capitalize Phy, phy or any 
variant in the process that would be great. See below for more specific 
feedback.

> ---

[snip]

> -/* Interface Mode definitions */
> +/**
> + * enum phy_interface_t - Interface Mode definitions
> + *
> + * @PHY_INTERFACE_MODE_NA: Not Applicable - don't touch
> + * @PHY_INTERFACE_MODE_INTERNAL: No interface, MAC and PHY combined
> + * @PHY_INTERFACE_MODE_MII: Median-independent interface
> + * @PHY_INTERFACE_MODE_GMII: Gigabit median-independent interface
> + * @PHY_INTERFACE_MODE_SGMII: Serial gigabit media-independent interface
> + * @PHY_INTERFACE_MODE_TBI: ???

Ten Bit interface, you can find some information about it here: 
http://www.actel.com/ipdocs/CoreSGMII_HB.pdf

> + * @PHY_INTERFACE_MODE_REVMII: Reverse Media Independent Interface
> + * @PHY_INTERFACE_MODE_RMII: Reduced Media Independent Interface
> + * @PHY_INTERFACE_MODE_RGMII: Reduced gigabit media-independent interface
> + * @PHY_INTERFACE_MODE_RGMII_ID: RGMII with Internal RX+TX delay
> + * @PHY_INTERFACE_MODE_RGMII_RXID: RGMII with Internal RX delay
> + * @PHY_INTERFACE_MODE_RGMII_TXID: RGMII with Internal RX delay
> + * @PHY_INTERFACE_MODE_RTBI: Reduced TBI
> + * @PHY_INTERFACE_MODE_SMII: ??? MII
> + * @PHY_INTERFACE_MODE_XGMII: 10 gigabit media-independent interface
> + * @PHY_INTERFACE_MODE_XLGMII:40 gigabit media-independent interface
> + * @PHY_INTERFACE_MODE_MOCA: Multimetia over Coax

Multimedia.

[snip]

>   
> +/**
> + * struct mdio_bus_stats - Statistics counters for MDIO busses
> + * @transfers: Total number of transfers, i.e. @writes + @reads
> + * @errors: Number of MDIO transfers that returned an error
> + * @writes: Number of write transfers
> + * @reads: Number of read transfers
> + * @syncp: Synchronization for incrimenting statistics

typo: incrementing.

[snip]

> + * struct mii_bus - Represents an MDIO bus
> + *
> + * @owner: Who owns this device
> + * @name: User friendly name for this MDIO device, or driver name
> + * @id: Identifer for this bus, typical from bus hierarchy

s/Identifer/Identifier/g even unique identifier would be better.

[snip]

> -	/* list of all PHYs on bus */
> +	/** @mdio_map: list of all PHYs on bus */

s/PHYs/MDIO devices/

[snip]

> + * @mdio: MDIO bus this PHY is on
> + * @drv: Pointer to the driver for this PHY instance
> + * @phy_id: UID for this device found during discovery
> + * @c45_ids: 802.3-c45 Device Identifers if is_c45.

Identifiers.

[snip]

> + * @interface: mii, rmii, rgmii etc.

You could reference the enumeration now that you defined its values.
-- 
Florian