netdev - Re: [PATCH v2] net: phy: realtek: read actual speed on rtl8211f to detect downshift

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: <7d8bf728-7d73-fa8c-d63d-49e9e6c872fd@gmail.com>
Date:   Tue, 24 Nov 2020 23:22:03 +0100
From:   Heiner Kallweit <hkallweit1@...il.com>
To:     Antonio Borneo <antonio.borneo@...com>,
        Andrew Lunn <andrew@...n.ch>,
        Russell King <linux@...linux.org.uk>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
        Yonglong Liu <liuyonglong@...wei.com>,
        Willy Liu <willy.liu@...ltek.com>
Cc:     linuxarm@...wei.com, Salil Mehta <salil.mehta@...wei.com>,
        linux-stm32@...md-mailman.stormreply.com,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] net: phy: realtek: read actual speed on rtl8211f to
 detect downshift

Am 24.11.2020 um 22:59 schrieb Antonio Borneo:
> The rtl8211f supports downshift and before commit 5502b218e001
> ("net: phy: use phy_resolve_aneg_linkmode in genphy_read_status")
> the read-back of register MII_CTRL1000 was used to detect the
> negotiated link speed.
> The code added in commit d445dff2df60 ("net: phy: realtek: read
> actual speed to detect downshift") is working fine also for this
> phy and it's trivial re-using it to restore the downshift
> detection on rtl8211f.
> 
> Add the phy specific read_status() pointing to the existing
> function rtlgen_read_status().
> 
> Signed-off-by: Antonio Borneo <antonio.borneo@...com>
> Link: https://lore.kernel.org/r/478f871a-583d-01f1-9cc5-2eea56d8c2a7@huawei.com
> ---
> To: Andrew Lunn <andrew@...n.ch>
> To: Heiner Kallweit <hkallweit1@...il.com>
> To: Russell King <linux@...linux.org.uk>
> To: "David S. Miller" <davem@...emloft.net>
> To: Jakub Kicinski <kuba@...nel.org>
> To: netdev@...r.kernel.org
> To: Yonglong Liu <liuyonglong@...wei.com>
> To: Willy Liu <willy.liu@...ltek.com>
> Cc: linuxarm@...wei.com
> Cc: Salil Mehta <salil.mehta@...wei.com>
> Cc: linux-stm32@...md-mailman.stormreply.com
> Cc: linux-kernel@...r.kernel.org
> In-Reply-To: <20201124143848.874894-1-antonio.borneo@...com>
> 
> V1 => V2
> 	move from a generic implementation affecting every phy
> 	to a rtl8211f specific implementation
> ---
>  drivers/net/phy/realtek.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
> index 575580d3ffe0..8ff8a4edc173 100644
> --- a/drivers/net/phy/realtek.c
> +++ b/drivers/net/phy/realtek.c
> @@ -621,6 +621,7 @@ static struct phy_driver realtek_drvs[] = {
>  		PHY_ID_MATCH_EXACT(0x001cc916),
>  		.name		= "RTL8211F Gigabit Ethernet",
>  		.config_init	= &rtl8211f_config_init,
> +		.read_status	= rtlgen_read_status,
>  		.ack_interrupt	= &rtl8211f_ack_interrupt,
>  		.config_intr	= &rtl8211f_config_intr,
>  		.suspend	= genphy_suspend,
> 
> base-commit: 9bd2702d292cb7b565b09e949d30288ab7a26d51
> 

Pefect would be to make this a fix for 5502b218e001,
but rtlgen_read_status() was added one year after this change.
Marking the change that added rtlgen_read_status() as "Fixes"
would be technically ok, but as it's not actually broken not
everybody may be happy with this.
Having said that I'd be fine with treating this as an improvement,
downshift should be a rare case.