On 28.03.2019 11:16, Dirk Behme wrote: > Hi Geert, > > On 28.03.2019 10:24, Geert Uytterhoeven wrote: >> Hi Eugeniu, >> >> On Wed, Mar 27, 2019 at 7:36 PM Eugeniu Rosca >> 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. Just for the archives: We are testing the attached patch. Best regards Dirk