Re: [PATCH iproute2-next 2/2] dpll: add frequency monitoring support

[Ivan Vecera <ivecera@redhat.com>]

Hi David,

On 5/2/26 8:29 PM, David Ahern wrote:
> On 4/28/26 9:21 AM, Ivan Vecera wrote:
>> diff --git a/man/man8/dpll.8 b/man/man8/dpll.8
>> index 89f17af74923..59ec4208f251 100644
>> --- a/man/man8/dpll.8
>> +++ b/man/man8/dpll.8
>> @@ -111,7 +111,7 @@ Temperature (if supported)
>>   Type (PPS or EEC)
>>   .RE
>>   
>> -.SS dpll device set id ID [ mode { automatic | manual } ] [ phase-offset-monitor { enable | disable } ] [ phase-offset-avg-factor FACTOR ]
>> +.SS dpll device set id ID [ mode { automatic | manual } ] [ phase-offset-monitor { enable | disable } ] [ phase-offset-avg-factor FACTOR ] [ frequency-monitor { enable | disable } ]
> 
> very long line; please fix up.

Will fix in v2.

> 
> Keep Petr reviewed by on patch 1.
> 
> Claude has some comments on the man page in general:
> 
> ● I found three issues. Let me show them clearly:
> 
>    Issue 1 — Typo: "locked-ho-ack" should be "locked-ho-acq" (line 107)
> 
>    The man page description of device show output says locked-ho-ack but
> the code (dpll_lock_status_name) returns locked-ho-acq
>    (ACQuired, not ACKnowledge).
> 
>    Issue 2 — Wrong mode values in output description (lines 104–105)
> 
>    The "Output includes" section says:
> 
>    ▎ Operating mode (manual, automatic, holdover, freerun)
> 
>    But dpll_mode_map and DPLL_MODE_* only define manual and automatic.
> Holdover and freerun are not DPLL modes — they are lock statuses.
>     This should read (manual, automatic).
> 
>    Issue 3 — Undocumented but accepted alternate values for monitors
> (lines 134, 144)
> 
>    The man page documents { enable | disable | true | false | 0 | 1 } for
> phase-offset-monitor and frequency-monitor. The tool's own
>    help text shows only { enable | disable } and the error message says
> "use enable/disable". While str_to_bool technically accepts
>    true/false/0/1, the canonical interface is enable | disable. The extra
> forms in the man page are inconsistent with the help text.

Will send a separate fix for the dpll man page (non-next)...

Thanks,
Ivan