Hi Wolfram, > > > @@ -217,7 +228,17 @@ static void rcar_i2c_init(struct rcar_i2c_priv *priv) > > > rcar_i2c_write(priv, ICMCR, MDBS); > > > rcar_i2c_write(priv, ICMSR, 0); > > > /* start clock */ > > > - rcar_i2c_write(priv, ICCCR, priv->icccr); > > > + if (priv->flags & ID_P_FMPLUS) { > > > + rcar_i2c_write(priv, ICCCR, 0); > > > + rcar_i2c_write(priv, ICMPR, priv->clock_val); > > > + rcar_i2c_write(priv, ICHPR, 3 * priv->clock_val); > > > + rcar_i2c_write(priv, ICLPR, 3 * priv->clock_val); > > > + rcar_i2c_write(priv, ICCCR2, FMPE | CDFD | HLSE | SME); > > > + } else { > > > + rcar_i2c_write(priv, ICCCR, priv->clock_val); > > > + if (priv->devtype >= I2C_RCAR_GEN3) > > > + rcar_i2c_write(priv, ICCCR2, 0); > > > > is this last bit part of the FM+ enabling or is it part of the > > GEN4 support? > > It is "disabling FM+" for lower speeds. Since we never used ICCCR2 > before FM+, we need to make sure it is cleared properly. OK... I'm missing some hardware details here :) > > > + for (scgd = 0; scgd < 0x40; scgd++) { > > > + scl = ick / (20 + (scgd * 8) + round); > > > + if (scl <= t.bus_freq_hz) > > > + break; > > > + } > > > + > > > + if (scgd == 0x40) { > > > > would be nice to give a meaning to this 0x40 constant... either > > having it in a define or a comment, at least. > > This code existed before and was just moved into an if-body. It will be > updated in another series following this one. OK, thanks! Reviewed-by: Andi Shyti <andi.shyti@xxxxxxxxxx> Andi