[PATCHES] Fix PPP in libbsd and optimize ATSAM console

Mon Feb 7 15:40:21 UTC 2022

On Wed, Feb 2, 2022 at 8:20 AM Christian MAUDERER
<christian.mauderer at embedded-brains.de> wrote:
>
> Hello,
>
> I received no comments for these in the last two weeks. I assume there
> are no further objections and I can push the patches to master in about
> a week so that the bug is fixed at least for 6.
>
Yeah, that's fine with me, thanks for getting it tested on another
BSP, and for following up.

If you can advise how the API change affects other BSPs on 5, I think
that can be accepted with a proper release note/documentation of what
those users may expect.

> Best regards
>
> Christian
>
> Am 27.01.22 um 13:25 schrieb Christian MAUDERER:
> > Hello,
> >
> > again: What can I do to make the patches acceptable?
> >
> > Best regards
> >
> > Christian
> >
> > Am 21.01.22 um 11:57 schrieb Christian MAUDERER:
> >> Ping.
> >>
> >> Am 18.01.22 um 12:22 schrieb Christian MAUDERER:
> >>> Hello,
> >>>
> >>> I noted that I still have this patch set open. I first posted it in
> >>> August 2021 and later pinged it in September 2021. Both times no
> >>> conclusion has been found. I would like to finally finish this topic
> >>> and get the patches in an acceptable state. To simplify it a bit,
> >>> let's only discuss the following two:
> >>>
> >>> * [PATCH rtems 2/2] termios: Pass number of sent chars to l_start
> >>>    https://lists.rtems.org/pipermail/devel/2021-August/068892.html
> >>>
> >>> * [PATCH rtems-libbsd] ppp: Fix transmitting data
> >>>    https://lists.rtems.org/pipermail/devel/2021-August/068893.html
> >>>
> >>> The third one (or rather first one) is a BSP specific optimization
> >>> and not necessary for the other two. I'll not push that together with
> >>> these two. I'll start a separate discussion for it as soon as we are
> >>> done with these two.
> >>>
> >>>  From my point of view, the best case would be to apply the patches
> >>> to master and 5. Back when I started the discussion I created tickets
> >>> for both:
> >>>
> >>>    https://devel.rtems.org/ticket/4493
> >>>    https://devel.rtems.org/ticket/4494
> >>>
> >>>
> >>> @Gedare: In the following mail thread you asked about tests on
> >>> further BSPs:
> >>>
> >>> https://lists.rtems.org/pipermail/devel/2021-August/068908.html
> >>>
> >>> I now finally tested the patches on an i.MXRT BSP. PPP works with the
> >>> patches and doesn't work without them on that target. That's what I
> >>> expected. The patches are tested on three serial drivers now:
> >>>
> >>> - ATSAM and i.MXRT: Don't work without patches. Work with them.
> >>> - SC16IS752: Works for small packets only without patches. Works well
> >>> with them.
> >>>
> >>>
> >>> @Chris: We had two open discussions:
> >>>
> >>> 1. A style topic:
> >>> https://lists.rtems.org/pipermail/devel/2021-August/068912.html
> >>>
> >>> Like said there, I would prefer not to reformat the whole file as
> >>> long as we don't do that at least a bit automatic for most files.
> >>>
> >>> 2. A discussion about the direction and the necessary API change:
> >>> https://lists.rtems.org/pipermail/devel/2021-September/069468.html
> >>>
> >>> Like I explained: That PPP bug exists basically on all BSPs.
> >>> Depending on how many bytes a serial driver can process in it's write
> >>> function, PPP works better or worse. My assumption is that at the
> >>> moment, the PPP runs only on BSPs where it is "well enough" that no
> >>> one noted the bug.
> >>>
> >>> To fix that bug, the PPP needs the information how many characters
> >>> have been sent out. My patch set uses our RTEMS default path via
> >>> rtems_termios_dequeue_characters().
> >>>
> >>> I'm not entirely happy that I had to change the API. But I didn't
> >>> find a better solution and the API is still compatible. It will only
> >>> trigger a warning if someone doesn't use that extra parameter. If you
> >>> have a better idea how the bug could be fixed, I'm open to adapt the
> >>> patches. But I need a suggestion what would be a better solution
> >>> because - like I said - I didn't find one.
> >>>
> >>> Best regards
> >>>
> >>> Christian
> >>>
> >>> Am 12.08.21 um 13:41 schrieb Christian Mauderer:
> >>>> Hello,
> >>>>
> >>>> this set of patches fixes PPP. Basically the current implementation in
> >>>> libbsd can't work with console drivers that can't buffer a lot of
> >>>> characters. The pppstart() function just assumes that the low level
> >>>> console write can send an arbitrary number of characters without
> >>>> checking how many characters are actually send.
> >>>>
> >>>> In the extreme case of the ATSAM interfaces (only one character can be
> >>>> send at once) it's not even possible to establish a PPP connection with
> >>>> that. For UARTS that have some FIFO establishing might work but bigger
> >>>> packets won't go through. I opened tickets for 6 and 5 here:
> >>>>
> >>>> https://devel.rtems.org/ticket/4493
> >>>> https://devel.rtems.org/ticket/4494
> >>>>
> >>>> I would like to apply the patches to both branches (5 and 6).
> >>>>
> >>>> The solution I implemented in this patch set is the following: PPP
> >>>> output processing is done in the line discipline function l_start (or
> >>>> rather the function where l_start points to: pppstart). Our device
> >>>> writes don't return how many characters have been sent. Instead, that
> >>>> feedback is given via rtems_termios_dequeue(). Luckily that's the
> >>>> function that calls the l_start function.
> >>>>
> >>>> The RTEMS patch for termios extends the l_start function so that it
> >>>> gets
> >>>> the number of characters as a second argument. This extension shouldn't
> >>>> be a problem for existing code. In the worst case it will trigger a
> >>>> warning that the function doesn't match the pointer type. But in the
> >>>> linked code, that additional argument will just be ignored.
> >>>>
> >>>> The libbsd patch extends the pppstart function so that it then handles
> >>>> that new information.
> >>>>
> >>>> Patches belonging to this set:
> >>>> * [PATCH rtems 1/2] bsps/atsam: Improve UART / USART tx performance
> >>>> * [PATCH rtems 2/2] termios: Pass number of sent chars to l_start
> >>>> * [PATCH rtems-libbsd] ppp: Fix transmitting data
> >>>>
> >>>> Best regards
> >>>>
> >>>> Christian
> >>>>
> >>>
> >>
> >
>
> --
> --------------------------------------------
> embedded brains GmbH
> Herr Christian MAUDERER
> Dornierstr. 4
> 82178 Puchheim
> Germany
> email: christian.mauderer at embedded-brains.de
> phone: +49-89-18 94 741 - 18
> fax:   +49-89-18 94 741 - 08
>
> Registergericht: Amtsgericht München
> Registernummer: HRB 157899
> Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
> Unsere Datenschutzerklärung finden Sie hier:
> https://embedded-brains.de/datenschutzerklaerung/