lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a8f9792b-93f1-b0b7-2600-38ac3c0e3832@machnikowski.net>
Date:   Fri, 9 Dec 2022 15:09:08 +0100
From:   Maciek Machnikowski <maciek@...hnikowski.net>
To:     Jiri Pirko <jiri@...nulli.us>
Cc:     Jakub Kicinski <kuba@...nel.org>,
        "'Kubalewski, Arkadiusz'" <arkadiusz.kubalewski@...el.com>,
        'Vadim Fedorenko' <vfedorenko@...ek.ru>,
        'Jonathan Lemon' <jonathan.lemon@...il.com>,
        'Paolo Abeni' <pabeni@...hat.com>, netdev@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-clk@...r.kernel.org
Subject: Re: [RFC PATCH v4 0/4] Create common DPLL/clock configuration API



On 12/9/2022 12:07 PM, Jiri Pirko wrote:
> Thu, Dec 08, 2022 at 07:08:04PM CET, maciek@...hnikowski.net wrote:
>> On 12/8/2022 12:21 AM, Jakub Kicinski wrote:
>> 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.
> 
> 
> Looking at the documentation of the chips, they all have mupltiple DPLLs
> on a die. Arkadiusz, in your proposed implementation, do you model each
> DPLL separatelly? If yes, then I understand the urgency of need of a
> shared pin. So all DPLLs sharing the pin are part of the same chip?
> 
> Question: can we have an entity, that would be 1:1 mapped to the actual
> device/chip here? Let's call is "a synchronizer". It would contain
> multiple DPLLs, user-facing-sources(input_connector),
> user-facing-outputs(output_connector), i/o pins.
> 
> An example:
>                                SYNCHRONIZER
> 
>                               ┌───────────────────────────────────────┐
>                               │                                       │
>                               │                                       │
>   SyncE in connector          │              ┌─────────┐              │     SyncE out connector
>                 ┌───┐         │in pin 1      │DPLL_1   │     out pin 1│    ┌───┐
>                 │   ├─────────┼──────────────┤         ├──────────────┼────┤   │
>                 │   │         │              │         │              │    │   │
>                 └───┘         │              │         │              │    └───┘
>                               │              │         │              │
>                               │           ┌──┤         │              │
>    GNSS in connector          │           │  └─────────┘              │
>                 ┌───┐         │in pin 2   │                  out pin 2│     EXT SMA connector
>                 │   ├─────────┼───────────┘                           │    ┌───┐
>                 │   │         │                           ┌───────────┼────┤   │
>                 └───┘         │                           │           │    │   │
>                               │                           │           │    └───┘
>                               │                           │           │
>    EXT SMA connector          │                           │           │
>                 ┌───┐   mux   │in pin 3      ┌─────────┐  │           │
>                 │   ├────┬────┼───────────┐  │         │  │           │
>                 │   │    │    │           │  │DPLL_2   │  │           │
>                 └───┘    │    │           │  │         │  │           │
>                          │    │           └──┤         ├──┘           │
>                          │    │              │         │              │
>    EXT SMA connector     │    │              │         │              │
>                 ┌───┐    │    │              │         │              │
>                 │   ├────┘    │              └─────────┘              │
>                 │   │         │                                       │
>                 └───┘         └───────────────────────────────────────┘
> 
> Do I get that remotelly correct?

It looks goot, hence two corrections are needed:
- all inputs can go to all DPLLs, and a single source can drive more
  than one DPLL
- The external mux for SMA connector should not be a part of the
  Synchronizer subsystem - I believe there's already a separate MUX
  subsystem in the kernel and all external connections should be handled
  by a devtree or a similar concept.

The only "muxing" thing that could potentially be modeled is a
synchronizer output to synchronizer input relation. Some synchronizers
does that internally and can use the output of one DPLL as a source for
another.

Also, in theory, the DPLL->output relation may change, however I assume
we can skip support for that at the beginning.

So something like this would be roughly correct:
       ┌───────────────────────────┐
       │                           │
┌──┐   │ src0   ┌─────────┐   out0 │    ┌──┐
│  ├───┼────────┤ DPLL1   ├────────┼────┤  │
└──┘   │        │         │        │    └──┘
       │        │         │        │
       │        │         │   out1 │    ┌──┐
┌──┐   │ src1   │         ├───┬────┼────┤  │
│  ├───┼──┬─────┤         │   │    │    └──┘
└──┘   │  │     └─────────┘   │    │
       │  │   ┌───────────────┘    │
       │  │   │   src_dpll1        │
       │  │   │ ┌─────────┐   out2 │    ┌──┐
       │  │   └─┤ DPLL2   ├────────┼────┤  │
       │  │     │         │        │    └──┘
       │  └─────┤         │        │
┌──┐   │ src2   │         │        │
│  ├───┼────────┤         │        │
└──┘   │        │         │        │
       │        └─────────┘        │
       │                           │
       │                           │
       │                           │
       └───────────────────────────┘

> synch
> synchronizer_register(synch)
>    dpll_1
>    synchronizer_dpll_register(synch, dpll_1)
>    dpll_2
>    synchronizer_dpll_register(synch, dpll_2)
>    source_pin_1
>    synchronizer_pin_register(synch, source_pin_1)
>    output_pin_1
>    synchronizer_pin_register(synch, output_pin_1)
>    output_pin_2
>    synchronizer_pin_register(synch, output_pin_2)
> 
> synch_board
>    synchronizer_board_register(synch_board)
>    synch
>    synchronizer_board_sync_register(synch_board, synch)
>    source_connector_1
>    synchronizer_board_connector_register(synch_board, source_connector_1, source_pin_1)
>    output_connector_1
>    synchronizer_board_connector_register(synch_board, output_connector_1, output_pin_1)
>    output_connector_2
>    synchronizer_board_connector_register(synch_board, output_connector_2, output_pin_2)

I'd rather not use pins at all - just stick to sources and outputs. Both
can use some labels to be identifiable.


> Thinking about it a bit more, this should be probably good to describe
> by device tree. The synchronizer itself dplls and pins it contains
> have constanc geometry, according to the synchronizer device type.
> 
> The Connector-pin linkages may vary according to the board.
> 
> So to divide it, there should be one synchronizer driver. Then probably
> some other one to connect/select/mux the connectors to the synchronizer.

Agreed - we should not model external board connections inside the
synchronizer driver subsystem.
-Maciek

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ