On 04/06/2019 17:28, Richard Cochran wrote:
> On Mon, Jun 03, 2019 at 03:12:42PM +0300, Ido Schimmel wrote:
>
>> +static int
>> +mlxsw_sp1_ptp_update_phc_settime(struct mlxsw_sp_ptp_clock *clock, u64 nsec)
>
> Six words ^^^
>
> What is wrong with "mlxsw_phc_settime" ?
I can drop the "update". But as Jiri mentioned, it is aligned with the rest of
mlxsw code.
>
>> +{
>> + struct mlxsw_core *mlxsw_core = clock->core;
>> + char mtutc_pl[MLXSW_REG_MTUTC_LEN];
>> + char mtpps_pl[MLXSW_REG_MTPPS_LEN];
>> + u64 next_sec_in_nsec, cycles;
>> + u32 next_sec;
>> + int err;
>> +
>> + next_sec = nsec / NSEC_PER_SEC + 1;
>> + next_sec_in_nsec = next_sec * NSEC_PER_SEC;
>> +
>> + spin_lock(&clock->lock);
>> + cycles = mlxsw_sp1_ptp_ns2cycles(&clock->tc, next_sec_in_nsec);
>> + spin_unlock(&clock->lock);
>> +
>> + mlxsw_reg_mtpps_vpin_pack(mtpps_pl, cycles);
>> + err = mlxsw_reg_write(mlxsw_core, MLXSW_REG(mtpps), mtpps_pl);
>> + if (err)
>> + return err;
>> +
>> + mlxsw_reg_mtutc_pack(mtutc_pl,
>> + MLXSW_REG_MTUTC_OPERATION_SET_TIME_AT_NEXT_SEC,
>> + 0, next_sec);
>> + return mlxsw_reg_write(mlxsw_core, MLXSW_REG(mtutc), mtutc_pl);
>> +}
>> +
>> +static int mlxsw_sp1_ptp_adjfine(struct ptp_clock_info *ptp, long
>> scaled_ppm)
>> +{
>> + struct mlxsw_sp_ptp_clock *clock =
>> + container_of(ptp, struct mlxsw_sp_ptp_clock, ptp_info);
>> + int neg_adj = 0;
>> + u32 diff;
>> + u64 adj;
>> + s32 ppb;
>> +
>> + ppb = ptp_clock_scaled_ppm_to_ppb(scaled_ppm);
>
> Now I see why you did this. Nice try.
I didn't try anything.
The reason is that the hardware units is in ppb and not in scaled_ppm(or ppm),
so I just converted to ppb in order to set the hardware.
But I got your point, I will change my calculation to use scaled_ppm (to get a
more finer resolution) and not ppb, and convert to ppb just before setting the
hardware. Is that make sense?
But I'm still need to expose scaled_ppm_to_ppb.
>
> The 'scaled_ppm' has a finer resolution than ppb. Please make use of
> the finer resolution in your calculation. It does make a difference.
Will change, thanks for that!
>
>> +
>> + if (ppb < 0) {
>> + neg_adj = 1;
>> + ppb = -ppb;
>> + }
>> +
>> + adj = clock->nominal_c_mult;
>> + adj *= ppb;
>> + diff = div_u64(adj, NSEC_PER_SEC);
>> +
>> + spin_lock(&clock->lock);
>> + timecounter_read(&clock->tc);
>> + clock->cycles.mult = neg_adj ? clock->nominal_c_mult - diff :
>> + clock->nominal_c_mult + diff;
>> + spin_unlock(&clock->lock);
>> +
>> + return mlxsw_sp1_ptp_update_phc_adjfreq(clock, neg_adj ? -ppb : ppb);
>> +}
>
> Thanks,
> Richard
>
