| Message ID | 20240627123911.227480-4-o.rempel@pengutronix.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,v1,1/1] net: dsa: microchip: add regmap_range for KSZ9563 chip | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/apply | fail | Patch does not apply to net-next-1 |
On Thu, Jun 27, 2024 at 02:39:11PM +0200, Oleksij Rempel wrote: > The VPHY is a compatibility functionality to be able to attach network > drivers without fixed-link support to the switch, which generally > should not be needed with linux network drivers. Sorry, I don't have much to base my judgement upon. I did search for the "VPHY" string and found it to be accessed in the dev_ops->r_phy() and dev_ops->w_phy() implementations, suggesting that it is more than just that? These methods are used for accessing the registers of the embedded PHYs for user ports. I don't see what is the connection with RGMII on the CPU port.
On Fri, Jun 28, 2024 at 01:38:18AM +0300, Vladimir Oltean wrote: > On Thu, Jun 27, 2024 at 02:39:11PM +0200, Oleksij Rempel wrote: > > The VPHY is a compatibility functionality to be able to attach network > > drivers without fixed-link support to the switch, which generally > > should not be needed with linux network drivers. > > Sorry, I don't have much to base my judgement upon. I did search for the > "VPHY" string and found it to be accessed in the dev_ops->r_phy() and > dev_ops->w_phy() implementations, suggesting that it is more than just > that? These methods are used for accessing the registers of the embedded > PHYs for user ports. I don't see what is the connection with RGMII on > the CPU port. My understanding of the VPHY concept in the LAN937x switches is as follows: The VPHY in the LAN937x series provides an emulated MII management interface (MDIO) per IEEE 802.3, allowing a MAC with an unmodified driver to interact with the switch as if it is attached to a single port PHY. This emulation includes a full bank of registers that comply with IEEE 802.3 and supports auto-negotiation to configure link parameters. At the same time, this functionality seems to be used to handle clock crossings for integrated PHYs. To avoid a degradation of SPI_CLK, an indirect mechanism has been added to the VPHY for accessing the PHY registers. However, when VPHY mode is enabled, it defaults to a 100Mbit link speed during auto-negotiation, particularly affecting RGMII links. This behavior overrides the MAC configuration set via the P_XMII_CTRL_1 register, which should allow for choosing between 10, 100, and 1000Mbit speeds, as done similarly in the KSZ9477 variants. The problem arises because, with VPHY enabled, the MAC configuration on the CPU port is ignored, and the system is stuck at the default 100Mbit speed. Disabling the VPHY output ensures that the MAC configuration set via the P_XMII_CTRL_1 register is respected, allowing the CPU port to operate at the desired speed (10, 100, or 1000Mbit). I tried to configure the CPU MAC by using the VPHY interface but had no luck with it (maybe I handled it wrong). This change was tested on my system, and I do not see a visible degradation in the functionality of the integrated PHYs. This might still work because the SPI clock on my board is limited to 5MHz. The following article seems to confirm the behavior I observed and supports the proposed solution: https://microchip.my.site.com/s/article/LAN937X-The-required-configuration-for-the-external-MAC-port-to-operate-at-RGMII-to-RGMII-1Gbps-link-speed Regards, Oleksij
Am Freitag, dem 28.06.2024 um 01:38 +0300 schrieb Vladimir Oltean: > On Thu, Jun 27, 2024 at 02:39:11PM +0200, Oleksij Rempel wrote: > > The VPHY is a compatibility functionality to be able to attach network > > drivers without fixed-link support to the switch, which generally > > should not be needed with linux network drivers. > > Sorry, I don't have much to base my judgement upon. I did search for the > "VPHY" string and found it to be accessed in the dev_ops->r_phy() and > dev_ops->w_phy() implementations, suggesting that it is more than just > that? These methods are used for accessing the registers of the embedded > PHYs for user ports. I don't see what is the connection with RGMII on > the CPU port. There is a bit of a mixup with the names here. The VPHY (as in virtual PHY) is a emulated PHY register space accessible via MDIO to allow operating systems that don't support the concept of direct MAC to MAC connections to work with the switch. However, it is buggy and the emulated auto-negotiation does the wrong thing for RGMII interfaces. As this part isn't needed for Linux we disable it with this patch, or to be precise the VPHY itself isn't disabled, but rather the result of the VPHY state machine isn't allowed to override explicit link configurations in other registers anymore. The VPHY used by the driver to access the registers of real PHYs is described in the datasheet like this: "Direct access to the PHY registers via SPI requires a reduced SPI_CLK frequency. This is due to the latency incurred from clock crossings from the internal bus and into the PHY. To avoid a degradation of SPI_CLK, an indirect mechanism has been added to the VPHY for accessing the PHY registers via Indirect Address Register, Indirect Data Register, and Indirect Control Register." This mechanism is located in the VPHY register range, but otherwise has nothing to do with the VPHY state machine and is also not affected by this patch. Regards, Lucas
diff --git a/drivers/net/dsa/microchip/lan937x_main.c b/drivers/net/dsa/microchip/lan937x_main.c index b6ef48a655735..9cd3ff38ed66b 100644 --- a/drivers/net/dsa/microchip/lan937x_main.c +++ b/drivers/net/dsa/microchip/lan937x_main.c @@ -400,6 +400,9 @@ int lan937x_setup(struct dsa_switch *ds) lan937x_cfg(dev, REG_SW_GLOBAL_OUTPUT_CTRL__1, (SW_CLK125_ENB | SW_CLK25_ENB), true); + /* disable VPHY output*/ + ksz_rmw32(dev, REG_SW_CFG_STRAP_OVR, SW_VPHY_DISABLE, SW_VPHY_DISABLE); + return 0; } diff --git a/drivers/net/dsa/microchip/lan937x_reg.h b/drivers/net/dsa/microchip/lan937x_reg.h index e36bcb155f545..be553e23a964e 100644 --- a/drivers/net/dsa/microchip/lan937x_reg.h +++ b/drivers/net/dsa/microchip/lan937x_reg.h @@ -37,6 +37,10 @@ #define SW_CLK125_ENB BIT(1) #define SW_CLK25_ENB BIT(0) +/* 2 - PHY Control */ +#define REG_SW_CFG_STRAP_OVR 0x214 +#define SW_VPHY_DISABLE BIT(31) + /* 3 - Operation Control */ #define REG_SW_OPERATION 0x0300