Improve error handling in Rockchip Inno USB 2.0 PHY driver

PATCH Applied Dragan Simic 3 participants 7 messages Sep 5, 2024 – Oct 7, 2024

Subsystems (2)

Arm Rockchip Generic PHY framework

lore

Share & tools

lore

Dragan Simic

author

2024-09-05 08:29 UTC #0 lore raw

v3 cover Improve error handling in Rockchip Inno USB 2.0 PHY driver

This is a small series that improves error handling in the probe path of the Rockchip Innosilicon USB 2.0 PHY driver, by returning the actual error code in one place and by using dev_err_probe() properly in multiple places. It also performs a bunch of small, rather trivial code cleanups, to make the code neater and a bit easier to read.

Changes in v3:
- Collected Reviewed-by tags from Heiko for patches 1/3 and 2/3
- Brought back one empty line as a separator of dissimilar operations in patch 1/3, as suggested by Heiko [2]
- Dropped one backward conversion of dev_err_probe() to dev_err() in patch 3/3, as suggested by Heiko, [3] and adjusted the summary and description of patch 3/3 accordingly

Changes in v2:
- Expanded into a small series, after a suggestion from Heiko [1] to use dev_err_probe(), because it all happens in the probe path

Link to v2: https://lore.kernel.org/linux-phy/[email protected]/T/#u
Link to v1: https://lore.kernel.org/linux-phy/5fa7796d71e2f46344e972bc98a54539f55b6109.1723551599.git.dsimic@manjaro.org/T/#u

[1] https://lore.kernel.org/linux-phy/4927264.xgNZFEDtJV@diego/
[2] https://lore.kernel.org/linux-phy/5307900.6fTUFtlzNn@diego/
[3] https://lore.kernel.org/linux-phy/6073817.31tnzDBltd@diego/

Dragan Simic (3):
phy: phy-rockchip-inno-usb2: Perform trivial code cleanups
phy: phy-rockchip-inno-usb2: Handle failed extcon allocation better
phy: phy-rockchip-inno-usb2: Use dev_err_probe() in the probe path

 drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 56 +++++++------------
 1 file changed, 21 insertions(+), 35 deletions(-)

Dragan Simic

author

2024-09-05 08:30 UTC #1 lore raw

PATCH v3 1/3 phy: phy-rockchip-inno-usb2: Perform trivial code cleanups

Perform a few trivial code cleanups, e.g. to obey the reverse Christmas tree rule, to avoid use of "{ ... }" code blocks where they aren't really needed, or to avoid line wrapping by using the 100-column width better.

No intended functional changes are introduced by these code cleanups.

Reviewed-by Heiko Stuebner <[email protected]>

Signed-off-by Dragan Simic <[email protected]>

---
 drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 26 +++++++------------
 1 file changed, 9 insertions(+), 17 deletions(-)

drivers/phy/rockchip/phy-rockchip-inno-usb2.c +9 ~~-17~~

index 4f71373ae6e1..03b772200fed 100644
@@ -418,9 +418,9 @@ rockchip_usb2phy_clk480m_register(struct rockchip_usb2phy *rphy)
 
 static int rockchip_usb2phy_extcon_register(struct rockchip_usb2phy *rphy)
 {
-	int ret;
 	struct device_node *node = rphy->dev->of_node;
 	struct extcon_dev *edev;
+	int ret;
 
 	if (of_property_read_bool(node, "extcon")) {
 		edev = extcon_get_edev_by_phandle(rphy->dev, 0);

@@ -1327,21 +1327,19 @@ static int rockchip_usb2phy_probe(struct platform_device *pdev)
 	struct rockchip_usb2phy *rphy;
 	const struct rockchip_usb2phy_cfg *phy_cfgs;
 	unsigned int reg;
-	int index, ret;
+	int index = 0, ret;
 
 	rphy = devm_kzalloc(dev, sizeof(*rphy), GFP_KERNEL);
 	if (!rphy)
 		return -ENOMEM;
 
 	if (!dev->parent || !dev->parent->of_node) {
 		rphy->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,usbgrf");
 		if (IS_ERR(rphy->grf)) {
 			dev_err(dev, "failed to locate usbgrf\n");
 			return PTR_ERR(rphy->grf);
 		}
-	}
-
-	else {
+	} else {
 		rphy->grf = syscon_node_to_regmap(dev->parent->of_node);
 		if (IS_ERR(rphy->grf))
 			return PTR_ERR(rphy->grf);

@@ -1358,16 +1356,14 @@ static int rockchip_usb2phy_probe(struct platform_device *pdev)
 	}
 
 	if (of_property_read_u32_index(np, "reg", 0, &reg)) {
-		dev_err(dev, "the reg property is not assigned in %pOFn node\n",
-			np);
+		dev_err(dev, "the reg property is not assigned in %pOFn node\n", np);
 		return -EINVAL;
 	}
 
 	/* support address_cells=2 */
 	if (of_property_count_u32_elems(np, "reg") > 2 && reg == 0) {
 		if (of_property_read_u32_index(np, "reg", 1, &reg)) {
-			dev_err(dev, "the reg property is not assigned in %pOFn node\n",
-				np);
+			dev_err(dev, "the reg property is not assigned in %pOFn node\n", np);
 			return -EINVAL;
 		}
 	}

@@ -1386,8 +1382,7 @@ static int rockchip_usb2phy_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	/* find out a proper config which can be matched with dt. */
-	index = 0;
+	/* find a proper config that can be matched with the DT */
 	do {
 		if (phy_cfgs[index].reg == reg) {
 			rphy->phy_cfg = &phy_cfgs[index];

@@ -1407,10 +1402,9 @@ static int rockchip_usb2phy_probe(struct platform_device *pdev)
 		return PTR_ERR(rphy->phy_reset);
 
 	rphy->clk = devm_clk_get_optional_enabled(dev, "phyclk");
-	if (IS_ERR(rphy->clk)) {
+	if (IS_ERR(rphy->clk))
 		return dev_err_probe(&pdev->dev, PTR_ERR(rphy->clk),
 				     "failed to get phyclk\n");
-	}
 
 	ret = rockchip_usb2phy_clk480m_register(rphy);
 	if (ret) {

@@ -1446,13 +1440,11 @@ static int rockchip_usb2phy_probe(struct platform_device *pdev)
 
 		/* initialize otg/host port separately */
 		if (of_node_name_eq(child_np, "host-port")) {
-			ret = rockchip_usb2phy_host_port_init(rphy, rport,
-							      child_np);
+			ret = rockchip_usb2phy_host_port_init(rphy, rport, child_np);
 			if (ret)
 				goto put_child;
 		} else {
-			ret = rockchip_usb2phy_otg_port_init(rphy, rport,
-							     child_np);
+			ret = rockchip_usb2phy_otg_port_init(rphy, rport, child_np);
 			if (ret)
 				goto put_child;
 		}

Signature · 3 lines

linux-phy mailing list
[email protected]
https://lists.infradead.org/mailman/listinfo/linux-phy

Dragan Simic

author

2024-09-05 08:31 UTC #2 lore raw

PATCH v3 2/3 phy: phy-rockchip-inno-usb2: Handle failed extcon allocation better

In reply to Dragan Simic

Return the actual error code upon failure to allocate extcon device, instead of hardcoding -ENOMEM. Use dev_err_probe() to also log appropriate messages, which is fine because the containing function is used in the probe path.

Helped-by Heiko Stubner <[email protected]>

Reviewed-by Heiko Stuebner <[email protected]>

Signed-off-by Dragan Simic <[email protected]>

---
 drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/phy/rockchip/phy-rockchip-inno-usb2.c +2 -1

index 03b772200fed..cf6b67dd9f10 100644
@@ -435,7 +435,8 @@ static int rockchip_usb2phy_extcon_register(struct rockchip_usb2phy *rphy)
 						rockchip_usb2phy_extcon_cable);
 
 		if (IS_ERR(edev))
-			return -ENOMEM;
+			return dev_err_probe(rphy->dev, PTR_ERR(edev),
+					     "failed to allocate extcon device\n");
 
 		ret = devm_extcon_dev_register(rphy->dev, edev);
 		if (ret) {

Signature · 3 lines

linux-phy mailing list
[email protected]
https://lists.infradead.org/mailman/listinfo/linux-phy

Dragan Simic

author

2024-09-05 08:32 UTC #3 lore raw

PATCH v3 3/3 phy: phy-rockchip-inno-usb2: Use dev_err_probe() in the probe path

In reply to Dragan Simic

Improve error handling in the probe path by using function dev_err_probe() instead of function dev_err(), where appropriate.

Signed-off-by Dragan Simic <[email protected]>

---
 drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 27 +++++++------------
 1 file changed, 10 insertions(+), 17 deletions(-)

drivers/phy/rockchip/phy-rockchip-inno-usb2.c +10 ~~-17~~

index cf6b67dd9f10..6e5214862b8a 100644
@@ -424,25 +424,22 @@ static int rockchip_usb2phy_extcon_register(struct rockchip_usb2phy *rphy)
 
 	if (of_property_read_bool(node, "extcon")) {
 		edev = extcon_get_edev_by_phandle(rphy->dev, 0);
-		if (IS_ERR(edev)) {
-			if (PTR_ERR(edev) != -EPROBE_DEFER)
-				dev_err(rphy->dev, "Invalid or missing extcon\n");
-			return PTR_ERR(edev);
-		}
+		if (IS_ERR(edev))
+			return dev_err_probe(rphy->dev, PTR_ERR(edev),
+					     "invalid or missing extcon\n");
 	} else {
 		/* Initialize extcon device */
 		edev = devm_extcon_dev_allocate(rphy->dev,
 						rockchip_usb2phy_extcon_cable);
 
 		if (IS_ERR(edev))
 			return dev_err_probe(rphy->dev, PTR_ERR(edev),
 					     "failed to allocate extcon device\n");
 
 		ret = devm_extcon_dev_register(rphy->dev, edev);
-		if (ret) {
-			dev_err(rphy->dev, "failed to register extcon device\n");
-			return ret;
-		}
+		if (ret)
+			return dev_err_probe(rphy->dev, ret,
+					     "failed to register extcon device\n");
 	}
 
 	rphy->edev = edev;

@@ -1408,10 +1405,8 @@ static int rockchip_usb2phy_probe(struct platform_device *pdev)
 				     "failed to get phyclk\n");
 
 	ret = rockchip_usb2phy_clk480m_register(rphy);
-	if (ret) {
-		dev_err(dev, "failed to register 480m output clock\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to register 480m output clock\n");
 
 	if (rphy->phy_cfg->phy_tuning) {
 		ret = rphy->phy_cfg->phy_tuning(rphy);

@@ -1431,8 +1426,7 @@ static int rockchip_usb2phy_probe(struct platform_device *pdev)
 
 		phy = devm_phy_create(dev, child_np, &rockchip_usb2phy_ops);
 		if (IS_ERR(phy)) {
-			dev_err_probe(dev, PTR_ERR(phy), "failed to create phy\n");
-			ret = PTR_ERR(phy);
+			ret = dev_err_probe(dev, PTR_ERR(phy), "failed to create phy\n");
 			goto put_child;
 		}

@@ -1467,8 +1461,7 @@ static int rockchip_usb2phy_probe(struct platform_device *pdev)
 						"rockchip_usb2phy",
 						rphy);
 		if (ret) {
-			dev_err(rphy->dev,
-				"failed to request usb2phy irq handle\n");
+			dev_err_probe(rphy->dev, ret, "failed to request usb2phy irq handle\n");
 			goto put_child;
 		}
 	}

Signature · 3 lines

linux-phy mailing list
[email protected]
https://lists.infradead.org/mailman/listinfo/linux-phy

Heiko Stübner

maintainer Reviewed-by on v3 3/3

2024-09-05 09:14 UTC #4 lore raw

Re: [PATCH v3 3/3] phy: phy-rockchip-inno-usb2: Use dev_err_probe() in the probe path

Am Donnerstag, 5. September 2024, 10:28:24 CEST schrieb Dragan Simic:

Quoted 4 lines

Improve error handling in the probe path by using function dev_err_probe() instead of function dev_err(), where appropriate.
Signed-off-by Dragan Simic <[email protected]>

Reviewed-by Heiko Stuebner <[email protected]>

Dragan Simic

author

2024-10-04 06:46 UTC #5 lore raw

In reply to Dragan Simic

Hello,

Quoted 5 lines · Dragan Simic

This is a small series that improves error handling in the probe path of the Rockchip Innosilicon USB 2.0 PHY driver, by returning the actual error code in one place and by using dev_err_probe() properly in multiple places. It also performs a bunch of small, rather trivial code cleanups, to make the code neater and a bit easier to read.

Just a brief reminder about this patch series... Please let me know if something more is needed for these patches to be merged.

Quoted 28 lines

Changes in v3:
- Collected Reviewed-by tags from Heiko for patches 1/3 and 2/3
- Brought back one empty line as a separator of dissimilar operations in patch 1/3, as suggested by Heiko [2]
- Dropped one backward conversion of dev_err_probe() to dev_err() in patch 3/3, as suggested by Heiko, [3] and adjusted the summary and description of patch 3/3 accordingly
Changes in v2:
- Expanded into a small series, after a suggestion from Heiko [1] to use dev_err_probe(), because it all happens in the probe path
Link to v2:
https://lore.kernel.org/linux-phy/[email protected]/T/#u
Link to v1:
https://lore.kernel.org/linux-phy/5fa7796d71e2f46344e972bc98a54539f55b6109.1723551599.git.dsimic@manjaro.org/T/#u
[1] https://lore.kernel.org/linux-phy/4927264.xgNZFEDtJV@diego/
[2] https://lore.kernel.org/linux-phy/5307900.6fTUFtlzNn@diego/
[3] https://lore.kernel.org/linux-phy/6073817.31tnzDBltd@diego/
Dragan Simic (3):
phy: phy-rockchip-inno-usb2: Perform trivial code cleanups
phy: phy-rockchip-inno-usb2: Handle failed extcon allocation better
phy: phy-rockchip-inno-usb2: Use dev_err_probe() in the probe path
 drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 56 +++++++------------
 1 file changed, 21 insertions(+), 35 deletions(-)

Vinod Koul

maintainer Applied

2024-10-07 15:48 UTC #6 lore raw

In reply to Dragan Simic

Quoted 14 lines · Dragan Simic

This is a small series that improves error handling in the probe path of the Rockchip Innosilicon USB 2.0 PHY driver, by returning the actual error code in one place and by using dev_err_probe() properly in multiple places. It also performs a bunch of small, rather trivial code cleanups, to make the code neater and a bit easier to read.
Changes in v3:
- Collected Reviewed-by tags from Heiko for patches 1/3 and 2/3
- Brought back one empty line as a separator of dissimilar operations in patch 1/3, as suggested by Heiko [2]
- Dropped one backward conversion of dev_err_probe() to dev_err() in patch 3/3, as suggested by Heiko, [3] and adjusted the summary and description of patch 3/3 accordingly

Quoted 1 line

[...]

Applied, thanks!

[1/3] phy: phy-rockchip-inno-usb2: Perform trivial code cleanups

      commit: 449d2a523a0c3ca8f4c993b8e37795c2aa174da6
[2/3] phy: phy-rockchip-inno-usb2: Handle failed extcon allocation better
      commit: 595ad7a336bf21f9d111a033820cd95d70343bd1
[3/3] phy: phy-rockchip-inno-usb2: Use dev_err_probe() in the probe path
      commit: 40452520850683f6771094ca218ff206d1fcb022

Best regards,

Signature · 1 line

~Vinod