[Intel-wired-lan] [PATCH iwl-net v2] ice: dpll: fix phase offset value

[Michal Schmidt]

On Mon, Dec 18, 2023 at 4:02 PM Arkadiusz Kubalewski
<arkadiusz.kubalewski at intel.com> wrote:
>
> Stop dividing the phase_offset value received from firmware. This fault
> is present since the initial implementation.
> The phase_offset value received from firmware is in 0.01ps resolution.
> Dpll subsystem is using the value in 0.001ps, raw value is adjusted
> before providing it to the user.
>
> The user can observe the value of phase offset with response to
> `pin-get` netlink message of dpll subsystem for an active pin:
> $ ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/dpll.yaml \
>         --do pin-get --json '{"id":2}'
>
> Where example of correct response would be:
> {'board-label': 'C827_0-RCLKA',
>  'capabilities': 6,
>  'clock-id': 4658613174691613800,
>  'frequency': 1953125,
>  'id': 2,
>  'module-name': 'ice',
>  'parent-device': [{'direction': 'input',
>                     'parent-id': 6,
>                     'phase-offset': -216839550,
>                     'prio': 9,
>                     'state': 'connected'},
>                    {'direction': 'input',
>                     'parent-id': 7,
>                     'phase-offset': -42930,
>                     'prio': 8,
>                     'state': 'connected'}],
>  'phase-adjust': 0,
>  'phase-adjust-max': 16723,
>  'phase-adjust-min': -16723,
>  'type': 'mux'}
>
> Provided phase-offset value (-42930) shall be divided by the user with
> DPLL_PHASE_OFFSET_DIVIDER to get actual value of -42.930 ps.
>
> Before the fix, the response was not correct:
> {'board-label': 'C827_0-RCLKA',
>  'capabilities': 6,
>  'clock-id': 4658613174691613800,
>  'frequency': 1953125,
>  'id': 2,
>  'module-name': 'ice',
>  'parent-device': [{'direction': 'input',
>                     'parent-id': 6,
>                     'phase-offset': -216839,
>                     'prio': 9,
>                     'state': 'connected'},
>                    {'direction': 'input',
>                     'parent-id': 7,
>                     'phase-offset': -42,
>                     'prio': 8,
>                     'state': 'connected'}],
>  'phase-adjust': 0,
>  'phase-adjust-max': 16723,
>  'phase-adjust-min': -16723,
>  'type': 'mux'}
>
> Where phase-offset value (-42), after division
> (DPLL_PHASE_OFFSET_DIVIDER) would be: -0.042 ps.

The following is not an objection to the patch, just a related point:

Why does the documentation for "phase-offset-divider" in
Documentation/netlink/specs/dpll.yaml not mention the units at all? It
tells you one has to divide the raw value by the divider, but it does
not tell you that the result is then in picoseconds.
Actually, why is the divider defined at all? Wouldn't it have been
enough to document that the raw value is in femtoseconds?

Michal