RT-Socket-CAN driver for EMS CPC PCI card

PATCH In review Wolfgang Grandegger 2 participants 3 messages Aug 22, 2007 – Aug 24, 2007

lore

Share & tools

lore

Wolfgang Grandegger

author

2007-08-22 10:58 UTC #0 lore raw

PATCH 1/1 RT-Socket-CAN driver for EMS CPC PCI card

Hello,

the following patch changes:

2007-08-22 Wolfgang Grandegger <[email protected]>

* ksrc/drivers/can/sja1000: Add the RT-Socket-CAN SJA1000 driver rtcan_ems_pci.c for the EMS CPC PCI card from EMS Dr. Thomas Wuensche (http://www.ems-wuensche.de).

I would like to apply it to Xenomai's trunk and v2.3.x branch.

Wolfgang.

Modules.frag +1 -0

Index: scripts/Modules.frag
===================================================================
@@ -20,4 +20,5 @@ DRIVERS-$(CONFIG_XENO_DRIVERS_CAN_SJA100
 DRIVERS-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PEAK_PCI) += drivers/xenomai/can/sja1000/xeno_can_peak_pci.o
 DRIVERS-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PEAK_DNG) += drivers/xenomai/can/sja1000/xeno_can_peak_dng.o
 DRIVERS-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_IXXAT_PCI) += drivers/xenomai/can/sja1000/xeno_can_ixxat_pci.o
+DRIVERS-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_EMS_PCI) += drivers/xenomai/can/sja1000/xeno_can_ems_pci.o

ChangeLog +6 -0

Index: ChangeLog
===================================================================
@@ -1,3 +1,9 @@
+2007-08-22  Wolfgang Grandegger  <[email protected]>
+
+	* ksrc/drivers/can/sja1000: Add the RT-Socket-CAN SJA1000 driver
+	rtcan_ems_pci.c for the EMS CPC PCI card from EMS Dr. Thomas
+	Wuensche (http://www.ems-wuensche.de).
+
 2007-08-18  Philippe Gerum  <[email protected]>
 
 	* RELEASE: Xenomai 2.4-rc2

drivers/can/sja1000/Kconfig +10 -0

Index: ksrc/drivers/can/sja1000/Kconfig
===================================================================
@@ -42,6 +42,16 @@ config XENO_DRIVERS_CAN_SJA1000_IXXAT_PC
 	must be enabled.
 
 
+config XENO_DRIVERS_CAN_SJA1000_EMS_PCI
+	depends on XENO_DRIVERS_CAN_SJA1000
+	tristate "EMS CPC PCI Card"
+	help
+
+	This driver is for the 2 channel CPC PCI card from EMS Dr. Thomas
+	Wünsche (http://www.ems-wuensche.de). To get the second channel
+	working, Xenomai's shared interrupt support must be enabled.
+
+
 config XENO_DRIVERS_CAN_SJA1000_PEAK_DNG
 	depends on XENO_DRIVERS_CAN_SJA1000
 	tristate "PEAK Parallel Port Dongle"

drivers/can/sja1000/rtcan_ems_pci.c +345 -0

345 lines not on this page · raw message

drivers/can/sja1000/Config.in +1 -0

Index: ksrc/drivers/can/sja1000/Config.in
===================================================================
@@ -15,6 +15,7 @@ dep_tristate '  Memory mapped controller
 if [ "$CONFIG_PCI" = "y" ]; then
 	dep_tristate '  PEAK PCI cards' CONFIG_XENO_DRIVERS_CAN_SJA1000_PEAK_PCI $CONFIG_XENO_DRIVERS_CAN_SJA1000
 	dep_tristate '  IXXAT PCI cards' CONFIG_XENO_DRIVERS_CAN_SJA1000_IXXAT_PCI $CONFIG_XENO_DRIVERS_CAN_SJA1000
+	dep_tristate '  EMS CPC PCI cards' CONFIG_XENO_DRIVERS_CAN_SJA1000_EMS_PCI $CONFIG_XENO_DRIVERS_CAN_SJA1000
 fi
 
 dep_tristate '  PEAK Parallel Port Dongle' CONFIG_XENO_DRIVERS_CAN_SJA1000_PEAK_DNG $CONFIG_XENO_DRIVERS_CAN_SJA1000

drivers/can/sja1000/Makefile +7 -0

Index: ksrc/drivers/can/sja1000/Makefile
===================================================================
@@ -8,6 +8,7 @@ obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000) +
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PEAK_PCI) += xeno_can_peak_pci.o
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PEAK_DNG) += xeno_can_peak_dng.o
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_IXXAT_PCI) += xeno_can_ixxat_pci.o
+obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_EMS_PCI) += xeno_can_ems_pci.o
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ISA) += xeno_can_isa.o
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_MEM) += xeno_can_mem.o

@@ -15,6 +16,7 @@ xeno_can_sja1000-y := rtcan_sja1000.o rt
 xeno_can_peak_pci-y := rtcan_peak_pci.o
 xeno_can_peak_dng-y := rtcan_peak_dng.o
 xeno_can_ixxat_pci-y := rtcan_ixxat_pci.o
+xeno_can_ems_pci-y := rtcan_ems_pci.o
 xeno_can_isa-y := rtcan_isa.o
 xeno_can_mem-y := rtcan_mem.o

@@ -28,6 +30,7 @@ obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000) +
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PEAK_PCI) += xeno_can_peak_pci.o 
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PEAK_DNG) += xeno_can_peak_dng.o 
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_IXXAT_PCI) += xeno_can_ixxat_pci.o
+obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_EMS_PCI) += xeno_can_ems_pci.o
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ISA) += xeno_can_isa.o 
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_MEM) += xeno_can_mem.o

@@ -37,6 +40,7 @@ xeno_can_sja1000-objs := rtcan_sja1000.o
 xeno_can_peak_pci-objs := rtcan_peak_pci.o
 xeno_can_peak_dng-objs := rtcan_peak_dng.o
 xeno_can_ixxat_pci-objs := rtcan_ixxat_pci.o
+xeno_can_ems_pci-objs := rtcan_ems_pci.o
 xeno_can_isa-objs := rtcan_isa.o
 xeno_can_mem-objs := rtcan_mem.o

@@ -58,6 +62,9 @@ xeno_can_peak_dng.o: $(xeno_can_peak_dng
 xeno_can_ixxat_pci.o: $(xeno_can_ixxat_pci-objs)
 	$(LD) -r -o $@ $(xeno_can_ixxat_pci-objs)
 
+xeno_can_ems_pci.o: $(xeno_can_ems_pci-objs)
+	$(LD) -r -o $@ $(xeno_can_ems_pci-objs)
+
 xeno_can_isa.o: $(xeno_can_isa-objs)
 	$(LD) -r -o $@ $(xeno_can_isa-objs)

Jan Kiszka

2007-08-24 18:54 UTC #1 lore raw

Quoted 4 lines · Wolfgang Grandegger

Hello,
the following patch changes:

Quoted 8 lines

2007-08-22 Wolfgang Grandegger <[email protected]>
* ksrc/drivers/can/sja1000: Add the RT-Socket-CAN SJA1000 driver rtcan_ems_pci.c for the EMS CPC PCI card from EMS Dr. Thomas Wuensche (http://www.ems-wuensche.de).
I would like to apply it to Xenomai's trunk and v2.3.x branch.

I few quick comments from my side:

- Could you submit new code already Lindent'ified? This keeps the effort bounded to convert the rest one day.

Quoted 4 lines

Index: ksrc/drivers/can/sja1000/rtcan_ems_pci.c
===================================================================
drivers/can/sja1000/rtcan_ems_pci.c +0 -0

...

Quoted 1 line

+#define EMS_PCI_SINGLE 0 /* this is a single channel device */

This define is unused - is this intended?

drivers/can/sja1000/rtcan_ems_pci.c line 167

Quoted 8 lines

drivers/can/sja1000/rtcan_ems_pci.c +8 -0

+static int rtcan_ems_pci_add_chan(struct pci_dev *pdev, int channel,
+				   struct rtcan_device **master_dev)
+{
+    struct rtcan_device *dev;
+    struct rtcan_sja1000 *chip;
+    struct rtcan_ems_pci *board;
+    unsigned long addr;
+    int ret, init_step = 1;

ret!=0 always means error here? In that case I find "err" a more telling variable name ("if (err) ..." -- personal flavour).

drivers/can/sja1000/rtcan_ems_pci.c line 238

Quoted 6 lines

drivers/can/sja1000/rtcan_ems_pci.c +6 -0

+    /* Register and setup interrupt handling */
+#ifdef CONFIG_XENO_OPT_SHIRQ_LEVEL
+    chip->irq_flags = RTDM_IRQTYPE_SHARED;
+#else
+    chip->irq_flags = 0;
+#endif

That #ifdef is redundant, just use RTDM_IRQTYPE_SHARED unconditionally.

drivers/can/sja1000/rtcan_ems_pci.c line 293

Quoted 13 lines

drivers/can/sja1000/rtcan_ems_pci.c +13 -0

+#ifdef CONFIG_XENO_OPT_SHIRQ_LEVEL
+    if ((ret = rtcan_ems_pci_add_chan(pdev, EMS_PCI_MASTER,
+				      &master_dev)))
+	    goto failure_cleanup;
+    if ((ret = rtcan_ems_pci_add_chan(pdev, EMS_PCI_SLAVE,
+				      &master_dev)))
+	    goto failure_cleanup;
+#else
+    printk("Shared interrupts not enabled, using single channel!\n");
+    if ((ret = rtcan_ems_pci_add_chan(pdev, EMS_PCI_MASTER,
+				      &master_dev)))
+	    goto failure_cleanup;
+#endif

The master registration should be moved out off the #ifdef - it's identical.

Besides this, I have no concerns merging it, also into 2.3. Thanks for your effort!

Jan

Wolfgang Grandegger

author

2007-08-24 19:29 UTC #2 lore raw

Quoted 11 lines · Jan Kiszka

Quoted 10 lines · Wolfgang Grandegger
Hello,
the following patch changes:
2007-08-22 Wolfgang Grandegger <[email protected]>
* ksrc/drivers/can/sja1000: Add the RT-Socket-CAN SJA1000 driver rtcan_ems_pci.c for the EMS CPC PCI card from EMS Dr. Thomas Wuensche (http://www.ems-wuensche.de).

Quoted 7 lines

Quoted 2 lines
I would like to apply it to Xenomai's trunk and v2.3.x branch.
I few quick comments from my side:
- Could you submit new code already Lindent'ified? This keeps the effort bounded to convert the rest one day.

I was already thinking about it.

Quoted 8 lines

Quoted 4 lines
Index: ksrc/drivers/can/sja1000/rtcan_ems_pci.c
===================================================================
drivers/can/sja1000/rtcan_ems_pci.c +0 -0
...
Quoted 1 line
+#define EMS_PCI_SINGLE 0 /* this is a single channel device */
This define is unused - is this intended?

Well, there are more. I'm going to remove them.

Quoted 11 lines

drivers/can/sja1000/rtcan_ems_pci.c line 167
Quoted 8 lines
drivers/can/sja1000/rtcan_ems_pci.c +8 -0
+static int rtcan_ems_pci_add_chan(struct pci_dev *pdev, int channel,
+				   struct rtcan_device **master_dev)
+{
+    struct rtcan_device *dev;
+    struct rtcan_sja1000 *chip;
+    struct rtcan_ems_pci *board;
+    unsigned long addr;
+    int ret, init_step = 1;
ret!=0 always means error here? In that case I find "err" a more telling variable name ("if (err) ..." -- personal flavour).

Agreed.

Quoted 8 lines

drivers/can/sja1000/rtcan_ems_pci.c line 238
Quoted 6 lines
drivers/can/sja1000/rtcan_ems_pci.c +6 -0
+    /* Register and setup interrupt handling */
+#ifdef CONFIG_XENO_OPT_SHIRQ_LEVEL
+    chip->irq_flags = RTDM_IRQTYPE_SHARED;
+#else
+    chip->irq_flags = 0;
+#endif
That #ifdef is redundant, just use RTDM_IRQTYPE_SHARED unconditionally.

OK.

Quoted 7 lines

drivers/can/sja1000/rtcan_ems_pci.c +7 -0

+#ifdef CONFIG_XENO_OPT_SHIRQ_LEVEL
+    if ((ret = rtcan_ems_pci_add_chan(pdev, EMS_PCI_MASTER,
+				      &master_dev)))
+	    goto failure_cleanup;
+    if ((ret = rtcan_ems_pci_add_chan(pdev, EMS_PCI_SLAVE,
+				      &master_dev)))
+	    goto failure_cleanup;

Quoted 8 lines

drivers/can/sja1000/rtcan_ems_pci.c line 293
Quoted 6 lines
drivers/can/sja1000/rtcan_ems_pci.c +6 -0
+#else
+    printk("Shared interrupts not enabled, using single channel!\n");
+    if ((ret = rtcan_ems_pci_add_chan(pdev, EMS_PCI_MASTER,
+				      &master_dev)))
+	    goto failure_cleanup;
+#endif
The master registration should be moved out off the #ifdef - it's identical.

Yes, right.

Quoted 2 lines

Besides this, I have no concerns merging it, also into 2.3. Thanks for your effort!

OK, thanks for reviewing the code.

Wolfgang.