diff mbox series

[net-next,v2,3/3] net: dp83869: Add speed optimization feature

Message ID 20200902203444.29167-4-dmurphy@ti.com
State Superseded
Delegated to: David Miller
Headers show
Series DP83869 Feature additions | expand

Commit Message

Dan Murphy Sept. 2, 2020, 8:34 p.m. UTC 
Set the speed optimization bit on the DP83869 PHY.

Speed optimization, also known as link downshift, enables fallback to 100M
operation after multiple consecutive failed attempts at Gigabit link
establishment. Such a case could occur if cabling with only four wires
(two twisted pairs) were connected instead of the standard cabling with
eight wires (four twisted pairs).

The number of failed link attempts before falling back to 100M operation is
configurable. By default, four failed link attempts are required before
falling back to 100M.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 drivers/net/phy/dp83869.c | 116 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 116 insertions(+)

Comments

Jakub Kicinski Sept. 3, 2020, 2:06 a.m. UTC | #1 
On Wed, 2 Sep 2020 15:34:44 -0500 Dan Murphy wrote:
> Set the speed optimization bit on the DP83869 PHY.
> 
> Speed optimization, also known as link downshift, enables fallback to 100M
> operation after multiple consecutive failed attempts at Gigabit link
> establishment. Such a case could occur if cabling with only four wires
> (two twisted pairs) were connected instead of the standard cabling with
> eight wires (four twisted pairs).
> 
> The number of failed link attempts before falling back to 100M operation is
> configurable. By default, four failed link attempts are required before
> falling back to 100M.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>

there seems to be lots of checkpatch warnings here:

ERROR: switch and case should be at the same indent
#111: FILE: drivers/net/phy/dp83869.c:342:
+	switch (cnt) {
+		case DP83869_DOWNSHIFT_1_COUNT:
[...]
+		case DP83869_DOWNSHIFT_2_COUNT:
[...]
+		case DP83869_DOWNSHIFT_4_COUNT:
[...]
+		case DP83869_DOWNSHIFT_8_COUNT:
[...]
+		default:

CHECK: Alignment should match open parenthesis
#139: FILE: drivers/net/phy/dp83869.c:370:
+static int dp83869_get_tunable(struct phy_device *phydev,
+				struct ethtool_tunable *tuna, void *data)

CHECK: Alignment should match open parenthesis
#150: FILE: drivers/net/phy/dp83869.c:381:
+static int dp83869_set_tunable(struct phy_device *phydev,
+				struct ethtool_tunable *tuna, const void *data)

WARNING: please, no spaces at the start of a line
#168: FILE: drivers/net/phy/dp83869.c:669:
+       ret = phy_modify(phydev, DP83869_CFG2, DP83869_DOWNSHIFT_EN,$

ERROR: code indent should use tabs where possible
#169: FILE: drivers/net/phy/dp83869.c:670:
+                        DP83869_DOWNSHIFT_EN);$

WARNING: please, no spaces at the start of a line
#169: FILE: drivers/net/phy/dp83869.c:670:
+                        DP83869_DOWNSHIFT_EN);$

WARNING: please, no spaces at the start of a line
#170: FILE: drivers/net/phy/dp83869.c:671:
+       if (ret)$

WARNING: suspect code indent for conditional statements (7, 15)
#170: FILE: drivers/net/phy/dp83869.c:671:
+       if (ret)
+               return ret;

ERROR: code indent should use tabs where possible
#171: FILE: drivers/net/phy/dp83869.c:672:
+               return ret;$

WARNING: please, no spaces at the start of a line
#171: FILE: drivers/net/phy/dp83869.c:672:
+               return ret;$

total: 3 errors, 5 warnings, 2 checks, 152 lines checked
kernel test robot Sept. 3, 2020, 4:10 a.m. UTC | #2 
Hi Dan,

I love your patch! Perhaps something to improve:

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

url:    https://github.com/0day-ci/linux/commits/Dan-Murphy/DP83869-Feature-additions/20200903-043618
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git dc1a9bf2c8169d9f607502162af1858a73a18cb8
config: i386-randconfig-m021-20200902 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

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

smatch warnings:
drivers/net/phy/dp83869.c:669 dp83869_config_init() warn: inconsistent indenting

# https://github.com/0day-ci/linux/commit/307ae0a0e8406ceadb72d448df96322bbd23aa92
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Dan-Murphy/DP83869-Feature-additions/20200903-043618
git checkout 307ae0a0e8406ceadb72d448df96322bbd23aa92
vim +669 drivers/net/phy/dp83869.c

   662	
   663	static int dp83869_config_init(struct phy_device *phydev)
   664	{
   665		struct dp83869_private *dp83869 = phydev->priv;
   666		int ret, val;
   667	
   668	       /* Force speed optimization for the PHY even if it strapped */
 > 669	       ret = phy_modify(phydev, DP83869_CFG2, DP83869_DOWNSHIFT_EN,
   670	                        DP83869_DOWNSHIFT_EN);
   671	       if (ret)
   672	               return ret;
   673	
   674		ret = dp83869_configure_mode(phydev, dp83869);
   675		if (ret)
   676			return ret;
   677	
   678		/* Enable Interrupt output INT_OE in CFG4 register */
   679		if (phy_interrupt_is_valid(phydev)) {
   680			val = phy_read(phydev, DP83869_CFG4);
   681			val |= DP83869_INT_OE;
   682			phy_write(phydev, DP83869_CFG4, val);
   683		}
   684	
   685		if (dp83869->port_mirroring != DP83869_PORT_MIRRORING_KEEP)
   686			dp83869_config_port_mirroring(phydev);
   687	
   688		/* Clock output selection if muxing property is set */
   689		if (dp83869->clk_output_sel != DP83869_CLK_O_SEL_REF_CLK)
   690			ret = phy_modify_mmd(phydev,
   691					     DP83869_DEVADDR, DP83869_IO_MUX_CFG,
   692					     DP83869_IO_MUX_CFG_CLK_O_SEL_MASK,
   693					     dp83869->clk_output_sel <<
   694					     DP83869_IO_MUX_CFG_CLK_O_SEL_SHIFT);
   695	
   696		if (phy_interface_is_rgmii(phydev)) {
   697			ret = phy_write_mmd(phydev, DP83869_DEVADDR, DP83869_RGMIIDCTL,
   698					    dp83869->rx_int_delay |
   699				dp83869->tx_int_delay << DP83869_RGMII_CLK_DELAY_SHIFT);
   700			if (ret)
   701				return ret;
   702	
   703			val = phy_read_mmd(phydev, DP83869_DEVADDR, DP83869_RGMIICTL);
   704			val &= ~(DP83869_RGMII_TX_CLK_DELAY_EN |
   705				 DP83869_RGMII_RX_CLK_DELAY_EN);
   706	
   707			if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
   708				val |= (DP83869_RGMII_TX_CLK_DELAY_EN |
   709					DP83869_RGMII_RX_CLK_DELAY_EN);
   710	
   711			if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)
   712				val |= DP83869_RGMII_TX_CLK_DELAY_EN;
   713	
   714			if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
   715				val |= DP83869_RGMII_RX_CLK_DELAY_EN;
   716	
   717			ret = phy_write_mmd(phydev, DP83869_DEVADDR, DP83869_RGMIICTL,
   718					    val);
   719		}
   720	
   721		return ret;
   722	}
   723	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Dan Murphy Sept. 3, 2020, 11:05 a.m. UTC | #3 
Jakub

On 9/2/20 9:06 PM, Jakub Kicinski wrote:
> On Wed, 2 Sep 2020 15:34:44 -0500 Dan Murphy wrote:
>> Set the speed optimization bit on the DP83869 PHY.
>>
>> Speed optimization, also known as link downshift, enables fallback to 100M
>> operation after multiple consecutive failed attempts at Gigabit link
>> establishment. Such a case could occur if cabling with only four wires
>> (two twisted pairs) were connected instead of the standard cabling with
>> eight wires (four twisted pairs).
>>
>> The number of failed link attempts before falling back to 100M operation is
>> configurable. By default, four failed link attempts are required before
>> falling back to 100M.
>>
>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> there seems to be lots of checkpatch warnings here:
>
> ERROR: switch and case should be at the same indent
> #111: FILE: drivers/net/phy/dp83869.c:342:
> +	switch (cnt) {
> +		case DP83869_DOWNSHIFT_1_COUNT:
> [...]
> +		case DP83869_DOWNSHIFT_2_COUNT:
> [...]
> +		case DP83869_DOWNSHIFT_4_COUNT:
> [...]
> +		case DP83869_DOWNSHIFT_8_COUNT:
> [...]
> +		default:
>
> CHECK: Alignment should match open parenthesis
> #139: FILE: drivers/net/phy/dp83869.c:370:
> +static int dp83869_get_tunable(struct phy_device *phydev,
> +				struct ethtool_tunable *tuna, void *data)
>
> CHECK: Alignment should match open parenthesis
> #150: FILE: drivers/net/phy/dp83869.c:381:
> +static int dp83869_set_tunable(struct phy_device *phydev,
> +				struct ethtool_tunable *tuna, const void *data)
>
> WARNING: please, no spaces at the start of a line
> #168: FILE: drivers/net/phy/dp83869.c:669:
> +       ret = phy_modify(phydev, DP83869_CFG2, DP83869_DOWNSHIFT_EN,$
>
> ERROR: code indent should use tabs where possible
> #169: FILE: drivers/net/phy/dp83869.c:670:
> +                        DP83869_DOWNSHIFT_EN);$
>
> WARNING: please, no spaces at the start of a line
> #169: FILE: drivers/net/phy/dp83869.c:670:
> +                        DP83869_DOWNSHIFT_EN);$
>
> WARNING: please, no spaces at the start of a line
> #170: FILE: drivers/net/phy/dp83869.c:671:
> +       if (ret)$
>
> WARNING: suspect code indent for conditional statements (7, 15)
> #170: FILE: drivers/net/phy/dp83869.c:671:
> +       if (ret)
> +               return ret;
>
> ERROR: code indent should use tabs where possible
> #171: FILE: drivers/net/phy/dp83869.c:672:
> +               return ret;$
>
> WARNING: please, no spaces at the start of a line
> #171: FILE: drivers/net/phy/dp83869.c:672:
> +               return ret;$
>
> total: 3 errors, 5 warnings, 2 checks, 152 lines checked

I will fix these.

Dan
diff mbox series

Patch

diff --git a/drivers/net/phy/dp83869.c b/drivers/net/phy/dp83869.c
index 5045df9515a5..06a228dac92b 100644
--- a/drivers/net/phy/dp83869.c
+++ b/drivers/net/phy/dp83869.c
@@ -11,6 +11,7 @@ 
 #include <linux/of.h>
 #include <linux/phy.h>
 #include <linux/delay.h>
+#include <linux/bitfield.h>
 
 #include <dt-bindings/net/ti-dp83869.h>
 
@@ -20,6 +21,7 @@ 
 #define MII_DP83869_PHYCTRL	0x10
 #define MII_DP83869_MICR	0x12
 #define MII_DP83869_ISR		0x13
+#define DP83869_CFG2		0x14
 #define DP83869_CTRL		0x1f
 #define DP83869_CFG4		0x1e
 
@@ -121,6 +123,18 @@ 
 #define DP83869_WOL_SEC_EN		BIT(5)
 #define DP83869_WOL_ENH_MAC		BIT(7)
 
+/* CFG2 bits */
+#define DP83869_DOWNSHIFT_EN		(BIT(8) | BIT(9))
+#define DP83869_DOWNSHIFT_ATTEMPT_MASK	(BIT(10) | BIT(11))
+#define DP83869_DOWNSHIFT_1_COUNT_VAL	0
+#define DP83869_DOWNSHIFT_2_COUNT_VAL	1
+#define DP83869_DOWNSHIFT_4_COUNT_VAL	2
+#define DP83869_DOWNSHIFT_8_COUNT_VAL	3
+#define DP83869_DOWNSHIFT_1_COUNT	1
+#define DP83869_DOWNSHIFT_2_COUNT	2
+#define DP83869_DOWNSHIFT_4_COUNT	4
+#define DP83869_DOWNSHIFT_8_COUNT	8
+
 enum {
 	DP83869_PORT_MIRRORING_KEEP,
 	DP83869_PORT_MIRRORING_EN,
@@ -281,6 +295,99 @@  static void dp83869_get_wol(struct phy_device *phydev,
 		wol->wolopts = 0;
 }
 
+static int dp83869_get_downshift(struct phy_device *phydev, u8 *data)
+{
+	int val, cnt, enable, count;
+
+	val = phy_read(phydev, DP83869_CFG2);
+	if (val < 0)
+		return val;
+
+	enable = FIELD_GET(DP83869_DOWNSHIFT_EN, val);
+	cnt = FIELD_GET(DP83869_DOWNSHIFT_ATTEMPT_MASK, val);
+
+	switch (cnt) {
+	case DP83869_DOWNSHIFT_1_COUNT_VAL:
+		count = DP83869_DOWNSHIFT_1_COUNT;
+		break;
+	case DP83869_DOWNSHIFT_2_COUNT_VAL:
+		count = DP83869_DOWNSHIFT_2_COUNT;
+		break;
+	case DP83869_DOWNSHIFT_4_COUNT_VAL:
+		count = DP83869_DOWNSHIFT_4_COUNT;
+		break;
+	case DP83869_DOWNSHIFT_8_COUNT_VAL:
+		count = DP83869_DOWNSHIFT_8_COUNT;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	*data = enable ? count : DOWNSHIFT_DEV_DISABLE;
+
+	return 0;
+}
+
+static int dp83869_set_downshift(struct phy_device *phydev, u8 cnt)
+{
+	int val, count;
+
+	if (cnt > DP83869_DOWNSHIFT_8_COUNT)
+		return -E2BIG;
+
+	if (!cnt)
+		return phy_clear_bits(phydev, DP83869_CFG2,
+				      DP83869_DOWNSHIFT_EN);
+
+	switch (cnt) {
+		case DP83869_DOWNSHIFT_1_COUNT:
+			count = DP83869_DOWNSHIFT_1_COUNT_VAL;
+			break;
+		case DP83869_DOWNSHIFT_2_COUNT:
+			count = DP83869_DOWNSHIFT_2_COUNT_VAL;
+			break;
+		case DP83869_DOWNSHIFT_4_COUNT:
+			count = DP83869_DOWNSHIFT_4_COUNT_VAL;
+			break;
+		case DP83869_DOWNSHIFT_8_COUNT:
+			count = DP83869_DOWNSHIFT_8_COUNT_VAL;
+			break;
+		default:
+			phydev_err(phydev,
+				   "Downshift count must be 1, 2, 4 or 8\n");
+			return -EINVAL;
+	}
+
+	val = DP83869_DOWNSHIFT_EN;
+	val |= FIELD_PREP(DP83869_DOWNSHIFT_ATTEMPT_MASK, count);
+
+	return phy_modify(phydev, DP83869_CFG2,
+			  DP83869_DOWNSHIFT_EN | DP83869_DOWNSHIFT_ATTEMPT_MASK,
+			  val);
+}
+
+static int dp83869_get_tunable(struct phy_device *phydev,
+				struct ethtool_tunable *tuna, void *data)
+{
+	switch (tuna->id) {
+	case ETHTOOL_PHY_DOWNSHIFT:
+		return dp83869_get_downshift(phydev, data);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int dp83869_set_tunable(struct phy_device *phydev,
+				struct ethtool_tunable *tuna, const void *data)
+{
+	switch (tuna->id) {
+	case ETHTOOL_PHY_DOWNSHIFT:
+		return dp83869_set_downshift(phydev, *(const u8 *)data);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
 static int dp83869_config_port_mirroring(struct phy_device *phydev)
 {
 	struct dp83869_private *dp83869 = phydev->priv;
@@ -558,6 +665,12 @@  static int dp83869_config_init(struct phy_device *phydev)
 	struct dp83869_private *dp83869 = phydev->priv;
 	int ret, val;
 
+       /* Force speed optimization for the PHY even if it strapped */
+       ret = phy_modify(phydev, DP83869_CFG2, DP83869_DOWNSHIFT_EN,
+                        DP83869_DOWNSHIFT_EN);
+       if (ret)
+               return ret;
+
 	ret = dp83869_configure_mode(phydev, dp83869);
 	if (ret)
 		return ret;
@@ -656,6 +769,9 @@  static struct phy_driver dp83869_driver[] = {
 		.ack_interrupt	= dp83869_ack_interrupt,
 		.config_intr	= dp83869_config_intr,
 
+		.get_tunable	= dp83869_get_tunable,
+		.set_tunable	= dp83869_set_tunable,
+
 		.get_wol	= dp83869_get_wol,
 		.set_wol	= dp83869_set_wol,