| Message ID | 20211116074130.107554-1-yoshihiro.shimoda.uh@renesas.com |
|---|---|
| Headers | show |
| Series | treewide: Initial support for R-Car S4-8 | expand |
Hi Shimoda-san, On Tue, Nov 16, 2021 at 8:42 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > Add support for identifying the R-Car S4-8 (R8A779F0) SoC. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Thanks for your patch! > --- a/drivers/soc/renesas/Kconfig > +++ b/drivers/soc/renesas/Kconfig > @@ -256,6 +256,13 @@ config ARCH_R8A779A0 > help > This enables support for the Renesas R-Car V3U SoC. > > +config ARCH_R8A779F0 > + bool "ARM64 Platform support for R-Car S4-8" > + select ARCH_RCAR_GEN3 > + select SYSC_R8A779F0 > + help > + This enables support for the Renesas R-Car S4-8 SoC. > + Please keep sort order (alphabetical, not by part number). > config ARCH_R8A774C0 > bool "ARM64 Platform support for RZ/G2E" > select ARCH_RCAR_GEN3 > diff --git a/drivers/soc/renesas/renesas-soc.c b/drivers/soc/renesas/renesas-soc.c > index 7961b0be1850..857a42a82747 100644 > --- a/drivers/soc/renesas/renesas-soc.c > +++ b/drivers/soc/renesas/renesas-soc.c > @@ -33,6 +33,11 @@ static const struct renesas_family fam_rcar_gen3 __initconst __maybe_unused = { > .reg = 0xfff00044, /* PRR (Product Register) */ > }; > > +static const struct renesas_family fam_rcar_gen4 __initconst __maybe_unused = { > + .name = "R-Car Gen4", > + .reg = 0xfff00044, /* PRR (Product Register) */ Please drop ".reg", which is only meant for existing SoCs. For new SoCs, we rely on the presence of a "renesas,prr" node in DT. > +}; The rest looks good to me. 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
Hi Shimoda-san, On Tue, Nov 16, 2021 at 8:42 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > Add support for identifying the R-Car S4-8 (R8A779F0) SoC. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Thanks for your patch! > --- a/drivers/soc/renesas/Kconfig > +++ b/drivers/soc/renesas/Kconfig > @@ -256,6 +256,13 @@ config ARCH_R8A779A0 > help > This enables support for the Renesas R-Car V3U SoC. > > +config ARCH_R8A779F0 > + bool "ARM64 Platform support for R-Car S4-8" > + select ARCH_RCAR_GEN3 > + select SYSC_R8A779F0 "SYSC_R8A779F0" is only defined in [PATCH 10/16], so you may want to reorder your series. Or I will while applying later ;-) > + help > + This enables support for the Renesas R-Car S4-8 SoC. > + > config ARCH_R8A774C0 > bool "ARM64 Platform support for RZ/G2E" > select ARCH_RCAR_GEN3 > diff --git a/drivers/soc/renesas/renesas-soc.c b/drivers/soc/renesas/renesas-soc.c > index 7961b0be1850..857a42a82747 100644 > --- a/drivers/soc/renesas/renesas-soc.c > +++ b/drivers/soc/renesas/renesas-soc.c > @@ -33,6 +33,11 @@ static const struct renesas_family fam_rcar_gen3 __initconst __maybe_unused = { > .reg = 0xfff00044, /* PRR (Product Register) */ > }; > > +static const struct renesas_family fam_rcar_gen4 __initconst __maybe_unused = { > + .name = "R-Car Gen4", > + .reg = 0xfff00044, /* PRR (Product Register) */ > +}; > + > static const struct renesas_family fam_rmobile __initconst __maybe_unused = { > .name = "R-Mobile", > .reg = 0xe600101c, /* CCCR (Common Chip Code Register) */ > @@ -214,6 +219,11 @@ static const struct renesas_soc soc_rcar_v3u __initconst __maybe_unused = { > .id = 0x59, > }; > > +static const struct renesas_soc soc_rcar_s4 __initconst __maybe_unused = { > + .family = &fam_rcar_gen4, > + .id = 0x5a, > +}; > + > static const struct renesas_soc soc_shmobile_ag5 __initconst __maybe_unused = { > .family = &fam_shmobile, > .id = 0x37, > @@ -319,6 +329,9 @@ static const struct of_device_id renesas_socs[] __initconst = { > #ifdef CONFIG_ARCH_R8A779A0 > { .compatible = "renesas,r8a779a0", .data = &soc_rcar_v3u }, > #endif > +#ifdef CONFIG_ARCH_R8A779F0 > + { .compatible = "renesas,r8a779f0", .data = &soc_rcar_s4 }, > +#endif > #if defined(CONFIG_ARCH_R9A07G044) > { .compatible = "renesas,r9a07g044", .data = &soc_rz_g2l }, > #endif > -- > 2.25.1 >
Hi Shimoda-san, On Tue, Nov 16, 2021 at 8:42 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > Add support for R-Car S4-8 (R8A779F0) to the R-Car RST driver. > The register map of R-Car S4-8 is the same as R-Car V3U so that > renames "V3U" and "r8a779a0" with "Gen4". > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Thanks for your patch! > --- a/drivers/soc/renesas/rcar-rst.c > +++ b/drivers/soc/renesas/rcar-rst.c > @@ -12,7 +12,7 @@ > > #define WDTRSTCR_RESET 0xA55A0002 > #define WDTRSTCR 0x0054 > -#define V3U_WDTRSTCR 0x0010 > +#define GEN4_WDTRSTCR 0x0010 V3U_WDTRSTCR handling is not present upstream, as it should be handled by the boot loader (ATF?), like on other R-Car Gen3 SoCs. Likewise, GEN4_WDTRSTCR should not become present upstream. So please split this in two patches: 1. A patch against upstream, just adding basic R-Car S4-8 support, 2. An optional second patch to enable GEN4_WDTRSTCR in renesas-drivers, to serve as an interim solution until the bootloader is fixed. 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
On Tue, Nov 16, 2021 at 8:42 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > Add serial support for R-Car Gen4 SoC. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> 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
Hi Geert-san, Thank you for your review1 > From: Geert Uytterhoeven, Sent: Friday, November 19, 2021 4:04 AM > > Hi Shimoda-san, > > On Tue, Nov 16, 2021 at 8:42 AM Yoshihiro Shimoda > <yoshihiro.shimoda.uh@renesas.com> wrote: > > Add support for identifying the R-Car S4-8 (R8A779F0) SoC. > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > Thanks for your patch! > > > --- a/drivers/soc/renesas/Kconfig > > +++ b/drivers/soc/renesas/Kconfig > > @@ -256,6 +256,13 @@ config ARCH_R8A779A0 > > help > > This enables support for the Renesas R-Car V3U SoC. > > > > +config ARCH_R8A779F0 > > + bool "ARM64 Platform support for R-Car S4-8" > > + select ARCH_RCAR_GEN3 > > + select SYSC_R8A779F0 > > + help > > + This enables support for the Renesas R-Car S4-8 SoC. > > + > > Please keep sort order (alphabetical, not by part number). I'm sorry. I didn't realized that. I'll fix it in v2. > > config ARCH_R8A774C0 > > bool "ARM64 Platform support for RZ/G2E" > > select ARCH_RCAR_GEN3 > > diff --git a/drivers/soc/renesas/renesas-soc.c b/drivers/soc/renesas/renesas-soc.c > > index 7961b0be1850..857a42a82747 100644 > > --- a/drivers/soc/renesas/renesas-soc.c > > +++ b/drivers/soc/renesas/renesas-soc.c > > @@ -33,6 +33,11 @@ static const struct renesas_family fam_rcar_gen3 __initconst __maybe_unused = { > > .reg = 0xfff00044, /* PRR (Product Register) */ > > }; > > > > +static const struct renesas_family fam_rcar_gen4 __initconst __maybe_unused = { > > + .name = "R-Car Gen4", > > + .reg = 0xfff00044, /* PRR (Product Register) */ > > Please drop ".reg", which is only meant for existing SoCs. > For new SoCs, we rely on the presence of a "renesas,prr" node in DT. I got it. I'll fix it in v2. Best regards, Yoshihiro Shimoda
Hi Geert-san, Thank you for your review! > From: Geert Uytterhoeven, Sent: Friday, November 19, 2021 4:05 AM > > Hi Shimoda-san, > > On Tue, Nov 16, 2021 at 8:42 AM Yoshihiro Shimoda > <yoshihiro.shimoda.uh@renesas.com> wrote: > > Add support for identifying the R-Car S4-8 (R8A779F0) SoC. > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > Thanks for your patch! > > > --- a/drivers/soc/renesas/Kconfig > > +++ b/drivers/soc/renesas/Kconfig > > @@ -256,6 +256,13 @@ config ARCH_R8A779A0 > > help > > This enables support for the Renesas R-Car V3U SoC. > > > > +config ARCH_R8A779F0 > > + bool "ARM64 Platform support for R-Car S4-8" > > + select ARCH_RCAR_GEN3 > > + select SYSC_R8A779F0 > > "SYSC_R8A779F0" is only defined in [PATCH 10/16], so you may want to > reorder your series. Or I will while applying later ;-) Oops. Thank you for pointed it out. I'll reorder my series on v2. Best regards, Yoshihiro Shimoda
Hi Geert-san, Thank you for your review! > From: Geert Uytterhoeven, Sent: Friday, November 19, 2021 4:11 AM > > Hi Shimoda-san, > > On Tue, Nov 16, 2021 at 8:42 AM Yoshihiro Shimoda > <yoshihiro.shimoda.uh@renesas.com> wrote: > > Add support for R-Car S4-8 (R8A779F0) to the R-Car RST driver. > > The register map of R-Car S4-8 is the same as R-Car V3U so that > > renames "V3U" and "r8a779a0" with "Gen4". > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > Thanks for your patch! > > > --- a/drivers/soc/renesas/rcar-rst.c > > +++ b/drivers/soc/renesas/rcar-rst.c > > @@ -12,7 +12,7 @@ > > > > #define WDTRSTCR_RESET 0xA55A0002 > > #define WDTRSTCR 0x0054 > > -#define V3U_WDTRSTCR 0x0010 > > +#define GEN4_WDTRSTCR 0x0010 > > V3U_WDTRSTCR handling is not present upstream, as it should be > handled by the boot loader (ATF?), like on other R-Car Gen3 SoCs. > Likewise, GEN4_WDTRSTCR should not become present upstream. I'm sorry. I made this patch on renesas-driver and I didn't realized this V3U_WDTRSTCR handling is not present upstream. > So please split this in two patches: > 1. A patch against upstream, just adding basic R-Car S4-8 support, > 2. An optional second patch to enable GEN4_WDTRSTCR in > renesas-drivers, to serve as an interim solution until the > bootloader is fixed. I got it. I'll make the 1. patch in v2 at this time. Best regards, Yoshihiro Shimoda
Hi Shimoda-san, Thanks for your patch! On Tue, Nov 16, 2021 at 8:42 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > Add support for R-Car S4-8 (R8A779F0) SoC power areas and register > access. This register specification is similar with R-Car V3U. similar to > So, introduces rcar-gen4-sysc.c for both V3U and S4-8. introduce. That makes perfect sense, as "the R-Car V3U SoC is based on the R-Car Gen 4 architecture". (https://www.renesas.com/us/en/products/automotive-products/automotive-system-chips-socs/r-car-v3u-best-class-r-car-v3u-asil-d-system-chip-automated-driving) > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > --- a/drivers/soc/renesas/Kconfig > +++ b/drivers/soc/renesas/Kconfig > @@ -359,6 +359,9 @@ config SYSC_R8A77970 > config SYSC_R8A779A0 > bool "System Controller support for R-Car V3U" if COMPILE_TEST > > +config SYSC_R8A779F0 > + bool "System Controller support for R-Car S4-8" if COMPILE_TEST > + Please retain sort order (alphabetically). > config SYSC_RMOBILE > bool "System Controller support for R-Mobile" if COMPILE_TEST > > diff --git a/drivers/soc/renesas/Makefile b/drivers/soc/renesas/Makefile > index 9b29bed2a597..f6c5f8c3818c 100644 > --- a/drivers/soc/renesas/Makefile > +++ b/drivers/soc/renesas/Makefile > @@ -25,11 +25,12 @@ obj-$(CONFIG_SYSC_R8A77980) += r8a77980-sysc.o > obj-$(CONFIG_SYSC_R8A77990) += r8a77990-sysc.o > obj-$(CONFIG_SYSC_R8A77995) += r8a77995-sysc.o > obj-$(CONFIG_SYSC_R8A779A0) += r8a779a0-sysc.o > +obj-$(CONFIG_SYSC_R8A779F0) += r8a779f0-sysc.o > ifdef CONFIG_SMP > obj-$(CONFIG_ARCH_R9A06G032) += r9a06g032-smp.o > endif > > # Family > obj-$(CONFIG_RST_RCAR) += rcar-rst.o > -obj-$(CONFIG_SYSC_RCAR) += rcar-sysc.o > +obj-$(CONFIG_SYSC_RCAR) += rcar-sysc.o rcar-gen4-sysc.o This means all R-Car kernels will always include support for both R-Car Gen1/2/3 and R-Car Gen4. I think this should be split. The rest looks good to me, but I think it wouldn't hurt to split this patch in two parts: one patch to generalize r8a779a0-sysc.c for R-Car Gen4, and a second patch to introduce support for R-Car S4-8. 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
Hi Geert-san, > From: Geert Uytterhoeven, Sent: Wednesday, November 24, 2021 1:18 AM > > Hi Shimoda-san, > > Thanks for your patch! > > On Tue, Nov 16, 2021 at 8:42 AM Yoshihiro Shimoda > <yoshihiro.shimoda.uh@renesas.com> wrote: > > Add support for R-Car S4-8 (R8A779F0) SoC power areas and register > > access. This register specification is similar with R-Car V3U. > > similar to > > > So, introduces rcar-gen4-sysc.c for both V3U and S4-8. > > introduce. I'll fix these works. > That makes perfect sense, as "the R-Car V3U SoC is based on the R-Car > Gen 4 architecture". > (https://www.renesas.com/us/en/products/automotive-products/automotive-system-chips-socs/r-car-v3u-best-class-r-car- > v3u-asil-d-system-chip-automated-driving) I got it. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > > --- a/drivers/soc/renesas/Kconfig > > +++ b/drivers/soc/renesas/Kconfig > > @@ -359,6 +359,9 @@ config SYSC_R8A77970 > > config SYSC_R8A779A0 > > bool "System Controller support for R-Car V3U" if COMPILE_TEST > > > > +config SYSC_R8A779F0 > > + bool "System Controller support for R-Car S4-8" if COMPILE_TEST > > + > > Please retain sort order (alphabetically). Oops. I'll fix it on v2. > > config SYSC_RMOBILE > > bool "System Controller support for R-Mobile" if COMPILE_TEST > > > > diff --git a/drivers/soc/renesas/Makefile b/drivers/soc/renesas/Makefile > > index 9b29bed2a597..f6c5f8c3818c 100644 > > --- a/drivers/soc/renesas/Makefile > > +++ b/drivers/soc/renesas/Makefile > > @@ -25,11 +25,12 @@ obj-$(CONFIG_SYSC_R8A77980) += r8a77980-sysc.o > > obj-$(CONFIG_SYSC_R8A77990) += r8a77990-sysc.o > > obj-$(CONFIG_SYSC_R8A77995) += r8a77995-sysc.o > > obj-$(CONFIG_SYSC_R8A779A0) += r8a779a0-sysc.o > > +obj-$(CONFIG_SYSC_R8A779F0) += r8a779f0-sysc.o > > ifdef CONFIG_SMP > > obj-$(CONFIG_ARCH_R9A06G032) += r9a06g032-smp.o > > endif > > > > # Family > > obj-$(CONFIG_RST_RCAR) += rcar-rst.o > > -obj-$(CONFIG_SYSC_RCAR) += rcar-sysc.o > > +obj-$(CONFIG_SYSC_RCAR) += rcar-sysc.o rcar-gen4-sysc.o > > This means all R-Car kernels will always include support for both > R-Car Gen1/2/3 and R-Car Gen4. > I think this should be split. > > The rest looks good to me, but I think it wouldn't hurt to split this > patch in two parts: one patch to generalize r8a779a0-sysc.c for R-Car > Gen4, and a second patch to introduce support for R-Car S4-8. I got it. I'll split this patch in two parts in v2. Best regards, Yoshihiro Shimoda
Hi Shimoda-san, Thanks for your patch! On Tue, Nov 16, 2021 at 8:42 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > Initial support for R-Car S4-8 (r8a779f0), including core, module > clocks, resets, and register access, because register specification > differs from R-Car Gen2/3. The register layout of V3U is a similar > with R-Car S4-8 so that renames CLK_REG_LAYOUT_RCAR_V3U as > CLK_REG_LAYOUT_RCAR_GEN4. However, PLL names differ between V3U > and S4-8. This is a small difference, so I think more code can be shared between R-Car V3U and S4-8 (see below). > Inspired by patches in the BSP by LUU HOAI. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > drivers/clk/renesas/Kconfig | 10 ++ > drivers/clk/renesas/Makefile | 2 + > drivers/clk/renesas/r8a779a0-cpg-mssr.c | 2 +- > drivers/clk/renesas/r8a779f0-cpg-mssr.c | 188 ++++++++++++++++++++++++ > drivers/clk/renesas/rcar-gen4-cpg.c | 141 ++++++++++++++++++ > drivers/clk/renesas/rcar-gen4-cpg.h | 76 ++++++++++ > drivers/clk/renesas/renesas-cpg-mssr.c | 42 ++++-- > drivers/clk/renesas/renesas-cpg-mssr.h | 3 +- > 8 files changed, 448 insertions(+), 16 deletions(-) > create mode 100644 drivers/clk/renesas/r8a779f0-cpg-mssr.c > create mode 100644 drivers/clk/renesas/rcar-gen4-cpg.c > create mode 100644 drivers/clk/renesas/rcar-gen4-cpg.h Just like for the SYSC driver, I think it wouldn't hurt to split this patch in two parts: one patch to generalize r8a779a0-cpg-mssr.c for R-Car Gen4, and a second patch to introduce support for R-Car S4-8. > +static const struct cpg_core_clk r8a779f0_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(".pll3", CLK_PLL3, CLK_TYPE_GEN4_PLL3, CLK_MAIN), > + DEF_BASE(".pll2", CLK_PLL2, CLK_TYPE_GEN4_PLL2, CLK_MAIN), > + DEF_BASE(".pll6", CLK_PLL6, CLK_TYPE_GEN4_PLL6, CLK_MAIN), > + DEF_BASE(".pll5", CLK_PLL5, CLK_TYPE_GEN4_PLL5, CLK_MAIN), Please sort PLLn by index. > + > + 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(".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(".sdsrc", CLK_SDSRC, CLK_PLL5_DIV2, 2, 1), This relies on the default setting of the SD-IF0 Clock Frequency Control Register 1 (SD0CKCR1)? > + DEF_RATE(".oco", CLK_OCO, 32768), > + > + DEF_BASE(".rpcsrc", CLK_RPCSRC, CLK_TYPE_GEN4_RPCSRC, CLK_PLL5), > + DEF_BASE(".rpc", R8A779F0_CLK_RPC, CLK_TYPE_GEN4_RPC, CLK_RPCSRC), > + DEF_BASE("rpcd2", R8A779F0_CLK_RPCD2, CLK_TYPE_GEN4_RPCD2, R8A779F0_CLK_RPC), > + > + /* Core Clock Outputs */ > + DEF_FIXED("s0d2", R8A779F0_CLK_S0D2, CLK_S0, 2, 1), > + DEF_FIXED("s0d3", R8A779F0_CLK_S0D3, CLK_S0, 3, 1), > + DEF_FIXED("s0d4", R8A779F0_CLK_S0D4, CLK_S0, 4, 1), > + DEF_FIXED("cl16m", R8A779F0_CLK_CL16M, CLK_S0, 48, 1), > + DEF_FIXED("s0d2_mm", R8A779F0_CLK_S0D2_MM, CLK_S0, 2, 1), > + DEF_FIXED("s0d3_mm", R8A779F0_CLK_S0D3_MM, CLK_S0, 3, 1), > + DEF_FIXED("s0d4_mm", R8A779F0_CLK_S0D4_MM, CLK_S0, 4, 1), > + DEF_FIXED("cl16m_mm", R8A779F0_CLK_CL16M_MM, CLK_S0, 48, 1), > + DEF_FIXED("s0d2_rt", R8A779F0_CLK_S0D2_RT, CLK_S0, 2, 1), > + DEF_FIXED("s0d3_rt", R8A779F0_CLK_S0D3_RT, CLK_S0, 3, 1), > + DEF_FIXED("s0d4_rt", R8A779F0_CLK_S0D4_RT, CLK_S0, 4, 1), > + DEF_FIXED("s0d6_rt", R8A779F0_CLK_S0D6_RT, CLK_S0, 6, 1), > + DEF_FIXED("cl16m_rt", R8A779F0_CLK_CL16M_RT, CLK_S0, 48, 1), > + DEF_FIXED("s0d3_per", R8A779F0_CLK_S0D3_PER, CLK_S0, 3, 1), > + DEF_FIXED("s0d6_per", R8A779F0_CLK_S0D6_PER, CLK_S0, 6, 1), > + DEF_FIXED("s0d12_per", R8A779F0_CLK_S0D12_PER, CLK_S0, 12, 1), > + DEF_FIXED("s0d24_per", R8A779F0_CLK_S0D24_PER, CLK_S0, 24, 1), > + DEF_FIXED("cl16m_per", R8A779F0_CLK_CL16M_PER, CLK_S0, 48, 1), > + DEF_FIXED("s0d2_hsc", R8A779F0_CLK_S0D2_HSC, CLK_S0, 2, 1), > + DEF_FIXED("s0d3_hsc", R8A779F0_CLK_S0D3_HSC, CLK_S0, 3, 1), > + DEF_FIXED("s0d4_hsc", R8A779F0_CLK_S0D4_HSC, CLK_S0, 4, 1), > + DEF_FIXED("s0d6_hsc", R8A779F0_CLK_S0D6_HSC, CLK_S0, 6, 1), > + DEF_FIXED("s0d12_hsc", R8A779F0_CLK_S0D12_HSC, CLK_S0, 12, 1), > + DEF_FIXED("cl16m_hsc", R8A779F0_CLK_CL16M_HSC, CLK_S0, 48, 1), > + DEF_FIXED("s0d2_cc", R8A779F0_CLK_S0D2_CC, CLK_S0, 2, 1), > + DEF_FIXED("rsw", R8A779F0_CLK_RSW2, CLK_PLL5, 2, 1), "rsw2"? > + DEF_FIXED("cbfusa", R8A779F0_CLK_CBFUSA, CLK_EXTAL, 2, 1), > + DEF_FIXED("cpex", R8A779F0_CLK_CPEX, CLK_EXTAL, 2, 1), > + > + DEF_GEN4_SD("sd0", R8A779F0_CLK_SD0, CLK_SDSRC, 0x870), > + DEF_DIV6P1("mso", R8A779F0_CLK_MSO, CLK_PLL5_DIV4, 0x087C), 0x87c > + > + DEF_GEN4_OSC("osc", R8A779F0_CLK_OSC, CLK_EXTAL, 8), > + DEF_GEN4_MDSEL("r", R8A779F0_CLK_R, 29, CLK_EXTALR, 1, CLK_OCO, 1), > +}; > + > +static const struct mssr_mod_clk r8a779f0_mod_clks[] __initconst = { > + DEF_MOD("scif0", 702, R8A779F0_CLK_S0D12_PER), > + DEF_MOD("scif1", 703, R8A779F0_CLK_S0D12_PER), > + DEF_MOD("scif3", 704, R8A779F0_CLK_S0D12_PER), > + DEF_MOD("scif4", 705, R8A779F0_CLK_S0D12_PER), > +}; > + > +/* > + * CPG Clock Data > + */ > +/* > + * MD EXTAL PLL1 PLL2 PLL3 PLL5 PLL6 OSC > + * 14 13 (MHz) > + * ---------------------------------------------------------------- > + * 0 0 16.66 / 1 x200 x150 x200 x200 x134 /15 EXTAL is 16 MHz? > + * 0 1 20 / 1 x160 x120 x160 x160 x106 /19 > + * 1 0 Prohibited setting > + * 1 1 40 / 2 x160 x120 x160 x160 x106 /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 PLL5 mult/div PLL6 mult/div OSC prediv */ > + { 1, 200, 1, 150, 1, 200, 1, 200, 1, 134, 1, 16, }, OSC prediv is 15? > + { 1, 160, 1, 120, 1, 160, 1, 160, 1, 106, 1, 19, }, > + { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, }, > + { 2, 160, 1, 120, 1, 160, 1, 160, 1, 106, 1, 38, }, > +}; > --- /dev/null > +++ b/drivers/clk/renesas/rcar-gen4-cpg.c > @@ -0,0 +1,141 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * R-Car Gen4 Clock Pulse Generator > + * > + * Copyright (C) 2021 Renesas Electronics Corp. > + * > + * Based on rcar-gen3-cpg.c > + * > + * Copyright (C) 2015-2018 Glider bvba > + * Copyright (C) 2019 Renesas Electronics Corp. > + */ > + > +#include <linux/bug.h> > +#include <linux/bitfield.h> > +#include <linux/clk.h> > +#include <linux/clk-provider.h> > +#include <linux/device.h> > +#include <linux/err.h> > +#include <linux/init.h> > +#include <linux/io.h> > +#include <linux/pm.h> > +#include <linux/slab.h> > +#include <linux/sys_soc.h> Several of these includes are not needed. > + > +#include "renesas-cpg-mssr.h" > +#include "rcar-gen4-cpg.h" > +#include "rcar-cpg-lib.h" > + > +static const struct clk_div_table cpg_rpcsrc_div_table[] = { > + { 2, 5 }, { 3, 6 }, { 0, 0 }, The datasheet says { 0, 4 } and { 1, 6} are also supported, just like on R-Car V3U. > +}; > +struct clk * __init rcar_gen4_cpg_clk_register(struct device *dev, > + const struct cpg_core_clk *core, const struct cpg_mssr_info *info, > + struct clk **clks, void __iomem *base, > + struct raw_notifier_head *notifiers) > +{ > + const struct clk *parent; > + unsigned int mult = 1; > + unsigned int div = 1; > + > + parent = clks[core->parent & 0xffff]; /* some types use high bits */ > + if (IS_ERR(parent)) > + return ERR_CAST(parent); > + > + switch (core->type) { > + case CLK_TYPE_GEN4_MAIN: > + div = cpg_pll_config->extal_div; > + break; > + > + case CLK_TYPE_GEN4_PLL1: > + mult = cpg_pll_config->pll1_mult; > + div = cpg_pll_config->pll1_div; > + break; > + > + case CLK_TYPE_GEN4_PLL2: > + mult = cpg_pll_config->pll2_mult; > + div = cpg_pll_config->pll2_div; > + break; > + > + case CLK_TYPE_GEN4_PLL3: > + mult = cpg_pll_config->pll3_mult; > + div = cpg_pll_config->pll3_div; > + break; > + > + case CLK_TYPE_GEN4_PLL5: > + mult = cpg_pll_config->pll5_mult; > + div = cpg_pll_config->pll5_div; > + break; > + > + case CLK_TYPE_GEN4_PLL6: > + mult = cpg_pll_config->pll6_mult; > + div = cpg_pll_config->pll6_div; > + break; The Z clock handling for R-Car S4-8 seems to be the same as for R-Car V3U, so you can move that here, too. That leaves us with the different PLLn handling: 1. I think you can just move the PLL2X_3C clock type for R-Car V3U here, too. It's only 4 lines of code. Then rcar-gen4-cpg.c can be shared by R-Car V3U and R-Car S4-8. 2. Future full PLLn handling for R-Car S4-8 (using the PLLnCR1 registers) will need switching from CLK_TYPE_GEN4_PLLn to a new clock type anyway. > + case CLK_TYPE_GEN4_SD: > + return cpg_sd_clk_register(core->name, base, core->offset, > + __clk_get_name(parent), notifiers, > + 0); This should be changed to: return cpg_sd_clk_register(core->name, base + core->offset, __clk_get_name(parent)); due to the recent changes in renesas-clk to handle the SDH clock. > + > + default: > + return ERR_PTR(-EINVAL); > + } > + > + return clk_register_fixed_factor(NULL, core->name, > + __clk_get_name(parent), 0, mult, div); > +} > --- /dev/null > +++ b/drivers/clk/renesas/rcar-gen4-cpg.h > @@ -0,0 +1,76 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * R-Car Gen4 Clock Pulse Generator > + * > + * Copyright (C) 2021 Renesas Electronics Corp. > + * > + */ > + > +#ifndef __CLK_RENESAS_RCAR_GEN4_CPG_H__ > +#define __CLK_RENESAS_RCAR_GEN4_CPG_H__ > + > +enum rcar_gen4_clk_types { > + CLK_TYPE_GEN4_MAIN = CLK_TYPE_CUSTOM, > + CLK_TYPE_GEN4_PLL1, > + CLK_TYPE_GEN4_PLL2, > + CLK_TYPE_GEN4_PLL3, > + CLK_TYPE_GEN4_PLL5, > + CLK_TYPE_GEN4_PLL6, > + CLK_TYPE_GEN4_SD, > + CLK_TYPE_GEN4_R, So far unused. > + CLK_TYPE_GEN4_MDSEL, /* Select parent/divider using mode pin */ > + CLK_TYPE_GEN4_Z, > + CLK_TYPE_GEN4_ZG, Both unused. > + CLK_TYPE_GEN4_OSC, /* OSC EXTAL predivider and fixed divider */ > + CLK_TYPE_GEN4_RPCSRC, > + CLK_TYPE_GEN4_RPC, > + CLK_TYPE_GEN4_RPCD2, > + > + /* SoC specific definitions start here */ > + CLK_TYPE_GEN4_SOC_BASE, > +}; > + > +#define DEF_GEN4_SD(_name, _id, _parent, _offset) \ > + DEF_BASE(_name, _id, CLK_TYPE_GEN4_SD, _parent, .offset = _offset) > + > +#define DEF_GEN4_MDSEL(_name, _id, _md, _parent0, _div0, _parent1, _div1) \ > + DEF_BASE(_name, _id, CLK_TYPE_GEN4_MDSEL, \ > + (_parent0) << 16 | (_parent1), \ > + .div = (_div0) << 16 | (_div1), .offset = _md) > + > +#define DEF_GEN4_PE(_name, _id, _parent_clean, _div_clean, _parent_sscg, \ > + _div_sscg) \ > + DEF_GEN4_MDSEL(_name, _id, 12, _parent_clean, _div_clean, \ > + _parent_sscg, _div_sscg) R-Car S4 does not have a PE clock, so please drop this. 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
On Tue, Nov 16, 2021 at 8:42 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > Add initial support for the Renesas R8A779F0 (R-Car S4-8) support. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> 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
On Tue, Nov 16, 2021 at 8:43 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > Enable the Renesas R-Car S4-8 (R8A779F0) SoC in the ARM64 defconfig. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> 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
On Tue, Nov 16, 2021 at 8:42 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > Initial support for the Renesas Spider CPU and BreakOut boards > support. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > Tested-by: Takehito Nakamura <takehito.nakamura.nx@renesas.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> (assuming memory size, extal clock frequency, and serial console port are correct) 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
Hi Geert-san, Thank you for your review! > From: Geert Uytterhoeven, Sent: Wednesday, November 24, 2021 10:48 PM > > Hi Shimoda-san, > > Thanks for your patch! > > On Tue, Nov 16, 2021 at 8:42 AM Yoshihiro Shimoda > <yoshihiro.shimoda.uh@renesas.com> wrote: > > Initial support for R-Car S4-8 (r8a779f0), including core, module > > clocks, resets, and register access, because register specification > > differs from R-Car Gen2/3. The register layout of V3U is a similar > > with R-Car S4-8 so that renames CLK_REG_LAYOUT_RCAR_V3U as > > CLK_REG_LAYOUT_RCAR_GEN4. However, PLL names differ between V3U > > and S4-8. > > This is a small difference, so I think more code can be shared between > R-Car V3U and S4-8 (see below). I got it. > > Inspired by patches in the BSP by LUU HOAI. > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > > drivers/clk/renesas/Kconfig | 10 ++ > > drivers/clk/renesas/Makefile | 2 + > > drivers/clk/renesas/r8a779a0-cpg-mssr.c | 2 +- > > drivers/clk/renesas/r8a779f0-cpg-mssr.c | 188 ++++++++++++++++++++++++ > > drivers/clk/renesas/rcar-gen4-cpg.c | 141 ++++++++++++++++++ > > drivers/clk/renesas/rcar-gen4-cpg.h | 76 ++++++++++ > > drivers/clk/renesas/renesas-cpg-mssr.c | 42 ++++-- > > drivers/clk/renesas/renesas-cpg-mssr.h | 3 +- > > 8 files changed, 448 insertions(+), 16 deletions(-) > > create mode 100644 drivers/clk/renesas/r8a779f0-cpg-mssr.c > > create mode 100644 drivers/clk/renesas/rcar-gen4-cpg.c > > create mode 100644 drivers/clk/renesas/rcar-gen4-cpg.h > > Just like for the SYSC driver, I think it wouldn't hurt to split this > patch in two parts: one patch to generalize r8a779a0-cpg-mssr.c for > R-Car Gen4, and a second patch to introduce support for R-Car S4-8. I got it. I'll split this patch in two parts in v2. > > +static const struct cpg_core_clk r8a779f0_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(".pll3", CLK_PLL3, CLK_TYPE_GEN4_PLL3, CLK_MAIN), > > + DEF_BASE(".pll2", CLK_PLL2, CLK_TYPE_GEN4_PLL2, CLK_MAIN), > > + DEF_BASE(".pll6", CLK_PLL6, CLK_TYPE_GEN4_PLL6, CLK_MAIN), > > + DEF_BASE(".pll5", CLK_PLL5, CLK_TYPE_GEN4_PLL5, CLK_MAIN), > > Please sort PLLn by index. Oops. I'll fix the order. > > + > > + 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(".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(".sdsrc", CLK_SDSRC, CLK_PLL5_DIV2, 2, 1), > > This relies on the default setting of the SD-IF0 Clock Frequency > Control Register 1 (SD0CKCR1)? You're correct. So, we should not use DEF_FIXED for SDSRC... > > + DEF_RATE(".oco", CLK_OCO, 32768), > > + > > + DEF_BASE(".rpcsrc", CLK_RPCSRC, CLK_TYPE_GEN4_RPCSRC, CLK_PLL5), > > + DEF_BASE(".rpc", R8A779F0_CLK_RPC, CLK_TYPE_GEN4_RPC, CLK_RPCSRC), > > + DEF_BASE("rpcd2", R8A779F0_CLK_RPCD2, CLK_TYPE_GEN4_RPCD2, R8A779F0_CLK_RPC), > > + > > + /* Core Clock Outputs */ > > + DEF_FIXED("s0d2", R8A779F0_CLK_S0D2, CLK_S0, 2, 1), > > + DEF_FIXED("s0d3", R8A779F0_CLK_S0D3, CLK_S0, 3, 1), > > + DEF_FIXED("s0d4", R8A779F0_CLK_S0D4, CLK_S0, 4, 1), > > + DEF_FIXED("cl16m", R8A779F0_CLK_CL16M, CLK_S0, 48, 1), > > + DEF_FIXED("s0d2_mm", R8A779F0_CLK_S0D2_MM, CLK_S0, 2, 1), > > + DEF_FIXED("s0d3_mm", R8A779F0_CLK_S0D3_MM, CLK_S0, 3, 1), > > + DEF_FIXED("s0d4_mm", R8A779F0_CLK_S0D4_MM, CLK_S0, 4, 1), > > + DEF_FIXED("cl16m_mm", R8A779F0_CLK_CL16M_MM, CLK_S0, 48, 1), > > + DEF_FIXED("s0d2_rt", R8A779F0_CLK_S0D2_RT, CLK_S0, 2, 1), > > + DEF_FIXED("s0d3_rt", R8A779F0_CLK_S0D3_RT, CLK_S0, 3, 1), > > + DEF_FIXED("s0d4_rt", R8A779F0_CLK_S0D4_RT, CLK_S0, 4, 1), > > + DEF_FIXED("s0d6_rt", R8A779F0_CLK_S0D6_RT, CLK_S0, 6, 1), > > + DEF_FIXED("cl16m_rt", R8A779F0_CLK_CL16M_RT, CLK_S0, 48, 1), > > + DEF_FIXED("s0d3_per", R8A779F0_CLK_S0D3_PER, CLK_S0, 3, 1), > > + DEF_FIXED("s0d6_per", R8A779F0_CLK_S0D6_PER, CLK_S0, 6, 1), > > + DEF_FIXED("s0d12_per", R8A779F0_CLK_S0D12_PER, CLK_S0, 12, 1), > > + DEF_FIXED("s0d24_per", R8A779F0_CLK_S0D24_PER, CLK_S0, 24, 1), > > + DEF_FIXED("cl16m_per", R8A779F0_CLK_CL16M_PER, CLK_S0, 48, 1), > > + DEF_FIXED("s0d2_hsc", R8A779F0_CLK_S0D2_HSC, CLK_S0, 2, 1), > > + DEF_FIXED("s0d3_hsc", R8A779F0_CLK_S0D3_HSC, CLK_S0, 3, 1), > > + DEF_FIXED("s0d4_hsc", R8A779F0_CLK_S0D4_HSC, CLK_S0, 4, 1), > > + DEF_FIXED("s0d6_hsc", R8A779F0_CLK_S0D6_HSC, CLK_S0, 6, 1), > > + DEF_FIXED("s0d12_hsc", R8A779F0_CLK_S0D12_HSC, CLK_S0, 12, 1), > > + DEF_FIXED("cl16m_hsc", R8A779F0_CLK_CL16M_HSC, CLK_S0, 48, 1), > > + DEF_FIXED("s0d2_cc", R8A779F0_CLK_S0D2_CC, CLK_S0, 2, 1), > > + DEF_FIXED("rsw", R8A779F0_CLK_RSW2, CLK_PLL5, 2, 1), > > "rsw2"? Yes. I'll fix it. > > + DEF_FIXED("cbfusa", R8A779F0_CLK_CBFUSA, CLK_EXTAL, 2, 1), > > + DEF_FIXED("cpex", R8A779F0_CLK_CPEX, CLK_EXTAL, 2, 1), > > + > > + DEF_GEN4_SD("sd0", R8A779F0_CLK_SD0, CLK_SDSRC, 0x870), > > + DEF_DIV6P1("mso", R8A779F0_CLK_MSO, CLK_PLL5_DIV4, 0x087C), > > 0x87c Oops. I'll fix it. > > + > > + DEF_GEN4_OSC("osc", R8A779F0_CLK_OSC, CLK_EXTAL, 8), > > + DEF_GEN4_MDSEL("r", R8A779F0_CLK_R, 29, CLK_EXTALR, 1, CLK_OCO, 1), > > +}; > > + > > +static const struct mssr_mod_clk r8a779f0_mod_clks[] __initconst = { > > + DEF_MOD("scif0", 702, R8A779F0_CLK_S0D12_PER), > > + DEF_MOD("scif1", 703, R8A779F0_CLK_S0D12_PER), > > + DEF_MOD("scif3", 704, R8A779F0_CLK_S0D12_PER), > > + DEF_MOD("scif4", 705, R8A779F0_CLK_S0D12_PER), > > +}; > > + > > +/* > > + * CPG Clock Data > > + */ > > +/* > > + * MD EXTAL PLL1 PLL2 PLL3 PLL5 PLL6 OSC > > + * 14 13 (MHz) > > + * ---------------------------------------------------------------- > > + * 0 0 16.66 / 1 x200 x150 x200 x200 x134 /15 > > EXTAL is 16 MHz? Oops. You're correct. I'll fix it. > > + * 0 1 20 / 1 x160 x120 x160 x160 x106 /19 > > + * 1 0 Prohibited setting > > + * 1 1 40 / 2 x160 x120 x160 x160 x106 /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 PLL5 mult/div PLL6 mult/div OSC prediv */ > > + { 1, 200, 1, 150, 1, 200, 1, 200, 1, 134, 1, 16, }, > > OSC prediv is 15? Thank you for the indicate. I'll fix it. > > + { 1, 160, 1, 120, 1, 160, 1, 160, 1, 106, 1, 19, }, > > + { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, }, > > + { 2, 160, 1, 120, 1, 160, 1, 160, 1, 106, 1, 38, }, > > +}; > > > --- /dev/null > > +++ b/drivers/clk/renesas/rcar-gen4-cpg.c > > @@ -0,0 +1,141 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * R-Car Gen4 Clock Pulse Generator > > + * > > + * Copyright (C) 2021 Renesas Electronics Corp. > > + * > > + * Based on rcar-gen3-cpg.c > > + * > > + * Copyright (C) 2015-2018 Glider bvba > > + * Copyright (C) 2019 Renesas Electronics Corp. > > + */ > > + > > +#include <linux/bug.h> > > +#include <linux/bitfield.h> > > +#include <linux/clk.h> > > +#include <linux/clk-provider.h> > > +#include <linux/device.h> > > +#include <linux/err.h> > > +#include <linux/init.h> > > +#include <linux/io.h> > > +#include <linux/pm.h> > > +#include <linux/slab.h> > > +#include <linux/sys_soc.h> > > Several of these includes are not needed. I got it. I'll remove unnecessary header files. > > + > > +#include "renesas-cpg-mssr.h" > > +#include "rcar-gen4-cpg.h" > > +#include "rcar-cpg-lib.h" > > + > > +static const struct clk_div_table cpg_rpcsrc_div_table[] = { > > + { 2, 5 }, { 3, 6 }, { 0, 0 }, > > The datasheet says { 0, 4 } and { 1, 6} are also supported, > just like on R-Car V3U. I got it. > > +}; > > > +struct clk * __init rcar_gen4_cpg_clk_register(struct device *dev, > > + const struct cpg_core_clk *core, const struct cpg_mssr_info *info, > > + struct clk **clks, void __iomem *base, > > + struct raw_notifier_head *notifiers) > > +{ > > + const struct clk *parent; > > + unsigned int mult = 1; > > + unsigned int div = 1; > > + > > + parent = clks[core->parent & 0xffff]; /* some types use high bits */ > > + if (IS_ERR(parent)) > > + return ERR_CAST(parent); > > + > > + switch (core->type) { > > + case CLK_TYPE_GEN4_MAIN: > > + div = cpg_pll_config->extal_div; > > + break; > > + > > + case CLK_TYPE_GEN4_PLL1: > > + mult = cpg_pll_config->pll1_mult; > > + div = cpg_pll_config->pll1_div; > > + break; > > + > > + case CLK_TYPE_GEN4_PLL2: > > + mult = cpg_pll_config->pll2_mult; > > + div = cpg_pll_config->pll2_div; > > + break; > > + > > + case CLK_TYPE_GEN4_PLL3: > > + mult = cpg_pll_config->pll3_mult; > > + div = cpg_pll_config->pll3_div; > > + break; > > + > > + case CLK_TYPE_GEN4_PLL5: > > + mult = cpg_pll_config->pll5_mult; > > + div = cpg_pll_config->pll5_div; > > + break; > > + > > + case CLK_TYPE_GEN4_PLL6: > > + mult = cpg_pll_config->pll6_mult; > > + div = cpg_pll_config->pll6_div; > > + break; > > The Z clock handling for R-Car S4-8 seems to be the same as for R-Car > V3U, so you can move that here, too. > > That leaves us with the different PLLn handling: > 1. I think you can just move the PLL2X_3C clock type for R-Car > V3U here, too. It's only 4 lines of code. Then rcar-gen4-cpg.c > can be shared by R-Car V3U and R-Car S4-8. I got it. I'll modify it. > 2. Future full PLLn handling for R-Car S4-8 (using the PLLnCR1 > registers) will need switching from CLK_TYPE_GEN4_PLLn to a > new clock type anyway. I got it. > > + case CLK_TYPE_GEN4_SD: > > + return cpg_sd_clk_register(core->name, base, core->offset, > > + __clk_get_name(parent), notifiers, > > + 0); > > This should be changed to: > > return cpg_sd_clk_register(core->name, base + core->offset, > __clk_get_name(parent)); > > due to the recent changes in renesas-clk to handle the SDH clock. Thank you for the comment. I'll rebase it on the recent renesas-clk. > > + > > + default: > > + return ERR_PTR(-EINVAL); > > + } > > + > > + return clk_register_fixed_factor(NULL, core->name, > > + __clk_get_name(parent), 0, mult, div); > > +} > > > --- /dev/null > > +++ b/drivers/clk/renesas/rcar-gen4-cpg.h > > @@ -0,0 +1,76 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +/* > > + * R-Car Gen4 Clock Pulse Generator > > + * > > + * Copyright (C) 2021 Renesas Electronics Corp. > > + * > > + */ > > + > > +#ifndef __CLK_RENESAS_RCAR_GEN4_CPG_H__ > > +#define __CLK_RENESAS_RCAR_GEN4_CPG_H__ > > + > > +enum rcar_gen4_clk_types { > > + CLK_TYPE_GEN4_MAIN = CLK_TYPE_CUSTOM, > > + CLK_TYPE_GEN4_PLL1, > > + CLK_TYPE_GEN4_PLL2, > > + CLK_TYPE_GEN4_PLL3, > > + CLK_TYPE_GEN4_PLL5, > > + CLK_TYPE_GEN4_PLL6, > > + CLK_TYPE_GEN4_SD, > > + CLK_TYPE_GEN4_R, > > So far unused. I'll remove CLK_TYPE_GEN4_R. > > + CLK_TYPE_GEN4_MDSEL, /* Select parent/divider using mode pin */ > > + CLK_TYPE_GEN4_Z, > > + CLK_TYPE_GEN4_ZG, > > Both unused. I'll remove CLK_TYPE_GEN4_Z[G]. > > + CLK_TYPE_GEN4_OSC, /* OSC EXTAL predivider and fixed divider */ > > + CLK_TYPE_GEN4_RPCSRC, > > + CLK_TYPE_GEN4_RPC, > > + CLK_TYPE_GEN4_RPCD2, > > + > > + /* SoC specific definitions start here */ > > + CLK_TYPE_GEN4_SOC_BASE, > > +}; > > + > > +#define DEF_GEN4_SD(_name, _id, _parent, _offset) \ > > + DEF_BASE(_name, _id, CLK_TYPE_GEN4_SD, _parent, .offset = _offset) > > + > > +#define DEF_GEN4_MDSEL(_name, _id, _md, _parent0, _div0, _parent1, _div1) \ > > + DEF_BASE(_name, _id, CLK_TYPE_GEN4_MDSEL, \ > > + (_parent0) << 16 | (_parent1), \ > > + .div = (_div0) << 16 | (_div1), .offset = _md) > > + > > +#define DEF_GEN4_PE(_name, _id, _parent_clean, _div_clean, _parent_sscg, \ > > + _div_sscg) \ > > + DEF_GEN4_MDSEL(_name, _id, 12, _parent_clean, _div_clean, \ > > + _parent_sscg, _div_sscg) > > R-Car S4 does not have a PE clock, so please drop this. I got it. Best regards, Yoshihiro Shimoda
Hi Shimoda-san, On Mon, Nov 29, 2021 at 9:36 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > > From: Geert Uytterhoeven, Sent: Wednesday, November 24, 2021 10:48 PM > > On Tue, Nov 16, 2021 at 8:42 AM Yoshihiro Shimoda > > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > Initial support for R-Car S4-8 (r8a779f0), including core, module > > > clocks, resets, and register access, because register specification > > > differs from R-Car Gen2/3. The register layout of V3U is a similar > > > with R-Car S4-8 so that renames CLK_REG_LAYOUT_RCAR_V3U as > > > CLK_REG_LAYOUT_RCAR_GEN4. However, PLL names differ between V3U > > > and S4-8. > > > > > > Inspired by patches in the BSP by LUU HOAI. > > > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > > + > > > + 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(".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(".sdsrc", CLK_SDSRC, CLK_PLL5_DIV2, 2, 1), > > > > This relies on the default setting of the SD-IF0 Clock Frequency > > Control Register 1 (SD0CKCR1)? > > You're correct. So, we should not use DEF_FIXED for SDSRC... You can use DEF_FIXED in the initial version, and add proper SD0CKCR1 support later. This is similar to the handling of the various PLLs: currently they're treated as fixed ratio clocks, later they can become programmable by adding support for the PLLnCR1 registers. 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
Hi Shimoda-san, On Wed, Nov 24, 2021 at 3:06 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Tue, Nov 16, 2021 at 8:42 AM Yoshihiro Shimoda > <yoshihiro.shimoda.uh@renesas.com> wrote: > > Initial support for the Renesas Spider CPU and BreakOut boards > > support. > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > Tested-by: Takehito Nakamura <takehito.nakamura.nx@renesas.com> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > (assuming memory size, extal clock frequency, and serial console port > are correct) (with the schematics) So the console is actually SCIF3 pinmuxed to the HSCIF0 pins on the Debug Serial USB connector on the CPU board? 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
Hi Geert-san, > From: Geert Uytterhoeven, Sent: Wednesday, December 8, 2021 6:21 PM > > Hi Shimoda-san, > > On Wed, Nov 24, 2021 at 3:06 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > On Tue, Nov 16, 2021 at 8:42 AM Yoshihiro Shimoda > > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > Initial support for the Renesas Spider CPU and BreakOut boards > > > support. > > > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > > Tested-by: Takehito Nakamura <takehito.nakamura.nx@renesas.com> > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > (assuming memory size, extal clock frequency, and serial console port > > are correct) > > (with the schematics) > So the console is actually SCIF3 pinmuxed to the HSCIF0 pins on the > Debug Serial USB connector on the CPU board? Yes, the console is SCIF3. Best regards, Yoshihiro Shimoda