Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1658183 > unrolled thread
| Started by | "Mario Molitor" <Mario_Molitor@web.de> |
|---|---|
| First post | 2017-06-06 00:20 +0200 |
| Last post | 2017-06-06 07:50 +0200 |
| Articles | 2 — 2 participants |
Back to article view | Back to linux.kernel
Stmmac: fix for hw timestamp of GMAC 3 unit "Mario Molitor" <Mario_Molitor@web.de> - 2017-06-06 00:20 +0200
Re: Stmmac: fix for hw timestamp of GMAC 3 unit Giuseppe CAVALLARO <peppe.cavallaro@st.com> - 2017-06-06 07:50 +0200
| From | "Mario Molitor" <Mario_Molitor@web.de> |
|---|---|
| Date | 2017-06-06 00:20 +0200 |
| Subject | Stmmac: fix for hw timestamp of GMAC 3 unit |
| Message-ID | <tP8vT-e0-3@gated-at.bofh.it> |
[Multipart message — attachments visible in raw view] — view raw
Dear stmmac maintainer group,
I have found an problem in stmmac driver of linux kernel and I hope for a fix in the mainline kernel.
At the moment I have two patch files which fix this problem for me.
The problem seems created with the commit d2042052a0aa6a54f01a0c9e14243ec040b100e2 and ba1ffd74df74a9efa5290f87632a0ed55f1aa387 has breakage the functionality of GMAC3 unit.
I assume that these commits were only tested with a GMAC4 unit.
I have got seen this problem with execution of ptp4l daemon on system with linux 4.11 Kernel.
===============================================================================
root@QuantumXsoc:~ ptp4l -f /etc/ptp.cfg -i eth0 -m
ptp4l[47.928]: selected /dev/ptp0 as PTP clock
ptp4l[47.937]: port 1: INITIALIZING to LISTENING on INITIALIZE
ptp4l[47.938]: port 0: INITIALIZING to LISTENING on INITIALIZE
ptp4l[47.938]: port 1: link up
[ 48.282709] socfpga-dwmac ff702000.ethernet eth0: get valid RX hw timestamp 0
[ 48.316316] socfpga-dwmac ff702000.ethernet eth0: get valid RX hw timestamp 0
[ 48.340260] socfpga-dwmac ff702000.ethernet eth0: get valid RX hw timestamp 0
[ 48.456738] socfpga-dwmac ff702000.ethernet eth0: get valid RX hw timestamp 0
ptp4l[48.457]: port 1: received DELAY_REQ without timestamp
[ 48.488442] socfpga-dwmac ff702000.ethernet eth0: get valid RX hw timestamp 0
[ 48.495599] socfpga-dwmac ff702000.ethernet eth0: get valid RX hw timestamp 0
ptp4l[48.489]: port 1: received SYNC without timestamp
....
================================================================================
I have found two kind of problems and for this two patches (based on the 4.11 kernel) which fix this problem.
1.) PTP_TCR_SNAPTYPSEL_1
The first problem was for my point of view the change of definition PTP_TCR_SNAPTYPSEL_1. I think according the
CYCLON V documention only the bit 16 of snaptypesel should be set. (more information see Table 17-20 (cv_5v4.pdf) : Timestamp Snapshot Dependency on Register Bits)
I believe that the GMAC4 have another necessary definition.
( patch 0001-stmmac-fix-ptp-header-for-GMAC3-hw-timestamp.patch )
================================================================================================
From 2d54d58dc8548d98572eb5fbfe488ec59b4c0ef5 Mon Sep 17 00:00:00 2001
From: Mario Molitor <mario_molitor@web.de>
Date: Mon, 5 Jun 2017 18:58:49 +0200
Subject: [PATCH 1/2] stmmac: fix ptp header for GMAC3 hw timestamp
According the CYCLON V documention only the bit 16 of snaptypesel should set.
(more information see Table 17-20 (cv_5v4.pdf) : Timestamp Snapshot Dependency on Register Bits)
fixed: d2042052a0aa6a54f01a0c9e14243ec040b100e2
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 15 ++++++++++++---
drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h | 3 ++-
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 4498a38..13a1ac9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -471,7 +471,10 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev, struct ifreq *ifr)
/* PTP v1, UDP, any kind of event packet */
config.rx_filter = HWTSTAMP_FILTER_PTP_V1_L4_EVENT;
/* take time stamp for all event messages */
- snap_type_sel = PTP_TCR_SNAPTYPSEL_1;
+ if (priv->plat->has_gmac4)
+ snap_type_sel = PTP_GMAC4_TCR_SNAPTYPSEL_1;
+ else
+ snap_type_sel = PTP_TCR_SNAPTYPSEL_1;
ptp_over_ipv4_udp = PTP_TCR_TSIPV4ENA;
ptp_over_ipv6_udp = PTP_TCR_TSIPV6ENA;
@@ -503,7 +506,10 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev, struct ifreq *ifr)
config.rx_filter = HWTSTAMP_FILTER_PTP_V2_L4_EVENT;
ptp_v2 = PTP_TCR_TSVER2ENA;
/* take time stamp for all event messages */
- snap_type_sel = PTP_TCR_SNAPTYPSEL_1;
+ if (priv->plat->has_gmac4)
+ snap_type_sel = PTP_GMAC4_TCR_SNAPTYPSEL_1;
+ else
+ snap_type_sel = PTP_TCR_SNAPTYPSEL_1;
ptp_over_ipv4_udp = PTP_TCR_TSIPV4ENA;
ptp_over_ipv6_udp = PTP_TCR_TSIPV6ENA;
@@ -537,7 +543,10 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev, struct ifreq *ifr)
config.rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
ptp_v2 = PTP_TCR_TSVER2ENA;
/* take time stamp for all event messages */
- snap_type_sel = PTP_TCR_SNAPTYPSEL_1;
+ if (priv->plat->has_gmac4)
+ snap_type_sel = PTP_GMAC4_TCR_SNAPTYPSEL_1;
+ else
+ snap_type_sel = PTP_TCR_SNAPTYPSEL_1;
ptp_over_ipv4_udp = PTP_TCR_TSIPV4ENA;
ptp_over_ipv6_udp = PTP_TCR_TSIPV6ENA;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h
index 48fb72f..f4b31d6 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h
@@ -59,7 +59,8 @@
/* Enable Snapshot for Messages Relevant to Master */
#define PTP_TCR_TSMSTRENA BIT(15)
/* Select PTP packets for Taking Snapshots */
-#define PTP_TCR_SNAPTYPSEL_1 GENMASK(17, 16)
+#define PTP_TCR_SNAPTYPSEL_1 BIT(16)
+#define PTP_GMAC4_TCR_SNAPTYPSEL_1 GENMASK(17, 16)
/* Enable MAC address for PTP Frame Filtering */
#define PTP_TCR_TSENMACADDR BIT(18)
--
2.7.4
================================================================================================
2.) stmmac_get_rx_hwtstamp() and stmmac_get_tx_hwtstamp()
The second problem what i have got seen that the logic of timestamp available check was changed to a negative logic.
I have changed this in the function stmmac_get_rx_hwtstamp() and stmmac_get_tx_hwtstamp().
This changes fix the Problem with GMAC3, but to prevent a breakage of GMAC4 it is necessary to addapt the functions dwmac4_wrback_get_rx_timestamp_status() and dwmac4_wrback_get_tx_timestamp_status().
I have also change printout from info-level to the debug-level.
( patch 0002-stmmac-fix-for-hw-timestamp-of-GMAC3-unit.patch )
================================================================================================
From ca39f10d78840192ffe801e42a813d8e4939c3e2 Mon Sep 17 00:00:00 2001
From: Mario Molitor <mario_molitor@web.de>
Date: Mon, 5 Jun 2017 19:10:23 +0200
Subject: [PATCH 2/2] stmmac: fix for hw timestamp of GMAC3 unit
1.) Bugfix of function stmmac_get_tx_hwtstamp.
Corrected the tx timestamp available check (same as 4.8 and older)
Change printout from info syslevel to debug.
2.) Bugfix of function stmmac_get_rx_hwtstamp.
Corrected the rx timestamp available check (same as 4.8 and older)
Change printout from info syslevel to debug.
fixed: ba1ffd74df74a9efa5290f87632a0ed55f1aa387
---
drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c | 11 +++++++----
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 10 +++++-----
2 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
index 843ec69..cd0c0ee 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
@@ -214,13 +214,13 @@ static int dwmac4_wrback_get_tx_timestamp_status(struct dma_desc *p)
{
/* Context type from W/B descriptor must be zero */
if (le32_to_cpu(p->des3) & TDES3_CONTEXT_TYPE)
- return -EINVAL;
+ return 0;
/* Tx Timestamp Status is 1 so des0 and des1'll have valid values */
if (le32_to_cpu(p->des3) & TDES3_TIMESTAMP_STATUS)
- return 0;
+ return 1;
- return 1;
+ return 0;
}
static inline u64 dwmac4_get_timestamp(void *desc, u32 ats)
@@ -282,7 +282,10 @@ static int dwmac4_wrback_get_rx_timestamp_status(void *desc, u32 ats)
}
}
exit:
- return ret;
+ if (likely(ret == 0))
+ return 1;
+ else
+ return 0;
}
static void dwmac4_rd_init_rx_desc(struct dma_desc *p, int disable_rx_ic,
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 13a1ac9..84561e0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -359,14 +359,14 @@ static void stmmac_get_tx_hwtstamp(struct stmmac_priv *priv,
return;
/* check tx tstamp status */
- if (!priv->hw->desc->get_tx_timestamp_status(p)) {
+ if (priv->hw->desc->get_tx_timestamp_status(p)) {
/* get the valid tstamp */
ns = priv->hw->desc->get_timestamp(p, priv->adv_ts);
memset(&shhwtstamp, 0, sizeof(struct skb_shared_hwtstamps));
shhwtstamp.hwtstamp = ns_to_ktime(ns);
- netdev_info(priv->dev, "get valid TX hw timestamp %llu\n", ns);
+ netdev_dbg(priv->dev, "get valid TX hw timestamp %llu\n", ns);
/* pass tstamp to stack */
skb_tstamp_tx(skb, &shhwtstamp);
}
@@ -393,19 +393,19 @@ static void stmmac_get_rx_hwtstamp(struct stmmac_priv *priv, struct dma_desc *p,
return;
/* Check if timestamp is available */
- if (!priv->hw->desc->get_rx_timestamp_status(p, priv->adv_ts)) {
+ if (priv->hw->desc->get_rx_timestamp_status(p, priv->adv_ts)) {
/* For GMAC4, the valid timestamp is from CTX next desc. */
if (priv->plat->has_gmac4)
ns = priv->hw->desc->get_timestamp(np, priv->adv_ts);
else
ns = priv->hw->desc->get_timestamp(p, priv->adv_ts);
- netdev_info(priv->dev, "get valid RX hw timestamp %llu\n", ns);
+ netdev_dbg(priv->dev, "get valid RX hw timestamp %llu\n", ns);
shhwtstamp = skb_hwtstamps(skb);
memset(shhwtstamp, 0, sizeof(struct skb_shared_hwtstamps));
shhwtstamp->hwtstamp = ns_to_ktime(ns);
} else {
- netdev_err(priv->dev, "cannot get RX hw timestamp\n");
+ netdev_dbg(priv->dev, "cannot get RX hw timestamp\n");
}
}
--
2.7.4
================================================================================================
My problem is that I can't test this change with GMAC 4 unit, because I have only GMAC3 hardware.
I verified the operation only with Cyclone V SoC Development Kit.
I look forward to receiving constructive comments and I hope I can help to fix this problem on the mainline kernel.
Best regards,
Mario Molitor
[toc] | [next] | [standalone]
| From | Giuseppe CAVALLARO <peppe.cavallaro@st.com> |
|---|---|
| Date | 2017-06-06 07:50 +0200 |
| Message-ID | <tPfxo-4CR-1@gated-at.bofh.it> |
| In reply to | #1658183 |
Hi Mario thanks for your tests, and, at first glance, your patches seem to be sensible so, please, send the changes as patches for net.git kernel. Regards Peppe On 6/6/2017 12:11 AM, Mario Molitor wrote: > Dear stmmac maintainer group, > > I have found an problem in stmmac driver of linux kernel and I hope for a fix in the mainline kernel. > At the moment I have two patch files which fix this problem for me. > The problem seems created with the commit d2042052a0aa6a54f01a0c9e14243ec040b100e2 and ba1ffd74df74a9efa5290f87632a0ed55f1aa387 has breakage the functionality of GMAC3 unit. > I assume that these commits were only tested with a GMAC4 unit. > I have got seen this problem with execution of ptp4l daemon on system with linux 4.11 Kernel. > > =============================================================================== > root@QuantumXsoc:~ ptp4l -f /etc/ptp.cfg -i eth0 -m > ptp4l[47.928]: selected /dev/ptp0 as PTP clock > ptp4l[47.937]: port 1: INITIALIZING to LISTENING on INITIALIZE > ptp4l[47.938]: port 0: INITIALIZING to LISTENING on INITIALIZE > ptp4l[47.938]: port 1: link up > [ 48.282709] socfpga-dwmac ff702000.ethernet eth0: get valid RX hw timestamp 0 > [ 48.316316] socfpga-dwmac ff702000.ethernet eth0: get valid RX hw timestamp 0 > [ 48.340260] socfpga-dwmac ff702000.ethernet eth0: get valid RX hw timestamp 0 > [ 48.456738] socfpga-dwmac ff702000.ethernet eth0: get valid RX hw timestamp 0 > ptp4l[48.457]: port 1: received DELAY_REQ without timestamp > [ 48.488442] socfpga-dwmac ff702000.ethernet eth0: get valid RX hw timestamp 0 > [ 48.495599] socfpga-dwmac ff702000.ethernet eth0: get valid RX hw timestamp 0 > ptp4l[48.489]: port 1: received SYNC without timestamp > .... > ================================================================================ > > I have found two kind of problems and for this two patches (based on the 4.11 kernel) which fix this problem. > > 1.) PTP_TCR_SNAPTYPSEL_1 > > The first problem was for my point of view the change of definition PTP_TCR_SNAPTYPSEL_1. I think according the > CYCLON V documention only the bit 16 of snaptypesel should be set. (more information see Table 17-20 (cv_5v4.pdf) : Timestamp Snapshot Dependency on Register Bits) > I believe that the GMAC4 have another necessary definition. > > ( patch 0001-stmmac-fix-ptp-header-for-GMAC3-hw-timestamp.patch ) > ================================================================================================ > >From 2d54d58dc8548d98572eb5fbfe488ec59b4c0ef5 Mon Sep 17 00:00:00 2001 > From: Mario Molitor <mario_molitor@web.de> > Date: Mon, 5 Jun 2017 18:58:49 +0200 > Subject: [PATCH 1/2] stmmac: fix ptp header for GMAC3 hw timestamp > > According the CYCLON V documention only the bit 16 of snaptypesel should set. > (more information see Table 17-20 (cv_5v4.pdf) : Timestamp Snapshot Dependency on Register Bits) > > fixed: d2042052a0aa6a54f01a0c9e14243ec040b100e2 > --- > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 15 ++++++++++++--- > drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h | 3 ++- > 2 files changed, 14 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index 4498a38..13a1ac9 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > @@ -471,7 +471,10 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev, struct ifreq *ifr) > /* PTP v1, UDP, any kind of event packet */ > config.rx_filter = HWTSTAMP_FILTER_PTP_V1_L4_EVENT; > /* take time stamp for all event messages */ > - snap_type_sel = PTP_TCR_SNAPTYPSEL_1; > + if (priv->plat->has_gmac4) > + snap_type_sel = PTP_GMAC4_TCR_SNAPTYPSEL_1; > + else > + snap_type_sel = PTP_TCR_SNAPTYPSEL_1; > > ptp_over_ipv4_udp = PTP_TCR_TSIPV4ENA; > ptp_over_ipv6_udp = PTP_TCR_TSIPV6ENA; > @@ -503,7 +506,10 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev, struct ifreq *ifr) > config.rx_filter = HWTSTAMP_FILTER_PTP_V2_L4_EVENT; > ptp_v2 = PTP_TCR_TSVER2ENA; > /* take time stamp for all event messages */ > - snap_type_sel = PTP_TCR_SNAPTYPSEL_1; > + if (priv->plat->has_gmac4) > + snap_type_sel = PTP_GMAC4_TCR_SNAPTYPSEL_1; > + else > + snap_type_sel = PTP_TCR_SNAPTYPSEL_1; > > ptp_over_ipv4_udp = PTP_TCR_TSIPV4ENA; > ptp_over_ipv6_udp = PTP_TCR_TSIPV6ENA; > @@ -537,7 +543,10 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev, struct ifreq *ifr) > config.rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT; > ptp_v2 = PTP_TCR_TSVER2ENA; > /* take time stamp for all event messages */ > - snap_type_sel = PTP_TCR_SNAPTYPSEL_1; > + if (priv->plat->has_gmac4) > + snap_type_sel = PTP_GMAC4_TCR_SNAPTYPSEL_1; > + else > + snap_type_sel = PTP_TCR_SNAPTYPSEL_1; > > ptp_over_ipv4_udp = PTP_TCR_TSIPV4ENA; > ptp_over_ipv6_udp = PTP_TCR_TSIPV6ENA; > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h > index 48fb72f..f4b31d6 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h > @@ -59,7 +59,8 @@ > /* Enable Snapshot for Messages Relevant to Master */ > #define PTP_TCR_TSMSTRENA BIT(15) > /* Select PTP packets for Taking Snapshots */ > -#define PTP_TCR_SNAPTYPSEL_1 GENMASK(17, 16) > +#define PTP_TCR_SNAPTYPSEL_1 BIT(16) > +#define PTP_GMAC4_TCR_SNAPTYPSEL_1 GENMASK(17, 16) > /* Enable MAC address for PTP Frame Filtering */ > #define PTP_TCR_TSENMACADDR BIT(18) >
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web