[PATCH] net: phy: add qca8081 ethernet phy driver

[PATCH] net: phy: add qca8081 ethernet phy driver

[Luo Jie]

qca8081 is industry’s lowest cost and power 1-port 2.5G/1G Ethernet PHY
chip, which implements SGMII/SGMII+ for interface to SoC.

Signed-off-by: Luo Jie <luoj@codeaurora.org>
---
 drivers/net/phy/Kconfig   |   6 +
 drivers/net/phy/Makefile  |   1 +
 drivers/net/phy/qca808x.c | 573 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 580 insertions(+)
 create mode 100644 drivers/net/phy/qca808x.c

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index c56f703ae998..26cb1c2ffd17 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -343,3 +343,9 @@ endif # PHYLIB
 config MICREL_KS8995MA
 	tristate "Micrel KS8995MA 5-ports 10/100 managed Ethernet switch"
 	depends on SPI
+
+config QCA808X_PHY
+	tristate "Qualcomm Atheros QCA808X PHYs"
+	depends on REGULATOR
+	help
+	  Currently supports the QCA8081 model
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 172bb193ae6a..9ef477d79588 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -84,3 +84,4 @@ obj-$(CONFIG_STE10XP)		+= ste10Xp.o
 obj-$(CONFIG_TERANETICS_PHY)	+= teranetics.o
 obj-$(CONFIG_VITESSE_PHY)	+= vitesse.o
 obj-$(CONFIG_XILINX_GMII2RGMII) += xilinx_gmii2rgmii.o
+obj-$(CONFIG_QCA808X_PHY)	+= qca808x.o
diff --git a/drivers/net/phy/qca808x.c b/drivers/net/phy/qca808x.c
new file mode 100644
index 000000000000..6cb8abae8c8f
--- /dev/null
+++ b/drivers/net/phy/qca808x.c
@@ -0,0 +1,573 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for Qualcomm QCA808x PHY
+ * Author: Luo Jie <luoj@codeaurora.org>
+ */
+
+#include <linux/module.h>
+#include <linux/etherdevice.h>
+#include <linux/phy.h>
+#include <linux/bitfield.h>
+
+#define QCA8081_PHY_ID					0x004DD101
+
+/* MII special status */
+#define QCA808X_PHY_SPEC_STATUS				0x11
+#define QCA808X_STATUS_FULL_DUPLEX			BIT(13)
+#define QCA808X_STATUS_LINK_PASS			BIT(10)
+#define QCA808X_STATUS_SPEED_MASK			GENMASK(9, 7)
+#define QCA808X_STATUS_SPEED_100MBS			1
+#define QCA808X_STATUS_SPEED_1000MBS			2
+#define QCA808X_STATUS_SPEED_2500MBS			4
+
+/* MII interrupt enable & status */
+#define QCA808X_PHY_INTR_MASK				0x12
+#define QCA808X_PHY_INTR_STATUS				0x13
+#define QCA808X_INTR_ENABLE_FAST_RETRAIN_FAIL		BIT(15)
+#define QCA808X_INTR_ENABLE_SPEED_CHANGED		BIT(14)
+#define QCA808X_INTR_ENABLE_DUPLEX_CHANGED		BIT(13)
+#define QCA808X_INTR_ENABLE_PAGE_RECEIVED		BIT(12)
+#define QCA808X_INTR_ENABLE_LINK_FAIL			BIT(11)
+#define QCA808X_INTR_ENABLE_LINK_SUCCESS		BIT(10)
+#define QCA808X_INTR_ENABLE_POE				BIT(1)
+#define QCA808X_INTR_ENABLE_WOL				BIT(0)
+
+/* MII DBG address & data */
+#define QCA808X_PHY_DEBUG_ADDR				0x1d
+#define QCA808X_PHY_DEBUG_DATA				0x1e
+
+/* Conifg seed */
+#define QCA808X_PHY_DEBUG_LOCAL_SEED			9
+#define QCA808X_MASTER_SLAVE_SEED_ENABLE		BIT(1)
+#define QCA808X_MASTER_SLAVE_SEED_CFG			GENMASK(12, 2)
+#define QCA808X_MASTER_SLAVE_SEED_RANGE			0x32
+
+/* ADC threshold */
+#define QCA808X_PHY_DEBUG_ADC_THRESHOLD			0x2c80
+#define QCA808X_ADC_THRESHOLD_MASK			GENMASK(7, 0)
+#define QCA808X_ADC_THRESHOLD_80MV			0
+#define QCA808X_ADC_THRESHOLD_100MV			0xf0
+#define QCA808X_ADC_THRESHOLD_200MV			0x0f
+#define QCA808X_ADC_THRESHOLD_300MV			0xff
+
+/* PMA control */
+#define QCA808X_PHY_MMD1_PMA_CONTROL			0x0
+#define QCA808X_PMA_CONTROL_SPEED_MASK			(BIT(13) | BIT(6))
+#define QCA808X_PMA_CONTROL_2500M			(BIT(13) | BIT(6))
+#define QCA808X_PMA_CONTROL_1000M			BIT(6)
+#define QCA808X_PMA_CONTROL_100M			BIT(13)
+#define QCA808X_PMA_CONTROL_10M				0x0
+
+/* PMA capable */
+#define QCA808X_PHY_MMD1_PMA_CAP_REG			0x4
+#define QCA808X_STATUS_2500T_FD_CAPS			BIT(13)
+
+/* PMA type */
+#define QCA808X_PHY_MMD1_PMA_TYPE			0x7
+#define QCA808X_PMA_TYPE_MASK				GENMASK(5, 0)
+#define QCA808X_PMA_TYPE_2500M				0x30
+#define QCA808X_PMA_TYPE_1000M				0xc
+#define QCA808X_PMA_TYPE_100M				0xe
+#define QCA808X_PMA_TYPE_10M				0xf
+
+/* CLD control */
+#define QCA808X_PHY_MMD3_ADDR_CLD_CTRL7			0x8007
+#define QCA808X_8023AZ_AFE_CTRL_MASK			GENMASK(8, 4)
+#define QCA808X_8023AZ_AFE_EN				0x90
+
+/* AZ control */
+#define QCA808X_PHY_MMD3_AZ_TRAINING_CTRL		0x8008
+#define QCA808X_MMD3_AZ_TRAINING_VAL			0x1c32
+
+/* WOL control */
+#define QCA808X_PHY_MMD3_WOL_CTRL			0x8012
+#define QCA808X_WOL_EN					BIT(5)
+
+#define QCA808X_PHY_MMD3_LOCAL_MAC_ADDR_0_15_OFFSET	0x804c
+#define QCA808X_PHY_MMD3_LOCAL_MAC_ADDR_16_31_OFFSET	0x804b
+#define QCA808X_PHY_MMD3_LOCAL_MAC_ADDR_32_47_OFFSET	0x804a
+
+/* AN 2.5G */
+#define QCA808X_PHY_MMD7_AUTONEGOTIATION_CONTROL	0x20
+#define QCA808X_ADVERTISE_2500FULL			BIT(7)
+#define QCA808X_FAST_RETRAIN_2500BT			BIT(5)
+#define QCA808X_ADV_LOOP_TIMING				BIT(0)
+
+/* Fast retrain related registers */
+#define QCA808X_PHY_MMD1_FAST_RETRAIN_STATUS_CTL	0x93
+#define QCA808X_FAST_RETRAIN_CTRL			0x1
+
+#define QCA808X_PHY_MMD1_MSE_THRESHOLD_20DB		0x8014
+#define QCA808X_MSE_THRESHOLD_20DB_VALUE		0x529
+
+#define QCA808X_PHY_MMD1_MSE_THRESHOLD_17DB		0x800E
+#define QCA808X_MSE_THRESHOLD_17DB_VALUE		0x341
+
+#define QCA808X_PHY_MMD1_MSE_THRESHOLD_27DB		0x801E
+#define QCA808X_MSE_THRESHOLD_27DB_VALUE		0x419
+
+#define QCA808X_PHY_MMD1_MSE_THRESHOLD_28DB		0x8020
+#define QCA808X_MSE_THRESHOLD_28DB_VALUE		0x341
+
+#define QCA808X_PHY_MMD7_TOP_OPTION1			0x901c
+#define QCA808X_TOP_OPTION1_DATA			0x0
+
+#define QCA808X_PHY_MMD7_ADDR_EEE_LP_ADVERTISEMENT	0x40
+#define QCA808X_EEE_ADV_THP				0x8
+
+#define QCA808X_PHY_MMD3_DEBUG_1			0xa100
+#define QCA808X_MMD3_DEBUG_1_VALUE			0x9203
+#define QCA808X_PHY_MMD3_DEBUG_2			0xa101
+#define QCA808X_MMD3_DEBUG_2_VALUE			0x48ad
+#define QCA808X_PHY_MMD3_DEBUG_3			0xa103
+#define QCA808X_MMD3_DEBUG_3_VALUE			0x1698
+#define QCA808X_PHY_MMD3_DEBUG_4			0xa105
+#define QCA808X_MMD3_DEBUG_4_VALUE			0x8001
+#define QCA808X_PHY_MMD3_DEBUG_5			0xa106
+#define QCA808X_MMD3_DEBUG_5_VALUE			0x1111
+#define QCA808X_PHY_MMD3_DEBUG_6			0xa011
+#define QCA808X_MMD3_DEBUG_6_VALUE			0x5f85
+
+static int qca808x_debug_reg_read(struct phy_device *phydev, u16 reg)
+{
+	int ret;
+
+	ret = phy_write(phydev, QCA808X_PHY_DEBUG_ADDR, reg);
+	if (ret)
+		return ret;
+
+	return phy_read(phydev, QCA808X_PHY_DEBUG_DATA);
+}
+
+static int qca808x_debug_reg_modify(struct phy_device *phydev, u16 reg,
+				 u16 mask, u16 val)
+{
+	u16 phy_data;
+	int ret;
+
+	ret = qca808x_debug_reg_read(phydev, reg);
+	if (ret < 0)
+		return ret;
+
+	phy_data = ret & 0xffff;
+	phy_data &= ~mask;
+	phy_data |= val;
+
+	return phy_write(phydev, QCA808X_PHY_DEBUG_DATA, phy_data);
+}
+
+static int qca808x_get_2500caps(struct phy_device *phydev)
+{
+	int phy_data;
+
+	phy_data = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, QCA808X_PHY_MMD1_PMA_CAP_REG);
+
+	return (phy_data & QCA808X_STATUS_2500T_FD_CAPS) ? 1 : 0;
+}
+
+static int qca808x_get_features(struct phy_device *phydev)
+{
+	linkmode_mod_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, phydev->supported,
+			qca808x_get_2500caps(phydev));
+
+	return genphy_read_abilities(phydev);
+}
+
+static int qca808x_phy_fast_retrain_cfg(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = phy_write_mmd(phydev, MDIO_MMD_AN, QCA808X_PHY_MMD7_AUTONEGOTIATION_CONTROL,
+			QCA808X_ADVERTISE_2500FULL |
+			QCA808X_FAST_RETRAIN_2500BT |
+			QCA808X_ADV_LOOP_TIMING);
+	if (ret)
+		return ret;
+
+	phy_write_mmd(phydev, MDIO_MMD_PMAPMD, QCA808X_PHY_MMD1_FAST_RETRAIN_STATUS_CTL,
+			QCA808X_FAST_RETRAIN_CTRL);
+	phy_write_mmd(phydev, MDIO_MMD_PMAPMD, QCA808X_PHY_MMD1_MSE_THRESHOLD_20DB,
+			QCA808X_MSE_THRESHOLD_20DB_VALUE);
+	phy_write_mmd(phydev, MDIO_MMD_PMAPMD, QCA808X_PHY_MMD1_MSE_THRESHOLD_17DB,
+			QCA808X_MSE_THRESHOLD_17DB_VALUE);
+	phy_write_mmd(phydev, MDIO_MMD_PMAPMD, QCA808X_PHY_MMD1_MSE_THRESHOLD_27DB,
+			QCA808X_MSE_THRESHOLD_27DB_VALUE);
+	phy_write_mmd(phydev, MDIO_MMD_PMAPMD, QCA808X_PHY_MMD1_MSE_THRESHOLD_28DB,
+			QCA808X_MSE_THRESHOLD_28DB_VALUE);
+	phy_write_mmd(phydev, MDIO_MMD_AN, QCA808X_PHY_MMD7_ADDR_EEE_LP_ADVERTISEMENT,
+			QCA808X_EEE_ADV_THP);
+	phy_write_mmd(phydev, MDIO_MMD_AN, QCA808X_PHY_MMD7_TOP_OPTION1,
+			QCA808X_TOP_OPTION1_DATA);
+	phy_write_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_DEBUG_1,
+			QCA808X_MMD3_DEBUG_1_VALUE);
+	phy_write_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_DEBUG_4,
+			QCA808X_MMD3_DEBUG_4_VALUE);
+	phy_write_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_DEBUG_5,
+			QCA808X_MMD3_DEBUG_5_VALUE);
+	phy_write_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_DEBUG_3,
+			QCA808X_MMD3_DEBUG_3_VALUE);
+	phy_write_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_DEBUG_6,
+			QCA808X_MMD3_DEBUG_6_VALUE);
+	phy_write_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_DEBUG_2,
+			QCA808X_MMD3_DEBUG_2_VALUE);
+
+	return 0;
+}
+
+static int qca808x_phy_ms_random_seed_set(struct phy_device *phydev)
+{
+	u16 seed_value = (prandom_u32() % QCA808X_MASTER_SLAVE_SEED_RANGE) << 2;
+
+	return qca808x_debug_reg_modify(phydev, QCA808X_PHY_DEBUG_LOCAL_SEED,
+			QCA808X_MASTER_SLAVE_SEED_CFG, seed_value);
+}
+
+static int qca808x_phy_ms_seed_enable(struct phy_device *phydev, bool enable)
+{
+	u16 seed_enable = 0;
+
+	if (enable)
+		seed_enable = QCA808X_MASTER_SLAVE_SEED_ENABLE;
+
+	return qca808x_debug_reg_modify(phydev, QCA808X_PHY_DEBUG_LOCAL_SEED,
+			QCA808X_MASTER_SLAVE_SEED_ENABLE, seed_enable);
+}
+
+static int qca808x_config_init(struct phy_device *phydev)
+{
+	int ret;
+
+	/* Active adc&vga on 802.3az for the link 1000M and 100M */
+	ret = phy_modify_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_ADDR_CLD_CTRL7,
+			QCA808X_8023AZ_AFE_CTRL_MASK, QCA808X_8023AZ_AFE_EN);
+	if (ret)
+		return ret;
+
+	/* Adjust the threshold on 802.3az for the link 1000M */
+	ret = phy_write_mmd(phydev, MDIO_MMD_PCS,
+			QCA808X_PHY_MMD3_AZ_TRAINING_CTRL, QCA808X_MMD3_AZ_TRAINING_VAL);
+	if (ret)
+		return ret;
+
+	/* Config the fast retrain for the link 2500M */
+	ret = qca808x_phy_fast_retrain_cfg(phydev);
+	if (ret)
+		return ret;
+
+	/* Configure ramdom seed to make phy linked as slave mode for link 2500M */
+	ret = qca808x_phy_ms_random_seed_set(phydev);
+	if (ret)
+		return ret;
+
+	/* Enable seed */
+	ret = qca808x_phy_ms_seed_enable(phydev, true);
+	if (ret)
+		return ret;
+
+	/* Configure adc threshold as 100mv for the link 10M */
+	return qca808x_debug_reg_modify(phydev, QCA808X_PHY_DEBUG_ADC_THRESHOLD,
+			QCA808X_ADC_THRESHOLD_MASK, QCA808X_ADC_THRESHOLD_100MV);
+}
+
+static int qca808x_ack_interrupt(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = phy_read(phydev, QCA808X_PHY_INTR_STATUS);
+
+	return (ret < 0) ? ret : 0;
+}
+
+static int qca808x_config_intr(struct phy_device *phydev)
+{
+	int ret;
+	int intr_ctl = 0;
+
+	if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
+		ret = qca808x_ack_interrupt(phydev);
+		if (ret)
+			return ret;
+
+		intr_ctl = phy_read(phydev, QCA808X_PHY_INTR_MASK);
+		if (intr_ctl < 0)
+			return intr_ctl;
+
+		intr_ctl |= QCA808X_INTR_ENABLE_FAST_RETRAIN_FAIL |
+			QCA808X_INTR_ENABLE_SPEED_CHANGED |
+			QCA808X_INTR_ENABLE_DUPLEX_CHANGED |
+			QCA808X_INTR_ENABLE_LINK_FAIL |
+			QCA808X_INTR_ENABLE_LINK_SUCCESS;
+		ret = phy_write(phydev, QCA808X_PHY_INTR_MASK, intr_ctl);
+	} else {
+		ret = phy_write(phydev, QCA808X_PHY_INTR_MASK, intr_ctl);
+		if (ret)
+			return ret;
+
+		ret = qca808x_ack_interrupt(phydev);
+	}
+
+	return ret;
+}
+
+static irqreturn_t qca808x_handle_interrupt(struct phy_device *phydev)
+{
+	int intr_status, intr_mask;
+
+	intr_status = phy_read(phydev, QCA808X_PHY_INTR_STATUS);
+	if (intr_status < 0) {
+		phy_error(phydev);
+		return IRQ_NONE;
+	}
+
+	intr_mask = phy_read(phydev, QCA808X_PHY_INTR_MASK);
+	if (intr_mask < 0) {
+		phy_error(phydev);
+		return IRQ_NONE;
+	}
+
+	if (!(intr_status & intr_mask))
+		return IRQ_NONE;
+
+	phy_start_machine(phydev);
+
+	return IRQ_HANDLED;
+}
+
+static int qca808x_set_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol)
+{
+	struct net_device *ndev = phydev->attached_dev;
+	const u8 *mac;
+	int ret;
+	unsigned int i, offsets[] = {
+		QCA808X_PHY_MMD3_LOCAL_MAC_ADDR_32_47_OFFSET,
+		QCA808X_PHY_MMD3_LOCAL_MAC_ADDR_16_31_OFFSET,
+		QCA808X_PHY_MMD3_LOCAL_MAC_ADDR_0_15_OFFSET,
+	};
+
+	if (!ndev)
+		return -ENODEV;
+
+	if (wol->wolopts & WAKE_MAGIC) {
+		mac = (const u8 *) ndev->dev_addr;
+		if (!is_valid_ether_addr(mac))
+			return -EINVAL;
+
+		for (i = 0; i < 3; i++)
+			phy_write_mmd(phydev, MDIO_MMD_PCS, offsets[i],
+				      mac[(i * 2) + 1] | (mac[(i * 2)] << 8));
+
+		/* clear the pending wol interrupt */
+		phy_read(phydev, QCA808X_PHY_INTR_STATUS);
+
+		ret = phy_modify(phydev, QCA808X_PHY_INTR_MASK, 0, QCA808X_INTR_ENABLE_WOL);
+		if (ret)
+			return ret;
+
+		ret = phy_modify_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_WOL_CTRL,
+				0, QCA808X_WOL_EN);
+	} else {
+		ret = phy_modify(phydev, QCA808X_PHY_INTR_MASK, QCA808X_INTR_ENABLE_WOL, 0);
+		if (ret)
+			return ret;
+
+		ret = phy_modify_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_WOL_CTRL,
+				QCA808X_WOL_EN, 0);
+	}
+
+	return ret;
+}
+
+static void qca808x_get_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol)
+{
+	int ret;
+
+	wol->supported = WAKE_MAGIC;
+	wol->wolopts = 0;
+
+	ret = phy_read_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_WOL_CTRL);
+	if (ret < 0)
+		return;
+
+	if (ret & QCA808X_WOL_EN)
+		wol->wolopts |= WAKE_MAGIC;
+}
+
+static int qca808x_suspend(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = phy_read_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_WOL_CTRL);
+	if (ret < 0)
+		return ret;
+
+	if (ret & QCA808X_WOL_EN)
+		return 0;
+
+	return genphy_suspend(phydev);
+}
+
+static int qca808x_speed_forced(struct phy_device *phydev)
+{
+	u16 speed_ctrl, type_ctrl;
+	int ret;
+
+	switch (phydev->speed) {
+	case SPEED_2500:
+		speed_ctrl = QCA808X_PMA_CONTROL_2500M;
+		type_ctrl = QCA808X_PMA_TYPE_2500M;
+		break;
+	case SPEED_1000:
+		speed_ctrl = QCA808X_PMA_CONTROL_1000M;
+		type_ctrl = QCA808X_PMA_TYPE_1000M;
+		break;
+	case SPEED_100:
+		speed_ctrl = QCA808X_PMA_CONTROL_100M;
+		type_ctrl = QCA808X_PMA_TYPE_100M;
+		break;
+	default:
+		speed_ctrl = QCA808X_PMA_CONTROL_10M;
+		type_ctrl = QCA808X_PMA_TYPE_10M;
+		break;
+	}
+
+	ret = phy_modify_mmd(phydev, MDIO_MMD_PMAPMD, QCA808X_PHY_MMD1_PMA_CONTROL,
+			QCA808X_PMA_CONTROL_SPEED_MASK, speed_ctrl);
+	if (ret)
+		return ret;
+
+	return phy_modify_mmd(phydev, MDIO_MMD_PMAPMD, QCA808X_PHY_MMD1_PMA_TYPE,
+			QCA808X_PMA_TYPE_MASK, type_ctrl);
+}
+
+static int qca808x_config_aneg(struct phy_device *phydev)
+{
+	int phy_ctrl = 0;
+	int ret = 0;
+
+	if (phydev->autoneg != AUTONEG_ENABLE) {
+		ret = genphy_setup_forced(phydev);
+		if (ret)
+			return ret;
+
+		ret = qca808x_speed_forced(phydev);
+	} else {
+		ret = __genphy_config_aneg(phydev, ret);
+		if (ret)
+			return ret;
+
+		if (linkmode_test_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, phydev->advertising))
+			phy_ctrl = QCA808X_ADVERTISE_2500FULL;
+
+		ret = phy_modify_mmd(phydev, MDIO_MMD_AN, QCA808X_PHY_MMD7_AUTONEGOTIATION_CONTROL,
+				QCA808X_ADVERTISE_2500FULL, phy_ctrl);
+	}
+
+	return ret;
+}
+
+static int qca808x_get_speed(struct phy_device *phydev)
+{
+	int ret = phy_read(phydev, QCA808X_PHY_SPEC_STATUS);
+
+	if (ret < 0)
+		return ret;
+
+	switch (FIELD_GET(QCA808X_STATUS_SPEED_MASK, ret)) {
+	case QCA808X_STATUS_SPEED_2500MBS:
+		phydev->speed = SPEED_2500;
+		break;
+	case QCA808X_STATUS_SPEED_1000MBS:
+		phydev->speed = SPEED_1000;
+		break;
+	case QCA808X_STATUS_SPEED_100MBS:
+		phydev->speed = SPEED_100;
+		break;
+	default:
+		phydev->speed = SPEED_10;
+		break;
+	}
+
+	return 0;
+}
+
+static int qca808x_read_status(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = genphy_read_status(phydev);
+	if (ret)
+		return ret;
+
+	if (!phydev->link) {
+		ret = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_10GBT_STAT);
+		if (ret < 0)
+			return ret;
+
+		linkmode_mod_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, phydev->lp_advertising,
+				ret & MDIO_AN_10GBT_STAT_LP2_5G);
+
+		/* generate random seed as a lower value to make PHY linked as SLAVE easily,
+		 * excpet for master/slave configuration fault detected.
+		 */
+		if (qca808x_get_2500caps(phydev) == 1) {
+			ret = phy_read(phydev, MII_STAT1000);
+			if (ret < 0)
+				return ret;
+
+			if (ret & LPA_1000MSFAIL) {
+				qca808x_phy_ms_seed_enable(phydev, false);
+			} else {
+				qca808x_phy_ms_random_seed_set(phydev);
+				qca808x_phy_ms_seed_enable(phydev, true);
+			}
+		}
+	}
+
+	return qca808x_get_speed(phydev);
+}
+
+static int qca808x_soft_reset(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = genphy_soft_reset(phydev);
+	if (ret < 0)
+		return ret;
+
+	if (phydev->autoneg == AUTONEG_DISABLE) {
+		ret = qca808x_speed_forced(phydev);
+		if (ret)
+			return ret;
+	}
+
+	return qca808x_phy_ms_seed_enable(phydev, true);
+}
+
+static struct phy_driver qca808x_phy_driver[] = {
+	{
+		PHY_ID_MATCH_EXACT(QCA8081_PHY_ID),
+		.name			= "QCA8081 PHY",
+		.get_features		= qca808x_get_features,
+		.config_init		= qca808x_config_init,
+		.read_status		= qca808x_read_status,
+		.soft_reset		= qca808x_soft_reset,
+		.config_intr		= qca808x_config_intr,
+		.handle_interrupt	= qca808x_handle_interrupt,
+		.config_aneg		= qca808x_config_aneg,
+		.set_wol		= qca808x_set_wol,
+		.get_wol		= qca808x_get_wol,
+		.suspend		= qca808x_suspend,
+		.resume			= genphy_resume,
+	},
+};
+module_phy_driver(qca808x_phy_driver);
+
+static struct mdio_device_id __maybe_unused qca808x_phy_tbl[] = {
+	{ PHY_ID_MATCH_EXACT(QCA8081_PHY_ID) },
+	{ },
+};
+MODULE_DEVICE_TABLE(mdio, qca808x_phy_tbl);
+
+MODULE_DESCRIPTION("Qualcomm Technologies, Inc. QCA8081 PHY driver");
+MODULE_AUTHOR("Luo Jie");
+MODULE_LICENSE("GPL");
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] net: phy: add qca8081 ethernet phy driver

[Andrew Lunn]

On Mon, Aug 16, 2021 at 07:34:40PM +0800, Luo Jie wrote:
> qca8081 is industry’s lowest cost and power 1-port 2.5G/1G Ethernet PHY
> chip, which implements SGMII/SGMII+ for interface to SoC.

Hi Luo

No Marketing claims in the commit message please. Even if it is
correct now, it will soon be wrong with newer generations of
devices.

And what is SGMII+? Please reference a document. Is it actually
2500BaseX?

> Signed-off-by: Luo Jie <luoj@codeaurora.org>
> ---
>  drivers/net/phy/Kconfig   |   6 +
>  drivers/net/phy/Makefile  |   1 +
>  drivers/net/phy/qca808x.c | 573 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 580 insertions(+)
>  create mode 100644 drivers/net/phy/qca808x.c
> 
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index c56f703ae998..26cb1c2ffd17 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -343,3 +343,9 @@ endif # PHYLIB
>  config MICREL_KS8995MA
>  	tristate "Micrel KS8995MA 5-ports 10/100 managed Ethernet switch"
>  	depends on SPI
> +
> +config QCA808X_PHY
> +	tristate "Qualcomm Atheros QCA808X PHYs"
> +	depends on REGULATOR
> +	help
> +	  Currently supports the QCA8081 model

This file is sorted on the tristate text. So it should appear directly
after AT803X_PHY.

> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index 172bb193ae6a..9ef477d79588 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -84,3 +84,4 @@ obj-$(CONFIG_STE10XP)		+= ste10Xp.o
>  obj-$(CONFIG_TERANETICS_PHY)	+= teranetics.o
>  obj-$(CONFIG_VITESSE_PHY)	+= vitesse.o
>  obj-$(CONFIG_XILINX_GMII2RGMII) += xilinx_gmii2rgmii.o
> +obj-$(CONFIG_QCA808X_PHY)	+= qca808x.o

And this file is sorted by CONFIG_ so should be after
CONFIG_NXP_TJA11XX_PHY.

Keeping things sorted reduces the likelyhood of a merge conflict.

> +#include <linux/module.h>
> +#include <linux/etherdevice.h>
> +#include <linux/phy.h>
> +#include <linux/bitfield.h>
> +
> +#define QCA8081_PHY_ID					0x004DD101
> +
> +/* MII special status */
> +#define QCA808X_PHY_SPEC_STATUS				0x11
> +#define QCA808X_STATUS_FULL_DUPLEX			BIT(13)
> +#define QCA808X_STATUS_LINK_PASS			BIT(10)
> +#define QCA808X_STATUS_SPEED_MASK			GENMASK(9, 7)
> +#define QCA808X_STATUS_SPEED_100MBS			1
> +#define QCA808X_STATUS_SPEED_1000MBS			2
> +#define QCA808X_STATUS_SPEED_2500MBS			4
> +
> +/* MII interrupt enable & status */
> +#define QCA808X_PHY_INTR_MASK				0x12
> +#define QCA808X_PHY_INTR_STATUS				0x13
> +#define QCA808X_INTR_ENABLE_FAST_RETRAIN_FAIL		BIT(15)
> +#define QCA808X_INTR_ENABLE_SPEED_CHANGED		BIT(14)
> +#define QCA808X_INTR_ENABLE_DUPLEX_CHANGED		BIT(13)
> +#define QCA808X_INTR_ENABLE_PAGE_RECEIVED		BIT(12)
> +#define QCA808X_INTR_ENABLE_LINK_FAIL			BIT(11)
> +#define QCA808X_INTR_ENABLE_LINK_SUCCESS		BIT(10)
> +#define QCA808X_INTR_ENABLE_POE				BIT(1)
> +#define QCA808X_INTR_ENABLE_WOL				BIT(0)
> +
> +/* MII DBG address & data */
> +#define QCA808X_PHY_DEBUG_ADDR				0x1d
> +#define QCA808X_PHY_DEBUG_DATA				0x1e
> +

A lot of these registers look the same as the at803x. So i'm thinking
you should merge these two drivers. There is a lot of code which is
identical, or very similar, which you can share.

> +static int qca808x_get_2500caps(struct phy_device *phydev)
> +{
> +	int phy_data;
> +
> +	phy_data = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, QCA808X_PHY_MMD1_PMA_CAP_REG);
> +
> +	return (phy_data & QCA808X_STATUS_2500T_FD_CAPS) ? 1 : 0;

So the PHY ignores the standard and does not set bit
MDIO_PMA_NG_EXTABLE_2_5GBT in MDIO_MMD_PMAPMD, MDIO_PMA_NG_EXTABLE ?
Please compare the registers against the standards. If they are
actually being followed, please you the linux names for these
registers, and try to use the generic code.

> +static int qca808x_phy_ms_random_seed_set(struct phy_device *phydev)
> +{
> +	u16 seed_value = (prandom_u32() % QCA808X_MASTER_SLAVE_SEED_RANGE) << 2;
> +
> +	return qca808x_debug_reg_modify(phydev, QCA808X_PHY_DEBUG_LOCAL_SEED,
> +			QCA808X_MASTER_SLAVE_SEED_CFG, seed_value);
> +}
> +
> +static int qca808x_phy_ms_seed_enable(struct phy_device *phydev, bool enable)
> +{
> +	u16 seed_enable = 0;
> +
> +	if (enable)
> +		seed_enable = QCA808X_MASTER_SLAVE_SEED_ENABLE;
> +
> +	return qca808x_debug_reg_modify(phydev, QCA808X_PHY_DEBUG_LOCAL_SEED,
> +			QCA808X_MASTER_SLAVE_SEED_ENABLE, seed_enable);
> +}

This is interesting. I've not seen any other PHY does this. is there
documentation? Is the datasheet available?

> +static int qca808x_soft_reset(struct phy_device *phydev)
> +{
> +	int ret;
> +
> +	ret = genphy_soft_reset(phydev);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (phydev->autoneg == AUTONEG_DISABLE) {
> +		ret = qca808x_speed_forced(phydev);
> +		if (ret)
> +			return ret;

That is unusual. After a reset, you would expect the config_aneg()
function to be called, and it should set things like this. Why is it
needed?

I don't see anything handling the host side. Generally, devices like
this use SGMII for 10/100/1G. When 2.5G is in use they swap their host
interface to 2500BaseX. See mv3310_update_interface() as an example.

	  Andrew

Re: [PATCH] net: phy: add qca8081 ethernet phy driver

[Russell King (Oracle)]

On Mon, Aug 16, 2021 at 07:34:40PM +0800, Luo Jie wrote:
> +/* PMA control */
> +#define QCA808X_PHY_MMD1_PMA_CONTROL			0x0
> +#define QCA808X_PMA_CONTROL_SPEED_MASK			(BIT(13) | BIT(6))
> +#define QCA808X_PMA_CONTROL_2500M			(BIT(13) | BIT(6))
> +#define QCA808X_PMA_CONTROL_1000M			BIT(6)
> +#define QCA808X_PMA_CONTROL_100M			BIT(13)
> +#define QCA808X_PMA_CONTROL_10M				0x0

I don't think this is (a) correct, (b) any different from the IEEE 802.3
standard definitions. Please use the standard definitions in
include/uapi/linux/mdio.h.

> +/* PMA capable */
> +#define QCA808X_PHY_MMD1_PMA_CAP_REG			0x4
> +#define QCA808X_STATUS_2500T_FD_CAPS			BIT(13)

Again, this is IEEE 802.3 standard, nothing special here. As this is
a standard bit, include/uapi/linux/mdio.h should be augmented with
it rather than introducing private definitions. Note that this is
_not_ 2500T but merely indicates that the "PMA/PMD is capable of
operating at 2.5 Gb/s". IEEE 802.3 makes no mention of the underlying
technology used to achieve 2.5Gbps.

> +/* PMA type */
> +#define QCA808X_PHY_MMD1_PMA_TYPE			0x7
> +#define QCA808X_PMA_TYPE_MASK				GENMASK(5, 0)
> +#define QCA808X_PMA_TYPE_2500M				0x30
> +#define QCA808X_PMA_TYPE_1000M				0xc
> +#define QCA808X_PMA_TYPE_100M				0xe
> +#define QCA808X_PMA_TYPE_10M				0xf

This is the PMA type selection register... another IEEE 802.3 standard
register. You omit the underlying technology for these definitions.
From the IEEE 802.3 standard:

0x30 2.5GBASE-T PMA
0x0f 10BASE-T PMA/PMD
0x0e 100BASE-TX PMA/PMD
0x0c 1000BASE-T PMA/PMD

and we have definitions for all these already:

#define MDIO_PMA_CTRL2_1000BT           0x000c  /* 1000BASE-T type */
#define MDIO_PMA_CTRL2_100BTX           0x000e  /* 100BASE-TX type */
#define MDIO_PMA_CTRL2_10BT             0x000f  /* 10BASE-T type */
#define MDIO_PMA_CTRL2_2_5GBT           0x0030  /* 2.5GBaseT type */

Please make use of these.

> +/* AN 2.5G */
> +#define QCA808X_PHY_MMD7_AUTONEGOTIATION_CONTROL	0x20
> +#define QCA808X_ADVERTISE_2500FULL			BIT(7)
> +#define QCA808X_FAST_RETRAIN_2500BT			BIT(5)
> +#define QCA808X_ADV_LOOP_TIMING				BIT(0)
> +
> +/* Fast retrain related registers */
> +#define QCA808X_PHY_MMD1_FAST_RETRAIN_STATUS_CTL	0x93
> +#define QCA808X_FAST_RETRAIN_CTRL			0x1

I suspect I'm going to find that the above are standard registers
as well.

I think I'll also ask that once you've corrected the above, that you
also look to see which of the Clause 45 generic support functions you
can make use of in this driver, and switch it over to those - and then
post a revised version.

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH] net: phy: add qca8081 ethernet phy driver

[Heiner Kallweit]

On 16.08.2021 13:34, Luo Jie wrote:
> qca8081 is industry’s lowest cost and power 1-port 2.5G/1G Ethernet PHY
> chip, which implements SGMII/SGMII+ for interface to SoC.
> 
> Signed-off-by: Luo Jie <luoj@codeaurora.org>
> ---
>  drivers/net/phy/Kconfig   |   6 +
>  drivers/net/phy/Makefile  |   1 +
>  drivers/net/phy/qca808x.c | 573 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 580 insertions(+)
>  create mode 100644 drivers/net/phy/qca808x.c
> 
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index c56f703ae998..26cb1c2ffd17 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -343,3 +343,9 @@ endif # PHYLIB
>  config MICREL_KS8995MA
>  	tristate "Micrel KS8995MA 5-ports 10/100 managed Ethernet switch"
>  	depends on SPI
> +
> +config QCA808X_PHY
> +	tristate "Qualcomm Atheros QCA808X PHYs"
> +	depends on REGULATOR
> +	help
> +	  Currently supports the QCA8081 model
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index 172bb193ae6a..9ef477d79588 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -84,3 +84,4 @@ obj-$(CONFIG_STE10XP)		+= ste10Xp.o
>  obj-$(CONFIG_TERANETICS_PHY)	+= teranetics.o
>  obj-$(CONFIG_VITESSE_PHY)	+= vitesse.o
>  obj-$(CONFIG_XILINX_GMII2RGMII) += xilinx_gmii2rgmii.o
> +obj-$(CONFIG_QCA808X_PHY)	+= qca808x.o
> diff --git a/drivers/net/phy/qca808x.c b/drivers/net/phy/qca808x.c
> new file mode 100644
> index 000000000000..6cb8abae8c8f
> --- /dev/null
> +++ b/drivers/net/phy/qca808x.c
> @@ -0,0 +1,573 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Driver for Qualcomm QCA808x PHY
> + * Author: Luo Jie <luoj@codeaurora.org>
> + */
> +
> +#include <linux/module.h>
> +#include <linux/etherdevice.h>
> +#include <linux/phy.h>
> +#include <linux/bitfield.h>
> +
> +#define QCA8081_PHY_ID					0x004DD101
> +
> +/* MII special status */
> +#define QCA808X_PHY_SPEC_STATUS				0x11
> +#define QCA808X_STATUS_FULL_DUPLEX			BIT(13)
> +#define QCA808X_STATUS_LINK_PASS			BIT(10)
> +#define QCA808X_STATUS_SPEED_MASK			GENMASK(9, 7)
> +#define QCA808X_STATUS_SPEED_100MBS			1
> +#define QCA808X_STATUS_SPEED_1000MBS			2
> +#define QCA808X_STATUS_SPEED_2500MBS			4
> +
> +/* MII interrupt enable & status */
> +#define QCA808X_PHY_INTR_MASK				0x12
> +#define QCA808X_PHY_INTR_STATUS				0x13
> +#define QCA808X_INTR_ENABLE_FAST_RETRAIN_FAIL		BIT(15)
> +#define QCA808X_INTR_ENABLE_SPEED_CHANGED		BIT(14)
> +#define QCA808X_INTR_ENABLE_DUPLEX_CHANGED		BIT(13)
> +#define QCA808X_INTR_ENABLE_PAGE_RECEIVED		BIT(12)
> +#define QCA808X_INTR_ENABLE_LINK_FAIL			BIT(11)
> +#define QCA808X_INTR_ENABLE_LINK_SUCCESS		BIT(10)
> +#define QCA808X_INTR_ENABLE_POE				BIT(1)
> +#define QCA808X_INTR_ENABLE_WOL				BIT(0)
> +
> +/* MII DBG address & data */
> +#define QCA808X_PHY_DEBUG_ADDR				0x1d
> +#define QCA808X_PHY_DEBUG_DATA				0x1e
> +
> +/* Conifg seed */
> +#define QCA808X_PHY_DEBUG_LOCAL_SEED			9
> +#define QCA808X_MASTER_SLAVE_SEED_ENABLE		BIT(1)
> +#define QCA808X_MASTER_SLAVE_SEED_CFG			GENMASK(12, 2)
> +#define QCA808X_MASTER_SLAVE_SEED_RANGE			0x32
> +
> +/* ADC threshold */
> +#define QCA808X_PHY_DEBUG_ADC_THRESHOLD			0x2c80
> +#define QCA808X_ADC_THRESHOLD_MASK			GENMASK(7, 0)
> +#define QCA808X_ADC_THRESHOLD_80MV			0
> +#define QCA808X_ADC_THRESHOLD_100MV			0xf0
> +#define QCA808X_ADC_THRESHOLD_200MV			0x0f
> +#define QCA808X_ADC_THRESHOLD_300MV			0xff
> +
> +/* PMA control */
> +#define QCA808X_PHY_MMD1_PMA_CONTROL			0x0
> +#define QCA808X_PMA_CONTROL_SPEED_MASK			(BIT(13) | BIT(6))
> +#define QCA808X_PMA_CONTROL_2500M			(BIT(13) | BIT(6))
> +#define QCA808X_PMA_CONTROL_1000M			BIT(6)
> +#define QCA808X_PMA_CONTROL_100M			BIT(13)
> +#define QCA808X_PMA_CONTROL_10M				0x0
> +
> +/* PMA capable */
> +#define QCA808X_PHY_MMD1_PMA_CAP_REG			0x4
> +#define QCA808X_STATUS_2500T_FD_CAPS			BIT(13)
> +
> +/* PMA type */
> +#define QCA808X_PHY_MMD1_PMA_TYPE			0x7
> +#define QCA808X_PMA_TYPE_MASK				GENMASK(5, 0)
> +#define QCA808X_PMA_TYPE_2500M				0x30
> +#define QCA808X_PMA_TYPE_1000M				0xc
> +#define QCA808X_PMA_TYPE_100M				0xe
> +#define QCA808X_PMA_TYPE_10M				0xf
> +
> +/* CLD control */
> +#define QCA808X_PHY_MMD3_ADDR_CLD_CTRL7			0x8007
> +#define QCA808X_8023AZ_AFE_CTRL_MASK			GENMASK(8, 4)
> +#define QCA808X_8023AZ_AFE_EN				0x90
> +
> +/* AZ control */
> +#define QCA808X_PHY_MMD3_AZ_TRAINING_CTRL		0x8008
> +#define QCA808X_MMD3_AZ_TRAINING_VAL			0x1c32
> +
> +/* WOL control */
> +#define QCA808X_PHY_MMD3_WOL_CTRL			0x8012
> +#define QCA808X_WOL_EN					BIT(5)
> +
> +#define QCA808X_PHY_MMD3_LOCAL_MAC_ADDR_0_15_OFFSET	0x804c
> +#define QCA808X_PHY_MMD3_LOCAL_MAC_ADDR_16_31_OFFSET	0x804b
> +#define QCA808X_PHY_MMD3_LOCAL_MAC_ADDR_32_47_OFFSET	0x804a
> +
> +/* AN 2.5G */
> +#define QCA808X_PHY_MMD7_AUTONEGOTIATION_CONTROL	0x20
> +#define QCA808X_ADVERTISE_2500FULL			BIT(7)
> +#define QCA808X_FAST_RETRAIN_2500BT			BIT(5)
> +#define QCA808X_ADV_LOOP_TIMING				BIT(0)
> +
> +/* Fast retrain related registers */
> +#define QCA808X_PHY_MMD1_FAST_RETRAIN_STATUS_CTL	0x93
> +#define QCA808X_FAST_RETRAIN_CTRL			0x1
> +
> +#define QCA808X_PHY_MMD1_MSE_THRESHOLD_20DB		0x8014
> +#define QCA808X_MSE_THRESHOLD_20DB_VALUE		0x529
> +
> +#define QCA808X_PHY_MMD1_MSE_THRESHOLD_17DB		0x800E
> +#define QCA808X_MSE_THRESHOLD_17DB_VALUE		0x341
> +
> +#define QCA808X_PHY_MMD1_MSE_THRESHOLD_27DB		0x801E
> +#define QCA808X_MSE_THRESHOLD_27DB_VALUE		0x419
> +
> +#define QCA808X_PHY_MMD1_MSE_THRESHOLD_28DB		0x8020
> +#define QCA808X_MSE_THRESHOLD_28DB_VALUE		0x341
> +
> +#define QCA808X_PHY_MMD7_TOP_OPTION1			0x901c
> +#define QCA808X_TOP_OPTION1_DATA			0x0
> +
> +#define QCA808X_PHY_MMD7_ADDR_EEE_LP_ADVERTISEMENT	0x40
> +#define QCA808X_EEE_ADV_THP				0x8
> +
> +#define QCA808X_PHY_MMD3_DEBUG_1			0xa100
> +#define QCA808X_MMD3_DEBUG_1_VALUE			0x9203
> +#define QCA808X_PHY_MMD3_DEBUG_2			0xa101
> +#define QCA808X_MMD3_DEBUG_2_VALUE			0x48ad
> +#define QCA808X_PHY_MMD3_DEBUG_3			0xa103
> +#define QCA808X_MMD3_DEBUG_3_VALUE			0x1698
> +#define QCA808X_PHY_MMD3_DEBUG_4			0xa105
> +#define QCA808X_MMD3_DEBUG_4_VALUE			0x8001
> +#define QCA808X_PHY_MMD3_DEBUG_5			0xa106
> +#define QCA808X_MMD3_DEBUG_5_VALUE			0x1111
> +#define QCA808X_PHY_MMD3_DEBUG_6			0xa011
> +#define QCA808X_MMD3_DEBUG_6_VALUE			0x5f85
> +
> +static int qca808x_debug_reg_read(struct phy_device *phydev, u16 reg)
> +{
> +	int ret;
> +
> +	ret = phy_write(phydev, QCA808X_PHY_DEBUG_ADDR, reg);
> +	if (ret)
> +		return ret;
> +
> +	return phy_read(phydev, QCA808X_PHY_DEBUG_DATA);
> +}
> +
> +static int qca808x_debug_reg_modify(struct phy_device *phydev, u16 reg,
> +				 u16 mask, u16 val)
> +{
> +	u16 phy_data;
> +	int ret;
> +
> +	ret = qca808x_debug_reg_read(phydev, reg);
> +	if (ret < 0)
> +		return ret;
> +
> +	phy_data = ret & 0xffff;
> +	phy_data &= ~mask;
> +	phy_data |= val;
> +
> +	return phy_write(phydev, QCA808X_PHY_DEBUG_DATA, phy_data);
> +}
> +
> +static int qca808x_get_2500caps(struct phy_device *phydev)
> +{
> +	int phy_data;
> +
> +	phy_data = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, QCA808X_PHY_MMD1_PMA_CAP_REG);
> +
> +	return (phy_data & QCA808X_STATUS_2500T_FD_CAPS) ? 1 : 0;
> +}
> +
> +static int qca808x_get_features(struct phy_device *phydev)
> +{
> +	linkmode_mod_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, phydev->supported,
> +			qca808x_get_2500caps(phydev));
> +
> +	return genphy_read_abilities(phydev);
> +}
> +
> +static int qca808x_phy_fast_retrain_cfg(struct phy_device *phydev)
> +{
> +	int ret;
> +
> +	ret = phy_write_mmd(phydev, MDIO_MMD_AN, QCA808X_PHY_MMD7_AUTONEGOTIATION_CONTROL,
> +			QCA808X_ADVERTISE_2500FULL |
> +			QCA808X_FAST_RETRAIN_2500BT |
> +			QCA808X_ADV_LOOP_TIMING);
> +	if (ret)
> +		return ret;
> +
> +	phy_write_mmd(phydev, MDIO_MMD_PMAPMD, QCA808X_PHY_MMD1_FAST_RETRAIN_STATUS_CTL,
> +			QCA808X_FAST_RETRAIN_CTRL);
> +	phy_write_mmd(phydev, MDIO_MMD_PMAPMD, QCA808X_PHY_MMD1_MSE_THRESHOLD_20DB,
> +			QCA808X_MSE_THRESHOLD_20DB_VALUE);
> +	phy_write_mmd(phydev, MDIO_MMD_PMAPMD, QCA808X_PHY_MMD1_MSE_THRESHOLD_17DB,
> +			QCA808X_MSE_THRESHOLD_17DB_VALUE);
> +	phy_write_mmd(phydev, MDIO_MMD_PMAPMD, QCA808X_PHY_MMD1_MSE_THRESHOLD_27DB,
> +			QCA808X_MSE_THRESHOLD_27DB_VALUE);
> +	phy_write_mmd(phydev, MDIO_MMD_PMAPMD, QCA808X_PHY_MMD1_MSE_THRESHOLD_28DB,
> +			QCA808X_MSE_THRESHOLD_28DB_VALUE);
> +	phy_write_mmd(phydev, MDIO_MMD_AN, QCA808X_PHY_MMD7_ADDR_EEE_LP_ADVERTISEMENT,
> +			QCA808X_EEE_ADV_THP);
> +	phy_write_mmd(phydev, MDIO_MMD_AN, QCA808X_PHY_MMD7_TOP_OPTION1,
> +			QCA808X_TOP_OPTION1_DATA);
> +	phy_write_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_DEBUG_1,
> +			QCA808X_MMD3_DEBUG_1_VALUE);
> +	phy_write_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_DEBUG_4,
> +			QCA808X_MMD3_DEBUG_4_VALUE);
> +	phy_write_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_DEBUG_5,
> +			QCA808X_MMD3_DEBUG_5_VALUE);
> +	phy_write_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_DEBUG_3,
> +			QCA808X_MMD3_DEBUG_3_VALUE);
> +	phy_write_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_DEBUG_6,
> +			QCA808X_MMD3_DEBUG_6_VALUE);
> +	phy_write_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_DEBUG_2,
> +			QCA808X_MMD3_DEBUG_2_VALUE);
> +
> +	return 0;
> +}
> +
> +static int qca808x_phy_ms_random_seed_set(struct phy_device *phydev)
> +{
> +	u16 seed_value = (prandom_u32() % QCA808X_MASTER_SLAVE_SEED_RANGE) << 2;
> +
> +	return qca808x_debug_reg_modify(phydev, QCA808X_PHY_DEBUG_LOCAL_SEED,
> +			QCA808X_MASTER_SLAVE_SEED_CFG, seed_value);
> +}
> +
> +static int qca808x_phy_ms_seed_enable(struct phy_device *phydev, bool enable)
> +{
> +	u16 seed_enable = 0;
> +
> +	if (enable)
> +		seed_enable = QCA808X_MASTER_SLAVE_SEED_ENABLE;
> +
> +	return qca808x_debug_reg_modify(phydev, QCA808X_PHY_DEBUG_LOCAL_SEED,
> +			QCA808X_MASTER_SLAVE_SEED_ENABLE, seed_enable);
> +}
> +
> +static int qca808x_config_init(struct phy_device *phydev)
> +{
> +	int ret;
> +
> +	/* Active adc&vga on 802.3az for the link 1000M and 100M */
> +	ret = phy_modify_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_ADDR_CLD_CTRL7,
> +			QCA808X_8023AZ_AFE_CTRL_MASK, QCA808X_8023AZ_AFE_EN);
> +	if (ret)
> +		return ret;
> +
> +	/* Adjust the threshold on 802.3az for the link 1000M */
> +	ret = phy_write_mmd(phydev, MDIO_MMD_PCS,
> +			QCA808X_PHY_MMD3_AZ_TRAINING_CTRL, QCA808X_MMD3_AZ_TRAINING_VAL);
> +	if (ret)
> +		return ret;
> +
> +	/* Config the fast retrain for the link 2500M */
> +	ret = qca808x_phy_fast_retrain_cfg(phydev);
> +	if (ret)
> +		return ret;
> +
> +	/* Configure ramdom seed to make phy linked as slave mode for link 2500M */
> +	ret = qca808x_phy_ms_random_seed_set(phydev);
> +	if (ret)
> +		return ret;
> +
> +	/* Enable seed */
> +	ret = qca808x_phy_ms_seed_enable(phydev, true);
> +	if (ret)
> +		return ret;
> +
> +	/* Configure adc threshold as 100mv for the link 10M */
> +	return qca808x_debug_reg_modify(phydev, QCA808X_PHY_DEBUG_ADC_THRESHOLD,
> +			QCA808X_ADC_THRESHOLD_MASK, QCA808X_ADC_THRESHOLD_100MV);
> +}
> +
> +static int qca808x_ack_interrupt(struct phy_device *phydev)
> +{
> +	int ret;
> +
> +	ret = phy_read(phydev, QCA808X_PHY_INTR_STATUS);
> +
> +	return (ret < 0) ? ret : 0;
> +}
> +
> +static int qca808x_config_intr(struct phy_device *phydev)
> +{
> +	int ret;
> +	int intr_ctl = 0;
> +
> +	if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
> +		ret = qca808x_ack_interrupt(phydev);
> +		if (ret)
> +			return ret;
> +
> +		intr_ctl = phy_read(phydev, QCA808X_PHY_INTR_MASK);
> +		if (intr_ctl < 0)
> +			return intr_ctl;
> +
> +		intr_ctl |= QCA808X_INTR_ENABLE_FAST_RETRAIN_FAIL |
> +			QCA808X_INTR_ENABLE_SPEED_CHANGED |
> +			QCA808X_INTR_ENABLE_DUPLEX_CHANGED |
> +			QCA808X_INTR_ENABLE_LINK_FAIL |
> +			QCA808X_INTR_ENABLE_LINK_SUCCESS;

Do you actually need all these interrupt sources? Your interrupt handler
does standard link change handling only.

> +		ret = phy_write(phydev, QCA808X_PHY_INTR_MASK, intr_ctl);
> +	} else {
> +		ret = phy_write(phydev, QCA808X_PHY_INTR_MASK, intr_ctl);
> +		if (ret)
> +			return ret;
> +
> +		ret = qca808x_ack_interrupt(phydev);
> +	}
> +
> +	return ret;
> +}
> +
> +static irqreturn_t qca808x_handle_interrupt(struct phy_device *phydev)
> +{
> +	int intr_status, intr_mask;
> +
> +	intr_status = phy_read(phydev, QCA808X_PHY_INTR_STATUS);
> +	if (intr_status < 0) {
> +		phy_error(phydev);
> +		return IRQ_NONE;
> +	}
> +
> +	intr_mask = phy_read(phydev, QCA808X_PHY_INTR_MASK);
> +	if (intr_mask < 0) {
> +		phy_error(phydev);
> +		return IRQ_NONE;
> +	}
> +
> +	if (!(intr_status & intr_mask))
> +		return IRQ_NONE;
> +
> +	phy_start_machine(phydev);
> +

phy_mac_interrupt() is the correct call here, even though it does the
same as phy_start_machine().

> +	return IRQ_HANDLED;
> +}
> +
> +static int qca808x_set_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol)
> +{
> +	struct net_device *ndev = phydev->attached_dev;
> +	const u8 *mac;
> +	int ret;
> +	unsigned int i, offsets[] = {

offsets[] should be static const.

> +		QCA808X_PHY_MMD3_LOCAL_MAC_ADDR_32_47_OFFSET,
> +		QCA808X_PHY_MMD3_LOCAL_MAC_ADDR_16_31_OFFSET,
> +		QCA808X_PHY_MMD3_LOCAL_MAC_ADDR_0_15_OFFSET,
> +	};
> +
> +	if (!ndev)
> +		return -ENODEV;
> +
> +	if (wol->wolopts & WAKE_MAGIC) {
> +		mac = (const u8 *) ndev->dev_addr;
> +		if (!is_valid_ether_addr(mac))
> +			return -EINVAL;
> +
> +		for (i = 0; i < 3; i++)

ARRAY_SIZE(offsets) would be better than a magic number. And in general:
Instead of the loop, wouldn't it be simpler and better readable to write:

phy_write_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_LOCAL_MAC_ADDR_32_47_OFFSET, mac[1] | (mac[0] < 8));
phy_write_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_LOCAL_MAC_ADDR_16_31_OFFSET, mac[3] | (mac[2] < 8));
phy_write_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_LOCAL_MAC_ADDR_0_15_OFFSET, mac[5] | (mac[4] < 8));

> +			phy_write_mmd(phydev, MDIO_MMD_PCS, offsets[i],
> +				      mac[(i * 2) + 1] | (mac[(i * 2)] << 8));
> +
> +		/* clear the pending wol interrupt */
> +		phy_read(phydev, QCA808X_PHY_INTR_STATUS);
> +
> +		ret = phy_modify(phydev, QCA808X_PHY_INTR_MASK, 0, QCA808X_INTR_ENABLE_WOL);
> +		if (ret)
> +			return ret;
> +
> +		ret = phy_modify_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_WOL_CTRL,
> +				0, QCA808X_WOL_EN);
> +	} else {
> +		ret = phy_modify(phydev, QCA808X_PHY_INTR_MASK, QCA808X_INTR_ENABLE_WOL, 0);
> +		if (ret)
> +			return ret;
> +
> +		ret = phy_modify_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_WOL_CTRL,
> +				QCA808X_WOL_EN, 0);
> +	}
> +
> +	return ret;
> +}
> +
> +static void qca808x_get_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol)
> +{
> +	int ret;
> +
> +	wol->supported = WAKE_MAGIC;
> +	wol->wolopts = 0;
> +
> +	ret = phy_read_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_WOL_CTRL);
> +	if (ret < 0)
> +		return;
> +
> +	if (ret & QCA808X_WOL_EN)
> +		wol->wolopts |= WAKE_MAGIC;
> +}
> +
> +static int qca808x_suspend(struct phy_device *phydev)
> +{
> +	int ret;
> +
> +	ret = phy_read_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_WOL_CTRL);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (ret & QCA808X_WOL_EN)
> +		return 0;

This isn't needed, phy_suspend() checks for WoL being enabled.

> +
> +	return genphy_suspend(phydev);
> +}
> +
> +static int qca808x_speed_forced(struct phy_device *phydev)
> +{
> +	u16 speed_ctrl, type_ctrl;
> +	int ret;
> +
> +	switch (phydev->speed) {
> +	case SPEED_2500:
> +		speed_ctrl = QCA808X_PMA_CONTROL_2500M;
> +		type_ctrl = QCA808X_PMA_TYPE_2500M;
> +		break;
> +	case SPEED_1000:
> +		speed_ctrl = QCA808X_PMA_CONTROL_1000M;
> +		type_ctrl = QCA808X_PMA_TYPE_1000M;
> +		break;
> +	case SPEED_100:
> +		speed_ctrl = QCA808X_PMA_CONTROL_100M;
> +		type_ctrl = QCA808X_PMA_TYPE_100M;
> +		break;
> +	default:
> +		speed_ctrl = QCA808X_PMA_CONTROL_10M;
> +		type_ctrl = QCA808X_PMA_TYPE_10M;
> +		break;
> +	}
> +
> +	ret = phy_modify_mmd(phydev, MDIO_MMD_PMAPMD, QCA808X_PHY_MMD1_PMA_CONTROL,
> +			QCA808X_PMA_CONTROL_SPEED_MASK, speed_ctrl);
> +	if (ret)
> +		return ret;
> +
> +	return phy_modify_mmd(phydev, MDIO_MMD_PMAPMD, QCA808X_PHY_MMD1_PMA_TYPE,
> +			QCA808X_PMA_TYPE_MASK, type_ctrl);
> +}
> +
> +static int qca808x_config_aneg(struct phy_device *phydev)
> +{
> +	int phy_ctrl = 0;
> +	int ret = 0;
> +
> +	if (phydev->autoneg != AUTONEG_ENABLE) {
> +		ret = genphy_setup_forced(phydev);
> +		if (ret)
> +			return ret;
> +
> +		ret = qca808x_speed_forced(phydev);
> +	} else {
> +		ret = __genphy_config_aneg(phydev, ret);
> +		if (ret)
> +			return ret;
> +
> +		if (linkmode_test_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, phydev->advertising))
> +			phy_ctrl = QCA808X_ADVERTISE_2500FULL;
> +
> +		ret = phy_modify_mmd(phydev, MDIO_MMD_AN, QCA808X_PHY_MMD7_AUTONEGOTIATION_CONTROL,
> +				QCA808X_ADVERTISE_2500FULL, phy_ctrl);
> +	}
> +
> +	return ret;
> +}
> +
> +static int qca808x_get_speed(struct phy_device *phydev)
> +{
> +	int ret = phy_read(phydev, QCA808X_PHY_SPEC_STATUS);
> +
> +	if (ret < 0)
> +		return ret;
> +
> +	switch (FIELD_GET(QCA808X_STATUS_SPEED_MASK, ret)) {
> +	case QCA808X_STATUS_SPEED_2500MBS:
> +		phydev->speed = SPEED_2500;
> +		break;
> +	case QCA808X_STATUS_SPEED_1000MBS:
> +		phydev->speed = SPEED_1000;
> +		break;
> +	case QCA808X_STATUS_SPEED_100MBS:
> +		phydev->speed = SPEED_100;
> +		break;
> +	default:
> +		phydev->speed = SPEED_10;
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +static int qca808x_read_status(struct phy_device *phydev)
> +{
> +	int ret;
> +
> +	ret = genphy_read_status(phydev);
> +	if (ret)
> +		return ret;
> +
> +	if (!phydev->link) {

Why is all the following needed in read_status() if link is down?
When using polling mode then this would be executed with each poll
if link is down.

> +		ret = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_10GBT_STAT);
> +		if (ret < 0)
> +			return ret;
> +
> +		linkmode_mod_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, phydev->lp_advertising,
> +				ret & MDIO_AN_10GBT_STAT_LP2_5G);
> +
> +		/* generate random seed as a lower value to make PHY linked as SLAVE easily,
> +		 * excpet for master/slave configuration fault detected.
> +		 */
> +		if (qca808x_get_2500caps(phydev) == 1) {
> +			ret = phy_read(phydev, MII_STAT1000);
> +			if (ret < 0)
> +				return ret;
> +
> +			if (ret & LPA_1000MSFAIL) {
> +				qca808x_phy_ms_seed_enable(phydev, false);
> +			} else {
> +				qca808x_phy_ms_random_seed_set(phydev);
> +				qca808x_phy_ms_seed_enable(phydev, true);
> +			}
> +		}
> +	}
> +
> +	return qca808x_get_speed(phydev);
> +}
> +
> +static int qca808x_soft_reset(struct phy_device *phydev)
> +{
> +	int ret;
> +
> +	ret = genphy_soft_reset(phydev);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (phydev->autoneg == AUTONEG_DISABLE) {

Isn't this what genphy_soft_reset() does anyway?

> +		ret = qca808x_speed_forced(phydev);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return qca808x_phy_ms_seed_enable(phydev, true);
> +}
> +
> +static struct phy_driver qca808x_phy_driver[] = {
> +	{
> +		PHY_ID_MATCH_EXACT(QCA8081_PHY_ID),
> +		.name			= "QCA8081 PHY",
> +		.get_features		= qca808x_get_features,
> +		.config_init		= qca808x_config_init,
> +		.read_status		= qca808x_read_status,
> +		.soft_reset		= qca808x_soft_reset,
> +		.config_intr		= qca808x_config_intr,
> +		.handle_interrupt	= qca808x_handle_interrupt,
> +		.config_aneg		= qca808x_config_aneg,
> +		.set_wol		= qca808x_set_wol,
> +		.get_wol		= qca808x_get_wol,
> +		.suspend		= qca808x_suspend,
> +		.resume			= genphy_resume,
> +	},
> +};
> +module_phy_driver(qca808x_phy_driver);
> +
> +static struct mdio_device_id __maybe_unused qca808x_phy_tbl[] = {
> +	{ PHY_ID_MATCH_EXACT(QCA8081_PHY_ID) },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(mdio, qca808x_phy_tbl);
> +
> +MODULE_DESCRIPTION("Qualcomm Technologies, Inc. QCA8081 PHY driver");
> +MODULE_AUTHOR("Luo Jie");
> +MODULE_LICENSE("GPL");
> 

Re: [PATCH] net: phy: add qca8081 ethernet phy driver

[Jie Luo]

On 8/16/2021 9:56 PM, Andrew Lunn wrote:
> On Mon, Aug 16, 2021 at 07:34:40PM +0800, Luo Jie wrote:
>> qca8081 is industry’s lowest cost and power 1-port 2.5G/1G Ethernet PHY
>> chip, which implements SGMII/SGMII+ for interface to SoC.
> Hi Luo
>
> No Marketing claims in the commit message please. Even if it is
> correct now, it will soon be wrong with newer generations of
> devices.
>
> And what is SGMII+? Please reference a document. Is it actually
> 2500BaseX?

Hi Andrew,

thanks for the comments, will remove the claims in the next patch.

SGMII+ is for 2500BaseX, which is same as SGMII, but the clock frequency 
of SGMII+ is 2.5 times SGMII.

>> Signed-off-by: Luo Jie <luoj@codeaurora.org>
>> ---
>>   drivers/net/phy/Kconfig   |   6 +
>>   drivers/net/phy/Makefile  |   1 +
>>   drivers/net/phy/qca808x.c | 573 ++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 580 insertions(+)
>>   create mode 100644 drivers/net/phy/qca808x.c
>>
>> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
>> index c56f703ae998..26cb1c2ffd17 100644
>> --- a/drivers/net/phy/Kconfig
>> +++ b/drivers/net/phy/Kconfig
>> @@ -343,3 +343,9 @@ endif # PHYLIB
>>   config MICREL_KS8995MA
>>   	tristate "Micrel KS8995MA 5-ports 10/100 managed Ethernet switch"
>>   	depends on SPI
>> +
>> +config QCA808X_PHY
>> +	tristate "Qualcomm Atheros QCA808X PHYs"
>> +	depends on REGULATOR
>> +	help
>> +	  Currently supports the QCA8081 model
> This file is sorted on the tristate text. So it should appear directly
> after AT803X_PHY.
will update it in the next patch.
>
>> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
>> index 172bb193ae6a..9ef477d79588 100644
>> --- a/drivers/net/phy/Makefile
>> +++ b/drivers/net/phy/Makefile
>> @@ -84,3 +84,4 @@ obj-$(CONFIG_STE10XP)		+= ste10Xp.o
>>   obj-$(CONFIG_TERANETICS_PHY)	+= teranetics.o
>>   obj-$(CONFIG_VITESSE_PHY)	+= vitesse.o
>>   obj-$(CONFIG_XILINX_GMII2RGMII) += xilinx_gmii2rgmii.o
>> +obj-$(CONFIG_QCA808X_PHY)	+= qca808x.o
> And this file is sorted by CONFIG_ so should be after
> CONFIG_NXP_TJA11XX_PHY.
>
> Keeping things sorted reduces the likelyhood of a merge conflict.
will update it in the next patch.
>
>> +#include <linux/module.h>
>> +#include <linux/etherdevice.h>
>> +#include <linux/phy.h>
>> +#include <linux/bitfield.h>
>> +
>> +#define QCA8081_PHY_ID					0x004DD101
>> +
>> +/* MII special status */
>> +#define QCA808X_PHY_SPEC_STATUS				0x11
>> +#define QCA808X_STATUS_FULL_DUPLEX			BIT(13)
>> +#define QCA808X_STATUS_LINK_PASS			BIT(10)
>> +#define QCA808X_STATUS_SPEED_MASK			GENMASK(9, 7)
>> +#define QCA808X_STATUS_SPEED_100MBS			1
>> +#define QCA808X_STATUS_SPEED_1000MBS			2
>> +#define QCA808X_STATUS_SPEED_2500MBS			4
>> +
>> +/* MII interrupt enable & status */
>> +#define QCA808X_PHY_INTR_MASK				0x12
>> +#define QCA808X_PHY_INTR_STATUS				0x13
>> +#define QCA808X_INTR_ENABLE_FAST_RETRAIN_FAIL		BIT(15)
>> +#define QCA808X_INTR_ENABLE_SPEED_CHANGED		BIT(14)
>> +#define QCA808X_INTR_ENABLE_DUPLEX_CHANGED		BIT(13)
>> +#define QCA808X_INTR_ENABLE_PAGE_RECEIVED		BIT(12)
>> +#define QCA808X_INTR_ENABLE_LINK_FAIL			BIT(11)
>> +#define QCA808X_INTR_ENABLE_LINK_SUCCESS		BIT(10)
>> +#define QCA808X_INTR_ENABLE_POE				BIT(1)
>> +#define QCA808X_INTR_ENABLE_WOL				BIT(0)
>> +
>> +/* MII DBG address & data */
>> +#define QCA808X_PHY_DEBUG_ADDR				0x1d
>> +#define QCA808X_PHY_DEBUG_DATA				0x1e
>> +
> A lot of these registers look the same as the at803x. So i'm thinking
> you should merge these two drivers. There is a lot of code which is
> identical, or very similar, which you can share.

Hi Andrew,

qca8081 supports IEEE1588 feature, the IEEE1588 code may be submitted in 
the near future,

so it may be a good idea to keep it out from at803x code.

>
>> +static int qca808x_get_2500caps(struct phy_device *phydev)
>> +{
>> +	int phy_data;
>> +
>> +	phy_data = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, QCA808X_PHY_MMD1_PMA_CAP_REG);
>> +
>> +	return (phy_data & QCA808X_STATUS_2500T_FD_CAPS) ? 1 : 0;
> So the PHY ignores the standard and does not set bit
> MDIO_PMA_NG_EXTABLE_2_5GBT in MDIO_MMD_PMAPMD, MDIO_PMA_NG_EXTABLE ?
> Please compare the registers against the standards. If they are
> actually being followed, please you the linux names for these
> registers, and try to use the generic code.
will update it to use the generic code in the next patch.
>
>> +static int qca808x_phy_ms_random_seed_set(struct phy_device *phydev)
>> +{
>> +	u16 seed_value = (prandom_u32() % QCA808X_MASTER_SLAVE_SEED_RANGE) << 2;
>> +
>> +	return qca808x_debug_reg_modify(phydev, QCA808X_PHY_DEBUG_LOCAL_SEED,
>> +			QCA808X_MASTER_SLAVE_SEED_CFG, seed_value);
>> +}
>> +
>> +static int qca808x_phy_ms_seed_enable(struct phy_device *phydev, bool enable)
>> +{
>> +	u16 seed_enable = 0;
>> +
>> +	if (enable)
>> +		seed_enable = QCA808X_MASTER_SLAVE_SEED_ENABLE;
>> +
>> +	return qca808x_debug_reg_modify(phydev, QCA808X_PHY_DEBUG_LOCAL_SEED,
>> +			QCA808X_MASTER_SLAVE_SEED_ENABLE, seed_enable);
>> +}
> This is interesting. I've not seen any other PHY does this. is there
> documentation? Is the datasheet available?

this piece of code is for configuring the random seed to a lower value 
to make the PHY linked

as the SLAVE mode for fixing some IOT issue, for master/slave 
auto-negotiation, please refer to

https://www.ieee802.org/3/an/public/jul04/lynskey_2_0704.pdf.

>
>> +static int qca808x_soft_reset(struct phy_device *phydev)
>> +{
>> +	int ret;
>> +
>> +	ret = genphy_soft_reset(phydev);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	if (phydev->autoneg == AUTONEG_DISABLE) {
>> +		ret = qca808x_speed_forced(phydev);
>> +		if (ret)
>> +			return ret;
> That is unusual. After a reset, you would expect the config_aneg()
> function to be called, and it should set things like this. Why is it
> needed?
>
> I don't see anything handling the host side. Generally, devices like
> this use SGMII for 10/100/1G. When 2.5G is in use they swap their host
> interface to 2500BaseX. See mv3310_update_interface() as an example.
>
> 	  Andrew

thanks Andrew for the comments. will remove qca808x_speed_forced from 
reset function,

and add the phydev interface update in the next patch.

Re: [PATCH] net: phy: add qca8081 ethernet phy driver

[Jie Luo]

On 8/16/2021 10:01 PM, Russell King (Oracle) wrote:
> On Mon, Aug 16, 2021 at 07:34:40PM +0800, Luo Jie wrote:
>> +/* PMA control */
>> +#define QCA808X_PHY_MMD1_PMA_CONTROL			0x0
>> +#define QCA808X_PMA_CONTROL_SPEED_MASK			(BIT(13) | BIT(6))
>> +#define QCA808X_PMA_CONTROL_2500M			(BIT(13) | BIT(6))
>> +#define QCA808X_PMA_CONTROL_1000M			BIT(6)
>> +#define QCA808X_PMA_CONTROL_100M			BIT(13)
>> +#define QCA808X_PMA_CONTROL_10M				0x0
> I don't think this is (a) correct, (b) any different from the IEEE 802.3
> standard definitions. Please use the standard definitions in
> include/uapi/linux/mdio.h.
thanks Russell, will update it in the next patch.
>
>> +/* PMA capable */
>> +#define QCA808X_PHY_MMD1_PMA_CAP_REG			0x4
>> +#define QCA808X_STATUS_2500T_FD_CAPS			BIT(13)
> Again, this is IEEE 802.3 standard, nothing special here. As this is
> a standard bit, include/uapi/linux/mdio.h should be augmented with
> it rather than introducing private definitions. Note that this is
> _not_ 2500T but merely indicates that the "PMA/PMD is capable of
> operating at 2.5 Gb/s". IEEE 802.3 makes no mention of the underlying
> technology used to achieve 2.5Gbps.
thanks for the comments, will update it to use the standard register for 
the 2.5G capability.
>> +/* PMA type */
>> +#define QCA808X_PHY_MMD1_PMA_TYPE			0x7
>> +#define QCA808X_PMA_TYPE_MASK				GENMASK(5, 0)
>> +#define QCA808X_PMA_TYPE_2500M				0x30
>> +#define QCA808X_PMA_TYPE_1000M				0xc
>> +#define QCA808X_PMA_TYPE_100M				0xe
>> +#define QCA808X_PMA_TYPE_10M				0xf
> This is the PMA type selection register... another IEEE 802.3 standard
> register. You omit the underlying technology for these definitions.
>  From the IEEE 802.3 standard:
>
> 0x30 2.5GBASE-T PMA
> 0x0f 10BASE-T PMA/PMD
> 0x0e 100BASE-TX PMA/PMD
> 0x0c 1000BASE-T PMA/PMD
>
> and we have definitions for all these already:
>
> #define MDIO_PMA_CTRL2_1000BT           0x000c  /* 1000BASE-T type */
> #define MDIO_PMA_CTRL2_100BTX           0x000e  /* 100BASE-TX type */
> #define MDIO_PMA_CTRL2_10BT             0x000f  /* 10BASE-T type */
> #define MDIO_PMA_CTRL2_2_5GBT           0x0030  /* 2.5GBaseT type */
>
> Please make use of these.
will use it in the next patch.
>
>> +/* AN 2.5G */
>> +#define QCA808X_PHY_MMD7_AUTONEGOTIATION_CONTROL	0x20
>> +#define QCA808X_ADVERTISE_2500FULL			BIT(7)
>> +#define QCA808X_FAST_RETRAIN_2500BT			BIT(5)
>> +#define QCA808X_ADV_LOOP_TIMING				BIT(0)
>> +
>> +/* Fast retrain related registers */
>> +#define QCA808X_PHY_MMD1_FAST_RETRAIN_STATUS_CTL	0x93
>> +#define QCA808X_FAST_RETRAIN_CTRL			0x1
> I suspect I'm going to find that the above are standard registers
> as well.
>
> I think I'll also ask that once you've corrected the above, that you
> also look to see which of the Clause 45 generic support functions you
> can make use of in this driver, and switch it over to those - and then
> post a revised version.
>
> Thanks.

thanks Russell for review comments, will refer to the standard registers 
and update

it in the next patch.

>

Re: [PATCH] net: phy: add qca8081 ethernet phy driver

[Andrew Lunn]

On Tue, Aug 17, 2021 at 09:10:44PM +0800, Jie Luo wrote:
> 
> On 8/16/2021 9:56 PM, Andrew Lunn wrote:
> > On Mon, Aug 16, 2021 at 07:34:40PM +0800, Luo Jie wrote:
> > > qca8081 is industry’s lowest cost and power 1-port 2.5G/1G Ethernet PHY
> > > chip, which implements SGMII/SGMII+ for interface to SoC.
> > Hi Luo
> > 
> > No Marketing claims in the commit message please. Even if it is
> > correct now, it will soon be wrong with newer generations of
> > devices.
> > 
> > And what is SGMII+? Please reference a document. Is it actually
> > 2500BaseX?
> 
> Hi Andrew,
> 
> thanks for the comments, will remove the claims in the next patch.
> 
> SGMII+ is for 2500BaseX, which is same as SGMII, but the clock frequency of
> SGMII+ is 2.5 times SGMII.

25000BaseX is not SGMII over clocked at 2.5GHz.

If it is using 2500BaseX then call it 2500BaseX, because 2500BaseX is
well defined in the standards, and SGMII overclocked to 2.5G is not
standardised.

> > A lot of these registers look the same as the at803x. So i'm thinking
> > you should merge these two drivers. There is a lot of code which is
> > identical, or very similar, which you can share.
> 
> Hi Andrew,
> 
> qca8081 supports IEEE1588 feature, the IEEE1588 code may be submitted in the
> near future,
> 
> so it may be a good idea to keep it out from at803x code.

Please merge it. A lot of the code is the same, and a lot of the new
code you are adding will go away once you use the helpers. And
probably you can improve the features of the older PHYs at the same
time, where features are the same between them.

> > > +static int qca808x_phy_ms_seed_enable(struct phy_device *phydev, bool enable)
> > > +{
> > > +	u16 seed_enable = 0;
> > > +
> > > +	if (enable)
> > > +		seed_enable = QCA808X_MASTER_SLAVE_SEED_ENABLE;
> > > +
> > > +	return qca808x_debug_reg_modify(phydev, QCA808X_PHY_DEBUG_LOCAL_SEED,
> > > +			QCA808X_MASTER_SLAVE_SEED_ENABLE, seed_enable);
> > > +}
> > This is interesting. I've not seen any other PHY does this. is there
> > documentation? Is the datasheet available?
> 
> this piece of code is for configuring the random seed to a lower value to
> make the PHY linked
> 
> as the SLAVE mode for fixing some IOT issue, for master/slave
> auto-negotiation, please refer to
> 
> https://www.ieee802.org/3/an/public/jul04/lynskey_2_0704.pdf.

And what happens when this device is used in an Ethernet switch? A
next generation of a qca8k? Take a look at
genphy_setup_master_slave().  Use MASTER_SLAVE_CFG_MASTER_PREFERRED or
MASTER_SLAVE_CFG_SLAVE_PREFERRED to decide how to bias the
master/slave decision.

     Andrew

Re: [PATCH] net: phy: add qca8081 ethernet phy driver

[Jie Luo]

On 8/16/2021 11:27 PM, Heiner Kallweit wrote:
> On 16.08.2021 13:34, Luo Jie wrote:
>> qca8081 is industry’s lowest cost and power 1-port 2.5G/1G Ethernet PHY
>> chip, which implements SGMII/SGMII+ for interface to SoC.
>>
>> Signed-off-by: Luo Jie <luoj@codeaurora.org>
>> ---
>>   drivers/net/phy/Kconfig   |   6 +
>>   drivers/net/phy/Makefile  |   1 +
>>   drivers/net/phy/qca808x.c | 573 ++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 580 insertions(+)
>>   create mode 100644 drivers/net/phy/qca808x.c
>>
>> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
>> index c56f703ae998..26cb1c2ffd17 100644
>> --- a/drivers/net/phy/Kconfig
>> +++ b/drivers/net/phy/Kconfig
>> @@ -343,3 +343,9 @@ endif # PHYLIB
>>   config MICREL_KS8995MA
>>   	tristate "Micrel KS8995MA 5-ports 10/100 managed Ethernet switch"
>>   	depends on SPI
>> +
>> +config QCA808X_PHY
>> +	tristate "Qualcomm Atheros QCA808X PHYs"
>> +	depends on REGULATOR
>> +	help
>> +	  Currently supports the QCA8081 model
>> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
>> index 172bb193ae6a..9ef477d79588 100644
>> --- a/drivers/net/phy/Makefile
>> +++ b/drivers/net/phy/Makefile
>> @@ -84,3 +84,4 @@ obj-$(CONFIG_STE10XP)		+= ste10Xp.o
>>   obj-$(CONFIG_TERANETICS_PHY)	+= teranetics.o
>>   obj-$(CONFIG_VITESSE_PHY)	+= vitesse.o
>>   obj-$(CONFIG_XILINX_GMII2RGMII) += xilinx_gmii2rgmii.o
>> +obj-$(CONFIG_QCA808X_PHY)	+= qca808x.o
>> diff --git a/drivers/net/phy/qca808x.c b/drivers/net/phy/qca808x.c
>> new file mode 100644
>> index 000000000000..6cb8abae8c8f
>> --- /dev/null
>> +++ b/drivers/net/phy/qca808x.c
>> @@ -0,0 +1,573 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Driver for Qualcomm QCA808x PHY
>> + * Author: Luo Jie <luoj@codeaurora.org>
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/etherdevice.h>
>> +#include <linux/phy.h>
>> +#include <linux/bitfield.h>
>> +
>> +#define QCA8081_PHY_ID					0x004DD101
>> +
>> +/* MII special status */
>> +#define QCA808X_PHY_SPEC_STATUS				0x11
>> +#define QCA808X_STATUS_FULL_DUPLEX			BIT(13)
>> +#define QCA808X_STATUS_LINK_PASS			BIT(10)
>> +#define QCA808X_STATUS_SPEED_MASK			GENMASK(9, 7)
>> +#define QCA808X_STATUS_SPEED_100MBS			1
>> +#define QCA808X_STATUS_SPEED_1000MBS			2
>> +#define QCA808X_STATUS_SPEED_2500MBS			4
>> +
>> +/* MII interrupt enable & status */
>> +#define QCA808X_PHY_INTR_MASK				0x12
>> +#define QCA808X_PHY_INTR_STATUS				0x13
>> +#define QCA808X_INTR_ENABLE_FAST_RETRAIN_FAIL		BIT(15)
>> +#define QCA808X_INTR_ENABLE_SPEED_CHANGED		BIT(14)
>> +#define QCA808X_INTR_ENABLE_DUPLEX_CHANGED		BIT(13)
>> +#define QCA808X_INTR_ENABLE_PAGE_RECEIVED		BIT(12)
>> +#define QCA808X_INTR_ENABLE_LINK_FAIL			BIT(11)
>> +#define QCA808X_INTR_ENABLE_LINK_SUCCESS		BIT(10)
>> +#define QCA808X_INTR_ENABLE_POE				BIT(1)
>> +#define QCA808X_INTR_ENABLE_WOL				BIT(0)
>> +
>> +/* MII DBG address & data */
>> +#define QCA808X_PHY_DEBUG_ADDR				0x1d
>> +#define QCA808X_PHY_DEBUG_DATA				0x1e
>> +
>> +/* Conifg seed */
>> +#define QCA808X_PHY_DEBUG_LOCAL_SEED			9
>> +#define QCA808X_MASTER_SLAVE_SEED_ENABLE		BIT(1)
>> +#define QCA808X_MASTER_SLAVE_SEED_CFG			GENMASK(12, 2)
>> +#define QCA808X_MASTER_SLAVE_SEED_RANGE			0x32
>> +
>> +/* ADC threshold */
>> +#define QCA808X_PHY_DEBUG_ADC_THRESHOLD			0x2c80
>> +#define QCA808X_ADC_THRESHOLD_MASK			GENMASK(7, 0)
>> +#define QCA808X_ADC_THRESHOLD_80MV			0
>> +#define QCA808X_ADC_THRESHOLD_100MV			0xf0
>> +#define QCA808X_ADC_THRESHOLD_200MV			0x0f
>> +#define QCA808X_ADC_THRESHOLD_300MV			0xff
>> +
>> +/* PMA control */
>> +#define QCA808X_PHY_MMD1_PMA_CONTROL			0x0
>> +#define QCA808X_PMA_CONTROL_SPEED_MASK			(BIT(13) | BIT(6))
>> +#define QCA808X_PMA_CONTROL_2500M			(BIT(13) | BIT(6))
>> +#define QCA808X_PMA_CONTROL_1000M			BIT(6)
>> +#define QCA808X_PMA_CONTROL_100M			BIT(13)
>> +#define QCA808X_PMA_CONTROL_10M				0x0
>> +
>> +/* PMA capable */
>> +#define QCA808X_PHY_MMD1_PMA_CAP_REG			0x4
>> +#define QCA808X_STATUS_2500T_FD_CAPS			BIT(13)
>> +
>> +/* PMA type */
>> +#define QCA808X_PHY_MMD1_PMA_TYPE			0x7
>> +#define QCA808X_PMA_TYPE_MASK				GENMASK(5, 0)
>> +#define QCA808X_PMA_TYPE_2500M				0x30
>> +#define QCA808X_PMA_TYPE_1000M				0xc
>> +#define QCA808X_PMA_TYPE_100M				0xe
>> +#define QCA808X_PMA_TYPE_10M				0xf
>> +
>> +/* CLD control */
>> +#define QCA808X_PHY_MMD3_ADDR_CLD_CTRL7			0x8007
>> +#define QCA808X_8023AZ_AFE_CTRL_MASK			GENMASK(8, 4)
>> +#define QCA808X_8023AZ_AFE_EN				0x90
>> +
>> +/* AZ control */
>> +#define QCA808X_PHY_MMD3_AZ_TRAINING_CTRL		0x8008
>> +#define QCA808X_MMD3_AZ_TRAINING_VAL			0x1c32
>> +
>> +/* WOL control */
>> +#define QCA808X_PHY_MMD3_WOL_CTRL			0x8012
>> +#define QCA808X_WOL_EN					BIT(5)
>> +
>> +#define QCA808X_PHY_MMD3_LOCAL_MAC_ADDR_0_15_OFFSET	0x804c
>> +#define QCA808X_PHY_MMD3_LOCAL_MAC_ADDR_16_31_OFFSET	0x804b
>> +#define QCA808X_PHY_MMD3_LOCAL_MAC_ADDR_32_47_OFFSET	0x804a
>> +
>> +/* AN 2.5G */
>> +#define QCA808X_PHY_MMD7_AUTONEGOTIATION_CONTROL	0x20
>> +#define QCA808X_ADVERTISE_2500FULL			BIT(7)
>> +#define QCA808X_FAST_RETRAIN_2500BT			BIT(5)
>> +#define QCA808X_ADV_LOOP_TIMING				BIT(0)
>> +
>> +/* Fast retrain related registers */
>> +#define QCA808X_PHY_MMD1_FAST_RETRAIN_STATUS_CTL	0x93
>> +#define QCA808X_FAST_RETRAIN_CTRL			0x1
>> +
>> +#define QCA808X_PHY_MMD1_MSE_THRESHOLD_20DB		0x8014
>> +#define QCA808X_MSE_THRESHOLD_20DB_VALUE		0x529
>> +
>> +#define QCA808X_PHY_MMD1_MSE_THRESHOLD_17DB		0x800E
>> +#define QCA808X_MSE_THRESHOLD_17DB_VALUE		0x341
>> +
>> +#define QCA808X_PHY_MMD1_MSE_THRESHOLD_27DB		0x801E
>> +#define QCA808X_MSE_THRESHOLD_27DB_VALUE		0x419
>> +
>> +#define QCA808X_PHY_MMD1_MSE_THRESHOLD_28DB		0x8020
>> +#define QCA808X_MSE_THRESHOLD_28DB_VALUE		0x341
>> +
>> +#define QCA808X_PHY_MMD7_TOP_OPTION1			0x901c
>> +#define QCA808X_TOP_OPTION1_DATA			0x0
>> +
>> +#define QCA808X_PHY_MMD7_ADDR_EEE_LP_ADVERTISEMENT	0x40
>> +#define QCA808X_EEE_ADV_THP				0x8
>> +
>> +#define QCA808X_PHY_MMD3_DEBUG_1			0xa100
>> +#define QCA808X_MMD3_DEBUG_1_VALUE			0x9203
>> +#define QCA808X_PHY_MMD3_DEBUG_2			0xa101
>> +#define QCA808X_MMD3_DEBUG_2_VALUE			0x48ad
>> +#define QCA808X_PHY_MMD3_DEBUG_3			0xa103
>> +#define QCA808X_MMD3_DEBUG_3_VALUE			0x1698
>> +#define QCA808X_PHY_MMD3_DEBUG_4			0xa105
>> +#define QCA808X_MMD3_DEBUG_4_VALUE			0x8001
>> +#define QCA808X_PHY_MMD3_DEBUG_5			0xa106
>> +#define QCA808X_MMD3_DEBUG_5_VALUE			0x1111
>> +#define QCA808X_PHY_MMD3_DEBUG_6			0xa011
>> +#define QCA808X_MMD3_DEBUG_6_VALUE			0x5f85
>> +
>> +static int qca808x_debug_reg_read(struct phy_device *phydev, u16 reg)
>> +{
>> +	int ret;
>> +
>> +	ret = phy_write(phydev, QCA808X_PHY_DEBUG_ADDR, reg);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return phy_read(phydev, QCA808X_PHY_DEBUG_DATA);
>> +}
>> +
>> +static int qca808x_debug_reg_modify(struct phy_device *phydev, u16 reg,
>> +				 u16 mask, u16 val)
>> +{
>> +	u16 phy_data;
>> +	int ret;
>> +
>> +	ret = qca808x_debug_reg_read(phydev, reg);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	phy_data = ret & 0xffff;
>> +	phy_data &= ~mask;
>> +	phy_data |= val;
>> +
>> +	return phy_write(phydev, QCA808X_PHY_DEBUG_DATA, phy_data);
>> +}
>> +
>> +static int qca808x_get_2500caps(struct phy_device *phydev)
>> +{
>> +	int phy_data;
>> +
>> +	phy_data = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, QCA808X_PHY_MMD1_PMA_CAP_REG);
>> +
>> +	return (phy_data & QCA808X_STATUS_2500T_FD_CAPS) ? 1 : 0;
>> +}
>> +
>> +static int qca808x_get_features(struct phy_device *phydev)
>> +{
>> +	linkmode_mod_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, phydev->supported,
>> +			qca808x_get_2500caps(phydev));
>> +
>> +	return genphy_read_abilities(phydev);
>> +}
>> +
>> +static int qca808x_phy_fast_retrain_cfg(struct phy_device *phydev)
>> +{
>> +	int ret;
>> +
>> +	ret = phy_write_mmd(phydev, MDIO_MMD_AN, QCA808X_PHY_MMD7_AUTONEGOTIATION_CONTROL,
>> +			QCA808X_ADVERTISE_2500FULL |
>> +			QCA808X_FAST_RETRAIN_2500BT |
>> +			QCA808X_ADV_LOOP_TIMING);
>> +	if (ret)
>> +		return ret;
>> +
>> +	phy_write_mmd(phydev, MDIO_MMD_PMAPMD, QCA808X_PHY_MMD1_FAST_RETRAIN_STATUS_CTL,
>> +			QCA808X_FAST_RETRAIN_CTRL);
>> +	phy_write_mmd(phydev, MDIO_MMD_PMAPMD, QCA808X_PHY_MMD1_MSE_THRESHOLD_20DB,
>> +			QCA808X_MSE_THRESHOLD_20DB_VALUE);
>> +	phy_write_mmd(phydev, MDIO_MMD_PMAPMD, QCA808X_PHY_MMD1_MSE_THRESHOLD_17DB,
>> +			QCA808X_MSE_THRESHOLD_17DB_VALUE);
>> +	phy_write_mmd(phydev, MDIO_MMD_PMAPMD, QCA808X_PHY_MMD1_MSE_THRESHOLD_27DB,
>> +			QCA808X_MSE_THRESHOLD_27DB_VALUE);
>> +	phy_write_mmd(phydev, MDIO_MMD_PMAPMD, QCA808X_PHY_MMD1_MSE_THRESHOLD_28DB,
>> +			QCA808X_MSE_THRESHOLD_28DB_VALUE);
>> +	phy_write_mmd(phydev, MDIO_MMD_AN, QCA808X_PHY_MMD7_ADDR_EEE_LP_ADVERTISEMENT,
>> +			QCA808X_EEE_ADV_THP);
>> +	phy_write_mmd(phydev, MDIO_MMD_AN, QCA808X_PHY_MMD7_TOP_OPTION1,
>> +			QCA808X_TOP_OPTION1_DATA);
>> +	phy_write_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_DEBUG_1,
>> +			QCA808X_MMD3_DEBUG_1_VALUE);
>> +	phy_write_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_DEBUG_4,
>> +			QCA808X_MMD3_DEBUG_4_VALUE);
>> +	phy_write_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_DEBUG_5,
>> +			QCA808X_MMD3_DEBUG_5_VALUE);
>> +	phy_write_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_DEBUG_3,
>> +			QCA808X_MMD3_DEBUG_3_VALUE);
>> +	phy_write_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_DEBUG_6,
>> +			QCA808X_MMD3_DEBUG_6_VALUE);
>> +	phy_write_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_DEBUG_2,
>> +			QCA808X_MMD3_DEBUG_2_VALUE);
>> +
>> +	return 0;
>> +}
>> +
>> +static int qca808x_phy_ms_random_seed_set(struct phy_device *phydev)
>> +{
>> +	u16 seed_value = (prandom_u32() % QCA808X_MASTER_SLAVE_SEED_RANGE) << 2;
>> +
>> +	return qca808x_debug_reg_modify(phydev, QCA808X_PHY_DEBUG_LOCAL_SEED,
>> +			QCA808X_MASTER_SLAVE_SEED_CFG, seed_value);
>> +}
>> +
>> +static int qca808x_phy_ms_seed_enable(struct phy_device *phydev, bool enable)
>> +{
>> +	u16 seed_enable = 0;
>> +
>> +	if (enable)
>> +		seed_enable = QCA808X_MASTER_SLAVE_SEED_ENABLE;
>> +
>> +	return qca808x_debug_reg_modify(phydev, QCA808X_PHY_DEBUG_LOCAL_SEED,
>> +			QCA808X_MASTER_SLAVE_SEED_ENABLE, seed_enable);
>> +}
>> +
>> +static int qca808x_config_init(struct phy_device *phydev)
>> +{
>> +	int ret;
>> +
>> +	/* Active adc&vga on 802.3az for the link 1000M and 100M */
>> +	ret = phy_modify_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_ADDR_CLD_CTRL7,
>> +			QCA808X_8023AZ_AFE_CTRL_MASK, QCA808X_8023AZ_AFE_EN);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* Adjust the threshold on 802.3az for the link 1000M */
>> +	ret = phy_write_mmd(phydev, MDIO_MMD_PCS,
>> +			QCA808X_PHY_MMD3_AZ_TRAINING_CTRL, QCA808X_MMD3_AZ_TRAINING_VAL);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* Config the fast retrain for the link 2500M */
>> +	ret = qca808x_phy_fast_retrain_cfg(phydev);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* Configure ramdom seed to make phy linked as slave mode for link 2500M */
>> +	ret = qca808x_phy_ms_random_seed_set(phydev);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* Enable seed */
>> +	ret = qca808x_phy_ms_seed_enable(phydev, true);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* Configure adc threshold as 100mv for the link 10M */
>> +	return qca808x_debug_reg_modify(phydev, QCA808X_PHY_DEBUG_ADC_THRESHOLD,
>> +			QCA808X_ADC_THRESHOLD_MASK, QCA808X_ADC_THRESHOLD_100MV);
>> +}
>> +
>> +static int qca808x_ack_interrupt(struct phy_device *phydev)
>> +{
>> +	int ret;
>> +
>> +	ret = phy_read(phydev, QCA808X_PHY_INTR_STATUS);
>> +
>> +	return (ret < 0) ? ret : 0;
>> +}
>> +
>> +static int qca808x_config_intr(struct phy_device *phydev)
>> +{
>> +	int ret;
>> +	int intr_ctl = 0;
>> +
>> +	if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
>> +		ret = qca808x_ack_interrupt(phydev);
>> +		if (ret)
>> +			return ret;
>> +
>> +		intr_ctl = phy_read(phydev, QCA808X_PHY_INTR_MASK);
>> +		if (intr_ctl < 0)
>> +			return intr_ctl;
>> +
>> +		intr_ctl |= QCA808X_INTR_ENABLE_FAST_RETRAIN_FAIL |
>> +			QCA808X_INTR_ENABLE_SPEED_CHANGED |
>> +			QCA808X_INTR_ENABLE_DUPLEX_CHANGED |
>> +			QCA808X_INTR_ENABLE_LINK_FAIL |
>> +			QCA808X_INTR_ENABLE_LINK_SUCCESS;
> Do you actually need all these interrupt sources? Your interrupt handler
> does standard link change handling only.

Hi Heiner,

Thanks for the comments, will check and update it in the next patch.

>
>> +		ret = phy_write(phydev, QCA808X_PHY_INTR_MASK, intr_ctl);
>> +	} else {
>> +		ret = phy_write(phydev, QCA808X_PHY_INTR_MASK, intr_ctl);
>> +		if (ret)
>> +			return ret;
>> +
>> +		ret = qca808x_ack_interrupt(phydev);
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static irqreturn_t qca808x_handle_interrupt(struct phy_device *phydev)
>> +{
>> +	int intr_status, intr_mask;
>> +
>> +	intr_status = phy_read(phydev, QCA808X_PHY_INTR_STATUS);
>> +	if (intr_status < 0) {
>> +		phy_error(phydev);
>> +		return IRQ_NONE;
>> +	}
>> +
>> +	intr_mask = phy_read(phydev, QCA808X_PHY_INTR_MASK);
>> +	if (intr_mask < 0) {
>> +		phy_error(phydev);
>> +		return IRQ_NONE;
>> +	}
>> +
>> +	if (!(intr_status & intr_mask))
>> +		return IRQ_NONE;
>> +
>> +	phy_start_machine(phydev);
>> +
> phy_mac_interrupt() is the correct call here, even though it does the
> same as phy_start_machine().
Thanks for the comments, will update to use phy_mac_interrupt() in next 
patch.
>
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static int qca808x_set_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol)
>> +{
>> +	struct net_device *ndev = phydev->attached_dev;
>> +	const u8 *mac;
>> +	int ret;
>> +	unsigned int i, offsets[] = {
> offsets[] should be static const.
>
>> +		QCA808X_PHY_MMD3_LOCAL_MAC_ADDR_32_47_OFFSET,
>> +		QCA808X_PHY_MMD3_LOCAL_MAC_ADDR_16_31_OFFSET,
>> +		QCA808X_PHY_MMD3_LOCAL_MAC_ADDR_0_15_OFFSET,
>> +	};
>> +
>> +	if (!ndev)
>> +		return -ENODEV;
>> +
>> +	if (wol->wolopts & WAKE_MAGIC) {
>> +		mac = (const u8 *) ndev->dev_addr;
>> +		if (!is_valid_ether_addr(mac))
>> +			return -EINVAL;
>> +
>> +		for (i = 0; i < 3; i++)
> ARRAY_SIZE(offsets) would be better than a magic number. And in general:
> Instead of the loop, wouldn't it be simpler and better readable to write:
>
> phy_write_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_LOCAL_MAC_ADDR_32_47_OFFSET, mac[1] | (mac[0] < 8));
> phy_write_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_LOCAL_MAC_ADDR_16_31_OFFSET, mac[3] | (mac[2] < 8));
> phy_write_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_LOCAL_MAC_ADDR_0_15_OFFSET, mac[5] | (mac[4] < 8));
thanks for the suggestion, will update to take this in the next patch.
>
>> +			phy_write_mmd(phydev, MDIO_MMD_PCS, offsets[i],
>> +				      mac[(i * 2) + 1] | (mac[(i * 2)] << 8));
>> +
>> +		/* clear the pending wol interrupt */
>> +		phy_read(phydev, QCA808X_PHY_INTR_STATUS);
>> +
>> +		ret = phy_modify(phydev, QCA808X_PHY_INTR_MASK, 0, QCA808X_INTR_ENABLE_WOL);
>> +		if (ret)
>> +			return ret;
>> +
>> +		ret = phy_modify_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_WOL_CTRL,
>> +				0, QCA808X_WOL_EN);
>> +	} else {
>> +		ret = phy_modify(phydev, QCA808X_PHY_INTR_MASK, QCA808X_INTR_ENABLE_WOL, 0);
>> +		if (ret)
>> +			return ret;
>> +
>> +		ret = phy_modify_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_WOL_CTRL,
>> +				QCA808X_WOL_EN, 0);
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static void qca808x_get_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol)
>> +{
>> +	int ret;
>> +
>> +	wol->supported = WAKE_MAGIC;
>> +	wol->wolopts = 0;
>> +
>> +	ret = phy_read_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_WOL_CTRL);
>> +	if (ret < 0)
>> +		return;
>> +
>> +	if (ret & QCA808X_WOL_EN)
>> +		wol->wolopts |= WAKE_MAGIC;
>> +}
>> +
>> +static int qca808x_suspend(struct phy_device *phydev)
>> +{
>> +	int ret;
>> +
>> +	ret = phy_read_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_WOL_CTRL);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	if (ret & QCA808X_WOL_EN)
>> +		return 0;
> This isn't needed, phy_suspend() checks for WoL being enabled.
will update to use phy_suspend() in next patch, thanks for this comment.
>
>> +
>> +	return genphy_suspend(phydev);
>> +}
>> +
>> +static int qca808x_speed_forced(struct phy_device *phydev)
>> +{
>> +	u16 speed_ctrl, type_ctrl;
>> +	int ret;
>> +
>> +	switch (phydev->speed) {
>> +	case SPEED_2500:
>> +		speed_ctrl = QCA808X_PMA_CONTROL_2500M;
>> +		type_ctrl = QCA808X_PMA_TYPE_2500M;
>> +		break;
>> +	case SPEED_1000:
>> +		speed_ctrl = QCA808X_PMA_CONTROL_1000M;
>> +		type_ctrl = QCA808X_PMA_TYPE_1000M;
>> +		break;
>> +	case SPEED_100:
>> +		speed_ctrl = QCA808X_PMA_CONTROL_100M;
>> +		type_ctrl = QCA808X_PMA_TYPE_100M;
>> +		break;
>> +	default:
>> +		speed_ctrl = QCA808X_PMA_CONTROL_10M;
>> +		type_ctrl = QCA808X_PMA_TYPE_10M;
>> +		break;
>> +	}
>> +
>> +	ret = phy_modify_mmd(phydev, MDIO_MMD_PMAPMD, QCA808X_PHY_MMD1_PMA_CONTROL,
>> +			QCA808X_PMA_CONTROL_SPEED_MASK, speed_ctrl);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return phy_modify_mmd(phydev, MDIO_MMD_PMAPMD, QCA808X_PHY_MMD1_PMA_TYPE,
>> +			QCA808X_PMA_TYPE_MASK, type_ctrl);
>> +}
>> +
>> +static int qca808x_config_aneg(struct phy_device *phydev)
>> +{
>> +	int phy_ctrl = 0;
>> +	int ret = 0;
>> +
>> +	if (phydev->autoneg != AUTONEG_ENABLE) {
>> +		ret = genphy_setup_forced(phydev);
>> +		if (ret)
>> +			return ret;
>> +
>> +		ret = qca808x_speed_forced(phydev);
>> +	} else {
>> +		ret = __genphy_config_aneg(phydev, ret);
>> +		if (ret)
>> +			return ret;
>> +
>> +		if (linkmode_test_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, phydev->advertising))
>> +			phy_ctrl = QCA808X_ADVERTISE_2500FULL;
>> +
>> +		ret = phy_modify_mmd(phydev, MDIO_MMD_AN, QCA808X_PHY_MMD7_AUTONEGOTIATION_CONTROL,
>> +				QCA808X_ADVERTISE_2500FULL, phy_ctrl);
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static int qca808x_get_speed(struct phy_device *phydev)
>> +{
>> +	int ret = phy_read(phydev, QCA808X_PHY_SPEC_STATUS);
>> +
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	switch (FIELD_GET(QCA808X_STATUS_SPEED_MASK, ret)) {
>> +	case QCA808X_STATUS_SPEED_2500MBS:
>> +		phydev->speed = SPEED_2500;
>> +		break;
>> +	case QCA808X_STATUS_SPEED_1000MBS:
>> +		phydev->speed = SPEED_1000;
>> +		break;
>> +	case QCA808X_STATUS_SPEED_100MBS:
>> +		phydev->speed = SPEED_100;
>> +		break;
>> +	default:
>> +		phydev->speed = SPEED_10;
>> +		break;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int qca808x_read_status(struct phy_device *phydev)
>> +{
>> +	int ret;
>> +
>> +	ret = genphy_read_status(phydev);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (!phydev->link) {
> Why is all the following needed in read_status() if link is down?
> When using polling mode then this would be executed with each poll
> if link is down.

Hi Heiner,

thanks for the review comments, will move the 
ETHTOOL_LINK_MODE_2500baseT_Full_BIT out of

the condition if link is down.

About the master/slave configuration, the PHY need to configure a 
different lower SEED value to

make PHY linked as the SLAVE mode to fix some IOT issue that leads to 
the link down.

if there is a master/slave configuration fault detected, then disable 
the SEED config.

>> +		ret = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_10GBT_STAT);
>> +		if (ret < 0)
>> +			return ret;
>> +
>> +		linkmode_mod_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, phydev->lp_advertising,
>> +				ret & MDIO_AN_10GBT_STAT_LP2_5G);
>> +
>> +		/* generate random seed as a lower value to make PHY linked as SLAVE easily,
>> +		 * excpet for master/slave configuration fault detected.
>> +		 */
>> +		if (qca808x_get_2500caps(phydev) == 1) {
>> +			ret = phy_read(phydev, MII_STAT1000);
>> +			if (ret < 0)
>> +				return ret;
>> +
>> +			if (ret & LPA_1000MSFAIL) {
>> +				qca808x_phy_ms_seed_enable(phydev, false);
>> +			} else {
>> +				qca808x_phy_ms_random_seed_set(phydev);
>> +				qca808x_phy_ms_seed_enable(phydev, true);
>> +			}
>> +		}
>> +	}
>> +
>> +	return qca808x_get_speed(phydev);
>> +}
>> +
>> +static int qca808x_soft_reset(struct phy_device *phydev)
>> +{
>> +	int ret;
>> +
>> +	ret = genphy_soft_reset(phydev);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	if (phydev->autoneg == AUTONEG_DISABLE) {
> Isn't this what genphy_soft_reset() does anyway?
Thanks Heiner for the comments, genphy_soft_reset does not support 2.5G, 
the qca8081 may need to force 2.5G here.
>
>> +		ret = qca808x_speed_forced(phydev);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	return qca808x_phy_ms_seed_enable(phydev, true);
>> +}
>> +
>> +static struct phy_driver qca808x_phy_driver[] = {
>> +	{
>> +		PHY_ID_MATCH_EXACT(QCA8081_PHY_ID),
>> +		.name			= "QCA8081 PHY",
>> +		.get_features		= qca808x_get_features,
>> +		.config_init		= qca808x_config_init,
>> +		.read_status		= qca808x_read_status,
>> +		.soft_reset		= qca808x_soft_reset,
>> +		.config_intr		= qca808x_config_intr,
>> +		.handle_interrupt	= qca808x_handle_interrupt,
>> +		.config_aneg		= qca808x_config_aneg,
>> +		.set_wol		= qca808x_set_wol,
>> +		.get_wol		= qca808x_get_wol,
>> +		.suspend		= qca808x_suspend,
>> +		.resume			= genphy_resume,
>> +	},
>> +};
>> +module_phy_driver(qca808x_phy_driver);
>> +
>> +static struct mdio_device_id __maybe_unused qca808x_phy_tbl[] = {
>> +	{ PHY_ID_MATCH_EXACT(QCA8081_PHY_ID) },
>> +	{ },
>> +};
>> +MODULE_DEVICE_TABLE(mdio, qca808x_phy_tbl);
>> +
>> +MODULE_DESCRIPTION("Qualcomm Technologies, Inc. QCA8081 PHY driver");
>> +MODULE_AUTHOR("Luo Jie");
>> +MODULE_LICENSE("GPL");
>>

Re: [PATCH] net: phy: add qca8081 ethernet phy driver

[Russell King (Oracle)]

On Tue, Aug 17, 2021 at 09:10:44PM +0800, Jie Luo wrote:
> 
> On 8/16/2021 9:56 PM, Andrew Lunn wrote:
> > On Mon, Aug 16, 2021 at 07:34:40PM +0800, Luo Jie wrote:
> > > qca8081 is industry’s lowest cost and power 1-port 2.5G/1G Ethernet PHY
> > > chip, which implements SGMII/SGMII+ for interface to SoC.
> > Hi Luo
> > 
> > No Marketing claims in the commit message please. Even if it is
> > correct now, it will soon be wrong with newer generations of
> > devices.
> > 
> > And what is SGMII+? Please reference a document. Is it actually
> > 2500BaseX?
> 
> Hi Andrew,
> 
> thanks for the comments, will remove the claims in the next patch.
> 
> SGMII+ is for 2500BaseX, which is same as SGMII, but the clock frequency of
> SGMII+ is 2.5 times SGMII.

Bah. Sounds like more industry wide misunderstanding of what SGMII
vs 1000Base-X is to me... or just plain abuse of the term "SGMII"!

I think you'll find that "SGMII+" is nothing but a term Qualcomm use
to refer to 2500BASE-X without the 16-bit in-band control word to
the host.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH] net: phy: add qca8081 ethernet phy driver

[Michael Walle]

> qca8081 supports IEEE1588 feature, the IEEE1588 code may be submitted in 
> the near future,
> 
> so it may be a good idea to keep it out from at803x code.

The AR8031 also supports PTP. Unfortunately, there is no public datasheet
for the QCA8081, so I can't have a look if both are similar.

See also,
https://lore.kernel.org/netdev/20200228180226.22986-1-michael@walle.cc/

-michael

Re: [PATCH] net: phy: add qca8081 ethernet phy driver

[Jie Luo]

On 8/17/2021 9:33 PM, Andrew Lunn wrote:
> On Tue, Aug 17, 2021 at 09:10:44PM +0800, Jie Luo wrote:
>> On 8/16/2021 9:56 PM, Andrew Lunn wrote:
>>> On Mon, Aug 16, 2021 at 07:34:40PM +0800, Luo Jie wrote:
>>>> qca8081 is industry’s lowest cost and power 1-port 2.5G/1G Ethernet PHY
>>>> chip, which implements SGMII/SGMII+ for interface to SoC.
>>> Hi Luo
>>>
>>> No Marketing claims in the commit message please. Even if it is
>>> correct now, it will soon be wrong with newer generations of
>>> devices.
>>>
>>> And what is SGMII+? Please reference a document. Is it actually
>>> 2500BaseX?
>> Hi Andrew,
>>
>> thanks for the comments, will remove the claims in the next patch.
>>
>> SGMII+ is for 2500BaseX, which is same as SGMII, but the clock frequency of
>> SGMII+ is 2.5 times SGMII.
> 25000BaseX is not SGMII over clocked at 2.5GHz.
>
> If it is using 2500BaseX then call it 2500BaseX, because 2500BaseX is
> well defined in the standards, and SGMII overclocked to 2.5G is not
> standardised.

will update it to use 2500BaseX in the next patch, thanks for this comment.

>
>>> A lot of these registers look the same as the at803x. So i'm thinking
>>> you should merge these two drivers. There is a lot of code which is
>>> identical, or very similar, which you can share.
>> Hi Andrew,
>>
>> qca8081 supports IEEE1588 feature, the IEEE1588 code may be submitted in the
>> near future,
>>
>> so it may be a good idea to keep it out from at803x code.
> Please merge it. A lot of the code is the same, and a lot of the new
> code you are adding will go away once you use the helpers. And
> probably you can improve the features of the older PHYs at the same
> time, where features are the same between them.
thanks for the comment, will update the patch to merge it into at803x code.
>
>>>> +static int qca808x_phy_ms_seed_enable(struct phy_device *phydev, bool enable)
>>>> +{
>>>> +	u16 seed_enable = 0;
>>>> +
>>>> +	if (enable)
>>>> +		seed_enable = QCA808X_MASTER_SLAVE_SEED_ENABLE;
>>>> +
>>>> +	return qca808x_debug_reg_modify(phydev, QCA808X_PHY_DEBUG_LOCAL_SEED,
>>>> +			QCA808X_MASTER_SLAVE_SEED_ENABLE, seed_enable);
>>>> +}
>>> This is interesting. I've not seen any other PHY does this. is there
>>> documentation? Is the datasheet available?
>> this piece of code is for configuring the random seed to a lower value to
>> make the PHY linked
>>
>> as the SLAVE mode for fixing some IOT issue, for master/slave
>> auto-negotiation, please refer to
>>
>> https://www.ieee802.org/3/an/public/jul04/lynskey_2_0704.pdf.
> And what happens when this device is used in an Ethernet switch? A
> next generation of a qca8k? Take a look at
> genphy_setup_master_slave().  Use MASTER_SLAVE_CFG_MASTER_PREFERRED or
> MASTER_SLAVE_CFG_SLAVE_PREFERRED to decide how to bias the
> master/slave decision.
>
>       Andrew

Hi Andrew, thanks for this comment, qca8081 is mainly used in the IPQ 
SOC chip currently.

qca801 is configured as MASTER_SLAVE_CFG_SLAVE_PREFERRED by default.

the SEED random lower value is for making the qca8081 linked as SLAVE 
mode when the link

partner is also configured as MASTER_SLAVE_CFG_SLAVE_PREFERRED in 
auto-negotiation.

.

Re: [PATCH] net: phy: add qca8081 ethernet phy driver

[Jie Luo]

On 8/18/2021 3:41 PM, Michael Walle wrote:
>> qca8081 supports IEEE1588 feature, the IEEE1588 code may be submitted in
>> the near future,
>>
>> so it may be a good idea to keep it out from at803x code.
> The AR8031 also supports PTP. Unfortunately, there is no public datasheet
> for the QCA8081, so I can't have a look if both are similar.
>
> See also,
> https://lore.kernel.org/netdev/20200228180226.22986-1-michael@walle.cc/
>
> -michael

Hi Michael,

Thanks for this comment. it is true that AR8031 supports basic PTP 
features.

please refer to the following link for the outline features of qca801.

https://www.qualcomm.com/products/qca8081

Re: [PATCH] net: phy: add qca8081 ethernet phy driver

[Andrew Lunn]

On Wed, Aug 18, 2021 at 09:34:40PM +0800, Jie Luo wrote:
> 
> On 8/18/2021 3:41 PM, Michael Walle wrote:
> > > qca8081 supports IEEE1588 feature, the IEEE1588 code may be submitted in
> > > the near future,
> > > 
> > > so it may be a good idea to keep it out from at803x code.
> > The AR8031 also supports PTP. Unfortunately, there is no public datasheet
> > for the QCA8081, so I can't have a look if both are similar.
> > 
> > See also,
> > https://lore.kernel.org/netdev/20200228180226.22986-1-michael@walle.cc/
> > 
> > -michael
> 
> Hi Michael,
> 
> Thanks for this comment. it is true that AR8031 supports basic PTP features.
> 
> please refer to the following link for the outline features of qca801.
> 
> https://www.qualcomm.com/products/qca8081

Is the PTP hardware in the qca8081 the same as the ar8031? When you
add PTP support, will it be for both PHYs?

What about the cable diagnostics? The at803x already has this
implemented. Will the same work for the qca8081?

    Andrew

Re: [PATCH] net: phy: add qca8081 ethernet phy driver

[Jie Luo]

On 8/19/2021 1:09 AM, Andrew Lunn wrote:
> On Wed, Aug 18, 2021 at 09:34:40PM +0800, Jie Luo wrote:
>> On 8/18/2021 3:41 PM, Michael Walle wrote:
>>>> qca8081 supports IEEE1588 feature, the IEEE1588 code may be submitted in
>>>> the near future,
>>>>
>>>> so it may be a good idea to keep it out from at803x code.
>>> The AR8031 also supports PTP. Unfortunately, there is no public datasheet
>>> for the QCA8081, so I can't have a look if both are similar.
>>>
>>> See also,
>>> https://lore.kernel.org/netdev/20200228180226.22986-1-michael@walle.cc/
>>>
>>> -michael
>> Hi Michael,
>>
>> Thanks for this comment. it is true that AR8031 supports basic PTP features.
>>
>> please refer to the following link for the outline features of qca801.
>>
>> https://www.qualcomm.com/products/qca8081
> Is the PTP hardware in the qca8081 the same as the ar8031? When you
> add PTP support, will it be for both PHYs?
>
> What about the cable diagnostics? The at803x already has this
> implemented. Will the same work for the qca8081?
>
>      Andrew

Hi Andrew,

qca8081 enhances the ptp feature from AR8031, the new ptp driver should 
be for qca8081, which is fully

tested on qca8081, but it is not verified on AR8031 currently.

the cable diagnostics feature of qca8081 is almost same as ar8031 but 
the register is changed, will correct it

for qca8081 in the next patch correspondingly.

thanks.