| Message ID | E1tLKNm-006eTd-FD@rmk-PC.armlinux.org.uk (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,v2] net: mvpp2: tai: warn once if we fail to update our timestamp | expand |
On 12/11/2024 11:55 AM, Russell King wrote: > The hardware timestamps for packets contain a truncated seconds field, > only containing two bits of seconds. In order to provide the full > number of seconds, we need to keep track of the full hardware clock by > reading it every two seconds. > > However, if we fail to read the clock, we silently ignore the error. > Print a warning indicating that the PP2 TAI clock timestamps have > become unreliable. > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> > -- > v2: correct dev_warn_once() indentation > --- > drivers/net/ethernet/marvell/mvpp2/mvpp2_tai.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_tai.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_tai.c > index 95862aff49f1..6b60beb1f3ed 100644 > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_tai.c > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_tai.c > @@ -54,6 +54,7 @@ > #define TCSR_CAPTURE_0_VALID BIT(0) > > struct mvpp2_tai { > + struct device *dev; > struct ptp_clock_info caps; > struct ptp_clock *ptp_clock; > void __iomem *base; > @@ -303,7 +304,8 @@ static long mvpp22_tai_aux_work(struct ptp_clock_info *ptp) > { > struct mvpp2_tai *tai = ptp_to_tai(ptp); > > - mvpp22_tai_gettimex64(ptp, &tai->stamp, NULL); > + if (mvpp22_tai_gettimex64(ptp, &tai->stamp, NULL) < 0) > + dev_warn_once(tai->dev, "PTP timestamps are unreliable"); Only small nitpick/question - shouldn't text end with '\n'? I see in the code that most of calls of dev_warn_once has '\n' at the end. > > return msecs_to_jiffies(2000); > } > @@ -401,6 +403,7 @@ int mvpp22_tai_probe(struct device *dev, struct mvpp2 *priv) > > spin_lock_init(&tai->lock); > > + tai->dev = dev; > tai->base = priv->iface_base; > > /* The step size consists of three registers - a 16-bit nanosecond step Nevertheless the code itself looks good to me, nice improvement. Reviewed-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com>
On Wed, Dec 11, 2024 at 12:48:21PM +0100, Mateusz Polchlopek wrote: > On 12/11/2024 11:55 AM, Russell King wrote: > > The hardware timestamps for packets contain a truncated seconds field, > > only containing two bits of seconds. In order to provide the full > > number of seconds, we need to keep track of the full hardware clock by > > reading it every two seconds. > > > > However, if we fail to read the clock, we silently ignore the error. > > Print a warning indicating that the PP2 TAI clock timestamps have > > become unreliable. > > > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> > > -- > > v2: correct dev_warn_once() indentation > > --- > > drivers/net/ethernet/marvell/mvpp2/mvpp2_tai.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_tai.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_tai.c > > index 95862aff49f1..6b60beb1f3ed 100644 > > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_tai.c > > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_tai.c > > @@ -54,6 +54,7 @@ > > #define TCSR_CAPTURE_0_VALID BIT(0) > > struct mvpp2_tai { > > + struct device *dev; > > struct ptp_clock_info caps; > > struct ptp_clock *ptp_clock; > > void __iomem *base; > > @@ -303,7 +304,8 @@ static long mvpp22_tai_aux_work(struct ptp_clock_info *ptp) > > { > > struct mvpp2_tai *tai = ptp_to_tai(ptp); > > - mvpp22_tai_gettimex64(ptp, &tai->stamp, NULL); > > + if (mvpp22_tai_gettimex64(ptp, &tai->stamp, NULL) < 0) > > + dev_warn_once(tai->dev, "PTP timestamps are unreliable"); > > Only small nitpick/question - shouldn't text end with '\n'? I see in the > code that most of calls of dev_warn_once has '\n' at the end. Yes, thanks for spotting.
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_tai.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_tai.c index 95862aff49f1..6b60beb1f3ed 100644 --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_tai.c +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_tai.c @@ -54,6 +54,7 @@ #define TCSR_CAPTURE_0_VALID BIT(0) struct mvpp2_tai { + struct device *dev; struct ptp_clock_info caps; struct ptp_clock *ptp_clock; void __iomem *base; @@ -303,7 +304,8 @@ static long mvpp22_tai_aux_work(struct ptp_clock_info *ptp) { struct mvpp2_tai *tai = ptp_to_tai(ptp); - mvpp22_tai_gettimex64(ptp, &tai->stamp, NULL); + if (mvpp22_tai_gettimex64(ptp, &tai->stamp, NULL) < 0) + dev_warn_once(tai->dev, "PTP timestamps are unreliable"); return msecs_to_jiffies(2000); } @@ -401,6 +403,7 @@ int mvpp22_tai_probe(struct device *dev, struct mvpp2 *priv) spin_lock_init(&tai->lock); + tai->dev = dev; tai->base = priv->iface_base; /* The step size consists of three registers - a 16-bit nanosecond step
The hardware timestamps for packets contain a truncated seconds field, only containing two bits of seconds. In order to provide the full number of seconds, we need to keep track of the full hardware clock by reading it every two seconds. However, if we fail to read the clock, we silently ignore the error. Print a warning indicating that the PP2 TAI clock timestamps have become unreliable. Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> -- v2: correct dev_warn_once() indentation --- drivers/net/ethernet/marvell/mvpp2/mvpp2_tai.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)