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:   Fri, 19 May 2023 20:50:15 +0200
From:   Oleksij Rempel <o.rempel@pengutronix.de>
To:     Vladimir Oltean <olteanv@gmail.com>
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, UNGLinuxDriver@microchip.com,
        Eric Dumazet <edumazet@google.com>, kernel@pengutronix.de,
        netdev@vger.kernel.org, Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.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: <20230519185015.GA18246@pengutronix.de>
References: <20230519124700.635041-1-o.rempel@pengutronix.de>
 <20230519124700.635041-2-o.rempel@pengutronix.de>
 <20230519143004.luvz73jiyvnqxk4y@skbuf>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20230519143004.luvz73jiyvnqxk4y@skbuf>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

Hi Vladimir,

On Fri, May 19, 2023 at 05:30:04PM +0300, Vladimir Oltean wrote:
> On Fri, May 19, 2023 at 02:46:59PM +0200, Oleksij Rempel wrote:
> > +void ksz8_phylink_mac_link_up(struct ksz_device *dev, int port,
> > +			      unsigned int mode, phy_interface_t interface,
> > +			      struct phy_device *phydev, int speed, int duplex,
> > +			      bool tx_pause, bool rx_pause)
> > +{
> > +	/* If the port is the CPU port, apply special handling. Only the CPU
> > +	 * port is configured via global registers.
> > +	 */
> > +	if (dev->cpu_port == port)
> > +		ksz8_cpu_port_link_up(dev, speed, duplex, tx_pause, rx_pause);
> > +}
> 
> I'm sorry, but this is also baking in assumptions related to the
> topology of the tree (that the xMII port is used as a CPU port).
> The ksz8 driver may make this assumption in other places too,
> but I don't want to make it even worse to fix. Is the
> !dev->info->internal_phy[port] condition not enough here?

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.

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.

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |