Hello Taniya Das,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/37305
to review the following change.
Change subject: sc7180: clock: Add support for QUP DFSR configuration ......................................................................
sc7180: clock: Add support for QUP DFSR configuration
Support configuring the qup dfsr registers.
Tested: validated DFSR clock configuration and M/N/D values.
Change-Id: I146ac7c2197606965265f2a770769312af76041e 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, 108 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/37305/1
diff --git a/src/soc/qualcomm/sc7180/clock.c b/src/soc/qualcomm/sc7180/clock.c index 97b7b28..658f883 100644 --- a/src/soc/qualcomm/sc7180/clock.c +++ b/src/soc/qualcomm/sc7180/clock.c @@ -60,6 +60,61 @@ } };
+struct clock_config qup_wrap_cfg[] = { + { + .hz = SRC_XO_HZ, /* 19.2KHz */ + .src = SRC_XO_19_2MHZ, + .div = DIV(1), + }, + { + .hz = 32 * MHz, + .src = SRC_GPLL0_EVEN_300MHZ, + .div = DIV(1), + .m = 8, + .n = 75, + .d_2 = 150, + }, + { + .hz = 48 * MHz, + .src = SRC_GPLL0_EVEN_300MHZ, + .div = DIV(1), + .m = 4, + .n = 25, + .d_2 = 50, + }, + { + .hz = 64 * MHz, + .src = SRC_GPLL0_EVEN_300MHZ, + .div = DIV(1), + .m = 16, + .n = 75, + .d_2 = 150, + }, + { + .hz = 96 * MHz, + .src = SRC_GPLL0_EVEN_300MHZ, + .div = DIV(1), + .m = 8, + .n = 25, + .d_2 = 50, + }, + { + .hz = 100 * MHz, + .src = SRC_GPLL0_EVEN_300MHZ, + .div = DIV(3), + }, + { + .hz = SRC_XO_HZ, /* 19.2KHz */ + .src = SRC_XO_19_2MHZ, + .div = DIV(1), + }, + { + .hz = SRC_XO_HZ, /* 19.2KHz */ + .src = SRC_XO_19_2MHZ, + .div = DIV(1), + }, +}; + static int clock_configure_gpll0(void) { setbits_le32(&gcc->gpll0.user_ctl_u, 1 << SCALE_FREQ_SHFT); @@ -173,6 +228,39 @@ return 0; }
+void clock_configure_dfsr(int qup) +{ + int idx, s = qup % QUP_WRAP1_S0; + uint32_t reg_val; + struct sc7180_qupv3_clock *qup_clk = qup < QUP_WRAP1_S0 ? + &gcc->qup_wrap0_s[s] : &gcc->qup_wrap1_s[s]; + + setbits_le32(&qup_clk->dfsr_clk.cmd_dfsr, 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) | + (qup_wrap_cfg[idx].div << CLK_CTL_CFG_SRC_DIV_SHFT); + + write32(&qup_clk->dfsr_clk.perf_dfsr[idx], reg_val); + + if (qup_wrap_cfg[idx].m == 0) + continue; + + setbits_le32(&qup_clk->dfsr_clk.cmd_dfsr, + RCG_MODE_DUAL_EDGE << CLK_CTL_CFG_MODE_SHFT); + + reg_val = qup_wrap_cfg[idx].m & CLK_CTL_RCG_MND_BMSK; + write32(&qup_clk->dfsr_clk.perf_m_dfsr[idx], reg_val); + + reg_val = ~(qup_wrap_cfg[idx].n - qup_wrap_cfg[idx].m) + & CLK_CTL_RCG_MND_BMSK; + write32(&qup_clk->dfsr_clk.perf_n_dfsr[idx], reg_val); + + reg_val = ~(qup_wrap_cfg[idx].d_2) & CLK_CTL_RCG_MND_BMSK; + write32(&qup_clk->dfsr_clk.perf_d_dfsr[idx], reg_val); + } +} + void clock_configure_qup(int qup, uint32_t hz) { int s = qup % QUP_WRAP1_S0; diff --git a/src/soc/qualcomm/sc7180/include/soc/clock.h b/src/soc/qualcomm/sc7180/include/soc/clock.h index 39cde8c..2e44b60 100644 --- a/src/soc/qualcomm/sc7180/include/soc/clock.h +++ b/src/soc/qualcomm/sc7180/include/soc/clock.h @@ -56,9 +56,22 @@ u32 d_2; };
+struct sc7180_dfsr_clock { + u32 cmd_dfsr; + u8 _res0[0x20 - 0x1c]; + u32 perf_dfsr[8]; + u8 _res1[0x60 - 0x40]; + u32 perf_m_dfsr[8]; + u8 _res2[0xa0 - 0x80]; + u32 perf_n_dfsr[8]; + u8 _res3[0xe0 - 0xc0]; + u32 perf_d_dfsr[8]; + u8 _res4[0x130 - 0x100]; +}; + struct sc7180_qupv3_clock { struct sc7180_mnd_clock mnd_clk; - u8 _res[0x130 - 0x18]; + struct sc7180_dfsr_clock dfsr_clk; };
struct sc7180_gpll { @@ -171,6 +184,11 @@ CLK_CTL_BCR_BLK_ARES_SHFT = 0, };
+enum clk_ctl_dfsr { + CLK_CTL_CMD_DFSR_BMSK = 0x1, + CLK_CTL_CMD_DFSR_SHFT = 0, +}; + enum clk_qup { QUP_WRAP0_S0, QUP_WRAP0_S1, @@ -210,5 +228,6 @@ int clock_reset_bcr(void *bcr_addr, bool reset); void clock_configure_qup(int qup, uint32_t hz); void clock_enable_qup(int qup); +void clock_configure_dfsr(int qup);
#endif // __SOC_QUALCOMM_SC7180_CLOCK_H__
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37305 )
Change subject: sc7180: clock: Add support for QUP DFSR configuration ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37305/1/src/soc/qualcomm/sc7180/clo... File src/soc/qualcomm/sc7180/clock.c:
https://review.coreboot.org/c/coreboot/+/37305/1/src/soc/qualcomm/sc7180/clo... PS1, Line 231: void clock_configure_dfsr(int qup) This doesn't seem to get called anywhere in this patch train. What is this used for? Don't recall anything like this from SDM845.
Taniya Das has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37305 )
Change subject: sc7180: clock: Add support for QUP DFSR configuration ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37305/1/src/soc/qualcomm/sc7180/clo... File src/soc/qualcomm/sc7180/clock.c:
https://review.coreboot.org/c/coreboot/+/37305/1/src/soc/qualcomm/sc7180/clo... PS1, Line 231: void clock_configure_dfsr(int qup)
This doesn't seem to get called anywhere in this patch train. […]
Hi Julius,
This would be invoked by the QUPv3 firmware driver to enable the QUPv3 SE clocks to dynamic frequency switch mode. The firmware driver is aware of which SEs to be kept in the DFS mode.
The clock driver in kernel would read these registers and re-populate the frequencies supported for the DFS controlled clocks.
Also the GENI driver on the HLOS expects the SEs in DFS mode.
SDM845: I guess there was some portion of the code in the firmware loading code, but it was incomplete.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37305 )
Change subject: sc7180: clock: Add support for QUP DFSR configuration ......................................................................
Patch Set 1: Code-Review+2
(2 comments)
https://review.coreboot.org/c/coreboot/+/37305/1/src/soc/qualcomm/sc7180/clo... File src/soc/qualcomm/sc7180/clock.c:
https://review.coreboot.org/c/coreboot/+/37305/1/src/soc/qualcomm/sc7180/clo... PS1, Line 231: void clock_configure_dfsr(int qup)
This would be invoked by the QUPv3 firmware driver to enable the QUPv3 SE clocks to dynamic frequency switch mode. The firmware driver is aware of which SEs to be kept in the DFS mode.
How are we aware of that? Is it just based on type (e.g. all SPI and all I2C)? Or do we need to hardcode it for every QUP?
https://review.coreboot.org/c/coreboot/+/37305/1/src/soc/qualcomm/sc7180/clo... PS1, Line 233: int idx, s = qup % QUP_WRAP1_S0; nit: please write
int idx; int s = qup % QUP_WRAP1_S0;
for clarity.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37305 )
Change subject: sc7180: clock: Add support for QUP DFSR configuration ......................................................................
Patch Set 1:
(1 comment)
Starting from here it needs a rebase again to merge.
https://review.coreboot.org/c/coreboot/+/37305/1/src/soc/qualcomm/sc7180/clo... File src/soc/qualcomm/sc7180/clock.c:
https://review.coreboot.org/c/coreboot/+/37305/1/src/soc/qualcomm/sc7180/clo... PS1, Line 249: setbits_le32(&qup_clk->dfsr_clk.cmd_dfsr, We recently changed how this is supposed to be done: you're now just supposed to use clrbits32()/setbits32()/clrsetbits32() instead of the _le32() variants for normal register accesses. Please update all patches still in flight here to follow that new convention.
mturney mturney has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37305 )
Change subject: sc7180: clock: Add support for QUP DFSR configuration ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37305/1/src/soc/qualcomm/sc7180/clo... File src/soc/qualcomm/sc7180/clock.c:
https://review.coreboot.org/c/coreboot/+/37305/1/src/soc/qualcomm/sc7180/clo... PS1, Line 249: setbits_le32(&qup_clk->dfsr_clk.cmd_dfsr,
We recently changed how this is supposed to be done: you're now just supposed to use clrbits32()/set […]
Next push has this corrected here and in following patches: * spi_qup driver * spi I2C driver * qup v3 fw config
Hello Julius Werner, build bot (Jenkins), Taniya Das,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37305
to look at the new patch set (#2).
Change subject: sc7180: clock: Add support for QUP DFSR configuration ......................................................................
sc7180: clock: Add support for QUP DFSR configuration
Support configuring the qup dfsr registers.
Tested: validated DFSR clock configuration and M/N/D values.
Change-Id: I146ac7c2197606965265f2a770769312af76041e 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, 108 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/37305/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/+/37305
to look at the new patch set (#3).
Change subject: sc7180: clock: Add support for QUP DFSR configuration ......................................................................
sc7180: clock: Add support for QUP DFSR configuration
Support configuring the qup dfsr registers.
Tested: validated DFSR clock configuration and M/N/D values.
Change-Id: I146ac7c2197606965265f2a770769312af76041e 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, 109 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/37305/3
mturney mturney has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37305 )
Change subject: sc7180: clock: Add support for QUP DFSR configuration ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37305/1/src/soc/qualcomm/sc7180/clo... File src/soc/qualcomm/sc7180/clock.c:
https://review.coreboot.org/c/coreboot/+/37305/1/src/soc/qualcomm/sc7180/clo... PS1, Line 233: int idx, s = qup % QUP_WRAP1_S0;
nit: please write […]
Done
https://review.coreboot.org/c/coreboot/+/37305/1/src/soc/qualcomm/sc7180/clo... PS1, Line 249: setbits_le32(&qup_clk->dfsr_clk.cmd_dfsr,
Next push has this corrected here and in following patches: […]
Done
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37305 )
Change subject: sc7180: clock: Add support for QUP DFSR configuration ......................................................................
Patch Set 3: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37305 )
Change subject: sc7180: clock: Add support for QUP DFSR configuration ......................................................................
sc7180: clock: Add support for QUP DFSR configuration
Support configuring the qup dfsr registers.
Tested: validated DFSR clock configuration and M/N/D values.
Change-Id: I146ac7c2197606965265f2a770769312af76041e Signed-off-by: Taniya Das tdas@codeaurora.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/37305 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, 109 insertions(+), 1 deletion(-)
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 9092a4e..b447a54 100644 --- a/src/soc/qualcomm/sc7180/clock.c +++ b/src/soc/qualcomm/sc7180/clock.c @@ -60,6 +60,61 @@ } };
+struct clock_config qup_wrap_cfg[] = { + { + .hz = SRC_XO_HZ, /* 19.2KHz */ + .src = SRC_XO_19_2MHZ, + .div = DIV(1), + }, + { + .hz = 32 * MHz, + .src = SRC_GPLL0_EVEN_300MHZ, + .div = DIV(1), + .m = 8, + .n = 75, + .d_2 = 150, + }, + { + .hz = 48 * MHz, + .src = SRC_GPLL0_EVEN_300MHZ, + .div = DIV(1), + .m = 4, + .n = 25, + .d_2 = 50, + }, + { + .hz = 64 * MHz, + .src = SRC_GPLL0_EVEN_300MHZ, + .div = DIV(1), + .m = 16, + .n = 75, + .d_2 = 150, + }, + { + .hz = 96 * MHz, + .src = SRC_GPLL0_EVEN_300MHZ, + .div = DIV(1), + .m = 8, + .n = 25, + .d_2 = 50, + }, + { + .hz = 100 * MHz, + .src = SRC_GPLL0_EVEN_300MHZ, + .div = DIV(3), + }, + { + .hz = SRC_XO_HZ, /* 19.2KHz */ + .src = SRC_XO_19_2MHZ, + .div = DIV(1), + }, + { + .hz = SRC_XO_HZ, /* 19.2KHz */ + .src = SRC_XO_19_2MHZ, + .div = DIV(1), + }, +}; + static int clock_configure_gpll0(void) { setbits32(&gcc->gpll0.user_ctl_u, 1 << SCALE_FREQ_SHFT); @@ -173,6 +228,40 @@ return 0; }
+void clock_configure_dfsr(int qup) +{ + int idx; + int s = qup % QUP_WRAP1_S0; + uint32_t reg_val; + 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)); + + for (idx = 0; idx < ARRAY_SIZE(qup_wrap_cfg); idx++) { + reg_val = (qup_wrap_cfg[idx].src << CLK_CTL_CFG_SRC_SEL_SHFT) | + (qup_wrap_cfg[idx].div << CLK_CTL_CFG_SRC_DIV_SHFT); + + write32(&qup_clk->dfsr_clk.perf_dfsr[idx], reg_val); + + if (qup_wrap_cfg[idx].m == 0) + continue; + + setbits32(&qup_clk->dfsr_clk.cmd_dfsr, + RCG_MODE_DUAL_EDGE << CLK_CTL_CFG_MODE_SHFT); + + reg_val = qup_wrap_cfg[idx].m & CLK_CTL_RCG_MND_BMSK; + write32(&qup_clk->dfsr_clk.perf_m_dfsr[idx], reg_val); + + reg_val = ~(qup_wrap_cfg[idx].n - qup_wrap_cfg[idx].m) + & CLK_CTL_RCG_MND_BMSK; + write32(&qup_clk->dfsr_clk.perf_n_dfsr[idx], reg_val); + + reg_val = ~(qup_wrap_cfg[idx].d_2) & CLK_CTL_RCG_MND_BMSK; + write32(&qup_clk->dfsr_clk.perf_d_dfsr[idx], reg_val); + } +} + void clock_configure_qup(int qup, uint32_t hz) { int s = qup % QUP_WRAP1_S0; diff --git a/src/soc/qualcomm/sc7180/include/soc/clock.h b/src/soc/qualcomm/sc7180/include/soc/clock.h index 39cde8c..2e44b60 100644 --- a/src/soc/qualcomm/sc7180/include/soc/clock.h +++ b/src/soc/qualcomm/sc7180/include/soc/clock.h @@ -56,9 +56,22 @@ u32 d_2; };
+struct sc7180_dfsr_clock { + u32 cmd_dfsr; + u8 _res0[0x20 - 0x1c]; + u32 perf_dfsr[8]; + u8 _res1[0x60 - 0x40]; + u32 perf_m_dfsr[8]; + u8 _res2[0xa0 - 0x80]; + u32 perf_n_dfsr[8]; + u8 _res3[0xe0 - 0xc0]; + u32 perf_d_dfsr[8]; + u8 _res4[0x130 - 0x100]; +}; + struct sc7180_qupv3_clock { struct sc7180_mnd_clock mnd_clk; - u8 _res[0x130 - 0x18]; + struct sc7180_dfsr_clock dfsr_clk; };
struct sc7180_gpll { @@ -171,6 +184,11 @@ CLK_CTL_BCR_BLK_ARES_SHFT = 0, };
+enum clk_ctl_dfsr { + CLK_CTL_CMD_DFSR_BMSK = 0x1, + CLK_CTL_CMD_DFSR_SHFT = 0, +}; + enum clk_qup { QUP_WRAP0_S0, QUP_WRAP0_S1, @@ -210,5 +228,6 @@ int clock_reset_bcr(void *bcr_addr, bool reset); void clock_configure_qup(int qup, uint32_t hz); void clock_enable_qup(int qup); +void clock_configure_dfsr(int qup);
#endif // __SOC_QUALCOMM_SC7180_CLOCK_H__
-
Julius Werner (Code Review) -
mturney mturney (Code Review)
-
Patrick Georgi (Code Review)
-
Taniya Das (Code Review)