On Mon, Feb 7, 2022 at 9:05 AM Christian MAUDERER <christian.maude...@embedded-brains.de> wrote: > > Hello Gedare, > > Am 07.02.22 um 16:40 schrieb Gedare Bloom: > > On Wed, Feb 2, 2022 at 8:20 AM Christian MAUDERER > > <christian.maude...@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. > > Thanks for the response. > > > > > 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. > There are two relevant changes in cpukit/include/rtems/termiostypes.h: > > 1. I added a field to the struct rtems_termios_tty. With that the binary > interface changes. > This should be ok but needs to be noted. Without looking, the main concern would be potential memory corruption.
> 2. I added a parameter to one of the line discipline functions. > > - int (*l_start)(struct rtems_termios_tty *tp); > + int (*l_start)(struct rtems_termios_tty *tp,int len); > > If someone is using the line discipline functions that should trigger a > warning that the function prototype doesn't match. I think that an extra > parameter in a function pointer is just ignored otherwise. At least I > don't know of a case where it would have an effect. Please correct me if > I'm wrong. > Yes, it should just be a warning. There should be clear guidance about it in the release note. > Best regards > > Christian > > > > >> 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.maude...@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/ > > -- > -------------------------------------------- > embedded brains GmbH > Herr Christian MAUDERER > Dornierstr. 4 > 82178 Puchheim > Germany > email: christian.maude...@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/ _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel