| 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
| ||
|
Message-ID: <158380574777.149997.1766994748078874683@swboyd.mtv.corp.google.com>
Date: Mon, 09 Mar 2020 19:02:27 -0700
From: Stephen Boyd <sboyd@...nel.org>
To: Mark Rutland <mark.rutland@....com>,
Michael Turquette <mturquette@...libre.com>,
Rob Herring <robh+dt@...nel.org>,
Sergey.Semin@...kalelectronics.ru
Cc: Serge Semin <Sergey.Semin@...kalelectronics.ru>,
Serge Semin <fancer.lancer@...il.com>,
Alexey Malahov <Alexey.Malahov@...kalelectronics.ru>,
Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
Paul Burton <paulburton@...nel.org>,
Ralf Baechle <ralf@...ux-mips.org>, linux-clk@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/5] dt-bindings: clk: Add Baikal-T1 CCU PLLs bindings
Quoting Sergey.Semin@...kalelectronics.ru (2020-03-06 05:00:44)
> From: Serge Semin <Sergey.Semin@...kalelectronics.ru>
>
> Baikal-T1 Clocks Control Unit is responsible for transformation of a
> signal coming from an external oscillator into clocks of various
> frequencies to propagate them then to the corresponding clocks
> consumers (either individual IP-blocks or clock domains). In order
> to create a set of high-frequency clocks the external signal is
> firstly handled by the embedded into CCU PLLs. So the corresponding
> dts-node is just a normal clock-provider node with standard set of
> properties.
>
> Signed-off-by: Serge Semin <Sergey.Semin@...kalelectronics.ru>
> Signed-off-by: Alexey Malahov <Alexey.Malahov@...kalelectronics.ru>
SoB chain is backwards. Is Alexey the author? Or Co-developed-by?
> Cc: Thomas Bogendoerfer <tsbogend@...ha.franken.de>
> Cc: Paul Burton <paulburton@...nel.org>
> Cc: Ralf Baechle <ralf@...ux-mips.org>
> ---
> .../bindings/clock/be,bt1-ccu-pll.yaml | 139 ++++++++++++++++++
> include/dt-bindings/clock/bt1-ccu.h | 17 +++
> 2 files changed, 156 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/be,bt1-ccu-pll.yaml
> create mode 100644 include/dt-bindings/clock/bt1-ccu.h
>
> diff --git a/Documentation/devicetree/bindings/clock/be,bt1-ccu-pll.yaml b/Documentation/devicetree/bindings/clock/be,bt1-ccu-pll.yaml
> new file mode 100644
> index 000000000000..f2e397cc147b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/be,bt1-ccu-pll.yaml
> @@ -0,0 +1,139 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Copyright (C) 2019 - 2020 BAIKAL ELECTRONICS, JSC
> +#
> +# Baikal-T1 Clocks Control Unit PLL Device Tree Bindings.
> +#
I don't think we need any of these comments besides the license
identifier line. Can you dual license this?
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/be,bt1-ccu-pll.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Baikal-T1 Clock Control Unit PLLs
> +
> +maintainers:
> + - Serge Semin <fancer.lancer@...il.com>
> +
> +description: |
> + Clocks Control Unit is the core of Baikal-T1 SoC responsible for the chip
> + subsystems clocking and resetting. The CCU is connected with an external
> + fixed rate oscillator, which signal is transformed into clocks of various
> + frequencies and then propagated to either individual IP-blocks or to groups
> + of blocks (clock domains). The transformation is done by means of PLLs and
> + gateable/non-gateable dividers embedded into the CCU. It's logically divided
> + into the next components:
> + 1) External oscillator (normally XTAL's 25 MHz crystal oscillator, but
> + in general can provide any frequency supported by the CCU PLLs).
> + 2) PLLs clocks generators (PLLs) - described in this bindings file.
> + 3) AXI-bus clock dividers (AXI).
> + 4) System devices reference clock dividers (SYS).
> + which are connected with each other as shown on the next figure:
Please add a newline here
> + +---------------+
> + | Baikal-T1 CCU |
> + | +----+------|- MIPS P5600 cores
> + | +-|PLLs|------|- DDR controller
> + | | +----+ |
> + +----+ | | | | |
> + |XTAL|--|-+ | | +---+-|
> + +----+ | | | +-|AXI|-|- AXI-bus
> + | | | +---+-|
> + | | | |
> + | | +----+---+-|- APB-bus
> + | +-------|SYS|-|- Low-speed Devices
> + | +---+-|- High-speed Devices
> + +---------------+
And here.
> + Each CCU sub-block is represented as a separate dts-node and has an
> + individual driver to be bound with.
> +
> + In order to create signals of wide range frequencies the external oscillator
> + output is primarily connected to a set of CCU PLLs. There are five PLLs
> + to create a clock for the MIPS P5600 cores, the embedded DDR controller,
> + SATA, Ethernet and PCIe domains. The last three domains though named by the
> + biggest system interfaces in fact include nearly all of the rest SoC
> + peripherals. Each of the PLLs is based on True Circuits TSMC CLN28HPM core
> + with an interface wrapper (so called safe PLL' clocks switcher) to simplify
> + the PLL configuration procedure. The PLLs work as depicted on the next
> + diagram:
Same, space out the diagrams.
> + +--------------------------+
> + | |
> + +-->+---+ +---+ +---+ | +---+ 0|\
> + CLKF--->|/NF|--->|PFD|...|VCO|-+->|/OD|--->| |
> + +---+ +->+---+ +---+ /->+---+ | |--->CLKOUT
> + CLKOD---------C----------------+ 1| |
> + +--------C--------------------------->|/
> + | | ^
> + Rclk-+->+---+ | |
> + CLKR--->|/NR|-+ |
> + +---+ |
> + BYPASS--------------------------------------+
> + BWADJ--->
> + where Rclk is the reference clock coming from XTAL, NR - reference clock
> + divider, NF - PLL clock multiplier, OD - VCO output clock divider, CLKOUT -
> + output clock, BWADJ is the PLL bandwidth adjustment parameter. At this moment
> + the binding supports the PLL dividers configuration in accordance with a
> + requested rate, while bypassing and bandwidth adjustment settings can be
> + added in future if it gets to be necessary.
> +
> + The PLLs CLKOUT is then either directly connected with the corresponding
> + clocks consumer (like P5600 cores or DDR controller) or passed over a CCU
> + divider to create a signal required for the clock domain.
> +
> + The CCU PLL dts-node uses the common clock bindings [1] with no custom
> + parameters. The list of exported clocks can be found in
> + 'dt-bindings/clock/bt1-ccu.h'.
> +
> + [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
Don't think we need to mention this binding anymore. But it's good that
we know what exported clock ids are.
> +
> +allOf:
> + - $ref: /schemas/clock/clock.yaml#
> +
> +properties:
> + compatible:
> + const: be,bt1-ccu-pll
> +
> + reg:
> + description: CCU PLLs sub-block base address.
> + maxItems: 1
> +
> + "#clock-cells":
> + description: |
> + Clocks are referenced by the node phandle and an unique identifier
> + from 'dt-bindings/clock/bt1-ccu.h'.
Don't think we need this description.
> + const: 1
> +
> + clocks:
> + description: Phandle of CCU External reference clock.
> + maxItems: 1
> +
> + clock-names:
> + const: ref_clk
Can we drop _clk? It's redundant.
> +
> + clock-output-names: true
> +
> + assigned-clocks: true
> +
> + assigned-clock-rates: true
> +
> +additionalProperties: false
> +
> +required:
> + - compatible
> + - reg
> + - "#clock-cells"
> + - clocks
> + - clock-names
> +
> +examples:
> + - |
> + ccu_pll: ccu_pll@...4D000 {
Drop the phandle unless it's actually used.
> + compatible = "be,bt1-ccu-pll";
> + reg = <0x1F04D000 0x028>;
Lowercase hex please. That size is oddly small.
> + #clock-cells = <1>;
> +
> + clocks = <&osc25>;
> + clock-names = "ref_clk";
> +
> + clock-output-names = "cpu_pll", "sata_pll", "ddr_pll",
> + "pcie_pll", "eth_pll";
> + };
> +...
> diff --git a/include/dt-bindings/clock/bt1-ccu.h b/include/dt-bindings/clock/bt1-ccu.h
> new file mode 100644
> index 000000000000..86e63162ade0
> --- /dev/null
> +++ b/include/dt-bindings/clock/bt1-ccu.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2019 BAIKAL ELECTRONICS, JSC
> + *
> + * Baikal-T1 CCU clock indeces.
> + */
> +#ifndef __DT_BINDINGS_CLOCK_BT1_CCU_H
> +#define __DT_BINDINGS_CLOCK_BT1_CCU_H
> +
> +/* Baikal-T1 CCU PLL indeces. */
Please drop this comment. It's not useful.
> +#define CCU_CPU_PLL 0
> +#define CCU_SATA_PLL 1
> +#define CCU_DDR_PLL 2
> +#define CCU_PCIE_PLL 3
> +#define CCU_ETH_PLL 4
> +
> +#endif /* __DT_BINDINGS_CLOCK_BT1_CCU_H */
> --
> 2.25.1
>
Powered by blists - more mailing lists