<div dir="ltr">hi, so do you want me make this change? <br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 12 Jul 2023 at 10:54, Gedare Bloom <<a href="mailto:gedare@rtems.org">gedare@rtems.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Wed, Jul 12, 2023 at 8:06 AM Joel Sherrill <<a href="mailto:joel@rtems.org" target="_blank">joel@rtems.org</a>> wrote:<br>
><br>
><br>
> Pavel this was filed as <a href="https://devel.rtems.org/ticket/4903" rel="noreferrer" target="_blank">https://devel.rtems.org/ticket/4903</a>. The ticket submitter<br>
> didn't give a patch, just a code change snippet and explanation. Zack turned it into<br>
> a patch. You will need to post that on the ticket to get the submitter's feedback.<br>
><br>
> I hope you don't mind copying that to the ticket and seeing what you think is<br>
> really up.<br>
><br>
I posted it over there. Thanks Pavel and Joel.<br>
<br>
> Thanks.<br>
><br>
> --joel<br>
><br>
> On Wed, Jul 12, 2023 at 8:49 AM Pavel Pisa <<a href="mailto:ppisa4lists@pikron.com" target="_blank">ppisa4lists@pikron.com</a>> wrote:<br>
>><br>
>> Hello Zack and Gedare,<br>
>><br>
>> On Tuesday 11 of July 2023 19:52:27 Gedare Bloom wrote:<br>
>> > Thanks for the patch. Someone should probably test it, or identify in<br>
>> > the documentation why this calculation was off-by-1. Pavel, any clues?<br>
>> > On Sun, Jul 9, 2023 at 10:09 PM zack <<a href="mailto:zakthertemsdev@gmail.com" target="_blank">zakthertemsdev@gmail.com</a>> wrote:<br>
>> > > Fixes #4903<br>
>> > > diff --git a/bsps/arm/tms570/console/tms570-sci.c<br>
>> > > b/bsps/arm/tms570/console/tms570-sci.c index 768770a4c8..59a0b7e6f1<br>
>> > > 100644<br>
>> > > --- a/bsps/arm/tms570/console/tms570-sci.c<br>
>> > > +++ b/bsps/arm/tms570/console/tms570-sci.c<br>
>> > > @@ -311,7 +311,7 @@ bool tms570_sci_set_attributes(<br>
>> > > /* Apply baudrate to the hardware */<br>
>> > > baudrate *= 2 * 16;<br>
>> > > bauddiv = (BSP_PLL_OUT_CLOCK + baudrate / 2) / baudrate;<br>
>> > > - ctx->regs->BRS = bauddiv? bauddiv - 1: 0;<br>
>> > > + ctx->regs->BRS = bauddiv? bauddiv - 2: 0;<br>
>><br>
>> I think that change is not correct. The actual used values<br>
>> for BSP_PLL_OUT_CLOCK and baudrate should be provided to analyze<br>
>> the case. The code can result in some rounding error and can<br>
>> be enhanced if fractional divider is used or even super finegrained<br>
>> fractional divider. But these options are available only for<br>
>> for SCI/LIN peripheral case.<br>
>><br>
>> According to<br>
>><br>
>> TMS570LS31x/21x 16/32-Bit RISC Flash Microcontroller<br>
>> Technical Reference Manual<br>
>> Literature Number: SPNU499B<br>
>><br>
>> 26.2.3 SCI Baud Rate<br>
>><br>
>> SCICLK Frequency = VCLK Frequency / (P + 1 + M / 16)<br>
>><br>
>> Asynchronous baud value = SCICLK Frequency / 16<br>
>><br>
>> So the substraction of one corresponds to the manual.<br>
>><br>
>> Actual code does not use M part. It would be problem if it is<br>
>> leftover from some boot/monitor but it is part of BRS 32-bit<br>
>> register which is overwritten in the whole, so such problem<br>
>> should not appear either.<br>
>><br>
>> So I vote against the proposed change for now and suggest<br>
>> to do analysis what happens in the computation and what<br>
>> are input values and output. Change would/could affect<br>
>> negatively large number of combinations of the baudrate<br>
>> and clocks.<br>
>><br>
>> I would consider to discuss if the rounding formula<br>
>> could/should be updated, but I think that it is the best<br>
>> which cane be achieved for rations which do not result<br>
>> in exact ratio.<br>
>><br>
>> (BSP_PLL_OUT_CLOCK + baudrate / 2) / baudrate;<br>
>><br>
>> If there is interrest then code can be enhanced by fraction<br>
>> dividers for SCI/LIN peripheral case. The field with variant<br>
>> should be added into tms570_sci_context and in this case<br>
>> the alternative formula can be used<br>
>><br>
>> long long bauddiv;<br>
>> bauddiv = (BSP_PLL_OUT_CLOCK * 16ll + baudrate / 2) / baudrate;<br>
>> ctx->regs->BRS = ((bauddiv >> 4) & 0xffffff) | ((bauddiv & 0xf) << 24);<br>
>><br>
>> which should be rewritten after header for SCI/LIN update to<br>
>><br>
>> ctx->regs->BRS = TMS570_LIN_BRS_P(bauddiv >> 4) | TMS570_LIN_BRS_M(bauddiv & 0xf);<br>
><br>
><br>
><br>
>><br>
>><br>
>> Best wishes,<br>
>><br>
>><br>
>> Pavel<br>
>> --<br>
>> Pavel Pisa<br>
>> phone: +420 603531357<br>
>> e-mail: <a href="mailto:pisa@cmp.felk.cvut.cz" target="_blank">pisa@cmp.felk.cvut.cz</a><br>
>> Department of Control Engineering FEE CVUT<br>
>> Karlovo namesti 13, 121 35, Prague 2<br>
>> university: <a href="http://control.fel.cvut.cz/" rel="noreferrer" target="_blank">http://control.fel.cvut.cz/</a><br>
>> personal: <a href="http://cmp.felk.cvut.cz/~pisa" rel="noreferrer" target="_blank">http://cmp.felk.cvut.cz/~pisa</a><br>
>> projects: <a href="https://www.openhub.net/accounts/ppisa" rel="noreferrer" target="_blank">https://www.openhub.net/accounts/ppisa</a><br>
>> CAN related:<a href="http://canbus.pages.fel.cvut.cz/" rel="noreferrer" target="_blank">http://canbus.pages.fel.cvut.cz/</a><br>
>> RISC-V education: <a href="https://comparch.edu.cvut.cz/" rel="noreferrer" target="_blank">https://comparch.edu.cvut.cz/</a><br>
>> Open Technologies Research Education and Exchange Services<br>
>> <a href="https://gitlab.fel.cvut.cz/otrees/org/-/wikis/home" rel="noreferrer" target="_blank">https://gitlab.fel.cvut.cz/otrees/org/-/wikis/home</a><br>
>><br>
>> _______________________________________________<br>
>> devel mailing list<br>
>> <a href="mailto:devel@rtems.org" target="_blank">devel@rtems.org</a><br>
>> <a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.rtems.org/mailman/listinfo/devel</a><br>
</blockquote></div>