netdev - Re: [PATCH net-next] net: phy: swphy: Support all normal speeds when link down

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 for Android: free password hash cracker in your pocket

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

Message-ID: <c80ab3f0-66f8-8446-f7d5-eb6add2ce9d2@seco.com>
Date:   Mon, 5 Dec 2022 10:28:39 -0500
From:   Sean Anderson <sean.anderson@...o.com>
To:     Andrew Lunn <andrew@...n.ch>, netdev <netdev@...r.kernel.org>
Cc:     Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>
Subject: Re: [PATCH net-next] net: phy: swphy: Support all normal speeds when
 link down

On 12/4/22 12:41, Andrew Lunn wrote:
> The software PHY emulator validation function is happy to accept any
> link speed if the link is down. swphy_read_reg() however triggers a
> WARN_ON(). Change this to report all the standard 1G link speeds are
> supported. Once the speed is known the supported link modes will
> change, which is a bit odd, but for emulation is probably O.K.
> 
> Suggested-by: Russell King (Oracle) <linux@...linux.org.uk>
> Signed-off-by: Andrew Lunn <andrew@...n.ch>
> ---
> 
> Hi Sean
> 
> Does this fix your problem?

Well, it removes the WARN, which is all I was after.

--Sean

> drivers/net/phy/swphy.c | 30 +++++++++++++++++++++++-------
>  1 file changed, 23 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/phy/swphy.c b/drivers/net/phy/swphy.c
> index 59f1ba4d49bc..63bd98217092 100644
> --- a/drivers/net/phy/swphy.c
> +++ b/drivers/net/phy/swphy.c
> @@ -29,8 +29,10 @@ enum {
>  	SWMII_SPEED_10 = 0,
>  	SWMII_SPEED_100,
>  	SWMII_SPEED_1000,
> +	SWMII_SPEED_UNKNOWN,
>  	SWMII_DUPLEX_HALF = 0,
>  	SWMII_DUPLEX_FULL,
> +	SWMII_DUPLEX_UNKNOWN,
>  };
>  
>  /*
> @@ -51,6 +53,11 @@ static const struct swmii_regs speed[] = {
>  		.lpagb = LPA_1000FULL | LPA_1000HALF,
>  		.estat = ESTATUS_1000_TFULL | ESTATUS_1000_THALF,
>  	},
> +	[SWMII_SPEED_UNKNOWN] = {
> +		.bmsr  = BMSR_ESTATEN | BMSR_100FULL | BMSR_100HALF |
> +			 BMSR_10FULL | BMSR_10HALF,
> +		.estat = ESTATUS_1000_TFULL | ESTATUS_1000_THALF,
> +	},
>  };
>  
>  static const struct swmii_regs duplex[] = {
> @@ -66,6 +73,11 @@ static const struct swmii_regs duplex[] = {
>  		.lpagb = LPA_1000FULL,
>  		.estat = ESTATUS_1000_TFULL,
>  	},
> +	[SWMII_DUPLEX_UNKNOWN] = {
> +		.bmsr  = BMSR_ESTATEN | BMSR_100FULL | BMSR_100HALF |
> +			 BMSR_10FULL | BMSR_10HALF,
> +		.estat = ESTATUS_1000_TFULL | ESTATUS_1000_THALF,
> +	},
>  };
>  
>  static int swphy_decode_speed(int speed)
> @@ -87,8 +99,9 @@ static int swphy_decode_speed(int speed)
>   * @state: software phy status
>   *
>   * This checks that we can represent the state stored in @state can be
> - * represented in the emulated MII registers.  Returns 0 if it can,
> - * otherwise returns -EINVAL.
> + * represented in the emulated MII registers. Invalid speed is allowed
> + * when the link is down, but the speed must be valid when the link is
> + * up. Returns 0 if it can, otherwise returns -EINVAL.
>   */
>  int swphy_validate_state(const struct fixed_phy_status *state)
>  {
> @@ -123,11 +136,14 @@ int swphy_read_reg(int reg, const struct fixed_phy_status *state)
>  	if (reg > MII_REGS_NUM)
>  		return -1;
>  
> -	speed_index = swphy_decode_speed(state->speed);
> -	if (WARN_ON(speed_index < 0))
> -		return 0;
> -
> -	duplex_index = state->duplex ? SWMII_DUPLEX_FULL : SWMII_DUPLEX_HALF;
> +	if (state->link) {
> +		speed_index = swphy_decode_speed(state->speed);
> +		duplex_index = state->duplex ? SWMII_DUPLEX_FULL :
> +			SWMII_DUPLEX_HALF;
> +	} else {
> +		speed_index = SWMII_SPEED_UNKNOWN;
> +		duplex_index = SWMII_DUPLEX_UNKNOWN;
> +	}
>  
>  	bmsr |= speed[speed_index].bmsr & duplex[duplex_index].bmsr;
>  	estat |= speed[speed_index].estat & duplex[duplex_index].estat;