Jan Kiszka wrote:
Wolfgang Grandegger wrote:
@@ -894,6 +937,12 @@
     /* We got access */
+    /* Push message onto stack for loopback when TX done */
+    if (sock->tx_loopback)
+    rtcan_tx_push(dev, sock, frame);
Hmm, I would prefer to define something like

if (rtcan_tx_loopback_enabled(sock))
    rtcan_tx_push(dev, sock, frame);

with rtcan_tx_loopback_enabled resolving to 0 in the configured-out
case. Thus some #ifdefs could be moved from the code to central places
in header files.
OK, here I can avoid the #ifdef's ...

Index: ksrc/drivers/can/mscan/rtcan_mscan.c
--- ksrc/drivers/can/mscan/rtcan_mscan.c    (revision 1834)
+++ ksrc/drivers/can/mscan/rtcan_mscan.c    (working copy)
@@ -251,6 +251,21 @@
     regs->cantier = 0;
     /* Wake up a sender */
+    if (dev->tx_socket) {
Same here. A macro like rtcan_tx_loopback_pending(dev) would avoid the
...but not here. Or does the optimizer remove the if block?

AFAIK, all relevant gcc versions kill blocks like

if (0) {

from the binary. But one problem might appear: the block content must be
compilable in any case...

I'm going to checks this.

I think the timestamp should rather be passed to rtcan_tc_loopback and
set there. Or, if passing that value increases the code size needlessly,
rtcan_tx_loopback should be defined like

static inline void
rtcan_tx_loopback(struct rtcan_device *dev, nanosecs_abs_t timestamp)
    <set timestamp>

where __rtcan_tx_loopback is the uninlined code. That makes the driver
code leaner. Locking will hopefully change anyway, so nothing to do on
this part for now.
Or do it directly in rtcan_tx_loopback() and rtcan_recv(). Would it be
OK to read the time in these functions as well or should it be done, as
is, at the beginning of the ISR? I have already planned some similar
rewrite when Xenomai 2.3. is out, hopefully together with the new locking.

A few cycles offset of the timestamp doesn't matter here. We should just
avoid reading it more than once as this can be a costly operation on
some systems.

Hm, what does "costly operation" mean? Does it make sense then to read the time only when really necessary e.g. not for pure TX done interrupts and if nobody request them?



Xenomai-core mailing list

Reply via email to