Date:   Fri, 27 Jan 2023 16:14:48 +0100
From:   Simon Horman <simon.horman@corigine.com>
To:     Alexandru Tachici <alexandru.tachici@analog.com>
Cc:     linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
        davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
        pabeni@redhat.com, richardcochran@gmail.com,
        yangyingliang@huawei.com, weiyongjun1@huawei.com,
        robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
        devicetree@vger.kernel.org, lennart@lfdomain.com
Subject: Re: [net-next v2 1/3] net: ethernet: adi: adin1110: add PTP clock
 support
Message-ID: <Y9PqaJGwAUAZo2Rx@corigine.com>
References: <20230126133346.61097-1-alexandru.tachici@analog.com>
 <20230126133346.61097-2-alexandru.tachici@analog.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230126133346.61097-2-alexandru.tachici@analog.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?1gpwz8iEQnLuPIjdL2Sj3UTFB/FrempFj55WmDF7Yxr9gSBweSJgELnBTwwn?=
 =?us-ascii?Q?LbQVxruQOTH5P+p2OPq+EZHUYrz2mSOVgnG5OnLV8THKe2jLiNVREuMNcmTq?=
 =?us-ascii?Q?UNYUfJ7FcNcb98N2OjKSB//o2PIauEaz+vHHNCPofzidSHvPpTPiIOQ6SAM/?=
 =?us-ascii?Q?EHCN7U1LHvKMPS81vxlatB0dfUaTSWvLUSakxO5Egl7ZeQXH+5STTb24npEL?=
 =?us-ascii?Q?UYN8DWvRcfgtS3it8w6CZKvZ8t6Sq1wzv6JbNaBiBFpKt69mtSZ3z/OOGPos?=
 =?us-ascii?Q?iKFcHMNhHjc1I6S0vqBNGgRhlIhE8iCW+a1Mp5OS6xC6nt9ThZZwfVED0CTt?=
 =?us-ascii?Q?dxuHvVDO3erL567jJ6MBCo+lmFij+Il8z4QI4V7rQ6X78rtY7phkUBxLf/+v?=
 =?us-ascii?Q?cGwHnCBaUTIG18asvQUol48bCUTUq/grsKYqjm/GRNEdrBwQHBLkEDMpk086?=
 =?us-ascii?Q?Stb3USnagKapOBhD4RWyiMm9c+HJaqWl6nXUmWvCjx/wgC00jfPyib/D3ZuU?=
 =?us-ascii?Q?0RT44RIUX5da80lIP72wPLwdzjaY3xXaCW3XO1t0cXQlIcLHCeR+4diPv7ry?=
 =?us-ascii?Q?MvIXphHzXW3NnuLegyibirmTJ2M5otp7VV5ZYDmEPjElE8jBAMSf93UJysoP?=
 =?us-ascii?Q?CUBtmTsi1hcgA1jrxviIig20cjueLgrVt+RwkZVdZ3GdKMqA9Q3wKu85i99Z?=
 =?us-ascii?Q?5atQdAMzjD4lBZc3UmNy9gG9+Vy9awLMztIWX8njrVpUW4t//nQQyyodo56Q?=
 =?us-ascii?Q?emLpneTAuwZ6fbeiLwnlAwBbqa/Vd6ntOcY62MR46NiXzc7K0lD7Q3EVmp4n?=
 =?us-ascii?Q?eVDSED50cdd6dKPGt3OKQnL5101cLk+H8omKOA+cx7PTMDOhANPP5XDwU78z?=
 =?us-ascii?Q?ith7KvnGZ+GYaVs2rxmjoGeE0RM1iylEiU5XZ+112c5v/mtEWPLCKVPz0o7n?=
 =?us-ascii?Q?gxqoK8WojVVzHeKa8DWq9QVy00pfItcyzk9IRTAAL7KHsUiBqeVM3P/lmydw?=
 =?us-ascii?Q?Q9w8g5rm73BZu+hviL74W1+aOTs7Eh35ou86lEIs3KgJKnOMpjPkhQCYrd4w?=
 =?us-ascii?Q?F3GUBeJW0qGaqb48ch+QTW6n6+do+9Yo1dMLVi4gq2pFHDT3tyHm+zyBI/4m?=
 =?us-ascii?Q?dN1P7mz/WNysp4X4cJ8tQR5DuCpevsfT4WLfI8VfQU+BEMV0YPGhamwv6k1m?=
 =?us-ascii?Q?3GAFYAzlSV/IxAxlMF6T/j8e0gz8PNiQ6s+gFuH2X7Z6Pe4wNwHqlbx+m9dK?=
 =?us-ascii?Q?zYgZt8gXpml444WsltQ6Bo4EgeSG3prLcdeRx9nJYUTc1HTFR+bwo216BFfY?=
 =?us-ascii?Q?4J5ZuJ0gQOHHhP/EQyYcBnIYHtR6pIya5OhkB/ZJROPrwjD9Nbjcm7i3yIJt?=
 =?us-ascii?Q?5SymfYI9mizRFwGuG8o7O1XI32xhf8inreYR+BDL+w4SpnNvtqoI5z2C1flG?=
 =?us-ascii?Q?cEuYhq0qy8UtWZj4tZeXZ42C8hDQX2BbTBjWvfvDQFgLjqZN7FuCeEGU2E/Y?=
 =?us-ascii?Q?Ykeo5WzlOQExmFbndoIOO+v019qnclhqdSE8JLDD3YQ62dNd00m88pqTlBVH?=
 =?us-ascii?Q?5sVIeoDiCLqxF5FHumGgt0/NFIXaxeGYjk0mc6Zt9oeCeC0BVxYLCdWVuFQ8?=
 =?us-ascii?Q?8fLe5yTyqtTwW6U4PKpnZe8eJFDxRoNE9+2IdOb/olMdJOFiYwkdrODb/m3/?=
 =?us-ascii?Q?QDIQPA=3D=3D?=
X-OriginatorOrg: corigine.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 5b243729-0325-42f5-c3b8-08db00793f46
X-MS-Exchange-CrossTenant-AuthSource: PH0PR13MB4842.namprd13.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Jan 2023 15:14:55.7733
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: fe128f2c-073b-4c20-818e-7246a585940c
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: M2fogo38GRi+b9gJqXZTVEk8f+jVUmlrSSiNAh1jMlRL4X/p8g/Z2LsFSFjm5rCyU+wITfG2C1XTUvJFnenyTBHmoGobrxb0KvH5AA4pD7Y=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH2PR13MB3751
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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