diff mbox series

[net-next,v4,4/4] net: phy: dp83td510: Add support for the DP83TD510 Ethernet PHY

Message ID 20201117201555.26723-5-dmurphy@ti.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series DP83TD510 Single Pair 10Mbps Ethernet PHY | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit fail Errors and warnings before: 0 this patch: 1
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch warning WARNING: ENOTSUPP is not a SUSV4 error code, prefer EOPNOTSUPP WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? WARNING: line length of 81 exceeds 80 columns WARNING: line length of 84 exceeds 80 columns WARNING: line length of 85 exceeds 80 columns WARNING: line length of 86 exceeds 80 columns WARNING: please write a paragraph that describes the config symbol fully
netdev/build_allmodconfig_warn fail Errors and warnings before: 0 this patch: 1
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

Dan Murphy Nov. 17, 2020, 8:15 p.m. UTC
The DP83TD510E is an ultra-low power Ethernet physical layer transceiver
that supports 10M single pair cable.

The device supports both 2.4-V p2p and 1-V p2p output voltage as defined
by IEEE 802.3cg 10Base-T1L specfications. These modes can be forced via
the device tree or the device is defaulted to auto negotiation to
determine the proper p2p voltage.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---

v4 - Considerable rework of the code after secondary test setup was created.
This version also uses the handle_interrupt call back and reduces the
configuration arrays as it was determined that 80% of the array was the same.

 drivers/net/phy/Kconfig     |   6 +
 drivers/net/phy/Makefile    |   1 +
 drivers/net/phy/dp83td510.c | 505 ++++++++++++++++++++++++++++++++++++
 3 files changed, 512 insertions(+)
 create mode 100644 drivers/net/phy/dp83td510.c

Comments

Ioana Ciornei Nov. 17, 2020, 8:57 p.m. UTC | #1
On Tue, Nov 17, 2020 at 02:15:55PM -0600, Dan Murphy wrote:
> The DP83TD510E is an ultra-low power Ethernet physical layer transceiver
> that supports 10M single pair cable.
> 
> The device supports both 2.4-V p2p and 1-V p2p output voltage as defined
> by IEEE 802.3cg 10Base-T1L specfications. These modes can be forced via
> the device tree or the device is defaulted to auto negotiation to
> determine the proper p2p voltage.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> 
> v4 - Considerable rework of the code after secondary test setup was created.
> This version also uses the handle_interrupt call back and reduces the
> configuration arrays as it was determined that 80% of the array was the same.
> 
>  drivers/net/phy/Kconfig     |   6 +
>  drivers/net/phy/Makefile    |   1 +
>  drivers/net/phy/dp83td510.c | 505 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 512 insertions(+)
>  create mode 100644 drivers/net/phy/dp83td510.c
> 

[snip]

> +static int dp83td510_ack_interrupt(struct phy_device *phydev)
> +{
> +	int ret;
> +
> +	ret = phy_read(phydev, DP83TD510_INT_REG1);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = phy_read(phydev, DP83TD510_INT_REG2);
> +	if (ret < 0)
> +		return ret;
> +
> +	phy_trigger_machine(phydev);
> +
> +	return 0;
> +}
> +
> +static irqreturn_t dp83td510_handle_interrupt(struct phy_device *phydev)
> +{
> +	int ret;
> +
> +	ret = dp83td510_ack_interrupt(phydev);
> +	if (ret)
> +		return IRQ_NONE;
> +
> +	return IRQ_HANDLED;
> +}

From what I can see in the datasheet, the INT_REG1 and INT_REG2 are used
for both interrupt configuration and interrupt status.

If this is the case, the state machine should only be triggered if the
interrupt was triggered (eg DP83TD510_INT1_LINK is set), not if _any_
bit from the register is set. This is broken since anytime you have
interrupts enabled, the lower half of the register will be non-zero
since that contains you interrupt enabled bits.

The .handle_interrupt() should look something like:

	ret = phy_read(phydev, DP83TD510_INT_REG1);
	if (ret < 0)
		return ret;
	
	if (!(ret & (DP83TD510_INT1_ESD | DP83TD510_INT1_LINK | DP83TD510_INT1_RHF)))
		return IRQ_NONE;

	ret = phy_read(phydev, DP83TD510_INT_REG2);
	if (ret < 0)
		return ret;
	
	if (!(ret & (DP83TD510_INT2_POR | DP83TD510_INT2_POL | DP83TD510_INT2_PAGE)))
		return IRQ_NONE;

	phy_trigger_machine(phydev);

	return IRQ_HANDLED;

> +
> +static int dp83td510_config_intr(struct phy_device *phydev)
> +{
> +	int int_status;
> +	int gen_cfg_val;
> +	int ret;
> +
> +	if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
> +		int_status = phy_read(phydev, DP83TD510_INT_REG1);
> +		if (int_status < 0)
> +			return int_status;
> +
> +		int_status = (DP83TD510_INT1_ESD_EN | DP83TD510_INT1_LINK_EN |
> +			      DP83TD510_INT1_RHF_EN);
> +
> +		ret = phy_write(phydev, DP83TD510_INT_REG1, int_status);
> +		if (ret)
> +			return ret;
> +
> +		int_status = phy_read(phydev, DP83TD510_INT_REG2);
> +		if (int_status < 0)
> +			return int_status;
> +
> +		int_status = (DP83TD510_INT2_POR | DP83TD510_INT2_POL |
> +				DP83TD510_INT2_PAGE);
> +

Shouldn't you use DP83TD510_INT2_POR_EN, DP83TD510_INT2_POL_EN etc here?
It seems that you are setting up the bits corresponding with the
interrupt status and not the interrupt enable.

Ioana
kernel test robot Nov. 18, 2020, 7:36 a.m. UTC | #2
Hi Dan,

I love your patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Dan-Murphy/DP83TD510-Single-Pair-10Mbps-Ethernet-PHY/20201118-041918
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 72308ecbf33b145641aba61071be31a85ebfd92c
config: arm-randconfig-m031-20201118 (attached as .config)
compiler: arm-linux-gnueabi-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/ec556cedbd640ecfa25713eadf48e5b7ee74fda3
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Dan-Murphy/DP83TD510-Single-Pair-10Mbps-Ethernet-PHY/20201118-041918
        git checkout ec556cedbd640ecfa25713eadf48e5b7ee74fda3
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm 

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

All errors (new ones prefixed by >>):

   drivers/net/phy/dp83td510.c: In function 'dp83869_of_init':
>> drivers/net/phy/dp83td510.c:450:2: error: 'dp83td510' undeclared (first use in this function)
     450 |  dp83td510->hi_diff_output = DP83TD510_2_4V_P2P
         |  ^~~~~~~~~
   drivers/net/phy/dp83td510.c:450:2: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/net/phy/dp83td510.c:451:2: error: expected ';' before 'dp83td510'
     451 |  dp83td510->tx_fifo_depth = DP83TD510_FIFO_DEPTH_5_B_NIB
         |  ^~~~~~~~~
   drivers/net/phy/dp83td510.c:453:1: error: no return statement in function returning non-void [-Werror=return-type]
     453 | }
         | ^
   drivers/net/phy/dp83td510.c: In function 'dp83td510_probe':
>> drivers/net/phy/dp83td510.c:468:8: error: implicit declaration of function 'dp83td510_of_init'; did you mean 'dp83td510_config_init'? [-Werror=implicit-function-declaration]
     468 |  ret = dp83td510_of_init(phydev);
         |        ^~~~~~~~~~~~~~~~~
         |        dp83td510_config_init
   At top level:
   drivers/net/phy/dp83td510.c:448:12: warning: 'dp83869_of_init' defined but not used [-Wunused-function]
     448 | static int dp83869_of_init(struct phy_device *phydev)
         |            ^~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/dp83td510 +450 drivers/net/phy/dp83td510.c

   390	
   391	#if IS_ENABLED(CONFIG_OF_MDIO)
   392	static int dp83td510_of_init(struct phy_device *phydev)
   393	{
   394		struct dp83td510_private *dp83td510 = phydev->priv;
   395		struct device *dev = &phydev->mdio.dev;
   396		struct device_node *of_node = dev->of_node;
   397		s32 rx_int_delay;
   398		s32 tx_int_delay;
   399		int ret;
   400	
   401		if (!of_node)
   402			return -ENODEV;
   403	
   404		ret = of_property_read_u32(phydev->mdio.dev.of_node,
   405					   "max-tx-rx-p2p-microvolt",
   406					   &dp83td510->hi_diff_output);
   407		if (ret)
   408			dp83td510->hi_diff_output = DP83TD510_2_4V_P2P;
   409	
   410		if (dp83td510->hi_diff_output != DP83TD510_2_4V_P2P &&
   411		    dp83td510->hi_diff_output != DP83TD510_1_1V_P2P)
   412			return -EINVAL;
   413	
   414		if (of_property_read_u32(phydev->mdio.dev.of_node, "tx-fifo-depth",
   415					 &dp83td510->tx_fifo_depth))
   416			dp83td510->tx_fifo_depth = DP83TD510_FIFO_DEPTH_5_B_NIB;
   417	
   418		switch (dp83td510->tx_fifo_depth) {
   419		case 4:
   420			dp83td510->tx_fifo_depth = DP83TD510_FIFO_DEPTH_4_B_NIB;
   421			break;
   422		case 6:
   423			dp83td510->tx_fifo_depth = DP83TD510_FIFO_DEPTH_6_B_NIB;
   424			break;
   425		case 8:
   426			dp83td510->tx_fifo_depth = DP83TD510_FIFO_DEPTH_8_B_NIB;
   427			break;
   428		case 5:
   429		default:
   430			dp83td510->tx_fifo_depth = DP83TD510_FIFO_DEPTH_5_B_NIB;
   431		}
   432	
   433		rx_int_delay = phy_get_internal_delay(phydev, dev, NULL, 0, true);
   434		if (rx_int_delay <= 0)
   435			dp83td510->rgmii_delay = DP83TD510_RX_CLK_SHIFT;
   436		else
   437			dp83td510->rgmii_delay = 0;
   438	
   439		tx_int_delay = phy_get_internal_delay(phydev, dev, NULL, 0, false);
   440		if (tx_int_delay <= 0)
   441			dp83td510->rgmii_delay |= DP83TD510_TX_CLK_SHIFT;
   442		else
   443			dp83td510->rgmii_delay &= ~DP83TD510_TX_CLK_SHIFT;
   444	
   445		return 0;
   446	}
   447	#else
   448	static int dp83869_of_init(struct phy_device *phydev)
   449	{
 > 450		dp83td510->hi_diff_output = DP83TD510_2_4V_P2P
 > 451		dp83td510->tx_fifo_depth = DP83TD510_FIFO_DEPTH_5_B_NIB
   452		return 0;
   453	}
   454	#endif /* CONFIG_OF_MDIO */
   455	
   456	static int dp83td510_probe(struct phy_device *phydev)
   457	{
   458		struct dp83td510_private *dp83td510;
   459		int ret;
   460	
   461		dp83td510 = devm_kzalloc(&phydev->mdio.dev, sizeof(*dp83td510),
   462					 GFP_KERNEL);
   463		if (!dp83td510)
   464			return -ENOMEM;
   465	
   466		phydev->priv = dp83td510;
   467	
 > 468		ret = dp83td510_of_init(phydev);
   469		if (ret)
   470			return ret;
   471	
   472		return dp83td510_config_init(phydev);
   473	}
   474	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Andrew Lunn Nov. 20, 2020, 1:49 a.m. UTC | #3
> +static int dp83td510_config_init(struct phy_device *phydev)
> +{
> +	struct dp83td510_private *dp83td510 = phydev->priv;
> +	int ret = 0;
> +
> +	if (phy_interface_is_rgmii(phydev)) {

> +		if (dp83td510->rgmii_delay) {
> +			ret = phy_set_bits_mmd(phydev, DP83TD510_DEVADDR,
> +					       DP83TD510_MAC_CFG_1,
> +					       dp83td510->rgmii_delay);

Just to be safe, you should always write rgmii_delay, even if it is
zero. We have had too many bugs with RGMII delays which cause bad
backwards compatibility problems, so i would prefer to do a write
which might be unneeded, that find a bug here in a few years time.

> +			if (ret)
> +				return ret;
> +		}
> +	}
> +
> +	if (phydev->interface == PHY_INTERFACE_MODE_RMII) {
> +		ret = phy_modify(phydev, DP83TD510_GEN_CFG,
> +				 DP83TD510_FIFO_DEPTH_MASK,
> +				 dp83td510->tx_fifo_depth);

So there is no need to set the FIFO depth for the other three RGMII
modes? Or should this also be phy_interface_is_rgmii(phydev)?

> +#if IS_ENABLED(CONFIG_OF_MDIO)
> +static int dp83td510_of_init(struct phy_device *phydev)
> +{
> +	struct dp83td510_private *dp83td510 = phydev->priv;
> +	struct device *dev = &phydev->mdio.dev;
> +	struct device_node *of_node = dev->of_node;

You need to move this assignment to later in order to keep with
reverse christmas tree.

> +#else
> +static int dp83869_of_init(struct phy_device *phydev)
> +{
> +	dp83td510->hi_diff_output = DP83TD510_2_4V_P2P
> +	dp83td510->tx_fifo_depth = DP83TD510_FIFO_DEPTH_5_B_NIB

You don't have DT, so there is no fine control, but you still need to
do the basic 2ns delay as indicated by the phydev->interface value. So
i think you still need to set dp83td510->rgmii_delay depending on
which RGMII mode is requested.

      Andrew
Dan Murphy Nov. 30, 2020, 4:57 p.m. UTC | #4
Andrew

On 11/19/20 7:49 PM, Andrew Lunn wrote:
>> +static int dp83td510_config_init(struct phy_device *phydev)
>> +{
>> +	struct dp83td510_private *dp83td510 = phydev->priv;
>> +	int ret = 0;
>> +
>> +	if (phy_interface_is_rgmii(phydev)) {
>> +		if (dp83td510->rgmii_delay) {
>> +			ret = phy_set_bits_mmd(phydev, DP83TD510_DEVADDR,
>> +					       DP83TD510_MAC_CFG_1,
>> +					       dp83td510->rgmii_delay);
> Just to be safe, you should always write rgmii_delay, even if it is
> zero. We have had too many bugs with RGMII delays which cause bad
> backwards compatibility problems, so i would prefer to do a write
> which might be unneeded, that find a bug here in a few years time.

OK.


>
>> +			if (ret)
>> +				return ret;
>> +		}
>> +	}
>> +
>> +	if (phydev->interface == PHY_INTERFACE_MODE_RMII) {
>> +		ret = phy_modify(phydev, DP83TD510_GEN_CFG,
>> +				 DP83TD510_FIFO_DEPTH_MASK,
>> +				 dp83td510->tx_fifo_depth);
> So there is no need to set the FIFO depth for the other three RGMII
> modes? Or should this also be phy_interface_is_rgmii(phydev)?

According to the data sheet the FIFO depth is for RMII.

"Fifo depth for RMII Tx fifo"

But I will ask the HW team for clarification.


>
>> +#if IS_ENABLED(CONFIG_OF_MDIO)
>> +static int dp83td510_of_init(struct phy_device *phydev)
>> +{
>> +	struct dp83td510_private *dp83td510 = phydev->priv;
>> +	struct device *dev = &phydev->mdio.dev;
>> +	struct device_node *of_node = dev->of_node;
> You need to move this assignment to later in order to keep with
> reverse christmas tree.
Well this is only used once so I will just remove the of_node declaration
>
>> +#else
>> +static int dp83869_of_init(struct phy_device *phydev)
>> +{
>> +	dp83td510->hi_diff_output = DP83TD510_2_4V_P2P
>> +	dp83td510->tx_fifo_depth = DP83TD510_FIFO_DEPTH_5_B_NIB
> You don't have DT, so there is no fine control, but you still need to
> do the basic 2ns delay as indicated by the phydev->interface value. So
> i think you still need to set dp83td510->rgmii_delay depending on
> which RGMII mode is requested.

The RGMII delay is fixed in the PHY.  The user can either turn it on or 
off. The default is 'off' which is 0.

I can explicitly set the rgmii_delay to 0 in non-OF cases.

Dan
diff mbox series

Patch

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 698bea312adc..017252e1504c 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -302,6 +302,12 @@  config DP83869_PHY
 	  Currently supports the DP83869 PHY.  This PHY supports copper and
 	  fiber connections.
 
+config DP83TD510_PHY
+	tristate "Texas Instruments DP83TD510 10M Single Pair Ethernet PHY"
+	help
+	  Support for the DP83TD510 Ethernet PHY. This PHY supports a 10M single
+	  pair Ethernet connection.
+
 config VITESSE_PHY
 	tristate "Vitesse PHYs"
 	help
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index a13e402074cf..bf62ce211eb4 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -56,6 +56,7 @@  obj-$(CONFIG_DP83848_PHY)	+= dp83848.o
 obj-$(CONFIG_DP83867_PHY)	+= dp83867.o
 obj-$(CONFIG_DP83869_PHY)	+= dp83869.o
 obj-$(CONFIG_DP83TC811_PHY)	+= dp83tc811.o
+obj-$(CONFIG_DP83TD510_PHY)	+= dp83td510.o
 obj-$(CONFIG_FIXED_PHY)		+= fixed_phy.o
 obj-$(CONFIG_ICPLUS_PHY)	+= icplus.o
 obj-$(CONFIG_INTEL_XWAY_PHY)	+= intel-xway.o
diff --git a/drivers/net/phy/dp83td510.c b/drivers/net/phy/dp83td510.c
new file mode 100644
index 000000000000..a4456e0da447
--- /dev/null
+++ b/drivers/net/phy/dp83td510.c
@@ -0,0 +1,505 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/* Driver for the Texas Instruments DP83TD510 PHY
+ * Copyright (C) 2020 Texas Instruments Incorporated - https://www.ti.com/
+ */
+
+#include <linux/ethtool.h>
+#include <linux/etherdevice.h>
+#include <linux/kernel.h>
+#include <linux/mii.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/phy.h>
+#include <linux/netdevice.h>
+
+#define DP83TD510E_PHY_ID	0x20000180
+#define DP83TD510_DEVADDR_AN	0x7
+#define DP83TD510_DEVADDR	0x1f
+#define DP83TD510_PMD_DEVADDR	0x1
+
+#define DP83TD510_PHY_STAT	0x10
+#define DP83TD510_GEN_CFG	0x11
+#define DP83TD510_INT_REG1	0x12
+#define DP83TD510_INT_REG2	0x13
+#define DP83TD510_MAC_CFG_1	0x17
+#define DP83TD510_CTRL_REG	0x1f
+
+#define DP83TD510_ANEG_CTRL	0x200
+#define DP83TD510_PMD_CTRL	0x834
+#define DP83TD510_M_S_CTRL	0x8f6
+
+#define DP83TD510_SOR_1		0x467
+
+#define DP83TD510_HW_RESET	BIT(15)
+#define DP83TD510_SW_RESET	BIT(14)
+
+#define DP83TD510_LINK_STS	BIT(0)
+
+/* GEN CFG bits */
+#define DP83TD510_INT_OE	BIT(0)
+#define DP83TD510_INT_EN	BIT(1)
+
+/* INT REG 1 bits */
+#define DP83TD510_INT1_ESD_EN	BIT(3)
+#define DP83TD510_INT1_LINK_EN	BIT(5)
+#define DP83TD510_INT1_RHF_EN	BIT(7)
+#define DP83TD510_INT1_ESD	BIT(11)
+#define DP83TD510_INT1_LINK	BIT(13)
+#define DP83TD510_INT1_RHF	BIT(15)
+
+/* INT REG 2 bits */
+#define DP83TD510_INT2_POR_EN	BIT(0)
+#define DP83TD510_INT2_POL_EN	BIT(1)
+#define DP83TD510_INT2_PAGE_EN	BIT(5)
+#define DP83TD510_INT2_POR	BIT(8)
+#define DP83TD510_INT2_POL	BIT(9)
+#define DP83TD510_INT2_PAGE	BIT(13)
+
+/* MAC CFG bits */
+#define DP83TD510_RX_CLK_SHIFT	BIT(12)
+#define DP83TD510_TX_CLK_SHIFT	BIT(11)
+
+#define DP83TD510_MASTER_MODE	BIT(14)
+#define DP83TD510_AUTO_NEG_EN	BIT(12)
+#define DP83TD510_RGMII		BIT(8)
+
+#define DP83TD510_FIFO_DEPTH_MASK	GENMASK(6, 5)
+#define DP83TD510_FIFO_DEPTH_4_B_NIB	0
+#define DP83TD510_FIFO_DEPTH_5_B_NIB	BIT(5)
+#define DP83TD510_FIFO_DEPTH_6_B_NIB	BIT(6)
+#define DP83TD510_FIFO_DEPTH_8_B_NIB	(BIT(5) | BIT(6))
+
+#define DP83TD510_2_4V		BIT(12)
+#define DP83TD510_2_4V_P2P	2400
+#define DP83TD510_1_1V_P2P	1100
+#define DP83TD510_AUTO_NEG_P2P	0
+
+const int dp83td510_feature_array[4] = {
+	ETHTOOL_LINK_MODE_Autoneg_BIT,
+	ETHTOOL_LINK_MODE_10baseT1L_Half_BIT,
+	ETHTOOL_LINK_MODE_10baseT1L_Full_BIT,
+	ETHTOOL_LINK_MODE_TP_BIT,
+};
+
+struct dp83td510_private {
+	u32 hi_diff_output;
+	u32 tx_fifo_depth;
+	u32 rgmii_delay;
+	bool is_rgmii;
+};
+
+struct dp83td510_init_reg {
+	int reg;
+	int val;
+};
+
+static struct dp83td510_init_reg dp83td510_errata[] = {
+	{ 0x608, 0x003b }, /* disable_0_transition */
+	{ 0x862, 0x39f8 }, /* AGC Gain during Autoneg */
+	{ 0x81a, 0x67c0 }, /* deq offset for 1V swing */
+	{ 0x81c, 0xfb62 }, /* deq offset for 2.4V swing */
+	{ 0x830, 0x05a3 }, /* Enable energy lost fallback */
+	{ 0x855, 0x1b55 }, /* MSE Threshold change */
+	{ 0x831, 0x0403 }, /* energy detect threshold */
+	{ 0x856, 0x1800 }, /* good1 MSE threshold change */
+	{ 0x857, 0x8fa0 }, /* Enable fallback to phase 1 on watchdog trigger */
+	{ 0x871, 0x000c }, /* TED input changed to slicer_in without FFE */
+	{ 0x883, 0x022e }, /* Enable Rx Filter for Short Cable detection */
+	{ 0x402, 0x1800 }, /* Adjust LD swing */
+	{ 0x878, 0x2248 }, /* Change PI up/down polarity */
+	{ 0x10c, 0x0008 }, /* tx filter coefficient */
+	{ 0x112, 0x1212 }, /* tx filter scaling factor */
+	{ 0x809, 0x5c80 }, /* AGC retrain */
+	{ 0x803, 0x1529 }, /* Master Ph1 Back-off */
+	{ 0x804, 0x1a33 }, /* Master Ph1 Back-off */
+	{ 0x805, 0x1f3d }, /* Master Ph1 Back-off */
+	{ 0x850, 0x045b }, /* hybrid gain & delay */
+	{ 0x874, 0x6967 }, /* kp step 0 for master */
+	{ 0x852, 0x7800 }, /* FAGC init gain */
+	{ 0x806, 0x1e1e }, /* Master/Slave Ph2 Back-off */
+	{ 0x807, 0x2525 }, /* Master/Slave Ph2 Back-off */
+	{ 0x808, 0x2c2c }, /* Master/Slave Ph2 Back-off */
+	{ 0x850, 0x0590 }, /* Hybrid Gain/Delay Code */
+	{ 0x827, 0x4000 }, /* Echo Fixed Delay */
+	{ 0x849, 0x0fe4 }, /* Hybrid Cal enable */
+	{ 0x84b, 0x04b5 }, /* Echo Score Sel */
+	{ 0x018, 0x0043 }, /* CRS/RX_DV pin as RX_DV for RMII repeater mode */
+};
+
+static int dp83td510_ack_interrupt(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = phy_read(phydev, DP83TD510_INT_REG1);
+	if (ret < 0)
+		return ret;
+
+	ret = phy_read(phydev, DP83TD510_INT_REG2);
+	if (ret < 0)
+		return ret;
+
+	phy_trigger_machine(phydev);
+
+	return 0;
+}
+
+static irqreturn_t dp83td510_handle_interrupt(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = dp83td510_ack_interrupt(phydev);
+	if (ret)
+		return IRQ_NONE;
+
+	return IRQ_HANDLED;
+}
+
+static int dp83td510_config_intr(struct phy_device *phydev)
+{
+	int int_status;
+	int gen_cfg_val;
+	int ret;
+
+	if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
+		int_status = phy_read(phydev, DP83TD510_INT_REG1);
+		if (int_status < 0)
+			return int_status;
+
+		int_status = (DP83TD510_INT1_ESD_EN | DP83TD510_INT1_LINK_EN |
+			      DP83TD510_INT1_RHF_EN);
+
+		ret = phy_write(phydev, DP83TD510_INT_REG1, int_status);
+		if (ret)
+			return ret;
+
+		int_status = phy_read(phydev, DP83TD510_INT_REG2);
+		if (int_status < 0)
+			return int_status;
+
+		int_status = (DP83TD510_INT2_POR | DP83TD510_INT2_POL |
+				DP83TD510_INT2_PAGE);
+
+		ret = phy_write(phydev, DP83TD510_INT_REG2, int_status);
+		if (ret)
+			return ret;
+
+		gen_cfg_val = phy_read(phydev, DP83TD510_GEN_CFG);
+		if (gen_cfg_val < 0)
+			return gen_cfg_val;
+
+		gen_cfg_val |= DP83TD510_INT_OE | DP83TD510_INT_EN;
+
+	} else {
+		ret = phy_write(phydev, DP83TD510_INT_REG1, 0);
+		if (ret)
+			return ret;
+
+		ret = phy_write(phydev, DP83TD510_INT_REG2, 0);
+		if (ret)
+			return ret;
+
+		gen_cfg_val = phy_read(phydev, DP83TD510_GEN_CFG);
+		if (gen_cfg_val < 0)
+			return gen_cfg_val;
+
+		gen_cfg_val &= ~DP83TD510_INT_EN;
+	}
+
+	ret = phy_write(phydev, DP83TD510_GEN_CFG, gen_cfg_val);
+	if (ret)
+		return ret;
+
+	return dp83td510_ack_interrupt(phydev);
+}
+
+static int dp83td510_configure_mode(struct phy_device *phydev, int pmd_ctrl)
+{
+	struct dp83td510_private *dp83td510 = phydev->priv;
+	int ret;
+	int i;
+
+	ret = phy_set_bits(phydev, DP83TD510_CTRL_REG, DP83TD510_HW_RESET);
+	if (ret < 0)
+		return ret;
+
+	if (dp83td510->hi_diff_output == DP83TD510_2_4V_P2P)
+		ret = phy_write_mmd(phydev, DP83TD510_DEVADDR,
+				    DP83TD510_M_S_CTRL, DP83TD510_2_4V);
+	else
+		ret = phy_write_mmd(phydev, DP83TD510_DEVADDR,
+				    DP83TD510_M_S_CTRL, 0);
+
+	if (phydev->autoneg) {
+		ret = phy_modify_mmd(phydev, DP83TD510_DEVADDR_AN,
+				     DP83TD510_ANEG_CTRL,
+				     DP83TD510_AUTO_NEG_EN,
+				     DP83TD510_AUTO_NEG_EN);
+		if (ret)
+			return ret;
+	} else {
+		ret = phy_modify_mmd(phydev, DP83TD510_DEVADDR_AN,
+				     DP83TD510_ANEG_CTRL,
+				     DP83TD510_AUTO_NEG_EN, 0);
+		if (ret)
+			return ret;
+	}
+
+	ret = phy_modify_mmd(phydev, DP83TD510_PMD_DEVADDR,
+			     DP83TD510_PMD_CTRL,
+			     DP83TD510_MASTER_MODE, pmd_ctrl);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < ARRAY_SIZE(dp83td510_errata); i++) {
+		ret = phy_write_mmd(phydev, DP83TD510_DEVADDR,
+				    dp83td510_errata[i].reg,
+				    dp83td510_errata[i].val);
+
+		if (ret)
+			return ret;
+	}
+
+	return phy_set_bits(phydev, DP83TD510_CTRL_REG, DP83TD510_SW_RESET);
+}
+
+static int dp83td510_set_master_slave(struct phy_device *phydev)
+{
+	int mst_slave_cfg;
+
+	mst_slave_cfg = phy_read_mmd(phydev, DP83TD510_PMD_DEVADDR,
+				     DP83TD510_PMD_CTRL);
+	if (mst_slave_cfg < 0)
+		return mst_slave_cfg;
+
+	if (mst_slave_cfg & DP83TD510_MASTER_MODE) {
+		if (phydev->autoneg) {
+			phydev->master_slave_get = MASTER_SLAVE_CFG_MASTER_PREFERRED;
+			phydev->master_slave_set = MASTER_SLAVE_CFG_MASTER_PREFERRED;
+		} else {
+			phydev->master_slave_get = MASTER_SLAVE_CFG_MASTER_FORCE;
+			phydev->master_slave_set = MASTER_SLAVE_CFG_MASTER_FORCE;
+		}
+	} else {
+		if (phydev->autoneg) {
+			phydev->master_slave_get = MASTER_SLAVE_CFG_SLAVE_PREFERRED;
+			phydev->master_slave_set = MASTER_SLAVE_CFG_SLAVE_PREFERRED;
+		} else {
+			phydev->master_slave_get = MASTER_SLAVE_CFG_SLAVE_FORCE;
+			phydev->master_slave_set = MASTER_SLAVE_CFG_SLAVE_FORCE;
+		}
+	}
+
+	return 0;
+}
+
+static int dp83td510_read_status(struct phy_device *phydev)
+{
+	int phy_status;
+	int phy_bmcr;
+
+	phy_status = phy_read(phydev, DP83TD510_PHY_STAT);
+	if (phy_status < 0)
+		return phy_status;
+
+	phy_bmcr = phy_read(phydev, MII_BMCR);
+	if (phy_bmcr < 0)
+		return phy_bmcr;
+
+	phydev->link = phy_status & DP83TD510_LINK_STS;
+	if (phydev->link) {
+		phydev->duplex = phy_bmcr & BMCR_FULLDPLX ? DUPLEX_FULL : DUPLEX_HALF;
+		phydev->speed = SPEED_10;
+	} else {
+		phydev->speed = SPEED_UNKNOWN;
+		phydev->duplex = DUPLEX_UNKNOWN;
+	}
+
+	return dp83td510_set_master_slave(phydev);
+}
+
+static int dp83td510_get_features(struct phy_device *phydev)
+{
+	linkmode_set_bit_array(dp83td510_feature_array,
+			       ARRAY_SIZE(dp83td510_feature_array),
+			       phydev->supported);
+
+	return 0;
+}
+
+static int dp83td510_config_aneg(struct phy_device *phydev)
+{
+	int pmd_ctrl;
+
+	switch (phydev->master_slave_set) {
+	case MASTER_SLAVE_CFG_MASTER_PREFERRED:
+	case MASTER_SLAVE_CFG_MASTER_FORCE:
+		pmd_ctrl = DP83TD510_MASTER_MODE;
+		break;
+	case MASTER_SLAVE_CFG_SLAVE_PREFERRED:
+	case MASTER_SLAVE_CFG_SLAVE_FORCE:
+		pmd_ctrl = 0;
+		break;
+	case MASTER_SLAVE_CFG_UNKNOWN:
+	case MASTER_SLAVE_CFG_UNSUPPORTED:
+	default:
+		phydev_warn(phydev, "Unsupported Master/Slave mode\n");
+		return -ENOTSUPP;
+	}
+
+	return dp83td510_configure_mode(phydev, pmd_ctrl);
+}
+
+static int dp83td510_config_init(struct phy_device *phydev)
+{
+	struct dp83td510_private *dp83td510 = phydev->priv;
+	int ret = 0;
+
+	if (phy_interface_is_rgmii(phydev)) {
+		if (dp83td510->rgmii_delay) {
+			ret = phy_set_bits_mmd(phydev, DP83TD510_DEVADDR,
+					       DP83TD510_MAC_CFG_1,
+					       dp83td510->rgmii_delay);
+			if (ret)
+				return ret;
+		}
+	}
+
+	if (phydev->interface == PHY_INTERFACE_MODE_RMII) {
+		ret = phy_modify(phydev, DP83TD510_GEN_CFG,
+				 DP83TD510_FIFO_DEPTH_MASK,
+				 dp83td510->tx_fifo_depth);
+		if (ret)
+			return ret;
+	}
+
+	return dp83td510_set_master_slave(phydev);
+}
+
+static int dp83td510_phy_reset(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = phy_set_bits(phydev, DP83TD510_CTRL_REG, DP83TD510_SW_RESET);
+	if (ret < 0)
+		return ret;
+
+	usleep_range(10, 20);
+
+	return dp83td510_config_init(phydev);
+}
+
+#if IS_ENABLED(CONFIG_OF_MDIO)
+static int dp83td510_of_init(struct phy_device *phydev)
+{
+	struct dp83td510_private *dp83td510 = phydev->priv;
+	struct device *dev = &phydev->mdio.dev;
+	struct device_node *of_node = dev->of_node;
+	s32 rx_int_delay;
+	s32 tx_int_delay;
+	int ret;
+
+	if (!of_node)
+		return -ENODEV;
+
+	ret = of_property_read_u32(phydev->mdio.dev.of_node,
+				   "max-tx-rx-p2p-microvolt",
+				   &dp83td510->hi_diff_output);
+	if (ret)
+		dp83td510->hi_diff_output = DP83TD510_2_4V_P2P;
+
+	if (dp83td510->hi_diff_output != DP83TD510_2_4V_P2P &&
+	    dp83td510->hi_diff_output != DP83TD510_1_1V_P2P)
+		return -EINVAL;
+
+	if (of_property_read_u32(phydev->mdio.dev.of_node, "tx-fifo-depth",
+				 &dp83td510->tx_fifo_depth))
+		dp83td510->tx_fifo_depth = DP83TD510_FIFO_DEPTH_5_B_NIB;
+
+	switch (dp83td510->tx_fifo_depth) {
+	case 4:
+		dp83td510->tx_fifo_depth = DP83TD510_FIFO_DEPTH_4_B_NIB;
+		break;
+	case 6:
+		dp83td510->tx_fifo_depth = DP83TD510_FIFO_DEPTH_6_B_NIB;
+		break;
+	case 8:
+		dp83td510->tx_fifo_depth = DP83TD510_FIFO_DEPTH_8_B_NIB;
+		break;
+	case 5:
+	default:
+		dp83td510->tx_fifo_depth = DP83TD510_FIFO_DEPTH_5_B_NIB;
+	}
+
+	rx_int_delay = phy_get_internal_delay(phydev, dev, NULL, 0, true);
+	if (rx_int_delay <= 0)
+		dp83td510->rgmii_delay = DP83TD510_RX_CLK_SHIFT;
+	else
+		dp83td510->rgmii_delay = 0;
+
+	tx_int_delay = phy_get_internal_delay(phydev, dev, NULL, 0, false);
+	if (tx_int_delay <= 0)
+		dp83td510->rgmii_delay |= DP83TD510_TX_CLK_SHIFT;
+	else
+		dp83td510->rgmii_delay &= ~DP83TD510_TX_CLK_SHIFT;
+
+	return 0;
+}
+#else
+static int dp83869_of_init(struct phy_device *phydev)
+{
+	dp83td510->hi_diff_output = DP83TD510_2_4V_P2P
+	dp83td510->tx_fifo_depth = DP83TD510_FIFO_DEPTH_5_B_NIB
+	return 0;
+}
+#endif /* CONFIG_OF_MDIO */
+
+static int dp83td510_probe(struct phy_device *phydev)
+{
+	struct dp83td510_private *dp83td510;
+	int ret;
+
+	dp83td510 = devm_kzalloc(&phydev->mdio.dev, sizeof(*dp83td510),
+				 GFP_KERNEL);
+	if (!dp83td510)
+		return -ENOMEM;
+
+	phydev->priv = dp83td510;
+
+	ret = dp83td510_of_init(phydev);
+	if (ret)
+		return ret;
+
+	return dp83td510_config_init(phydev);
+}
+
+static struct phy_driver dp83td510_driver[] = {
+	{
+		PHY_ID_MATCH_MODEL(DP83TD510E_PHY_ID),
+		.name		= "TI DP83TD510E",
+		.probe          = dp83td510_probe,
+		.config_init	= dp83td510_config_init,
+		.soft_reset	= dp83td510_phy_reset,
+
+		/* IRQ related */
+		.handle_interrupt = dp83td510_handle_interrupt,
+		.config_intr	= dp83td510_config_intr,
+		.config_aneg	= dp83td510_config_aneg,
+		.read_status	= dp83td510_read_status,
+
+		.get_features	= dp83td510_get_features,
+
+		.suspend	= genphy_suspend,
+		.resume		= genphy_resume,
+	},
+};
+module_phy_driver(dp83td510_driver);
+
+static struct mdio_device_id __maybe_unused dp83td510_tbl[] = {
+	{ PHY_ID_MATCH_MODEL(DP83TD510E_PHY_ID) },
+	{ }
+};
+MODULE_DEVICE_TABLE(mdio, dp83td510_tbl);
+
+MODULE_DESCRIPTION("Texas Instruments DP83TD510E PHY driver");
+MODULE_AUTHOR("Dan Murphy <dmurphy@ti.com");
+MODULE_LICENSE("GPL v2");