| 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: <7vszdea2djl43oojvw3vlrip23f7cfyxkyn6jw3wc2f7yowht5@bgsc2pqscujc>
Date: Sun, 11 May 2025 17:48:11 -0500
From: Bjorn Andersson <andersson@...nel.org>
To: Stephan Gerhold <stephan.gerhold@...aro.org>
Cc: Jassi Brar <jassisinghbrar@...il.com>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
Michael Turquette <mturquette@...libre.com>, Stephen Boyd <sboyd@...nel.org>, linux-arm-msm@...r.kernel.org,
linux-kernel@...r.kernel.org, devicetree@...r.kernel.org, linux-clk@...r.kernel.org,
Georgi Djakov <djakov@...nel.org>, Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
Subject: Re: [PATCH 1/4] dt-bindings: mailbox: qcom,apcs: Add separate node
for clock-controller
On Tue, May 06, 2025 at 03:10:08PM +0200, Stephan Gerhold wrote:
> APCS "global" is sort of a "miscellaneous" hardware block that combines
> multiple registers inside the application processor subsystem. Two distinct
> use cases are currently stuffed together in a single device tree node:
>
> - Mailbox: to communicate with other remoteprocs in the system.
> - Clock: for controlling the CPU frequency.
>
> These two use cases have unavoidable circular dependencies: the mailbox is
> needed as early as possible during boot to start controlling shared
> resources like clocks and power domains, while the clock controller needs
> one of these shared clocks as its parent. Currently, there is no way to
> distinguish these two use cases for generic mechanisms like fw_devlink.
>
> This is currently blocking conversion of the deprecated custom "qcom,ipc"
> properties to the standard "mboxes", see e.g. commit d92e9ea2f0f9
> ("arm64: dts: qcom: msm8939: revert use of APCS mbox for RPM"):
> 1. remoteproc &rpm needs mboxes = <&apcs1_mbox 8>;
> 2. The clock controller inside &apcs1_mbox needs
> clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>.
> 3. &rpmcc is a child of remoteproc &rpm
>
> The mailbox itself does not need any clocks and should probe early to
> unblock the rest of the boot process. The "clocks" are only needed for the
> separate clock controller. In Linux, these are already two separate drivers
> that can probe independently.
>
Why does this circular dependency need to be broken in the DeviceTree
representation?
As you describe, the mailbox probes and register the mailbox controller
and it registers the clock controller. The mailbox device isn't affected
by the clock controller failing to find rpmcc...
Regards,
Bjorn
> Break up the circular dependency chain in the device tree by separating the
> clock controller into a separate child node. Deprecate the old approach of
> specifying the clock properties as part of the root node, but keep them for
> backwards compatibility.
>
> Signed-off-by: Stephan Gerhold <stephan.gerhold@...aro.org>
> ---
> .../bindings/mailbox/qcom,apcs-kpss-global.yaml | 169 ++++++++++++++-------
> 1 file changed, 118 insertions(+), 51 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
> index a58a018f3f7b9f8edd70d7c1bd137844ff2549df..3a0a304bb65a68b2d4a1df79b3243ddac6bf88b2 100644
> --- a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
> +++ b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
> @@ -72,6 +72,7 @@ properties:
> description: phandles to the parent clocks of the clock driver
> minItems: 2
> maxItems: 3
> + deprecated: true
>
> '#mbox-cells':
> const: 1
> @@ -82,6 +83,23 @@ properties:
> clock-names:
> minItems: 2
> maxItems: 3
> + deprecated: true
> +
> + clock-controller:
> + type: object
> + additionalProperties: false
> + properties:
> + clocks:
> + description: phandles to the parent clocks of the clock driver
> + minItems: 2
> + maxItems: 3
> +
> + '#clock-cells':
> + enum: [0, 1]
> +
> + clock-names:
> + minItems: 2
> + maxItems: 3
>
> required:
> - compatible
> @@ -90,6 +108,76 @@ required:
>
> additionalProperties: false
>
> +# Clocks should be specified either on the parent node or on the child node
> +oneOf:
> + - required:
> + - clock-controller
> + properties:
> + clocks: false
> + clock-names: false
> + '#clock-cells': false
> + - properties:
> + clock-controller: false
> +
> +$defs:
> + msm8916-apcs-clock-controller:
> + properties:
> + clocks:
> + items:
> + - description: primary pll parent of the clock driver
> + - description: auxiliary parent
> + clock-names:
> + items:
> + - const: pll
> + - const: aux
> + '#clock-cells':
> + const: 0
> +
> + msm8939-apcs-clock-controller:
> + properties:
> + clocks:
> + items:
> + - description: primary pll parent of the clock driver
> + - description: auxiliary parent
> + - description: reference clock
> + clock-names:
> + items:
> + - const: pll
> + - const: aux
> + - const: ref
> + '#clock-cells':
> + const: 0
> +
> + sdx55-apcs-clock-controller:
> + properties:
> + clocks:
> + items:
> + - description: reference clock
> + - description: primary pll parent of the clock driver
> + - description: auxiliary parent
> + clock-names:
> + items:
> + - const: ref
> + - const: pll
> + - const: aux
> + '#clock-cells':
> + const: 0
> +
> + ipq6018-apcs-clock-controller:
> + properties:
> + clocks:
> + items:
> + - description: primary pll parent of the clock driver
> + - description: XO clock
> + - description: GCC GPLL0 clock source
> + clock-names:
> + items:
> + - const: pll
> + - const: xo
> + - const: gpll0
> + '#clock-cells':
> + const: 1
> +
> allOf:
> - if:
> properties:
> @@ -98,15 +186,10 @@ allOf:
> enum:
> - qcom,msm8916-apcs-kpss-global
> then:
> + $ref: "#/$defs/msm8916-apcs-clock-controller"
> properties:
> - clocks:
> - items:
> - - description: primary pll parent of the clock driver
> - - description: auxiliary parent
> - clock-names:
> - items:
> - - const: pll
> - - const: aux
> + clock-controller:
> + $ref: "#/$defs/msm8916-apcs-clock-controller"
>
> - if:
> properties:
> @@ -115,17 +198,10 @@ allOf:
> enum:
> - qcom,msm8939-apcs-kpss-global
> then:
> + $ref: "#/$defs/msm8939-apcs-clock-controller"
> properties:
> - clocks:
> - items:
> - - description: primary pll parent of the clock driver
> - - description: auxiliary parent
> - - description: reference clock
> - clock-names:
> - items:
> - - const: pll
> - - const: aux
> - - const: ref
> + clock-controller:
> + $ref: "#/$defs/msm8939-apcs-clock-controller"
>
> - if:
> properties:
> @@ -134,17 +210,10 @@ allOf:
> enum:
> - qcom,sdx55-apcs-gcc
> then:
> + $ref: "#/$defs/sdx55-apcs-clock-controller"
> properties:
> - clocks:
> - items:
> - - description: reference clock
> - - description: primary pll parent of the clock driver
> - - description: auxiliary parent
> - clock-names:
> - items:
> - - const: ref
> - - const: pll
> - - const: aux
> + clock-controller:
> + $ref: "#/$defs/sdx55-apcs-clock-controller"
>
> - if:
> properties:
> @@ -153,17 +222,10 @@ allOf:
> enum:
> - qcom,ipq6018-apcs-apps-global
> then:
> + $ref: "#/$defs/ipq6018-apcs-clock-controller"
> properties:
> - clocks:
> - items:
> - - description: primary pll parent of the clock driver
> - - description: XO clock
> - - description: GCC GPLL0 clock source
> - clock-names:
> - items:
> - - const: pll
> - - const: xo
> - - const: gpll0
> + clock-controller:
> + $ref: "#/$defs/ipq6018-apcs-clock-controller"
>
> - if:
> properties:
> @@ -179,19 +241,7 @@ allOf:
> properties:
> clocks: false
> clock-names: false
> -
> - - if:
> - properties:
> - compatible:
> - contains:
> - enum:
> - - qcom,ipq6018-apcs-apps-global
> - then:
> - properties:
> - '#clock-cells':
> - const: 1
> - else:
> - properties:
> + clock-controller: false
> '#clock-cells':
> const: 0
>
> @@ -216,6 +266,23 @@ examples:
> };
>
> # Example apcs with qcs404
> + - |
> + #define GCC_APSS_AHB_CLK_SRC 1
> + #define GCC_GPLL0_AO_OUT_MAIN 123
> + mailbox@...1000 {
> + compatible = "qcom,qcs404-apcs-apps-global",
> + "qcom,msm8916-apcs-kpss-global", "syscon";
> + reg = <0x0b011000 0x1000>;
> + #mbox-cells = <1>;
> +
> + apcs_clk: clock-controller {
> + clocks = <&apcs_hfpll>, <&gcc GCC_GPLL0_AO_OUT_MAIN>;
> + clock-names = "pll", "aux";
> + #clock-cells = <0>;
> + };
> + };
> +
> + # Example apcs with qcs404 (deprecated: use clock-controller subnode)
> - |
> #define GCC_APSS_AHB_CLK_SRC 1
> #define GCC_GPLL0_AO_OUT_MAIN 123
>
> --
> 2.47.2
>
Powered by blists - more mailing lists