From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id C2D7EC54735 for ; Tue, 27 Aug 2024 11:00:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:MIME-Version:References:In-Reply-To:Message-ID:Subject:CC:To: From:Date:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=kjLLo7djgaUrjXym6hHQRu/ye5VOvVbRb0CSoyrw/wU=; b=FgzwpmfuCGtlAjxg4hOSaEeMwu 5l36WdUg4Qtco9UEKWhXVj3jIu5IBdkFVcJs4t6QJHLM59bYo/x5YVT2Hb8+ZMU4sokxdCcyI/OcX 9Pg3X9YrwxcuTMHtg6P7+jhI8sqoR3M0JrABZWE+KrGG9dT7uLiKAVpb/9Ine30F1oycvLP9/MXUo UJ3RN/urbEKtGAzQAWlBniHVTKQ04zFfMZyeNylPAb4SfmhjFqOYoKo+qNOq/fi2E41UzdmKJWg+L J2xlHbb8/6Ifm/tRm0tErWqLK7fP8Oq6eMUzcObJuP/pvYMWnjq/Gwnd/9m20YpRrSYo8t5U5tAqj FqdryhSQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sitw1-0000000AuhR-0EWK; Tue, 27 Aug 2024 11:00:25 +0000 Received: from frasgout.his.huawei.com ([185.176.79.56]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sittL-0000000Au89-0Npe for linux-arm-kernel@lists.infradead.org; Tue, 27 Aug 2024 10:57:40 +0000 Received: from mail.maildlp.com (unknown [172.18.186.216]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4WtPXv0Ypyz6J6rX; Tue, 27 Aug 2024 18:53:35 +0800 (CST) Received: from lhrpeml500005.china.huawei.com (unknown [7.191.163.240]) by mail.maildlp.com (Postfix) with ESMTPS id 05273140B39; Tue, 27 Aug 2024 18:57:33 +0800 (CST) Received: from localhost (10.203.177.66) by lhrpeml500005.china.huawei.com (7.191.163.240) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.39; Tue, 27 Aug 2024 11:57:32 +0100 Date: Tue, 27 Aug 2024 11:57:31 +0100 From: Jonathan Cameron To: Yangtao Li CC: , , , , , , , , , , , , , , , , , , , , , , , , , Subject: Re: [net-next v3 6/9] net: ethernet: broadcom: bcm63xx_enet: Convert to devm_clk_get_enabled() Message-ID: <20240827115731.00007469@Huawei.com> In-Reply-To: <20240827095712.2672820-7-frank.li@vivo.com> References: <20240827095712.2672820-1-frank.li@vivo.com> <20240827095712.2672820-7-frank.li@vivo.com> Organization: Huawei Technologies Research and Development (UK) Ltd. X-Mailer: Claws Mail 4.1.0 (GTK 3.24.33; x86_64-w64-mingw32) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.203.177.66] X-ClientProxiedBy: lhrpeml500006.china.huawei.com (7.191.161.198) To lhrpeml500005.china.huawei.com (7.191.163.240) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240827_035739_460008_137A7A80 X-CRM114-Status: GOOD ( 21.66 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, 27 Aug 2024 03:57:09 -0600 Yangtao Li 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 Another one where this is mixing devm and not which makes care hard to review and may introduce subtle bugs. Use devm_alloc_etherdev() and devm_register_netdev() and take all the cleanup handling managed. Much simpler to review that way. J > --- > v3: > -Reduce the number of clk variables > > drivers/net/ethernet/broadcom/bcm63xx_enet.c | 47 ++++++-------------- > drivers/net/ethernet/broadcom/bcm63xx_enet.h | 6 --- > 2 files changed, 13 insertions(+), 40 deletions(-) > > diff --git a/drivers/net/ethernet/broadcom/bcm63xx_enet.c b/drivers/net/ethernet/broadcom/bcm63xx_enet.c > index 3c0e3b9828be..dcc741837d50 100644 > --- a/drivers/net/ethernet/broadcom/bcm63xx_enet.c > +++ b/drivers/net/ethernet/broadcom/bcm63xx_enet.c > @@ -1718,6 +1718,7 @@ static int bcm_enet_probe(struct platform_device *pdev) > struct bcm63xx_enet_platform_data *pd; > int irq, irq_rx, irq_tx; > struct mii_bus *bus; > + struct clk *clk; > int i, ret; > > if (!bcm_enet_shared_base[0]) > @@ -1752,14 +1753,11 @@ static int bcm_enet_probe(struct platform_device *pdev) > priv->irq_rx = irq_rx; > priv->irq_tx = irq_tx; > > - priv->mac_clk = devm_clk_get(&pdev->dev, "enet"); > - if (IS_ERR(priv->mac_clk)) { > - ret = PTR_ERR(priv->mac_clk); > + clk = devm_clk_get_enabled(&pdev->dev, "enet"); > + if (IS_ERR(clk)) { > + ret = PTR_ERR(clk); > goto out; > } > - ret = clk_prepare_enable(priv->mac_clk); > - if (ret) > - goto out; > > /* initialize default and fetch platform data */ > priv->rx_ring_size = BCMENET_DEF_RX_DESC; > @@ -1789,15 +1787,11 @@ static int bcm_enet_probe(struct platform_device *pdev) > > if (priv->has_phy && !priv->use_external_mii) { > /* using internal PHY, enable clock */ > - priv->phy_clk = devm_clk_get(&pdev->dev, "ephy"); > - if (IS_ERR(priv->phy_clk)) { > - ret = PTR_ERR(priv->phy_clk); > - priv->phy_clk = NULL; > - goto out_disable_clk_mac; > + clk = devm_clk_get_enabled(&pdev->dev, "ephy"); > + if (IS_ERR(clk)) { > + ret = PTR_ERR(clk); > + goto out; > } > - ret = clk_prepare_enable(priv->phy_clk); > - if (ret) > - goto out_disable_clk_mac; > } > > /* do minimal hardware init to be able to probe mii bus */ > @@ -1889,10 +1883,7 @@ static int bcm_enet_probe(struct platform_device *pdev) > out_uninit_hw: > /* turn off mdc clock */ > enet_writel(priv, 0, ENET_MIISC_REG); > - clk_disable_unprepare(priv->phy_clk); > > -out_disable_clk_mac: > - clk_disable_unprepare(priv->mac_clk); > out: > free_netdev(dev); > return ret; > @@ -1927,10 +1918,6 @@ static void bcm_enet_remove(struct platform_device *pdev) > bcm_enet_mdio_write_mii); > } > > - /* disable hw block clocks */ > - clk_disable_unprepare(priv->phy_clk); > - clk_disable_unprepare(priv->mac_clk); > - > free_netdev(dev); > } > > @@ -2648,6 +2635,7 @@ static int bcm_enetsw_probe(struct platform_device *pdev) > struct bcm63xx_enetsw_platform_data *pd; > struct resource *res_mem; > int ret, irq_rx, irq_tx; > + struct clk *mac_clk; > > if (!bcm_enet_shared_base[0]) > return -EPROBE_DEFER; > @@ -2694,14 +2682,11 @@ static int bcm_enetsw_probe(struct platform_device *pdev) > goto out; > } > > - priv->mac_clk = devm_clk_get(&pdev->dev, "enetsw"); > - if (IS_ERR(priv->mac_clk)) { > - ret = PTR_ERR(priv->mac_clk); > + mac_clk = devm_clk_get_enabled(&pdev->dev, "enetsw"); > + if (IS_ERR(mac_clk)) { > + ret = PTR_ERR(mac_clk); > goto out; > } > - ret = clk_prepare_enable(priv->mac_clk); > - if (ret) > - goto out; > > priv->rx_chan = 0; > priv->tx_chan = 1; > @@ -2720,7 +2705,7 @@ static int bcm_enetsw_probe(struct platform_device *pdev) > > ret = register_netdev(dev); > if (ret) > - goto out_disable_clk; > + goto out; > > netif_carrier_off(dev); > platform_set_drvdata(pdev, dev); > @@ -2729,8 +2714,6 @@ static int bcm_enetsw_probe(struct platform_device *pdev) > > return 0; > > -out_disable_clk: > - clk_disable_unprepare(priv->mac_clk); > out: > free_netdev(dev); > return ret; > @@ -2740,16 +2723,12 @@ static int bcm_enetsw_probe(struct platform_device *pdev) > /* exit func, stops hardware and unregisters netdevice */ > static void bcm_enetsw_remove(struct platform_device *pdev) > { > - struct bcm_enet_priv *priv; > struct net_device *dev; > > /* stop netdevice */ > dev = platform_get_drvdata(pdev); > - priv = netdev_priv(dev); > unregister_netdev(dev); > > - clk_disable_unprepare(priv->mac_clk); > - > free_netdev(dev); > } > > diff --git a/drivers/net/ethernet/broadcom/bcm63xx_enet.h b/drivers/net/ethernet/broadcom/bcm63xx_enet.h > index 78f1830fb3cb..e98838b8b92f 100644 > --- a/drivers/net/ethernet/broadcom/bcm63xx_enet.h > +++ b/drivers/net/ethernet/broadcom/bcm63xx_enet.h > @@ -316,12 +316,6 @@ struct bcm_enet_priv { > /* lock mib update between userspace request and workqueue */ > struct mutex mib_update_lock; > > - /* mac clock */ > - struct clk *mac_clk; > - > - /* phy clock if internal phy is used */ > - struct clk *phy_clk; > - > /* network device reference */ > struct net_device *net_dev; >