linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] net: phy: adin1100: Add initial support for ADIN1100 industrial PHY
@ 2021-06-24 14:53 alexandru.tachici
  2021-06-24 14:53 ` [PATCH 1/4] " alexandru.tachici
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: alexandru.tachici @ 2021-06-24 14:53 UTC (permalink / raw)
  To: netdev, linux-kernel; +Cc: andrew, davem, kuba, linux, Alexandru Tachici

From: Alexandru Tachici <alexandru.tachici@analog.com>

The ADIN1100 is a low power single port 10BASE-T1L transceiver designed for
industrial Ethernet applications and is compliant with the IEEE 802.3cg
Ethernet standard for long reach 10 Mb/s Single Pair Ethernet.

1. Add basic support for ADIN1100.

Alexandru Ardelean (1):
  net: phy: adin1100: Add initial support for ADIN1100 industrial PHY

1. Allow user to access error and frame counters through ethtool.

2. Allow user to set the master-slave configuration of ADIN1100.

3. Convert MSE to SQI using a predefined table and allow user access
through ethtool.

Alexandru Tachici (3):
  net: phy: adin1100: Add ethtool get_stats support
  net: phy: adin1100: Add ethtool master-slave support
  net: phy: adin1100: Add SQI support

 drivers/net/phy/Kconfig    |   7 +
 drivers/net/phy/Makefile   |   1 +
 drivers/net/phy/adin1100.c | 642 +++++++++++++++++++++++++++++++++++++
 3 files changed, 650 insertions(+)
 create mode 100644 drivers/net/phy/adin1100.c

--
2.25.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/4] net: phy: adin1100: Add initial support for ADIN1100 industrial PHY
  2021-06-24 14:53 [PATCH 0/4] net: phy: adin1100: Add initial support for ADIN1100 industrial PHY alexandru.tachici
@ 2021-06-24 14:53 ` alexandru.tachici
  2021-06-24 17:15   ` Andrew Lunn
                     ` (2 more replies)
  2021-06-24 14:53 ` [PATCH 2/4] net: phy: adin1100: Add ethtool get_stats support alexandru.tachici
                   ` (2 subsequent siblings)
  3 siblings, 3 replies; 9+ messages in thread
From: alexandru.tachici @ 2021-06-24 14:53 UTC (permalink / raw)
  To: netdev, linux-kernel
  Cc: andrew, davem, kuba, linux, Alexandru Ardelean, Alexandru Tachici

From: Alexandru Ardelean <alexandru.ardelean@analog.com>

The ADIN1100 is a low power single port 10BASE-T1L transceiver designed for
industrial Ethernet applications and is compliant with the IEEE 802.3cg
Ethernet standard for long reach 10 Mb/s Single Pair Ethernet.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com>
---
 drivers/net/phy/Kconfig    |   7 +
 drivers/net/phy/Makefile   |   1 +
 drivers/net/phy/adin1100.c | 437 +++++++++++++++++++++++++++++++++++++
 3 files changed, 445 insertions(+)
 create mode 100644 drivers/net/phy/adin1100.c

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index c56f703ae998..8f9f61fe0020 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -83,6 +83,13 @@ config ADIN_PHY
 	  - ADIN1300 - Robust,Industrial, Low Latency 10/100/1000 Gigabit
 	    Ethernet PHY
 
+config ADIN1100_PHY
+	tristate "Analog Devices Industrial Ethernet T1L PHYs"
+	help
+	  Adds support for the Analog Devices Industrial T1L Ethernet PHYs.
+	  Currently supports the:
+	  - ADIN1100 - Robust,Industrial, Low Power 10BASE-T1L Ethernet PHY
+
 config AQUANTIA_PHY
 	tristate "Aquantia PHYs"
 	help
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 172bb193ae6a..3bd6a369d60c 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -31,6 +31,7 @@ sfp-obj-$(CONFIG_SFP)		+= sfp-bus.o
 obj-y				+= $(sfp-obj-y) $(sfp-obj-m)
 
 obj-$(CONFIG_ADIN_PHY)		+= adin.o
+obj-$(CONFIG_ADIN1100_PHY)	+= adin1100.o
 obj-$(CONFIG_AMD_PHY)		+= amd.o
 aquantia-objs			+= aquantia_main.o
 ifdef CONFIG_HWMON
diff --git a/drivers/net/phy/adin1100.c b/drivers/net/phy/adin1100.c
new file mode 100644
index 000000000000..8d85a4d00d80
--- /dev/null
+++ b/drivers/net/phy/adin1100.c
@@ -0,0 +1,437 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
+/**
+ *  Driver for Analog Devices Industrial Ethernet T1L PHYs
+ *
+ * Copyright 2020 Analog Devices Inc.
+ */
+#include <linux/kernel.h>
+#include <linux/bitfield.h>
+#include <linux/delay.h>
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/mii.h>
+#include <linux/phy.h>
+#include <linux/property.h>
+
+#define PHY_ID_ADIN1100				0x0283bc81
+
+static const int phy_10_features_array[] = {
+	ETHTOOL_LINK_MODE_10baseT_Full_BIT,
+};
+
+#define ADIN_B10L_PCS_CNTRL			0x08e6
+#define   ADIN_PCS_CNTRL_B10L_LB_PCS_EN		BIT(14)
+
+#define ADIN_B10L_PMA_CNTRL			0x08f6
+#define   ADIN_PMA_CNTRL_B10L_LB_PMA_LOC_EN	BIT(0)
+
+#define ADIN_B10L_PMA_STAT			0x08f7
+#define   ADIN_PMA_STAT_B10L_LB_PMA_LOC_ABLE	BIT(13)
+#define   ADIN_PMA_STAT_B10L_TX_LVL_HI_ABLE	BIT(12)
+
+#define ADIN_AN_CONTROL				0x0200
+#define   ADIN_AN_RESTART			BIT(9)
+#define   ADIN_AN_EN				BIT(12)
+
+#define ADIN_AN_STATUS				0x0201
+#define ADIN_AN_ADV_ABILITY_L			0x0202
+#define ADIN_AN_ADV_ABILITY_M			0x0203
+#define ADIN_AN_ADV_ABILITY_H			0x0204U
+#define   ADIN_AN_ADV_B10L_TX_LVL_HI_ABL	BIT(13)
+#define   ADIN_AN_ADV_B10L_TX_LVL_HI_REQ	BIT(12)
+
+#define ADIN_AN_LP_ADV_ABILITY_L		0x0205
+
+#define ADIN_AN_LP_ADV_ABILITY_M		0x0206
+#define   ADIN_AN_LP_ADV_B10L			BIT(14)
+#define   ADIN_AN_LP_ADV_B1000			BIT(7)
+#define   ADIN_AN_LP_ADV_B10S_FD		BIT(6)
+#define   ADIN_AN_LP_ADV_B100			BIT(5)
+#define   ADIN_AN_LP_ADV_MST			BIT(4)
+
+#define ADIN_AN_LP_ADV_ABILITY_H		0x0207
+#define   ADIN_AN_LP_ADV_B10L_EEE		BIT(14)
+#define   ADIN_AN_LP_ADV_B10L_TX_LVL_HI_ABL	BIT(13)
+#define   ADIN_AN_LP_ADV_B10L_TX_LVL_HI_REQ	BIT(12)
+#define   ADIN_AN_LP_ADV_B10S_HD		BIT(11)
+
+#define ADIN_CRSM_SFT_RST			0x8810
+#define   ADIN_CRSM_SFT_RST_EN			BIT(0)
+
+#define ADIN_CRSM_SFT_PD_CNTRL			0x8812
+#define   ADIN_CRSM_SFT_PD_CNTRL_EN		BIT(0)
+
+#define ADIN_CRSM_STAT				0x8818
+#define   ADIN_CRSM_SFT_PD_RDY			BIT(1)
+#define   ADIN_CRSM_SYS_RDY			BIT(0)
+
+#define ADIN_MAC_IF_LOOPBACK			0x803d
+#define   ADIN_MAC_IF_LOOPBACK_EN		BIT(0)
+#define   ADIN_MAC_IF_REMOTE_LOOPBACK_EN	BIT(2)
+
+/**
+ * struct adin_priv - ADIN PHY driver private data
+ * tx_level_24v			set if the PHY supports 2.4V TX levels (10BASE-T1L)
+ */
+struct adin_priv {
+	unsigned int		tx_level_24v:1;
+};
+
+static int adin_match_phy_device(struct phy_device *phydev)
+{
+	struct mii_bus *bus = phydev->mdio.bus;
+	int phy_addr = phydev->mdio.addr;
+	u32 id;
+	int rc;
+
+	mutex_lock(&bus->mdio_lock);
+
+	/* Need to call __mdiobus_read() directly here, because at this point
+	 * in time, the driver isn't attached to the PHY device.
+	 */
+	rc = __mdiobus_read(bus, phy_addr, MDIO_DEVID1);
+	if (rc < 0) {
+		mutex_unlock(&bus->mdio_lock);
+		return rc;
+	}
+
+	id = rc << 16;
+
+	rc = __mdiobus_read(bus, phy_addr, MDIO_DEVID2);
+	mutex_unlock(&bus->mdio_lock);
+
+	if (rc < 0)
+		return rc;
+
+	id |= rc;
+
+	switch (id) {
+	case PHY_ID_ADIN1100:
+		return 1;
+	default:
+		return 0;
+	}
+}
+
+static void adin_mii_adv_m_to_ethtool_adv_t(unsigned long *advertising, u32 adv)
+{
+	if (adv & ADIN_AN_LP_ADV_B10L)
+		linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, advertising);
+	if (adv & ADIN_AN_LP_ADV_B1000) {
+		linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT, advertising);
+		linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, advertising);
+	}
+	if (adv & ADIN_AN_LP_ADV_B10S_FD)
+		linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, advertising);
+	if (adv & ADIN_AN_LP_ADV_B100)
+		linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, advertising);
+}
+
+static void adin_mii_adv_h_to_ethtool_adv_t(unsigned long *advertising, u32 adv)
+{
+	if (adv & ADIN_AN_LP_ADV_B10S_HD)
+		linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, advertising);
+}
+
+static int adin_read_lpa(struct phy_device *phydev)
+{
+	int val;
+
+	linkmode_zero(phydev->lp_advertising);
+
+	val = phy_read_mmd(phydev, MDIO_MMD_AN, ADIN_AN_STATUS);
+	if (val < 0)
+		return val;
+
+	if (!(val & MDIO_AN_STAT1_COMPLETE)) {
+		phydev->pause = 0;
+		phydev->asym_pause = 0;
+
+		return 0;
+	}
+
+	linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
+			 phydev->lp_advertising);
+
+	/* Read the link partner's base page advertisement */
+	val = phy_read_mmd(phydev, MDIO_MMD_AN, ADIN_AN_LP_ADV_ABILITY_L);
+	if (val < 0)
+		return val;
+
+	phydev->pause = val & LPA_PAUSE_CAP ? 1 : 0;
+	phydev->asym_pause = val & LPA_PAUSE_ASYM ? 1 : 0;
+
+	val = phy_read_mmd(phydev, MDIO_MMD_AN, ADIN_AN_LP_ADV_ABILITY_M);
+	if (val < 0)
+		return val;
+
+	adin_mii_adv_m_to_ethtool_adv_t(phydev->lp_advertising, val);
+
+	val = phy_read_mmd(phydev, MDIO_MMD_AN, ADIN_AN_LP_ADV_ABILITY_H);
+	if (val < 0)
+		return val;
+
+	adin_mii_adv_h_to_ethtool_adv_t(phydev->lp_advertising, val);
+
+	return 0;
+}
+
+static int adin_read_status(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = genphy_c45_read_link(phydev);
+	if (ret)
+		return ret;
+
+	phydev->speed = SPEED_UNKNOWN;
+	phydev->duplex = DUPLEX_UNKNOWN;
+	phydev->pause = 0;
+	phydev->asym_pause = 0;
+
+	if (phydev->autoneg == AUTONEG_ENABLE) {
+		ret = adin_read_lpa(phydev);
+		if (ret)
+			return ret;
+
+		phy_resolve_aneg_linkmode(phydev);
+	} else {
+		/* Only one mode & duplex supported */
+		linkmode_zero(phydev->lp_advertising);
+		phydev->speed = SPEED_10;
+		phydev->duplex = DUPLEX_FULL;
+	}
+
+	return ret;
+}
+
+static int adin_config_aneg(struct phy_device *phydev)
+{
+	struct adin_priv *priv = phydev->priv;
+	int ret;
+
+	/* No sense to continue if auto-neg is disabled,
+	 * only one link-mode supported.
+	 */
+	if (phydev->autoneg == AUTONEG_DISABLE)
+		return 0;
+
+	if (priv->tx_level_24v)
+		ret = phy_set_bits_mmd(phydev, MDIO_MMD_AN,
+				       ADIN_AN_ADV_ABILITY_H,
+				       ADIN_AN_ADV_B10L_TX_LVL_HI_ABL |
+				       ADIN_AN_ADV_B10L_TX_LVL_HI_REQ);
+	else
+		ret = phy_clear_bits_mmd(phydev, MDIO_MMD_AN,
+					 ADIN_AN_ADV_ABILITY_H,
+					 ADIN_AN_ADV_B10L_TX_LVL_HI_ABL |
+					 ADIN_AN_ADV_B10L_TX_LVL_HI_REQ);
+
+	if (ret < 0)
+		return ret;
+
+	return phy_set_bits_mmd(phydev, MDIO_MMD_AN, ADIN_AN_CONTROL, ADIN_AN_RESTART);
+}
+
+static void adin_link_change_notify(struct phy_device *phydev)
+{
+	bool tx_level_24v;
+	bool lp_tx_level_24v;
+	int val, mask;
+
+	if (phydev->state != PHY_RUNNING || phydev->autoneg == AUTONEG_DISABLE)
+		return;
+
+	val = phy_read_mmd(phydev, MDIO_MMD_AN, ADIN_AN_LP_ADV_ABILITY_H);
+	if (val < 0)
+		return;
+
+	mask = ADIN_AN_LP_ADV_B10L_TX_LVL_HI_ABL | ADIN_AN_LP_ADV_B10L_TX_LVL_HI_REQ;
+	lp_tx_level_24v = (val & mask) == mask;
+
+	val = phy_read_mmd(phydev, MDIO_MMD_AN, ADIN_AN_ADV_ABILITY_H);
+	if (val < 0)
+		return;
+
+	mask = ADIN_AN_ADV_B10L_TX_LVL_HI_ABL | ADIN_AN_ADV_B10L_TX_LVL_HI_REQ;
+	tx_level_24v = (val & mask) == mask;
+
+	if (tx_level_24v && lp_tx_level_24v)
+		phydev_info(phydev, "Negotiated 2.4V TX level\n");
+	else
+		phydev_info(phydev, "Negotiated 1.0V TX level\n");
+}
+
+static int adin_set_powerdown_mode(struct phy_device *phydev, bool en)
+{
+	int ret, timeout;
+	u16 val;
+
+	if (en)
+		val = ADIN_CRSM_SFT_PD_CNTRL_EN;
+	else
+		val = 0;
+
+	ret = phy_write_mmd(phydev, MDIO_MMD_VEND1,
+			    ADIN_CRSM_SFT_PD_CNTRL, val);
+	if (ret < 0)
+		return ret;
+
+	timeout = 30;
+	while (timeout-- > 0) {
+		ret = phy_read_mmd(phydev, MDIO_MMD_VEND1,
+				   ADIN_CRSM_STAT);
+		if (ret < 0)
+			return ret;
+
+		if ((ret & ADIN_CRSM_SFT_PD_RDY) == val)
+			return 0;
+
+		mdelay(1);
+	}
+
+	return -ETIMEDOUT;
+}
+
+static int adin_suspend(struct phy_device *phydev)
+{
+	return adin_set_powerdown_mode(phydev, true);
+}
+
+static int adin_resume(struct phy_device *phydev)
+{
+	return adin_set_powerdown_mode(phydev, false);
+}
+
+static int adin_set_loopback(struct phy_device *phydev, bool enable)
+{
+	int ret;
+
+	if (enable)
+		return phy_set_bits_mmd(phydev, MDIO_MMD_PCS,
+					ADIN_B10L_PCS_CNTRL,
+					ADIN_PCS_CNTRL_B10L_LB_PCS_EN);
+
+	/* MAC interface block loopback */
+	ret = phy_clear_bits_mmd(phydev, MDIO_MMD_VEND1, ADIN_MAC_IF_LOOPBACK,
+				 ADIN_MAC_IF_LOOPBACK_EN |
+				 ADIN_MAC_IF_REMOTE_LOOPBACK_EN);
+	if (ret < 0)
+		return ret;
+
+	/* PCS loopback (according to 10BASE-T1L spec) */
+	ret = phy_clear_bits_mmd(phydev, MDIO_MMD_PCS, ADIN_B10L_PCS_CNTRL,
+				 ADIN_PCS_CNTRL_B10L_LB_PCS_EN);
+	if (ret < 0)
+		return ret;
+
+	/* PMA loopback (according to 10BASE-T1L spec) */
+	return phy_clear_bits_mmd(phydev, MDIO_MMD_PMAPMD, ADIN_B10L_PMA_CNTRL,
+				  ADIN_PMA_CNTRL_B10L_LB_PMA_LOC_EN);
+}
+
+static int adin_config_init(struct phy_device *phydev)
+{
+	struct adin_priv *priv = phydev->priv;
+	struct device *dev = &phydev->mdio.dev;
+	int ret;
+
+	ret = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, ADIN_B10L_PMA_STAT);
+	if (ret < 0)
+		return ret;
+
+	/* This depends on the voltage level from the power source */
+	priv->tx_level_24v = !!(ret & ADIN_PMA_STAT_B10L_TX_LVL_HI_ABLE);
+
+	phydev_dbg(phydev, "PHY supports 2.4V TX level: %s\n",
+		   priv->tx_level_24v ? "yes" : "no");
+
+	if (priv->tx_level_24v &&
+	    device_property_present(dev, "adi,disable-2-4-v-tx-level")) {
+		phydev_info(phydev,
+			    "PHY supports 2.4V TX level, but disabled via config\n");
+		priv->tx_level_24v = 0;
+	}
+
+	return 0;
+}
+
+static int adin_soft_reset(struct phy_device *phydev)
+{
+	int timeout;
+	int ret;
+
+	ret = phy_set_bits_mmd(phydev, MDIO_MMD_VEND1, ADIN_CRSM_SFT_RST, ADIN_CRSM_SFT_RST_EN);
+	if (ret < 0)
+		return ret;
+
+	timeout = 30;
+	while (timeout >= 0) {
+		ret = phy_read_mmd(phydev, MDIO_MMD_VEND1, ADIN_CRSM_STAT);
+		if (ret < 0)
+			return ret;
+
+		if (ret & ADIN_CRSM_SYS_RDY)
+			return 0;
+
+		usleep_range(10000, 15000);
+		timeout -= 10;
+	}
+
+	return -ETIMEDOUT;
+}
+
+static int adin_get_features(struct phy_device *phydev)
+{
+	linkmode_set_bit_array(phy_basic_ports_array, ARRAY_SIZE(phy_basic_ports_array),
+			       phydev->supported);
+
+	linkmode_set_bit_array(phy_10_features_array, ARRAY_SIZE(phy_10_features_array),
+			       phydev->supported);
+
+	return 0;
+}
+
+static int adin_probe(struct phy_device *phydev)
+{
+	struct device *dev = &phydev->mdio.dev;
+	struct adin_priv *priv;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	phydev->priv = priv;
+
+	return 0;
+}
+
+static struct phy_driver adin_driver[] = {
+	{
+		PHY_ID_MATCH_MODEL(PHY_ID_ADIN1100),
+		.name			= "ADIN1100",
+		.get_features		= adin_get_features,
+		.match_phy_device	= adin_match_phy_device,
+		.soft_reset		= adin_soft_reset,
+		.probe			= adin_probe,
+		.config_init		= adin_config_init,
+		.config_aneg		= adin_config_aneg,
+		.link_change_notify	= adin_link_change_notify,
+		.read_status		= adin_read_status,
+		.set_loopback		= adin_set_loopback,
+		.suspend		= adin_suspend,
+		.resume			= adin_resume,
+	},
+};
+
+module_phy_driver(adin_driver);
+
+static struct mdio_device_id __maybe_unused adin_tbl[] = {
+	{ PHY_ID_MATCH_MODEL(PHY_ID_ADIN1100) },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(mdio, adin_tbl);
+MODULE_DESCRIPTION("Analog Devices Industrial Ethernet T1L PHY driver");
+MODULE_LICENSE("Dual BSD/GPL");
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/4] net: phy: adin1100: Add ethtool get_stats support
  2021-06-24 14:53 [PATCH 0/4] net: phy: adin1100: Add initial support for ADIN1100 industrial PHY alexandru.tachici
  2021-06-24 14:53 ` [PATCH 1/4] " alexandru.tachici
@ 2021-06-24 14:53 ` alexandru.tachici
  2021-06-24 17:19   ` Andrew Lunn
  2021-06-24 14:53 ` [PATCH 3/4] net: phy: adin1100: Add ethtool master-slave support alexandru.tachici
  2021-06-24 14:53 ` [PATCH 4/4] net: phy: adin1100: Add SQI support alexandru.tachici
  3 siblings, 1 reply; 9+ messages in thread
From: alexandru.tachici @ 2021-06-24 14:53 UTC (permalink / raw)
  To: netdev, linux-kernel; +Cc: andrew, davem, kuba, linux, Alexandru Tachici

From: Alexandru Tachici <alexandru.tachici@analog.com>

The PHY has multiple error counters and one frame counter.
This change enables the frame checker and allows ethtool
to retrieve the counters values.

Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com>
---
 drivers/net/phy/adin1100.c | 77 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 77 insertions(+)

diff --git a/drivers/net/phy/adin1100.c b/drivers/net/phy/adin1100.c
index 8d85a4d00d80..f0674a0e8e8a 100644
--- a/drivers/net/phy/adin1100.c
+++ b/drivers/net/phy/adin1100.c
@@ -56,6 +56,8 @@ static const int phy_10_features_array[] = {
 #define   ADIN_AN_LP_ADV_B10L_TX_LVL_HI_REQ	BIT(12)
 #define   ADIN_AN_LP_ADV_B10S_HD		BIT(11)
 
+#define ADIN_FC_EN				0x8001
+
 #define ADIN_CRSM_SFT_RST			0x8810
 #define   ADIN_CRSM_SFT_RST_EN			BIT(0)
 
@@ -70,11 +72,32 @@ static const int phy_10_features_array[] = {
 #define   ADIN_MAC_IF_LOOPBACK_EN		BIT(0)
 #define   ADIN_MAC_IF_REMOTE_LOOPBACK_EN	BIT(2)
 
+struct adin_hw_stat {
+	const char *string;
+	u16 reg1;
+	u16 reg2;
+};
+
+static const struct adin_hw_stat adin_hw_stats[] = {
+	{ "total_frames_error_count",		0x8008 },
+	{ "total_frames_count",			0x8009, 0x800A }, /* hi, lo */
+	{ "length_error_frames_count",		0x800B },
+	{ "alignment_error_frames_count",	0x800C },
+	{ "symbol_error_count",			0x800D },
+	{ "oversized_frames_count",		0x800E },
+	{ "undersized_frames_count",		0x800F },
+	{ "odd_nibble_frames_count",		0x8010 },
+	{ "odd_preamble_packet_count",		0x8011 },
+	{ "false_carrier_events_count",		0x8013 },
+};
+
 /**
  * struct adin_priv - ADIN PHY driver private data
  * tx_level_24v			set if the PHY supports 2.4V TX levels (10BASE-T1L)
+ * stats:			statistic counters for the PHY
  */
 struct adin_priv {
+	u64			stats[ARRAY_SIZE(adin_hw_stats)];
 	unsigned int		tx_level_24v:1;
 };
 
@@ -354,6 +377,10 @@ static int adin_config_init(struct phy_device *phydev)
 		priv->tx_level_24v = 0;
 	}
 
+	ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, ADIN_FC_EN, 1);
+	if (ret < 0)
+		return ret;
+
 	return 0;
 }
 
@@ -393,6 +420,53 @@ static int adin_get_features(struct phy_device *phydev)
 	return 0;
 }
 
+static int adin_get_sset_count(struct phy_device *phydev)
+{
+	return ARRAY_SIZE(adin_hw_stats);
+}
+
+static void adin_get_strings(struct phy_device *phydev, u8 *data)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(adin_hw_stats); i++)
+		strlcpy(&data[i * ETH_GSTRING_LEN], adin_hw_stats[i].string, ETH_GSTRING_LEN);
+}
+
+static u64 adin_get_stat(struct phy_device *phydev, int i)
+{
+	const struct adin_hw_stat *stat = &adin_hw_stats[i];
+	struct adin_priv *priv = phydev->priv;
+	u64 val;
+	int ret;
+
+	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, stat->reg1);
+	if (ret < 0)
+		return (u64)(~0);
+
+	val = (0xffff & ret);
+
+	if (stat->reg2 != 0) {
+		ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, stat->reg2);
+		if (ret < 0)
+			return (u64)(~0);
+
+		val = (val << 16) + (0xffff & ret);
+	}
+
+	priv->stats[i] += val;
+
+	return priv->stats[i];
+}
+
+static void adin_get_stats(struct phy_device *phydev, struct ethtool_stats *stats, u64 *data)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(adin_hw_stats); i++)
+		data[i] = adin_get_stat(phydev, i);
+}
+
 static int adin_probe(struct phy_device *phydev)
 {
 	struct device *dev = &phydev->mdio.dev;
@@ -422,6 +496,9 @@ static struct phy_driver adin_driver[] = {
 		.set_loopback		= adin_set_loopback,
 		.suspend		= adin_suspend,
 		.resume			= adin_resume,
+		.get_sset_count		= adin_get_sset_count,
+		.get_strings		= adin_get_strings,
+		.get_stats		= adin_get_stats,
 	},
 };
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/4] net: phy: adin1100: Add ethtool master-slave support
  2021-06-24 14:53 [PATCH 0/4] net: phy: adin1100: Add initial support for ADIN1100 industrial PHY alexandru.tachici
  2021-06-24 14:53 ` [PATCH 1/4] " alexandru.tachici
  2021-06-24 14:53 ` [PATCH 2/4] net: phy: adin1100: Add ethtool get_stats support alexandru.tachici
@ 2021-06-24 14:53 ` alexandru.tachici
  2021-06-24 14:53 ` [PATCH 4/4] net: phy: adin1100: Add SQI support alexandru.tachici
  3 siblings, 0 replies; 9+ messages in thread
From: alexandru.tachici @ 2021-06-24 14:53 UTC (permalink / raw)
  To: netdev, linux-kernel; +Cc: andrew, davem, kuba, linux, Alexandru Tachici

From: Alexandru Tachici <alexandru.tachici@analog.com>

Allow user to select the advertised master-slave
configuration through ethtool.

Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com>
---
 drivers/net/phy/adin1100.c | 78 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 77 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/adin1100.c b/drivers/net/phy/adin1100.c
index f0674a0e8e8a..a8ddf5a9879a 100644
--- a/drivers/net/phy/adin1100.c
+++ b/drivers/net/phy/adin1100.c
@@ -36,7 +36,11 @@ static const int phy_10_features_array[] = {
 
 #define ADIN_AN_STATUS				0x0201
 #define ADIN_AN_ADV_ABILITY_L			0x0202
+#define   ADIN_AN_ADV_FORCE_MS			BIT(12)
+
 #define ADIN_AN_ADV_ABILITY_M			0x0203
+#define   ADIN_AN_ADV_MST			BIT(4)
+
 #define ADIN_AN_ADV_ABILITY_H			0x0204U
 #define   ADIN_AN_ADV_B10L_TX_LVL_HI_ABL	BIT(13)
 #define   ADIN_AN_ADV_B10L_TX_LVL_HI_REQ	BIT(12)
@@ -68,6 +72,10 @@ static const int phy_10_features_array[] = {
 #define   ADIN_CRSM_SFT_PD_RDY			BIT(1)
 #define   ADIN_CRSM_SYS_RDY			BIT(0)
 
+#define AN_PHY_INST_STATUS			0x8030
+#define   ADIN_IS_CFG_SLV			BIT(2)
+#define   ADIN_IS_CFG_MST			BIT(3)
+
 #define ADIN_MAC_IF_LOOPBACK			0x803d
 #define   ADIN_MAC_IF_LOOPBACK_EN		BIT(0)
 #define   ADIN_MAC_IF_REMOTE_LOOPBACK_EN	BIT(2)
@@ -203,6 +211,7 @@ static int adin_read_lpa(struct phy_device *phydev)
 static int adin_read_status(struct phy_device *phydev)
 {
 	int ret;
+	int cfg;
 
 	ret = genphy_c45_read_link(phydev);
 	if (ret)
@@ -212,6 +221,8 @@ static int adin_read_status(struct phy_device *phydev)
 	phydev->duplex = DUPLEX_UNKNOWN;
 	phydev->pause = 0;
 	phydev->asym_pause = 0;
+	phydev->master_slave_get = MASTER_SLAVE_CFG_UNKNOWN;
+	phydev->master_slave_state = MASTER_SLAVE_STATE_UNKNOWN;
 
 	if (phydev->autoneg == AUTONEG_ENABLE) {
 		ret = adin_read_lpa(phydev);
@@ -226,7 +237,37 @@ static int adin_read_status(struct phy_device *phydev)
 		phydev->duplex = DUPLEX_FULL;
 	}
 
-	return ret;
+	ret = phy_read_mmd(phydev, MDIO_MMD_AN, ADIN_AN_ADV_ABILITY_L);
+	if (ret < 0)
+		return ret;
+
+	cfg = phy_read_mmd(phydev, MDIO_MMD_AN, ADIN_AN_ADV_ABILITY_M);
+	if (cfg < 0)
+		return cfg;
+
+	if (ret & ADIN_AN_ADV_FORCE_MS) {
+		if (cfg & ADIN_AN_ADV_MST)
+			phydev->master_slave_get = MASTER_SLAVE_CFG_MASTER_FORCE;
+		else
+			phydev->master_slave_get = MASTER_SLAVE_CFG_SLAVE_FORCE;
+	} else {
+		if (cfg & ADIN_AN_ADV_MST)
+			phydev->master_slave_get = MASTER_SLAVE_CFG_MASTER_PREFERRED;
+		else
+			phydev->master_slave_get = MASTER_SLAVE_CFG_SLAVE_PREFERRED;
+	}
+
+	ret = phy_read_mmd(phydev, MDIO_MMD_AN, AN_PHY_INST_STATUS);
+	if (ret < 0)
+		return ret;
+
+	if (ret & ADIN_IS_CFG_SLV)
+		phydev->master_slave_state = MASTER_SLAVE_STATE_SLAVE;
+
+	if (ret & ADIN_IS_CFG_MST)
+		phydev->master_slave_state = MASTER_SLAVE_STATE_MASTER;
+
+	return 0;
 }
 
 static int adin_config_aneg(struct phy_device *phydev)
@@ -240,6 +281,41 @@ static int adin_config_aneg(struct phy_device *phydev)
 	if (phydev->autoneg == AUTONEG_DISABLE)
 		return 0;
 
+	switch (phydev->master_slave_set) {
+	case MASTER_SLAVE_CFG_MASTER_FORCE:
+	case MASTER_SLAVE_CFG_SLAVE_FORCE:
+		ret = phy_set_bits_mmd(phydev, MDIO_MMD_AN, ADIN_AN_ADV_ABILITY_L,
+				       ADIN_AN_ADV_FORCE_MS);
+		if (ret < 0)
+			return ret;
+		break;
+	case MASTER_SLAVE_CFG_MASTER_PREFERRED:
+	case MASTER_SLAVE_CFG_SLAVE_PREFERRED:
+		ret = phy_clear_bits_mmd(phydev, MDIO_MMD_AN, ADIN_AN_ADV_ABILITY_L,
+					 ADIN_AN_ADV_FORCE_MS);
+		break;
+	default:
+		break;
+	}
+
+	switch (phydev->master_slave_set) {
+	case MASTER_SLAVE_CFG_MASTER_FORCE:
+	case MASTER_SLAVE_CFG_MASTER_PREFERRED:
+		ret = phy_set_bits_mmd(phydev, MDIO_MMD_AN, ADIN_AN_ADV_ABILITY_M, ADIN_AN_ADV_MST);
+		if (ret < 0)
+			return ret;
+		break;
+	case MASTER_SLAVE_CFG_SLAVE_FORCE:
+	case MASTER_SLAVE_CFG_SLAVE_PREFERRED:
+		ret = phy_clear_bits_mmd(phydev, MDIO_MMD_AN, ADIN_AN_ADV_ABILITY_M,
+					 ADIN_AN_ADV_MST);
+		if (ret < 0)
+			return ret;
+		break;
+	default:
+		break;
+	}
+
 	if (priv->tx_level_24v)
 		ret = phy_set_bits_mmd(phydev, MDIO_MMD_AN,
 				       ADIN_AN_ADV_ABILITY_H,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/4] net: phy: adin1100: Add SQI support
  2021-06-24 14:53 [PATCH 0/4] net: phy: adin1100: Add initial support for ADIN1100 industrial PHY alexandru.tachici
                   ` (2 preceding siblings ...)
  2021-06-24 14:53 ` [PATCH 3/4] net: phy: adin1100: Add ethtool master-slave support alexandru.tachici
@ 2021-06-24 14:53 ` alexandru.tachici
  3 siblings, 0 replies; 9+ messages in thread
From: alexandru.tachici @ 2021-06-24 14:53 UTC (permalink / raw)
  To: netdev, linux-kernel; +Cc: andrew, davem, kuba, linux, Alexandru Tachici

From: Alexandru Tachici <alexandru.tachici@analog.com>

Determine the SQI from MSE using a predefined table
for the 10BASE-T1L.

Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com>
---
 drivers/net/phy/adin1100.c | 52 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/drivers/net/phy/adin1100.c b/drivers/net/phy/adin1100.c
index a8ddf5a9879a..661ccb4a2045 100644
--- a/drivers/net/phy/adin1100.c
+++ b/drivers/net/phy/adin1100.c
@@ -62,6 +62,8 @@ static const int phy_10_features_array[] = {
 
 #define ADIN_FC_EN				0x8001
 
+#define ADIN_MSE_VAL				0x830B
+
 #define ADIN_CRSM_SFT_RST			0x8810
 #define   ADIN_CRSM_SFT_RST_EN			BIT(0)
 
@@ -80,6 +82,8 @@ static const int phy_10_features_array[] = {
 #define   ADIN_MAC_IF_LOOPBACK_EN		BIT(0)
 #define   ADIN_MAC_IF_REMOTE_LOOPBACK_EN	BIT(2)
 
+#define ADIN_SQI_MAX	7
+
 struct adin_hw_stat {
 	const char *string;
 	u16 reg1;
@@ -99,6 +103,22 @@ static const struct adin_hw_stat adin_hw_stats[] = {
 	{ "false_carrier_events_count",		0x8013 },
 };
 
+struct adin_mse_sqi_range {
+	u16 start;
+	u16 end;
+};
+
+static const struct adin_mse_sqi_range adin_mse_sqi_map[] = {
+	{ 0x0A74, 0xFFFF },
+	{ 0x084E, 0x0A74 },
+	{ 0x0698, 0x084E },
+	{ 0x053D, 0x0698 },
+	{ 0x0429, 0x053D },
+	{ 0x034E, 0x0429 },
+	{ 0x02A0, 0x034E },
+	{ 0x0000, 0x02A0 },
+};
+
 /**
  * struct adin_priv - ADIN PHY driver private data
  * tx_level_24v			set if the PHY supports 2.4V TX levels (10BASE-T1L)
@@ -543,6 +563,36 @@ static void adin_get_stats(struct phy_device *phydev, struct ethtool_stats *stat
 		data[i] = adin_get_stat(phydev, i);
 }
 
+static int adin_get_sqi(struct phy_device *phydev)
+{
+	u16 mse_val;
+	int sqi;
+	int ret;
+
+	ret = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_STAT1);
+	if (ret < 0)
+		return ret;
+	else if (!(ret & MDIO_STAT1_LSTATUS))
+		return 0;
+
+	ret = phy_read_mmd(phydev, MDIO_STAT1, ADIN_MSE_VAL);
+	if (ret < 0)
+		return ret;
+
+	mse_val = 0xFFFF & ret;
+	for (sqi = 0; sqi < ARRAY_SIZE(adin_mse_sqi_map); sqi++) {
+		if (mse_val >= adin_mse_sqi_map[sqi].start && mse_val <= adin_mse_sqi_map[sqi].end)
+			return sqi;
+	}
+
+	return -EINVAL;
+}
+
+static int adin_get_sqi_max(struct phy_device *phydev)
+{
+	return ADIN_SQI_MAX;
+}
+
 static int adin_probe(struct phy_device *phydev)
 {
 	struct device *dev = &phydev->mdio.dev;
@@ -575,6 +625,8 @@ static struct phy_driver adin_driver[] = {
 		.get_sset_count		= adin_get_sset_count,
 		.get_strings		= adin_get_strings,
 		.get_stats		= adin_get_stats,
+		.get_sqi		= adin_get_sqi,
+		.get_sqi_max		= adin_get_sqi_max,
 	},
 };
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/4] net: phy: adin1100: Add initial support for ADIN1100 industrial PHY
  2021-06-24 14:53 ` [PATCH 1/4] " alexandru.tachici
@ 2021-06-24 17:15   ` Andrew Lunn
  2021-06-24 17:23   ` Andrew Lunn
  2021-06-24 22:26   ` kernel test robot
  2 siblings, 0 replies; 9+ messages in thread
From: Andrew Lunn @ 2021-06-24 17:15 UTC (permalink / raw)
  To: alexandru.tachici
  Cc: netdev, linux-kernel, davem, kuba, linux, Alexandru Ardelean

> +static const int phy_10_features_array[] = {
> +	ETHTOOL_LINK_MODE_10baseT_Full_BIT,
> +};

Since this is a T1 PHY, please add a
ETHTOOL_LINK_MODE_10baseT1_Full_BIT definition, and use that. It looks
like the device also supports half duplex? So add
ETHTOOL_LINK_MODE_10baseT1_Half_BIT as well.

What does genphy_read_abilities() determine for this device? Is there
a standardized way to detect 10BaseT1?

> +
> +#define ADIN_B10L_PCS_CNTRL			0x08e6
> +#define   ADIN_PCS_CNTRL_B10L_LB_PCS_EN		BIT(14)
> +
> +#define ADIN_B10L_PMA_CNTRL			0x08f6
> +#define   ADIN_PMA_CNTRL_B10L_LB_PMA_LOC_EN	BIT(0)
> +
> +#define ADIN_B10L_PMA_STAT			0x08f7
> +#define   ADIN_PMA_STAT_B10L_LB_PMA_LOC_ABLE	BIT(13)
> +#define   ADIN_PMA_STAT_B10L_TX_LVL_HI_ABLE	BIT(12)
> +
> +#define ADIN_AN_CONTROL				0x0200
> +#define   ADIN_AN_RESTART			BIT(9)
> +#define   ADIN_AN_EN				BIT(12)
> +
> +#define ADIN_AN_STATUS				0x0201
> +#define ADIN_AN_ADV_ABILITY_L			0x0202
> +#define ADIN_AN_ADV_ABILITY_M			0x0203
> +#define ADIN_AN_ADV_ABILITY_H			0x0204U
> +#define   ADIN_AN_ADV_B10L_TX_LVL_HI_ABL	BIT(13)
> +#define   ADIN_AN_ADV_B10L_TX_LVL_HI_REQ	BIT(12)
> +
> +#define ADIN_AN_LP_ADV_ABILITY_L		0x0205
> +
> +#define ADIN_AN_LP_ADV_ABILITY_M		0x0206
> +#define   ADIN_AN_LP_ADV_B10L			BIT(14)
> +#define   ADIN_AN_LP_ADV_B1000			BIT(7)
> +#define   ADIN_AN_LP_ADV_B10S_FD		BIT(6)
> +#define   ADIN_AN_LP_ADV_B100			BIT(5)
> +#define   ADIN_AN_LP_ADV_MST			BIT(4)
> +
> +#define ADIN_AN_LP_ADV_ABILITY_H		0x0207
> +#define   ADIN_AN_LP_ADV_B10L_EEE		BIT(14)
> +#define   ADIN_AN_LP_ADV_B10L_TX_LVL_HI_ABL	BIT(13)
> +#define   ADIN_AN_LP_ADV_B10L_TX_LVL_HI_REQ	BIT(12)
> +#define   ADIN_AN_LP_ADV_B10S_HD		BIT(11)
> +
> +#define ADIN_CRSM_SFT_RST			0x8810
> +#define   ADIN_CRSM_SFT_RST_EN			BIT(0)
> +
> +#define ADIN_CRSM_SFT_PD_CNTRL			0x8812
> +#define   ADIN_CRSM_SFT_PD_CNTRL_EN		BIT(0)
> +
> +#define ADIN_CRSM_STAT				0x8818
> +#define   ADIN_CRSM_SFT_PD_RDY			BIT(1)
> +#define   ADIN_CRSM_SYS_RDY			BIT(0)
> +
> +#define ADIN_MAC_IF_LOOPBACK			0x803d
> +#define   ADIN_MAC_IF_LOOPBACK_EN		BIT(0)
> +#define   ADIN_MAC_IF_REMOTE_LOOPBACK_EN	BIT(2)
> +
> +/**
> + * struct adin_priv - ADIN PHY driver private data
> + * tx_level_24v			set if the PHY supports 2.4V TX levels (10BASE-T1L)
> + */
> +struct adin_priv {
> +	unsigned int		tx_level_24v:1;
> +};
> +

> +static int adin_match_phy_device(struct phy_device *phydev)
> +{
> +	struct mii_bus *bus = phydev->mdio.bus;
> +	int phy_addr = phydev->mdio.addr;
> +	u32 id;
> +	int rc;
> +
> +	mutex_lock(&bus->mdio_lock);
> +
> +	/* Need to call __mdiobus_read() directly here, because at this point
> +	 * in time, the driver isn't attached to the PHY device.
> +	 */
> +	rc = __mdiobus_read(bus, phy_addr, MDIO_DEVID1);
> +	if (rc < 0) {
> +		mutex_unlock(&bus->mdio_lock);
> +		return rc;
> +	}
> +
> +	id = rc << 16;
> +
> +	rc = __mdiobus_read(bus, phy_addr, MDIO_DEVID2);
> +	mutex_unlock(&bus->mdio_lock);
> +
> +	if (rc < 0)
> +		return rc;
> +
> +	id |= rc;
> +
> +	switch (id) {
> +	case PHY_ID_ADIN1100:
> +		return 1;
> +	default:
> +		return 0;
> +	}
> +}

I must be missing something here. Why do you need this?

> +static void adin_mii_adv_m_to_ethtool_adv_t(unsigned long *advertising, u32 adv)
> +{
> +	if (adv & ADIN_AN_LP_ADV_B10L)
> +		linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, advertising);
> +	if (adv & ADIN_AN_LP_ADV_B1000) {
> +		linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT, advertising);
> +		linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, advertising);
> +	}
> +	if (adv & ADIN_AN_LP_ADV_B10S_FD)
> +		linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, advertising);
> +	if (adv & ADIN_AN_LP_ADV_B100)
> +		linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, advertising);

Since this is a T1 PHY, i assume these are all T1 link modes? Please
use ETHTOOL_LINK_MODE_1000baseT1_Half_BIT,
ETHTOOL_LINK_MODE_100baseT1_Full_BIT, etc.


> +static void adin_link_change_notify(struct phy_device *phydev)
> +{
> +	bool tx_level_24v;
> +	bool lp_tx_level_24v;
> +	int val, mask;
> +
> +	if (phydev->state != PHY_RUNNING || phydev->autoneg == AUTONEG_DISABLE)
> +		return;
> +
> +	val = phy_read_mmd(phydev, MDIO_MMD_AN, ADIN_AN_LP_ADV_ABILITY_H);
> +	if (val < 0)
> +		return;
> +
> +	mask = ADIN_AN_LP_ADV_B10L_TX_LVL_HI_ABL | ADIN_AN_LP_ADV_B10L_TX_LVL_HI_REQ;
> +	lp_tx_level_24v = (val & mask) == mask;
> +
> +	val = phy_read_mmd(phydev, MDIO_MMD_AN, ADIN_AN_ADV_ABILITY_H);
> +	if (val < 0)
> +		return;
> +
> +	mask = ADIN_AN_ADV_B10L_TX_LVL_HI_ABL | ADIN_AN_ADV_B10L_TX_LVL_HI_REQ;
> +	tx_level_24v = (val & mask) == mask;
> +
> +	if (tx_level_24v && lp_tx_level_24v)
> +		phydev_info(phydev, "Negotiated 2.4V TX level\n");
> +	else
> +		phydev_info(phydev, "Negotiated 1.0V TX level\n");>

We have ETHTOOL_LINK_MODE_Autoneg_BIT to indicate autoneg is
supported.  Maybe we should add ETHTOOL_LINK_MODE_2400mv_BIT? Are
there other voltages defined in the standards?

> +static int adin_set_powerdown_mode(struct phy_device *phydev, bool en)
> +{
> +	int ret, timeout;
> +	u16 val;
> +
> +	if (en)
> +		val = ADIN_CRSM_SFT_PD_CNTRL_EN;
> +	else
> +		val = 0;
> +
> +	ret = phy_write_mmd(phydev, MDIO_MMD_VEND1,
> +			    ADIN_CRSM_SFT_PD_CNTRL, val);
> +	if (ret < 0)
> +		return ret;
> +
> +	timeout = 30;
> +	while (timeout-- > 0) {
> +		ret = phy_read_mmd(phydev, MDIO_MMD_VEND1,
> +				   ADIN_CRSM_STAT);
> +		if (ret < 0)
> +			return ret;
> +
> +		if ((ret & ADIN_CRSM_SFT_PD_RDY) == val)
> +			return 0;
> +
> +		mdelay(1);
> +	}
> +
> +	return -ETIMEDOUT;

We already have phy_read_poll_timeout(). Please add a
phy_read_mmd_poll_timeout() function.

> +static int adin_set_loopback(struct phy_device *phydev, bool enable)
> +{
> +	int ret;
> +
> +	if (enable)
> +		return phy_set_bits_mmd(phydev, MDIO_MMD_PCS,
> +					ADIN_B10L_PCS_CNTRL,
> +					ADIN_PCS_CNTRL_B10L_LB_PCS_EN);
> +
> +	/* MAC interface block loopback */
> +	ret = phy_clear_bits_mmd(phydev, MDIO_MMD_VEND1, ADIN_MAC_IF_LOOPBACK,
> +				 ADIN_MAC_IF_LOOPBACK_EN |
> +				 ADIN_MAC_IF_REMOTE_LOOPBACK_EN);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* PCS loopback (according to 10BASE-T1L spec) */
> +	ret = phy_clear_bits_mmd(phydev, MDIO_MMD_PCS, ADIN_B10L_PCS_CNTRL,
> +				 ADIN_PCS_CNTRL_B10L_LB_PCS_EN);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* PMA loopback (according to 10BASE-T1L spec) */
> +	return phy_clear_bits_mmd(phydev, MDIO_MMD_PMAPMD, ADIN_B10L_PMA_CNTRL,
> +				  ADIN_PMA_CNTRL_B10L_LB_PMA_LOC_EN);

Why three loopbacks at the same time. The outgoing frame it going to
hit the first loopback, and never reach the other two.

    Andrew

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/4] net: phy: adin1100: Add ethtool get_stats support
  2021-06-24 14:53 ` [PATCH 2/4] net: phy: adin1100: Add ethtool get_stats support alexandru.tachici
@ 2021-06-24 17:19   ` Andrew Lunn
  0 siblings, 0 replies; 9+ messages in thread
From: Andrew Lunn @ 2021-06-24 17:19 UTC (permalink / raw)
  To: alexandru.tachici; +Cc: netdev, linux-kernel, davem, kuba, linux

> +static u64 adin_get_stat(struct phy_device *phydev, int i)
> +{
> +	const struct adin_hw_stat *stat = &adin_hw_stats[i];
> +	struct adin_priv *priv = phydev->priv;
> +	u64 val;
> +	int ret;
> +
> +	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, stat->reg1);
> +	if (ret < 0)
> +		return (u64)(~0);

U64_MAX is more common.

	Andrew

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/4] net: phy: adin1100: Add initial support for ADIN1100 industrial PHY
  2021-06-24 14:53 ` [PATCH 1/4] " alexandru.tachici
  2021-06-24 17:15   ` Andrew Lunn
@ 2021-06-24 17:23   ` Andrew Lunn
  2021-06-24 22:26   ` kernel test robot
  2 siblings, 0 replies; 9+ messages in thread
From: Andrew Lunn @ 2021-06-24 17:23 UTC (permalink / raw)
  To: alexandru.tachici
  Cc: netdev, linux-kernel, davem, kuba, linux, Alexandru Ardelean

> +static int adin_config_init(struct phy_device *phydev)
> +{
> +	struct adin_priv *priv = phydev->priv;
> +	struct device *dev = &phydev->mdio.dev;
> +	int ret;
> +
> +	ret = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, ADIN_B10L_PMA_STAT);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* This depends on the voltage level from the power source */
> +	priv->tx_level_24v = !!(ret & ADIN_PMA_STAT_B10L_TX_LVL_HI_ABLE);
> +
> +	phydev_dbg(phydev, "PHY supports 2.4V TX level: %s\n",
> +		   priv->tx_level_24v ? "yes" : "no");
> +
> +	if (priv->tx_level_24v &&
> +	    device_property_present(dev, "adi,disable-2-4-v-tx-level")) {
> +		phydev_info(phydev,
> +			    "PHY supports 2.4V TX level, but disabled via config\n");
> +		priv->tx_level_24v = 0;

Please document this in the device tree binding. I also suspect Rob
will prefer something like adi,disable-2400mv-tx-level

       Andrew

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/4] net: phy: adin1100: Add initial support for ADIN1100 industrial PHY
  2021-06-24 14:53 ` [PATCH 1/4] " alexandru.tachici
  2021-06-24 17:15   ` Andrew Lunn
  2021-06-24 17:23   ` Andrew Lunn
@ 2021-06-24 22:26   ` kernel test robot
  2 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2021-06-24 22:26 UTC (permalink / raw)
  To: alexandru.tachici, netdev, linux-kernel
  Cc: kbuild-all, andrew, davem, kuba, linux, Alexandru Ardelean,
	Alexandru Tachici

[-- Attachment #1: Type: text/plain, Size: 2844 bytes --]

Hi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net/master]
[also build test WARNING on net-next/master ipvs/master linus/master v5.13-rc7 next-20210624]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/alexandru-tachici-analog-com/net-phy-adin1100-Add-initial-support-for-ADIN1100-industrial-PHY/20210624-224722
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git c2f5c57d99debf471a1b263cdf227e55f1364e95
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/73c3c54f7007f735d9b8bcad03423c011f7ab15f
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review alexandru-tachici-analog-com/net-phy-adin1100-Add-initial-support-for-ADIN1100-industrial-PHY/20210624-224722
        git checkout 73c3c54f7007f735d9b8bcad03423c011f7ab15f
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=sh 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/net/phy/adin1100.c:3: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
    *  Driver for Analog Devices Industrial Ethernet T1L PHYs
   drivers/net/phy/adin1100.c:79: warning: Function parameter or member 'tx_level_24v' not described in 'adin_priv'

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for SND_ATMEL_SOC_PDC
   Depends on SOUND && !UML && SND && SND_SOC && SND_ATMEL_SOC && HAS_DMA
   Selected by
   - SND_ATMEL_SOC_SSC && SOUND && !UML && SND && SND_SOC && SND_ATMEL_SOC
   - SND_ATMEL_SOC_SSC_PDC && SOUND && !UML && SND && SND_SOC && SND_ATMEL_SOC && ATMEL_SSC


vim +3 drivers/net/phy/adin1100.c

   > 3	 *  Driver for Analog Devices Industrial Ethernet T1L PHYs
     4	 *
     5	 * Copyright 2020 Analog Devices Inc.
     6	 */
     7	#include <linux/kernel.h>
     8	#include <linux/bitfield.h>
     9	#include <linux/delay.h>
    10	#include <linux/errno.h>
    11	#include <linux/init.h>
    12	#include <linux/module.h>
    13	#include <linux/mii.h>
    14	#include <linux/phy.h>
    15	#include <linux/property.h>
    16	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 54794 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-06-24 22:27 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-24 14:53 [PATCH 0/4] net: phy: adin1100: Add initial support for ADIN1100 industrial PHY alexandru.tachici
2021-06-24 14:53 ` [PATCH 1/4] " alexandru.tachici
2021-06-24 17:15   ` Andrew Lunn
2021-06-24 17:23   ` Andrew Lunn
2021-06-24 22:26   ` kernel test robot
2021-06-24 14:53 ` [PATCH 2/4] net: phy: adin1100: Add ethtool get_stats support alexandru.tachici
2021-06-24 17:19   ` Andrew Lunn
2021-06-24 14:53 ` [PATCH 3/4] net: phy: adin1100: Add ethtool master-slave support alexandru.tachici
2021-06-24 14:53 ` [PATCH 4/4] net: phy: adin1100: Add SQI support alexandru.tachici

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).