From: Maciek Machnikowski <maciek@machnikowski.net>
To: "Kubalewski, Arkadiusz" <arkadiusz.kubalewski@intel.com>,
Jiri Pirko <jiri@resnulli.us>
Cc: Jakub Kicinski <kuba@kernel.org>,
'Vadim Fedorenko' <vfedorenko@novek.ru>,
'Jonathan Lemon' <jonathan.lemon@gmail.com>,
'Paolo Abeni' <pabeni@redhat.com>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"linux-clk@vger.kernel.org" <linux-clk@vger.kernel.org>
Subject: Re: [RFC PATCH v4 0/4] Create common DPLL/clock configuration API
Date: Wed, 11 Jan 2023 16:54:13 +0100 [thread overview]
Message-ID: <c08622be-a054-5204-1875-b54f77626bff@machnikowski.net> (raw)
In-Reply-To: <DM6PR11MB4657E9C7F64FC9DF713696059BFC9@DM6PR11MB4657.namprd11.prod.outlook.com>
On 1/11/2023 4:30 PM, Kubalewski, Arkadiusz wrote:
>> From: Maciek Machnikowski <maciek@machnikowski.net>
>> Sent: Wednesday, January 11, 2023 3:40 PM
>> To: Kubalewski, Arkadiusz <arkadiusz.kubalewski@intel.com>; Jiri Pirko
>> <jiri@resnulli.us>
>>
>>
>> On 1/11/2023 3:17 PM, Kubalewski, Arkadiusz wrote:
>>>> From: Maciek Machnikowski <maciek@machnikowski.net>
>>>> Sent: Tuesday, January 10, 2023 3:59 PM
>>>> To: Kubalewski, Arkadiusz <arkadiusz.kubalewski@intel.com>; Jiri Pirko
>>>> <jiri@resnulli.us>
>>>>
>>>> On 1/10/2023 11:54 AM, Kubalewski, Arkadiusz wrote:
>>>>>> From: Jiri Pirko <jiri@resnulli.us>
>>>>>> Sent: Monday, January 9, 2023 5:30 PM
>>>>>>>
>>>>>>> Hi guys,
>>>>>>>
>>>>>>> We have been trying to figure out feasibility of new approach
>> proposed
>>>> on
>>>>>> our
>>>>>>> latest meeting - to have a single object which encapsulates multiple
>>>>>> DPLLs.
>>>>>>>
>>>>>>> Please consider following example:
>>>>>>>
>>>>>>> Shared common inputs:
>>>>>>> i0 - GPS / external
>>>>>>> i1 - SMA1 / external
>>>>>>> i2 - SMA2 / external
>>>>>>> i3 - MUX0 / clk recovered from PHY0.X driven by MAC0
>>>>>>> i4 - MUX1 / clk recovered from PHY1.X driven by MAC1
>>>>>>>
>>>>>>> +---------------------------------------------------------+
>>>>>>> | Channel A / FW0 +---+ |
>>>>>>> | i0--| | |
>>>>>>> | +---+ | | |
>>>>>>> | PHY0.0--| | i1--| D | |
>>>>>>> | | | | P | |
>>>>>>> | PHY0.1--| M | i2--| L | +---+ +--------+ |
>>>>>>> | | U | | L |---| |---| PHY0.0 |--|
>>>>>>> | PHY0.2--| X |-+---------i3--| 0 | | | +--------+ |
>>>>>>> | | 0 | |+------+ | |---| M |---| PHY0.1 |--|
>>>>>>> | ... --| | || MUX1 |-i4--| | | A | +--------+ |
>>>>>>> | | | |+------+ +---+ | C |---| PHY0.2 |--|
>>>>>>> | PHY0.7--| | | i0--| | | 0 | +--------+ |
>>>>>>> | +---+ | | |---| |---| ... |--|
>>>>>>> | | i1--| D | | | +--------+ |
>>>>>>> | | | P |---| |---| PHY0.7 |--|
>>>>>>> | | i2--| L | +---+ +--------+ |
>>>>>>> | | | L | |
>>>>>>> | \---------i3--| 1 | |
>>>>>>> | +------+ | | |
>>>>>>> | | MUX1 |-i4--| | |
>>>>>>> | +------+ +---+ |
>>>>>>> +---------------------------------------------------------+
>>>>>>> | Channel B / FW1 +---+ |
>>>>>>> | i0--| | |
>>>>>>> | | | |
>>>>>>> | i1--| D | |
>>>>>>> | +---+ | P | |
>>>>>>> | PHY1.0--| | i2--| L | +---+ +--------+ |
>>>>>>> | | | +------+ | L |---| |---| PHY1.0 |--|
>>>>>>> | PHY1.1--| M | | MUX0 |-i3--| 0 | | | +--------+ |
>>>>>>> | | U | +------+ | |---| M |---| PHY1.1 |--|
>>>>>>> | PHY1.2--| X |-+---------i4--| | | A | +--------+ |
>>>>>>> | | 1 | | +---+ | C |---| PHY1.2 |--|
>>>>>>> | ... --| | | i0--| | | 1 | +--------+ |
>>>>>>> | | | | | |---| |---| ... |--|
>>>>>>> | PHY1.7--| | | i1--| D | | | +--------+ |
>>>>>>> | +---+ | | P |---| |---| PHY1.7 |--|
>>>>>>> | | i2--| L | +---+ +--------+ |
>>>>>>> | |+------+ | L | |
>>>>>>> | || MUX0 |-i3--| 1 | |
>>>>>>> | |+------+ | | |
>>>>>>> | \---------i4--| | |
>>>>>>> | +---+ |
>>>>>>> +---------------------------------------------------------+
>>>>>>
>>>>>> What is "a channel" here? Are these 2 channels part of the same
>> physival
>>>>>> chip? Could you add the synchronizer chip/device entities to your
>>>> drawing?
>>>>>>
>>>>>
>>>>> No.
>>>>> A "Synchronization Channel" on a switch would allow to separate groups
>>>>> of physical ports. Each channel/group has own "Synchronizer Chip",
>> which
>>>> is
>>>>> used to drive PHY clocks of that group.
>>>>>
>>>>> "Synchronizer chip" would be the 2 DPLLs on old draw, something like
>>>> this:
>>>>> +--------------------------------------------------------------+
>>>>> | Channel A / FW0 +-------------+ +---+ +--------+ |
>>>>> | i0--|Synchronizer0|---| |---| PHY0.0 |--|
>>>>> | +---+ | | | | +--------+ |
>>>>> | PHY0.0--| | i1--| |---| M |---| PHY0.1 |--|
>>>>> | | | | +-----+ | | A | +--------+ |
>>>>> | PHY0.1--| M | i2--| |DPLL0| | | C |---| PHY0.2 |--|
>>>>> | | U | | +-----+ | | 0 | +--------+ |
>>>>> | PHY0.2--| X |--+---i3--| +-----+ |---| |---| ... |--|
>>>>> | | 0 | | | |DPLL1| | | | +--------+ |
>>>>> | ... --| | | /-i4--| +-----+ |---| |---| PHY0.7 |--|
>>>>> | | | | | +-------------+ +---+ +--------+ |
>>>>> | PHY0.7--| | | | |
>>>>> | +---+ | | |
>>>>> +----------------|-|-------------------------------------------+
>>>>> | Channel B / FW1| | +-------------+ +---+ +--------+ |
>>>>> | | | i0--|Synchronizer1|---| |---| PHY1.0 |--|
>>>>> | +---+ | | | | | | +--------+ |
>>>>> | PHY1.0--| | | | i1--| |---| M |---| PHY1.1 |--|
>>>>> | | | | | | +-----+ | | A | +--------+ |
>>>>> | PHY1.1--| M | | | i2--| |DPLL0| | | C |---| PHY1.2 |--|
>>>>> | | U | | | | +-----+ | | 1 | +--------+ |
>>>>> | PHY1.2--| X | \-|-i3--| +-----+ |---| |---| ... |--|
>>>>> | | 1 | | | |DPLL1| | | | +--------+ |
>>>>> | ... --| |----+-i4--| +-----+ |---| |---| PHY1.7 |--|
>>>>> | | | +-------------+ +---+ +--------+ |
>>>>> | PHY1.7--| | |
>>>>> | +---+ |
>>>>> +--------------------------------------------------------------+
>>>>> Also, please keep in mind that is an example, there could be easily 4
>>>>> (or more) channels wired similarly.
>>>>>
>>>>
>>>>
>>>> Hi,
>>>>
>>>> This model tries to put too much into the synchronizer subsystem. The
>>>> synchronizer device should only model inputs, DPLLs and outputs.
>>>>
>>>> The PHY lane to Synchronizer input muxing should be done in the
>>>> PHY/netdev subsystem. That's why I wanted to start with the full model
>>>> to specifically address this topic.
>>>>
>>>> The netdev should have an assigned list of Synchronizer inputs that it
>>>> can recover its SyncE clocks into. It can be done by having a connection
>>>> between the synchronizer input object(s) and the netdev, just like the
>>>> netdev is connected to PHC clocks in the PHC subsystem. This is the
>>>> model I initially presented about a year ago for solving this specific
>>>> issue.
>>>>
>>>> Analogically, the netdev will be connected to a given output, however
>>>> changing anything in the physical clock configuration sounds dangerous.
>>>>
>>>> Does that sound reasonable?
>>>>
>>>> Regards
>>>> Maciek
>>>
>>> It sounds reasonable to some point.
>>> You have mentioned list of Synchronizer inputs. If there is a list of
>> inputs
>>> it means it was created somewhere. I assume dpll subsystem? If so you
>> would
>>> like to export that list out of dpll subsystem, thus other entities would
>> need
>>> to find such list, then find particular source and somehow register with
>> it.
>>> All of this was proposed as part of netdev, I don't see any benefit in
>> having
>>> this parts separated from dpll, as only dpll would use it, right?
>>> The same behavior is now provided by the MUX type pin, enclosed within
>> dpll
>>> subsystem.
>>>
>>> BR,
>>> Arkadiusz
>>
>> The synchronizer object should expose the list of inputs that represent
>> possible sources of a given chip. The list will be the same for all
>> DPLLs used by the same device, so it can be a single set of sources
>> linked to multiple DPLLs inside the package. A netdev can then point to
>> a given input of a synchronizer that it's connected to.
>> The phy lane->recovered clock (or directly a synchronizer input) muxing
>> should stay in the netdev subsystem, or in the PHY driver.
>>
>> The reason, and benefit, of such split is when you create a board with a
>> netdev X and a synchronizer Y that is not instantiated by the same
>> driver. In this scenario you'd get the ice driver to instantiate
>> connections and the DPLL vendor's driver for the synchronizer. In such
>> case the netdev driver will simply send a netlink message to the
>> input/source with a requested configuration, such as expected frequency,
>> and everything from this point can be handled by a completely different
>> driver creating clean and logical split.
>>
>> If we mix the phy lanes into the DPLL subsystem it'll get very
>> challenging to add PHY lanes to the existing synchronizer exposed by a
>> different driver.
>
> This is possible right now:
> 1. obtain a dpll object:
> struct dpll_device *dpll_device_get_by_clock_id(u64 clock_id,
> enum dpll_type type, u8 idx);
> 2. register new pin with muxed type pin:
> int dpll_muxed_pin_register(struct dpll_device *dpll,
> const char *parent_pin_description,
> struct dpll_pin *pin,
> struct dpll_pin_ops *ops, void *priv);
>
> To find dpll driver must know clock_id, type of dpll and its index given
> when dpll was registered.
> To register a pin, parent_pin_description of MUX type pin given on registering
> it with dpll device.
That would mean you need to repeat this process for all the DPLLs that
are co-packaged in a single synchronizer. Some chips have up to 8 DPLLs,
so you'd need to register number of phy lanes x number of DPLL times,
say 8x8 = 64 times - that's simply too messy in the long term.
>> Exporting and link between the synchronizer and the netdev is still a
>> must no matter which way we go. And IMO it's best to link netdev to
>> synchronizer sources, as that's the most natural way.
>>
>
> The link is now just information for userspace Linux network interface index
> in DPLLA_PIN_NETIFINDEX attribute.
That's not the right way. We need to know:
- which DPLL pins/sources are driven by which netdev (to be able to
identify a source of frequency that is currently driving a given DPLL)
- which DPLL generates a frequency for a given netdev (to know which
DPLL to check for a specific netdev)
So we need to have 2 connections.
next prev parent reply other threads:[~2023-01-11 15:55 UTC|newest]
Thread overview: 87+ 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 ` [RFC PATCH v4 1/4] dpll: add dpll_attr/dpll_pin_attr helper classes Vadim Fedorenko
2022-11-29 21:37 ` [RFC PATCH v4 2/4] dpll: Add DPLL framework base functions Vadim Fedorenko
2022-11-30 15:21 ` Jiri Pirko
2022-11-30 16:23 ` Jiri Pirko
2022-12-23 16:45 ` Kubalewski, Arkadiusz
2023-01-02 12:28 ` Jiri Pirko
2022-11-30 16:37 ` Jiri Pirko
2022-12-02 11:27 ` Kubalewski, Arkadiusz
2022-12-02 12:39 ` Jiri Pirko
2022-12-02 14:54 ` Kubalewski, Arkadiusz
2022-12-02 16:15 ` Jiri Pirko
[not found] ` <20221202212206.3619bd5f@kernel.org>
2022-12-05 10:32 ` Jiri Pirko
2022-12-06 0:19 ` Jakub Kicinski
2022-12-06 8:50 ` Jiri Pirko
2022-12-06 17:27 ` Jakub Kicinski
2022-12-07 13:10 ` Jiri Pirko
2022-12-07 16:59 ` Jakub Kicinski
2022-12-08 8:14 ` Jiri Pirko
2022-12-08 16:19 ` Jakub Kicinski
2022-12-08 16:33 ` Jiri Pirko
2022-12-08 17:05 ` Jakub Kicinski
2022-12-09 9:29 ` Jiri Pirko
2022-12-09 16:19 ` Jakub Kicinski
2022-12-12 13:36 ` Jiri Pirko
2022-12-13 18:08 ` Kubalewski, Arkadiusz
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-12-19 9:13 ` Paolo Abeni
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-30 12:41 ` Jiri Pirko
2022-12-02 11:27 ` Kubalewski, Arkadiusz
2022-12-02 12:48 ` Jiri Pirko
2022-12-02 14:39 ` Kubalewski, Arkadiusz
2022-12-02 16:20 ` Jiri Pirko
2022-12-08 0:35 ` Kubalewski, Arkadiusz
2022-12-08 8:19 ` Jiri Pirko
2022-12-07 2:33 ` Jakub Kicinski
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-12-02 11:27 ` Kubalewski, Arkadiusz
2022-12-02 16:12 ` Jiri Pirko
2022-12-07 2:47 ` Jakub Kicinski
2022-12-07 14:09 ` netdev.dump
2022-12-07 23:21 ` Jakub Kicinski
2022-12-08 11:28 ` Jiri Pirko
2022-12-09 0:39 ` Jakub Kicinski
2022-12-09 0:56 ` Kubalewski, Arkadiusz
2022-12-08 18:08 ` Maciek Machnikowski
2022-12-09 11:07 ` Jiri Pirko
2022-12-09 14:09 ` Maciek Machnikowski
2022-12-09 16:31 ` Jakub Kicinski
2022-12-09 17:11 ` Maciek Machnikowski
2022-12-12 13:58 ` Jiri Pirko
2023-01-09 14:43 ` Kubalewski, Arkadiusz
2023-01-09 16:30 ` Jiri Pirko
2023-01-10 10:54 ` Kubalewski, Arkadiusz
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:40 ` Maciek Machnikowski
2023-01-11 15:30 ` Kubalewski, Arkadiusz
2023-01-11 15:54 ` Maciek Machnikowski [this message]
2023-01-11 16:27 ` Kubalewski, Arkadiusz
2023-01-10 20:05 ` Jakub Kicinski
2023-01-11 8:19 ` Jiri Pirko
2023-01-11 14:16 ` Kubalewski, Arkadiusz
2023-01-11 15:04 ` Jiri Pirko
2023-01-11 15:30 ` Kubalewski, Arkadiusz
2023-01-11 16:14 ` Jiri Pirko
2023-01-12 12:15 ` Kubalewski, Arkadiusz
2023-01-12 14:43 ` Jiri Pirko
2022-12-09 0:46 ` Kubalewski, Arkadiusz
2022-12-07 14:51 ` Jiri Pirko
[not found] ` <20221207091946.3115742f@kernel.org>
2022-12-08 12:02 ` Jiri Pirko
2022-12-09 0:54 ` Jakub Kicinski
2022-12-08 18:23 ` Kubalewski, Arkadiusz
2022-12-08 0:27 ` Kubalewski, Arkadiusz
2022-12-08 11:58 ` Jiri Pirko
2022-12-08 23:05 ` Kubalewski, Arkadiusz
2022-12-09 10:01 ` Jiri Pirko
2023-01-12 12:23 ` Kubalewski, Arkadiusz
2023-01-12 14:50 ` Jiri Pirko
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=c08622be-a054-5204-1875-b54f77626bff@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).