[PATCH v2 0/3] net/phy: micrel: Center FLP timing at 16ms

netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH v2 0/3] net/phy: micrel: Center FLP timing at 16ms
@ 2015-06-05 19:55 Jaeden Amero
  2015-06-05 19:55 ` [PATCH v2 1/3] net/phy: micrel: Be more const correct Jaeden Amero
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Jaeden Amero @ 2015-06-05 19:55 UTC (permalink / raw)
  To: Florian Fainelli, netdev, linux-kernel; +Cc: Jaeden Amero

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().

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 | 56 +++++++++++++++++++++++++++++++++++-------------
 1 file changed, 41 insertions(+), 15 deletions(-)

-- 
2.1.4

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/3] net/phy: micrel: Be more const correct
  2015-06-05 19:55 [PATCH v2 0/3] net/phy: micrel: Center FLP timing at 16ms Jaeden Amero
@ 2015-06-05 19:55 ` Jaeden Amero
  2015-06-05 19:55 ` [PATCH v2 2/3] net/phy: micrel: Comment MMD address of extended registers Jaeden Amero
  2015-06-05 19:55 ` [PATCH v2 3/3] net/phy: micrel: Center FLP timing at 16ms Jaeden Amero
  2 siblings, 0 replies; 6+ messages in thread
From: Jaeden Amero @ 2015-06-05 19:55 UTC (permalink / raw)
  To: Florian Fainelli, netdev, linux-kernel; +Cc: Jaeden Amero

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 <jaeden.amero@ni.com>
---
 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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 2/3] net/phy: micrel: Comment MMD address of extended registers
  2015-06-05 19:55 [PATCH v2 0/3] net/phy: micrel: Center FLP timing at 16ms Jaeden Amero
  2015-06-05 19:55 ` [PATCH v2 1/3] net/phy: micrel: Be more const correct Jaeden Amero
@ 2015-06-05 19:55 ` Jaeden Amero
  2015-06-05 19:55 ` [PATCH v2 3/3] net/phy: micrel: Center FLP timing at 16ms Jaeden Amero
  2 siblings, 0 replies; 6+ messages in thread
From: Jaeden Amero @ 2015-06-05 19:55 UTC (permalink / raw)
  To: Florian Fainelli, netdev, linux-kernel; +Cc: Jaeden Amero

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 <jaeden.amero@ni.com>
---
 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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 3/3] net/phy: micrel: Center FLP timing at 16ms
  2015-06-05 19:55 [PATCH v2 0/3] net/phy: micrel: Center FLP timing at 16ms Jaeden Amero
  2015-06-05 19:55 ` [PATCH v2 1/3] net/phy: micrel: Be more const correct Jaeden Amero
  2015-06-05 19:55 ` [PATCH v2 2/3] net/phy: micrel: Comment MMD address of extended registers Jaeden Amero
@ 2015-06-05 19:55 ` Jaeden Amero
  2015-06-05 21:09   ` Sergei Shtylyov
  2 siblings, 1 reply; 6+ messages in thread
From: Jaeden Amero @ 2015-06-05 19:55 UTC (permalink / raw)
  To: Florian Fainelli, netdev, linux-kernel; +Cc: Jaeden Amero

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.

	After KSZ9031[RNX or MNX] power-up/reset, program the following
	register sequence to set the FLP timing to 16ms

	Write Register Dh = 0x0000 // Set up register address for MMD – Device Address 0h
	Write Register Eh = 0x0004 // Select Register 4h of MMD – Device Address 0h
	Write Register Dh = 0x4000 // Select register data for MMD – Device Address 0h, Register 4h
	Write Register Eh = 0x0006 // Write value 0x0006 to MMD – Device Address 0h, Register 4h
	Write Register Dh = 0x0000 // Set up register address for MMD – Device Address 0h
	Write Register Eh = 0x0003 // Select Register 3h of MMD – Device Address 0h
	Write Register Dh = 0x4000 // Select register data for MMD – Device Address 0h, Register 3h
	Write Register Eh = 0x1A80 // Write value 0x1A80 to MMD – Device Address 0h, Register 3h
	Write Register 0h, Bit [9] = 1 // Restart Auto-Negotiation

Signed-off-by: Jaeden Amero <jaeden.amero@ni.com>
---
 drivers/net/phy/micrel.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index f23765e..e156e36 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;
@@ -441,6 +461,7 @@ static int ksz9031_config_init(struct phy_device *phydev)
 		"txd2-skew-ps", "txd3-skew-ps"
 	};
 	static const char *control_skews[2] = {"txen-skew-ps", "rxdv-skew-ps"};
+	int result;
 
 	if (!of_node && dev->parent->of_node)
 		of_node = dev->parent->of_node;
@@ -462,7 +483,10 @@ static int ksz9031_config_init(struct phy_device *phydev)
 				MII_KSZ9031RN_TX_DATA_PAD_SKEW, 4,
 				tx_data_skews, 4);
 	}
-	return 0;
+
+	result = ksz9031_center_flp_timing(phydev);
+
+	return result;
 }
 
 #define KSZ8873MLL_GLOBAL_CONTROL_4	0x06
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 3/3] net/phy: micrel: Center FLP timing at 16ms
  2015-06-05 19:55 ` [PATCH v2 3/3] net/phy: micrel: Center FLP timing at 16ms Jaeden Amero
@ 2015-06-05 21:09   ` Sergei Shtylyov
  2015-06-05 22:36     ` Jaeden Amero
  0 siblings, 1 reply; 6+ messages in thread
From: Sergei Shtylyov @ 2015-06-05 21:09 UTC (permalink / raw)
  To: Jaeden Amero, Florian Fainelli, netdev, linux-kernel

Hello.

On 06/05/2015 10:55 PM, Jaeden Amero wrote:

> 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.

> 	After KSZ9031[RNX or MNX] power-up/reset, program the following
> 	register sequence to set the FLP timing to 16ms

> 	Write Register Dh = 0x0000 // Set up register address for MMD – Device Address 0h
> 	Write Register Eh = 0x0004 // Select Register 4h of MMD – Device Address 0h
> 	Write Register Dh = 0x4000 // Select register data for MMD – Device Address 0h, Register 4h
> 	Write Register Eh = 0x0006 // Write value 0x0006 to MMD – Device Address 0h, Register 4h
> 	Write Register Dh = 0x0000 // Set up register address for MMD – Device Address 0h
> 	Write Register Eh = 0x0003 // Select Register 3h of MMD – Device Address 0h
> 	Write Register Dh = 0x4000 // Select register data for MMD – Device Address 0h, Register 3h
> 	Write Register Eh = 0x1A80 // Write value 0x1A80 to MMD – Device Address 0h, Register 3h
> 	Write Register 0h, Bit [9] = 1 // Restart Auto-Negotiation

> Signed-off-by: Jaeden Amero <jaeden.amero@ni.com>
> ---
>   drivers/net/phy/micrel.c | 26 +++++++++++++++++++++++++-
>   1 file changed, 25 insertions(+), 1 deletion(-)

> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index f23765e..e156e36 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
[...]
> @@ -441,6 +461,7 @@ static int ksz9031_config_init(struct phy_device *phydev)
>   		"txd2-skew-ps", "txd3-skew-ps"
>   	};
>   	static const char *control_skews[2] = {"txen-skew-ps", "rxdv-skew-ps"};
> +	int result;

    You hardly need this variable...

>
>   	if (!of_node && dev->parent->of_node)
>   		of_node = dev->parent->of_node;
> @@ -462,7 +483,10 @@ static int ksz9031_config_init(struct phy_device *phydev)
>   				MII_KSZ9031RN_TX_DATA_PAD_SKEW, 4,
>   				tx_data_skews, 4);
>   	}
> -	return 0;
> +
> +	result = ksz9031_center_flp_timing(phydev);
> +
> +	return result;

    Why not just return directly?

[...]

WBR, Sergei

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 3/3] net/phy: micrel: Center FLP timing at 16ms
  2015-06-05 21:09   ` Sergei Shtylyov
@ 2015-06-05 22:36     ` Jaeden Amero
  0 siblings, 0 replies; 6+ messages in thread
From: Jaeden Amero @ 2015-06-05 22:36 UTC (permalink / raw)
  To: Sergei Shtylyov, Florian Fainelli, netdev, linux-kernel

On 06/05/2015 04:09 PM, Sergei Shtylyov wrote:
>> @@ -441,6 +461,7 @@ static int ksz9031_config_init(struct phy_device
>> *phydev)
>>           "txd2-skew-ps", "txd3-skew-ps"
>>       };
>>       static const char *control_skews[2] = {"txen-skew-ps",
>> "rxdv-skew-ps"};
>> +    int result;
> 
>    You hardly need this variable...
> 
>>
>>       if (!of_node && dev->parent->of_node)
>>           of_node = dev->parent->of_node;
>> @@ -462,7 +483,10 @@ static int ksz9031_config_init(struct phy_device
>> *phydev)
>>                   MII_KSZ9031RN_TX_DATA_PAD_SKEW, 4,
>>                   tx_data_skews, 4);
>>       }
>> -    return 0;
>> +
>> +    result = ksz9031_center_flp_timing(phydev);
>> +
>> +    return result;
> 
>    Why not just return directly?
> 

Genius.

Thanks for the feedback. I'll post a v3.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2015-06-05 22:36 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-06-05 19:55 [PATCH v2 0/3] net/phy: micrel: Center FLP timing at 16ms Jaeden Amero
2015-06-05 19:55 ` [PATCH v2 1/3] net/phy: micrel: Be more const correct Jaeden Amero
2015-06-05 19:55 ` [PATCH v2 2/3] net/phy: micrel: Comment MMD address of extended registers Jaeden Amero
2015-06-05 19:55 ` [PATCH v2 3/3] net/phy: micrel: Center FLP timing at 16ms Jaeden Amero
2015-06-05 21:09   ` Sergei Shtylyov
2015-06-05 22:36     ` Jaeden Amero

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).