| 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
| ||
|
Message-ID: <20230519233613.op7ziqzy5wu6zjv6@skbuf>
Date: Sat, 20 May 2023 02:36:13 +0300
From: Vladimir Oltean <olteanv@...il.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>,
Woojung Huh <woojung.huh@...rochip.com>,
Arun Ramadoss <arun.ramadoss@...rochip.com>,
"Russell King (Oracle)" <linux@...linux.org.uk>,
Simon Horman <simon.horman@...igine.com>,
kernel@...gutronix.de, linux-kernel@...r.kernel.org,
netdev@...r.kernel.org, UNGLinuxDriver@...rochip.com
Subject: Re: [PATCH net-next v4 2/2] net: dsa: microchip: ksz8: Add function
to configure ports with integrated PHYs
On Fri, May 19, 2023 at 02:47:00PM +0200, Oleksij Rempel wrote:
> This patch introduces the function 'ksz8_phy_port_link_up' to the
> Microchip KSZ8xxx driver. This function is responsible for setting up
> flow control and duplex settings for the ports that are integrated with
> PHYs.
>
> The KSZ8795 switch supports asynchronous pause control, which can't be
s/asynchronous/asymmetric/
> fully utilized since a single bit controls both RX and TX pause. Despite
> this, the flow control can be adjusted based on the auto-negotiation
> process, taking into account the capabilities of both link partners.
>
> On the other hand, the KSZ8873's PORT_FORCE_FLOW_CTRL bit can be set by
> the hardware bootstrap, which ignores the auto-negotiation result.
> Therefore, even in auto-negotiation mode, we need to ensure that this
> bit is correctly set.
>
> When auto-negotiation isn't in use, we enforce synchronous pause control
s/synchronous/symmetric/
> for the KSZ8795 switch.
>
> Please note, forcing flow control disable on a port while still
> advertising pause support isn't possible. While this scenario
> might not be practical or desired, it's important to be aware of 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>
> ---
> drivers/net/dsa/microchip/ksz8795.c | 79 +++++++++++++++++++++++++++++
> 1 file changed, 79 insertions(+)
>
> diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
> index 9eedccbf5b7c..148a93f79538 100644
> --- a/drivers/net/dsa/microchip/ksz8795.c
> +++ b/drivers/net/dsa/microchip/ksz8795.c
> @@ -1371,6 +1371,83 @@ void ksz8_config_cpu_port(struct dsa_switch *ds)
> }
> }
>
> +/**
> + * ksz8_phy_port_link_up - Configures ports with integrated PHYs
> + * @dev: The KSZ device instance.
> + * @port: The port number to configure.
> + * @duplex: The desired duplex mode.
> + * @tx_pause: If true, enables transmit pause.
> + * @rx_pause: If true, enables receive pause.
> + *
> + * Description:
> + * The function configures flow control settings for a given port based on the
> + * desired settings and current duplex mode.
> + *
> + * According to the KSZ8873 datasheet, the PORT_FORCE_FLOW_CTRL bit in the
> + * Port Control 2 register (0x1A for Port 1, 0x22 for Port 2, 0x32 for Port 3)
> + * determines how flow control is handled on the port:
> + * "1 = will always enable full-duplex flow control on the port, regardless
> + * of AN result.
> + * 0 = full-duplex flow control is enabled based on AN result."
> + *
> + * This means that the flow control behavior depends on the state of this bit:
> + * - If PORT_FORCE_FLOW_CTRL is set to 1, the switch will ignore AN results and
> + * force flow control on the port.
> + * - If PORT_FORCE_FLOW_CTRL is set to 0, the switch will enable or disable
> + * flow control based on the AN results.
> + *
> + * However, there is a potential limitation in this configuration. It is
> + * currently not possible to force disable flow control on a port if we still
> + * advertise pause support. While such a configuration is not currently
> + * supported by Linux, and may not make practical sense, it's important to be
> + * aware of this limitation when working with the KSZ8873 and similar devices.
> + */
> +static void ksz8_phy_port_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
same
> + * control both RX and TX pause, we can't enforce asynchronous pause
same
> + * 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
auto-negotiation
> + * 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
same
> + * pause control for both variants of switches - KSZ8873 and KSZ8795.
> + */
> + if (duplex) {
> + bool aneg_en = false;
> +
> + ret = ksz_pread8(dev, port, regs[P_FORCE_CTRL], &ctrl);
> + if (ret)
> + return;
> +
> + if (ksz_is_ksz88x3(dev)) {
> + if ((ctrl & PORT_AUTO_NEG_ENABLE))
> + aneg_en = true;
> + } else {
> + if (!(ctrl & PORT_AUTO_NEG_DISABLE))
> + aneg_en = true;
> + }
> +
> + if (!aneg_en && (tx_pause || rx_pause))
> + ctrl |= PORT_FORCE_FLOW_CTRL;
> + }
> +
> + ksz_prmw8(dev, port, regs[P_STP_CTRL], PORT_FORCE_FLOW_CTRL, ctrl);
> +}
> +
> /**
> * ksz8_cpu_port_link_up - Configures the CPU port of the switch.
> * @dev: The KSZ device instance.
> @@ -1420,6 +1497,8 @@ void ksz8_phylink_mac_link_up(struct ksz_device *dev, int port,
> */
> if (dev->cpu_port == port)
> ksz8_cpu_port_link_up(dev, speed, duplex, tx_pause, rx_pause);
> + else if (dev->info->internal_phy[port])
> + ksz8_phy_port_link_up(dev, port, duplex, tx_pause, rx_pause);
> }
>
> static int ksz8_handle_global_errata(struct dsa_switch *ds)
> --
> 2.39.2
>
Powered by blists - more mailing lists