From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Cc: Magnus Damm <magnus.damm@gmail.com>,
Rob Herring <robh+dt@kernel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
Greg KH <gregkh@linuxfoundation.org>,
Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@vger.kernel.org>,
"open list:SERIAL DRIVERS" <linux-serial@vger.kernel.org>
Subject: Re: [PATCH v2 5/7] clk: renesas: cpg-mssr: Add support for R-Car V4H
Date: Wed, 27 Apr 2022 16:26:09 +0200 [thread overview]
Message-ID: <CAMuHMdUNs+Cv58dvDcG_e1PheSzjFcAPP_QiTYQ-iULTWjZSAA@mail.gmail.com> (raw)
In-Reply-To: <20220425064201.459633-6-yoshihiro.shimoda.uh@renesas.com>
Hi Shimoda-san,
On Mon, Apr 25, 2022 at 8:42 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> Initial CPG support for R-Car V4H (r8a779g0).
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Thanks for your patch!
> --- /dev/null
> +++ b/drivers/clk/renesas/r8a779g0-cpg-mssr.c
> @@ -0,0 +1,217 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * r8a779g0 Clock Pulse Generator / Module Standby and Software Reset
> + *
> + * Copyright (C) 2022 Renesas Electronics Corp.
> + *
> + * Based on r8a779g0-cpg-mssr.c
I.e. based on itself? ;-)
> +static const struct cpg_core_clk r8a779g0_core_clks[] __initconst = {
> + /* External Clock Inputs */
> + DEF_INPUT("extal", CLK_EXTAL),
> + DEF_INPUT("extalr", CLK_EXTALR),
> +
> + /* Internal Core Clocks */
> + DEF_BASE(".main", CLK_MAIN, CLK_TYPE_GEN4_MAIN, CLK_EXTAL),
> + DEF_BASE(".pll1", CLK_PLL1, CLK_TYPE_GEN4_PLL1, CLK_MAIN),
> + DEF_BASE(".pll2", CLK_PLL2, CLK_TYPE_GEN4_PLL2, CLK_MAIN),
> + DEF_BASE(".pll3", CLK_PLL3, CLK_TYPE_GEN4_PLL3, CLK_MAIN),
> + DEF_BASE(".pll4", CLK_PLL4, CLK_TYPE_GEN4_PLL4, CLK_MAIN),
> + DEF_BASE(".pll5", CLK_PLL5, CLK_TYPE_GEN4_PLL5, CLK_MAIN),
> + DEF_BASE(".pll6", CLK_PLL6, CLK_TYPE_GEN4_PLL6, CLK_MAIN),
> +
> + DEF_FIXED(".pll1_div2", CLK_PLL1_DIV2, CLK_PLL1, 2, 1),
> + DEF_FIXED(".pll2_div2", CLK_PLL2_DIV2, CLK_PLL2, 2, 1),
> + DEF_FIXED(".pll3_div2", CLK_PLL3_DIV2, CLK_PLL3, 2, 1),
> + DEF_FIXED(".pll4_div2", CLK_PLL4_DIV2, CLK_PLL4, 2, 1),
> + DEF_FIXED(".pll5_div2", CLK_PLL5_DIV2, CLK_PLL5, 2, 1),
> + DEF_FIXED(".pll5_div4", CLK_PLL5_DIV4, CLK_PLL5_DIV2, 2, 1),
> + DEF_FIXED(".pll6_div2", CLK_PLL6_DIV2, CLK_PLL6, 2, 1),
> + DEF_FIXED(".s0", CLK_S0, CLK_PLL1_DIV2, 2, 1),
> + DEF_FIXED(".s0_vio", CLK_S0_VIO, CLK_PLL1_DIV2, 2, 1),
> + DEF_FIXED(".s0_vc", CLK_S0_VC, CLK_PLL1_DIV2, 2, 1),
> + DEF_FIXED(".s0_hsc", CLK_S0_HSC, CLK_PLL1_DIV2, 2, 1),
> + DEF_FIXED(".sv_vip", CLK_SV_VIP, CLK_PLL1_DIV2, 2, 1),
CLK_SV_VIP runs at 640 instead of 800 MHz, so CLK_PLL1 / 5?
> + DEF_FIXED(".sv_ir", CLK_SV_IR, CLK_PLL1_DIV2, 2, 1),
Likewise for CLK_SV_IR.
> + DEF_BASE(".sdsrc", CLK_SDSRC, CLK_TYPE_GEN4_SDSRC, CLK_PLL5),
> + DEF_RATE(".oco", CLK_OCO, 32768),
> +
> + DEF_BASE(".rpcsrc", CLK_RPCSRC, CLK_TYPE_GEN4_RPCSRC, CLK_PLL5),
> + DEF_BASE(".rpc", R8A779G0_CLK_RPC, CLK_TYPE_GEN4_RPC, CLK_RPCSRC),
"rpc".
> + DEF_BASE("rpcd2", R8A779G0_CLK_RPCD2, CLK_TYPE_GEN4_RPCD2, R8A779G0_CLK_RPC),
Please move "rpc" and "rpcd2" to Core Clock Outputs below,
as they are not Internal Core Clocks.
> + DEF_FIXED(".vio", CLK_VIO, CLK_PLL5_DIV2, 3, 1),
> + DEF_FIXED(".vc", CLK_VC, CLK_PLL5_DIV2, 3, 1),
> +
> + /* Core Clock Outputs */
> + DEF_FIXED("s0d2", R8A779G0_CLK_S0D2, CLK_S0, 2, 1),
> + DEF_FIXED("s0d3", R8A779G0_CLK_S0D3, CLK_S0, 3, 1),
> + DEF_FIXED("s0d4", R8A779G0_CLK_S0D4, CLK_S0, 4, 1),
> + DEF_FIXED("cl16m", R8A779G0_CLK_CL16M, CLK_S0, 48, 1),
> + DEF_FIXED("s0d1_vio", R8A779G0_CLK_S0D1_VIO, CLK_S0_VIO, 1, 1),
> + DEF_FIXED("s0d2_vio", R8A779G0_CLK_S0D2_VIO, CLK_S0_VIO, 2, 1),
> + DEF_FIXED("s0d4_vio", R8A779G0_CLK_S0D4_VIO, CLK_S0_VIO, 4, 1),
> + DEF_FIXED("s0d8_vio", R8A779G0_CLK_S0D8_VIO, CLK_S0_VIO, 8, 1),
> + DEF_FIXED("s0d1_vc", R8A779G0_CLK_S0D1_VC, CLK_S0_VC, 1, 1),
> + DEF_FIXED("s0d2_vc", R8A779G0_CLK_S0D2_VC, CLK_S0_VC, 2, 1),
> + DEF_FIXED("s0d4_vc", R8A779G0_CLK_S0D4_VC, CLK_S0_VC, 4, 1),
> + DEF_FIXED("s0d2_mm", R8A779G0_CLK_S0D2_MM, CLK_S0, 2, 1),
> + DEF_FIXED("s0d4_mm", R8A779G0_CLK_S0D4_MM, CLK_S0, 4, 1),
> + DEF_FIXED("cl16m_mm", R8A779G0_CLK_CL16M_MM, CLK_S0, 48, 1),
> + DEF_FIXED("s0d2_u3dg", R8A779G0_CLK_S0D2_U3DG, CLK_S0, 2, 1),
> + DEF_FIXED("s0d4_u3dg", R8A779G0_CLK_S0D4_U3DG, CLK_S0, 4, 1),
> + DEF_FIXED("s0d2_rt", R8A779G0_CLK_S0D2_RT, CLK_S0, 2, 1),
> + DEF_FIXED("s0d3_rt", R8A779G0_CLK_S0D3_RT, CLK_S0, 3, 1),
> + DEF_FIXED("s0d4_rt", R8A779G0_CLK_S0D4_RT, CLK_S0, 4, 1),
> + DEF_FIXED("s0d6_rt", R8A779G0_CLK_S0D6_RT, CLK_S0, 6, 1),
> + DEF_FIXED("s0d24_rt", R8A779G0_CLK_S0D24_RT, CLK_S0, 24, 1),
> + DEF_FIXED("cl16m_rt", R8A779G0_CLK_CL16M_RT, CLK_S0, 48, 1),
> + DEF_FIXED("s0d2_per", R8A779G0_CLK_S0D2_PER, CLK_S0, 2, 1),
> + DEF_FIXED("s0d3_per", R8A779G0_CLK_S0D3_PER, CLK_S0, 3, 1),
Missing "s0d4_per".
> + DEF_FIXED("s0d6_per", R8A779G0_CLK_S0D6_PER, CLK_S0, 6, 1),
> + DEF_FIXED("s0d12_per", R8A779G0_CLK_S0D12_PER, CLK_S0, 12, 1),
> + DEF_FIXED("s0d24_per", R8A779G0_CLK_S0D24_PER, CLK_S0, 24, 1),
> + DEF_FIXED("cl16m_per", R8A779G0_CLK_CL16M_PER, CLK_S0, 48, 1),
> + DEF_FIXED("s0d1_hsc", R8A779G0_CLK_S0D1_HSC, CLK_S0_HSC, 1, 1),
> + DEF_FIXED("s0d2_hsc", R8A779G0_CLK_S0D2_HSC, CLK_S0_HSC, 2, 1),
> + DEF_FIXED("s0d4_hsc", R8A779G0_CLK_S0D4_HSC, CLK_S0_HSC, 4, 1),
> + DEF_FIXED("cl16m_hsc", R8A779G0_CLK_CL16M_HSC, CLK_S0_HSC, 48, 1),
> + DEF_FIXED("s0d2_cc", R8A779G0_CLK_S0D2_CC, CLK_S0, 2, 1),
> + DEF_FIXED("svd1_ir", R8A779G0_CLK_SVD1_IR, CLK_SV_IR, 1, 1),
> + DEF_FIXED("svd2_ir", R8A779G0_CLK_SVD2_IR, CLK_SV_IR, 2, 1),
> + DEF_FIXED("svd1_vip", R8A779G0_CLK_SVD1_VIP, CLK_SV_VIP, 1, 1),
> + DEF_FIXED("svd2_vip", R8A779G0_CLK_SVD2_VIP, CLK_SV_VIP, 2, 1),
> + DEF_FIXED("s0d2_cc", R8A779G0_CLK_S0D2_CC, CLK_S0, 2, 1),
"s0d2_cc" is already defined 5 lines above.
> + DEF_FIXED("cbfusa", R8A779G0_CLK_CBFUSA, CLK_EXTAL, 2, 1),
> + DEF_FIXED("cpex", R8A779G0_CLK_CPEX, CLK_EXTAL, 2, 1),
> + DEF_FIXED("viobus", R8A779G0_CLK_VIOBUS, CLK_VIO, 1, 1),
> + DEF_FIXED("viobusd2", R8A779G0_CLK_VIOBUSD2, CLK_VIO, 2, 1),
> + DEF_FIXED("vcbus", R8A779G0_CLK_VCBUS, CLK_VC, 1, 1),
> + DEF_FIXED("vcbusd2", R8A779G0_CLK_VCBUSD2, CLK_VC, 2, 1),
> +
> + DEF_GEN4_SD("sd0", R8A779G0_CLK_SD0, CLK_SDSRC, 0x870),
> + DEF_DIV6P1("mso", R8A779G0_CLK_MSO, CLK_PLL5_DIV4, 0x87c),
> +
> + DEF_GEN4_OSC("osc", R8A779G0_CLK_OSC, CLK_EXTAL, 8),
> + DEF_GEN4_MDSEL("r", R8A779G0_CLK_R, 29, CLK_EXTALR, 1, CLK_OCO, 1),
> +};
> +
> +static const struct mssr_mod_clk r8a779g0_mod_clks[] __initconst = {
> + DEF_MOD("hscif0", 514, R8A779G0_CLK_S0D3_PER),
> + DEF_MOD("hscif1", 515, R8A779G0_CLK_S0D3_PER),
> + DEF_MOD("hscif2", 516, R8A779G0_CLK_S0D3_PER),
> + DEF_MOD("hscif3", 517, R8A779G0_CLK_S0D3_PER),
> +};
> +
> +/*
> + * CPG Clock Data
> + */
> +/*
> + * MD EXTAL PLL1 PLL2 PLL3 PLL4 PLL5 PLL6 OSC
> + * 14 13 (MHz)
> + * ----------------------------------------------------------------
You may want to make the horizontal line longer.
> + * 0 0 16.66 / 1 x192 x204 x192 x144 x192 x168 /8
> + * 0 1 20 / 1 x160 x170 x160 x120 x160 x140 /8
> + * 1 0 Prohibited setting
> + * 1 1 33.33 / 2 x192 x204 x192 x144 x192 x168 /8
The last column values (OSC) should be /15, /19, resp. /38.
> + */
> +#define CPG_PLL_CONFIG_INDEX(md) ((((md) & BIT(14)) >> 13) | \
> + (((md) & BIT(13)) >> 13))
> +
> +static const struct rcar_gen4_cpg_pll_config cpg_pll_configs[4] = {
> + /* EXTAL div PLL1 mult/div PLL2 mult/div PLL3 mult/div PLL4 mult/div PLL5 mult/div PLL6 mult/div OSC prediv */
> + { 1, 192, 1, 204, 1, 192, 1, 144, 1, 192, 1, 168, 1, 8, },
> + { 1, 160, 1, 170, 1, 160, 1, 120, 1, 160, 1, 140, 1, 8, },
> + { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, },
> + { 2, 192, 1, 204, 1, 192, 1, 144, 1, 192, 1, 168, 1, 8, },
The last column values should be 15, 19, 0, resp. 38.
The rest LGTM.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
next prev parent reply other threads:[~2022-04-27 14:26 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-25 6:41 [PATCH v2 0/7] treewide: Initial support for R-Car V4H Yoshihiro Shimoda
2022-04-25 6:41 ` [PATCH v2 1/7] dt-bindings: power: Add r8a779g0 SYSC power domain definitions Yoshihiro Shimoda
2022-04-25 6:41 ` [PATCH v2 2/7] dt-bindings: clock: Add r8a779g0 CPG Core Clock Definitions Yoshihiro Shimoda
2022-04-25 8:26 ` Geert Uytterhoeven
2022-04-25 18:39 ` Krzysztof Kozlowski
2022-04-25 6:41 ` [PATCH v2 3/7] dt-bindings: serial: renesas,hscif: Document r8a779g0 bindings Yoshihiro Shimoda
2022-04-25 8:13 ` Geert Uytterhoeven
2022-04-25 6:41 ` [PATCH v2 4/7] clk: renesas: rcar-gen4: Add CLK_TYPE_GEN4_PLL4 Yoshihiro Shimoda
2022-04-25 14:48 ` Geert Uytterhoeven
2022-04-25 6:41 ` [PATCH v2 5/7] clk: renesas: cpg-mssr: Add support for R-Car V4H Yoshihiro Shimoda
2022-04-27 14:26 ` Geert Uytterhoeven [this message]
2022-04-28 7:16 ` Yoshihiro Shimoda
2022-04-25 6:42 ` [PATCH v2 6/7] arm64: dts: renesas: Add Renesas R8A779G0 SoC support Yoshihiro Shimoda
2022-04-27 14:37 ` Geert Uytterhoeven
2022-04-28 7:27 ` Yoshihiro Shimoda
2022-04-25 6:42 ` [PATCH v2 7/7] arm64: dts: renesas: Add Renesas White Hawk boards support Yoshihiro Shimoda
2022-04-27 14:52 ` Geert Uytterhoeven
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAMuHMdUNs+Cv58dvDcG_e1PheSzjFcAPP_QiTYQ-iULTWjZSAA@mail.gmail.com \
--to=geert@linux-m68k.org \
--cc=devicetree@vger.kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
--cc=magnus.damm@gmail.com \
--cc=robh+dt@kernel.org \
--cc=yoshihiro.shimoda.uh@renesas.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).