[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9462eda6-1b99-4b9d-80f7-727ddd040ad7@intel.com>
Date: Wed, 9 Apr 2025 11:11:58 -0700
From: Jacob Keller <jacob.e.keller@...el.com>
To: "Olech, Milena" <milena.olech@...el.com>,
"intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "Nguyen, Anthony L"
<anthony.l.nguyen@...el.com>, "Kitszel, Przemyslaw"
<przemyslaw.kitszel@...el.com>, "Lobakin, Aleksander"
<aleksander.lobakin@...el.com>, Willem de Bruijn <willemb@...gle.com>, "Mina
Almasry" <almasrymina@...gle.com>, "Salin, Samuel" <samuel.salin@...el.com>
Subject: Re: [Intel-wired-lan] [PATCH v10 iwl-next 03/11] virtchnl: add PTP
virtchnl definitions
On 4/9/2025 4:51 AM, Olech, Milena wrote:
> On 4/8/2025 11:12 PM, Jacob Keller wrote:
>> On 4/8/2025 3:30 AM, Milena Olech wrote:
>>> +/**
>>> + * struct virtchnl2_ptp_adj_dev_clk_fine: Associated with message
>>> + * VIRTCHNL2_OP_PTP_ADJ_DEV_CLK_FINE.
>>> + * @incval: Source timer increment value per clock cycle
>>> + *
>>> + * PF/VF sends this message to adjust the frequency of the main timer by the
>>> + * indicated scaled ppm.
>>> + */
This comment should be rephrased then. The text implies the value being
sent is the scaled PPM value.
>>
>> Do we want to encode scaled_ppm here in the virtchnl interface? I
>> suppose its not that big a deal but it is kind of an implementation
>> quirk of the Linux APIs. We could use parts per trillion or something
>> similar..
>>
>> I suppose there is little value in translating from scaled_ppm to some
>> other format, due to accumulated error, and scaled_ppm is higher
>> precision than ppb. Ok.
>
> I'm not sure I fully understand your concern, but you think that we
> could use another naming convention, or provide to control plane raw
> scaled ppm value?
>
> Please notice that in current shape, we negotiate also basic increment
> value in PTP capabilities, to adjust scaled ppm - as it is done in any
> other product - and then the diff is sent through virtchnl message.
>
No. What I am trying to get at is that i don't think it makes sense to
encode the use of scaled_ppm in the virtchnl message. You didn't do that
which is good. But the comment makes it seem like you did, because it
seems like the message itself adjusts the main timer by the scaled PPM
indicated within the message. In fact the driver calculates the new
invcalue and sends it.
Its not a big deal either way to me, I just misinterpreted the meaning
of the comment.
> Thanks,
> Milena
Powered by blists - more mailing lists