Re: [PATCH net-next v3 11/11] net: mscc: ocelot: make use of SerDes PHYs for handling their configuration
flat view
From: Florian Fainelli <f.fainelli@gmail.com>
Date: 2018-09-15 21:25:13
Also in:
linux-devicetree, linux-mips, lkml
On 09/14/18 01:16, Quentin Schulz wrote:
Previously, the SerDes muxing was hardcoded to a given mode in the MAC controller driver. Now, the SerDes muxing is configured within the Device Tree and is enforced in the MAC controller driver so we can have a lot of different SerDes configurations. Make use of the SerDes PHYs in the MAC controller to set up the SerDes according to the SerDes<->switch port mapping and the communication mode with the Ethernet PHY.
This looks good, just a few comments below: [snip]
quoted hunk ↗ jump to hunk
+ err = of_get_phy_mode(portnp); + if (err < 0) + ocelot->ports[port]->phy_mode = PHY_INTERFACE_MODE_NA; + else + ocelot->ports[port]->phy_mode = err; + + switch (ocelot->ports[port]->phy_mode) { + case PHY_INTERFACE_MODE_NA: + continue;
Would not you want to issue a message indicating that the Device Tree must be updated here? AFAICT with your patch series, this should no longer be a condition that you will hit unless you kept the old DTB around, right?
quoted hunk ↗ jump to hunk
+ case PHY_INTERFACE_MODE_SGMII: + phy_mode = PHY_MODE_SGMII; + break; + case PHY_INTERFACE_MODE_QSGMII: + phy_mode = PHY_MODE_QSGMII; + break; + default: + dev_err(ocelot->dev, + "invalid phy mode for port%d, (Q)SGMII only\n", + port); + return -EINVAL; + } + + serdes = devm_of_phy_get(ocelot->dev, portnp, NULL); + if (IS_ERR(serdes)) { + err = PTR_ERR(serdes); + if (err == -EPROBE_DEFER) {
This can be simplified into: if (err == -EPROBE_DEFER) dev_dbg(); else dev_err(); goto err_probe_ports;
quoted hunk ↗ jump to hunk
+ dev_dbg(ocelot->dev, "deferring probe\n"); + goto err_probe_ports; + } + + dev_err(ocelot->dev, "missing SerDes phys for port%d\n", + port); goto err_probe_ports; } + + ocelot->ports[port]->serdes = serdes; } register_netdevice_notifier(&ocelot_netdevice_nb);
-- Florian