2023-06-29 19:24:55

by Andrew Halaney

Subject: [PATCH 3/3] net: stmmac: dwmac-qcom-ethqos: Log more errors in probe

These are useful to see when debugging a probe failure.

Signed-off-by: Andrew Halaney <[email protected]>
---
.../ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)

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);
--
2.41.0

2023-06-29 20:52:31

by Andrew Lunn

Subject: Re: [PATCH 3/3] net: stmmac: dwmac-qcom-ethqos: Log more errors in probe

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

2023-06-29 21:33:48

by Andrew Halaney

Subject: Re: [PATCH 3/3] net: stmmac: dwmac-qcom-ethqos: Log more errors in probe

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