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

From: Divya.Koppera
Date: Mon Nov 18 2024 - 05:47:00 EST


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