[PATCH] TMS570 console driver, SCI frame error (baudrate calculation error)

[Joel Sherrill]

Pavel this was filed as https://devel.rtems.org/ticket/4903. The ticket
submitter
didn't give a patch, just a code change snippet and explanation. Zack
turned it into
a patch. You will need to post that on the ticket to get the submitter's
feedback.

I hope you don't mind copying that to the ticket and seeing what you think
is
really up.

Thanks.

--joel

On Wed, Jul 12, 2023 at 8:49 AM Pavel Pisa <ppisa4lists at pikron.com> wrote:

> Hello Zack and Gedare,
>
> On Tuesday 11 of July 2023 19:52:27 Gedare Bloom wrote:
> > Thanks for the patch. Someone should probably test it, or identify in
> > the documentation why this calculation was off-by-1. Pavel, any clues?
> > On Sun, Jul 9, 2023 at 10:09 PM zack <zakthertemsdev at gmail.com> wrote:
> > > Fixes #4903
> > > diff --git a/bsps/arm/tms570/console/tms570-sci.c
> > > b/bsps/arm/tms570/console/tms570-sci.c index 768770a4c8..59a0b7e6f1
> > > 100644
> > > --- a/bsps/arm/tms570/console/tms570-sci.c
> > > +++ b/bsps/arm/tms570/console/tms570-sci.c
> > > @@ -311,7 +311,7 @@ bool tms570_sci_set_attributes(
> > >    /* Apply baudrate to the hardware */
> > >    baudrate *= 2 * 16;
> > >    bauddiv = (BSP_PLL_OUT_CLOCK + baudrate / 2) / baudrate;
> > > -  ctx->regs->BRS = bauddiv? bauddiv - 1: 0;
> > > +  ctx->regs->BRS = bauddiv? bauddiv - 2: 0;
>
> I think that change is not correct. The actual used values
> for BSP_PLL_OUT_CLOCK and baudrate should be provided to analyze
> the case. The code can result in some rounding error and can
> be enhanced if fractional divider is used or even super finegrained
> fractional divider. But these options are available only for
> for SCI/LIN peripheral case.
>
> According to
>
> TMS570LS31x/21x 16/32-Bit RISC Flash Microcontroller
> Technical Reference Manual
> Literature Number: SPNU499B
>
> 26.2.3 SCI Baud Rate
>
>   SCICLK Frequency = VCLK Frequency / (P + 1 + M / 16)
>
>   Asynchronous baud value = SCICLK Frequency / 16
>
> So the substraction of one corresponds to the manual.
>
> Actual code does not use M part. It would be problem if it is
> leftover from some boot/monitor but it is part of BRS 32-bit
> register which is overwritten in the whole, so such problem
> should not appear either.
>
> So I vote against the proposed change for now and suggest
> to do analysis what happens in the computation and what
> are input values and output. Change would/could affect
> negatively large number of combinations of the baudrate
> and clocks.
>
> I would consider to discuss if the rounding formula
> could/should be updated, but I think that it is the best
> which cane be achieved for rations which do not result
> in exact ratio.
>
>   (BSP_PLL_OUT_CLOCK + baudrate / 2) / baudrate;
>
> If there is interrest then code can be enhanced by fraction
> dividers for SCI/LIN peripheral case. The field with variant
> should be added into tms570_sci_context and in this case
> the alternative formula can be used
>
>   long long bauddiv;
>   bauddiv = (BSP_PLL_OUT_CLOCK * 16ll + baudrate / 2) / baudrate;
>   ctx->regs->BRS = ((bauddiv >> 4) & 0xffffff) | ((bauddiv & 0xf) << 24);
>
> which should be rewritten after header for SCI/LIN update to
>
>   ctx->regs->BRS = TMS570_LIN_BRS_P(bauddiv >> 4) |
> TMS570_LIN_BRS_M(bauddiv & 0xf);
>



>
> Best wishes,
>
>
>                 Pavel
> --
>                 Pavel Pisa
>     phone:      +420 603531357
>     e-mail:     pisa at cmp.felk.cvut.cz
>     Department of Control Engineering FEE CVUT
>     Karlovo namesti 13, 121 35, Prague 2
>     university: http://control.fel.cvut.cz/
>     personal:   http://cmp.felk.cvut.cz/~pisa
>     projects:   https://www.openhub.net/accounts/ppisa
>     CAN related:http://canbus.pages.fel.cvut.cz/
>     RISC-V education: https://comparch.edu.cvut.cz/
>     Open Technologies Research Education and Exchange Services
>     https://gitlab.fel.cvut.cz/otrees/org/-/wikis/home
>
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20230712/3c2ce0ec/attachment.htm>