linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/2] arm64: dts: mt8195: Add Ethernet controller
@ 2022-12-23  1:50 Biao Huang
  2022-12-23  1:50 ` [PATCH v5 1/2] stmmac: dwmac-mediatek: enable 2ns delay only for special cases Biao Huang
  2022-12-23  1:50 ` [PATCH v5 2/2] arm64: dts: mt8195: Add Ethernet controller Biao Huang
  0 siblings, 2 replies; 14+ messages in thread
From: Biao Huang @ 2022-12-23  1:50 UTC (permalink / raw)
  To: Andrew Lunn, AngeloGioacchino Del Regno, Rob Herring,
	Krzysztof Kozlowski, Matthias Brugger
  Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Maxime Coquelin, Richard Cochran, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel, Biao Huang, macpaul.lin

Changes in v5:
1. reorder the clocks as Angelo's comments
2. add a driver patch to fix rgmii-id issue, then we can
use a ususal way rgmii/rgmii-id as Andrew's comments.

Changes in v4:
1. remove {address,size}-cells = <0> to avoid warning as Angelo's feedback.
2. Add reviewd-by as Angelo's comments.

Changes in v3:
1. move stmmac-axi-config, rx-queues-config, tx-queues-configs inside ethernet node
as Angelo's comments.
2. add {address,size}-cells = <0> in ethernet node as Angelo's comments.

Changes in v2:
1. modify pinctrl node used by ethernet to match rules in pinctrl-mt8195.yaml,
which is pointed by Krzysztof.
2. remove "mac-address" property in ethernet node as comments of Krzysztof.

Changes in v1:
add dts node for MT8195 Ethernet controller

Biao Huang (2):
  stmmac: dwmac-mediatek: enable 2ns delay only for special cases
  arm64: dts: mt8195: Add Ethernet controller

 arch/arm64/boot/dts/mediatek/mt8195-demo.dts  | 81 +++++++++++++++++
 arch/arm64/boot/dts/mediatek/mt8195.dtsi      | 86 +++++++++++++++++++
 .../ethernet/stmicro/stmmac/dwmac-mediatek.c  |  7 +-
 3 files changed, 173 insertions(+), 1 deletion(-)

-- 
2.18.0



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v5 1/2] stmmac: dwmac-mediatek: enable 2ns delay only for special cases
  2022-12-23  1:50 [PATCH v5 0/2] arm64: dts: mt8195: Add Ethernet controller Biao Huang
@ 2022-12-23  1:50 ` Biao Huang
  2022-12-23  9:24   ` AngeloGioacchino Del Regno
  2022-12-23 14:18   ` Andrew Lunn
  2022-12-23  1:50 ` [PATCH v5 2/2] arm64: dts: mt8195: Add Ethernet controller Biao Huang
  1 sibling, 2 replies; 14+ messages in thread
From: Biao Huang @ 2022-12-23  1:50 UTC (permalink / raw)
  To: Andrew Lunn, AngeloGioacchino Del Regno, Rob Herring,
	Krzysztof Kozlowski, Matthias Brugger
  Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Maxime Coquelin, Richard Cochran, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel, Biao Huang, macpaul.lin

In current driver, MAC will always enable 2ns delay in RGMII mode,
but that will lead to transmission failures for "rgmii-id"/"rgmii-txid"
cases.

Modify the implementation of fix_mac_speed() to ensure the 2ns delay
will only take effect for "rgmii-rxid"/"rgmii" cases, then user can
choose phy-mode freely.

Fixes: f2d356a6ab71 ("stmmac: dwmac-mediatek: add support for mt8195")
Signed-off-by: Biao Huang <biao.huang@mediatek.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
index d42e1afb6521..cde4fb81d596 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
@@ -447,7 +447,9 @@ static void mt8195_fix_mac_speed(void *priv, unsigned int speed)
 {
 	struct mediatek_dwmac_plat_data *priv_plat = priv;
 
-	if ((phy_interface_mode_is_rgmii(priv_plat->phy_mode))) {
+	switch (priv_plat->phy_mode) {
+	case PHY_INTERFACE_MODE_RGMII:
+	case PHY_INTERFACE_MODE_RGMII_RXID:
 		/* prefer 2ns fixed delay which is controlled by TXC_PHASE_CTRL,
 		 * when link speed is 1Gbps with RGMII interface,
 		 * Fall back to delay macro circuit for 10/100Mbps link speed.
@@ -462,6 +464,9 @@ static void mt8195_fix_mac_speed(void *priv, unsigned int speed)
 					   MT8195_RGMII_TXC_PHASE_CTRL);
 		else
 			mt8195_set_delay(priv_plat);
+		break;
+	default:
+		break;
 	}
 }
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v5 2/2] arm64: dts: mt8195: Add Ethernet controller
  2022-12-23  1:50 [PATCH v5 0/2] arm64: dts: mt8195: Add Ethernet controller Biao Huang
  2022-12-23  1:50 ` [PATCH v5 1/2] stmmac: dwmac-mediatek: enable 2ns delay only for special cases Biao Huang
@ 2022-12-23  1:50 ` Biao Huang
  2022-12-23  9:24   ` AngeloGioacchino Del Regno
  1 sibling, 1 reply; 14+ messages in thread
From: Biao Huang @ 2022-12-23  1:50 UTC (permalink / raw)
  To: Andrew Lunn, AngeloGioacchino Del Regno, Rob Herring,
	Krzysztof Kozlowski, Matthias Brugger
  Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Maxime Coquelin, Richard Cochran, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel, Biao Huang, macpaul.lin

Add Ethernet controller node for mt8195.

Signed-off-by: Biao Huang <biao.huang@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8195-demo.dts | 81 ++++++++++++++++++
 arch/arm64/boot/dts/mediatek/mt8195.dtsi     | 86 ++++++++++++++++++++
 2 files changed, 167 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8195-demo.dts b/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
index 4fbd99eb496a..2e6979c47aa6 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
@@ -258,6 +258,66 @@ &mt6359_vsram_others_ldo_reg {
 };
 
 &pio {
+	eth_default_pins: eth-default-pins {
+		pins-txd {
+			pinmux = <PINMUX_GPIO77__FUNC_GBE_TXD3>,
+				 <PINMUX_GPIO78__FUNC_GBE_TXD2>,
+				 <PINMUX_GPIO79__FUNC_GBE_TXD1>,
+				 <PINMUX_GPIO80__FUNC_GBE_TXD0>;
+			drive-strength = <MTK_DRIVE_8mA>;
+		};
+		pins-cc {
+			pinmux = <PINMUX_GPIO85__FUNC_GBE_TXC>,
+				 <PINMUX_GPIO88__FUNC_GBE_TXEN>,
+				 <PINMUX_GPIO87__FUNC_GBE_RXDV>,
+				 <PINMUX_GPIO86__FUNC_GBE_RXC>;
+			drive-strength = <MTK_DRIVE_8mA>;
+		};
+		pins-rxd {
+			pinmux = <PINMUX_GPIO81__FUNC_GBE_RXD3>,
+				 <PINMUX_GPIO82__FUNC_GBE_RXD2>,
+				 <PINMUX_GPIO83__FUNC_GBE_RXD1>,
+				 <PINMUX_GPIO84__FUNC_GBE_RXD0>;
+		};
+		pins-mdio {
+			pinmux = <PINMUX_GPIO89__FUNC_GBE_MDC>,
+				 <PINMUX_GPIO90__FUNC_GBE_MDIO>;
+			input-enable;
+		};
+		pins-power {
+			pinmux = <PINMUX_GPIO91__FUNC_GPIO91>,
+				 <PINMUX_GPIO92__FUNC_GPIO92>;
+			output-high;
+		};
+	};
+
+	eth_sleep_pins: eth-sleep-pins {
+		pins-txd {
+			pinmux = <PINMUX_GPIO77__FUNC_GPIO77>,
+				 <PINMUX_GPIO78__FUNC_GPIO78>,
+				 <PINMUX_GPIO79__FUNC_GPIO79>,
+				 <PINMUX_GPIO80__FUNC_GPIO80>;
+		};
+		pins-cc {
+			pinmux = <PINMUX_GPIO85__FUNC_GPIO85>,
+				 <PINMUX_GPIO88__FUNC_GPIO88>,
+				 <PINMUX_GPIO87__FUNC_GPIO87>,
+				 <PINMUX_GPIO86__FUNC_GPIO86>;
+		};
+		pins-rxd {
+			pinmux = <PINMUX_GPIO81__FUNC_GPIO81>,
+				 <PINMUX_GPIO82__FUNC_GPIO82>,
+				 <PINMUX_GPIO83__FUNC_GPIO83>,
+				 <PINMUX_GPIO84__FUNC_GPIO84>;
+		};
+		pins-mdio {
+			pinmux = <PINMUX_GPIO89__FUNC_GPIO89>,
+				 <PINMUX_GPIO90__FUNC_GPIO90>;
+			input-disable;
+			bias-disable;
+		};
+	};
+
 	gpio_keys_pins: gpio-keys-pins {
 		pins {
 			pinmux = <PINMUX_GPIO106__FUNC_GPIO106>;
@@ -434,6 +494,27 @@ &xhci0 {
 	status = "okay";
 };
 
+&eth {
+	phy-mode ="rgmii-id";
+	phy-handle = <&ethernet_phy0>;
+	snps,reset-gpio = <&pio 93 GPIO_ACTIVE_HIGH>;
+	snps,reset-delays-us = <0 10000 10000>;
+	pinctrl-names = "default", "sleep";
+	pinctrl-0 = <&eth_default_pins>;
+	pinctrl-1 = <&eth_sleep_pins>;
+	status = "okay";
+
+	mdio {
+		compatible = "snps,dwmac-mdio";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		ethernet_phy0: ethernet-phy@1 {
+			compatible = "ethernet-phy-id001c.c916";
+			reg = <0x1>;
+		};
+	};
+};
+
 &xhci1 {
 	vusb33-supply = <&mt6359_vusb_ldo_reg>;
 	status = "okay";
diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
index 5d31536f4c48..b90d38d87aa4 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
@@ -1046,6 +1046,92 @@ spis1: spi@1101e000 {
 			status = "disabled";
 		};
 
+		eth: ethernet@11021000 {
+			compatible = "mediatek,mt8195-gmac", "snps,dwmac-5.10a";
+			reg = <0 0x11021000 0 0x4000>;
+			interrupts = <GIC_SPI 716 IRQ_TYPE_LEVEL_HIGH 0>;
+			interrupt-names = "macirq";
+			clock-names = "axi",
+				      "apb",
+				      "mac_main",
+				      "ptp_ref",
+				      "rmii_internal",
+				      "mac_cg";
+			clocks = <&pericfg_ao CLK_PERI_AO_ETHERNET>,
+				 <&pericfg_ao CLK_PERI_AO_ETHERNET_BUS>,
+				 <&topckgen CLK_TOP_SNPS_ETH_250M>,
+				 <&topckgen CLK_TOP_SNPS_ETH_62P4M_PTP>,
+				 <&topckgen CLK_TOP_SNPS_ETH_50M_RMII>,
+				 <&pericfg_ao CLK_PERI_AO_ETHERNET_MAC>;
+			assigned-clocks = <&topckgen CLK_TOP_SNPS_ETH_250M>,
+					  <&topckgen CLK_TOP_SNPS_ETH_62P4M_PTP>,
+					  <&topckgen CLK_TOP_SNPS_ETH_50M_RMII>;
+			assigned-clock-parents = <&topckgen CLK_TOP_ETHPLL_D2>,
+						 <&topckgen CLK_TOP_ETHPLL_D8>,
+						 <&topckgen CLK_TOP_ETHPLL_D10>;
+			power-domains = <&spm MT8195_POWER_DOMAIN_ETHER>;
+			mediatek,pericfg = <&infracfg_ao>;
+			snps,axi-config = <&stmmac_axi_setup>;
+			snps,mtl-rx-config = <&mtl_rx_setup>;
+			snps,mtl-tx-config = <&mtl_tx_setup>;
+			snps,txpbl = <16>;
+			snps,rxpbl = <16>;
+			snps,clk-csr = <0>;
+			status = "disabled";
+
+			stmmac_axi_setup: stmmac-axi-config {
+				snps,wr_osr_lmt = <0x7>;
+				snps,rd_osr_lmt = <0x7>;
+				snps,blen = <0 0 0 0 16 8 4>;
+			};
+
+			mtl_rx_setup: rx-queues-config {
+				snps,rx-queues-to-use = <4>;
+				snps,rx-sched-sp;
+				queue0 {
+					snps,dcb-algorithm;
+					snps,map-to-dma-channel = <0x0>;
+				};
+				queue1 {
+					snps,dcb-algorithm;
+					snps,map-to-dma-channel = <0x0>;
+				};
+				queue2 {
+					snps,dcb-algorithm;
+					snps,map-to-dma-channel = <0x0>;
+				};
+				queue3 {
+					snps,dcb-algorithm;
+					snps,map-to-dma-channel = <0x0>;
+				};
+			};
+
+			mtl_tx_setup: tx-queues-config {
+				snps,tx-queues-to-use = <4>;
+				snps,tx-sched-wrr;
+				queue0 {
+					snps,weight = <0x10>;
+					snps,dcb-algorithm;
+					snps,priority = <0x0>;
+				};
+				queue1 {
+					snps,weight = <0x11>;
+					snps,dcb-algorithm;
+					snps,priority = <0x1>;
+				};
+				queue2 {
+					snps,weight = <0x12>;
+					snps,dcb-algorithm;
+					snps,priority = <0x2>;
+				};
+				queue3 {
+					snps,weight = <0x13>;
+					snps,dcb-algorithm;
+					snps,priority = <0x3>;
+				};
+			};
+		};
+
 		xhci0: usb@11200000 {
 			compatible = "mediatek,mt8195-xhci",
 				     "mediatek,mtk-xhci";
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v5 1/2] stmmac: dwmac-mediatek: enable 2ns delay only for special cases
  2022-12-23  1:50 ` [PATCH v5 1/2] stmmac: dwmac-mediatek: enable 2ns delay only for special cases Biao Huang
@ 2022-12-23  9:24   ` AngeloGioacchino Del Regno
  2022-12-23 14:18   ` Andrew Lunn
  1 sibling, 0 replies; 14+ messages in thread
From: AngeloGioacchino Del Regno @ 2022-12-23  9:24 UTC (permalink / raw)
  To: Biao Huang, Andrew Lunn, Rob Herring, Krzysztof Kozlowski,
	Matthias Brugger
  Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Maxime Coquelin, Richard Cochran, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel, macpaul.lin

Il 23/12/22 02:50, Biao Huang ha scritto:
> In current driver, MAC will always enable 2ns delay in RGMII mode,
> but that will lead to transmission failures for "rgmii-id"/"rgmii-txid"
> cases.
> 
> Modify the implementation of fix_mac_speed() to ensure the 2ns delay
> will only take effect for "rgmii-rxid"/"rgmii" cases, then user can
> choose phy-mode freely.
> 
> Fixes: f2d356a6ab71 ("stmmac: dwmac-mediatek: add support for mt8195")
> Signed-off-by: Biao Huang <biao.huang@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v5 2/2] arm64: dts: mt8195: Add Ethernet controller
  2022-12-23  1:50 ` [PATCH v5 2/2] arm64: dts: mt8195: Add Ethernet controller Biao Huang
@ 2022-12-23  9:24   ` AngeloGioacchino Del Regno
  2022-12-28  6:23     ` Biao Huang (黄彪)
  0 siblings, 1 reply; 14+ messages in thread
From: AngeloGioacchino Del Regno @ 2022-12-23  9:24 UTC (permalink / raw)
  To: Biao Huang, Andrew Lunn, Rob Herring, Krzysztof Kozlowski,
	Matthias Brugger
  Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Maxime Coquelin, Richard Cochran, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel, macpaul.lin

Il 23/12/22 02:50, Biao Huang ha scritto:
> Add Ethernet controller node for mt8195.
> 
> Signed-off-by: Biao Huang <biao.huang@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v5 1/2] stmmac: dwmac-mediatek: enable 2ns delay only for special cases
  2022-12-23  1:50 ` [PATCH v5 1/2] stmmac: dwmac-mediatek: enable 2ns delay only for special cases Biao Huang
  2022-12-23  9:24   ` AngeloGioacchino Del Regno
@ 2022-12-23 14:18   ` Andrew Lunn
  2022-12-26  3:50     ` Biao Huang (黄彪)
  1 sibling, 1 reply; 14+ messages in thread
From: Andrew Lunn @ 2022-12-23 14:18 UTC (permalink / raw)
  To: Biao Huang
  Cc: AngeloGioacchino Del Regno, Rob Herring, Krzysztof Kozlowski,
	Matthias Brugger, Giuseppe Cavallaro, Alexandre Torgue,
	Jose Abreu, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Maxime Coquelin, Richard Cochran, devicetree,
	linux-arm-kernel, linux-mediatek, linux-kernel, macpaul.lin

On Fri, Dec 23, 2022 at 09:50:28AM +0800, Biao Huang wrote:
> In current driver, MAC will always enable 2ns delay in RGMII mode,
> but that will lead to transmission failures for "rgmii-id"/"rgmii-txid"
> cases.
> 
> Modify the implementation of fix_mac_speed() to ensure the 2ns delay
> will only take effect for "rgmii-rxid"/"rgmii" cases, then user can
> choose phy-mode freely.

This does not seem correct. There are three ways the delays can be added:

1) The MAC
2) The PHY
3) Extra long lines on the board.

What the four RGMII modes tell you is what is needed in addition to
whatever the board provides. So it describes the combination of 1) and
2). Your board does not appear to be applying any delays, so you
should be using rgmii-id.

The MAC and PHY driver then need to decide how to add these delays,
and in most cases, the MAC does nothing, and passes phy-mode to the
PHY and the PHY adds the delay.

The MAC can add delays, but if it does, it need to mask out the delays
it added to the value passed to the PHY. Otherwise the PHY will add
the delay as well.

>  
> -	if ((phy_interface_mode_is_rgmii(priv_plat->phy_mode))) {
> +	switch (priv_plat->phy_mode) {
> +	case PHY_INTERFACE_MODE_RGMII:
> +	case PHY_INTERFACE_MODE_RGMII_RXID:
>  		/* prefer 2ns fixed delay which is controlled by TXC_PHASE_CTRL,
>  		 * when link speed is 1Gbps with RGMII interface,
>  		 * Fall back to delay macro circuit for 10/100Mbps link speed.

So this is wrong. PHY_INTERFACE_MODE_RGMII means the board is adding
the delay via long lines. You should not be added any delay at all.

For PHY_INTERFACE_MODE_RGMII_RXID, you need to mask the RXID bit from
phy_mode when connecting the MAC to the PHY. Otherwise the PHY is
going to add this delay as well.

	 Andrew

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v5 1/2] stmmac: dwmac-mediatek: enable 2ns delay only for special cases
  2022-12-23 14:18   ` Andrew Lunn
@ 2022-12-26  3:50     ` Biao Huang (黄彪)
  2022-12-26 15:19       ` Andrew Lunn
  0 siblings, 1 reply; 14+ messages in thread
From: Biao Huang (黄彪) @ 2022-12-26  3:50 UTC (permalink / raw)
  To: andrew
  Cc: linux-mediatek, robh+dt, linux-kernel, peppe.cavallaro, joabreu,
	devicetree, kuba, edumazet, alexandre.torgue, mcoquelin.stm32,
	pabeni, richardcochran, krzysztof.kozlowski+dt,
	Macpaul Lin (林智斌),
	matthias.bgg, davem, linux-arm-kernel, angelogioacchino.delregno

Dear Andrew,
	Thanks for your comments.

On Fri, 2022-12-23 at 15:18 +0100, Andrew Lunn wrote:
> On Fri, Dec 23, 2022 at 09:50:28AM +0800, Biao Huang wrote:
> > In current driver, MAC will always enable 2ns delay in RGMII mode,
> > but that will lead to transmission failures for "rgmii-id"/"rgmii-
> > txid"
> > cases.
> > 
> > Modify the implementation of fix_mac_speed() to ensure the 2ns
> > delay
> > will only take effect for "rgmii-rxid"/"rgmii" cases, then user can
> > choose phy-mode freely.
> 
> This does not seem correct. There are three ways the delays can be
> added:
> 
> 1) The MAC
> 2) The PHY
> 3) Extra long lines on the board.
> 
> What the four RGMII modes tell you is what is needed in addition to
> whatever the board provides. So it describes the combination of 1)
> and
> 2). Your board does not appear to be applying any delays, so you
> should be using rgmii-id.
> 
> The MAC and PHY driver then need to decide how to add these delays,
> and in most cases, the MAC does nothing, and passes phy-mode to the
> PHY and the PHY adds the delay.
> 
> The MAC can add delays, but if it does, it need to mask out the
> delays
> it added to the value passed to the PHY. Otherwise the PHY will add
> the delay as well.
in the ethernet-controller.yaml,
 77       # RX and TX delays are added by the MAC when required
 78       - rgmii
 79
 80       # RGMII with internal RX and TX delays provided by the PHY,
 81       # the MAC should not add the RX or TX delays in this case
 82       - rgmii-id
 83
 84       # RGMII with internal RX delay provided by the PHY, the MAC
 85       # should not add an RX delay in this case
 86       - rgmii-rxid
 87
 88       # RGMII with internal TX delay provided by the PHY, the MAC
 89       # should not add an TX delay in this case
 90       - rgmii-txid

so, why don't MAC set delay according to rgmii-** ?
rgmii-rxid   --> mac set tx, but not set rx delay
rgmii-txid   --> mac set rx, but not set tx delay
...

and PHY seems set delay according to rgmii-** in their dirver.

> 
> >  
> > -	if ((phy_interface_mode_is_rgmii(priv_plat->phy_mode))) {
> > +	switch (priv_plat->phy_mode) {
> > +	case PHY_INTERFACE_MODE_RGMII:
> > +	case PHY_INTERFACE_MODE_RGMII_RXID:
> >  		/* prefer 2ns fixed delay which is controlled by
> > TXC_PHASE_CTRL,
> >  		 * when link speed is 1Gbps with RGMII interface,
> >  		 * Fall back to delay macro circuit for 10/100Mbps link
> > speed.
> 
> So this is wrong. PHY_INTERFACE_MODE_RGMII means the board is adding
> the delay via long lines. You should not be added any delay at all.
> 
> For PHY_INTERFACE_MODE_RGMII_RXID, you need to mask the RXID bit from
> phy_mode when connecting the MAC to the PHY. Otherwise the PHY is
> going to add this delay as well.

These lines only set tx delay when phy_mode is RGMII/RGMI_RXID, when
phy will not add tx delay internally.
We don't want to restrict phy-mode to rgmii-id only, so I don't think
we need mask RXID bit, then, User can use rgmii-rxid/-txid/-id as their
will.

If I misunderstood, please correct me. Thanks.
> 
> 	 Andrew
Best Regards!
Biao

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v5 1/2] stmmac: dwmac-mediatek: enable 2ns delay only for special cases
  2022-12-26  3:50     ` Biao Huang (黄彪)
@ 2022-12-26 15:19       ` Andrew Lunn
  2022-12-27  7:09         ` Biao Huang (黄彪)
  0 siblings, 1 reply; 14+ messages in thread
From: Andrew Lunn @ 2022-12-26 15:19 UTC (permalink / raw)
  To: Biao Huang (黄彪)
  Cc: linux-mediatek, robh+dt, linux-kernel, peppe.cavallaro, joabreu,
	devicetree, kuba, edumazet, alexandre.torgue, mcoquelin.stm32,
	pabeni, richardcochran, krzysztof.kozlowski+dt,
	Macpaul Lin (林智斌),
	matthias.bgg, davem, linux-arm-kernel, angelogioacchino.delregno

> We don't want to restrict phy-mode to rgmii-id only, so I don't think
> we need mask RXID bit, then, User can use rgmii-rxid/-txid/-id as their
> will.

The phy-mode is a property of the board. It describes what delays are
required, in addition to what the board itself provides via extra long
lines. As such, there is no choice, it is set by physics.

How you implement those delays is then up to the MAC and PHY. 95% of
all MAC drivers simply pass phy-mode to the PHY and let the PHY add
the delays. The Renesas MAC driver is one of the exceptions, it often
adds the delays, and then passes a different phy-mode to the PHY in
order that there are not double delays.

So you can be different to everybody else, have MAC do one delay, and
the PHY do the other. Just be aware that nobody else does this, no DT
developers will expect it, etc.

Or you can just do what everybody else does and let the PHY add the
delays.

What some MACs do is implement rx-internal-delay-ps and
tx-internal-delay-ps These allow fine tuning of the delays. These are
expected to be small values, adding 10s or 100s of pico seconds. They
are not expected to be 2000ps.

    Andrew

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v5 1/2] stmmac: dwmac-mediatek: enable 2ns delay only for special cases
  2022-12-26 15:19       ` Andrew Lunn
@ 2022-12-27  7:09         ` Biao Huang (黄彪)
  2022-12-27 16:20           ` Andrew Lunn
  2022-12-28 16:00           ` Andrew Lunn
  0 siblings, 2 replies; 14+ messages in thread
From: Biao Huang (黄彪) @ 2022-12-27  7:09 UTC (permalink / raw)
  To: andrew
  Cc: linux-kernel, linux-mediatek, robh+dt, peppe.cavallaro,
	devicetree, joabreu, kuba, edumazet, alexandre.torgue,
	mcoquelin.stm32, pabeni, richardcochran, krzysztof.kozlowski+dt,
	Macpaul Lin (林智斌),
	matthias.bgg, davem, linux-arm-kernel, angelogioacchino.delregno

Dear Andrew,
	Thanks for your comments~

On Mon, 2022-12-26 at 16:19 +0100, Andrew Lunn wrote:
> > We don't want to restrict phy-mode to rgmii-id only, so I don't
> > think
> > we need mask RXID bit, then, User can use rgmii-rxid/-txid/-id as
> > their
> > will.
> 
> The phy-mode is a property of the board. It describes what delays are
> required, in addition to what the board itself provides via extra
> long
> lines. As such, there is no choice, it is set by physics.
> 
> How you implement those delays is then up to the MAC and PHY. 95% of
> all MAC drivers simply pass phy-mode to the PHY and let the PHY add
> the delays. The Renesas MAC driver is one of the exceptions, it often
> adds the delays, and then passes a different phy-mode to the PHY in
> order that there are not double delays.
> 
> So you can be different to everybody else, have MAC do one delay, and
> the PHY do the other. Just be aware that nobody else does this, no DT
> developers will expect it, etc.
> 
> Or you can just do what everybody else does and let the PHY add the
> delays.
> 
> What some MACs do is implement rx-internal-delay-ps and
> tx-internal-delay-ps These allow fine tuning of the delays. These are
> expected to be small values, adding 10s or 100s of pico seconds. They
> are not expected to be 2000ps.
OK, as to this series, Is the following modification enough?
    1. phy-mode = rgmii-id;  in dts.
    2. remove the fix_mac_speed() in driver.
> 
>     Andrew
Best Regards!
Biao

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v5 1/2] stmmac: dwmac-mediatek: enable 2ns delay only for special cases
  2022-12-27  7:09         ` Biao Huang (黄彪)
@ 2022-12-27 16:20           ` Andrew Lunn
  2022-12-28  6:22             ` Biao Huang (黄彪)
  2022-12-28 16:00           ` Andrew Lunn
  1 sibling, 1 reply; 14+ messages in thread
From: Andrew Lunn @ 2022-12-27 16:20 UTC (permalink / raw)
  To: Biao Huang (黄彪)
  Cc: linux-kernel, linux-mediatek, robh+dt, peppe.cavallaro,
	devicetree, joabreu, kuba, edumazet, alexandre.torgue,
	mcoquelin.stm32, pabeni, richardcochran, krzysztof.kozlowski+dt,
	Macpaul Lin (林智斌),
	matthias.bgg, davem, linux-arm-kernel, angelogioacchino.delregno

> OK, as to this series, Is the following modification enough?
>     1. phy-mode = rgmii-id;  in dts.
>     2. remove the fix_mac_speed() in driver.

Yep, that is good.

     Andrew

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v5 1/2] stmmac: dwmac-mediatek: enable 2ns delay only for special cases
  2022-12-27 16:20           ` Andrew Lunn
@ 2022-12-28  6:22             ` Biao Huang (黄彪)
  0 siblings, 0 replies; 14+ messages in thread
From: Biao Huang (黄彪) @ 2022-12-28  6:22 UTC (permalink / raw)
  To: andrew
  Cc: linux-kernel, linux-mediatek, robh+dt, peppe.cavallaro, joabreu,
	devicetree, kuba, edumazet, alexandre.torgue, mcoquelin.stm32,
	pabeni, richardcochran, krzysztof.kozlowski+dt,
	Macpaul Lin (林智斌),
	matthias.bgg, davem, linux-arm-kernel, angelogioacchino.delregno

Dear Andrew,
	Thanks for your comments~

On Tue, 2022-12-27 at 17:20 +0100, Andrew Lunn wrote:
> > OK, as to this series, Is the following modification enough?
> >     1. phy-mode = rgmii-id;  in dts.
> >     2. remove the fix_mac_speed() in driver.
> 
> Yep, that is good.
OK, will update in next send.
> 
>      Andrew
Best Regards!
Biao

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v5 2/2] arm64: dts: mt8195: Add Ethernet controller
  2022-12-23  9:24   ` AngeloGioacchino Del Regno
@ 2022-12-28  6:23     ` Biao Huang (黄彪)
  0 siblings, 0 replies; 14+ messages in thread
From: Biao Huang (黄彪) @ 2022-12-28  6:23 UTC (permalink / raw)
  To: matthias.bgg, angelogioacchino.delregno, andrew, robh+dt,
	krzysztof.kozlowski+dt
  Cc: linux-mediatek, linux-kernel, peppe.cavallaro, joabreu,
	devicetree, kuba, pabeni, alexandre.torgue, mcoquelin.stm32,
	edumazet, richardcochran, linux-arm-kernel,
	Macpaul Lin (林智斌),
	davem

Dear Angelo,
	Thanks for your comments~

On Fri, 2022-12-23 at 10:24 +0100, AngeloGioacchino Del Regno wrote:
> Reviewed-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
Will add in next send.

Best Regards!
Biao

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v5 1/2] stmmac: dwmac-mediatek: enable 2ns delay only for special cases
  2022-12-27  7:09         ` Biao Huang (黄彪)
  2022-12-27 16:20           ` Andrew Lunn
@ 2022-12-28 16:00           ` Andrew Lunn
  2022-12-29  0:37             ` Biao Huang (黄彪)
  1 sibling, 1 reply; 14+ messages in thread
From: Andrew Lunn @ 2022-12-28 16:00 UTC (permalink / raw)
  To: Biao Huang (黄彪)
  Cc: linux-kernel, linux-mediatek, robh+dt, peppe.cavallaro,
	devicetree, joabreu, kuba, edumazet, alexandre.torgue,
	mcoquelin.stm32, pabeni, richardcochran, krzysztof.kozlowski+dt,
	Macpaul Lin (林智斌),
	matthias.bgg, davem, linux-arm-kernel, angelogioacchino.delregno

> ************* MEDIATEK Confidentiality Notice ********************
> The information contained in this e-mail message (including any
> attachments) may be confidential, proprietary, privileged, or otherwise
> exempt from disclosure under applicable laws. It is intended to be
> conveyed only to the designated recipient(s). Any use, dissemination,
> distribution, printing, retaining or copying of this e-mail (including its
> attachments) by unintended recipient(s) is strictly prohibited and may
> be unlawful. If you are not an intended recipient of this e-mail, or believe
> that you have received this e-mail in error, please notify the sender
> immediately (by replying to this e-mail), delete any and all copies of
> this e-mail (including any attachments) from your system, and do not
> disclose the content of this e-mail to any other person. Thank you!

Please remove this. You are knowingly posting to a public mailing
list, which is publicly archived, etc.

      Andrew



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v5 1/2] stmmac: dwmac-mediatek: enable 2ns delay only for special cases
  2022-12-28 16:00           ` Andrew Lunn
@ 2022-12-29  0:37             ` Biao Huang (黄彪)
  0 siblings, 0 replies; 14+ messages in thread
From: Biao Huang (黄彪) @ 2022-12-29  0:37 UTC (permalink / raw)
  To: andrew
  Cc: linux-kernel, linux-mediatek, robh+dt, peppe.cavallaro, joabreu,
	devicetree, kuba, edumazet, alexandre.torgue, mcoquelin.stm32,
	pabeni, richardcochran, krzysztof.kozlowski+dt,
	Macpaul Lin (林智斌),
	matthias.bgg, davem, linux-arm-kernel, angelogioacchino.delregno

On Wed, 2022-12-28 at 17:00 +0100, Andrew Lunn wrote:
> > ************* MEDIATEK Confidentiality Notice ********************
> > The information contained in this e-mail message (including any
> > attachments) may be confidential, proprietary, privileged, or
> > otherwise
> > exempt from disclosure under applicable laws. It is intended to be
> > conveyed only to the designated recipient(s). Any use,
> > dissemination,
> > distribution, printing, retaining or copying of this e-mail
> > (including its
> > attachments) by unintended recipient(s) is strictly prohibited and
> > may
> > be unlawful. If you are not an intended recipient of this e-mail,
> > or believe
> > that you have received this e-mail in error, please notify the
> > sender
> > immediately (by replying to this e-mail), delete any and all copies
> > of
> > this e-mail (including any attachments) from your system, and do
> > not
> > disclose the content of this e-mail to any other person. Thank you!
> 
> Please remove this. You are knowingly posting to a public mailing
> list, which is publicly archived, etc.
OK, I reply with the wrong mailbox, sorry for that.
> 
>       Andrew
> 
Best Regards!
Biao
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2022-12-29  0:37 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-23  1:50 [PATCH v5 0/2] arm64: dts: mt8195: Add Ethernet controller Biao Huang
2022-12-23  1:50 ` [PATCH v5 1/2] stmmac: dwmac-mediatek: enable 2ns delay only for special cases Biao Huang
2022-12-23  9:24   ` AngeloGioacchino Del Regno
2022-12-23 14:18   ` Andrew Lunn
2022-12-26  3:50     ` Biao Huang (黄彪)
2022-12-26 15:19       ` Andrew Lunn
2022-12-27  7:09         ` Biao Huang (黄彪)
2022-12-27 16:20           ` Andrew Lunn
2022-12-28  6:22             ` Biao Huang (黄彪)
2022-12-28 16:00           ` Andrew Lunn
2022-12-29  0:37             ` Biao Huang (黄彪)
2022-12-23  1:50 ` [PATCH v5 2/2] arm64: dts: mt8195: Add Ethernet controller Biao Huang
2022-12-23  9:24   ` AngeloGioacchino Del Regno
2022-12-28  6:23     ` Biao Huang (黄彪)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).