[PATCHES] Fix PPP in libbsd and optimize ATSAM console

Tue Jan 18 11:22:04 UTC 2022

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/