Re: [PATCH v3 2/3] netlink: add master/slave configuration support

[Michal Kubecek]

On Tue, Jun 09, 2020 at 10:47:17AM +0200, Oleksij Rempel wrote:
> This UAPI is needed for BroadR-Reach 100BASE-T1 devices. Due to lack of
> auto-negotiation support, we needed to be able to configure the
> MASTER-SLAVE role of the port manually or from an application in user
> space.
> 
> The same UAPI can be used for 1000BASE-T or MultiGBASE-T devices to
> force MASTER or SLAVE role. See IEEE 802.3-2018:
> 22.2.4.3.7 MASTER-SLAVE control register (Register 9)
> 22.2.4.3.8 MASTER-SLAVE status register (Register 10)
> 40.5.2 MASTER-SLAVE configuration resolution
> 45.2.1.185.1 MASTER-SLAVE config value (1.2100.14)
> 45.2.7.10 MultiGBASE-T AN control 1 register (Register 7.32)
> 
> The MASTER-SLAVE role affects the clock configuration:
> 
> -------------------------------------------------------------------------------
> When the  PHY is configured as MASTER, the PMA Transmit function shall
> source TX_TCLK from a local clock source. When configured as SLAVE, the
> PMA Transmit function shall source TX_TCLK from the clock recovered from
> data stream provided by MASTER.
> 
> iMX6Q                     KSZ9031                XXX
> ------\                /-----------\        /------------\
>       |                |           |        |            |
>  MAC  |<----RGMII----->| PHY Slave |<------>| PHY Master |
>       |<--- 125 MHz ---+-<------/  |        | \          |
> ------/                \-----------/        \------------/
>                                                ^
>                                                 \-TX_TCLK
> 
> -------------------------------------------------------------------------------
> 
> Since some clock or link related issues are only reproducible in a
> specific MASTER-SLAVE-role, MAC and PHY configuration, it is beneficial
> to provide generic (not 100BASE-T1 specific) interface to the user space
> for configuration flexibility and trouble shooting.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx>
> ---
>  ethtool.8.in           | 19 ++++++++++++++++
>  ethtool.c              |  1 +
>  netlink/desc-ethtool.c |  2 ++
>  netlink/settings.c     | 50 ++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 72 insertions(+)
> 
> diff --git a/ethtool.8.in b/ethtool.8.in
> index f8b09e0..dca7c7a 100644
> --- a/ethtool.8.in
> +++ b/ethtool.8.in
> @@ -52,6 +52,10 @@
>  .\"
>  .ds MA \fIxx\fP\fB:\fP\fIyy\fP\fB:\fP\fIzz\fP\fB:\fP\fIaa\fP\fB:\fP\fIbb\fP\fB:\fP\fIcc\fP
>  .\"
> +.\"	\(*MS - master-slave property
> +.\"
> +.ds MS \fBpreferred-master\fP|\fBpreferred-slave\fP|\fBforced-master\fP|\fBforced-slave\fP
> +.\"
>  .\"	\(*PA - IP address
>  .\"
>  .ds PA \fIip-address\fP

It would be nice if we could forbid line breaks around the dashes
somehow to prevent effects like

       ethtool -s devname [speed N] [duplex half|full] [port tp|aui|bnc|mii]
              [mdix auto|on|off] [autoneg on|off] [advertise N[/M] |
              advertise mode on|off ...]  [phyad N] [xcvr internal|external]
              [wol N[/M] | wol p|u|m|b|a|g|s|f|d...]
              [sopass xx:yy:zz:aa:bb:cc] [master-slave preferred-
              master|preferred-slave|forced-master|forced-slave] [msglvl
              N[/M] | msglvl type on|off ...]

(this is for 80 columns). But I have no idea how and it's a cosmetic
issue that can certainly be left for later.

> @@ -255,6 +259,7 @@ ethtool \- query or control network driver and hardware settings
>  .RB [ wol \ \fIN\fP[\fB/\fP\fIM\fP]
>  .RB | \ wol \ \*(WO]
>  .RB [ sopass \ \*(MA]
> +.RB [ master-slave \ \*(MS]
>  .RB [ msglvl
>  .IR N\fP[/\fIM\fP] \ |
>  .BI msglvl \ type
> @@ -646,6 +651,20 @@ Sets full or half duplex mode.
>  .A4 port tp aui bnc mii fibre da
>  Selects device port.
>  .TP
> +.BR master-slave \ \*(MS
> +Configure MASTER/SLAVE role of the PHY. When the PHY is configured as MASTER,
> +the PMA Transmit function shall source TX_TCLK from a local clock source. When
> +configured as SLAVE, the PMA Transmit function shall source TX_TCLK from the
> +clock recovered from data stream provided by MASTER. Not all devices support this.
> +.TS
> +nokeep;
> +lB	l.
> +preferred-master Prefer MASTER role on autonegotiation
> +preferred-slave	 Prefer SLAVE role on autonegotiation
> +forced-master    Force the PHY in MASTER role. Can be used without autonegotiation
> +forced-slave     Force the PHY in SLAVE role. Can be used without autonegotiation
> +.TE
> +.TP
>  .A3 mdix auto on off
>  Selects MDI-X mode for port. May be used to override the automatic
>  detection feature of most adapters. An argument of \fBauto\fR means

This table is not formatted correctly. I played with the source a bit
and it seems the columns should be separated (on each line) by one
tabulator.

With this fixed,

Reviewed-by: Michal Kubecek <mkubecek@xxxxxxx>

Michal

Attachment: signature.asc
Description: PGP signature