linux-kernel - Re: [net-next v2 1/3] net: ethernet: adi: adin1110: add PTP clock 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 linux-cve-announce PHC
Open Source and information security mailing list archives

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

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

Message-ID: <Y9PqaJGwAUAZo2Rx@corigine.com>
Date:   Fri, 27 Jan 2023 16:14:48 +0100
From:   Simon Horman <simon.horman@...igine.com>
To:     Alexandru Tachici <alexandru.tachici@...log.com>
Cc:     linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
        davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
        pabeni@...hat.com, richardcochran@...il.com,
        yangyingliang@...wei.com, weiyongjun1@...wei.com,
        robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
        devicetree@...r.kernel.org, lennart@...omain.com
Subject: Re: [net-next v2 1/3] net: ethernet: adi: adin1110: add PTP clock
 support

On Thu, Jan 26, 2023 at 03:33:44PM +0200, Alexandru Tachici wrote:
> Add control for the PHC inside the ADIN1110/2111.
> Device contains a syntonized counter driven by a 120 MHz
> clock  with 8 ns resolution.
> 
> Time is stored on two registers: a 32bit seconds register and
> a 32bit nanoseconds register.
> 
> For adjusting the clock timing, device uses an addend register.
> Can generate an output signal on the TS_TIMER pin.
> For reading the timestamp the current tiem is saved by setting the
> TS_CAPT pin via gpio in order to snapshot both seconds and nanoseconds
> in different registers that the live ones.
> 
> Signed-off-by: Alexandru Tachici <alexandru.tachici@...log.com>

...

> @@ -150,6 +188,11 @@ struct adin1110_port_priv {
>  struct adin1110_priv {
>  	struct mutex			lock; /* protect spi */
>  	spinlock_t			state_lock; /* protect RX mode */
> +	bool				ts_rx_append;
> +	struct ptp_clock_info		ptp;
> +	struct ptp_clock		*ptp_clock;
> +	struct gpio_desc		*ts_capt;
> +	struct ptp_pin_desc		ptp_pins[ADIN_MAC_MAX_PTP_PINS];
>  	struct mii_bus			*mii_bus;
>  	struct spi_device		*spidev;
>  	bool				append_crc;

nit: I'm unsure if this is important or not.
     But (on x86_64) the arrangement above leads to a 7-byte hole in the
     structure between ts_rx_append and ptp.

     Probably there is much scope to make adin1110_priv more
     cache-line friendly. But in this case an improvement may be
     to locate ts_rx_append immediately before or after append_crc,
     as there is also a 7-byte hole after append_crc.

     Likewise, I think there may be room to improve on the cache-line
     friendliness of the changes to this structure in patch 2/3.