diff mbox series

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

Message ID 20241023161958.12056-4-ansuelsmth@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: dsa: Add Airoha AN8855 support | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next, async
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 5 this patch: 5
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 12 of 12 maintainers
netdev/build_clang success Errors and warnings before: 4 this patch: 4
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 8 this patch: 8
netdev/checkpatch warning WARNING: line length of 85 exceeds 80 columns WARNING: line length of 90 exceeds 80 columns WARNING: line length of 95 exceeds 80 columns WARNING: line length of 99 exceeds 80 columns WARNING: please write a help paragraph that fully describes the config symbol
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Christian Marangi Oct. 23, 2024, 4:19 p.m. UTC
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

Comments

Andrew Lunn Oct. 23, 2024, 4:53 p.m. UTC | #1
> +	/* 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
Andrew Lunn Oct. 23, 2024, 5 p.m. UTC | #2
> +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
Christian Marangi Oct. 23, 2024, 5:07 p.m. UTC | #3
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.
Christian Marangi Oct. 25, 2024, 10:59 a.m. UTC | #4
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.
Christian Marangi Oct. 25, 2024, 11:01 a.m. UTC | #5
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
Andrew Lunn Oct. 25, 2024, 12:56 p.m. UTC | #6
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
diff mbox series

Patch

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");