All of lore.kernel.org
 help / color / mirror / Atom feed
From: Maciek Machnikowski <maciek@machnikowski.net>
To: Jakub Kicinski <kuba@kernel.org>
Cc: 'Jiri Pirko' <jiri@resnulli.us>,
	"'Kubalewski, Arkadiusz'" <arkadiusz.kubalewski@intel.com>,
	'Vadim Fedorenko' <vfedorenko@novek.ru>,
	'Jonathan Lemon' <jonathan.lemon@gmail.com>,
	'Paolo Abeni' <pabeni@redhat.com>,
	netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-clk@vger.kernel.org
Subject: Re: [RFC PATCH v4 0/4] Create common DPLL/clock configuration API
Date: Thu, 8 Dec 2022 19:08:04 +0100	[thread overview]
Message-ID: <6e252f6d-283e-7138-164f-092709bc1292@machnikowski.net> (raw)
In-Reply-To: <20221207152157.6185b52b@kernel.org>

On 12/8/2022 12:21 AM, Jakub Kicinski wrote:
> On Wed, 7 Dec 2022 15:09:03 +0100 netdev.dump@gmail.com wrote:
>>> -----Original Message-----
>>> From: Jakub Kicinski <kuba@kernel.org>
>> pins between the DPLLs exposed by a single driver, but not really outside of
>> it.
>> And that can be done simply by putting the pin ptr from the DPLLA into the
>> pin
>> list of DPLLB.
> 
> Are you saying within the driver it's somehow easier? The driver state
> is mostly per bus device, so I don't see how.
> 
>> If we want the kitchen-and-sink solution, we need to think about corner
>> cases.
>> Which pin should the API give to the userspace app - original, or
>> muxed/parent?
> 
> IDK if I parse but I think both. If selected pin is not directly
> attached the core should configure muxes.
> 
>> How would a teardown look like - if Driver A registered DPLLA with Pin1 and
>> Driver B added the muxed pin then how should Driver A properly
>> release its pins? Should it just send a message to driver B and trust that
>> it
>> will receive it in time before we tear everything apart?
> 
> Trivial.
> 
>> There are many problems with that approach, and the submitted patch is not
>> explaining any of them. E.g. it contains the dpll_muxed_pin_register but no
>> free 
>> counterpart + no flows.
> 
> SMOC.
> 
>> If we want to get shared pins, we need a good example of how this mechanism
>> can be used.
> 
> Agreed.

My main complaint about the current pins implementation is that they put
everything in a single bag. In a netdev world - it would be like we put
TX queues and RX queues together, named them "Queues", expose a list to
the userspace and let the user figure out which ones which by reading a
"TX" flag.

All DPLLs I know have a Sources block, DPLLs and Output blocks. See:

https://www.renesas.com/us/en/products/clocks-timing/jitter-attenuators-frequency-translation/8a34044-multichannel-dpll-dco-four-eight-channels#overview

https://ww1.microchip.com/downloads/aemDocuments/documents/TIM/ProductDocuments/ProductBrief/ZL3063x-System-Synchronizers-with-up-to-5-Channels-10-Inputs-20-Outputs-Product-Brief-DS20006634.pdf

https://www.sitime.com/support/resource-library/product-briefs/cascade-sit9514x-clock-system-chip-family

https://www.ti.com/lit/ds/symlink/lmk5b33414.pdf?ts=1670516132647&ref_url=https%253A%252F%252Fwww.ti.com%252Fclocks-timing%252Fjitter-cleaners-synchronizers%252Fproducts.html

If we model everything as "pins" we won't be able to correctly extend
the API to add new features.

Sources can configure the expected frequency, input signal monitoring
(on multiple layers), expected signal levels, input termination and so
on. Outputs will need the enable flag, signal format, frequency, phase
offset etc. Multiple DPLLs can reuse a single source inside the same
package simultaneously.

A source should be able to link to a pin or directly to the netdev for
some embedded solutions. We don't need to go through the pin abstraction
at all.

An optional pin entity should only represent a physical connection with
a name and maybe a 3-state selection of In/Out/HiZ and then link to
sources or output of the DPLL(s).

Finally, the DPLL object should keep track of the source priority list,
have a proper status (locked/unlocked/holdover/freerunning), implement
the NCO mode, lock thresholds, bandwidths, auto-switch mode and so on.

Current implementation creates a lot of ambiguity, mixes input pins with
output pins and assigns priories to pins. Every SW entity will receive a
big list of pins and will need to parse it.

I prefer the approach that the ptp subsystem set - with its abstraction
of input/output channels and pins that can be assigned to them. While
not perfect - it represents reality much closer.

Thanks
Maciek

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Maciek Machnikowski <maciek@machnikowski.net>
To: Jakub Kicinski <kuba@kernel.org>
Cc: 'Jiri Pirko' <jiri@resnulli.us>,
	"'Kubalewski, Arkadiusz'" <arkadiusz.kubalewski@intel.com>,
	'Vadim Fedorenko' <vfedorenko@novek.ru>,
	'Jonathan Lemon' <jonathan.lemon@gmail.com>,
	'Paolo Abeni' <pabeni@redhat.com>,
	netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-clk@vger.kernel.org
Subject: Re: [RFC PATCH v4 0/4] Create common DPLL/clock configuration API
Date: Thu, 8 Dec 2022 19:08:04 +0100	[thread overview]
Message-ID: <6e252f6d-283e-7138-164f-092709bc1292@machnikowski.net> (raw)
In-Reply-To: <20221207152157.6185b52b@kernel.org>

On 12/8/2022 12:21 AM, Jakub Kicinski wrote:
> On Wed, 7 Dec 2022 15:09:03 +0100 netdev.dump@gmail.com wrote:
>>> -----Original Message-----
>>> From: Jakub Kicinski <kuba@kernel.org>
>> pins between the DPLLs exposed by a single driver, but not really outside of
>> it.
>> And that can be done simply by putting the pin ptr from the DPLLA into the
>> pin
>> list of DPLLB.
> 
> Are you saying within the driver it's somehow easier? The driver state
> is mostly per bus device, so I don't see how.
> 
>> If we want the kitchen-and-sink solution, we need to think about corner
>> cases.
>> Which pin should the API give to the userspace app - original, or
>> muxed/parent?
> 
> IDK if I parse but I think both. If selected pin is not directly
> attached the core should configure muxes.
> 
>> How would a teardown look like - if Driver A registered DPLLA with Pin1 and
>> Driver B added the muxed pin then how should Driver A properly
>> release its pins? Should it just send a message to driver B and trust that
>> it
>> will receive it in time before we tear everything apart?
> 
> Trivial.
> 
>> There are many problems with that approach, and the submitted patch is not
>> explaining any of them. E.g. it contains the dpll_muxed_pin_register but no
>> free 
>> counterpart + no flows.
> 
> SMOC.
> 
>> If we want to get shared pins, we need a good example of how this mechanism
>> can be used.
> 
> Agreed.

My main complaint about the current pins implementation is that they put
everything in a single bag. In a netdev world - it would be like we put
TX queues and RX queues together, named them "Queues", expose a list to
the userspace and let the user figure out which ones which by reading a
"TX" flag.

All DPLLs I know have a Sources block, DPLLs and Output blocks. See:

https://www.renesas.com/us/en/products/clocks-timing/jitter-attenuators-frequency-translation/8a34044-multichannel-dpll-dco-four-eight-channels#overview

https://ww1.microchip.com/downloads/aemDocuments/documents/TIM/ProductDocuments/ProductBrief/ZL3063x-System-Synchronizers-with-up-to-5-Channels-10-Inputs-20-Outputs-Product-Brief-DS20006634.pdf

https://www.sitime.com/support/resource-library/product-briefs/cascade-sit9514x-clock-system-chip-family

https://www.ti.com/lit/ds/symlink/lmk5b33414.pdf?ts=1670516132647&ref_url=https%253A%252F%252Fwww.ti.com%252Fclocks-timing%252Fjitter-cleaners-synchronizers%252Fproducts.html

If we model everything as "pins" we won't be able to correctly extend
the API to add new features.

Sources can configure the expected frequency, input signal monitoring
(on multiple layers), expected signal levels, input termination and so
on. Outputs will need the enable flag, signal format, frequency, phase
offset etc. Multiple DPLLs can reuse a single source inside the same
package simultaneously.

A source should be able to link to a pin or directly to the netdev for
some embedded solutions. We don't need to go through the pin abstraction
at all.

An optional pin entity should only represent a physical connection with
a name and maybe a 3-state selection of In/Out/HiZ and then link to
sources or output of the DPLL(s).

Finally, the DPLL object should keep track of the source priority list,
have a proper status (locked/unlocked/holdover/freerunning), implement
the NCO mode, lock thresholds, bandwidths, auto-switch mode and so on.

Current implementation creates a lot of ambiguity, mixes input pins with
output pins and assigns priories to pins. Every SW entity will receive a
big list of pins and will need to parse it.

I prefer the approach that the ptp subsystem set - with its abstraction
of input/output channels and pins that can be assigned to them. While
not perfect - it represents reality much closer.

Thanks
Maciek

  parent reply	other threads:[~2022-12-08 18:09 UTC|newest]

Thread overview: 172+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-29 21:37 [RFC PATCH v4 0/4] Create common DPLL/clock configuration API Vadim Fedorenko
2022-11-29 21:37 ` Vadim Fedorenko
2022-11-29 21:37 ` [RFC PATCH v4 1/4] dpll: add dpll_attr/dpll_pin_attr helper classes Vadim Fedorenko
2022-11-29 21:37   ` Vadim Fedorenko
2022-11-29 21:37 ` [RFC PATCH v4 2/4] dpll: Add DPLL framework base functions Vadim Fedorenko
2022-11-29 21:37   ` Vadim Fedorenko
2022-11-30 15:21   ` Jiri Pirko
2022-11-30 15:21     ` Jiri Pirko
2022-11-30 16:23     ` Jiri Pirko
2022-11-30 16:23       ` Jiri Pirko
2022-12-23 16:45     ` Kubalewski, Arkadiusz
2022-12-23 16:45       ` Kubalewski, Arkadiusz
2023-01-02 12:28       ` Jiri Pirko
2023-01-02 12:28         ` Jiri Pirko
2022-11-30 16:37   ` Jiri Pirko
2022-11-30 16:37     ` Jiri Pirko
2022-12-02 11:27     ` Kubalewski, Arkadiusz
2022-12-02 11:27       ` Kubalewski, Arkadiusz
2022-12-02 12:39       ` Jiri Pirko
2022-12-02 12:39         ` Jiri Pirko
2022-12-02 14:54         ` Kubalewski, Arkadiusz
2022-12-02 14:54           ` Kubalewski, Arkadiusz
2022-12-02 16:15           ` Jiri Pirko
2022-12-02 16:15             ` Jiri Pirko
     [not found]             ` <20221202212206.3619bd5f@kernel.org>
2022-12-05 10:32               ` Jiri Pirko
2022-12-05 10:32                 ` Jiri Pirko
2022-12-06  0:19                 ` Jakub Kicinski
2022-12-06  0:19                   ` Jakub Kicinski
2022-12-06  8:50                   ` Jiri Pirko
2022-12-06  8:50                     ` Jiri Pirko
2022-12-06 17:27                     ` Jakub Kicinski
2022-12-06 17:27                       ` Jakub Kicinski
2022-12-07 13:10                       ` Jiri Pirko
2022-12-07 13:10                         ` Jiri Pirko
2022-12-07 16:59                         ` Jakub Kicinski
2022-12-07 16:59                           ` Jakub Kicinski
2022-12-08  8:14                           ` Jiri Pirko
2022-12-08  8:14                             ` Jiri Pirko
2022-12-08 16:19                             ` Jakub Kicinski
2022-12-08 16:19                               ` Jakub Kicinski
2022-12-08 16:33                               ` Jiri Pirko
2022-12-08 16:33                                 ` Jiri Pirko
2022-12-08 17:05                                 ` Jakub Kicinski
2022-12-08 17:05                                   ` Jakub Kicinski
2022-12-09  9:29                                   ` Jiri Pirko
2022-12-09  9:29                                     ` Jiri Pirko
2022-12-09 16:19                                     ` Jakub Kicinski
2022-12-09 16:19                                       ` Jakub Kicinski
2022-12-12 13:36                                       ` Jiri Pirko
2022-12-12 13:36                                         ` Jiri Pirko
2022-12-13 18:08                                         ` Kubalewski, Arkadiusz
2022-12-13 18:08                                           ` Kubalewski, Arkadiusz
2022-12-14  7:32                                           ` Jiri Pirko
2022-12-14  7:32                                             ` Jiri Pirko
2022-11-29 21:37 ` [RFC PATCH v4 3/4] dpll: documentation on DPLL subsystem interface Vadim Fedorenko
2022-11-29 21:37   ` Vadim Fedorenko
2022-12-02 12:43   ` kernel test robot
2022-12-19  9:13   ` Paolo Abeni
2022-12-19  9:13     ` Paolo Abeni
2023-01-12 13:45     ` Kubalewski, Arkadiusz
2023-01-12 13:45       ` Kubalewski, Arkadiusz
2022-11-29 21:37 ` [RFC PATCH v4 4/4] ptp_ocp: implement DPLL ops Vadim Fedorenko
2022-11-29 21:37   ` Vadim Fedorenko
2022-11-30  8:30   ` kernel test robot
2022-11-30 12:41   ` Jiri Pirko
2022-11-30 12:41     ` Jiri Pirko
2022-12-02 11:27     ` Kubalewski, Arkadiusz
2022-12-02 11:27       ` Kubalewski, Arkadiusz
2022-12-02 12:48       ` Jiri Pirko
2022-12-02 12:48         ` Jiri Pirko
2022-12-02 14:39         ` Kubalewski, Arkadiusz
2022-12-02 14:39           ` Kubalewski, Arkadiusz
2022-12-02 16:20           ` Jiri Pirko
2022-12-02 16:20             ` Jiri Pirko
2022-12-08  0:35             ` Kubalewski, Arkadiusz
2022-12-08  0:35               ` Kubalewski, Arkadiusz
2022-12-08  8:19               ` Jiri Pirko
2022-12-08  8:19                 ` Jiri Pirko
2022-12-07  2:33           ` Jakub Kicinski
2022-12-07  2:33             ` Jakub Kicinski
2022-12-07 13:19             ` Jiri Pirko
2022-12-07 13:19               ` Jiri Pirko
     [not found]               ` <20221207090524.3f562eeb@kernel.org>
2022-12-08 11:22                 ` Jiri Pirko
2022-12-09  0:36                   ` Jakub Kicinski
2022-12-09  9:32                     ` Jiri Pirko
2022-11-30 12:32 ` [RFC PATCH v4 0/4] Create common DPLL/clock configuration API Jiri Pirko
2022-11-30 12:32   ` Jiri Pirko
2022-12-02 11:27   ` Kubalewski, Arkadiusz
2022-12-02 11:27     ` Kubalewski, Arkadiusz
2022-12-02 16:12     ` Jiri Pirko
2022-12-02 16:12       ` Jiri Pirko
2022-12-07  2:47       ` Jakub Kicinski
2022-12-07  2:47         ` Jakub Kicinski
2022-12-07 14:09         ` netdev.dump
2022-12-07 14:09           ` netdev.dump
2022-12-07 23:21           ` Jakub Kicinski
2022-12-07 23:21             ` Jakub Kicinski
2022-12-08 11:28             ` Jiri Pirko
2022-12-08 11:28               ` Jiri Pirko
2022-12-09  0:39               ` Jakub Kicinski
2022-12-09  0:39                 ` Jakub Kicinski
2022-12-09  0:56                 ` Kubalewski, Arkadiusz
2022-12-09  0:56                   ` Kubalewski, Arkadiusz
2022-12-08 18:08             ` Maciek Machnikowski [this message]
2022-12-08 18:08               ` Maciek Machnikowski
2022-12-09 11:07               ` Jiri Pirko
2022-12-09 11:07                 ` Jiri Pirko
2022-12-09 14:09                 ` Maciek Machnikowski
2022-12-09 14:09                   ` Maciek Machnikowski
2022-12-09 16:31                   ` Jakub Kicinski
2022-12-09 16:31                     ` Jakub Kicinski
2022-12-09 17:11                     ` Maciek Machnikowski
2022-12-09 17:11                       ` Maciek Machnikowski
2022-12-12 13:58                     ` Jiri Pirko
2022-12-12 13:58                       ` Jiri Pirko
2023-01-09 14:43                       ` Kubalewski, Arkadiusz
2023-01-09 14:43                         ` Kubalewski, Arkadiusz
2023-01-09 16:30                         ` Jiri Pirko
2023-01-09 16:30                           ` Jiri Pirko
2023-01-10 10:54                           ` Kubalewski, Arkadiusz
2023-01-10 10:54                             ` Kubalewski, Arkadiusz
2023-01-10 14:28                             ` Jiri Pirko
2023-01-10 14:28                               ` Jiri Pirko
     [not found]                             ` <645a5bfd-0092-2f39-0ff2-3ffb27ccf8fe@machnikowski.net>
2023-01-11 14:17                               ` Kubalewski, Arkadiusz
2023-01-11 14:17                                 ` Kubalewski, Arkadiusz
2023-01-11 14:40                                 ` Maciek Machnikowski
2023-01-11 14:40                                   ` Maciek Machnikowski
2023-01-11 15:30                                   ` Kubalewski, Arkadiusz
2023-01-11 15:30                                     ` Kubalewski, Arkadiusz
2023-01-11 15:54                                     ` Maciek Machnikowski
2023-01-11 15:54                                       ` Maciek Machnikowski
2023-01-11 16:27                                       ` Kubalewski, Arkadiusz
2023-01-11 16:27                                         ` Kubalewski, Arkadiusz
2023-01-10 20:05                         ` Jakub Kicinski
2023-01-10 20:05                           ` Jakub Kicinski
2023-01-11  8:19                           ` Jiri Pirko
2023-01-11  8:19                             ` Jiri Pirko
2023-01-11 14:16                             ` Kubalewski, Arkadiusz
2023-01-11 14:16                               ` Kubalewski, Arkadiusz
2023-01-11 15:04                               ` Jiri Pirko
2023-01-11 15:04                                 ` Jiri Pirko
2023-01-11 15:30                                 ` Kubalewski, Arkadiusz
2023-01-11 15:30                                   ` Kubalewski, Arkadiusz
2023-01-11 16:14                                   ` Jiri Pirko
2023-01-11 16:14                                     ` Jiri Pirko
2023-01-12 12:15                                     ` Kubalewski, Arkadiusz
2023-01-12 12:15                                       ` Kubalewski, Arkadiusz
2023-01-12 14:43                                       ` Jiri Pirko
2023-01-12 14:43                                         ` Jiri Pirko
2022-12-09  0:46             ` Kubalewski, Arkadiusz
2022-12-09  0:46               ` Kubalewski, Arkadiusz
2022-12-07 14:51         ` Jiri Pirko
2022-12-07 14:51           ` Jiri Pirko
     [not found]           ` <20221207091946.3115742f@kernel.org>
2022-12-08 12:02             ` Jiri Pirko
2022-12-08 12:02               ` Jiri Pirko
2022-12-09  0:54               ` Jakub Kicinski
2022-12-08 18:23             ` Kubalewski, Arkadiusz
2022-12-08 18:23               ` Kubalewski, Arkadiusz
2022-12-08  0:27       ` Kubalewski, Arkadiusz
2022-12-08  0:27         ` Kubalewski, Arkadiusz
2022-12-08 11:58         ` Jiri Pirko
2022-12-08 11:58           ` Jiri Pirko
2022-12-08 23:05           ` Kubalewski, Arkadiusz
2022-12-08 23:05             ` Kubalewski, Arkadiusz
2022-12-09 10:01             ` Jiri Pirko
2022-12-09 10:01               ` Jiri Pirko
2023-01-12 12:23 ` Kubalewski, Arkadiusz
2023-01-12 12:23   ` Kubalewski, Arkadiusz
2023-01-12 14:50   ` Jiri Pirko
2023-01-12 14:50     ` Jiri Pirko
2023-01-12 19:09   ` Jakub Kicinski
2023-01-12 19:09     ` Jakub Kicinski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6e252f6d-283e-7138-164f-092709bc1292@machnikowski.net \
    --to=maciek@machnikowski.net \
    --cc=arkadiusz.kubalewski@intel.com \
    --cc=jiri@resnulli.us \
    --cc=jonathan.lemon@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=vfedorenko@novek.ru \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.