Re: [PATCH net-next v3 1/7] net: dsa: mt7530: empty default case on mt7530_setup_port5()

["Arınç ÜNAL" <arinc.unal@arinc9.com>]

On 2.02.2024 14:40, Russell King (Oracle) wrote:
> While reviewing this change, but not related to it, I notice that this
> function sets the TX delay based on the RGMII interface mode. This isn't
> correct. I've explained why this is this many times in the past, but
> essentially it comes down to the model:
> 
> 
> phy-mode in NIC node	Network driver	PCB		PHY
> rgmii			no delays	delays		no delays
> rgmii-id		no delays	no delays	tx/rx delays
> rgmii-txid		no delays	no delays	tx delays
> rgmii-rxid		no delays	no delays	rx delays
> 
> Then we have rx-internal-delay-ps and tx-internal-delay-ps in the NIC
> node which define the RGMII delays at the local end and similar
> properties for the PHY node.
> 
> 
> So, if we take the view that, when a switch is connected to a NIC in
> RGMII mode, then the phy-mode specified delays still should not impact
> the local NIC.
> 
> Now, for the switch, we specify the phy-mode in the port node as well.
> Consider the case of a switch port connected to a RGMII PHY. This has
> to operate in exactly the same way as a normal NIC - that is, the
> RGMII delays at the port should be ignored as it's the responsibility
> of a PHY.
> 
> The final scenario to examine is the case of a RGMII switch port
> connected to a NIC. The NIC's phy-mode has no way to be communicated
> to DSA or vice versa, so neither phy-mode can impact the other side
> of the RGMII link, but should only place the link into RGMII mode.
> Given everything I've said above, the only way to configure RGMII
> delays is via the rx-internal-delay-ps and tx-internal-delay-ps
> properties. So, DSA drivers should _not_ be configuring their ports
> with RGMII delays based on the RGMII phy interface mode.
> 
> The above is my purely logically reasoned point of view on this
> subject. Others may have other (to me completely illogical)
> interpretations that can only lead to interoperability issues.

I will address this with the next patch series. Thank you for explaining it
in detail.

Arınç