[Xenomai-help] RT-Socket-CAN update, please read

[Xenomai-help] RT-Socket-CAN update, please read

[Wolfgang Grandegger]

[-- Attachment #1: Type: text/plain, Size: 430 bytes --]

Hello,

I have just commited various modifications and improvements for 
RT-Socket-CAN to the SVN trunk (v2.3-devel). The modification you might 
realize sooner than later is the renaming of the RT-Socket-CAN kernel 
modules from "xeno_rtcan_*" to "xeno_can_*" to comply with the usual 
RTDM driver naming scheme. I have attached the full ChangeLog entry.

Any feedback is welcome, also on the new TX-Loopback feature.

Wolfgang.

[-- Attachment #2: ChangeLog --]
[-- Type: text/plain, Size: 2227 bytes --]

2006-11-24  Wolfgang Grandegger  <wg@domain.hid>

	* ksrc/drivers/can/README: Update list of supported CAN controllers
	and boards.

	* 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/rtcan_virt.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,
	ksrc/drivers/can/sja1000/rtcan_sja1000.c,
	ksrc/drivers/can/mscan/rtcan_mscan.c: timestamps are now read and
	copied in rtcan_recv() and rtcan_tx_loopbcak().

	* 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_CAN_DEBUG is set.

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

	* ksrc/drivers/can/rtcan_virt.c (rtcan_virt_init): fix bug in
	cleanup code.

	* ksrc/drivers/can/sja1000/rtcan_sja1000.c,
	ksrc/drivers/can/sja1000/rtcan_isa.c,
	ksrc/drivers/can/sja1000/rtcan_mem.c,
	ksrc/drivers/can/sja1000/rtcan_peak_pci.c,
	ksrc/drivers/can/sja1000/rtcan_peak_dng.c: Remove rtcan_dev_free()
	from rtcan_sja1000_unregister() to allow proper cleanup after the
	device has been unregistered.

	* ksrc/drivers/can/sja1000/rtcan_ixxat_pci.c,
	ksrc/drivers/can/sja1000/Kconfig,
	ksrc/drivers/can/sja1000/Config.in,
	ksrc/drivers/can/sja1000/Makefile: SJA1000 driver for IXXAT PCI
	cards added.

	* src/drivers/Makefile, ksrc/drivers/can/*, scripts/Modules.frag,
	src/utils/can/README: Replace "rtcan" with "can" in macro definitions
	CONFIG_XENO_DRIVERS_RTCAN_* and module names xeno_rtcan_* to comply to
	the common naming scheme.

Re: [Xenomai-help] RT-Socket-CAN update, please read

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 776 bytes --]

Wolfgang Grandegger wrote:
> Hello,
> 
> I have just commited various modifications and improvements for
> RT-Socket-CAN to the SVN trunk (v2.3-devel). The modification you might
> realize sooner than later is the renaming of the RT-Socket-CAN kernel
> modules from "xeno_rtcan_*" to "xeno_can_*" to comply with the usual
> RTDM driver naming scheme. I have attached the full ChangeLog entry.

One wish for the future: if possible, please try to break up such large
commits into smaller pieces. They should be compilable on their own in
order to hunt potential issues (also with the help of users) on a more
fine-grained base by switching back and forth svn revisions. quilt can
help to manage such pieces, I'm using it even for private stuff.

Thanks,
Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]

Re: [Xenomai-help] RT-Socket-CAN update, please read

[Wolfgang Grandegger]

Jan Kiszka wrote:
> Wolfgang Grandegger wrote:
>> Hello,
>>
>> I have just commited various modifications and improvements for
>> RT-Socket-CAN to the SVN trunk (v2.3-devel). The modification you might
>> realize sooner than later is the renaming of the RT-Socket-CAN kernel
>> modules from "xeno_rtcan_*" to "xeno_can_*" to comply with the usual
>> RTDM driver naming scheme. I have attached the full ChangeLog entry.
> 
> One wish for the future: if possible, please try to break up such large
> commits into smaller pieces. They should be compilable on their own in
> order to hunt potential issues (also with the help of users) on a more
> fine-grained base by switching back and forth svn revisions. quilt can
> help to manage such pieces, I'm using it even for private stuff.

I know, I agree, but it makes more work and testing takes more time and 
"free" time" is rare :-(. Nevertheless, I did not change fundamental or 
critical things.

Wolfgang.

[Xenomai-core] Re: [Xenomai-help] RT-Socket-CAN update, please read

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2388 bytes --]

Hi Wolfgang,

some more comments after looking at code and running a compiler:

...
> 	* ksrc/drivers/can/rtcan_raw.c,
> 	ksrc/drivers/can/sja1000/rtcan_sja1000.c,
> 	ksrc/drivers/can/mscan/rtcan_mscan.c: timestamps are now read and
> 	copied in rtcan_recv() and rtcan_tx_loopbcak().

...thus multiple times in the same IRQ context if there are multiple
packets pending. Do we gain enough from the new design that helps to
compensate the drawback (considering slow rtdm_clock_read()
implementations)? Moreover, we may see a feature one day that the RTDM
layer will provide you timestamps for an IRQ event (e.g. when the
handler will be running at higher latency as a thread).

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

Suggestion: N x "depends on XYZ" => 1 x "if XZY != n ... endif"

...
> 	* ksrc/drivers/can/sja1000/rtcan_sja1000.c,
> 	ksrc/drivers/can/sja1000/rtcan_isa.c,
> 	ksrc/drivers/can/sja1000/rtcan_mem.c,
> 	ksrc/drivers/can/sja1000/rtcan_peak_pci.c,
> 	ksrc/drivers/can/sja1000/rtcan_peak_dng.c: Remove rtcan_dev_free()
> 	from rtcan_sja1000_unregister() to allow proper cleanup after the
> 	device has been unregistered.

[2.4.33 build for x86]
rtcan_peak_dng.c: In function `rtcan_peak_dng_exit_one':
rtcan_peak_dng.c:315: warning: implicit declaration of function
`rtcan_free_dev'

[Not related to this change]
In order to use the parport dongle also with recent PnP ports (e.g. in
notebooks), you may want to have a look at the code I added to irqbench
for this purpose:

http://www.rts.uni-hannover.de/xenomai/lxr/source/ksrc/drivers/testing/irqbench.c?v=SVN-trunk#496

The background is that once you unload the parport module, the port will
be powered off.

...
> 	* src/drivers/Makefile, ksrc/drivers/can/*, scripts/Modules.frag,
> 	src/utils/can/README: Replace "rtcan" with "can" in macro definitions
> 	CONFIG_XENO_DRIVERS_RTCAN_* and module names xeno_rtcan_* to comply to
> 	the common naming scheme.

Hmm, wouldn't shortening some file names also make sense then?

Jan


PS: 2.4 help patching and updating work nicely for me - except for
choices which is obviously a 2.4 bug. I saw that the kernel then
sometimes stuffs all information into the first item. Maybe scriptable
for Xenomai... :->


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]

[Xenomai-core] Re: [Xenomai-help] RT-Socket-CAN update, please read

[Wolfgang Grandegger]

Jan Kiszka wrote:
> Hi Wolfgang,
> 
> some more comments after looking at code and running a compiler:
> 
> ...
>> 	* ksrc/drivers/can/rtcan_raw.c,
>> 	ksrc/drivers/can/sja1000/rtcan_sja1000.c,
>> 	ksrc/drivers/can/mscan/rtcan_mscan.c: timestamps are now read and
>> 	copied in rtcan_recv() and rtcan_tx_loopbcak().
> 
> ...thus multiple times in the same IRQ context if there are multiple
> packets pending. Do we gain enough from the new design that helps to
> compensate the drawback (considering slow rtdm_clock_read()

I think you agree, that it's unlikely, that an RX interrupt is handled 
together with an TX done or an error interrupt. And we save reading the 
timestamp when there is just a TX done interrupt (without TX loopback, 
of course). It makes the driver dependent code simpler and cleaner.

> implementations)? Moreover, we may see a feature one day that the RTDM
> layer will provide you timestamps for an IRQ event (e.g. when the
> handler will be running at higher latency as a thread).

Then we need to modify the interface to the driver anyhow.

> ...
>> 	* ksrc/drivers/can/mscan/Kconfig, ksrc/drivers/can/sja1000/Kconfig,
>> 	ksrc/drivers/can/Kconfig: add more help for kernel parameters.
> 
> Suggestion: N x "depends on XYZ" => 1 x "if XZY != n ... endif"

OK, no priority though.

> ...
>> 	* ksrc/drivers/can/sja1000/rtcan_sja1000.c,
>> 	ksrc/drivers/can/sja1000/rtcan_isa.c,
>> 	ksrc/drivers/can/sja1000/rtcan_mem.c,
>> 	ksrc/drivers/can/sja1000/rtcan_peak_pci.c,
>> 	ksrc/drivers/can/sja1000/rtcan_peak_dng.c: Remove rtcan_dev_free()
>> 	from rtcan_sja1000_unregister() to allow proper cleanup after the
>> 	device has been unregistered.
> 
> [2.4.33 build for x86]
> rtcan_peak_dng.c: In function `rtcan_peak_dng_exit_one':
> rtcan_peak_dng.c:315: warning: implicit declaration of function
> `rtcan_free_dev'

Yes, I realized already this bug.

> [Not related to this change]
> In order to use the parport dongle also with recent PnP ports (e.g. in
> notebooks), you may want to have a look at the code I added to irqbench
> for this purpose:
> 
> http://www.rts.uni-hannover.de/xenomai/lxr/source/ksrc/drivers/testing/irqbench.c?v=SVN-trunk#496
> 
> The background is that once you unload the parport module, the port will
> be powered off.

Are there still laptops with parallel ports? Anyhow, I'm going to check 
that.

> ...
>> 	* src/drivers/Makefile, ksrc/drivers/can/*, scripts/Modules.frag,
>> 	src/utils/can/README: Replace "rtcan" with "can" in macro definitions
>> 	CONFIG_XENO_DRIVERS_RTCAN_* and module names xeno_rtcan_* to comply to
>> 	the common naming scheme.
> 
> Hmm, wouldn't shortening some file names also make sense then?

Well, we could remove "rtcan_", but I have not time to do it now.

> Jan
> 
> 
> PS: 2.4 help patching and updating work nicely for me - except for
> choices which is obviously a 2.4 bug. I saw that the kernel then
> sometimes stuffs all information into the first item. Maybe scriptable
> for Xenomai... :->

Yes, I observed the same behavior.

Wolfgang.