Hello Richard, Quoting Richard Cochran (2020-06-25 15:22:26) > On Tue, Jun 23, 2020 at 04:30:12PM +0200, Antoine Tenart wrote: > > @@ -978,9 +1483,32 @@ static int __vsc8584_init_ptp(struct phy_device > > *phydev) > > > > vsc85xx_ts_eth_cmp1_sig(phydev); > > > > + vsc8531->mii_ts.rxtstamp = vsc85xx_rxtstamp; > > + vsc8531->mii_ts.txtstamp = vsc85xx_txtstamp; > > + vsc8531->mii_ts.hwtstamp = vsc85xx_hwtstamp; > > + vsc8531->mii_ts.ts_info = vsc85xx_ts_info; > > + phydev->mii_ts = &vsc8531->mii_ts; > > + > > + memcpy(&vsc8531->ptp->caps, &vsc85xx_clk_caps, > > sizeof(vsc85xx_clk_caps)); > > + > > + vsc8531->ptp->ptp_clock = ptp_clock_register(&vsc8531->ptp->caps, > > + &phydev->mdio.dev); > > + if (IS_ERR(vsc8531->ptp->ptp_clock)) > > + return PTR_ERR(vsc8531->ptp->ptp_clock); > > The ptp_clock_register() method can also return NULL: > > * Returns a valid pointer on success or PTR_ERR on failure. If PHC > * support is missing at the configuration level, this function > * returns NULL, and drivers are expected to gracefully handle that > * case separately.
If ptp_clock_register returns NULL because PHC support is missing, the clock won't be exported. Beside that, I don't think we can do much in the error path, so this should be safe. Thanks, Antoine -- Antoine Ténart, Bootlin Embedded Linux and Kernel engineering https://bootlin.com