| Message ID | 20231120111820.87398-1-claudiu.beznea.uj@bp.renesas.com |
|---|---|
| Headers | show |
| Series | irqchip/renesas-rzg2l: add support for RZ/G3S SoC | expand |
Hi Claudiu, On Mon, 20 Nov 2023, Claudiu wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > IA55 interrupt controller is available on RZ/G3S SoC. Add IA55 pclk and > its reset. > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Thanks for your patch! > --- a/drivers/clk/renesas/r9a08g045-cpg.c > +++ b/drivers/clk/renesas/r9a08g045-cpg.c > @@ -188,6 +188,7 @@ static const struct cpg_core_clk r9a08g045_core_clks[] __initconst = { > > static const struct rzg2l_mod_clk r9a08g045_mod_clks[] = { > DEF_MOD("gic_gicclk", R9A08G045_GIC600_GICCLK, R9A08G045_CLK_P1, 0x514, 0), > + DEF_MOD("ia55_pclk", R9A08G045_IA55_PCLK, R9A08G045_CLK_P2, 0x518, 0), This conflicts with [1], which you sent just before. If that patch goes in first, I guess this new entry should gain ", MSTOP(PERI_CPU, BIT(13))", just like the entry for ia55_clk? > DEF_MOD("ia55_clk", R9A08G045_IA55_CLK, R9A08G045_CLK_P1, 0x518, 1), > DEF_MOD("dmac_aclk", R9A08G045_DMAC_ACLK, R9A08G045_CLK_P3, 0x52c, 0), > DEF_MOD("sdhi0_imclk", R9A08G045_SDHI0_IMCLK, CLK_SD0_DIV4, 0x554, 0), Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> [1] "clk: renesas: rzg2l-cpg: Add support for MSTOP" https://lore.kernel.org/r/20231120070024.4079344-4-claudiu.beznea.uj@bp.renesas.com Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Mon, 20 Nov 2023, Claudiu wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > Use tabs instead of spaces in definition of TINT_EXTRACT_HWIRQ() > and TINT_EXTRACT_GPIOINT() macros to align with coding style > requirements described in Documentation/process/coding-style.rst, > "Indentation" chapter. > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Mon, Nov 20, 2023 at 1:00 PM Claudiu <claudiu.beznea@tuxon.dev> wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > Align struct member names to tabs to follow the requirements from > maintainer-tip file. 3 tabs were used at the moment as the next commits > will add a new member which requires 3 tabs for a better view. > > Link: https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#struct-declarations-and-initializers > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert
On Tue, Nov 21, 2023 at 7:16 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > Document structure members to follow the requirements specified in > maintainer-tip, section 4.3.7. Struct declarations and initializers. > > Link: https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#struct-declarations-and-initializers > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert
On Mon, 20 Nov 2023, Claudiu wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > The RZ/G2L manual (chapter "IRQ Status Control Register (ISCR)") describes > the operation to clear interrupts through the ISCR register as follows: > > [Write operation] > When "Falling-edge detection", "Rising-edge detection" or > "Falling/Rising-edge detection" is set in IITSR: > - In case ISTAT is 1 > 0: IRQn interrupt detection status is cleared. > 1: Invalid to write. > - In case ISTAT is 0 > Invalid to write. > > When “Low-level detection” is set in IITSR.: > Invalid to write. > > Take the interrupt type into account when clearing interrupts through > the ISCR register to avoid writing the ISCR when interrupt type is > level. > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Claudiu, Thanks for your patch! On Mon, 20 Nov 2023, Claudiu wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > There are 2 TITSR registers available on IA55 interrupt controller. A ... the IA55 interrupt controller. > single macro could be used to access both of them. Add a macro that > retrieves TITSR register offset based on it's index. This macro is the TITSR register offset ... its index > useful in commit that adds suspend/resume support to access both TITSR > registers in a for loop. This macro will be useful to access both TITSR registers in a for loop when adding suspend/resume support later/ > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > --- a/drivers/irqchip/irq-renesas-rzg2l.c > +++ b/drivers/irqchip/irq-renesas-rzg2l.c > @@ -28,8 +28,7 @@ > #define ISCR 0x10 > #define IITSR 0x14 > #define TSCR 0x20 > -#define TITSR0 0x24 > -#define TITSR1 0x28 > +#define TITSR(n) (0x24 + (n) * 4) > #define TITSR0_MAX_INT 16 > #define TITSEL_WIDTH 0x2 > #define TSSR(n) (0x30 + ((n) * 4)) > @@ -200,8 +199,7 @@ static int rzg2l_tint_set_edge(struct irq_data *d, unsigned int type) > struct rzg2l_irqc_priv *priv = irq_data_to_priv(d); > unsigned int hwirq = irqd_to_hwirq(d); > u32 titseln = hwirq - IRQC_TINT_START; > - u32 offset; > - u8 sense; > + u8 index, sense; > u32 reg; > > switch (type & IRQ_TYPE_SENSE_MASK) { > @@ -217,17 +215,17 @@ static int rzg2l_tint_set_edge(struct irq_data *d, unsigned int type) > return -EINVAL; > } > > - offset = TITSR0; > + index = 0; > if (titseln >= TITSR0_MAX_INT) { > titseln -= TITSR0_MAX_INT; > - offset = TITSR1; > + index = 1; > } You can remove this if you would use ... > > raw_spin_lock(&priv->lock); > - reg = readl_relaxed(priv->base + offset); > + reg = readl_relaxed(priv->base + TITSR(index)); ... TITSR(titseln / TITSR0_MAX_INT) here. > reg &= ~(IRQ_MASK << (titseln * TITSEL_WIDTH)); > reg |= sense << (titseln * TITSEL_WIDTH); > - writel_relaxed(reg, priv->base + offset); > + writel_relaxed(reg, priv->base + TITSR(index)); > raw_spin_unlock(&priv->lock); > > return 0; > -- > 2.39.2 > > 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 21.11.2023 11:59, Geert Uytterhoeven wrote: > Hi Claudiu, > > On Mon, 20 Nov 2023, Claudiu wrote: >> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >> >> IA55 interrupt controller is available on RZ/G3S SoC. Add IA55 pclk and >> its reset. >> >> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > Thanks for your patch! > >> --- a/drivers/clk/renesas/r9a08g045-cpg.c >> +++ b/drivers/clk/renesas/r9a08g045-cpg.c >> @@ -188,6 +188,7 @@ static const struct cpg_core_clk >> r9a08g045_core_clks[] __initconst = { >> >> static const struct rzg2l_mod_clk r9a08g045_mod_clks[] = { >> DEF_MOD("gic_gicclk", R9A08G045_GIC600_GICCLK, >> R9A08G045_CLK_P1, 0x514, 0), >> + DEF_MOD("ia55_pclk", R9A08G045_IA55_PCLK, R9A08G045_CLK_P2, >> 0x518, 0), > > This conflicts with [1], which you sent just before. Sorry for that, I intended to adapt the one that will have ended up last in your tree. > > If that patch goes in first, I guess this new entry should gain > ", MSTOP(PERI_CPU, BIT(13))", just like the entry for ia55_clk? That's right. Thank you, Claudiu Beznea > >> DEF_MOD("ia55_clk", R9A08G045_IA55_CLK, R9A08G045_CLK_P1, >> 0x518, 1), >> DEF_MOD("dmac_aclk", R9A08G045_DMAC_ACLK, R9A08G045_CLK_P3, >> 0x52c, 0), >> DEF_MOD("sdhi0_imclk", R9A08G045_SDHI0_IMCLK, CLK_SD0_DIV4, >> 0x554, 0), > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > [1] "clk: renesas: rzg2l-cpg: Add support for MSTOP" > > https://lore.kernel.org/r/20231120070024.4079344-4-claudiu.beznea.uj@bp.renesas.com > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- > geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like > that. > -- Linus Torvalds
On Tue, Nov 21, 2023 at 10:59 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Mon, 20 Nov 2023, Claudiu wrote: > > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > > > IA55 interrupt controller is available on RZ/G3S SoC. Add IA55 pclk and > > its reset. > > > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > Thanks for your patch! > > > --- a/drivers/clk/renesas/r9a08g045-cpg.c > > +++ b/drivers/clk/renesas/r9a08g045-cpg.c > > @@ -188,6 +188,7 @@ static const struct cpg_core_clk r9a08g045_core_clks[] __initconst = { > > > > static const struct rzg2l_mod_clk r9a08g045_mod_clks[] = { > > DEF_MOD("gic_gicclk", R9A08G045_GIC600_GICCLK, R9A08G045_CLK_P1, 0x514, 0), > > + DEF_MOD("ia55_pclk", R9A08G045_IA55_PCLK, R9A08G045_CLK_P2, 0x518, 0), > > This conflicts with [1], which you sent just before. > > If that patch goes in first, I guess this new entry should gain > ", MSTOP(PERI_CPU, BIT(13))", just like the entry for ia55_clk? > > > DEF_MOD("ia55_clk", R9A08G045_IA55_CLK, R9A08G045_CLK_P1, 0x518, 1), > > DEF_MOD("dmac_aclk", R9A08G045_DMAC_ACLK, R9A08G045_CLK_P3, 0x52c, 0), > > DEF_MOD("sdhi0_imclk", R9A08G045_SDHI0_IMCLK, CLK_SD0_DIV4, 0x554, 0), > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > [1] "clk: renesas: rzg2l-cpg: Add support for MSTOP" > https://lore.kernel.org/r/20231120070024.4079344-4-claudiu.beznea.uj@bp.renesas.com As the MSTOP support is on hold, I will queue this in renesas-clk-for-v6.8. Gr{oetje,eeting}s, Geert
On Wed, Nov 22, 2023 at 7:16 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > Add IA55 interrupt controller node and set it as interrupt parent for pin > controller. > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> i.e. will queue in renesas-devel for v6.8. Gr{oetje,eeting}s, Geert
From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Hi, Series adds support for IA55 available on RZ/G3S SoC. Patches are split as follows: - 1/9 adds IA55 clock - 2-4/9 minor cleanups to align with the suggestions at [1] and coding style recommendations - 5/9 implement restriction described in HW manual for ISCR register - 6/9 add a macro to retrieve TITSR base address based on it's index - 7/9 add suspend to RAM support - 8/9 updates documentation - 9/9 adds IA55 device tree node Thank you, Claudiu Beznea [1] https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#struct-declarations-and-initializers Changes in v3: - kept driver private data object as pointer - moved patch 1/9 from v2 (dt-bindings: interrupt-controller: renesas,rzg2l-irqc: Document RZ/G3S) after IRQ controller driver feature patches Changes in v2: - collected Conor's tag - updated commit description according to code review comments - added patches 4, 5 according to review recommendations - updated patch 7/9 to retrieve only TITSR base address; dropped the rest of the changes for the moment - in patch 8/9 use local variable in suspend/resume functions for controller's base address, indent initialized structures members to tabs, updated private driver data structure name - patch 3/7 from v1 was replaced by patch 7/9 in v2 - patch 5/7 from v1 was renamed "Add support for suspend to RAM" - cleanup patches were kept at the beginning of the series and features at the end Claudiu Beznea (9): clk: renesas: r9a08g045: Add IA55 pclk and its reset irqchip/renesas-rzg2l: Use tabs instead of spaces irqchip/renesas-rzg2l: Align struct member names to tabs irqchip/renesas-rzg2l: Document structure members irqchip/renesas-rzg2l: Implement restriction when writing ISCR register irqchip/renesas-rzg2l: Add macro to retrieve TITSR register offset based on register's index irqchip/renesas-rzg2l: Add support for suspend to RAM dt-bindings: interrupt-controller: renesas,rzg2l-irqc: Document RZ/G3S arm64: dts: renesas: r9108g045: Add IA55 interrupt controller node .../renesas,rzg2l-irqc.yaml | 5 +- arch/arm64/boot/dts/renesas/r9a08g045.dtsi | 68 +++++++++++ drivers/clk/renesas/r9a08g045-cpg.c | 3 + drivers/irqchip/irq-renesas-rzg2l.c | 110 +++++++++++++----- 4 files changed, 158 insertions(+), 28 deletions(-)