linux-kernel - Re: [PATCH net-next v3 2/2] net: dsa: microchip: ksz8: Add function to configure downstream ports for KSZ8xxx

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 for Android: free password hash cracker in your pocket

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

Message-ID: <ZGc7S7hp5DMmNh2W@corigine.com>
Date:   Fri, 19 May 2023 11:03:07 +0200
From:   Simon Horman <simon.horman@...igine.com>
To:     Oleksij Rempel <o.rempel@...gutronix.de>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Andrew Lunn <andrew@...n.ch>,
        Eric Dumazet <edumazet@...gle.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Vladimir Oltean <olteanv@...il.com>,
        Woojung Huh <woojung.huh@...rochip.com>,
        Arun Ramadoss <arun.ramadoss@...rochip.com>,
        "Russell King (Oracle)" <linux@...linux.org.uk>,
        kernel@...gutronix.de, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org, UNGLinuxDriver@...rochip.com
Subject: Re: [PATCH net-next v3 2/2] net: dsa: microchip: ksz8: Add function
 to configure downstream ports for KSZ8xxx

On Thu, May 18, 2023 at 11:29:13AM +0200, Oleksij Rempel wrote:
> This patch introduces the function 'ksz8_downstream_link_up' to the
> Microchip KSZ8xxx driver. This function configures the flow control settings
> for the downstream ports of the switch based on desired settings and the
> current duplex mode.
> 
> The KSZ8795 switch, unlike the KSZ8873, supports asynchronous pause control.
> However, a single bit controls both RX and TX pause, so we can't enforce
> asynchronous pause control. The flow control can be set based on the
> auto-negotiation process, depending on the capabilities of both link partners.
> 
> For the KSZ8873, the PORT_FORCE_FLOW_CTRL bit can be set by the hardware
> bootstrap, ignoring the auto-negotiation result. Therefore, even in
> auto-negotiation mode, we need to ensure that the PORT_FORCE_FLOW_CTRL bit is
> correctly set.
> 
> In the absence of auto-negotiation, we will enforce synchronous pause control
> for the KSZ8795 switch.
> 
> Note: It is currently not possible to force disable flow control on a port if
> we still advertise pause support. This configuration is not currently supported
> by Linux, and it may not make practical sense. However, it's essential to
> understand this limitation when working with the KSZ8873 and similar devices.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@...gutronix.de>

Reviewed-by: Simon Horman <simon.horman@...igine.com>

> +static void ksz8_downstream_link_up(struct ksz_device *dev, int port,
> +				   int duplex, bool tx_pause, bool rx_pause)
> +{
> +	const u16 *regs = dev->info->regs;
> +	u8 ctrl = 0;
> +	int ret;
> +
> +	/*
> +	 * The KSZ8795 switch differs from the KSZ8873 by supporting
> +	 * asynchronous pause control. However, since a single bit is used to
> +	 * control both RX and TX pause, we can't enforce asynchronous pause
> +	 * control - both TX and RX pause will be either enabled or disabled
> +	 * together.
> +	 *
> +	 * If auto-negotiation is enabled, we usually allow the flow control to
> +	 * be determined by the auto-negotiation process based on the
> +	 * capabilities of both link partners. However, for KSZ8873, the
> +	 * PORT_FORCE_FLOW_CTRL bit may be set by the hardware bootstrap,
> +	 * ignoring the auto-negotiation result. Thus, even in auto-negotiatio
> +	 * mode, we need to ensure that the PORT_FORCE_FLOW_CTRL bit is
> +	 * properly cleared.
> +	 *
> +	 * In the absence of auto-negotiation, we will enforce synchronous
> +	 * pause control for both variants of switches - KSZ8873 and KSZ8795.
> +	 */

nit: multi-line comments in networking code are like this:

	/* This is
	 * a wrap.
	 */

...