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

[Katakam, Harini]

Hi Jakub,

> -----Original Message-----
> From: Jakub Kicinski <kuba@xxxxxxxxxx>
> Sent: Thursday, March 23, 2023 10:54 AM
> To: Katakam, Harini <harini.katakam@xxxxxxx>
> Cc: nicolas.ferre@xxxxxxxxxxxxx; davem@xxxxxxxxxxxxx;
> richardcochran@xxxxxxxxx; claudiu.beznea@xxxxxxxxxxxxx;
> andrei.pistirica@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx;
> pabeni@xxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> Simek, Michal <michal.simek@xxxxxxx>; harinikatakamlinux@xxxxxxxxx
> 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