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