AW: [PATCH 02/18] ptpd: Add VERSION file

[Gabriel.Moyano]

> > ---
> >   freebsd/contrib/ptpd/VERSION | 11 +++
> >   1 file changed, 11 insertions(+)
> >   create mode 100644 freebsd/contrib/ptpd/VERSION
> >
> > diff --git a/freebsd/contrib/ptpd/VERSION
> > b/freebsd/contrib/ptpd/VERSION new file mode 100644 index
> > ..4c95e563
> > --- /dev/null
> > +++ b/freebsd/contrib/ptpd/VERSION
> > @@ -0,0 +1,11 @@
> > +Import from:
> > +
> > +https://github.com/ptpd/ptpd
> > +
> > +Commit:
> > +
> > +1ec9e650b03e6bd75dd3179fb5f09862ebdc54bf
> > +
> > +Date:
> > +
> > +Wed Sep 4 20:12:53 2019 -0400
> 
> The freebsd directory mirrors the FreeBSD source tree. Since the ptpd is not
> included in FreeBSD it should be placed outside the freebsd directory, for
> example ptpd at the top level or rtemsbsd/ptpd.
> 
Sure, I can move the files to rtemsbsd/ptpd. But before submitting the patches 
again, is there any other change suggested?
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: [PATCH 00/18] Port PTPd to rtems-libbsd

[Gabriel.Moyano]

> On 12/4/2023 11:54 pm, Gabriel Moyano wrote:
> > These commits are for porting PTPd to rtems-libbsd and are based on the
> master branch.
> >
> > This work is a joint effort with Chris Johns, which we started some time
> ago.
> > Originally, we wanted to port some of the commits to PTPd upstream but
> unfortunately the project is no longer maintained (our pull request has been
> open for more than a year).
> > For this reason, we decided to add PTPd files as contrib.
> > In fact, this is the first commit.
> >
> > One important feature introduced to PTPd is the support for kqueue,
> which was done by Chris.
> > Also, a new test for running a PTPd instance was added to the test suit.
> >
> > I'm happy to hear feedback from you.
> 
> Thank for you for all the work I know has gone into getting this package
> together. It has been a journey. It is great to see the work reach this 
> finished
> level ready to merge.

Thanks you too.

> Is this for the master, 6-freebsd-12 or both branches of libbsd?
> 

It is for the master, once approved I'll create the one for 6-freebsd-12. I 
understand that there will be some changes needed.
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: Project Discussion for GSoC 2023

[Gabriel.Moyano]

Hi Viraj,

Please find the code in this branch: 
https://github.com/GabrielDai/rtems-libbsd/tree/port-greth-to-5-freebsd-12

Since this driver depends on drvmgr, you’ll have to compile your bsp with 
support for it (make sure your bsp register the driver for your gaisler card 
using the drvmgr functions).
Moreover, it is needed to add the following line for your bsp in 
“nexus-devices.h” (rtems-libbsd):
SYSINIT_DRIVER_REFERENCE(grcard, pci);
This line will add a device (called grcard).
During booting (if the gaisler card get recognized) it will create child 
devices (greth#) for the ethernet interfaces.

I hope this helps you.

Have fun coding,
Gabriel

Von: Viraj Jagadale 
Gesendet: Dienstag, 21. Februar 2023 17:36
An: Moyano Heredia, Victor Gabriel ; dan...@gaisler.com; 
devel@rtems.org
Betreff: Re: Project Discussion for GSoC 2023

Hi Daniel and Gabriel,

Thank you for responding. I don't own an ARTY A7-100T board. As suggested, I 
will study the GRETH_GBIT manual at the same time. Can I refer to the DP83848 
or LAN8742A documentation for stm32, both of which support legacy stack and 
lwip? If not, could you please recommend a device?

Please share the driver for rtems-libbsd with me as it will be very useful as a 
starting point.

Regards,
Viraj.

On Tue, Feb 21, 2023 at 2:00 PM 
mailto:gabriel.moy...@dlr.de>> wrote:
Hi Viraj,

We ported the driver for greth to rtems-libbsd in the past but never got to 
submit it (it might need some further refinement). The driver depends on the 
driver manager (drvmgr). Not sure if this is the best approach but it is a good 
starting point and we’d be happy to share it.

Best regards,
Gabriel


Von: devel mailto:devel-boun...@rtems.org>> Im Auftrag 
von Daniel Hellstrom
Gesendet: Montag, 20. Februar 2023 15:59
An: Viraj Jagadale 
mailto:virajjagadale...@gmail.com>>; 
devel@rtems.org
Cc: kinsey.mo...@rtems.org; 
and...@chichak.ca
Betreff: Re: Project Discussion for GSoC 2023


Hi Viraj,

There is an old GRETH network driver as part of the old network stack that 
would be a good reference.

Note that the old driver supports two IPs (GRETH 10/100, and GRETH_GBIT 
10/100/1000). The GRETH_GBIT IP is mostly backwards compatible with the GRETH, 
but as some additional functionality to off load the CPU with UDP/TCP 
check-summing, unaligned DMA, and scatter-gather DMA for example. One approach 
could be to begin to focus on the more capable GRETH_GBIT IP first, section 14:

https://www.gaisler.com/doc/gr740/GR740-UM-DS-2-5.pdf

or from the IP manual, section XX:

https://www.gaisler.com/products/grlib/grip.pdf

Please keep in mind that the GRETH driver will be used by both SPARC/LEON3 BSP 
and RISC-V/NOEL-V BSP in the future.

It sounds as a good approach to look at the interface of the LWIP stack towards 
the Network Device Driver, for example the DEC driver to learn however the best 
would be if there is a MAC device supported both by legacy stack and the LWIP? 
Simultaneously you could study the GRETH_GBIT manual with register and DMA 
interface and the old device driver source code?

If you have a ARTY A7-100T board you could use the RISC-V design to get access 
to the GRETH IP easily get started with. The GRMON eval version would also work 
together with it for a hardware-debugger (no additional cost required) which 
you can connect GDB for source debugging if you wish:

https://www.gaisler.com/index.php/products/processors/noel-v-examples

https://www.gaisler.com/index.php/products/debug-tools/grmon3

Kind Regards,
Daniel






On 2023-02-13 04:25, Viraj Jagadale wrote:
Dear Community,
I am interested in contributing to RTEMS and will be participating in GSoC 
2023. I am interested in projects #4595 
and #4596 because I am passionate about 
networking. I'm currently concentrating solely on #4595. I'm aware that I'll 
need to study and comprehend the lwip stack documentation, as well as how the 
Ethernet protocol is implemented and networking services are provided for 
applications. Then I'll have to devise a strategy for designing the driver 
architecture. I'm thinking about reading the RTEMS Legacy Networking User 
Manual and understanding the DEC 21140 example to get started. I'm not sure if 
this is the right approach, and I'll need your advice. I am also willing to 
help with existing bugs and documentation updates.

Regards,
Viraj Jagadale.
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: Project Discussion for GSoC 2023

[Gabriel.Moyano]

Hi Viraj,

We ported the driver for greth to rtems-libbsd in the past but never got to 
submit it (it might need some further refinement). The driver depends on the 
driver manager (drvmgr). Not sure if this is the best approach but it is a good 
starting point and we’d be happy to share it.

Best regards,
Gabriel


Von: devel  Im Auftrag von Daniel Hellstrom
Gesendet: Montag, 20. Februar 2023 15:59
An: Viraj Jagadale ; devel@rtems.org
Cc: kinsey.mo...@rtems.org; and...@chichak.ca
Betreff: Re: Project Discussion for GSoC 2023


Hi Viraj,

There is an old GRETH network driver as part of the old network stack that 
would be a good reference.

Note that the old driver supports two IPs (GRETH 10/100, and GRETH_GBIT 
10/100/1000). The GRETH_GBIT IP is mostly backwards compatible with the GRETH, 
but as some additional functionality to off load the CPU with UDP/TCP 
check-summing, unaligned DMA, and scatter-gather DMA for example. One approach 
could be to begin to focus on the more capable GRETH_GBIT IP first, section 14:

https://www.gaisler.com/doc/gr740/GR740-UM-DS-2-5.pdf

or from the IP manual, section XX:

https://www.gaisler.com/products/grlib/grip.pdf

Please keep in mind that the GRETH driver will be used by both SPARC/LEON3 BSP 
and RISC-V/NOEL-V BSP in the future.

It sounds as a good approach to look at the interface of the LWIP stack towards 
the Network Device Driver, for example the DEC driver to learn however the best 
would be if there is a MAC device supported both by legacy stack and the LWIP? 
Simultaneously you could study the GRETH_GBIT manual with register and DMA 
interface and the old device driver source code?

If you have a ARTY A7-100T board you could use the RISC-V design to get access 
to the GRETH IP easily get started with. The GRMON eval version would also work 
together with it for a hardware-debugger (no additional cost required) which 
you can connect GDB for source debugging if you wish:

https://www.gaisler.com/index.php/products/processors/noel-v-examples

https://www.gaisler.com/index.php/products/debug-tools/grmon3

Kind Regards,
Daniel






On 2023-02-13 04:25, Viraj Jagadale wrote:
Dear Community,
I am interested in contributing to RTEMS and will be participating in GSoC 
2023. I am interested in projects #4595 
and #4596 because I am passionate about 
networking. I'm currently concentrating solely on #4595. I'm aware that I'll 
need to study and comprehend the lwip stack documentation, as well as how the 
Ethernet protocol is implemented and networking services are provided for 
applications. Then I'll have to devise a strategy for designing the driver 
architecture. I'm thinking about reading the RTEMS Legacy Networking User 
Manual and understanding the DEC 21140 example to get started. I'm not sure if 
this is the right approach, and I'll need your advice. I am also willing to 
help with existing bugs and documentation updates.

Regards,
Viraj Jagadale.
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: [PATCH] sppps01: Add test case for early returns of pps_event()

[Gabriel.Moyano]

Hi Kinsey,

Sorry for the late response, I wasn't available.

The patch added a new test case (PPSEventEarlyReturns) for increasing the test 
coverage. The test case checks whether the function pps_event() (in file 
kern_tc.c) returns early. According to the result you shared, the test is 
failing in lines 115 and 119, which means that there is no early return of the 
function pps_event() due to saving the timecounter in the pps_state object 
(line 2225 of kern_tc.c). My guess is that saving the timecounter is happening 
in the previous call of pps_event() (in line 109 of the test case). 
Unfortunately I cannot understand why this is failing in intermittently manner 
(apparently just in QEMU).

I hope this helps you to understand/debug.

Best regards,
Gabriel

> This change seems to have made the test intermittently failing on
> AArch64 under QEMU:
> 
> *** BEGIN OF TEST SPPPS 1 ***
> *** TEST VERSION: 6.0.0.4142fd12385473426b461560b76cee6e903dc2cd
> *** TEST STATE: EXPECTED_PASS
> *** TEST BUILD: RTEMS_POSIX_API RTEMS_SMP
> *** TEST TOOLS: 12.1.1 20220622 (RTEMS 6, RSB 
> a4608ce16e94066aea82f22849e24d4fcd90699b, Newlib 27fd806) A:SPPPS 1
> S:Platform:RTEMS
> S:Compiler:12.1.1 20220622 (RTEMS 6, RSB 
> a4608ce16e94066aea82f22849e24d4fcd90699b, Newlib 27fd806)
> S:Version:6.0.0.4142fd12385473426b461560b76cee6e903dc2cd
> S:BSP:xilinx_zynqmp_lp64_qemu
> S:BuildLabel:DEFAULT
> S:TargetHash:SHA256:huGRj7kHeBWgSJolmeboNSyCgeX2U62J9RiNQ18YPFc=
> S:RTEMS_DEBUG:0
> S:RTEMS_MULTIPROCESSING:0
> S:RTEMS_POSIX_API:1
> S:RTEMS_PROFILING:0
> S:RTEMS_SMP:1
> B:WakeupTaskWithPPSEvent
> P:0:0:UI1:init.c:195
> P:1:0:UI1:init.c:209
> P:2:0:UI1:init.c:211
> P:3:0:PPSE:init.c:160
> P:4:0:UI1:init.c:214
> P:5:0:PPSE:init.c:162
> P:6:0:PPSE:init.c:164
> P:7:0:UI1:init.c:221
> E:WakeupTaskWithPPSEvent:N:8:F:0:D:0.008009
> B:WaitPPSEventDefaultHandler
> P:0:0:UI1:init.c:73
> P:1:0:UI1:init.c:77
> E:WaitPPSEventDefaultHandler:N:2:F:0:D:0.000508
> B:PPSEventEarlyReturns
> P:0:0:UI1:init.c:103
> P:1:0:UI1:init.c:110
> F:2:0:UI1:init.c:115:1 == 0
> F:3:0:UI1:init.c:119:2 == 1
> E:PPSEventEarlyReturns:N:4:F:2:D:0.001525
> Z:SPPPS 1:C:3:N:14:F:2:D:0.013109
> Y:ReportHash:SHA256:B-CITRKa5X9NJy8JfEPbCr_F_tm6UNBuwfKzHcxhWks=
> 
> [ RTEMS shutdown ]
> CPU: 0
> RTEMS version: 6.0.0.4142fd12385473426b461560b76cee6e903dc2cd
> RTEMS tools: 12.1.1 20220622 (RTEMS 6, RSB 
> a4608ce16e94066aea82f22849e24d4fcd90699b, Newlib 27fd806) executing thread ID:
> 0x08a010001 executing thread name: UI1
> 
> 
> It appears to work consistently on hardware (at least for a sample size
> of around 10 test runs) and before this patch it worked consistently on
> QEMU as well.
> 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

One test more for PPS API

[Gabriel.Moyano]

Hi Sebastian,

I added this test https://lists.rtems.org/pipermail/devel/2022-June/071926.html 
some time ago. Could you give me your feedback?

Thanks in advance,
Gabriel


___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Porting PPS API support to RTEMS 5

[Gabriel.Moyano]

Hello everyone,

I ported the PPS API support from RTEMS 6 to RTEMS 5 and wanted to ask if 
somebody is interest to have the changes in the upstream. If yes, I can submit 
the patches.

Best regards,
Gabriel
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: [PATCH] kern_tc.c: Update pps_event() for uniprocessor configurations

[Gabriel.Moyano]

> On 10.06.22 09:20, Gabriel Moyano wrote:
> > Since pps->capgen is not used in uniprocessor configurations, there is
> > no need to verified if it is equal to zero
> 
> The pps->capgen is used in uniprocessor configurations. The difference to the 
> SMP configuration is that zero is not a special value.
> 
Thx, I'll improve this description in the next patch version
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: AW: AW: AW: [PATCH] kern_tc.c: Update pps_event() for uniprocessor configurations

[Gabriel.Moyano]

> On 08.06.22 09:54, gabriel.moy...@dlr.de wrote:
> >> I don't know why there is this "if" in the code. I will ask on a 
> >> FreeBSD mailing list.
> >>
> > I think it is for the case that th_generation has changed in
> > between saving the th and th_counter. If this happens pps->capgen
> > is set to
> > 0 and later pps_event() returns earlier. Since for uniprocessor
> > th_generation equal to 0 is not used, I guess we can removed this
> > if for those configurations
>  I asked on a FreeBSD mailing list:
> 
>  https://lists.freebsd.org/archives/freebsd-hackers/2022-June/001165
>  .h
>  tml
> 
> >>> Thanks for asking.
> >>> I'll prepare and send a new patch removing the "if" for uniprocessor 
> >>> configurations just in case.
> >> Please wait with a new patch for a response from FreeBSD.
> >>
> > What is your suggestion here? Should we check the generation only once? Or 
> > should we leave the code as is and just remove the
> "if" in pps_capture() for uniprocessor configurations since th_generation 
> equal to zero is not used?
> 
> We should first leave the code as is. I don't know when I have time to send 
> patches to FreeBSD.
> 

I would like it to be considered to remove the parts where th_generation is 
checked to be equal to zero just for uniprocessor configurations.
The reason is that porting back these changes to RTEMS 5, the test sppps01 
fails because th_generation starts with value zero. Not sure why in RTEMS 6 
th_generation starts with one but since in uniprocessor configuration 
th_generation equal zero is not used, I think it makes sense to not consider 
that case.
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: AW: AW: [PATCH] kern_tc.c: Update pps_event() for uniprocessor configurations

[Gabriel.Moyano]

>  I don't know why there is this "if" in the code. I will ask on a FreeBSD 
>  mailing list.
> 
> >>> I think it is for the case that th_generation has changed in between
> >>> saving the th and th_counter. If this happens pps->capgen is set to
> >>> 0 and later pps_event() returns earlier. Since for uniprocessor
> >>> th_generation equal to 0 is not used, I guess we can removed this if
> >>> for those configurations
> >> I asked on a FreeBSD mailing list:
> >>
> >> https://lists.freebsd.org/archives/freebsd-hackers/2022-June/001165.h
> >> tml
> >>
> > Thanks for asking.
> > I'll prepare and send a new patch removing the "if" for uniprocessor 
> > configurations just in case.
> 
> Please wait with a new patch for a response from FreeBSD.
> 
What is your suggestion here? Should we check the generation only once? Or 
should we leave the code as is and just remove the "if" in pps_capture() for 
uniprocessor configurations since th_generation equal to zero is not used?
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: AW: [PATCH] kern_tc.c: Update pps_event() for uniprocessor configurations

[Gabriel.Moyano]

> On 01/06/2022 08:55, gabriel.moy...@dlr.de wrote:
> >> On 30/05/2022 09:29, Gabriel Moyano wrote:
> >>> Since pps->capgen is not used in uniprocessor configurations, there
> >>> is no need to verified if it is equal to zero
> >>>
> >>> Update #2349
> >>> ---
> >>>cpukit/score/src/kern_tc.c | 4 
> >>>1 file changed, 4 insertions(+)
> >>>
> >>> diff --git a/cpukit/score/src/kern_tc.c b/cpukit/score/src/kern_tc.c
> >>> index 92739d8edd..897f81511e 100644
> >>> --- a/cpukit/score/src/kern_tc.c
> >>> +++ b/cpukit/score/src/kern_tc.c
> >>> @@ -2165,7 +2165,11 @@ pps_event(struct pps_state *pps, int event)
> >>>   if ((event & pps->ppsparam.mode) == 0)
> >>>   return;
> >>>   /* If the timecounter was wound up underneath us, bail out. */
> >>> +#if defined(RTEMS_SMP)
> >>>   if (pps->capgen == 0 || pps->capgen !=
> >>> +#else
> >>> + if (pps->capgen !=
> >>> +#endif
> >>>   atomic_load_acq_int(>capth->th_generation))
> >>>   return;
> >>>
> >>
> >> I think this fix is incomplete. What is with:
> >>
> >> void
> >> pps_capture(struct pps_state *pps)
> >> {
> >>struct timehands *th;
> >>
> >>KASSERT(pps != NULL, ("NULL pps pointer in pps_capture"));
> >>th = timehands;
> >>pps->capgen = atomic_load_acq_int(>th_generation);
> >>pps->capth = th;
> >> #ifdef FFCLOCK
> >>pps->capffth = fftimehands;
> >> #endif
> >>pps->capcount = th->th_counter->tc_get_timecount(th->th_counter);
> >>atomic_thread_fence_acq();
> >>if (pps->capgen != th->th_generation)
> >>pps->capgen = 0;
> >> }
> >>
> >> I don't know why there is this "if" in the code. I will ask on a FreeBSD 
> >> mailing list.
> >>
> >
> > I think it is for the case that th_generation has changed in between
> > saving the th and th_counter. If this happens pps->capgen is set to 0
> > and later pps_event() returns earlier. Since for uniprocessor
> > th_generation equal to 0 is not used, I guess we can removed this if
> > for those configurations
> 
> I asked on a FreeBSD mailing list:
> 
> https://lists.freebsd.org/archives/freebsd-hackers/2022-June/001165.html
> 

Thanks for asking.
I'll prepare and send a new patch removing the "if" for uniprocessor 
configurations just in case.
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: [PATCH] kern_tc.c: Update pps_event() for uniprocessor configurations

[Gabriel.Moyano]

> On 30/05/2022 09:29, Gabriel Moyano wrote:
> > Since pps->capgen is not used in uniprocessor configurations, there is
> > no need to verified if it is equal to zero
> >
> > Update #2349
> > ---
> >   cpukit/score/src/kern_tc.c | 4 
> >   1 file changed, 4 insertions(+)
> >
> > diff --git a/cpukit/score/src/kern_tc.c b/cpukit/score/src/kern_tc.c
> > index 92739d8edd..897f81511e 100644
> > --- a/cpukit/score/src/kern_tc.c
> > +++ b/cpukit/score/src/kern_tc.c
> > @@ -2165,7 +2165,11 @@ pps_event(struct pps_state *pps, int event)
> > if ((event & pps->ppsparam.mode) == 0)
> > return;
> > /* If the timecounter was wound up underneath us, bail out. */
> > +#if defined(RTEMS_SMP)
> > if (pps->capgen == 0 || pps->capgen !=
> > +#else
> > +   if (pps->capgen !=
> > +#endif
> > atomic_load_acq_int(>capth->th_generation))
> > return;
> >
> 
> I think this fix is incomplete. What is with:
> 
> void
> pps_capture(struct pps_state *pps)
> {
>   struct timehands *th;
> 
>   KASSERT(pps != NULL, ("NULL pps pointer in pps_capture"));
>   th = timehands;
>   pps->capgen = atomic_load_acq_int(>th_generation);
>   pps->capth = th;
> #ifdef FFCLOCK
>   pps->capffth = fftimehands;
> #endif
>   pps->capcount = th->th_counter->tc_get_timecount(th->th_counter);
>   atomic_thread_fence_acq();
>   if (pps->capgen != th->th_generation)
>   pps->capgen = 0;
> }
> 
> I don't know why there is this "if" in the code. I will ask on a FreeBSD 
> mailing list.
> 

I think it is for the case that th_generation has changed in between saving the 
th and th_counter. If this happens pps->capgen is set to 0 and later 
pps_event() returns earlier. Since for uniprocessor th_generation equal to 0 is 
not used, I guess we can removed this if for those configurations
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: AW: [PATCH 1/1] kern_tc.c: th_generation starts with 1 after overflow for single-core

[Gabriel.Moyano]

> On 27.05.22 11:49, gabriel.moy...@dlr.de wrote:
> >> On 27.05.22 10:51, Sebastian Huber wrote:
> >>> Hello Gabriel,
> >>>
> >>> the uniprocessor version uses an optimization at the reader side:
> >>>
> >>> #if defined(RTEMS_SMP)
> >>>   } while (gen == 0 || gen != th->th_generation); #else
> >>>   } while (gen != th->th_generation); #endif
> >>>
> >>> This is possible since the windup happens with interrupts disabled.
> >>> I guess you need this optimization somewhere in the PPS/NTP code.
> > Yes, you are right that is for the PPS code.
> > The value of th_generation is saved in pps_capture() and I shouldn't add a 
> > while waiting it to be different that 0 there.
> > If its value is 0, then the pps_event() returns early.
> > This is something that could happen in very particular circumstance (pps 
> > event happens when the th_generation is 0).
> 
> In uniprocessor configurations, we don't need the 0 special value. It is only 
> required in SMP configurations since one processor may
> observe a timecounter update which is in progress on another processor.

Being that case, the easiest solution will be to not check if pps->capgen == 0  
in pps_event() for uniprocessors configurations.

/* If the timecounter was wound up underneath us, bail out. */
#if defined(RTEMS_SMP)
if (pps->capgen == 0 || pps->capgen !=
#else
if (pps->capgen !=
#endif
atomic_load_acq_int(>capth->th_generation))
return;

what do you think?

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: [PATCH 1/1] kern_tc.c: th_generation starts with 1 after overflow for single-core

[Gabriel.Moyano]

> On 27.05.22 10:51, Sebastian Huber wrote:
> > Hello Gabriel,
> >
> > the uniprocessor version uses an optimization at the reader side:
> >
> > #if defined(RTEMS_SMP)
> >  } while (gen == 0 || gen != th->th_generation); #else
> >  } while (gen != th->th_generation); #endif
> >
> > This is possible since the windup happens with interrupts disabled. I
> > guess you need this optimization somewhere in the PPS/NTP code.

Yes, you are right that is for the PPS code.
The value of th_generation is saved in pps_capture() and I shouldn't add a 
while waiting it to be different that 0 there.
If its value is 0, then the pps_event() returns early.
This is something that could happen in very particular circumstance (pps event 
happens when the th_generation is 0).

> > Could you please add the details to the commit message and not the
> > cover letter. The cover letter is not committed.

Sure.

> > Could you please have a look at:
> >
> > https://lists.rtems.org/pipermail/devel/2022-May/071609.html
> 
> It would be also good to have a test case for the problem you are trying to 
> fix. The PPS/NTP code is a bit complicated and the test
> coverage of this area is far below the score average.
> 

One way to test this is waiting to the overflow of th_generation, do you think 
that it is feasibly to add such a test if its duration is a long time?

Btw. Could you check this 
https://lists.rtems.org/pipermail/devel/2022-May/071662.html ?
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: [PATCH] score: Fix pps_fetch()

[Gabriel.Moyano]

Hi Sebastian,

It is ok. Thx for adding this, I must have removed this "if" accidentally. 

> Return early only if there was a timeout, otherwise return the PPS info.
> 
> Update #2349.
> ---
>  cpukit/score/src/kern_tc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/cpukit/score/src/kern_tc.c b/cpukit/score/src/kern_tc.c index 
> c09900f096..2e3709a9ad 100644
> --- a/cpukit/score/src/kern_tc.c
> +++ b/cpukit/score/src/kern_tc.c
> @@ -1980,7 +1980,8 @@ pps_fetch(struct pps_fetch_args *fapi, struct pps_state 
> *pps)  #else /* __rtems__ */
>   _Assert(pps->wait != NULL);
>   err = (*pps->wait)(pps, fapi->timeout);
> - return (err);
> + if (err != 0)
> + return (err);
>  #endif /* __rtems__ */
>   }
>   }
> --
> 2.35.3

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: [PATCH v3 04/11] kern_tc.c: Replace FREEBSD event mechanism by adding pointers to function

[Gabriel.Moyano]

Thx, I'll send a new version of the commits today. The issue in the NTP test 
will also be resolved.

> FREEBSD should be FreeBSD.
> 
> On 04/05/2022 14:12, Gabriel Moyano wrote:
> > ---
> >   cpukit/include/sys/timepps.h | 21 
> >   cpukit/score/src/kern_tc.c   | 38 
> >   2 files changed, 59 insertions(+)
> >
> > diff --git a/cpukit/include/sys/timepps.h
> > b/cpukit/include/sys/timepps.h index 5703381ffa..b734c6f841 100644
> > --- a/cpukit/include/sys/timepps.h
> > +++ b/cpukit/include/sys/timepps.h
> > @@ -164,6 +164,27 @@ struct pps_state {
> > int ppscap;
> > struct timecounter *ppstc;
> > unsignedppscount[3];
> > +#ifdef __rtems__
> > +/**
> > + * @brief  Wait for an event.
> > + *
> > + * Called internally when time_pps_fetch() is used.
> > + * It is initialized with an empty function by default.
> 
> initialized by pps_init() to a handler which just returns 0?
> 
> Is 0 a good default? You will end up in an endless loop I guess. Why not 
> return ETIMEDOUT by default? Please add a test case for this
> scenario.
> 
> > + *
> > + * @param pps is the pointer to the object.
> > + *
> > + * @param timeout
> > + *
> > + * @return 0 if no error otherwise a negative value.
> 
> Please list all valid status codes. Probably:
> 
> @retval 0 A wakeup event was received.
> 
> @retval ETIMEDOUT A timeout occurred while waiting for the event.
> 
> > + */
> > +int (*wait)(struct pps_state *pps, struct timespec timeout);
> 
> Add a blank line here.
> 
> Please review the entire patch set so that all changes are done in the coding 
> style of the original file.  For the details see:
> 
> https://www.freebsd.org/cgi/man.cgi?query=style=9
> 
> In your patches this is mostly space vs. tabs.
> 
> > +/**
> > + * @brief Wakeup the tasks waiting for an event.
> > + *
> > + * @param pps is the pointer to the object.
> > + */
> > +void (*wakeup)(struct pps_state *pps); #endif /* __rtems__ */
> > /*
> >  * The following fields are valid if the driver calls pps_init_abi().
> >  */
> > diff --git a/cpukit/score/src/kern_tc.c b/cpukit/score/src/kern_tc.c
> > index f7d0a0b4ba..6ca408e4ab 100644
> > --- a/cpukit/score/src/kern_tc.c
> > +++ b/cpukit/score/src/kern_tc.c
> > @@ -1917,9 +1917,15 @@ abi_aware(struct pps_state *pps, int vers)
> >   static int
> >   pps_fetch(struct pps_fetch_args *fapi, struct pps_state *pps)
> >   {
> > +#ifndef __rtems__
> > int err, timo;
> > +#else /* __rtems__ */
> > +   int err;
> > +#endif /* __rtems__ */
> > pps_seq_t aseq, cseq;
> > +#ifndef __rtems__
> > struct timeval tv;
> > +#endif /* __rtems__ */
> >
> > if (fapi->tsformat && fapi->tsformat != PPS_TSFMT_TSPEC)
> > return (EINVAL);
> > @@ -1932,6 +1938,7 @@ pps_fetch(struct pps_fetch_args *fapi, struct 
> > pps_state *pps)
> >  * sleep a long time.
> >  */
> > if (fapi->timeout.tv_sec || fapi->timeout.tv_nsec) {
> > +#ifndef __rtems__
> > if (fapi->timeout.tv_sec == -1)
> > timo = 0x7fff;
> > else {
> > @@ -1939,10 +1946,12 @@ pps_fetch(struct pps_fetch_args *fapi, struct 
> > pps_state *pps)
> > tv.tv_usec = fapi->timeout.tv_nsec / 1000;
> > timo = tvtohz();
> > }
> > +#endif /* __rtems__ */
> > aseq = atomic_load_int(>ppsinfo.assert_sequence);
> > cseq = atomic_load_int(>ppsinfo.clear_sequence);
> > while (aseq == atomic_load_int(>ppsinfo.assert_sequence) &&
> > cseq == atomic_load_int(>ppsinfo.clear_sequence)) {
> > +#ifndef __rtems__
> > if (abi_aware(pps, 1) && pps->driver_mtx != NULL) {
> > if (pps->flags & PPSFLAG_MTX_SPIN) {
> > err = msleep_spin(pps, pps->driver_mtx, 
> > @@ -1954,6 +1963,10 @@
> > pps_fetch(struct pps_fetch_args *fapi, struct pps_state *pps)
> > } else {
> > err = tsleep(pps, PCATCH, "ppsfch", timo);
> > }
> > +#else /* __rtems__ */
> > +_Assert(pps->wait != NULL);
> > +err = (*pps->wait)(pps, fapi->timeout); #endif /*
> > +__rtems__ */
> > if (err == EWOULDBLOCK) {
> > if (fapi->timeout.tv_sec == -1) {
> > continue;
> 
> I would remove the err == EWOULDBLOCK ... block if you remove the block 
> above, since the wait handler has to deal with the
> timeout.tv_sec == -1 condition in this case.
> 
> > @@ -2058,9 +2071,29 @@ pps_ioctl(u_long cmd, caddr_t data, struct pps_state 
> > *pps)
> > }
> >   }
> >
> > +#ifdef __rtems__
> > +static int
> > +default_wait(struct pps_state *pps, struct timespec timeout) {
> 
> Add a blank line.
> 
> > +(void) pps;
> 
> (void)pps;
> 
> See FreeBSD style 

AW: [PATCH v3 00/11] ENABLE PPS API in RTEMS6

[Gabriel.Moyano]

> Hello Gabriel,
> 
> I get a test failure in spntp01 now using the sparc/gr740 and 
> arm/xilinx_zynq_a9_qemu BSP:
> 
> F:23:0:UI1:init.c:84:2 == 0
> 
> Did you observe this test failure also?
> 
Yes,  thanks for noticing it. It is due the changes in [PATCH v3 09/11]. The 
default value for pps_shift is PPS_FAVG which is equals to 2.
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: [PATCH v3 00/11] ENABLE PPS API in RTEMS6

[Gabriel.Moyano]

> This is the 3rd version of the patches for enabling the PPS API in RTEMS6.
> It contains the changes suggested by Sebastian from yesterday (03/05/2022).
> 
> Gabriel Moyano (11):
>   kern_ntptime.c: Disable freebsd features
>   kern_ntptime.c: Add lmax() qmin() definitions
>   kern_tc.c: Add atomic dependencies required by the PPS API
>   kern_tc.c: Replace FREEBSD event mechanism by adding pointers to
> function
>   timecounter.h: Rename tc_getfrequency() to
> _Timecounter_Get_frequency()
>   kern_tc.c: Add definitions required by PPS API
>   kern_tc.c: Enable PPS API support
>   kern_ntptime.c: Add define in order to remove warning
>   timepps.h: PPS_SYNC defined by default
>   timecounter.h: Add _Timecounter_Discipline()
>   testsuites/sptests: Add sppps01 test
> 
>  cpukit/include/rtems/score/timecounter.h  |  25 
>  cpukit/include/sys/timepps.h  |  32 
>  cpukit/include/sys/timetc.h   |   3 +
>  cpukit/score/src/kern_ntptime.c   |  19 +--
>  cpukit/score/src/kern_tc.c|  54 ++-
>  spec/build/testsuites/sptests/grp.yml |   2 +
>  spec/build/testsuites/sptests/sppps01.yml |  19 +++
>  testsuites/sptests/sppps01/init.c | 173 ++
>  8 files changed, 312 insertions(+), 15 deletions(-)  create mode 100644 
> spec/build/testsuites/sptests/sppps01.yml
>  create mode 100644 testsuites/sptests/sppps01/init.c

Is there any feedback for improvement? Or are these commits ready to be pushed?
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: [PATCH v2 12/12] testsuites/sptests: Add sppps01 test

[Gabriel.Moyano]

> On 29/04/2022 09:20, Gabriel Moyano wrote:
> > +- Copyright (C) 2022 German Aerospace Center (DLR)
> 
> Is this a legal entity or shouldn't this be "Deutsche Zentrum für Luft- und 
> Raumfahrt e. V. (DLR)"?
> 
I would have also preferred that one but I asked Jan (in CC) about it and the 
one in English is the official one.
Moreover, there are already some files using the English version.
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: [PATCH v2 11/12] timepps.h: Move hardpps to the _ namespace

[Gabriel.Moyano]

> On 29/04/2022 09:20, Gabriel Moyano wrote:
> > ---
> >   cpukit/include/sys/timepps.h | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/cpukit/include/sys/timepps.h
> > b/cpukit/include/sys/timepps.h index b13ac6bf26..25370f3f4b 100644
> > --- a/cpukit/include/sys/timepps.h
> > +++ b/cpukit/include/sys/timepps.h
> > @@ -28,6 +28,7 @@
> >   #include 
> >   #include 
> >   #define PPS_SYNC
> > +#define hardpps _PPS_Discipline_cpu_clock
> >   #endif /* __rtems__ */
> >
> >   #define PPS_API_VERS_11
> 
> This function needs Doxygen documentation somewhere. What is the "cpu_clock"? 
> I would place it into the _Timecounter
> namespace.
> 
I was wondering if it makes sense to place it into _Timecounter.
There is a very good description where hardpps() is defined, would you like to 
copy it?
Just for clarification, the name "cpu_clock" comes from that documentation.
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: [PATCH v2 05/12] kern_tc.c: Replace FREEBSD event mechanism by adding pointers to function

[Gabriel.Moyano]

> On 29/04/2022 09:20, Gabriel Moyano wrote:
> > ---
> >   cpukit/include/sys/timepps.h |  6 ++
> >   cpukit/score/src/kern_tc.c   | 25 +
> >   2 files changed, 31 insertions(+)
> >
> > diff --git a/cpukit/include/sys/timepps.h
> > b/cpukit/include/sys/timepps.h index 5703381ffa..0d666a4f2e 100644
> > --- a/cpukit/include/sys/timepps.h
> > +++ b/cpukit/include/sys/timepps.h
> > @@ -26,6 +26,7 @@
> >   #include 
> >   #ifdef __rtems__
> >   #include 
> > +#include 
> >   #endif /* __rtems__ */
> >
> >   #define PPS_API_VERS_11
> > @@ -164,6 +165,11 @@ struct pps_state {
> > int ppscap;
> > struct timecounter *ppstc;
> > unsignedppscount[3];
> > +#ifdef __rtems__
> > +rtems_id task_waiting;
> 
> You don't need task_waiting, this is an implementation detail of potential 
> wait/wakeup implementations.

What do you think here of replacing it with a void* because some variable is 
required to save the tasks waiting

> > +int (*wait)(struct pps_state *pps, struct timespec timeout); /* Wait 
> > for an event. Called internally when time_pps_fetch() is
> used. It shall not be NULL*/
> > +void (*wakeup)(struct pps_state *pps); /* Used to wakeup tasks
> > + waiting for an event. It shall not be NULL*/
> 
> Please fix the formatting and take care that you keep the line limit.
> 
> > +#endif /* __rtems__ */
> > /*
> >  * The following fields are valid if the driver calls pps_init_abi().
> >  */
> > diff --git a/cpukit/score/src/kern_tc.c b/cpukit/score/src/kern_tc.c
> > index f7d0a0b4ba..76e3e056de 100644
> > --- a/cpukit/score/src/kern_tc.c
> > +++ b/cpukit/score/src/kern_tc.c
> > @@ -1917,9 +1917,15 @@ abi_aware(struct pps_state *pps, int vers)
> >   static int
> >   pps_fetch(struct pps_fetch_args *fapi, struct pps_state *pps)
> >   {
> > +#ifndef __rtems__
> > int err, timo;
> > +#else /* __rtems__ */
> > +   int err;
> > +#endif /* __rtems__ */
> > pps_seq_t aseq, cseq;
> > +#ifndef __rtems__
> > struct timeval tv;
> > +#endif /* __rtems__ */
> >
> > if (fapi->tsformat && fapi->tsformat != PPS_TSFMT_TSPEC)
> > return (EINVAL);
> > @@ -1932,6 +1938,7 @@ pps_fetch(struct pps_fetch_args *fapi, struct 
> > pps_state *pps)
> >  * sleep a long time.
> >  */
> > if (fapi->timeout.tv_sec || fapi->timeout.tv_nsec) {
> > +#ifndef __rtems__
> > if (fapi->timeout.tv_sec == -1)
> > timo = 0x7fff;
> > else {
> > @@ -1939,10 +1946,12 @@ pps_fetch(struct pps_fetch_args *fapi, struct 
> > pps_state *pps)
> > tv.tv_usec = fapi->timeout.tv_nsec / 1000;
> > timo = tvtohz();
> > }
> > +#endif /* __rtems__ */
> > aseq = atomic_load_int(>ppsinfo.assert_sequence);
> > cseq = atomic_load_int(>ppsinfo.clear_sequence);
> > while (aseq == atomic_load_int(>ppsinfo.assert_sequence) &&
> > cseq == atomic_load_int(>ppsinfo.clear_sequence)) {
> > +#ifndef __rtems__
> > if (abi_aware(pps, 1) && pps->driver_mtx != NULL) {
> > if (pps->flags & PPSFLAG_MTX_SPIN) {
> > err = msleep_spin(pps, pps->driver_mtx, 
> > @@ -1954,6 +1963,12 @@
> > pps_fetch(struct pps_fetch_args *fapi, struct pps_state *pps)
> > } else {
> > err = tsleep(pps, PCATCH, "ppsfch", timo);
> > }
> > +#else /* __rtems__ */
> > +if (pps->wait != NULL)
> > +err = (*pps->wait)(pps, fapi->timeout);
> > +else
> > +err = EAGAIN;
> > +#endif /* __rtems__ */
> 
> Replace the NULL check with an _Assert().  Check in pps_init() that the 
> handler are not NULL, otherwise generate a new fatal error.
> You need tests for the fatal errors.

Actually the synchronization can work without these functions; wait() is called 
in pps_time_fetch() and wakeup() wakes up the waiting task(s). This is also why 
there is one _Assert() in pps_init() but if you consider better to add other 
for wait(), it can be done.
 
> > if (err == EWOULDBLOCK) {
> > if (fapi->timeout.tv_sec == -1) {
> > continue;
> > @@ -2061,6 +2076,11 @@ pps_ioctl(u_long cmd, caddr_t data, struct pps_state 
> > *pps)
> >   void
> >   pps_init(struct pps_state *pps)
> >   {
> > +#ifdef __rtems__
> > +if (pps->wait != NULL)
> > +_Assert(pps->wakeup != NULL);
> > +pps->task_waiting = RTEMS_INVALID_ID; #endif /* __rtems__ */
> > pps->ppscap |= PPS_TSFMT_TSPEC | PPS_CANWAIT;
> > if (pps->ppscap & PPS_CAPTUREASSERT)
> > pps->ppscap |= PPS_OFFSETASSERT;
> > @@ -2227,7 +2247,12 @@ pps_event(struct pps_state *pps, int event)
> >   #endif
> >
> > /* Wakeup anyone sleeping in pps_fetch().  */
> > +#ifndef __rtems__
> > wakeup(pps);
> > +#else 

AW: AW: AW: AW: [PATCH 07/12] kern_tc.c: Remove verification of th_generation for pps functions

[Gabriel.Moyano]

> On 27.04.22 15:13, gabriel.moy...@dlr.de wrote:
> >> On 07/04/2022 15:16, Sebastian Huber wrote:
> >>> On 07/04/2022 15:10, gabriel.moy...@dlr.de wrote:
> > Which sequence of function calls and timings cases the problem?
> > This should be definitely a test case.
>  The generation is updated every time tc_windup() is called. So it
>  is more or less a race condition when it happens.
> >>>
> >>> This is not a race condition. The sequence counter is supposed to
> >>> ensure a consistent state. You can't simply remove this.
> >>>
> >>> Maybe the PPS support requires that we use two timehands even for
> >>> the uniprocessor configuration. For this, we have to understand
> >>> under which conditions and use cases the generation change is an issue.
> >>
> >> In kern_tc.c there is this comment, which is a result from a discussion on 
> >> a FreeBSD mailing list:
> >>
> >> /*
> >>* In FreeBSD, the timehands count is a tuning option from two to 16.  
> >> The
> >>* tuning option was added since it is inexpensive and some FreeBSD 
> >> users asked
> >>* for it to play around.  The default value is two.  One system which 
> >> did not
> >>* work with two timehands was a system with one processor and a 
> >> specific PPS
> >>* device.
> >>*
> >>* For RTEMS, in uniprocessor configurations, just use one timehand 
> >> since the
> >>* update is done with interrupt disabled.
> >>*
> >>* In SMP configurations, use a fixed set of two timehands until someone
> >>* reports an issue.
> >>*/
> >>
> >> We definitely need a test case for this corner case. Adding a second
> >> timehand has a size impact. On a 32-bit machine there is
> >> sizeof(struct
> >> timehands) == 120.
> >>
> > What do you think about something in the middle between adding a second 
> > timehand and not adding it at all for non-SMP
> configurations?
> > Let me explain better the idea; the pps_event() uses only 3 variables of 
> > the object timehand: th_offsetcount, th_bintime and
> th_scale. The values of them could be saved instruct  pps_state during 
> pps_capture(). This will only happen in case of non-SMP
> configurations. Then, the verifications need to be adapted for SMP or for 
> non-SMP, e.g. the th_generation will be checked at the end
> of pps_capture() for both configurations but only for SMP in pps_event().
> 
> I would keep the synchronization as it is in FreeBSD for now and add a test 
> case which fails in non-SMP configurations. This can be
> used to discuss the trade-offs. We need such a test case anyway.
> 
Unfortunately I could not reproduce the error again. Not sure what has changed 
that now it is not possible to reproduce it. So, I'll prepare new patches 
(without this trade-off solution) and send them again in order to get new 
feedback 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: AW: AW: AW: AW: [PATCH 11/12] kern_ntptime: Add define in order to remove warning

[Gabriel.Moyano]

> On 08/04/2022 08:28, gabriel.moy...@dlr.de wrote:
>  PPS device drivers should use the kernel space API. Since we don't
>  have a user and kernel space in RTEMS, the kerne space API should be 
>  available also if __rtems__ is defined.
> >>> So, you mean to remove the #ifdef _KERNEL using #ifndef__rtems__.
> >> It is an
> >>
> >> #ifdef _KERNEL
> >> ...
> >> #else /* !_KERNEL */
> >> ...
> >> #endif /* KERNEL */
> >>
> >> In RTEMS we need both APIs, one for PPS drivers and the other for 
> >> applications.
> >>
> >> If we don't want to change the header, then PPS drivers have to do 
> >> something like this:
> >>
> >> #define _KERNEL
> >> #include 
> > Yes, that's why added the #define _KERNEL in kern_ntptime.c.
> > What it is your suggestion here?
> 
> We have currently no API which needs a _KERNEL define. Maybe we should 
> postpone the decision and keep the need for _KERNEL
> define at this point in time.
> 
Is it possible to comment them out with /* */? Because adding the #define 
_KERNEL and #include  could generate some issues while compiling 
and this depends on the position where the header was included
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: AW: AW: [PATCH 07/12] kern_tc.c: Remove verification of th_generation for pps functions

[Gabriel.Moyano]

> On 07/04/2022 15:16, Sebastian Huber wrote:
> > On 07/04/2022 15:10, gabriel.moy...@dlr.de wrote:
> >>> Which sequence of function calls and timings cases the problem? This
> >>> should be definitely a test case.
> >> The generation is updated every time tc_windup() is called. So it is
> >> more or less a race condition when it happens.
> >
> > This is not a race condition. The sequence counter is supposed to
> > ensure a consistent state. You can't simply remove this.
> >
> > Maybe the PPS support requires that we use two timehands even for the
> > uniprocessor configuration. For this, we have to understand under
> > which conditions and use cases the generation change is an issue.
> 
> In kern_tc.c there is this comment, which is a result from a discussion on a 
> FreeBSD mailing list:
> 
> /*
>   * In FreeBSD, the timehands count is a tuning option from two to 16.  The
>   * tuning option was added since it is inexpensive and some FreeBSD users 
> asked
>   * for it to play around.  The default value is two.  One system which did 
> not
>   * work with two timehands was a system with one processor and a specific PPS
>   * device.
>   *
>   * For RTEMS, in uniprocessor configurations, just use one timehand since the
>   * update is done with interrupt disabled.
>   *
>   * In SMP configurations, use a fixed set of two timehands until someone
>   * reports an issue.
>   */
> 
> We definitely need a test case for this corner case. Adding a second timehand 
> has a size impact. On a 32-bit machine there is
> sizeof(struct
> timehands) == 120.
> 
What do you think about something in the middle between adding a second 
timehand and not adding it at all for non-SMP configurations?
Let me explain better the idea; the pps_event() uses only 3 variables of the 
object timehand: th_offsetcount, th_bintime and th_scale. The values of them 
could be saved instruct  pps_state during pps_capture(). This will only happen 
in case of non-SMP configurations. Then, the verifications need to be adapted 
for SMP or for non-SMP, e.g. the th_generation will be checked at the end of 
pps_capture() for both configurations but only for SMP in pps_event().
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: AW: [PATCH 12/12] spec/build/cpukit: Add option for enabling PPS synchronization

[Gabriel.Moyano]

> >
> >> In general, the patch set lacks test cases.
> >
> > I was thinking that the next step could be to add a generic device which is 
> > required to use the API (a file descriptor is needed). This is
> something that wanted to ask in the mailing list first. When this device is 
> added, also the test cases can be. Furthermore I wanted to
> split the submitted changes in order to make a review easier.
> 
> A file descriptor should be optional. It should be possible to add a PPS 
> driver just using struct pps_state.
> 
What do you mean with optional?

The RFC 2783 defines only a user API. For example for config the function 
time_pps_setparams() is needed. If we don't follow the RFC 2783, we will need 
to create an RTEMS API. Is that what you mean?
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: AW: AW: AW: [PATCH 11/12] kern_ntptime: Add define in order to remove warning

[Gabriel.Moyano]

> >> PPS device drivers should use the kernel space API. Since we don't
> >> have a user and kernel space in RTEMS, the kerne space API should be 
> >> available also if __rtems__ is defined.
> >
> > So, you mean to remove the #ifdef _KERNEL using #ifndef__rtems__.
> 
> It is an
> 
> #ifdef _KERNEL
> ...
> #else /* !_KERNEL */
> ...
> #endif /* KERNEL */
> 
> In RTEMS we need both APIs, one for PPS drivers and the other for 
> applications.
> 
> If we don't want to change the header, then PPS drivers have to do something 
> like this:
> 
> #define _KERNEL
> #include 

Yes, that's why added the #define _KERNEL in kern_ntptime.c.
What it is your suggestion here? 

> There should be a new chapter in
> 
> https://docs.rtems.org/branches/master/bsp-howto/index.html
> 
> for the PPS drivers.

ok

> >
> >>>
> >>> hardpps() is declared if _KERNEL is defined. That's why I added it
> >>> before including timepps.h in kern_ntptime.c It was for removing a 
> >>> warning.
> >>>
>  The hardpps() needs to be added to the _ namespace, for example 
>  _NTP_Pulse_per_second_event().
> >>>
> >>> Here I didn't follow you
> >>
> >> The FreeBSD kernel has its own symbol namespace which is separated
> >> from the application. In RTEMS, the kernel and application share a symbol 
> >> namespace. Kernel functions should use the _
> namespace.
> >>
> > And here you mean to do something similar to what have to be done for 
> > tc_getfrequency() ( declare
> _Timecounter_Get_frequency())?
> 
> Yes, maybe add the declaration and documentation to 
>  and name the function
> _Timecouter_Process_pulse_per_second_event().
> 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: AW: AW: [PATCH 11/12] kern_ntptime: Add define in order to remove warning

[Gabriel.Moyano]

> On 07/04/2022 14:51, gabriel.moy...@dlr.de wrote:
> >> On 07/04/2022 11:56, gabriel.moy...@dlr.de wrote:
>  On 07/04/2022 10:36, Gabriel Moyano wrote:
> > ---
> > cpukit/score/src/kern_ntptime.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/cpukit/score/src/kern_ntptime.c
> > b/cpukit/score/src/kern_ntptime.c index d4a233f67e..d6ea739f5b
> > 100644
> > --- a/cpukit/score/src/kern_ntptime.c
> > +++ b/cpukit/score/src/kern_ntptime.c
> > @@ -58,6 +58,9 @@ __FBSDID("$FreeBSD$");
> > #include 
> > #include 
> > #include 
> > +#ifdef __rtems__
> > +#define_KERNEL
> > +#endif /* __rtems__ */
> > #include 
> > #ifndef __rtems__
> > #include 
>  Who is supposed to use the _KERNEL  API?
> 
> >>> hardpps()
> >>
> >> The pps_*() API should be accessible for RTEMS drivers without having to 
> >> define _KERNEL.
> >
> > The API functions start with time_pps_*().
> 
> This is the user space API. It uses a file descriptor.
> 
> PPS device drivers should use the kernel space API. Since we don't have a 
> user and kernel space in RTEMS, the kerne space API should
> be available also if __rtems__ is defined.

So, you mean to remove the #ifdef _KERNEL using #ifndef__rtems__.

> >
> > hardpps() is declared if _KERNEL is defined. That's why I added it
> > before including timepps.h in kern_ntptime.c It was for removing a warning.
> >
> >> The hardpps() needs to be added to the _ namespace, for example 
> >> _NTP_Pulse_per_second_event().
> >
> > Here I didn't follow you
> 
> The FreeBSD kernel has its own symbol namespace which is separated from the 
> application. In RTEMS, the kernel and application
> share a symbol namespace. Kernel functions should use the _ namespace.
> 
And here you mean to do something similar to what have to be done for 
tc_getfrequency() ( declare _Timecounter_Get_frequency())?
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: AW: [PATCH 05/12] kern_tc.c: Replace atomic functions required by PPS API

[Gabriel.Moyano]

> On 07/04/2022 11:55, gabriel.moy...@dlr.de wrote:
> >> On 07/04/2022 10:36, Gabriel Moyano wrote:
> >>> ---
> >>>cpukit/include/sys/timepps.h | 4 
> >>>cpukit/score/src/kern_tc.c   | 7 +++
> >>>2 files changed, 11 insertions(+)
> >>>
> >>> diff --git a/cpukit/include/sys/timepps.h
> >>> b/cpukit/include/sys/timepps.h index 621afb08ec..5703381ffa 100644
> >>> --- a/cpukit/include/sys/timepps.h
> >>> +++ b/cpukit/include/sys/timepps.h
> >>> @@ -32,7 +32,11 @@
> >>>
> >>>typedef int pps_handle_t;
> >>>
> >>> +#ifndef __rtems__
> >>>typedef unsigned pps_seq_t;
> >>> +#else /* __rtems__ */
> >>> +typedef Atomic_Uint  pps_seq_t;
> >>> +#endif /* __rtems__ */
> >>>
> >>>typedef struct ntp_fp {
> >>>   unsigned intintegral;
> >>> diff --git a/cpukit/score/src/kern_tc.c b/cpukit/score/src/kern_tc.c
> >>> index e57da2c0ca..77f7a9212c 100644
> >>> --- a/cpukit/score/src/kern_tc.c
> >>> +++ b/cpukit/score/src/kern_tc.c
> >>> @@ -1932,10 +1932,17 @@ pps_fetch(struct pps_fetch_args *fapi, struct 
> >>> pps_state *pps)
> >>>   tv.tv_usec = fapi->timeout.tv_nsec / 1000;
> >>>   timo = tvtohz();
> >>>   }
> >>> +#ifndef __rtems__
> >>>   aseq = atomic_load_int(>ppsinfo.assert_sequence);
> >>>   cseq = atomic_load_int(>ppsinfo.clear_sequence);
> >>>   while (aseq == 
> >>> atomic_load_int(>ppsinfo.assert_sequence) &&
> >>>   cseq == 
> >>> atomic_load_int(>ppsinfo.clear_sequence)) {
> >>> +#else /* __rtems__ */
> >>> + aseq = atomic_load_acq_int(>ppsinfo.assert_sequence);
> >>> + cseq = atomic_load_acq_int(>ppsinfo.clear_sequence);
> >>> + while (aseq == 
> >>> atomic_load_acq_int(>ppsinfo.assert_sequence) &&
> >>> + cseq == atomic_load_acq_int(>ppsinfo.clear_sequence)) {
> >>> +#endif /* __rtems__ */
> >>>   if (abi_aware(pps, 1) && pps->driver_mtx != 
> >>> NULL) {
> >>>   if (pps->flags & PPSFLAG_MTX_SPIN) {
> >>>   err = msleep_spin(pps, 
> >>> pps->driver_mtx,
> >> Why do you need this change?
> >>
> > If you mean why atomic_load_int() was replaced by atomic_load_acq_int(), it 
> > is because atomic_load_int()  is not defined in rtems
> but in rtems-libbsd.
> 
> In FreeBSD, it seems the sequence is sometimes incremented using a normal 
> increment, for example:
> 
>   *pcount = pps->capcount;
>   (*pseq)++;
>   *tsp = ts;
> 
> So, just add the atomic_load_int() as an relaxed atomic load to the other 
> inline functions at the top of the file.

atomic_load_acq_int() is defined also at the beginning of the file. Do you want 
to repeat the function but with other name (atomic_load_int()) ? 


> I guess the synchronization needs a review in FreeBSD.
> 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: AW: [PATCH 07/12] kern_tc.c: Remove verification of th_generation for pps functions

[Gabriel.Moyano]

> On 07/04/2022 11:56, gabriel.moy...@dlr.de wrote:
> >> On 07/04/2022 10:36, Gabriel Moyano wrote:
> >>> ---
> >>>cpukit/score/src/kern_tc.c | 6 ++
> >>>1 file changed, 6 insertions(+)
> >>>
> >>> diff --git a/cpukit/score/src/kern_tc.c b/cpukit/score/src/kern_tc.c
> >>> index a5c7b4b841..a8ba268ea3 100644
> >>> --- a/cpukit/score/src/kern_tc.c
> >>> +++ b/cpukit/score/src/kern_tc.c
> >>> @@ -2109,9 +2109,11 @@ pps_capture(struct pps_state *pps)
> >>>   pps->capffth = fftimehands;
> >>>#endif
> >>>   pps->capcount =
> >>> th->th_counter->tc_get_timecount(th->th_counter);
> >>> +#ifndef __rtems__
> >>>   atomic_thread_fence_acq();
> >>>   if (pps->capgen != th->th_generation)
> >>>   pps->capgen = 0;
> >>> +#endif /* __rtems__ */
> >>>}
> >>>
> >>>void
> >>> @@ -2135,10 +2137,12 @@ pps_event(struct pps_state *pps, int event)
> >>>   /* Nothing to do if not currently set to capture this event 
> >>> type. */
> >>>   if ((event & pps->ppsparam.mode) == 0)
> >>>   return;
> >>> +#ifndef __rtems__
> >>>   /* If the timecounter was wound up underneath us, bail out. */
> >>>   if (pps->capgen == 0 || pps->capgen !=
> >>>   atomic_load_acq_int(>capth->th_generation))
> >>>   return;
> >>> +#endif /* __rtems__ */
> >>>
> >>>   /* Things would be easier with arrays. */
> >>>   if (event == PPS_CAPTUREASSERT) { @@ -2189,10 +2193,12 @@
> >>> pps_event(struct pps_state *pps, int event)
> >>>   bintime_addx(, pps->capth->th_scale * tcount);
> >>>   bintime2timespec(, );
> >>>
> >>> +#ifndef __rtems__
> >>>   /* If the timecounter was wound up underneath us, bail out. */
> >>>   atomic_thread_fence_acq();
> >>>   if (pps->capgen != pps->capth->th_generation)
> >>>   return;
> >>> +#endif /* __rtems__ */
> >>>
> >>>   *pcount = pps->capcount;
> >>>   (*pseq)++;
> >> Why is this change justified?
> > Since only one timehands object is used, the generation is updated more 
> > frequently. This can even happen between pps_capture()
> and pps_event(), making the synchronization impossible.
> 
> It seems you didn't test with SMP configurations, since here we have two 
> timehands.
>

Yes, you are right, I didn't test it with SMP configurations (for now).
On possible solution will be to have these verifications only for SMP.
 
> Which sequence of function calls and timings cases the problem? This should 
> be definitely a test case.

The generation is updated every time tc_windup() is called. So it is more or 
less a race condition when it happens.
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: AW: [PATCH 08/12] kern_tc.c: Add definition of tc_getfrequency in kern_tc

[Gabriel.Moyano]

> On 07/04/2022 12:43, gabriel.moy...@dlr.de wrote:
> >
> >> On 07/04/2022 10:36, Gabriel Moyano wrote:
> >>> ---
> >>>cpukit/score/src/kern_tc.c | 2 +-
> >>>1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/cpukit/score/src/kern_tc.c b/cpukit/score/src/kern_tc.c
> >>> index a8ba268ea3..dd718b3bab 100644
> >>> --- a/cpukit/score/src/kern_tc.c
> >>> +++ b/cpukit/score/src/kern_tc.c
> >>> @@ -1506,7 +1506,6 @@ unlock:
> >>>#endif /* __rtems__ */
> >>>}
> >>>
> >>> -#ifndef __rtems__
> >>>/* Report the frequency of the current timecounter. */
> >>>uint64_t
> >>>tc_getfrequency(void)
> >>> @@ -1515,6 +1514,7 @@ tc_getfrequency(void)
> >>>   return (timehands->th_counter->tc_frequency);
> >>>}
> >>>
> >>> +#ifndef __rtems__
> >>>static bool
> >>>sleeping_on_old_rtc(struct thread *td)
> >>>{
> >> Please add this new global function to .
> >>
> > uint64_t tc_getfrequency(void) is already declared in
> > cpukit/include/sys/timetc.h
> 
> Yes, this is fine, please just have a look how the other global functions in 
> this file are used:
> 
> nm build/arm/xilinx_zynq_a9_qemu/cpukit/score/src/kern_tc.c.65.o -g 
> --defined-only
>  T rtems_clock_get_boot_time
>  T rtems_clock_get_boot_time_bintime
>  T rtems_clock_get_boot_time_timeval
>  T rtems_clock_get_monotonic
>  T rtems_clock_get_monotonic_bintime
>  T rtems_clock_get_monotonic_coarse
>  T rtems_clock_get_monotonic_coarse_bintime
>  T rtems_clock_get_monotonic_coarse_timeval
>  T rtems_clock_get_monotonic_sbintime
>  T rtems_clock_get_monotonic_timeval
>  T rtems_clock_get_realtime
>  T rtems_clock_get_realtime_bintime
>  T rtems_clock_get_realtime_coarse
>  T rtems_clock_get_realtime_coarse_bintime
>  T rtems_clock_get_realtime_coarse_timeval
>  T rtems_clock_get_realtime_timeval
>  D _Timecounter
>  T _Timecounter_Bintime
>  T _Timecounter_Binuptime
>  T _Timecounter_Getbintime
>  T _Timecounter_Getbinuptime
>  T _Timecounter_Getboottime
>  T _Timecounter_Getboottimebin
>  T _Timecounter_Getmicrotime
>  T _Timecounter_Getmicrouptime
>  T _Timecounter_Getnanotime
>  T _Timecounter_Getnanouptime
>  T _Timecounter_Install
>  T _Timecounter_Microtime
>  T _Timecounter_Microuptime
>  T _Timecounter_Nanotime
>  T _Timecounter_Nanouptime
>  T _Timecounter_Sbinuptime
>  T _Timecounter_Set_clock
>  T _Timecounter_Set_NTP_update_second
>  T _Timecounter_Tick
>  T _Timecounter_Tick_simple
>  D _Timecounter_Time_second
>  D _Timecounter_Time_uptime
> 
Sorry, I didn't follow you here. tc_getfrequency() is already declared. Do you 
want to change its name?
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: AW: [PATCH 12/12] spec/build/cpukit: Add option for enabling PPS synchronization

[Gabriel.Moyano]

> On 07/04/2022 11:57, gabriel.moy...@dlr.de wrote:
> >> On 07/04/2022 10:36, Gabriel Moyano wrote:
> >>> ---
> >>>spec/build/cpukit/cpuopts.yml|  2 ++
> >>>spec/build/cpukit/optppssync.yml | 16 
> >>>2 files changed, 18 insertions(+)
> >>>create mode 100644 spec/build/cpukit/optppssync.yml
> >>>
> >>> diff --git a/spec/build/cpukit/cpuopts.yml
> >>> b/spec/build/cpukit/cpuopts.yml index 301d49ccea..db0e05395f 100644
> >>> --- a/spec/build/cpukit/cpuopts.yml
> >>> +++ b/spec/build/cpukit/cpuopts.yml
> >>> @@ -49,6 +49,8 @@ links:
> >>>  uid: optparavirt
> >>>- role: build-dependency
> >>>  uid: optposix
> >>> +- role: build-dependency
> >>> +  uid: optppssync
> >>>- role: build-dependency
> >>>  uid: optprofiling
> >>>- role: build-dependency
> >>> diff --git a/spec/build/cpukit/optppssync.yml
> >>> b/spec/build/cpukit/optppssync.yml
> >>> new file mode 100644
> >>> index 00..6766cf0d40
> >>> --- /dev/null
> >>> +++ b/spec/build/cpukit/optppssync.yml
> >>> @@ -0,0 +1,16 @@
> >>> +SPDX-License-Identifier: CC-BY-SA-4.0 OR BSD-2-Clause
> >>> +actions:
> >>> +- get-boolean: null
> >>> +- env-enable: null
> >>> +- define-condition: null
> >>> +build-type: option
> >>> +copyrights:
> >>> +- Copyright (C) 2022 German Aerospace Center (DLR)
> >>> +default: false
> >>> +default-by-variant: []
> >>> +description: |
> >>> +  Enable time synchronization using a Pulse Per Second signal
> >>> +enabled-by: true
> >>> +links: []
> >>> +name: PPS_SYNC
> >>> +type: build
> >>
> >> If this should really be a CPU option, then it needs a proper RTEMS_* 
> >> prefix.
> >
> > Do you mean that the name should be RTEMS_PPS_SYNC?
> 
> We should avoid new CPU options in general.

May I ask why?

> >
> >> Why do we need the option. What is the overhead if it is always enabled.
> >
> > The PPS_SYNC option comes from FREEBSD. Without it, you can still use the 
> > API. For example a task can wait for an event calling
> time_pps_fetch().
> > But if it is not enabled, hardpps() isn't called and time_freq is not 
> > updated.
> 
> Is something from PPS_SYNC used in the normal timekeeping?

Defining it just enables the time synchronization with a PPS signal. It can be 
defined by default or it can be added as an option to keep similarities with 
FreeBSD.
 
> >
> >> In general, the patch set lacks test cases.
> >
> > I was thinking that the next step could be to add a generic device which is 
> > required to use the API (a file descriptor is needed). This is
> something that wanted to ask in the mailing list first. When this device is 
> added, also the test cases can be. Furthermore I wanted to
> split the submitted changes in order to make a review easier.
> 
> A file descriptor should be optional. It should be possible to add a PPS 
> driver just using struct pps_state.

The struct pps_state type is used only internally. In order to be able to use 
the API a file descriptor is required (API functions start with time_pps_*()).
The "generic device" I have in mind will have an object of struct pps_state. 

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: AW: [PATCH 11/12] kern_ntptime: Add define in order to remove warning

[Gabriel.Moyano]

> On 07/04/2022 11:56, gabriel.moy...@dlr.de wrote:
> >> On 07/04/2022 10:36, Gabriel Moyano wrote:
> >>> ---
> >>>cpukit/score/src/kern_ntptime.c | 3 +++
> >>>1 file changed, 3 insertions(+)
> >>>
> >>> diff --git a/cpukit/score/src/kern_ntptime.c
> >>> b/cpukit/score/src/kern_ntptime.c index d4a233f67e..d6ea739f5b
> >>> 100644
> >>> --- a/cpukit/score/src/kern_ntptime.c
> >>> +++ b/cpukit/score/src/kern_ntptime.c
> >>> @@ -58,6 +58,9 @@ __FBSDID("$FreeBSD$");
> >>>#include 
> >>>#include 
> >>>#include 
> >>> +#ifdef __rtems__
> >>> +#define  _KERNEL
> >>> +#endif /* __rtems__ */
> >>>#include 
> >>>#ifndef __rtems__
> >>>#include 
> >> Who is supposed to use the _KERNEL  API?
> >>
> > hardpps()
> 
> The pps_*() API should be accessible for RTEMS drivers without having to 
> define _KERNEL.

The API functions start with time_pps_*().

hardpps() is declared if _KERNEL is defined. That's why I added it before 
including timepps.h in kern_ntptime.c
It was for removing a warning.

> The hardpps() needs to be added to the _ namespace, for example 
> _NTP_Pulse_per_second_event().

Here I didn't follow you
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: [PATCH 12/12] spec/build/cpukit: Add option for enabling PPS synchronization

[Gabriel.Moyano]

> On 07/04/2022 10:36, Gabriel Moyano wrote:
> > ---
> >   spec/build/cpukit/cpuopts.yml|  2 ++
> >   spec/build/cpukit/optppssync.yml | 16 
> >   2 files changed, 18 insertions(+)
> >   create mode 100644 spec/build/cpukit/optppssync.yml
> >
> > diff --git a/spec/build/cpukit/cpuopts.yml
> > b/spec/build/cpukit/cpuopts.yml index 301d49ccea..db0e05395f 100644
> > --- a/spec/build/cpukit/cpuopts.yml
> > +++ b/spec/build/cpukit/cpuopts.yml
> > @@ -49,6 +49,8 @@ links:
> > uid: optparavirt
> >   - role: build-dependency
> > uid: optposix
> > +- role: build-dependency
> > +  uid: optppssync
> >   - role: build-dependency
> > uid: optprofiling
> >   - role: build-dependency
> > diff --git a/spec/build/cpukit/optppssync.yml
> > b/spec/build/cpukit/optppssync.yml
> > new file mode 100644
> > index 00..6766cf0d40
> > --- /dev/null
> > +++ b/spec/build/cpukit/optppssync.yml
> > @@ -0,0 +1,16 @@
> > +SPDX-License-Identifier: CC-BY-SA-4.0 OR BSD-2-Clause
> > +actions:
> > +- get-boolean: null
> > +- env-enable: null
> > +- define-condition: null
> > +build-type: option
> > +copyrights:
> > +- Copyright (C) 2022 German Aerospace Center (DLR)
> > +default: false
> > +default-by-variant: []
> > +description: |
> > +  Enable time synchronization using a Pulse Per Second signal
> > +enabled-by: true
> > +links: []
> > +name: PPS_SYNC
> > +type: build
> 
> If this should really be a CPU option, then it needs a proper RTEMS_* prefix. 

Do you mean that the name should be RTEMS_PPS_SYNC?

> Why do we need the option. What is the overhead if it is always enabled.

The PPS_SYNC option comes from FREEBSD. Without it, you can still use the API. 
For example a task can wait for an event calling time_pps_fetch().
But if it is not enabled, hardpps() isn't called and time_freq is not updated.

> In general, the patch set lacks test cases.

I was thinking that the next step could be to add a generic device which is 
required to use the API (a file descriptor is needed). This is something that 
wanted to ask in the mailing list first. When this device is added, also the 
test cases can be. Furthermore I wanted to split the submitted changes in order 
to make a review easier.
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: [PATCH 06/12] kern_tc.c: Replace FREEBSD event mechanism by adding pointers to function

[Gabriel.Moyano]

> On 07/04/2022 10:36, Gabriel Moyano wrote:
> > +#else /* __rtems__ */
> > +if (pps->wait_event != NULL)
> > +err = (*pps->wait_event)(pps, fapi->timeout);
> > +else
> > +err = EAGAIN;
> > +#endif /* __rtems__ */
> > if (err == EWOULDBLOCK) {
> > if (fapi->timeout.tv_sec == -1) {
> > continue;
> > @@ -2227,7 +2240,12 @@ pps_event(struct pps_state *pps, int event)
> >   #endif
> >
> > /* Wakeup anyone sleeping in pps_fetch().  */
> > +#ifndef __rtems__
> > wakeup(pps);
> > +#else /* __rtems__ */
> > +if (pps->wakeup != NULL)
> > +(*pps->wakeup)(pps);
> > +#endif /* __rtems__ */
> 
> I would not allow a NULL pointer here. The driver shall provide callbacks.

Do you mean to add an assert?
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: [PATCH 11/12] kern_ntptime: Add define in order to remove warning

[Gabriel.Moyano]

> On 07/04/2022 10:36, Gabriel Moyano wrote:
> > ---
> >   cpukit/score/src/kern_ntptime.c | 3 +++
> >   1 file changed, 3 insertions(+)
> >
> > diff --git a/cpukit/score/src/kern_ntptime.c
> > b/cpukit/score/src/kern_ntptime.c index d4a233f67e..d6ea739f5b 100644
> > --- a/cpukit/score/src/kern_ntptime.c
> > +++ b/cpukit/score/src/kern_ntptime.c
> > @@ -58,6 +58,9 @@ __FBSDID("$FreeBSD$");
> >   #include 
> >   #include 
> >   #include 
> > +#ifdef __rtems__
> > +#define_KERNEL
> > +#endif /* __rtems__ */
> >   #include 
> >   #ifndef __rtems__
> >   #include 
> 
> Who is supposed to use the _KERNEL  API?
> 
hardpps()
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: [PATCH 07/12] kern_tc.c: Remove verification of th_generation for pps functions

[Gabriel.Moyano]

> On 07/04/2022 10:36, Gabriel Moyano wrote:
> > ---
> >   cpukit/score/src/kern_tc.c | 6 ++
> >   1 file changed, 6 insertions(+)
> >
> > diff --git a/cpukit/score/src/kern_tc.c b/cpukit/score/src/kern_tc.c
> > index a5c7b4b841..a8ba268ea3 100644
> > --- a/cpukit/score/src/kern_tc.c
> > +++ b/cpukit/score/src/kern_tc.c
> > @@ -2109,9 +2109,11 @@ pps_capture(struct pps_state *pps)
> > pps->capffth = fftimehands;
> >   #endif
> > pps->capcount = th->th_counter->tc_get_timecount(th->th_counter);
> > +#ifndef __rtems__
> > atomic_thread_fence_acq();
> > if (pps->capgen != th->th_generation)
> > pps->capgen = 0;
> > +#endif /* __rtems__ */
> >   }
> >
> >   void
> > @@ -2135,10 +2137,12 @@ pps_event(struct pps_state *pps, int event)
> > /* Nothing to do if not currently set to capture this event type. */
> > if ((event & pps->ppsparam.mode) == 0)
> > return;
> > +#ifndef __rtems__
> > /* If the timecounter was wound up underneath us, bail out. */
> > if (pps->capgen == 0 || pps->capgen !=
> > atomic_load_acq_int(>capth->th_generation))
> > return;
> > +#endif /* __rtems__ */
> >
> > /* Things would be easier with arrays. */
> > if (event == PPS_CAPTUREASSERT) {
> > @@ -2189,10 +2193,12 @@ pps_event(struct pps_state *pps, int event)
> > bintime_addx(, pps->capth->th_scale * tcount);
> > bintime2timespec(, );
> >
> > +#ifndef __rtems__
> > /* If the timecounter was wound up underneath us, bail out. */
> > atomic_thread_fence_acq();
> > if (pps->capgen != pps->capth->th_generation)
> > return;
> > +#endif /* __rtems__ */
> >
> > *pcount = pps->capcount;
> > (*pseq)++;
> 
> Why is this change justified?

Since only one timehands object is used, the generation is updated more 
frequently. This can even happen between pps_capture() and pps_event(), making 
the synchronization impossible.
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: [PATCH 06/12] kern_tc.c: Replace FREEBSD event mechanism by adding pointers to function

[Gabriel.Moyano]

> On 07/04/2022 10:36, Gabriel Moyano wrote:
> > diff --git a/cpukit/include/sys/timepps.h
> > b/cpukit/include/sys/timepps.h index 5703381ffa..a72579d5d7 100644
> > --- a/cpukit/include/sys/timepps.h
> > +++ b/cpukit/include/sys/timepps.h
> > @@ -149,6 +149,12 @@ struct mtx;
> >
> >   #define   PPSFLAG_MTX_SPIN0x01/* Driver mtx is MTX_SPIN type. 
> > */
> >
> > +#ifdef __rtems__
> > +struct pps_state;
> > +typedef int (*wait_event_func)(struct pps_state *pps, struct timespec
> > +timeout); typedef void (*wakeup_func)(struct pps_state *pps); #endif
> > +/* __rtems__ */
> > +
> >   struct pps_state {
> > /* Capture information. */
> > struct timehands *capth;
> > @@ -164,6 +170,11 @@ struct pps_state {
> > int ppscap;
> > struct timecounter *ppstc;
> > unsignedppscount[3];
> > +#ifdef __rtems__
> > +wait_event_func wait_event;
> > +wakeup_func wakeup;
> > +#endif /* __rtems__ */
> > +
> > /*
> >  * The following fields are valid if the driver calls pps_init_abi().
> >  */
> 
> Why do we need the typedefs?

Just for clarity

> Please don't change the formatting.

Do you mean that wait_event should be waitevent?

> No changes outside the #ifdef __rtems__ markers please.

Sorry, could you point to the changes?

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: [PATCH 05/12] kern_tc.c: Replace atomic functions required by PPS API

[Gabriel.Moyano]

> On 07/04/2022 10:36, Gabriel Moyano wrote:
> > ---
> >   cpukit/include/sys/timepps.h | 4 
> >   cpukit/score/src/kern_tc.c   | 7 +++
> >   2 files changed, 11 insertions(+)
> >
> > diff --git a/cpukit/include/sys/timepps.h
> > b/cpukit/include/sys/timepps.h index 621afb08ec..5703381ffa 100644
> > --- a/cpukit/include/sys/timepps.h
> > +++ b/cpukit/include/sys/timepps.h
> > @@ -32,7 +32,11 @@
> >
> >   typedef int pps_handle_t;
> >
> > +#ifndef __rtems__
> >   typedef unsigned pps_seq_t;
> > +#else /* __rtems__ */
> > +typedef Atomic_Uintpps_seq_t;
> > +#endif /* __rtems__ */
> >
> >   typedef struct ntp_fp {
> > unsigned intintegral;
> > diff --git a/cpukit/score/src/kern_tc.c b/cpukit/score/src/kern_tc.c
> > index e57da2c0ca..77f7a9212c 100644
> > --- a/cpukit/score/src/kern_tc.c
> > +++ b/cpukit/score/src/kern_tc.c
> > @@ -1932,10 +1932,17 @@ pps_fetch(struct pps_fetch_args *fapi, struct 
> > pps_state *pps)
> > tv.tv_usec = fapi->timeout.tv_nsec / 1000;
> > timo = tvtohz();
> > }
> > +#ifndef __rtems__
> > aseq = atomic_load_int(>ppsinfo.assert_sequence);
> > cseq = atomic_load_int(>ppsinfo.clear_sequence);
> > while (aseq == atomic_load_int(>ppsinfo.assert_sequence) &&
> > cseq == atomic_load_int(>ppsinfo.clear_sequence)) {
> > +#else /* __rtems__ */
> > +   aseq = atomic_load_acq_int(>ppsinfo.assert_sequence);
> > +   cseq = atomic_load_acq_int(>ppsinfo.clear_sequence);
> > +   while (aseq == 
> > atomic_load_acq_int(>ppsinfo.assert_sequence) &&
> > +   cseq == atomic_load_acq_int(>ppsinfo.clear_sequence)) {
> > +#endif /* __rtems__ */
> > if (abi_aware(pps, 1) && pps->driver_mtx != NULL) {
> > if (pps->flags & PPSFLAG_MTX_SPIN) {
> > err = msleep_spin(pps, pps->driver_mtx,
> 
> Why do you need this change?
> 

If you mean why atomic_load_int() was replaced by atomic_load_acq_int(), it is 
because atomic_load_int()  is not defined in rtems but in rtems-libbsd.
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: AW: AW: Dependencies of PPS API in rtems-libbsd

[Gabriel.Moyano]

> On 30/03/2022 15:19, gabriel.moy...@dlr.de wrote:
> >>> Ok, it seems the pps_event() could be called by interrupt service
> >>> routines. In this case, you cannot use a mutex and condition variables.
> >>> You have to use a thread queue directly. Use the thread queue ISR
> >>> lock for mutual exclusion. Use _Thread_queue_Enqueue() to emulate
> >>> sleep() and
> >>> _Thread_queue_Flush_critical() to emulate wakeup(). Check all
> >>> critical sections that they can be protected by an ISR lock (no blocking 
> >>> calls and short).
> >> Is this even so if pps_event() calls signal or broadcast? Because 
> >> pps_fetch() is the only function that is waiting (and locking a
> mutex).
> > Do you mean something like this?
> >
> [...]
> >
> > Unfortunately something should be missing because it doesn't work.
> 
> In cpukit/score/src/futex.c you have a working example.
> 
> You would have to replace the mutex with the thread queue lock. I had a look 
> at some FreeBSD drivers which use this stuff and it
> would not be possible to support them with this approach. You could move the
> 
>   if (abi_aware(pps, 1) && pps->driver_mtx != NULL) {
>   if (pps->flags & PPSFLAG_MTX_SPIN) {
>   err = msleep_spin(pps, pps->driver_mtx,
>   "ppsfch", timo);
>   } else {
>   err = msleep(pps, pps->driver_mtx, 
> PCATCH,
>   "ppsfch", timo);
>   }
>   } else {
>   err = tsleep(pps, PCATCH, "ppsfch", timo);
>   }
> 
> and
> 
>   /* Wakeup anyone sleeping in pps_fetch().  */
>   wakeup(pps);
> 
> blocks into RTEMS-specific sleep/wakeup callouts in struct pps_state.
>

Just reading this email a second time, do you mean stop trying the thread queue 
approach and use another RTEMS-specific solution for waking up tasks which wait 
in pps_fetch()? For example use RTEMS signals or events?
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: AW: AW: Dependencies of PPS API in rtems-libbsd

[Gabriel.Moyano]

> 
> On 30/03/2022 15:19, gabriel.moy...@dlr.de wrote:
> >>> Ok, it seems the pps_event() could be called by interrupt service
> >>> routines. In this case, you cannot use a mutex and condition variables.
> >>> You have to use a thread queue directly. Use the thread queue ISR
> >>> lock for mutual exclusion. Use _Thread_queue_Enqueue() to emulate
> >>> sleep() and
> >>> _Thread_queue_Flush_critical() to emulate wakeup(). Check all
> >>> critical sections that they can be protected by an ISR lock (no blocking 
> >>> calls and short).
> >> Is this even so if pps_event() calls signal or broadcast? Because 
> >> pps_fetch() is the only function that is waiting (and locking a
> mutex).
> > Do you mean something like this?
> >
> [...]
> >
> > Unfortunately something should be missing because it doesn't work.
> 
> In cpukit/score/src/futex.c you have a working example.

The code I sent to you is based on condition.c, which is very similar, but 
probably something is wrong because it hangs after calling 
_Thread_queue_Enqueue() (my question was pointing into that direction)

> You would have to replace the mutex with the thread queue lock. I had a look 
> at some FreeBSD drivers which use this stuff and it
> would not be possible to support them with this approach. You could move the
> 
>   if (abi_aware(pps, 1) && pps->driver_mtx != NULL) {
>   if (pps->flags & PPSFLAG_MTX_SPIN) {
>   err = msleep_spin(pps, pps->driver_mtx,
>   "ppsfch", timo);
>   } else {
>   err = msleep(pps, pps->driver_mtx, 
> PCATCH,
>   "ppsfch", timo);
>   }
>   } else {
>   err = tsleep(pps, PCATCH, "ppsfch", timo);
>   }
> 
> and
> 
>   /* Wakeup anyone sleeping in pps_fetch().  */
>   wakeup(pps);
> 
> blocks into RTEMS-specific sleep/wakeup callouts in struct pps_state.

Yes, these are the parts that I'm implementing. For now I'm focusing to emulate 
the behavior of tsleep() because it will be enough to get something working.
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: AW: Dependencies of PPS API in rtems-libbsd

[Gabriel.Moyano]

> > >> I guess you want to enable tc_poll_pps in struct timecounter as well?
> > >
> > > I didn't plan to do that but it can be done just removing some #ifndef, 
> > > right?
> >
> > Is this handler not use for the PPS support? If it is currently unused, 
> > then please let it disabled.
> 
> Ok. For now I was working with interrupts but I guess that the same can be 
> done with polling, that's for that handler.
> 
> >
> > >
> > >> For what do you need the sleep() and wakeup() support? Is this only used 
> > >> by the RFC 2783 PPS-API implementation?
> > >
> > > Yes, they are required by pps_fetch() and pps_event()
> > >
> > >> If you want to keep implement this in RTEMS, then you can convert
> > >> this to use a condition variable from  or .
> > >
> > > Do you mean to add a condition variable, for example in struct
> > > pps_state, and to replace sleep() and wakeup() by wait and signal?
> > > It
> > is good idea if we don't want to use the first functions.
> >
> > Ok, it seems the pps_event() could be called by interrupt service
> > routines. In this case, you cannot use a mutex and condition variables.
> > You have to use a thread queue directly. Use the thread queue ISR lock
> > for mutual exclusion. Use _Thread_queue_Enqueue() to emulate sleep()
> > and
> > _Thread_queue_Flush_critical() to emulate wakeup(). Check all critical
> > sections that they can be protected by an ISR lock (no blocking calls and 
> > short).
> 
> Is this even so if pps_event() calls signal or broadcast? Because pps_fetch() 
> is the only function that is waiting (and locking a mutex).

Do you mean something like this?

_Thread_queue_Queue_initialize(>waiting_queue.Queue, NULL);

// To emulate sleep()
Thread_queue_Context queue_context;
_Thread_queue_Context_initialize(_context);
_Thread_queue_Context_set_enqueue_do_nothing_extra(_context);
Thread_Control *executing = _Thread_Executing;
_Thread_queue_Queue_acquire_critical(
>waiting_queue.Queue,
>Potpourri_stats,
_context.Lock_context.Lock_context
);

_Thread_queue_Context_set_thread_state(
_context,
STATES_BLOCKED
);

_Thread_queue_Enqueue(
>waiting_queue.Queue,
&_Thread_queue_Operations_FIFO,
executing,
_context
);

// To emulate wakeup()
Thread_queue_Context queue_context;
_Thread_queue_Context_initialize(_context);
_Thread_queue_Flush_critical(
>waiting_queue.Queue,
&_Thread_queue_Operations_FIFO,
_Thread_queue_Flush_default_filter,
_context
);

Unfortunately something should be missing because it doesn't work.

> 
> >
> > >
> > >> All the uses of sleep() and wakeup() need to be clearly identified and 
> > >> if possible avoided.
> > >
> > > May I ask, what is the reason for avoiding them?
> >
> > The sleep() and wakeup() synchronization is nice, but not suitable for 
> > real-time systems (use of hash tables).
> >
> > >
> > > What do you think about coping the functions lmax()/qmin() and the
> > > define for ENOIOCTL? For lmax()/qmin() I saw that the file
> > where are defined (libkern.h) is in libnetworking. Not sure what it is the 
> > best option here, I don't like when the things are
> duplicated.
> >
> > Since you refer to libnetworking I guess you work with RTEMS 5. Please
> > first get it working on RTEMS 6 and then we think about a back port.
> 
> Yes, I'm working with RTEMS 5 but my next step is port the changes to RTEMS 6 
> and then submit the patches.
> 
> >
> > >
> > > What about tvtohz(), which is defined in 
> > > rtemsbsd/rtems/rtems-kernel-timesupport.c?
> 
> tvtohz() is used for converting timeval to hz, which is required by msleep(), 
> msleep_spin() and tsleep but because they are not used,
> tvtohz() is not needed.
> 
> >
> > Maybe just copy the dependencies to kern_tc.c. Later we can try to move 
> > them to shared locations if necessary.
> 
> I did this for lmax(), qmin() and ENOIOCTL.
> 
> Thanks for your answers.
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: AW: Dependencies of PPS API in rtems-libbsd

[Gabriel.Moyano]

> >> I guess you want to enable tc_poll_pps in struct timecounter as well?
> >
> > I didn't plan to do that but it can be done just removing some #ifndef, 
> > right?
> 
> Is this handler not use for the PPS support? If it is currently unused, then 
> please let it disabled.

Ok. For now I was working with interrupts but I guess that the same can be done 
with polling, that's for that handler.

> 
> >
> >> For what do you need the sleep() and wakeup() support? Is this only used 
> >> by the RFC 2783 PPS-API implementation?
> >
> > Yes, they are required by pps_fetch() and pps_event()
> >
> >> If you want to keep implement this in RTEMS, then you can convert
> >> this to use a condition variable from  or .
> >
> > Do you mean to add a condition variable, for example in struct pps_state, 
> > and to replace sleep() and wakeup() by wait and signal? It
> is good idea if we don't want to use the first functions.
> 
> Ok, it seems the pps_event() could be called by interrupt service routines. 
> In this case, you cannot use a mutex and condition
> variables.
> You have to use a thread queue directly. Use the thread queue ISR lock for 
> mutual exclusion. Use _Thread_queue_Enqueue() to
> emulate sleep() and
> _Thread_queue_Flush_critical() to emulate wakeup(). Check all critical 
> sections that they can be protected by an ISR lock (no blocking
> calls and short).

Is this even so if pps_event() calls signal or broadcast? Because pps_fetch() 
is the only function that is waiting (and locking a mutex).

> 
> >
> >> All the uses of sleep() and wakeup() need to be clearly identified and if 
> >> possible avoided.
> >
> > May I ask, what is the reason for avoiding them?
> 
> The sleep() and wakeup() synchronization is nice, but not suitable for 
> real-time systems (use of hash tables).
> 
> >
> > What do you think about coping the functions lmax()/qmin() and the define 
> > for ENOIOCTL? For lmax()/qmin() I saw that the file
> where are defined (libkern.h) is in libnetworking. Not sure what it is the 
> best option here, I don't like when the things are duplicated.
> 
> Since you refer to libnetworking I guess you work with RTEMS 5. Please first 
> get it working on RTEMS 6 and then we think about a back
> port.

Yes, I'm working with RTEMS 5 but my next step is port the changes to RTEMS 6 
and then submit the patches.

> 
> >
> > What about tvtohz(), which is defined in 
> > rtemsbsd/rtems/rtems-kernel-timesupport.c?

tvtohz() is used for converting timeval to hz, which is required by msleep(), 
msleep_spin() and tsleep but because they are not used, tvtohz() is not needed.

> 
> Maybe just copy the dependencies to kern_tc.c. Later we can try to move them 
> to shared locations if necessary.

I did this for lmax(), qmin() and ENOIOCTL.

Thanks for your answers.
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: Dependencies of PPS API in rtems-libbsd

[Gabriel.Moyano]

Hello Sebastian,

> On 15/03/2022 16:31, gabriel.moy...@dlr.de wrote:
> > I'm working on enabling PPS support in RTEMS
> 
> does this mean you want to define PPS_SYNC for kern_tc.c and kern_ntptime.c 
> in RTEMS?

yes

> I guess you want to enable tc_poll_pps in struct timecounter as well?

I didn't plan to do that but it can be done just removing some #ifndef, right? 

> For what do you need the sleep() and wakeup() support? Is this only used by 
> the RFC 2783 PPS-API implementation?

Yes, they are required by pps_fetch() and pps_event()
 
> If you want to keep implement this in RTEMS, then you can convert this to use 
> a condition variable from  or
> .

Do you mean to add a condition variable, for example in struct pps_state, and 
to replace sleep() and wakeup() by wait and signal? It is good idea if we don't 
want to use the first functions.

> All the uses of sleep() and wakeup() need to be clearly identified and if 
> possible avoided.

May I ask, what is the reason for avoiding them?

What do you think about coping the functions lmax()/qmin() and the define for 
ENOIOCTL? For lmax()/qmin() I saw that the file where are defined (libkern.h) 
is in libnetworking. Not sure what it is the best option here, I don't like 
when the things are duplicated.

What about tvtohz(), which is defined in 
rtemsbsd/rtems/rtems-kernel-timesupport.c?

Best regards,
Gabriel


___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Dependencies of PPS API in rtems-libbsd

[Gabriel.Moyano]

Hi everybody,

I'm working on enabling PPS support in RTEMS (actually it is already running 
and I'm preparing the commits to send them) but have couple of questions about 
functions/macros that are defined in rtems-libbsd repository. Since the 
repository where the changes need to be applied is just the rtems repo, I'm 
wondering what is the best way to add those dependencies.

Let me mention which functions/macros I'm talking, where they can be found and 
then I'll comment what I did.

The functions are:
int tvtohz(struct timeval *tv); found in 
rtemsbsd/rtems/rtems-kernel-timesupport.c
void wakeup(void *chan); found in freebsd/sys/sys/systm.h
long lmax(long a, long b); found in freebsd/sys/sys/libkern.h
quad_t qmin(quad_t a, quad_t b); found in 
freebsd/sys/sys/libkern.h

Some macros that define new functions based on _sleep():
msleep()
msleep_spin()
tsleep()
These macros can be found in freebsd/sys/sys/systm.h

And another macro:
ENOIOCTL; found in rtemsbsd/include/machine/rtems-bsd-kernel-space.h

Some easy solution is to copy the definition in the file where is needed. This 
was done for lmax(), qmin() and ENOIOCTL.
In case of tvtohz() and wakeup() can be used a pointer to function. For doing 
the same with msleep(), msleep_spin() and tsleep(), they need to be defined as 
a function first; or a maybe a pointer to _sleep() can be used and then the 
definition of the macros copied (I did the first).

The next question is when/where to set these pointers. I thought that a good 
place where this can happen is while rtems_bsd_initialize() is called. For 
doing this a kernel module and some macros in rtems-bsd-config.h need to be 
added.

Please let me know if there is a better way to do these things.
Thanks in advance.

Gabriel

--
Deutsches Zentrum für Luft- und Raumfahrt e.V. (DLR)
Institute for Software Technology | SC-SRV-OSS | Lilienthalplatz 7 | 38108 
Braunschweig  | Germany

Gabriel Moyano | Research Scientist in Onboard Software Systems group








___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: [PATCH 1/2] kern_ntptime.c: Import from FreeBSD

[Gabriel.Moyano]

> +#ifdef PPS_SYNC
> +SYSCTL_INT(_kern_ntp_pll, OID_AUTO, pps_shiftmax, CTLFLAG_RW,
> +_shiftmax, 0, "Max interval duration (sec) (shift)");
> +SYSCTL_INT(_kern_ntp_pll, OID_AUTO, pps_shift, CTLFLAG_RW,
> +_shift, 0, "Interval duration (sec) (shift)");
> +SYSCTL_LONG(_kern_ntp_pll, OID_AUTO, time_monitor, CTLFLAG_RD,
> +_monitor, 0, "Last time offset scaled (ns)");
> +
> +SYSCTL_S64(_kern_ntp_pll, OID_AUTO, pps_freq, CTLFLAG_RD | CTLFLAG_MPSAFE,
> +_freq, 0,
> +"Scaled frequency offset (ns/sec)"); SYSCTL_S64(_kern_ntp_pll,
> +OID_AUTO, time_freq, CTLFLAG_RD | CTLFLAG_MPSAFE,
> +_freq, 0,
> +"Frequency offset (ns/sec)");
> +#endif
> +

Is there anyway to port these SYSCTLs to rtems?
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: [PATCH 0/2] Add NTP support

[Gabriel.Moyano]

Is there anyone who would like to have NTP support in RTEMS 5? Just wondering 
if it makes sense to port it back to RTEMS 5
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: AW: [PATCH 4/4] kern_ntptime: sys_ntp_adjtime() does not return error when modes is 0 or MOD_TAI

[Gabriel.Moyano]

> >> What is the reason for this patch?
> >>
> > This change is something I would like to have a second opinion. So, thanks 
> > for the question.
> >
> > Originally at the end of sys_ntp_adjtime(), if the function copyout() 
> > doesn't return an error, the value returned by
> ntp_is_time_error(), which variable name is retval, is saved in 
> td->td_retval[0]. Chris modified this, being retval saved in the variable
> error, which is the one returned by sys_ntp_adjtime().
> 
> In libbsd, copyout() is just a memcpy() which returns 0.
> 
> >
> > Under some situations, ntp_adjtime() is called with modes = 0 just for 
> > retrieving some values (for example
> herehttps://github.com/ptpd/ptpd/blob/master/src/dep/sys.c#L2036, please 
> notice that here adjtimex() is ntp_adjtime()). In those
> cases, returning retval may not be meaningful. Other similar situation 
> happens when mode is MOD_TAI
> (https://github.com/ptpd/ptpd/blob/master/src/dep/sys.c#L2070).
> >
> > This is more or less the reason. If there is better way to solve this, is 
> > welcome.
> 
> I don't think the changes make sense. The td->td_retval[0] is the return 
> value of the system call if it is successful.  The "error" indicates
> if the system call was successful (== 0), otherwise it returns -1 with errno 
> set to "error".
> 
So, do you also propose to remove the line "error = retval;" ? I think it is 
better keeping the return value meaning of sys_ntp_adjtime(), i.e. just 
returning 0 (successful) or -1 (in case of error)
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: [PATCH 0/4] NTP support (master)

[Gabriel.Moyano]

> Hello Gabriel,
> I reviewed the patch set and the kern_ntptime.c. Changes in FreeBSD in 
> kern_ntptime.c are infrequently. The module is pretty self
> contained. I suggest to import this file into the RTEMS sources due to the 
> coupling with kern_tc.c which is already present in RTEMS.

Sounds good.
There still some functions needed by PPS API which are in rtems-libbsd. So it 
is still required to set some pointers to function before running a 
synchronization using PPS.

> The ntp_update_second() is called by _Timecounter_Windup() under protection 
> of the _Timecounter_Lock. I don't think we need the
> extra ntp_lock.
>
> I will work on a patch which includes the current kern_ntptime.c to RTEMS. It 
> should be ready by end of next week.
> 
> Do you have plans to enable the PPS_SYNC option?
> 

Yes.  Actually, I have something working.
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: [PATCH 4/4] kern_ntptime: sys_ntp_adjtime() does not return error when modes is 0 or MOD_TAI

[Gabriel.Moyano]

> 
> What is the reason for this patch?
> 
This change is something I would like to have a second opinion. So, thanks for 
the question.

Originally at the end of sys_ntp_adjtime(), if the function copyout() doesn't 
return an error, the value returned by ntp_is_time_error(), which variable name 
is retval, is saved in td->td_retval[0]. Chris modified this, being retval 
saved in the variable error, which is the one returned by sys_ntp_adjtime().

Under some situations, ntp_adjtime() is called with modes = 0 just for 
retrieving some values (for example here 
https://github.com/ptpd/ptpd/blob/master/src/dep/sys.c#L2036, please notice 
that here adjtimex() is ntp_adjtime()). In those cases, returning retval may 
not be meaningful. Other similar situation happens when mode is MOD_TAI 
(https://github.com/ptpd/ptpd/blob/master/src/dep/sys.c#L2070). 

This is more or less the reason. If there is better way to solve this, is 
welcome.
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: AW: AW: [PATCH 00/27] Update kern_tc in rtems 5 for ntp support

[Gabriel.Moyano]

> > > >> I am fine with this change being pushed to the 5 branch but I think it 
> > > >> needs to built with the tier 1 archs (i386, powerpc, arm).
> > > >
> > > > I could compile them for the BSPs of those archs.
> > >
> > > Thanks.
> > >
> > > > Does it make sense to run some test in qemu?
> > >
> > > Test results are always welcome.
> > >
> > I've run the following tests:
> > - sptimecounter02, 03 and 04 are pass on pc686 and Xilinx_zynq_a9.
> > - sptimecounter01 doesn't print something since everything happens inside 
> > of boot_card(). How the result of this test is verified?
> 
> Re-run the test using rtems-test.
> - sptimecounter01, 02, 03 and 04 are marked as pass on Xilinx_zynq_a9 and psim
> - sptimecounter02, 03 and 04 are marked as pass on pc686
> - sptimecounter01 is marked as invalid on pc686. This result is with and 
> without the patch set. I guess it is because nothing is printed
> out.
> 
> > Are there any other tests that can be interested to run?
> 
> If there isn't any other test to run, I would say that the patch set is ready.

After running the whole testsuite for pc686, xilinx_zynq_a9 and psim; there are 
no new failures introduced.

Results for pc686:

Passed:561
Failed:  5
User Input:  6
Expected Fail:   0
Indeterminate:   0
Benchmark:   3
Timeout:13
Invalid: 6
Wrong Version:   0
Wrong Build: 0
Wrong Tools: 0
--
Total: 594
Failures:
 dl06.exe
 psx12.exe
 psxfenv01.exe
 spfatal30.exe
 tmcontext01.exe
User Input:
 dl10.exe
 monitor.exe
 termios.exe
 top.exe
 capture.exe
 fileio.exe
Benchmark:
 dhrystone.exe
 linpack.exe
 whetstone.exe
Timeouts:
 dl05.exe
 psxintrcritical01.exe
 cdtest.exe
 cxx_iostream.exe
 spintrcritical06.exe
 spintrcritical07.exe
 spintrcritical11.exe
 spintrcritical12.exe
 spintrcritical13.exe
 spintrcritical14.exe
 spintrcritical15.exe
 spintrcritical18.exe
 spintrcritical24.exe
Invalid:
 spfatal09.exe
 spfatal12.exe
 spinternalerror01.exe
 spstkalloc02.exe
 spstkalloc03.exe
 sptimecounter01.exe

Results for psim:

Passed:567
Failed:  0
User Input:  6
Expected Fail:  16
Indeterminate:   0
Benchmark:   3
Timeout: 1
Invalid: 0
Wrong Version:   0
Wrong Build: 0
Wrong Tools: 0
--
Total: 593
User Input:
 dl10.exe
 monitor.exe
 termios.exe
 top.exe
 capture.exe
 fileio.exe
Expected Fail:
 fsimfsgeneric01.exe
 block11.exe
 rbheap01.exe
 termios01.exe
 ttest01.exe
 psx12.exe
 psxchroot01.exe
 psxfenv01.exe
 psximfs02.exe
 psxpipe01.exe
 spextensions01.exe
 spfatal31.exe
 spfifo02.exe
 spmountmgr01.exe
 spprivenv01.exe
 spstdthreads01.exe
Benchmark:
 linpack.exe
 dhrystone.exe
 whetstone.exe
Timeouts:
 fsrfsbitmap01.exe

Results for xilinx_zynq_a9:

Passed:572
Failed:  1
User Input:  6
Expected Fail:   2
Indeterminate:   0
Benchmark:   3
Timeout:10
Invalid: 0
Wrong Version:   0
Wrong Build: 0
Wrong Tools: 0
--
Total: 594
Failures:
 psx12.exe
User Input:
 dl10.exe
 monitor.exe
 termios.exe
 top.exe
 capture.exe
 fileio.exe
Expected Fail:
 dl06.exe
 psxfenv01.exe
Benchmark:
 whetstone.exe
 linpack.exe
 dhrystone.exe
Timeouts:
 psxintrcritical01.exe
 spintrcritical06.exe
 spintrcritical07.exe
 spintrcritical11.exe
 spintrcritical12.exe
 spintrcritical13.exe
 spintrcritical14.exe
 spintrcritical15.exe
 spintrcritical18.exe
 spintrcritical24.exe
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: AW: AW: [PATCH 00/27] Update kern_tc in rtems 5 for ntp support

[Gabriel.Moyano]

> > >> I am fine with this change being pushed to the 5 branch but I think it 
> > >> needs to built with the tier 1 archs (i386, powerpc, arm).
> > >
> > > I could compile them for the BSPs of those archs.
> >
> > Thanks.
> >
> > > Does it make sense to run some test in qemu?
> >
> > Test results are always welcome.
> >
> I've run the following tests:
> - sptimecounter02, 03 and 04 are pass on pc686 and Xilinx_zynq_a9.
> - sptimecounter01 doesn't print something since everything happens inside of 
> boot_card(). How the result of this test is verified?

Re-run the test using rtems-test.
- sptimecounter01, 02, 03 and 04 are marked as pass on Xilinx_zynq_a9 and psim
- sptimecounter02, 03 and 04 are marked as pass on pc686
- sptimecounter01 is marked as invalid on pc686. This result is with and 
without the patch set. I guess it is because nothing is printed out.

> Are there any other tests that can be interested to run?

If there isn't any other test to run, I would say that the patch set is ready.
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: AW: AW: [PATCH 00/27] Update kern_tc in rtems 5 for ntp support

[Gabriel.Moyano]

> >> I am fine with this change being pushed to the 5 branch but I think it 
> >> needs to built with the tier 1 archs (i386, powerpc, arm).
> >
> > I could compile them for the BSPs of those archs.
> 
> Thanks.
> 
> > Does it make sense to run some test in qemu?
> 
> Test results are always welcome.
> 
I've run the following tests: 
- sptimecounter02, 03 and 04 are pass on pc686 and Xilinx_zynq_a9.
- sptimecounter01 doesn't print something since everything happens inside of 
boot_card(). How the result of this test is verified?

Are there any other tests that can be interested to run?

For ppc, what bsp and configuration can be used with qemu? (is there some 
example showing how to run qemu for ppc?)


___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: AW: [PATCH 00/27] Update kern_tc in rtems 5 for ntp support

[Gabriel.Moyano]

> I am fine with this change being pushed to the 5 branch but I think it needs 
> to built with the tier 1 archs (i386, powerpc, arm).

I could compile them for the BSPs of those archs. Does it make sense to run 
some test in qemu?

> Once pushed I would appreciate an update to:
> 
> https://git.rtems.org/rtems-release/tree/rtems-notes-5.md
> 
> under "## RTEMS 5.2 Release Notes".
> 
> Thanks
> Chris
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: [PATCH 00/27] Update kern_tc in rtems 5 for ntp support

[Gabriel.Moyano]

Hi Sebastian,

Thx for your quick answer.

> > These commits port to rtems 5 the last changes in kern_tc and timecounter 
> > pushed by Sebastian Huber.
> > Additionally the last commit closes the ticket 4549, which is a clone of 
> > 2348(NTP support) for rtems 5.
> 
>  From my point of view this patch set acceptable for RTEMS 5 provided you 
> tested it with at least one BSP and libbsd.

Yes, I tested it using the bsp pc686 and since I'm working in including ptpd 
into rtems-libbsd, the handler ntp_update_second is set from libbsd.

> The patch set should have no API/ABI impact on applications.

Sorry I didn't verified if the ABI is broken. We are checking this with my team.

Best regards,
Gabriel



___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Question about ticket 2349

[Gabriel.Moyano]

Hi Sebastian,

In my team we are wondering what is required to add PPS support and since 
you've added the ticket (https://devel.rtems.org/ticket/2349), you may be the 
first person to ask about.

Thanks in advance.

Best regards,
Gabriel

--
Deutsches Zentrum für Luft- und Raumfahrt e.V. (DLR)
Institute for Software Technology | SC-SRV-OSS | Lilienthalplatz 7 | 38108 
Braunschweig  | Germany

Gabriel Moyano | Research Scientist in Onboard Software Systems group
gabriel.moy...@dlr.de
DLR.de

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

commit hash of rtems-libbsd

[Gabriel.Moyano]

Hi everyone,

is there any macro defined while compiling rtems-libbsd with the commit hash of 
the current commit?

Thx!
Gabriel
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: question about posix timer expiration

[Gabriel.Moyano]

Hi Chris,

Did you compile ptpd with kqueue only? I’ve tried with posix timer, which are 
implemented in rtems, but it seems that the handler with siginfo_t is not 
called correctly. I’ve managed to get it working using a workaround but I’ll 
continue with kqueue, thx!.

Regards,
Gabriel


Von: Chris Johns 
Gesendet: Mittwoch, 19. Mai 2021 10:30
An: Moyano Heredia, Victor Gabriel 
Cc: devel@rtems.org
Betreff: Re: question about posix timer expiration




On 18 May 2021, at 10:02 pm, 
gabriel.moy...@dlr.de wrote:

From further investigation I’ve found that when a timer expires, 
_POSIX_Timer_TSR() is called and this uses pthread_kill() to send a signal. 
Shouldn’t ptimer->inf.sigev_value be used here in order to create a siginfo_t 
object with the right value?

Did anyone have to deal with this while using posix timers?

Thanks!

Von: devel mailto:devel-boun...@rtems.org>> Im Auftrag 
von gabriel.moy...@dlr.de
Gesendet: Mittwoch, 12. Mai 2021 14:37
An: devel@rtems.org
Betreff: question about posix timer expiration

Hello everyone,

Currently I am trying to get running ptpd on rtems (following the Chris’ work, 
thx btw).

Ptpd uses posix timers and when a timer expires a handler with this signature 
void(int sig, siginfo_t *info, void *ucontext) is called.

Is this on libbsd? If it is I would have expected kqueue being used so I am not 
sure where this is happening.

Chris


 Unfortunately (*info) doesn’t have the right values, eg. info->si_code should 
be SI_TIMER but it is SI_USER and also info->si_signo should be the same as sig 
but it’s not.



Diving into the code I’ve found where the handler is called (also where *info 
is created). This is done in the function _POSIX_signals_Check_signal() in the 
file psignalunblockthread.c. It doesn’t seems that info is updated with the 
values (I guess) should take from object ptimer, which was created by 
create_timer().



My question is: is this handler fully supported? Or maybe I’m missing something 
else.



Thanks in advance,

Gabriel

——
Deutsches Zentrum für Luft- und Raumfahrt e.V. (DLR)
Institute for Software Technology | SC-OSS | Lilienthalplatz 7 | 38108 
Braunschweig  | Germany

Gabriel Moyano | Research Scientist in Onboard Software Systems group
gabriel.moy...@dlr.de
DLR.de

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: question about posix timer expiration

[Gabriel.Moyano]

>From further investigation I've found that when a timer expires, 
>_POSIX_Timer_TSR() is called and this uses pthread_kill() to send a signal. 
>Shouldn't ptimer->inf.sigev_value be used here in order to create a siginfo_t 
>object with the right value?

Did anyone have to deal with this while using posix timers?

Thanks!

Von: devel  Im Auftrag von gabriel.moy...@dlr.de
Gesendet: Mittwoch, 12. Mai 2021 14:37
An: devel@rtems.org
Betreff: question about posix timer expiration

Hello everyone,

Currently I am trying to get running ptpd on rtems (following the Chris' work, 
thx btw).

Ptpd uses posix timers and when a timer expires a handler with this signature 
void(int sig, siginfo_t *info, void *ucontext) is called. Unfortunately (*info) 
doesn't have the right values, eg. info->si_code should be SI_TIMER but it is 
SI_USER and also info->si_signo should be the same as sig but it's not.



Diving into the code I've found where the handler is called (also where *info 
is created). This is done in the function _POSIX_signals_Check_signal() in the 
file psignalunblockthread.c. It doesn't seems that info is updated with the 
values (I guess) should take from object ptimer, which was created by 
create_timer().



My question is: is this handler fully supported? Or maybe I'm missing something 
else.



Thanks in advance,

Gabriel

--
Deutsches Zentrum für Luft- und Raumfahrt e.V. (DLR)
Institute for Software Technology | SC-OSS | Lilienthalplatz 7 | 38108 
Braunschweig  | Germany

Gabriel Moyano | Research Scientist in Onboard Software Systems group
gabriel.moy...@dlr.de
DLR.de

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

question about posix timer expiration

[Gabriel.Moyano]

Hello everyone,

Currently I am trying to get running ptpd on rtems (following the Chris' work, 
thx btw).

Ptpd uses posix timers and when a timer expires a handler with this signature 
void(int sig, siginfo_t *info, void *ucontext) is called. Unfortunately (*info) 
doesn't have the right values, eg. info->si_code should be SI_TIMER but it is 
SI_USER and also info->si_signo should be the same as sig but it's not.



Diving into the code I've found where the handler is called (also where *info 
is created). This is done in the function _POSIX_signals_Check_signal() in the 
file psignalunblockthread.c. It doesn't seems that info is updated with the 
values (I guess) should take from object ptimer, which was created by 
create_timer().



My question is: is this handler fully supported? Or maybe I'm missing something 
else.



Thanks in advance,

Gabriel

--
Deutsches Zentrum für Luft- und Raumfahrt e.V. (DLR)
Institute for Software Technology | SC-OSS | Lilienthalplatz 7 | 38108 
Braunschweig  | Germany

Gabriel Moyano | Research Scientist in Onboard Software Systems group
gabriel.moy...@dlr.de
DLR.de

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: [PATCH 0/1] grlib/genirq: Issue when enabling/disabling interrupt

[Gabriel.Moyano]

>Yes, rtems 5 also has it. I can create a ticket.

Here is the link to the ticket https://devel.rtems.org/ticket/4385#ticket.
BTW the proposed solution for the master branch can be also applied to 5 as 
well.

>Regarding the proposed solution, I wanted to start a thread for discussing it 
>(maybe there is better was to do it).
>
>
>>Is this a bug in rtems 5 also?
>>
>>If so, does it warrant a back-port fix and a ticket?
>>
>>On Mon, Apr 12, 2021 at 3:16 AM Moyano, Gabriel  wrote:
>>>
>>> Hello everyone,
>>>
>>> I've found what can be an issue in the function genirq_set_active(): under 
>>> some conditions it can return a value greater than 1.
>>>
>>> This function is used by genirq_enable() and genirq_disable() and both of 
>>> them returns the value returned by genirq_set_active(). According to the 
>>> documentation in genirq.h, they should return -1, 0 or 1.
>>>
>>> When this issue can happen? If there are 3 entries in the list of IRQ and 2 
>>> of them are already enabled, the variable `enabled` would be 2, because of 
>>> `enabled += isrentry->enabled`.
>>>
>>> As a possible solution, the value of `enabled` can changed to 1 if it's 
>>> greater than 1 (see the patch) or maybe improve the search.
>>>
>>> Thanks in advance,
>>>
>>> Gabriel Moyano
>>>
>>>
>>>
>>> Moyano, Gabriel (1):
>>>   grlib/genirq: Taking into account that it could be more than one ISR
>>> enabled/disabled
>>>
>>>  bsps/shared/grlib/irq/genirq.c | 5 +
>>>  1 file changed, 5 insertions(+)
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: [PATCH 0/1] grlib/genirq: Issue when enabling/disabling interrupt

[Gabriel.Moyano]

Yes, rtems 5 also has it. I can create a ticket.

Regarding the proposed solution, I wanted to start a thread for discussing it 
(maybe there is better was to do it).


>Is this a bug in rtems 5 also?
>
>If so, does it warrant a back-port fix and a ticket?
>
>On Mon, Apr 12, 2021 at 3:16 AM Moyano, Gabriel  wrote:
>>
>> Hello everyone,
>>
>> I've found what can be an issue in the function genirq_set_active(): under 
>> some conditions it can return a value greater than 1.
>>
>> This function is used by genirq_enable() and genirq_disable() and both of 
>> them returns the value returned by genirq_set_active(). According to the 
>> documentation in genirq.h, they should return -1, 0 or 1.
>>
>> When this issue can happen? If there are 3 entries in the list of IRQ and 2 
>> of them are already enabled, the variable `enabled` would be 2, because of 
>> `enabled += isrentry->enabled`.
>>
>> As a possible solution, the value of `enabled` can changed to 1 if it's 
>> greater than 1 (see the patch) or maybe improve the search.
>>
>> Thanks in advance,
>>
>> Gabriel Moyano
>>
>>
>>
>> Moyano, Gabriel (1):
>>   grlib/genirq: Taking into account that it could be more than one ISR
>> enabled/disabled
>>
>>  bsps/shared/grlib/irq/genirq.c | 5 +
>>  1 file changed, 5 insertions(+)
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: [PATCH 0/2] Import ehci_pci

[Gabriel.Moyano]

Sorry, I have forgotten to mention that this patch is for the master

-Ursprüngliche Nachricht-
Von: Moyano Heredia, Victor Gabriel 
Gesendet: Montag, 22. Februar 2021 11:53
An: devel@rtems.org
Cc: Moyano Heredia, Victor Gabriel 
Betreff: [PATCH 0/2] Import ehci_pci

Import ehci_pci from freebsd-org using freebsd-to-rtems.py

Moyano, Gabriel (2):
  ehci_pci: Import from freebsd-org
  ehci_pci: Add to build system

 freebsd/sys/dev/usb/controller/ehci_pci.c | 593 ++
 freebsd/sys/dev/usb/usb_pci.h |  43 ++
 libbsd.py |   2 +
 3 files changed, 638 insertions(+)
 create mode 100644 freebsd/sys/dev/usb/controller/ehci_pci.c
 create mode 100644 freebsd/sys/dev/usb/usb_pci.h

-- 
2.17.1

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: [PATCH 0/2] ehci_pci - ticket #4263

[Gabriel.Moyano]

Sorry, I have forgotten to mention that this patch is for 5-freebsd-12

-Ursprüngliche Nachricht-
Von: Moyano Heredia, Victor Gabriel 
Gesendet: Montag, 22. Februar 2021 11:49
An: devel@rtems.org
Cc: Moyano Heredia, Victor Gabriel 
Betreff: [PATCH 0/2] ehci_pci - ticket #4263

Import ehci_pci from freebsd-org.
Solve https://devel.rtems.org/ticket/4263

Moyano, Gabriel (2):
  ehci_pci: Import from freebsd-org
  ehci_pci: Add to build system

 freebsd/sys/dev/usb/controller/ehci_pci.c | 592 ++
 freebsd/sys/dev/usb/usb_pci.h |  43 ++
 libbsd.py |   2 +
 3 files changed, 637 insertions(+)
 create mode 100644 freebsd/sys/dev/usb/controller/ehci_pci.c
 create mode 100644 freebsd/sys/dev/usb/usb_pci.h

-- 
2.17.1

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Porting ptpd to rtems

[Gabriel.Moyano]

Hi Chris,

I hope you are doing well.

Last year I asked about running ptpd on RTEMS 
https://lists.rtems.org/pipermail/devel/2020-September/062208.html.
May I ask about the last state of this?

Thanks in advance,
Gabriel

--
Deutsches Zentrum für Luft- und Raumfahrt e.V. (DLR)
Institute for Software Technology | SC-OSS | Lilienthalplatz 7 | 38108 
Braunschweig  | Germany

Gabriel Moyano | Research Scientist in Onboard Software Systems group
DLR.de

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: AW: [PATCH 1/3] Remove duplicate GRETH driver

[Gabriel.Moyano]

> Hi Jiri,
>
> My understanding was that one driver version was meant to be used 
> with
>>> drvmgr (greth.c) and the other without it (greth2.c). May I ask why 
>>> do you've chosen to remove greth.c and not greth2.c?
>>>
>>> I have fixed-up the greth.c file to avoid inline SPARC assembly code, 
>>> but the file is not used even when RTEMS is compiled with 
>>> --enable-drvmgr. The problem is that both greth2.c and greth.c are 
>>> compiled, and as they define the same symbols, greth2.c is pulled in 
>>> first by chance.
 
I think the symbols of greth.c are linked into the final binary when 
CONFIGURE_DRIVER_AMBAPP_GAISLER_GRETH is defined (drvmgr_confdefs.h).
I hope this helps

>>>I need to disable the building of the network files 
>>> in bsps/shared/shared-sources.am when -- enable-drvmgr is defined. 
>>> Does anyone know how to do this? My skills in m4 etc. are limited ... 
>>> :-(
>>>
>> If 99% of the code are the same, would it be an option to have just one 
>> driver implementation and in the drvmgr just use a wrapper for the driver?

>This is my idea to, but the driver manager code is sprinkled out in the file 
>so it might take quite a few >ifdefs to fix. In any case, I still need to fix 
>the m4 macros to detect if driver manager is defined or not ...


>>
>> Best regards,
>>
>> Jan
>>
 The problem I had was that greth.c contains SPARC assembly code and 
 cannot be built on any other architecture. I will change my patch to 
 disable greth.c on non-SPARC targets or try to replace the asssembly 
 code with macros as in greth2.c. Thanks for the feedback!

 An other issue is that the two files are 99% identical, but only 
 greth,c seems to be maintained. PHY handling and multi-cast support 
 are areas where the files have diverged. But this is an other discussion 
 ...
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

AW: [PATCH 1/3] Remove duplicate GRETH driver

[Gabriel.Moyano]

Hi Jiri,

My understanding was that one driver version was meant to be used with drvmgr 
(greth.c) and the other without it (greth2.c). May I ask why do you've chosen 
to remove greth.c and not greth2.c?

Thanks,
Gabriel

-Ursprüngliche Nachricht-
Von: devel  Im Auftrag von Jiri Gaisler
Gesendet: Sonntag, 25. Oktober 2020 23:26
An: devel@rtems.org
Betreff: [PATCH 1/3] Remove duplicate GRETH driver

* bsps/shared/net/greth2.c is being used instead
---
 bsps/shared/grlib-sources.am  |4 -
 bsps/shared/grlib/net/README  |7 -
 bsps/shared/grlib/net/greth.c | 1655 -
 bsps/shared/grlib/net/network_interface_add.c |   62 -
 4 files changed, 1728 deletions(-)
 delete mode 100644 bsps/shared/grlib/net/README
 delete mode 100644 bsps/shared/grlib/net/greth.c
 delete mode 100644 bsps/shared/grlib/net/network_interface_add.c

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

RE: legacy stack or libbsd

[Gabriel.Moyano]

>> Hi Chris,
>> 
>>> I will get back around to it. There are 2 sets of changes. In PTP itself a
>>> change is to add kqueue support because select in libbsd does not support
>>> signals. The PTP code runs a number of timers and they run of an alarm 
>>> signal.
>>> The other piece of support is to the score to bring in the FreeBSD kern_ntp
>>> support to sit besides the timercounters.
>> 
>> May I ask what are the necessary tasks to get kern_ntp support? We are also 
>> interested on getting running PTP on RTEMS.
>
>Thank you for your interest and I am sorry I did not have time to answer the
>question.
>
>I will try and take a look next week to see if I can get the RTEMS side of the
>changes in a state they can posted for review.

Thanks!
Let me know if I can help with something

Gabriel
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

RE: legacy stack or libbsd

[Gabriel.Moyano]

Hi Chris,

>I will get back around to it. There are 2 sets of changes. In PTP itself a
>change is to add kqueue support because select in libbsd does not support
>signals. The PTP code runs a number of timers and they run of an alarm signal.
>The other piece of support is to the score to bring in the FreeBSD kern_ntp
>support to sit besides the timercounters.

May I ask what are the necessary tasks to get kern_ntp support? We are also 
interested on getting running PTP on RTEMS.

Best regards,
Gabriel


--
Deutsches Zentrum für Luft- und Raumfahrt e.V. (DLR)
Institute for Software Technology | SC-OSS | Lilienthalplatz 7 | 38108 
Braunschweig  | Germany

Gabriel Moyano | Research Scientist in Onboard Software Systems group
gabriel.moy...@dlr.de
DLR.de



___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

RE: legacy stack or libbsd

[Gabriel.Moyano]

>On 5/8/20 2:13 pm, Gedare Bloom wrote:
>> On Tue, Aug 4, 2020 at 6:41 PM Chris Johns  wrote:
>
>>> On 5/8/20 10:23 am, Chris Johns wrote:
 On 5/8/20 2:04 am, Heinz Junkes wrote:
> Because the libbsd stack does not support some things yet (e.g. ntp)
>
 I have PTP running on libbsd. I currently have no time (ha) to clean up 
 the work
>> Is it precisely no time? ;)
>
>About +/- 0, I am not sure which! 
>
 and create a patch. It requires some extra support in the score.
>
>> Is the hacked approach available, or you will get around to it eventually?
>
>I will get back around to it. There are 2 sets of changes. In PTP itself a
>change is to add kqueue support because select in libbsd does not support
>signals. The PTP code runs a number of timers and they run of an alarm signal.
>The other piece of support is to the score to bring in the FreeBSD kern_ntp
>support to sit besides the timercounters.
>
>There will be some unresolved issues like RTC driver support. I have this
>commented out as I do not need it.
>
>I would like to upstream the code however adding kqueue will trip that 
>interface
>on FreeBSD and may be others so I am not sure how much work that will create.

Hi Chris,

We are also interested on getting running PTP on RTEMS. We can get this done in 
a joint effort.
Please let me know how we can help.

Best regards,
Gabriel


--
Deutsches Zentrum für Luft- und Raumfahrt e.V. (DLR)
Institute for Software Technology | SC-OSS | Lilienthalplatz 7 | 38108 
Braunschweig  | Germany

Gabriel Moyano | Research Scientist in Onboard Software Systems group
gabriel.moy...@dlr.de
DLR.de


___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

RE: busdma: question about header

[Gabriel.Moyano]

Thank you very much Sebastian!

-Original Message-
From: Sebastian Huber [mailto:sebastian.hu...@embedded-brains.de] 
Sent: Mittwoch, 5. August 2020 15:07
To: Moyano Heredia, Victor Gabriel; devel@rtems.org
Subject: Re: busdma: question about header

Hello Gabriel,

this should be fixed now.

-- 
Sebastian Huber, embedded brains GmbH
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

busdma: question about header

[Gabriel.Moyano]

Hello,

After one of the last changes on the file rtems-kernel-bus-dma.c, commit 
5e3780023 on branch 6-freebsd-12, it is not possible to compile the tests for 
i386 because  is not found. How is the best way to solve 
this issue? Adding an #ifndef __i386__? Something like this:

#ifndef __i386__
  #include 
#endif

Maybe this issue affects others architectures too.

Cheers,
Gabriel

--
Deutsches Zentrum für Luft- und Raumfahrt e.V. (DLR)
Institute for Software Technology | SC-OSS | Lilienthalplatz 7 | 38108 
Braunschweig  | Germany

Gabriel Moyano | Research Scientist in Onboard Software Systems group


___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

RE: Command line for Qemu and libbsd for pc386

[Gabriel.Moyano]

Hi Joel,

Before running qemu, some qtabs should be created. How to do that it is 
described on file libbsd.txt (line 333). Then I usually run qemu as follows:

qemu-system-i386 -append --console=/dev/com1 -no-reboot -serial stdio -monitor 
none -nographic -netdev tap,ifname=qtap1,script=no,downscript=no,id=n1 –device 
e1000,netdev=n1,mac=52:55:00:d1:55:01 -kernel path/to/exe/file

Please note that if you are running two instances, the second one should have 
different MAC address and  ifname, e.g. mac=52:55:00:d1:55:02 and ifname=qtab2

If hope this helps.

Cheers,
Gabriel

From: devel [mailto:devel-boun...@rtems.org] On Behalf Of Joel Sherrill
Sent: Mittwoch, 24. Juni 2020 19:23
To: rtems-de...@rtems.org
Subject: Command line for Qemu and libbsd for pc386

Hi

Does someone have a qemu command line handy for running libbsd network 
applications on Qemu?

I have lots of notes and old examples but can't seem to trip the right 
combination today.

Thanks.

--joel
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

RE: [PATCH v2 0/1] testsuite: A description for each test added

[Gabriel.Moyano]

Hi everyone,

I am wondering why this patch was not accepted. Is there anything bad or 
missing? (I was following this 
https://www.mail-archive.com/devel@rtems.org/msg23478.html)

-Original Message-
From: Moyano Heredia, Victor Gabriel 
Sent: Freitag, 3. April 2020 09:35
To: devel@rtems.org
Cc: Moyano Heredia, Victor Gabriel
Subject: [PATCH v2 0/1] testsuite: A description for each test added

I've moved the description before the copyright header and added some brief 
tags where needed.

Moyano, Gabriel (1):
  testsuite: A description for each test added

 testsuite/arphole/test_main.c|  6 ++--
 testsuite/cdev01/test_main.c |  7 +
 testsuite/commands01/test_main.c |  7 +
 testsuite/condvar01/test_main.c  |  7 +
 testsuite/crypto01/test_main.c   |  6 
 testsuite/debugger01/test_main.c |  6 
 testsuite/dhcpcd01/test_main.c   |  8 +
 testsuite/dhcpcd02/test_main.c   |  6 
 testsuite/epoch01/test_main.c|  8 +
 testsuite/evdev01/init.c |  6 
 testsuite/foobarclient/test_main.c   | 10 ++
 testsuite/foobarserver/test_main.c   | 12 
 testsuite/ftpd01/test_main.c |  6 
 testsuite/ftpd02/test_main.c |  6 
 testsuite/init01/test_main.c |  4 ++-
 testsuite/ipsec01/test_main.c|  8 +
 testsuite/lagg01/test_main.c |  8 +
 testsuite/log01/test_main.c  |  6 
 testsuite/loopback01/test_main.c |  9 --
 testsuite/media01/test_main.c|  8 +
 testsuite/mutex01/test_main.c|  6 
 testsuite/netshell01/test_main.c | 11 ---
 testsuite/nfs01/test_main.c  |  6 
 testsuite/openssl01/test_main.c  |  6 
 testsuite/openssl02/test_main.c  |  7 +
 testsuite/pf01/test_main.c   |  6 
 testsuite/pf02/test_main.c   |  8 +
 testsuite/ping01/test_main.c |  6 
 testsuite/ppp01/test_main.c  | 26 
 testsuite/program01/test_main.c  |  6 
 testsuite/rcconf01/test_main.c   |  8 +
 testsuite/rcconf02/test_main.c   | 52 +---
 testsuite/rwlock01/test_main.c   |  6 
 testsuite/selectpollkqueue01/test_main.c |  6 
 testsuite/sleep01/test_main.c| 10 ++
 testsuite/smp01/test_main.c  |  8 +
 testsuite/swi01/init.c   |  8 +
 testsuite/syscalls01/test_main.c |  6 
 testsuite/telnetd01/test_main.c  |  6 
 testsuite/termios/test_main.c|  6 
 testsuite/termios01/test_main.c  |  8 +
 testsuite/termios02/test_main.c  |  9 +-
 testsuite/termios03/test_main.c  |  8 -
 testsuite/termios04/test_main.c  |  8 -
 testsuite/termios05/test_main.c  |  8 -
 testsuite/termios06/test_main.c  |  7 +
 testsuite/thread01/test_main.c   |  9 ++
 testsuite/timeout01/init.c   |  6 
 testsuite/unix01/test_main.c |  6 
 testsuite/usb01/init.c   |  8 +
 testsuite/usbkbd01/init.c|  8 +
 testsuite/usbmouse01/init.c  | 13 +---
 testsuite/usbserial01/init.c |  8 +
 testsuite/vlan01/test_main.c |  6 
 testsuite/zerocopy01/test_main.c |  8 +
 55 files changed, 413 insertions(+), 55 deletions(-)

-- 
2.12.3

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

RE: [PATCH 3/3] testsuite/dhcpd0x: Tests automatized

[Gabriel.Moyano]

Hi Sebastian,

Sorry to bother you for this again. 

When you say "mark a test as interactive", do you mean to add 
TEST_STATE_USER_INPUT in the test?

Regards,
Gabriel

-Original Message-
From: devel [mailto:devel-boun...@rtems.org] On Behalf Of gabriel.moy...@dlr.de
Sent: Dienstag, 31. März 2020 11:58
To: sebastian.hu...@embedded-brains.de; devel@rtems.org
Subject: RE: [PATCH 3/3] testsuite/dhcpd0x: Tests automatized

Is this meaning to leave them unchanged? (or there is a define for this?)

-Original Message-
From: Sebastian Huber [mailto:sebastian.hu...@embedded-brains.de] 
Sent: Dienstag, 31. März 2020 11:46
To: Moyano Heredia, Victor Gabriel; devel@rtems.org
Subject: Re: [PATCH 3/3] testsuite/dhcpd0x: Tests automatized

On 31/03/2020 11:44, gabriel.moy...@dlr.de wrote:

> If both tests are interactive, should both of them include "the user input" 
> define? Otherwise RTEMS tester will be waiting until the timeout.
I would mark them as interactive for now.
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

RE: [PATCH 3/3] testsuite/dhcpd0x: Tests automatized

[Gabriel.Moyano]

Is this meaning to leave them unchanged? (or there is a define for this?)

-Original Message-
From: Sebastian Huber [mailto:sebastian.hu...@embedded-brains.de] 
Sent: Dienstag, 31. März 2020 11:46
To: Moyano Heredia, Victor Gabriel; devel@rtems.org
Subject: Re: [PATCH 3/3] testsuite/dhcpd0x: Tests automatized

On 31/03/2020 11:44, gabriel.moy...@dlr.de wrote:

> If both tests are interactive, should both of them include "the user input" 
> define? Otherwise RTEMS tester will be waiting until the timeout.
I would mark them as interactive for now.
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

RE: [PATCH 3/3] testsuite/dhcpd0x: Tests automatized

[Gabriel.Moyano]

If both tests are interactive, should both of them include "the user input" 
define? Otherwise RTEMS tester will be waiting until the timeout.

-Original Message-
From: Sebastian Huber [mailto:sebastian.hu...@embedded-brains.de] 
Sent: Dienstag, 31. März 2020 11:35
To: Moyano Heredia, Victor Gabriel; devel@rtems.org
Subject: Re: [PATCH 3/3] testsuite/dhcpd0x: Tests automatized

On 31/03/2020 11:30, gabriel.moy...@dlr.de wrote:

> The short answer to your question (Do we see the environment in the test 
> output after this change?) is no.  I understood that the idea of the test is 
> to verify that the hook is called. If this is not correct, the change can be 
> removed.
> Let me ask you if you consider useful the change in testsuite/dhcpcd02.

This test is only to see if the DHCP client works basically and what 
information is provided by the server. I think this is more an 
interactive test. You can also plug/unplug the cable and see what happens.

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

RE: [PATCH 3/3] testsuite/dhcpd0x: Tests automatized

[Gabriel.Moyano]

Hi Sebastian,

The short answer to your question (Do we see the environment in the test output 
after this change?) is no.  I understood that the idea of the test is to verify 
that the hook is called. If this is not correct, the change can be removed. 
Let me ask you if you consider useful the change in testsuite/dhcpcd02.

Regarding the path file, this could be changed to only "/".

Best regards,
Gabriel

-Original Message-
From: Sebastian Huber [mailto:sebastian.hu...@embedded-brains.de] 
Sent: Dienstag, 31. März 2020 11:12
To: Moyano Heredia, Victor Gabriel; devel@rtems.org
Subject: Re: [PATCH 3/3] testsuite/dhcpd0x: Tests automatized


On 31/03/2020 10:57, Moyano, Gabriel wrote:
> diff --git a/testsuite/dhcpcd01/test_main.c b/testsuite/dhcpcd01/test_main.c
> index 358b4ac8..f04c3270 100644
> --- a/testsuite/dhcpcd01/test_main.c
> +++ b/testsuite/dhcpcd01/test_main.c
> @@ -34,6 +34,7 @@
>   
>   #include 
>   #include 
> +#include 
>   
>   #define TEST_NAME "LIBBSD DHCPCD 1"
>   
> @@ -42,11 +43,17 @@ dhcpcd_hook_handler(rtems_dhcpcd_hook *hook, char *const 
> *env)
>   {
>   
>   (void)hook;
> + int fd;
> +
> + fd = open("/var/hook_out", O_CREAT | O_WRONLY,
> + S_IRWXU | S_IRWXG | S_IRWXO);
>   
>   while (*env != NULL) {
> - printf("%s\n", *env);
> + dprintf(fd, "%s\n", *env);
Do we see the environment in the test output after this change? What 
happens if "/var" doesn't exist? I would do the synchronization with the 
main thread with an event or semaphore instead of a file.
>   ++env;
>   }
> +
> + close(fd);
>   }
>   
>   static rtems_dhcpcd_hook dhcpcd_hook = {
> @@ -57,11 +64,22 @@ static rtems_dhcpcd_hook dhcpcd_hook = {
>   static void
>   test_main(void)
>   {
> + int fd;
>   
> + // Add hook
>   rtems_dhcpcd_add_hook(_hook);
What is the purpose of this comment? For a function with the name 
rtems_dhcpcd_add_hook() it should be obvious that it adds a hook.
>   
> - rtems_task_delete(RTEMS_SELF);
> - assert(0);
> + // Verify whether an output file is created by the hook
Please use the FreeBSD coding style.
> + while(true)
> + {
> + fd = open("/var/hook_out", O_RDONLY);
> + if (fd >= 0)
> + {
> + close(fd);
> + exit(0);
> + }
> + rtems_task_wake_after(RTEMS_MILLISECONDS_TO_TICKS(1000));
> + }
>   }
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: Description of rtems-libbsd testsuite applications

[Gabriel.Moyano]

Hello Sebastian,

I followed your recommendations regarding rtems_test_begin(), rtems_test_end() 
and RTEMS_TEST_STATE_USER_INPUT. Now I'm working on getting running the 
rtems-libbsd testsuit with RTEMS Tester.

I would like to ask you 2 questions:


1.   About arphole: In the arp_processor() function there is an "if" 
statement which verify 3 conditions. One of them is that spa (source protocol 
address) should be equal to INADDR_ANY which is 0.0.0.0 and this never happens. 
From my understanding, if the spa could be any address there is no need to test 
this condition. Is this right?



2.   About dhcpcd01 and dhcpcd02: RTEMS Tester gives a timeout error as a 
result. I think this is because no exit() call happens. Dhcpcd creates a file 
under /var/db/dhcpcd_.lease after obtaining a lease. Maybe this could 
be used as exit condition in test_main() in case of dhcpcd02. A similar 
solutions could be implemented for the dhcp hook in dhcpcd01 but instead of 
printing out the environment variables, writing them into a file and checking 
whether this file exists in test_main(). What do you think?

Please let me know if you have any suggestion.

Thanks for your help,
Gabriel

--
Deutsches Zentrum für Luft- und Raumfahrt e.V. (DLR)
Simulation and Software Technology | Lilienthalplatz 7 | 38108 Braunschweig  | 
Germany

Dipl.-Ing Gabriel Moyano | Research Scientist in Onboard Software Systems group
DLR.de

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel