netdev - Re: [PATCH 1/2] net: phy: realtek: add combo mode support for RTL8211FS

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: <aa36e5a4-e7d2-4755-b2a1-58dc5a60af1c@lunn.ch>
Date: Tue, 3 Dec 2024 04:53:32 +0100
From: Andrew Lunn <andrew@...n.ch>
To: 万致远 <kmlinuxm@...il.com>
Cc: kuba@...nel.org, netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
	willy.liu@...ltek.com, Yuki Lee <febrieac@...look.com>
Subject: Re: [PATCH 1/2] net: phy: realtek: add combo mode support for
 RTL8211FS

On Tue, Dec 03, 2024 at 11:08:22AM +0800, 万致远 wrote:
> On 2024/12/3 7:52, Andrew Lunn wrote:
> >> +static int rtl8211f_config_aneg(struct phy_device *phydev)
> >> +{
> >> +    int ret;
> >> +
> >> +    struct rtl821x_priv *priv = phydev->priv;
> >> +
> >> +    ret = genphy_read_abilities(phydev);
> >> +    if (ret < 0)
> >> +            return ret;
> >> +
> >> +    linkmode_copy(phydev->advertising, phydev->supported);
> >
> > This is all very unusual for config_aneg(). genphy_read_abilities()
> > will have been done very early on during phy_probe(). So why do it
> > now? And why overwrite how the user might of configured what is to be
> > advertised?
> >
> 
> These codes are migrated from Rockchip SDK and I'm not familiar with this part.
> 
> I will use `linkmode_and` instead of `linkmode_copy` in my next
> version of patch like Marvell does.

No, it needs a lot more work than just that. Spend some time to really
understand how the marvell driver handles either copper or fibre, and
assume the Rockchip SDK is poor quality code.

It might also be that the marvell scheme does not work. It will depend
on how the PHY actually works.

Andrew