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 2797DCA0EFF for ; Wed, 27 Aug 2025 08:36:09 +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:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=KGZ9fOwje4MlqtMCuczbLxb/Olo2qc1yHQaGkujfbWg=; b=eNf+JFJWxTQwF0o9scRzGGQZlf rTTdQlKEx10e/pB+ObJMvn8n9r99gEKqMiE+AMm+NBlYp4/X/q8Ma3Xmss3zfoW9FtHmZ3IxElbNZ 8slv2giMgHttS2Mv+OdbVNH3gNkMOCueXgOnxYkcKe9vyxWjVsWbuWO0A+NAtpCAvA08T2eufkDc4 LXGyt1v2ZqWpJ0XCF2jdiIPohN36lPxWpe6/F7rjs8olZ6WG0OrRjKnU+0t1QEqfAC9rPUQK1D2Iy +o8OJaKgcKDySzrdHhZnCNYzbRD0aobS67KsFWZpFX66VTZyAaFHWvF/V7Zi3LkKbHO7scOXHyCla vc6B4/cw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1urBdU-0000000Egu6-2AQk; Wed, 27 Aug 2025 08:36:04 +0000 Received: from pandora.armlinux.org.uk ([2001:4d48:ad52:32c8:5054:ff:fe00:142]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1urBT3-0000000EehK-2C4V for linux-arm-kernel@lists.infradead.org; Wed, 27 Aug 2025 08:25:19 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=armlinux.org.uk; s=pandora-2019; h=Sender:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=KGZ9fOwje4MlqtMCuczbLxb/Olo2qc1yHQaGkujfbWg=; b=0PEvmLKdfidIG6CQlBbJ38O/N6 35bcf3xx776W1e4TOIvZSQ1ph3SkNL4KmOvzUsxl3WOTm3FPJhEC21oe/elTgB+K1ar2o40rqS3pb l4uc1hMPr+dT7TlvbY9TexsZXSRxN61QOcriD1wl2ckYcv6ae92U0oerZimP3HdoHPNEAV9B2lYVe eQtW9kz80lNScz26Qdhn8ssO3pmqStU/7nq3IRR07KV1+b16otoYYKktKYPjPzl0upNPIFAqO3uq5 Je5W8yMySwD1lERtOW6Kq+GI/Ajrl7ievIhVgEZSBDj2CJ5DeWVic3Dv/YtsXkNJFupr4IGppXXYa StGQS9dA==; Received: from shell.armlinux.org.uk ([fd8f:7570:feb6:1:5054:ff:fe00:4ec]:33890) by pandora.armlinux.org.uk with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.98.2) (envelope-from ) id 1urBSi-000000000Cj-3ECE; Wed, 27 Aug 2025 09:24:56 +0100 Received: from linux by shell.armlinux.org.uk with local (Exim 4.98.2) (envelope-from ) id 1urBSd-0000000022P-2QG2; Wed, 27 Aug 2025 09:24:51 +0100 Date: Wed, 27 Aug 2025 09:24:51 +0100 From: "Russell King (Oracle)" To: weishangjuan@eswincomputing.com Cc: devicetree@vger.kernel.org, andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org, linux-arm-kernel@lists.infradead.org, mcoquelin.stm32@gmail.com, alexandre.torgue@foss.st.com, yong.liang.choong@linux.intel.com, vladimir.oltean@nxp.com, faizal.abdul.rahim@linux.intel.com, prabhakar.mahadev-lad.rj@bp.renesas.com, inochiama@gmail.com, jan.petrous@oss.nxp.com, jszhang@kernel.org, p.zabel@pengutronix.de, boon.khai.ng@altera.com, 0x1207@gmail.com, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, ningyu@eswincomputing.com, linmin@eswincomputing.com, lizhi2@eswincomputing.com Subject: Re: [PATCH v4 2/2] ethernet: eswin: Add eic7700 ethernet driver Message-ID: References: <20250827081135.2243-1-weishangjuan@eswincomputing.com> <20250827081418.2347-1-weishangjuan@eswincomputing.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250827081418.2347-1-weishangjuan@eswincomputing.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250827_012517_718885_432F01EA X-CRM114-Status: GOOD ( 22.76 ) 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 Wed, Aug 27, 2025 at 04:14:17PM +0800, weishangjuan@eswincomputing.com wrote: > +struct eic7700_qos_priv { > + struct device *dev; > + struct regmap *hsp_regmap; > + struct clk *clk_tx; Consider putting a pointer to the plat_dat here instead of clk_tx. > + struct clk *clk_axi; > + struct clk *clk_cfg; Consider moving these into plat_dat->clks. > + u32 tx_delay_ps; > + u32 rx_delay_ps; > +}; > + > +/** > + * eic7700_apply_delay - Update TX or RX delay bits in delay parameter value. > + * @delay_ps: Delay in picoseconds (capped at 12.7ns). > + * @reg: Pointer to register value to modify. > + * @is_rx: True for RX delay (bits 30:24), false for TX delay (bits 14:8). > + * > + * Converts delay to 0.1ns units, caps at 0x7F, and sets appropriate bits. > + * Only RX or TX bits are updated; other bits remain unchanged. > + */ > +static inline void eic7700_apply_delay(u32 delay_ps, u32 *reg, bool is_rx) > +{ > + if (!reg) > + return; > + > + u32 val = min(delay_ps / 100, EIC7700_MAX_DELAY_UNIT); > + > + if (is_rx) { > + *reg &= ~EIC7700_ETH_RX_ADJ_DELAY; > + *reg |= (val << 24) & EIC7700_ETH_RX_ADJ_DELAY; > + } else { > + *reg &= ~EIC7700_ETH_TX_ADJ_DELAY; > + *reg |= (val << 8) & EIC7700_ETH_TX_ADJ_DELAY; > + } > +} > + > +static int eic7700_clks_config(void *priv, bool enabled) > +{ > + struct eic7700_qos_priv *dwc = (struct eic7700_qos_priv *)priv; > + int ret = 0; > + > + if (enabled) { > + ret = clk_prepare_enable(dwc->clk_tx); > + if (ret < 0) { > + dev_err(dwc->dev, "Failed to enable tx clock: %d\n", > + ret); > + goto err; > + } > + > + ret = clk_prepare_enable(dwc->clk_axi); > + if (ret < 0) { > + dev_err(dwc->dev, "Failed to enable axi clock: %d\n", > + ret); > + goto err_tx; > + } > + > + ret = clk_prepare_enable(dwc->clk_cfg); > + if (ret < 0) { > + dev_err(dwc->dev, "Failed to enable cfg clock: %d\n", > + ret); > + goto err_axi; > + } You can then use clk_bulk_prepare_enable() here without the complex unwinding if one enable fails. > + } else { > + clk_disable_unprepare(dwc->clk_tx); > + clk_disable_unprepare(dwc->clk_axi); > + clk_disable_unprepare(dwc->clk_cfg); and clk_bulk_disable_unprepare() here. > + } > + return ret; > + > +err_axi: > + clk_disable_unprepare(dwc->clk_axi); > +err_tx: > + clk_disable_unprepare(dwc->clk_tx); > +err: > + return ret; > +} > + > +static int eic7700_dwmac_probe(struct platform_device *pdev) > +{ > + struct plat_stmmacenet_data *plat_dat; > + struct stmmac_resources stmmac_res; > + struct eic7700_qos_priv *dwc_priv; > + u32 eth_axi_lp_ctrl_offset; > + u32 eth_phy_ctrl_offset; > + u32 eth_phy_ctrl_regset; > + u32 eth_rxd_dly_offset; > + u32 eth_dly_param = 0; > + int ret; > + > + ret = stmmac_get_platform_resources(pdev, &stmmac_res); > + if (ret) > + return dev_err_probe(&pdev->dev, ret, > + "failed to get resources\n"); > + > + plat_dat = devm_stmmac_probe_config_dt(pdev, stmmac_res.mac); > + if (IS_ERR(plat_dat)) > + return dev_err_probe(&pdev->dev, PTR_ERR(plat_dat), > + "dt configuration failed\n"); > + > + dwc_priv = devm_kzalloc(&pdev->dev, sizeof(*dwc_priv), GFP_KERNEL); > + if (!dwc_priv) > + return -ENOMEM; > + > + dwc_priv->dev = &pdev->dev; > + > + /* Read rx-internal-delay-ps and update rx_clk delay */ > + if (!of_property_read_u32(pdev->dev.of_node, > + "rx-internal-delay-ps", > + &dwc_priv->rx_delay_ps)) { > + eic7700_apply_delay(dwc_priv->rx_delay_ps, > + ð_dly_param, true); > + } else { > + dev_warn(&pdev->dev, "can't get rx-internal-delay-ps\n"); > + } > + > + /* Read tx-internal-delay-ps and update tx_clk delay */ > + if (!of_property_read_u32(pdev->dev.of_node, > + "tx-internal-delay-ps", > + &dwc_priv->tx_delay_ps)) { > + eic7700_apply_delay(dwc_priv->tx_delay_ps, > + ð_dly_param, false); > + } else { > + dev_warn(&pdev->dev, "can't get tx-internal-delay-ps\n"); > + } > + > + dwc_priv->hsp_regmap = > + syscon_regmap_lookup_by_phandle(pdev->dev.of_node, > + "eswin,hsp-sp-csr"); > + if (IS_ERR(dwc_priv->hsp_regmap)) > + return dev_err_probe(&pdev->dev, > + PTR_ERR(dwc_priv->hsp_regmap), > + "Failed to get hsp-sp-csr regmap\n"); > + > + ret = of_property_read_u32_index(pdev->dev.of_node, > + "eswin,hsp-sp-csr", > + 1, ð_phy_ctrl_offset); > + if (ret) > + return dev_err_probe(&pdev->dev, > + ret, > + "can't get eth_phy_ctrl_offset\n"); > + > + regmap_read(dwc_priv->hsp_regmap, eth_phy_ctrl_offset, > + ð_phy_ctrl_regset); > + eth_phy_ctrl_regset |= > + (EIC7700_ETH_TX_CLK_SEL | EIC7700_ETH_PHY_INTF_SELI); > + regmap_write(dwc_priv->hsp_regmap, eth_phy_ctrl_offset, > + eth_phy_ctrl_regset); > + > + ret = of_property_read_u32_index(pdev->dev.of_node, > + "eswin,hsp-sp-csr", > + 2, ð_axi_lp_ctrl_offset); > + if (ret) > + return dev_err_probe(&pdev->dev, > + ret, > + "can't get eth_axi_lp_ctrl_offset\n"); > + > + regmap_write(dwc_priv->hsp_regmap, eth_axi_lp_ctrl_offset, > + EIC7700_ETH_CSYSREQ_VAL); > + > + ret = of_property_read_u32_index(pdev->dev.of_node, > + "eswin,hsp-sp-csr", > + 3, ð_rxd_dly_offset); > + if (ret) > + return dev_err_probe(&pdev->dev, > + ret, > + "can't get eth_rxd_dly_offset\n"); > + > + regmap_write(dwc_priv->hsp_regmap, eth_rxd_dly_offset, > + eth_dly_param); > + > + dwc_priv->clk_tx = devm_clk_get(&pdev->dev, "tx"); > + if (IS_ERR(dwc_priv->clk_tx)) > + return dev_err_probe(&pdev->dev, > + PTR_ERR(dwc_priv->clk_tx), > + "error getting tx clock\n"); > + > + dwc_priv->clk_axi = devm_clk_get(&pdev->dev, "axi"); > + if (IS_ERR(dwc_priv->clk_axi)) > + return dev_err_probe(&pdev->dev, > + PTR_ERR(dwc_priv->clk_axi), > + "error getting axi clock\n"); > + > + dwc_priv->clk_cfg = devm_clk_get(&pdev->dev, "cfg"); > + if (IS_ERR(dwc_priv->clk_cfg)) > + return dev_err_probe(&pdev->dev, > + PTR_ERR(dwc_priv->clk_cfg), > + "error getting cfg clock\n"); These then become devm_clk_bulk_get_all(). > + > + ret = eic7700_clks_config(dwc_priv, true); > + if (ret) > + return dev_err_probe(&pdev->dev, > + ret, > + "error enable clock\n"); Maybe even devm_clk_bulk_get_all_enabled() which will omit this step... > + > + plat_dat->clk_tx_i = dwc_priv->clk_tx; > + plat_dat->set_clk_tx_rate = stmmac_set_clk_tx_rate; > + plat_dat->bsp_priv = dwc_priv; > + plat_dat->clks_config = eic7700_clks_config; > + > + ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res); > + if (ret) { > + eic7700_clks_config(dwc_priv, false); ... and means you don't need this call... > + return dev_err_probe(&pdev->dev, > + ret, > + "Failed to driver probe\n"); > + } > + > + return ret; > +} > + > +static void eic7700_dwmac_remove(struct platform_device *pdev) > +{ > + struct eic7700_qos_priv *dwc_priv = get_stmmac_bsp_priv(&pdev->dev); > + > + stmmac_pltfr_remove(pdev); > + eic7700_clks_config(dwc_priv, false); > +} ... and you can remove this function entirely ... > + > +static const struct of_device_id eic7700_dwmac_match[] = { > + { .compatible = "eswin,eic7700-qos-eth" }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, eic7700_dwmac_match); > + > +static struct platform_driver eic7700_dwmac_driver = { > + .probe = eic7700_dwmac_probe, > + .remove = eic7700_dwmac_remove, ... replacing this with stmmac_pltfr_remove(). Thanks. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!