RE: [RFC PATCH v9 00/10] Create common DPLL configuration API

Kubalewski, Arkadiusz
2023-07-10T10:10:45Z

>From: Jiri Pirko <jiri@resnulli.us>
>Sent: Wednesday, June 28, 2023 1:16 PM
>Wed, Jun 28, 2023 at 11:15:11AM CEST, arkadiusz.kubalewski@intel.com wrote:
>>>From: Jiri Pirko <jiri@resnulli.us>
>>>Sent: Tuesday, June 27, 2023 12:18 PM
>>>
>>>Fri, Jun 23, 2023 at 02:38:10PM CEST, arkadiusz.kubalewski@intel.com
>>>wrote:
>>>
>>>>v8 -> v9:
>>>
>>>Could you please address all the unresolved issues from v8 and send v10?
>>>I'm not reviewing this one.
>>>
>>>Thanks!
>>
>>Sure, will do, but first missing to-do/discuss list:
>>1) remove mode_set as not used by any driver

I have implemented in ice (also added back the DPLL_MODE_FREERUN).

>>2) remove "no-added-value" static functions descriptions in
>> dpll_core/dpll_netlink

Removed.

>>3) merge patches [ 03/10, 04/10, 05/10 ] into patches that are compiling
>> after each patch apply

Hope Vadim will decide on this, the thing is merging in two patches
doesn't make much sense as there won't be any linking until both patches
are there, so most sense it would be if 3 are merged into one, but
then we will be back to one big blob patch issue.

>>4) remove function return values descriptions/lists

Fixed.

>>5) Fix patch [05/10]:
>> - status Supported
>> - additional maintainers
>> - remove callback:
>> int (*source_pin_idx_get)(...) from `struct dpll_device_ops`
>>6) Fix patch [08/10]: rethink ice mutex locking scheme

Fixed.

>>7) Fix patch [09/10]: multiple comments on
>>https://lore.kernel.org/netdev/ZIQu+%2Fo4J0ZBspVg@nanopsycho/#t
>>8) add PPS DPLL phase offset to the netlink get-device API
>>

Added few things on this matter
- 1 dpll level attribute:
- phase-shift - measuring the phase difference between dpll input
and it's output
- 1 dpll-pin tuple level attribute:
- pin-phase-adjust - set/get phase adjust of a pin on a dpll
- 2 pin level attributes:
- pin-phase-adjust-min - provide user with min value that can be set
- pin-phase-adjust-max - provide user with max value that can be set
- a constant:
- DPLL_PHASE_SHIFT_DIVIDER similar to DPLL_TEMP_DIVIDER for producing
fraction value of measured DPLL_A_PHASE_SHIFT
- implemented in dpll netlink and in ice

>
>You are missing removal of pin->prop.package_label = dev_name(dev); in
>ice.
>

I didn't touch it, as we still need to discuss it, Jakub didn't respond
on v8 thread.
I don't see why we shall not name it the way. This is most meaningful
label for those pins for the user right now.

Thank you!
Arkadiusz

>
>>Thank you!
>>Arkadiusz