When SPI transfer is being prepared, the spi-s3c64xx driver will call
clk_set_rate() to change the rate of SPI source clock (IPCLK). But IPCLK
is a gate (leaf) clock, so it must propagate the rate change up the
clock tree, so that corresponding DIV clocks can actually change their
divider values. Add CLK_SET_RATE_PARENT flag to corresponding clocks for
all SPI instances in Exynos850 (spi_0, spi_1 and spi_2) to make it
possible. This change involves next clocks:
usi_spi_0:
Clock Block Div range
--------------------------------------------
gout_spi0_ipclk CMU_PERI -
dout_peri_spi0 CMU_PERI /1..32
mout_peri_spi_user CMU_PERI -
dout_peri_ip CMU_TOP /1..16
usi_cmgp0:
Clock Block Div range
--------------------------------------------
gout_cmgp_usi0_ipclk CMU_CMGP -
dout_cmgp_usi0 CMU_CMGP /1..32
mout_cmgp_usi0 CMU_CMGP -
gout_clkcmu_cmgp_bus CMU_APM -
dout_apm_bus CMU_APM /1..8
usi_cmgp1:
Clock Block Div range
--------------------------------------------
gout_cmgp_usi1_ipclk CMU_CMGP -
dout_cmgp_usi1 CMU_CMGP /1..32
mout_cmgp_usi1 CMU_CMGP -
gout_clkcmu_cmgp_bus CMU_APM -
dout_apm_bus CMU_APM /1..8
With input clock of 400 MHz, this scheme provides next IPCLK rate range,
for each SPI block:
SPI0: 781 kHz ... 400 MHz
SPI1/2: 1.6 MHz ... 400 MHz
Accounting for internal /4 divider in SPI blocks, and because the max
SPI frequency is limited at 50 MHz, it gives us next SPI SCK rates:
SPI0: 200 kHz ... 49.9 MHz
SPI1/2: 400 kHz ... 49.9 MHz
Which should cover all possible applications of SPI bus. Of course,
setting SPI frequency to values as low as 500 kHz will also affect the
common bus dividers (dout_apm_bus or dout_peri_ip), which in turn
effectively lowers the rates for all leaf bus clocks derived from those
dividers, like HSI2C and I3C clocks. But at least it gives the board
designer a choice, whether to keep all clocks (SPI/HSI2C/I3C) at high
frequencies, or make all those clocks have lower frequencies. Not
propagating the rate change to those common dividers would limit this
choice to "only high frequencies are allowed for SPI/HSI2C/I3C" option,
making the common dividers useless. This decision follows the "Worse is
better" approach, relying on the users/engineers to know the system
internals when working with such low-level features, instead of trying
to account for all possible use-cases.
Fixes: 7dd05578198b ("clk: samsung: Introduce Exynos850 clock driver")
Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
---
Changes in v2:
- Fixed indentation to make checkpatch strict happy
drivers/clk/samsung/clk-exynos850.c | 33 +++++++++++++++--------------
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/drivers/clk/samsung/clk-exynos850.c b/drivers/clk/samsung/clk-exynos850.c
index 01913dc4eb27..82cfa22c0788 100644
--- a/drivers/clk/samsung/clk-exynos850.c
+++ b/drivers/clk/samsung/clk-exynos850.c
@@ -605,7 +605,7 @@ static const struct samsung_div_clock apm_div_clks[] __initconst = {
static const struct samsung_gate_clock apm_gate_clks[] __initconst = {
GATE(CLK_GOUT_CLKCMU_CMGP_BUS, "gout_clkcmu_cmgp_bus", "dout_apm_bus",
- CLK_CON_GAT_CLKCMU_CMGP_BUS, 21, 0, 0),
+ CLK_CON_GAT_CLKCMU_CMGP_BUS, 21, CLK_SET_RATE_PARENT, 0),
GATE(CLK_GOUT_CLKCMU_CHUB_BUS, "gout_clkcmu_chub_bus",
"mout_clkcmu_chub_bus",
CLK_CON_GAT_GATE_CLKCMU_CHUB_BUS, 21, 0, 0),
@@ -974,19 +974,19 @@ static const struct samsung_fixed_rate_clock cmgp_fixed_clks[] __initconst = {
static const struct samsung_mux_clock cmgp_mux_clks[] __initconst = {
MUX(CLK_MOUT_CMGP_ADC, "mout_cmgp_adc", mout_cmgp_adc_p,
CLK_CON_MUX_CLK_CMGP_ADC, 0, 1),
- MUX(CLK_MOUT_CMGP_USI0, "mout_cmgp_usi0", mout_cmgp_usi0_p,
- CLK_CON_MUX_MUX_CLK_CMGP_USI_CMGP0, 0, 1),
- MUX(CLK_MOUT_CMGP_USI1, "mout_cmgp_usi1", mout_cmgp_usi1_p,
- CLK_CON_MUX_MUX_CLK_CMGP_USI_CMGP1, 0, 1),
+ MUX_F(CLK_MOUT_CMGP_USI0, "mout_cmgp_usi0", mout_cmgp_usi0_p,
+ CLK_CON_MUX_MUX_CLK_CMGP_USI_CMGP0, 0, 1, CLK_SET_RATE_PARENT, 0),
+ MUX_F(CLK_MOUT_CMGP_USI1, "mout_cmgp_usi1", mout_cmgp_usi1_p,
+ CLK_CON_MUX_MUX_CLK_CMGP_USI_CMGP1, 0, 1, CLK_SET_RATE_PARENT, 0),
};
static const struct samsung_div_clock cmgp_div_clks[] __initconst = {
DIV(CLK_DOUT_CMGP_ADC, "dout_cmgp_adc", "gout_clkcmu_cmgp_bus",
CLK_CON_DIV_DIV_CLK_CMGP_ADC, 0, 4),
- DIV(CLK_DOUT_CMGP_USI0, "dout_cmgp_usi0", "mout_cmgp_usi0",
- CLK_CON_DIV_DIV_CLK_CMGP_USI_CMGP0, 0, 5),
- DIV(CLK_DOUT_CMGP_USI1, "dout_cmgp_usi1", "mout_cmgp_usi1",
- CLK_CON_DIV_DIV_CLK_CMGP_USI_CMGP1, 0, 5),
+ DIV_F(CLK_DOUT_CMGP_USI0, "dout_cmgp_usi0", "mout_cmgp_usi0",
+ CLK_CON_DIV_DIV_CLK_CMGP_USI_CMGP0, 0, 5, CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_CMGP_USI1, "dout_cmgp_usi1", "mout_cmgp_usi1",
+ CLK_CON_DIV_DIV_CLK_CMGP_USI_CMGP1, 0, 5, CLK_SET_RATE_PARENT, 0),
};
static const struct samsung_gate_clock cmgp_gate_clks[] __initconst = {
@@ -1001,12 +1001,12 @@ static const struct samsung_gate_clock cmgp_gate_clks[] __initconst = {
"gout_clkcmu_cmgp_bus",
CLK_CON_GAT_GOUT_CMGP_GPIO_PCLK, 21, CLK_IGNORE_UNUSED, 0),
GATE(CLK_GOUT_CMGP_USI0_IPCLK, "gout_cmgp_usi0_ipclk", "dout_cmgp_usi0",
- CLK_CON_GAT_GOUT_CMGP_USI_CMGP0_IPCLK, 21, 0, 0),
+ CLK_CON_GAT_GOUT_CMGP_USI_CMGP0_IPCLK, 21, CLK_SET_RATE_PARENT, 0),
GATE(CLK_GOUT_CMGP_USI0_PCLK, "gout_cmgp_usi0_pclk",
"gout_clkcmu_cmgp_bus",
CLK_CON_GAT_GOUT_CMGP_USI_CMGP0_PCLK, 21, 0, 0),
GATE(CLK_GOUT_CMGP_USI1_IPCLK, "gout_cmgp_usi1_ipclk", "dout_cmgp_usi1",
- CLK_CON_GAT_GOUT_CMGP_USI_CMGP1_IPCLK, 21, 0, 0),
+ CLK_CON_GAT_GOUT_CMGP_USI_CMGP1_IPCLK, 21, CLK_SET_RATE_PARENT, 0),
GATE(CLK_GOUT_CMGP_USI1_PCLK, "gout_cmgp_usi1_pclk",
"gout_clkcmu_cmgp_bus",
CLK_CON_GAT_GOUT_CMGP_USI_CMGP1_PCLK, 21, 0, 0),
@@ -1557,8 +1557,9 @@ static const struct samsung_mux_clock peri_mux_clks[] __initconst = {
mout_peri_uart_user_p, PLL_CON0_MUX_CLKCMU_PERI_UART_USER, 4, 1),
MUX(CLK_MOUT_PERI_HSI2C_USER, "mout_peri_hsi2c_user",
mout_peri_hsi2c_user_p, PLL_CON0_MUX_CLKCMU_PERI_HSI2C_USER, 4, 1),
- MUX(CLK_MOUT_PERI_SPI_USER, "mout_peri_spi_user", mout_peri_spi_user_p,
- PLL_CON0_MUX_CLKCMU_PERI_SPI_USER, 4, 1),
+ MUX_F(CLK_MOUT_PERI_SPI_USER, "mout_peri_spi_user",
+ mout_peri_spi_user_p, PLL_CON0_MUX_CLKCMU_PERI_SPI_USER, 4, 1,
+ CLK_SET_RATE_PARENT, 0),
};
static const struct samsung_div_clock peri_div_clks[] __initconst = {
@@ -1568,8 +1569,8 @@ static const struct samsung_div_clock peri_div_clks[] __initconst = {
CLK_CON_DIV_DIV_CLK_PERI_HSI2C_1, 0, 5),
DIV(CLK_DOUT_PERI_HSI2C2, "dout_peri_hsi2c2", "gout_peri_hsi2c2",
CLK_CON_DIV_DIV_CLK_PERI_HSI2C_2, 0, 5),
- DIV(CLK_DOUT_PERI_SPI0, "dout_peri_spi0", "mout_peri_spi_user",
- CLK_CON_DIV_DIV_CLK_PERI_SPI_0, 0, 5),
+ DIV_F(CLK_DOUT_PERI_SPI0, "dout_peri_spi0", "mout_peri_spi_user",
+ CLK_CON_DIV_DIV_CLK_PERI_SPI_0, 0, 5, CLK_SET_RATE_PARENT, 0),
};
static const struct samsung_gate_clock peri_gate_clks[] __initconst = {
@@ -1611,7 +1612,7 @@ static const struct samsung_gate_clock peri_gate_clks[] __initconst = {
"mout_peri_bus_user",
CLK_CON_GAT_GOUT_PERI_PWM_MOTOR_PCLK, 21, 0, 0),
GATE(CLK_GOUT_SPI0_IPCLK, "gout_spi0_ipclk", "dout_peri_spi0",
- CLK_CON_GAT_GOUT_PERI_SPI_0_IPCLK, 21, 0, 0),
+ CLK_CON_GAT_GOUT_PERI_SPI_0_IPCLK, 21, CLK_SET_RATE_PARENT, 0),
GATE(CLK_GOUT_SPI0_PCLK, "gout_spi0_pclk", "mout_peri_bus_user",
CLK_CON_GAT_GOUT_PERI_SPI_0_PCLK, 21, 0, 0),
GATE(CLK_GOUT_SYSREG_PERI_PCLK, "gout_sysreg_peri_pclk",
--
2.39.2Hi, Sam, On 1/25/24 01:38, Sam Protsenko wrote: > Which should cover all possible applications of SPI bus. Of course, > setting SPI frequency to values as low as 500 kHz will also affect the > common bus dividers (dout_apm_bus or dout_peri_ip), which in turn > effectively lowers the rates for all leaf bus clocks derived from those > dividers, like HSI2C and I3C clocks. But at least it gives the board > designer a choice, whether to keep all clocks (SPI/HSI2C/I3C) at high > frequencies, or make all those clocks have lower frequencies. Not > propagating the rate change to those common dividers would limit this > choice to "only high frequencies are allowed for SPI/HSI2C/I3C" option, > making the common dividers useless. This decision follows the "Worse is > better" approach, relying on the users/engineers to know the system > internals when working with such low-level features, instead of trying > to account for all possible use-cases. Depending on clock frequencies in DT and on the order of probe, one may end up with SPI changing the frequency for I3C for example, there's no protection on that. The more conservative approach, to which I lean, is to propagate the clock just to the first divider, which is dedicated to the end note, thus you'll avoid such problems. I think this fine tuning that you allow is more suitable for downstream. Maybe this is more of a personal preference, I don't know. Curious what others think. The patch is looking fine though, and if the approach is considered acceptable: Reviewed-by: Tudor Ambarus <tudor.ambarus@linaro.org>
On Wed, 24 Jan 2024 19:38:56 -0600, Sam Protsenko wrote:
> When SPI transfer is being prepared, the spi-s3c64xx driver will call
> clk_set_rate() to change the rate of SPI source clock (IPCLK). But IPCLK
> is a gate (leaf) clock, so it must propagate the rate change up the
> clock tree, so that corresponding DIV clocks can actually change their
> divider values. Add CLK_SET_RATE_PARENT flag to corresponding clocks for
> all SPI instances in Exynos850 (spi_0, spi_1 and spi_2) to make it
> possible. This change involves next clocks:
>
> [...]
Applied, thanks!
[1/3] clk: samsung: exynos850: Propagate SPI IPCLK rate change
https://git.kernel.org/krzk/linux/c/67c15187d4910ee353374676d4dddf09d8cb227e
Best regards,
--
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
© 2016 - 2025 Red Hat, Inc.