Jan Kiszka wrote:
Wolfgang Grandegger wrote:
Jan Kiszka wrote:
Wolfgang Grandegger wrote:
@@ -894,6 +937,12 @@
/* We got access */
+#ifdef CONFIG_XENO_DRIVERS_RTCAN_TX_LOOPBACK
+ /* Push message onto stack for loopback when TX done */
+ if (sock->tx_loopback)
+ rtcan_tx_push(dev, sock, frame);
+#endif /* CONFIG_XENO_DRIVERS_RTCAN_TX_LOOPBACK */
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 */
rtdm_sem_up(&dev->tx_sem);
+
+#ifdef CONFIG_XENO_DRIVERS_RTCAN_TX_LOOPBACK
+ if (dev->tx_socket) {
Same here. A macro like rtcan_tx_loopback_pending(dev) would avoid the
#ifdef.
...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>
__rtcan_tx_loopback(dev);
}
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?
I'm thinking about the TSC emulation on old or lowest-end x86 systems.
Also, converting the ticks into nanoseconds may take some cycles on slow
32 bit systems. So it might be a good idea to check - where possible -
if timestamps are needed.
I have now moved the reading and copying of the timestamp into the
common functions rtcan_recv() and rtcan_tx_loopback() as it is unlikely
that more than one interrupt source is handled by the ISR. A further
improvement would be to maintain a timestamp request count in struct
rtcan_device (it's on my TODO list).
Wolfgang.
Jan
_______________________________________________
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core
