Re: [Xenomai-core] [PATCH] RT-Socket-CAN, TX loopback and further improvements

2006-11-15 Thread Wolfgang Grandegger

Jan Kiszka wrote:

Wolfgang Grandegger wrote:

Hi Jan,

Jan Kiszka wrote:

Hi Wolfgang,

[...deletions...]


Last remark: For the sake of completeness, tx-loopback should also be
implemented in the virtual CAN driver.

The virtual CAN driver already works like TX loopback. It routes TX
messages to local foreign sockets (but without sending them to any bus).


There is a minor difference: When application A is attached to a socket
on virtual CAN device 1, B to device 2, and C to 1 again, a message sent
by A will only be received by B. TX loopback as you are introducing it
should make C receive it as well - in order to have a fully compatible
test environment.


Ah, I missed the case (rx_dev == tx_dev). This should work then:

/* Deliver to all other devices on the virtual bus */
for (i = 0; i  devices; i++) {
rx_dev = rtcan_virt_devs[i];
if (rx_dev-state == CAN_STATE_ACTIVE) {
if (tx_dev != rx_dev) {
rx_frame-can_ifindex = rx_dev-ifindex;
rtcan_rcv(rx_dev, skb);
} else if (rtcan_tx_loopback_pending(tx_dev))
rtcan_tx_loopback(tx_dev);
}
}

Thanks.

Wolfgang.

___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core


[Xenomai-core] [PATCH] RT-Socket-CAN, TX loopback and further improvements

2006-11-14 Thread Wolfgang Grandegger

Hi Jan,

attached is a patch implementing the TX loopback, apart from some other 
fixes and improvements. The TX loopback to foreign local sockets allows 
to have a net-alike behavior of the CAN bus. Local sockets listening to 
a device can receive TX messages as well. As discussed, the TX lookback 
is performed when TX is done (TX done interrupt). As I still think that 
it will not be used very often, I made it a configurable kernel option. 
When it's selected, it's _on_ by default and it can be switched off or 
on again with setsockopt() (to make the Linux Socket-CAN people happy). 
As long as it's not enabled, it adds little overhead to the driver. If 
you have no objections I will check it in.


Here is the ChangeLog entry:

2006-11-14  Wolfgang Grandegger  [EMAIL PROTECTED]

* ksrc/drivers/can/rtcan_dev.h,
ksrc/drivers/can/rtcan_socket.h,
ksrc/drivers/can/rtcan_socket.c,
ksrc/drivers/can/rtcan_raw.c,
ksrc/drivers/can/rtcan_modules.c,
ksrc/drivers/can/sja1000/rtcan_sja1000.c,
ksrc/drivers/can/mscan/rtcan_mscan.c,
ksrc/drivers/can/Kconfig,
ksrc/drivers/can/Config.in,
src/utils/can/rtcansend.c,
include/rtdm/rtcan.h,: Add feature TX loopback to local sockets.

* ksrc/drivers/can/rtcan_raw.c, include/rtdm/rtcan.h:
Remove locks for the setting and reading of the RX and TX
timeout values and add a warning to the documentation.

* src/utils/rtcansend.c: use sendto() by default to avoid
binding a default filter and add option -s for using bind()
and send().

* src/utils/rtcanrecv.c: add option -R for relative
timestamps.

* ksrc/drivers/can/rtcan_internal.h: use now RTCAN_ASSERT macros
when CONFIG_XENO_DRIVERS_RTCAN_DEBUG is set.

* ksrc/drivers/can/mscan/Kconfig,
ksrc/drivers/can/sja1000/Kconfig,
ksrc/drivers/can/Kconfig: add more help for kernel parameters.

Thanks.

Wolfgang.







___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core


Re: [Xenomai-core] [PATCH] RT-Socket-CAN, TX loopback and further improvements

2006-11-14 Thread Wolfgang Grandegger

Attached is the missing patch!

Wolfgang Grandegger wrote:

Hi Jan,

attached is a patch implementing the TX loopback, apart from some other 
fixes and improvements. The TX loopback to foreign local sockets allows 
to have a net-alike behavior of the CAN bus. Local sockets listening to 
a device can receive TX messages as well. As discussed, the TX lookback 
is performed when TX is done (TX done interrupt). As I still think that 
it will not be used very often, I made it a configurable kernel option. 
When it's selected, it's _on_ by default and it can be switched off or 
on again with setsockopt() (to make the Linux Socket-CAN people happy). 
As long as it's not enabled, it adds little overhead to the driver. If 
you have no objections I will check it in.


Here is the ChangeLog entry:

2006-11-14  Wolfgang Grandegger  [EMAIL PROTECTED]

* ksrc/drivers/can/rtcan_dev.h,
ksrc/drivers/can/rtcan_socket.h,
ksrc/drivers/can/rtcan_socket.c,
ksrc/drivers/can/rtcan_raw.c,
ksrc/drivers/can/rtcan_modules.c,
ksrc/drivers/can/sja1000/rtcan_sja1000.c,
ksrc/drivers/can/mscan/rtcan_mscan.c,
ksrc/drivers/can/Kconfig,
ksrc/drivers/can/Config.in,
src/utils/can/rtcansend.c,
include/rtdm/rtcan.h,: Add feature TX loopback to local sockets.

* ksrc/drivers/can/rtcan_raw.c, include/rtdm/rtcan.h:
Remove locks for the setting and reading of the RX and TX
timeout values and add a warning to the documentation.

* src/utils/rtcansend.c: use sendto() by default to avoid
binding a default filter and add option -s for using bind()
and send().

* src/utils/rtcanrecv.c: add option -R for relative
timestamps.

* ksrc/drivers/can/rtcan_internal.h: use now RTCAN_ASSERT macros
when CONFIG_XENO_DRIVERS_RTCAN_DEBUG is set.

* ksrc/drivers/can/mscan/Kconfig,
ksrc/drivers/can/sja1000/Kconfig,
ksrc/drivers/can/Kconfig: add more help for kernel parameters.

Thanks.

Wolfgang.







___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core




Index: include/rtdm/rtcan.h
===
--- include/rtdm/rtcan.h	(revision 1834)
+++ include/rtdm/rtcan.h	(working copy)
@@ -120,6 +120,7 @@
  * - Level @b SOL_CAN_RAW : CAN RAW protocol (see @ref CAN_PROTO_RAW)
  *   - Option @ref CAN_RAW_FILTER : CAN filter list
  *   - Option @ref CAN_RAW_ERR_FILTER : CAN error mask
+ *   - Option @ref CAN_RAW_TX_LOOPBACK : CAN TX loopback to local sockets
  *   .
  * .
  * @n
@@ -543,11 +544,9 @@
  * - -EFAULT (It was not possible to access user space memory area at the
  *specified address.)
  * - -ENOMEM (Not enough memory to fulfill the operation)
- * - -EINVAL (Invalid address family, or invalid length of address structure)
- * - -ENODEV (Invalid CAN interface index)
- * - -EBADF  (Socket is about to be closed)
- * - -EAGAIN (Too many receivers. Old binding (if any) is still active.
- *Close some sockets and try again.)
+ * - -EINVAL (Invalid length optlen)
+ * - -ENOSPC (No space to store filter list, check RT-Socket-CAN kernel
+ *parameters)
  * .
  */
 #define CAN_RAW_FILTER  0x1
@@ -577,16 +576,39 @@
  * Specific return values:
  * - -EFAULT (It was not possible to access user space memory area at the
  *specified address.)
- * - -ENOMEM (Not enough memory to fulfill the operation)
- * - -EINVAL (Invalid address family, or invalid length of address structure)
- * - -ENODEV (Invalid CAN interface index)
- * - -EBADF  (Socket is about to be closed)
- * - -EAGAIN (Too many receivers. Old binding (if any) is still active.
- *Close some sockets and try again.)
+ * - -EINVAL (Invalid length optlen)
  * .
  */
 #define CAN_RAW_ERR_FILTER  0x2
 
+/**
+ * CAN TX loopback
+ *
+ * The TX loopback to other local sockets can be selected with this
+ * @c setsockopt.
+ *
+ * @note The TX loopback feature must be enabled in the kernel and then
+ * the loopback to other local TX sockets is enabled by default.
+ *
+ * @n
+ * @param [in] level @b SOL_CAN_RAW
+ *
+ * @param [in] optname @b CAN_RAW_TX_LOOPBACK
+ *
+ * @param [in] optval Pointer to integer value.
+ *
+ * @param [in] optlen Size of int: sizeof(int).
+ *
+ * Environments: non-RT (RT optional)@n
+ * @n
+ * Specific return values:
+ * - -EFAULT (It was not possible to access user space memory area at the
+ *specified address.)
+ * - -EINVAL (Invalid length optlen)
+ * - -EOPNOTSUPP (not supported, check RT-Socket-CAN kernel parameters).
+ */
+#define CAN_RAW_TX_LOOPBACK  0x3
+
 /** @} */
 
 /*!
@@ -933,6 +955,9 @@
  *
  * The default value for a newly created socket is an infinite timeout.
  *
+ * @note The setting of the timeout value is not done atomically to avoid
+ * locks. Please set the value before receiving messages 

Re: [Xenomai-core] [PATCH] RT-Socket-CAN, TX loopback and further improvements

2006-11-14 Thread Wolfgang Grandegger

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?


Wolfgang.


Jan




___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core


Re: [Xenomai-core] [PATCH] RT-Socket-CAN, TX loopback and further improvements

2006-11-14 Thread Jan Kiszka
Wolfgang Grandegger wrote:
 Hi Jan,
 
 Jan Kiszka wrote:
 Hi Wolfgang,
 [...deletions...]
 
 Last remark: For the sake of completeness, tx-loopback should also be
 implemented in the virtual CAN driver.
 
 The virtual CAN driver already works like TX loopback. It routes TX
 messages to local foreign sockets (but without sending them to any bus).

There is a minor difference: When application A is attached to a socket
on virtual CAN device 1, B to device 2, and C to 1 again, a message sent
by A will only be received by B. TX loopback as you are introducing it
should make C receive it as well - in order to have a fully compatible
test environment.

Jan

___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core