Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 31 Aug 2022 18:18:59 +0300
From:   Vladimir Oltean <olteanv@gmail.com>
To:     Oleksij Rempel <o.rempel@pengutronix.de>
Cc:     Arun.Ramadoss@microchip.com, andrew@lunn.ch,
        linux-kernel@vger.kernel.org, UNGLinuxDriver@microchip.com,
        vivien.didelot@gmail.com, san@skov.dk, linux@armlinux.org.uk,
        f.fainelli@gmail.com, kuba@kernel.org, edumazet@google.com,
        pabeni@redhat.com, netdev@vger.kernel.org,
        Woojung.Huh@microchip.com, davem@davemloft.net
Subject: Re: [Patch net-next v2 0/9] net: dsa: microchip: add support for
 phylink mac config and link up
Message-ID: <20220831151859.ubpkt5aljrp3hiph@skbuf>
References: <20220724092823.24567-1-arun.ramadoss@microchip.com>
 <20220830065533.GA18106@pengutronix.de>
 <67690ec6367c9dc6d2df720dcf98e6e332d2105b.camel@microchip.com>
 <20220830095830.flxd3fw4sqyn425m@skbuf>
 <20220830160546.GB16715@pengutronix.de>
 <20220831074324.GD16715@pengutronix.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220831074324.GD16715@pengutronix.de>
Precedence: bulk

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?