| Message ID | 20250530143135.366417-7-prabhakar.mahadev-lad.rj@bp.renesas.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add RIIC support for RZ/T2H and RZ/N2H SoCs | expand |
On Fri, May 30, 2025 at 03:31:35PM +0100, Prabhakar wrote: > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Add support for the Renesas RZ/T2H (R9A09G077) SoC, which features a > different interrupt layout for the RIIC controller. Unlike other SoCs > with individual error interrupts, RZ/T2H uses a combined error interrupt > (EEI). > > Introduce a new IRQ descriptor table for RZ/T2H, along with a custom > ISR (`riic_eei_isr`) to handle STOP and NACK detection from the shared > interrupt. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
On Fri, May 30, 2025 at 03:31:35PM +0100, Prabhakar wrote: > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Add support for the Renesas RZ/T2H (R9A09G077) SoC, which features a > different interrupt layout for the RIIC controller. Unlike other SoCs > with individual error interrupts, RZ/T2H uses a combined error interrupt > (EEI). > > Introduce a new IRQ descriptor table for RZ/T2H, along with a custom > ISR (`riic_eei_isr`) to handle STOP and NACK detection from the shared > interrupt. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com> # on RZ/A1
Hi Prabhakar, On Fri, 30 May 2025 at 16:31, Prabhakar <prabhakar.csengg@gmail.com> wrote: > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Add support for the Renesas RZ/T2H (R9A09G077) SoC, which features a > different interrupt layout for the RIIC controller. Unlike other SoCs > with individual error interrupts, RZ/T2H uses a combined error interrupt > (EEI). > > Introduce a new IRQ descriptor table for RZ/T2H, along with a custom > ISR (`riic_eei_isr`) to handle STOP and NACK detection from the shared > interrupt. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Thanks for your patch! > --- a/drivers/i2c/busses/i2c-riic.c > +++ b/drivers/i2c/busses/i2c-riic.c > @@ -326,6 +327,19 @@ static irqreturn_t riic_stop_isr(int irq, void *data) > return IRQ_HANDLED; > } > > +static irqreturn_t riic_eei_isr(int irq, void *data) > +{ > + u8 icsr2 = riic_readb(data, RIIC_ICSR2); > + > + if (icsr2 & ICSR2_NACKF) > + return riic_tend_isr(irq, data); > + > + if (icsr2 & ICSR2_STOP) > + return riic_stop_isr(irq, data); Just wondering: can both ICSR2_NACKF and ICSR2_STOP be set? As riic_tend_isr() clears only ICSR2_NACKF, while riic_stop_isr() clears all bits, the two calls could be chained, if needed. > + > + return IRQ_NONE; > +} > + > static u32 riic_func(struct i2c_adapter *adap) > { > return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL; Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert
Hi Geert, Thank you for the review. On Fri, Jun 6, 2025 at 2:37 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Prabhakar, > > On Fri, 30 May 2025 at 16:31, Prabhakar <prabhakar.csengg@gmail.com> wrote: > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > Add support for the Renesas RZ/T2H (R9A09G077) SoC, which features a > > different interrupt layout for the RIIC controller. Unlike other SoCs > > with individual error interrupts, RZ/T2H uses a combined error interrupt > > (EEI). > > > > Introduce a new IRQ descriptor table for RZ/T2H, along with a custom > > ISR (`riic_eei_isr`) to handle STOP and NACK detection from the shared > > interrupt. > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Thanks for your patch! > > > --- a/drivers/i2c/busses/i2c-riic.c > > +++ b/drivers/i2c/busses/i2c-riic.c > > @@ -326,6 +327,19 @@ static irqreturn_t riic_stop_isr(int irq, void *data) > > return IRQ_HANDLED; > > } > > > > +static irqreturn_t riic_eei_isr(int irq, void *data) > > +{ > > + u8 icsr2 = riic_readb(data, RIIC_ICSR2); > > + > > + if (icsr2 & ICSR2_NACKF) > > + return riic_tend_isr(irq, data); > > + > > + if (icsr2 & ICSR2_STOP) > > + return riic_stop_isr(irq, data); > > Just wondering: can both ICSR2_NACKF and ICSR2_STOP be set? > As riic_tend_isr() clears only ICSR2_NACKF, while riic_stop_isr() > clears all bits, the two calls could be chained, if needed. > In the normal working scenario when verified both these bits were never set together, hence I took this path. Cheers, Prabhakar
diff --git a/drivers/i2c/busses/i2c-riic.c b/drivers/i2c/busses/i2c-riic.c index a4df00cb470c..1f9299f5effa 100644 --- a/drivers/i2c/busses/i2c-riic.c +++ b/drivers/i2c/busses/i2c-riic.c @@ -79,6 +79,7 @@ #define ICIER_SPIE BIT(3) #define ICSR2_NACKF BIT(4) +#define ICSR2_STOP BIT(3) #define ICBR_RESERVED GENMASK(7, 5) /* Should be 1 on writes */ @@ -326,6 +327,19 @@ static irqreturn_t riic_stop_isr(int irq, void *data) return IRQ_HANDLED; } +static irqreturn_t riic_eei_isr(int irq, void *data) +{ + u8 icsr2 = riic_readb(data, RIIC_ICSR2); + + if (icsr2 & ICSR2_NACKF) + return riic_tend_isr(irq, data); + + if (icsr2 & ICSR2_STOP) + return riic_stop_isr(irq, data); + + return IRQ_NONE; +} + static u32 riic_func(struct i2c_adapter *adap) { return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL; @@ -497,6 +511,13 @@ static const struct riic_irq_desc riic_irqs[] = { { .res_num = 5, .isr = riic_tend_isr, .name = "riic-nack" }, }; +static const struct riic_irq_desc riic_rzt2h_irqs[] = { + { .res_num = 0, .isr = riic_tend_isr, .name = "riic-tend" }, + { .res_num = 1, .isr = riic_rdrf_isr, .name = "riic-rdrf" }, + { .res_num = 2, .isr = riic_tdre_isr, .name = "riic-tdre" }, + { .res_num = 3, .isr = riic_eei_isr, .name = "riic-eei" }, +}; + static int riic_i2c_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; @@ -643,6 +664,12 @@ static const struct riic_of_data riic_rz_v2h_info = { .num_irqs = ARRAY_SIZE(riic_irqs), }; +static const struct riic_of_data riic_rz_t2h_info = { + .regs = riic_rz_v2h_regs, + .irqs = riic_rzt2h_irqs, + .num_irqs = ARRAY_SIZE(riic_rzt2h_irqs), +}; + static int riic_i2c_suspend(struct device *dev) { struct riic_dev *riic = dev_get_drvdata(dev); @@ -695,6 +722,7 @@ static const struct dev_pm_ops riic_i2c_pm_ops = { static const struct of_device_id riic_i2c_dt_ids[] = { { .compatible = "renesas,riic-r7s72100", .data = &riic_rz_a1h_info, }, { .compatible = "renesas,riic-r9a09g057", .data = &riic_rz_v2h_info }, + { .compatible = "renesas,riic-r9a09g077", .data = &riic_rz_t2h_info }, { .compatible = "renesas,riic-rz", .data = &riic_rz_a_info }, { /* Sentinel */ } };