diff mbox series

[net-next,v3,4/9] net: mdio: hisi-femac: Convert to devm_clk_get_enabled()

Message ID 20240827095712.2672820-5-frank.li@vivo.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series net: convert to devm_clk_get_enabled() and devm_clk_get_optional_enabled() | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 16 this patch: 16
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 16 this patch: 16
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 16 this patch: 16
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 54 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-08-27--18-00 (tests: 714)

Commit Message

Yangtao Li Aug. 27, 2024, 9:57 a.m. UTC
Convert devm_clk_get(), clk_prepare_enable() to a single
call to devm_clk_get_enabled(), as this is exactly
what this function does.

Signed-off-by: Yangtao Li <frank.li@vivo.com>
---
 drivers/net/mdio/mdio-hisi-femac.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

Comments

Jonathan Cameron Aug. 27, 2024, 10:50 a.m. UTC | #1
On Tue, 27 Aug 2024 03:57:07 -0600
Yangtao Li <frank.li@vivo.com> wrote:

> Convert devm_clk_get(), clk_prepare_enable() to a single
> call to devm_clk_get_enabled(), as this is exactly
> what this function does.
> 
> Signed-off-by: Yangtao Li <frank.li@vivo.com>

Mixing an matching devm and otherwise can lead to subtle bugs
and generally makes the code harder to review

I'd also use devm_mdiobus_alloc_size() and devm_mdiobus_register()
and drop the remove() callback entirely.

I would not take this patch on its own as it causes a reordering
of cleanup we probably don't want.

As a general rule, single action devm cleanup series (so only using
one new type) fall into this trap. Much better to look at all the
improvements in each driver that are enabled by new infrastructure
rather than doing a single type change series like this one.

Thanks,

Jonathan


> ---
>  drivers/net/mdio/mdio-hisi-femac.c | 18 +++++-------------
>  1 file changed, 5 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/mdio/mdio-hisi-femac.c b/drivers/net/mdio/mdio-hisi-femac.c
> index 6703f626ee83..f6fcb9e11624 100644
> --- a/drivers/net/mdio/mdio-hisi-femac.c
> +++ b/drivers/net/mdio/mdio-hisi-femac.c
> @@ -21,7 +21,6 @@
>  #define BIT_WR_DATA_OFFSET	16
>  
>  struct hisi_femac_mdio_data {
> -	struct clk *clk;
>  	void __iomem *membase;
>  };
>  
> @@ -74,6 +73,7 @@ static int hisi_femac_mdio_probe(struct platform_device *pdev)
>  	struct device_node *np = pdev->dev.of_node;
>  	struct mii_bus *bus;
>  	struct hisi_femac_mdio_data *data;
> +	struct clk *clk;
>  	int ret;
>  
>  	bus = mdiobus_alloc_size(sizeof(*data));
> @@ -93,26 +93,20 @@ static int hisi_femac_mdio_probe(struct platform_device *pdev)
>  		goto err_out_free_mdiobus;
>  	}
>  
> -	data->clk = devm_clk_get(&pdev->dev, NULL);
> -	if (IS_ERR(data->clk)) {
> -		ret = PTR_ERR(data->clk);
> +	clk = devm_clk_get_prepared(&pdev->dev, NULL);
> +	if (IS_ERR(clk)) {
> +		ret = PTR_ERR(clk);
>  		goto err_out_free_mdiobus;
>  	}
>  
> -	ret = clk_prepare_enable(data->clk);
> -	if (ret)
> -		goto err_out_free_mdiobus;
> -
>  	ret = of_mdiobus_register(bus, np);
>  	if (ret)
> -		goto err_out_disable_clk;
> +		goto err_out_free_mdiobus;
>  
>  	platform_set_drvdata(pdev, bus);
>  
>  	return 0;
>  
> -err_out_disable_clk:
> -	clk_disable_unprepare(data->clk);
>  err_out_free_mdiobus:
>  	mdiobus_free(bus);
>  	return ret;
> @@ -121,10 +115,8 @@ static int hisi_femac_mdio_probe(struct platform_device *pdev)
>  static void hisi_femac_mdio_remove(struct platform_device *pdev)
>  {
>  	struct mii_bus *bus = platform_get_drvdata(pdev);
> -	struct hisi_femac_mdio_data *data = bus->priv;
>  
>  	mdiobus_unregister(bus);
> -	clk_disable_unprepare(data->clk);
>  	mdiobus_free(bus);
>  }
>
diff mbox series

Patch

diff --git a/drivers/net/mdio/mdio-hisi-femac.c b/drivers/net/mdio/mdio-hisi-femac.c
index 6703f626ee83..f6fcb9e11624 100644
--- a/drivers/net/mdio/mdio-hisi-femac.c
+++ b/drivers/net/mdio/mdio-hisi-femac.c
@@ -21,7 +21,6 @@ 
 #define BIT_WR_DATA_OFFSET	16
 
 struct hisi_femac_mdio_data {
-	struct clk *clk;
 	void __iomem *membase;
 };
 
@@ -74,6 +73,7 @@  static int hisi_femac_mdio_probe(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	struct mii_bus *bus;
 	struct hisi_femac_mdio_data *data;
+	struct clk *clk;
 	int ret;
 
 	bus = mdiobus_alloc_size(sizeof(*data));
@@ -93,26 +93,20 @@  static int hisi_femac_mdio_probe(struct platform_device *pdev)
 		goto err_out_free_mdiobus;
 	}
 
-	data->clk = devm_clk_get(&pdev->dev, NULL);
-	if (IS_ERR(data->clk)) {
-		ret = PTR_ERR(data->clk);
+	clk = devm_clk_get_prepared(&pdev->dev, NULL);
+	if (IS_ERR(clk)) {
+		ret = PTR_ERR(clk);
 		goto err_out_free_mdiobus;
 	}
 
-	ret = clk_prepare_enable(data->clk);
-	if (ret)
-		goto err_out_free_mdiobus;
-
 	ret = of_mdiobus_register(bus, np);
 	if (ret)
-		goto err_out_disable_clk;
+		goto err_out_free_mdiobus;
 
 	platform_set_drvdata(pdev, bus);
 
 	return 0;
 
-err_out_disable_clk:
-	clk_disable_unprepare(data->clk);
 err_out_free_mdiobus:
 	mdiobus_free(bus);
 	return ret;
@@ -121,10 +115,8 @@  static int hisi_femac_mdio_probe(struct platform_device *pdev)
 static void hisi_femac_mdio_remove(struct platform_device *pdev)
 {
 	struct mii_bus *bus = platform_get_drvdata(pdev);
-	struct hisi_femac_mdio_data *data = bus->priv;
 
 	mdiobus_unregister(bus);
-	clk_disable_unprepare(data->clk);
 	mdiobus_free(bus);
 }