diff mbox

[v3,01/11] net: phy: Add rockchip phy driver support

Message ID 1501654546-17292-2-git-send-email-david.wu@rock-chips.com (mailing list archive)
State New, archived
Headers show

Commit Message

David Wu Aug. 2, 2017, 6:15 a.m. UTC
Support internal ethernet phy currently.

Signed-off-by: David Wu <david.wu@rock-chips.com>
---
 drivers/net/phy/Kconfig    |   5 +
 drivers/net/phy/Makefile   |   1 +
 drivers/net/phy/rockchip.c | 229 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 235 insertions(+)
 create mode 100644 drivers/net/phy/rockchip.c

Comments

Corentin Labbe Aug. 2, 2017, 6:20 a.m. UTC | #1
Hello I have some minor comment below

> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mii.h>
> +#include <linux/ethtool.h>
> +#include <linux/phy.h>
> +#include <linux/netdevice.h>

in alphabetic order please

[...]
> +static int rockchip_init_tstmode(struct phy_device *phydev)
> +{
> +	int ret;
> +
> +	/* Enable access to Analog and DSP register banks */
> +	ret = phy_write(phydev, SMI_ADDR_TSTCNTL, 0x0400);
> +	if (ret)
> +		return ret;
> +
> +	ret = phy_write(phydev, SMI_ADDR_TSTCNTL, 0x0000);
> +	if (ret)
> +		return ret;
> +
> +	return phy_write(phydev, SMI_ADDR_TSTCNTL, 0x0400);
> +}
> +
> +static int rockchip_close_tstmode(struct phy_device *phydev)
> +{
> +	/* Back to basic register bank */
> +	return phy_write(phydev, SMI_ADDR_TSTCNTL, 0x0000);

The reuse of 0x0000 and 0x0400 seems to promote a define use

[...]
> +static struct phy_driver rockchip_phy_driver[] = {
> +{
> +	.phy_id			= 0x1234d400,
> +	.phy_id_mask		= 0xfffffff0,
> +	.name			= "Rockchip internal EPHY",
> +	.features		= (PHY_BASIC_FEATURES | SUPPORTED_Pause
> +				   | SUPPORTED_Asym_Pause),
> +	.flags			= PHY_IS_INTERNAL,
> +	.link_change_notify	= rockchip_link_change_notify,
> +	.soft_reset		= genphy_soft_reset,
> +	.config_init		= rockchip_internal_phy_config_init,
> +	.config_aneg		= rockchip_config_aneg,
> +	.read_status		= genphy_read_status,
> +	.suspend		= genphy_suspend,
> +	.resume			= rockchip_phy_resume,
> +},
> +};
> +
> +module_phy_driver(rockchip_phy_driver);
> +
> +static struct mdio_device_id __maybe_unused rockchip_phy_tbl[] = {
> +	{ 0x1234d400, 0xfffffff0 },

Same comment for phy_id, use a define

Regards
Corentin Labbe
Andrew Lunn Aug. 2, 2017, 1:21 p.m. UTC | #2
> +static struct phy_driver rockchip_phy_driver[] = {
> +{
> +	.phy_id			= 0x1234d400,
> +	.phy_id_mask		= 0xfffffff0,
> +	.name			= "Rockchip internal EPHY",
> +	.features		= (PHY_BASIC_FEATURES | SUPPORTED_Pause
> +				   | SUPPORTED_Asym_Pause),

Please take a look at Documentation/networking/phy.txt and
Fixes: 529ed1275263 ("net: phy: phy drivers should not set SUPPORTED_[Asym_]Pause")

Pause frames / flow control

 The PHY does not participate directly in flow control/pause frames except by
 making sure that the SUPPORTED_Pause and SUPPORTED_AsymPause bits are set in
 MII_ADVERTISE to indicate towards the link partner that the Ethernet MAC
 controller supports such a thing. Since flow control/pause frames generation
 involves the Ethernet MAC driver, it is recommended that this driver takes care
 of properly indicating advertisement and support for such features by setting
 the SUPPORTED_Pause and SUPPORTED_AsymPause bits accordingly. This can be done
 either before or after phy_connect() and/or as a result of implementing the
 ethtool::set_pauseparam feature.

	Andrew
David Wu Aug. 9, 2017, 7:01 a.m. UTC | #3
Hi Andrew,

在 2017/8/2 21:21, Andrew Lunn 写道:
>> +static struct phy_driver rockchip_phy_driver[] = {
>> +{
>> +	.phy_id			= 0x1234d400,
>> +	.phy_id_mask		= 0xfffffff0,
>> +	.name			= "Rockchip internal EPHY",
>> +	.features		= (PHY_BASIC_FEATURES | SUPPORTED_Pause
>> +				   | SUPPORTED_Asym_Pause),
> 
> Please take a look at Documentation/networking/phy.txt and
> Fixes: 529ed1275263 ("net: phy: phy drivers should not set SUPPORTED_[Asym_]Pause")
> 
> Pause frames / flow control
> 
>   The PHY does not participate directly in flow control/pause frames except by
>   making sure that the SUPPORTED_Pause and SUPPORTED_AsymPause bits are set in
>   MII_ADVERTISE to indicate towards the link partner that the Ethernet MAC
>   controller supports such a thing. Since flow control/pause frames generation
>   involves the Ethernet MAC driver, it is recommended that this driver takes care
>   of properly indicating advertisement and support for such features by setting
>   the SUPPORTED_Pause and SUPPORTED_AsymPause bits accordingly. This can be done
>   either before or after phy_connect() and/or as a result of implementing the
>   ethtool::set_pauseparam feature.
> 

Thanks for the reminder, I'll remove it.

> 	Andrew
> 
> 
>
diff mbox

Patch

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 2dda720..22cc702 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -334,6 +334,11 @@  config REALTEK_PHY
 	---help---
 	  Supports the Realtek 821x PHY.
 
+config ROCKCHIP_PHY
+        tristate "Driver for Rockchip Ethernet PHYs"
+        ---help---
+          Currently supports the internal Ethernet PHY.
+
 config SMSC_PHY
 	tristate "SMSC PHYs"
 	---help---
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 8e9b9f3..350520e 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -66,6 +66,7 @@  obj-$(CONFIG_MICROSEMI_PHY)	+= mscc.o
 obj-$(CONFIG_NATIONAL_PHY)	+= national.o
 obj-$(CONFIG_QSEMI_PHY)		+= qsemi.o
 obj-$(CONFIG_REALTEK_PHY)	+= realtek.o
+obj-$(CONFIG_ROCKCHIP_PHY)	+= rockchip.o
 obj-$(CONFIG_SMSC_PHY)		+= smsc.o
 obj-$(CONFIG_STE10XP)		+= ste10Xp.o
 obj-$(CONFIG_TERANETICS_PHY)	+= teranetics.o
diff --git a/drivers/net/phy/rockchip.c b/drivers/net/phy/rockchip.c
new file mode 100644
index 0000000..c1f07d6
--- /dev/null
+++ b/drivers/net/phy/rockchip.c
@@ -0,0 +1,229 @@ 
+/**
+ * drivers/net/phy/rockchip.c
+ *
+ * Driver for ROCKCHIP Ethernet PHYs
+ *
+ * Copyright (c) 2017, Fuzhou Rockchip Electronics Co., Ltd
+ *
+ * David Wu <david.wu@rock-chips.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mii.h>
+#include <linux/ethtool.h>
+#include <linux/phy.h>
+#include <linux/netdevice.h>
+
+#define MII_INTERNAL_CTRL_STATUS	17
+#define SMI_ADDR_TSTCNTL		20
+#define SMI_ADDR_TSTREAD1		21
+#define SMI_ADDR_TSTREAD2		22
+#define SMI_ADDR_TSTWRITE		23
+#define MII_SPECIAL_CONTROL_STATUS	31
+
+#define MII_AUTO_MDIX_EN		BIT(7)
+#define MII_MDIX_EN			BIT(6)
+
+#define MII_SPEED_10			BIT(2)
+#define MII_SPEED_100			BIT(3)
+
+#define TSTCNTL_RD			(BIT(15) | BIT(10))
+#define TSTCNTL_WR			(BIT(14) | BIT(10))
+
+#define WR_ADDR_A7CFG			0x18
+
+static int rockchip_init_tstmode(struct phy_device *phydev)
+{
+	int ret;
+
+	/* Enable access to Analog and DSP register banks */
+	ret = phy_write(phydev, SMI_ADDR_TSTCNTL, 0x0400);
+	if (ret)
+		return ret;
+
+	ret = phy_write(phydev, SMI_ADDR_TSTCNTL, 0x0000);
+	if (ret)
+		return ret;
+
+	return phy_write(phydev, SMI_ADDR_TSTCNTL, 0x0400);
+}
+
+static int rockchip_close_tstmode(struct phy_device *phydev)
+{
+	/* Back to basic register bank */
+	return phy_write(phydev, SMI_ADDR_TSTCNTL, 0x0000);
+}
+
+static int rockchip_internal_phy_analog_init(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = rockchip_init_tstmode(phydev);
+	if (ret)
+		return ret;
+
+	/*
+	 * Adjust tx amplitude to make sginal better,
+	 * the default value is 0x8.
+	 */
+	ret = phy_write(phydev, SMI_ADDR_TSTWRITE, 0xB);
+	if (ret)
+		return ret;
+	ret = phy_write(phydev, SMI_ADDR_TSTCNTL, TSTCNTL_WR | WR_ADDR_A7CFG);
+	if (ret)
+		return ret;
+
+	return rockchip_close_tstmode(phydev);
+}
+
+static int rockchip_internal_phy_config_init(struct phy_device *phydev)
+{
+	int val, ret;
+
+	/*
+	 * The auto MIDX has linked problem on some board,
+	 * workround to disable auto MDIX.
+	 */
+	val = phy_read(phydev, MII_INTERNAL_CTRL_STATUS);
+	if (val < 0)
+		return val;
+	val &= ~MII_AUTO_MDIX_EN;
+	ret = phy_write(phydev, MII_INTERNAL_CTRL_STATUS, val);
+	if (ret)
+		return ret;
+
+	return rockchip_internal_phy_analog_init(phydev);
+}
+
+static void rockchip_link_change_notify(struct phy_device *phydev)
+{
+	int speed = SPEED_10;
+
+	if (phydev->autoneg == AUTONEG_ENABLE) {
+		int reg = phy_read(phydev, MII_SPECIAL_CONTROL_STATUS);
+
+		if (reg < 0) {
+			phydev_err(phydev, "phy_read err: %d.\n", reg);
+			return;
+		}
+
+		if (reg & MII_SPEED_100)
+			speed = SPEED_100;
+		else if (reg & MII_SPEED_10)
+			speed = SPEED_10;
+	} else {
+		int bmcr = phy_read(phydev, MII_BMCR);
+
+		if (bmcr < 0) {
+			phydev_err(phydev, "phy_read err: %d.\n", bmcr);
+			return;
+		}
+
+		if (bmcr & BMCR_SPEED100)
+			speed = SPEED_100;
+		else
+			speed = SPEED_10;
+	}
+
+	/*
+	 * If mode switch happens from 10BT to 100BT, all DSP/AFE
+	 * registers are set to default values. So any AFE/DSP
+	 * registers have to be re-initialized in this case.
+	 */
+	if ((phydev->speed == SPEED_10) && (speed == SPEED_100)) {
+		int ret = rockchip_internal_phy_analog_init(phydev);
+		if (ret)
+			phydev_err(phydev, "rockchip_internal_phy_analog_init err: %d.\n",
+				   ret);
+	}
+}
+
+static int rockchip_set_polarity(struct phy_device *phydev, int polarity)
+{
+	int reg, err, val;
+
+	/* get the current settings */
+	reg = phy_read(phydev, MII_INTERNAL_CTRL_STATUS);
+	if (reg < 0)
+		return reg;
+
+	reg &= ~MII_AUTO_MDIX_EN;
+	val = reg;
+	switch (polarity) {
+	case ETH_TP_MDI:
+		val &= ~MII_MDIX_EN;
+		break;
+	case ETH_TP_MDI_X:
+		val |= MII_MDIX_EN;
+		break;
+	case ETH_TP_MDI_AUTO:
+	case ETH_TP_MDI_INVALID:
+	default:
+		return 0;
+	}
+
+	if (val != reg) {
+		/* Set the new polarity value in the register */
+		err = phy_write(phydev, MII_INTERNAL_CTRL_STATUS, val);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
+static int rockchip_config_aneg(struct phy_device *phydev)
+{
+	int err;
+
+	err = rockchip_set_polarity(phydev, phydev->mdix);
+	if (err < 0)
+		return err;
+
+	return genphy_config_aneg(phydev);
+}
+
+static int rockchip_phy_resume(struct phy_device *phydev)
+{
+	genphy_resume(phydev);
+
+	return rockchip_internal_phy_config_init(phydev);
+}
+
+static struct phy_driver rockchip_phy_driver[] = {
+{
+	.phy_id			= 0x1234d400,
+	.phy_id_mask		= 0xfffffff0,
+	.name			= "Rockchip internal EPHY",
+	.features		= (PHY_BASIC_FEATURES | SUPPORTED_Pause
+				   | SUPPORTED_Asym_Pause),
+	.flags			= PHY_IS_INTERNAL,
+	.link_change_notify	= rockchip_link_change_notify,
+	.soft_reset		= genphy_soft_reset,
+	.config_init		= rockchip_internal_phy_config_init,
+	.config_aneg		= rockchip_config_aneg,
+	.read_status		= genphy_read_status,
+	.suspend		= genphy_suspend,
+	.resume			= rockchip_phy_resume,
+},
+};
+
+module_phy_driver(rockchip_phy_driver);
+
+static struct mdio_device_id __maybe_unused rockchip_phy_tbl[] = {
+	{ 0x1234d400, 0xfffffff0 },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(mdio, rockchip_phy_tbl);
+
+MODULE_AUTHOR("David Wu <david.wu@rock-chips.com>");
+MODULE_DESCRIPTION("Rockchip Ethernet PHY driver");
+MODULE_LICENSE("GPL v2");