Skip to contentSkip to conversation

phyter bug fixes

PATCH In review Richard Cochran 2 participants 5 messages May 25, 2015

Subsystems (4)
Networking Network drivers Ethernet PHY PTP hardware clock support
lore
Share & tools
lore

Richard Cochran

author maintainer
#0 lore raw

cover phyter bug fixes

While working on a project using the phyter, I noticed some bugs that have crept in over time. This series fixes those bugs. These patches are also meant for stable.

Thanks,
Richard

Richard Cochran (3):
net: dp83640: fix broken calibration routine.
net: dp83640: reinforce locking rules.
net: dp83640: fix improper double spin locking.

 drivers/net/phy/dp83640.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)
Signature · 1 line
2.1.4
author maintainer
#1 lore raw

PATCH 1/3 net: dp83640: fix broken calibration routine.

Currently, the calibration function that corrects the initial offsets among multiple devices only works the first time. If the function is called more than once, the calibration fails and bogus offsets will be programmed into the devices.

In a well hidden spot, the device documentation tells that trigger indexes 0 and 1 are special in allowing the TRIG_IF_LATE flag to actually work.

This patch fixes the issue by using one of the special triggers during the recalibration method.

Signed-off-by Richard Cochran <[email protected]>
---
 drivers/net/phy/dp83640.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/phy/dp83640.c +1 -1
index 496e02f..7a068d9 100644
@@ -47,7 +47,7 @@
 #define PSF_TX		0x1000
 #define EXT_EVENT	1
 #define CAL_EVENT	7
-#define CAL_TRIGGER	7
+#define CAL_TRIGGER	1
 #define DP83640_N_PINS	12
 
 #define MII_DP83640_MICR 0x11
Signature · 1 line
2.1.4
author maintainer
#2 lore raw

PATCH 2/3 net: dp83640: reinforce locking rules.

In reply to Richard Cochran

Callers of the ext_write function are supposed to hold a mutex that protects the state of the dialed page, but one caller was missing the lock from the very start, and over time the code has been changed without following the rule. This patch cleans up the call sites in violation of the rule.

Signed-off-by Richard Cochran <[email protected]>
---
 drivers/net/phy/dp83640.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)
drivers/net/phy/dp83640.c +16 -1
index 7a068d9..e570036 100644
@@ -496,7 +496,9 @@ static int ptp_dp83640_enable(struct ptp_clock_info *ptp,
 			else
 				evnt |= EVNT_RISE;
 		}
+		mutex_lock(&clock->extreg_lock);
 		ext_write(0, phydev, PAGE5, PTP_EVNT, evnt);
+		mutex_unlock(&clock->extreg_lock);
 		return 0;
 
 	case PTP_CLK_REQ_PEROUT:

@@ -532,6 +534,8 @@ static u8 status_frame_src[6] = { 0x08, 0x00, 0x17, 0x0B, 0x6B, 0x0F };
 
 static void enable_status_frames(struct phy_device *phydev, bool on)
 {
+	struct dp83640_private *dp83640 = phydev->priv;
+	struct dp83640_clock *clock = dp83640->clock;
 	u16 cfg0 = 0, ver;
 
 	if (on)

@@ -539,9 +543,13 @@ static void enable_status_frames(struct phy_device *phydev, bool on)
 
 	ver = (PSF_PTPVER & VERSIONPTP_MASK) << VERSIONPTP_SHIFT;
 
+	mutex_lock(&clock->extreg_lock);
+
 	ext_write(0, phydev, PAGE5, PSF_CFG0, cfg0);
 	ext_write(0, phydev, PAGE6, PSF_CFG1, ver);
 
+	mutex_unlock(&clock->extreg_lock);
+
 	if (!phydev->attached_dev) {
 		pr_warn("expected to find an attached netdevice\n");
 		return;

@@ -1173,11 +1181,18 @@ static int dp83640_config_init(struct phy_device *phydev)
 
 	if (clock->chosen && !list_empty(&clock->phylist))
 		recalibrate(clock);
-	else
+	else {
+		mutex_lock(&clock->extreg_lock);
 		enable_broadcast(phydev, clock->page, 1);
+		mutex_unlock(&clock->extreg_lock);
+	}
 
 	enable_status_frames(phydev, true);
+
+	mutex_lock(&clock->extreg_lock);
 	ext_write(0, phydev, PAGE4, PTP_CTL, PTP_ENABLE);
+	mutex_unlock(&clock->extreg_lock);
+
 	return 0;
 }
Signature · 1 line
2.1.4
author maintainer
#3 lore raw

PATCH 3/3 net: dp83640: fix improper double spin locking.

In reply to Richard Cochran

A pair of nested spin locks was introduced in commit 63502b8d0 "dp83640: Fix receive timestamp race condition".

Unfortunately the 'flags' parameter was reused for the inner lock, clobbering the originally saved IRQ state. This patch fixes the issue by changing the inner lock to plain spin_lock without irqsave.

Signed-off-by Richard Cochran <[email protected]>
---
 drivers/net/phy/dp83640.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/net/phy/dp83640.c +2 -2
index e570036..00cb41e 100644
@@ -846,7 +846,7 @@ static void decode_rxts(struct dp83640_private *dp83640,
 	list_del_init(&rxts->list);
 	phy2rxts(phy_rxts, rxts);
 
-	spin_lock_irqsave(&dp83640->rx_queue.lock, flags);
+	spin_lock(&dp83640->rx_queue.lock);
 	skb_queue_walk(&dp83640->rx_queue, skb) {
 		struct dp83640_skb_info *skb_info;
 

@@ -861,7 +861,7 @@ static void decode_rxts(struct dp83640_private *dp83640,
 			break;
 		}
 	}
-	spin_unlock_irqrestore(&dp83640->rx_queue.lock, flags);
+	spin_unlock(&dp83640->rx_queue.lock);
 
 	if (!shhwtstamps)
 		list_add_tail(&rxts->list, &dp83640->rxts);
Signature · 1 line
2.1.4
maintainer
#4 lore raw
In reply to Richard Cochran

From: Richard Cochran <[email protected]>
Date: Mon, 25 May 2015 11:55:42 +0200

Quoted 3 lines

While working on a project using the phyter, I noticed some bugs that have crept in over time. This series fixes those bugs. These patches are also meant for stable.

Series applied and queued up for -stable as well, thanks.