tmakin/tegra-linux-noble
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

net: phy: aquantia: fix -ETIMEDOUT PHY probe failure when firmware not present

Browse Source 
The author of the blamed commit apparently did not notice something
about aqr_wait_reset_complete(): it polls the exact same register -
MDIO_MMD_VEND1:VEND1_GLOBAL_FW_ID - as aqr_firmware_load().

Thus, the entire logic after the introduction of aqr_wait_reset_complete() is
now completely side-stepped, because if aqr_wait_reset_complete()
succeeds, MDIO_MMD_VEND1:VEND1_GLOBAL_FW_ID could have only been a
non-zero value. The handling of the case where the register reads as 0
is dead code, due to the previous -ETIMEDOUT having stopped execution
and returning a fatal error to the caller. We never attempt to load
new firmware if no firmware is present.

Based on static code analysis, I guess we should simply introduce a
switch/case statement based on the return code from aqr_wait_reset_complete(),
to determine whether to load firmware or not. I am not intending to
change the procedure through which the driver determines whether to load
firmware or not, as I am unaware of alternative possibilities.

At the same time, Russell King suggests that if aqr_wait_reset_complete()
is expected to return -ETIMEDOUT as part of normal operation and not
just catastrophic failure, the use of phy_read_mmd_poll_timeout() is
improper, since that has an embedded print inside. Just open-code a
call to read_poll_timeout() to avoid printing -ETIMEDOUT, but continue
printing actual read errors from the MDIO bus.

http://nvbugs/4980644

Fixes: ad649a1fac37 ("net: phy: aquantia: wait for FW reset before checking the vendor ID")
Reported-by: Clark Wang <xiaoning.wang@nxp.com>
Reported-by: Jon Hunter <jonathanh@nvidia.com>
Closes: https://lore.kernel.org/netdev/8ac00a45-ac61-41b4-9f74-d18157b8b6bf@nvidia.com/
Reported-by: Hans-Frieder Vogt <hfdevel@gmx.net>
Closes: https://lore.kernel.org/netdev/c7c1a3ae-be97-4929-8d89-04c8aa870209@gmx.net/
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Tested-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Tested-by: Hans-Frieder Vogt <hfdevel@gmx.net>
Link: https://patch.msgid.link/20240913121230.2620122-1-vladimir.oltean@nxp.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
(cherry picked from commit 194ef9d0de9021df4a0ba8b112f91e56adaddd22)
Signed-off-by: Revanth Kumar Uppala <ruppala@nvidia.com>
Reviewed-by: Jon Hunter <jonathanh@nvidia.com>
Signed-off-by: Vishwaroop A <va@nvidia.com>
Acked-by: Noah Wager <noah.wager@canonical.com>
Acked-by: Jacob Martin <jacob.martin@canonical.com>
Signed-off-by: Noah Wager <noah.wager@canonical.com>
This commit is contained in:
Vladimir Oltean
2024-09-13 15:12:30 +03:00
committed by Noah Wager
parent efea388b8f
commit e55ca3c752
2 changed files with 38 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files
drivers/net/phy/aquantia/aquantia_firmware.c
+23 -17
View File
@@ -353,26 +353,32 @@ int aqr_firmware_load(struct phy_device *phydev)
{
int ret;
ret = aqr_wait_reset_complete(phydev);
if (ret)
return ret;
/* Check if the firmware is not already loaded by pooling
* the current version returned by the PHY. If 0 is returned,
* no firmware is loaded.
/* Check if the firmware is not already loaded by polling
* the current version returned by the PHY.
*/
ret = phy_read_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_FW_ID);
if (ret > 0)
goto exit;
ret = aqr_wait_reset_complete(phydev);
switch (ret) {
case 0:
/* Some firmware is loaded => do nothing */
return 0;
case -ETIMEDOUT:
/* VEND1_GLOBAL_FW_ID still reads 0 after 2 seconds of polling.
* We don't have full confidence that no firmware is loaded (in
* theory it might just not have loaded yet), but we will
* assume that, and load a new image.
*/
ret = aqr_firmware_load_nvmem(phydev);
if (!ret)
return ret;
ret = aqr_firmware_load_nvmem(phydev);
if (!ret)
goto exit;
ret = aqr_firmware_load_fs(phydev);
if (ret)
ret = aqr_firmware_load_fs(phydev);
if (ret)
return ret;
break;
default:
/* PHY read error, propagate it to the caller */
return ret;
}
exit:
return 0;
}
drivers/net/phy/aquantia/aquantia_main.c
+15 -6
View File
@@ -669,6 +669,9 @@ static int aqr107_set_tunable(struct phy_device *phydev,
}
}
#define AQR_FW_WAIT_SLEEP_US 20000
#define AQR_FW_WAIT_TIMEOUT_US 2000000
/* If we configure settings whilst firmware is still initializing the chip,
* then these settings may be overwritten. Therefore make sure chip
* initialization has completed. Use presence of the firmware ID as
@@ -678,13 +681,19 @@ static int aqr107_set_tunable(struct phy_device *phydev,
*/
int aqr_wait_reset_complete(struct phy_device *phydev)
{
int val;
int ret, val;
return phy_read_mmd_poll_timeout(phydev, MDIO_MMD_VEND1,
VEND1_GLOBAL_FW_ID, val,
((val & VEND1_GLOBAL_FW_ID_MASK) != 0 &&
(val & VEND1_GLOBAL_FW_ID_MASK) != VEND1_GLOBAL_FW_ID_MASK),
20000, 2000000, false);
ret = read_poll_timeout(phy_read_mmd, val, val != 0,
AQR_FW_WAIT_SLEEP_US, AQR_FW_WAIT_TIMEOUT_US,
false, phydev, MDIO_MMD_VEND1,
VEND1_GLOBAL_FW_ID);
if (val < 0) {
phydev_err(phydev, "Failed to read VEND1_GLOBAL_FW_ID: %pe\n",
ERR_PTR(val));
return val;
}
return ret;
}
static void aqr107_chip_info(struct phy_device *phydev)