[PATCH 0/4] clk: samsung: introduce nMUX to reparent MUX clocks

[PATCH 0/4] clk: samsung: introduce nMUX to reparent MUX clocks

[Tudor Ambarus]

Introduce nMUX() for MUX clocks that can be reparented. There are MUX
clocks that are dedicated per IP. The reparenting of the MUX is
safe without affecting other IPs.

Follow with 2 patches for GS101 to propagate the USI SPI clock rate. nMUX
is used in GS101 to reparent the USI MUX to oscclk. Tested with USI6 and
USI13 SPI.

Fix USI SPI clock propagation in exynos850 by allowing MUX reparenting
for the dedicated USI MUX clocks. Stop propagating the rate change from
the USI muxes to the common bus dividers (dout_apm_bus and dout_peri_ip).
Other leaf clocks (HSI2C, I3C) are no longer affected on SPI rate
change. USI SPI clock range remains the same. Exynos850 patch is not
tested!

All 4 patches can go as fixes, since they fix the clock rate range for
gs101, and stop affecting other leaf clocks for exynos850. I don't mind
however if K choses to queue these for next.

Tudor Ambarus (4):
  clk: samsung: introduce nMUX for MUX clks that can reparented
  clk: samsung: gs101: propagate PERIC1 USI SPI clock rate
  clk: samsung: gs101: propagate PERIC0 USI SPI clock rate
  clk: samsung: exynos850: fix propagation of SPI IPCLK rate

 drivers/clk/samsung/clk-exynos850.c |  15 +-
 drivers/clk/samsung/clk-gs101.c     | 225 +++++++++++++++-------------
 drivers/clk/samsung/clk.h           |  20 +++
 3 files changed, 147 insertions(+), 113 deletions(-)

-- 
2.44.0.278.ge034bb2e1d-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/4] clk: samsung: introduce nMUX for MUX clks that can reparented

[Tudor Ambarus]

All samsung MUX clocks that are defined with MUX() set the
CLK_SET_RATE_NO_REPARENT flag in __MUX(), which prevents muxes to be
reparented during clk_set_rate.

Introduce nMUX() for MUX clocks that can be reparented. GS101 SoC will use
nMUX as there are MUX clocks which are dedicated per IP, thus
reparenting the MUX will not affect other IPs. Exynos850 can use it too.

Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
 drivers/clk/samsung/clk.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/clk/samsung/clk.h b/drivers/clk/samsung/clk.h
index a763309e6f12..75e2fa8d7da9 100644
--- a/drivers/clk/samsung/clk.h
+++ b/drivers/clk/samsung/clk.h
@@ -146,6 +146,26 @@ struct samsung_mux_clock {
 #define MUX_F(_id, cname, pnames, o, s, w, f, mf)		\
 	__MUX(_id, cname, pnames, o, s, w, f, mf)
 
+/* Used by MUX clocks where reparenting is allowed. */
+#define __nMUX(_id, cname, pnames, o, s, w, f, mf)		\
+	{							\
+		.id		= _id,				\
+		.name		= cname,			\
+		.parent_names	= pnames,			\
+		.num_parents	= ARRAY_SIZE(pnames),		\
+		.flags		= f,				\
+		.offset		= o,				\
+		.shift		= s,				\
+		.width		= w,				\
+		.mux_flags	= mf,				\
+	}
+
+#define nMUX(_id, cname, pnames, o, s, w)			\
+	__nMUX(_id, cname, pnames, o, s, w, 0, 0)
+
+#define nMUX_F(_id, cname, pnames, o, s, w, f, mf)		\
+	__nMUX(_id, cname, pnames, o, s, w, f, mf)
+
 /**
  * struct samsung_div_clock - information about div clock
  * @id: platform specific id of the clock
-- 
2.44.0.278.ge034bb2e1d-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/4] clk: samsung: gs101: propagate PERIC1 USI SPI clock rate

[Tudor Ambarus]

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 MUX/DIV clocks can actually change
their values. Add CLK_SET_RATE_PARENT flag to corresponding clocks for
all USI instances in GS101 PERIC1: USI{0, 9, 10, 11, 12, 13}. This change
involves next clocks:

PERIC1 USI*:

    Clock                              Div range    MUX Selection
    -------------------------------------------------------------------
    gout_peric1_peric1_top0_ipclk_*    -            -
    dout_peric1_usi*_usi               /1..16       -
    mout_peric1_usi*_usi_user          -            {24.5 MHz, 400 MHz}

With input clock of 400 MHz this scheme provides the following IPCLK
rate range, for each USI block:

    PERIC1 USI*:       1.5 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:

    PERIC1 USI_SPI*:   384 KHz ... 49.9 MHz

Which shall be fine for the applications of the SPI bus.

Fixes: 63b4bd1259d9 ("clk: samsung: gs101: add support for cmu_peric1")
Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
 drivers/clk/samsung/clk-gs101.c | 90 ++++++++++++++++++---------------
 1 file changed, 48 insertions(+), 42 deletions(-)

diff --git a/drivers/clk/samsung/clk-gs101.c b/drivers/clk/samsung/clk-gs101.c
index d065e343a85d..ddf2d57eed68 100644
--- a/drivers/clk/samsung/clk-gs101.c
+++ b/drivers/clk/samsung/clk-gs101.c
@@ -3230,47 +3230,53 @@ static const struct samsung_mux_clock peric1_mux_clks[] __initconst = {
 	MUX(CLK_MOUT_PERIC1_I3C_USER,
 	    "mout_peric1_i3c_user", mout_peric1_nonbususer_p,
 	    PLL_CON0_MUX_CLKCMU_PERIC1_I3C_USER, 4, 1),
-	MUX(CLK_MOUT_PERIC1_USI0_USI_USER,
-	    "mout_peric1_usi0_usi_user", mout_peric1_nonbususer_p,
-	    PLL_CON0_MUX_CLKCMU_PERIC1_USI0_USI_USER, 4, 1),
-	MUX(CLK_MOUT_PERIC1_USI10_USI_USER,
-	    "mout_peric1_usi10_usi_user", mout_peric1_nonbususer_p,
-	    PLL_CON0_MUX_CLKCMU_PERIC1_USI10_USI_USER, 4, 1),
-	MUX(CLK_MOUT_PERIC1_USI11_USI_USER,
-	    "mout_peric1_usi11_usi_user", mout_peric1_nonbususer_p,
-	    PLL_CON0_MUX_CLKCMU_PERIC1_USI11_USI_USER, 4, 1),
-	MUX(CLK_MOUT_PERIC1_USI12_USI_USER,
-	    "mout_peric1_usi12_usi_user", mout_peric1_nonbususer_p,
-	    PLL_CON0_MUX_CLKCMU_PERIC1_USI12_USI_USER, 4, 1),
-	MUX(CLK_MOUT_PERIC1_USI13_USI_USER,
-	    "mout_peric1_usi13_usi_user", mout_peric1_nonbususer_p,
-	    PLL_CON0_MUX_CLKCMU_PERIC1_USI13_USI_USER, 4, 1),
-	MUX(CLK_MOUT_PERIC1_USI9_USI_USER,
-	    "mout_peric1_usi9_usi_user", mout_peric1_nonbususer_p,
-	    PLL_CON0_MUX_CLKCMU_PERIC1_USI9_USI_USER, 4, 1),
+	nMUX(CLK_MOUT_PERIC1_USI0_USI_USER,
+	     "mout_peric1_usi0_usi_user", mout_peric1_nonbususer_p,
+	     PLL_CON0_MUX_CLKCMU_PERIC1_USI0_USI_USER, 4, 1),
+	nMUX(CLK_MOUT_PERIC1_USI10_USI_USER,
+	     "mout_peric1_usi10_usi_user", mout_peric1_nonbususer_p,
+	     PLL_CON0_MUX_CLKCMU_PERIC1_USI10_USI_USER, 4, 1),
+	nMUX(CLK_MOUT_PERIC1_USI11_USI_USER,
+	     "mout_peric1_usi11_usi_user", mout_peric1_nonbususer_p,
+	     PLL_CON0_MUX_CLKCMU_PERIC1_USI11_USI_USER, 4, 1),
+	nMUX(CLK_MOUT_PERIC1_USI12_USI_USER,
+	     "mout_peric1_usi12_usi_user", mout_peric1_nonbususer_p,
+	     PLL_CON0_MUX_CLKCMU_PERIC1_USI12_USI_USER, 4, 1),
+	nMUX(CLK_MOUT_PERIC1_USI13_USI_USER,
+	     "mout_peric1_usi13_usi_user", mout_peric1_nonbususer_p,
+	     PLL_CON0_MUX_CLKCMU_PERIC1_USI13_USI_USER, 4, 1),
+	nMUX(CLK_MOUT_PERIC1_USI9_USI_USER,
+	     "mout_peric1_usi9_usi_user", mout_peric1_nonbususer_p,
+	     PLL_CON0_MUX_CLKCMU_PERIC1_USI9_USI_USER, 4, 1),
 };
 
 static const struct samsung_div_clock peric1_div_clks[] __initconst = {
 	DIV(CLK_DOUT_PERIC1_I3C, "dout_peric1_i3c", "mout_peric1_i3c_user",
 	    CLK_CON_DIV_DIV_CLK_PERIC1_I3C, 0, 4),
-	DIV(CLK_DOUT_PERIC1_USI0_USI,
-	    "dout_peric1_usi0_usi", "mout_peric1_usi0_usi_user",
-	    CLK_CON_DIV_DIV_CLK_PERIC1_USI0_USI, 0, 4),
-	DIV(CLK_DOUT_PERIC1_USI10_USI,
-	    "dout_peric1_usi10_usi", "mout_peric1_usi10_usi_user",
-	    CLK_CON_DIV_DIV_CLK_PERIC1_USI10_USI, 0, 4),
-	DIV(CLK_DOUT_PERIC1_USI11_USI,
-	    "dout_peric1_usi11_usi", "mout_peric1_usi11_usi_user",
-	    CLK_CON_DIV_DIV_CLK_PERIC1_USI11_USI, 0, 4),
-	DIV(CLK_DOUT_PERIC1_USI12_USI,
-	    "dout_peric1_usi12_usi", "mout_peric1_usi12_usi_user",
-	    CLK_CON_DIV_DIV_CLK_PERIC1_USI12_USI, 0, 4),
-	DIV(CLK_DOUT_PERIC1_USI13_USI,
-	    "dout_peric1_usi13_usi", "mout_peric1_usi13_usi_user",
-	    CLK_CON_DIV_DIV_CLK_PERIC1_USI13_USI, 0, 4),
-	DIV(CLK_DOUT_PERIC1_USI9_USI,
-	    "dout_peric1_usi9_usi", "mout_peric1_usi9_usi_user",
-	    CLK_CON_DIV_DIV_CLK_PERIC1_USI9_USI, 0, 4),
+	DIV_F(CLK_DOUT_PERIC1_USI0_USI,
+	      "dout_peric1_usi0_usi", "mout_peric1_usi0_usi_user",
+	      CLK_CON_DIV_DIV_CLK_PERIC1_USI0_USI, 0, 4,
+	      CLK_SET_RATE_PARENT, 0),
+	DIV_F(CLK_DOUT_PERIC1_USI10_USI,
+	      "dout_peric1_usi10_usi", "mout_peric1_usi10_usi_user",
+	      CLK_CON_DIV_DIV_CLK_PERIC1_USI10_USI, 0, 4,
+	      CLK_SET_RATE_PARENT, 0),
+	DIV_F(CLK_DOUT_PERIC1_USI11_USI,
+	      "dout_peric1_usi11_usi", "mout_peric1_usi11_usi_user",
+	      CLK_CON_DIV_DIV_CLK_PERIC1_USI11_USI, 0, 4,
+	      CLK_SET_RATE_PARENT, 0),
+	DIV_F(CLK_DOUT_PERIC1_USI12_USI,
+	      "dout_peric1_usi12_usi", "mout_peric1_usi12_usi_user",
+	      CLK_CON_DIV_DIV_CLK_PERIC1_USI12_USI, 0, 4,
+	      CLK_SET_RATE_PARENT, 0),
+	DIV_F(CLK_DOUT_PERIC1_USI13_USI,
+	      "dout_peric1_usi13_usi", "mout_peric1_usi13_usi_user",
+	      CLK_CON_DIV_DIV_CLK_PERIC1_USI13_USI, 0, 4,
+	      CLK_SET_RATE_PARENT, 0),
+	DIV_F(CLK_DOUT_PERIC1_USI9_USI,
+	      "dout_peric1_usi9_usi", "mout_peric1_usi9_usi_user",
+	      CLK_CON_DIV_DIV_CLK_PERIC1_USI9_USI, 0, 4,
+	      CLK_SET_RATE_PARENT, 0),
 };
 
 static const struct samsung_gate_clock peric1_gate_clks[] __initconst = {
@@ -3305,27 +3311,27 @@ static const struct samsung_gate_clock peric1_gate_clks[] __initconst = {
 	GATE(CLK_GOUT_PERIC1_PERIC1_TOP0_IPCLK_1,
 	     "gout_peric1_peric1_top0_ipclk_1", "dout_peric1_usi0_usi",
 	     CLK_CON_GAT_GOUT_BLK_PERIC1_UID_PERIC1_TOP0_IPCLKPORT_IPCLK_1,
-	     21, 0, 0),
+	     21, CLK_SET_RATE_PARENT, 0),
 	GATE(CLK_GOUT_PERIC1_PERIC1_TOP0_IPCLK_2,
 	     "gout_peric1_peric1_top0_ipclk_2", "dout_peric1_usi9_usi",
 	     CLK_CON_GAT_GOUT_BLK_PERIC1_UID_PERIC1_TOP0_IPCLKPORT_IPCLK_2,
-	     21, 0, 0),
+	     21, CLK_SET_RATE_PARENT, 0),
 	GATE(CLK_GOUT_PERIC1_PERIC1_TOP0_IPCLK_3,
 	     "gout_peric1_peric1_top0_ipclk_3", "dout_peric1_usi10_usi",
 	     CLK_CON_GAT_GOUT_BLK_PERIC1_UID_PERIC1_TOP0_IPCLKPORT_IPCLK_3,
-	     21, 0, 0),
+	     21, CLK_SET_RATE_PARENT, 0),
 	GATE(CLK_GOUT_PERIC1_PERIC1_TOP0_IPCLK_4,
 	     "gout_peric1_peric1_top0_ipclk_4", "dout_peric1_usi11_usi",
 	     CLK_CON_GAT_GOUT_BLK_PERIC1_UID_PERIC1_TOP0_IPCLKPORT_IPCLK_4,
-	     21, 0, 0),
+	     21, CLK_SET_RATE_PARENT, 0),
 	GATE(CLK_GOUT_PERIC1_PERIC1_TOP0_IPCLK_5,
 	     "gout_peric1_peric1_top0_ipclk_5", "dout_peric1_usi12_usi",
 	     CLK_CON_GAT_GOUT_BLK_PERIC1_UID_PERIC1_TOP0_IPCLKPORT_IPCLK_5,
-	     21, 0, 0),
+	     21, CLK_SET_RATE_PARENT, 0),
 	GATE(CLK_GOUT_PERIC1_PERIC1_TOP0_IPCLK_6,
 	     "gout_peric1_peric1_top0_ipclk_6", "dout_peric1_usi13_usi",
 	     CLK_CON_GAT_GOUT_BLK_PERIC1_UID_PERIC1_TOP0_IPCLKPORT_IPCLK_6,
-	     21, 0, 0),
+	     21, CLK_SET_RATE_PARENT, 0),
 	GATE(CLK_GOUT_PERIC1_PERIC1_TOP0_IPCLK_8,
 	     "gout_peric1_peric1_top0_ipclk_8", "dout_peric1_i3c",
 	     CLK_CON_GAT_GOUT_BLK_PERIC1_UID_PERIC1_TOP0_IPCLKPORT_IPCLK_8,
-- 
2.44.0.278.ge034bb2e1d-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/4] clk: samsung: gs101: propagate PERIC0 USI SPI clock rate

[Tudor Ambarus]

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 MUX/DIV clocks can actually change
their values. Add CLK_SET_RATE_PARENT flag to corresponding clocks for
all USI instances in GS101 PERIC0: USI{1-8, 14}. This change involves the
following clocks:

PERIC0 USI*:

    Clock                              Div range    MUX Selection
    -------------------------------------------------------------------
    gout_peric0_peric0_top0_ipclk_*    -            -
    dout_peric0_usi*_usi               /1..16       -
    mout_peric0_usi*_usi_user          -            {24.5 MHz, 400 MHz}

With input clock of 400 MHz this scheme provides the following IPCLK
rate range, for each USI block:

    PERIC0 USI*:       1.5 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:

    PERIC0 USI_SPI*:   384 KHz ... 49.9 MHz

Which shall be fine for the applications of the SPI bus.

Fixes: 893f133a040b ("clk: samsung: gs101: add support for cmu_peric0")
Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
 drivers/clk/samsung/clk-gs101.c | 135 +++++++++++++++++---------------
 1 file changed, 72 insertions(+), 63 deletions(-)

diff --git a/drivers/clk/samsung/clk-gs101.c b/drivers/clk/samsung/clk-gs101.c
index ddf2d57eed68..bd3c1b02715b 100644
--- a/drivers/clk/samsung/clk-gs101.c
+++ b/drivers/clk/samsung/clk-gs101.c
@@ -2763,33 +2763,33 @@ static const struct samsung_mux_clock peric0_mux_clks[] __initconst = {
 	MUX(CLK_MOUT_PERIC0_USI0_UART_USER,
 	    "mout_peric0_usi0_uart_user", mout_peric0_usi0_uart_user_p,
 	    PLL_CON0_MUX_CLKCMU_PERIC0_USI0_UART_USER, 4, 1),
-	MUX(CLK_MOUT_PERIC0_USI14_USI_USER,
-	    "mout_peric0_usi14_usi_user", mout_peric0_usi_usi_user_p,
-	    PLL_CON0_MUX_CLKCMU_PERIC0_USI14_USI_USER, 4, 1),
-	MUX(CLK_MOUT_PERIC0_USI1_USI_USER,
-	    "mout_peric0_usi1_usi_user", mout_peric0_usi_usi_user_p,
-	    PLL_CON0_MUX_CLKCMU_PERIC0_USI1_USI_USER, 4, 1),
-	MUX(CLK_MOUT_PERIC0_USI2_USI_USER,
-	    "mout_peric0_usi2_usi_user", mout_peric0_usi_usi_user_p,
-	    PLL_CON0_MUX_CLKCMU_PERIC0_USI2_USI_USER, 4, 1),
-	MUX(CLK_MOUT_PERIC0_USI3_USI_USER,
-	    "mout_peric0_usi3_usi_user", mout_peric0_usi_usi_user_p,
-	    PLL_CON0_MUX_CLKCMU_PERIC0_USI3_USI_USER, 4, 1),
-	MUX(CLK_MOUT_PERIC0_USI4_USI_USER,
-	    "mout_peric0_usi4_usi_user", mout_peric0_usi_usi_user_p,
-	    PLL_CON0_MUX_CLKCMU_PERIC0_USI4_USI_USER, 4, 1),
-	MUX(CLK_MOUT_PERIC0_USI5_USI_USER,
-	    "mout_peric0_usi5_usi_user", mout_peric0_usi_usi_user_p,
-	    PLL_CON0_MUX_CLKCMU_PERIC0_USI5_USI_USER, 4, 1),
-	MUX(CLK_MOUT_PERIC0_USI6_USI_USER,
-	    "mout_peric0_usi6_usi_user", mout_peric0_usi_usi_user_p,
-	    PLL_CON0_MUX_CLKCMU_PERIC0_USI6_USI_USER, 4, 1),
-	MUX(CLK_MOUT_PERIC0_USI7_USI_USER,
-	    "mout_peric0_usi7_usi_user", mout_peric0_usi_usi_user_p,
-	    PLL_CON0_MUX_CLKCMU_PERIC0_USI7_USI_USER, 4, 1),
-	MUX(CLK_MOUT_PERIC0_USI8_USI_USER,
-	    "mout_peric0_usi8_usi_user", mout_peric0_usi_usi_user_p,
-	    PLL_CON0_MUX_CLKCMU_PERIC0_USI8_USI_USER, 4, 1),
+	nMUX(CLK_MOUT_PERIC0_USI14_USI_USER,
+	     "mout_peric0_usi14_usi_user", mout_peric0_usi_usi_user_p,
+	     PLL_CON0_MUX_CLKCMU_PERIC0_USI14_USI_USER, 4, 1),
+	nMUX(CLK_MOUT_PERIC0_USI1_USI_USER,
+	     "mout_peric0_usi1_usi_user", mout_peric0_usi_usi_user_p,
+	     PLL_CON0_MUX_CLKCMU_PERIC0_USI1_USI_USER, 4, 1),
+	nMUX(CLK_MOUT_PERIC0_USI2_USI_USER,
+	     "mout_peric0_usi2_usi_user", mout_peric0_usi_usi_user_p,
+	     PLL_CON0_MUX_CLKCMU_PERIC0_USI2_USI_USER, 4, 1),
+	nMUX(CLK_MOUT_PERIC0_USI3_USI_USER,
+	     "mout_peric0_usi3_usi_user", mout_peric0_usi_usi_user_p,
+	     PLL_CON0_MUX_CLKCMU_PERIC0_USI3_USI_USER, 4, 1),
+	nMUX(CLK_MOUT_PERIC0_USI4_USI_USER,
+	     "mout_peric0_usi4_usi_user", mout_peric0_usi_usi_user_p,
+	     PLL_CON0_MUX_CLKCMU_PERIC0_USI4_USI_USER, 4, 1),
+	nMUX(CLK_MOUT_PERIC0_USI5_USI_USER,
+	     "mout_peric0_usi5_usi_user", mout_peric0_usi_usi_user_p,
+	     PLL_CON0_MUX_CLKCMU_PERIC0_USI5_USI_USER, 4, 1),
+	nMUX(CLK_MOUT_PERIC0_USI6_USI_USER,
+	     "mout_peric0_usi6_usi_user", mout_peric0_usi_usi_user_p,
+	     PLL_CON0_MUX_CLKCMU_PERIC0_USI6_USI_USER, 4, 1),
+	nMUX(CLK_MOUT_PERIC0_USI7_USI_USER,
+	     "mout_peric0_usi7_usi_user", mout_peric0_usi_usi_user_p,
+	     PLL_CON0_MUX_CLKCMU_PERIC0_USI7_USI_USER, 4, 1),
+	nMUX(CLK_MOUT_PERIC0_USI8_USI_USER,
+	     "mout_peric0_usi8_usi_user", mout_peric0_usi_usi_user_p,
+	     PLL_CON0_MUX_CLKCMU_PERIC0_USI8_USI_USER, 4, 1),
 };
 
 static const struct samsung_div_clock peric0_div_clks[] __initconst = {
@@ -2798,33 +2798,42 @@ static const struct samsung_div_clock peric0_div_clks[] __initconst = {
 	DIV(CLK_DOUT_PERIC0_USI0_UART,
 	    "dout_peric0_usi0_uart", "mout_peric0_usi0_uart_user",
 	    CLK_CON_DIV_DIV_CLK_PERIC0_USI0_UART, 0, 4),
-	DIV(CLK_DOUT_PERIC0_USI14_USI,
-	    "dout_peric0_usi14_usi", "mout_peric0_usi14_usi_user",
-	    CLK_CON_DIV_DIV_CLK_PERIC0_USI14_USI, 0, 4),
-	DIV(CLK_DOUT_PERIC0_USI1_USI,
-	    "dout_peric0_usi1_usi", "mout_peric0_usi1_usi_user",
-	    CLK_CON_DIV_DIV_CLK_PERIC0_USI1_USI, 0, 4),
-	DIV(CLK_DOUT_PERIC0_USI2_USI,
-	    "dout_peric0_usi2_usi", "mout_peric0_usi2_usi_user",
-	    CLK_CON_DIV_DIV_CLK_PERIC0_USI2_USI, 0, 4),
-	DIV(CLK_DOUT_PERIC0_USI3_USI,
-	    "dout_peric0_usi3_usi", "mout_peric0_usi3_usi_user",
-	    CLK_CON_DIV_DIV_CLK_PERIC0_USI3_USI, 0, 4),
-	DIV(CLK_DOUT_PERIC0_USI4_USI,
-	    "dout_peric0_usi4_usi", "mout_peric0_usi4_usi_user",
-	    CLK_CON_DIV_DIV_CLK_PERIC0_USI4_USI, 0, 4),
-	DIV(CLK_DOUT_PERIC0_USI5_USI,
-	    "dout_peric0_usi5_usi", "mout_peric0_usi5_usi_user",
-	    CLK_CON_DIV_DIV_CLK_PERIC0_USI5_USI, 0, 4),
-	DIV(CLK_DOUT_PERIC0_USI6_USI,
-	    "dout_peric0_usi6_usi", "mout_peric0_usi6_usi_user",
-	    CLK_CON_DIV_DIV_CLK_PERIC0_USI6_USI, 0, 4),
-	DIV(CLK_DOUT_PERIC0_USI7_USI,
-	    "dout_peric0_usi7_usi", "mout_peric0_usi7_usi_user",
-	    CLK_CON_DIV_DIV_CLK_PERIC0_USI7_USI, 0, 4),
-	DIV(CLK_DOUT_PERIC0_USI8_USI,
-	    "dout_peric0_usi8_usi", "mout_peric0_usi8_usi_user",
-	    CLK_CON_DIV_DIV_CLK_PERIC0_USI8_USI, 0, 4),
+	DIV_F(CLK_DOUT_PERIC0_USI14_USI,
+	      "dout_peric0_usi14_usi", "mout_peric0_usi14_usi_user",
+	      CLK_CON_DIV_DIV_CLK_PERIC0_USI14_USI, 0, 4,
+	      CLK_SET_RATE_PARENT, 0),
+	DIV_F(CLK_DOUT_PERIC0_USI1_USI,
+	      "dout_peric0_usi1_usi", "mout_peric0_usi1_usi_user",
+	      CLK_CON_DIV_DIV_CLK_PERIC0_USI1_USI, 0, 4,
+	      CLK_SET_RATE_PARENT, 0),
+	DIV_F(CLK_DOUT_PERIC0_USI2_USI,
+	      "dout_peric0_usi2_usi", "mout_peric0_usi2_usi_user",
+	      CLK_CON_DIV_DIV_CLK_PERIC0_USI2_USI, 0, 4,
+	      CLK_SET_RATE_PARENT, 0),
+	DIV_F(CLK_DOUT_PERIC0_USI3_USI,
+	      "dout_peric0_usi3_usi", "mout_peric0_usi3_usi_user",
+	      CLK_CON_DIV_DIV_CLK_PERIC0_USI3_USI, 0, 4,
+	      CLK_SET_RATE_PARENT, 0),
+	DIV_F(CLK_DOUT_PERIC0_USI4_USI,
+	      "dout_peric0_usi4_usi", "mout_peric0_usi4_usi_user",
+	      CLK_CON_DIV_DIV_CLK_PERIC0_USI4_USI, 0, 4,
+	      CLK_SET_RATE_PARENT, 0),
+	DIV_F(CLK_DOUT_PERIC0_USI5_USI,
+	      "dout_peric0_usi5_usi", "mout_peric0_usi5_usi_user",
+	      CLK_CON_DIV_DIV_CLK_PERIC0_USI5_USI, 0, 4,
+	      CLK_SET_RATE_PARENT, 0),
+	DIV_F(CLK_DOUT_PERIC0_USI6_USI,
+	      "dout_peric0_usi6_usi", "mout_peric0_usi6_usi_user",
+	      CLK_CON_DIV_DIV_CLK_PERIC0_USI6_USI, 0, 4,
+	      CLK_SET_RATE_PARENT, 0),
+	DIV_F(CLK_DOUT_PERIC0_USI7_USI,
+	      "dout_peric0_usi7_usi", "mout_peric0_usi7_usi_user",
+	      CLK_CON_DIV_DIV_CLK_PERIC0_USI7_USI, 0, 4,
+	      CLK_SET_RATE_PARENT, 0),
+	DIV_F(CLK_DOUT_PERIC0_USI8_USI,
+	      "dout_peric0_usi8_usi", "mout_peric0_usi8_usi_user",
+	      CLK_CON_DIV_DIV_CLK_PERIC0_USI8_USI, 0, 4,
+	      CLK_SET_RATE_PARENT, 0),
 };
 
 static const struct samsung_gate_clock peric0_gate_clks[] __initconst = {
@@ -2857,11 +2866,11 @@ static const struct samsung_gate_clock peric0_gate_clks[] __initconst = {
 	GATE(CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_0,
 	     "gout_peric0_peric0_top0_ipclk_0", "dout_peric0_usi1_usi",
 	     CLK_CON_GAT_GOUT_BLK_PERIC0_UID_PERIC0_TOP0_IPCLKPORT_IPCLK_0,
-	     21, 0, 0),
+	     21, CLK_SET_RATE_PARENT, 0),
 	GATE(CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_1,
 	     "gout_peric0_peric0_top0_ipclk_1", "dout_peric0_usi2_usi",
 	     CLK_CON_GAT_GOUT_BLK_PERIC0_UID_PERIC0_TOP0_IPCLKPORT_IPCLK_1,
-	     21, 0, 0),
+	     21, CLK_SET_RATE_PARENT, 0),
 	GATE(CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_10,
 	     "gout_peric0_peric0_top0_ipclk_10", "dout_peric0_i3c",
 	     CLK_CON_GAT_GOUT_BLK_PERIC0_UID_PERIC0_TOP0_IPCLKPORT_IPCLK_10,
@@ -2889,27 +2898,27 @@ static const struct samsung_gate_clock peric0_gate_clks[] __initconst = {
 	GATE(CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_2,
 	     "gout_peric0_peric0_top0_ipclk_2", "dout_peric0_usi3_usi",
 	     CLK_CON_GAT_GOUT_BLK_PERIC0_UID_PERIC0_TOP0_IPCLKPORT_IPCLK_2,
-	     21, 0, 0),
+	     21, CLK_SET_RATE_PARENT, 0),
 	GATE(CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_3,
 	     "gout_peric0_peric0_top0_ipclk_3", "dout_peric0_usi4_usi",
 	     CLK_CON_GAT_GOUT_BLK_PERIC0_UID_PERIC0_TOP0_IPCLKPORT_IPCLK_3,
-	     21, 0, 0),
+	     21, CLK_SET_RATE_PARENT, 0),
 	GATE(CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_4,
 	     "gout_peric0_peric0_top0_ipclk_4", "dout_peric0_usi5_usi",
 	     CLK_CON_GAT_GOUT_BLK_PERIC0_UID_PERIC0_TOP0_IPCLKPORT_IPCLK_4,
-	     21, 0, 0),
+	     21, CLK_SET_RATE_PARENT, 0),
 	GATE(CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_5,
 	     "gout_peric0_peric0_top0_ipclk_5", "dout_peric0_usi6_usi",
 	     CLK_CON_GAT_GOUT_BLK_PERIC0_UID_PERIC0_TOP0_IPCLKPORT_IPCLK_5,
-	     21, 0, 0),
+	     21, CLK_SET_RATE_PARENT, 0),
 	GATE(CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_6,
 	     "gout_peric0_peric0_top0_ipclk_6", "dout_peric0_usi7_usi",
 	     CLK_CON_GAT_GOUT_BLK_PERIC0_UID_PERIC0_TOP0_IPCLKPORT_IPCLK_6,
-	     21, 0, 0),
+	     21, CLK_SET_RATE_PARENT, 0),
 	GATE(CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_7,
 	     "gout_peric0_peric0_top0_ipclk_7", "dout_peric0_usi8_usi",
 	     CLK_CON_GAT_GOUT_BLK_PERIC0_UID_PERIC0_TOP0_IPCLKPORT_IPCLK_7,
-	     21, 0, 0),
+	     21, CLK_SET_RATE_PARENT, 0),
 	GATE(CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_8,
 	     "gout_peric0_peric0_top0_ipclk_8", "dout_peric0_i3c",
 	     CLK_CON_GAT_GOUT_BLK_PERIC0_UID_PERIC0_TOP0_IPCLKPORT_IPCLK_8,
@@ -2990,7 +2999,7 @@ static const struct samsung_gate_clock peric0_gate_clks[] __initconst = {
 	GATE(CLK_GOUT_PERIC0_PERIC0_TOP1_IPCLK_2,
 	     "gout_peric0_peric0_top1_ipclk_2", "dout_peric0_usi14_usi",
 	     CLK_CON_GAT_GOUT_BLK_PERIC0_UID_PERIC0_TOP1_IPCLKPORT_IPCLK_2,
-	     21, 0, 0),
+	     21, CLK_SET_RATE_PARENT, 0),
 	/* Disabling this clock makes the system hang. Mark the clock as critical. */
 	GATE(CLK_GOUT_PERIC0_PERIC0_TOP1_PCLK_0,
 	     "gout_peric0_peric0_top1_pclk_0", "mout_peric0_bus_user",
-- 
2.44.0.278.ge034bb2e1d-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/4] clk: samsung: exynos850: fix propagation of SPI IPCLK rate

[Tudor Ambarus]

Fix propagation of SPI IPCLK rate by allowing MUX reparenting for the
dedicated USI MUX clocks. Since these muxes feed just the USI blocks,
reparenting of the muxes do not affect other IPs.

Do not propagate the rate change the from USI muxes to the common bus
dividers (dout_apm_bus and dout_peri_ip). The leaf clocks (HSI2C, I3C)
that are derived from the common bus dividers are no longer affected by
the SPI clock rate change.

This change involves the following clock path propagation:

usi_spi_0:
    Clock                  Div range    MUX Selection
    ---------------------------------------------------------------------
    gout_spi0_ipclk        -            -
    dout_peri_spi0         /1..32       -
    mout_peri_spi_user     -            { oscclk (26 MHz), dout_peri_ip }

    *Note that the clock rate is no longer propagated to dout_peri_ip.

usi_cmgp0:

    Clock                  Div range    MUX Selection
    ---------------------------------------------------------------------
    gout_cmgp_usi0_ipclk   -           -
    dout_cmgp_usi0         /1..32      -
    mout_cmgp_usi0         -           { clk_rco_cmgp (49.152 MHz)
                                         gout_clkcmu_cmgp_bus }

    *Note that the clock rate is no longer propagated to
     gout_clkcmu_cmgp_bus and dout_apm_bus.

usi_cmgp1:

    Clock                  Div range   MUX Selection
    ---------------------------------------------------------------------
    gout_cmgp_usi1_ipclk   -           -
    dout_cmgp_usi1         /1..32      -
    mout_cmgp_usi1         -           { clk_rco_cmgp (49.152 MHz)
                                         gout_clkcmu_cmgp_bus }

    *Note that the clock rate is no longer propagated to
     gout_clkcmu_cmgp_bus and dout_apm_bus.

This comes with no significant clock range modification. Before this
patch the claimed clock ranges are:

    SPI0:   200 kHz ... 49.9 MHz
    SPI1/2: 400 kHz ... 49.9 MHz

After this patch the clock ranges are:
    SPI0:   203.125 kHz ... 49.9 MHz
    SPI1/2: 384 kHz     ... 49.9 MHz

For SPI1/2 we get an even lower frequency than what was before. For SPI0
the benefit of not modifying common bus clocks, thus other leaf IP nodes
is greater than the change in frequency from 200 to ~203 KHz.

Not tested, the patch was written solely by reading the code.

Fixes: 67c15187d491 ("clk: samsung: exynos850: Propagate SPI IPCLK rate change")
Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
 drivers/clk/samsung/clk-exynos850.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos850.c b/drivers/clk/samsung/clk-exynos850.c
index 82cfa22c0788..42b4b4075aeb 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, CLK_SET_RATE_PARENT, 0),
+	     CLK_CON_GAT_CLKCMU_CMGP_BUS, 21, 0, 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,10 +974,10 @@ 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_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),
+	nMUX(CLK_MOUT_CMGP_USI0, "mout_cmgp_usi0", mout_cmgp_usi0_p,
+	     CLK_CON_MUX_MUX_CLK_CMGP_USI_CMGP0, 0, 1),
+	nMUX(CLK_MOUT_CMGP_USI1, "mout_cmgp_usi1", mout_cmgp_usi1_p,
+	     CLK_CON_MUX_MUX_CLK_CMGP_USI_CMGP1, 0, 1),
 };
 
 static const struct samsung_div_clock cmgp_div_clks[] __initconst = {
@@ -1557,9 +1557,8 @@ 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_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),
+	nMUX(CLK_MOUT_PERI_SPI_USER, "mout_peri_spi_user",
+	     mout_peri_spi_user_p, PLL_CON0_MUX_CLKCMU_PERI_SPI_USER, 4, 1),
 };
 
 static const struct samsung_div_clock peri_div_clks[] __initconst = {
-- 
2.44.0.278.ge034bb2e1d-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] clk: samsung: exynos850: fix propagation of SPI IPCLK rate

[Sam Protsenko]

On Thu, Feb 29, 2024 at 6:20 AM Tudor Ambarus <tudor.ambarus@linaro.org> wrote:
>
> Fix propagation of SPI IPCLK rate by allowing MUX reparenting for the
> dedicated USI MUX clocks. Since these muxes feed just the USI blocks,
> reparenting of the muxes do not affect other IPs.
>

It was actually done for a reason (at least in case of clk-exynos850
driver). Those top-level MUXes use CLK_SET_RATE_NO_REPARENT flag via
MUX() / MUX_F() macros to avoid re-parenting. See below for the
explanation on why.

> Do not propagate the rate change the from USI muxes to the common bus
> dividers (dout_apm_bus and dout_peri_ip). The leaf clocks (HSI2C, I3C)

The propagation to those dividers was implemented intentionally,
because AFAIU this is precisely their purpose. Not using those for the
derivation of HSI2C/SPI clock rates makes them effectively useless,
which as I understand wasn't the HW designer's intention. It's all
explained in details in the commit message of the patch which adds
this propagation.

> that are derived from the common bus dividers are no longer affected by
> the SPI clock rate change.
>
> This change involves the following clock path propagation:
>
> usi_spi_0:
>     Clock                  Div range    MUX Selection
>     ---------------------------------------------------------------------
>     gout_spi0_ipclk        -            -
>     dout_peri_spi0         /1..32       -
>     mout_peri_spi_user     -            { oscclk (26 MHz), dout_peri_ip }

AFAIK, the OSCCLK only purpose is to be used during suspend (PM
state). When implementing clk-exynos850.c I specifically avoided using
OSCCLK clock for the regular use-cases, and I believe other existing
Exynos clock drivers don't use OSCCLK during normal operation too.
It's easy to see from the clock diagrams in the TRM: all CMUs have
top-level MUXes that have two parents (normal clock and OSCCLK). In
fact, the TRM mentions it:

    "All CMUs have MUXs to change the OSCCLK during power-down mode"

Even if OSCCLK can be used in some cases for driving HW blocks, the
top-level MUXes are not related to those cases.

>
>     *Note that the clock rate is no longer propagated to dout_peri_ip.
>
> usi_cmgp0:
>
>     Clock                  Div range    MUX Selection
>     ---------------------------------------------------------------------
>     gout_cmgp_usi0_ipclk   -           -
>     dout_cmgp_usi0         /1..32      -
>     mout_cmgp_usi0         -           { clk_rco_cmgp (49.152 MHz)

I'm not sure the RCO should be used during normal operation either.
RCO purpose seems to be similar OSCCLK -- to serve as a substitute
clock during suspend, or maybe for calibration/debugging purposes. But
from the clock diagram it's clear they are not intended for the
regular operation. The only difference from OSSCLK is the frequency,
which is usually 49.152 MHz or 24.576 MHz for RCO (basically multiples
of 32,768 Hz), which hints those clocks are designed to drive some 1
Hz (1 sec) based timers.

>                                          gout_clkcmu_cmgp_bus }
>
>     *Note that the clock rate is no longer propagated to
>      gout_clkcmu_cmgp_bus and dout_apm_bus.
>
> usi_cmgp1:
>
>     Clock                  Div range   MUX Selection
>     ---------------------------------------------------------------------
>     gout_cmgp_usi1_ipclk   -           -
>     dout_cmgp_usi1         /1..32      -
>     mout_cmgp_usi1         -           { clk_rco_cmgp (49.152 MHz)
>                                          gout_clkcmu_cmgp_bus }
>
>     *Note that the clock rate is no longer propagated to
>      gout_clkcmu_cmgp_bus and dout_apm_bus.
>
> This comes with no significant clock range modification. Before this
> patch the claimed clock ranges are:
>
>     SPI0:   200 kHz ... 49.9 MHz
>     SPI1/2: 400 kHz ... 49.9 MHz
>
> After this patch the clock ranges are:
>     SPI0:   203.125 kHz ... 49.9 MHz
>     SPI1/2: 384 kHz     ... 49.9 MHz
>

So as I see it, instead of using dividers designed exactly for the
purpose of changing I2C/SPI clock rates this patch instead uses OSCCLK
clock, which is not intended for normal I2C/SPI operation.

> For SPI1/2 we get an even lower frequency than what was before. For SPI0
> the benefit of not modifying common bus clocks, thus other leaf IP nodes
> is greater than the change in frequency from 200 to ~203 KHz.
>
> Not tested, the patch was written solely by reading the code.
>
> Fixes: 67c15187d491 ("clk: samsung: exynos850: Propagate SPI IPCLK rate change")

I fail to see how this patch fixes anything. Instead it looks to me it
replaces the (already) correctly implemented logic with incorrect one.
The SPI clocks are already functional and work exactly as intended
without this patch. The motivation was explained in commit
67c15187d491 ("clk: samsung: exynos850: Propagate SPI IPCLK rate
change"), which was thoroughly tested on E850-96 for all 3 SPI
instances, for all possible DMA/IRQ/polling combinations, with all
possible clock frequencies, and it seems to cover all possible SPI
cases. This patch seems to just change the behavior to something else
without solid examples of how the already implemented scheme (where I
specifically avoided doing what's done in this patch) could be broken
or sub-optimal.

[snip]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] clk: samsung: exynos850: fix propagation of SPI IPCLK rate

[Tudor Ambarus]

Hi, Sam,

On 3/1/24 00:13, Sam Protsenko wrote:
>>     mout_peri_spi_user     -            { oscclk (26 MHz), dout_peri_ip }
> AFAIK, the OSCCLK only purpose is to be used during suspend (PM
> state). When implementing clk-exynos850.c I specifically avoided using
> OSCCLK clock for the regular use-cases, and I believe other existing

Ok.

> Exynos clock drivers don't use OSCCLK during normal operation too.

I saw.

> It's easy to see from the clock diagrams in the TRM: all CMUs have
> top-level MUXes that have two parents (normal clock and OSCCLK). In
> fact, the TRM mentions it:
> 
>     "All CMUs have MUXs to change the OSCCLK during power-down mode"
> 

typo in datasheet, s/the/to, but I get what you're saying.

> Even if OSCCLK can be used in some cases for driving HW blocks, the
> top-level MUXes are not related to those cases.

This is what I'm challenging, yes. Do you know why we can't use oscclk
to drive hw blocks in normal operation mode, i.e. not low power modes?

Since the datasheet does not specify any other usage of oscclk, I think
too that it's safer to not use it to drive HW blocks. So unless someone
else intervenes and clarifies this aspect, please ignore the entire
patch set.

Re-parenting the MUX to oscclk allows the same clock range as before and
with the benefit of not affecting the clock rates of HSI2C/I3C for SPI
clock rates below 500 KHz. This is what I'm trying to fix here, I think
it's not a good idea to allow SPI to modify the clock rate of HSI2C/I3C
at run-time.

How about specifying CLK_SET_RATE_GATE to the common bus divider? It
will prevent SPI from changing the rate while the clock is prepared.
Thus HSI2C/I3C will no longer be affected behind the curtains.

Thanks,
ta

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] clk: samsung: exynos850: fix propagation of SPI IPCLK rate

[Tudor Ambarus]

Hi, Sam!

On 3/1/24 00:13, Sam Protsenko wrote:
> I fail to see how this patch fixes anything. Instead it looks to me it
> replaces the (already) correctly implemented logic with incorrect one.

I opened another thread asking for feedback on whether it's safe to
re-parent the USI MUX to OSCCLK at run-time, find it here:
https://lore.kernel.org/linux-samsung-soc/71df1d6b-f40b-4896-a672-c5f0f526fb1f@linaro.org/T/#m588abb87eb5fd8817d71d06b94c91eb84928e06b

Jaewon came up with the idea on verifying what the downstream clock
driver does. I added some prints in the driver, and indeed the USI MUX
re-parents to OSCCLK on low SPI clock rates in the GS101 case.

Thus I'll respin this patch set fixing GS101 on low USI clock rates by
re-parenting the USI MUX to OSCCLK. I'll leave exynos850 out if I don't
hear back from you, but I think it deserves the same fix. Allowing SPI
to modify the clock rate of HSI2C/I3C at run-time is bad IMO.
Re-parenting the USI MUX to OSCCLK fixes this problem, HSI2C/I3C will no
longer be affected on low SPI clock rates.

Cheers,
ta

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] clk: samsung: exynos850: fix propagation of SPI IPCLK rate

[Sam Protsenko]

On Fri, Mar 22, 2024 at 4:39 AM Tudor Ambarus <tudor.ambarus@linaro.org> wrote:
>
> Hi, Sam!
>
> On 3/1/24 00:13, Sam Protsenko wrote:
> > I fail to see how this patch fixes anything. Instead it looks to me it
> > replaces the (already) correctly implemented logic with incorrect one.
>
> I opened another thread asking for feedback on whether it's safe to
> re-parent the USI MUX to OSCCLK at run-time, find it here:
> https://lore.kernel.org/linux-samsung-soc/71df1d6b-f40b-4896-a672-c5f0f526fb1f@linaro.org/T/#m588abb87eb5fd8817d71d06b94c91eb84928e06b
>
> Jaewon came up with the idea on verifying what the downstream clock
> driver does. I added some prints in the driver, and indeed the USI MUX
> re-parents to OSCCLK on low SPI clock rates in the GS101 case.
>
> Thus I'll respin this patch set fixing GS101 on low USI clock rates by
> re-parenting the USI MUX to OSCCLK. I'll leave exynos850 out if I don't
> hear back from you, but I think it deserves the same fix. Allowing SPI
> to modify the clock rate of HSI2C/I3C at run-time is bad IMO.
> Re-parenting the USI MUX to OSCCLK fixes this problem, HSI2C/I3C will no
> longer be affected on low SPI clock rates.
>

Yes, please leave Exynos850 out of it, if possible. It's fine with me
if you send it for gs101, as it's you who is going to maintain that
platform further, so it's for the maintainers to decide. I'll refrain
from reviewing that particular patch.

For Exynos850 driver, I'm convinced the SPI clock derivation is
already implemented in the correct way (exactly as it was designed in
HW), and doing anything else would be a hack, and frankly this sole
fact is already enough of argumentation for me. There is also the
whole bunch of use-cases which I think could be affected by using
OSCCLK, e.g.: clock signal integrity, runtime PM concerns, possible
interference in case of automatic clock control enablement, etc. I
don't even want to think about all possible pitfalls which
implementation of this non-standard and undocumented behavior could
create. So as the only person who currently supports Exynos850 drivers
(apart from the maintainers, of course), I would strictly oppose this
particular OSCCLK change.

> Cheers,
> ta

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] clk: samsung: exynos850: fix propagation of SPI IPCLK rate

[Tudor Ambarus]

On 3/22/24 18:09, Sam Protsenko wrote:
> Yes, please leave Exynos850 out of it, if possible. 

Sure, Sam, thanks for the feedback!

Cheers,
ta

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel