There are two problems with the upstream camcc implementation at the
moment which this series addresses.
1. The camcc block has two power-domains MXC and MMCX however, the yaml
description doesn't include MXC.
2. The code for the GDSC definitions for x1e80100 camcc fails to list
the titan_top_gdsc as the parent GDSC of the other GDSCs.
This series addresses both of those bugs. There is currently no upstream
camcc dtsi for x1e80100 so the yaml change won't affect the ABI.
Signed-off-by: Bryan O'Donoghue <[email protected]>
---
Bryan O'Donoghue (2):
dt-bindings: clock: qcom: Fix x1e80100 camcc power-domain declaration
clk: qcom: camcc-x1e80100: Set titan_top_gdsc as the parent GDSC of subordinate GDSCs
.../bindings/clock/qcom,sm8450-camcc.yaml | 37 ++++++++++++++++++----
drivers/clk/qcom/camcc-x1e80100.c | 7 ++++
2 files changed, 38 insertions(+), 6 deletions(-)
---
base-commit: 8ffc8b1bbd505e27e2c8439d326b6059c906c9dd
change-id: 20240310-linux-next-camcc-fixes-a68322916421
Best regards,
--
Bryan O'Donoghue <[email protected]>
camcc on x1e80100 requires two power domains MXC and MMCX. Define those as
part of the schema.
Fixes: 76126a5129b5 ("clk: qcom: Add camcc clock driver for x1e80100")
Signed-off-by: Bryan O'Donoghue <[email protected]>
---
.../bindings/clock/qcom,sm8450-camcc.yaml | 37 ++++++++++++++++++----
1 file changed, 31 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
index fa0e5b6b02b81..eae9b73ae1f16 100644
--- a/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
@@ -19,9 +19,6 @@ description: |
include/dt-bindings/clock/qcom,sc8280xp-camcc.h
include/dt-bindings/clock/qcom,x1e80100-camcc.h
-allOf:
- - $ref: qcom,gcc.yaml#
-
properties:
compatible:
enum:
@@ -38,9 +35,8 @@ properties:
- description: Sleep clock source
power-domains:
- maxItems: 1
- description:
- A phandle and PM domain specifier for the MMCX power domain.
+ minItems: 1
+ maxItems: 2
required-opps:
maxItems: 1
@@ -56,6 +52,35 @@ required:
- power-domains
- required-opps
+allOf:
+ - $ref: qcom,gcc.yaml#
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,sc8280xp-camcc
+ - qcom,sm8450-camcc
+ - qcom,sm8550-camcc
+ then:
+ properties:
+ power-domains:
+ description:
+ A phandle and PM domain specifier for the MMCX power domain.
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,x1e80100-camcc
+ then:
+ properties:
+ power-domains:
+ items:
+ - description: A phandle and PM domain specifier for the MXC power domain.
+ - description: A phandle and PM domain specifier for the MMCX power domain.
+
unevaluatedProperties: false
examples:
--
2.43.1
The Titan TOP GDSC is the parent GDSC for all other GDSCs in the CAMCC
block. None of the subordinate blocks will switch on without the parent
GDSC switched on.
Fixes: 76126a5129b5 ("clk: qcom: Add camcc clock driver for x1e80100")
Signed-off-by: Bryan O'Donoghue <[email protected]>
---
drivers/clk/qcom/camcc-x1e80100.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/clk/qcom/camcc-x1e80100.c b/drivers/clk/qcom/camcc-x1e80100.c
index 46bb225906bff..d421da57697a2 100644
--- a/drivers/clk/qcom/camcc-x1e80100.c
+++ b/drivers/clk/qcom/camcc-x1e80100.c
@@ -2212,6 +2212,8 @@ static struct clk_branch cam_cc_sfe_0_fast_ahb_clk = {
},
};
+static struct gdsc cam_cc_titan_top_gdsc;
+
static struct gdsc cam_cc_bps_gdsc = {
.gdscr = 0x10004,
.en_rest_wait_val = 0x2,
@@ -2221,6 +2223,7 @@ static struct gdsc cam_cc_bps_gdsc = {
.name = "cam_cc_bps_gdsc",
},
.pwrsts = PWRSTS_OFF_ON,
+ .parent = &cam_cc_titan_top_gdsc.pd,
.flags = POLL_CFG_GDSCR | RETAIN_FF_ENABLE,
};
@@ -2233,6 +2236,7 @@ static struct gdsc cam_cc_ife_0_gdsc = {
.name = "cam_cc_ife_0_gdsc",
},
.pwrsts = PWRSTS_OFF_ON,
+ .parent = &cam_cc_titan_top_gdsc.pd,
.flags = POLL_CFG_GDSCR | RETAIN_FF_ENABLE,
};
@@ -2245,6 +2249,7 @@ static struct gdsc cam_cc_ife_1_gdsc = {
.name = "cam_cc_ife_1_gdsc",
},
.pwrsts = PWRSTS_OFF_ON,
+ .parent = &cam_cc_titan_top_gdsc.pd,
.flags = POLL_CFG_GDSCR | RETAIN_FF_ENABLE,
};
@@ -2257,6 +2262,7 @@ static struct gdsc cam_cc_ipe_0_gdsc = {
.name = "cam_cc_ipe_0_gdsc",
},
.pwrsts = PWRSTS_OFF_ON,
+ .parent = &cam_cc_titan_top_gdsc.pd,
.flags = POLL_CFG_GDSCR | RETAIN_FF_ENABLE,
};
@@ -2269,6 +2275,7 @@ static struct gdsc cam_cc_sfe_0_gdsc = {
.name = "cam_cc_sfe_0_gdsc",
},
.pwrsts = PWRSTS_OFF_ON,
+ .parent = &cam_cc_titan_top_gdsc.pd,
.flags = POLL_CFG_GDSCR | RETAIN_FF_ENABLE,
};
--
2.43.1
On 3/11/2024 6:03 AM, Bryan O'Donoghue wrote:
> The Titan TOP GDSC is the parent GDSC for all other GDSCs in the CAMCC
> block. None of the subordinate blocks will switch on without the parent
> GDSC switched on.
>
> Fixes: 76126a5129b5 ("clk: qcom: Add camcc clock driver for x1e80100")
> Signed-off-by: Bryan O'Donoghue <[email protected]>
Acked-by: Rajendra Nayak <[email protected]>
> ---
> drivers/clk/qcom/camcc-x1e80100.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/clk/qcom/camcc-x1e80100.c b/drivers/clk/qcom/camcc-x1e80100.c
> index 46bb225906bff..d421da57697a2 100644
> --- a/drivers/clk/qcom/camcc-x1e80100.c
> +++ b/drivers/clk/qcom/camcc-x1e80100.c
> @@ -2212,6 +2212,8 @@ static struct clk_branch cam_cc_sfe_0_fast_ahb_clk = {
> },
> };
>
> +static struct gdsc cam_cc_titan_top_gdsc;
> +
> static struct gdsc cam_cc_bps_gdsc = {
> .gdscr = 0x10004,
> .en_rest_wait_val = 0x2,
> @@ -2221,6 +2223,7 @@ static struct gdsc cam_cc_bps_gdsc = {
> .name = "cam_cc_bps_gdsc",
> },
> .pwrsts = PWRSTS_OFF_ON,
> + .parent = &cam_cc_titan_top_gdsc.pd,
> .flags = POLL_CFG_GDSCR | RETAIN_FF_ENABLE,
> };
>
> @@ -2233,6 +2236,7 @@ static struct gdsc cam_cc_ife_0_gdsc = {
> .name = "cam_cc_ife_0_gdsc",
> },
> .pwrsts = PWRSTS_OFF_ON,
> + .parent = &cam_cc_titan_top_gdsc.pd,
> .flags = POLL_CFG_GDSCR | RETAIN_FF_ENABLE,
> };
>
> @@ -2245,6 +2249,7 @@ static struct gdsc cam_cc_ife_1_gdsc = {
> .name = "cam_cc_ife_1_gdsc",
> },
> .pwrsts = PWRSTS_OFF_ON,
> + .parent = &cam_cc_titan_top_gdsc.pd,
> .flags = POLL_CFG_GDSCR | RETAIN_FF_ENABLE,
> };
>
> @@ -2257,6 +2262,7 @@ static struct gdsc cam_cc_ipe_0_gdsc = {
> .name = "cam_cc_ipe_0_gdsc",
> },
> .pwrsts = PWRSTS_OFF_ON,
> + .parent = &cam_cc_titan_top_gdsc.pd,
> .flags = POLL_CFG_GDSCR | RETAIN_FF_ENABLE,
> };
>
> @@ -2269,6 +2275,7 @@ static struct gdsc cam_cc_sfe_0_gdsc = {
> .name = "cam_cc_sfe_0_gdsc",
> },
> .pwrsts = PWRSTS_OFF_ON,
> + .parent = &cam_cc_titan_top_gdsc.pd,
> .flags = POLL_CFG_GDSCR | RETAIN_FF_ENABLE,
> };
>
>
On 11/03/2024 01:33, Bryan O'Donoghue wrote:
> camcc on x1e80100 requires two power domains MXC and MMCX. Define those as
> part of the schema.
>
..
>
> required-opps:
> maxItems: 1
> @@ -56,6 +52,35 @@ required:
> - power-domains
> - required-opps
>
> +allOf:
> + - $ref: qcom,gcc.yaml#
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - qcom,sc8280xp-camcc
> + - qcom,sm8450-camcc
> + - qcom,sm8550-camcc
> + then:
> + properties:
> + power-domains:
> + description:
> + A phandle and PM domain specifier for the MMCX power domain.
This does not narrow the number of items. You need items with
description, just like you have for x1e. And drop redundant parts. This
is just "MMCX power domain"
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - qcom,x1e80100-camcc
> + then:
> + properties:
> + power-domains:
> + items:
> + - description: A phandle and PM domain specifier for the MXC power domain.
Drop redundant parts of description:
MXC power domain
> + - description: A phandle and PM domain specifier for the MMCX power domain.
MMCX power domain
Best regards,
Krzysztof
On 3/11/24 01:33, Bryan O'Donoghue wrote:
> The Titan TOP GDSC is the parent GDSC for all other GDSCs in the CAMCC
> block. None of the subordinate blocks will switch on without the parent
> GDSC switched on.
>
> Fixes: 76126a5129b5 ("clk: qcom: Add camcc clock driver for x1e80100")
> Signed-off-by: Bryan O'Donoghue <[email protected]>
> ---
Reviewed-by: Konrad Dybcio <[email protected]>
Konrad