lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CADrjBPqxbgSxEGdor059eGXfpmE4O4e49EroXkpRW0vZEAqGJA@mail.gmail.com>
Date: Thu, 28 Mar 2024 11:33:35 +0000
From: Peter Griffin <peter.griffin@...aro.org>
To: Tudor Ambarus <tudor.ambarus@...aro.org>
Cc: krzysztof.kozlowski@...aro.org, alim.akhtar@...sung.com, 
	linux-arm-kernel@...ts.infradead.org, linux-samsung-soc@...r.kernel.org, 
	linux-kernel@...r.kernel.org, andre.draszik@...aro.org, 
	willmcvicker@...gle.com, kernel-team@...roid.com, s.nawrocki@...sung.com, 
	cw00.choi@...sung.com, mturquette@...libre.com, sboyd@...nel.org, 
	semen.protsenko@...aro.org, linux-clk@...r.kernel.org, 
	jaewon02.kim@...sung.com
Subject: Re: [PATCH v2 2/3] clk: samsung: gs101: propagate PERIC1 USI SPI
 clock rate

Hi Tudor,

On Tue, 26 Mar 2024 at 17:28, Tudor Ambarus <tudor.ambarus@...aro.org> 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 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 the following 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.
>
> Note that with this we allow the reparenting of the MUX_USIx clocks to
> OSCCLK. Each instance of the USI IP has its own MUX_USI clock, thus the
> reparenting of a MUX_USI clock corresponds to a single instance of the
> USI IP. The datasheet mentions OSCCLK just in the low-power mode
> context, but the downstream driver reparents too the MUX_USI clocks to
> OSCCLK. Follow the downstream driver and do the same.
>
> Fixes: 63b4bd1259d9 ("clk: samsung: gs101: add support for cmu_peric1")
> Signed-off-by: Tudor Ambarus <tudor.ambarus@...aro.org>
> ---

Thanks Tudor for looking into this! I agree with the approach outlined above.
Given that
1) Samsung engineers OK'd it
2) Downstream clock driver does it (and also has other features
enabled like automatic clock gating enabled, which implies it should
not cause any issues there if we enable it in the future upstream).
3) We don't want to change clock frequencies of other IP instances

Reviewed-by: Peter Griffin <peter.griffin@...aro.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.396.g6e790dbe36-goog
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ