[PATCH 3/4] arm64: dts: socfpga: agilex5: enable gmac2 on the Agilex5 dev kit

Matthew Gerlach posted 4 patches 2 months, 3 weeks ago
There is a newer version of this series
[PATCH 3/4] arm64: dts: socfpga: agilex5: enable gmac2 on the Agilex5 dev kit
Posted by Matthew Gerlach 2 months, 3 weeks ago
Enable gmac2 on the Agilex5 SOCFGPA Development Kit. The MAC is connected
to a RGMII PHY on a daughter card. The necessary clock delays are
implemented between the MAC and PHY by the IO ring of the Agilex5.

Signed-off-by: Matthew Gerlach <matthew.gerlach@altera.com>
---
 .../boot/dts/intel/socfpga_agilex5_socdk.dts   | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/arm64/boot/dts/intel/socfpga_agilex5_socdk.dts b/arch/arm64/boot/dts/intel/socfpga_agilex5_socdk.dts
index d3b913b7902c..5436646ec7ad 100644
--- a/arch/arm64/boot/dts/intel/socfpga_agilex5_socdk.dts
+++ b/arch/arm64/boot/dts/intel/socfpga_agilex5_socdk.dts
@@ -10,6 +10,9 @@ / {
 
 	aliases {
 		serial0 = &uart0;
+		ethernet0 = &gmac0;
+		ethernet1 = &gmac1;
+		ethernet2 = &gmac2;
 	};
 
 	chosen {
@@ -37,6 +40,21 @@ &gpio0 {
 	status = "okay";
 };
 
+&gmac2 {
+	status = "okay";
+	phy-mode = "rgmii";	/* Delays implemented by the IO ring of the Agilex5 SOCFPGA. */
+	phy-handle = <&emac2_phy0>;
+	max-frame-size = <9000>;
+	mdio0 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "snps,dwmac-mdio";
+		emac2_phy0: ethernet-phy@0 {
+			reg = <0>;
+		};
+	};
+};
+
 &gpio1 {
 	status = "okay";
 };
-- 
2.49.0
Re: [PATCH 3/4] arm64: dts: socfpga: agilex5: enable gmac2 on the Agilex5 dev kit
Posted by Andrew Lunn 2 months, 3 weeks ago
> +&gmac2 {
> +	status = "okay";
> +	phy-mode = "rgmii";	/* Delays implemented by the IO ring of the Agilex5 SOCFPGA. */

Please could you explain in more details what this means.

The normal meaning for 'rgmii' is that the PCB implements the delay. I
just want to fully understand what this IO ring is, and if it is part
of the PCB.

> +	phy-handle = <&emac2_phy0>;
> +	max-frame-size = <9000>;
> +	mdio0 {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		compatible = "snps,dwmac-mdio";
> +		emac2_phy0: ethernet-phy@0 {
> +			reg = <0>;
> +		};

Please add a newline in here to separate the inner node from the
rest.

    Andrew

---
pw-bot: cr
Re: [PATCH 3/4] arm64: dts: socfpga: agilex5: enable gmac2 on the Agilex5 dev kit
Posted by Matthew Gerlach 2 months, 3 weeks ago

On 7/14/25 10:25 AM, Andrew Lunn wrote:
> > +&gmac2 {
> > +	status = "okay";
> > +	phy-mode = "rgmii";	/* Delays implemented by the IO ring of the Agilex5 SOCFPGA. */
>
> Please could you explain in more details what this means.
>
> The normal meaning for 'rgmii' is that the PCB implements the delay. I
> just want to fully understand what this IO ring is, and if it is part
> of the PCB.

The IO ring is the logic in the Agilex5 that controls the pins on the 
chip. It is this logic that sits between the MAC IP in the Agilex5 and 
the pins connected to the PCB that is inserting the necessary delays. 
Technically the PCB is not implementing the delays, but the "wires" 
between the MAC and the external pins of the Agilex5 are implementing 
the delay. It seems to me that "rgmii" is a more accurate description of 
the hardware than "rgmii-id" in this case.

>
> > +	phy-handle = <&emac2_phy0>;
> > +	max-frame-size = <9000>;
> > +	mdio0 {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +		compatible = "snps,dwmac-mdio";
> > +		emac2_phy0: ethernet-phy@0 {
> > +			reg = <0>;
> > +		};
>
> Please add a newline in here to separate the inner node from the
> rest.
>
>      Andrew

I will add a newline before the emac2_phy0 node as suggested in v2.

Thanks for the feedback,
Matthew Gerlach
>
> ---
> pw-bot: cr
Re: [PATCH 3/4] arm64: dts: socfpga: agilex5: enable gmac2 on the Agilex5 dev kit
Posted by Andrew Lunn 2 months, 3 weeks ago
On Mon, Jul 14, 2025 at 11:09:33AM -0700, Matthew Gerlach wrote:
> 
> 
> On 7/14/25 10:25 AM, Andrew Lunn wrote:
> > > +&gmac2 {
> > > +	status = "okay";
> > > +	phy-mode = "rgmii";	/* Delays implemented by the IO ring of the Agilex5 SOCFPGA. */
> > 
> > Please could you explain in more details what this means.
> > 
> > The normal meaning for 'rgmii' is that the PCB implements the delay. I
> > just want to fully understand what this IO ring is, and if it is part
> > of the PCB.
> 
> The IO ring is the logic in the Agilex5 that controls the pins on the chip.
> It is this logic that sits between the MAC IP in the Agilex5 and the pins
> connected to the PCB that is inserting the necessary delays. Technically the
> PCB is not implementing the delays, but the "wires" between the MAC and the
> external pins of the Agilex5 are implementing the delay. It seems to me that
> "rgmii" is a more accurate description of the hardware than "rgmii-id" in
> this case.

Is this delay hard coded, physically impossible to be disabled? A
syntheses option? Can it be changed at run time? Is the IO ring under
the control of a pinctrl driver? Can i use the standard 'skew-delay'
DT property to control this delay?

For silicon, if the delay cannot be removed, we have MAC drivers masks
the phy-mode to indicate it has implemented the delay. The MAC driver
should also return -EINVAL for any other RGMII mode than rgmii-id,
because that is the only RGMII mode which is possible.

Since this is an FPGA, it is a bit more complex, so i want to fully
understand what is going on, what the different options are.

	Andrew
Re: [PATCH 3/4] arm64: dts: socfpga: agilex5: enable gmac2 on the Agilex5 dev kit
Posted by Matthew Gerlach 2 months, 3 weeks ago

On 7/14/25 11:52 AM, Andrew Lunn wrote:
> On Mon, Jul 14, 2025 at 11:09:33AM -0700, Matthew Gerlach wrote:
> > 
> > 
> > On 7/14/25 10:25 AM, Andrew Lunn wrote:
> > > > +&gmac2 {
> > > > +	status = "okay";
> > > > +	phy-mode = "rgmii";	/* Delays implemented by the IO ring of the Agilex5 SOCFPGA. */
> > > 
> > > Please could you explain in more details what this means.
> > > 
> > > The normal meaning for 'rgmii' is that the PCB implements the delay. I
> > > just want to fully understand what this IO ring is, and if it is part
> > > of the PCB.
> > 
> > The IO ring is the logic in the Agilex5 that controls the pins on the chip.
> > It is this logic that sits between the MAC IP in the Agilex5 and the pins
> > connected to the PCB that is inserting the necessary delays. Technically the
> > PCB is not implementing the delays, but the "wires" between the MAC and the
> > external pins of the Agilex5 are implementing the delay. It seems to me that
> > "rgmii" is a more accurate description of the hardware than "rgmii-id" in
> > this case.
>
> Is this delay hard coded, physically impossible to be disabled? A
> syntheses option? Can it be changed at run time? Is the IO ring under
> the control of a pinctrl driver? Can i use the standard 'skew-delay'
> DT property to control this delay?
The delay is not hard coded. It is a synthesis option that can be 
disabled. The value cannot be changed at run time, and the IO ring is 
not under control of a pinctrl driver; so I don't think the standard 
'skew-delay' DT property can be used.
>
> For silicon, if the delay cannot be removed, we have MAC drivers masks
> the phy-mode to indicate it has implemented the delay. The MAC driver
> should also return -EINVAL for any other RGMII mode than rgmii-id,
> because that is the only RGMII mode which is possible.
The delay in the IO ring can be disabled, but implementing the delay in 
the IO ring allows for RGMII phys that don't implement the delay. 
Currently the driver, 
drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c, and its bindings, 
Documentation/devicetree/bindings/net/altr,socfpga-stmmac.yaml, allow 
all rgmii phy-modes.
>
> Since this is an FPGA, it is a bit more complex, so i want to fully
> understand what is going on, what the different options are.
In this particular instantiation, the hard MAC controller is directly 
connected to pins via the IO ring, and the FPGA is not involved.

Matthew Gerlach
>
> 	Andrew
Re: [PATCH 3/4] arm64: dts: socfpga: agilex5: enable gmac2 on the Agilex5 dev kit
Posted by Andrew Lunn 2 months, 3 weeks ago
> Currently the
> driver, drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c, and its
> bindings, Documentation/devicetree/bindings/net/altr,socfpga-stmmac.yaml,
> allow all rgmii phy-modes.

There is nothing unusual about allowing all four RGMII modes. Somebody
might design a board with the 2ns delay via extra long clocks lines,
or some other form of delay lines. Such a board would need 'rgmii',
and they do exist, a board like this was added this cycle. There is
one rather odd board with a DT file which has extra long clock line
for TX, but not RX! That needs rgmii-rxid. The majority of boards
don't have any delays on the PCB, so need 'rgmii-id'.

	Andrew
Re: [PATCH 3/4] arm64: dts: socfpga: agilex5: enable gmac2 on the Agilex5 dev kit
Posted by Andrew Lunn 2 months, 3 weeks ago
On Mon, Jul 14, 2025 at 03:29:21PM -0700, Matthew Gerlach wrote:
> 
> 
> On 7/14/25 11:52 AM, Andrew Lunn wrote:
> > On Mon, Jul 14, 2025 at 11:09:33AM -0700, Matthew Gerlach wrote:
> > > > > On 7/14/25 10:25 AM, Andrew Lunn wrote:
> > > > > +&gmac2 {
> > > > > +	status = "okay";
> > > > > +	phy-mode = "rgmii";	/* Delays implemented by the IO ring of the Agilex5 SOCFPGA. */
> > > > > > Please could you explain in more details what this means.
> > > > > > The normal meaning for 'rgmii' is that the PCB implements the
> > delay. I
> > > > just want to fully understand what this IO ring is, and if it is part
> > > > of the PCB.
> > > > The IO ring is the logic in the Agilex5 that controls the pins on
> > the chip.
> > > It is this logic that sits between the MAC IP in the Agilex5 and the pins
> > > connected to the PCB that is inserting the necessary delays. Technically the
> > > PCB is not implementing the delays, but the "wires" between the MAC and the
> > > external pins of the Agilex5 are implementing the delay. It seems to me that
> > > "rgmii" is a more accurate description of the hardware than "rgmii-id" in
> > > this case.
> > 
> > Is this delay hard coded, physically impossible to be disabled? A
> > syntheses option? Can it be changed at run time? Is the IO ring under
> > the control of a pinctrl driver? Can i use the standard 'skew-delay'
> > DT property to control this delay?

> The delay is not hard coded. It is a synthesis option that can be disabled.

Is there a register you can read which tells you if it is
enabled/disabled?

> The delay in the IO ring can be disabled, but implementing the delay in the
> IO ring allows for RGMII phys that don't implement the delay.

All RGMII PHYs which Linux support have the ability to do delays. And
we recommend the PHY does the delay, just to keep all systems the
same. There are a few exceptions, mostly because the MAC has hard
coded delays which cannot be disabled, but i guess 90% of systems have
the PHY doing the 2ns delays.

So, phy-mode should be set to 'rgmii-id', since the PCB does not add
the delays.

Ideally, you want to read from the IO ring if it is synthesised to do
the 2ns delays. Assuming it is enabled, you then mask the phy-mode
before connecting to the PHY, so avoiding double delays.

	Andrew
Re: [PATCH 3/4] arm64: dts: socfpga: agilex5: enable gmac2 on the Agilex5 dev kit
Posted by Matthew Gerlach 2 months, 2 weeks ago

On 7/14/25 3:50 PM, Andrew Lunn wrote:
> On Mon, Jul 14, 2025 at 03:29:21PM -0700, Matthew Gerlach wrote:
> > 
> > 
> > On 7/14/25 11:52 AM, Andrew Lunn wrote:
> > > On Mon, Jul 14, 2025 at 11:09:33AM -0700, Matthew Gerlach wrote:
> > > > > > On 7/14/25 10:25 AM, Andrew Lunn wrote:
> > > > > > +&gmac2 {
> > > > > > +	status = "okay";
> > > > > > +	phy-mode = "rgmii";	/* Delays implemented by the IO ring of the Agilex5 SOCFPGA. */
> > > > > > > Please could you explain in more details what this means.
> > > > > > > The normal meaning for 'rgmii' is that the PCB implements the
> > > delay. I
> > > > > just want to fully understand what this IO ring is, and if it is part
> > > > > of the PCB.
> > > > > The IO ring is the logic in the Agilex5 that controls the pins on
> > > the chip.
> > > > It is this logic that sits between the MAC IP in the Agilex5 and the pins
> > > > connected to the PCB that is inserting the necessary delays. Technically the
> > > > PCB is not implementing the delays, but the "wires" between the MAC and the
> > > > external pins of the Agilex5 are implementing the delay. It seems to me that
> > > > "rgmii" is a more accurate description of the hardware than "rgmii-id" in
> > > > this case.
> > > 
> > > Is this delay hard coded, physically impossible to be disabled? A
> > > syntheses option? Can it be changed at run time? Is the IO ring under
> > > the control of a pinctrl driver? Can i use the standard 'skew-delay'
> > > DT property to control this delay?
>
> > The delay is not hard coded. It is a synthesis option that can be disabled.
>
> Is there a register you can read which tells you if it is
> enabled/disabled?

There are a collection of registers that could be examined to determine 
if delay has been inserted by IO ring.

>
> > The delay in the IO ring can be disabled, but implementing the delay in the
> > IO ring allows for RGMII phys that don't implement the delay.
>
> All RGMII PHYs which Linux support have the ability to do delays. And
> we recommend the PHY does the delay, just to keep all systems the
> same. There are a few exceptions, mostly because the MAC has hard
> coded delays which cannot be disabled, but i guess 90% of systems have
> the PHY doing the 2ns delays.
>
> So, phy-mode should be set to 'rgmii-id', since the PCB does not add
> the delays.

Understood. The PCB is not adding delays; so I'll change the phy-mode to 
be rgmii-id.

>
> Ideally, you want to read from the IO ring if it is synthesised to do
> the 2ns delays. Assuming it is enabled, you then mask the phy-mode
> before connecting to the PHY, so avoiding double delays.
>
> 	Andrew
>

Currently, the registers in question are considered secure and only 
accessible by
Arm Trusted Firmware (ATF). I will investigate implementing this delay 
detecting
logic in a future patchset.

Thanks for the feedback,

Matthew Gerlach