Re: [PATCH] lan743x: Added fixed link support

From: Andrew Lunn <andrew@lunn.ch>
To: Roelof Berg <rberg@berg-solutions.de>
Cc: Bryan Whitehead <bryan.whitehead@microchip.com>,
	Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>,
	"David S. Miller" <davem@davemloft.net>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] lan743x: Added fixed link support
Date: Mon, 18 May 2020 01:50:26 +0200	[thread overview]
Message-ID: <20200517235026.GD610998@lunn.ch> (raw)
In-Reply-To: <6E144634-8E2F-48F7-A0A4-6073164F2B70@berg-solutions.de>

> >> +			/* Configure MAC to fixed link parameters */
> >> +			data = lan743x_csr_read(adapter, MAC_CR);
> >> +			/* Disable auto negotiation */
> >> +			data &= ~(MAC_CR_ADD_ | MAC_CR_ASD_);
> > 
> > Why does the MAC care about autoneg? In general, all the MAC needs to
> > know is the speed and duplex.
> > 
> 

> My assumption is, that in fixed-link mode we should switch off the
> autonegotiation between MAC and remote peer (e.g. a switch). I
> didn’t test, if it would also wun with the hardware doing
> auto-negotiation, however it feels cleaner to me to prevent the
> hardware from initiating any auto-negotiation in fixed-link mode.

The MAC is not involved in autoneg. autoneg is between two PHYs. They
talk with each other, and then phylibs sees the results and tells the
MAC the results of the negotiation. That happens via this call
back. So i have no idea what this is doing in general in the MAC. And
in your setup, you don't have any PHYs at all. So there is no
auto-neg. You should read the datasheet and understand what this is
controlling. It might need to be disabled in general.

> Using get_phy_mode() in all cases is not possible on a PC as it
> returns SGMII on a standard PC.

Why do you think that?

> > I don't understand this comment.
> > 
> 
> See above the lengthy section. On a PC SGMII is returned when I call of_get_phy_mode(phynode, &phyifc);

There are two things possible here:

A PC has no OF support, so you are using:

https://elixir.bootlin.com/linux/latest/source/include/linux/of_net.h#L19

So you get the error code -ENODEV, and phyifc is not changed.

Or you are using:

https://elixir.bootlin.com/linux/latest/source/drivers/of/of_net.c#L25

There is unlikely to be a device node, so phyifc is set to
PHY_INTERFACE_MODE_NA and -ENODEV is returned.

So if of_get_phy_mode() returns an error, use RMII. Otherwise use what
value it set phyifc to.

      Andrew