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:   Sat, 20 May 2023 18:17:08 +0300
From:   Vladimir Oltean <olteanv@gmail.com>
To:     Oleksij Rempel <o.rempel@pengutronix.de>
Cc:     Woojung Huh <woojung.huh@microchip.com>,
        Andrew Lunn <andrew@lunn.ch>,
        Arun Ramadoss <arun.ramadoss@microchip.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Simon Horman <simon.horman@corigine.com>,
        "Russell King (Oracle)" <linux@armlinux.org.uk>,
        linux-kernel@vger.kernel.org, Eric Dumazet <edumazet@google.com>,
        Paolo Abeni <pabeni@redhat.com>, kernel@pengutronix.de,
        netdev@vger.kernel.org, Jakub Kicinski <kuba@kernel.org>,
        UNGLinuxDriver@microchip.com,
        "David S. Miller" <davem@davemloft.net>
Subject: Re: [PATCH net-next v4 1/2] net: dsa: microchip: ksz8: Make flow
 control, speed, and duplex on CPU port configurable
Message-ID: <20230520151708.24duenxufth4xsh5@skbuf>
References: <20230519124700.635041-1-o.rempel@pengutronix.de>
 <20230519124700.635041-2-o.rempel@pengutronix.de>
 <20230519143004.luvz73jiyvnqxk4y@skbuf>
 <20230519185015.GA18246@pengutronix.de>
 <20230519203449.pc5vbfgbfc6rdo6i@skbuf>
 <20230520050317.GC18246@pengutronix.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230520050317.GC18246@pengutronix.de>
Precedence: bulk

On Sat, May 20, 2023 at 07:03:17AM +0200, Oleksij Rempel wrote:
> On Fri, May 19, 2023 at 11:34:49PM +0300, Vladimir Oltean wrote:
> > On Fri, May 19, 2023 at 08:50:15PM +0200, Oleksij Rempel wrote:
> > > Thank you for your feedback. I see your point. 
> > > 
> > > We need to remember that the KSZ switch series has different types of
> > > ports. Specifically, for the KSZ8 series, there's a unique port. This
> > > port is unique because it's the only one that can be configured with
> > > global registers, and it is only one supports tail tagging. This special
> > > port is already referenced in the driver by "dev->cpu_port", so I continued
> > > using it in my patch.
> > 
> > Ok, I understand, so for the KSZ8 family, the assumption about which
> > port will use tail tagging is baked into the hardware.
> > 
> > > It is important to note that while this port has an xMII interface, it
> > > is not the only port that could have an xMII interface. Therefore, using
> > > "dev->info->internal_phy" may not be the best way to identify this port,
> > > because there can be ports that are not global/cpu, have an xMII
> > > interface, but don't have an internal PHY.
> > 
> > Right, but since we're talking about phylink, the goal is to identify
> > the xMII ports, not the CPU ports... This is a particularly denatured
> > case because the xMII port is global and is also the CPU port.
> 
> I see. Do you have any suggestions for a better or more suitable
> implementation? I'm open to ideas.

Trying to answer here for both questions. In the RFC/RFT patch set I had
posted, I introduced the concept of "wacky" registers, which are registers
which should be per port (and are accessed as per-port by the driver),
but because there is a single such port in the switch, the hardware
design degenerated into moving them in the global area. Nonetheless,
treating the xMII global registers as per-port makes it possible for the
common driver to share more code between KSZ8 and others.

If you look at ksz9477_phylink_mac_link_up() - renamed to just
ksz_phylink_mac_link_up() in my patch set - hard enough, you can see
that it makes an attempt to generalize the "link up" procedure for all
switch families, via these regs and fields. At the end of that regfield
series, I theoretically converted KSZ8765/KSZ8794/KSZ8795 to reuse
ksz9477_phylink_mac_link_up(). Theoretically because no one commented
on whether the result still worked.

I think that regfields and that KSZ_WACKY_REG_FIELD_8() are an avenue
worth exploring here.