Re: [PATCH net-next 2/2] net: phy: aquantia: allow forcing order of MDI pairs

[Daniel Golle <daniel@makrotopia.org>]

On Fri, Aug 23, 2024 at 03:28:39AM +0200, Andrew Lunn wrote:
> On Wed, Aug 21, 2024 at 05:18:44PM +0100, Daniel Golle wrote:
> > On Wed, Aug 21, 2024 at 06:07:06PM +0200, Andrew Lunn wrote:
> > > On Wed, Aug 21, 2024 at 01:46:50PM +0100, Daniel Golle wrote:
> > > > Normally, the MDI reversal configuration is taken from the MDI_CFG pin.
> > > > However, some hardware designs require overriding the value configured
> > > > by that bootstrap pin. The PHY allows doing that by setting a bit which
> > > > allows ignoring the state of the MDI_CFG pin and configuring whether
> > > > the order of MDI pairs should be normal (ABCD) or reverse (DCBA).
> > > > 
> > > > Introduce two boolean properties which allow forcing either normal or
> > > > reverse order of the MDI pairs from DT.
> > > 
> > > How does this interact with ethtool -s eth42 [mdix auto|on|off]
> > > 
> > > In general, you want mdix auto, so the two ends figure out how the
> > > cable is wired and so it just works.
> > 
> > It looks like Aquantia only supports swapping pair (1,2) with pair (3,6)
> > like it used to be for MDI-X on 100MBit/s networks.
> > 
> > When all 4 pairs are in use (for 1000MBit/s or faster) the link does not
> > come up with pair order is not configured correctly, either using MDI_CFG
> > pin or using the "PMA Receive Reserved Vendor Provisioning 1" register.
> > 
> > And yes, I did verify that Auto MDI-X is enabled in the
> > "Autonegotiation Reserved Vendor Provisioning 1" register.
> 
> Is it possible to read the strap configuration?  All DT needs to
> indicate is that the strap is inverted.

Sadly no, because there is only a single r/w bit which could already
have been changed by the bootloader as well. We risk that vendor
bootloader updates then require modifying DT again once they "fix it"
there. I'd rather have two properties to force either ABCD or DCBA pair
order.

If it got to be a single property, then we can either use a string with
pre-defined values "abcd" and "dcba", or use macro defined integer
values in include/dt-bindings such as

#define AQUANTIA_MII_PAIR_ORDER_ABCD 0
#define AQUANTIA_MII_PAIR_ORDER_DCBA 1

In both when using a single property for overriding the bootstrap value,
absence of that property would mean to not touch what ever has been
setup by bootstrap pin (or force-mode configured by the bootloader,
which I've also seen already, and that also seems to survive PHY
resets).