Hello Richard,
On Fri, Jul 05, 2019 at 09:47:36AM -0700, Richard Cochran wrote:
> On Mon, Jul 01, 2019 at 12:03:27PM +0200, Antoine Tenart wrote:
>
> > +void ocelot_get_hwtimestamp(struct ocelot *ocelot, struct timespec64 *ts)
> > +{
> > + /* Read current PTP time to get seconds */
> > + u32 val = ocelot_read_rix(ocelot, PTP_PIN_CFG, TOD_ACC_PIN);
>
> This register is protected by ocelot->ptp_clock_lock from other code
> paths, but not in this one!
Oops. I'll fix it.
> > +static int ocelot_init_timestamp(struct ocelot *ocelot)
> > +{
> > + ocelot->ptp_info = ocelot_ptp_clock_info;
> > +
> > + ocelot->ptp_clock = ptp_clock_register(&ocelot->ptp_info, ocelot->dev);
> > + if (IS_ERR(ocelot->ptp_clock))
> > + return PTR_ERR(ocelot->ptp_clock);
>
> You need to handle the NULL case:
Will do.
> 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.
Thanks,
Antoine
--
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
