On Wed, Oct 9, 2024 at 4:01 PM Théo Lebrun <theo.lebrun@xxxxxxxxxxx> wrote: > Current BRCR computation is: > > brcr = floor(i2cclk / (clkfreq * div)) > > With brcr: "baud rate counter", an internal clock divider, > and i2cclk: input clock rate (24MHz, 38.4MHz or 48MHz), > and clkfreq: desired bus rate, > and div: speed-mode dependent divider (2 for standard, 3 otherwise). > > Assume i2cclk=48MHz, clkfreq=3.4MHz, div=3, > then brcr = floor(48MHz / (3.4MHz * 3)) = 4 > and resulting bus rate = 48MHz / (4 * 3) = 4MHz > > Assume i2cclk=38.4MHz, clkfreq=1.0MHz, div=3, > then brcr = floor(38.4MHz / (1.0MHz * 3)) = 12 > and resulting bus rate = 38.4MHz / (12 * 3) = 1066kHz > > The current computation means we always pick the smallest divider that > gives a bus rate above target. We should instead pick the largest > divider that gives a bus rate below target, using: > > brcr = floor(i2cclk / (clkfreq * div)) + 1 > > If we redo the above examples: > > Assume i2cclk=48MHz, clkfreq=3.4MHz, div=3, > then brcr = floor(48MHz / (3.4MHz * 3)) + 1 = 5 > and resulting bus rate = 48MHz / (5 * 3) = 3.2MHz > > Assume i2cclk=38.4MHz, clkfreq=1.0MHz, div=3, > then brcr = floor(38.4MHz / (1.0MHz * 3)) + 1 = 13 > and resulting bus rate = 38.4MHz / (13 * 3) = 985kHz > > In kernel C code, floor(x) is DIV_ROUND_DOWN() and, > floor(x)+1 is DIV_ROUND_UP(). > > This is much less of an issue with slower bus rates (ie those currently > supported), because the gap from one divider to the next is much > smaller. It however keeps us from always using bus rates superior to > the target. > > This fix is required for later on supporting faster bus rates: > I2C_FREQ_MODE_FAST_PLUS (1MHz) and I2C_FREQ_MODE_HIGH_SPEED (3.4MHz). > > Signed-off-by: Théo Lebrun <theo.lebrun@xxxxxxxxxxx> Excellent, thanks for iterating! Reviewed-by: Linus Walleij <linus.walleij@xxxxxxxxxx> Yours, Linus Walleij
