RE: [PATCH net-next v7 5/5] net: phy: microchip_t1 : Add initialization of ptp for lan887x

Divya.Koppera
2024-12-18T12:03:28Z

Hi Jakub,

> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Wednesday, December 18, 2024 8:44 AM
> To: Divya Koppera - I30481 <Divya.Koppera@microchip.com>
> Cc: andrew@lunn.ch; Arun Ramadoss - I17769
> <Arun.Ramadoss@microchip.com>; UNGLinuxDriver
> <UNGLinuxDriver@microchip.com>; hkallweit1@gmail.com;
> linux@armlinux.org.uk; davem@davemloft.net; edumazet@google.com;
> pabeni@redhat.com; netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> richardcochran@gmail.com; vadim.fedorenko@linux.dev
> Subject: Re: [PATCH net-next v7 5/5] net: phy: microchip_t1 : Add initialization
> of ptp for lan887x
>
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> On Fri, 13 Dec 2024 17:44:03 +0530 Divya Koppera wrote:
> > static int lan887x_phy_init(struct phy_device *phydev) {
> > + struct lan887x_priv *priv = phydev->priv;
> > int ret;
> >
> > + if (!priv->init_done && phy_interrupt_is_valid(phydev)) {
> > + priv->clock = mchp_rds_ptp_probe(phydev, MDIO_MMD_VEND1,
> > + MCHP_RDS_PTP_LTC_BASE_ADDR,
> > + MCHP_RDS_PTP_PORT_BASE_ADDR);
> > + if (IS_ERR(priv->clock))
> > + return PTR_ERR(priv->clock);
> > +
> > + priv->init_done = true;
> > + }
>
> If this only has to happen once, why not call mchp_rds_ptp_probe() from
> lan887x_probe() ? If there is some inherent reason the function needs to be
> protected from multiple calls maybe it's better to let
> mchp_rds_ptp_probe() handle that case ?

Valid phy interrupt irq number will be available during phy init procedure. Hence, we are calling ptp probe in phy init as
interrupts are mandatorily enabled for ptp to work.

Thanks,
Divya