netdev - Re: [PATCH net-next 1/4] net: phy: Allow loopback speed selection for PHY drivers

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: <436283e7-16c3-46ef-9970-13ddbf3a2de3@engleder-embedded.com>
Date: Tue, 29 Oct 2024 06:58:12 +0100
From: Gerhard Engleder <gerhard@...leder-embedded.com>
To: Lee Trager <lee@...ger.us>
Cc: netdev@...r.kernel.org, andrew@...n.ch, pabeni@...hat.com,
 kuba@...nel.org, hkallweit1@...il.com, linux@...linux.org.uk,
 davem@...emloft.net, edumazet@...gle.com
Subject: Re: [PATCH net-next 1/4] net: phy: Allow loopback speed selection for
 PHY drivers

On 29.10.24 00:23, Lee Trager wrote:
> On 10/28/24 1:38 PM, Gerhard Engleder wrote:
>> -int genphy_loopback(struct phy_device *phydev, bool enable)
>> +int genphy_loopback(struct phy_device *phydev, bool enable, int speed)
>>   {
>>       if (enable) {
>>           u16 ctl = BMCR_LOOPBACK;
>>           int ret, val;
>> +        if (speed == SPEED_10 || speed == SPEED_100 ||
>> +            speed == SPEED_1000)
>> +            phydev->speed = speed;
> Why is this limited to 1000? Shouldn't the max loopback speed be limited 
> by max hardware speed? We currently have definitions going up to 
> SPEED_800000 so some devices should support higher than 1000.

This generic loopback implementation only supports those three speeds
currently. If there is the need for higher speed, then there should be
PHY specific implementations in the PHY drivers.

>   Why is speed defined as an int? It can never be negative, I normally 
> see it defined as u32.

speed is defined within phy_device also as int. I aligned the data type
to this structure.

Gerhard