netdev - Re: [PATCH 0/3] Patch series for a PTP Grandmaster use case using stmmac/gmac3 ptp clock

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20200603151719.GA21184@orolia.com>
Date:   Wed, 3 Jun 2020 17:17:19 +0200
From:   Olivier Dautricourt <olivier.dautricourt@...lia.com>
To:     Richard Cochran <richardcochran@...il.com>
Cc:     Julien Beraud <julien.beraud@...lia.com>,
        Giuseppe Cavallaro <peppe.cavallaro@...com>,
        Alexandre Torgue <alexandre.torgue@...com>,
        Jose Abreu <joabreu@...opsys.com>,
        "David S . Miller" <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH 0/3] Patch series for a PTP Grandmaster use case using
 stmmac/gmac3 ptp clock

The 05/26/2020 21:05, Richard Cochran wrote:
> On Fri, May 15, 2020 at 03:26:47PM +0200, Julien Beraud wrote:
> > The frequency adjustments of the mac's clock are done by changing the value of
> > addend, so that the number of clock cycles that make the accumulator overflow
> > slightly change.
> 
> This is typical.
> 
> > The value of sub-second-increment is set to 2 / ptp_clk_freq, because using an
> > accumulator with the same number of bits as the addend register makes it
> > impossible to overflow at every addition.
> >
> > This mode allows to implement a PTP Slave, constantly adjusting the clock's freq
> > to match the master.
> 
> Right.  And I would stick with this.  With the ts2phc program
> (linuxptp master branch) you can use the EXTTS to discipline the clock
> to match the external time source.

The advantage for us here is that the coarse mode allows to set the
sub-second-increment to 1 / ptp_clk_freq and increase the precision of
the timestamps in that mode. Because we don't have to deal with the accumulator
overflow anymore.

> 
> > -> In coarse mode, the accumulator and the addend register are not used and the
> > value of sub-second-increment is added to the clock at every ptp_ref_clock
> > cycle.
> 
> That sounds like simply configuring a fixed rate.

Yes it is. But we want that rate not to be the same that in fine mode, by
configuring it at runtime.

> 
> > This mode allows to implement a Grand Master where we can feed the stmmac's ptp
> > ref clock input with a frequency that's controlled externally, making it
> > useless to do frequency adjustments with the MAC.
> >
> > We want our devices to be able to switch from Master to Slave at runtime.
> 
> If I understand correctly, what you are really asking for is the
> ability to switch clock sources.  This normally done through the
> device tree, and changing the device tree at run time is done with
> overlays (which I think is still a WIP).

In our setup the source clock switch is actually done in an fpga, so the
device tree node is still the same in both cases and we configure the input clock
through a gpio control from the ptp client.

> > So the question is what interface could we use to configure a timestamping
> > clock that has more than one functioning mode and which mode can be changed at
> > runtime, but not while timestamping is running ?
> 
> I think this must be decided at boot time using the DTS.  In any case,
> the PHC time stamping API is not the right place for this.  If you end
> up making a custom method (via debugfs for example), then your PHC
> should then return an error when user space attempts to adjust it.

I think the problem here relies on the fact that the configuration of the mac
is done in the stmmac_hwstamp_set function while what we are really
configuring is the time stamping + ptp_clock functionning.
In the end there is only one register where both are configured
(Control register). But this also makes sense because if there is no
active timestamping the ptp clock can't work. Also as Julien explained
it in his previous mail, the mode switch (writing 2 registers) needs
the timestamping to be reinitialized in any case.

So for something upstreamable i'm kind of stuck here.
This kind of mode configuration does not seems to fit in the current kernel API
because it affects both timestamping (sub-second-increment value) and the ptp clock
functionning (no fine adjustment possible while in coarse mode).

If we don't find a solution i'll at least resubmit my first patch
which is independent of the other two.

Thanks,
-- 
Olivier Dautricourt