| Message ID | 20201222222637.3204929-1-robert.marko@sartura.hr |
|---|---|
| Headers | show |
| Series | Add support for Qualcomm QCA807x PHYs | expand |
> +++ 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. > + 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" > help > diff --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. > +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. > + return ETHTOOL_A_CABLE_RESULT_CODE_CROSS_SHORT; > + default: > + return ETHTOOL_A_CABLE_RESULT_CODE_UNSPEC; > + } > +} > +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. > +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. > +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. > + { > + 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
On Tue, 22 Dec 2020 23:26:36 +0100 Robert Marko wrote: > This adds driver for the Qualcomm QCA8072 and QCA8075 PHY-s. > > They are 2 or 5 port IEEE 802.3 clause 22 compliant > 10BASE-Te, 100BASE-TX and 1000BASE-T PHY-s. > > They feature 2 SerDes, one for PSGMII or QSGMII connection with MAC, > while second one is SGMII for connection to MAC or fiber. > > Both models have a combo port that supports 1000BASE-X and 100BASE-FX > fiber. > > Each PHY inside of QCA807x series has 2 digitally controlled output only > pins that natively drive LED-s. > But some vendors used these to driver generic LED-s controlled by > user space, so lets enable registering each PHY as GPIO controller and > add driver for it. > > This also adds the ability to specify DT properties so that 1000 Base-T > LED will also be lit up for 100 and 10 Base connections. > > This is usually done by U-boot, but boards running mainline U-boot are > not configuring this yet. > > These PHY-s are commonly used in Qualcomm IPQ40xx, IPQ60xx and IPQ807x > boards. > > Signed-off-by: Robert Marko <robert.marko@sartura.hr> > Cc: Luka Perkov <luka.perkov@sartura.hr> You need to rebase this on a more current tree: ../drivers/net/phy/qca807x.c:770:4: error: ‘struct phy_driver’ has no member named ‘ack_interrupt’; did you mean ‘handle_interrupt’? 770 | .ack_interrupt = qca807x_ack_intr, | ^~~~~~~~~~~~~ | handle_interrupt ../drivers/net/phy/qca807x.c:770:20: error: initialization of ‘irqreturn_t (*)(struct phy_device *)’ {aka ‘enum irqreturn (*)(struct phy_device *)’} from incompatible pointer type ‘int (*)(struct phy_device *)’ [-Werror=incompatible-pointer-types] 770 | .ack_interrupt = qca807x_ack_intr, | ^~~~~~~~~~~~~~~~