Re: [PATCH v1 3/3] ARM: dts: bcm2711: Enable GENET support for the RPi4

Florian Fainelli
2019-10-11T19:32:09Z

On 10/11/19 11:48 AM, matthias.bgg@kernel.org wrote:
> From: Matthias Brugger <mbrugger@suse.com>
>
> Enable Gigabit Ethernet support on the Raspberry Pi 4
> Model B.
>
> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
>
> ---
>
> arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 22 ++++++++++++++++++++++
> arch/arm/boot/dts/bcm2711.dtsi | 18 ++++++++++++++++++
> 2 files changed, 40 insertions(+)
>
> diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> index cccc1ccd19be..958553d62670 100644
> --- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> +++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> @@ -97,6 +97,28 @@
> status = "okay";
> };
>
> +&genet {
> + phy-handle = <&phy1>;
> + phy-mode = "rgmii";

Can you check that things still work against David Miller's net-next?
Tree, in particular the BCM54213PE PHY might be matched by the BCM54210E
entry in drivers/net/phy/broadcom.c and I just fixed an issue in how
RGMII delays were configured:

https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/commit/?id=fea7fda7f50a6059220f83251e70709e45cc8040

This might require you to change the 'phy-mode' property to what is
appropriate.

> + status = "okay";
> + dma-burst-sz = <0x08>;
> +
> + mdio@e14 {
> + compatible = "brcm,genet-mdio-v5";
> + reg = <0xe14 0x8>;
> + reg-names = "mdio";
> + #address-cells = <0x0>;
> + #size-cells = <0x1>;
> +
> + phy1: ethernet-phy@1 {
> + compatible = "ethernet-phy-ieee802.3-c22";

This does not hurt, but this compatibility string is not required.

> + /* No PHY interrupt */
> + max-speed = <1000>;

And this property is not required either, since the PHY library will
determine the PHY's capabilities.

Other than those patches, I believe you also need something like this
(inspired by the Rpi downstream patch):

diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c
b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index 970e478a9017..94d1dd5d56bf 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -273,11 +273,12 @@ int bcmgenet_mii_probe(struct net_device *dev)
struct bcmgenet_priv *priv = netdev_priv(dev);
struct device_node *dn = priv->pdev->dev.of_node;
struct phy_device *phydev;
- u32 phy_flags;
+ u32 phy_flags = 0;
int ret;

/* Communicate the integrated PHY revision */
- phy_flags = priv->gphy_rev;
+ if (priv->internal_phy)
+ phy_flags = priv->gphy_rev;

/* Initialize link state variables that bcmgenet_mii_setup() uses */
priv->old_link = -1;

to prevent the internal PHY revision, which we stick into
phydev->dev_flags from incorrectly setting bits in
drivers/net/phy/broadcom.c. I will probably send this as a fix in the
next few hours.
--
Florian