| 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: <f472ab62-a248-4753-a2cf-a8b85ae88638@gmail.com>
Date: Mon, 21 Oct 2024 17:59:43 +0300
From: Ivaylo Ivanov <ivo.ivanov.ivanov1@...il.com>
To: Krzysztof Kozlowski <krzk@...nel.org>,
Sylwester Nawrocki <s.nawrocki@...sung.com>,
Chanwoo Choi <cw00.choi@...sung.com>, Alim Akhtar <alim.akhtar@...sung.com>,
Michael Turquette <mturquette@...libre.com>, Stephen Boyd
<sboyd@...nel.org>, Rob Herring <robh@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Tomasz Figa <tomasz.figa@...il.com>
Cc: linux-samsung-soc@...r.kernel.org, linux-clk@...r.kernel.org,
devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 1/3] dt-bindings: clock: Add Exynos8895 SoC CMU
bindings
On 10/21/24 12:38, Krzysztof Kozlowski wrote:
> On 20/10/2024 19:48, Ivaylo Ivanov wrote:
>> Provide dt-schema documentation for Exynos8895 SoC clock controller.
>> Add device tree clock binding definitions for the following CMU blocks:
>> - CMU_TOP
>> - CMU_FSYS0/1
>> - CMU_PERIC0/1
>> - CMU_PERIS
>>
>> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@...il.com>
>
> A nit, subject: drop second/last, redundant "bindings". The
> "dt-bindings" prefix is already stating that these are bindings.
> See also:
> https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18
>
>> +
>> +title: Samsung Exynos8895 SoC clock controller
>> +
>> +maintainers:
>> + - Ivaylo Ivanov <ivo.ivanov.ivanov1@...il.com>
>> + - Chanwoo Choi <cw00.choi@...sung.com>
>> + - Krzysztof Kozlowski <krzk@...nel.org>
>> + - Sylwester Nawrocki <s.nawrocki@...sung.com>
>> + - Tomasz Figa <tomasz.figa@...il.com>
> Please drop Sylwester and Tomasz, they opted out from clocks.
>
>> +
>> +description: |
>> + Exynos8895 clock controller is comprised of several CMU units, generating
>> + clocks for different domains. Those CMU units are modeled as separate device
>> + tree nodes, and might depend on each other. The root clock in that root tree
>> + is an external clock: OSCCLK (26 MHz). This external clock must be defined
>> + as a fixed-rate clock in dts.
>> +
>> + CMU_TOP is a top-level CMU, where all base clocks are prepared using PLLs and
>> + dividers; all other clocks of function blocks (other CMUs) are usually
>> + derived from CMU_TOP.
>> +
>> + Each clock is assigned an identifier and client nodes can use this identifier
>> + to specify the clock which they consume. All clocks available for usage
>> + in clock consumer nodes are defined as preprocessor macros in
>> + 'include/dt-bindings/clock/samsung,exynos8895.h' header.
>> +
>> +properties:
>> + compatible:
>> + enum:
>> + - samsung,exynos8895-cmu-top
>> + - samsung,exynos8895-cmu-fsys0
>> + - samsung,exynos8895-cmu-fsys1
>> + - samsung,exynos8895-cmu-peric0
>> + - samsung,exynos8895-cmu-peric1
>> + - samsung,exynos8895-cmu-peris
> Alphabetical order.
>
>> +
>> + clocks:
>> + minItems: 1
>> + maxItems: 16
>> +
>> + clock-names:
>> + minItems: 1
>> + maxItems: 16
>> +
>> + "#clock-cells":
>> + const: 1
>> +
>> + reg:
>> + maxItems: 1
>> +
> required: block should go here (I know that other Samsung clock bindings
> do not follow this convention).
>
>> +allOf:
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + const: samsung,exynos8895-cmu-top
>> +
>> + then:
>> + properties:
>> + clocks:
>> + items:
>> + - description: External reference clock (26 MHz)
>> +
>> + clock-names:
>> + items:
>> + - const: oscclk
>> +
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + const: samsung,exynos8895-cmu-fsys0
>> +
>> + then:
>> + properties:
>> + clocks:
>> + items:
>> + - description: External reference clock (26 MHz)
>> + - description: CMU_FSYS0 BUS clock (from CMU_TOP)
>> + - description: CMU_FSYS0 DPGTC clock (from CMU_TOP)
>> + - description: CMU_FSYS0 MMC_EMBD clock (from CMU_TOP)
>> + - description: CMU_FSYS0 UFS_EMBD clock (from CMU_TOP)
>> + - description: CMU_FSYS0 USBDRD30 clock (from CMU_TOP)
>> +
>> + clock-names:
>> + items:
>> + - const: oscclk
>> + - const: bus
>> + - const: dpgtc
>> + - const: mmc_embd
> mmc
>
>> + - const: ufs_embd
> ufs
>
>> + - const: usbdrd30
>> +
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + const: samsung,exynos8895-cmu-fsys1
>> +
>> + then:
>> + properties:
>> + clocks:
>> + items:
>> + - description: External reference clock (26 MHz)
>> + - description: CMU_FSYS1 BUS clock (from CMU_TOP)
>> + - description: CMU_FSYS1 MMC_CARD clock (from CMU_TOP)
>> + - description: CMU_FSYS1 PCIE clock (from CMU_TOP)
>> + - description: CMU_FSYS1 UFS_CARD clock (from CMU_TOP)
>> +
>> + clock-names:
>> + items:
>> + - const: oscclk
>> + - const: bus
>> + - const: mmc_card
> mmc
> Although now I wonder, why this is different FSYS. Is it for different
> mmc controller?
FSYS0 provides clocks for embedded storages (UFS and eMMC), whereas FSYS1
clocks external storage cards (UFS and MMC cards). As far as I can tell, there's
only one MMC controller, so perhaps it could be set to drive an eMMC or an SD
card. On retail devices, UFS_EMBD and MMC_CARD are used.
Thanks for the review, will fix the issues in the next patchset.
Best regards, Ivo.
>
>> + - const: pcie
>> + - const: ufs_card
> ufs
>
> Keep the order as in GS101 file.
>
>> +
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + const: samsung,exynos8895-cmu-peric0
>> +
>> + then:
>> + properties:
>> + clocks:
>> + items:
>> + - description: External reference clock (26 MHz)
>> + - description: CMU_PERIC0 BUS clock (from CMU_TOP)
>> + - description: CMU_PERIC0 UART_DBG clock (from CMU_TOP)
>> + - description: CMU_PERIC0 USI00 clock (from CMU_TOP)
>> + - description: CMU_PERIC0 USI01 clock (from CMU_TOP)
>> + - description: CMU_PERIC0 USI02 clock (from CMU_TOP)
>> + - description: CMU_PERIC0 USI03 clock (from CMU_TOP)
>> +
>> + clock-names:
>> + items:
>> + - const: oscclk
>> + - const: bus
>> + - const: uart_dbg
> uart
>
>> + - const: usi00
> usi0
>
>> + - const: usi01
> usi1
>
>> + - const: usi02
> usi2
>
>> + - const: usi03
> usi3
>
>> +
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + const: samsung,exynos8895-cmu-peric1
>> +
>> + then:
>> + properties:
>> + clocks:
>> + items:
>> + - description: External reference clock (26 MHz)
>> + - description: CMU_PERIC1 BUS clock (from CMU_TOP)
>> + - description: CMU_PERIC1 SPEEDY2 clock (from CMU_TOP)
>> + - description: CMU_PERIC1 SPI_CAM0 clock (from CMU_TOP)
>> + - description: CMU_PERIC1 SPI_CAM1 clock (from CMU_TOP)
>> + - description: CMU_PERIC1 UART_BT clock (from CMU_TOP)
>> + - description: CMU_PERIC1 USI04 clock (from CMU_TOP)
>> + - description: CMU_PERIC1 USI05 clock (from CMU_TOP)
>> + - description: CMU_PERIC1 USI06 clock (from CMU_TOP)
>> + - description: CMU_PERIC1 USI07 clock (from CMU_TOP)
>> + - description: CMU_PERIC1 USI08 clock (from CMU_TOP)
>> + - description: CMU_PERIC1 USI09 clock (from CMU_TOP)
>> + - description: CMU_PERIC1 USI10 clock (from CMU_TOP)
>> + - description: CMU_PERIC1 USI11 clock (from CMU_TOP)
>> + - description: CMU_PERIC1 USI12 clock (from CMU_TOP)
>> + - description: CMU_PERIC1 USI13 clock (from CMU_TOP)
>> +
>> + clock-names:
>> + items:
>> + - const: oscclk
>> + - const: bus
>> + - const: speedy2
> speedy
>
>> + - const: cam0
>> + - const: cam1
>> + - const: uart_bt
> uart
>
>> + - const: usi04
> usi4, etc
>
>> + - const: usi05
>> + - const: usi06
>> + - const: usi07
>> + - const: usi08
>> + - const: usi09
>> + - const: usi10
>> + - const: usi11
>> + - const: usi12
>> + - const: usi13
>> +
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + const: samsung,exynos8895-cmu-peris
>> +
>> + then:
>> + properties:
>> + clocks:
>> + items:
>> + - description: External reference clock (26 MHz)
>> + - description: CMU_PERIS BUS clock (from CMU_TOP)
>> +
>> + clock-names:
>> + items:
>> + - const: oscclk
>> + - const: bus
>> +
>> +required:
>> + - compatible
>> + - "#clock-cells"
>> + - clocks
>> + - clock-names
>> + - reg
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + # Clock controller node for CMU_FSYS1
>> + - |
>> + #include <dt-bindings/clock/samsung,exynos8895.h>
>> +
>> + cmu_fsys1: clock-controller@...00000 {
>> + compatible = "samsung,exynos8895-cmu-fsys1";
>> + reg = <0x11400000 0x8000>;
>> + #clock-cells = <1>;
>> +
>> + clocks = <&oscclk>,
>> + <&cmu_top CLK_DOUT_CMU_FSYS1_BUS>,
>> + <&cmu_top CLK_DOUT_CMU_FSYS1_MMC_CARD>,
>> + <&cmu_top CLK_DOUT_CMU_FSYS1_PCIE>,
>> + <&cmu_top CLK_DOUT_CMU_FSYS1_UFS_CARD>;
>> + clock-names = "oscclk", "bus", "mmc_card",
>> + "pcie", "ufs_card";
>> + };
>> +
>
> Best regards,
> Krzysztof
>
Powered by blists - more mailing lists