Re: [PATCH 3/4] net: phy: Add Qualcomm QCA807x driver
flat view
From: Andrew Lunn <andrew@lunn.ch>
Date: 2020-12-23 01:24:53
Also in:
linux-arm-msm, lkml, netdev
quoted hunk ↗ jump to hunk
+++ b/drivers/net/phy/Kconfig@@ -264,6 +264,16 @@ config QSEMI_PHY help Currently supports the qs6612 +config QCA807X_PHY + tristate "Qualcomm QCA807X PHYs"
Kconfig is sorted based on the tristate string. So this should be after AT803X_PHY.
quoted hunk ↗ jump to hunk
+ depends on OF_MDIO + help + Adds support for the Qualcomm QCA807x PHYs. + These are 802.3 Clause 22 compliant PHYs supporting gigabit + ethernet as well as 100Base-FX and 1000Base-X fibre. + + Currently supports the QCA8072 and QCA8075 models. + config REALTEK_PHY tristate "Realtek PHYs" helpdiff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile index ca0a313423b9..5f463ce0f711 100644 --- a/drivers/net/phy/Makefile +++ b/drivers/net/phy/Makefile@@ -74,6 +74,7 @@ obj-$(CONFIG_MICROSEMI_PHY) += mscc/ obj-$(CONFIG_NATIONAL_PHY) += national.o obj-$(CONFIG_NXP_TJA11XX_PHY) += nxp-tja11xx.o obj-$(CONFIG_QSEMI_PHY) += qsemi.o +obj-$(CONFIG_QCA807X_PHY) += qca807x.o obj-$(CONFIG_REALTEK_PHY) += realtek.o
One line earlier please.
quoted hunk ↗ jump to hunk
+static int qca807x_cable_test_report_trans(int result) +{ + switch (result) { + case QCA807X_CDT_RESULTS_OK: + return ETHTOOL_A_CABLE_RESULT_CODE_OK; + case QCA807X_CDT_RESULTS_OPEN: + return ETHTOOL_A_CABLE_RESULT_CODE_OPEN; + case QCA807X_CDT_RESULTS_SAME_SHORT: + return ETHTOOL_A_CABLE_RESULT_CODE_SAME_SHORT; + case QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI1_SAME_OK: + case QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI2_SAME_OK: + case QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI3_SAME_OK: + case QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI1_SAME_OPEN: + case QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI2_SAME_OPEN: + case QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI3_SAME_OPEN: + case QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI1_SAME_SHORT: + case QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI2_SAME_SHORT: + case QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI3_SAME_SHORT:
Feel free to add an optional pair to the netlink message, indicating which pair this pair is shorted to.
quoted hunk ↗ jump to hunk
+ return ETHTOOL_A_CABLE_RESULT_CODE_CROSS_SHORT; + default: + return ETHTOOL_A_CABLE_RESULT_CODE_UNSPEC; + } +}
quoted hunk ↗ jump to hunk
+static int qca807x_cable_test_start(struct phy_device *phydev) +{ + int val, ret; + + val = phy_read(phydev, QCA807X_CDT); + /* Enable inter-pair short check as well */ + val &= ~QCA807X_CDT_ENABLE_INTER_PAIR_SHORT; + val |= QCA807X_CDT_ENABLE; + ret = phy_write(phydev, QCA807X_CDT, val);
What happens when you are using the PHY as a media converted to Fibre? Should we return -EOPNOTSUPP here? I'm assuming it cannot do cable test on a fibre pair.
quoted hunk ↗ jump to hunk
+static int qca807x_read_fiber_status(struct phy_device *phydev, bool combo_port) +{ + int ss, err, page, lpa, old_link = phydev->link; + + /* Check whether fiber page is set and set if needed */ + page = phy_read(phydev, QCA807X_CHIP_CONFIGURATION); + if (page & QCA807X_BT_BX_REG_SEL) { + page &= ~QCA807X_BT_BX_REG_SEL; + phy_write(phydev, QCA807X_CHIP_CONFIGURATION, page); + } + + /* Update the link, but return if there was an error */ + err = genphy_update_link(phydev); + if (err) + return err; + + /* why bother the PHY if nothing can have changed */ + if (phydev->autoneg == AUTONEG_ENABLE && old_link && phydev->link) + return 0; + + phydev->speed = SPEED_UNKNOWN; + phydev->duplex = DUPLEX_UNKNOWN; + phydev->pause = 0; + phydev->asym_pause = 0; + + if (phydev->autoneg == AUTONEG_ENABLE && phydev->autoneg_complete) { + lpa = phy_read(phydev, MII_LPA); + if (lpa < 0) + return lpa; + + linkmode_mod_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, + phydev->lp_advertising, lpa & LPA_LPACK); + linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT, + phydev->lp_advertising, lpa & LPA_1000XFULL); + linkmode_mod_bit(ETHTOOL_LINK_MODE_Pause_BIT, + phydev->lp_advertising, lpa & LPA_1000XPAUSE); + linkmode_mod_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, + phydev->lp_advertising, + lpa & LPA_1000XPAUSE_ASYM); + + phy_resolve_aneg_linkmode(phydev); + }
This looks a lot like genphy_c37_read_status(). You could call it, and then over write the speed and duplex with values from the PHY specific registers.
quoted hunk ↗ jump to hunk
+static int qca807x_config_intr(struct phy_device *phydev) +{ + int ret, val; + + val = phy_read(phydev, QCA807X_INTR_ENABLE); + + if (phydev->interrupts == PHY_INTERRUPT_ENABLED) { + /* Check for combo port as it has fewer interrupts */ + if (phy_read(phydev, QCA807X_CHIP_CONFIGURATION)) { + val |= QCA807X_INTR_ENABLE_SPEED_CHANGED; + val |= QCA807X_INTR_ENABLE_LINK_FAIL; + val |= QCA807X_INTR_ENABLE_LINK_SUCCESS; + } else { + val |= QCA807X_INTR_ENABLE_AUTONEG_ERR; + val |= QCA807X_INTR_ENABLE_SPEED_CHANGED; + val |= QCA807X_INTR_ENABLE_DUPLEX_CHANGED; + val |= QCA807X_INTR_ENABLE_LINK_FAIL; + val |= QCA807X_INTR_ENABLE_LINK_SUCCESS; + } + ret = phy_write(phydev, QCA807X_INTR_ENABLE, val); + } else { + ret = phy_write(phydev, QCA807X_INTR_ENABLE, 0); + } + + return ret; +} + +static int qca807x_ack_intr(struct phy_device *phydev) +{ + int ret; + + ret = phy_read(phydev, QCA807X_INTR_STATUS); + + return (ret < 0) ? ret : 0; +}
You need to rework this to follow what Ioana has done for interrupt handling.
quoted hunk ↗ jump to hunk
+ { + PHY_ID_MATCH_EXACT(PHY_ID_QCA8075), + .name = "Qualcomm QCA8075", + .flags = PHY_POLL_CABLE_TEST, + /* PHY_GBIT_FEATURES */ + .probe = qca807x_probe, + .config_init = qca807x_config, + .read_status = qca807x_read_status, + .config_intr = qca807x_config_intr, + .ack_interrupt = qca807x_ack_intr, + .soft_reset = genphy_soft_reset, + .get_tunable = qca807x_get_tunable, + .set_tunable = qca807x_set_tunable, + .cable_test_start = qca807x_cable_test_start, + .cable_test_get_status = qca807x_cable_test_get_status, + }, + { + PHY_ID_MATCH_EXACT(PHY_ID_QCA807X_PSGMII), + .name = "Qualcomm QCA807x PSGMII", + .probe = qca807x_psgmii_config,
This looks odd.
Andrew