Hi Geert,
On 28.03.2019 10:24, Geert Uytterhoeven wrote:
Hi Eugeniu,
On Wed, Mar 27, 2019 at 7:36 PM Eugeniu Rosca <erosca@xxxxxxxxxxxxxx> wrote:
We've recently switched from rcar-3.7.x to rcar-3.9.x [1] kernel and the
latter contains this patch [2] by virtue of rcar-3.9.0 commit [3], which
mirrors v4.18-rc1 commit [4] in mainline.
JFYI, quite far away in the delivery chain, we've received below report:
With this patch [2-4] there are reports about broken data
communication with 115200 baud with SXM module. Reverting
this patch results in successful communication, again.
While this scarce information barely helps anybody, I thought that
sharing it with you might be beneficial in case you collect several
reports linked to this specific commit in future, meaning it potentially
adds a regression.
Also, if you are aware of any userland changes that might be
required/assumed by this patch or in case you have any alternative
ideas how to avoid reverting this patch, your feedback would be very
appreciated.
Thanks for your report!
[TLDR: skip to the suggested fix below; I only noticed the bug after
writing the below paragraphs, which are still useful questions to
let us reproduce the issue]
Which SoC are you using?
I assume this is on a custom board, as Salvator-X(S) and ULCB have
external SCIF clock crystals, which allow to use a perfect 115200 bps,
hence the affected code path is not exercised:
sh-sci e6550000.serial: BRG: 115200+0 bps using DL 4 SR 32
sh-sci e6550000.serial: Using clk scif for 115200+0 bps
Does your board have an external SCIF clock? Which frequency?
Can you check the clock values and deviation for your configuration, by
changing the calls to print the above information from dev_dbg() to
dev_info()?
Does adding the DIV_ROUND_CLOSEST(), as suggested in my review
of the posted patch, help?
Perhaps the sampling point shift is inverted? Does -shift work better?
[possible solution]
+ int shift = min(-8, max(7, deviation / 2));
Oops, min and max are exchanged!
I guess using
int shift = clamp(deviation / 2, -8, 7)
instead fixes the issue?
Uh, that was fast :) Many thanks!
We will test this as fast as possible! But due to the long delivery
chain Eugeniu mentioned this will take some time. I'll try my best to
come back to you as fast as possible.
Thanks again!
Dirk
