| Message ID | 20230519124700.635041-3-o.rempel@pengutronix.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Fine-Tune Flow Control and Speed Configurations in Microchip KSZ8xxx DSA Driver | expand |
| Context | Check | Description |
|---|---|---|
| netdev/series_format | success | Posting correctly formatted |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 8 this patch: 8 |
| netdev/cc_maintainers | success | CCed 10 of 10 maintainers |
| netdev/build_clang | success | Errors and warnings before: 8 this patch: 8 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/deprecated_api | success | None detected |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 8 this patch: 8 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 91 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
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@pengutronix.de> > Reviewed-by: Simon Horman <simon.horman@corigine.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 >
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 + * 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. + */ + 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)