From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
The SM8750 features a "traditional" GPU_CC block, much of which is
controlled through the GMU microcontroller. Additionally, there's
an separate GX_CC block, where the GX GDSC is moved.
Add bindings to accommodate for that.
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
.../bindings/clock/qcom,sm8450-gpucc.yaml | 5 ++
.../bindings/clock/qcom,sm8750-gxcc.yaml | 61 ++++++++++++++++++++++
include/dt-bindings/clock/qcom,sm8750-gpucc.h | 53 +++++++++++++++++++
3 files changed, 119 insertions(+)
diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8450-gpucc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8450-gpucc.yaml
index 02968632fb3af34d6b3983a6a24aa742db1d59b1..d1b3557ab344b071d16dba4d5c6a267b7ab70573 100644
--- a/Documentation/devicetree/bindings/clock/qcom,sm8450-gpucc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,sm8450-gpucc.yaml
@@ -20,6 +20,7 @@ description: |
include/dt-bindings/clock/qcom,sm8550-gpucc.h
include/dt-bindings/reset/qcom,sm8450-gpucc.h
include/dt-bindings/reset/qcom,sm8650-gpucc.h
+ include/dt-bindings/reset/qcom,sm8750-gpucc.h
include/dt-bindings/reset/qcom,x1e80100-gpucc.h
properties:
@@ -31,6 +32,7 @@ properties:
- qcom,sm8475-gpucc
- qcom,sm8550-gpucc
- qcom,sm8650-gpucc
+ - qcom,sm8750-gpucc
- qcom,x1e80100-gpucc
- qcom,x1p42100-gpucc
@@ -40,6 +42,9 @@ properties:
- description: GPLL0 main branch source
- description: GPLL0 div branch source
+ power-domains:
+ maxItems: 1
+
required:
- compatible
- clocks
diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8750-gxcc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8750-gxcc.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..b900c19156f5a2ba4e0f7c95276c771f615fdf23
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/qcom,sm8750-gxcc.yaml
@@ -0,0 +1,61 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/qcom,sm8750-gxcc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Graphics Clock & Reset Controller on SM8750
+
+maintainers:
+ - Konrad Dybcio <konradybcio@kernel.org>
+
+description: |
+ Qualcomm graphics clock control module provides the clocks, resets and power
+ domains on Qualcomm SoCs.
+
+ See also:
+ include/dt-bindings/reset/qcom,sm8750-gpucc.h
+
+properties:
+ compatible:
+ enum:
+ - qcom,sm8750-gxcc
+
+ reg:
+ maxItems: 1
+
+ power-domains:
+ items:
+ - description: GFX voltage rail
+ - description: MX_COLLAPSIBLE voltage rail
+ - description: GPU_CC_CX GDSC
+
+ '#power-domain-cells':
+ const: 1
+
+required:
+ - compatible
+ - power-domains
+ - '#power-domain-cells'
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/qcom,sm8750-gpucc.h>
+ #include <dt-bindings/power/qcom,rpmhpd.h>
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ clock-controller@3d64000 {
+ compatible = "qcom,sm8750-gxcc";
+ reg = <0x0 0x03d64000 0x0 0x6000>;
+ power-domains = <&rpmhpd RPMHPD_GFX>,
+ <&rpmhpd RPMHPD_MXC>,
+ <&gpucc GPU_CC_CX_GDSC>;
+ #power-domain-cells = <1>;
+ };
+ };
+...
diff --git a/include/dt-bindings/clock/qcom,sm8750-gpucc.h b/include/dt-bindings/clock/qcom,sm8750-gpucc.h
new file mode 100644
index 0000000000000000000000000000000000000000..98e2f5df78740bf298c6b1065972d7e58ee81713
--- /dev/null
+++ b/include/dt-bindings/clock/qcom,sm8750-gpucc.h
@@ -0,0 +1,53 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+#ifndef _DT_BINDINGS_CLK_QCOM_GPU_CC_SM8750_H
+#define _DT_BINDINGS_CLK_QCOM_GPU_CC_SM8750_H
+
+/* GPU_CC clocks */
+#define GPU_CC_AHB_CLK 0
+#define GPU_CC_CB_CLK 1
+#define GPU_CC_CX_ACCU_SHIFT_CLK 2
+#define GPU_CC_CX_FF_CLK 3
+#define GPU_CC_CX_GMU_CLK 4
+#define GPU_CC_CXO_AON_CLK 5
+#define GPU_CC_CXO_CLK 6
+#define GPU_CC_DEMET_CLK 7
+#define GPU_CC_DPM_CLK 8
+#define GPU_CC_FF_CLK_SRC 9
+#define GPU_CC_FREQ_MEASURE_CLK 10
+#define GPU_CC_GMU_CLK_SRC 11
+#define GPU_CC_GX_ACCU_SHIFT_CLK 12
+#define GPU_CC_GX_ACD_AHB_FF_CLK 13
+#define GPU_CC_GX_AHB_FF_CLK 14
+#define GPU_CC_GX_GMU_CLK 15
+#define GPU_CC_GX_RCG_AHB_FF_CLK 16
+#define GPU_CC_HLOS1_VOTE_GPU_SMMU_CLK 17
+#define GPU_CC_HUB_AON_CLK 18
+#define GPU_CC_HUB_CLK_SRC 19
+#define GPU_CC_HUB_CX_INT_CLK 20
+#define GPU_CC_HUB_DIV_CLK_SRC 21
+#define GPU_CC_MEMNOC_GFX_CLK 22
+#define GPU_CC_PLL0 23
+#define GPU_CC_PLL0_OUT_EVEN 24
+#define GPU_CC_RSCC_HUB_AON_CLK 25
+#define GPU_CC_RSCC_XO_AON_CLK 26
+#define GPU_CC_SLEEP_CLK 27
+
+/* GPU_CC power domains */
+#define GPU_CC_CX_GDSC 0
+
+/* GPU_CC resets */
+#define GPU_CC_GPU_CC_CB_BCR 0
+#define GPU_CC_GPU_CC_CX_BCR 1
+#define GPU_CC_GPU_CC_FAST_HUB_BCR 2
+#define GPU_CC_GPU_CC_FF_BCR 3
+#define GPU_CC_GPU_CC_GMU_BCR 4
+#define GPU_CC_GPU_CC_GX_BCR 5
+#define GPU_CC_GPU_CC_XO_BCR 6
+
+/* GX_CC power domains */
+#define GX_CC_GX_GDSC 0
+
+#endif
--
2.50.1On 23/07/2025 22:38, Konrad Dybcio wrote: > + }; > +... > diff --git a/include/dt-bindings/clock/qcom,sm8750-gpucc.h b/include/dt-bindings/clock/qcom,sm8750-gpucc.h > new file mode 100644 > index 0000000000000000000000000000000000000000..98e2f5df78740bf298c6b1065972d7e58ee81713 > --- /dev/null > +++ b/include/dt-bindings/clock/qcom,sm8750-gpucc.h > @@ -0,0 +1,53 @@ > +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ > +/* > + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries. This needs fixes... it's some sudden move of removal of copyright dates everywhere across Qualcomm. Please don't use internal rules outside of Qualcomm projects. Best regards, Krzysztof
On Wed, Jul 23, 2025 at 10:38:48PM +0200, Konrad Dybcio wrote:
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>
> The SM8750 features a "traditional" GPU_CC block, much of which is
> controlled through the GMU microcontroller. Additionally, there's
> an separate GX_CC block, where the GX GDSC is moved.
>
> Add bindings to accommodate for that.
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> ---
> .../bindings/clock/qcom,sm8450-gpucc.yaml | 5 ++
> .../bindings/clock/qcom,sm8750-gxcc.yaml | 61 ++++++++++++++++++++++
> include/dt-bindings/clock/qcom,sm8750-gpucc.h | 53 +++++++++++++++++++
> 3 files changed, 119 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8450-gpucc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8450-gpucc.yaml
> index 02968632fb3af34d6b3983a6a24aa742db1d59b1..d1b3557ab344b071d16dba4d5c6a267b7ab70573 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,sm8450-gpucc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,sm8450-gpucc.yaml
> @@ -20,6 +20,7 @@ description: |
> include/dt-bindings/clock/qcom,sm8550-gpucc.h
> include/dt-bindings/reset/qcom,sm8450-gpucc.h
> include/dt-bindings/reset/qcom,sm8650-gpucc.h
> + include/dt-bindings/reset/qcom,sm8750-gpucc.h
> include/dt-bindings/reset/qcom,x1e80100-gpucc.h
>
> properties:
> @@ -31,6 +32,7 @@ properties:
> - qcom,sm8475-gpucc
> - qcom,sm8550-gpucc
> - qcom,sm8650-gpucc
> + - qcom,sm8750-gpucc
> - qcom,x1e80100-gpucc
> - qcom,x1p42100-gpucc
>
> @@ -40,6 +42,9 @@ properties:
> - description: GPLL0 main branch source
> - description: GPLL0 div branch source
>
> + power-domains:
> + maxItems: 1
This should be a different binding or you need to restrict other
variants here.
> +
> required:
> - compatible
> - clocks
> diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8750-gxcc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8750-gxcc.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..b900c19156f5a2ba4e0f7c95276c771f615fdf23
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/qcom,sm8750-gxcc.yaml
There is nothing for clocks in the binding. Place power domain providers
in their directory.
> @@ -0,0 +1,61 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/qcom,sm8750-gxcc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Graphics Clock & Reset Controller on SM8750
There is no clocks nor resets here. Only power domains.
> +
> +maintainers:
> + - Konrad Dybcio <konradybcio@kernel.org>
> +
> +description: |
> + Qualcomm graphics clock control module provides the clocks, resets and power
Also confusing.
> + domains on Qualcomm SoCs.
> +
> + See also:
> + include/dt-bindings/reset/qcom,sm8750-gpucc.h
reset or clock path?
> +
> +properties:
> + compatible:
> + enum:
> + - qcom,sm8750-gxcc
> +
> + reg:
> + maxItems: 1
> +
> + power-domains:
> + items:
> + - description: GFX voltage rail
> + - description: MX_COLLAPSIBLE voltage rail
> + - description: GPU_CC_CX GDSC
> +
> + '#power-domain-cells':
> + const: 1
> +
> +required:
> + - compatible
> + - power-domains
> + - '#power-domain-cells'
> +
You miss ref... or this is a bit confusing.
> +unevaluatedProperties: false
additionalProperties instead
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/qcom,sm8750-gpucc.h>
> + #include <dt-bindings/power/qcom,rpmhpd.h>
> +
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + clock-controller@3d64000 {
No, clock controllers have clock-cells. This cannot be a clock
controller if it does not have any clocks for anyone to use.
Best regards,
Krzysztof
On 7/24/25 10:18 AM, Krzysztof Kozlowski wrote:
> On Wed, Jul 23, 2025 at 10:38:48PM +0200, Konrad Dybcio wrote:
>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>
>> The SM8750 features a "traditional" GPU_CC block, much of which is
>> controlled through the GMU microcontroller. Additionally, there's
>> an separate GX_CC block, where the GX GDSC is moved.
>>
>> Add bindings to accommodate for that.
>>
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>> ---
>> .../bindings/clock/qcom,sm8450-gpucc.yaml | 5 ++
>> .../bindings/clock/qcom,sm8750-gxcc.yaml | 61 ++++++++++++++++++++++
>> include/dt-bindings/clock/qcom,sm8750-gpucc.h | 53 +++++++++++++++++++
>> 3 files changed, 119 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8450-gpucc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8450-gpucc.yaml
>> index 02968632fb3af34d6b3983a6a24aa742db1d59b1..d1b3557ab344b071d16dba4d5c6a267b7ab70573 100644
>> --- a/Documentation/devicetree/bindings/clock/qcom,sm8450-gpucc.yaml
>> +++ b/Documentation/devicetree/bindings/clock/qcom,sm8450-gpucc.yaml
>> @@ -20,6 +20,7 @@ description: |
>> include/dt-bindings/clock/qcom,sm8550-gpucc.h
>> include/dt-bindings/reset/qcom,sm8450-gpucc.h
>> include/dt-bindings/reset/qcom,sm8650-gpucc.h
>> + include/dt-bindings/reset/qcom,sm8750-gpucc.h
>> include/dt-bindings/reset/qcom,x1e80100-gpucc.h
>>
>> properties:
>> @@ -31,6 +32,7 @@ properties:
>> - qcom,sm8475-gpucc
>> - qcom,sm8550-gpucc
>> - qcom,sm8650-gpucc
>> + - qcom,sm8750-gpucc
>> - qcom,x1e80100-gpucc
>> - qcom,x1p42100-gpucc
>>
>> @@ -40,6 +42,9 @@ properties:
>> - description: GPLL0 main branch source
>> - description: GPLL0 div branch source
>>
>> + power-domains:
>> + maxItems: 1
>
> This should be a different binding or you need to restrict other
> variants here.
Actually looks like this is the same case as the recent videocc changes
(15 year old technical debt catching up to us..)
I'll send a mass-fixup for this.
Some platforms require 2 and some require 3 entries here. Do I have to
restrict them very specifically, or can I do:
power-domains:
description:
Power domains required for the clock controller to operate
minItems: 2
items:
- description: CX power domain
- description: MX power domain
- description: MXC power domain
?
Konrad
On 25/07/2025 11:30, Konrad Dybcio wrote: >>> >>> @@ -40,6 +42,9 @@ properties: >>> - description: GPLL0 main branch source >>> - description: GPLL0 div branch source >>> >>> + power-domains: >>> + maxItems: 1 >> >> This should be a different binding or you need to restrict other >> variants here. > > Actually looks like this is the same case as the recent videocc changes > (15 year old technical debt catching up to us..) > > I'll send a mass-fixup for this. > > Some platforms require 2 and some require 3 entries here. Do I have to > restrict them very specifically, or can I do: > > power-domains: > description: > Power domains required for the clock controller to operate > minItems: 2 > items: > - description: CX power domain > - description: MX power domain > - description: MXC power domain > > ? This is correct and should be in top level, but you still need to restrict them per each variant (minItems: 3 or maxItems: 2). > > Konrad Best regards, Krzysztof
On 7/28/25 7:01 AM, Krzysztof Kozlowski wrote: > On 25/07/2025 11:30, Konrad Dybcio wrote: >>>> >>>> @@ -40,6 +42,9 @@ properties: >>>> - description: GPLL0 main branch source >>>> - description: GPLL0 div branch source >>>> >>>> + power-domains: >>>> + maxItems: 1 >>> >>> This should be a different binding or you need to restrict other >>> variants here. >> >> Actually looks like this is the same case as the recent videocc changes >> (15 year old technical debt catching up to us..) >> >> I'll send a mass-fixup for this. >> >> Some platforms require 2 and some require 3 entries here. Do I have to >> restrict them very specifically, or can I do: >> >> power-domains: >> description: >> Power domains required for the clock controller to operate >> minItems: 2 >> items: >> - description: CX power domain >> - description: MX power domain >> - description: MXC power domain >> >> ? > > This is correct and should be in top level, but you still need to > restrict them per each variant (minItems: 3 or maxItems: 2). So I was happy about how simple it was, until I realized we also need to poke the VDD_GFX domain. It does however not necessarily exist on all platforms and I don't want the binding to become a spaghetti of ifs.. CX & MX is present on all(?) platforms GFX & MXC's (any combination of those, unfortunately) presence varies Is there anything better I can do than creating a separate case for: * CX_MX * CX_MX_GFX * CX_MX_MXC * CX_MX_GFX_MXC ? Konrad
On 7/28/25 1:02 PM, Konrad Dybcio wrote: > On 7/28/25 7:01 AM, Krzysztof Kozlowski wrote: >> On 25/07/2025 11:30, Konrad Dybcio wrote: >>>>> >>>>> @@ -40,6 +42,9 @@ properties: >>>>> - description: GPLL0 main branch source >>>>> - description: GPLL0 div branch source >>>>> >>>>> + power-domains: >>>>> + maxItems: 1 >>>> >>>> This should be a different binding or you need to restrict other >>>> variants here. >>> >>> Actually looks like this is the same case as the recent videocc changes >>> (15 year old technical debt catching up to us..) >>> >>> I'll send a mass-fixup for this. >>> >>> Some platforms require 2 and some require 3 entries here. Do I have to >>> restrict them very specifically, or can I do: >>> >>> power-domains: >>> description: >>> Power domains required for the clock controller to operate >>> minItems: 2 >>> items: >>> - description: CX power domain >>> - description: MX power domain >>> - description: MXC power domain >>> >>> ? >> >> This is correct and should be in top level, but you still need to >> restrict them per each variant (minItems: 3 or maxItems: 2). > > So I was happy about how simple it was, until I realized we also need > to poke the VDD_GFX domain. It does however not necessarily exist on > all platforms and I don't want the binding to become a spaghetti of ifs.. > > CX & MX is present on all(?) platforms > GFX & MXC's (any combination of those, unfortunately) presence varies > > Is there anything better I can do than creating a separate case for: > > * CX_MX > * CX_MX_GFX > * CX_MX_MXC > * CX_MX_GFX_MXC Doesn't seem like it, turned out this wasn't as terrible a mess as I had imagined.. Konrad
On 7/24/25 10:18 AM, Krzysztof Kozlowski wrote:
> On Wed, Jul 23, 2025 at 10:38:48PM +0200, Konrad Dybcio wrote:
>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>
>> The SM8750 features a "traditional" GPU_CC block, much of which is
>> controlled through the GMU microcontroller. Additionally, there's
>> an separate GX_CC block, where the GX GDSC is moved.
>>
>> Add bindings to accommodate for that.
>>
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>> ---
[...]
>> +title: Qualcomm Graphics Clock & Reset Controller on SM8750
>
> There is no clocks nor resets here. Only power domains.
There are clocks and resets in this IP block (inside the register
space mentioned in the dt patch/example), but the OS is not supposed
to poke at them (it can in theory, but we have a uC - the GMU -
doing the same thing so it would be stepping on one another's toes..).
Not sure how to express that.
I could for example add #define indices in include/dt-bindings, listing
out the clocks and never consume them. Does that sound fair?
>
>> +
>> +maintainers:
>> + - Konrad Dybcio <konradybcio@kernel.org>
>> +
>> +description: |
>> + Qualcomm graphics clock control module provides the clocks, resets and power
>
> Also confusing.
>
>> + domains on Qualcomm SoCs.
>> +
>> + See also:
>> + include/dt-bindings/reset/qcom,sm8750-gpucc.h
>
> reset or clock path?
Ugh, clock
>
>> +
>> +properties:
>> + compatible:
>> + enum:
>> + - qcom,sm8750-gxcc
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + power-domains:
>> + items:
>> + - description: GFX voltage rail
>> + - description: MX_COLLAPSIBLE voltage rail
>> + - description: GPU_CC_CX GDSC
>> +
>> + '#power-domain-cells':
>> + const: 1
>> +
>> +required:
>> + - compatible
>> + - power-domains
>> + - '#power-domain-cells'
>> +
>
> You miss ref... or this is a bit confusing.
ref to what? qcom,gcc? I specifically omitted it, as that adds
requirements which you stated above.
Konrad
>
>> +unevaluatedProperties: false
>
> additionalProperties instead
>
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/clock/qcom,sm8750-gpucc.h>
>> + #include <dt-bindings/power/qcom,rpmhpd.h>
>> +
>> + soc {
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> +
>> + clock-controller@3d64000 {
>
> No, clock controllers have clock-cells. This cannot be a clock
> controller if it does not have any clocks for anyone to use.
>
> Best regards,
> Krzysztof
>
On 24/07/2025 12:53, Konrad Dybcio wrote: > On 7/24/25 10:18 AM, Krzysztof Kozlowski wrote: >> On Wed, Jul 23, 2025 at 10:38:48PM +0200, Konrad Dybcio wrote: >>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> >>> >>> The SM8750 features a "traditional" GPU_CC block, much of which is >>> controlled through the GMU microcontroller. Additionally, there's >>> an separate GX_CC block, where the GX GDSC is moved. >>> >>> Add bindings to accommodate for that. >>> >>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> >>> --- > > [...] > >>> +title: Qualcomm Graphics Clock & Reset Controller on SM8750 >> >> There is no clocks nor resets here. Only power domains. > > There are clocks and resets in this IP block (inside the register > space mentioned in the dt patch/example), but the OS is not supposed > to poke at them (it can in theory, but we have a uC - the GMU - > doing the same thing so it would be stepping on one another's toes..). > Not sure how to express that. > > I could for example add #define indices in include/dt-bindings, listing > out the clocks and never consume them. Does that sound fair? Explain that in the binding description. > >> >>> + >>> +maintainers: >>> + - Konrad Dybcio <konradybcio@kernel.org> >>> + >>> +description: | >>> + Qualcomm graphics clock control module provides the clocks, resets and power >> >> Also confusing. >> >>> + domains on Qualcomm SoCs. >>> + >>> + See also: >>> + include/dt-bindings/reset/qcom,sm8750-gpucc.h >> >> reset or clock path? > > Ugh, clock > >> >>> + >>> +properties: >>> + compatible: >>> + enum: >>> + - qcom,sm8750-gxcc >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + power-domains: >>> + items: >>> + - description: GFX voltage rail >>> + - description: MX_COLLAPSIBLE voltage rail >>> + - description: GPU_CC_CX GDSC >>> + >>> + '#power-domain-cells': >>> + const: 1 >>> + >>> +required: >>> + - compatible >>> + - power-domains >>> + - '#power-domain-cells' >>> + >> >> You miss ref... or this is a bit confusing. > ref to what? qcom,gcc? I specifically omitted it, as that adds > requirements which you stated above. Yes, qcom,gcc. If that was missing intentionally, it is fine assuming you implement the rest of comments. Best regards, Krzysztof
On 7/24/25 4:42 PM, Krzysztof Kozlowski wrote: > On 24/07/2025 12:53, Konrad Dybcio wrote: >> On 7/24/25 10:18 AM, Krzysztof Kozlowski wrote: >>> On Wed, Jul 23, 2025 at 10:38:48PM +0200, Konrad Dybcio wrote: >>>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> >>>> >>>> The SM8750 features a "traditional" GPU_CC block, much of which is >>>> controlled through the GMU microcontroller. Additionally, there's >>>> an separate GX_CC block, where the GX GDSC is moved. >>>> >>>> Add bindings to accommodate for that. >>>> >>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> >>>> --- [...] > Yes, qcom,gcc. If that was missing intentionally, it is fine assuming > you implement the rest of comments. With the description addition that you suggested above, should I keep this file in clocks/ after all? Konrad
On 25/07/2025 11:23, Konrad Dybcio wrote: > On 7/24/25 4:42 PM, Krzysztof Kozlowski wrote: >> On 24/07/2025 12:53, Konrad Dybcio wrote: >>> On 7/24/25 10:18 AM, Krzysztof Kozlowski wrote: >>>> On Wed, Jul 23, 2025 at 10:38:48PM +0200, Konrad Dybcio wrote: >>>>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> >>>>> >>>>> The SM8750 features a "traditional" GPU_CC block, much of which is >>>>> controlled through the GMU microcontroller. Additionally, there's >>>>> an separate GX_CC block, where the GX GDSC is moved. >>>>> >>>>> Add bindings to accommodate for that. >>>>> >>>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> >>>>> --- > > [...] > >> Yes, qcom,gcc. If that was missing intentionally, it is fine assuming >> you implement the rest of comments. > > With the description addition that you suggested above, should I keep > this file in clocks/ after all? Good point, I don't know, this is unusual case. The question is whether there could be user of this binding/DTS, which would need/use clock-cells? If none of possible users could use it as a clock controller, I think it is not a clock controller from how SW sees it. IOW, it does not matter what it is fully (in bigger picture) if it cannot be used in that way. If all users of the binding can use it only as power domain provided, I would move it to power with rest of power domains. Also rename the node name to power-controller or power-domain. Best regards, Krzysztof
On 7/28/25 7:05 AM, Krzysztof Kozlowski wrote: > On 25/07/2025 11:23, Konrad Dybcio wrote: >> On 7/24/25 4:42 PM, Krzysztof Kozlowski wrote: >>> On 24/07/2025 12:53, Konrad Dybcio wrote: >>>> On 7/24/25 10:18 AM, Krzysztof Kozlowski wrote: >>>>> On Wed, Jul 23, 2025 at 10:38:48PM +0200, Konrad Dybcio wrote: >>>>>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> >>>>>> >>>>>> The SM8750 features a "traditional" GPU_CC block, much of which is >>>>>> controlled through the GMU microcontroller. Additionally, there's >>>>>> an separate GX_CC block, where the GX GDSC is moved. >>>>>> >>>>>> Add bindings to accommodate for that. >>>>>> >>>>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> >>>>>> --- >> >> [...] >> >>> Yes, qcom,gcc. If that was missing intentionally, it is fine assuming >>> you implement the rest of comments. >> >> With the description addition that you suggested above, should I keep >> this file in clocks/ after all? > > Good point, I don't know, this is unusual case. The question is whether > there could be user of this binding/DTS, which would need/use > clock-cells? If none of possible users could use it as a clock > controller, I think it is not a clock controller from how SW sees it. > IOW, it does not matter what it is fully (in bigger picture) if it > cannot be used in that way. > > If all users of the binding can use it only as power domain provided, I > would move it to power with rest of power domains. Also rename the node > name to power-controller or power-domain. The hardware block can be accessed from the CPU directly, skipping the microcontroller (although that is undesirable and the only "real" use for it I can think about is someone trying to get rid of a blob). I can add clock/reset-cells to describe the hardware accurately, but the Linux driver(s - this is a block that exists on many >=2024 SoCs as you may imagine) will continue to only provide a single power domain. With that, I think clock/ makes sense, as this is essentially the same hardware template as other instances of QCOM_*CC Konrad
© 2016 - 2025 Red Hat, Inc.