| Message ID | 20240618073821.619751-1-o.rempel@pengutronix.de (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 8851346912a1fa33e7a5966fe51f07313b274627 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v1,1/1] net: stmmac: Assign configured channel value to EXTTS event | expand |
On 18.06.2024 09:38, Oleksij Rempel wrote: > Assign the configured channel value to the EXTTS event in the timestamp > interrupt handler. Without assigning the correct channel, applications > like ts2phc will refuse to accept the event, resulting in errors such > as: > ... > ts2phc[656.834]: config item end1.ts2phc.pin_index is 0 > ts2phc[656.834]: config item end1.ts2phc.channel is 3 > ts2phc[656.834]: config item end1.ts2phc.extts_polarity is 2 > ts2phc[656.834]: config item end1.ts2phc.extts_correction is 0 > ... > ts2phc[656.862]: extts on unexpected channel > ts2phc[658.141]: extts on unexpected channel > ts2phc[659.140]: extts on unexpected channel > > Fixes: f4da56529da60 ("net: stmmac: Add support for external trigger timestamping") > Cc: stable@vger.kernel.org > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > --- Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com> > drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c > index f05bd757dfe52..5ef52ef2698fb 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c > @@ -218,6 +218,7 @@ static void timestamp_interrupt(struct stmmac_priv *priv) > { > u32 num_snapshot, ts_status, tsync_int; > struct ptp_clock_event event; > + u32 acr_value, channel; > unsigned long flags; > u64 ptp_time; > int i; > @@ -243,12 +244,15 @@ static void timestamp_interrupt(struct stmmac_priv *priv) > num_snapshot = (ts_status & GMAC_TIMESTAMP_ATSNS_MASK) >> > GMAC_TIMESTAMP_ATSNS_SHIFT; > > + acr_value = readl(priv->ptpaddr + PTP_ACR); > + channel = ilog2(FIELD_GET(PTP_ACR_MASK, acr_value)); > + > for (i = 0; i < num_snapshot; i++) { > read_lock_irqsave(&priv->ptp_lock, flags); > get_ptptime(priv->ptpaddr, &ptp_time); > read_unlock_irqrestore(&priv->ptp_lock, flags); > event.type = PTP_CLOCK_EXTTS; > - event.index = 0; > + event.index = channel; > event.timestamp = ptp_time; > ptp_clock_event(priv->ptp_clock, &event); > }
On Tue, 2024-06-18 at 09:38 +0200, Oleksij Rempel wrote: > @@ -243,12 +244,15 @@ static void timestamp_interrupt(struct stmmac_priv *priv) > num_snapshot = (ts_status & GMAC_TIMESTAMP_ATSNS_MASK) >> > GMAC_TIMESTAMP_ATSNS_SHIFT; > > + acr_value = readl(priv->ptpaddr + PTP_ACR); > + channel = ilog2(FIELD_GET(PTP_ACR_MASK, acr_value)); Side note for a possible net-next follow-up: as the driver already has an equal snippet in stmmac_enable(), you con consider factor it out in a common, more readable helper. Cheers, Paolo
Hello: This patch was applied to netdev/net.git (main) by Paolo Abeni <pabeni@redhat.com>: On Tue, 18 Jun 2024 09:38:21 +0200 you wrote: > Assign the configured channel value to the EXTTS event in the timestamp > interrupt handler. Without assigning the correct channel, applications > like ts2phc will refuse to accept the event, resulting in errors such > as: > ... > ts2phc[656.834]: config item end1.ts2phc.pin_index is 0 > ts2phc[656.834]: config item end1.ts2phc.channel is 3 > ts2phc[656.834]: config item end1.ts2phc.extts_polarity is 2 > ts2phc[656.834]: config item end1.ts2phc.extts_correction is 0 > ... > ts2phc[656.862]: extts on unexpected channel > ts2phc[658.141]: extts on unexpected channel > ts2phc[659.140]: extts on unexpected channel > > [...] Here is the summary with links: - [net,v1,1/1] net: stmmac: Assign configured channel value to EXTTS event https://git.kernel.org/netdev/net/c/8851346912a1 You are awesome, thank you!
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c index f05bd757dfe52..5ef52ef2698fb 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c @@ -218,6 +218,7 @@ static void timestamp_interrupt(struct stmmac_priv *priv) { u32 num_snapshot, ts_status, tsync_int; struct ptp_clock_event event; + u32 acr_value, channel; unsigned long flags; u64 ptp_time; int i; @@ -243,12 +244,15 @@ static void timestamp_interrupt(struct stmmac_priv *priv) num_snapshot = (ts_status & GMAC_TIMESTAMP_ATSNS_MASK) >> GMAC_TIMESTAMP_ATSNS_SHIFT; + acr_value = readl(priv->ptpaddr + PTP_ACR); + channel = ilog2(FIELD_GET(PTP_ACR_MASK, acr_value)); + for (i = 0; i < num_snapshot; i++) { read_lock_irqsave(&priv->ptp_lock, flags); get_ptptime(priv->ptpaddr, &ptp_time); read_unlock_irqrestore(&priv->ptp_lock, flags); event.type = PTP_CLOCK_EXTTS; - event.index = 0; + event.index = channel; event.timestamp = ptp_time; ptp_clock_event(priv->ptp_clock, &event); }
Assign the configured channel value to the EXTTS event in the timestamp interrupt handler. Without assigning the correct channel, applications like ts2phc will refuse to accept the event, resulting in errors such as: ... ts2phc[656.834]: config item end1.ts2phc.pin_index is 0 ts2phc[656.834]: config item end1.ts2phc.channel is 3 ts2phc[656.834]: config item end1.ts2phc.extts_polarity is 2 ts2phc[656.834]: config item end1.ts2phc.extts_correction is 0 ... ts2phc[656.862]: extts on unexpected channel ts2phc[658.141]: extts on unexpected channel ts2phc[659.140]: extts on unexpected channel Fixes: f4da56529da60 ("net: stmmac: Add support for external trigger timestamping") Cc: stable@vger.kernel.org Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> --- drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)