Re: [PATCH net-next v3 7/8] net: dsa: lantiq_gswip: store switch API version in priv

On Thu, Aug 21, 2025 at 04:58:23AM +0200, Andrew Lunn wrote:

quoted

quoted

+	priv->version = le16_to_cpu((__le16 __force)version);

I've researched this a bit more and came to the conclusion that while the
above works fine because all Lantiq SoCs with built-in switch are
big-endian machines it is still wrong.
I base this conclusion on the fact that when dealing with more recent
MDIO-connected switches (MaxLinear GSW1xx series) the host endian doesn't
play a role in the driver -- when dealing with 16-bit values on the MDIO
bus, the bus abstraction takes care of converting from/to host endianess.

I agree that all MDIO bus registers are host endian, 16 bit. The shift
register in the hardware is responsible for putting the bits on the
wire in the correct order for MDIO.

quoted

Hence I believe this should simply be a swab16() which will always result
in the version being in the right byte order to use comparative operators
in a meaningful way.

How is this described in the datasheet? And is version special, or do
all registers need swapping?

The (anyway public) datasheets I have access to don't describe the VERSION
register at all. In the existing precompiler macros, however, you can see
that most-significant and least-significant byte are swapped. REV is
more significant than MOD:

#define GSWIP_VERSION                   0x013
#define  GSWIP_VERSION_REV_SHIFT        0
#define  GSWIP_VERSION_REV_MASK         GENMASK(7, 0)
#define  GSWIP_VERSION_MOD_SHIFT        8
#define  GSWIP_VERSION_MOD_MASK         GENMASK(15, 8)
#define   GSWIP_VERSION_2_0             0x100
#define   GSWIP_VERSION_2_1             0x021
#define   GSWIP_VERSION_2_2             0x122
#define   GSWIP_VERSION_2_2_ETC         0x022

Now I'd like to add
#define   GSWIP_VERSION_2_3             0x023

and then have a simple way to make features available starting from a
GSWIP_VERSION. Now in order for GSWIP_VERSION_2_3 to be greater than
GSWIP_VERSION_2_2, and GSWIP_VERSION_2_1 to be greater than
GSWIP_VERSION_2_0, the bytes need to be swapped.

I don't think the vendor even considered any specific order of the two
bytes but just defines them as separate 8-bit fields, considering it a
16-bit unsigned integer is my interpretation which came from the need for
comparability.