| Message ID | 20230623123820.42850-1-arkadiusz.kubalewski@intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Create common DPLL configuration API | expand |
Fri, Jun 23, 2023 at 02:38:10PM CEST, arkadiusz.kubalewski@intel.com wrote: >Implement common API for clock/DPLL configuration and status reporting. >The API utilises netlink interface as transport for commands and event >notifications. This API aim to extend current pin configuration and >make it flexible and easy to cover special configurations. > >Netlink interface is based on ynl spec, it allows use of in-kernel >tools/net/ynl/cli.py application to control the interface with properly >formated command and json attribute strings. Here are few command >examples of how it works with `ice` driver on supported NIC: I don't understand. The discussion in the RFCv8 thread is still going on. The things I mentioned there are ignored. Like for example: 1) mode_set op removal 2) odd ice dpll locking scheme (either fix or describe why it is ok - that's the unfinished discussion) 3) header file bits squash I suggested. Vadim wrote that it sounds reasonable, yet nothing changed I thought we are past this. Why I have to point the same issues over and over?
On Fri, 23 Jun 2023 17:19:11 +0200 Jiri Pirko wrote: > I don't understand. The discussion in the RFCv8 thread is still going > on. The things I mentioned there are ignored. Like for example: > 1) mode_set op removal > 2) odd ice dpll locking scheme (either fix or describe why it is ok - > that's the unfinished discussion) > 3) header file bits squash I suggested. Vadim wrote that it sounds > reasonable, yet nothing changed > > I thought we are past this. Why I have to point the same issues over and > over? FWIW I'm lost in the previous thread, so for me there's value in refreshing the series. But you're right, at the very least there should be a summary of outstanding issues / open items / ongoing discussions in the cover letter.
Fri, Jun 23, 2023 at 05:53:36PM CEST, kuba@kernel.org wrote: >On Fri, 23 Jun 2023 17:19:11 +0200 Jiri Pirko wrote: >> I don't understand. The discussion in the RFCv8 thread is still going >> on. The things I mentioned there are ignored. Like for example: >> 1) mode_set op removal >> 2) odd ice dpll locking scheme (either fix or describe why it is ok - >> that's the unfinished discussion) >> 3) header file bits squash I suggested. Vadim wrote that it sounds >> reasonable, yet nothing changed >> >> I thought we are past this. Why I have to point the same issues over and >> over? > >FWIW I'm lost in the previous thread, so for me there's value in >refreshing the series. > >But you're right, at the very least there should be a summary of >outstanding issues / open items / ongoing discussions in the cover >letter. Well I would like to conclude discussion in one thread before sending the next one. What should I do? Should I start the same discussion pointing out the same issues in this thread again? This can't work. Even concluded items are ignored, like 3) IDK, this is very frustrating for me. I have to double check everything just in case it was not ignored. I don't understand this, there is no justification.
On Sat, 24 Jun 2023 11:23:36 +0200 Jiri Pirko wrote: > Well I would like to conclude discussion in one thread before sending > the next one. What should I do? Should I start the same discussion > pointing out the same issues in this thread again? This can't work. > > Even concluded items are ignored, like 3) > > IDK, this is very frustrating for me. I have to double check everything > just in case it was not ignored. I don't understand this, there is no > justification. Yes, the open items need to be clearly stated on a new posting.
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!
>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 2) remove "no-added-value" static functions descriptions in dpll_core/dpll_netlink 3) merge patches [ 03/10, 04/10, 05/10 ] into patches that are compiling after each patch apply 4) remove function return values descriptions/lists 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 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 Thank you! Arkadiusz
>From: Kubalewski, Arkadiusz >Sent: Wednesday, June 28, 2023 11:15 AM > >>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 >2) remove "no-added-value" static functions descriptions in > dpll_core/dpll_netlink >3) merge patches [ 03/10, 04/10, 05/10 ] into patches that are compiling > after each patch apply >4) remove function return values descriptions/lists >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 >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 > >Thank you! >Arkadiusz If someone has any objections please state them now, I will work on all above except 5) and 7). Vadim, could you take care of those 2 points? Thank you! Arkadiusz
On 28/06/2023 10:27, Kubalewski, Arkadiusz wrote: >> From: Kubalewski, Arkadiusz >> Sent: Wednesday, June 28, 2023 11:15 AM >> >>> 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 >> 2) remove "no-added-value" static functions descriptions in >> dpll_core/dpll_netlink >> 3) merge patches [ 03/10, 04/10, 05/10 ] into patches that are compiling >> after each patch apply >> 4) remove function return values descriptions/lists >> 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 >> 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 >> >> Thank you! >> Arkadiusz > > If someone has any objections please state them now, I will work on > all above except 5) and 7). > Vadim, could you take care of those 2 points? > Yeah, sure, I'll update 5 and 7. I'm not sure about 8) - do we really need this info, I believe every supported DPLL device exports PTP device as well. But I'm Ok to add this feature too. > Thank you! > Arkadiusz
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 >2) remove "no-added-value" static functions descriptions in > dpll_core/dpll_netlink >3) merge patches [ 03/10, 04/10, 05/10 ] into patches that are compiling > after each patch apply >4) remove function return values descriptions/lists >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 >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 > You are missing removal of pin->prop.package_label = dev_name(dev); in ice. >Thank you! >Arkadiusz
Wed, Jun 28, 2023 at 01:11:19PM CEST, vadim.fedorenko@linux.dev wrote: >On 28/06/2023 10:27, Kubalewski, Arkadiusz wrote: >> > From: Kubalewski, Arkadiusz >> > Sent: Wednesday, June 28, 2023 11:15 AM >> > >> > > 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 >> > 2) remove "no-added-value" static functions descriptions in >> > dpll_core/dpll_netlink >> > 3) merge patches [ 03/10, 04/10, 05/10 ] into patches that are compiling >> > after each patch apply >> > 4) remove function return values descriptions/lists >> > 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 >> > 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 >> > >> > Thank you! >> > Arkadiusz >> >> If someone has any objections please state them now, I will work on >> all above except 5) and 7). >> Vadim, could you take care of those 2 points? >> >Yeah, sure, I'll update 5 and 7. >I'm not sure about 8) - do we really need this info, I believe every >supported DPLL device exports PTP device as well. But I'm Ok to add this >feature too. Could you add the notification work while you are at it? I don't want that to be forgotten. Thanks! > >> Thank you! >> Arkadiusz >
On 28/06/2023 14:09, Jiri Pirko wrote: > Wed, Jun 28, 2023 at 01:11:19PM CEST, vadim.fedorenko@linux.dev wrote: >> On 28/06/2023 10:27, Kubalewski, Arkadiusz wrote: >>>> From: Kubalewski, Arkadiusz >>>> Sent: Wednesday, June 28, 2023 11:15 AM >>>> >>>>> 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 >>>> 2) remove "no-added-value" static functions descriptions in >>>> dpll_core/dpll_netlink >>>> 3) merge patches [ 03/10, 04/10, 05/10 ] into patches that are compiling >>>> after each patch apply >>>> 4) remove function return values descriptions/lists >>>> 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 >>>> 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 >>>> >>>> Thank you! >>>> Arkadiusz >>> >>> If someone has any objections please state them now, I will work on >>> all above except 5) and 7). >>> Vadim, could you take care of those 2 points? >>> >> Yeah, sure, I'll update 5 and 7. >> I'm not sure about 8) - do we really need this info, I believe every >> supported DPLL device exports PTP device as well. But I'm Ok to add this >> feature too. > > Could you add the notification work while you are at it? I don't want > that to be forgotten. Thanks! Sure, Jiri, I'm working on it for ptp_ocp. >> >>> Thank you! >>> Arkadiusz >>
Wed, Jun 28, 2023 at 03:22:00PM CEST, vadim.fedorenko@linux.dev wrote: >On 28/06/2023 14:09, Jiri Pirko wrote: >> Wed, Jun 28, 2023 at 01:11:19PM CEST, vadim.fedorenko@linux.dev wrote: >> > On 28/06/2023 10:27, Kubalewski, Arkadiusz wrote: >> > > > From: Kubalewski, Arkadiusz >> > > > Sent: Wednesday, June 28, 2023 11:15 AM >> > > > >> > > > > 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 >> > > > 2) remove "no-added-value" static functions descriptions in >> > > > dpll_core/dpll_netlink >> > > > 3) merge patches [ 03/10, 04/10, 05/10 ] into patches that are compiling >> > > > after each patch apply >> > > > 4) remove function return values descriptions/lists >> > > > 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 >> > > > 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 >> > > > >> > > > Thank you! >> > > > Arkadiusz >> > > >> > > If someone has any objections please state them now, I will work on >> > > all above except 5) and 7). >> > > Vadim, could you take care of those 2 points? >> > > >> > Yeah, sure, I'll update 5 and 7. >> > I'm not sure about 8) - do we really need this info, I believe every >> > supported DPLL device exports PTP device as well. But I'm Ok to add this >> > feature too. >> >> Could you add the notification work while you are at it? I don't want >> that to be forgotten. Thanks! > >Sure, Jiri, I'm working on it for ptp_ocp. Yep, cool! > >> > >> > > Thank you! >> > > Arkadiusz >> > >
>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
Mon, Jul 10, 2023 at 12:07:30PM CEST, arkadiusz.kubalewski@intel.com wrote: >>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). Uh :/ Why exactly is it needed in this initial submission? > >>>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 Again, why do we need this in this initial submission? Why it can't be a follow-up patchset to extend this? This way we never converge :/ Please focus on what we have now and bring it in. Let the extensions to be addressed later on, please. >- 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. This is not meaningful, at all. dev_name() changes upon which pci slot you plug the card into. package_label should be an actual label on a silicon package. Why you think this two are related in aby way, makes me really wonder. Could you elaborate the meaningfulness of this? > >Thank you! >Arkadiusz > >> >>>Thank you! >>>Arkadiusz
>From: Jiri Pirko <jiri@resnulli.us> >Sent: Monday, July 10, 2023 2:10 PM > >Mon, Jul 10, 2023 at 12:07:30PM CEST, arkadiusz.kubalewski@intel.com wrote: >>>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). > >Uh :/ Why exactly is it needed in this initial submission? > Without mode-set there is no need for device-set at all, right? So it is better to implement at least one set command, so we don't need remove device-set command entirely. > >> >>>>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 > >Again, why do we need this in this initial submission? Why it can't be a >follow-up patchset to extend this? This way we never converge :/ >Please focus on what we have now and bring it in. Let the extensions to >be addressed later on, please. > Well AFAIK, RHEL is doing some monitoring software, so the end-users need this. > > >>- 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. > >This is not meaningful, at all. dev_name() changes upon which pci slot >you plug the card into. package_label should be an actual label on a >silicon package. Why you think this two are related in aby way, makes me >really wonder. Could you elaborate the meaningfulness of this? > Without this, from end-user perspective, it would be very confusing. As in ice without any label there would 4 pins which differs only with id. What names would you suggest? Thank you! Arkadiusz > >> >>Thank you! >>Arkadiusz >> >>> >>>>Thank you! >>>>Arkadiusz
Tue, Jul 11, 2023 at 12:34:11PM CEST, arkadiusz.kubalewski@intel.com wrote: >>From: Jiri Pirko <jiri@resnulli.us> >>Sent: Monday, July 10, 2023 2:10 PM >> >>Mon, Jul 10, 2023 at 12:07:30PM CEST, arkadiusz.kubalewski@intel.com wrote: >>>>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). >> >>Uh :/ Why exactly is it needed in this initial submission? >> > >Without mode-set there is no need for device-set at all, right? >So it is better to implement at least one set command, so we don't >need remove device-set command entirely. The enum cmd valu could stay as a placeholder, the rest can go. > >> >>> >>>>>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 >> >>Again, why do we need this in this initial submission? Why it can't be a >>follow-up patchset to extend this? This way we never converge :/ >>Please focus on what we have now and bring it in. Let the extensions to >>be addressed later on, please. >> > >Well AFAIK, RHEL is doing some monitoring software, so the end-users need this. They need it for the initial submission? Why? Why can't they wait 1 week for follow-up patchset? > >> >> >>>- 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. >> >>This is not meaningful, at all. dev_name() changes upon which pci slot >>you plug the card into. package_label should be an actual label on a >>silicon package. Why you think this two are related in aby way, makes me >>really wonder. Could you elaborate the meaningfulness of this? >> > >Without this, from end-user perspective, it would be very confusing. >As in ice without any label there would 4 pins which differs only with id. There you go, it does not have any label, yet you are trying hard to make up some. Does not make sense. >What names would you suggest? That is the point I made previously. For synce usecase, the label does not make sense. There should be no label. You reference the pin by ID from netdev, that is enough. I think better to add the check to pin-register so future synce pin users don't have similar weird ideas. Could you please add this check? Thanks! > >Thank you! >Arkadiusz > >> >>> >>>Thank you! >>>Arkadiusz >>> >>>> >>>>>Thank you! >>>>>Arkadiusz
>From: Jiri Pirko <jiri@resnulli.us> >Sent: Tuesday, July 11, 2023 1:53 PM > >Tue, Jul 11, 2023 at 12:34:11PM CEST, arkadiusz.kubalewski@intel.com wrote: >>>From: Jiri Pirko <jiri@resnulli.us> >>>Sent: Monday, July 10, 2023 2:10 PM >>> >>>Mon, Jul 10, 2023 at 12:07:30PM CEST, arkadiusz.kubalewski@intel.com wrote: >>>>>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). >>> >>>Uh :/ Why exactly is it needed in this initial submission? >>> >> >>Without mode-set there is no need for device-set at all, right? >>So it is better to implement at least one set command, so we don't >>need remove device-set command entirely. > >The enum cmd valu could stay as a placeholder, the rest can go. > It doesn't make much sense to have a command which is not implemented, same as you wanted to remove enum values which are not used. > >> >>> >>>> >>>>>>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 >>> >>>Again, why do we need this in this initial submission? Why it can't be a >>>follow-up patchset to extend this? This way we never converge :/ >>>Please focus on what we have now and bring it in. Let the extensions to >>>be addressed later on, please. >>> >> >>Well AFAIK, RHEL is doing some monitoring software, so the end-users need >>this. > >They need it for the initial submission? Why? Why can't they wait 1 week >for follow-up patchset? > Probably best if they could respond here, though I know they are waiting for the dpll interface for a long time already. > >> >>> >>> >>>>- 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. >>> >>>This is not meaningful, at all. dev_name() changes upon which pci slot >>>you plug the card into. package_label should be an actual label on a >>>silicon package. Why you think this two are related in aby way, makes me >>>really wonder. Could you elaborate the meaningfulness of this? >>> >> >>Without this, from end-user perspective, it would be very confusing. >>As in ice without any label there would 4 pins which differs only with id. > >There you go, it does not have any label, yet you are trying hard to >make up some. Does not make sense. > Don't get this, they have the label, but you ask to remove it.. > >>What names would you suggest? > >That is the point I made previously. For synce usecase, the label does >not make sense. There should be no label. You reference the pin by ID >from netdev, that is enough. > Yea I understand, you are trying to hide this information from the user, while I am trying not to hide anything, and let the user know all the information can be somehow useful, this is the difference. The one might want to ask how label indicating internal pin label is useful, it is transparency, the label is there for identification nothing else. You mean SyncE use case where a software daemon controls the dpll for SyncE implementation, and this is already implemented and works as you described. >I think better to add the check to pin-register so future synce pin >users don't have similar weird ideas. Could you please add this check? > Don't think it is way to go, and I don't think there is anything good with preventing device drivers from labeling their pins the way they want. I don't understand why all the pins shall be targeted differently then SyncE ones, I mean the SyncE pins are special case (of dpll subsystem), and for that case, they are also targetable by netdevice, but what I don't understand is why they shall be only targetable with netdevice, they are still part of dpll subsystem not a SyncE subsystem, and with this understanding shall be targetable like all the other pins. While without the label this will not be possible. Thank you! Arkadiusz >Thanks! > > [...]
On Tue, 11 Jul 2023 17:17:51 +0000 Kubalewski, Arkadiusz wrote: > >I think better to add the check to pin-register so future synce pin > >users don't have similar weird ideas. Could you please add this check? > > Don't think it is way to go, and I don't think there is anything good > with preventing device drivers from labeling their pins the way they want. We had a long argument about how label should have a clearly defined meaning. We're not going to rehash it on every revision. What did I miss :|
>From: Jakub Kicinski <kuba@kernel.org> >Sent: Tuesday, July 11, 2023 10:15 PM > >On Tue, 11 Jul 2023 17:17:51 +0000 Kubalewski, Arkadiusz wrote: >> >I think better to add the check to pin-register so future synce pin >> >users don't have similar weird ideas. Could you please add this check? >> >> Don't think it is way to go, and I don't think there is anything good >> with preventing device drivers from labeling their pins the way they >>want. > >We had a long argument about how label should have a clearly defined >meaning. We're not going to rehash it on every revision. What did I miss :| Well, as I understand we are discussing if dpll subsystem shall prevent labeling the SyncE type pins. I have labeled them in ice explicitly with the name of a pci device they belong to. You haven't miss much, mostly the problem is described in this thread. Thank you! Arkadiusz
On Wed, 12 Jul 2023 09:19:53 +0000 Kubalewski, Arkadiusz wrote: > >> Don't think it is way to go, and I don't think there is anything good > >> with preventing device drivers from labeling their pins the way they > >> want. > > > >We had a long argument about how label should have a clearly defined > >meaning. We're not going to rehash it on every revision. What did I miss :| > > Well, as I understand we are discussing if dpll subsystem shall prevent > labeling the SyncE type pins. I have labeled them in ice explicitly with > the name of a pci device they belong to. > > You haven't miss much, mostly the problem is described in this thread. Please read this thread: https://lore.kernel.org/all/20230503191643.12a6e559@kernel.org/
Implement common API for clock/DPLL configuration and status reporting. The API utilises netlink interface as transport for commands and event notifications. This API aim to extend current pin configuration and make it flexible and easy to cover special configurations. Netlink interface is based on ynl spec, it allows use of in-kernel tools/net/ynl/cli.py application to control the interface with properly formated command and json attribute strings. Here are few command examples of how it works with `ice` driver on supported NIC: - dump dpll devices $# ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/dpll.yaml \ --dump device-get [{'clock-id': 282574471561216, 'id': 0, 'lock-status': 'unlocked', 'mode': 'automatic', 'module-name': 'ice', 'type': 'eec'}, {'clock-id': 282574471561216, 'id': 1, 'lock-status': 'unlocked', 'mode': 'automatic', 'module-name': 'ice', 'type': 'pps'}] - get single pin info: $# ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/dpll.yaml \ --do pin-get --json '{"pin-id":2}' {'clock-id': 282574471561216, 'module-name': 'ice', 'pin-board-label': 'C827_0-RCLKA', 'pin-dpll-caps': 6, 'pin-frequency': 1953125, 'pin-id': 2, 'pin-parenti-device': [{'id': 0, 'pin-direction': 'input', 'pin-prio': 11, 'pin-state': 'selectable'}, {'id': 1, 'pin-direction': 'input', 'pin-prio': 9, 'pin-state': 'selectable'}], 'pin-type': 'mux'} - set pin's state on dpll: $# ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/dpll.yaml \ --do pin-set --json '{"pin-id":2, "pin-parent-device":{"id":1, "pin-state":2}}' - set pin's prio on dpll: $# ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/dpll.yaml \ --do pin-set --json '{"pin-id":2, "pin-parent-device":{"id":1, "pin-prio":4}}' - set pin's state on parent pin: $# ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/dpll.yaml \ --do pin-set --json '{"pin-id":13, \ "pin-parent-pin":{"pin-id":2, "pin-state":1}}' v8 -> v9: [00/10] Create common DPLL configuration API - update examples to reflect new pin-parent nest split [01/10] dpll: documentation on DPLL subsystem interface - fix docs build warnings - separate netlink command/attribute list - replace enum description with uapi header - add brief explanation what is a DPLL - fix EOPNOTSUPP typo - fix typo .state_get -> .state_on_dpll_get [02/10] dpll: spec: Add Netlink spec in YAML - regenerate policy max values - add missing enum descriptions - split pin-parent nest: - pin-parent-device - for configuration of pin-device tuple - pin-parent-pin - for configuration od pin-pin tuple - fix typos: - s/working-modes/working modes/ - s/differentiate/differentiates/ - s/valid input, auto selected by dpll/input pin auto selected by dpll/ - remove FREERUN and HOLDOVER modes [03/10] dpll: core: Add DPLL framework base functions - fix description in spdx header. - remove refcount check if refcount was already set - do not validate dpll ptr in dpll_device_put(..) - fix return -ENOMEM on failed memory alloc - do not validate pin ptr in dpll_pin_put(..) - return -EINVAL in case of module/clock_id mismatch - do not {} around one-line xa_for_each() macro - move dpll_<x>_registration structs to dpll_core.c - rephrase doc comment on device and pin id struct members - remove ref in case of memory allocation fail - check for required ops on pin/device registration - mark pin with DPLL_REGISTERED once pin is registered with dpll [04/10] dpll: netlink: Add DPLL framework base functions - fix pin-id-get/device-id-get behavior - reshuffle order of functions - avoid forward declarations - functions for adding pin/device handle next to each other - pass ops callback return values to the user - remove dpll_cmd_pin_fill_details(..) function, merge the code into __dpll_cmd_pin_dump_one(..) - rename __dpll_cmd_pin_dump_one() to dpll_cmd_pin_get_one() - use WARN_ON macro when dpll ref is missing - remove redundant pin's dpll list not empty check - remove double spaces inside if statement - add extack message when set command is not possible - do not return error when callback is not required - WARN_ON missing ops moved to dpll_core.c - use DPLL_REGISTERED if pin was registered with dpll - fix pin-id-get return and add extack errors - fix device-id-get return and add extack errors - drop pointless init of variables - add macro for iterating over marked pins/devices - move dpll_set_from_nlattr() for consistent order - use GENL_REQ_ATTR_CHECK() for checking attibute presence - fill extack if pin/device was not found - drop pointless init of variables - WARN_ON if dpll not registered on send event - rename goto labels to indicate error path - fix docs - drop pointless init of variables - verify pin in notify with a mark - prevent ops->mode_set call if missing callback - move static dpll_msg_add_pin_handle() from pin<->netdev patch - split pin-parent nest: - pin-parent-device - for configuration of pin-device tuple - pin-parent-pin - for configuration od pin-pin tuple [06/10] netdev: expose DPLL pin handle for netdevice - net_device->dpll_pin is only valid if IS_ENABLED(CONFIG_DPLL) fix the code in net/core/rtnetlink.c to respect that. - move dpll_msg_add_pin_handle to "dpll: netlink" patch + export the function with this patch [07/10] ice: add admin commands to access cgu configuration - rename MAX_NETLIST_SIZE -> ICE_MAX_NETLIST_SIZE - simplify function: s64 convert_s48_to_s64(s64 signed_48) - do not assign 0 to field that is already 0 [08/10] ice: implement dpll interface to control cgu - drop pointless 0 assignement - ice_dpll_init(..) returns void instead of int - fix context description of the functions - fix ice_dpll_init(..) traces - fix use package_label instead pf board_label for rclk pin - be consistent on cgu presence naming - remove indent in ice_dpll_deinit(..) - remove unused struct field lock_err_num - fix kworker resched behavior - remove debug log from ice_dpll_deinit_worker(..) - reorder ice internal functions - release resources directly on error path - remove redundant NULL checks when releasing resources - do not assign NULL to pointers after releasing resources - simplify variable assignement - fix 'int ret;' declarations across the ice_dpll.c - remove leftover ice_dpll_find(..) - get pf pointer from dpll_priv without type cast - improve error reporting - fix documentation - fix ice_dpll_update_state(..) flow - fix return in case out of range prio set v7 -> v8: [0/10] Create common DPLL configuration API - reorder the patches in patch series - split patch "[RFC PATCH v7 2/8] dpll: Add DPLL framework base functions" into 3 smaller patches for easier review: - [03/10] dpll: core: Add DPLL framework base functions - [04/10] dpll: netlink: Add DPLL framework base functions - [05/10] dpll: api header: Add DPLL framework base - add cli.py usage examples in commit message [01/10] dpll: documentation on DPLL subsystem interface - fix DPLL_MODE_MANUAL documentation - remove DPLL_MODE_NCO - remove DPLL_LOCK_STATUS_CALIBRATING - add grepability Use full names of commands, attributes and values of dpll subsystem in the documentation - align documentation with changes introduced in v8 - fix typos - fix phrases to better show the intentions - move dpll.rst to Documentation/driver-api/ [02/10] dpll: spec: Add Netlink spec in YAML - remove unspec attribute values - add 10 KHZ and 77,5 KHZ frequency defines - fix documentation - remove assigned values from subset attributes - reorder dpll attributes - fix `device` nested attribute usage, device get is not used on pin-get - temperature with 3 digit float precision - remove enum from subset definitions - move pin-direction to pin-dpll tuple/subset - remove DPLL_MODE_NCO - remove DPLL_LOCK_STATUS_CALIBRATING - fix naming scheme od notification interface functions - separate notifications for pins - rename attribute enum name: dplla -> dpll_a - rename pin-idx to pin-id - remove attributes: pin-parent-idx, device - replace bus-name and dev-name attributes with module-name - replace pin-label with 3 new attributes: pin-board-label, pin-panel-label, pin-package-label - add device-id-get and pin-id-get commands - remove rclk-dev-name atribute - rename DPLL_PIN_DIRECTION_SOURCE -> DPLL_PIN_DIRECTION_INPUT [03/10] dpll: core: Add DPLL framework base functions [04/10] dpll: netlink: Add DPLL framework base functions [05/10] dpll: api header: Add DPLL framework base - remove unspec attributes after removing from dpll netlink spec - move pin-direction to pin-dpll tuple - pass parent_priv on state_on_pin_<get/set> - align with new notification definitions from netlink spec - use separated notifications for dpll pins and devices - format notification messages as corresponding get netlink commands - rename pin-idx to pin-id - remove attributes pin-parent-idx, device - use DPLL_A_PIN_PARENT to hold information on parent pin or dpll device - refactor lookup for pins and dplls for dpll subsystem - replace bus-name, dev-name with module-name - replace pin-label with 3 new attributes: pin-board-label, pin-panel-label, pin-package-label - add device-id-get and pin-id-get commands - rename dpll_xa_lock to dpll_lock - improve doxygen in dpll_core.c - remove unused parent and dev fields from dpll_device struct - use u32 for pin_idx in dpll_pin_alloc - use driver provided pin properties struct - verify pin/dpll owner on registering pin - remove const arg modifier for helper _priv functions - remove function declaration _get_by_name() - update SPDX headers - parse netlink set attributes with nlattr array - remove rclk-dev-name attribute - remove device pointer from dpll_pin_register/dpll_device_register - remove redundant doxygen from dpll header - use module_name() to get name of module - add missing/remove outdated kdocs - fix call frequency_set only if available - fix call direction_set only for pin-dpll tuple [06/10] netdev: expose DPLL pin handle for netdevice - rebased on top of v8 changes - use dpll_msg_add_pin_handle() in dpll_pin_find_from_nlattr() and dpll_msg_add_pin_parents() - fixed handle to use DPLL_A_PIN_ID and removed temporary comments - added documentation record for dpll_pin pointer - fixed compilation of net/core/dev.c when CONFIG_DPLL is not enabled - adjusted patch description a bit [07/10] ice: add admin commands to access cgu configuration - Remove unspec attributes after removing from dpll netlink spec. [08/10] ice: implement dpll interface to control cgu - remove unspec attributes - do not store pin flags received in set commands - use pin state field to provide pin state to the caller - remove include of uapi header - remove redundant check against null arguments - propagate lock function return value to the caller - use switch case instead of if statements - fix dev_dbg to dev_err for error cases - fix dpll/pin lookup on dpll subsytem callbacks - fix extack of dpll subsystem callbacks - remove double negation and variable cast - simplify ice_dpll_pin_state_set function - pass parent_priv on state_on_pin_<get/set> - remove parent hw_idx lookup - fix use const qualifier for dpll/dpll_pin ops - fix IS_ERR macros usage in ice_dpll - add notify previous source state change - fix mutex locking on releasing pins - use '|=' instead of '+=' when modifing capabilities field - rename ice_dpll_register_pins function - clock_id function to return clock ID on the stack instead of using an output variable - DPLL_LOCK_STATUS_CALIBRATING was removed, return: DPLL_LOCK_STATUS_LOCKED - if dpll was locked DPLL_LOCK_STATUS_LOCKED_HO_ACQ - if dpll was locked and holdover is acquired - propagate and use dpll_priv to obtain pf pointer in corresponding functions. - remove null check for pf pointer - adapt to `dpll: core: fix notification scheme` - expose pf related pin to corresponding netdevice - fix dpll init error path - fix dpll pins naming scheme `source` -> `input` - replace pin-label with pin-board-label - dpll remove parent and dev fields from dpll_device - remove device pointer from dpll_pin_register/dpll_device_register - rename DPLL_PIN_DIRECTION_SOURCE -> DPLL_PIN_DIRECTION_INPUT [09/10] ptp_ocp: implement DPLL ops - replace pin-label with pin-board-label - dpll remove parent and dev fields from dpll_device - remove device pointer from dpll_pin_register/dpll_device_register - rename DPLL_PIN_DIRECTION_SOURCE -> DPLL_PIN_DIRECTION_INPUT [10/10] mlx5: Implement SyncE support using DPLL infrastructure - rebased on top of v8 changes: - changed notification scheme - no need to fill pin label - implemented locked_ho_acq status - rename DPLL_PIN_DIRECTION_SOURCE -> DPLL_PIN_DIRECTION_INPUT - remove device pointer from dpll_pin_register/dpll_device_register - fixed MSEES register writes - adjusted pin state and lock state values reported - fixed a white space issue v6 -> v7: * YAML spec: - remove nested 'pin' attribute - clean up definitions on top of the latest changes * pin object: - pin xarray uses id provided by the driver - remove usage of PIN_IDX_INVALID in set function - source_pin_get() returns object instead of idx - fixes in frequency support API * device and pin operations are const now * small fixes in naming in Makefile and in the functions * single mutex for the subsystem to avoid possible ABBA locks * no special *_priv() helpers anymore, private data is passed as void* * no netlink filters by name anymore, only index is supported * update ptp_ocp and ice drivers to follow new API version * add mlx5e driver as a new customer of the subsystem v5 -> v6: * rework pin part to better fit shared pins use cases * add YAML spec to easy generate user-space apps * simple implementation in ptp_ocp is back again v4 -> v5: * fix code issues found during last reviews: - replace cookie with clock id - follow one naming schema in dpll subsys - move function comments to dpll_core.c, fix exports - remove single-use helper functions - merge device register with alloc - lock and unlock mutex on dpll device release - move dpll_type to uapi header - rename DPLLA_DUMP_FILTER to DPLLA_FILTER - rename dpll_pin_state to dpll_pin_mode - rename DPLL_MODE_FORCED to DPLL_MODE_MANUAL - remove DPLL_CHANGE_PIN_TYPE enum value * rewrite framework once again (Arkadiusz) - add clock class: Provide userspace with clock class value of DPLL with dpll device dump netlink request. Clock class is assigned by driver allocating a dpll device. Clock class values are defined as specified in: ITU-T G.8273.2/Y.1368.2 recommendation. - dpll device naming schema use new pattern: "dpll_%s_%d_%d", where: - %s - dev_name(parent) of parent device, - %d (1) - enum value of dpll type, - %d (2) - device index provided by parent device. - new muxed/shared pin registration: Let the kernel module to register a shared or muxed pin without finding it or its parent. Instead use a parent/shared pin description to find correct pin internally in dpll_core, simplifing a dpll API * Implement complex DPLL design in ice driver (Arkadiusz) * Remove ptp_ocp driver from the series for now v3 -> v4: * redesign framework to make pins dynamically allocated (Arkadiusz) * implement shared pins (Arkadiusz) v2 -> v3: * implement source select mode (Arkadiusz) * add documentation * implementation improvements (Jakub) v1 -> v2: * implement returning supported input/output types * ptp_ocp: follow suggestions from Jonathan * add linux-clk mailing list v0 -> v1: * fix code style and errors * add linux-arm mailing list *** BLURB HERE *** Arkadiusz Kubalewski (3): dpll: spec: Add Netlink spec in YAML ice: add admin commands to access cgu configuration ice: implement dpll interface to control cgu Jiri Pirko (2): netdev: expose DPLL pin handle for netdevice mlx5: Implement SyncE support using DPLL infrastructure Vadim Fedorenko (5): dpll: documentation on DPLL subsystem interface dpll: core: Add DPLL framework base functions dpll: netlink: Add DPLL framework base functions dpll: api header: Add DPLL framework base functions ptp_ocp: implement DPLL ops Documentation/driver-api/dpll.rst | 428 ++++ Documentation/driver-api/index.rst | 1 + Documentation/netlink/specs/dpll.yaml | 472 ++++ MAINTAINERS | 8 + drivers/Kconfig | 2 + drivers/Makefile | 1 + drivers/dpll/Kconfig | 7 + drivers/dpll/Makefile | 9 + drivers/dpll/dpll_core.c | 976 ++++++++ drivers/dpll/dpll_core.h | 92 + drivers/dpll/dpll_netlink.c | 1271 +++++++++++ drivers/dpll/dpll_netlink.h | 17 + drivers/dpll/dpll_nl.c | 163 ++ drivers/dpll/dpll_nl.h | 51 + drivers/net/ethernet/intel/Kconfig | 1 + drivers/net/ethernet/intel/ice/Makefile | 3 +- drivers/net/ethernet/intel/ice/ice.h | 5 + .../net/ethernet/intel/ice/ice_adminq_cmd.h | 240 +- drivers/net/ethernet/intel/ice/ice_common.c | 462 ++++ drivers/net/ethernet/intel/ice/ice_common.h | 43 + drivers/net/ethernet/intel/ice/ice_dpll.c | 2002 +++++++++++++++++ drivers/net/ethernet/intel/ice/ice_dpll.h | 101 + drivers/net/ethernet/intel/ice/ice_lib.c | 17 +- drivers/net/ethernet/intel/ice/ice_main.c | 7 + drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 413 ++++ drivers/net/ethernet/intel/ice/ice_ptp_hw.h | 230 ++ drivers/net/ethernet/intel/ice/ice_type.h | 1 + .../net/ethernet/mellanox/mlx5/core/Kconfig | 8 + .../net/ethernet/mellanox/mlx5/core/Makefile | 3 + drivers/net/ethernet/mellanox/mlx5/core/dev.c | 17 + .../net/ethernet/mellanox/mlx5/core/dpll.c | 432 ++++ drivers/ptp/Kconfig | 1 + drivers/ptp/ptp_ocp.c | 329 ++- include/linux/dpll.h | 164 ++ include/linux/mlx5/driver.h | 2 + include/linux/mlx5/mlx5_ifc.h | 59 +- include/linux/netdevice.h | 20 + include/uapi/linux/dpll.h | 183 ++ include/uapi/linux/if_link.h | 2 + net/core/dev.c | 22 + net/core/rtnetlink.c | 35 + 41 files changed, 8241 insertions(+), 59 deletions(-) create mode 100644 Documentation/driver-api/dpll.rst create mode 100644 Documentation/netlink/specs/dpll.yaml create mode 100644 drivers/dpll/Kconfig create mode 100644 drivers/dpll/Makefile create mode 100644 drivers/dpll/dpll_core.c create mode 100644 drivers/dpll/dpll_core.h create mode 100644 drivers/dpll/dpll_netlink.c create mode 100644 drivers/dpll/dpll_netlink.h create mode 100644 drivers/dpll/dpll_nl.c create mode 100644 drivers/dpll/dpll_nl.h create mode 100644 drivers/net/ethernet/intel/ice/ice_dpll.c create mode 100644 drivers/net/ethernet/intel/ice/ice_dpll.h create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/dpll.c create mode 100644 include/linux/dpll.h create mode 100644 include/uapi/linux/dpll.h