Re: [Patch net-next v2 0/9] net: dsa: microchip: add support for phylink mac config and link up

[Vladimir Oltean]

On Wed, Aug 31, 2022 at 09:43:24AM +0200, Oleksij Rempel wrote:
> > > Should we point them to ksz8795_xmii_ctrl0 and ksz8795_xmii_ctrl1? I don't know.
> > > Could you find out what these should be set to?
> > 
> > xmii_ctrl0/1 are missing and it make no sense to add it.
> > KSZ8873 switch is controlling CPU port MII configuration over global,
> > not port based register.
> > 
> > I'll better define separate ops for this chip.
> 
> Hm, not only KSZ8830/KSZ8863/KSZ8873 are affected. ksz8795 compatible
> series with defined .xmii_ctrl0/.xmii_ctrl1 are broken too. Because it
> is writing to the global config register over ksz_pwrite8 function. It
> means, we are writing to 0xa6 instead of 0x06. And to 0xf6 instead of
> 0x56.

What do you mean by "global config register"? The Is_1Gbps bit is still
part of a port register, it's just that this particular register is only
defined for the 5th port (port #4, the only xMII port on KSZ7895 AFAIU).
That doesn't necessarily make it a "global" register.

Datasheet says:

Register 22 (0x16): Reserved
Register 38 (0x26): Reserved
Register 54 (0x36): Reserved
Register 70 (0x46): Reserved
Register 86 (0x56): Port 5 Interface Control 6

I wonder if it's ok to modify the regs table like this, because we
should then only touch P_XMII_CTRL_1 using port 4:

 static const u16 ksz8795_regs[] = {
 	[REG_IND_CTRL_0]		= 0x6E,
 	[REG_IND_DATA_8]		= 0x70,
 	[REG_IND_DATA_CHECK]		= 0x72,
 	[REG_IND_DATA_HI]		= 0x71,
 	[REG_IND_DATA_LO]		= 0x75,
 	[REG_IND_MIB_CHECK]		= 0x74,
 	[REG_IND_BYTE]			= 0xA0,
 	[P_FORCE_CTRL]			= 0x0C,
 	[P_LINK_STATUS]			= 0x0E,
 	[P_LOCAL_CTRL]			= 0x07,
 	[P_NEG_RESTART_CTRL]		= 0x0D,
 	[P_REMOTE_STATUS]		= 0x08,
 	[P_SPEED_STATUS]		= 0x09,
 	[S_TAIL_TAG_CTRL]		= 0x0C,
 	[P_STP_CTRL]			= 0x02,
 	[S_START_CTRL]			= 0x01,
 	[S_BROADCAST_CTRL]		= 0x06,
 	[S_MULTICAST_CTRL]		= 0x04,
 	[P_XMII_CTRL_0]			= 0x06,
-	[P_XMII_CTRL_1]			= 0x56,
+	[P_XMII_CTRL_1]			= 0x06,
 };

Arun, what is your proposed solution?