[PATCHES] Fix PPP in libbsd and optimize ATSAM console

Thu Jan 27 12:25:52 UTC 2022

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/