Message ID | 20210514192218.13022-1-prabhakar.mahadev-lad.rj@bp.renesas.com |
---|---|
Headers | show |
Series | Add new Renesas RZ/G2L SoC and Renesas RZ/G2L SMARC EVK support | expand |
On Fri, 14 May 2021 20:22:03 +0100, Lad Prabhakar wrote: > Add device tree bindings documentation for Renesas RZ/G2UL SoC. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> > Reviewed-by: Chris Paterson <Chris.Paterson2@renesas.com> > --- > Documentation/devicetree/bindings/arm/renesas.yaml | 6 ++++++ > 1 file changed, 6 insertions(+) > Acked-by: Rob Herring <robh@kernel.org>
On Fri, 14 May 2021 20:22:05 +0100, Lad Prabhakar wrote: > Document Renesas SMARC EVK boards which are based on RZ/G2L (R9A07G044L) > SoC. The SMARC EVK consists of RZ/G2L SoM module and SMARC carrier board, > the SoM module sits on top of carrier board. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> > Reviewed-by: Chris Paterson <Chris.Paterson2@renesas.com> > --- > Documentation/devicetree/bindings/arm/renesas.yaml | 3 +++ > 1 file changed, 3 insertions(+) > Acked-by: Rob Herring <robh@kernel.org>
On Fri, 14 May 2021 20:22:13 +0100, Lad Prabhakar wrote: > Document the device tree bindings of the Renesas RZ/G2L SoC clock > driver in Documentation/devicetree/bindings/clock/renesas,rzg2l-cpg.yaml. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> > --- > .../bindings/clock/renesas,rzg2l-cpg.yaml | 80 +++++++++++++++++++ > 1 file changed, 80 insertions(+) > create mode 100644 Documentation/devicetree/bindings/clock/renesas,rzg2l-cpg.yaml > Reviewed-by: Rob Herring <robh@kernel.org>
Hi Prabhakar, On Fri, May 14, 2021 at 9:23 PM Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > Add device tree bindings documentation for Renesas RZ/G2UL SoC. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> > Reviewed-by: Chris Paterson <Chris.Paterson2@renesas.com> Thanks for your patch! > --- a/Documentation/devicetree/bindings/arm/renesas.yaml > +++ b/Documentation/devicetree/bindings/arm/renesas.yaml > @@ -302,6 +302,12 @@ properties: > - renesas,rzn1d400-db # RZN1D-DB (RZ/N1D Demo Board for the RZ/N1D 400 pins package) > - const: renesas,r9a06g032 > > + - description: RZ/G2UL (R9A07G043) > + items: > + - enum: > + - renesas,r9a07g043u11 # Single Cortex-A55 RZ/G2UL Is there any specific reason you're including the final "1", unlike the RZ/G2{L,LC} binding? As RZ/G2UL is always single-core, perhaps this compatible value can be dropped? > + - const: renesas,r9a07g043 > + > additionalProperties: true For now, there are no users of this binding? I assume you're posting it already, as RZ/G2UL is pin-compatible with RZ/G2LC, and thus can be used interchangeably on the G2L SOM? However, the DTS board part in this series is for RZ/G2L, not RZ/GLC? Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> i.e. will queue in renesas-devel for v5.14, after the above have been resolved. 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 Prabhakar, On Fri, May 14, 2021 at 9:23 PM Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > Document Renesas SMARC EVK boards which are based on RZ/G2L (R9A07G044L) > SoC. The SMARC EVK consists of RZ/G2L SoM module and SMARC carrier board, > the SoM module sits on top of carrier board. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> > Reviewed-by: Chris Paterson <Chris.Paterson2@renesas.com> > --- a/Documentation/devicetree/bindings/arm/renesas.yaml > +++ b/Documentation/devicetree/bindings/arm/renesas.yaml > @@ -310,6 +310,9 @@ properties: > > - description: RZ/G2{L,LC} (R9A07G044) > items: > + - enum: > + - renesas,smarc-r9a07g044l1 # SMARC EVK with single Cortex-A55 > + - renesas,smarc-r9a07g044l2 # SMARC EVK with dual Cortex-A55 > - enum: > - renesas,r9a07g044c1 # Single Cortex-A55 RZ/G2LC > - renesas,r9a07g044c2 # Dual Cortex-A55 RZ/G2LC Likewise, do we care (at the top level) if this is a board with an SoC die that has one or two Cortex-A55 cores enabled? 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 Prabhakar, On Fri, May 14, 2021 at 9:23 PM Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > Add ARCH_R9A07G044{L,LC} as a configuration symbol for the new Renesas > RZ/G2{L,LC} SoC's. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> Thanks for your patch! > --- a/drivers/soc/renesas/Kconfig > +++ b/drivers/soc/renesas/Kconfig > @@ -279,6 +279,16 @@ config ARCH_R8A774B1 > help > This enables support for the Renesas RZ/G2N SoC. > > +config ARCH_R9A07G044L > + bool "ARM64 Platform support for RZ/G2L SoC" Please drop the "SoC", for consistency with other entries. > + help > + This enables support for the Renesas RZ/G2L SoC. > + > +config ARCH_R9A07G044LC > + bool "ARM64 Platform support for RZ/G2LC SoC" Likewise. > + help > + This enables support for the Renesas RZ/G2LC SoC. > + > endif # ARM64 Given LSI DEVID is the same, do we need both, or can we do with a single ARCH_R9A07G044? 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 Prabhakar, On Fri, May 14, 2021 at 9:23 PM Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > From: Biju Das <biju.das.jz@bp.renesas.com> > > Add serial support for RZ/G2L SoC with earlycon and > extended mode register support. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Thanks for your patch! > --- a/drivers/tty/serial/sh-sci.c > +++ b/drivers/tty/serial/sh-sci.c > @@ -306,6 +306,7 @@ static const struct sci_port_params sci_port_params[SCIx_NR_REGTYPES] = { > [SCFDR] = { 0x0E, 16 }, > [SCSPTR] = { 0x10, 16 }, > [SCLSR] = { 0x12, 16 }, > + [SEMR] = { 0x14, 8 }, This is the parameter section for RZ/T and RZ/A2. Please update the comments above, to say this also applies to RZ/G2L. I can confirm the documentation for RZ/T1 and RZ/A2 agrees about the existence and behavior of SEMR. > }, > .fifosize = 16, > .overrun_reg = SCLSR, > @@ -2527,6 +2528,8 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios, > case 27: smr_val |= SCSMR_SRC_27; break; > } > smr_val |= cks; > + if (sci_getreg(port, SEMR)->size) > + serial_port_out(port, SEMR, 0); As this is done in both branches of the if() statement, I think it should be moved up. > serial_port_out(port, SCSCR, scr_val | s->hscif_tot); > serial_port_out(port, SCSMR, smr_val); > serial_port_out(port, SCBRR, brr); > @@ -2561,6 +2564,8 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios, > scr_val = s->cfg->scscr & (SCSCR_CKE1 | SCSCR_CKE0); > smr_val |= serial_port_in(port, SCSMR) & > (SCSMR_CKEDG | SCSMR_SRC_MASK | SCSMR_CKS); > + if (sci_getreg(port, SEMR)->size) > + serial_port_out(port, SEMR, 0); (else branch) > serial_port_out(port, SCSCR, scr_val | s->hscif_tot); > serial_port_out(port, SCSMR, smr_val); > } > @@ -3170,6 +3175,10 @@ static const struct of_device_id of_sci_match[] = { > .compatible = "renesas,scif-r7s9210", > .data = SCI_OF_DATA(PORT_SCIF, SCIx_RZ_SCIFA_REGTYPE), > }, > + { > + .compatible = "renesas,scif-r9a07g044", > + .data = SCI_OF_DATA(PORT_SCIF, SCIx_RZ_SCIFA_REGTYPE), > + }, > /* Family-specific types */ > { > .compatible = "renesas,rcar-gen1-scif", 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 Prabhakar, On Fri, May 14, 2021 at 9:24 PM Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > Add CPG core wrapper for RZ/G2L family. > > Based on a patch in the BSP by Binh Nguyen > <binh.nguyen.jz@renesas.com>. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> Thanks for your patch! > --- a/drivers/clk/renesas/Kconfig > +++ b/drivers/clk/renesas/Kconfig > @@ -34,6 +34,10 @@ config CLK_RENESAS > select CLK_R9A06G032 if ARCH_R9A06G032 > select CLK_SH73A0 if ARCH_SH73A0 > > +config CLK_RENESAS_RZG2L > + bool "Renesas RZ/G2L SoC clock support" if COMPILE_TEST && !ARCH_RENESAS > + default y if ARCH_RENESAS Why do you need this symbol here, and why does it default to y? I guess the plan is to share this by the RZ/G2L(C) and RZ/G2UL clock drivers, so probably you want to move it to the family-specific section below, like CLK_RCAR_GEN3_CPG. > + > if CLK_RENESAS > > # SoC > diff --git a/drivers/clk/renesas/Makefile b/drivers/clk/renesas/Makefile > index ef0d2bba92bf..e4f3d7fab67c 100644 > --- a/drivers/clk/renesas/Makefile > +++ b/drivers/clk/renesas/Makefile > @@ -36,6 +36,7 @@ obj-$(CONFIG_CLK_RCAR_CPG_LIB) += rcar-cpg-lib.o > obj-$(CONFIG_CLK_RCAR_GEN2_CPG) += rcar-gen2-cpg.o > obj-$(CONFIG_CLK_RCAR_GEN3_CPG) += rcar-gen3-cpg.o > obj-$(CONFIG_CLK_RCAR_USB2_CLOCK_SEL) += rcar-usb2-clock-sel.o > +obj-$(CONFIG_CLK_RENESAS_RZG2L) += renesas-rzg2l-cpg.o As CLK_RENESAS_RZG2L defaults to y, this is compiled by default on ARCH_RENESAS. > > # Generic > obj-$(CONFIG_CLK_RENESAS_CPG_MSSR) += renesas-cpg-mssr.o > diff --git a/drivers/clk/renesas/renesas-rzg2l-cpg.c b/drivers/clk/renesas/renesas-rzg2l-cpg.c > new file mode 100644 > index 000000000000..dc54ffc6114c > --- /dev/null > +++ b/drivers/clk/renesas/renesas-rzg2l-cpg.c > @@ -0,0 +1,958 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * RZ/G2L Clock Pulse Generator > + * > + * Copyright (C) 2021 Renesas Electronics Corp. > + * > + * Based on renesas-cpg-mssr.c > + * > + * Copyright (C) 2015 Glider bvba > + * Copyright (C) 2013 Ideas On Board SPRL > + * Copyright (C) 2015 Renesas Electronics Corp. > + */ > + > +#include <linux/clk.h> > +#include <linux/clk-provider.h> > +#include <linux/clk/renesas.h> > +#include <linux/delay.h> > +#include <linux/device.h> > +#include <linux/init.h> > +#include <linux/mod_devicetable.h> > +#include <linux/module.h> > +#include <linux/of_address.h> > +#include <linux/of_device.h> > +#include <linux/platform_device.h> > +#include <linux/pm_clock.h> > +#include <linux/pm_domain.h> > +#include <linux/psci.h> Unused > +#include <linux/reset-controller.h> > +#include <linux/slab.h> > + > +#include <dt-bindings/clock/renesas-cpg-mssr.h> > + > +#include "renesas-rzg2l-cpg.h" > + > +#ifdef DEBUG > +#define WARN_DEBUG(x) WARN_ON(x) > +#else > +#define WARN_DEBUG(x) do { } while (0) > +#endif > + > +#define DIV_RSMASK(v, s, m) ((v >> s) & m) > +#define GET_REG(val) ((val >> 20) & 0xfff) > +#define GET_SHIFT(val) ((val >> 12) & 0xff) > +#define GET_WIDTH(val) ((val >> 8) & 0xf) > + > +#define KDIV(val) DIV_RSMASK(val, 16, 0xffff) > +#define MDIV(val) DIV_RSMASK(val, 6, 0x3ff) > +#define PDIV(val) DIV_RSMASK(val, 0, 0x3f) > +#define SDIV(val) DIV_RSMASK(val, 0, 0x7) > +#define REFDIV(val) DIV_RSMASK(val, 8, 0x3f) > +#define POSTDIV1(val) DIV_RSMASK(val, 0, 0x7) > +#define POSTDIV2(val) DIV_RSMASK(val, 4, 0x7) > +#define FRACIN(val) DIV_RSMASK(val, 8, 0xffffff) > +#define INITIN(val) DIV_RSMASK(val, 16, 0xfff) > + > +/** > + * Clock Pulse Generator Private Data struct cpg_mssr_priv - Clock Pulse Generator Private Data > + * > + * @rcdev: Optional reset controller entity > + * @dev: CPG/MSSR device CPG? > + * @base: CPG/MSSR register block base address CPG? > + * @rmw_lock: protects RMW register accesses > + * @clks: Array containing all Core and Module Clocks > + * @num_core_clks: Number of Core Clocks in clks[] > + * @num_mod_clks: Number of Module Clocks in clks[] > + * @last_dt_core_clk: ID of the last Core Clock exported to DT > + * @notifiers: Notifier chain to save/restore clock state for system resume > + * @info: Pointer to platform data > + */ > +struct cpg_mssr_priv { rzg2l_cpg_priv? > +#ifdef CONFIG_RESET_CONTROLLER > + struct reset_controller_dev rcdev; > +#endif > + struct device *dev; > + void __iomem *base; > + spinlock_t rmw_lock; > + > + struct clk **clks; > + unsigned int num_core_clks; > + unsigned int num_mod_clks; > + unsigned int last_dt_core_clk; > + > + struct raw_notifier_head notifiers; > + const struct cpg_mssr_info *info; > +}; > +struct div2_clk { > + struct clk_hw hw; > + unsigned int conf; > + const struct clk_div_table *dtable; > + unsigned int confs; > + const struct clk_div_table *dtables; Please don't interleave pointers and integers on a 64-bit platform, as it requires padding to keep the pointers naturally aligned. Same for struct pll_clk below. > + void __iomem *base; > + struct cpg_mssr_priv *priv; > +}; > + > +#define to_d2clk(_hw) container_of(_hw, struct div2_clk, hw) > + > +static unsigned int div2_clock_get_div(unsigned int val, > + const struct clk_div_table *t, > + int length) unsigned int > +{ > + int i; unsigned int (for all positive loop counters) > + > + for (i = 0; i <= length; i++) > + if (val == t[i].val) > + return t[i].div; > + > + /* return div=1 if failed */ > + return 1; > +} > +static long rzg2l_cpg_div2_clk_round_rate(struct clk_hw *hw, > + unsigned long rate, > + unsigned long *parent_rate) Please implement .determine_rate() instead of .round_rate() in new drivers. > +{ > + unsigned long best_diff = (unsigned long)-1; > + unsigned int best_div, best_divs, div, divs; > + struct div2_clk *d2clk = to_d2clk(hw); > + unsigned long diff; > + int i, j, n, ns; > + > + n = BIT(GET_WIDTH(d2clk->conf)) - 1; > + ns = BIT(GET_WIDTH(d2clk->confs)) - 1; > + for (i = 0; i <= n; i++) { > + for (j = 0; j <= ns; j++) { > + div = div2_clock_get_div(i, d2clk->dtable, n); > + divs = div2_clock_get_div(j, d2clk->dtables, ns); > + diff = abs(*parent_rate - (rate * div * divs)); > + if (best_diff > diff) { > + best_diff = diff; > + best_div = div; > + best_divs = divs; > + } > + } > + } > + > + return DIV_ROUND_CLOSEST_ULL((u64)*parent_rate, best_div * best_divs); > +} > +/** > + * struct mstp_clock - MSTP gating clock > + * @hw: handle between common and hardware-specific interfaces > + * @bit: 16bits register offset, 8bits ON/MON, 8bits RESET > + * @priv: CPG/MSSR private data > + */ > +struct mstp_clock { > + struct clk_hw hw; > + u32 bit; I think the code accessing this field can be simplified by not packing everything into a 32-bit value: u16 off; u8 onoff; u8 reset; > + struct cpg_mssr_priv *priv; > +}; > diff --git a/drivers/clk/renesas/renesas-rzg2l-cpg.h b/drivers/clk/renesas/renesas-rzg2l-cpg.h > new file mode 100644 > index 000000000000..8dce6b5546b1 > --- /dev/null > +++ b/drivers/clk/renesas/renesas-rzg2l-cpg.h > @@ -0,0 +1,221 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * RZ/G2L Clock Pulse Generator > + * > + * Copyright (C) 2021 Renesas Electronics Corp. > + * > + */ > + > +#ifndef __RENESAS_RZG2L_CPG_H__ > +#define __RENESAS_RZG2L_CPG_H__ > + > +/* Register offset */ > +/* n : 0, 1, 2 : PLL1, PLL4, PLL6 */ > +#define PLL146_CLK1_R(n) (0x04 + (16 * n)) > +#define PLL146_CLK2_R(n) (0x08 + (16 * n)) > +#define PLL146_MON_R(n) (0x0C + (16 * n)) > +#define PLL146_STBY_R(n) (0x00 + (16 * n)) > + > +/* n : 0, 1, 2 : PLL2, PLL3, PLL5 */ > +#define PLL235_CLK1_R(n) (0x104 + (32 * n)) > +#define PLL235_CLK3_R(n) (0x10c + (32 * n)) > +#define PLL235_CLK4_R(n) (0x110 + (32 * n)) > +#define PLL235_MON_R(n) (0x11C + (32 * n)) > +#define PLL235_STBY_R(n) (0x100 + (32 * n)) > + > +#define PLL1_DIV_R (0x200) > +#define PLL2_DIV_R (0x204) > +#define PLL3A_DIV_R (0x208) > +#define PLL3B_DIV_R (0x20c) > +#define PLL6_DIV_R (0x210) > +#define PL2SDHI_SEL_R (0x218) > +#define CLK_STATUS_R (0x280) > +#define CA55_SSEL_R (0x400) > +#define PL2_SSEL_R (0x404) > +#define PL3_SSEL_R (0x408) > +#define PL4_DSEL_R (0x21C) > +#define PL5_SSEL_R (0x410) > +#define PL6_SSEL_R (0x414) > +#define PL6_ETH_SSEL_R (0x418) > +#define PL5_SDIV_R (0x420) > +#define OTHERFUNC1_R (0xBE8) > +#define CLK_ON_R(reg) (0x500 + reg) > +#define CLK_MON_R(reg) (0x680 + reg) > +#define CLK_RST_R(reg) (0x800 + reg) > +#define CLK_MRST_R(reg) (0x980 + reg) The register offsets are only used by renesas-rzg2l-cpg.c, so they can be moved there. > + > +#define SEL_PLL1 (CA55_SSEL_R << 20 | 0 << 12 | 1 << 8) > +#define SEL_PLL2_1 (PL2_SSEL_R << 20 | 0 << 12 | 1 << 8) > +#define SEL_PLL2_2 (PL2_SSEL_R << 20 | 4 << 12 | 1 << 8) > +#define SEL_PLL3_1 (PL3_SSEL_R << 20 | 0 << 12 | 1 << 8) > +#define SEL_PLL3_2 (PL3_SSEL_R << 20 | 4 << 12 | 1 << 8) > +#define SEL_PLL3_3 (PL3_SSEL_R << 20 | 8 << 12 | 1 << 8) > +#define SEL_PLL4 (PL4_DSEL_R << 20 | 8 << 12 | 1 << 8) > +#define SEL_PLL5_1 (PL5_SSEL_R << 20 | 0 << 12 | 1 << 8) > +#define SEL_PLL5_2 (PL5_SSEL_R << 20 | 4 << 12 | 1 << 8) > +#define SEL_PLL5_3 (PL5_SSEL_R << 20 | 0 << 12 | 1 << 8) > +#define SEL_PLL5_4 (OTHERFUNC1_R << 20 | 0 << 12 | 1 << 8) > +#define SEL_PLL6_1 (PL6_SSEL_R << 20 | 0 << 12 | 1 << 8) > +#define SEL_PLL6_2 (PL6_ETH_SSEL_R << 20 | 0 << 12 | 1 << 8) > +#define SEL_ETH (PL6_ETH_SSEL_R << 20 | 4 << 12 | 1 << 8) > +#define SEL_G1_1 (PL6_SSEL_R << 20 | 4 << 12 | 1 << 8) > +#define SEL_G1_2 (PL6_SSEL_R << 20 | 8 << 12 | 1 << 8) > +#define SEL_G2 (PL6_SSEL_R << 20 | 12 << 12 | 1 << 8) > +#define SEL_SDHI0 (PL2SDHI_SEL_R << 20 | 0 << 12 | 2 << 8) > +#define SEL_SDHI1 (PL2SDHI_SEL_R << 20 | 4 << 12 | 2 << 8) > +#define DIVPL1 (PLL1_DIV_R << 20 | 0 << 12 | 2 << 8) > +#define DIVPL2A (PLL2_DIV_R << 20 | 0 << 12 | 3 << 8) > +#define DIVPL2B (PLL2_DIV_R << 20 | 4 << 12 | 3 << 8) > +#define DIVPL2C (PLL2_DIV_R << 20 | 8 << 12 | 3 << 8) > +#define DIVDSILPCL (PLL2_DIV_R << 20 | 12 << 12 | 2 << 8) > +#define DIVPL3A (PLL3A_DIV_R << 20 | 0 << 12 | 3 << 8) > +#define DIVPL3B (PLL3A_DIV_R << 20 | 4 << 12 | 3 << 8) > +#define DIVPL3C (PLL3A_DIV_R << 20 | 8 << 12 | 3 << 8) > +#define DIVPL3CLK200FIX (PLL3B_DIV_R << 20 | 0 << 12 | 3 << 8) > +#define DIVGPU (PLL6_DIV_R << 20 | 0 << 12 | 2 << 8) > +#define DIVDSIB (PL5_SDIV_R << 20 | 8 << 12 | 4 << 8) > +#define DIVDSIA (PL5_SDIV_R << 20 | 0 << 12 | 2 << 8) > + > +#define PLL146_STBY(n) (PLL146_STBY_R(n) << 20 | 2 << 16 | 0 << 12) > +#define PLL146_MON(n) (PLL146_MON_R(n) << 20 | 4 << 16 | 0 << 12) > +#define PLL235_STBY(n) (PLL235_STBY_R(n) << 20 | 2 << 16 | 0 << 12) > +#define PLL235_MON(n) (PLL235_MON_R(n) << 20 | 4 << 16 | 0 << 12) > + > +#define PLL146_CONF(n) (PLL146_CLK1_R(n) << 22 | PLL146_CLK2_R(n) << 12 | 0) > +#define PLL235_CONF(n) (PLL235_CLK1_R(n) << 22 | PLL235_CLK3_R(n) << 12 | PLL235_CLK4_R(n)) > + > +#define GET_REG1(val) ((val >> 22) & 0x3ff) > +#define GET_REG2(val) ((val >> 12) & 0x3ff) > +#define GET_REG3(val) (val & 0x3ff) The GET_REG() macros are only used by renesas-rzg2l-cpg.c. > + > +#define MSSR(off, on, res) ((off & 0xffff) << 16 | \ > + (on & 0xff) << 8 | (res & 0xff)) > +#define MSSR_OFF(val) ((val >> 16) & 0xffff) > +#define MSSR_ON(val) ((val >> 8) & 0xff) > +#define MSSR_RES(val) (val & 0xff) Can be removed after mstp_clock.bit split. > +/* > + * Definitions of Module Clocks > + * @name: handle between common and hardware-specific interfaces > + * @id: clock index in array containing all Core and Module Clocks > + * @parent: id of parent clock > + * @bit: 16bits register offset, 8bits ON/MON, 8bits RESET > + */ > +struct mssr_mod_clk { rzg2l_mod_clk? > + const char *name; > + unsigned int id; > + unsigned int parent; > + unsigned int bit; Same comment as for mstp_clock.bit. > +}; > + > +#define DEF_MOD(_name, _id, _parent, _bit) \ > + { .name = _name, .id = MOD_CLK_BASE + _id, .parent = _parent,\ > + .bit = _bit } > + > +/** > + * SoC-specific CPG/MSSR Description > + * > + * @core_clks: Array of Core Clock definitions > + * @num_core_clks: Number of entries in core_clks[] > + * @last_dt_core_clk: ID of the last Core Clock exported to DT > + * @num_total_core_clks: Total number of Core Clocks (exported + internal) > + * > + * @pll_info: array of PLL register info > + * @pll_info_size: Total number of PLL registers info > + * > + * @mod_clks: Array of Module Clock definitions > + * @num_mod_clks: Number of entries in mod_clks[] > + * @num_hw_mod_clks: Number of Module Clocks supported by the hardware > + * > + * @crit_mod_clks: Array with Module Clock IDs of critical clocks that > + * should not be disabled without a knowledgeable driver > + * @num_crit_mod_clks: Number of entries in crit_mod_clks[] > + */ > +struct cpg_mssr_info { rzg2l_cpg_info? > + /* Core Clocks */ > + const struct cpg_core_clk *core_clks; > + unsigned int num_core_clks; > + unsigned int last_dt_core_clk; > + unsigned int num_total_core_clks; > + > + /* Module Clocks */ > + const struct mssr_mod_clk *mod_clks; > + unsigned int num_mod_clks; > + unsigned int num_hw_mod_clks; > + > + /* Critical Module Clocks that should not be disabled */ > + const unsigned int *crit_mod_clks; > + unsigned int num_crit_mod_clks; > +}; > + > +#endif 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 Prabhakar, On Fri, May 14, 2021 at 9:23 PM Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > Document the device tree bindings of the Renesas RZ/G2L SoC clock > driver in Documentation/devicetree/bindings/clock/renesas,rzg2l-cpg.yaml. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> Thanks for your patch! > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/renesas,rzg2l-cpg.yaml > @@ -0,0 +1,80 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: "http://devicetree.org/schemas/clock/renesas,rzg2l-cpg.yaml#" > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > + > +title: Renesas RZ/G2L Clock Pulse Generator / Module Stop and Software Reset (Module Standby Mode > + > +maintainers: > + - Geert Uytterhoeven <geert+renesas@glider.be> > + > +description: | > + On Renesas RZ/G2L SoC, the CPG (Clock Pulse Generator) and MSTP > + (Module Stop and Software Reset) share the same register block. > + > + They provide the following functionalities: > + - The CPG block generates various core clocks, > + - The MSTP block provides two functions: > + 1. Module Stop, providing a Clock Domain to control the clock supply > + to individual SoC devices, > + 2. Reset Control, to perform a software reset of individual SoC devices. > + > +properties: > + compatible: > + const: renesas,r9a07g044l-cpg # RZ/G2L renesas,r9a07g044-cpg? I believe it's the same block on RZ/G2L ('044l) and RZ/G2LC ('044c). > + '#clock-cells': > + description: | > + - For CPG core clocks, the two clock specifier cells must be "CPG_CORE" > + and a core clock reference, as defined in > + <dt-bindings/clock/*-cpg-mssr.h> <dt-bindings/clock/r9a07g044l-cpg.h> > + - For module clocks, the two clock specifier cells must be "CPG_MOD" and > + a module number, as defined in the datasheet. Also in <dt-bindings/clock/r9a07g044l-cpg.h>? > + const: 2 > + > + '#power-domain-cells': > + description: > + SoC devices that are part of the CPG/MSTP Clock Domain and can be > + power-managed through Module Stop should refer to the CPG device node > + in their "power-domains" property, as documented by the generic PM Domain > + bindings in Documentation/devicetree/bindings/power/power-domain.yaml. > + const: 0 > + > + '#reset-cells': > + description: > + The single reset specifier cell must be the module number, as defined in > + the datasheet. Also in <dt-bindings/clock/r9a07g044l-cpg.h>? > + const: 1 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 Prabhakar, On Fri, May 14, 2021 at 9:24 PM Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > Add initial DTSI for RZ/G2{L,LC} SoC's. > > File structure: > r9a07g044.dtsi => RZ/G2L family SoC common parts > r9a07g044l.dtsi => Specific to RZ/G2L (R9A07G044L) SoC > r9a07g044l1.dtsi => Specific to RZ/G2L (R9A07G044L single cortex A55) SoC > r9a07g044l2.dtsi => Specific to RZ/G2L (R9A07G044L dual cortex A55) SoC > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> Thanks for your patch! > index 000000000000..c625d302f889 > --- /dev/null > +++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi > @@ -0,0 +1,70 @@ > +// SPDX-License-Identifier: GPL-2.0 Do we want to use SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) for new DTS files? This actually also applies to <dt-bindings/...> files. > +/* > + * Device Tree Source for the RZ/G2L and RZ/G2LC common SoC parts > + * > + * Copyright (C) 2021 Renesas Electronics Corp. > + */ > + > +#include <dt-bindings/interrupt-controller/arm-gic.h> > +#include <dt-bindings/clock/r9a07g044l-cpg.h> > + > +/ { > + compatible = "renesas,r9a07g044"; > + #address-cells = <2>; > + #size-cells = <2>; > + > + extal_clk: extal { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + /* This value must be overridden by the board */ > + clock-frequency = <0>; > + }; > + > + psci { > + compatible = "arm,psci-1.0", "arm,psci-0.2"; > + method = "smc"; > + }; > + > + soc: soc { > + compatible = "simple-bus"; > + interrupt-parent = <&gic>; > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + > + scif0: serial@1004b800 { > + compatible = "renesas,scif-r9a07g044"; > + reg = <0 0x1004b800 0 0x400>; > + interrupts = > + <GIC_SPI 380 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 382 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 383 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 381 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 384 IRQ_TYPE_LEVEL_HIGH>; "make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/serial/renesas,scif.yaml": arch/arm64/boot/dts/renesas/r9a07g044l2-smarc.dt.yaml: serial@1004b800: interrupts: 'oneOf' conditional failed, one must be fixed: [[0, 380, 4], [0, 382, 4], [0, 383, 4], [0, 381, 4], [0, 384, 4]] is too long Additional items are not allowed ([0, 382, 4], [0, 383, 4], [0, 381, 4], [0, 384, 4] were unexpected) Additional items are not allowed ([0, 384, 4] was unexpected) [[0, 380, 4], [0, 382, 4], [0, 383, 4], [0, 381, 4], [0, 384, 4]] is too short One interrupt is missing. According to the documentation, "tei" and "dri" share an interrupt, so they should map to the same interrupt number. Please add interrupt-names. > + clocks = <&cpg CPG_MOD R9A07G044_CLK_SCIF0>; > + clock-names = "fck"; > + power-domains = <&cpg>; > + resets = <&cpg R9A07G044_CLK_SCIF0>; > + status = "disabled"; > + }; > + > + devid: chipid@11020a04 { > + compatible = "renesas,devid"; > + reg = <0 0x11020a04 0 4>; > + }; > + > + gic: interrupt-controller@11900000 { > + compatible = "arm,gic-v3"; > + #interrupt-cells = <3>; > + #address-cells = <0>; > + interrupt-controller; > + reg = <0x0 0x11900000 0 0x40000>, > + <0x0 0x11940000 0 0x60000>; > + interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_LOW>; > + clocks = <&cpg CPG_MOD R9A07G044_CLK_GIC600>; > + clock-names = "gic6000"; This looks like a weird name ;-) In addition, it should be the consumer clock name, not the provider clock name. > + power-domains = <&cpg>; > + resets = <&cpg R9A07G044_CLK_GIC600>; "make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml": arch/arm64/boot/dts/renesas/r9a07g044l2-smarc.dt.yaml: interrupt-controller@11900000: 'clock-names', 'clocks', 'power-domains', 'resets' do not match any of the regexes: '^(msi-controller|gic-its|interrupt-controller)@[0-9a-f]+$', '^gic-its@', '^interrupt-controller@[0-9a-f]+$', 'pinctrl-[0-9]+' These properties should be added to the GIC-v3 bindings, cfr. the normal GIC bindings. > + }; > + }; > +}; > diff --git a/arch/arm64/boot/dts/renesas/r9a07g044l.dtsi b/arch/arm64/boot/dts/renesas/r9a07g044l.dtsi > new file mode 100644 > index 000000000000..8d396b9100c1 > --- /dev/null > +++ b/arch/arm64/boot/dts/renesas/r9a07g044l.dtsi > @@ -0,0 +1,21 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Device Tree Source for the RZ/G2L common SoC parts > + * > + * Copyright (C) 2021 Renesas Electronics Corp. > + */ > + > +/dts-v1/; > +#include "r9a07g044.dtsi" > + > +&soc { > + cpg: clock-controller@11010000 { > + compatible = "renesas,r9a07g044l-cpg"; > + reg = <0 0x11010000 0 0x10000>; > + clocks = <&extal_clk>; > + clock-names = "extal"; > + #clock-cells = <2>; > + #reset-cells = <1>; > + #power-domain-cells = <0>; > + }; As I think this is shared by RZ/G2L and RZ/G2LC, it belongs to r9a07g044.dtsi. > +}; > diff --git a/arch/arm64/boot/dts/renesas/r9a07g044l1.dtsi b/arch/arm64/boot/dts/renesas/r9a07g044l1.dtsi > new file mode 100644 > index 000000000000..44d4504e44c3 > --- /dev/null > +++ b/arch/arm64/boot/dts/renesas/r9a07g044l1.dtsi > @@ -0,0 +1,43 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Device Tree Source for the RZ/G2L R9A07G044L1 common parts > + * > + * Copyright (C) 2021 Renesas Electronics Corp. > + */ > + > +/dts-v1/; > +#include "r9a07g044l.dtsi" > + > +/ { > + compatible = "renesas,r9a07g044l1"; > + #address-cells = <2>; > + #size-cells = <2>; #{address,size}-cells already defined in r9a07g044.dtsi. > + > + cpus { > + #address-cells = <1>; > + #size-cells = <0>; > + > + a55_0: cpu@0 { > + compatible = "arm,cortex-a55"; > + reg = <0>; > + device_type = "cpu"; > + next-level-cache = <&L3_CA55>; > + enable-method = "psci"; > + }; > + > + L3_CA55: cache-controller-0 { > + compatible = "cache"; > + cache-unified; > + cache-size = <0x40000>; > + }; I think the first CPU core should be in the base r9a07g044.dtsi file. > + }; > + > + timer { > + compatible = "arm,armv8-timer"; > + interrupts-extended = > + <&gic GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(1) | IRQ_TYPE_LEVEL_LOW)>, > + <&gic GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(1) | IRQ_TYPE_LEVEL_LOW)>, > + <&gic GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(1) | IRQ_TYPE_LEVEL_LOW)>, > + <&gic GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(1) | IRQ_TYPE_LEVEL_LOW)>; Also in the base file, with interrupts-extended overridden where needed? > + }; > +}; > diff --git a/arch/arm64/boot/dts/renesas/r9a07g044l2.dtsi b/arch/arm64/boot/dts/renesas/r9a07g044l2.dtsi > new file mode 100644 > index 000000000000..33bb35e1c369 > --- /dev/null > +++ b/arch/arm64/boot/dts/renesas/r9a07g044l2.dtsi > @@ -0,0 +1,62 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Device Tree Source for the RZ/G2L R9A07G044L2 common parts > + * > + * Copyright (C) 2021 Renesas Electronics Corp. > + */ > + > +/dts-v1/; > +#include "r9a07g044l.dtsi" > + > +/ { > + compatible = "renesas,r9a07g044l2"; > + #address-cells = <2>; > + #size-cells = <2>; > + > + cpus { > + #address-cells = <1>; > + #size-cells = <0>; > + > + cpu-map { > + cluster0 { > + core0 { > + cpu = <&a55_0>; > + }; > + core1 { > + cpu = <&a55_1>; > + }; > + }; > + }; > + > + a55_0: cpu@0 { > + compatible = "arm,cortex-a55"; > + reg = <0>; > + device_type = "cpu"; > + next-level-cache = <&L3_CA55>; > + enable-method = "psci"; > + }; > + > + a55_1: cpu@1 { > + compatible = "arm,cortex-a55"; > + reg = <0x100>; > + device_type = "cpu"; > + next-level-cache = <&L3_CA55>; > + enable-method = "psci"; > + }; > + > + L3_CA55: cache-controller-0 { > + compatible = "cache"; > + cache-unified; > + cache-size = <0x40000>; > + }; I think (at least) the first CPU core should be in the base r9a07g044.dtsi file. Probably the second CPU core should be in the base file, too, and removed by /delete-node/ in r9a07g044l1.dtsi? > + }; > + > + 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)>; > + }; Also in the base file, with interrupts-extended overridden where needed? > +}; > -- > 2.17.1 > -- 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 Prabhakar, On Fri, May 14, 2021 at 9:24 PM Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > Add basic support for RZ/G2L SMARC EVK (based on R9A07G044L2): > - memory > - External input clock > - SCIF > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> Thanks for your patch! > --- a/arch/arm64/boot/dts/renesas/Makefile > +++ b/arch/arm64/boot/dts/renesas/Makefile > @@ -62,3 +62,5 @@ dtb-$(CONFIG_ARCH_R8A77990) += r8a77990-ebisu.dtb > dtb-$(CONFIG_ARCH_R8A77995) += r8a77995-draak.dtb > > dtb-$(CONFIG_ARCH_R8A779A0) += r8a779a0-falcon.dtb > + > +dtb-$(CONFIG_ARCH_R9A07G044L) += r9a07g044l2-smarc.dtb > diff --git a/arch/arm64/boot/dts/renesas/g2l-smarc.dtsi b/arch/arm64/boot/dts/renesas/g2l-smarc.dtsi > new file mode 100644 > index 000000000000..9b95d73fb798 > --- /dev/null > +++ b/arch/arm64/boot/dts/renesas/g2l-smarc.dtsi rzg2l-smarc? The rest looks good to me (taking into account compatible value discussions). 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, Thank you for the review. On Fri, May 21, 2021 at 2:23 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Prabhakar, > > On Fri, May 14, 2021 at 9:23 PM Lad Prabhakar > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > Add device tree bindings documentation for Renesas RZ/G2UL SoC. > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> > > Reviewed-by: Chris Paterson <Chris.Paterson2@renesas.com> > > Thanks for your patch! > > > --- a/Documentation/devicetree/bindings/arm/renesas.yaml > > +++ b/Documentation/devicetree/bindings/arm/renesas.yaml > > @@ -302,6 +302,12 @@ properties: > > - renesas,rzn1d400-db # RZN1D-DB (RZ/N1D Demo Board for the RZ/N1D 400 pins package) > > - const: renesas,r9a06g032 > > > > + - description: RZ/G2UL (R9A07G043) > > + items: > > + - enum: > > + - renesas,r9a07g043u11 # Single Cortex-A55 RZ/G2UL > > Is there any specific reason you're including the final "1", unlike the > RZ/G2{L,LC} binding? > To be consistent with the RZ/G2L family of SoC's "1" is appended to the compatible string. > As RZ/G2UL is always single-core, perhaps this compatible value can be > dropped? > Do agree with you. > > + - const: renesas,r9a07g043 > > + > > additionalProperties: true > > For now, there are no users of this binding? > I assume you're posting it already, as RZ/G2UL is pin-compatible with RZ/G2LC, > and thus can be used interchangeably on the G2L SOM? > However, the DTS board part in this series is for RZ/G2L, not RZ/GLC? > Intention here is to start with RZ/G2L SoC first so that the core changes (pinctrl/CPG) hit upstream and for the rest of the SoC's it will be followed up. Cheers, Prabhakar > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > i.e. will queue in renesas-devel for v5.14, after the above have been > resolved. > > 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, Thank you for the review. On Fri, May 21, 2021 at 2:25 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Prabhakar, > > On Fri, May 14, 2021 at 9:23 PM Lad Prabhakar > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > Add ARCH_R9A07G044{L,LC} as a configuration symbol for the new Renesas > > RZ/G2{L,LC} SoC's. > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> > > Thanks for your patch! > > > --- a/drivers/soc/renesas/Kconfig > > +++ b/drivers/soc/renesas/Kconfig > > @@ -279,6 +279,16 @@ config ARCH_R8A774B1 > > help > > This enables support for the Renesas RZ/G2N SoC. > > > > +config ARCH_R9A07G044L > > + bool "ARM64 Platform support for RZ/G2L SoC" > > Please drop the "SoC", for consistency with other entries. > Oops will do that. > > + help > > + This enables support for the Renesas RZ/G2L SoC. > > + > > +config ARCH_R9A07G044LC > > + bool "ARM64 Platform support for RZ/G2LC SoC" > > Likewise. > will do. > > + help > > + This enables support for the Renesas RZ/G2LC SoC. > > + > > endif # ARM64 > > Given LSI DEVID is the same, do we need both, or can we do with a > single ARCH_R9A07G044? > The reason behind adding separate configs was in case if we wanted to just build an image for RZ/G2L and not RZ/G2LC this would increase image size and also build unneeded dtb's. Cheers, Prabhakar > 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, Thank you for the review. On Fri, May 21, 2021 at 4:41 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Prabhakar, > > On Fri, May 14, 2021 at 9:24 PM Lad Prabhakar > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > Add basic support for RZ/G2L SMARC EVK (based on R9A07G044L2): > > - memory > > - External input clock > > - SCIF > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > Thanks for your patch! > > > --- a/arch/arm64/boot/dts/renesas/Makefile > > +++ b/arch/arm64/boot/dts/renesas/Makefile > > @@ -62,3 +62,5 @@ dtb-$(CONFIG_ARCH_R8A77990) += r8a77990-ebisu.dtb > > dtb-$(CONFIG_ARCH_R8A77995) += r8a77995-draak.dtb > > > > dtb-$(CONFIG_ARCH_R8A779A0) += r8a779a0-falcon.dtb > > + > > +dtb-$(CONFIG_ARCH_R9A07G044L) += r9a07g044l2-smarc.dtb > > diff --git a/arch/arm64/boot/dts/renesas/g2l-smarc.dtsi b/arch/arm64/boot/dts/renesas/g2l-smarc.dtsi > > new file mode 100644 > > index 000000000000..9b95d73fb798 > > --- /dev/null > > +++ b/arch/arm64/boot/dts/renesas/g2l-smarc.dtsi > > rzg2l-smarc? > Agreed will do. Cheers, Prabhakar > The rest looks good to me (taking into account compatible value > discussions). > > 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, Thank you for the review. On Fri, May 21, 2021 at 4:35 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Prabhakar, > > On Fri, May 14, 2021 at 9:24 PM Lad Prabhakar > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > Add initial DTSI for RZ/G2{L,LC} SoC's. > > > > File structure: > > r9a07g044.dtsi => RZ/G2L family SoC common parts > > r9a07g044l.dtsi => Specific to RZ/G2L (R9A07G044L) SoC > > r9a07g044l1.dtsi => Specific to RZ/G2L (R9A07G044L single cortex A55) SoC > > r9a07g044l2.dtsi => Specific to RZ/G2L (R9A07G044L dual cortex A55) SoC > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > Thanks for your patch! > > > index 000000000000..c625d302f889 > > --- /dev/null > > +++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi > > @@ -0,0 +1,70 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > Do we want to use > > SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > for new DTS files? > This actually also applies to <dt-bindings/...> files. > > > +/* > > + * Device Tree Source for the RZ/G2L and RZ/G2LC common SoC parts > > + * > > + * Copyright (C) 2021 Renesas Electronics Corp. > > + */ > > + > > +#include <dt-bindings/interrupt-controller/arm-gic.h> > > +#include <dt-bindings/clock/r9a07g044l-cpg.h> > > + > > +/ { > > + compatible = "renesas,r9a07g044"; > > + #address-cells = <2>; > > + #size-cells = <2>; > > + > > + extal_clk: extal { > > + compatible = "fixed-clock"; > > + #clock-cells = <0>; > > + /* This value must be overridden by the board */ > > + clock-frequency = <0>; > > + }; > > + > > + psci { > > + compatible = "arm,psci-1.0", "arm,psci-0.2"; > > + method = "smc"; > > + }; > > + > > + soc: soc { > > + compatible = "simple-bus"; > > + interrupt-parent = <&gic>; > > + #address-cells = <2>; > > + #size-cells = <2>; > > + ranges; > > + > > + scif0: serial@1004b800 { > > + compatible = "renesas,scif-r9a07g044"; > > + reg = <0 0x1004b800 0 0x400>; > > + interrupts = > > + <GIC_SPI 380 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 382 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 383 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 381 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 384 IRQ_TYPE_LEVEL_HIGH>; > > "make dtbs_check > DT_SCHEMA_FILES=Documentation/devicetree/bindings/serial/renesas,scif.yaml": > > arch/arm64/boot/dts/renesas/r9a07g044l2-smarc.dt.yaml: > serial@1004b800: interrupts: 'oneOf' conditional failed, one must be > fixed: > [[0, 380, 4], [0, 382, 4], [0, 383, 4], [0, 381, 4], [0, 384, 4]] > is too long > Additional items are not allowed ([0, 382, 4], [0, 383, 4], [0, > 381, 4], [0, 384, 4] were unexpected) > Additional items are not allowed ([0, 384, 4] was unexpected) > [[0, 380, 4], [0, 382, 4], [0, 383, 4], [0, 381, 4], [0, 384, 4]] > is too short > > One interrupt is missing. According to the documentation, "tei" and > "dri" share an interrupt, so they should map to the same interrupt number. > Please add interrupt-names. > Agreed will do. > > + clocks = <&cpg CPG_MOD R9A07G044_CLK_SCIF0>; > > + clock-names = "fck"; > > + power-domains = <&cpg>; > > + resets = <&cpg R9A07G044_CLK_SCIF0>; > > + status = "disabled"; > > + }; > > + > > + devid: chipid@11020a04 { > > + compatible = "renesas,devid"; > > + reg = <0 0x11020a04 0 4>; > > + }; > > + > > + gic: interrupt-controller@11900000 { > > + compatible = "arm,gic-v3"; > > + #interrupt-cells = <3>; > > + #address-cells = <0>; > > + interrupt-controller; > > + reg = <0x0 0x11900000 0 0x40000>, > > + <0x0 0x11940000 0 0x60000>; > > + interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_LOW>; > > + clocks = <&cpg CPG_MOD R9A07G044_CLK_GIC600>; > > + clock-names = "gic6000"; > > This looks like a weird name ;-) > In addition, it should be the consumer clock name, not the provider > clock name. > Agreed will rename that. > > + power-domains = <&cpg>; > > + resets = <&cpg R9A07G044_CLK_GIC600>; > > "make dtbs_check > DT_SCHEMA_FILES=Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml": > > arch/arm64/boot/dts/renesas/r9a07g044l2-smarc.dt.yaml: > interrupt-controller@11900000: 'clock-names', 'clocks', > 'power-domains', 'resets' do not match any of the regexes: > '^(msi-controller|gic-its|interrupt-controller)@[0-9a-f]+$', > '^gic-its@', '^interrupt-controller@[0-9a-f]+$', 'pinctrl-[0-9]+' > > These properties should be added to the GIC-v3 bindings, cfr. the normal > GIC bindings. > Agreed will do that. > > > + }; > > + }; > > +}; > > diff --git a/arch/arm64/boot/dts/renesas/r9a07g044l.dtsi b/arch/arm64/boot/dts/renesas/r9a07g044l.dtsi > > new file mode 100644 > > index 000000000000..8d396b9100c1 > > --- /dev/null > > +++ b/arch/arm64/boot/dts/renesas/r9a07g044l.dtsi > > @@ -0,0 +1,21 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Device Tree Source for the RZ/G2L common SoC parts > > + * > > + * Copyright (C) 2021 Renesas Electronics Corp. > > + */ > > + > > +/dts-v1/; > > +#include "r9a07g044.dtsi" > > + > > +&soc { > > + cpg: clock-controller@11010000 { > > + compatible = "renesas,r9a07g044l-cpg"; > > + reg = <0 0x11010000 0 0x10000>; > > + clocks = <&extal_clk>; > > + clock-names = "extal"; > > + #clock-cells = <2>; > > + #reset-cells = <1>; > > + #power-domain-cells = <0>; > > + }; > > As I think this is shared by RZ/G2L and RZ/G2LC, it belongs to > r9a07g044.dtsi. > As some of the IP blocks present on RZ/G2L aren't present in RZ/G2LC the clocks for the IP will be missing hence this is added to SoC specific dtsi. > > +}; > > diff --git a/arch/arm64/boot/dts/renesas/r9a07g044l1.dtsi b/arch/arm64/boot/dts/renesas/r9a07g044l1.dtsi > > new file mode 100644 > > index 000000000000..44d4504e44c3 > > --- /dev/null > > +++ b/arch/arm64/boot/dts/renesas/r9a07g044l1.dtsi > > @@ -0,0 +1,43 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Device Tree Source for the RZ/G2L R9A07G044L1 common parts > > + * > > + * Copyright (C) 2021 Renesas Electronics Corp. > > + */ > > + > > +/dts-v1/; > > +#include "r9a07g044l.dtsi" > > + > > +/ { > > + compatible = "renesas,r9a07g044l1"; > > + #address-cells = <2>; > > + #size-cells = <2>; > > #{address,size}-cells already defined in r9a07g044.dtsi. > Will drop that. > > + > > + cpus { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + a55_0: cpu@0 { > > + compatible = "arm,cortex-a55"; > > + reg = <0>; > > + device_type = "cpu"; > > + next-level-cache = <&L3_CA55>; > > + enable-method = "psci"; > > + }; > > + > > + L3_CA55: cache-controller-0 { > > + compatible = "cache"; > > + cache-unified; > > + cache-size = <0x40000>; > > + }; > > I think the first CPU core should be in the base r9a07g044.dtsi file. > OK will move that. > > + }; > > + > > + timer { > > + compatible = "arm,armv8-timer"; > > + interrupts-extended = > > + <&gic GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(1) | IRQ_TYPE_LEVEL_LOW)>, > > + <&gic GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(1) | IRQ_TYPE_LEVEL_LOW)>, > > + <&gic GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(1) | IRQ_TYPE_LEVEL_LOW)>, > > + <&gic GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(1) | IRQ_TYPE_LEVEL_LOW)>; > > Also in the base file, with interrupts-extended overridden where > needed? > OK will override here with the timer node moved from r9a07g044l2.dtsi to common SoC file. > > + }; > > +}; > > diff --git a/arch/arm64/boot/dts/renesas/r9a07g044l2.dtsi b/arch/arm64/boot/dts/renesas/r9a07g044l2.dtsi > > new file mode 100644 > > index 000000000000..33bb35e1c369 > > --- /dev/null > > +++ b/arch/arm64/boot/dts/renesas/r9a07g044l2.dtsi > > @@ -0,0 +1,62 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Device Tree Source for the RZ/G2L R9A07G044L2 common parts > > + * > > + * Copyright (C) 2021 Renesas Electronics Corp. > > + */ > > + > > +/dts-v1/; > > +#include "r9a07g044l.dtsi" > > + > > +/ { > > + compatible = "renesas,r9a07g044l2"; > > + #address-cells = <2>; > > + #size-cells = <2>; > > + > > + cpus { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + cpu-map { > > + cluster0 { > > + core0 { > > + cpu = <&a55_0>; > > + }; > > + core1 { > > + cpu = <&a55_1>; > > + }; > > + }; > > + }; > > + > > + a55_0: cpu@0 { > > + compatible = "arm,cortex-a55"; > > + reg = <0>; > > + device_type = "cpu"; > > + next-level-cache = <&L3_CA55>; > > + enable-method = "psci"; > > + }; > > + > > + a55_1: cpu@1 { > > + compatible = "arm,cortex-a55"; > > + reg = <0x100>; > > + device_type = "cpu"; > > + next-level-cache = <&L3_CA55>; > > + enable-method = "psci"; > > + }; > > + > > + L3_CA55: cache-controller-0 { > > + compatible = "cache"; > > + cache-unified; > > + cache-size = <0x40000>; > > + }; > > I think (at least) the first CPU core should be in the base > r9a07g044.dtsi file. > Probably the second CPU core should be in the base file, too, and > removed by /delete-node/ in r9a07g044l1.dtsi? > Agreed will do that. > > + }; > > + > > + 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)>; > > + }; > > Also in the base file, with interrupts-extended overridden where > needed? > Will move this node to common SoC r9a07g044.dtsi file and override in r9a07g044l1.dtsi with CPU maks to 1. Cheers, Prabhakar > > +}; > > -- > > 2.17.1 > > > > > -- > 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, Thank you for the review. On Fri, May 21, 2021 at 4:04 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Prabhakar, > > On Fri, May 14, 2021 at 9:23 PM Lad Prabhakar > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > Document the device tree bindings of the Renesas RZ/G2L SoC clock > > driver in Documentation/devicetree/bindings/clock/renesas,rzg2l-cpg.yaml. > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> > > Thanks for your patch! > > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/clock/renesas,rzg2l-cpg.yaml > > @@ -0,0 +1,80 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: "http://devicetree.org/schemas/clock/renesas,rzg2l-cpg.yaml#" > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > > + > > +title: Renesas RZ/G2L Clock Pulse Generator / Module Stop and Software Reset > > (Module Standby Mode > > + > > +maintainers: > > + - Geert Uytterhoeven <geert+renesas@glider.be> > > + > > +description: | > > + On Renesas RZ/G2L SoC, the CPG (Clock Pulse Generator) and MSTP > > + (Module Stop and Software Reset) share the same register block. > > + > > + They provide the following functionalities: > > + - The CPG block generates various core clocks, > > + - The MSTP block provides two functions: > > + 1. Module Stop, providing a Clock Domain to control the clock supply > > + to individual SoC devices, > > + 2. Reset Control, to perform a software reset of individual SoC devices. > > + > > +properties: > > + compatible: > > + const: renesas,r9a07g044l-cpg # RZ/G2L > > renesas,r9a07g044-cpg? > As some IP blocks present in RZ/G2L aren't present in RZ/G2LC clock handling will differ so as a result SoC specific compatible string is added. > I believe it's the same block on RZ/G2L ('044l) and RZ/G2LC ('044c). > > > + '#clock-cells': > > + description: | > > + - For CPG core clocks, the two clock specifier cells must be "CPG_CORE" > > + and a core clock reference, as defined in > > + <dt-bindings/clock/*-cpg-mssr.h> > > <dt-bindings/clock/r9a07g044l-cpg.h> > Indeed > > + - For module clocks, the two clock specifier cells must be "CPG_MOD" and > > + a module number, as defined in the datasheet. > > Also in <dt-bindings/clock/r9a07g044l-cpg.h>? > Agreed. > > + const: 2 > > + > > + '#power-domain-cells': > > + description: > > + SoC devices that are part of the CPG/MSTP Clock Domain and can be > > + power-managed through Module Stop should refer to the CPG device node > > + in their "power-domains" property, as documented by the generic PM Domain > > + bindings in Documentation/devicetree/bindings/power/power-domain.yaml. > > + const: 0 > > + > > + '#reset-cells': > > + description: > > + The single reset specifier cell must be the module number, as defined in > > + the datasheet. > > Also in <dt-bindings/clock/r9a07g044l-cpg.h>? > Agreed. Cheers, Prabhakar > > + const: 1 > > 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 Prabhakar, On Fri, May 14, 2021 at 9:24 PM Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > Add initial DTSI for RZ/G2{L,LC} SoC's. > > File structure: > r9a07g044.dtsi => RZ/G2L family SoC common parts > r9a07g044l.dtsi => Specific to RZ/G2L (R9A07G044L) SoC > r9a07g044l1.dtsi => Specific to RZ/G2L (R9A07G044L single cortex A55) SoC > r9a07g044l2.dtsi => Specific to RZ/G2L (R9A07G044L dual cortex A55) SoC > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > --- /dev/null > +++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi > @@ -0,0 +1,70 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Device Tree Source for the RZ/G2L and RZ/G2LC common SoC parts > + * > + * Copyright (C) 2021 Renesas Electronics Corp. > + */ > + > +#include <dt-bindings/interrupt-controller/arm-gic.h> > +#include <dt-bindings/clock/r9a07g044l-cpg.h> > + > +/ { > + compatible = "renesas,r9a07g044"; > --- /dev/null > +++ b/arch/arm64/boot/dts/renesas/r9a07g044l1.dtsi > @@ -0,0 +1,43 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Device Tree Source for the RZ/G2L R9A07G044L1 common parts > + * > + * Copyright (C) 2021 Renesas Electronics Corp. > + */ > + > +/dts-v1/; > +#include "r9a07g044l.dtsi" > + > +/ { > + compatible = "renesas,r9a07g044l1"; This overwrites the main compatible value set by r9a07g044.dtsi before. As per your bindings, you want both: compatible = "renesas,r9a07g044l1", "renesas,r9a07g044". 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 Prabhakar, On Fri, May 21, 2021 at 6:54 PM Lad, Prabhakar <prabhakar.csengg@gmail.com> wrote: > On Fri, May 21, 2021 at 2:23 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > On Fri, May 14, 2021 at 9:23 PM Lad Prabhakar > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > > Add device tree bindings documentation for Renesas RZ/G2UL SoC. > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> > > > Reviewed-by: Chris Paterson <Chris.Paterson2@renesas.com> > > > > Thanks for your patch! > > > > > --- a/Documentation/devicetree/bindings/arm/renesas.yaml > > > +++ b/Documentation/devicetree/bindings/arm/renesas.yaml > > > @@ -302,6 +302,12 @@ properties: > > > - renesas,rzn1d400-db # RZN1D-DB (RZ/N1D Demo Board for the RZ/N1D 400 pins package) > > > - const: renesas,r9a06g032 > > > > > > + - description: RZ/G2UL (R9A07G043) > > > + items: > > > + - enum: > > > + - renesas,r9a07g043u11 # Single Cortex-A55 RZ/G2UL > > > > Is there any specific reason you're including the final "1", unlike the > > RZ/G2{L,LC} binding? > > > To be consistent with the RZ/G2L family of SoC's "1" is appended to > the compatible string. No, for RZ/G2L you have: renesas,r9a07g044c1 for r9a07g044c12 renesas,r9a07g044c2 for r9a07g044c22 renesas,r9a07g044l1 for r9a07g044l13 and r9a07g044l14 renesas,r9a07g044l2 for r9a07g044l23 and r9a07g044l24 i.e. the compatible value lacks the final digit. For RZ/G2UL, I do not know if we have to distinguish between r9a07g043u11 and r9a07g043u12. > > As RZ/G2UL is always single-core, perhaps this compatible value can be > > dropped? > > > Do agree with you. In light of the continued discussion for [PATCH 02/16], perhaps it's good to keep it anyway? 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, On Thu, May 27, 2021 at 12:29 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Prabhakar, > > On Fri, May 21, 2021 at 6:54 PM Lad, Prabhakar > <prabhakar.csengg@gmail.com> wrote: > > On Fri, May 21, 2021 at 2:23 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > > On Fri, May 14, 2021 at 9:23 PM Lad Prabhakar > > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > > > Add device tree bindings documentation for Renesas RZ/G2UL SoC. > > > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > Reviewed-by: Chris Paterson <Chris.Paterson2@renesas.com> > > > > > > Thanks for your patch! > > > > > > > --- a/Documentation/devicetree/bindings/arm/renesas.yaml > > > > +++ b/Documentation/devicetree/bindings/arm/renesas.yaml > > > > @@ -302,6 +302,12 @@ properties: > > > > - renesas,rzn1d400-db # RZN1D-DB (RZ/N1D Demo Board for the RZ/N1D 400 pins package) > > > > - const: renesas,r9a06g032 > > > > > > > > + - description: RZ/G2UL (R9A07G043) > > > > + items: > > > > + - enum: > > > > + - renesas,r9a07g043u11 # Single Cortex-A55 RZ/G2UL > > > > > > Is there any specific reason you're including the final "1", unlike the > > > RZ/G2{L,LC} binding? > > > > > To be consistent with the RZ/G2L family of SoC's "1" is appended to > > the compatible string. > My bad, the reason for adding 1 in the end was there are two variants of RZ/G2UL [1]. For the next respin I'll include renesas,r9a07g043u12 too. > No, for RZ/G2L you have: > > renesas,r9a07g044c1 for r9a07g044c12 > renesas,r9a07g044c2 for r9a07g044c22 > renesas,r9a07g044l1 for r9a07g044l13 and r9a07g044l14 > renesas,r9a07g044l2 for r9a07g044l23 and r9a07g044l24 > > i.e. the compatible value lacks the final digit. > > For RZ/G2UL, I do not know if we have to distinguish between > r9a07g043u11 and r9a07g043u12. > Some IP blocks are missing in type2 compared to type1. And at the higher level we might want to know the exact SoC type the board is built ? > > > As RZ/G2UL is always single-core, perhaps this compatible value can be > > > dropped? > > > > > Do agree with you. > > In light of the continued discussion for [PATCH 02/16], perhaps it's > good to keep it anyway? > Yes will keep the compatible string. [1] https://www.renesas.com/us/en/products/microcontrollers-microprocessors/rz-arm-based-high-end-32-64-bit-mpus/rzg2ul-general-purpose-microprocessors-single-core-arm-cortex-a55-10-ghz-cpu-2ch-giga-bit-ethernet Cheers, Prabhakar
Hi Prabhakar, On Fri, May 21, 2021 at 7:21 PM Lad, Prabhakar <prabhakar.csengg@gmail.com> wrote: > On Fri, May 21, 2021 at 2:25 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > On Fri, May 14, 2021 at 9:23 PM Lad Prabhakar > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > > Add ARCH_R9A07G044{L,LC} as a configuration symbol for the new Renesas > > > RZ/G2{L,LC} SoC's. > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > Thanks for your patch! > > > > > --- a/drivers/soc/renesas/Kconfig > > > +++ b/drivers/soc/renesas/Kconfig > > > @@ -279,6 +279,16 @@ config ARCH_R8A774B1 > > > help > > > This enables support for the Renesas RZ/G2N SoC. > > > > > > +config ARCH_R9A07G044L > > > + bool "ARM64 Platform support for RZ/G2L SoC" > > > > Please drop the "SoC", for consistency with other entries. > > > Oops will do that. > > > > + help > > > + This enables support for the Renesas RZ/G2L SoC. > > > + > > > +config ARCH_R9A07G044LC > > > + bool "ARM64 Platform support for RZ/G2LC SoC" > > > > Likewise. > > > will do. > > > > + help > > > + This enables support for the Renesas RZ/G2LC SoC. > > > + > > > endif # ARM64 > > > > Given LSI DEVID is the same, do we need both, or can we do with a > > single ARCH_R9A07G044? > > > The reason behind adding separate configs was in case if we wanted to > just build an image for RZ/G2L and not RZ/G2LC this would increase > image size and also build unneeded dtb's. How would it increase image size? I understand clock and pin control are the same blocks. Gr{oetje,eeting}s, Geert
Hi Prabhakar, On Fri, May 21, 2021 at 8:43 PM Lad, Prabhakar <prabhakar.csengg@gmail.com> wrote: > On Fri, May 21, 2021 at 4:04 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > On Fri, May 14, 2021 at 9:23 PM Lad Prabhakar > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > > Document the device tree bindings of the Renesas RZ/G2L SoC clock > > > driver in Documentation/devicetree/bindings/clock/renesas,rzg2l-cpg.yaml. > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > Thanks for your patch! > > > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/clock/renesas,rzg2l-cpg.yaml > > > @@ -0,0 +1,80 @@ > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: "http://devicetree.org/schemas/clock/renesas,rzg2l-cpg.yaml#" > > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > > > + > > > +title: Renesas RZ/G2L Clock Pulse Generator / Module Stop and Software Reset > > > > (Module Standby Mode > > > + > > > +maintainers: > > > + - Geert Uytterhoeven <geert+renesas@glider.be> > > > + > > > +description: | > > > + On Renesas RZ/G2L SoC, the CPG (Clock Pulse Generator) and MSTP > > > + (Module Stop and Software Reset) share the same register block. > > > + > > > + They provide the following functionalities: > > > + - The CPG block generates various core clocks, > > > + - The MSTP block provides two functions: > > > + 1. Module Stop, providing a Clock Domain to control the clock supply > > > + to individual SoC devices, > > > + 2. Reset Control, to perform a software reset of individual SoC devices. > > > + > > > +properties: > > > + compatible: > > > + const: renesas,r9a07g044l-cpg # RZ/G2L > > > > renesas,r9a07g044-cpg? > > > As some IP blocks present in RZ/G2L aren't present in RZ/G2LC clock > handling will differ so as a result SoC specific compatible string is > added. The RZ/G2L Hardware User's Manual Rev. 0.41 doesn't mention any differences between the CPG on RZ/G2L and RZ/G2LC. So I think it's safe to have a single driver for both members. Gr{oetje,eeting}s, Geert
Hi Geert, On Thu, May 27, 2021 at 12:17 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Prabhakar, > > On Fri, May 14, 2021 at 9:24 PM Lad Prabhakar > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > Add initial DTSI for RZ/G2{L,LC} SoC's. > > > > File structure: > > r9a07g044.dtsi => RZ/G2L family SoC common parts > > r9a07g044l.dtsi => Specific to RZ/G2L (R9A07G044L) SoC > > r9a07g044l1.dtsi => Specific to RZ/G2L (R9A07G044L single cortex A55) SoC > > r9a07g044l2.dtsi => Specific to RZ/G2L (R9A07G044L dual cortex A55) SoC > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > --- /dev/null > > +++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi > > @@ -0,0 +1,70 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Device Tree Source for the RZ/G2L and RZ/G2LC common SoC parts > > + * > > + * Copyright (C) 2021 Renesas Electronics Corp. > > + */ > > + > > +#include <dt-bindings/interrupt-controller/arm-gic.h> > > +#include <dt-bindings/clock/r9a07g044l-cpg.h> > > + > > +/ { > > + compatible = "renesas,r9a07g044"; > > > --- /dev/null > > +++ b/arch/arm64/boot/dts/renesas/r9a07g044l1.dtsi > > @@ -0,0 +1,43 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Device Tree Source for the RZ/G2L R9A07G044L1 common parts > > + * > > + * Copyright (C) 2021 Renesas Electronics Corp. > > + */ > > + > > +/dts-v1/; > > +#include "r9a07g044l.dtsi" > > + > > +/ { > > + compatible = "renesas,r9a07g044l1"; > > This overwrites the main compatible value set by r9a07g044.dtsi before. > As per your bindings, you want both: > > compatible = "renesas,r9a07g044l1", "renesas,r9a07g044". > Agreed will fix that in next respin. Cheers, Prabhakar
Hi Prabhakar, On Fri, May 14, 2021 at 9:24 PM Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > Add CPG core wrapper for RZ/G2L family. > > Based on a patch in the BSP by Binh Nguyen > <binh.nguyen.jz@renesas.com>. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> > --- /dev/null > +++ b/drivers/clk/renesas/renesas-rzg2l-cpg.c > +static int rzg2l_mod_clock_endisable(struct clk_hw *hw, bool enable) > +{ > + struct mstp_clock *clock = to_mod_clock(hw); > + struct cpg_mssr_priv *priv = clock->priv; > + unsigned int reg = MSSR_OFF(clock->bit) * 4; The "* 4" here makes it difficult to review the module clock tables. E.g. DEF_MOD("gic", R9A07G044_CLK_GIC600, R9A07G044_CLK_P1, MSSR(5, BIT(0), (BIT(0) | BIT(1)))), The "5" means the CLK_ON_GIC600 register is at offset CLK_ON_R(5 * 4) = 0x514. Removing the "* 4" means you could use "MSSR(0x14, BIT(0), (BIT(0) | BIT(1))" instead. Unless it has unpleasant side effects, I'd even consider putting the full CLK_ON offset there, i.e. "MSSR(0x514, BIT(0), (BIT(0) | BIT(1))" and change the macros like: #define CLK_ON_R(reg) (reg) #define CLK_MON_R(reg) (0x680 - 0x500 + (reg)) > --- /dev/null > +++ b/drivers/clk/renesas/renesas-rzg2l-cpg.h > +#define CLK_ON_R(reg) (0x500 + reg) > +#define CLK_MON_R(reg) (0x680 + reg) > +#define CLK_RST_R(reg) (0x800 + reg) > +#define CLK_MRST_R(reg) (0x980 + reg) The last three don't seem to be documented? 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, On Thu, May 27, 2021 at 1:04 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Prabhakar, > > On Fri, May 14, 2021 at 9:24 PM Lad Prabhakar > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > Add CPG core wrapper for RZ/G2L family. > > > > Based on a patch in the BSP by Binh Nguyen > > <binh.nguyen.jz@renesas.com>. > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> > > > --- /dev/null > > +++ b/drivers/clk/renesas/renesas-rzg2l-cpg.c > > > +static int rzg2l_mod_clock_endisable(struct clk_hw *hw, bool enable) > > +{ > > + struct mstp_clock *clock = to_mod_clock(hw); > > + struct cpg_mssr_priv *priv = clock->priv; > > + unsigned int reg = MSSR_OFF(clock->bit) * 4; > > The "* 4" here makes it difficult to review the module clock tables. > > E.g. > > DEF_MOD("gic", R9A07G044_CLK_GIC600, > R9A07G044_CLK_P1, > MSSR(5, BIT(0), (BIT(0) | BIT(1)))), > > The "5" means the CLK_ON_GIC600 register is at offset CLK_ON_R(5 * 4) > = 0x514. Removing the "* 4" means you could use > "MSSR(0x14, BIT(0), (BIT(0) | BIT(1))" instead. > > Unless it has unpleasant side effects, I'd even consider putting > the full CLK_ON offset there, i.e. > "MSSR(0x514, BIT(0), (BIT(0) | BIT(1))" and change the macros like: > > #define CLK_ON_R(reg) (reg) > #define CLK_MON_R(reg) (0x680 - 0x500 + (reg)) > OK will do that. > > --- /dev/null > > +++ b/drivers/clk/renesas/renesas-rzg2l-cpg.h > > > +#define CLK_ON_R(reg) (0x500 + reg) > > +#define CLK_MON_R(reg) (0x680 + reg) > > +#define CLK_RST_R(reg) (0x800 + reg) > > +#define CLK_MRST_R(reg) (0x980 + reg) > > The last three don't seem to be documented? > I have asked Chris to send the document across. Cheers, Prabhakar