[PATCHES] Fix PPP in libbsd and optimize ATSAM console

Wed Feb 2 15:20:32 UTC 2022

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.

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/