> From: Prabhakar <prabhakar.csengg@xxxxxxxxx> > Sent: 03 February 2025 14:35 > Subject: [PATCH v7] i2c: riic: Implement bus recovery > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > > Implement bus recovery by reinitializing the hardware to reset the bus > state and generating 9 clock cycles (and a stop condition) to release > the SDA line. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > Tested-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx> > Reviewed-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx> Reviewed-by: Fabrizio Castro <fabrizio.castro.jz@xxxxxxxxxxx> > --- > Resending this patch which was part of v6 [0] series. > > [0] https://lore.kernel.org/lkml/Z4ZCJYPgvS0Ke39g@shikoro/T/ > > Hi Wolfram, > > Ive replied to your comments on v2 here [1] as to why the generic > recovery algorithm was not used. > > [1] https://lore.kernel.org/all/CA+V-a8s4-g9vxyfYMgnKMK=Oej9kDBwWsWehWLYTkxw-06w-2g@xxxxxxxxxxxxxx/ > > Cheers, > Prabhakar > > v6->v7 > - None > > v2->v6 > - Included RB and TB from Claudiu. > > v1->v2 > - Used single register read to check SDA/SCL lines > --- > drivers/i2c/busses/i2c-riic.c | 100 ++++++++++++++++++++++++++++++---- > 1 file changed, 90 insertions(+), 10 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-riic.c b/drivers/i2c/busses/i2c-riic.c > index d7dddd6c296a..888825423d94 100644 > --- a/drivers/i2c/busses/i2c-riic.c > +++ b/drivers/i2c/busses/i2c-riic.c > @@ -51,6 +51,7 @@ > > #define ICCR1_ICE BIT(7) > #define ICCR1_IICRST BIT(6) > +#define ICCR1_CLO BIT(5) > #define ICCR1_SOWP BIT(4) > #define ICCR1_SCLI BIT(1) > #define ICCR1_SDAI BIT(0) > @@ -69,6 +70,7 @@ > #define ICMR3_ACKBT BIT(3) > > #define ICFER_FMPE BIT(7) > +#define ICFER_MALE BIT(1) > > #define ICIER_TIE BIT(7) > #define ICIER_TEIE BIT(6) > @@ -82,6 +84,8 @@ > > #define RIIC_INIT_MSG -1 > > +#define RIIC_RECOVERY_CLK_CNT 9 > + > enum riic_reg_list { > RIIC_ICCR1 = 0, > RIIC_ICCR2, > @@ -151,13 +155,16 @@ static int riic_bus_barrier(struct riic_dev *riic) > ret = readb_poll_timeout(riic->base + riic->info->regs[RIIC_ICCR2], val, > !(val & ICCR2_BBSY), 10, riic->adapter.timeout); > if (ret) > - return ret; > + goto i2c_recover; > > if ((riic_readb(riic, RIIC_ICCR1) & (ICCR1_SDAI | ICCR1_SCLI)) != > (ICCR1_SDAI | ICCR1_SCLI)) > - return -EBUSY; > + goto i2c_recover; > > return 0; > + > +i2c_recover: > + return i2c_recover_bus(&riic->adapter); > } > > static int riic_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num) > @@ -332,7 +339,7 @@ static const struct i2c_algorithm riic_algo = { > .functionality = riic_func, > }; > > -static int riic_init_hw(struct riic_dev *riic) > +static int riic_init_hw(struct riic_dev *riic, bool recover) > { > int ret; > unsigned long rate; > @@ -414,9 +421,11 @@ static int riic_init_hw(struct riic_dev *riic) > rate / total_ticks, ((brl + 3) * 100) / (brl + brh + 6), > t->scl_fall_ns / ns_per_tick, t->scl_rise_ns / ns_per_tick, cks, brl, brh); > > - ret = pm_runtime_resume_and_get(dev); > - if (ret) > - return ret; > + if (!recover) { > + ret = pm_runtime_resume_and_get(dev); > + if (ret) > + return ret; > + } > > /* Changing the order of accessing IICRST and ICE may break things! */ > riic_writeb(riic, ICCR1_IICRST | ICCR1_SOWP, RIIC_ICCR1); > @@ -434,8 +443,74 @@ static int riic_init_hw(struct riic_dev *riic) > > riic_clear_set_bit(riic, ICCR1_IICRST, 0, RIIC_ICCR1); > > - pm_runtime_mark_last_busy(dev); > - pm_runtime_put_autosuspend(dev); > + if (!recover) { > + pm_runtime_mark_last_busy(dev); > + pm_runtime_put_autosuspend(dev); > + } > + return 0; > +} > + > +static int riic_recover_bus(struct i2c_adapter *adap) > +{ > + struct riic_dev *riic = i2c_get_adapdata(adap); > + struct device *dev = riic->adapter.dev.parent; > + int ret; > + u8 val; > + > + ret = riic_init_hw(riic, true); > + if (ret) > + return ret; > + > + /* output extra SCL clock cycles with master arbitration-lost detection disabled */ > + riic_clear_set_bit(riic, ICFER_MALE, 0, RIIC_ICFER); > + > + for (unsigned int i = 0; i < RIIC_RECOVERY_CLK_CNT; i++) { > + riic_clear_set_bit(riic, 0, ICCR1_CLO, RIIC_ICCR1); > + ret = readb_poll_timeout(riic->base + riic->info->regs[RIIC_ICCR1], val, > + !(val & ICCR1_CLO), 0, 100); > + if (ret) { > + dev_err(dev, "SCL clock cycle timeout\n"); > + return ret; > + } > + } > + > + /* > + * The last clock cycle may have driven the SDA line high, so add a > + * short delay to allow the line to stabilize before checking the status. > + */ > + udelay(5); > + > + /* > + * If an incomplete byte write occurs, the SDA line may remain low > + * even after 9 clock pulses, indicating the bus is not released. > + * To resolve this, send an additional clock pulse to simulate a STOP > + * condition and ensure proper bus release. > + */ > + if ((riic_readb(riic, RIIC_ICCR1) & (ICCR1_SDAI | ICCR1_SCLI)) != > + (ICCR1_SDAI | ICCR1_SCLI)) { > + riic_clear_set_bit(riic, 0, ICCR1_CLO, RIIC_ICCR1); > + ret = readb_poll_timeout(riic->base + riic->info->regs[RIIC_ICCR1], val, > + !(val & ICCR1_CLO), 0, 100); > + if (ret) { > + dev_err(dev, "SCL clock cycle timeout occurred while issuing the STOP condition\n"); > + return ret; > + } > + /* delay to make sure SDA line goes back HIGH again */ > + udelay(5); > + } > + > + /* clear any flags set */ > + riic_writeb(riic, 0, RIIC_ICSR2); > + /* read back register to confirm writes */ > + riic_readb(riic, RIIC_ICSR2); > + > + /* restore back ICFER_MALE */ > + riic_clear_set_bit(riic, 0, ICFER_MALE, RIIC_ICFER); > + > + if ((riic_readb(riic, RIIC_ICCR1) & (ICCR1_SDAI | ICCR1_SCLI)) != > + (ICCR1_SDAI | ICCR1_SCLI)) > + return -EINVAL; > + > return 0; > } > > @@ -447,6 +522,10 @@ static const struct riic_irq_desc riic_irqs[] = { > { .res_num = 5, .isr = riic_tend_isr, .name = "riic-nack" }, > }; > > +static struct i2c_bus_recovery_info riic_bri = { > + .recover_bus = riic_recover_bus, > +}; > + > static int riic_i2c_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > @@ -493,6 +572,7 @@ static int riic_i2c_probe(struct platform_device *pdev) > strscpy(adap->name, "Renesas RIIC adapter", sizeof(adap->name)); > adap->owner = THIS_MODULE; > adap->algo = &riic_algo; > + adap->bus_recovery_info = &riic_bri; > adap->dev.parent = dev; > adap->dev.of_node = dev->of_node; > > @@ -505,7 +585,7 @@ static int riic_i2c_probe(struct platform_device *pdev) > pm_runtime_use_autosuspend(dev); > pm_runtime_enable(dev); > > - ret = riic_init_hw(riic); > + ret = riic_init_hw(riic, false); > if (ret) > goto out; > > @@ -613,7 +693,7 @@ static int riic_i2c_resume(struct device *dev) > if (ret) > return ret; > > - ret = riic_init_hw(riic); > + ret = riic_init_hw(riic, false); > if (ret) { > /* > * In case this happens there is no way to recover from this > -- > 2.43.0
