linux-kernel - Re: [PATCH 4/6] net: ethernet: ti: cpts: add ptp pps support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 30 Nov 2016 14:43:57 -0600
From:   Grygorii Strashko <grygorii.strashko@...com>
To:     Richard Cochran <richardcochran@...il.com>,
        Murali Karicheri <m-karicheri2@...com>,
        Wingman Kwok <w-kwok2@...com>
CC:     "David S. Miller" <davem@...emloft.net>, <netdev@...r.kernel.org>,
        Mugunthan V N <mugunthanvnm@...com>,
        Sekhar Nori <nsekhar@...com>, <linux-kernel@...r.kernel.org>,
        <linux-omap@...r.kernel.org>, Rob Herring <robh+dt@...nel.org>,
        <devicetree@...r.kernel.org>
Subject: Re: [PATCH 4/6] net: ethernet: ti: cpts: add ptp pps support



On 11/30/2016 12:45 PM, Richard Cochran wrote:
> On Mon, Nov 28, 2016 at 05:04:26PM -0600, Grygorii Strashko wrote:
>> +static cycle_t cpts_cc_ns2cyc(struct cpts *cpts, u64 nsecs)
>> +{
>> +	cycle_t cyc = (nsecs << cpts->cc.shift) + nsecs;
>> +
>> +	do_div(cyc, cpts->cc.mult);
>> +
>> +	return cyc;
>> +}
> 
> So you set the comparison value once per second, based on cc.mult.
> But when the clock is being actively synchronized, user space calls to
> clock_adjtimex() will change cc.mult.  This can happen several times
> per second, depending on the PTP Sync rate.
> 

Right.

> In order to produce the PPS edge correctly, you would have to adjust
> the comparison value whenever cc.mult changes, 

yes. And that is done in cpts_ptp_adjfreq()
	if (cpts->ts_comp_enabled)
		cpts->ts_comp_one_sec_cycs = cpts_cc_ns2cyc(cpts, NSEC_PER_SEC);
	^^^ re-calculate reload value for 
 
	cpts_ts_comp_settime(cpts, ns);
	^^^ adjust the ts_comp

> but of course this is unworkable.
> 

Sry, but this is questionable - code for pps comes from TI internal
branches (SDK releases) where it survived for a pretty long time.
I'm, of course, agree that without HW support for freq adjustment
this PPS feature is not super precise and has some limitation,
but that is what we agree to live with. 

Murali, do you have any comments regarding usability of SW
freq freq adjustment approach? 

> So I'll have to say NAK for this patch.
> 

:) 


-- 
regards,
-grygorii