All of lore.kernel.org
 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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 22:26     ` kernel test robot
  2021-06-24 17:23   ` Andrew Lunn
  2021-06-24 22:26     ` kernel test robot
  2 siblings, 0 replies; 10+ 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] 10+ messages in thread

* Re: [PATCH 1/4] net: phy: adin1100: Add initial support for ADIN1100 industrial PHY
@ 2021-06-24 22:26     ` kernel test robot
  0 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2021-06-24 22:26 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2907 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(a)lists.01.org

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

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

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

Thread overview: 10+ 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 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.