Re: [PATCH RFC 2/7] clk: samsung: Add definitions of some audio related clocks

Krzysztof Kozlowski
2017-04-22T15:27:05Z

On Fri, Apr 21, 2017 at 07:19:46PM +0200, Sylwester Nawrocki wrote:
> This patch adds missing definitions of mux clocks required for using
> EPLL as the audio subsystem root clock on exynos5420/exynos5422 SoCs.
>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
> drivers/clk/samsung/clk-exynos5420.c | 13 ++++++++-----
> include/dt-bindings/clock/exynos5420.h | 3 +++
> 2 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/clk/samsung/clk-exynos5420.c b/drivers/clk/samsung/clk-exynos5420.c
> index cdc092a..87c711a 100644
> --- a/drivers/clk/samsung/clk-exynos5420.c
> +++ b/drivers/clk/samsung/clk-exynos5420.c
> @@ -477,8 +477,7 @@ static void __init exynos5420_clk_sleep_init(void) {}
> "mout_sclk_mpll", "ff_dout_spll2",
> "mout_sclk_spll", "mout_sclk_epll"};
> PNAME(mout_mau_epll_clk_5800_p) = { "mout_sclk_epll", "mout_sclk_dpll",
> - "mout_sclk_mpll",
> - "ff_dout_spll2" };
> + "mout_sclk_mpll", "ff_dout_spll2" };

Please split up cleaning from adding new features/clocks. While in
cleaning mode, you might also consider fixing some checkpatch errors and
warnings. There are few (without --strict).

> PNAME(mout_group8_5800_p) = { "dout_aclk432_scaler", "dout_sclk_sw" };
> PNAME(mout_group9_5800_p) = { "dout_osc_div", "mout_sw_aclk432_scaler" };
> PNAME(mout_group10_5800_p) = { "dout_aclk432_cam", "dout_sclk_sw" };
> @@ -487,6 +486,7 @@ static void __init exynos5420_clk_sleep_init(void) {}
> PNAME(mout_group13_5800_p) = { "dout_osc_div", "mout_sw_aclkfl1_550_cam" };
> PNAME(mout_group14_5800_p) = { "dout_aclk550_cam", "dout_sclk_sw" };
> PNAME(mout_group15_5800_p) = { "dout_osc_div", "mout_sw_aclk550_cam" };
> +PNAME(mout_group16_5800_p) = { "dout_osc_div", "mout_mau_epll_clk" };
>
> /* fixed rate clocks generated outside the soc */
> static struct samsung_fixed_rate_clock
> @@ -536,8 +536,8 @@ static void __init exynos5420_clk_sleep_init(void) {}
>
> MUX(CLK_MOUT_MX_MSPLL_CCORE, "mout_mx_mspll_ccore",
> mout_mx_mspll_ccore_p, SRC_TOP7, 16, 2),
> - MUX(0, "mout_mau_epll_clk", mout_mau_epll_clk_5800_p, SRC_TOP7,
> - 20, 2),
> + MUX(CLK_MOUT_MAU_EPLL, "mout_mau_epll_clk", mout_mau_epll_clk_5800_p,
> + SRC_TOP7, 20, 2),

How about not changing the indention level of new line? On the other
hand, if you want to change it so maybe align it with opening
parenthesis?

(same in other places below)

Best regards,
Krzysztof