Hello,
It turns out that my LinkStation LS421DE has a bug on the ethernet port wich makes the connection to behave randomly unstable. After hours of testing I catched the problem.
It is a discrepancy between the ethernet voltage and the PHY voltage:
- The ethernet at the Marvell SoC is configured by default to operate at 3.3 volts
- The PHY only supports 1.8 volts.
The SoC can be configured to force the RGMII pins use the 1.8volts correct voltage by changing the value of a couple of bytes in the internal registers. Looks like the manufacturer wasn't aware of this, because it isn't configured by the bootloader nor the stock firmware.
In Openwrt of course the problem is still present. So I looked for a solution. There are several options to fix it:
- Setting the registers from userspace: easy, it can be done with devmem, but Openwrt comes with /dev/mem disabled. So it needs compiling an Openwrt image with /dev/mem enabled. Not easy for all users.
- Patching the bootloader: I already did it patching the stuff where the RAM is initialized, it apparently works fine. But again this is not an easy option, to flash a new bootloader which can cause a potential brick.
- Patching the kernel: this seems to be the best option, it won't require user intervention once Openwrt is installed. I also made a patch, see below.
This is the mvneta driver, where I made the patch:
https://elixir.bootlin.com/linux/v5.4.41/source/drivers/net/ethernet/marvell/mvneta.c
I noticed this is not the only one manufacturer which doesn't set the voltage correctly. Therefore other devices may benefit the patch.
However I'm not sure if patching mvneta is the best idea for fixing this issue. From my point of view it is ok, because it unconditionally will establish the correct voltage without requiring any manual intervention, and saving headaches because the bug isn't easy to detect.
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -459,6 +459,8 @@
bool neta_armada3700;
u16 rx_offset_correction;
const struct mbus_dram_target_info *dram_target_info;
+
+ phys_addr_t regaddr;
};
/* The mvneta_tx_desc and mvneta_rx_desc structures describe the
@@ -3653,6 +3655,63 @@
.mac_link_up = mvneta_mac_link_up,
};
+#define MARVELL_PHY_ID_88E1318S 0x01410e90
+#define MARVELL_PHY_ID_88E1518 0x01410dd1
+#define VDDO_PADS_VOLTAGE_OFFS 0x184E0
+
+/* Armada 370/XP RGMII pins voltage is 3v3 as default. It must be set
+ * to 1v8 with Marvell 88E1318S/88E1518 PHYs since they only support
+ * this voltage. The connection will be randomly unstable otherwise.
+ */
+static void mvneta_fix_voltage(struct mvneta_port *pp)
+{
+ void __iomem *vddo_addr;
+ bool is_port0;
+ u32 reg, phyid;
+
+ /* 0x70000 is the port0, 0x74000 is the port1 */
+ is_port0 = !(pp->regaddr & GENMASK(14, 12));
+
+ /* Get the PHY ID */
+ phyid = pp->dev->phydev->phy_id;
+
+ /* base address: 0xf1000000 or 0xd0000000 */
+ vddo_addr = ioremap((pp->regaddr & GENMASK(31,24)) +
+ VDDO_PADS_VOLTAGE_OFFS, 4);
+
+ /* Set the RGMII pins voltage at the SoC to 1v8 */
+ if (phyid == MARVELL_PHY_ID_88E1318S ||
+ phyid == MARVELL_PHY_ID_88E1518) {
+ if (of_machine_is_compatible("marvell,armada370")){
+ if (is_port0) {
+ reg = readl(vddo_addr);
+ /* MPP pins 5 to 16 */
+ reg &= ~GENMASK(3,2);
+ } else {
+ reg = readl(vddo_addr);
+ /* MPP pins 17 to 31 */
+ reg &= ~GENMASK(5,4);
+ }
+ }
+ else if (of_machine_is_compatible("marvell,armadaxp")) {
+ if (is_port0) {
+ reg = readl(vddo_addr);
+ /* MPP pins 0 to 11 */
+ reg &= ~GENMASK(1,0);
+ } else {
+ reg = readl(vddo_addr);
+ /* MPP pins 12 to 23 */
+ reg &= ~GENMASK(3,2);
+ }
+ }
+ else
+ return;
+
+ writel(reg, vddo_addr);
+ netdev_info(pp->dev, "VDDO voltage asserted: 1v8\n");
+ }
+}
+
static int mvneta_mdio_probe(struct mvneta_port *pp)
{
struct ethtool_wolinfo wol = { .cmd = ETHTOOL_GWOL };
@@ -3661,6 +3720,9 @@
if (err)
netdev_err(pp->dev, "could not attach PHY: %d\n", err);
+ if (of_machine_is_compatible("marvell,armada-370-xp"))
+ mvneta_fix_voltage(pp);
+
phylink_ethtool_get_wol(pp->phylink, &wol);
device_set_wakeup_capable(&pp->dev->dev, !!wol.supported);
@@ -4553,6 +4615,7 @@
pp->comphy = comphy;
pp->phy_interface = phy_mode;
pp->dn = dn;
+ pp->regaddr = pdev->resource->start;
pp->rxq_def = rxq_def;
pp->indir[0] = rxq_def;
Another option could be to use device tree adding a property to the mvneta driver. But again I'm touching this driver. Or maybe the MVEBU pinctrl driver... I don't see how to articulate both drivers with the device tree to achieve the target.
Is it good this patch?, would be accepted by Openwrt or upstream?
Regards
Daniel
