2015-06-05 23:00:44

by Jaeden Amero

[permalink] [raw]
Subject: [PATCH v4 0/3] net/phy: micrel: Center FLP timing at 16ms

In v2, we add an additional cleanup commit to make an array of strings
static const and to improve const correctness generally. We also no longer
unnecessarily initialize the result variable in
ksz9031_center_flp_timing().

In v3, we remove the unnecessary result variable from ksz9031_config_init()
introduced by a previous version of "net/phy: micrel: Center FLP timing at
16ms".

In v4, we modify the commit message of "net/phy: micrel: Center FLP timing
at 16ms" to replace the awkward quotation of the data sheet's programming
procedure with an explanation of why we program the FLP burst registers and
restart auto-negotiation where we do (config_init).

Jaeden Amero (3):
net/phy: micrel: Be more const correct
net/phy: micrel: Comment MMD address of extended registers
net/phy: micrel: Center FLP timing at 16ms

drivers/net/phy/micrel.c | 53 ++++++++++++++++++++++++++++++++++--------------
1 file changed, 38 insertions(+), 15 deletions(-)

--
2.1.4


2015-06-05 23:00:48

by Jaeden Amero

[permalink] [raw]
Subject: [PATCH v4 1/3] net/phy: micrel: Be more const correct

In a few places in this driver, we weren't using const where we could
have. Use const more.

In addition, change the arrays of strings in ksz9031_config_init() to be
not only const, but also static.

Signed-off-by: Jaeden Amero <[email protected]>
---
drivers/net/phy/micrel.c | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index ebdc357..59cc5d4 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -288,9 +288,10 @@ static int kszphy_config_init(struct phy_device *phydev)
}

static int ksz9021_load_values_from_of(struct phy_device *phydev,
- struct device_node *of_node, u16 reg,
- char *field1, char *field2,
- char *field3, char *field4)
+ const struct device_node *of_node,
+ u16 reg,
+ const char *field1, const char *field2,
+ const char *field3, const char *field4)
{
int val1 = -1;
int val2 = -2;
@@ -336,8 +337,8 @@ static int ksz9021_load_values_from_of(struct phy_device *phydev,

static int ksz9021_config_init(struct phy_device *phydev)
{
- struct device *dev = &phydev->dev;
- struct device_node *of_node = dev->of_node;
+ const struct device *dev = &phydev->dev;
+ const struct device_node *of_node = dev->of_node;

if (!of_node && dev->parent->of_node)
of_node = dev->parent->of_node;
@@ -389,9 +390,9 @@ static int ksz9031_extended_read(struct phy_device *phydev,
}

static int ksz9031_of_load_skew_values(struct phy_device *phydev,
- struct device_node *of_node,
+ const struct device_node *of_node,
u16 reg, size_t field_sz,
- char *field[], u8 numfields)
+ const char *field[], u8 numfields)
{
int val[4] = {-1, -2, -3, -4};
int matches = 0;
@@ -427,18 +428,18 @@ static int ksz9031_of_load_skew_values(struct phy_device *phydev,

static int ksz9031_config_init(struct phy_device *phydev)
{
- struct device *dev = &phydev->dev;
- struct device_node *of_node = dev->of_node;
- char *clk_skews[2] = {"rxc-skew-ps", "txc-skew-ps"};
- char *rx_data_skews[4] = {
+ const struct device *dev = &phydev->dev;
+ const struct device_node *of_node = dev->of_node;
+ static const char *clk_skews[2] = {"rxc-skew-ps", "txc-skew-ps"};
+ static const char *rx_data_skews[4] = {
"rxd0-skew-ps", "rxd1-skew-ps",
"rxd2-skew-ps", "rxd3-skew-ps"
};
- char *tx_data_skews[4] = {
+ static const char *tx_data_skews[4] = {
"txd0-skew-ps", "txd1-skew-ps",
"txd2-skew-ps", "txd3-skew-ps"
};
- char *control_skews[2] = {"txen-skew-ps", "rxdv-skew-ps"};
+ static const char *control_skews[2] = {"txen-skew-ps", "rxdv-skew-ps"};

if (!of_node && dev->parent->of_node)
of_node = dev->parent->of_node;
@@ -519,7 +520,7 @@ ksz9021_wr_mmd_phyreg(struct phy_device *phydev, int ptrad, int devnum,
static int kszphy_probe(struct phy_device *phydev)
{
const struct kszphy_type *type = phydev->drv->driver_data;
- struct device_node *np = phydev->dev.of_node;
+ const struct device_node *np = phydev->dev.of_node;
struct kszphy_priv *priv;
struct clk *clk;
int ret;
--
2.1.4

2015-06-05 23:01:28

by Jaeden Amero

[permalink] [raw]
Subject: [PATCH v4 2/3] net/phy: micrel: Comment MMD address of extended registers

There are some defines for a few pad skew related extended registers.
Specify for which MMD Address (dev_addr) they are for.

Signed-off-by: Jaeden Amero <[email protected]>
---
drivers/net/phy/micrel.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 59cc5d4..f23765e 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -366,6 +366,7 @@ static int ksz9021_config_init(struct phy_device *phydev)
#define KSZ9031_PS_TO_REG 60

/* Extended registers */
+/* MMD Address 0x2 */
#define MII_KSZ9031RN_CONTROL_PAD_SKEW 4
#define MII_KSZ9031RN_RX_DATA_PAD_SKEW 5
#define MII_KSZ9031RN_TX_DATA_PAD_SKEW 6
--
2.1.4

2015-06-05 23:00:54

by Jaeden Amero

[permalink] [raw]
Subject: [PATCH v4 3/3] net/phy: micrel: Center FLP timing at 16ms

Link failures have been observed when using the KSZ9031 with HP 1810-8G
and HP 1910-8G network switches. Center the FLP timing at 16ms to help
avoid intermittent link failures.

>From the KSZ9031RNX and KSZ9031MNX data sheets revision 2.2, section
"Auto-Negotiation Timing":
The KSZ9031[RNX or MNX] Fast Link Pulse (FLP) burst-to-burst
transmit timing for Auto-Negotiation defaults to 8ms. IEEE 802.3
Standard specifies this timing to be 16ms +/-8ms. Some PHY link
partners need to receive the FLP with 16ms centered timing;
otherwise, there can be intermittent link failures and long
link-up times.

The PHY data sheet recommends configuring the FLP burst registers after
power-up/reset and immediately thereafter restarting auto-negotiation, so
we center the FLP timing at 16ms and then restart auto-negotiation in the
config_init for KSZ9031.

Signed-off-by: Jaeden Amero <[email protected]>
---
drivers/net/phy/micrel.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index f23765e..499185e 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -366,6 +366,10 @@ static int ksz9021_config_init(struct phy_device *phydev)
#define KSZ9031_PS_TO_REG 60

/* Extended registers */
+/* MMD Address 0x0 */
+#define MII_KSZ9031RN_FLP_BURST_TX_LO 3
+#define MII_KSZ9031RN_FLP_BURST_TX_HI 4
+
/* MMD Address 0x2 */
#define MII_KSZ9031RN_CONTROL_PAD_SKEW 4
#define MII_KSZ9031RN_RX_DATA_PAD_SKEW 5
@@ -427,6 +431,22 @@ static int ksz9031_of_load_skew_values(struct phy_device *phydev,
return ksz9031_extended_write(phydev, OP_DATA, 2, reg, newval);
}

+static int ksz9031_center_flp_timing(struct phy_device *phydev)
+{
+ int result;
+
+ /* Center KSZ9031RNX FLP timing at 16ms. */
+ result = ksz9031_extended_write(phydev, OP_DATA, 0,
+ MII_KSZ9031RN_FLP_BURST_TX_HI, 0x0006);
+ result = ksz9031_extended_write(phydev, OP_DATA, 0,
+ MII_KSZ9031RN_FLP_BURST_TX_LO, 0x1A80);
+
+ if (result)
+ return result;
+
+ return genphy_restart_aneg(phydev);
+}
+
static int ksz9031_config_init(struct phy_device *phydev)
{
const struct device *dev = &phydev->dev;
@@ -462,7 +482,8 @@ static int ksz9031_config_init(struct phy_device *phydev)
MII_KSZ9031RN_TX_DATA_PAD_SKEW, 4,
tx_data_skews, 4);
}
- return 0;
+
+ return ksz9031_center_flp_timing(phydev);
}

#define KSZ8873MLL_GLOBAL_CONTROL_4 0x06
--
2.1.4

2015-06-08 07:51:41

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v4 0/3] net/phy: micrel: Center FLP timing at 16ms

From: Jaeden Amero <[email protected]>
Date: Fri, 5 Jun 2015 18:00:23 -0500

> In v2, we add an additional cleanup commit to make an array of strings
> static const and to improve const correctness generally. We also no longer
> unnecessarily initialize the result variable in
> ksz9031_center_flp_timing().
>
> In v3, we remove the unnecessary result variable from ksz9031_config_init()
> introduced by a previous version of "net/phy: micrel: Center FLP timing at
> 16ms".
>
> In v4, we modify the commit message of "net/phy: micrel: Center FLP timing
> at 16ms" to replace the awkward quotation of the data sheet's programming
> procedure with an explanation of why we program the FLP burst registers and
> restart auto-negotiation where we do (config_init).

Series applied to net-next, thanks.