Re: [PATCH net-next v2] net: mvpp2: tai: warn once if we fail to update our timestamp

[Mateusz Polchlopek <mateusz.polchlopek@intel.com>]

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>