On 08/16/2017 08:59 AM, Benjamin Herrenschmidt wrote: > 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 :-) ah yes. that is useful I agree. C.