| Message ID | 20230629191725.1434142-3-ahalaney@redhat.com (mailing list archive) |
|---|---|
| State | Deferred |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [1/3] net: stmmac: dwmac-qcom-ethqos: Return device_get_phy_mode() errors properly | expand |
On Thu, Jun 29, 2023 at 02:14:18PM -0500, Andrew Halaney wrote:
> These are useful to see when debugging a probe failure.
Since this is used for debugging, maybe netdev_dbg(). Anybody actually
doing debugging should be able to turn that on.
Andrew
On Thu, Jun 29, 2023 at 10:32:24PM +0200, Andrew Lunn wrote: > On Thu, Jun 29, 2023 at 02:14:18PM -0500, Andrew Halaney wrote: > > These are useful to see when debugging a probe failure. > > Since this is used for debugging, maybe netdev_dbg(). Anybody actually > doing debugging should be able to turn that on. > In my opinion it is better to use dev_err_probe() as done here because: 1. If it's -EPROBE_DEFER it will be under debug level already 2. If it's anything else, its an error and the logs are useful I've ran into both ends of this now (failure of a platform dependency to load, be it a bug in the driver, or just failing to select said driver), and I've seen issues where new integrators (say you're bringing up a new board) leave something out, etc, and run into issues because of that. Thanks, Andrew
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c index a40869b2dd64..d792b7dd9fc3 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c @@ -706,7 +706,8 @@ static int qcom_ethqos_probe(struct platform_device *pdev) ret = stmmac_get_platform_resources(pdev, &stmmac_res); if (ret) - return ret; + return dev_err_probe(dev, ret, + "Failed to get platform resources\n"); plat_dat = devm_stmmac_probe_config_dt(pdev, stmmac_res.mac); if (IS_ERR(plat_dat)) { @@ -735,13 +736,16 @@ static int qcom_ethqos_probe(struct platform_device *pdev) ethqos->configure_func = ethqos_configure_sgmii; break; default: + dev_err(dev, "Unsupported phy mode %s\n", + phy_modes(ethqos->phy_mode)); return -EINVAL; } ethqos->pdev = pdev; ethqos->rgmii_base = devm_platform_ioremap_resource_byname(pdev, "rgmii"); if (IS_ERR(ethqos->rgmii_base)) - return PTR_ERR(ethqos->rgmii_base); + return dev_err_probe(dev, PTR_ERR(ethqos->rgmii_base), + "Failed to map rgmii resource\n"); ethqos->mac_base = stmmac_res.addr; @@ -753,7 +757,8 @@ static int qcom_ethqos_probe(struct platform_device *pdev) ethqos->link_clk = devm_clk_get(dev, data->link_clk_name ?: "rgmii"); if (IS_ERR(ethqos->link_clk)) - return PTR_ERR(ethqos->link_clk); + return dev_err_probe(dev, PTR_ERR(ethqos->link_clk), + "Failed to get link_clk\n"); ret = ethqos_clks_config(ethqos, true); if (ret) @@ -765,7 +770,8 @@ static int qcom_ethqos_probe(struct platform_device *pdev) ethqos->serdes_phy = devm_phy_optional_get(dev, "serdes"); if (IS_ERR(ethqos->serdes_phy)) - return PTR_ERR(ethqos->serdes_phy); + return dev_err_probe(dev, PTR_ERR(ethqos->serdes_phy), + "Failed to get serdes phy\n"); ethqos->speed = SPEED_1000; ethqos_update_link_clk(ethqos, SPEED_1000);
These are useful to see when debugging a probe failure. Signed-off-by: Andrew Halaney <ahalaney@redhat.com> --- .../ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)