[PATCH RFC V2 0/6] ARM: Enable GENET support for RPi 4

[PATCH RFC V2 0/6] ARM: Enable GENET support for RPi 4

[Stefan Wahren]

Raspberry Pi 4 uses the broadcom genet chip in version five.
This chip has a dma controller integrated. Up to now the maximal
burst size was hard-coded to 0x10. But it turns out that Raspberry Pi 4
does only work with the smaller maximal burst size of 0x8.

This series based on Matthias Brugger's V1 series [1], should help
to put all missing pieces together and hand my changes over to him.
So i doesn't address all comments from Florian and the series isn't
ready to be merged.

[1] - https://patchwork.kernel.org/cover/11186193/

Changes in V2:
- add 2 fixes for IRQ retrieval
- add support for missing PHY modes
- declare PHY mode RGMII RXID based on the default settings
- add alias to allow firmware append the MAC address

Matthias Brugger (1):
  net: bcmgenet: use optional max DMA burst size property

Stefan Wahren (5):
  net: bcmgenet: Fix error handling on IRQ retrieval
  net: bcmgenet: Avoid touching non-existent interrupt
  net: bcmgenet: Refactor register access in bcmgenet_mii_config
  net: bcmgenet: Add RGMII_RXID and RGMII_ID support
  ARM: dts: bcm2711-rpi-4: Enable GENET support

 arch/arm/boot/dts/bcm2711-rpi-4-b.dts          | 23 ++++++++++++
 arch/arm/boot/dts/bcm2711.dtsi                 | 19 ++++++++++
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 25 ++++++++++---
 drivers/net/ethernet/broadcom/genet/bcmgenet.h |  1 +
 drivers/net/ethernet/broadcom/genet/bcmmii.c   | 51 ++++++++++++++------------
 5 files changed, 90 insertions(+), 29 deletions(-)

--
2.7.4

[PATCH RFC V2 1/6] net: bcmgenet: Fix error handling on IRQ retrieval

[Stefan Wahren]

This fixes the error handling for the mandatory IRQs. There is no need
for the error message anymore, this is now handled by platform_get_irq.

Fixes: 1c1008c793fa ("net: bcmgenet: add main driver file")
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 4f689fb..105b3be 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -3464,13 +3464,16 @@ static int bcmgenet_probe(struct platform_device *pdev)

 	priv = netdev_priv(dev);
 	priv->irq0 = platform_get_irq(pdev, 0);
+	if (priv->irq0 < 0) {
+		err = priv->irq0;
+		goto err;
+	}
 	priv->irq1 = platform_get_irq(pdev, 1);
-	priv->wol_irq = platform_get_irq(pdev, 2);
-	if (!priv->irq0 || !priv->irq1) {
-		dev_err(&pdev->dev, "can't find IRQs\n");
-		err = -EINVAL;
+	if (priv->irq1 < 0) {
+		err = priv->irq1;
 		goto err;
 	}
+	priv->wol_irq = platform_get_irq(pdev, 2);

 	if (dn)
 		macaddr = of_get_mac_address(dn);
--
2.7.4

[PATCH RFC V2 2/6] net: bcmgenet: Avoid touching non-existent interrupt

[Stefan Wahren]

As platform_get_irq() now prints an error when the interrupt does not
exist, we are getting a confusing error message in case the optional
WOL IRQ is not defined:

  bcmgenet fd58000.ethernet: IRQ index 2 not found

Fix this by using the platform_irq_count() helper to avoid touching a
non-existent interrupt.

Fixes: 7723f4c5ecdb8d83 ("driver core: platform: Add an error message to platform_get_irq*()")
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 105b3be..ac554a6 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -3473,7 +3473,8 @@ static int bcmgenet_probe(struct platform_device *pdev)
 		err = priv->irq1;
 		goto err;
 	}
-	priv->wol_irq = platform_get_irq(pdev, 2);
+	if (platform_irq_count(pdev) > 2)
+		priv->wol_irq = platform_get_irq(pdev, 2);

 	if (dn)
 		macaddr = of_get_mac_address(dn);
--
2.7.4

[PATCH RFC V2 3/6] net: bcmgenet: use optional max DMA burst size property

[Stefan Wahren]

From: Matthias Brugger <mbrugger@suse.com>

Depending on the HW, the maximal usable DMA burst size can vary.
If not set accordingly a timeout in the transmit queue happens and no
package can be sent. Read to optional max-burst-sz property, if not
present, fallback to the standard value.

Signed-off-by: Matthias Brugger <mbrugger@suse.com>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 13 +++++++++++--
 drivers/net/ethernet/broadcom/genet/bcmgenet.h |  1 +
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index ac554a6..8d0093b 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -2578,7 +2578,8 @@ static int bcmgenet_init_dma(struct bcmgenet_priv *priv)
 	}

 	/* Init rDma */
-	bcmgenet_rdma_writel(priv, DMA_MAX_BURST_LENGTH, DMA_SCB_BURST_SIZE);
+	bcmgenet_rdma_writel(priv, priv->dma_max_burst_length,
+			     DMA_SCB_BURST_SIZE);

 	/* Initialize Rx queues */
 	ret = bcmgenet_init_rx_queues(priv->dev);
@@ -2591,7 +2592,8 @@ static int bcmgenet_init_dma(struct bcmgenet_priv *priv)
 	}

 	/* Init tDma */
-	bcmgenet_tdma_writel(priv, DMA_MAX_BURST_LENGTH, DMA_SCB_BURST_SIZE);
+	bcmgenet_tdma_writel(priv, priv->dma_max_burst_length,
+			     DMA_SCB_BURST_SIZE);

 	/* Initialize Tx queues */
 	bcmgenet_init_tx_queues(priv->dev);
@@ -3536,6 +3538,13 @@ static int bcmgenet_probe(struct platform_device *pdev)

 	clk_prepare_enable(priv->clk);

+	if (dn) {
+		of_property_read_u32(dn, "dma-burst-sz",
+				     &priv->dma_max_burst_length);
+	} else {
+		priv->dma_max_burst_length = DMA_MAX_BURST_LENGTH;
+	}
+
 	bcmgenet_set_hw_params(priv);

 	/* Mii wait queue */
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
index 7fbf573..22cde8c 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
@@ -664,6 +664,7 @@ struct bcmgenet_priv {
 	bool crc_fwd_en;

 	unsigned int dma_rx_chk_bit;
+	unsigned int dma_max_burst_length;

 	u32 msg_enable;

--
2.7.4

[PATCH RFC V2 4/6] net: bcmgenet: Refactor register access in bcmgenet_mii_config

[Stefan Wahren]

The register access in bcmgenet_mii_config() is a little bit opaque and
not easy to extend. In preparation for the missing RGMII PHY modes
move the real register access to the end of the function. This make
the code easier to read and extend.

Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
 drivers/net/ethernet/broadcom/genet/bcmmii.c | 41 ++++++++++++----------------
 1 file changed, 18 insertions(+), 23 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index 17bb8d6..8f7b2c0 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -223,9 +223,6 @@ int bcmgenet_mii_config(struct net_device *dev, bool init)
 	u32 port_ctrl;
 	u32 reg;

-	priv->ext_phy = !priv->internal_phy &&
-			(priv->phy_interface != PHY_INTERFACE_MODE_MOCA);
-
 	switch (priv->phy_interface) {
 	case PHY_INTERFACE_MODE_INTERNAL:
 	case PHY_INTERFACE_MODE_MOCA:
@@ -238,22 +235,12 @@ int bcmgenet_mii_config(struct net_device *dev, bool init)
 			port_ctrl = PORT_MODE_INT_GPHY;
 		else
 			port_ctrl = PORT_MODE_INT_EPHY;
-
-		bcmgenet_sys_writel(priv, port_ctrl, SYS_PORT_CTRL);
-
-		if (priv->internal_phy) {
-			phy_name = "internal PHY";
-		} else if (priv->phy_interface == PHY_INTERFACE_MODE_MOCA) {
-			phy_name = "MoCA";
-			bcmgenet_moca_phy_setup(priv);
-		}
 		break;

 	case PHY_INTERFACE_MODE_MII:
 		phy_name = "external MII";
 		phy_set_max_speed(phydev, SPEED_100);
-		bcmgenet_sys_writel(priv,
-				    PORT_MODE_EXT_EPHY, SYS_PORT_CTRL);
+		port_ctrl = PORT_MODE_EXT_EPHY;
 		break;

 	case PHY_INTERFACE_MODE_REVMII:
@@ -268,31 +255,34 @@ int bcmgenet_mii_config(struct net_device *dev, bool init)
 			port_ctrl = PORT_MODE_EXT_RVMII_50;
 		else
 			port_ctrl = PORT_MODE_EXT_RVMII_25;
-		bcmgenet_sys_writel(priv, port_ctrl, SYS_PORT_CTRL);
 		break;

 	case PHY_INTERFACE_MODE_RGMII:
 		/* RGMII_NO_ID: TXC transitions at the same time as TXD
 		 *		(requires PCB or receiver-side delay)
-		 * RGMII:	Add 2ns delay on TXC (90 degree shift)
 		 *
 		 * ID is implicitly disabled for 100Mbps (RG)MII operation.
 		 */
+		phy_name = "external RGMII (no delay)";
 		id_mode_dis = BIT(16);
-		/* fall through */
+		port_ctrl = PORT_MODE_EXT_GPHY;
+		break;
+
 	case PHY_INTERFACE_MODE_RGMII_TXID:
-		if (id_mode_dis)
-			phy_name = "external RGMII (no delay)";
-		else
-			phy_name = "external RGMII (TX delay)";
-		bcmgenet_sys_writel(priv,
-				    PORT_MODE_EXT_GPHY, SYS_PORT_CTRL);
+		/* RGMII_TXID:	Add 2ns delay on TXC (90 degree shift) */
+		phy_name = "external RGMII (TX delay)";
+		port_ctrl = PORT_MODE_EXT_GPHY;
 		break;
 	default:
 		dev_err(kdev, "unknown phy mode: %d\n", priv->phy_interface);
 		return -EINVAL;
 	}

+	bcmgenet_sys_writel(priv, port_ctrl, SYS_PORT_CTRL);
+
+	priv->ext_phy = !priv->internal_phy &&
+			(priv->phy_interface != PHY_INTERFACE_MODE_MOCA);
+
 	/* This is an external PHY (xMII), so we need to enable the RGMII
 	 * block for the interface to work
 	 */
@@ -304,6 +294,11 @@ int bcmgenet_mii_config(struct net_device *dev, bool init)
 		else
 			reg |= RGMII_MODE_EN;
 		bcmgenet_ext_writel(priv, reg, EXT_RGMII_OOB_CTRL);
+	} else if (priv->internal_phy) {
+		phy_name = "internal PHY";
+	} else if (priv->phy_interface == PHY_INTERFACE_MODE_MOCA) {
+		phy_name = "MoCA";
+		bcmgenet_moca_phy_setup(priv);
 	}

 	if (init) {
--
2.7.4

[PATCH RFC V2 5/6] net: bcmgenet: Add RGMII_RXID and RGMII_ID support

[Stefan Wahren]

This adds the missing support for the PHY modes RGMII_RXID and
RGMII_ID. This is necessary for the Raspberry Pi 4.

Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
 drivers/net/ethernet/broadcom/genet/bcmmii.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index 8f7b2c0..9091e5b 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -273,6 +273,16 @@ int bcmgenet_mii_config(struct net_device *dev, bool init)
 		phy_name = "external RGMII (TX delay)";
 		port_ctrl = PORT_MODE_EXT_GPHY;
 		break;
+
+	case PHY_INTERFACE_MODE_RGMII_RXID:
+		phy_name = "external RGMII (RX delay)";
+		port_ctrl = PORT_MODE_EXT_GPHY;
+		break;
+
+	case PHY_INTERFACE_MODE_RGMII_ID:
+		phy_name = "external RGMII (RX/TX delay)";
+		port_ctrl = PORT_MODE_EXT_GPHY;
+		break;
 	default:
 		dev_err(kdev, "unknown phy mode: %d\n", priv->phy_interface);
 		return -EINVAL;
--
2.7.4

[PATCH RFC V2 6/6] ARM: dts: bcm2711-rpi-4: Enable GENET support

[Stefan Wahren]

This enables the Gigabit Ethernet support on the Raspberry Pi 4.
The defined PHY mode is equivalent to the default register settings
in the downstream tree.

Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
Signed-off-by: Matthias Brugger <mbrugger@suse.com>
---
 arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 23 +++++++++++++++++++++++
 arch/arm/boot/dts/bcm2711.dtsi        | 19 +++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
index cccc1cc..904efe1 100644
--- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
+++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
@@ -19,6 +19,10 @@
 		reg = <0 0 0>;
 	};

+	aliases {
+		ethernet0 = &genet;
+	};
+
 	leds {
 		act {
 			gpios = <&gpio 42 GPIO_ACTIVE_HIGH>;
@@ -97,6 +101,25 @@
 	status = "okay";
 };

+&genet {
+	phy-handle = <&phy1>;
+	phy-mode = "rgmii-rxid";
+	status = "okay";
+
+	mdio@e14 {
+		compatible = "brcm,genet-mdio-v5";
+		reg = <0xe14 0x8>;
+		reg-names = "mdio";
+		#address-cells = <0x0>;
+		#size-cells = <0x1>;
+
+		phy1: ethernet-phy@1 {
+			/* No PHY interrupt */
+			reg = <0x1>;
+		};
+	};
+};
+
 /* uart0 communicates with the BT module */
 &uart0 {
 	pinctrl-names = "default";
diff --git a/arch/arm/boot/dts/bcm2711.dtsi b/arch/arm/boot/dts/bcm2711.dtsi
index ac83dac..ff24396 100644
--- a/arch/arm/boot/dts/bcm2711.dtsi
+++ b/arch/arm/boot/dts/bcm2711.dtsi
@@ -305,6 +305,25 @@
 			cpu-release-addr = <0x0 0x000000f0>;
 		};
 	};
+
+	scb {
+		compatible = "simple-bus";
+		#address-cells = <2>;
+		#size-cells = <1>;
+
+		ranges = <0x0 0x7c000000  0x0 0xfc000000  0x03800000>;
+
+		genet: ethernet@7d580000 {
+			compatible = "brcm,genet-v5";
+			reg = <0x0 0x7d580000 0x10000>;
+			#address-cells = <0x1>;
+			#size-cells = <0x1>;
+			interrupts = <GIC_SPI 157 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 158 IRQ_TYPE_LEVEL_HIGH>;
+			dma-burst-sz = <0x08>;
+			status = "disabled";
+		};
+	};
 };

 &clk_osc {
--
2.7.4

Re: [PATCH RFC V2 3/6] net: bcmgenet: use optional max DMA burst size property

[Florian Fainelli]

On 11/2/2019 6:41 AM, Stefan Wahren wrote:
> From: Matthias Brugger <mbrugger@suse.com>
> 
> Depending on the HW, the maximal usable DMA burst size can vary.
> If not set accordingly a timeout in the transmit queue happens and no
> package can be sent. Read to optional max-burst-sz property, if not
> present, fallback to the standard value.
> 
> Signed-off-by: Matthias Brugger <mbrugger@suse.com>

Missing your Signed-off-by here since you are carrying this patch from
Matthias.

> ---

[snip]

> +	if (dn) {
> +		of_property_read_u32(dn, "dma-burst-sz",
> +				     &priv->dma_max_burst_length);
> +	} else {
> +		priv->dma_max_burst_length = DMA_MAX_BURST_LENGTH;
> +	}

I would maintain the previous position provided on Matthias' patch
series, which is the integration of the GENETv5 hardware block in 2711
is done in a way that is different enough (due to the SCB/AXI bridge)
that a separate compatibility string would be in order. Once you that
defined that "brcm,bcm2711-genet-v5" compatibility string defined, you
can derive the DMA burst size off of it.

If adding a compatibility string is not practical because of the
downstream DTBs, then can we at least fix this patch in two ways:

- define the property in the binding document
- spell out the property in full names: max-dma-burst-size so as to
reflect what it does

Thanks!
-- 
Florian

Re: [PATCH RFC V2 2/6] net: bcmgenet: Avoid touching non-existent interrupt

[Florian Fainelli]

On 11/2/2019 6:41 AM, Stefan Wahren wrote:
> As platform_get_irq() now prints an error when the interrupt does not
> exist, we are getting a confusing error message in case the optional
> WOL IRQ is not defined:
> 
>   bcmgenet fd58000.ethernet: IRQ index 2 not found
> 
> Fix this by using the platform_irq_count() helper to avoid touching a
> non-existent interrupt.
> 
> Fixes: 7723f4c5ecdb8d83 ("driver core: platform: Add an error message to platform_get_irq*()")
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
> ---
>  drivers/net/ethernet/broadcom/genet/bcmgenet.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index 105b3be..ac554a6 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -3473,7 +3473,8 @@ static int bcmgenet_probe(struct platform_device *pdev)
>  		err = priv->irq1;
>  		goto err;
>  	}
> -	priv->wol_irq = platform_get_irq(pdev, 2);
> +	if (platform_irq_count(pdev) > 2)
> +		priv->wol_irq = platform_get_irq(pdev, 2);

Or you could use platform_get_irq_optional() for the WoL IRQ line?
-- 
Florian

Re: [PATCH RFC V2 1/6] net: bcmgenet: Fix error handling on IRQ retrieval

[Florian Fainelli]

On 11/2/2019 6:41 AM, Stefan Wahren wrote:
> This fixes the error handling for the mandatory IRQs. There is no need
> for the error message anymore, this is now handled by platform_get_irq.
> 
> Fixes: 1c1008c793fa ("net: bcmgenet: add main driver file")
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>

Not sure if the Fixes tag is necessary here, this is kind of an
exceptional case anyway since you should be specifying valid interrupt
resources to begin with.
-- 
Florian

Re: [PATCH RFC V2 5/6] net: bcmgenet: Add RGMII_RXID and RGMII_ID support

[Florian Fainelli]

On 11/2/2019 6:41 AM, Stefan Wahren wrote:
> This adds the missing support for the PHY modes RGMII_RXID and
> RGMII_ID. This is necessary for the Raspberry Pi 4.

Are both used? Your next patch only uses "rgmii-rxid". Can you remind me
of the GTXC and RXC skew setting the PHY you are using comes up with?
And this was tested with Broadcom PHY (drivers/net/phy/broadcom.c)
driver or with the Generic PHY driver?
-- 
Florian

Re: [PATCH RFC V2 6/6] ARM: dts: bcm2711-rpi-4: Enable GENET support

[Florian Fainelli]

On 11/2/2019 6:41 AM, Stefan Wahren wrote:
> This enables the Gigabit Ethernet support on the Raspberry Pi 4.
> The defined PHY mode is equivalent to the default register settings
> in the downstream tree.
> 
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
> ---
>  arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 23 +++++++++++++++++++++++
>  arch/arm/boot/dts/bcm2711.dtsi        | 19 +++++++++++++++++++
>  2 files changed, 42 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> index cccc1cc..904efe1 100644
> --- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> +++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> @@ -19,6 +19,10 @@
>  		reg = <0 0 0>;
>  	};
> 
> +	aliases {
> +		ethernet0 = &genet;
> +	};
> +
>  	leds {
>  		act {
>  			gpios = <&gpio 42 GPIO_ACTIVE_HIGH>;
> @@ -97,6 +101,25 @@
>  	status = "okay";
>  };
> 
> +&genet {
> +	phy-handle = <&phy1>;
> +	phy-mode = "rgmii-rxid";
> +	status = "okay";
> +
> +	mdio@e14 {
> +		compatible = "brcm,genet-mdio-v5";
> +		reg = <0xe14 0x8>;
> +		reg-names = "mdio";
> +		#address-cells = <0x0>;
> +		#size-cells = <0x1>;

The MDIO controller is always present, therefore its node definition
would be better placed in bcm2711.dtsi. Other than that:

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH RFC V2 5/6] net: bcmgenet: Add RGMII_RXID and RGMII_ID support

[Stefan Wahren]

Am 02.11.19 um 21:00 schrieb Florian Fainelli:
>
> On 11/2/2019 6:41 AM, Stefan Wahren wrote:
>> This adds the missing support for the PHY modes RGMII_RXID and
>> RGMII_ID. This is necessary for the Raspberry Pi 4.
> Are both used?
Sorry, the wording was a little bit unlucky. Only the RGMII_RXID is used.
>  Your next patch only uses "rgmii-rxid". Can you remind me
> of the GTXC and RXC skew setting the PHY you are using comes up with?

I dumped the following register values before bcm54xx_config_clock_delay
tries to change the values:
SHDWSEL_MISC: 0x71E7
SHD_CLK_CTL: 0x0100

https://github.com/raspberrypi/linux/issues/3101#issuecomment-544266615

> And this was tested with Broadcom PHY (drivers/net/phy/broadcom.c)
> driver or with the Generic PHY driver?

After applying the series i'm getting the following output:

[    5.431262] bcmgenet fd580000.ethernet: failed to get enet clock
[    5.431271] bcmgenet fd580000.ethernet: GENET 5.0 EPHY: 0x0000
[    5.431283] bcmgenet fd580000.ethernet: failed to get enet-wol clock
[    5.431294] bcmgenet fd580000.ethernet: failed to get enet-eee clock
[    5.880660] libphy: bcmgenet MII bus: probed
[    5.970751] unimac-mdio unimac-mdio.-19: Broadcom UniMAC MDIO bus
[   11.570672] bcmgenet fd580000.ethernet: configuring instance for
external RGMII (RX delay)
[   11.583231] bcmgenet fd580000.ethernet eth0: Link is Down
[   16.811045] bcmgenet fd580000.ethernet eth0: Link is Up - 1Gbps/Full
- flow control rx/tx

So i assume it's the Broadcom PHY.

Regards
Stefan

Re: [PATCH RFC V2 2/6] net: bcmgenet: Avoid touching non-existent interrupt

[Stefan Wahren]

Am 02.11.19 um 20:34 schrieb Florian Fainelli:
> On 11/2/2019 6:41 AM, Stefan Wahren wrote:
>> As platform_get_irq() now prints an error when the interrupt does not
>> exist, we are getting a confusing error message in case the optional
>> WOL IRQ is not defined:
>>
>>   bcmgenet fd58000.ethernet: IRQ index 2 not found
>>
>> Fix this by using the platform_irq_count() helper to avoid touching a
>> non-existent interrupt.
>>
>> Fixes: 7723f4c5ecdb8d83 ("driver core: platform: Add an error message to platform_get_irq*()")
>> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
>> ---
>>  drivers/net/ethernet/broadcom/genet/bcmgenet.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>> index 105b3be..ac554a6 100644
>> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>> @@ -3473,7 +3473,8 @@ static int bcmgenet_probe(struct platform_device *pdev)
>>  		err = priv->irq1;
>>  		goto err;
>>  	}
>> -	priv->wol_irq = platform_get_irq(pdev, 2);
>> +	if (platform_irq_count(pdev) > 2)
>> +		priv->wol_irq = platform_get_irq(pdev, 2);
> Or you could use platform_get_irq_optional() for the WoL IRQ line?
Yes, this would be better.

Re: [PATCH RFC V2 3/6] net: bcmgenet: use optional max DMA burst size property

[Stefan Wahren]

Hi Florian,

Am 02.11.19 um 20:34 schrieb Florian Fainelli:
> On 11/2/2019 6:41 AM, Stefan Wahren wrote:
>> From: Matthias Brugger <mbrugger@suse.com>
>>
>> Depending on the HW, the maximal usable DMA burst size can vary.
>> If not set accordingly a timeout in the transmit queue happens and no
>> package can be sent. Read to optional max-burst-sz property, if not
>> present, fallback to the standard value.
>>
>> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
> Missing your Signed-off-by here since you are carrying this patch from
> Matthias.
oops
>
>> ---
> [snip]
>
>> +	if (dn) {
>> +		of_property_read_u32(dn, "dma-burst-sz",
>> +				     &priv->dma_max_burst_length);
>> +	} else {
>> +		priv->dma_max_burst_length = DMA_MAX_BURST_LENGTH;
>> +	}
> I would maintain the previous position provided on Matthias' patch
> series, which is the integration of the GENETv5 hardware block in 2711
> is done in a way that is different enough (due to the SCB/AXI bridge)
> that a separate compatibility string would be in order. Once you that
> defined that "brcm,bcm2711-genet-v5" compatibility string defined, you
> can derive the DMA burst size off of it.
this is what i meant with didn't address all your comments. I'm fine
with your suggestion, but too lazy to integrate it in this patch series.
I assumed Matthias already take care of it.
> If adding a compatibility string is not practical because of the
> downstream DTBs, then can we at least fix this patch in two ways:
>
> - define the property in the binding document
> - spell out the property in full names: max-dma-burst-size so as to
> reflect what it does

In case of incompatibilities with the downstream DTB, i will take care
of it in the downstream tree like last time.

Regards
Stefan

>
> Thanks!