diff mbox series

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

Message ID 20230629191725.1434142-3-ahalaney@redhat.com
State New
Headers
Series [1/3] net: stmmac: dwmac-qcom-ethqos: Return device_get_phy_mode() errors properly |

Commit Message

Andrew Halaney June 29, 2023, 7:14 p.m. UTC 
  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(-)

Comments

Andrew Lunn June 29, 2023, 8:32 p.m. UTC | #1 
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
Andrew Halaney June 29, 2023, 9:10 p.m. UTC | #2 
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 mbox series

Patch

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