netdev - Re: [PATCH net-next 1/2] dpll: add phase-adjust-gran pin attribute

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] [day] [month] [year] [list]

Message-ID: <10e525df-cd38-464a-8df5-ec59100ba40e@redhat.com>
Date: Wed, 29 Oct 2025 16:00:26 +0100
From: Ivan Vecera <ivecera@...hat.com>
To: Jiri Pirko <jiri@...nulli.us>
Cc: Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
 Michal Schmidt <mschmidt@...hat.com>, Petr Oros <poros@...hat.com>,
 Prathosh Satish <Prathosh.Satish@...rochip.com>,
 Vadim Fedorenko <vadim.fedorenko@...ux.dev>,
 Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>,
 Jonathan Corbet <corbet@....net>, "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
 Simon Horman <horms@...nel.org>, Donald Hunter <donald.hunter@...il.com>,
 linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next 1/2] dpll: add phase-adjust-gran pin attribute



On 10/29/25 3:20 PM, Jiri Pirko wrote:
> Wed, Oct 29, 2025 at 08:44:52AM +0100, ivecera@...hat.com wrote:
>> Hi Kuba,
>>
>> On 10/29/25 2:39 AM, Jakub Kicinski wrote:
>>> On Fri, 24 Oct 2025 16:49:26 +0200 Ivan Vecera wrote:
>>>> +      -
>>>> +        name: phase-adjust-gran
>>>> +        type: s32
>>>> +        doc: |
>>>> +          Granularity of phase adjustment, in picoseconds. The value of
>>>> +          phase adjustment must be a multiple of this granularity.
>>>
>>> Do we need this to be signed?
>>>
>> To have it unsigned brings a need to use explicit type casting in the core
>> and driver's code. The phase adjustment can be both positive and
>> negative it has to be signed. The granularity specifies that adjustment
>> has to be multiple of granularity value so the core checks for zero
>> remainder (this patch) and the driver converts the given adjustment
>> value using division by the granularity.
>>
>> If we would have phase-adjust-gran and corresponding structure fields
>> defined as u32 then we have to explicitly cast the granularity to s32
>> because for:
> 
> I prefer cast. The uapi should be clear. There is not point of having
> negative granularity.
> 
> 
I will use u32 for phase-adjust-gran and dpll_pin_properties.phase_gran.

OK?
>> <snip>
>> s32 phase_adjust, remainder;
>> u32 phase_gran = 1000;
>>
>> phase_adjust = 5000;
>> remainder = phase_adjust % phase_gran;
>> /* remainder = 0 -> OK for positive adjust */
>>
>> phase_adjust = -5000;
>> remainder = phase_adjust % phase_gran;
>> /* remainder = 296
>> * Wrong for negative adjustment because phase_adjust is casted to u32
>> * prior division -> 2^32 - 5000 = 4294962296.
>> * 4294962296 % 1000 = 296
>> */
>>
>> remainder = phase_adjust % (s32)phase_gran;
>> /* remainder = 0
>>   * Now OK because phase_adjust remains to be s32
>>   */
>> </snip>
>>
>> Similarly for division in the driver code if the granularity would be
>> u32.
>>
>> So I have proposed phase adjustment granularity to be s32 to avoid these
>> explicit type castings and potential bugs in drivers.
> 
> Cast in dpll core, no?
Depends... if the driver will use s32 (sse patch 2) then no castings are 
necessary.

Thanks,
Ivan