[net-next RFC PATCH v2 3/3] net: phy: Add Airoha AN8855 Internal Switch Gigabit PHY

Christian Marangi posted 3 patches 1 month ago
There is a newer version of this series
[net-next RFC PATCH v2 3/3] net: phy: Add Airoha AN8855 Internal Switch Gigabit PHY
Posted by Christian Marangi 1 month ago
Add support for Airoha AN8855 Internal Switch Gigabit PHY.

This is a simple PHY driver to configure and calibrate the PHY for the
AN8855 Switch with the use of NVMEM cells.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 MAINTAINERS                  |   1 +
 drivers/net/phy/Kconfig      |   5 +
 drivers/net/phy/Makefile     |   1 +
 drivers/net/phy/air_an8855.c | 265 +++++++++++++++++++++++++++++++++++
 4 files changed, 272 insertions(+)
 create mode 100644 drivers/net/phy/air_an8855.c

diff --git a/MAINTAINERS b/MAINTAINERS
index e3077d9feee2..cf34add2a0bb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -726,6 +726,7 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/net/dsa/airoha,an8855.yaml
 F:	drivers/net/dsa/an8855.c
 F:	drivers/net/dsa/an8855.h
+F:	drivers/net/phy/air_an8855.c
 
 AIROHA ETHERNET DRIVER
 M:	Lorenzo Bianconi <lorenzo@kernel.org>
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index ee3ea0b56d48..1d474038ea7f 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -79,6 +79,11 @@ config SFP
 
 comment "MII PHY device drivers"
 
+config AIR_AN8855_PHY
+	tristate "Airoha AN8855 Internal Gigabit PHY"
+	help
+	  Currently supports the internal Airoha AN8855 Switch PHY.
+
 config AIR_EN8811H_PHY
 	tristate "Airoha EN8811H 2.5 Gigabit PHY"
 	help
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 90f886844381..baba7894785b 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -35,6 +35,7 @@ obj-y				+= $(sfp-obj-y) $(sfp-obj-m)
 
 obj-$(CONFIG_ADIN_PHY)		+= adin.o
 obj-$(CONFIG_ADIN1100_PHY)	+= adin1100.o
+obj-$(CONFIG_AIR_AN8855_PHY)   += air_an8855.o
 obj-$(CONFIG_AIR_EN8811H_PHY)   += air_en8811h.o
 obj-$(CONFIG_AMD_PHY)		+= amd.o
 obj-$(CONFIG_AMCC_QT2025_PHY)	+= qt2025.o
diff --git a/drivers/net/phy/air_an8855.c b/drivers/net/phy/air_an8855.c
new file mode 100644
index 000000000000..c43ae7b76177
--- /dev/null
+++ b/drivers/net/phy/air_an8855.c
@@ -0,0 +1,265 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <linux/phy.h>
+#include <linux/module.h>
+#include <linux/bitfield.h>
+#include <linux/nvmem-consumer.h>
+
+#define AN8855_PHY_PAGE_CTRL			0x1f
+#define   AN8855_PHY_NORMAL_PAGE		0x0
+#define   AN8855_PHY_EXT_PAGE			0x1
+
+#define AN8855_PHY_EXT_REG_14			0x14
+#define   AN8855_PHY_EN_DOWN_SHFIT		BIT(4)
+
+/* R50 Calibration regs in MDIO_MMD_VEND1 */
+#define AN8855_PHY_R500HM_RSEL_TX_AB		0x174
+#define AN8855_PHY_R50OHM_RSEL_TX_A_EN		BIT(15)
+#define AN8855_PHY_R50OHM_RSEL_TX_A		GENMASK(14, 8)
+#define AN8855_PHY_R50OHM_RSEL_TX_B_EN		BIT(7)
+#define AN8855_PHY_R50OHM_RSEL_TX_B		GENMASK(6, 0)
+#define AN8855_PHY_R500HM_RSEL_TX_CD		0x175
+#define AN8855_PHY_R50OHM_RSEL_TX_C_EN		BIT(15)
+#define AN8855_PHY_R50OHM_RSEL_TX_C		GENMASK(14, 8)
+#define AN8855_PHY_R50OHM_RSEL_TX_D_EN		BIT(7)
+#define AN8855_PHY_R50OHM_RSEL_TX_D		GENMASK(6, 0)
+
+#define AN8855_SWITCH_EFUSE_R50O		GENMASK(30, 24)
+
+/* PHY TX PAIR DELAY SELECT Register */
+#define PHY_TX_PAIR_DLY_SEL_GBE			0x013
+/* PHY ADC Register */
+#define PHY_RXADC_CTRL				0x0d8
+#define PHY_RXADC_REV_0				0x0d9
+#define PHY_RXADC_REV_1				0x0da
+
+#define AN8855_PHY_ID			0xc0ff0410
+
+struct air_an8855_priv {
+	u8 calibration_data[4];
+};
+
+static const u8 dsa_r50ohm_table[] = {
+	127, 127, 127, 127, 127, 127, 127, 127, 127, 127,
+	127, 127, 127, 127, 127, 127, 127, 126, 122, 117,
+	112, 109, 104, 101,  97,  94,  90,  88,  84,  80,
+	78,  74,  72,  68,  66,  64,  61,  58,  56,  53,
+	51,  48,  47,  44,  42,  40,  38,  36,  34,  32,
+	31,  28,  27,  24,  24,  22,  20,  18,  16,  16,
+	14,  12,  11,   9
+};
+
+static int en8855_get_r50ohm_val(struct device *dev, const char *calib_name,
+				 u8 *dest)
+{
+	u32 shift_sel, val;
+	int ret;
+	int i;
+
+	ret = nvmem_cell_read_u32(dev, calib_name, &val);
+	if (ret)
+		return ret;
+
+	shift_sel = FIELD_GET(AN8855_SWITCH_EFUSE_R50O, val);
+	for (i = 0; i < ARRAY_SIZE(dsa_r50ohm_table); i++)
+		if (dsa_r50ohm_table[i] == shift_sel)
+			break;
+
+	if (i < 8 || i >= ARRAY_SIZE(dsa_r50ohm_table))
+		*dest = dsa_r50ohm_table[25];
+	else
+		*dest = dsa_r50ohm_table[i - 8];
+
+	return 0;
+}
+
+static int an8855_probe(struct phy_device *phydev)
+{
+	struct device *dev = &phydev->mdio.dev;
+	struct device_node *node = dev->of_node;
+	struct air_an8855_priv *priv;
+	int ret;
+
+	/* If we don't have a node, skip get calib */
+	if (!node)
+		return 0;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	ret = en8855_get_r50ohm_val(dev, "tx_a", &priv->calibration_data[0]);
+	if (ret)
+		return ret;
+
+	ret = en8855_get_r50ohm_val(dev, "tx_b", &priv->calibration_data[1]);
+	if (ret)
+		return ret;
+
+	ret = en8855_get_r50ohm_val(dev, "tx_c", &priv->calibration_data[2]);
+	if (ret)
+		return ret;
+
+	ret = en8855_get_r50ohm_val(dev, "tx_d", &priv->calibration_data[3]);
+	if (ret)
+		return ret;
+
+	phydev->priv = priv;
+
+	return 0;
+}
+
+static int an8855_config_init(struct phy_device *phydev)
+{
+	struct air_an8855_priv *priv = phydev->priv;
+	int ret;
+
+	/* Enable HW auto downshift */
+	ret = phy_write(phydev, AN8855_PHY_PAGE_CTRL, AN8855_PHY_EXT_PAGE);
+	if (ret)
+		return ret;
+	ret = phy_set_bits(phydev, AN8855_PHY_EXT_REG_14,
+			   AN8855_PHY_EN_DOWN_SHFIT);
+	if (ret)
+		return ret;
+	ret = phy_write(phydev, AN8855_PHY_PAGE_CTRL, AN8855_PHY_NORMAL_PAGE);
+	if (ret)
+		return ret;
+
+	/* Enable Asymmetric Pause Capability */
+	ret = phy_set_bits(phydev, MII_ADVERTISE, ADVERTISE_PAUSE_ASYM);
+	if (ret)
+		return ret;
+
+	/* Disable EEE */
+	ret = phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0);
+	if (ret)
+		return ret;
+
+	/* Apply calibration values, if needed. BIT(0) signal this */
+	if (phydev->dev_flags & BIT(0)) {
+		u8 *calibration_data = priv->calibration_data;
+
+		ret = phy_modify_mmd(phydev, MDIO_MMD_VEND1, AN8855_PHY_R500HM_RSEL_TX_AB,
+				     AN8855_PHY_R50OHM_RSEL_TX_A | AN8855_PHY_R50OHM_RSEL_TX_B,
+				     FIELD_PREP(AN8855_PHY_R50OHM_RSEL_TX_A, calibration_data[0]) |
+				     FIELD_PREP(AN8855_PHY_R50OHM_RSEL_TX_B, calibration_data[1]));
+		if (ret)
+			return ret;
+		ret = phy_modify_mmd(phydev, MDIO_MMD_VEND1, AN8855_PHY_R500HM_RSEL_TX_CD,
+				     AN8855_PHY_R50OHM_RSEL_TX_C | AN8855_PHY_R50OHM_RSEL_TX_D,
+				     FIELD_PREP(AN8855_PHY_R50OHM_RSEL_TX_C, calibration_data[2]) |
+				     FIELD_PREP(AN8855_PHY_R50OHM_RSEL_TX_D, calibration_data[3]));
+		if (ret)
+			return ret;
+	}
+
+	/* Apply values to decude signal noise */
+	ret = phy_write_mmd(phydev, MDIO_MMD_VEND1, PHY_TX_PAIR_DLY_SEL_GBE, 0x4040);
+	if (ret)
+		return ret;
+	ret = phy_write_mmd(phydev, MDIO_MMD_VEND1, PHY_RXADC_CTRL, 0x1010);
+	if (ret)
+		return ret;
+	ret = phy_write_mmd(phydev, MDIO_MMD_VEND1, PHY_RXADC_REV_0, 0x100);
+	if (ret)
+		return ret;
+	ret = phy_write_mmd(phydev, MDIO_MMD_VEND1, PHY_RXADC_REV_1, 0x100);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int an8855_get_downshift(struct phy_device *phydev, u8 *data)
+{
+	int val;
+	int ret;
+
+	ret = phy_write(phydev, AN8855_PHY_PAGE_CTRL, AN8855_PHY_EXT_PAGE);
+	if (ret)
+		return ret;
+
+	val = phy_read(phydev, AN8855_PHY_EXT_REG_14);
+	*data = val & AN8855_PHY_EXT_REG_14 ? DOWNSHIFT_DEV_DEFAULT_COUNT :
+					      DOWNSHIFT_DEV_DISABLE;
+
+	ret = phy_write(phydev, AN8855_PHY_PAGE_CTRL, AN8855_PHY_NORMAL_PAGE);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int an8855_set_downshift(struct phy_device *phydev, u8 cnt)
+{
+	int ret;
+
+	ret = phy_write(phydev, AN8855_PHY_PAGE_CTRL, AN8855_PHY_EXT_PAGE);
+	if (ret)
+		return ret;
+
+	if (cnt != DOWNSHIFT_DEV_DISABLE) {
+		ret = phy_set_bits(phydev, AN8855_PHY_EXT_REG_14,
+				   AN8855_PHY_EN_DOWN_SHFIT);
+		if (ret)
+			return ret;
+	} else {
+		ret = phy_clear_bits(phydev, AN8855_PHY_EXT_REG_14,
+				     AN8855_PHY_EN_DOWN_SHFIT);
+		if (ret)
+			return ret;
+	}
+
+	return phy_write(phydev, AN8855_PHY_PAGE_CTRL, AN8855_PHY_NORMAL_PAGE);
+}
+
+static int an8855_get_tunable(struct phy_device *phydev,
+			      struct ethtool_tunable *tuna, void *data)
+{
+	switch (tuna->id) {
+	case ETHTOOL_PHY_DOWNSHIFT:
+		return an8855_get_downshift(phydev, data);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int an8855_set_tunable(struct phy_device *phydev,
+			      struct ethtool_tunable *tuna, const void *data)
+{
+	switch (tuna->id) {
+	case ETHTOOL_PHY_DOWNSHIFT:
+		return an8855_set_downshift(phydev, *(const u8 *)data);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static struct phy_driver an8855_driver[] = {
+{
+	PHY_ID_MATCH_EXACT(AN8855_PHY_ID),
+	.name			= "Airoha AN8855 internal PHY",
+	/* PHY_GBIT_FEATURES */
+	.flags			= PHY_IS_INTERNAL,
+	.probe			= an8855_probe,
+	.config_init		= an8855_config_init,
+	.soft_reset		= genphy_soft_reset,
+	.get_tunable		= an8855_get_tunable,
+	.set_tunable		= an8855_set_tunable,
+	.suspend		= genphy_suspend,
+	.resume			= genphy_resume,
+}, };
+
+module_phy_driver(an8855_driver);
+
+static struct mdio_device_id __maybe_unused an8855_tbl[] = {
+	{ PHY_ID_MATCH_EXACT(AN8855_PHY_ID) },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(mdio, an8855_tbl);
+
+MODULE_DESCRIPTION("Airoha AN8855 PHY driver");
+MODULE_AUTHOR("Christian Marangi <ansuelsmth@gmail.com>");
+MODULE_LICENSE("GPL");
-- 
2.45.2
Re: [net-next RFC PATCH v2 3/3] net: phy: Add Airoha AN8855 Internal Switch Gigabit PHY
Posted by Andrew Lunn 1 month ago
> +static int an8855_config_init(struct phy_device *phydev)
> +{
> +	struct air_an8855_priv *priv = phydev->priv;
> +	int ret;
> +
> +	/* Enable HW auto downshift */
> +	ret = phy_write(phydev, AN8855_PHY_PAGE_CTRL, AN8855_PHY_EXT_PAGE);
> +	if (ret)
> +		return ret;
> +	ret = phy_set_bits(phydev, AN8855_PHY_EXT_REG_14,
> +			   AN8855_PHY_EN_DOWN_SHFIT);
> +	if (ret)
> +		return ret;
> +	ret = phy_write(phydev, AN8855_PHY_PAGE_CTRL, AN8855_PHY_NORMAL_PAGE);
> +	if (ret)
> +		return ret;

There are locking issues here, which is why we have the helpers
phy_select_page() and phy_restore_page(). The air_en8811h.c gets this
right.

Is there anything in common with the en8811h? Does it also support
downshift? Can its LED code be used here?

	   Andrew
Re: [net-next RFC PATCH v2 3/3] net: phy: Add Airoha AN8855 Internal Switch Gigabit PHY
Posted by Christian Marangi 1 month ago
On Wed, Oct 23, 2024 at 07:00:22PM +0200, Andrew Lunn wrote:
> > +static int an8855_config_init(struct phy_device *phydev)
> > +{
> > +	struct air_an8855_priv *priv = phydev->priv;
> > +	int ret;
> > +
> > +	/* Enable HW auto downshift */
> > +	ret = phy_write(phydev, AN8855_PHY_PAGE_CTRL, AN8855_PHY_EXT_PAGE);
> > +	if (ret)
> > +		return ret;
> > +	ret = phy_set_bits(phydev, AN8855_PHY_EXT_REG_14,
> > +			   AN8855_PHY_EN_DOWN_SHFIT);
> > +	if (ret)
> > +		return ret;
> > +	ret = phy_write(phydev, AN8855_PHY_PAGE_CTRL, AN8855_PHY_NORMAL_PAGE);
> > +	if (ret)
> > +		return ret;
> 
> There are locking issues here, which is why we have the helpers
> phy_select_page() and phy_restore_page(). The air_en8811h.c gets this
> right.

Ugh didn't think about it... The switch address is shared with the PHY
so yes this is a problem.

Consider that this page thing comes from my speculation... Not really
use if 1f select page... 
From what I observed
0x0 PHY page
0x1 this strange EXT
0x4 acess switch register (every PHY can access the switch)

> 
> Is there anything in common with the en8811h? Does it also support
> downshift? Can its LED code be used here?
> 

For some reason part of the LED are controlled by the switch and some
are by the PHY. I still have to investigate that (not giving priority to
it... just on my todo)

For downshift as you notice it's a single bit with no count...
From their comments in the original driver it's said "Enable HW
autodownshift"

Trying to reach them but currently it's all very obscure.

-- 
	Ansuel
Re: [net-next RFC PATCH v2 3/3] net: phy: Add Airoha AN8855 Internal Switch Gigabit PHY
Posted by Christian Marangi 1 month ago
On Wed, Oct 23, 2024 at 07:07:08PM +0200, Christian Marangi wrote:
> On Wed, Oct 23, 2024 at 07:00:22PM +0200, Andrew Lunn wrote:
> > > +static int an8855_config_init(struct phy_device *phydev)
> > > +{
> > > +	struct air_an8855_priv *priv = phydev->priv;
> > > +	int ret;
> > > +
> > > +	/* Enable HW auto downshift */
> > > +	ret = phy_write(phydev, AN8855_PHY_PAGE_CTRL, AN8855_PHY_EXT_PAGE);
> > > +	if (ret)
> > > +		return ret;
> > > +	ret = phy_set_bits(phydev, AN8855_PHY_EXT_REG_14,
> > > +			   AN8855_PHY_EN_DOWN_SHFIT);
> > > +	if (ret)
> > > +		return ret;
> > > +	ret = phy_write(phydev, AN8855_PHY_PAGE_CTRL, AN8855_PHY_NORMAL_PAGE);
> > > +	if (ret)
> > > +		return ret;
> > 
> > There are locking issues here, which is why we have the helpers
> > phy_select_page() and phy_restore_page(). The air_en8811h.c gets this
> > right.
> 
> Ugh didn't think about it... The switch address is shared with the PHY
> so yes this is a problem.
> 
> Consider that this page thing comes from my speculation... Not really
> use if 1f select page... 
> From what I observed
> 0x0 PHY page
> 0x1 this strange EXT
> 0x4 acess switch register (every PHY can access the switch)
>

Just to followup on this... I checked air_en8811h registers again and
they match MII access to the switch so yes my speculation is correct.

Also extra happy since I now know what those magic values means at least
for MII.

> > 
> > Is there anything in common with the en8811h? Does it also support
> > downshift? Can its LED code be used here?
> > 
> 
> For some reason part of the LED are controlled by the switch and some
> are by the PHY. I still have to investigate that (not giving priority to
> it... just on my todo)
> 
> For downshift as you notice it's a single bit with no count...
> From their comments in the original driver it's said "Enable HW
> autodownshift"
> 
> Trying to reach them but currently it's all very obscure.
> 
> -- 
> 	Ansuel

-- 
	Ansuel
Re: [net-next RFC PATCH v2 3/3] net: phy: Add Airoha AN8855 Internal Switch Gigabit PHY
Posted by Andrew Lunn 1 month ago
> +	/* Enable Asymmetric Pause Capability */
> +	ret = phy_set_bits(phydev, MII_ADVERTISE, ADVERTISE_PAUSE_ASYM);
> +	if (ret)
> +		return ret;

The PHY driver alone does not decide this. The MAC driver needs to
indicate it supports asym pause by calling phy_supports_asym_pause().

> +
> +	/* Disable EEE */
> +	ret = phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0);
> +	if (ret)
> +		return ret;

Again, the core code should handle this, unless EEE is broken and you
need to force it off.

	Andrew
Re: [net-next RFC PATCH v2 3/3] net: phy: Add Airoha AN8855 Internal Switch Gigabit PHY
Posted by Christian Marangi 1 month ago
On Wed, Oct 23, 2024 at 06:53:14PM +0200, Andrew Lunn wrote:
> > +	/* Enable Asymmetric Pause Capability */
> > +	ret = phy_set_bits(phydev, MII_ADVERTISE, ADVERTISE_PAUSE_ASYM);
> > +	if (ret)
> > +		return ret;
> 
> The PHY driver alone does not decide this. The MAC driver needs to
> indicate it supports asym pause by calling phy_supports_asym_pause().
>

Sorry for the stupid question, I couldn't find this OPs. Any hit how to
handle this?

> > +
> > +	/* Disable EEE */
> > +	ret = phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0);
> > +	if (ret)
> > +		return ret;
> 
> Again, the core code should handle this, unless EEE is broken and you
> need to force it off.
>

They confirmed this was done just to handle kernel init condition...
Will drop.

-- 
	Ansuel
Re: [net-next RFC PATCH v2 3/3] net: phy: Add Airoha AN8855 Internal Switch Gigabit PHY
Posted by Andrew Lunn 1 month ago
On Fri, Oct 25, 2024 at 12:59:54PM +0200, Christian Marangi wrote:
> On Wed, Oct 23, 2024 at 06:53:14PM +0200, Andrew Lunn wrote:
> > > +	/* Enable Asymmetric Pause Capability */
> > > +	ret = phy_set_bits(phydev, MII_ADVERTISE, ADVERTISE_PAUSE_ASYM);
> > > +	if (ret)
> > > +		return ret;
> > 
> > The PHY driver alone does not decide this. The MAC driver needs to
> > indicate it supports asym pause by calling phy_supports_asym_pause().
> >
> 
> Sorry for the stupid question, I couldn't find this OPs. Any hit how to
> handle this?

For phylink, set MAC_ASYM_PAUSE | MAC_SYM_PAUSE in the capabilities in
phylink_config.

For phylib, call phy_support_asym_pause(phydev) after connecting to the PHY.

	Andrew