| Message ID | 1599470390-29719-1-git-send-email-yoshihiro.shimoda.uh@renesas.com |
|---|---|
| Headers | show |
| Series | treewide: add initial support for R-Car V3U | expand |
On Mon, Sep 7, 2020 at 11:20 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > Add support for identifying the R-Car V3U (R8A779A0) SoC. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > --- a/drivers/soc/renesas/Kconfig > +++ b/drivers/soc/renesas/Kconfig > @@ -272,6 +272,12 @@ config ARCH_R8A77995 > help > This enables support for the Renesas R-Car D3 SoC. > > +config ARCH_R8A779A0 > + bool "Renesas R-Car V3U SoC Platform" Will update while applying to match Morimoto-san's Kconfig rework. Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> i.e. will queue in renesas-devel for v5.10. Gr{oetje,eeting}s, Geert
nHi Shimoda-san, On Mon, Sep 7, 2020 at 11:20 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > Add support for R-Car V3U (R8A779A0) SoC power areas and > register access, because register specification differs > than R-Car Gen2/3. > > Inspired by patches in the BSP by Tho Vu. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Thanks for your patch! > --- /dev/null > +++ b/drivers/soc/renesas/r8a779a0-sysc.c > @@ -0,0 +1,460 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Renesas R-Car V3U System Controller > + * > + * Copyright (C) 2020 Renesas Electronics Corp. > + */ > + > +#include <linux/bits.h> > +#include <linux/clk/renesas.h> > +#include <linux/delay.h> > +#include <linux/err.h> > +#include <linux/kernel.h> > +#include <linux/mm.h> > +#include <linux/of_address.h> > +#include <linux/pm_domain.h> > +#include <linux/slab.h> > +#include <linux/spinlock.h> > +#include <linux/io.h> > +#include <linux/soc/renesas/rcar-sysc.h> > +#include <linux/sys_soc.h> > +#include <linux/syscore_ops.h> The above 3 includes are not needed. > + > +#include <dt-bindings/power/r8a779a0-sysc.h> > + > +#include "rcar-sysc.h" You don't reuse the rcar-sysc driver itself, but you do reuse its header file. As the comments in rcar-sysc.h refer to registers that have been renamed (e.g. PWR*), and SYSCEXTMASK no longer exists, it might be easier for the casual reader to drop the include, copy the PD_* definitions, and define new r8a779a0_sysc_area and r8a779a0_sysc_info structures instead, using the new naming. > + > +static struct rcar_sysc_area r8a779a0_areas[] __initdata = { > + { "always-on", 0, 0, R8A779A0_PD_ALWAYS_ON, -1, PD_ALWAYS_ON }, > + { "a3e0", 0x1500, 0, R8A779A0_PD_A3E0, R8A779A0_PD_ALWAYS_ON, PD_SCU }, I think you can drop: - chan_offs: it's always 0x1000 + pdr * 64, - chan_bit: it's always zero, > +/* SYSC Common */ > +#define SYSCSR 0x000 /* SYSC Status Register */ > +#define SYSCPONSR(x) (0x800 + ((x) * 0x4)) /* Power-ON Status Register 0 */ > +#define SYSCPOFFSR(x) (0x808 + ((x) * 0x4)) /* Power-OFF Status Register */ > +#define SYSCISCR(x) (0x810 + ((x) * 0x4)) /* Interrupt Status/Clear Register */ > +#define SYSCIER(x) (0x820 + ((x) * 0x4)) /* Interrupt Enable Register */ > +#define SYSCIMR(x) (0x830 + ((x) * 0x4)) /* Interrupt Mask Register */ > + > +/* Power Domain Registers */ > +#define PDRSR(n) (0x1000 + ((n) * 0x40)) > +#define PDRONCR(n) (0x1004 + ((n) * 0x40)) > +#define PDROFFCR(n) (0x1008 + ((n) * 0x40)) > +#define PDRESR(n) (0x100C + ((n) * 0x40)) While PDRESRn is described, and shown in a figure, it was forgotten in the Table 9.2 ("List of Registers (Power Domain Registers for each power domain)"). > + > +/* Power State */ > +#define PW_ACTIVE 1 /* Active setting */ "/* PWRON/PWROFF */"? > + > +/* PDRSR */ > +#define PDRSR_OFF BIT(0) /* Power-OFF state */ > +#define PDRSR_ON BIT(4) /* Power-ON state */ > +#define PDRSR_OFF_STATE BIT(8) /* Processing Power-OFF sequence */ > +#define PDRSR_ON_STATE BIT(12) /* Processing Power-ON sequence */ > + > +#define SYSCSR_PONENB 1 /* Ready for power resume requests */ > +#define SYSCSR_POFFENB 0 /* Ready for power shutoff requests */ These two bits are now combined into a single BUSY bit mask: (doh, all bits sets is not busy?!?) #define SYSCSR_BUSY GENMASK(1, 0) /* All bit sets is not busy */ > + > +#define SYSCSR_RETRIES 1000 > +#define SYSCSR_DELAY_US 10 > + > +#define PDRESR_RETRIES 1000 > +#define PDRESR_DELAY_US 10 > + > +#define SYSCISR_RETRIES 1000 > +#define SYSCISR_DELAY_US 10 > + > +#define R8A779A0_NUM_PD_ALWAYS_ON 64 /* Always-on power area */ Just use R8A779A0_PD_ALWAYS_ON instead? > + > +#define NUM_DOMAINS_EACH_REG 32 BITS_PER_TYPE(u32)? > + > +struct rcar_sysc_ch { > + u16 chan_offs; > + u8 chan_bit; > + u8 isr_bit; > +}; As chan_offs is unused, and chan_bit is always zero, I think all use of this struct can just be replaced by "unsigned int pdr"? > + > +static void __iomem *rcar_sysc_base; s/rcar/r8a779a0/ everywhere? > +static DEFINE_SPINLOCK(rcar_sysc_lock); /* SMP CPUs + I/O devices */ > + > +static int rcar_sysc_pwr_on_off(const struct rcar_sysc_ch *sysc_ch, bool on) > +{ > + unsigned int sr_bit, reg_offs; sr_bit is no longer needed. > + int k; > + > + if (on) { > + sr_bit = SYSCSR_PONENB; > + reg_offs = PDRONCR(sysc_ch->isr_bit); > + } else { > + sr_bit = SYSCSR_POFFENB; > + reg_offs = PDROFFCR(sysc_ch->isr_bit); > + } > + > + /* Wait until SYSC is ready to accept a power request */ > + for (k = 0; k < SYSCSR_RETRIES; k++) { > + if (ioread32(rcar_sysc_base + SYSCSR) & BIT(sr_bit)) if ((ioread32(rcar_sysc_base + SYSCSR) & SYSCSR_BUSY) == SYSCSR_BUSY) > + break; > + udelay(SYSCSR_DELAY_US); > + } Perhaps you can use readl_poll_timeout()? > + > + if (k == SYSCSR_RETRIES) > + return -EAGAIN; > + > + /* Submit power shutoff or power resume request */ > + iowrite32(PW_ACTIVE, rcar_sysc_base + reg_offs); > + > + return 0; > +} > + > +static int clear_irq_flags(unsigned int reg_idx, unsigned int isr_mask) > +{ > + int k = 0; > + > + iowrite32(isr_mask, rcar_sysc_base + SYSCISCR(reg_idx)); > + > + for (k = 0; k < SYSCISR_RETRIES; k++) { > + if ((ioread32(rcar_sysc_base + SYSCISCR(reg_idx)) & isr_mask) == 0) > + break; > + > + udelay(SYSCISR_DELAY_US); > + } readl_poll_timeout()? > + > + if (k == SYSCISR_RETRIES) { > + pr_err("\n %s : Can not clear IRQ flags in SYSCISCR", __func__); > + return -EIO; > + } > + > + return 0; > +} > +static bool has_cpg_mstp; Please drop this and all related code, R-Car V3U does not use the legacy CPG/MSSR PM Domain. > +static const struct of_device_id rcar_sysc_matches[] __initconst = { > +#ifdef CONFIG_SYSC_R8A779A0 Please drop the #ifdef, as compilation of this file already depends on this symbol. > + { .compatible = "renesas,r8a779a0-sysc", .data = &r8a779a0_sysc_info }, > +#endif > + { /* sentinel */ } > +}; Gr{oetje,eeting}s, Geert
Hi Shimoda-san, On Mon, Sep 7, 2020 at 11:20 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > Add support for R-Car V3U (R8A779A0) to the R-Car RST driver. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Thanks for your patch! Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- a/drivers/soc/renesas/rcar-rst.c > +++ b/drivers/soc/renesas/rcar-rst.c > @@ -37,6 +37,10 @@ static const struct rst_config rcar_rst_gen3 __initconst = { > .modemr = 0x60, > }; > > +static const struct rst_config rcar_rst_r8a779a0 __initconst = { > + .modemr = 0x00, /* MODEMR0 and it has CPG related bits */ Do you need the bits from MODEMR1, too? Perhaps the time is ripe to add rcar_rst_read_mode_pins64(), so users can access more than 32 bits on SoCs that provide it (R-Car V3H and V3U)? At least the numbering is sane on R-Car V3U. On R-Car V3H, MD29 and higher are stored starting at bit 1 of the second MODEMR register... > +}; Gr{oetje,eeting}s, Geert
Hi Shimoda-san, Thanks for your patch! On Mon, Sep 7, 2020 at 11:20 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > Initial support for R-Car V3U (r8a779a0), including core, module > clocks and register access, because register specification differs > than R-Car Gen2/3. differs from > Inspired by patches in the BSP by LUU HOAI. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > --- a/drivers/clk/renesas/Kconfig > +++ b/drivers/clk/renesas/Kconfig > @@ -30,6 +30,7 @@ config CLK_RENESAS > select CLK_R8A77980 if ARCH_R8A77980 > select CLK_R8A77990 if ARCH_R8A77990 > select CLK_R8A77995 if ARCH_R8A77995 > + select CLK_R8A779A0 if ARCH_R8A779A0 > select CLK_R9A06G032 if ARCH_R9A06G032 > select CLK_SH73A0 if ARCH_SH73A0 > > @@ -145,6 +146,9 @@ config CLK_R8A77995 > bool "R-Car D3 clock support" if COMPILE_TEST > select CLK_RCAR_GEN3_CPG > > +config CLK_R8A779A0 > + bool "R-Car V3U clock support" if COMPILE_TEST Missing "select CLK_RENESAS_CPG_MSSR" > + > config CLK_R9A06G032 > bool "Renesas R9A06G032 clock driver" > help > --- /dev/null > +++ b/drivers/clk/renesas/r8a779a0-cpg-mssr.c > @@ -0,0 +1,479 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * r8a779a0 Clock Pulse Generator / Module Standby and Software Reset > + * > + * Copyright (C) 2020 Renesas Electronics Corp. > + * > + * Based on r8a7795-cpg-mssr.c > + * > + * Copyright (C) 2015 Glider bvba > + * Copyright (C) 2015 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/kernel.h> > +#include <linux/pm.h> > +#include <linux/slab.h> > +#include <linux/soc/renesas/rcar-rst.h> > +#include <linux/sys_soc.h> This include is not needed. > + > +#include <dt-bindings/clock/r8a779a0-cpg-mssr.h> > + > +#include "renesas-cpg-mssr.h" > + > +enum rcar_r8a779a0_clk_types { > + CLK_TYPE_R8A779A0_MAIN = CLK_TYPE_CUSTOM, > + CLK_TYPE_R8A779A0_PLL1, > + CLK_TYPE_R8A779A0_PLL20, > + CLK_TYPE_R8A779A0_PLL21, > + CLK_TYPE_R8A779A0_PLL30, > + CLK_TYPE_R8A779A0_PLL31, > + CLK_TYPE_R8A779A0_PLL5, > + CLK_TYPE_R8A779A0_MDSEL, /* Select parent/divider using mode pin */ > + CLK_TYPE_R8A779A0_Z, > + CLK_TYPE_R8A779A0_OSC, /* OSC EXTAL predivider and fixed divider */ > + > + /* SoC specific definitions start here */ > + CLK_TYPE_R8A779A0_SOC_BASE, This last definition is not needed. > +}; > + > +#define DEF_R8A779A0_MDSEL(_name, _id, _md, _parent0, _div0, _parent1, _div1) \ > + DEF_BASE(_name, _id, CLK_TYPE_R8A779A0_MDSEL, \ > + (_parent0) << 16 | (_parent1), \ > + .div = (_div0) << 16 | (_div1), .offset = _md) > + > +#define DEF_R8A779A0_OSC(_name, _id, _parent, _div) \ > + DEF_BASE(_name, _id, CLK_TYPE_R8A779A0_OSC, _parent, .div = _div) Perhaps you can just include "rcar-gen3-cpg.h", so you can use the DEF_GEN3_MDSEL() and DEF_GEN3_OSC() definitions now (and probably DEF_GEN3_SD later)? > + > +struct rcar_r8a779a0_cpg_pll_config { > + u8 extal_div; > + u8 pll1_mult; > + u8 pll1_div; > + u8 pll5_mult; > + u8 pll5_div; > + u8 osc_prediv; > +}; > + > +enum clk_ids { > + /* Core Clock Outputs exported to DT */ > + LAST_DT_CORE_CLK = R8A779A0_CLK_OSCCLK, > + > + /* External Input Clocks */ > + CLK_EXTAL, > + CLK_EXTALR, > + > + /* Internal Core Clocks */ > + CLK_MAIN, > + CLK_PLL1, > + CLK_PLL20, > + CLK_PLL21, > + CLK_PLL30, > + CLK_PLL31, > + CLK_PLL5, > + CLK_PLL1_DIV2, > + CLK_PLL20_DIV2, > + CLK_PLL21_DIV2, > + CLK_PLL30_DIV2, > + CLK_PLL31_DIV2, > + CLK_PLL5_DIV2, > + CLK_PLL5_DIV4, > + CLK_S1, > + CLK_S2, > + CLK_S3, > + CLK_SDSRC, > + CLK_RPCSRC, > + CLK_OCO, > + > + /* Module Clocks */ > + MOD_CLK_BASE > +}; > + > +static const struct cpg_core_clk r8a779a0_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_R8A779A0_MAIN, CLK_EXTAL), > + DEF_BASE(".pll1", CLK_PLL1, CLK_TYPE_R8A779A0_PLL1, CLK_MAIN), > + DEF_BASE(".pll20", CLK_PLL20, CLK_TYPE_R8A779A0_PLL20, CLK_MAIN), > + DEF_BASE(".pll21", CLK_PLL21, CLK_TYPE_R8A779A0_PLL21, CLK_MAIN), > + DEF_BASE(".pll30", CLK_PLL30, CLK_TYPE_R8A779A0_PLL30, CLK_MAIN), > + DEF_BASE(".pll31", CLK_PLL31, CLK_TYPE_R8A779A0_PLL31, CLK_MAIN), > + DEF_BASE(".pll5", CLK_PLL5, CLK_TYPE_R8A779A0_PLL5, CLK_MAIN), > + > + DEF_FIXED(".pll1_div2", CLK_PLL1_DIV2, CLK_PLL1, 2, 1), > + DEF_FIXED(".pll20_div2", CLK_PLL20_DIV2, CLK_PLL20, 2, 1), > + DEF_FIXED(".pll21_div2", CLK_PLL21_DIV2, CLK_PLL21, 2, 1), > + DEF_FIXED(".pll30_div2", CLK_PLL30_DIV2, CLK_PLL30, 2, 1), > + DEF_FIXED(".pll31_div2", CLK_PLL31_DIV2, CLK_PLL31, 2, 1), > + DEF_FIXED(".pll5_div2", CLK_PLL5_DIV2, CLK_PLL5, 2, 1), Please align the values in the last column. > + DEF_FIXED(".pll5_div4", CLK_PLL5_DIV4, CLK_PLL5_DIV2, 2, 1), > + DEF_FIXED(".s1", CLK_S1, CLK_PLL1_DIV2, 2, 1), > + DEF_FIXED(".s2", CLK_S2, CLK_PLL1_DIV2, 133, 50), div = 8, mul = 3 sounds more likely. Or perhaps S2 is derived from PLL5, which runs at a multiple of 400 MHz? Or perhaps S2 does not exist... > + DEF_FIXED(".s3", CLK_S3, CLK_PLL1_DIV2, 4, 1), > + DEF_RATE(".oco", CLK_OCO, 32768), > + > + /* Core Clock Outputs */ > + DEF_FIXED("zx", R8A779A0_CLK_ZX, CLK_PLL20_DIV2, 2, 1), > + DEF_FIXED("s1d1", R8A779A0_CLK_S1D1, CLK_S1, 1, 1), > + DEF_FIXED("s1d2", R8A779A0_CLK_S1D2, CLK_S1, 2, 1), > + DEF_FIXED("s1d4", R8A779A0_CLK_S1D4, CLK_S1, 4, 1), > + DEF_FIXED("s1d8", R8A779A0_CLK_S1D8, CLK_S1, 8, 1), > + DEF_FIXED("s1d12", R8A779A0_CLK_S1D12, CLK_S1, 12, 1), > + DEF_FIXED("s2d1", R8A779A0_CLK_S2D1, CLK_S2, 1, 1), > + DEF_FIXED("s2d2", R8A779A0_CLK_S2D2, CLK_S2, 2, 1), > + DEF_FIXED("s2d4", R8A779A0_CLK_S2D4, CLK_S2, 4, 1), > + DEF_FIXED("s3d1", R8A779A0_CLK_S3D1, CLK_S3, 1, 1), > + DEF_FIXED("s3d2", R8A779A0_CLK_S3D2, CLK_S3, 2, 1), > + DEF_FIXED("s3d4", R8A779A0_CLK_S3D4, CLK_S3, 4, 1), Please align the values in the last column. > + DEF_FIXED("zs", R8A779A0_CLK_ZS, CLK_PLL1_DIV2, 4, 1), > + DEF_FIXED("zt", R8A779A0_CLK_ZT, CLK_PLL1_DIV2, 2, 1), > + DEF_FIXED("ztr", R8A779A0_CLK_ZTR, CLK_PLL1_DIV2, 2, 1), > + DEF_FIXED("zr", R8A779A0_CLK_ZR, CLK_PLL1_DIV2, 1, 1), ZR = PLL6 / 4. Is PLL6 the same as PLL1, they always run at the same rate? There don't seem to be any registers documented to control PLL6VCO. > + DEF_FIXED("dsi", R8A779A0_CLK_DSI, CLK_PLL5_DIV4, 1, 1), > + DEF_FIXED("cnndsp", R8A779A0_CLK_CNNDSP, CLK_PLL5_DIV4, 1, 1), > + DEF_FIXED("vip", R8A779A0_CLK_VIP, CLK_PLL5, 5, 1), > + DEF_FIXED("adgh", R8A779A0_CLK_ADGH, CLK_PLL5_DIV4, 1, 1), > + DEF_FIXED("icu", R8A779A0_CLK_ICU, CLK_PLL5_DIV4, 2, 1), > + DEF_FIXED("icud2", R8A779A0_CLK_ICUD2, CLK_PLL5_DIV4, 4, 1), > + DEF_FIXED("vcbus", R8A779A0_CLK_VCBUS, CLK_PLL5_DIV4, 1, 1), > + > + DEF_FIXED("cbfusa", R8A779A0_CLK_CBFUSA, CLK_MAIN, 2, 1), > + > + DEF_DIV6P1("mso", R8A779A0_CLK_MSO, CLK_PLL5_DIV4, 0x87c), > + DEF_DIV6P1("canfd", R8A779A0_CLK_CANFD, CLK_PLL5_DIV4, 0x878), > + DEF_DIV6P1("csi0", R8A779A0_CLK_CSI0, CLK_PLL5_DIV4, 0x880), > + DEF_DIV6P1("fray", R8A779A0_CLK_FRAY, CLK_PLL5_DIV4, 0x88c), The FRAY register doesn't seem to be documented? > + DEF_DIV6P1("post", R8A779A0_CLK_POST, CLK_PLL5_DIV4, 0x890), > + DEF_DIV6P1("post2", R8A779A0_CLK_POST2, CLK_PLL5_DIV4, 0x894), > + DEF_DIV6P1("post3", R8A779A0_CLK_POST3, CLK_PLL5_DIV4, 0x898), > + > + DEF_R8A779A0_OSC("osc", R8A779A0_CLK_OSCCLK, CLK_EXTAL, 8), > + DEF_R8A779A0_MDSEL("r", R8A779A0_CLK_RCLK, 29, CLK_EXTALR, 1, CLK_OCO, 1), > +}; > + > +static const struct mssr_mod_clk r8a779a0_mod_clks[] __initconst = { > + DEF_MOD("scif0", 702, R8A779A0_CLK_S1D8), > + DEF_MOD("scif1", 703, R8A779A0_CLK_S1D8), > + DEF_MOD("scif3", 704, R8A779A0_CLK_S1D8), > + DEF_MOD("scif4", 705, R8A779A0_CLK_S1D8), > +}; > + > +static const unsigned int r8a779a0_crit_mod_clks[] __initconst = { > + MOD_CLK_ID(408), /* INTC-AP (GIC) */ There's no entry for this clock in r8a779a0_mod_clks[] above, so please drop this. Also, the INTC-AP clock is not documented, unfortunately. > +}; > + > +#define CPG_PLL20CR 0x0834 > +#define CPG_PLL21CR 0x0838 > +#define CPG_PLL30CR 0x083c > +#define CPG_PLL31CR 0x0840 > + > +static spinlock_t cpg_lock; > + > +static void cpg_reg_modify(void __iomem *reg, u32 clear, u32 set) > +{ > + unsigned long flags; > + u32 val; > + > + spin_lock_irqsave(&cpg_lock, flags); > + val = readl(reg); > + val &= ~clear; > + val |= set; > + writel(val, reg); > + spin_unlock_irqrestore(&cpg_lock, flags); > +}; > + > +/* > + * Z Clock & Z2 Clock All the Z clock code is currently unused. > +struct clk * __init rcar_r8a779a0_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) > +{ > + case CLK_TYPE_R8A779A0_PLL20: > + value = readl(base + CPG_PLL20CR); > + mult = (((value >> 24) & 0x7f) + 1) * 2; > + break; > + > + case CLK_TYPE_R8A779A0_PLL21: > + value = readl(base + CPG_PLL21CR); > + mult = (((value >> 24) & 0x7f) + 1) * 2; > + break; > + > + case CLK_TYPE_R8A779A0_PLL30: > + value = readl(base + CPG_PLL30CR); > + mult = (((value >> 24) & 0x7f) + 1) * 2; > + break; > + > + case CLK_TYPE_R8A779A0_PLL31: > + value = readl(base + CPG_PLL31CR); > + mult = (((value >> 24) & 0x7f) + 1) * 2; > + break; Perhaps CLK_TYPE_R8A779A0_PLL[23][01] can share a common type, encoding the register offset in cpg_core_clk.offset? > +const struct cpg_mssr_info r8a779a0_cpg_mssr_info __initconst = { > + /* Core Clocks */ > + .core_clks = r8a779a0_core_clks, > + .num_core_clks = ARRAY_SIZE(r8a779a0_core_clks), > + .last_dt_core_clk = LAST_DT_CORE_CLK, > + .num_total_core_clks = MOD_CLK_BASE, > + > + /* Module Clocks */ > + .mod_clks = r8a779a0_mod_clks, > + .num_mod_clks = ARRAY_SIZE(r8a779a0_mod_clks), > + .num_hw_mod_clks = 12 * 32, 15 * 32 > + > + /* Critical Module Clocks */ > + .crit_mod_clks = r8a779a0_crit_mod_clks, > + .num_crit_mod_clks = ARRAY_SIZE(r8a779a0_crit_mod_clks), > + > + /* Callbacks */ > + .init = r8a779a0_cpg_mssr_init, > + .cpg_clk_register = rcar_r8a779a0_cpg_clk_register, > + > + /* The device has only MSTP Control Register */ > + .mstpctrl = true, > +}; > diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c b/drivers/clk/renesas/renesas-cpg-mssr.c > index 5a306d2..c259e05 100644 > --- a/drivers/clk/renesas/renesas-cpg-mssr.c > +++ b/drivers/clk/renesas/renesas-cpg-mssr.c > @@ -57,8 +57,14 @@ static const u16 mstpsr[] = { > 0x9A0, 0x9A4, 0x9A8, 0x9AC, > }; > > -#define MSTPSR(i) mstpsr[i] > +static const u16 mstpsr_mstpcr_only[] = { > + 0x2E00, 0x2E04, 0x2E08, 0x2E0C, 0x2E10, 0x2E14, 0x2E18, 0x2E1C, > + 0x2E20, 0x2E24, 0x2E28, 0x2E2C, 0x2E30, 0x2E34, 0x2E38, > +}; > + > +static bool mstpcr_only; > > +#define MSTPSR(i) (mstpcr_only ? mstpsr_mstpcr_only[i] : mstpsr[i]) IMHO, ugly macro handling... > > /* > * System Module Stop Control Register offsets > @@ -69,7 +75,8 @@ static const u16 smstpcr[] = { > 0x990, 0x994, 0x998, 0x99C, > }; > > -#define SMSTPCR(i) smstpcr[i] > +#define SMSTPCR(i) (mstpcr_only ? (mstpsr_mstpcr_only[i] - 0x100) : \ > + smstpcr[i]) .. more ugly macro handling. Still, it doesn't handle the Software Reset and Clearing registers. > @@ -140,7 +147,7 @@ struct cpg_mssr_priv { > struct { > u32 mask; > u32 val; > - } smstpcr_saved[ARRAY_SIZE(smstpcr)]; > + } smstpcr_saved[ARRAY_SIZE(mstpsr_mstpcr_only)]; Basically the maximum number of registers to save. > @@ -938,6 +951,7 @@ static int __init cpg_mssr_common_init(struct device *dev, > priv->last_dt_core_clk = info->last_dt_core_clk; > RAW_INIT_NOTIFIER_HEAD(&priv->notifiers); > priv->stbyctrl = info->stbyctrl; > + mstpcr_only = info->mstpctrl; > > for (i = 0; i < nclks; i++) > priv->clks[i] = ERR_PTR(-ENOENT); > diff --git a/drivers/clk/renesas/renesas-cpg-mssr.h b/drivers/clk/renesas/renesas-cpg-mssr.h > index 1cc5694..061eb83 100644 > --- a/drivers/clk/renesas/renesas-cpg-mssr.h > +++ b/drivers/clk/renesas/renesas-cpg-mssr.h > @@ -131,6 +131,7 @@ struct cpg_mssr_info { > unsigned int last_dt_core_clk; > unsigned int num_total_core_clks; > bool stbyctrl; > + bool mstpctrl; Personally, I don't like adding a second boolean flag, to be checked in multiple places, hidden inside macros. I think this can be handled better by defining multiple registers layout variants using an enum, for 1. R-Car Gen2/3-style register layouts, 2. RZ/A-style register layouts, 3. R-Car V3U-style register layout. Then register tables pointers can be set up based on the enum value, and code can check the enum value where needed. Note that layout 1 can also be used for SH/R-Mobile, and we could easily add a 4th layout for R-Car Gen1, if we ever migrate SH/R-Mobile and/or R-Car Gen1 to CPG/MSSR. What do you think? > > /* Module Clocks */ > const struct mssr_mod_clk *mod_clks; Gr{oetje,eeting}s, Geert
Hi Shimoda-san, On Mon, Sep 7, 2020 at 11:20 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > Add initial support for the Renesas R8A77990 (R-Car V3U) support. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Thanks for your patch! > --- /dev/null > +++ b/arch/arm64/boot/dts/renesas/r8a779a0.dtsi > + soc: soc { > + > + sysc: system-controller@e6180000 { > + compatible = "renesas,r8a779a0-sysc"; > + reg = <0 0xe6180000 0 0x3078>; Length 0x4000? > + #power-domain-cells = <1>; > + }; > + > + scif0: serial@e6e60000 { > + compatible = "renesas,scif-r8a779a0", > + "renesas,rcar-gen3-scif", "renesas,scif"; > + reg = <0 0xe6e60000 0 64>; > + interrupts = <GIC_SPI 251 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&cpg CPG_MOD 702>, > + <&cpg CPG_CORE R8A779A0_CLK_S1D2>, > + <&scif_clk>; > + clock-names = "fck", "brg_int", "scif_clk"; > + power-domains = <&sysc R8A779A0_PD_ALWAYS_ON>; Missing resets property. > + status = "disabled"; > + }; > + > + gic: interrupt-controller@f1000000 { > + compatible = "arm,gic-v3"; > + #interrupt-cells = <3>; > + #address-cells = <0>; > + interrupt-controller; > + reg = <0x0 0xf1000000 0 0x20000>, > + <0x0 0xf1060000 0 0x110000>; > + interrupts = <GIC_PPI 9 > + (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>; "GIC_CPU_MASK_SIMPLE(1)", as currently only one CPU core is used. > + power-domains = <&sysc R8A779A0_PD_ALWAYS_ON>; > + }; > + > + prr: chipid@fff00044 { > + compatible = "renesas,prr"; > + reg = <0 0xfff00044 0 4>; > + }; > + }; > + > + timer { > + compatible = "arm,armv8-timer"; > + interrupts-extended = <&gic GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>, > + <&gic GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>, > + <&gic GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>, > + <&gic GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>; "GIC_CPU_MASK_SIMPLE(1)" for all 4 interrupts (and in the future "8", not "2"). > + }; > +}; > -- > 2.7.4 > -- 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 Mon, Sep 7, 2020 at 11:20 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > Initial support for the Renesas Falcon CPU and BreakOut boards > support. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Thanks for your patch! > --- /dev/null > +++ b/arch/arm64/boot/dts/renesas/falcon-cpu.dtsi > @@ -0,0 +1,44 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Device Tree Source for the Falcon CPU board > + * > + * Copyright (C) 2020 Renesas Electronics Corp. > + */ As this board contains the CPU, I had expected #include "r8a779a0.dtsi" here. > + > +/ { > + model = "Renesas Falcon CPU board"; > + compatible = "renesas,falcon-cpu"; + renesas,r8a779a0. Gr{oetje,eeting}s, Geert
Hi Geert-san, Thank you for your review! > From: Geert Uytterhoeven, Sent: Wednesday, September 9, 2020 12:22 AM > > Hi Shimoda-san, > > Thanks for your patch! > > On Mon, Sep 7, 2020 at 11:20 AM Yoshihiro Shimoda > <yoshihiro.shimoda.uh@renesas.com> wrote: > > Initial support for R-Car V3U (r8a779a0), including core, module > > clocks and register access, because register specification differs > > than R-Car Gen2/3. > > differs from Oops. I'll fix it. > > Inspired by patches in the BSP by LUU HOAI. > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > > --- a/drivers/clk/renesas/Kconfig > > +++ b/drivers/clk/renesas/Kconfig > > @@ -30,6 +30,7 @@ config CLK_RENESAS > > select CLK_R8A77980 if ARCH_R8A77980 > > select CLK_R8A77990 if ARCH_R8A77990 > > select CLK_R8A77995 if ARCH_R8A77995 > > + select CLK_R8A779A0 if ARCH_R8A779A0 > > select CLK_R9A06G032 if ARCH_R9A06G032 > > select CLK_SH73A0 if ARCH_SH73A0 > > > > @@ -145,6 +146,9 @@ config CLK_R8A77995 > > bool "R-Car D3 clock support" if COMPILE_TEST > > select CLK_RCAR_GEN3_CPG > > > > +config CLK_R8A779A0 > > + bool "R-Car V3U clock support" if COMPILE_TEST > > Missing "select CLK_RENESAS_CPG_MSSR" I got it. I'll add this. > > + > > config CLK_R9A06G032 > > bool "Renesas R9A06G032 clock driver" > > help > > > --- /dev/null > > +++ b/drivers/clk/renesas/r8a779a0-cpg-mssr.c > > @@ -0,0 +1,479 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * r8a779a0 Clock Pulse Generator / Module Standby and Software Reset > > + * > > + * Copyright (C) 2020 Renesas Electronics Corp. > > + * > > + * Based on r8a7795-cpg-mssr.c > > + * > > + * Copyright (C) 2015 Glider bvba > > + * Copyright (C) 2015 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/kernel.h> > > +#include <linux/pm.h> > > +#include <linux/slab.h> > > +#include <linux/soc/renesas/rcar-rst.h> > > +#include <linux/sys_soc.h> > > This include is not needed. I got it. > > + > > +#include <dt-bindings/clock/r8a779a0-cpg-mssr.h> > > + > > +#include "renesas-cpg-mssr.h" > > + > > +enum rcar_r8a779a0_clk_types { > > + CLK_TYPE_R8A779A0_MAIN = CLK_TYPE_CUSTOM, > > + CLK_TYPE_R8A779A0_PLL1, > > + CLK_TYPE_R8A779A0_PLL20, > > + CLK_TYPE_R8A779A0_PLL21, > > + CLK_TYPE_R8A779A0_PLL30, > > + CLK_TYPE_R8A779A0_PLL31, > > + CLK_TYPE_R8A779A0_PLL5, > > + CLK_TYPE_R8A779A0_MDSEL, /* Select parent/divider using mode pin */ > > + CLK_TYPE_R8A779A0_Z, > > + CLK_TYPE_R8A779A0_OSC, /* OSC EXTAL predivider and fixed divider */ > > + > > + /* SoC specific definitions start here */ > > + CLK_TYPE_R8A779A0_SOC_BASE, > > This last definition is not needed. I got it. > > +}; > > + > > +#define DEF_R8A779A0_MDSEL(_name, _id, _md, _parent0, _div0, _parent1, _div1) \ > > + DEF_BASE(_name, _id, CLK_TYPE_R8A779A0_MDSEL, \ > > + (_parent0) << 16 | (_parent1), \ > > + .div = (_div0) << 16 | (_div1), .offset = _md) > > + > > +#define DEF_R8A779A0_OSC(_name, _id, _parent, _div) \ > > + DEF_BASE(_name, _id, CLK_TYPE_R8A779A0_OSC, _parent, .div = _div) > > Perhaps you can just include "rcar-gen3-cpg.h", so you can use the > DEF_GEN3_MDSEL() and DEF_GEN3_OSC() definitions now (and probably > DEF_GEN3_SD later)? I think so. I'll include "rcar-gen3-cpg.h". > > + > > +struct rcar_r8a779a0_cpg_pll_config { > > + u8 extal_div; > > + u8 pll1_mult; > > + u8 pll1_div; > > + u8 pll5_mult; > > + u8 pll5_div; > > + u8 osc_prediv; > > +}; > > + > > +enum clk_ids { > > + /* Core Clock Outputs exported to DT */ > > + LAST_DT_CORE_CLK = R8A779A0_CLK_OSCCLK, > > + > > + /* External Input Clocks */ > > + CLK_EXTAL, > > + CLK_EXTALR, > > + > > + /* Internal Core Clocks */ > > + CLK_MAIN, > > + CLK_PLL1, > > + CLK_PLL20, > > + CLK_PLL21, > > + CLK_PLL30, > > + CLK_PLL31, > > + CLK_PLL5, > > + CLK_PLL1_DIV2, > > + CLK_PLL20_DIV2, > > + CLK_PLL21_DIV2, > > + CLK_PLL30_DIV2, > > + CLK_PLL31_DIV2, > > + CLK_PLL5_DIV2, > > + CLK_PLL5_DIV4, > > + CLK_S1, > > + CLK_S2, > > + CLK_S3, > > + CLK_SDSRC, > > + CLK_RPCSRC, > > + CLK_OCO, > > + > > + /* Module Clocks */ > > + MOD_CLK_BASE > > +}; > > + > > +static const struct cpg_core_clk r8a779a0_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_R8A779A0_MAIN, CLK_EXTAL), > > + DEF_BASE(".pll1", CLK_PLL1, CLK_TYPE_R8A779A0_PLL1, CLK_MAIN), > > + DEF_BASE(".pll20", CLK_PLL20, CLK_TYPE_R8A779A0_PLL20, CLK_MAIN), > > + DEF_BASE(".pll21", CLK_PLL21, CLK_TYPE_R8A779A0_PLL21, CLK_MAIN), > > + DEF_BASE(".pll30", CLK_PLL30, CLK_TYPE_R8A779A0_PLL30, CLK_MAIN), > > + DEF_BASE(".pll31", CLK_PLL31, CLK_TYPE_R8A779A0_PLL31, CLK_MAIN), > > + DEF_BASE(".pll5", CLK_PLL5, CLK_TYPE_R8A779A0_PLL5, CLK_MAIN), > > + > > + DEF_FIXED(".pll1_div2", CLK_PLL1_DIV2, CLK_PLL1, 2, 1), > > + DEF_FIXED(".pll20_div2", CLK_PLL20_DIV2, CLK_PLL20, 2, 1), > > + DEF_FIXED(".pll21_div2", CLK_PLL21_DIV2, CLK_PLL21, 2, 1), > > + DEF_FIXED(".pll30_div2", CLK_PLL30_DIV2, CLK_PLL30, 2, 1), > > + DEF_FIXED(".pll31_div2", CLK_PLL31_DIV2, CLK_PLL31, 2, 1), > > + DEF_FIXED(".pll5_div2", CLK_PLL5_DIV2, CLK_PLL5, 2, 1), > > Please align the values in the last column. I got it. > > + DEF_FIXED(".pll5_div4", CLK_PLL5_DIV4, CLK_PLL5_DIV2, 2, 1), > > + DEF_FIXED(".s1", CLK_S1, CLK_PLL1_DIV2, 2, 1), > > + DEF_FIXED(".s2", CLK_S2, CLK_PLL1_DIV2, 133, 50), > > div = 8, mul = 3 sounds more likely. > Or perhaps S2 is derived from PLL5, which runs at a multiple of 400 MHz? > Or perhaps S2 does not exist... As I mentioned patch 7/14, S2 doesn't exist... > > + DEF_FIXED(".s3", CLK_S3, CLK_PLL1_DIV2, 4, 1), > > + DEF_RATE(".oco", CLK_OCO, 32768), > > + > > + /* Core Clock Outputs */ > > + DEF_FIXED("zx", R8A779A0_CLK_ZX, CLK_PLL20_DIV2, 2, 1), > > + DEF_FIXED("s1d1", R8A779A0_CLK_S1D1, CLK_S1, 1, 1), > > + DEF_FIXED("s1d2", R8A779A0_CLK_S1D2, CLK_S1, 2, 1), > > + DEF_FIXED("s1d4", R8A779A0_CLK_S1D4, CLK_S1, 4, 1), > > + DEF_FIXED("s1d8", R8A779A0_CLK_S1D8, CLK_S1, 8, 1), > > + DEF_FIXED("s1d12", R8A779A0_CLK_S1D12, CLK_S1, 12, 1), > > + DEF_FIXED("s2d1", R8A779A0_CLK_S2D1, CLK_S2, 1, 1), > > + DEF_FIXED("s2d2", R8A779A0_CLK_S2D2, CLK_S2, 2, 1), > > + DEF_FIXED("s2d4", R8A779A0_CLK_S2D4, CLK_S2, 4, 1), > > + DEF_FIXED("s3d1", R8A779A0_CLK_S3D1, CLK_S3, 1, 1), > > + DEF_FIXED("s3d2", R8A779A0_CLK_S3D2, CLK_S3, 2, 1), > > + DEF_FIXED("s3d4", R8A779A0_CLK_S3D4, CLK_S3, 4, 1), > > Please align the values in the last column. I got it. > > + DEF_FIXED("zs", R8A779A0_CLK_ZS, CLK_PLL1_DIV2, 4, 1), > > + DEF_FIXED("zt", R8A779A0_CLK_ZT, CLK_PLL1_DIV2, 2, 1), > > + DEF_FIXED("ztr", R8A779A0_CLK_ZTR, CLK_PLL1_DIV2, 2, 1), > > + DEF_FIXED("zr", R8A779A0_CLK_ZR, CLK_PLL1_DIV2, 1, 1), > > ZR = PLL6 / 4. > Is PLL6 the same as PLL1, they always run at the same rate? > There don't seem to be any registers documented to control PLL6VCO. According to the internal manual, V3U doesn't have PLL6. And, ZR is connected to PLL1 / 2. > > + DEF_FIXED("dsi", R8A779A0_CLK_DSI, CLK_PLL5_DIV4, 1, 1), > > + DEF_FIXED("cnndsp", R8A779A0_CLK_CNNDSP, CLK_PLL5_DIV4, 1, 1), > > + DEF_FIXED("vip", R8A779A0_CLK_VIP, CLK_PLL5, 5, 1), > > + DEF_FIXED("adgh", R8A779A0_CLK_ADGH, CLK_PLL5_DIV4, 1, 1), > > + DEF_FIXED("icu", R8A779A0_CLK_ICU, CLK_PLL5_DIV4, 2, 1), > > + DEF_FIXED("icud2", R8A779A0_CLK_ICUD2, CLK_PLL5_DIV4, 4, 1), > > + DEF_FIXED("vcbus", R8A779A0_CLK_VCBUS, CLK_PLL5_DIV4, 1, 1), > > + > > + DEF_FIXED("cbfusa", R8A779A0_CLK_CBFUSA, CLK_MAIN, 2, 1), > > + > > + DEF_DIV6P1("mso", R8A779A0_CLK_MSO, CLK_PLL5_DIV4, 0x87c), > > + DEF_DIV6P1("canfd", R8A779A0_CLK_CANFD, CLK_PLL5_DIV4, 0x878), > > + DEF_DIV6P1("csi0", R8A779A0_CLK_CSI0, CLK_PLL5_DIV4, 0x880), > > + DEF_DIV6P1("fray", R8A779A0_CLK_FRAY, CLK_PLL5_DIV4, 0x88c), > > The FRAY register doesn't seem to be documented? It seems so... So, since we are not using this clock for now, I'll drop it. > > + DEF_DIV6P1("post", R8A779A0_CLK_POST, CLK_PLL5_DIV4, 0x890), > > + DEF_DIV6P1("post2", R8A779A0_CLK_POST2, CLK_PLL5_DIV4, 0x894), > > + DEF_DIV6P1("post3", R8A779A0_CLK_POST3, CLK_PLL5_DIV4, 0x898), > > + > > + DEF_R8A779A0_OSC("osc", R8A779A0_CLK_OSCCLK, CLK_EXTAL, 8), > > + DEF_R8A779A0_MDSEL("r", R8A779A0_CLK_RCLK, 29, CLK_EXTALR, 1, CLK_OCO, 1), > > +}; > > + > > +static const struct mssr_mod_clk r8a779a0_mod_clks[] __initconst = { > > + DEF_MOD("scif0", 702, R8A779A0_CLK_S1D8), > > + DEF_MOD("scif1", 703, R8A779A0_CLK_S1D8), > > + DEF_MOD("scif3", 704, R8A779A0_CLK_S1D8), > > + DEF_MOD("scif4", 705, R8A779A0_CLK_S1D8), > > +}; > > + > > +static const unsigned int r8a779a0_crit_mod_clks[] __initconst = { > > + MOD_CLK_ID(408), /* INTC-AP (GIC) */ > > There's no entry for this clock in r8a779a0_mod_clks[] above, so please > drop this. > Also, the INTC-AP clock is not documented, unfortunately. I got it. I'll drop it. > > +}; > > + > > +#define CPG_PLL20CR 0x0834 > > +#define CPG_PLL21CR 0x0838 > > +#define CPG_PLL30CR 0x083c > > +#define CPG_PLL31CR 0x0840 > > + > > +static spinlock_t cpg_lock; > > + > > +static void cpg_reg_modify(void __iomem *reg, u32 clear, u32 set) > > +{ > > + unsigned long flags; > > + u32 val; > > + > > + spin_lock_irqsave(&cpg_lock, flags); > > + val = readl(reg); > > + val &= ~clear; > > + val |= set; > > + writel(val, reg); > > + spin_unlock_irqrestore(&cpg_lock, flags); > > +}; > > + > > +/* > > + * Z Clock & Z2 Clock > > All the Z clock code is currently unused. Oops. You're correct. I'll drop Z clock code. > > +struct clk * __init rcar_r8a779a0_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) > > +{ > > > + case CLK_TYPE_R8A779A0_PLL20: > > + value = readl(base + CPG_PLL20CR); > > + mult = (((value >> 24) & 0x7f) + 1) * 2; > > + break; > > + > > + case CLK_TYPE_R8A779A0_PLL21: > > + value = readl(base + CPG_PLL21CR); > > + mult = (((value >> 24) & 0x7f) + 1) * 2; > > + break; > > + > > + case CLK_TYPE_R8A779A0_PLL30: > > + value = readl(base + CPG_PLL30CR); > > + mult = (((value >> 24) & 0x7f) + 1) * 2; > > + break; > > + > > + case CLK_TYPE_R8A779A0_PLL31: > > + value = readl(base + CPG_PLL31CR); > > + mult = (((value >> 24) & 0x7f) + 1) * 2; > > + break; > > Perhaps CLK_TYPE_R8A779A0_PLL[23][01] can share a common type, encoding > the register offset in cpg_core_clk.offset? I think so. If so, should I add a new macro in the r8a779a0-cpg-mssr.c to set .offset? > > +const struct cpg_mssr_info r8a779a0_cpg_mssr_info __initconst = { > > + /* Core Clocks */ > > + .core_clks = r8a779a0_core_clks, > > + .num_core_clks = ARRAY_SIZE(r8a779a0_core_clks), > > + .last_dt_core_clk = LAST_DT_CORE_CLK, > > + .num_total_core_clks = MOD_CLK_BASE, > > + > > + /* Module Clocks */ > > + .mod_clks = r8a779a0_mod_clks, > > + .num_mod_clks = ARRAY_SIZE(r8a779a0_mod_clks), > > + .num_hw_mod_clks = 12 * 32, > > 15 * 32 Oops. I'll fix it. > > + > > + /* Critical Module Clocks */ > > + .crit_mod_clks = r8a779a0_crit_mod_clks, > > + .num_crit_mod_clks = ARRAY_SIZE(r8a779a0_crit_mod_clks), > > + > > + /* Callbacks */ > > + .init = r8a779a0_cpg_mssr_init, > > + .cpg_clk_register = rcar_r8a779a0_cpg_clk_register, > > + > > + /* The device has only MSTP Control Register */ > > + .mstpctrl = true, > > +}; > > diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c b/drivers/clk/renesas/renesas-cpg-mssr.c > > index 5a306d2..c259e05 100644 > > --- a/drivers/clk/renesas/renesas-cpg-mssr.c > > +++ b/drivers/clk/renesas/renesas-cpg-mssr.c > > @@ -57,8 +57,14 @@ static const u16 mstpsr[] = { > > 0x9A0, 0x9A4, 0x9A8, 0x9AC, > > }; > > > > -#define MSTPSR(i) mstpsr[i] > > +static const u16 mstpsr_mstpcr_only[] = { > > + 0x2E00, 0x2E04, 0x2E08, 0x2E0C, 0x2E10, 0x2E14, 0x2E18, 0x2E1C, > > + 0x2E20, 0x2E24, 0x2E28, 0x2E2C, 0x2E30, 0x2E34, 0x2E38, > > +}; > > + > > +static bool mstpcr_only; > > > > +#define MSTPSR(i) (mstpcr_only ? mstpsr_mstpcr_only[i] : mstpsr[i]) > > IMHO, ugly macro handling... > > > > > /* > > * System Module Stop Control Register offsets > > @@ -69,7 +75,8 @@ static const u16 smstpcr[] = { > > 0x990, 0x994, 0x998, 0x99C, > > }; > > > > -#define SMSTPCR(i) smstpcr[i] > > +#define SMSTPCR(i) (mstpcr_only ? (mstpsr_mstpcr_only[i] - 0x100) : \ > > + smstpcr[i]) > > .. more ugly macro handling. > Still, it doesn't handle the Software Reset and Clearing registers. > > > @@ -140,7 +147,7 @@ struct cpg_mssr_priv { > > struct { > > u32 mask; > > u32 val; > > - } smstpcr_saved[ARRAY_SIZE(smstpcr)]; > > + } smstpcr_saved[ARRAY_SIZE(mstpsr_mstpcr_only)]; > > Basically the maximum number of registers to save. > > > @@ -938,6 +951,7 @@ static int __init cpg_mssr_common_init(struct device *dev, > > priv->last_dt_core_clk = info->last_dt_core_clk; > > RAW_INIT_NOTIFIER_HEAD(&priv->notifiers); > > priv->stbyctrl = info->stbyctrl; > > + mstpcr_only = info->mstpctrl; > > > > for (i = 0; i < nclks; i++) > > priv->clks[i] = ERR_PTR(-ENOENT); > > diff --git a/drivers/clk/renesas/renesas-cpg-mssr.h b/drivers/clk/renesas/renesas-cpg-mssr.h > > index 1cc5694..061eb83 100644 > > --- a/drivers/clk/renesas/renesas-cpg-mssr.h > > +++ b/drivers/clk/renesas/renesas-cpg-mssr.h > > @@ -131,6 +131,7 @@ struct cpg_mssr_info { > > unsigned int last_dt_core_clk; > > unsigned int num_total_core_clks; > > bool stbyctrl; > > + bool mstpctrl; > > Personally, I don't like adding a second boolean flag, to be checked in > multiple places, hidden inside macros. I understood it. > I think this can be handled better by defining multiple registers layout > variants using an enum, for > 1. R-Car Gen2/3-style register layouts, > 2. RZ/A-style register layouts, > 3. R-Car V3U-style register layout. > Then register tables pointers can be set up based on the enum value, and > code can check the enum value where needed. > > Note that layout 1 can also be used for SH/R-Mobile, and we could easily > add a 4th layout for R-Car Gen1, if we ever migrate SH/R-Mobile and/or > R-Car Gen1 to CPG/MSSR. > > What do you think? It's a nice idea! I think the enum value of layout 1 should be 0 for minimum changes :) So, I'll try to implement such a code. Best regards, Yoshihiro Shimoda
Hi Shimoda-san, On Wed, Sep 9, 2020 at 4:53 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > > From: Geert Uytterhoeven, Sent: Wednesday, September 9, 2020 12:22 AM > > On Mon, Sep 7, 2020 at 11:20 AM Yoshihiro Shimoda > > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > Initial support for R-Car V3U (r8a779a0), including core, module > > > clocks and register access, because register specification differs > > > than R-Car Gen2/3. > > > +struct clk * __init rcar_r8a779a0_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) > > > +{ > > > > > + case CLK_TYPE_R8A779A0_PLL20: > > > + value = readl(base + CPG_PLL20CR); > > > + mult = (((value >> 24) & 0x7f) + 1) * 2; > > > + break; > > > + > > > + case CLK_TYPE_R8A779A0_PLL21: > > > + value = readl(base + CPG_PLL21CR); > > > + mult = (((value >> 24) & 0x7f) + 1) * 2; > > > + break; > > > + > > > + case CLK_TYPE_R8A779A0_PLL30: > > > + value = readl(base + CPG_PLL30CR); > > > + mult = (((value >> 24) & 0x7f) + 1) * 2; > > > + break; > > > + > > > + case CLK_TYPE_R8A779A0_PLL31: > > > + value = readl(base + CPG_PLL31CR); > > > + mult = (((value >> 24) & 0x7f) + 1) * 2; > > > + break; > > > > Perhaps CLK_TYPE_R8A779A0_PLL[23][01] can share a common type, encoding > > the register offset in cpg_core_clk.offset? > > I think so. If so, should I add a new macro in the r8a779a0-cpg-mssr.c to set .offset? Indeed, a new macro similar to the existing DEF_GEN3_SD(). Gr{oetje,eeting}s, Geert
Hi Geert-san, Thank you for your review! > From: Geert Uytterhoeven, Sent: Tuesday, September 8, 2020 8:20 PM > > Hi Shimoda-san, > > On Mon, Sep 7, 2020 at 11:20 AM Yoshihiro Shimoda > <yoshihiro.shimoda.uh@renesas.com> wrote: > > Add support for R-Car V3U (R8A779A0) SoC power areas and > > register access, because register specification differs > > than R-Car Gen2/3. > > > > Inspired by patches in the BSP by Tho Vu. > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > Thanks for your patch! > > > --- /dev/null > > +++ b/drivers/soc/renesas/r8a779a0-sysc.c > > @@ -0,0 +1,460 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Renesas R-Car V3U System Controller > > + * > > + * Copyright (C) 2020 Renesas Electronics Corp. > > + */ > > + > > +#include <linux/bits.h> > > +#include <linux/clk/renesas.h> > > +#include <linux/delay.h> > > +#include <linux/err.h> > > +#include <linux/kernel.h> > > +#include <linux/mm.h> > > +#include <linux/of_address.h> > > +#include <linux/pm_domain.h> > > +#include <linux/slab.h> > > +#include <linux/spinlock.h> > > +#include <linux/io.h> > > +#include <linux/soc/renesas/rcar-sysc.h> > > +#include <linux/sys_soc.h> > > +#include <linux/syscore_ops.h> > > The above 3 includes are not needed. I got it. > > + > > +#include <dt-bindings/power/r8a779a0-sysc.h> > > + > > +#include "rcar-sysc.h" > > You don't reuse the rcar-sysc driver itself, but you do reuse its header > file. As the comments in rcar-sysc.h refer to registers that have been > renamed (e.g. PWR*), and SYSCEXTMASK no longer exists, it might be > easier for the casual reader to drop the include, copy the PD_* > definitions, and define new r8a779a0_sysc_area and r8a779a0_sysc_info > structures instead, using the new naming. I understood it. I'll do that. > > + > > +static struct rcar_sysc_area r8a779a0_areas[] __initdata = { > > + { "always-on", 0, 0, R8A779A0_PD_ALWAYS_ON, -1, PD_ALWAYS_ON }, > > + { "a3e0", 0x1500, 0, R8A779A0_PD_A3E0, R8A779A0_PD_ALWAYS_ON, PD_SCU }, > > I think you can drop: > - chan_offs: it's always 0x1000 + pdr * 64, > - chan_bit: it's always zero, I got it. > > +/* SYSC Common */ > > +#define SYSCSR 0x000 /* SYSC Status Register */ > > +#define SYSCPONSR(x) (0x800 + ((x) * 0x4)) /* Power-ON Status Register 0 */ > > +#define SYSCPOFFSR(x) (0x808 + ((x) * 0x4)) /* Power-OFF Status Register */ > > +#define SYSCISCR(x) (0x810 + ((x) * 0x4)) /* Interrupt Status/Clear Register */ > > +#define SYSCIER(x) (0x820 + ((x) * 0x4)) /* Interrupt Enable Register */ > > +#define SYSCIMR(x) (0x830 + ((x) * 0x4)) /* Interrupt Mask Register */ > > + > > +/* Power Domain Registers */ > > +#define PDRSR(n) (0x1000 + ((n) * 0x40)) > > +#define PDRONCR(n) (0x1004 + ((n) * 0x40)) > > +#define PDROFFCR(n) (0x1008 + ((n) * 0x40)) > > +#define PDRESR(n) (0x100C + ((n) * 0x40)) > > While PDRESRn is described, and shown in a figure, it was forgotten in > the Table 9.2 ("List of Registers (Power Domain Registers for each power > domain)"). You're right. > > + > > +/* Power State */ > > +#define PW_ACTIVE 1 /* Active setting */ > > "/* PWRON/PWROFF */"? I'll fix these lines like below: /* PWRON/PWROFF */ #define PWRON_PWROFF BIT(0) /* Power-ON/OFF request */ > > + > > +/* PDRSR */ > > +#define PDRSR_OFF BIT(0) /* Power-OFF state */ > > +#define PDRSR_ON BIT(4) /* Power-ON state */ > > +#define PDRSR_OFF_STATE BIT(8) /* Processing Power-OFF sequence */ > > +#define PDRSR_ON_STATE BIT(12) /* Processing Power-ON sequence */ > > + > > +#define SYSCSR_PONENB 1 /* Ready for power resume requests */ > > +#define SYSCSR_POFFENB 0 /* Ready for power shutoff requests */ > > These two bits are now combined into a single BUSY bit mask: > (doh, all bits sets is not busy?!?) > > #define SYSCSR_BUSY GENMASK(1, 0) /* All bit sets is not busy */ I got it. I'll fix it. > > + > > +#define SYSCSR_RETRIES 1000 > > +#define SYSCSR_DELAY_US 10 > > + > > +#define PDRESR_RETRIES 1000 > > +#define PDRESR_DELAY_US 10 > > + > > +#define SYSCISR_RETRIES 1000 > > +#define SYSCISR_DELAY_US 10 > > + > > +#define R8A779A0_NUM_PD_ALWAYS_ON 64 /* Always-on power area */ > > Just use R8A779A0_PD_ALWAYS_ON instead? I'll fix it. > > + > > +#define NUM_DOMAINS_EACH_REG 32 > > BITS_PER_TYPE(u32)? I'll fix it. > > + > > +struct rcar_sysc_ch { > > + u16 chan_offs; > > + u8 chan_bit; > > + u8 isr_bit; > > +}; > > As chan_offs is unused, and chan_bit is always zero, I think all use of > this struct can just be replaced by "unsigned int pdr"? I'll fix it. > > + > > +static void __iomem *rcar_sysc_base; > > s/rcar/r8a779a0/ everywhere? I think so. I'll rename it. > > +static DEFINE_SPINLOCK(rcar_sysc_lock); /* SMP CPUs + I/O devices */ > > + > > +static int rcar_sysc_pwr_on_off(const struct rcar_sysc_ch *sysc_ch, bool on) > > +{ > > + unsigned int sr_bit, reg_offs; > > sr_bit is no longer needed. I'll drop it. > > + int k; > > + > > + if (on) { > > + sr_bit = SYSCSR_PONENB; > > + reg_offs = PDRONCR(sysc_ch->isr_bit); > > + } else { > > + sr_bit = SYSCSR_POFFENB; > > + reg_offs = PDROFFCR(sysc_ch->isr_bit); > > + } > > + > > + /* Wait until SYSC is ready to accept a power request */ > > + for (k = 0; k < SYSCSR_RETRIES; k++) { > > + if (ioread32(rcar_sysc_base + SYSCSR) & BIT(sr_bit)) > > if ((ioread32(rcar_sysc_base + SYSCSR) & SYSCSR_BUSY) == SYSCSR_BUSY) > > > + break; > > + udelay(SYSCSR_DELAY_US); > > + } > > Perhaps you can use readl_poll_timeout()? I think so. I'll fix it. > > + > > + if (k == SYSCSR_RETRIES) > > + return -EAGAIN; > > + > > + /* Submit power shutoff or power resume request */ > > + iowrite32(PW_ACTIVE, rcar_sysc_base + reg_offs); > > + > > + return 0; > > +} > > + > > +static int clear_irq_flags(unsigned int reg_idx, unsigned int isr_mask) > > +{ > > + int k = 0; > > + > > + iowrite32(isr_mask, rcar_sysc_base + SYSCISCR(reg_idx)); > > + > > + for (k = 0; k < SYSCISR_RETRIES; k++) { > > + if ((ioread32(rcar_sysc_base + SYSCISCR(reg_idx)) & isr_mask) == 0) > > + break; > > + > > + udelay(SYSCISR_DELAY_US); > > + } > > readl_poll_timeout()? Yes, I'll use it. > > + > > + if (k == SYSCISR_RETRIES) { > > + pr_err("\n %s : Can not clear IRQ flags in SYSCISCR", __func__); > > + return -EIO; > > + } > > + > > + return 0; > > +} > > > +static bool has_cpg_mstp; > > Please drop this and all related code, R-Car V3U does not use the legacy > CPG/MSSR PM Domain. I'll drop it. > > +static const struct of_device_id rcar_sysc_matches[] __initconst = { > > +#ifdef CONFIG_SYSC_R8A779A0 > > Please drop the #ifdef, as compilation of this file already depends on > this symbol. Oops. I got it. Best regards, Yoshihiro Shimoda
Hi Geert-san, > From: Geert Uytterhoeven, Sent: Tuesday, September 8, 2020 8:36 PM > > Hi Shimoda-san, > > On Mon, Sep 7, 2020 at 11:20 AM Yoshihiro Shimoda > <yoshihiro.shimoda.uh@renesas.com> wrote: > > Add support for R-Car V3U (R8A779A0) to the R-Car RST driver. > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > Thanks for your patch! > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Thank you for your review! > > --- a/drivers/soc/renesas/rcar-rst.c > > +++ b/drivers/soc/renesas/rcar-rst.c > > @@ -37,6 +37,10 @@ static const struct rst_config rcar_rst_gen3 __initconst = { > > .modemr = 0x60, > > }; > > > > +static const struct rst_config rcar_rst_r8a779a0 __initconst = { > > + .modemr = 0x00, /* MODEMR0 and it has CPG related bits */ > > Do you need the bits from MODEMR1, too? > Perhaps the time is ripe to add rcar_rst_read_mode_pins64(), > so users can access more than 32 bits on SoCs that provide it (R-Car > V3H and V3U)? I think so. However, main users of rcar_rst_read_mode_pins() are cpg drivers for now. So, perhaps no one uses more than 32 bits for now. > At least the numbering is sane on R-Car V3U. On R-Car V3H, MD29 and > higher are stored starting at bit 1 of the second MODEMR register... Oh, it's strange assignment... Best regards, Yoshihiro Shimoda
Hi Shimoda-san, On Thu, Sep 10, 2020 at 6:45 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > > From: Geert Uytterhoeven, Sent: Tuesday, September 8, 2020 8:36 PM > > On Mon, Sep 7, 2020 at 11:20 AM Yoshihiro Shimoda > > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > Add support for R-Car V3U (R8A779A0) to the R-Car RST driver. > > > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > > --- a/drivers/soc/renesas/rcar-rst.c > > > +++ b/drivers/soc/renesas/rcar-rst.c > > > @@ -37,6 +37,10 @@ static const struct rst_config rcar_rst_gen3 __initconst = { > > > .modemr = 0x60, > > > }; > > > > > > +static const struct rst_config rcar_rst_r8a779a0 __initconst = { > > > + .modemr = 0x00, /* MODEMR0 and it has CPG related bits */ > > > > Do you need the bits from MODEMR1, too? > > Perhaps the time is ripe to add rcar_rst_read_mode_pins64(), > > so users can access more than 32 bits on SoCs that provide it (R-Car > > V3H and V3U)? > > I think so. However, main users of rcar_rst_read_mode_pins() > are cpg drivers for now. So, perhaps no one uses more than 32 bits for now. We can always add rcar_rst_read_mode_pins64() when it becomes really needed. Hence i.e. will queue this as-is in renesas-devel for v5.10. Gr{oetje,eeting}s, Geert
Hi Geert-san, Thank you for your review! > From: Geert Uytterhoeven, Sent: Wednesday, September 9, 2020 2:16 AM > > Hi Shimoda-san, > > On Mon, Sep 7, 2020 at 11:20 AM Yoshihiro Shimoda > <yoshihiro.shimoda.uh@renesas.com> wrote: > > Add initial support for the Renesas R8A77990 (R-Car V3U) support. > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > Thanks for your patch! > > > --- /dev/null > > +++ b/arch/arm64/boot/dts/renesas/r8a779a0.dtsi > > > + soc: soc { > > > + > > + sysc: system-controller@e6180000 { > > + compatible = "renesas,r8a779a0-sysc"; > > + reg = <0 0xe6180000 0 0x3078>; > > Length 0x4000? Yes, I'll fix it. > > + #power-domain-cells = <1>; > > + }; > > + > > + scif0: serial@e6e60000 { > > + compatible = "renesas,scif-r8a779a0", > > + "renesas,rcar-gen3-scif", "renesas,scif"; > > + reg = <0 0xe6e60000 0 64>; > > + interrupts = <GIC_SPI 251 IRQ_TYPE_LEVEL_HIGH>; > > + clocks = <&cpg CPG_MOD 702>, > > + <&cpg CPG_CORE R8A779A0_CLK_S1D2>, > > + <&scif_clk>; > > + clock-names = "fck", "brg_int", "scif_clk"; > > + power-domains = <&sysc R8A779A0_PD_ALWAYS_ON>; > > Missing resets property. Oops. I'll add it. > > + status = "disabled"; > > + }; > > + > > + gic: interrupt-controller@f1000000 { > > + compatible = "arm,gic-v3"; > > + #interrupt-cells = <3>; > > + #address-cells = <0>; > > + interrupt-controller; > > + reg = <0x0 0xf1000000 0 0x20000>, > > + <0x0 0xf1060000 0 0x110000>; > > + interrupts = <GIC_PPI 9 > > + (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>; > > "GIC_CPU_MASK_SIMPLE(1)", as currently only one CPU core is used. I got it. I'll fix it. > > + power-domains = <&sysc R8A779A0_PD_ALWAYS_ON>; > > + }; > > + > > + prr: chipid@fff00044 { > > + compatible = "renesas,prr"; > > + reg = <0 0xfff00044 0 4>; > > + }; > > + }; > > + > > + timer { > > + compatible = "arm,armv8-timer"; > > + interrupts-extended = <&gic GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>, > > + <&gic GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>, > > + <&gic GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>, > > + <&gic GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>; > > "GIC_CPU_MASK_SIMPLE(1)" for all 4 interrupts (and in the future "8", > not "2"). Oops. You're correct. I'll fix it. Best regards, Yoshihiro Shimoda
Hi Geert-san, Thank you for your review! > From: Geert Uytterhoeven, Sent: Wednesday, September 9, 2020 2:21 AM > > Hi Shimoda-san, > > On Mon, Sep 7, 2020 at 11:20 AM Yoshihiro Shimoda > <yoshihiro.shimoda.uh@renesas.com> wrote: > > Initial support for the Renesas Falcon CPU and BreakOut boards > > support. > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > Thanks for your patch! > > > --- /dev/null > > +++ b/arch/arm64/boot/dts/renesas/falcon-cpu.dtsi > > @@ -0,0 +1,44 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Device Tree Source for the Falcon CPU board > > + * > > + * Copyright (C) 2020 Renesas Electronics Corp. > > + */ > > As this board contains the CPU, I had expected > > #include "r8a779a0.dtsi" > > here. I got it. I'll add it. > > + > > +/ { > > + model = "Renesas Falcon CPU board"; > > + compatible = "renesas,falcon-cpu"; > > + renesas,r8a779a0. Oops. I'll add it. Also, I'll fix a compatible in the r8a779a0-falcon.dts. Best regards, Yoshihiro Shimoda
Hi Shimoda-san, On Thu, Sep 10, 2020 at 1:04 PM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > > From: Geert Uytterhoeven, Sent: Wednesday, September 9, 2020 2:21 AM > > On Mon, Sep 7, 2020 at 11:20 AM Yoshihiro Shimoda > > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > Initial support for the Renesas Falcon CPU and BreakOut boards > > > support. > > > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > > > Thanks for your patch! > > > > > --- /dev/null > > > +++ b/arch/arm64/boot/dts/renesas/falcon-cpu.dtsi > > > @@ -0,0 +1,44 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * Device Tree Source for the Falcon CPU board > > > + * > > > + * Copyright (C) 2020 Renesas Electronics Corp. > > > + */ > > > > As this board contains the CPU, I had expected > > > > #include "r8a779a0.dtsi" > > > > here. > > I got it. I'll add it. Thanks! BTW, I forgot to mention that the file should probably be named r8a779a0-falcon-cpu.dtsi, i.e. incl. the SoC part number prefix. Gr{oetje,eeting}s, Geert
Hi Geert-san, > From: Geert Uytterhoeven, Sent: Thursday, September 10, 2020 8:14 PM > > Hi Shimoda-san, > > On Thu, Sep 10, 2020 at 1:04 PM Yoshihiro Shimoda > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > From: Geert Uytterhoeven, Sent: Wednesday, September 9, 2020 2:21 AM > > > On Mon, Sep 7, 2020 at 11:20 AM Yoshihiro Shimoda > > > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > > Initial support for the Renesas Falcon CPU and BreakOut boards > > > > support. > > > > > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > > > > > Thanks for your patch! > > > > > > > --- /dev/null > > > > +++ b/arch/arm64/boot/dts/renesas/falcon-cpu.dtsi > > > > @@ -0,0 +1,44 @@ > > > > +// SPDX-License-Identifier: GPL-2.0 > > > > +/* > > > > + * Device Tree Source for the Falcon CPU board > > > > + * > > > > + * Copyright (C) 2020 Renesas Electronics Corp. > > > > + */ > > > > > > As this board contains the CPU, I had expected > > > > > > #include "r8a779a0.dtsi" > > > > > > here. > > > > I got it. I'll add it. > > Thanks! > > BTW, I forgot to mention that the file should probably be named > r8a779a0-falcon-cpu.dtsi, i.e. incl. the SoC part number prefix. I got it. I'll rename the file on v2 patch. Best regards, Yoshihiro Shimoda