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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 5 Sep 2020 10:02:58 -0700
From:   Richard Cochran <richardcochran@...il.com>
To:     Russell King <rmk+kernel@...linux.org.uk>
Cc:     Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Antoine Tenart <antoine.tenart@...tlin.com>,
        Matteo Croce <mcroce@...hat.com>,
        Andre Przywara <andre.przywara@....com>,
        Sven Auhagen <sven.auhagen@...eatech.de>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org
Subject: Re: [PATCH net-next v2 4/6] net: mvpp2: ptp: add TAI support

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@...linux.org.uk>

Acked-by: Richard Cochran <richardcochran@...il.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