Thank you for your patch. > On August 8, 2018 at 9:59 AM Wolfram Sang <wsa+rene...@sang-engineering.com> > wrote: > > > When doing a REP_START after a read message, the driver used to trigger > a STOP first which would then be overwritten by REP_START. This was the > only stable method found when doing the last refactoring. However, this > was not in accordance with the documentation. > > After research from our BSP team and myself, we now can implement a > version which works and is according to the documentation. The new > approach ensures the ICMCR register is only changed when really needed. > > Tested on a R-Car Gen2 (H2) and Gen3 with DMA (M3N). > > Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki...@renesas.com> > Signed-off-by: Wolfram Sang <wsa+rene...@sang-engineering.com> > --- > drivers/i2c/busses/i2c-rcar.c | 34 ++++++++++++++++++++-------------- > 1 file changed, 20 insertions(+), 14 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c > index a9f1880e2eae..43ad933df0f0 100644 > --- a/drivers/i2c/busses/i2c-rcar.c > +++ b/drivers/i2c/busses/i2c-rcar.c > @@ -113,9 +113,10 @@ > #define ID_ARBLOST (1 << 3) > #define ID_NACK (1 << 4) > /* persistent flags */ > +#define ID_P_REP_AFTER_RD BIT(29) > #define ID_P_NO_RXDMA BIT(30) /* HW forbids RXDMA sometimes */ > #define ID_P_PM_BLOCKED BIT(31) > -#define ID_P_MASK GENMASK(31, 30) > +#define ID_P_MASK GENMASK(31, 29) > > enum rcar_i2c_type { > I2C_RCAR_GEN1, > @@ -345,7 +346,10 @@ static void rcar_i2c_prepare_msg(struct rcar_i2c_priv > *priv) > rcar_i2c_write(priv, ICMSR, 0); > rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_START); > } else { > - rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_START); > + if (priv->flags & ID_P_REP_AFTER_RD) > + priv->flags &= ~ID_P_REP_AFTER_RD; > + else > + rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_START); > rcar_i2c_write(priv, ICMSR, 0); > } > rcar_i2c_write(priv, ICMIER, read ? RCAR_IRQ_RECV : RCAR_IRQ_SEND); > @@ -550,15 +554,15 @@ static void rcar_i2c_irq_recv(struct rcar_i2c_priv > *priv, u32 msr) > priv->pos++; > } > > - /* > - * If next received data is the _LAST_, go to STOP phase. Might be > - * overwritten by REP START when setting up a new msg. Not elegant > - * but the only stable sequence for REP START I have found so far. > - * If you want to change this code, make sure sending one transfer with > - * four messages (WR-RD-WR-RD) works! > - */ > - if (priv->pos + 1 >= msg->len) > - rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_STOP); > + /* If next received data is the _LAST_, go to new phase. */ > + if (priv->pos + 1 == msg->len) { > + if (priv->flags & ID_LAST_MSG) { > + rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_STOP); > + } else { > + rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_START); > + priv->flags |= ID_P_REP_AFTER_RD; > + } > + }
So "priv->pos + 1 <= msg->len" is an invariant? (The current code seems to imply that it isn't.) If it is, Reviewed-by: Ulrich Hecht <uli+rene...@fpond.eu> CU Uli