* [PATCH v1 net-next 0/3] net: stmmac: re-configure tas basetime after ptp time adjust
@ 2021-06-01 8:38 ` Xiaoliang Yang
0 siblings, 0 replies; 18+ messages in thread
From: Xiaoliang Yang @ 2021-06-01 8:38 UTC (permalink / raw)
To: davem, joabreu, kuba, alexandre.torgue, peppe.cavallaro, mcoquelin.stm32
Cc: netdev, boon.leong.ong, weifeng.voon, vee.khee.wong, tee.min.tan,
mohammad.athari.ismail, linux-stm32, linux-arm-kernel,
linux-kernel, leoyang.li, vladimir.oltean, qiangqing.zhang,
rui.sousa, mingkai.hu, yangbo.lu, xiaoliang.yang_1
If the DWMAC Ethernet device has already set the Qbv EST configuration
before using ptp to synchronize the time adjustment, the Qbv base time
may change to be the past time of the new current time. This is not
allowed by hardware.
This patch calculates and re-configures the Qbv basetime after ptp time
adjustment.
Xiaoliang Yang (3):
net: stmmac: separate the tas basetime calculation function
net: stmmac: add mutex lock to protect est parameters
net: stmmac: ptp: update tas basetime after ptp adjust
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 3 ++
.../net/ethernet/stmicro/stmmac/stmmac_ptp.c | 41 ++++++++++++++++-
.../net/ethernet/stmicro/stmmac/stmmac_tc.c | 46 +++++++++++++------
include/linux/stmmac.h | 1 +
4 files changed, 77 insertions(+), 14 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v1 net-next 0/3] net: stmmac: re-configure tas basetime after ptp time adjust
@ 2021-06-01 8:38 ` Xiaoliang Yang
0 siblings, 0 replies; 18+ messages in thread
From: Xiaoliang Yang @ 2021-06-01 8:38 UTC (permalink / raw)
To: davem, joabreu, kuba, alexandre.torgue, peppe.cavallaro, mcoquelin.stm32
Cc: netdev, boon.leong.ong, weifeng.voon, vee.khee.wong, tee.min.tan,
mohammad.athari.ismail, linux-stm32, linux-arm-kernel,
linux-kernel, leoyang.li, vladimir.oltean, qiangqing.zhang,
rui.sousa, mingkai.hu, yangbo.lu, xiaoliang.yang_1
If the DWMAC Ethernet device has already set the Qbv EST configuration
before using ptp to synchronize the time adjustment, the Qbv base time
may change to be the past time of the new current time. This is not
allowed by hardware.
This patch calculates and re-configures the Qbv basetime after ptp time
adjustment.
Xiaoliang Yang (3):
net: stmmac: separate the tas basetime calculation function
net: stmmac: add mutex lock to protect est parameters
net: stmmac: ptp: update tas basetime after ptp adjust
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 3 ++
.../net/ethernet/stmicro/stmmac/stmmac_ptp.c | 41 ++++++++++++++++-
.../net/ethernet/stmicro/stmmac/stmmac_tc.c | 46 +++++++++++++------
include/linux/stmmac.h | 1 +
4 files changed, 77 insertions(+), 14 deletions(-)
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v1 net-next 1/3] net: stmmac: separate the tas basetime calculation function
2021-06-01 8:38 ` Xiaoliang Yang
@ 2021-06-01 8:38 ` Xiaoliang Yang
-1 siblings, 0 replies; 18+ messages in thread
From: Xiaoliang Yang @ 2021-06-01 8:38 UTC (permalink / raw)
To: davem, joabreu, kuba, alexandre.torgue, peppe.cavallaro, mcoquelin.stm32
Cc: netdev, boon.leong.ong, weifeng.voon, vee.khee.wong, tee.min.tan,
mohammad.athari.ismail, linux-stm32, linux-arm-kernel,
linux-kernel, leoyang.li, vladimir.oltean, qiangqing.zhang,
rui.sousa, mingkai.hu, yangbo.lu, xiaoliang.yang_1
Separate the TAS basetime calculation function so that it can be
called by other functions.
Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 3 ++
.../net/ethernet/stmicro/stmmac/stmmac_tc.c | 38 ++++++++++++-------
2 files changed, 28 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index b6cd43eda7ac..1ce25c95cb06 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -355,6 +355,9 @@ void stmmac_selftest_run(struct net_device *dev,
void stmmac_selftest_get_strings(struct stmmac_priv *priv, u8 *data);
int stmmac_selftest_get_count(struct stmmac_priv *priv);
#else
+struct timespec64 stmmac_calc_tas_basetime(ktime_t old_base_time,
+ ktime_t current_time,
+ u64 cycle_time);
static inline void stmmac_selftest_run(struct net_device *dev,
struct ethtool_test *etest, u64 *buf)
{
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
index 4e70efc45458..d7d448c5a72b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
@@ -713,6 +713,29 @@ static int tc_setup_cls(struct stmmac_priv *priv,
return ret;
}
+struct timespec64 stmmac_calc_tas_basetime(ktime_t old_base_time,
+ ktime_t current_time,
+ u64 cycle_time)
+{
+ struct timespec64 time;
+
+ if (ktime_after(old_base_time, current_time)) {
+ time = ktime_to_timespec64(old_base_time);
+ } else {
+ s64 n;
+ ktime_t base_time;
+
+ n = div64_s64(ktime_sub_ns(current_time, old_base_time),
+ cycle_time);
+ base_time = ktime_add_ns(old_base_time,
+ (n + 1) * cycle_time);
+
+ time = ktime_to_timespec64(base_time);
+ }
+
+ return time;
+}
+
static int tc_setup_taprio(struct stmmac_priv *priv,
struct tc_taprio_qopt_offload *qopt)
{
@@ -816,19 +839,8 @@ static int tc_setup_taprio(struct stmmac_priv *priv,
/* Adjust for real system time */
priv->ptp_clock_ops.gettime64(&priv->ptp_clock_ops, ¤t_time);
current_time_ns = timespec64_to_ktime(current_time);
- if (ktime_after(qopt->base_time, current_time_ns)) {
- time = ktime_to_timespec64(qopt->base_time);
- } else {
- ktime_t base_time;
- s64 n;
-
- n = div64_s64(ktime_sub_ns(current_time_ns, qopt->base_time),
- qopt->cycle_time);
- base_time = ktime_add_ns(qopt->base_time,
- (n + 1) * qopt->cycle_time);
-
- time = ktime_to_timespec64(base_time);
- }
+ time = stmmac_calc_tas_basetime(qopt->base_time, current_time_ns,
+ qopt->cycle_time);
priv->plat->est->btr[0] = (u32)time.tv_nsec;
priv->plat->est->btr[1] = (u32)time.tv_sec;
--
2.17.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v1 net-next 1/3] net: stmmac: separate the tas basetime calculation function
@ 2021-06-01 8:38 ` Xiaoliang Yang
0 siblings, 0 replies; 18+ messages in thread
From: Xiaoliang Yang @ 2021-06-01 8:38 UTC (permalink / raw)
To: davem, joabreu, kuba, alexandre.torgue, peppe.cavallaro, mcoquelin.stm32
Cc: netdev, boon.leong.ong, weifeng.voon, vee.khee.wong, tee.min.tan,
mohammad.athari.ismail, linux-stm32, linux-arm-kernel,
linux-kernel, leoyang.li, vladimir.oltean, qiangqing.zhang,
rui.sousa, mingkai.hu, yangbo.lu, xiaoliang.yang_1
Separate the TAS basetime calculation function so that it can be
called by other functions.
Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 3 ++
.../net/ethernet/stmicro/stmmac/stmmac_tc.c | 38 ++++++++++++-------
2 files changed, 28 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index b6cd43eda7ac..1ce25c95cb06 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -355,6 +355,9 @@ void stmmac_selftest_run(struct net_device *dev,
void stmmac_selftest_get_strings(struct stmmac_priv *priv, u8 *data);
int stmmac_selftest_get_count(struct stmmac_priv *priv);
#else
+struct timespec64 stmmac_calc_tas_basetime(ktime_t old_base_time,
+ ktime_t current_time,
+ u64 cycle_time);
static inline void stmmac_selftest_run(struct net_device *dev,
struct ethtool_test *etest, u64 *buf)
{
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
index 4e70efc45458..d7d448c5a72b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
@@ -713,6 +713,29 @@ static int tc_setup_cls(struct stmmac_priv *priv,
return ret;
}
+struct timespec64 stmmac_calc_tas_basetime(ktime_t old_base_time,
+ ktime_t current_time,
+ u64 cycle_time)
+{
+ struct timespec64 time;
+
+ if (ktime_after(old_base_time, current_time)) {
+ time = ktime_to_timespec64(old_base_time);
+ } else {
+ s64 n;
+ ktime_t base_time;
+
+ n = div64_s64(ktime_sub_ns(current_time, old_base_time),
+ cycle_time);
+ base_time = ktime_add_ns(old_base_time,
+ (n + 1) * cycle_time);
+
+ time = ktime_to_timespec64(base_time);
+ }
+
+ return time;
+}
+
static int tc_setup_taprio(struct stmmac_priv *priv,
struct tc_taprio_qopt_offload *qopt)
{
@@ -816,19 +839,8 @@ static int tc_setup_taprio(struct stmmac_priv *priv,
/* Adjust for real system time */
priv->ptp_clock_ops.gettime64(&priv->ptp_clock_ops, ¤t_time);
current_time_ns = timespec64_to_ktime(current_time);
- if (ktime_after(qopt->base_time, current_time_ns)) {
- time = ktime_to_timespec64(qopt->base_time);
- } else {
- ktime_t base_time;
- s64 n;
-
- n = div64_s64(ktime_sub_ns(current_time_ns, qopt->base_time),
- qopt->cycle_time);
- base_time = ktime_add_ns(qopt->base_time,
- (n + 1) * qopt->cycle_time);
-
- time = ktime_to_timespec64(base_time);
- }
+ time = stmmac_calc_tas_basetime(qopt->base_time, current_time_ns,
+ qopt->cycle_time);
priv->plat->est->btr[0] = (u32)time.tv_nsec;
priv->plat->est->btr[1] = (u32)time.tv_sec;
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v1 net-next 2/3] net: stmmac: add mutex lock to protect est parameters
2021-06-01 8:38 ` Xiaoliang Yang
@ 2021-06-01 8:38 ` Xiaoliang Yang
-1 siblings, 0 replies; 18+ messages in thread
From: Xiaoliang Yang @ 2021-06-01 8:38 UTC (permalink / raw)
To: davem, joabreu, kuba, alexandre.torgue, peppe.cavallaro, mcoquelin.stm32
Cc: netdev, boon.leong.ong, weifeng.voon, vee.khee.wong, tee.min.tan,
mohammad.athari.ismail, linux-stm32, linux-arm-kernel,
linux-kernel, leoyang.li, vladimir.oltean, qiangqing.zhang,
rui.sousa, mingkai.hu, yangbo.lu, xiaoliang.yang_1
Add a mutex lock to protect est structure parameters so that the
EST parameters can be updated by other threads.
Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c | 8 ++++++++
include/linux/stmmac.h | 1 +
2 files changed, 9 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
index d7d448c5a72b..8e1f9a18ef59 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
@@ -798,14 +798,18 @@ static int tc_setup_taprio(struct stmmac_priv *priv,
GFP_KERNEL);
if (!plat->est)
return -ENOMEM;
+
+ mutex_init(&priv->plat->est->lock);
} else {
memset(plat->est, 0, sizeof(*plat->est));
}
size = qopt->num_entries;
+ mutex_lock(&priv->plat->est->lock);
priv->plat->est->gcl_size = size;
priv->plat->est->enable = qopt->enable;
+ mutex_unlock(&priv->plat->est->lock);
for (i = 0; i < size; i++) {
s64 delta_ns = qopt->entries[i].interval;
@@ -842,6 +846,7 @@ static int tc_setup_taprio(struct stmmac_priv *priv,
time = stmmac_calc_tas_basetime(qopt->base_time, current_time_ns,
qopt->cycle_time);
+ mutex_lock(&priv->plat->est->lock);
priv->plat->est->btr[0] = (u32)time.tv_nsec;
priv->plat->est->btr[1] = (u32)time.tv_sec;
@@ -859,6 +864,7 @@ static int tc_setup_taprio(struct stmmac_priv *priv,
ret = stmmac_est_configure(priv, priv->ioaddr, priv->plat->est,
priv->plat->clk_ptp_rate);
+ mutex_unlock(&priv->plat->est->lock);
if (ret) {
netdev_err(priv->dev, "failed to configure EST\n");
goto disable;
@@ -874,9 +880,11 @@ static int tc_setup_taprio(struct stmmac_priv *priv,
return 0;
disable:
+ mutex_lock(&priv->plat->est->lock);
priv->plat->est->enable = false;
stmmac_est_configure(priv, priv->ioaddr, priv->plat->est,
priv->plat->clk_ptp_rate);
+ mutex_unlock(&priv->plat->est->lock);
priv->plat->fpe_cfg->enable = false;
stmmac_fpe_configure(priv, priv->ioaddr,
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index e14a12df381b..c38b65aaf8c2 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -115,6 +115,7 @@ struct stmmac_axi {
#define EST_GCL 1024
struct stmmac_est {
+ struct mutex lock;
int enable;
u32 btr_offset[2];
u32 btr[2];
--
2.17.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v1 net-next 2/3] net: stmmac: add mutex lock to protect est parameters
@ 2021-06-01 8:38 ` Xiaoliang Yang
0 siblings, 0 replies; 18+ messages in thread
From: Xiaoliang Yang @ 2021-06-01 8:38 UTC (permalink / raw)
To: davem, joabreu, kuba, alexandre.torgue, peppe.cavallaro, mcoquelin.stm32
Cc: netdev, boon.leong.ong, weifeng.voon, vee.khee.wong, tee.min.tan,
mohammad.athari.ismail, linux-stm32, linux-arm-kernel,
linux-kernel, leoyang.li, vladimir.oltean, qiangqing.zhang,
rui.sousa, mingkai.hu, yangbo.lu, xiaoliang.yang_1
Add a mutex lock to protect est structure parameters so that the
EST parameters can be updated by other threads.
Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c | 8 ++++++++
include/linux/stmmac.h | 1 +
2 files changed, 9 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
index d7d448c5a72b..8e1f9a18ef59 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
@@ -798,14 +798,18 @@ static int tc_setup_taprio(struct stmmac_priv *priv,
GFP_KERNEL);
if (!plat->est)
return -ENOMEM;
+
+ mutex_init(&priv->plat->est->lock);
} else {
memset(plat->est, 0, sizeof(*plat->est));
}
size = qopt->num_entries;
+ mutex_lock(&priv->plat->est->lock);
priv->plat->est->gcl_size = size;
priv->plat->est->enable = qopt->enable;
+ mutex_unlock(&priv->plat->est->lock);
for (i = 0; i < size; i++) {
s64 delta_ns = qopt->entries[i].interval;
@@ -842,6 +846,7 @@ static int tc_setup_taprio(struct stmmac_priv *priv,
time = stmmac_calc_tas_basetime(qopt->base_time, current_time_ns,
qopt->cycle_time);
+ mutex_lock(&priv->plat->est->lock);
priv->plat->est->btr[0] = (u32)time.tv_nsec;
priv->plat->est->btr[1] = (u32)time.tv_sec;
@@ -859,6 +864,7 @@ static int tc_setup_taprio(struct stmmac_priv *priv,
ret = stmmac_est_configure(priv, priv->ioaddr, priv->plat->est,
priv->plat->clk_ptp_rate);
+ mutex_unlock(&priv->plat->est->lock);
if (ret) {
netdev_err(priv->dev, "failed to configure EST\n");
goto disable;
@@ -874,9 +880,11 @@ static int tc_setup_taprio(struct stmmac_priv *priv,
return 0;
disable:
+ mutex_lock(&priv->plat->est->lock);
priv->plat->est->enable = false;
stmmac_est_configure(priv, priv->ioaddr, priv->plat->est,
priv->plat->clk_ptp_rate);
+ mutex_unlock(&priv->plat->est->lock);
priv->plat->fpe_cfg->enable = false;
stmmac_fpe_configure(priv, priv->ioaddr,
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index e14a12df381b..c38b65aaf8c2 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -115,6 +115,7 @@ struct stmmac_axi {
#define EST_GCL 1024
struct stmmac_est {
+ struct mutex lock;
int enable;
u32 btr_offset[2];
u32 btr[2];
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v1 net-next 3/3] net: stmmac: ptp: update tas basetime after ptp adjust
2021-06-01 8:38 ` Xiaoliang Yang
@ 2021-06-01 8:38 ` Xiaoliang Yang
-1 siblings, 0 replies; 18+ messages in thread
From: Xiaoliang Yang @ 2021-06-01 8:38 UTC (permalink / raw)
To: davem, joabreu, kuba, alexandre.torgue, peppe.cavallaro, mcoquelin.stm32
Cc: netdev, boon.leong.ong, weifeng.voon, vee.khee.wong, tee.min.tan,
mohammad.athari.ismail, linux-stm32, linux-arm-kernel,
linux-kernel, leoyang.li, vladimir.oltean, qiangqing.zhang,
rui.sousa, mingkai.hu, yangbo.lu, xiaoliang.yang_1
After adjusting the ptp time, the Qbv base time may be the past time
of the new current time. dwmac5 hardware limited the base time cannot
be set as past time. This patch calculate the base time and reset the
Qbv configuration after ptp time adjust.
Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
---
.../net/ethernet/stmicro/stmmac/stmmac_ptp.c | 41 ++++++++++++++++++-
1 file changed, 40 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
index 4e86cdf2bc9f..c573bc8b2595 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
@@ -62,7 +62,8 @@ static int stmmac_adjust_time(struct ptp_clock_info *ptp, s64 delta)
u32 sec, nsec;
u32 quotient, reminder;
int neg_adj = 0;
- bool xmac;
+ bool xmac, est_rst = false;
+ int ret;
xmac = priv->plat->has_gmac4 || priv->plat->has_xgmac;
@@ -75,10 +76,48 @@ static int stmmac_adjust_time(struct ptp_clock_info *ptp, s64 delta)
sec = quotient;
nsec = reminder;
+ /* If EST is enabled, disabled it before adjust ptp time. */
+ if (priv->plat->est && priv->plat->est->enable) {
+ est_rst = true;
+ mutex_lock(&priv->plat->est->lock);
+ priv->plat->est->enable = false;
+ stmmac_est_configure(priv, priv->ioaddr, priv->plat->est,
+ priv->plat->clk_ptp_rate);
+ mutex_unlock(&priv->plat->est->lock);
+ }
+
spin_lock_irqsave(&priv->ptp_lock, flags);
stmmac_adjust_systime(priv, priv->ptpaddr, sec, nsec, neg_adj, xmac);
spin_unlock_irqrestore(&priv->ptp_lock, flags);
+ /* Caculate new basetime and re-configured EST after PTP time adjust. */
+ if (est_rst) {
+ struct timespec64 current_time, time;
+ ktime_t current_time_ns, basetime;
+ u64 cycle_time;
+
+ priv->ptp_clock_ops.gettime64(&priv->ptp_clock_ops, ¤t_time);
+ current_time_ns = timespec64_to_ktime(current_time);
+ time.tv_nsec = priv->plat->est->btr[0];
+ time.tv_sec = priv->plat->est->btr[1];
+ basetime = timespec64_to_ktime(time);
+ cycle_time = priv->plat->est->ctr[1] * NSEC_PER_SEC +
+ priv->plat->est->ctr[0];
+ time = stmmac_calc_tas_basetime(basetime,
+ current_time_ns,
+ cycle_time);
+
+ mutex_lock(&priv->plat->est->lock);
+ priv->plat->est->btr[0] = (u32)time.tv_nsec;
+ priv->plat->est->btr[1] = (u32)time.tv_sec;
+ priv->plat->est->enable = true;
+ ret = stmmac_est_configure(priv, priv->ioaddr, priv->plat->est,
+ priv->plat->clk_ptp_rate);
+ mutex_unlock(&priv->plat->est->lock);
+ if (ret)
+ netdev_err(priv->dev, "failed to configure EST\n");
+ }
+
return 0;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v1 net-next 3/3] net: stmmac: ptp: update tas basetime after ptp adjust
@ 2021-06-01 8:38 ` Xiaoliang Yang
0 siblings, 0 replies; 18+ messages in thread
From: Xiaoliang Yang @ 2021-06-01 8:38 UTC (permalink / raw)
To: davem, joabreu, kuba, alexandre.torgue, peppe.cavallaro, mcoquelin.stm32
Cc: netdev, boon.leong.ong, weifeng.voon, vee.khee.wong, tee.min.tan,
mohammad.athari.ismail, linux-stm32, linux-arm-kernel,
linux-kernel, leoyang.li, vladimir.oltean, qiangqing.zhang,
rui.sousa, mingkai.hu, yangbo.lu, xiaoliang.yang_1
After adjusting the ptp time, the Qbv base time may be the past time
of the new current time. dwmac5 hardware limited the base time cannot
be set as past time. This patch calculate the base time and reset the
Qbv configuration after ptp time adjust.
Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
---
.../net/ethernet/stmicro/stmmac/stmmac_ptp.c | 41 ++++++++++++++++++-
1 file changed, 40 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
index 4e86cdf2bc9f..c573bc8b2595 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
@@ -62,7 +62,8 @@ static int stmmac_adjust_time(struct ptp_clock_info *ptp, s64 delta)
u32 sec, nsec;
u32 quotient, reminder;
int neg_adj = 0;
- bool xmac;
+ bool xmac, est_rst = false;
+ int ret;
xmac = priv->plat->has_gmac4 || priv->plat->has_xgmac;
@@ -75,10 +76,48 @@ static int stmmac_adjust_time(struct ptp_clock_info *ptp, s64 delta)
sec = quotient;
nsec = reminder;
+ /* If EST is enabled, disabled it before adjust ptp time. */
+ if (priv->plat->est && priv->plat->est->enable) {
+ est_rst = true;
+ mutex_lock(&priv->plat->est->lock);
+ priv->plat->est->enable = false;
+ stmmac_est_configure(priv, priv->ioaddr, priv->plat->est,
+ priv->plat->clk_ptp_rate);
+ mutex_unlock(&priv->plat->est->lock);
+ }
+
spin_lock_irqsave(&priv->ptp_lock, flags);
stmmac_adjust_systime(priv, priv->ptpaddr, sec, nsec, neg_adj, xmac);
spin_unlock_irqrestore(&priv->ptp_lock, flags);
+ /* Caculate new basetime and re-configured EST after PTP time adjust. */
+ if (est_rst) {
+ struct timespec64 current_time, time;
+ ktime_t current_time_ns, basetime;
+ u64 cycle_time;
+
+ priv->ptp_clock_ops.gettime64(&priv->ptp_clock_ops, ¤t_time);
+ current_time_ns = timespec64_to_ktime(current_time);
+ time.tv_nsec = priv->plat->est->btr[0];
+ time.tv_sec = priv->plat->est->btr[1];
+ basetime = timespec64_to_ktime(time);
+ cycle_time = priv->plat->est->ctr[1] * NSEC_PER_SEC +
+ priv->plat->est->ctr[0];
+ time = stmmac_calc_tas_basetime(basetime,
+ current_time_ns,
+ cycle_time);
+
+ mutex_lock(&priv->plat->est->lock);
+ priv->plat->est->btr[0] = (u32)time.tv_nsec;
+ priv->plat->est->btr[1] = (u32)time.tv_sec;
+ priv->plat->est->enable = true;
+ ret = stmmac_est_configure(priv, priv->ioaddr, priv->plat->est,
+ priv->plat->clk_ptp_rate);
+ mutex_unlock(&priv->plat->est->lock);
+ if (ret)
+ netdev_err(priv->dev, "failed to configure EST\n");
+ }
+
return 0;
}
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v1 net-next 1/3] net: stmmac: separate the tas basetime calculation function
2021-06-01 8:38 ` Xiaoliang Yang
@ 2021-06-01 10:58 ` kernel test robot
-1 siblings, 0 replies; 18+ messages in thread
From: kernel test robot @ 2021-06-01 10:58 UTC (permalink / raw)
To: Xiaoliang Yang, davem, joabreu, kuba, alexandre.torgue,
peppe.cavallaro, mcoquelin.stm32
Cc: kbuild-all, netdev, boon.leong.ong, weifeng.voon, vee.khee.wong
[-- Attachment #1: Type: text/plain, Size: 2413 bytes --]
Hi Xiaoliang,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Xiaoliang-Yang/net-stmmac-re-configure-tas-basetime-after-ptp-time-adjust/20210601-163025
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 7fc6d3abc0844a9b8ef67937af465a417af6e9e9
config: arc-allyesconfig (attached as .config)
compiler: arceb-elf-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/5abe8df9d55824e7eff47e48b66f57260b0bfe50
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Xiaoliang-Yang/net-stmmac-re-configure-tas-basetime-after-ptp-time-adjust/20210601-163025
git checkout 5abe8df9d55824e7eff47e48b66f57260b0bfe50
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c:716:19: warning: no previous prototype for 'stmmac_calc_tas_basetime' [-Wmissing-prototypes]
716 | struct timespec64 stmmac_calc_tas_basetime(ktime_t old_base_time,
| ^~~~~~~~~~~~~~~~~~~~~~~~
vim +/stmmac_calc_tas_basetime +716 drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
715
> 716 struct timespec64 stmmac_calc_tas_basetime(ktime_t old_base_time,
717 ktime_t current_time,
718 u64 cycle_time)
719 {
720 struct timespec64 time;
721
722 if (ktime_after(old_base_time, current_time)) {
723 time = ktime_to_timespec64(old_base_time);
724 } else {
725 s64 n;
726 ktime_t base_time;
727
728 n = div64_s64(ktime_sub_ns(current_time, old_base_time),
729 cycle_time);
730 base_time = ktime_add_ns(old_base_time,
731 (n + 1) * cycle_time);
732
733 time = ktime_to_timespec64(base_time);
734 }
735
736 return time;
737 }
738
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 68164 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v1 net-next 1/3] net: stmmac: separate the tas basetime calculation function
@ 2021-06-01 10:58 ` kernel test robot
0 siblings, 0 replies; 18+ messages in thread
From: kernel test robot @ 2021-06-01 10:58 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 2475 bytes --]
Hi Xiaoliang,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Xiaoliang-Yang/net-stmmac-re-configure-tas-basetime-after-ptp-time-adjust/20210601-163025
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 7fc6d3abc0844a9b8ef67937af465a417af6e9e9
config: arc-allyesconfig (attached as .config)
compiler: arceb-elf-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/5abe8df9d55824e7eff47e48b66f57260b0bfe50
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Xiaoliang-Yang/net-stmmac-re-configure-tas-basetime-after-ptp-time-adjust/20210601-163025
git checkout 5abe8df9d55824e7eff47e48b66f57260b0bfe50
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c:716:19: warning: no previous prototype for 'stmmac_calc_tas_basetime' [-Wmissing-prototypes]
716 | struct timespec64 stmmac_calc_tas_basetime(ktime_t old_base_time,
| ^~~~~~~~~~~~~~~~~~~~~~~~
vim +/stmmac_calc_tas_basetime +716 drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
715
> 716 struct timespec64 stmmac_calc_tas_basetime(ktime_t old_base_time,
717 ktime_t current_time,
718 u64 cycle_time)
719 {
720 struct timespec64 time;
721
722 if (ktime_after(old_base_time, current_time)) {
723 time = ktime_to_timespec64(old_base_time);
724 } else {
725 s64 n;
726 ktime_t base_time;
727
728 n = div64_s64(ktime_sub_ns(current_time, old_base_time),
729 cycle_time);
730 base_time = ktime_add_ns(old_base_time,
731 (n + 1) * cycle_time);
732
733 time = ktime_to_timespec64(base_time);
734 }
735
736 return time;
737 }
738
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 68164 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v1 net-next 3/3] net: stmmac: ptp: update tas basetime after ptp adjust
2021-06-01 8:38 ` Xiaoliang Yang
@ 2021-06-01 11:21 ` kernel test robot
-1 siblings, 0 replies; 18+ messages in thread
From: kernel test robot @ 2021-06-01 11:21 UTC (permalink / raw)
To: Xiaoliang Yang, davem, joabreu, kuba, alexandre.torgue,
peppe.cavallaro, mcoquelin.stm32
Cc: kbuild-all, netdev, boon.leong.ong, weifeng.voon, vee.khee.wong
[-- Attachment #1: Type: text/plain, Size: 4734 bytes --]
Hi Xiaoliang,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Xiaoliang-Yang/net-stmmac-re-configure-tas-basetime-after-ptp-time-adjust/20210601-163025
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 7fc6d3abc0844a9b8ef67937af465a417af6e9e9
config: m68k-allmodconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/427b65e5ea008c937ca6bf7ceca8113db2c37f1a
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Xiaoliang-Yang/net-stmmac-re-configure-tas-basetime-after-ptp-time-adjust/20210601-163025
git checkout 427b65e5ea008c937ca6bf7ceca8113db2c37f1a
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=m68k
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c: In function 'stmmac_adjust_time':
>> drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c:106:10: error: implicit declaration of function 'stmmac_calc_tas_basetime' [-Werror=implicit-function-declaration]
106 | time = stmmac_calc_tas_basetime(basetime,
| ^~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c:106:10: error: incompatible types when assigning to type 'struct timespec64' from type 'int'
cc1: some warnings being treated as errors
vim +/stmmac_calc_tas_basetime +106 drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
48
49 /**
50 * stmmac_adjust_time
51 *
52 * @ptp: pointer to ptp_clock_info structure
53 * @delta: desired change in nanoseconds
54 *
55 * Description: this function will shift/adjust the hardware clock time.
56 */
57 static int stmmac_adjust_time(struct ptp_clock_info *ptp, s64 delta)
58 {
59 struct stmmac_priv *priv =
60 container_of(ptp, struct stmmac_priv, ptp_clock_ops);
61 unsigned long flags;
62 u32 sec, nsec;
63 u32 quotient, reminder;
64 int neg_adj = 0;
65 bool xmac, est_rst = false;
66 int ret;
67
68 xmac = priv->plat->has_gmac4 || priv->plat->has_xgmac;
69
70 if (delta < 0) {
71 neg_adj = 1;
72 delta = -delta;
73 }
74
75 quotient = div_u64_rem(delta, 1000000000ULL, &reminder);
76 sec = quotient;
77 nsec = reminder;
78
79 /* If EST is enabled, disabled it before adjust ptp time. */
80 if (priv->plat->est && priv->plat->est->enable) {
81 est_rst = true;
82 mutex_lock(&priv->plat->est->lock);
83 priv->plat->est->enable = false;
84 stmmac_est_configure(priv, priv->ioaddr, priv->plat->est,
85 priv->plat->clk_ptp_rate);
86 mutex_unlock(&priv->plat->est->lock);
87 }
88
89 spin_lock_irqsave(&priv->ptp_lock, flags);
90 stmmac_adjust_systime(priv, priv->ptpaddr, sec, nsec, neg_adj, xmac);
91 spin_unlock_irqrestore(&priv->ptp_lock, flags);
92
93 /* Caculate new basetime and re-configured EST after PTP time adjust. */
94 if (est_rst) {
95 struct timespec64 current_time, time;
96 ktime_t current_time_ns, basetime;
97 u64 cycle_time;
98
99 priv->ptp_clock_ops.gettime64(&priv->ptp_clock_ops, ¤t_time);
100 current_time_ns = timespec64_to_ktime(current_time);
101 time.tv_nsec = priv->plat->est->btr[0];
102 time.tv_sec = priv->plat->est->btr[1];
103 basetime = timespec64_to_ktime(time);
104 cycle_time = priv->plat->est->ctr[1] * NSEC_PER_SEC +
105 priv->plat->est->ctr[0];
> 106 time = stmmac_calc_tas_basetime(basetime,
107 current_time_ns,
108 cycle_time);
109
110 mutex_lock(&priv->plat->est->lock);
111 priv->plat->est->btr[0] = (u32)time.tv_nsec;
112 priv->plat->est->btr[1] = (u32)time.tv_sec;
113 priv->plat->est->enable = true;
114 ret = stmmac_est_configure(priv, priv->ioaddr, priv->plat->est,
115 priv->plat->clk_ptp_rate);
116 mutex_unlock(&priv->plat->est->lock);
117 if (ret)
118 netdev_err(priv->dev, "failed to configure EST\n");
119 }
120
121 return 0;
122 }
123
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 60487 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v1 net-next 3/3] net: stmmac: ptp: update tas basetime after ptp adjust
@ 2021-06-01 11:21 ` kernel test robot
0 siblings, 0 replies; 18+ messages in thread
From: kernel test robot @ 2021-06-01 11:21 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 4851 bytes --]
Hi Xiaoliang,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Xiaoliang-Yang/net-stmmac-re-configure-tas-basetime-after-ptp-time-adjust/20210601-163025
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 7fc6d3abc0844a9b8ef67937af465a417af6e9e9
config: m68k-allmodconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/427b65e5ea008c937ca6bf7ceca8113db2c37f1a
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Xiaoliang-Yang/net-stmmac-re-configure-tas-basetime-after-ptp-time-adjust/20210601-163025
git checkout 427b65e5ea008c937ca6bf7ceca8113db2c37f1a
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=m68k
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c: In function 'stmmac_adjust_time':
>> drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c:106:10: error: implicit declaration of function 'stmmac_calc_tas_basetime' [-Werror=implicit-function-declaration]
106 | time = stmmac_calc_tas_basetime(basetime,
| ^~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c:106:10: error: incompatible types when assigning to type 'struct timespec64' from type 'int'
cc1: some warnings being treated as errors
vim +/stmmac_calc_tas_basetime +106 drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
48
49 /**
50 * stmmac_adjust_time
51 *
52 * @ptp: pointer to ptp_clock_info structure
53 * @delta: desired change in nanoseconds
54 *
55 * Description: this function will shift/adjust the hardware clock time.
56 */
57 static int stmmac_adjust_time(struct ptp_clock_info *ptp, s64 delta)
58 {
59 struct stmmac_priv *priv =
60 container_of(ptp, struct stmmac_priv, ptp_clock_ops);
61 unsigned long flags;
62 u32 sec, nsec;
63 u32 quotient, reminder;
64 int neg_adj = 0;
65 bool xmac, est_rst = false;
66 int ret;
67
68 xmac = priv->plat->has_gmac4 || priv->plat->has_xgmac;
69
70 if (delta < 0) {
71 neg_adj = 1;
72 delta = -delta;
73 }
74
75 quotient = div_u64_rem(delta, 1000000000ULL, &reminder);
76 sec = quotient;
77 nsec = reminder;
78
79 /* If EST is enabled, disabled it before adjust ptp time. */
80 if (priv->plat->est && priv->plat->est->enable) {
81 est_rst = true;
82 mutex_lock(&priv->plat->est->lock);
83 priv->plat->est->enable = false;
84 stmmac_est_configure(priv, priv->ioaddr, priv->plat->est,
85 priv->plat->clk_ptp_rate);
86 mutex_unlock(&priv->plat->est->lock);
87 }
88
89 spin_lock_irqsave(&priv->ptp_lock, flags);
90 stmmac_adjust_systime(priv, priv->ptpaddr, sec, nsec, neg_adj, xmac);
91 spin_unlock_irqrestore(&priv->ptp_lock, flags);
92
93 /* Caculate new basetime and re-configured EST after PTP time adjust. */
94 if (est_rst) {
95 struct timespec64 current_time, time;
96 ktime_t current_time_ns, basetime;
97 u64 cycle_time;
98
99 priv->ptp_clock_ops.gettime64(&priv->ptp_clock_ops, ¤t_time);
100 current_time_ns = timespec64_to_ktime(current_time);
101 time.tv_nsec = priv->plat->est->btr[0];
102 time.tv_sec = priv->plat->est->btr[1];
103 basetime = timespec64_to_ktime(time);
104 cycle_time = priv->plat->est->ctr[1] * NSEC_PER_SEC +
105 priv->plat->est->ctr[0];
> 106 time = stmmac_calc_tas_basetime(basetime,
107 current_time_ns,
108 cycle_time);
109
110 mutex_lock(&priv->plat->est->lock);
111 priv->plat->est->btr[0] = (u32)time.tv_nsec;
112 priv->plat->est->btr[1] = (u32)time.tv_sec;
113 priv->plat->est->enable = true;
114 ret = stmmac_est_configure(priv, priv->ioaddr, priv->plat->est,
115 priv->plat->clk_ptp_rate);
116 mutex_unlock(&priv->plat->est->lock);
117 if (ret)
118 netdev_err(priv->dev, "failed to configure EST\n");
119 }
120
121 return 0;
122 }
123
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 60487 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v1 net-next 3/3] net: stmmac: ptp: update tas basetime after ptp adjust
2021-06-01 8:38 ` Xiaoliang Yang
@ 2021-06-02 10:18 ` Rui Sousa
-1 siblings, 0 replies; 18+ messages in thread
From: Rui Sousa @ 2021-06-02 10:18 UTC (permalink / raw)
To: Xiaoliang Yang
Cc: netdev, boon.leong.ong, weifeng.voon, vee.khee.wong, tee.min.tan,
mohammad.athari.ismail, linux-stm32, linux-arm-kernel,
linux-kernel, leoyang.li, vladimir.oltean, qiangqing.zhang,
mingkai.hu, yangbo.lu, davem, joabreu, kuba, alexandre.torgue,
peppe.cavallaro, mcoquelin.stm32
On 6/1/2021 10:38 AM, Xiaoliang Yang wrote:
Hi Xiaoliang,
> After adjusting the ptp time, the Qbv base time may be the past time
> of the new current time. dwmac5 hardware limited the base time cannot
> be set as past time. This patch calculate the base time and reset the
> Qbv configuration after ptp time adjust.
>
> Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
> ---
> .../net/ethernet/stmicro/stmmac/stmmac_ptp.c | 41 ++++++++++++++++++-
> 1 file changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> index 4e86cdf2bc9f..c573bc8b2595 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> @@ -62,7 +62,8 @@ static int stmmac_adjust_time(struct ptp_clock_info
> *ptp, s64 delta)
> u32 sec, nsec;
> u32 quotient, reminder;
> int neg_adj = 0;
> - bool xmac;
> + bool xmac, est_rst = false;
> + int ret;
>
> xmac = priv->plat->has_gmac4 || priv->plat->has_xgmac;
>
> @@ -75,10 +76,48 @@ static int stmmac_adjust_time(struct ptp_clock_info
> *ptp, s64 delta)
> sec = quotient;
> nsec = reminder;
>
> + /* If EST is enabled, disabled it before adjust ptp time. */
> + if (priv->plat->est && priv->plat->est->enable) {
> + est_rst = true;
> + mutex_lock(&priv->plat->est->lock);
> + priv->plat->est->enable = false;
> + stmmac_est_configure(priv, priv->ioaddr, priv->plat->est,
> + priv->plat->clk_ptp_rate);
> + mutex_unlock(&priv->plat->est->lock);
> + }
> +
> spin_lock_irqsave(&priv->ptp_lock, flags);
> stmmac_adjust_systime(priv, priv->ptpaddr, sec, nsec, neg_adj, xmac);
> spin_unlock_irqrestore(&priv->ptp_lock, flags);
>
> + /* Caculate new basetime and re-configured EST after PTP time
> adjust. */
> + if (est_rst) {
> + struct timespec64 current_time, time;
> + ktime_t current_time_ns, basetime;
> + u64 cycle_time;
> +
> + priv->ptp_clock_ops.gettime64(&priv->ptp_clock_ops,
> ¤t_time);
> + current_time_ns = timespec64_to_ktime(current_time);
> + time.tv_nsec = priv->plat->est->btr[0];
> + time.tv_sec = priv->plat->est->btr[1];
This time may no longer be what the user specified originally, it was
adjusted based on the gptp time when the configuration was first made.
IMHO, if we want to respect the user configuration then we need to do
the calculation here based on the original time.
Typically (using arbitrary units):
a) User configures basetime of 0, at gptp time 1000
b) btr is update to 1000, schedule starts
c) later, gptp time is updated to 500
d-1) with current patch, schedule will restart at 1000 (i.e remains
disabled for 500)
d-2) with my suggestion, schedule will restart at 500 (which matches the
user request, "start as soon as possible".
> + basetime = timespec64_to_ktime(time);
> + cycle_time = priv->plat->est->ctr[1] * NSEC_PER_SEC +
> + priv->plat->est->ctr[0];
> + time = stmmac_calc_tas_basetime(basetime,
> + current_time_ns,
> + cycle_time);
> +
> + mutex_lock(&priv->plat->est->lock);
Hmm... the locking needs to move up. Reading + writting btr/ctr should
be atomic.
> + priv->plat->est->btr[0] = (u32)time.tv_nsec;
> + priv->plat->est->btr[1] = (u32)time.tv_sec;
> + priv->plat->est->enable = true;
> + ret = stmmac_est_configure(priv, priv->ioaddr, priv->plat->est,
> + priv->plat->clk_ptp_rate);
> + mutex_unlock(&priv->plat->est->lock);
> + if (ret)
> + netdev_err(priv->dev, "failed to configure EST\n");
> + }
> +
> return 0;
> }
>
Br,
Rui Sousa
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v1 net-next 3/3] net: stmmac: ptp: update tas basetime after ptp adjust
@ 2021-06-02 10:18 ` Rui Sousa
0 siblings, 0 replies; 18+ messages in thread
From: Rui Sousa @ 2021-06-02 10:18 UTC (permalink / raw)
To: Xiaoliang Yang
Cc: netdev, boon.leong.ong, weifeng.voon, vee.khee.wong, tee.min.tan,
mohammad.athari.ismail, linux-stm32, linux-arm-kernel,
linux-kernel, leoyang.li, vladimir.oltean, qiangqing.zhang,
mingkai.hu, yangbo.lu, davem, joabreu, kuba, alexandre.torgue,
peppe.cavallaro, mcoquelin.stm32
On 6/1/2021 10:38 AM, Xiaoliang Yang wrote:
Hi Xiaoliang,
> After adjusting the ptp time, the Qbv base time may be the past time
> of the new current time. dwmac5 hardware limited the base time cannot
> be set as past time. This patch calculate the base time and reset the
> Qbv configuration after ptp time adjust.
>
> Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
> ---
> .../net/ethernet/stmicro/stmmac/stmmac_ptp.c | 41 ++++++++++++++++++-
> 1 file changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> index 4e86cdf2bc9f..c573bc8b2595 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> @@ -62,7 +62,8 @@ static int stmmac_adjust_time(struct ptp_clock_info
> *ptp, s64 delta)
> u32 sec, nsec;
> u32 quotient, reminder;
> int neg_adj = 0;
> - bool xmac;
> + bool xmac, est_rst = false;
> + int ret;
>
> xmac = priv->plat->has_gmac4 || priv->plat->has_xgmac;
>
> @@ -75,10 +76,48 @@ static int stmmac_adjust_time(struct ptp_clock_info
> *ptp, s64 delta)
> sec = quotient;
> nsec = reminder;
>
> + /* If EST is enabled, disabled it before adjust ptp time. */
> + if (priv->plat->est && priv->plat->est->enable) {
> + est_rst = true;
> + mutex_lock(&priv->plat->est->lock);
> + priv->plat->est->enable = false;
> + stmmac_est_configure(priv, priv->ioaddr, priv->plat->est,
> + priv->plat->clk_ptp_rate);
> + mutex_unlock(&priv->plat->est->lock);
> + }
> +
> spin_lock_irqsave(&priv->ptp_lock, flags);
> stmmac_adjust_systime(priv, priv->ptpaddr, sec, nsec, neg_adj, xmac);
> spin_unlock_irqrestore(&priv->ptp_lock, flags);
>
> + /* Caculate new basetime and re-configured EST after PTP time
> adjust. */
> + if (est_rst) {
> + struct timespec64 current_time, time;
> + ktime_t current_time_ns, basetime;
> + u64 cycle_time;
> +
> + priv->ptp_clock_ops.gettime64(&priv->ptp_clock_ops,
> ¤t_time);
> + current_time_ns = timespec64_to_ktime(current_time);
> + time.tv_nsec = priv->plat->est->btr[0];
> + time.tv_sec = priv->plat->est->btr[1];
This time may no longer be what the user specified originally, it was
adjusted based on the gptp time when the configuration was first made.
IMHO, if we want to respect the user configuration then we need to do
the calculation here based on the original time.
Typically (using arbitrary units):
a) User configures basetime of 0, at gptp time 1000
b) btr is update to 1000, schedule starts
c) later, gptp time is updated to 500
d-1) with current patch, schedule will restart at 1000 (i.e remains
disabled for 500)
d-2) with my suggestion, schedule will restart at 500 (which matches the
user request, "start as soon as possible".
> + basetime = timespec64_to_ktime(time);
> + cycle_time = priv->plat->est->ctr[1] * NSEC_PER_SEC +
> + priv->plat->est->ctr[0];
> + time = stmmac_calc_tas_basetime(basetime,
> + current_time_ns,
> + cycle_time);
> +
> + mutex_lock(&priv->plat->est->lock);
Hmm... the locking needs to move up. Reading + writting btr/ctr should
be atomic.
> + priv->plat->est->btr[0] = (u32)time.tv_nsec;
> + priv->plat->est->btr[1] = (u32)time.tv_sec;
> + priv->plat->est->enable = true;
> + ret = stmmac_est_configure(priv, priv->ioaddr, priv->plat->est,
> + priv->plat->clk_ptp_rate);
> + mutex_unlock(&priv->plat->est->lock);
> + if (ret)
> + netdev_err(priv->dev, "failed to configure EST\n");
> + }
> +
> return 0;
> }
>
Br,
Rui Sousa
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH v1 net-next 3/3] net: stmmac: ptp: update tas basetime after ptp adjust
2021-06-02 10:18 ` Rui Sousa
@ 2021-06-09 9:03 ` Xiaoliang Yang
-1 siblings, 0 replies; 18+ messages in thread
From: Xiaoliang Yang @ 2021-06-09 9:03 UTC (permalink / raw)
To: Rui Sousa
Cc: netdev, boon.leong.ong, weifeng.voon, vee.khee.wong, tee.min.tan,
mohammad.athari.ismail, linux-stm32, linux-arm-kernel,
linux-kernel, Leo Li, Vladimir Oltean, Joakim Zhang, Mingkai Hu,
Y.b. Lu, davem, joabreu, kuba, alexandre.torgue, peppe.cavallaro,
mcoquelin.stm32
Hi Rui,
On 2021-06-02 18:18, Rui Sousa wrote:
> > After adjusting the ptp time, the Qbv base time may be the past time
> > of the new current time. dwmac5 hardware limited the base time cannot
> > be set as past time. This patch calculate the base time and reset the
> > Qbv configuration after ptp time adjust.
> >
> > Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
> > ---
> > .../net/ethernet/stmicro/stmmac/stmmac_ptp.c | 41
> ++++++++++++++++++-
> > 1 file changed, 40 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> > b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> > index 4e86cdf2bc9f..c573bc8b2595 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> > @@ -62,7 +62,8 @@ static int stmmac_adjust_time(struct ptp_clock_info
> > *ptp, s64 delta)
> > u32 sec, nsec;
> > u32 quotient, reminder;
> > int neg_adj = 0;
> > - bool xmac;
> > + bool xmac, est_rst = false;
> > + int ret;
> >
> > xmac = priv->plat->has_gmac4 || priv->plat->has_xgmac;
> >
> > @@ -75,10 +76,48 @@ static int stmmac_adjust_time(struct
> > ptp_clock_info *ptp, s64 delta)
> > sec = quotient;
> > nsec = reminder;
> >
> > + /* If EST is enabled, disabled it before adjust ptp time. */
> > + if (priv->plat->est && priv->plat->est->enable) {
> > + est_rst = true;
> > + mutex_lock(&priv->plat->est->lock);
> > + priv->plat->est->enable = false;
> > + stmmac_est_configure(priv, priv->ioaddr, priv->plat->est,
> > + priv->plat->clk_ptp_rate);
> > + mutex_unlock(&priv->plat->est->lock);
> > + }
> > +
> > spin_lock_irqsave(&priv->ptp_lock, flags);
> > stmmac_adjust_systime(priv, priv->ptpaddr, sec, nsec, neg_adj,
> > xmac);
> > spin_unlock_irqrestore(&priv->ptp_lock, flags);
> >
> > + /* Caculate new basetime and re-configured EST after PTP time
> > adjust. */
> > + if (est_rst) {
> > + struct timespec64 current_time, time;
> > + ktime_t current_time_ns, basetime;
> > + u64 cycle_time;
> > +
> > + priv->ptp_clock_ops.gettime64(&priv->ptp_clock_ops,
> > ¤t_time);
> > + current_time_ns = timespec64_to_ktime(current_time);
> > + time.tv_nsec = priv->plat->est->btr[0];
> > + time.tv_sec = priv->plat->est->btr[1];
>
> This time may no longer be what the user specified originally, it was adjusted
> based on the gptp time when the configuration was first made.
> IMHO, if we want to respect the user configuration then we need to do the
> calculation here based on the original time.
> Typically (using arbitrary units):
> a) User configures basetime of 0, at gptp time 1000
> b) btr is update to 1000, schedule starts
> c) later, gptp time is updated to 500
> d-1) with current patch, schedule will restart at 1000 (i.e remains disabled for
> 500)
> d-2) with my suggestion, schedule will restart at 500 (which matches the user
> request, "start as soon as possible".
>
It is not the correct operation sequence for the user to configure Qbv before ptp clock synchronization. After adjusting the ptp clock time, it is no longer possible to determine whether the previously set basetime is what the user wants. I think our driver only needs to ensure that the set basetime meets the hardware regulations, and the hardware can work normally. So I only updated the past basetime. I am not sure if it is appropriate to reset EST configure in the PTP driver, but this case will cause the hardware to not work.
> > + basetime = timespec64_to_ktime(time);
> > + cycle_time = priv->plat->est->ctr[1] * NSEC_PER_SEC +
> > + priv->plat->est->ctr[0];
> > + time = stmmac_calc_tas_basetime(basetime,
> > + current_time_ns,
> > + cycle_time);
> > +
> > + mutex_lock(&priv->plat->est->lock);
>
> Hmm... the locking needs to move up. Reading + writting btr/ctr should be
> atomic.
I will modify this.
>
> > + priv->plat->est->btr[0] = (u32)time.tv_nsec;
> > + priv->plat->est->btr[1] = (u32)time.tv_sec;
> > + priv->plat->est->enable = true;
> > + ret = stmmac_est_configure(priv, priv->ioaddr,
> > +priv->plat->est,
> > + priv->plat->clk_ptp_rate);
> > + mutex_unlock(&priv->plat->est->lock);
> > + if (ret)
> > + netdev_err(priv->dev, "failed to configure EST\n");
> > + }
> > +
> > return 0;
> > }
Thanks,
xiaoliang
^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH v1 net-next 3/3] net: stmmac: ptp: update tas basetime after ptp adjust
@ 2021-06-09 9:03 ` Xiaoliang Yang
0 siblings, 0 replies; 18+ messages in thread
From: Xiaoliang Yang @ 2021-06-09 9:03 UTC (permalink / raw)
To: Rui Sousa
Cc: netdev, boon.leong.ong, weifeng.voon, vee.khee.wong, tee.min.tan,
mohammad.athari.ismail, linux-stm32, linux-arm-kernel,
linux-kernel, Leo Li, Vladimir Oltean, Joakim Zhang, Mingkai Hu,
Y.b. Lu, davem, joabreu, kuba, alexandre.torgue, peppe.cavallaro,
mcoquelin.stm32
Hi Rui,
On 2021-06-02 18:18, Rui Sousa wrote:
> > After adjusting the ptp time, the Qbv base time may be the past time
> > of the new current time. dwmac5 hardware limited the base time cannot
> > be set as past time. This patch calculate the base time and reset the
> > Qbv configuration after ptp time adjust.
> >
> > Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
> > ---
> > .../net/ethernet/stmicro/stmmac/stmmac_ptp.c | 41
> ++++++++++++++++++-
> > 1 file changed, 40 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> > b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> > index 4e86cdf2bc9f..c573bc8b2595 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> > @@ -62,7 +62,8 @@ static int stmmac_adjust_time(struct ptp_clock_info
> > *ptp, s64 delta)
> > u32 sec, nsec;
> > u32 quotient, reminder;
> > int neg_adj = 0;
> > - bool xmac;
> > + bool xmac, est_rst = false;
> > + int ret;
> >
> > xmac = priv->plat->has_gmac4 || priv->plat->has_xgmac;
> >
> > @@ -75,10 +76,48 @@ static int stmmac_adjust_time(struct
> > ptp_clock_info *ptp, s64 delta)
> > sec = quotient;
> > nsec = reminder;
> >
> > + /* If EST is enabled, disabled it before adjust ptp time. */
> > + if (priv->plat->est && priv->plat->est->enable) {
> > + est_rst = true;
> > + mutex_lock(&priv->plat->est->lock);
> > + priv->plat->est->enable = false;
> > + stmmac_est_configure(priv, priv->ioaddr, priv->plat->est,
> > + priv->plat->clk_ptp_rate);
> > + mutex_unlock(&priv->plat->est->lock);
> > + }
> > +
> > spin_lock_irqsave(&priv->ptp_lock, flags);
> > stmmac_adjust_systime(priv, priv->ptpaddr, sec, nsec, neg_adj,
> > xmac);
> > spin_unlock_irqrestore(&priv->ptp_lock, flags);
> >
> > + /* Caculate new basetime and re-configured EST after PTP time
> > adjust. */
> > + if (est_rst) {
> > + struct timespec64 current_time, time;
> > + ktime_t current_time_ns, basetime;
> > + u64 cycle_time;
> > +
> > + priv->ptp_clock_ops.gettime64(&priv->ptp_clock_ops,
> > ¤t_time);
> > + current_time_ns = timespec64_to_ktime(current_time);
> > + time.tv_nsec = priv->plat->est->btr[0];
> > + time.tv_sec = priv->plat->est->btr[1];
>
> This time may no longer be what the user specified originally, it was adjusted
> based on the gptp time when the configuration was first made.
> IMHO, if we want to respect the user configuration then we need to do the
> calculation here based on the original time.
> Typically (using arbitrary units):
> a) User configures basetime of 0, at gptp time 1000
> b) btr is update to 1000, schedule starts
> c) later, gptp time is updated to 500
> d-1) with current patch, schedule will restart at 1000 (i.e remains disabled for
> 500)
> d-2) with my suggestion, schedule will restart at 500 (which matches the user
> request, "start as soon as possible".
>
It is not the correct operation sequence for the user to configure Qbv before ptp clock synchronization. After adjusting the ptp clock time, it is no longer possible to determine whether the previously set basetime is what the user wants. I think our driver only needs to ensure that the set basetime meets the hardware regulations, and the hardware can work normally. So I only updated the past basetime. I am not sure if it is appropriate to reset EST configure in the PTP driver, but this case will cause the hardware to not work.
> > + basetime = timespec64_to_ktime(time);
> > + cycle_time = priv->plat->est->ctr[1] * NSEC_PER_SEC +
> > + priv->plat->est->ctr[0];
> > + time = stmmac_calc_tas_basetime(basetime,
> > + current_time_ns,
> > + cycle_time);
> > +
> > + mutex_lock(&priv->plat->est->lock);
>
> Hmm... the locking needs to move up. Reading + writting btr/ctr should be
> atomic.
I will modify this.
>
> > + priv->plat->est->btr[0] = (u32)time.tv_nsec;
> > + priv->plat->est->btr[1] = (u32)time.tv_sec;
> > + priv->plat->est->enable = true;
> > + ret = stmmac_est_configure(priv, priv->ioaddr,
> > +priv->plat->est,
> > + priv->plat->clk_ptp_rate);
> > + mutex_unlock(&priv->plat->est->lock);
> > + if (ret)
> > + netdev_err(priv->dev, "failed to configure EST\n");
> > + }
> > +
> > return 0;
> > }
Thanks,
xiaoliang
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v1 net-next 3/3] net: stmmac: ptp: update tas basetime after ptp adjust
2021-06-09 9:03 ` Xiaoliang Yang
@ 2021-06-16 17:12 ` Rui Sousa
-1 siblings, 0 replies; 18+ messages in thread
From: Rui Sousa @ 2021-06-16 17:12 UTC (permalink / raw)
To: Xiaoliang Yang
Cc: netdev, boon.leong.ong, weifeng.voon, vee.khee.wong, tee.min.tan,
mohammad.athari.ismail, linux-stm32, linux-arm-kernel,
linux-kernel, Leo Li, Vladimir Oltean, Joakim Zhang, Mingkai Hu,
Y.b. Lu, davem, joabreu, kuba, alexandre.torgue, peppe.cavallaro,
mcoquelin.stm32
On 6/9/2021 11:03 AM, Xiaoliang Yang wrote:
> Hi Rui,
>
Hi,
> On 2021-06-02 18:18, Rui Sousa wrote:
>>> After adjusting the ptp time, the Qbv base time may be the past time
>>> of the new current time. dwmac5 hardware limited the base time cannot
>>> be set as past time. This patch calculate the base time and reset the
>>> Qbv configuration after ptp time adjust.
>>>
>>> Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
>>> ---
>>> .../net/ethernet/stmicro/stmmac/stmmac_ptp.c | 41
>> ++++++++++++++++++-
>>> 1 file changed, 40 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
>>> b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
>>> index 4e86cdf2bc9f..c573bc8b2595 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
>>> @@ -62,7 +62,8 @@ static int stmmac_adjust_time(struct ptp_clock_info
>>> *ptp, s64 delta)
>>> u32 sec, nsec;
>>> u32 quotient, reminder;
>>> int neg_adj = 0;
>>> - bool xmac;
>>> + bool xmac, est_rst = false;
>>> + int ret;
>>>
>>> xmac = priv->plat->has_gmac4 || priv->plat->has_xgmac;
>>>
>>> @@ -75,10 +76,48 @@ static int stmmac_adjust_time(struct
>>> ptp_clock_info *ptp, s64 delta)
>>> sec = quotient;
>>> nsec = reminder;
>>>
>>> + /* If EST is enabled, disabled it before adjust ptp time. */
>>> + if (priv->plat->est && priv->plat->est->enable) {
>>> + est_rst = true;
>>> + mutex_lock(&priv->plat->est->lock);
>>> + priv->plat->est->enable = false;
>>> + stmmac_est_configure(priv, priv->ioaddr, priv->plat->est,
>>> + priv->plat->clk_ptp_rate);
>>> + mutex_unlock(&priv->plat->est->lock);
>>> + }
>>> +
>>> spin_lock_irqsave(&priv->ptp_lock, flags);
>>> stmmac_adjust_systime(priv, priv->ptpaddr, sec, nsec, neg_adj,
>>> xmac);
>>> spin_unlock_irqrestore(&priv->ptp_lock, flags);
>>>
>>> + /* Caculate new basetime and re-configured EST after PTP time
>>> adjust. */
>>> + if (est_rst) {
>>> + struct timespec64 current_time, time;
>>> + ktime_t current_time_ns, basetime;
>>> + u64 cycle_time;
>>> +
>>> + priv->ptp_clock_ops.gettime64(&priv->ptp_clock_ops,
>>> ¤t_time);
>>> + current_time_ns = timespec64_to_ktime(current_time);
>>> + time.tv_nsec = priv->plat->est->btr[0];
>>> + time.tv_sec = priv->plat->est->btr[1];
>>
>> This time may no longer be what the user specified originally, it was adjusted
>> based on the gptp time when the configuration was first made.
>> IMHO, if we want to respect the user configuration then we need to do the
>> calculation here based on the original time.
>> Typically (using arbitrary units):
>> a) User configures basetime of 0, at gptp time 1000
>> b) btr is update to 1000, schedule starts
>> c) later, gptp time is updated to 500
>> d-1) with current patch, schedule will restart at 1000 (i.e remains disabled for
>> 500)
>> d-2) with my suggestion, schedule will restart at 500 (which matches the user
>> request, "start as soon as possible".
>>
> It is not the correct operation sequence for the user to configure Qbv before ptp clock synchronization.
The way I see it, a ptp clock discontinuity may happen at any time
(change of grand master, grand master discontinuity, ...) outside of the
user control. So having the driver handle all corner cases looked like a
good solution to me.
> After adjusting the ptp clock time, it is no longer possible to determine whether the previously set basetime is what the user wants.
I may be assuming too much, but usually a Qbv schedule is determined by
some central identity based on an absolute time (not related to the
current time in the endpoint). So with this assumption I was considering
the time specified by the user as "correct", independently of the
current local ptp time.
> I think our driver only needs to ensure that the set basetime meets the hardware regulations, and the hardware can work normally. So I only updated the past basetime.
Understood, but from the point of view of the user I think the case you
are already handling and the one I mentioned are very similar. Qbv
schedule doesn't work as intended after the clock jump. Also, my
suggestion simplifies a bit the code (no conversion from hardware to
ktime), at the cost of adding extra data (software backup of the
original user ktime).
> I am not sure if it is appropriate to reset EST configure in the PTP driver,
I'm not 100% sure either and I was hoping to see other people comments
(and I haven't checked yet how other drivers are handling this).
That said, to handle this properly in userspace, IMHO you would need:
- some process monitoring the ptp clock and detecting jumps
- the process would need to be aware of the current Qbv schedule
- when a jump is detected, re-do the Qbv configuration
Overall, handling the issue transparently in the driver, seems like a
better solution.
> but this case will cause the hardware to not work.
>
>>> + basetime = timespec64_to_ktime(time);
>>> + cycle_time = priv->plat->est->ctr[1] * NSEC_PER_SEC +
>>> + priv->plat->est->ctr[0];
>>> + time = stmmac_calc_tas_basetime(basetime,
>>> + current_time_ns,
>>> + cycle_time);
>>> +
>>> + mutex_lock(&priv->plat->est->lock);
>>
>> Hmm... the locking needs to move up. Reading + writting btr/ctr should be
>> atomic.
> I will modify this.
>>
>>> + priv->plat->est->btr[0] = (u32)time.tv_nsec;
>>> + priv->plat->est->btr[1] = (u32)time.tv_sec;
>>> + priv->plat->est->enable = true;
>>> + ret = stmmac_est_configure(priv, priv->ioaddr,
>>> +priv->plat->est,
>>> + priv->plat->clk_ptp_rate);
>>> + mutex_unlock(&priv->plat->est->lock);
>>> + if (ret)
>>> + netdev_err(priv->dev, "failed to configure EST\n");
>>> + }
>>> +
>>> return 0;
>>> }
>
> Thanks,
> xiaoliang
>
Br,
Rui
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v1 net-next 3/3] net: stmmac: ptp: update tas basetime after ptp adjust
@ 2021-06-16 17:12 ` Rui Sousa
0 siblings, 0 replies; 18+ messages in thread
From: Rui Sousa @ 2021-06-16 17:12 UTC (permalink / raw)
To: Xiaoliang Yang
Cc: netdev, boon.leong.ong, weifeng.voon, vee.khee.wong, tee.min.tan,
mohammad.athari.ismail, linux-stm32, linux-arm-kernel,
linux-kernel, Leo Li, Vladimir Oltean, Joakim Zhang, Mingkai Hu,
Y.b. Lu, davem, joabreu, kuba, alexandre.torgue, peppe.cavallaro,
mcoquelin.stm32
On 6/9/2021 11:03 AM, Xiaoliang Yang wrote:
> Hi Rui,
>
Hi,
> On 2021-06-02 18:18, Rui Sousa wrote:
>>> After adjusting the ptp time, the Qbv base time may be the past time
>>> of the new current time. dwmac5 hardware limited the base time cannot
>>> be set as past time. This patch calculate the base time and reset the
>>> Qbv configuration after ptp time adjust.
>>>
>>> Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
>>> ---
>>> .../net/ethernet/stmicro/stmmac/stmmac_ptp.c | 41
>> ++++++++++++++++++-
>>> 1 file changed, 40 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
>>> b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
>>> index 4e86cdf2bc9f..c573bc8b2595 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
>>> @@ -62,7 +62,8 @@ static int stmmac_adjust_time(struct ptp_clock_info
>>> *ptp, s64 delta)
>>> u32 sec, nsec;
>>> u32 quotient, reminder;
>>> int neg_adj = 0;
>>> - bool xmac;
>>> + bool xmac, est_rst = false;
>>> + int ret;
>>>
>>> xmac = priv->plat->has_gmac4 || priv->plat->has_xgmac;
>>>
>>> @@ -75,10 +76,48 @@ static int stmmac_adjust_time(struct
>>> ptp_clock_info *ptp, s64 delta)
>>> sec = quotient;
>>> nsec = reminder;
>>>
>>> + /* If EST is enabled, disabled it before adjust ptp time. */
>>> + if (priv->plat->est && priv->plat->est->enable) {
>>> + est_rst = true;
>>> + mutex_lock(&priv->plat->est->lock);
>>> + priv->plat->est->enable = false;
>>> + stmmac_est_configure(priv, priv->ioaddr, priv->plat->est,
>>> + priv->plat->clk_ptp_rate);
>>> + mutex_unlock(&priv->plat->est->lock);
>>> + }
>>> +
>>> spin_lock_irqsave(&priv->ptp_lock, flags);
>>> stmmac_adjust_systime(priv, priv->ptpaddr, sec, nsec, neg_adj,
>>> xmac);
>>> spin_unlock_irqrestore(&priv->ptp_lock, flags);
>>>
>>> + /* Caculate new basetime and re-configured EST after PTP time
>>> adjust. */
>>> + if (est_rst) {
>>> + struct timespec64 current_time, time;
>>> + ktime_t current_time_ns, basetime;
>>> + u64 cycle_time;
>>> +
>>> + priv->ptp_clock_ops.gettime64(&priv->ptp_clock_ops,
>>> ¤t_time);
>>> + current_time_ns = timespec64_to_ktime(current_time);
>>> + time.tv_nsec = priv->plat->est->btr[0];
>>> + time.tv_sec = priv->plat->est->btr[1];
>>
>> This time may no longer be what the user specified originally, it was adjusted
>> based on the gptp time when the configuration was first made.
>> IMHO, if we want to respect the user configuration then we need to do the
>> calculation here based on the original time.
>> Typically (using arbitrary units):
>> a) User configures basetime of 0, at gptp time 1000
>> b) btr is update to 1000, schedule starts
>> c) later, gptp time is updated to 500
>> d-1) with current patch, schedule will restart at 1000 (i.e remains disabled for
>> 500)
>> d-2) with my suggestion, schedule will restart at 500 (which matches the user
>> request, "start as soon as possible".
>>
> It is not the correct operation sequence for the user to configure Qbv before ptp clock synchronization.
The way I see it, a ptp clock discontinuity may happen at any time
(change of grand master, grand master discontinuity, ...) outside of the
user control. So having the driver handle all corner cases looked like a
good solution to me.
> After adjusting the ptp clock time, it is no longer possible to determine whether the previously set basetime is what the user wants.
I may be assuming too much, but usually a Qbv schedule is determined by
some central identity based on an absolute time (not related to the
current time in the endpoint). So with this assumption I was considering
the time specified by the user as "correct", independently of the
current local ptp time.
> I think our driver only needs to ensure that the set basetime meets the hardware regulations, and the hardware can work normally. So I only updated the past basetime.
Understood, but from the point of view of the user I think the case you
are already handling and the one I mentioned are very similar. Qbv
schedule doesn't work as intended after the clock jump. Also, my
suggestion simplifies a bit the code (no conversion from hardware to
ktime), at the cost of adding extra data (software backup of the
original user ktime).
> I am not sure if it is appropriate to reset EST configure in the PTP driver,
I'm not 100% sure either and I was hoping to see other people comments
(and I haven't checked yet how other drivers are handling this).
That said, to handle this properly in userspace, IMHO you would need:
- some process monitoring the ptp clock and detecting jumps
- the process would need to be aware of the current Qbv schedule
- when a jump is detected, re-do the Qbv configuration
Overall, handling the issue transparently in the driver, seems like a
better solution.
> but this case will cause the hardware to not work.
>
>>> + basetime = timespec64_to_ktime(time);
>>> + cycle_time = priv->plat->est->ctr[1] * NSEC_PER_SEC +
>>> + priv->plat->est->ctr[0];
>>> + time = stmmac_calc_tas_basetime(basetime,
>>> + current_time_ns,
>>> + cycle_time);
>>> +
>>> + mutex_lock(&priv->plat->est->lock);
>>
>> Hmm... the locking needs to move up. Reading + writting btr/ctr should be
>> atomic.
> I will modify this.
>>
>>> + priv->plat->est->btr[0] = (u32)time.tv_nsec;
>>> + priv->plat->est->btr[1] = (u32)time.tv_sec;
>>> + priv->plat->est->enable = true;
>>> + ret = stmmac_est_configure(priv, priv->ioaddr,
>>> +priv->plat->est,
>>> + priv->plat->clk_ptp_rate);
>>> + mutex_unlock(&priv->plat->est->lock);
>>> + if (ret)
>>> + netdev_err(priv->dev, "failed to configure EST\n");
>>> + }
>>> +
>>> return 0;
>>> }
>
> Thanks,
> xiaoliang
>
Br,
Rui
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2021-06-16 17:14 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-01 8:38 [PATCH v1 net-next 0/3] net: stmmac: re-configure tas basetime after ptp time adjust Xiaoliang Yang
2021-06-01 8:38 ` Xiaoliang Yang
2021-06-01 8:38 ` [PATCH v1 net-next 1/3] net: stmmac: separate the tas basetime calculation function Xiaoliang Yang
2021-06-01 8:38 ` Xiaoliang Yang
2021-06-01 10:58 ` kernel test robot
2021-06-01 10:58 ` kernel test robot
2021-06-01 8:38 ` [PATCH v1 net-next 2/3] net: stmmac: add mutex lock to protect est parameters Xiaoliang Yang
2021-06-01 8:38 ` Xiaoliang Yang
2021-06-01 8:38 ` [PATCH v1 net-next 3/3] net: stmmac: ptp: update tas basetime after ptp adjust Xiaoliang Yang
2021-06-01 8:38 ` Xiaoliang Yang
2021-06-01 11:21 ` kernel test robot
2021-06-01 11:21 ` kernel test robot
2021-06-02 10:18 ` Rui Sousa
2021-06-02 10:18 ` Rui Sousa
2021-06-09 9:03 ` Xiaoliang Yang
2021-06-09 9:03 ` Xiaoliang Yang
2021-06-16 17:12 ` Rui Sousa
2021-06-16 17:12 ` Rui Sousa
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.