Re: AW: [PATCH 06/12] kern_tc.c: Replace FREEBSD event mechanism by adding pointers to function
On 07/04/2022 11:57, gabriel.moy...@dlr.de wrote:
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?
Yes, an _Assert() doesn't hurt.
Check for non-NULL in pps_init()? Since this function has no return
code, you need a fatal error. Alternatively, you could add an
RTEMS-specific API for PPS device drivers. It should not use a file
descriptor, just struct pps_state (maybe embedded in an RTEMS-specific
structure).
--
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.hu...@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/
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
Re: AW: [PATCH 06/12] kern_tc.c: Replace FREEBSD event mechanism by adding pointers to function
On 07/04/2022 11:56, gabriel.moy...@dlr.de wrote:
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
At the moment, they just pollute the namespace. I would remove them
until you have to store the callbacks in a variable.
Please don't change the formatting.
Do you mean that wait_event should be waitevent?
No, please use the FreeBSD coding style.
I would use "wait" and "wakeup", or "wait_for_event" and "send_event".
Please add comments how the callbacks are used and that they shall not
be NULL.
No changes outside the #ifdef __rtems__ markers please.
Sorry, could you point to the changes?
You add blank lines after the #endif.
--
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.hu...@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/
___
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
> 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 06/12] kern_tc.c: Replace FREEBSD event mechanism by adding pointers to function
> 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
