RE: [PATCH net-next v2 1/2] net: macb: Enable PTP unicast

["Katakam, Harini" <harini.katakam@amd.com>]

Hi Jakub,

> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Thursday, March 23, 2023 10:54 AM
> To: Katakam, Harini <harini.katakam@amd.com>
> Cc: nicolas.ferre@microchip.com; davem@davemloft.net;
> richardcochran@gmail.com; claudiu.beznea@microchip.com;
> andrei.pistirica@microchip.com; edumazet@google.com;
> pabeni@redhat.com; netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> Simek, Michal <michal.simek@amd.com>; harinikatakamlinux@gmail.com
> Subject: Re: [PATCH net-next v2 1/2] net: macb: Enable PTP unicast
> 
> On Tue, 21 Mar 2023 18:10:04 +0530 Harini Katakam wrote:
> > +#ifdef CONFIG_MACB_USE_HWSTAMP
> > +	if (gem_has_ptp(bp)) {
> > +		gem_writel(bp, RXPTPUNI, bottom);
> > +		gem_writel(bp, TXPTPUNI, bottom);
> > +	}
> > +#endif
> 
> You can use the same IS_ENABLED() trick here as you used in the if ()
> below, to avoid the direct preprocessor use. In fact why not just
> add the IS_ENABLED(CONFIG_MACB_USE_HWSTAMP) to the condition inside
> gem_has_ptp() ? It looks like all callers want that extra check.

Thanks for the suggestion.
I believe gem_has_ptp was originally defined for checking device
capability (Atmel/SiFive/AMD) and CONFIG_MACB_USE_HWSTAMP for the
kernel selection but I agree that there is currently no usecase for a 
scenario where gem_has_ptp is TRUE and MACB_USE_HWSTAMP is false. If
hear are no objections, I'll include this check inside gem_has_ptp and send v3.

Regards,
Harini