netdev - Re: [RFC PATCH net v1] net: phy: aquantia: Set phy speed to 2.5gbps for AQR115c

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] [day] [month] [year] [list]

Message-ID: <a4a4f5e2-f925-442c-b262-629de63022ba@quicinc.com>
Date: Wed, 18 Sep 2024 15:24:09 -0700
From: "Abhishek Chauhan (ABC)" <quic_abchauha@...cinc.com>
To: Andrew Lunn <andrew@...n.ch>
CC: "Russell King (Oracle)" <linux@...linux.org.uk>,
        Maxime Chevallier
	<maxime.chevallier@...tlin.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
        "Paolo
 Abeni" <pabeni@...hat.com>, <netdev@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, Andrew Halaney <ahalaney@...hat.com>,
        "Heiner
 Kallweit" <hkallweit1@...il.com>,
        Bartosz Golaszewski
	<bartosz.golaszewski@...aro.org>,
        "linux-tegra@...r.kernel.org"
	<linux-tegra@...r.kernel.org>,
        Brad Griffis <bgriffis@...dia.com>,
        "Vladimir
 Oltean" <vladimir.oltean@....com>,
        Jon Hunter <jonathanh@...dia.com>, <kernel@...cinc.com>
Subject: Re: [RFC PATCH net v1] net: phy: aquantia: Set phy speed to 2.5gbps
 for AQR115c



On 9/18/2024 2:45 PM, Andrew Lunn wrote:
>> Russell and Andrew 
>>
>> we added prints and understood what the phy is reporting as part of the 
>> genphy_c45_pma_read_abilities 
>>
>> [   12.041576] MDIO_STAT2: 0xb301
>>
>>
>> [   12.050722] MDIO_PMA_EXTABLE: 0x40fc
>>
>> >From the PMA extensible register we see that the phy is reporting that it supports
>>
>> #define MDIO_PMA_EXTABLE_10GBT		0x0004	/* 10GBASE-T ability */
>> #define MDIO_PMA_EXTABLE_10GBKX4	0x0008	/* 10GBASE-KX4 ability */
>> #define MDIO_PMA_EXTABLE_10GBKR		0x0010	/* 10GBASE-KR ability */
>> #define MDIO_PMA_EXTABLE_1000BT		0x0020	/* 1000BASE-T ability */
>> #define MDIO_PMA_EXTABLE_1000BKX	0x0040	/* 1000BASE-KX ability */
>> #define MDIO_PMA_EXTABLE_100BTX		0x0080	/* 100BASE-TX ability */
>> #define MDIO_PMA_EXTABLE_NBT		0x4000  /* 2.5/5GBASE-T ability */
>>
>> [   12.060265] MDIO_PMA_NG_EXTABLE: 0x3
>>
>> /* 2.5G/5G Extended abilities register. */
>> #define MDIO_PMA_NG_EXTABLE_2_5GBT	0x0001	/* 2.5GBASET ability */
>> #define MDIO_PMA_NG_EXTABLE_5GBT	0x0002	/* 5GBASET ability */
>>
>> I feel that the phy here is incorrectly reporting all these abilities as 
>> AQR115c supports speeds only upto 2.5Gbps 
>> https://www.marvell.com/content/dam/marvell/en/public-collateral/transceivers/marvell-phys-transceivers-aqrate-gen4-product-brief.pdf
>>
>> AQR115C / AQR115 Single port, 2.5Gbps / 1Gbps / 100Mbps / 10Mbps 7 x 7 mm / 7 x 11 mm
> 
> One things to check. Are you sure you have the correct firmware? Many
> of the registers which the standards say should be Read Only can be
> influenced by the firmware. So the wrong firmware, or provisioning
> taken from another device could result in the wrong capabilities being
> set.
> 

I did check with the hardware team and the firmware loaded is 
AQR-G4_v5.6.7-AQR_Marvell_NoSwap_XFI2500SGMII_ID44842_VER1922.cld
Only Marvell folks can tell me what is inside the FW. 
Let me double check with Marvell on this and ask them why is the phy 
reporting all these PMA capabilities. 

> You might want to report this issue to Marvell, but my guess would be,
> they don't care. I would guess the vendor driver ignores these
> registers and simply uses the product ID to determine what the device
> actually supports.
> 
>> I am thinking of solving this problem by having 
>> custom .get_features in the AQR115c driver to only set supported speeds 
>> upto 2.5gbps 
> 
> Yes, that is the correct solution.

> It would also be good if you could, in a separate patch, change the
> aqcs109_config_init() to not call phy_set_max_speed() and add a custom
> .get_features.
Let me raise this patch in a day or two for upstream review after 
testing it out locally on AQR115c 
> 
> 	Andrew