Re: [PATCH net-next v2 4/6] net: mvpp2: ptp: add TAI support

[Richard Cochran <richardcochran@gmail.com>]

On Fri, Sep 04, 2020 at 08:29:27AM +0100, Russell King wrote:
> Add support for the TAI block in the mvpp2.2 hardware.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

Acked-by: Richard Cochran <richardcochran@gmail.com>

A few minor questions/comments follow...

> diff --git a/drivers/net/ethernet/marvell/Kconfig b/drivers/net/ethernet/marvell/Kconfig
> index ef4f35ba077d..a599e44a36a8 100644
> --- a/drivers/net/ethernet/marvell/Kconfig
> +++ b/drivers/net/ethernet/marvell/Kconfig
> @@ -92,6 +92,12 @@ config MVPP2
>  	  This driver supports the network interface units in the
>  	  Marvell ARMADA 375, 7K and 8K SoCs.
>  
> +config MVPP2_PTP
> +	bool "Marvell Armada 8K Enable PTP support"
> +	depends on NETWORK_PHY_TIMESTAMPING
> +	depends on (PTP_1588_CLOCK = y && MVPP2 = y) || \
> +		   (PTP_1588_CLOCK && MVPP2 = m)

So I guess this incantation obviates the need for checking whether
ptp_clock_register() returns null?


> +static long mvpp22_tai_aux_work(struct ptp_clock_info *ptp)
> +{
> +	return 0;
> +}

Since this is a noop, you can leave 
	tai->caps.do_aux_work = mvpp22_tai_aux_work;
as null.

> +static void mvpp22_tai_set_step(struct mvpp2_tai *tai)
> +{
> +	void __iomem *base = tai->base;
> +	u32 nano, frac;
> +
> +	nano = upper_32_bits(tai->period);
> +	frac = lower_32_bits(tai->period);
> +
> +	/* As the fractional nanosecond is a signed offset, if the MSB (sign)
> +	 * bit is set, we have to increment the whole nanoseconds.
> +	 */
> +	if (frac >= 0x80000000)
> +		nano += 1;
> +
> +	mvpp2_tai_write(nano, base + MVPP22_TAI_TOD_STEP_NANO_CR);
> +	mvpp2_tai_write(frac >> 16, base + MVPP22_TAI_TOD_STEP_FRAC_HIGH);
> +	mvpp2_tai_write(frac, base + MVPP22_TAI_TOD_STEP_FRAC_LOW);
> +}
> +
> +static void mvpp22_tai_set_tod(struct mvpp2_tai *tai)
> +{
> +	struct timespec64 now;
> +
> +	ktime_get_real_ts64(&now);
> +	mvpp22_tai_settime64(&tai->caps, &now);
> +}
> +
> +static void mvpp22_tai_init(struct mvpp2_tai *tai)
> +{
> +	void __iomem *base = tai->base;
> +
> +	mvpp22_tai_set_step(tai);
> +
> +	/* Release the TAI reset */
> +	mvpp2_tai_modify(base + MVPP22_TAI_CR0, CR0_SW_NRESET, CR0_SW_NRESET);
> +
> +	mvpp22_tai_set_tod(tai);

The consensus on the list seems to be that new PHCs should start
ticking from time zero (1970), although some older drivers do use
ktime.  For new clocks, I'd prefer using zero.

> +}

Thanks,
Richard