diff mbox series

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

Message ID 14167d6b025e18d2a06ee429feea6551ef6f9c44.1724225528.git.dsimic@manjaro.org (mailing list archive)
State New, archived
Headers show
Series Improve error handling in Rockchip Inno USB 2.0 PHY driver | expand

Commit Message

Dragan Simic Aug. 21, 2024, 7:37 a.m. UTC 
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.

Signed-off-by: Dragan Simic <dsimic@manjaro.org>
---
 drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 27 +++++++------------
 1 file changed, 9 insertions(+), 18 deletions(-)

Comments

Heiko Stübner Aug. 21, 2024, 8:39 a.m. UTC | #1 
Am Mittwoch, 21. August 2024, 09:37:53 CEST schrieb Dragan Simic:
> 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.
> 
> Signed-off-by: Dragan Simic <dsimic@manjaro.org>

> @@ -445,7 +445,6 @@ static int rockchip_usb2phy_extcon_register(struct rockchip_usb2phy *rphy)
>  	}
>  
>  	rphy->edev = edev;
> -
>  	return 0;
>  }

depending on how nitpicky we want to be, I'm not very fond of this change.
Assigning the extcon-dev and returning "0" are two different actions, and
I think most drivers do actually use a blank line between those.

But that really is just a style preference for me, so anyway

Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Dragan Simic Aug. 21, 2024, 8:53 a.m. UTC | #2 
Hello Heiko,

On 2024-08-21 10:39, Heiko Stübner wrote:
> Am Mittwoch, 21. August 2024, 09:37:53 CEST schrieb Dragan Simic:
>> 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.
>> 
>> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
> 
>> @@ -445,7 +445,6 @@ static int rockchip_usb2phy_extcon_register(struct 
>> rockchip_usb2phy *rphy)
>>  	}
>> 
>>  	rphy->edev = edev;
>> -
>>  	return 0;
>>  }
> 
> depending on how nitpicky we want to be, I'm not very fond of this 
> change.
> Assigning the extcon-dev and returning "0" are two different actions, 
> and
> I think most drivers do actually use a blank line between those.
> 
> But that really is just a style preference for me, so anyway
> 
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>

Thanks for your review; I had similar thoughts, believe it or not. :)  
If
there will be v3, I'll bring the empty separator line back.
diff mbox series

Patch

diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
index 4f71373ae6e1..113bfc717ff0 100644
--- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
+++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
@@ -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);
@@ -445,7 +445,6 @@  static int rockchip_usb2phy_extcon_register(struct rockchip_usb2phy *rphy)
 	}
 
 	rphy->edev = edev;
-
 	return 0;
 }
 
@@ -1327,21 +1326,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 +1355,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 +1381,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 +1401,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 +1439,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;
 		}