| Message ID | 20240619184550.34524-6-brgl@bgdev.pl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | net: support 2.5G ethernet in dwmac-qcom-ethqos | expand |
On Wed, Jun 19, 2024 at 08:45:46PM +0200, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > Checking the firmware register before it boots makes no sense, it will > report 0 even if FW is loaded. Always wait for FW to boot before > continuing. Please split this patch up. One patch which renames the method to the more generic aqr_ since it is used by more than aqr107. Then add the new use of it. Is this actually a fix? What happens to the firmware if you try to download it while it is still booting? Or do you end up downloading firmware when it is not actually needed? Please expand the commit message. Andrew --- pw-bot: cr
On Wed, Jun 19, 2024 at 9:27 PM Andrew Lunn <andrew@lunn.ch> wrote: > > On Wed, Jun 19, 2024 at 08:45:46PM +0200, Bartosz Golaszewski wrote: > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > Checking the firmware register before it boots makes no sense, it will > > report 0 even if FW is loaded. Always wait for FW to boot before > > continuing. > > Please split this patch up. One patch which renames the method to the > more generic aqr_ since it is used by more than aqr107. Then add the > new use of it. > Will do. > Is this actually a fix? What happens to the firmware if you try to > download it while it is still booting? Or do you end up downloading > firmware when it is not actually needed? Please expand the commit > message. > It says '0' and the driver tries to load it from nvmem, then the filesystem and bails-out after these two fail. I'll extend the commit message. Bart > Andrew > > --- > pw-bot: cr
diff --git a/drivers/net/phy/aquantia/aquantia.h b/drivers/net/phy/aquantia/aquantia.h index b8502793962e..2465345081f8 100644 --- a/drivers/net/phy/aquantia/aquantia.h +++ b/drivers/net/phy/aquantia/aquantia.h @@ -201,5 +201,6 @@ int aqr_phy_led_hw_control_set(struct phy_device *phydev, u8 index, int aqr_phy_led_active_low_set(struct phy_device *phydev, int index, bool enable); int aqr_phy_led_polarity_set(struct phy_device *phydev, int index, unsigned long modes); +int aqr_wait_reset_complete(struct phy_device *phydev); #endif /* AQUANTIA_H */ diff --git a/drivers/net/phy/aquantia/aquantia_firmware.c b/drivers/net/phy/aquantia/aquantia_firmware.c index 0c9640ef153b..524627a36c6f 100644 --- a/drivers/net/phy/aquantia/aquantia_firmware.c +++ b/drivers/net/phy/aquantia/aquantia_firmware.c @@ -353,6 +353,10 @@ 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. diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c index 11da460698b0..eab779db225c 100644 --- a/drivers/net/phy/aquantia/aquantia_main.c +++ b/drivers/net/phy/aquantia/aquantia_main.c @@ -441,7 +441,7 @@ static int aqr107_set_tunable(struct phy_device *phydev, * The chip also provides a "reset completed" bit, but it's cleared after * read. Therefore function would time out if called again. */ -static int aqr107_wait_reset_complete(struct phy_device *phydev) +int aqr_wait_reset_complete(struct phy_device *phydev) { int val; @@ -494,7 +494,7 @@ static int aqr107_config_init(struct phy_device *phydev) WARN(phydev->interface == PHY_INTERFACE_MODE_XGMII, "Your devicetree is out of date, please update it. The AQR107 family doesn't support XGMII, maybe you mean USXGMII.\n"); - ret = aqr107_wait_reset_complete(phydev); + ret = aqr_wait_reset_complete(phydev); if (!ret) aqr107_chip_info(phydev); @@ -522,7 +522,7 @@ static int aqcs109_config_init(struct phy_device *phydev) phydev->interface != PHY_INTERFACE_MODE_2500BASEX) return -ENODEV; - ret = aqr107_wait_reset_complete(phydev); + ret = aqr_wait_reset_complete(phydev); if (!ret) aqr107_chip_info(phydev);