Hello Taniya Das,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/38389
to review the following change.
Change subject: TEMP: sc7180: clock: Fix QUP DFSR configuration for perf levels ......................................................................
TEMP: sc7180: clock: Fix QUP DFSR configuration for perf levels
Update the QUP DFSR cmd to clear the SW control and also update the perf registers when M is set. While at it also update the d_2 values.
Tested: validated DFSR clock configuration and M/N/D values.
Change-Id: I6bba1c6f99810963aaa607885ef400c523c0e905 --- M src/soc/qualcomm/sc7180/clock.c M src/soc/qualcomm/sc7180/include/soc/clock.h 2 files changed, 9 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/38389/1
diff --git a/src/soc/qualcomm/sc7180/clock.c b/src/soc/qualcomm/sc7180/clock.c index b447a54..213c37f 100644 --- a/src/soc/qualcomm/sc7180/clock.c +++ b/src/soc/qualcomm/sc7180/clock.c @@ -72,7 +72,7 @@ .div = DIV(1), .m = 8, .n = 75, - .d_2 = 150, + .d_2 = 75, }, { .hz = 48 * MHz, @@ -80,7 +80,7 @@ .div = DIV(1), .m = 4, .n = 25, - .d_2 = 50, + .d_2 = 25, }, { .hz = 64 * MHz, @@ -88,7 +88,7 @@ .div = DIV(1), .m = 16, .n = 75, - .d_2 = 150, + .d_2 = 75, }, { .hz = 96 * MHz, @@ -96,7 +96,7 @@ .div = DIV(1), .m = 8, .n = 25, - .d_2 = 50, + .d_2 = 25, }, { .hz = 100 * MHz, @@ -236,7 +236,9 @@ struct sc7180_qupv3_clock *qup_clk = qup < QUP_WRAP1_S0 ? &gcc->qup_wrap0_s[s] : &gcc->qup_wrap1_s[s];
- setbits32(&qup_clk->dfsr_clk.cmd_dfsr, BIT(CLK_CTL_CMD_DFSR_SHFT)); + clrsetbits32(&qup_clk->dfsr_clk.cmd_dfsr, + BIT(CLK_CTL_CMD_RCG_SW_CTL_SHFT), + BIT(CLK_CTL_CMD_DFSR_SHFT));
for (idx = 0; idx < ARRAY_SIZE(qup_wrap_cfg); idx++) { reg_val = (qup_wrap_cfg[idx].src << CLK_CTL_CFG_SRC_SEL_SHFT) | @@ -247,7 +249,7 @@ if (qup_wrap_cfg[idx].m == 0) continue;
- setbits32(&qup_clk->dfsr_clk.cmd_dfsr, + setbits32(&qup_clk->dfsr_clk.perf_dfsr[idx], RCG_MODE_DUAL_EDGE << CLK_CTL_CFG_MODE_SHFT);
reg_val = qup_wrap_cfg[idx].m & CLK_CTL_RCG_MND_BMSK; diff --git a/src/soc/qualcomm/sc7180/include/soc/clock.h b/src/soc/qualcomm/sc7180/include/soc/clock.h index 2e44b60..383e6d7 100644 --- a/src/soc/qualcomm/sc7180/include/soc/clock.h +++ b/src/soc/qualcomm/sc7180/include/soc/clock.h @@ -187,6 +187,7 @@ enum clk_ctl_dfsr { CLK_CTL_CMD_DFSR_BMSK = 0x1, CLK_CTL_CMD_DFSR_SHFT = 0, + CLK_CTL_CMD_RCG_SW_CTL_SHFT = 15, };
enum clk_qup {
Taniya Das has uploaded a new patch set (#2) to the change originally created by mturney mturney. ( https://review.coreboot.org/c/coreboot/+/38389 )
Change subject: sc7180: clock: Fix QUP DFSR configuration for perf levels ......................................................................
sc7180: clock: Fix QUP DFSR configuration for perf levels
Update the QUP DFSR cmd to clear the SW control and also update the perf registers when M is set. While at it also update the d_2 values.
Tested: validated DFSR clock configuration and M/N/D values.
Change-Id: I6bba1c6f99810963aaa607885ef400c523c0e905 --- M src/soc/qualcomm/sc7180/clock.c M src/soc/qualcomm/sc7180/include/soc/clock.h 2 files changed, 9 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/38389/2
Hello Julius Werner, build bot (Jenkins), Taniya Das,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38389
to look at the new patch set (#3).
Change subject: TEMP: sc7180: clock: Fix QUP DFSR configuration for perf levels ......................................................................
TEMP: sc7180: clock: Fix QUP DFSR configuration for perf levels
Update the QUP DFSR cmd to clear the SW control and also update the perf registers when M is set. While at it also update the d_2 values.
Tested: validated DFSR clock configuration and M/N/D values.
Change-Id: I6bba1c6f99810963aaa607885ef400c523c0e905 --- M src/soc/qualcomm/sc7180/clock.c M src/soc/qualcomm/sc7180/include/soc/clock.h 2 files changed, 9 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/38389/3
Hello Julius Werner, Ravi kumar, build bot (Jenkins), Taniya Das,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38389
to look at the new patch set (#10).
Change subject: sc7180: clock: Fix QUP DFSR configuration for perf levels ......................................................................
sc7180: clock: Fix QUP DFSR configuration for perf levels
Update the QUP DFSR cmd to clear the SW control and also update the perf registers when M is set. While at it also update the d_2 values.
Tested: validated DFSR clock configuration and M/N/D values.
Change-Id: I6bba1c6f99810963aaa607885ef400c523c0e905 --- M src/soc/qualcomm/sc7180/clock.c M src/soc/qualcomm/sc7180/include/soc/clock.h 2 files changed, 9 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/38389/10
Hello Julius Werner, Ravi kumar, build bot (Jenkins), Taniya Das,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38389
to look at the new patch set (#11).
Change subject: sc7180: clock: Fix QUP DFSR configuration for perf levels ......................................................................
sc7180: clock: Fix QUP DFSR configuration for perf levels
Update the QUP DFSR cmd to clear the SW control and also update the perf registers when M is set. While at it also update the d_2 values.
Tested: validated DFSR clock configuration and M/N/D values.
Change-Id: I6bba1c6f99810963aaa607885ef400c523c0e905 Signed-off-by: Taniya Das tdas@codeaurora.org --- M src/soc/qualcomm/sc7180/clock.c M src/soc/qualcomm/sc7180/include/soc/clock.h 2 files changed, 9 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/38389/11
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38389 )
Change subject: sc7180: clock: Fix QUP DFSR configuration for perf levels ......................................................................
Patch Set 12: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/38389/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38389/7//COMMIT_MSG@7 PS7, Line 7: TEMP Why is this TEMP? do we need this patch or not?
Taniya Das has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38389 )
Change subject: sc7180: clock: Fix QUP DFSR configuration for perf levels ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38389/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38389/7//COMMIT_MSG@7 PS7, Line 7: TEMP
Why is this TEMP? do we need this patch or not?
Hi Julius, This is required and thus we have to remove the TEMP.
Julius Werner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38389 )
Change subject: sc7180: clock: Fix QUP DFSR configuration for perf levels ......................................................................
sc7180: clock: Fix QUP DFSR configuration for perf levels
Update the QUP DFSR cmd to clear the SW control and also update the perf registers when M is set. While at it also update the d_2 values.
Tested: validated DFSR clock configuration and M/N/D values.
Change-Id: I6bba1c6f99810963aaa607885ef400c523c0e905 Signed-off-by: Taniya Das tdas@codeaurora.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/38389 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Julius Werner jwerner@chromium.org --- M src/soc/qualcomm/sc7180/clock.c M src/soc/qualcomm/sc7180/include/soc/clock.h 2 files changed, 9 insertions(+), 6 deletions(-)
Approvals: build bot (Jenkins): Verified Julius Werner: Looks good to me, approved
diff --git a/src/soc/qualcomm/sc7180/clock.c b/src/soc/qualcomm/sc7180/clock.c index b447a54..213c37f 100644 --- a/src/soc/qualcomm/sc7180/clock.c +++ b/src/soc/qualcomm/sc7180/clock.c @@ -72,7 +72,7 @@ .div = DIV(1), .m = 8, .n = 75, - .d_2 = 150, + .d_2 = 75, }, { .hz = 48 * MHz, @@ -80,7 +80,7 @@ .div = DIV(1), .m = 4, .n = 25, - .d_2 = 50, + .d_2 = 25, }, { .hz = 64 * MHz, @@ -88,7 +88,7 @@ .div = DIV(1), .m = 16, .n = 75, - .d_2 = 150, + .d_2 = 75, }, { .hz = 96 * MHz, @@ -96,7 +96,7 @@ .div = DIV(1), .m = 8, .n = 25, - .d_2 = 50, + .d_2 = 25, }, { .hz = 100 * MHz, @@ -236,7 +236,9 @@ struct sc7180_qupv3_clock *qup_clk = qup < QUP_WRAP1_S0 ? &gcc->qup_wrap0_s[s] : &gcc->qup_wrap1_s[s];
- setbits32(&qup_clk->dfsr_clk.cmd_dfsr, BIT(CLK_CTL_CMD_DFSR_SHFT)); + clrsetbits32(&qup_clk->dfsr_clk.cmd_dfsr, + BIT(CLK_CTL_CMD_RCG_SW_CTL_SHFT), + BIT(CLK_CTL_CMD_DFSR_SHFT));
for (idx = 0; idx < ARRAY_SIZE(qup_wrap_cfg); idx++) { reg_val = (qup_wrap_cfg[idx].src << CLK_CTL_CFG_SRC_SEL_SHFT) | @@ -247,7 +249,7 @@ if (qup_wrap_cfg[idx].m == 0) continue;
- setbits32(&qup_clk->dfsr_clk.cmd_dfsr, + setbits32(&qup_clk->dfsr_clk.perf_dfsr[idx], RCG_MODE_DUAL_EDGE << CLK_CTL_CFG_MODE_SHFT);
reg_val = qup_wrap_cfg[idx].m & CLK_CTL_RCG_MND_BMSK; diff --git a/src/soc/qualcomm/sc7180/include/soc/clock.h b/src/soc/qualcomm/sc7180/include/soc/clock.h index 2e44b60..383e6d7 100644 --- a/src/soc/qualcomm/sc7180/include/soc/clock.h +++ b/src/soc/qualcomm/sc7180/include/soc/clock.h @@ -187,6 +187,7 @@ enum clk_ctl_dfsr { CLK_CTL_CMD_DFSR_BMSK = 0x1, CLK_CTL_CMD_DFSR_SHFT = 0, + CLK_CTL_CMD_RCG_SW_CTL_SHFT = 15, };
enum clk_qup {
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38389 )
Change subject: sc7180: clock: Fix QUP DFSR configuration for perf levels ......................................................................
Patch Set 15:
Automatic boot test returned (PASS/FAIL/TOTAL): 3/0/3 Emulation targets: EMULATION_QEMU_X86_Q35 using payload TianoCore : SUCCESS : https://lava.9esec.io/r/499 EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/498 EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/497
Please note: This test is under development and might not be accurate at all!