On Wed, 2017-08-16 at 08:53 +0200, Cédric Le Goater wrote: > > > divisor = DIV_ROUND_UP(bus->parent_clk_frequency, bus->bus_frequency); > > > - clk_reg_val = bus->get_clk_reg_val(divisor); > > > + clk_reg_val = readl(bus->base + ASPEED_I2C_AC_TIMING_REG1); > > > + clk_reg_val &= (ASPEED_I2CD_TIME_TBUF_MASK | > > > + ASPEED_I2CD_TIME_THDSTA_MASK | > > > + ASPEED_I2CD_TIME_TACST_MASK); > > > > Instead of keeping the u-boot values (which appear to be hard-coded), > > should we instead put the known working values in the register? > > Yes. I was wondering where the initial setting was from on the AST400. Well, the AST2500 hard codes them in HW, so it makes some amount of sense to use whatever aspeed platform.S hard codes in u-boot for the 2400. The values look reasonably sane. If we ever see a need to change them, we can add DT props etc... but for now I'd just not bother. The way it is now, at least, if I have problems, I can tweak the values with devmem and try again without the driver overwriting them :-) Cheers, Ben.