Re: [PATCH v1 3/3] ARM: dts: bcm2711: Enable GENET support for the RPi4
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@kernel.org wrote:
>>> From: Matthias Brugger
>>>
>>> Enable Gigabit Ethernet support on the Raspberry Pi 4
>>> Model B.
>>>
>>> Signed-off-by: Matthias Brugger
>>>
>>> ---
>>>
>>> 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 1ccd19be..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";
>>> };
>>>
>>> + {
>>> + phy-handle = <>;
>>> + 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
Re: [PATCH v1 3/3] ARM: dts: bcm2711: Enable GENET support for the RPi4
Hi Florian,
Am 11.10.19 um 21:31 schrieb Florian Fainelli:
> On 10/11/19 11:48 AM, matthias@kernel.org wrote:
>> From: Matthias Brugger
>>
>> Enable Gigabit Ethernet support on the Raspberry Pi 4
>> Model B.
>>
>> Signed-off-by: Matthias Brugger
>>
>> ---
>>
>> 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 1ccd19be..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";
>> };
>>
>> + {
>> +phy-handle = <>;
>> +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
Re: [PATCH v1 3/3] ARM: dts: bcm2711: Enable GENET support for the RPi4
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@kernel.org wrote:
>>> From: Matthias Brugger
>>>
>>> Enable Gigabit Ethernet support on the Raspberry Pi 4
>>> Model B.
>>>
>>> Signed-off-by: Matthias Brugger
>>>
>>> ---
>>>
>>> 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 1ccd19be..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";
>>> };
>>>
>>> + {
>>> + phy-handle = <>;
>>> + 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
Re: [PATCH v1 3/3] ARM: dts: bcm2711: Enable GENET support for the RPi4
Hi Florian,
Am 11.10.19 um 21:31 schrieb Florian Fainelli:
> On 10/11/19 11:48 AM, matthias@kernel.org wrote:
>> From: Matthias Brugger
>>
>> Enable Gigabit Ethernet support on the Raspberry Pi 4
>> Model B.
>>
>> Signed-off-by: Matthias Brugger
>>
>> ---
>>
>> 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 1ccd19be..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";
>> };
>>
>> + {
>> +phy-handle = <>;
>> +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
Re: [PATCH v1 3/3] ARM: dts: bcm2711: Enable GENET support for the RPi4
Am 12.10.19 um 01:09 schrieb Stefan Wahren: > Am 11.10.19 um 20:48 schrieb matthias@kernel.org: >> From: Matthias Brugger >> >> 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 >
Re: [PATCH v1 3/3] ARM: dts: bcm2711: Enable GENET support for the RPi4
Am 11.10.19 um 20:48 schrieb matthias@kernel.org: > From: Matthias Brugger > > 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
Re: [PATCH v1 3/3] ARM: dts: bcm2711: Enable GENET support for the RPi4
On 10/11/19 4:09 PM, Stefan Wahren wrote: > Am 11.10.19 um 20:48 schrieb matthias@kernel.org: >> From: Matthias Brugger >> >> 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
Re: [PATCH v1 3/3] ARM: dts: bcm2711: Enable GENET support for the RPi4
On 10/11/19 11:48 AM, matthias@kernel.org wrote:
> From: Matthias Brugger
>
> Enable Gigabit Ethernet support on the Raspberry Pi 4
> Model B.
>
> Signed-off-by: Matthias Brugger
>
> ---
>
> 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 1ccd19be..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";
> };
>
> + {
> + phy-handle = <>;
> + 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
