Re: [PATCH net-next 7/9] mlxsw: spectrum_ptp: Add implementation for physical hardware clock operations

[Richard Cochran <richardcochran@gmail.com>]

On Mon, Jun 03, 2019 at 03:12:42PM +0300, Ido Schimmel wrote:
> +struct mlxsw_sp_ptp_clock *
> +mlxsw_sp1_ptp_clock_init(struct mlxsw_sp *mlxsw_sp, struct device *dev)
> +{
> +	u64 overflow_cycles, nsec, frac = 0;
> +	struct mlxsw_sp_ptp_clock *clock;
> +	int err;
> +
> +	clock = kzalloc(sizeof(*clock), GFP_KERNEL);
> +	if (!clock)
> +		return ERR_PTR(-ENOMEM);
> +
> +	spin_lock_init(&clock->lock);
> +	clock->cycles.read = mlxsw_sp1_ptp_read_frc;
> +	clock->cycles.shift = MLXSW_SP1_PTP_CLOCK_CYCLES_SHIFT;
> +	clock->cycles.mult = clocksource_khz2mult(MLXSW_SP1_PTP_CLOCK_FREQ_KHZ,
> +						  clock->cycles.shift);
> +	clock->nominal_c_mult = clock->cycles.mult;
> +	clock->cycles.mask = CLOCKSOURCE_MASK(MLXSW_SP1_PTP_CLOCK_MASK);
> +	clock->core = mlxsw_sp->core;
> +
> +	timecounter_init(&clock->tc, &clock->cycles,
> +			 ktime_to_ns(ktime_get_real()));
> +
> +	/* Calculate period in seconds to call the overflow watchdog - to make
> +	 * sure counter is checked at least twice every wrap around.
> +	 * The period is calculated as the minimum between max HW cycles count
> +	 * (The clock source mask) and max amount of cycles that can be
> +	 * multiplied by clock multiplier where the result doesn't exceed
> +	 * 64bits.
> +	 */
> +	overflow_cycles = div64_u64(~0ULL >> 1, clock->cycles.mult);
> +	overflow_cycles = min(overflow_cycles, div_u64(clock->cycles.mask, 3));
> +
> +	nsec = cyclecounter_cyc2ns(&clock->cycles, overflow_cycles, 0, &frac);
> +	clock->overflow_period = nsecs_to_jiffies(nsec);
> +
> +	INIT_DELAYED_WORK(&clock->overflow_work, mlxsw_sp1_ptp_clock_overflow);
> +	mlxsw_core_schedule_dw(&clock->overflow_work, 0);
> +
> +	clock->ptp_info = mlxsw_sp1_ptp_clock_info;
> +	clock->ptp = ptp_clock_register(&clock->ptp_info, dev);
> +	if (IS_ERR(clock->ptp)) {
> +		err = PTR_ERR(clock->ptp);
> +		dev_err(dev, "ptp_clock_register failed %d\n", err);
> +		goto err_ptp_clock_register;
> +	}
> +
> +	return clock;

You need to handle the case where ptp_clock_register() returns NULL...

/**
 * 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,
Richard