netdev - Re: [PATCH] net: mscc: ocelot: support PPS signal generation

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]

Message-ID: <CA+h21hp_3gAqehP6mPdF-bWN9kDoXLdMEAVhidCrmPAPTpR2eg@mail.gmail.com>
Date:   Thu, 26 Dec 2019 13:44:41 +0200
From:   Vladimir Oltean <olteanv@...il.com>
To:     "Y.b. Lu" <yangbo.lu@....com>
Cc:     netdev <netdev@...r.kernel.org>,
        "David S . Miller" <davem@...emloft.net>,
        Claudiu Manoil <claudiu.manoil@....com>,
        Vladimir Oltean <vladimir.oltean@....com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Microchip Linux Driver Support <UNGLinuxDriver@...rochip.com>,
        Richard Cochran <richardcochran@...il.com>
Subject: Re: [PATCH] net: mscc: ocelot: support PPS signal generation

On Thu, 26 Dec 2019 at 13:17, Y.b. Lu <yangbo.lu@....com> wrote:
> > Also, I think what you have implemented here is periodic output
> > (PTP_CLK_REQ_PEROUT) not PPS [input] (PTP_CLK_REQ_PPS). I have found
> > the PTP documentation to be rather confusing on what PTP_CLK_REQ_PPS
> > means, so I'm adding Richard in the hope that he may clarify (also
> > what's different between PTP_CLK_REQ_PPS and PTP_CLK_REQ_PPS).
>
> My understand is PTP_CLK_REQ_PEROUT is for periodical output, and PTP_CLK_REQ_PPS is for PPS event handling.
> This patch is to initialize PPS signal. You may check reference manual for how to generate PPS.
>
> "For the CLOCK action, the sync option makes the pin generate a single pulse, <WAFEFORM_LOW>
> nanoseconds after the time of day has increased the seconds. The pulse will get a width of
> <WAVEFORM_HIGH> nanoseconds. "
>
> If the sync option is not used, it is for periodical clock generation that you mentioned.
>

So basically this hardware emits a periodic signal with the frequency
f equal to:
- NSEC_PER_SEC / (WAFEFORM_LOW + WAFEFORM_HIGH) if PTP_PIN_SYNC is not set
- 1 Hz if PTP_PIN_SYNC is set

So the hardware hardcodes the frequency, basically, and makes
WAFEFORM_LOW be the phase adjustment. So all in all, it's still
PTP_CLK_REQ_PEROUT that needs to be treated for this. Maybe you have
to special-case the value of rq->perout.period.sec and
rq->perout.period.nsec.

What is the phase adjustment (pin start time) if the PTP_PIN_SYNC
option is not set?

Anyway, it's good that you figured it out, but it's not really ok to
hardcode it like this. On some boards there may be electrical issues
if the PTP pins just emit pulses by default.

> EXTTS is for capturing timestamps for external trigger pin which is input signal.

And isn't PTP_CLK_REQ_PPS the same, just that the input signal is
expected to have a particular waveform?
Some drivers, like ptp_qoriq, seem like they misinterpret the PPS
request as meaning "generate PPS output". I find this strange because
it's exactly Richard who added the code for it.

Thanks,
-Vladimir