diff mbox series

[net-next] net: fec: fix MAC internal delay doesn't work

Message ID 20210730114709.12385-1-qiangqing.zhang@nxp.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net-next] net: fec: fix MAC internal delay doesn't work | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/cc_maintainers fail 1 blamed authors not CCed: fugang.duan@nxp.com; 1 maintainers not CCed: fugang.duan@nxp.com
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch warning WARNING: line length of 81 exceeds 80 columns
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link

Commit Message

Joakim Zhang July 30, 2021, 11:47 a.m. UTC
This patch intends to fix MAC internal delay doesn't work, due to use
of_property_read_u32() incorrectly, and improve this feature a bit:
1) check the delay value if valid.
2) only enable "enet_2x_txclk" clock when require MAC internal delay.

Fixes: fc539459e900 ("net: fec: add MAC internal delayed clock feature support")
Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
Hi Andrew,

This fix targes to net-next as this is where the offending patch was applied.

When refine Andy Duan's patch to generic MAC internal delay property, I
did make a mistake, and didn't find it during test on i.MX8MM, since the
lack of chips which support this feature at my side. I am so sorry about this.
Please help review if there is some flaw in the logic.

Joakim
---
 drivers/net/ethernet/freescale/fec_main.c | 47 ++++++++++++++++++-----
 1 file changed, 37 insertions(+), 10 deletions(-)

Comments

Jakub Kicinski Aug. 2, 2021, 8:32 p.m. UTC | #1
On Fri, 30 Jul 2021 19:47:09 +0800 Joakim Zhang wrote:
> @@ -3806,6 +3827,10 @@ fec_probe(struct platform_device *pdev)
>  		fep->phy_interface = interface;
>  	}
>  
> +	ret = fec_enet_parse_rgmii_delay(fep, np);
> +	if (ret)
> +		goto failed_phy;

You're jumping to the wrong label, it looks like phy_node needs to be
released at this point.
Joakim Zhang Aug. 3, 2021, 2:01 a.m. UTC | #2
> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: 2021年8月3日 4:33
> To: Joakim Zhang <qiangqing.zhang@nxp.com>
> Cc: davem@davemloft.net; andrew@lunn.ch; netdev@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH net-next] net: fec: fix MAC internal delay doesn't work
> 
> On Fri, 30 Jul 2021 19:47:09 +0800 Joakim Zhang wrote:
> > @@ -3806,6 +3827,10 @@ fec_probe(struct platform_device *pdev)
> >  		fep->phy_interface = interface;
> >  	}
> >
> > +	ret = fec_enet_parse_rgmii_delay(fep, np);
> > +	if (ret)
> > +		goto failed_phy;
> 
> You're jumping to the wrong label, it looks like phy_node needs to be released
> at this point.

Yes, I will correct it. Thanks Jakub.

Best Regards,
Joakim Zhang
diff mbox series

Patch

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 40ea318d7396..a9b889bf375a 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2042,6 +2042,34 @@  static int fec_enet_clk_enable(struct net_device *ndev, bool enable)
 	return ret;
 }
 
+static int fec_enet_parse_rgmii_delay(struct fec_enet_private *fep,
+				      struct device_node *np)
+{
+	u32 rgmii_tx_delay, rgmii_rx_delay;
+
+	/* For rgmii tx internal delay, valid values are 0ps and 2000ps */
+	if (!of_property_read_u32(np, "tx-internal-delay-ps", &rgmii_tx_delay)) {
+		if (rgmii_tx_delay != 0 && rgmii_tx_delay != 2000) {
+			dev_err(&fep->pdev->dev, "The only allowed RGMII TX delay values are: 0ps, 2000ps");
+			return -EINVAL;
+		} else if (rgmii_tx_delay == 2000) {
+			fep->rgmii_txc_dly = true;
+		}
+	}
+
+	/* For rgmii rx internal delay, valid values are 0ps and 2000ps */
+	if (!of_property_read_u32(np, "rx-internal-delay-ps", &rgmii_rx_delay)) {
+		if (rgmii_rx_delay != 0 && rgmii_rx_delay != 2000) {
+			dev_err(&fep->pdev->dev, "The only allowed RGMII RX delay values are: 0ps, 2000ps");
+			return -EINVAL;
+		} else if (rgmii_rx_delay == 2000) {
+			fep->rgmii_rxc_dly = true;
+		}
+	}
+
+	return 0;
+}
+
 static int fec_enet_mii_probe(struct net_device *ndev)
 {
 	struct fec_enet_private *fep = netdev_priv(ndev);
@@ -3719,7 +3747,6 @@  fec_probe(struct platform_device *pdev)
 	char irq_name[8];
 	int irq_cnt;
 	struct fec_devinfo *dev_info;
-	u32 rgmii_delay;
 
 	fec_enet_get_queue_num(pdev, &num_tx_qs, &num_rx_qs);
 
@@ -3777,12 +3804,6 @@  fec_probe(struct platform_device *pdev)
 	if (ret)
 		goto failed_stop_mode;
 
-	/* For rgmii internal delay, valid values are 0ps and 2000ps */
-	if (of_property_read_u32(np, "tx-internal-delay-ps", &rgmii_delay))
-		fep->rgmii_txc_dly = true;
-	if (of_property_read_u32(np, "rx-internal-delay-ps", &rgmii_delay))
-		fep->rgmii_rxc_dly = true;
-
 	phy_node = of_parse_phandle(np, "phy-handle", 0);
 	if (!phy_node && of_phy_is_fixed_link(np)) {
 		ret = of_phy_register_fixed_link(np);
@@ -3806,6 +3827,10 @@  fec_probe(struct platform_device *pdev)
 		fep->phy_interface = interface;
 	}
 
+	ret = fec_enet_parse_rgmii_delay(fep, np);
+	if (ret)
+		goto failed_phy;
+
 	fep->clk_ipg = devm_clk_get(&pdev->dev, "ipg");
 	if (IS_ERR(fep->clk_ipg)) {
 		ret = PTR_ERR(fep->clk_ipg);
@@ -3835,9 +3860,11 @@  fec_probe(struct platform_device *pdev)
 	fep->clk_ref_rate = clk_get_rate(fep->clk_ref);
 
 	/* clk_2x_txclk is optional, depends on board */
-	fep->clk_2x_txclk = devm_clk_get(&pdev->dev, "enet_2x_txclk");
-	if (IS_ERR(fep->clk_2x_txclk))
-		fep->clk_2x_txclk = NULL;
+	if (fep->rgmii_txc_dly || fep->rgmii_rxc_dly) {
+		fep->clk_2x_txclk = devm_clk_get(&pdev->dev, "enet_2x_txclk");
+		if (IS_ERR(fep->clk_2x_txclk))
+			fep->clk_2x_txclk = NULL;
+	}
 
 	fep->bufdesc_ex = fep->quirks & FEC_QUIRK_HAS_BUFDESC_EX;
 	fep->clk_ptp = devm_clk_get(&pdev->dev, "ptp");