| 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
| ||
|
Date: Tue, 1 Nov 2022 11:36:23 -0400
From: Sean Anderson <sean.anderson@...o.com>
To: Vladimir Oltean <vladimir.oltean@....com>, netdev@...r.kernel.org
Cc: Claudiu Manoil <claudiu.manoil@....com>,
Alexandre Belloni <alexandre.belloni@...tlin.com>,
UNGLinuxDriver@...rochip.com,
Heiner Kallweit <hkallweit1@...il.com>,
Russell King <linux@...linux.org.uk>,
Colin Foster <colin.foster@...advantage.com>,
Andrew Lunn <andrew@...n.ch>,
Vivien Didelot <vivien.didelot@...il.com>,
Florian Fainelli <f.fainelli@...il.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH net-next 1/4] net: phy: aquantia: add AQR112 and AQR412
PHY IDs
On 11/1/22 07:48, Vladimir Oltean wrote:
> These are Gen3 Aquantia N-BASET PHYs which support 5GBASE-T,
> 2.5GBASE-T, 1000BASE-T and 100BASE-TX (not 10G); also EEE, Sync-E,
> PTP, PoE.
>
> The 112 is a single PHY package, the 412 is a quad PHY package.
>
> The system-side SERDES interface of these PHYs selects its protocol
> depending on the negotiated media side link speed. That protocol can be
> 1000BASE-X, 2500BASE-X, 10GBASE-R, SGMII, USXGMII.
>
> The configuration of which SERDES protocol to use for which link speed
> is made by firmware; even though it could be overwritten over MDIO by
> Linux, we assume that the firmware provisioning is ok for the board on
> which the driver probes.
>
> For cases when the system side runs at a fixed rate, we want phylib/phylink
> to detect the PAUSE rate matching ability of these PHYs, so we need to
> use the Aquantia rather than the generic C45 driver. This needs
> aqr107_read_status() -> aqr107_read_rate() to set phydev->rate_matching,
> as well as the aqr107_get_rate_matching() method.
>
> I am a bit unsure about the naming convention in the driver. Since
> AQR107 is a Gen2 PHY, I assume all functions prefixed with "aqr107_"
> rather than "aqr_" mean Gen2+ features. So I've reused this naming
> convention.
In Aquantia's BSP there are references to 6 generations of phys (where
the "first" generation is the first 28nm phy; implicitly making the 40nm
phys generation zero). As far as I can tell these are completely
different from the generations of phys you refer to, which seem to me
marketing names. Unfortunately, they don't have a mapping of phys to
generations, so I'm not even sure which phys are which generations. The
datasheets for all but the latest phys seem to have gone missing...
In any case, if it works, then I think it's reasonable to use these
functions.
> I've tested PHY "SGMII" statistics as well as the .link_change_notify
> method, which prints:
>
> Aquantia AQR412 mdio_mux-0.4:00: Link partner is Aquantia PHY, FW 4.3, fast-retrain downshift advertised, fast reframe advertised
>
> Tested SERDES protocols are usxgmii and 2500base-x (the latter with
> PAUSE rate matching). Tested link modes are 100/1000/2500 Base-T
> (with Aquantia link partner and with other link partners). No notable
> events observed.
>
> The placement of these PHY IDs in the driver is right before AQR113C,
> a Gen4 PHY.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
> ---
> drivers/net/phy/aquantia_main.c | 40 +++++++++++++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
>
> diff --git a/drivers/net/phy/aquantia_main.c b/drivers/net/phy/aquantia_main.c
> index 47a76df36b74..334a6904ca5a 100644
> --- a/drivers/net/phy/aquantia_main.c
> +++ b/drivers/net/phy/aquantia_main.c
> @@ -22,6 +22,8 @@
> #define PHY_ID_AQR107 0x03a1b4e0
> #define PHY_ID_AQCS109 0x03a1b5c2
> #define PHY_ID_AQR405 0x03a1b4b0
> +#define PHY_ID_AQR112 0x03a1b662
> +#define PHY_ID_AQR412 0x03a1b712
> #define PHY_ID_AQR113C 0x31c31c12
>
> #define MDIO_PHYXS_VEND_IF_STATUS 0xe812
> @@ -800,6 +802,42 @@ static struct phy_driver aqr_driver[] = {
> .handle_interrupt = aqr_handle_interrupt,
> .read_status = aqr_read_status,
> },
> +{
> + PHY_ID_MATCH_MODEL(PHY_ID_AQR112),
> + .name = "Aquantia AQR112",
> + .probe = aqr107_probe,
> + .config_aneg = aqr_config_aneg,
> + .config_intr = aqr_config_intr,
> + .handle_interrupt = aqr_handle_interrupt,
> + .get_tunable = aqr107_get_tunable,
> + .set_tunable = aqr107_set_tunable,
> + .suspend = aqr107_suspend,
> + .resume = aqr107_resume,
> + .read_status = aqr107_read_status,
> + .get_rate_matching = aqr107_get_rate_matching,
> + .get_sset_count = aqr107_get_sset_count,
> + .get_strings = aqr107_get_strings,
> + .get_stats = aqr107_get_stats,
> + .link_change_notify = aqr107_link_change_notify,
> +},
> +{
> + PHY_ID_MATCH_MODEL(PHY_ID_AQR412),
> + .name = "Aquantia AQR412",
> + .probe = aqr107_probe,
> + .config_aneg = aqr_config_aneg,
> + .config_intr = aqr_config_intr,
> + .handle_interrupt = aqr_handle_interrupt,
> + .get_tunable = aqr107_get_tunable,
> + .set_tunable = aqr107_set_tunable,
> + .suspend = aqr107_suspend,
> + .resume = aqr107_resume,
> + .read_status = aqr107_read_status,
> + .get_rate_matching = aqr107_get_rate_matching,
> + .get_sset_count = aqr107_get_sset_count,
> + .get_strings = aqr107_get_strings,
> + .get_stats = aqr107_get_stats,
> + .link_change_notify = aqr107_link_change_notify,
> +},
> {
> PHY_ID_MATCH_MODEL(PHY_ID_AQR113C),
> .name = "Aquantia AQR113C",
> @@ -831,6 +869,8 @@ static struct mdio_device_id __maybe_unused aqr_tbl[] = {
> { PHY_ID_MATCH_MODEL(PHY_ID_AQR107) },
> { PHY_ID_MATCH_MODEL(PHY_ID_AQCS109) },
> { PHY_ID_MATCH_MODEL(PHY_ID_AQR405) },
> + { PHY_ID_MATCH_MODEL(PHY_ID_AQR112) },
> + { PHY_ID_MATCH_MODEL(PHY_ID_AQR412) },
> { PHY_ID_MATCH_MODEL(PHY_ID_AQR113C) },
> { }
> };
Reviewed-by: Sean Anderson <seanga2@...il.com>
Powered by blists - more mailing lists