[PATCH v3 04/11] kern_tc.c: Replace FREEBSD event mechanism by adding pointers to function
Sebastian Huber
sebastian.huber at embedded-brains.de
Tue May 17 12:26:40 UTC 2022
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;
> unsigned ppscount[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&sektion=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 = 0x7fffffff;
> 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(&tv);
> }
> +#endif /* __rtems__ */
> aseq = atomic_load_int(&pps->ppsinfo.assert_sequence);
> cseq = atomic_load_int(&pps->ppsinfo.clear_sequence);
> while (aseq == atomic_load_int(&pps->ppsinfo.assert_sequence) &&
> cseq == atomic_load_int(&pps->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 guide.
> + (void) timeout;
> +
> + return 0;
return (0);
> +}
> +
> +static void
> +default_wakeup(struct pps_state *pps)
> +{
> + (void) pps;
> +}
> +#endif /* __rtems__ */
> void
> pps_init(struct pps_state *pps)
> {
> +#ifdef __rtems__
> + pps->wait = default_wait;
> + pps->wakeup = default_wakeup;
> +#endif /* __rtems__ */
> pps->ppscap |= PPS_TSFMT_TSPEC | PPS_CANWAIT;
> if (pps->ppscap & PPS_CAPTUREASSERT)
> pps->ppscap |= PPS_OFFSETASSERT;
> @@ -2227,7 +2260,12 @@ pps_event(struct pps_state *pps, int event)
> #endif
>
> /* Wakeup anyone sleeping in pps_fetch(). */
> +#ifndef __rtems__
> wakeup(pps);
> +#else /* __rtems__ */
> + _Assert(pps->wakeup != NULL);
> + (*pps->wakeup)(pps);
> +#endif /* __rtems__ */
> }
> #else /* __rtems__ */
> /* FIXME: https://devel.rtems.org/ticket/2349 */
--
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber at embedded-brains.de
phone: +49-89-18 94 741 - 16
fax: +49-89-18 94 741 - 08
Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/
More information about the devel
mailing list