On Mon, Mar 13, 2023 at 11:49:53AM -0700, Richard Cochran wrote: > On Sun, Mar 12, 2023 at 11:02:39PM -0400, Tianfei Zhang wrote: ... > > + dt->ptp_clock = ptp_clock_register(&dt->ptp_clock_ops, dev); > > + if (IS_ERR(dt->ptp_clock)) > > + return dev_err_probe(dt->dev, PTR_ERR(dt->ptp_clock), > > + "Unable to register PTP clock\n"); > > Need to handle NULL as well... > > /** > * ptp_clock_register() - register a PTP hardware clock driver > * > * @info: Structure describing the new clock. > * @parent: Pointer to the parent device of the new clock. > * > * 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. > */ I'm wondering why. The semantics of the above is similar to gpiod_get_optional() and since NULL is a valid return in such cases, the PTP has to handle this transparently to the user. Otherwise it's badly designed API which has to be fixed. TL;DR: If I'm mistaken, I would like to know why. -- With Best Regards, Andy Shevchenko
