Re: [PATCH net-next v6] net: phy: intel-xway: Add RGMII internal delay configuration

On 2022-01-12 19:25, Tim Harvey wrote:

On Wed, Jan 12, 2022 at 5:46 AM Russell King (Oracle)
[off-list ref] wrote:

quoted

On Tue, Jan 11, 2022 at 11:12:33AM -0800, Tim Harvey wrote:

quoted

I added a debug statement in xway_gphy_rgmii_init and here you can see
it gets called 'before' the link comes up from the NIC on a board that
has a cable plugged in at power-on. I can tell from testing that the
rx_delay/tx_delay set in xway_gphy_rgmii_init does not actually take
effect unless I then bring the link down and up again manually as you
indicate.

# dmesg | egrep "xway|nicvf"
[    6.855971] xway_gphy_rgmii_init mdio_thunder MDI_MIICTRL:0xb100
rx_delay=1500 tx_delay=500
[    6.999651] nicvf, ver 1.0
[    7.002478] nicvf 0000:05:00.1: Adding to iommu group 7
[    7.007785] nicvf 0000:05:00.1: enabling device (0004 -> 0006)
[    7.053189] nicvf 0000:05:00.2: Adding to iommu group 8
[    7.058511] nicvf 0000:05:00.2: enabling device (0004 -> 0006)
[   11.044616] nicvf 0000:05:00.2 eth1: Link is Up 1000 Mbps Full duplex

Does the kernel message about the link coming up reflect what is going
on physically with the link though?

If a network interface is down, it's entirely possible that the link 
is
already established at the hardware level, buit the "Link is Up" 
message
gets reported when the network interface is later brought up. So,
debugging this by looking at the kernel messages is unreliable.

Russell,

You are correct... the link doesn't come up at that point its already
linked. So we need to force a reset or an auto negotiation reset after
modifying the delays.

Tim

Setting BMCR_ANRESTART would work, but only if BMCR_ANENABLE is also or
already set. Otherwise BMCR_ANRESTART has no effect (see the note in the
datasheet).

This is the reason why I came up with the idea of BMCR_PDOWN.

Personally I would have no problem with setting BMCR_ANRESTART and
BMCR_ANENABLE, but it would possibly change the existing configuration
if (e.g. by the bootloader) aneg should be disabled.

Martin