* [PATCH v1 0/3] This series adds ethernet support for RPi4.
@ 2019-10-11 18:48 matthias.bgg
2019-10-11 18:48 ` [PATCH v1 1/3] dt-bindings: net: bcmgenet add property for max DMA burst size matthias.bgg
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: matthias.bgg @ 2019-10-11 18:48 UTC (permalink / raw)
To: Florian Fainelli, David S . Miller
Cc: Nicolas Saenz Julienne, Matthias Brugger, linux-rpi-kernel,
linux-arm-kernel, Stefan Wahren, Matthias Brugger, Doug Berger,
Eric Anholt, Mark Rutland, Rob Herring, bcm-kernel-feedback-list,
devicetree, linux-kernel, netdev
From: Matthias Brugger <mbrugger@suse.com>
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 adds a new optional property to the driver, dma-burst-sz.
The very same property is already used by another drivers in the kernel.
Matthias Brugger (3):
dt-bindings: net: bcmgenet add property for max DMA burst size
net: bcmgenet: use optional max DMA burst size property
ARM: dts: bcm2711: Enable GENET support for the RPi4
.../devicetree/bindings/net/brcm,bcmgenet.txt | 2 ++
arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 22 +++++++++++++++++++
arch/arm/boot/dts/bcm2711.dtsi | 18 +++++++++++++++
.../net/ethernet/broadcom/genet/bcmgenet.c | 13 +++++++++--
.../net/ethernet/broadcom/genet/bcmgenet.h | 1 +
5 files changed, 54 insertions(+), 2 deletions(-)
--
2.23.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v1 1/3] dt-bindings: net: bcmgenet add property for max DMA burst size 2019-10-11 18:48 [PATCH v1 0/3] This series adds ethernet support for RPi4 matthias.bgg @ 2019-10-11 18:48 ` matthias.bgg 2019-10-11 19:07 ` Florian Fainelli 2019-10-11 18:48 ` [PATCH v1 2/3] net: bcmgenet: use optional max DMA burst size property matthias.bgg 2019-10-11 18:48 ` [PATCH v1 3/3] ARM: dts: bcm2711: Enable GENET support for the RPi4 matthias.bgg 2 siblings, 1 reply; 14+ messages in thread From: matthias.bgg @ 2019-10-11 18:48 UTC (permalink / raw) To: Florian Fainelli, David S . Miller Cc: Nicolas Saenz Julienne, Matthias Brugger, linux-rpi-kernel, linux-arm-kernel, Stefan Wahren, Matthias Brugger, Mark Rutland, Rob Herring, devicetree, linux-kernel, netdev From: Matthias Brugger <mbrugger@suse.com> The maximal usable DMA burst size can vary in different SoCs. Add a optional property to configure the DMA channels properly. Signed-off-by: Matthias Brugger <mbrugger@suse.com> --- Documentation/devicetree/bindings/net/brcm,bcmgenet.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/net/brcm,bcmgenet.txt b/Documentation/devicetree/bindings/net/brcm,bcmgenet.txt index 3956af1d30f3..10a7169ec902 100644 --- a/Documentation/devicetree/bindings/net/brcm,bcmgenet.txt +++ b/Documentation/devicetree/bindings/net/brcm,bcmgenet.txt @@ -30,6 +30,8 @@ Optional properties: See Documentation/devicetree/bindings/net/fixed-link.txt for information on the property specifics +- dma-burst-sz: Maximal length of a burst. + Required child nodes: - mdio bus node: this node should always be present regardless of the PHY -- 2.23.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v1 1/3] dt-bindings: net: bcmgenet add property for max DMA burst size 2019-10-11 18:48 ` [PATCH v1 1/3] dt-bindings: net: bcmgenet add property for max DMA burst size matthias.bgg @ 2019-10-11 19:07 ` Florian Fainelli 0 siblings, 0 replies; 14+ messages in thread From: Florian Fainelli @ 2019-10-11 19:07 UTC (permalink / raw) To: matthias.bgg, David S . Miller Cc: Nicolas Saenz Julienne, linux-rpi-kernel, linux-arm-kernel, Stefan Wahren, Matthias Brugger, Mark Rutland, Rob Herring, devicetree, linux-kernel, netdev On 10/11/19 11:48 AM, matthias.bgg@kernel.org wrote: > From: Matthias Brugger <mbrugger@suse.com> > > The maximal usable DMA burst size can vary in different SoCs. > Add a optional property to configure the DMA channels properly. > > Signed-off-by: Matthias Brugger <mbrugger@suse.com> There are enough differences in the integration of GENET in 2711 (versus any other STB chip where it has been used) that a brcm,bcm2711-genet-v5 compatible string would be in order. From there, you can derive the DMA burst size without using additional properties. -- Florian ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v1 2/3] net: bcmgenet: use optional max DMA burst size property 2019-10-11 18:48 [PATCH v1 0/3] This series adds ethernet support for RPi4 matthias.bgg 2019-10-11 18:48 ` [PATCH v1 1/3] dt-bindings: net: bcmgenet add property for max DMA burst size matthias.bgg @ 2019-10-11 18:48 ` matthias.bgg 2019-10-11 19:01 ` Nicolas Saenz Julienne 2019-10-11 18:48 ` [PATCH v1 3/3] ARM: dts: bcm2711: Enable GENET support for the RPi4 matthias.bgg 2 siblings, 1 reply; 14+ messages in thread From: matthias.bgg @ 2019-10-11 18:48 UTC (permalink / raw) To: Florian Fainelli, David S . Miller Cc: Nicolas Saenz Julienne, Matthias Brugger, linux-rpi-kernel, linux-arm-kernel, Stefan Wahren, Matthias Brugger, Doug Berger, bcm-kernel-feedback-list, linux-kernel, netdev 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 12cb77ef1081..a7bb822a6d83 100644 --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c @@ -2576,7 +2576,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); @@ -2589,7 +2590,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); @@ -3522,6 +3524,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 4a8fc03d82fd..897f356eb376 100644 --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h @@ -663,6 +663,7 @@ struct bcmgenet_priv { bool crc_fwd_en; unsigned int dma_rx_chk_bit; + unsigned int dma_max_burst_length; u32 msg_enable; -- 2.23.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v1 2/3] net: bcmgenet: use optional max DMA burst size property 2019-10-11 18:48 ` [PATCH v1 2/3] net: bcmgenet: use optional max DMA burst size property matthias.bgg @ 2019-10-11 19:01 ` Nicolas Saenz Julienne 0 siblings, 0 replies; 14+ messages in thread From: Nicolas Saenz Julienne @ 2019-10-11 19:01 UTC (permalink / raw) To: matthias.bgg, Florian Fainelli, David S . Miller Cc: linux-rpi-kernel, linux-arm-kernel, Stefan Wahren, Matthias Brugger, Doug Berger, bcm-kernel-feedback-list, linux-kernel, netdev [-- Attachment #1: Type: text/plain, Size: 2816 bytes --] Hi Matthias! One small comment, I'll test everything later on. On Fri, 2019-10-11 at 20:48 +0200, matthias.bgg@kernel.org 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> > --- > > 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 12cb77ef1081..a7bb822a6d83 100644 > --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c > +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c > @@ -2576,7 +2576,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); > @@ -2589,7 +2590,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); > @@ -3522,6 +3524,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); You set the 'dma-burst-sz' binding as optional, though this assumes that if a device node is available dma_max_burst_length comes from the device tree. I think you should check of_property_read_u32's return value and on failure default to 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 4a8fc03d82fd..897f356eb376 100644 > --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h > +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h > @@ -663,6 +663,7 @@ struct bcmgenet_priv { > bool crc_fwd_en; > > unsigned int dma_rx_chk_bit; > + unsigned int dma_max_burst_length; > > u32 msg_enable; > Regards, Nicolas [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v1 3/3] ARM: dts: bcm2711: Enable GENET support for the RPi4 2019-10-11 18:48 [PATCH v1 0/3] This series adds ethernet support for RPi4 matthias.bgg 2019-10-11 18:48 ` [PATCH v1 1/3] dt-bindings: net: bcmgenet add property for max DMA burst size matthias.bgg 2019-10-11 18:48 ` [PATCH v1 2/3] net: bcmgenet: use optional max DMA burst size property matthias.bgg @ 2019-10-11 18:48 ` matthias.bgg 2019-10-11 19:31 ` Florian Fainelli 2019-10-11 23:09 ` Stefan Wahren 2 siblings, 2 replies; 14+ messages in thread From: matthias.bgg @ 2019-10-11 18:48 UTC (permalink / raw) To: Florian Fainelli, David S . Miller Cc: Nicolas Saenz Julienne, Matthias Brugger, linux-rpi-kernel, linux-arm-kernel, Stefan Wahren, Matthias Brugger, Eric Anholt, Mark Rutland, Rob Herring, bcm-kernel-feedback-list, devicetree, linux-kernel 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"; + 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"; + /* No PHY interrupt */ + max-speed = <1000>; + 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 ac83dac2e6ba..e2e837fcad59 100644 --- a/arch/arm/boot/dts/bcm2711.dtsi +++ b/arch/arm/boot/dts/bcm2711.dtsi @@ -305,6 +305,24 @@ 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>; + status = "disabled"; + }; + }; }; &clk_osc { -- 2.23.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v1 3/3] ARM: dts: bcm2711: Enable GENET support for the RPi4 2019-10-11 18:48 ` [PATCH v1 3/3] ARM: dts: bcm2711: Enable GENET support for the RPi4 matthias.bgg @ 2019-10-11 19:31 ` Florian Fainelli 2019-10-13 18:41 ` Stefan Wahren 2019-10-15 19:35 ` Stefan Wahren 2019-10-11 23:09 ` Stefan Wahren 1 sibling, 2 replies; 14+ messages in thread From: Florian Fainelli @ 2019-10-11 19:31 UTC (permalink / raw) To: matthias.bgg, David S . Miller Cc: Nicolas Saenz Julienne, linux-rpi-kernel, linux-arm-kernel, Stefan Wahren, Matthias Brugger, Eric Anholt, Mark Rutland, Rob Herring, bcm-kernel-feedback-list, devicetree, linux-kernel 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v1 3/3] ARM: dts: bcm2711: Enable GENET support for the RPi4 2019-10-11 19:31 ` Florian Fainelli @ 2019-10-13 18:41 ` Stefan Wahren 2019-10-13 19:19 ` Florian Fainelli 2019-10-15 19:35 ` Stefan Wahren 1 sibling, 1 reply; 14+ messages in thread From: Stefan Wahren @ 2019-10-13 18:41 UTC (permalink / raw) To: Florian Fainelli, matthias.bgg, David S . Miller Cc: Nicolas Saenz Julienne, linux-rpi-kernel, linux-arm-kernel, Matthias Brugger, Eric Anholt, Mark Rutland, Rob Herring, bcm-kernel-feedback-list, devicetree, linux-kernel Hi Florian, Am 11.10.19 um 21:31 schrieb Florian Fainelli: > 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. i applied your changes, kept the phy-mode above and the interfaces cames up. But there is a lot of packet loss using ping. After applying this downstream patch [1] the packet loss doesn't occur. Is the packet loss a possible cause of the wrong phy-mode and mentioned patch only a workaround? [1] - https://github.com/raspberrypi/linux/commit/360c8e98883f9cd075564be8a7fc25ac0785dee4 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 3/3] ARM: dts: bcm2711: Enable GENET support for the RPi4 2019-10-13 18:41 ` Stefan Wahren @ 2019-10-13 19:19 ` Florian Fainelli 0 siblings, 0 replies; 14+ messages in thread From: Florian Fainelli @ 2019-10-13 19:19 UTC (permalink / raw) To: Stefan Wahren, matthias.bgg, David S . Miller Cc: Nicolas Saenz Julienne, linux-rpi-kernel, linux-arm-kernel, Matthias Brugger, Eric Anholt, Mark Rutland, Rob Herring, bcm-kernel-feedback-list, devicetree, linux-kernel On 10/13/2019 11:41 AM, Stefan Wahren wrote: > Hi Florian, > > Am 11.10.19 um 21:31 schrieb Florian Fainelli: >> 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. > > i applied your changes, kept the phy-mode above and the interfaces cames > up. But there is a lot of packet loss using ping. After applying this > downstream patch [1] the packet loss doesn't occur. Packet loss is symptomatic of a mis-configured RGMII interface between the MAC and the PHY. > > Is the packet loss a possible cause of the wrong phy-mode and mentioned > patch only a workaround? The patch at [1] is not doing much with respect to RGMII delays, so it will just keep whatever was configured prior to Linux taking over the PHY. The addition of the BCM54213PE entry makes use of the bcm54xx_config_init() callback, which does not call bcm54xx_config_clock_delay() for the BCM54213PE PHY model, which is why it "solves" the packet loss by preserving whatever was already configured. I would suggest checking with the Pi folks whether 'rgmii' is really the right mode of operation here (that is, the PHY is not adding TXC or RXC delays at all), or it just happens to work by virtue of the PHY device defaulting to a certain mode *and* the PHY device driver in Linux not attempting to correctly change the RX/TX clock delays based on the phy_interface_t value (aka: maintain the status quo). Thanks for checking! > > [1] - > https://github.com/raspberrypi/linux/commit/360c8e98883f9cd075564be8a7fc25ac0785dee4 > -- Florian ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 3/3] ARM: dts: bcm2711: Enable GENET support for the RPi4 2019-10-11 19:31 ` Florian Fainelli 2019-10-13 18:41 ` Stefan Wahren @ 2019-10-15 19:35 ` Stefan Wahren 2019-10-15 19:39 ` Florian Fainelli 1 sibling, 1 reply; 14+ messages in thread From: Stefan Wahren @ 2019-10-15 19:35 UTC (permalink / raw) To: Florian Fainelli, matthias.bgg, David S . Miller Cc: Mark Rutland, devicetree, Matthias Brugger, Nicolas Saenz Julienne, linux-kernel, Eric Anholt, Rob Herring, bcm-kernel-feedback-list, linux-rpi-kernel, linux-arm-kernel Hi Florian, Am 11.10.19 um 21:31 schrieb Florian Fainelli: > 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. since i didn't get a reply from the Pi folks yet, i tried all the other rgmii variants on top of this branch [1]. But none of them worked. In case of "rgmii-id" i'm getting: unknown phy mode: 9 and for "rgmii-rxid": unknown phy mode: 10 In those cases the PHY didn't even probe. Did i missed something? [1] - https://github.com/lategoodbye/rpi-zero/commits/bcm2711-genet ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 3/3] ARM: dts: bcm2711: Enable GENET support for the RPi4 2019-10-15 19:35 ` Stefan Wahren @ 2019-10-15 19:39 ` Florian Fainelli 0 siblings, 0 replies; 14+ messages in thread From: Florian Fainelli @ 2019-10-15 19:39 UTC (permalink / raw) To: Stefan Wahren, Florian Fainelli, matthias.bgg, David S . Miller Cc: Mark Rutland, devicetree, Matthias Brugger, Nicolas Saenz Julienne, linux-kernel, Eric Anholt, Rob Herring, bcm-kernel-feedback-list, linux-rpi-kernel, linux-arm-kernel On 10/15/19 12:35 PM, Stefan Wahren wrote: > Hi Florian, > > Am 11.10.19 um 21:31 schrieb Florian Fainelli: >> 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. > > since i didn't get a reply from the Pi folks yet, i tried all the other > rgmii variants on top of this branch [1]. > > But none of them worked. > > In case of "rgmii-id" i'm getting: > > unknown phy mode: 9 > > and for "rgmii-rxid": > > unknown phy mode: 10 > > In those cases the PHY didn't even probe. Did i missed something? Well the GENET driver can only deal with two specific RGMII modes, so you would need something like this: diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c index 970e478a9017..a12656cccf89 100644 --- a/drivers/net/ethernet/broadcom/genet/bcmmii.c +++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c @@ -241,6 +241,8 @@ int bcmgenet_mii_config(struct net_device *dev, bool init) id_mode_dis = BIT(16); /* fall through */ case PHY_INTERFACE_MODE_RGMII_TXID: + case PHY_INTERFACE_MODE_RGMII_RXID: + case PHY_INTERFACE_MODE_RGMII_ID: if (id_mode_dis) phy_name = "external RGMII (no delay)"; else > > [1] - https://github.com/lategoodbye/rpi-zero/commits/bcm2711-genet > -- Florian ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v1 3/3] ARM: dts: bcm2711: Enable GENET support for the RPi4 2019-10-11 18:48 ` [PATCH v1 3/3] ARM: dts: bcm2711: Enable GENET support for the RPi4 matthias.bgg 2019-10-11 19:31 ` Florian Fainelli @ 2019-10-11 23:09 ` Stefan Wahren 2019-10-11 23:11 ` Florian Fainelli 2019-10-12 17:05 ` Stefan Wahren 1 sibling, 2 replies; 14+ messages in thread From: Stefan Wahren @ 2019-10-11 23:09 UTC (permalink / raw) To: matthias.bgg, Florian Fainelli, David S . Miller Cc: Nicolas Saenz Julienne, linux-rpi-kernel, linux-arm-kernel, Matthias Brugger, Eric Anholt, Mark Rutland, Rob Herring, bcm-kernel-feedback-list, devicetree, linux-kernel Am 11.10.19 um 20:48 schrieb matthias.bgg@kernel.org: > From: Matthias Brugger <mbrugger@suse.com> > > Enable Gigabit Ethernet support on the Raspberry Pi 4 > Model B. I asked some questions about genet to the RPi guys [1] which are relevant to this patch (missing clocks and interrupt, MAC address assignment) but i didn't get an answer yet. During my tests with a similiar patch series i noticed that the driver won't probe without a MAC address. How does it get into DT (via U-Boot)? [1] - https://github.com/raspberrypi/linux/issues/3101#issuecomment-534665860 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 3/3] ARM: dts: bcm2711: Enable GENET support for the RPi4 2019-10-11 23:09 ` Stefan Wahren @ 2019-10-11 23:11 ` Florian Fainelli 2019-10-12 17:05 ` Stefan Wahren 1 sibling, 0 replies; 14+ messages in thread From: Florian Fainelli @ 2019-10-11 23:11 UTC (permalink / raw) To: Stefan Wahren, matthias.bgg, Florian Fainelli, David S . Miller Cc: Nicolas Saenz Julienne, linux-rpi-kernel, linux-arm-kernel, Matthias Brugger, Eric Anholt, Mark Rutland, Rob Herring, bcm-kernel-feedback-list, devicetree, linux-kernel On 10/11/19 4:09 PM, Stefan Wahren wrote: > Am 11.10.19 um 20:48 schrieb matthias.bgg@kernel.org: >> From: Matthias Brugger <mbrugger@suse.com> >> >> Enable Gigabit Ethernet support on the Raspberry Pi 4 >> Model B. > > I asked some questions about genet to the RPi guys [1] which are > relevant to this patch (missing clocks and interrupt, MAC address > assignment) but i didn't get an answer yet. > > During my tests with a similiar patch series i noticed that the driver > won't probe without a MAC address. Yes, that is true, we have an internal patch for that that we just recently did, let me submit it so this does not block you. > > How does it get into DT (via U-Boot)? > > [1] - > https://github.com/raspberrypi/linux/issues/3101#issuecomment-534665860 > -- Florian ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 3/3] ARM: dts: bcm2711: Enable GENET support for the RPi4 2019-10-11 23:09 ` Stefan Wahren 2019-10-11 23:11 ` Florian Fainelli @ 2019-10-12 17:05 ` Stefan Wahren 1 sibling, 0 replies; 14+ messages in thread From: Stefan Wahren @ 2019-10-12 17:05 UTC (permalink / raw) To: matthias.bgg, Florian Fainelli, David S . Miller Cc: Nicolas Saenz Julienne, linux-rpi-kernel, linux-arm-kernel, Matthias Brugger, Eric Anholt, Mark Rutland, Rob Herring, bcm-kernel-feedback-list, devicetree, linux-kernel Am 12.10.19 um 01:09 schrieb Stefan Wahren: > Am 11.10.19 um 20:48 schrieb matthias.bgg@kernel.org: >> From: Matthias Brugger <mbrugger@suse.com> >> >> Enable Gigabit Ethernet support on the Raspberry Pi 4 >> Model B. > I asked some questions about genet to the RPi guys [1] which are > relevant to this patch (missing clocks and interrupt, MAC address > assignment) but i didn't get an answer yet. > > During my tests with a similiar patch series i noticed that the driver > won't probe without a MAC address. > > How does it get into DT (via U-Boot)? Okay, the bootloader uses the ethernet0 alias for the genet DT node. So this should also be added to the RPi 4 DTS. But i consider the MAC randomize patch still helpful. > > [1] - > https://github.com/raspberrypi/linux/issues/3101#issuecomment-534665860 > ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-10-15 19:39 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-11 18:48 [PATCH v1 0/3] This series adds ethernet support for RPi4 matthias.bgg 2019-10-11 18:48 ` [PATCH v1 1/3] dt-bindings: net: bcmgenet add property for max DMA burst size matthias.bgg 2019-10-11 19:07 ` Florian Fainelli 2019-10-11 18:48 ` [PATCH v1 2/3] net: bcmgenet: use optional max DMA burst size property matthias.bgg 2019-10-11 19:01 ` Nicolas Saenz Julienne 2019-10-11 18:48 ` [PATCH v1 3/3] ARM: dts: bcm2711: Enable GENET support for the RPi4 matthias.bgg 2019-10-11 19:31 ` Florian Fainelli 2019-10-13 18:41 ` Stefan Wahren 2019-10-13 19:19 ` Florian Fainelli 2019-10-15 19:35 ` Stefan Wahren 2019-10-15 19:39 ` Florian Fainelli 2019-10-11 23:09 ` Stefan Wahren 2019-10-11 23:11 ` Florian Fainelli 2019-10-12 17:05 ` Stefan Wahren
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).