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 49B44CA9ED0 for ; Mon, 12 Oct 2026 12:31:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: 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=LO0Yz/MP+2OBVxgPNxHXTTIRib3WwaNbYxgT8hpX6F8=; b=QOS3VDYI0+rxGZ n324rpS/xJHtrNxv1Qbo+CjvNTi/AdndsIZvqdTFdHqeVuyhh0LN5W4IHeNZl5aiRzC+hnmTGrz5y cZatggAq1RaLxf+/6OhDJqqUiskrxDhRSnDyqlf5/DGylV2ibS9FM8U69rWSsiFPjEAOjTzjD4dWG djeXm5yKnjXWEC8OHywa1jm39vFGGeaniEj1pAn9GoZvbAjX5quJXWF65iCiuiYeLCNL498ngwk06 BMSifXwDit46wUWg8Hosx+Pv/Rx34+cTnoPF7zXWC7omLwwDVE5/5zEZy/lIIaXVxs4CslIBL37O1 xm2OtH5hjHjW0kRSPmwQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.99.1 #2 (Red Hat Linux)) id 1xGFBO-0000000ASSD-3tg7; Mon, 12 Oct 2026 12:31:10 +0000 Received: from vps0.lunn.ch ([156.67.10.101]) by bombadil.infradead.org with esmtps (Exim 4.99.1 #2 (Red Hat Linux)) id 1xGFBK-0000000ASRX-3KVj; Mon, 12 Oct 2026 12:31:09 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lunn.ch; s=20171124; h=In-Reply-To:Content-Disposition:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:From:Sender:Reply-To:Subject: Date:Message-ID:To:Cc:MIME-Version:Content-Type:Content-Transfer-Encoding: Content-ID:Content-Description:Content-Disposition:In-Reply-To:References; bh=uNBmIpHaVMdDPWDAgphNw6C+bWviKjN+J5cDaNCeyIo=; b=cVoH3PQ31cx1sQg57/ZA8m9wCK PvVS1ul4HIWVPnW9Om2PVZ2B86SpoZyXddX+lTNtISRmgyR1elC3HkehvNXEEwPXdkQB0oyiGfRqA szBprodQMJvDOJ6QFUYd0B3/GIhxeSf8ZSPFM3V8ViOqO+t+pg2095sMw3pKUtv4XG+M=; Received: from andrew by vps0.lunn.ch with local (Exim 4.94.2) (envelope-from ) id 1xGFB3-00AJyk-VM; Mon, 12 Oct 2026 14:30:49 +0200 Date: Mon, 12 Oct 2026 14:30:49 +0200 From: Andrew Lunn To: Lucas Tanure Cc: xianwei.zhao@amlogic.com, Andrew Lunn , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Neil Armstrong , Kevin Hilman , Jerome Brunet , Martin Blumenstingl , Maxime Chevallier , Maxime Coquelin , Alexandre Torgue , netdev@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-amlogic@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v3 4/8] net: stmmac: dwmac-meson8b: apply the RGMII delays the T7 way Message-ID: References: <20261010105900.247342-1-tanure@linux.com> <20261010105900.247342-5-tanure@linux.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20261010105900.247342-5-tanure@linux.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.9.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20261012_053107_489110_B6643B42 X-CRM114-Status: GOOD ( 21.65 ) X-BeenThere: linux-amlogic@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-amlogic" Errors-To: linux-amlogic-bounces+linux-amlogic=archiver.kernel.org@lists.infradead.org > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c > @@ -82,6 +82,7 @@ struct meson8b_dwmac_data { > int (*set_phy_mode)(struct meson8b_dwmac *dwmac); > bool has_prg_eth1_rgmii_rx_delay; > bool has_pipeline_clk; > + bool mac_applies_dt_delays; Please can you invert this and call it broken delays. We should make it clear that new devices should not set this, because it wrong, broken. > +static int meson_dwmac_init_dt_delays(struct meson8b_dwmac *dwmac, > + struct plat_stmmacenet_data *plat_dat) > +{ > + struct device_node *np = dwmac->dev->of_node; > + u32 tx_delay_ps = 0, rx_delay_ps = 0; > + phy_interface_t phy_mode; > + bool mac_tx, mac_rx; > + > + mac_tx = !of_property_read_u32(np, "tx-internal-delay-ps", &tx_delay_ps); > + mac_rx = !of_property_read_u32(np, "rx-internal-delay-ps", &rx_delay_ps); > + > + /* one step is a quarter of the RGMII clock period, 2ns at 1Gbit/s */ > + if (tx_delay_ps > 6000 || tx_delay_ps % 2000) > + return dev_err_probe(dwmac->dev, -EINVAL, > + "The RGMII TX delay must be 0, 2000, 4000 or 6000ps\n"); tx-internal-delay is not listed in amlogic,meson-dwmac.yaml. Please could you add it, and list the valid values. Do you have access to the datasheet? Could you check these numbers are correct, because they are larger than typical. 4ns, and 6ns makes no sense, the RGMII standard only calls for 2ns. > + > + /* the RX delay line moves in 200ps steps and reaches 3000ps */ > + if (rx_delay_ps > 3000 || rx_delay_ps % 200) > + return dev_err_probe(dwmac->dev, -EINVAL, > + "The RGMII RX delay range is 0..3000ps in 200ps steps\n"); Please could you listing these values in the binding. Also, mark that amlogic,tx-delay-ns & amlogic,rx-delay-ns are not valid for this compatible. > + > + phy_mode = phy_fix_phy_mode_for_mac_delays(dwmac->phy_mode, mac_tx, > + mac_rx); This is not correct. This function assumes mac_tx and mac_rx are Boolean values, the MAC is applying the full 2ns delay. However, if rx-internal-delay-ps is only providing 200ps, that is not true. I would prefer you keep to the usual way of doing things. Allow the MAC to apply small tuning delays, but have the PHY do the 2ns. So don't make any modification to phy-mode. > static int meson8b_init_rgmii_delays(struct meson8b_dwmac *dwmac) > { > u32 tx_dly_config, rx_adj_config, cfg_rxclk_dly, delay_config; > @@ -411,32 +455,34 @@ static int meson8b_dwmac_probe(struct platform_device *pdev) > dwmac->dev = &pdev->dev; > dwmac->phy_mode = plat_dat->phy_interface; > > - /* use 2ns as fallback since this value was previously hardcoded */ > - if (of_property_read_u32(pdev->dev.of_node, "amlogic,tx-delay-ns", > - &dwmac->tx_delay_ns)) > - dwmac->tx_delay_ns = 2; > - > - /* RX delay defaults to 0ps since this is what many boards use */ > - if (of_property_read_u32(pdev->dev.of_node, "rx-internal-delay-ps", > - &dwmac->rx_delay_ps)) { > - if (!of_property_read_u32(pdev->dev.of_node, > - "amlogic,rx-delay-ns", > - &dwmac->rx_delay_ps)) > - /* convert ns to ps */ > - dwmac->rx_delay_ps *= 1000; > - } > - > - if (dwmac->data->has_prg_eth1_rgmii_rx_delay) { > - if (dwmac->rx_delay_ps > 3000 || dwmac->rx_delay_ps % 200) { > - dev_err(dwmac->dev, > - "The RGMII RX delay range is 0..3000ps in 200ps steps"); > - return -EINVAL; > + if (!dwmac->data->mac_applies_dt_delays) { I suggest you do this at a higher level. Either call this function if _broken_delay is set, or the other one. Andrew _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic