Hi Andi,
On Wed, Dec 11, 2024 at 12:20 PM Andi Shyti <andi.shyti@xxxxxxxxxx> wrote:
> On Fri, Nov 22, 2024 at 03:14:35PM +0100, Geert Uytterhoeven wrote:
> > Currently, the RIIC driver may run the I2C bus faster than requested,
> > which may cause subtle failures. E.g. Biju reported a measured bus
> > speed of 450 kHz instead of the expected maximum of 400 kHz on RZ/G2L.
> >
> > The initial calculation of the bus period uses DIV_ROUND_UP(), to make
> > sure the actual bus speed never becomes faster than the requested bus
> > speed. However, the subsequent division-by-two steps do not use
> > round-up, which may lead to a too-small period, hence a too-fast and
> > possible out-of-spec bus speed. E.g. on RZ/Five, requesting a bus speed
> > of 100 resp. 400 kHz will yield too-fast target bus speeds of 100806
> > resp. 403226 Hz instead of 97656 resp. 390625 Hz.
> >
> > Fix this by using DIV_ROUND_UP() in the subsequent divisions, too.
> >
> > Tested on RZ/A1H, RZ/A2M, and RZ/Five.
> >
> > Fixes: d982d66514192cdb ("i2c: riic: remove clock and frequency restrictions")
> > Reported-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> > ---
> > Apart from the rounding issue fixed by this patch, I could not find any
> > bugs in the calculation of the various parameters (based on the formulas
> > in the documentation). Still, the actual (measured) bus speed may still
> > be higher than the target bus speed. Hence this patch is not sufficient
> > to reduce the actual bus speed to safe levels, and I have not yet addded
> >
> > Closes: https://lore.kernel.org/TYCPR01MB11269BFE7A9D3DC605BA2A2A9864E2@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
>
> Can I add this in the commit log?
I'd rather not add the Closes-tag, as this patch does not fully fix
the issue.
> > On RZ/A1H (RSK+RZA1):
[...]
> It's a pity that all this description is lost. I love long
> explanations especially when they come from test results.
>
> Can I add it in the commit log?
What about starting to add Link: tags pointing to lore to I2C commits,
like most other subsystems do?
That way people can easily reach any background information (if
available), and find the corresponding email thread where to report
issues or follow-up information?
Thanks!
[1] https://elixir.bootlin.com/linux/v6.12.4/source/Documentation/maintainer/configure-git.rst#L33
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
