* [PATCH v1 net-next] net: dsa: felix: update base time of time-aware shaper when adjusting PTP time
@ 2022-06-16 6:46 Xiaoliang Yang
2022-06-16 7:05 ` Maxim Kochetkov
0 siblings, 1 reply; 3+ messages in thread
From: Xiaoliang Yang @ 2022-06-16 6:46 UTC (permalink / raw)
To: davem, netdev, linux-kernel
Cc: allan.nielsen, joergen.andreasen, horatiu.vultur, UNGLinuxDriver,
alexandre.belloni, fido_max, f.fainelli, vivien.didelot, pabeni,
kuba, edumazet, andrew, alexandru.marginean, claudiu.manoil,
vladimir.oltean, leoyang.li, xiaoliang.yang_1, yangbo.lu
When adjusting the PTP clock, the base time of the TAS configuration
will become unreliable. We need reset the TAS configuration by using a
new base time.
For example, if the driver gets a base time 0 of Qbv configuration from
user, and current time is 20000. The driver will set the TAS base time
to be 20000. After the PTP clock adjustment, the current time becomes
10000. If the TAS base time is still 20000, it will be a future time,
and TAS entry list will stop running. Another example, if the current
time becomes to be 10000000 after PTP clock adjust, a large time offset
can cause the hardware to hang.
This patch introduces a tas_clock_adjust() function to reset the TAS
module by using a new base time after the PTP clock adjustment. This can
avoid issues above.
Due to PTP clock adjustment can occur at any time, it may conflict with
the TAS configuration. We introduce a new TAS lock to serialize the
access to the TAS registers.
Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
---
drivers/net/dsa/ocelot/felix_vsc9959.c | 83 ++++++++++++++++++++++++--
drivers/net/ethernet/mscc/ocelot.c | 1 +
drivers/net/ethernet/mscc/ocelot_ptp.c | 12 +++-
include/soc/mscc/ocelot.h | 7 +++
4 files changed, 95 insertions(+), 8 deletions(-)
diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
index 570d0204b7be..dd9085ae0922 100644
--- a/drivers/net/dsa/ocelot/felix_vsc9959.c
+++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
@@ -1196,10 +1196,13 @@ static void vsc9959_tas_gcl_set(struct ocelot *ocelot, const u32 gcl_ix,
static int vsc9959_qos_port_tas_set(struct ocelot *ocelot, int port,
struct tc_taprio_qopt_offload *taprio)
{
+ struct ocelot_port *ocelot_port = ocelot->ports[port];
struct timespec64 base_ts;
int ret, i;
u32 val;
+ mutex_lock(&ocelot->tas_lock);
+
if (!taprio->enable) {
ocelot_rmw_rix(ocelot,
QSYS_TAG_CONFIG_INIT_GATE_STATE(0xFF),
@@ -1207,15 +1210,20 @@ static int vsc9959_qos_port_tas_set(struct ocelot *ocelot, int port,
QSYS_TAG_CONFIG_INIT_GATE_STATE_M,
QSYS_TAG_CONFIG, port);
+ mutex_unlock(&ocelot->tas_lock);
return 0;
}
if (taprio->cycle_time > NSEC_PER_SEC ||
- taprio->cycle_time_extension >= NSEC_PER_SEC)
- return -EINVAL;
+ taprio->cycle_time_extension >= NSEC_PER_SEC) {
+ ret = -EINVAL;
+ goto err;
+ }
- if (taprio->num_entries > VSC9959_TAS_GCL_ENTRY_MAX)
- return -ERANGE;
+ if (taprio->num_entries > VSC9959_TAS_GCL_ENTRY_MAX) {
+ ret = -ERANGE;
+ goto err;
+ }
/* Enable guard band. The switch will schedule frames without taking
* their length into account. Thus we'll always need to enable the
@@ -1236,8 +1244,10 @@ static int vsc9959_qos_port_tas_set(struct ocelot *ocelot, int port,
* config is pending, need reset the TAS module
*/
val = ocelot_read(ocelot, QSYS_PARAM_STATUS_REG_8);
- if (val & QSYS_PARAM_STATUS_REG_8_CONFIG_PENDING)
- return -EBUSY;
+ if (val & QSYS_PARAM_STATUS_REG_8_CONFIG_PENDING) {
+ ret = -EBUSY;
+ goto err;
+ }
ocelot_rmw_rix(ocelot,
QSYS_TAG_CONFIG_ENABLE |
@@ -1248,6 +1258,8 @@ static int vsc9959_qos_port_tas_set(struct ocelot *ocelot, int port,
QSYS_TAG_CONFIG_SCH_TRAFFIC_QUEUES_M,
QSYS_TAG_CONFIG, port);
+ ocelot_port->base_time = taprio->base_time;
+
vsc9959_new_base_time(ocelot, taprio->base_time,
taprio->cycle_time, &base_ts);
ocelot_write(ocelot, base_ts.tv_nsec, QSYS_PARAM_CFG_REG_1);
@@ -1271,9 +1283,67 @@ static int vsc9959_qos_port_tas_set(struct ocelot *ocelot, int port,
!(val & QSYS_TAS_PARAM_CFG_CTRL_CONFIG_CHANGE),
10, 100000);
+err:
+ mutex_unlock(&ocelot->tas_lock);
+
return ret;
}
+static void vsc9959_tas_clock_adjust(struct ocelot *ocelot)
+{
+ struct ocelot_port *ocelot_port;
+ struct timespec64 base_ts;
+ u64 cycletime;
+ int port;
+ u32 val;
+
+ mutex_lock(&ocelot->tas_lock);
+
+ for (port = 0; port < ocelot->num_phys_ports; port++) {
+ val = ocelot_read_rix(ocelot, QSYS_TAG_CONFIG, port);
+ if (!(val & QSYS_TAG_CONFIG_ENABLE))
+ continue;
+
+ ocelot_rmw(ocelot,
+ QSYS_TAS_PARAM_CFG_CTRL_PORT_NUM(port),
+ QSYS_TAS_PARAM_CFG_CTRL_PORT_NUM_M,
+ QSYS_TAS_PARAM_CFG_CTRL);
+
+ ocelot_rmw_rix(ocelot,
+ QSYS_TAG_CONFIG_INIT_GATE_STATE(0xFF),
+ QSYS_TAG_CONFIG_ENABLE |
+ QSYS_TAG_CONFIG_INIT_GATE_STATE_M,
+ QSYS_TAG_CONFIG, port);
+
+ cycletime = ocelot_read(ocelot, QSYS_PARAM_CFG_REG_4);
+ ocelot_port = ocelot->ports[port];
+
+ vsc9959_new_base_time(ocelot, ocelot_port->base_time,
+ cycletime, &base_ts);
+
+ ocelot_write(ocelot, base_ts.tv_nsec, QSYS_PARAM_CFG_REG_1);
+ ocelot_write(ocelot, lower_32_bits(base_ts.tv_sec),
+ QSYS_PARAM_CFG_REG_2);
+ val = upper_32_bits(base_ts.tv_sec);
+ ocelot_rmw(ocelot,
+ QSYS_PARAM_CFG_REG_3_BASE_TIME_SEC_MSB(val),
+ QSYS_PARAM_CFG_REG_3_BASE_TIME_SEC_MSB_M,
+ QSYS_PARAM_CFG_REG_3);
+
+ ocelot_rmw(ocelot, QSYS_TAS_PARAM_CFG_CTRL_CONFIG_CHANGE,
+ QSYS_TAS_PARAM_CFG_CTRL_CONFIG_CHANGE,
+ QSYS_TAS_PARAM_CFG_CTRL);
+
+ ocelot_rmw_rix(ocelot,
+ QSYS_TAG_CONFIG_INIT_GATE_STATE(0xFF) |
+ QSYS_TAG_CONFIG_ENABLE,
+ QSYS_TAG_CONFIG_ENABLE |
+ QSYS_TAG_CONFIG_INIT_GATE_STATE_M,
+ QSYS_TAG_CONFIG, port);
+ }
+ mutex_unlock(&ocelot->tas_lock);
+}
+
static int vsc9959_qos_port_cbs_set(struct dsa_switch *ds, int port,
struct tc_cbs_qopt_offload *cbs_qopt)
{
@@ -2210,6 +2280,7 @@ static const struct ocelot_ops vsc9959_ops = {
.psfp_filter_del = vsc9959_psfp_filter_del,
.psfp_stats_get = vsc9959_psfp_stats_get,
.cut_through_fwd = vsc9959_cut_through_fwd,
+ .tas_clock_adjust = vsc9959_tas_clock_adjust,
};
static const struct felix_info felix_info_vsc9959 = {
diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index 8da7e25a47c9..d4649e4ee0e7 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -3367,6 +3367,7 @@ int ocelot_init(struct ocelot *ocelot)
mutex_init(&ocelot->ptp_lock);
mutex_init(&ocelot->mact_lock);
mutex_init(&ocelot->fwd_domain_lock);
+ mutex_init(&ocelot->tas_lock);
spin_lock_init(&ocelot->ptp_clock_lock);
spin_lock_init(&ocelot->ts_id_lock);
snprintf(queue_name, sizeof(queue_name), "%s-stats",
diff --git a/drivers/net/ethernet/mscc/ocelot_ptp.c b/drivers/net/ethernet/mscc/ocelot_ptp.c
index 87ad2137ba06..522fdc38d4d0 100644
--- a/drivers/net/ethernet/mscc/ocelot_ptp.c
+++ b/drivers/net/ethernet/mscc/ocelot_ptp.c
@@ -72,15 +72,19 @@ int ocelot_ptp_settime64(struct ptp_clock_info *ptp,
ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
+
+ if (ocelot->ops->tas_clock_adjust)
+ ocelot->ops->tas_clock_adjust(ocelot);
+
return 0;
}
EXPORT_SYMBOL(ocelot_ptp_settime64);
int ocelot_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
{
+ struct ocelot *ocelot = container_of(ptp, struct ocelot, ptp_info);
+
if (delta > -(NSEC_PER_SEC / 2) && delta < (NSEC_PER_SEC / 2)) {
- struct ocelot *ocelot = container_of(ptp, struct ocelot,
- ptp_info);
unsigned long flags;
u32 val;
@@ -117,6 +121,10 @@ int ocelot_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
ocelot_ptp_settime64(ptp, &ts);
}
+
+ if (ocelot->ops->tas_clock_adjust)
+ ocelot->ops->tas_clock_adjust(ocelot);
+
return 0;
}
EXPORT_SYMBOL(ocelot_ptp_adjtime);
diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
index 5f88385a7748..3737570116c3 100644
--- a/include/soc/mscc/ocelot.h
+++ b/include/soc/mscc/ocelot.h
@@ -575,6 +575,7 @@ struct ocelot_ops {
int (*psfp_stats_get)(struct ocelot *ocelot, struct flow_cls_offload *f,
struct flow_stats *stats);
void (*cut_through_fwd)(struct ocelot *ocelot);
+ void (*tas_clock_adjust)(struct ocelot *ocelot);
};
struct ocelot_vcap_policer {
@@ -691,6 +692,9 @@ struct ocelot_port {
int bridge_num;
int speed;
+
+ /* Store the AdminBaseTime of EST fetched from userspace. */
+ s64 base_time;
};
struct ocelot {
@@ -757,6 +761,9 @@ struct ocelot {
/* Lock for serializing forwarding domain changes */
struct mutex fwd_domain_lock;
+ /* Lock for serializing Time-Aware Shaper changes */
+ struct mutex tas_lock;
+
struct workqueue_struct *owq;
u8 ptp:1;
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v1 net-next] net: dsa: felix: update base time of time-aware shaper when adjusting PTP time
2022-06-16 6:46 [PATCH v1 net-next] net: dsa: felix: update base time of time-aware shaper when adjusting PTP time Xiaoliang Yang
@ 2022-06-16 7:05 ` Maxim Kochetkov
2022-06-16 8:16 ` 回复: [EXT] " Xiaoliang Yang
0 siblings, 1 reply; 3+ messages in thread
From: Maxim Kochetkov @ 2022-06-16 7:05 UTC (permalink / raw)
To: Xiaoliang Yang, davem, netdev, linux-kernel
Cc: allan.nielsen, joergen.andreasen, horatiu.vultur, UNGLinuxDriver,
alexandre.belloni, f.fainelli, vivien.didelot, pabeni, kuba,
edumazet, andrew, alexandru.marginean, claudiu.manoil,
vladimir.oltean, leoyang.li, yangbo.lu
On 16.06.2022 09:46, Xiaoliang Yang wrote:
> When adjusting the PTP clock, the base time of the TAS configuration
> will become unreliable. We need reset the TAS configuration by using a
> new base time.
>
> For example, if the driver gets a base time 0 of Qbv configuration from
> user, and current time is 20000. The driver will set the TAS base time
> to be 20000. After the PTP clock adjustment, the current time becomes
> 10000. If the TAS base time is still 20000, it will be a future time,
> and TAS entry list will stop running. Another example, if the current
> time becomes to be 10000000 after PTP clock adjust, a large time offset
> can cause the hardware to hang.
>
> This patch introduces a tas_clock_adjust() function to reset the TAS
> module by using a new base time after the PTP clock adjustment. This can
> avoid issues above.
>
> Due to PTP clock adjustment can occur at any time, it may conflict with
> the TAS configuration. We introduce a new TAS lock to serialize the
> access to the TAS registers.
>
> Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
> ---
> drivers/net/dsa/ocelot/felix_vsc9959.c | 83 ++++++++++++++++++++++++--
> drivers/net/ethernet/mscc/ocelot.c | 1 +
> drivers/net/ethernet/mscc/ocelot_ptp.c | 12 +++-
> include/soc/mscc/ocelot.h | 7 +++
> 4 files changed, 95 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
> index 570d0204b7be..dd9085ae0922 100644
> --- a/drivers/net/dsa/ocelot/felix_vsc9959.c
> +++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
> @@ -1196,10 +1196,13 @@ static void vsc9959_tas_gcl_set(struct ocelot *ocelot, const u32 gcl_ix,
> static int vsc9959_qos_port_tas_set(struct ocelot *ocelot, int port,
> struct tc_taprio_qopt_offload *taprio)
> {
> + struct ocelot_port *ocelot_port = ocelot->ports[port];
> struct timespec64 base_ts;
> int ret, i;
> u32 val;
>
> + mutex_lock(&ocelot->tas_lock);
> +
> if (!taprio->enable) {
> ocelot_rmw_rix(ocelot,
> QSYS_TAG_CONFIG_INIT_GATE_STATE(0xFF),
> @@ -1207,15 +1210,20 @@ static int vsc9959_qos_port_tas_set(struct ocelot *ocelot, int port,
> QSYS_TAG_CONFIG_INIT_GATE_STATE_M,
> QSYS_TAG_CONFIG, port);
>
> + mutex_unlock(&ocelot->tas_lock);
> return 0;
> }
>
> if (taprio->cycle_time > NSEC_PER_SEC ||
> - taprio->cycle_time_extension >= NSEC_PER_SEC)
> - return -EINVAL;
> + taprio->cycle_time_extension >= NSEC_PER_SEC) {
> + ret = -EINVAL;
> + goto err;
> + }
>
> - if (taprio->num_entries > VSC9959_TAS_GCL_ENTRY_MAX)
> - return -ERANGE;
> + if (taprio->num_entries > VSC9959_TAS_GCL_ENTRY_MAX) {
> + ret = -ERANGE;
> + goto err;
> + }
>
> /* Enable guard band. The switch will schedule frames without taking
> * their length into account. Thus we'll always need to enable the
> @@ -1236,8 +1244,10 @@ static int vsc9959_qos_port_tas_set(struct ocelot *ocelot, int port,
> * config is pending, need reset the TAS module
> */
> val = ocelot_read(ocelot, QSYS_PARAM_STATUS_REG_8);
> - if (val & QSYS_PARAM_STATUS_REG_8_CONFIG_PENDING)
> - return -EBUSY;
> + if (val & QSYS_PARAM_STATUS_REG_8_CONFIG_PENDING) {
> + ret = -EBUSY;
> + goto err;
> + }
>
> ocelot_rmw_rix(ocelot,
> QSYS_TAG_CONFIG_ENABLE |
> @@ -1248,6 +1258,8 @@ static int vsc9959_qos_port_tas_set(struct ocelot *ocelot, int port,
> QSYS_TAG_CONFIG_SCH_TRAFFIC_QUEUES_M,
> QSYS_TAG_CONFIG, port);
>
> + ocelot_port->base_time = taprio->base_time;
> +
> vsc9959_new_base_time(ocelot, taprio->base_time,
> taprio->cycle_time, &base_ts);
> ocelot_write(ocelot, base_ts.tv_nsec, QSYS_PARAM_CFG_REG_1);
> @@ -1271,9 +1283,67 @@ static int vsc9959_qos_port_tas_set(struct ocelot *ocelot, int port,
> !(val & QSYS_TAS_PARAM_CFG_CTRL_CONFIG_CHANGE),
> 10, 100000);
>
> +err:
> + mutex_unlock(&ocelot->tas_lock);
> +
> return ret;
> }
>
> +static void vsc9959_tas_clock_adjust(struct ocelot *ocelot)
> +{
> + struct ocelot_port *ocelot_port;
> + struct timespec64 base_ts;
> + u64 cycletime;
> + int port;
> + u32 val;
> +
> + mutex_lock(&ocelot->tas_lock);
> +
> + for (port = 0; port < ocelot->num_phys_ports; port++) {
> + val = ocelot_read_rix(ocelot, QSYS_TAG_CONFIG, port);
> + if (!(val & QSYS_TAG_CONFIG_ENABLE))
> + continue;
> +
> + ocelot_rmw(ocelot,
> + QSYS_TAS_PARAM_CFG_CTRL_PORT_NUM(port),
> + QSYS_TAS_PARAM_CFG_CTRL_PORT_NUM_M,
> + QSYS_TAS_PARAM_CFG_CTRL);
> +
> + ocelot_rmw_rix(ocelot,
> + QSYS_TAG_CONFIG_INIT_GATE_STATE(0xFF),
> + QSYS_TAG_CONFIG_ENABLE |
> + QSYS_TAG_CONFIG_INIT_GATE_STATE_M,
> + QSYS_TAG_CONFIG, port);
> +
> + cycletime = ocelot_read(ocelot, QSYS_PARAM_CFG_REG_4);
> + ocelot_port = ocelot->ports[port];
> +
> + vsc9959_new_base_time(ocelot, ocelot_port->base_time,
> + cycletime, &base_ts);
> +
> + ocelot_write(ocelot, base_ts.tv_nsec, QSYS_PARAM_CFG_REG_1);
> + ocelot_write(ocelot, lower_32_bits(base_ts.tv_sec),
> + QSYS_PARAM_CFG_REG_2);
> + val = upper_32_bits(base_ts.tv_sec);
> + ocelot_rmw(ocelot,
> + QSYS_PARAM_CFG_REG_3_BASE_TIME_SEC_MSB(val),
> + QSYS_PARAM_CFG_REG_3_BASE_TIME_SEC_MSB_M,
> + QSYS_PARAM_CFG_REG_3);
> +
> + ocelot_rmw(ocelot, QSYS_TAS_PARAM_CFG_CTRL_CONFIG_CHANGE,
> + QSYS_TAS_PARAM_CFG_CTRL_CONFIG_CHANGE,
> + QSYS_TAS_PARAM_CFG_CTRL);
> +
> + ocelot_rmw_rix(ocelot,
> + QSYS_TAG_CONFIG_INIT_GATE_STATE(0xFF) |
> + QSYS_TAG_CONFIG_ENABLE,
> + QSYS_TAG_CONFIG_ENABLE |
> + QSYS_TAG_CONFIG_INIT_GATE_STATE_M,
> + QSYS_TAG_CONFIG, port);
> + }
> + mutex_unlock(&ocelot->tas_lock);
> +}
> +
> static int vsc9959_qos_port_cbs_set(struct dsa_switch *ds, int port,
> struct tc_cbs_qopt_offload *cbs_qopt)
> {
> @@ -2210,6 +2280,7 @@ static const struct ocelot_ops vsc9959_ops = {
> .psfp_filter_del = vsc9959_psfp_filter_del,
> .psfp_stats_get = vsc9959_psfp_stats_get,
> .cut_through_fwd = vsc9959_cut_through_fwd,
> + .tas_clock_adjust = vsc9959_tas_clock_adjust,
> };
>
> static const struct felix_info felix_info_vsc9959 = {
> diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
> index 8da7e25a47c9..d4649e4ee0e7 100644
> --- a/drivers/net/ethernet/mscc/ocelot.c
> +++ b/drivers/net/ethernet/mscc/ocelot.c
> @@ -3367,6 +3367,7 @@ int ocelot_init(struct ocelot *ocelot)
> mutex_init(&ocelot->ptp_lock);
> mutex_init(&ocelot->mact_lock);
> mutex_init(&ocelot->fwd_domain_lock);
> + mutex_init(&ocelot->tas_lock);
> spin_lock_init(&ocelot->ptp_clock_lock);
> spin_lock_init(&ocelot->ts_id_lock);
> snprintf(queue_name, sizeof(queue_name), "%s-stats",
> diff --git a/drivers/net/ethernet/mscc/ocelot_ptp.c b/drivers/net/ethernet/mscc/ocelot_ptp.c
> index 87ad2137ba06..522fdc38d4d0 100644
> --- a/drivers/net/ethernet/mscc/ocelot_ptp.c
> +++ b/drivers/net/ethernet/mscc/ocelot_ptp.c
> @@ -72,15 +72,19 @@ int ocelot_ptp_settime64(struct ptp_clock_info *ptp,
> ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
>
> spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
> +
> + if (ocelot->ops->tas_clock_adjust)
> + ocelot->ops->tas_clock_adjust(ocelot);
> +
> return 0;
> }
> EXPORT_SYMBOL(ocelot_ptp_settime64);
>
> int ocelot_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
> {
> + struct ocelot *ocelot = container_of(ptp, struct ocelot, ptp_info);
> +
> if (delta > -(NSEC_PER_SEC / 2) && delta < (NSEC_PER_SEC / 2)) {
> - struct ocelot *ocelot = container_of(ptp, struct ocelot,
> - ptp_info);
> unsigned long flags;
> u32 val;
>
> @@ -117,6 +121,10 @@ int ocelot_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
>
> ocelot_ptp_settime64(ptp, &ts);
> }
> +
> + if (ocelot->ops->tas_clock_adjust)
> + ocelot->ops->tas_clock_adjust(ocelot);
> +
Here tas_clock_adjust() may be called twice (first from
ocelot_ptp_settime64() )
> return 0;
> }
> EXPORT_SYMBOL(ocelot_ptp_adjtime);
> diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
> index 5f88385a7748..3737570116c3 100644
> --- a/include/soc/mscc/ocelot.h
> +++ b/include/soc/mscc/ocelot.h
> @@ -575,6 +575,7 @@ struct ocelot_ops {
> int (*psfp_stats_get)(struct ocelot *ocelot, struct flow_cls_offload *f,
> struct flow_stats *stats);
> void (*cut_through_fwd)(struct ocelot *ocelot);
> + void (*tas_clock_adjust)(struct ocelot *ocelot);
> };
>
> struct ocelot_vcap_policer {
> @@ -691,6 +692,9 @@ struct ocelot_port {
> int bridge_num;
>
> int speed;
> +
> + /* Store the AdminBaseTime of EST fetched from userspace. */
> + s64 base_time;
> };
>
> struct ocelot {
> @@ -757,6 +761,9 @@ struct ocelot {
> /* Lock for serializing forwarding domain changes */
> struct mutex fwd_domain_lock;
>
> + /* Lock for serializing Time-Aware Shaper changes */
> + struct mutex tas_lock;
> +
Why do we need extra mutex? ocelot_ptp_settime64() and
ocelot_ptp_adjtime() already uses
spin_lock_irqsave(&ocelot->ptp_clock_lock, flags)
> struct workqueue_struct *owq;
>
> u8 ptp:1;
^ permalink raw reply [flat|nested] 3+ messages in thread
* 回复: [EXT] Re: [PATCH v1 net-next] net: dsa: felix: update base time of time-aware shaper when adjusting PTP time
2022-06-16 7:05 ` Maxim Kochetkov
@ 2022-06-16 8:16 ` Xiaoliang Yang
0 siblings, 0 replies; 3+ messages in thread
From: Xiaoliang Yang @ 2022-06-16 8:16 UTC (permalink / raw)
To: Maxim Kochetkov, davem, netdev, linux-kernel
Cc: allan.nielsen, joergen.andreasen, horatiu.vultur, UNGLinuxDriver,
alexandre.belloni, f.fainelli, vivien.didelot, pabeni, kuba,
edumazet, andrew, Alexandru Marginean, Claudiu Manoil,
Vladimir Oltean, Leo Li, Y.B. Lu
On Thu, 16 Jun 2022 15:05 +0800 Maxim Kochetkov wrote:
> On 16.06.2022 09:46, Xiaoliang Yang wrote:
> > When adjusting the PTP clock, the base time of the TAS configuration
> > will become unreliable. We need reset the TAS configuration by using a
> > new base time.
> >
> > For example, if the driver gets a base time 0 of Qbv configuration
> > from user, and current time is 20000. The driver will set the TAS base
> > time to be 20000. After the PTP clock adjustment, the current time
> > becomes 10000. If the TAS base time is still 20000, it will be a
> > future time, and TAS entry list will stop running. Another example, if
> > the current time becomes to be 10000000 after PTP clock adjust, a
> > large time offset can cause the hardware to hang.
> >
> > This patch introduces a tas_clock_adjust() function to reset the TAS
> > module by using a new base time after the PTP clock adjustment. This
> > can avoid issues above.
> >
> > Due to PTP clock adjustment can occur at any time, it may conflict
> > with the TAS configuration. We introduce a new TAS lock to serialize
> > the access to the TAS registers.
> >
> > Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
> > ---
> > drivers/net/dsa/ocelot/felix_vsc9959.c | 83
> ++++++++++++++++++++++++--
> > drivers/net/ethernet/mscc/ocelot.c | 1 +
> > drivers/net/ethernet/mscc/ocelot_ptp.c | 12 +++-
> > include/soc/mscc/ocelot.h | 7 +++
> > 4 files changed, 95 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c
> > b/drivers/net/dsa/ocelot/felix_vsc9959.c
> > index 570d0204b7be..dd9085ae0922 100644
> > --- a/drivers/net/dsa/ocelot/felix_vsc9959.c
> > +++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
> > @@ -1196,10 +1196,13 @@ static void vsc9959_tas_gcl_set(struct ocelot
> *ocelot, const u32 gcl_ix,
> > static int vsc9959_qos_port_tas_set(struct ocelot *ocelot, int port,
> > struct tc_taprio_qopt_offload
> *taprio)
> > {
> > + struct ocelot_port *ocelot_port = ocelot->ports[port];
> > struct timespec64 base_ts;
> > int ret, i;
> > u32 val;
> >
> > + mutex_lock(&ocelot->tas_lock);
> > +
> > if (!taprio->enable) {
> > ocelot_rmw_rix(ocelot,
> >
> QSYS_TAG_CONFIG_INIT_GATE_STATE(0xFF),
> > @@ -1207,15 +1210,20 @@ static int vsc9959_qos_port_tas_set(struct
> ocelot *ocelot, int port,
> > QSYS_TAG_CONFIG_INIT_GATE_STATE_M,
> > QSYS_TAG_CONFIG, port);
> >
> > + mutex_unlock(&ocelot->tas_lock);
> > return 0;
> > }
> >
> > if (taprio->cycle_time > NSEC_PER_SEC ||
> > - taprio->cycle_time_extension >= NSEC_PER_SEC)
> > - return -EINVAL;
> > + taprio->cycle_time_extension >= NSEC_PER_SEC) {
> > + ret = -EINVAL;
> > + goto err;
> > + }
> >
> > - if (taprio->num_entries > VSC9959_TAS_GCL_ENTRY_MAX)
> > - return -ERANGE;
> > + if (taprio->num_entries > VSC9959_TAS_GCL_ENTRY_MAX) {
> > + ret = -ERANGE;
> > + goto err;
> > + }
> >
> > /* Enable guard band. The switch will schedule frames without taking
> > * their length into account. Thus we'll always need to enable
> > the @@ -1236,8 +1244,10 @@ static int vsc9959_qos_port_tas_set(struct
> ocelot *ocelot, int port,
> > * config is pending, need reset the TAS module
> > */
> > val = ocelot_read(ocelot, QSYS_PARAM_STATUS_REG_8);
> > - if (val & QSYS_PARAM_STATUS_REG_8_CONFIG_PENDING)
> > - return -EBUSY;
> > + if (val & QSYS_PARAM_STATUS_REG_8_CONFIG_PENDING) {
> > + ret = -EBUSY;
> > + goto err;
> > + }
> >
> > ocelot_rmw_rix(ocelot,
> > QSYS_TAG_CONFIG_ENABLE | @@ -1248,6
> +1258,8 @@
> > static int vsc9959_qos_port_tas_set(struct ocelot *ocelot, int port,
> > QSYS_TAG_CONFIG_SCH_TRAFFIC_QUEUES_M,
> > QSYS_TAG_CONFIG, port);
> >
> > + ocelot_port->base_time = taprio->base_time;
> > +
> > vsc9959_new_base_time(ocelot, taprio->base_time,
> > taprio->cycle_time, &base_ts);
> > ocelot_write(ocelot, base_ts.tv_nsec, QSYS_PARAM_CFG_REG_1);
> @@
> > -1271,9 +1283,67 @@ static int vsc9959_qos_port_tas_set(struct ocelot
> *ocelot, int port,
> > !(val &
> QSYS_TAS_PARAM_CFG_CTRL_CONFIG_CHANGE),
> > 10, 100000);
> >
> > +err:
> > + mutex_unlock(&ocelot->tas_lock);
> > +
> > return ret;
> > }
> >
> > +static void vsc9959_tas_clock_adjust(struct ocelot *ocelot) {
> > + struct ocelot_port *ocelot_port;
> > + struct timespec64 base_ts;
> > + u64 cycletime;
> > + int port;
> > + u32 val;
> > +
> > + mutex_lock(&ocelot->tas_lock);
> > +
> > + for (port = 0; port < ocelot->num_phys_ports; port++) {
> > + val = ocelot_read_rix(ocelot, QSYS_TAG_CONFIG, port);
> > + if (!(val & QSYS_TAG_CONFIG_ENABLE))
> > + continue;
> > +
> > + ocelot_rmw(ocelot,
> > +
> QSYS_TAS_PARAM_CFG_CTRL_PORT_NUM(port),
> > + QSYS_TAS_PARAM_CFG_CTRL_PORT_NUM_M,
> > + QSYS_TAS_PARAM_CFG_CTRL);
> > +
> > + ocelot_rmw_rix(ocelot,
> > +
> QSYS_TAG_CONFIG_INIT_GATE_STATE(0xFF),
> > + QSYS_TAG_CONFIG_ENABLE |
> > + QSYS_TAG_CONFIG_INIT_GATE_STATE_M,
> > + QSYS_TAG_CONFIG, port);
> > +
> > + cycletime = ocelot_read(ocelot, QSYS_PARAM_CFG_REG_4);
> > + ocelot_port = ocelot->ports[port];
> > +
> > + vsc9959_new_base_time(ocelot, ocelot_port->base_time,
> > + cycletime, &base_ts);
> > +
> > + ocelot_write(ocelot, base_ts.tv_nsec,
> QSYS_PARAM_CFG_REG_1);
> > + ocelot_write(ocelot, lower_32_bits(base_ts.tv_sec),
> > + QSYS_PARAM_CFG_REG_2);
> > + val = upper_32_bits(base_ts.tv_sec);
> > + ocelot_rmw(ocelot,
> > +
> QSYS_PARAM_CFG_REG_3_BASE_TIME_SEC_MSB(val),
> > +
> QSYS_PARAM_CFG_REG_3_BASE_TIME_SEC_MSB_M,
> > + QSYS_PARAM_CFG_REG_3);
> > +
> > + ocelot_rmw(ocelot,
> QSYS_TAS_PARAM_CFG_CTRL_CONFIG_CHANGE,
> > +
> QSYS_TAS_PARAM_CFG_CTRL_CONFIG_CHANGE,
> > + QSYS_TAS_PARAM_CFG_CTRL);
> > +
> > + ocelot_rmw_rix(ocelot,
> > + QSYS_TAG_CONFIG_INIT_GATE_STATE(0xFF)
> |
> > + QSYS_TAG_CONFIG_ENABLE,
> > + QSYS_TAG_CONFIG_ENABLE |
> > + QSYS_TAG_CONFIG_INIT_GATE_STATE_M,
> > + QSYS_TAG_CONFIG, port);
> > + }
> > + mutex_unlock(&ocelot->tas_lock); }
> > +
> > static int vsc9959_qos_port_cbs_set(struct dsa_switch *ds, int port,
> > struct tc_cbs_qopt_offload
> *cbs_qopt)
> > {
> > @@ -2210,6 +2280,7 @@ static const struct ocelot_ops vsc9959_ops = {
> > .psfp_filter_del = vsc9959_psfp_filter_del,
> > .psfp_stats_get = vsc9959_psfp_stats_get,
> > .cut_through_fwd = vsc9959_cut_through_fwd,
> > + .tas_clock_adjust = vsc9959_tas_clock_adjust,
> > };
> >
> > static const struct felix_info felix_info_vsc9959 = { diff --git
> > a/drivers/net/ethernet/mscc/ocelot.c
> > b/drivers/net/ethernet/mscc/ocelot.c
> > index 8da7e25a47c9..d4649e4ee0e7 100644
> > --- a/drivers/net/ethernet/mscc/ocelot.c
> > +++ b/drivers/net/ethernet/mscc/ocelot.c
> > @@ -3367,6 +3367,7 @@ int ocelot_init(struct ocelot *ocelot)
> > mutex_init(&ocelot->ptp_lock);
> > mutex_init(&ocelot->mact_lock);
> > mutex_init(&ocelot->fwd_domain_lock);
> > + mutex_init(&ocelot->tas_lock);
> > spin_lock_init(&ocelot->ptp_clock_lock);
> > spin_lock_init(&ocelot->ts_id_lock);
> > snprintf(queue_name, sizeof(queue_name), "%s-stats", diff --git
> > a/drivers/net/ethernet/mscc/ocelot_ptp.c
> > b/drivers/net/ethernet/mscc/ocelot_ptp.c
> > index 87ad2137ba06..522fdc38d4d0 100644
> > --- a/drivers/net/ethernet/mscc/ocelot_ptp.c
> > +++ b/drivers/net/ethernet/mscc/ocelot_ptp.c
> > @@ -72,15 +72,19 @@ int ocelot_ptp_settime64(struct ptp_clock_info
> *ptp,
> > ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
> >
> > spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
> > +
> > + if (ocelot->ops->tas_clock_adjust)
> > + ocelot->ops->tas_clock_adjust(ocelot);
> > +
> > return 0;
> > }
> > EXPORT_SYMBOL(ocelot_ptp_settime64);
> >
> > int ocelot_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
> > {
> > + struct ocelot *ocelot = container_of(ptp, struct ocelot,
> > + ptp_info);
> > +
> > if (delta > -(NSEC_PER_SEC / 2) && delta < (NSEC_PER_SEC / 2)) {
> > - struct ocelot *ocelot = container_of(ptp, struct ocelot,
> > - ptp_info);
> > unsigned long flags;
> > u32 val;
> >
> > @@ -117,6 +121,10 @@ int ocelot_ptp_adjtime(struct ptp_clock_info
> > *ptp, s64 delta)
> >
> > ocelot_ptp_settime64(ptp, &ts);
> > }
> > +
> > + if (ocelot->ops->tas_clock_adjust)
> > + ocelot->ops->tas_clock_adjust(ocelot);
> > +
>
> Here tas_clock_adjust() may be called twice (first from
> ocelot_ptp_settime64() )
>
Sure, it may be called twice here, I would correct that, thanks.
> > return 0;
> > }
> > EXPORT_SYMBOL(ocelot_ptp_adjtime);
> > diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
> > index 5f88385a7748..3737570116c3 100644
> > --- a/include/soc/mscc/ocelot.h
> > +++ b/include/soc/mscc/ocelot.h
> > @@ -575,6 +575,7 @@ struct ocelot_ops {
> > int (*psfp_stats_get)(struct ocelot *ocelot, struct flow_cls_offload *f,
> > struct flow_stats *stats);
> > void (*cut_through_fwd)(struct ocelot *ocelot);
> > + void (*tas_clock_adjust)(struct ocelot *ocelot);
> > };
> >
> > struct ocelot_vcap_policer {
> > @@ -691,6 +692,9 @@ struct ocelot_port {
> > int bridge_num;
> >
> > int speed;
> > +
> > + /* Store the AdminBaseTime of EST fetched from userspace. */
> > + s64 base_time;
> > };
> >
> > struct ocelot {
> > @@ -757,6 +761,9 @@ struct ocelot {
> > /* Lock for serializing forwarding domain changes */
> > struct mutex fwd_domain_lock;
> >
> > + /* Lock for serializing Time-Aware Shaper changes */
> > + struct mutex tas_lock;
> > +
> Why do we need extra mutex? ocelot_ptp_settime64() and
> ocelot_ptp_adjtime() already uses
> spin_lock_irqsave(&ocelot->ptp_clock_lock, flags)
>
This mutex is to serialize TAS configuration. The TAS is configured not
only through ptp adjust, but also via tc-taprio. Therefore I add a new
tas_lock.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-06-16 8:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-16 6:46 [PATCH v1 net-next] net: dsa: felix: update base time of time-aware shaper when adjusting PTP time Xiaoliang Yang
2022-06-16 7:05 ` Maxim Kochetkov
2022-06-16 8:16 ` 回复: [EXT] " Xiaoliang Yang
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.