Re: [PATCH] TMS570 console driver, SCI frame error (baudrate calculation error)
No, because we don't have a reproducible test case. On Fri, Jul 14, 2023 at 7:38 PM zack leung wrote: > > hi, so do you want me make this change? > > On Wed, 12 Jul 2023 at 10:54, Gedare Bloom wrote: >> >> On Wed, Jul 12, 2023 at 8:06 AM Joel Sherrill wrote: >> > >> > >> > 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. >> > >> I posted it over there. Thanks Pavel and Joel. >> >> > Thanks. >> > >> > --joel >> > >> > On Wed, Jul 12, 2023 at 8:49 AM Pavel Pisa 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 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) & 0xff) | ((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: p...@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@rtems.org >> >> http://lists.rtems.org/mailman/listinfo/devel ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH] TMS570 console driver, SCI frame error (baudrate calculation error)
hi, so do you want me make this change? On Wed, 12 Jul 2023 at 10:54, Gedare Bloom wrote: > On Wed, Jul 12, 2023 at 8:06 AM Joel Sherrill wrote: > > > > > > 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. > > > I posted it over there. Thanks Pavel and Joel. > > > Thanks. > > > > --joel > > > > On Wed, Jul 12, 2023 at 8:49 AM Pavel Pisa > 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 > 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) & 0xff) | ((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: p...@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@rtems.org > >> http://lists.rtems.org/mailman/listinfo/devel > ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH] TMS570 console driver, SCI frame error (baudrate calculation error)
On Wed, Jul 12, 2023 at 8:06 AM Joel Sherrill wrote: > > > 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. > I posted it over there. Thanks Pavel and Joel. > Thanks. > > --joel > > On Wed, Jul 12, 2023 at 8:49 AM Pavel Pisa 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 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) & 0xff) | ((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: p...@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@rtems.org >> http://lists.rtems.org/mailman/listinfo/devel ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH] TMS570 console driver, SCI frame error (baudrate calculation error)
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 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 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) & 0xff) | ((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: p...@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@rtems.org > http://lists.rtems.org/mailman/listinfo/devel ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH] TMS570 console driver, SCI frame error (baudrate calculation error)
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 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) & 0xff) | ((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: p...@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@rtems.org http://lists.rtems.org/mailman/listinfo/devel
[PATCH] TMS570 console driver, SCI frame error (baudrate calculation error)
fixes #4903 adressing an error in the TMS570 console driver --- bsps/arm/tms570/console/tms570-sci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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; ctx->regs->GCR1 |= TMS570_SCI_GCR1_SWnRST | TMS570_SCI_GCR1_TXENA | TMS570_SCI_GCR1_RXENA; -- 2.34.1 ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
[PATCH] TMS570 console driver, SCI frame error (baudrate calculation error)
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; ctx->regs->GCR1 |= TMS570_SCI_GCR1_SWnRST | TMS570_SCI_GCR1_TXENA | TMS570_SCI_GCR1_RXENA; -- 2.34.1 ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH] TMS570 console driver, SCI frame error (baudrate calculation error)
First, the patch commit message needs to reference the ticket (#4903). It would be nice to have Pavel's feedback but the submitter of https://devel.rtems.org/ticket/4903 provided the fix but not in patch format. What do you think we should do beyond trust the submitter of the ticket? bsps/arm/tms570/console/tms570-sci.c: 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; replacing with 2 fixed frame error ctx->regs->BRS = bauddiv? bauddiv - 2: 0; there is issue in baudrate calculation On Tue, Jul 11, 2023 at 12:52 PM Gedare Bloom wrote: > Hi Zack, > > Thanks for the patch. Someone should probably test it, or identify in > the documentation why this calculation was off-by-1. Pavel, any clues? > > Gedare > > On Sun, Jul 9, 2023 at 10:09 PM zack 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; > > > >ctx->regs->GCR1 |= TMS570_SCI_GCR1_SWnRST | TMS570_SCI_GCR1_TXENA | > > TMS570_SCI_GCR1_RXENA; > > -- > > 2.34.1 > > > > ___ > > devel mailing list > > devel@rtems.org > > http://lists.rtems.org/mailman/listinfo/devel > ___ > devel mailing list > devel@rtems.org > http://lists.rtems.org/mailman/listinfo/devel ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH] TMS570 console driver, SCI frame error (baudrate calculation error)
Hi Zack, Thanks for the patch. Someone should probably test it, or identify in the documentation why this calculation was off-by-1. Pavel, any clues? Gedare On Sun, Jul 9, 2023 at 10:09 PM zack 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; > >ctx->regs->GCR1 |= TMS570_SCI_GCR1_SWnRST | TMS570_SCI_GCR1_TXENA | > TMS570_SCI_GCR1_RXENA; > -- > 2.34.1 > > ___ > devel mailing list > devel@rtems.org > http://lists.rtems.org/mailman/listinfo/devel ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
[PATCH] TMS570 console driver, SCI frame error (baudrate calculation error)
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; ctx->regs->GCR1 |= TMS570_SCI_GCR1_SWnRST | TMS570_SCI_GCR1_TXENA | TMS570_SCI_GCR1_RXENA; -- 2.34.1 ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
[PATCH] TMS570 console driver, SCI frame error (baudrate calculation error)
--- bsps/arm/tms570/console/tms570-sci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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; ctx->regs->GCR1 |= TMS570_SCI_GCR1_SWnRST | TMS570_SCI_GCR1_TXENA | TMS570_SCI_GCR1_RXENA; -- 2.34.1 ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
