Re: [net-next v2 1/3] net: ethernet: adi: adin1110: add PTP clock support

From: Simon Horman
Date: Fri Jan 27 2023 - 10:15:07 EST


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@xxxxxxxxxx>

...

> @@ -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.