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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220831151859.ubpkt5aljrp3hiph@skbuf>
Date:   Wed, 31 Aug 2022 18:18:59 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     Oleksij Rempel <o.rempel@...gutronix.de>
Cc:     Arun.Ramadoss@...rochip.com, andrew@...n.ch,
        linux-kernel@...r.kernel.org, UNGLinuxDriver@...rochip.com,
        vivien.didelot@...il.com, san@...v.dk, linux@...linux.org.uk,
        f.fainelli@...il.com, kuba@...nel.org, edumazet@...gle.com,
        pabeni@...hat.com, netdev@...r.kernel.org,
        Woojung.Huh@...rochip.com, davem@...emloft.net
Subject: Re: [Patch net-next v2 0/9] net: dsa: microchip: add support for
 phylink mac config and link up

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?