RE: [PATCH net-next v4 3/5] net: phy: Kconfig: Add ptp library support and 1588 optional flag in Microchip phys

[Divya.Koppera]

Hi Jakub,

> -----Original Message-----
> From: Jakub Kicinski <kuba@xxxxxxxxxx>
> Sent: Saturday, November 16, 2024 6:07 AM
> To: Divya Koppera - I30481 <Divya.Koppera@xxxxxxxxxxxxx>
> Cc: andrew@xxxxxxx; Arun Ramadoss - I17769
> <Arun.Ramadoss@xxxxxxxxxxxxx>; UNGLinuxDriver
> <UNGLinuxDriver@xxxxxxxxxxxxx>; hkallweit1@xxxxxxxxx;
> linux@xxxxxxxxxxxxxxx; davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx;
> pabeni@xxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> richardcochran@xxxxxxxxx; vadim.fedorenko@xxxxxxxxx
> Subject: Re: [PATCH net-next v4 3/5] net: phy: Kconfig: Add ptp library
> support and 1588 optional flag in Microchip phys
> 
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> On Thu, 14 Nov 2024 17:34:53 +0530 Divya Koppera wrote:
> >  config MICROCHIP_T1_PHY
> >       tristate "Microchip T1 PHYs"
> > +     select MICROCHIP_PHYPTP if NETWORK_PHY_TIMESTAMPING
> > +     depends on PTP_1588_CLOCK_OPTIONAL
> 
> I presume the dependency is because select doesn't obey dependencies, but
> you only select PHYPTP if NETWORK_PHY_TIMESTAMPING.
> Maybe it's possible to create a intermediate meta-symbol which is
> NETWORK_PHY_TIMESTAMPING && PTP_1588_CLOCK_OPTIONAL and use
> that in the select.. if ... clause?
> 

This way is not harmful as it is clock optional. 

Suggestion is good as it will be more efficient. Will apply this in next revision.

> > +     help
> > +       Supports the LAN8XXX PHYs.
> > +
> > +config MICROCHIP_PHYPTP
> > +        tristate "Microchip PHY PTP"
> >       help
> 
> nit: tabs vs spaces
> 

Oh.. This didn't caught in check patch. Will change in next revision.

> > -       Supports the LAN87XX PHYs.
> > +       Currently supports LAN887X T1 PHY
> 
> This Kconfig is likely unsafe.
> You have to make sure PHYPTP is not a module when T1_PHY is built in.

I tried changing options in make menuconfig, PHYPTP takes the option from T1_PHY.

If T1_PHY chosen m or y, PHYPTP takes m or y accordingly.

Also tried modifying .config with t1_phy as y and PHYPTP as m, it automates and changed PHYPTP to y.

Thanks,
Divya

> --
> pw-bot: cr