Re: [PATCH net-next 8/8] net: mscc: PTP Hardware Clock (PHC) support

[Antoine Tenart <antoine.tenart@bootlin.com>]

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