[Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter

[Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter

[Thierry Bultel]

Hi,

I have ported the adv_pci driver from 
http://git.toiit.sgu.ru/people/samarkinpa/public/?p=adv_pci.git;a=tree;h=a57c5d22415f8dbf92eeae0e05ff482839d8c2f3;hb=a57c5d22415f8dbf92eeae0e05ff482839d8c2f3

I has been quite intensively been tested and cycled for several days in 
real industrial conditions.

The patch for 2.6.38-8 is attached to this mail.

Is it of interest and does is have a chance to be integrated in the 
xenomai mainline ?

Thanks
Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: sja1000_adv_pci_driver.patch
Type: text/x-patch
Size: 13390 bytes
Desc: not available
URL: <http://www.xenomai.org/pipermail/xenomai/attachments/20120629/e7b587ed/attachment.bin>

Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter

[Evgeny Sinelnikov]

2012/6/29 Thierry Bultel <thierry.bultel@wanadoo.fr>:
> Hi,
>
> I have ported the adv_pci driver from
> http://git.toiit.sgu.ru/people/samarkinpa/public/?p=adv_pci.git;a=tree;h=a57c5d22415f8dbf92eeae0e05ff482839d8c2f3;hb=a57c5d22415f8dbf92eeae0e05ff482839d8c2f3
>
> I has been quite intensively been tested and cycled for several days in real
> industrial conditions.
>
> The patch for 2.6.38-8 is attached to this mail.

This is realy cool, thank you. Paul realized this driver as his
graduate work under my direction. I'm very glad to see that is used in
practice.

> Is it of interest and does is have a chance to be integrated in the xenomai
> mainline ?

This is very interesting for later research at our laboratory at
Saratov State University.

-- 
Sin (Sinelnikov Evgeny)
Etersoft

Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter

[Wolfgang Grandegger]

Hi Thierry,

On 06/29/2012 08:29 AM, Thierry Bultel wrote:
> Hi,
> 
> I have ported the adv_pci driver from
> http://git.toiit.sgu.ru/people/samarkinpa/public/?p=adv_pci.git;a=tree;h=a57c5d22415f8dbf92eeae0e05ff482839d8c2f3;hb=a57c5d22415f8dbf92eeae0e05ff482839d8c2f3

Thanks for your contribution. This driver is not yet mainline and it's
the first time that I see it. I added the Linux-CAN ML to the CC as it
might be of general interest.

> I has been quite intensively been tested and cycled for several days in
> real industrial conditions.
> 
> The patch for 2.6.38-8 is attached to this mail.
> 
> Is it of interest and does is have a chance to be integrated in the
> xenomai mainline ?

There is interest, of course. But we need a proper patch against the git
head of xenomai-2.6. At a first glance I realized the following issues:

- The patch does high-check and remove the plx_pci driver, please fix.

- What is Advantech specific in that driver? It looks pretty generic to
  me and it could easily support other similar devices.

- The Linux driver mentioned above does support much more Advantech
  devices. Any chance to support them in your driver as well.

- Please send patches *inline*.

- Please respect the Linux coding style (check CodingStyle and use
  checkpatch.pl).

Thanks in advance.

Wolfgang.

Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter

[Wolfgang Grandegger]

Hi Thierry,

On 06/29/2012 08:29 AM, Thierry Bultel wrote:
> Hi,
> 
> I have ported the adv_pci driver from
> http://git.toiit.sgu.ru/people/samarkinpa/public/?p=adv_pci.git;a=tree;h=a57c5d22415f8dbf92eeae0e05ff482839d8c2f3;hb=a57c5d22415f8dbf92eeae0e05ff482839d8c2f3

Thanks for your contribution. This driver is not yet mainline and it's
the first time that I see it. I added the Linux-CAN ML to the CC as it
might be of general interest.

> I has been quite intensively been tested and cycled for several days in
> real industrial conditions.
> 
> The patch for 2.6.38-8 is attached to this mail.
> 
> Is it of interest and does is have a chance to be integrated in the
> xenomai mainline ?

There is interest, of course. But we need a proper patch against the git
head of xenomai-2.6. At a first glance I realized the following issues:

- The patch does high-check and remove the plx_pci driver, please fix.

- What is Advantech specific in that driver? It looks pretty generic to
  me and it could easily support other similar devices.

- The Linux driver mentioned above does support much more Advantech
  devices. Any chance to support them in your driver as well.

- Please send patches *inline*.

- Please respect the Linux coding style (check CodingStyle and use
  checkpatch.pl).

Thanks in advance.

Wolfgang.

Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter

[Thierry BULTEL]

Hi Wolfgang,

 

Thanks for your feedback.

 

The issue is that I did not realize that the patch would not have been accepted as such, so I will have to invest more time on than expected ...

so I will not be able to do so immediately.

 

Several comments, to your comments


- I agree for the multiple devices, theoretically supported by the original Linux driver. I intentionnally removed them, since I do not have the appropriate hardware to test.

- Linux code style. I really thought I had respected it, furthermore I took another (ixxat) RTDM driver as a model. Ok, I can doublecheck ...

- patch high-check vs plx. I do no see what you mean. If I made something wrong it is not intentional. Can you be more specific please ?

- Avantech specific ... not much but the classical vendor ID, and also, PCI banks mapping. Again, it is just like the ixxat one, all the driver intelligence is in the generic sja1000 code
- patches inline ... Really ??? In the mail content ? But that makes huge mails. I can do so of course, but I do not see the point.

 

Cheers

Thierry



> Message du 29/06/12 20:39
> De : "Wolfgang Grandegger" 
> A : "Thierry Bultel" 
> Copie à : xenomai@xenomai.org, "Linux-CAN" 
> Objet : Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter
> 
> Hi Thierry,
> 
> On 06/29/2012 08:29 AM, Thierry Bultel wrote:
> > Hi,
> > 
> > I have ported the adv_pci driver from
> > http://git.toiit.sgu.ru/people/samarkinpa/public/?p=adv_pci.git;a=tree;h=a57c5d22415f8dbf92eeae0e05ff482839d8c2f3;hb=a57c5d22415f8dbf92eeae0e05ff482839d8c2f3
> 
> Thanks for your contribution. This driver is not yet mainline and it's
> the first time that I see it. I added the Linux-CAN ML to the CC as it
> might be of general interest.
> 
> > I has been quite intensively been tested and cycled for several days in
> > real industrial conditions.
> > 
> > The patch for 2.6.38-8 is attached to this mail.
> > 
> > Is it of interest and does is have a chance to be integrated in the
> > xenomai mainline ?
> 
> There is interest, of course. But we need a proper patch against the git
> head of xenomai-2.6. At a first glance I realized the following issues:
> 
> - The patch does high-check and remove the plx_pci driver, please fix.
> 
> - What is Advantech specific in that driver? It looks pretty generic to
> me and it could easily support other similar devices.
> 
> - The Linux driver mentioned above does support much more Advantech
> devices. Any chance to support them in your driver as well.
> 
> - Please send patches *inline*.
> 
> - Please respect the Linux coding style (check CodingStyle and use
> checkpatch.pl).
> 
> Thanks in advance.
> 
> Wolfgang.
>

Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter

[Gilles Chanteperdrix]

On 07/03/2012 09:27 PM, Thierry BULTEL wrote:
> - patches inline ... Really ??? In the mail content ? But that makes
> huge mails. I can do so of course, but I do not see the point.

This allows people to comment your patch by simply replying to your
mail. And tools like git-apply and git-am are made to apply patches from
mails.

The biggest problem I see with your patch is this hunk:
@@ -41,21 +41,12 @@
        the second channel working, Xenomai's shared interrupt support
        must be enabled.

-config XENO_DRIVERS_CAN_SJA1000_PLX_PCI
+config XENO_DRIVERS_CAN_SJA1000_ADV_PCI
        depends on XENO_DRIVERS_CAN_SJA1000 && PCI
-       tristate "PLX90xx PCI-bridge based Cards"
+       tristate "ADVANTECH PCI 168x Card"
        help

-       This driver is for CAN interface cards based on
-       the PLX90xx PCI bridge.
-       Driver supports now:
-        - Adlink PCI-7841/cPCI-7841 card (http://www.adlinktech.com/)
-        - Adlink PCI-7841/cPCI-7841 SE card
-        - esd CAN-PCI/CPCI/PCI104/200 (http://www.esd.eu/)
-        - esd CAN-PCI/PMC/266
-        - esd CAN-PCIe/2000
-        - Marathon CAN-bus-PCI card (http://www.marathon.ru/)
-        - TEWS TECHNOLOGIES TPMC810 card (http://www.tews.com/)
+       This driver is for the Advantech 1680.

 config XENO_DRIVERS_CAN_SJA1000_EMS_PCI
        depends on XENO_DRIVERS_CAN_SJA1000 && PCI

You remove the config option for XENO_DRIVERS_SJA1000_PLX_PCI to add the
option for the driver you use. We naturally want to have the two options.

-- 
                                                                Gilles.

Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter

[Wolfgang Grandegger]

Hi Thierry,

On 07/03/2012 09:27 PM, Thierry BULTEL wrote:
> Hi Wolfgang,
> 
>  
> 
> Thanks for your feedback.
> 
>  
> 
> The issue is that I did not realize that the patch would not have been accepted as such, so I will have to invest more time on than expected ...
> 
> so I will not be able to do so immediately.

No problem.

> Several comments, to your comments
> 
> 
> - I agree for the multiple devices, theoretically supported by the original Linux driver. I intentionnally removed them, since I do not have the appropriate hardware to test.

Well, your patch should just add *your* new driver. It should *not*
change unrelated things.

> - Linux code style. I really thought I had respected it, furthermore I took another (ixxat) RTDM driver as a model. Ok, I can doublecheck ...

Well, checkplatch.pl does report:

  total: 45 errors, 138 warnings, 427 lines checked

For further information please have a look to

http://lxr.linux.no/#linux+v3.4.4/Documentation/CodingStyle

> - patch high-check vs plx. I do no see what you mean. If I made something wrong it is not intentional. Can you be more specific please ?

-config XENO_DRIVERS_CAN_SJA1000_PLX_PCI
+config XENO_DRIVERS_CAN_SJA1000_ADV_PCI
 	depends on XENO_DRIVERS_CAN_SJA1000 && PCI
-	tristate "PLX90xx PCI-bridge based Cards"
+	tristate "ADVANTECH PCI 168x Card"


I mean "high-check" in the sense of "replacing".

> - Avantech specific ... not much but the classical vendor ID, and also, PCI banks mapping. Again, it is just like the ixxat one, all the driver intelligence is in the generic sja1000 code

It would be nice if the driver could also support other advantec and
non-advantec devices. This could be achieved by by a generic driver, I
believe.

> - patches inline ... Really ??? In the mail content ? But that makes huge mails. I can do so of course, but I do not see the point.

Yes, it makes reviewing the patches much easier. For further information
please read:

http://lxr.linux.no/#linux+v3.4.4/Documentation/SubmittingPatches

Wolfgang.

Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter

[Wolfgang Grandegger]

Hi Thierry,

On 07/03/2012 09:27 PM, Thierry BULTEL wrote:
> Hi Wolfgang,
> 
>  
> 
> Thanks for your feedback.
> 
>  
> 
> The issue is that I did not realize that the patch would not have been accepted as such, so I will have to invest more time on than expected ...
> 
> so I will not be able to do so immediately.

No problem.

> Several comments, to your comments
> 
> 
> - I agree for the multiple devices, theoretically supported by the original Linux driver. I intentionnally removed them, since I do not have the appropriate hardware to test.

Well, your patch should just add *your* new driver. It should *not*
change unrelated things.

> - Linux code style. I really thought I had respected it, furthermore I took another (ixxat) RTDM driver as a model. Ok, I can doublecheck ...

Well, checkplatch.pl does report:

  total: 45 errors, 138 warnings, 427 lines checked

For further information please have a look to

http://lxr.linux.no/#linux+v3.4.4/Documentation/CodingStyle

> - patch high-check vs plx. I do no see what you mean. If I made something wrong it is not intentional. Can you be more specific please ?

-config XENO_DRIVERS_CAN_SJA1000_PLX_PCI
+config XENO_DRIVERS_CAN_SJA1000_ADV_PCI
 	depends on XENO_DRIVERS_CAN_SJA1000 && PCI
-	tristate "PLX90xx PCI-bridge based Cards"
+	tristate "ADVANTECH PCI 168x Card"


I mean "high-check" in the sense of "replacing".

> - Avantech specific ... not much but the classical vendor ID, and also, PCI banks mapping. Again, it is just like the ixxat one, all the driver intelligence is in the generic sja1000 code

It would be nice if the driver could also support other advantec and
non-advantec devices. This could be achieved by by a generic driver, I
believe.

> - patches inline ... Really ??? In the mail content ? But that makes huge mails. I can do so of course, but I do not see the point.

Yes, it makes reviewing the patches much easier. For further information
please read:

http://lxr.linux.no/#linux+v3.4.4/Documentation/SubmittingPatches

Wolfgang.

Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter

[dietmar.schindler]

> From: xenomai-bounces@xenomai.org [mailto:xenomai-bounces@xenomai.org] On
> Behalf Of Wolfgang Grandegger
> Sent: Tuesday, July 03, 2012 10:30 PM
> ...
> I mean "high-check" in the sense of "replacing".

So, you mean "high-jack" (to prevent future incomprehension).

--
Dietmar

p. s.: It is not within my power to prevent my employer's note from being added to this message.
________________________________________
manroland web systems GmbH -- Managing Director: Uwe Lüders
Registered Office: Augsburg -- Trade Register: AG Augsburg -- HRB-No. 26816 -- VAT: DE281389840

Confidentiality note:
This eMail and any files transmitted with it are confidential and intended solely for the use of the individual or entity to whom they are addressed. If you are not the intended recipient, you are hereby notified that any use or dissemination of this communication is strictly prohibited. If you have received this eMail in error, please notify us immediately via info@manroland-web.com, then delete this eMail.

- Please consider your environmental responsibility before printing this eMail
________________________________________

Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter

[Wolfgang Grandegger]

On 07/04/2012 08:24 AM, dietmar.schindler@manroland.com wrote:
>> From: xenomai-bounces@xenomai.org [mailto:xenomai-bounces@xenomai.org] On
>> Behalf Of Wolfgang Grandegger
>> Sent: Tuesday, July 03, 2012 10:30 PM
>> ...
>> I mean "high-check" in the sense of "replacing".
> 
> So, you mean "high-jack" (to prevent future incomprehension).

Puh, of course. Thanks for clarification and sorry for the confusion.

Wolfgang.

Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter

[Thierry Bultel]

Hi Wolfgang,

How are you ?

I have taken some time, to take (almost) all your remarks into account 
about my candidate patch.
Please see below, the new version, inline.

- code style / checkpatch : all ERRORs fixed, it remains a spurious "80 
lines" WARNING that I do not agree with.
- multiple devices: The driver now (theoretically) supports as many 
devices as its original linux version
- plx highjack: fixed, this was a bug, actually.

The patch is generated vs xenomai-2.6.0, because it is my testing 
environnement. I will unfortunately and
unlikely not have time to perform some tests with a more recent version.
I hope this is not a blocking issue and that you will be able to 
integrate it anyway.

Another topic, please, I would like to know if you have an estimate date 
for the availability of the RTDM
drivers for Flexcan, and the IMX6 serial as well ?

Cheers,
Thierry


diff -pruN xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/Kconfig 
xenomai-2.6.0/ksrc/drivers/can/sja1000/Kconfig
--- xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/Kconfig    2011-10-18 
20:17:18.000000000 +0200
+++ xenomai-2.6.0/ksrc/drivers/can/sja1000/Kconfig    2012-07-16 
16:44:12.606904812 +0200
@@ -41,6 +41,14 @@ config XENO_DRIVERS_CAN_SJA1000_IXXAT_PC
      the second channel working, Xenomai's shared interrupt support
      must be enabled.

+config XENO_DRIVERS_CAN_SJA1000_ADV_PCI
+    depends on XENO_DRIVERS_CAN_SJA1000 && PCI
+    tristate "ADVANTECH PCI Card"
+    help
+
+    This driver is for the ADVANTECH PCI cards (1 or more channels)
+
+
  config XENO_DRIVERS_CAN_SJA1000_PLX_PCI
      depends on XENO_DRIVERS_CAN_SJA1000 && PCI
      tristate "PLX90xx PCI-bridge based Cards"
diff -pruN xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/Makefile 
xenomai-2.6.0/ksrc/drivers/can/sja1000/Makefile
--- xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/Makefile    2011-10-18 
20:17:18.000000000 +0200
+++ xenomai-2.6.0/ksrc/drivers/can/sja1000/Makefile    2012-07-16 
14:00:38.682836737 +0200
@@ -9,6 +9,7 @@ obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PE
  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PEAK_DNG) += xeno_can_peak_dng.o
  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PLX_PCI) += xeno_can_plx_pci.o
  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_IXXAT_PCI) += xeno_can_ixxat_pci.o
+obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ADV_PCI) += xeno_can_adv_pci.o
  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_EMS_PCI) += xeno_can_ems_pci.o
  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ESD_PCI) += xeno_can_esd_pci.o
  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ISA) += xeno_can_isa.o
@@ -19,6 +20,7 @@ xeno_can_peak_pci-y := rtcan_peak_pci.o
  xeno_can_peak_dng-y := rtcan_peak_dng.o
  xeno_can_plx_pci-y := rtcan_plx_pci.o
  xeno_can_ixxat_pci-y := rtcan_ixxat_pci.o
+xeno_can_adv_pci-y := rtcan_adv_pci.o
  xeno_can_ems_pci-y := rtcan_ems_pci.o
  xeno_can_esd_pci-y := rtcan_esd_pci.o
  xeno_can_isa-y := rtcan_isa.o
@@ -35,6 +37,7 @@ obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PE
  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PEAK_DNG) += xeno_can_peak_dng.o
  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PLX_PCI) += xeno_can_plx_pci.o
  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_IXXAT_PCI) += xeno_can_ixxat_pci.o
+obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ADV_PCI) += xeno_can_adv_pci.o
  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_EMS_PCI) += xeno_can_ems_pci.o
  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ESD_PCI) += xeno_can_esd_pci.o
  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ISA) += xeno_can_isa.o
@@ -47,6 +50,7 @@ xeno_can_peak_pci-objs := rtcan_peak_pci
  xeno_can_peak_dng-objs := rtcan_peak_dng.o
  xeno_can_plx_pci-objs := rtcan_plx_pci.o
  xeno_can_ixxat_pci-objs := rtcan_ixxat_pci.o
+xeno_can_adv_pci-objs := rtcan_adv_pci.o
  xeno_can_ems_pci-objs := rtcan_ems_pci.o
  xeno_can_esd_pci-objs := rtcan_esd_pci.o
  xeno_can_isa-objs := rtcan_isa.o
@@ -73,6 +77,9 @@ xeno_can_plx_pci.o: $(xeno_can_plx_pci-o
  xeno_can_ixxat_pci.o: $(xeno_can_ixxat_pci-objs)
      $(LD) -r -o $@ $(xeno_can_ixxat_pci-objs)

+xeno_can_adv_pci.o: $(xeno_can_adv_pci-objs)
+    $(LD) -r -o $@ $(xeno_can_adv_pci-objs)
+
  xeno_can_ems_pci.o: $(xeno_can_ems_pci-objs)
      $(LD) -r -o $@ $(xeno_can_ems_pci-objs)

diff -pruN xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/rtcan_adv_pci.c 
xenomai-2.6.0/ksrc/drivers/can/sja1000/rtcan_adv_pci.c
--- xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/rtcan_adv_pci.c    
1970-01-01 01:00:00.000000000 +0100
+++ xenomai-2.6.0/ksrc/drivers/can/sja1000/rtcan_adv_pci.c    2012-07-16 
16:58:14.175087435 +0200
@@ -0,0 +1,351 @@
+/*
+ * Copyright (C) 2006 Wolfgang Grandegger <wg@grandegger.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ */
+
+#include <linux/module.h>
+#include <linux/ioport.h>
+#include <linux/delay.h>
+#include <linux/pci.h>
+#include <linux/io.h>
+
+#include <rtdm/rtdm_driver.h>
+
+#define ADV_PCI_BASE_SIZE    0x80
+
+/* CAN device profile */
+#include <rtdm/rtcan.h>
+#include <rtcan_dev.h>
+#include <rtcan_raw.h>
+#include <rtcan_internal.h>
+#include <rtcan_sja1000.h>
+#include <rtcan_sja1000_regs.h>
+
+#define RTCAN_DEV_NAME "rtcan%d"
+#define RTCAN_DRV_NAME "ADV-PCI-CAN"
+
+static char *adv_pci_board_name = "ADV-PCI";
+
+MODULE_AUTHOR("Thierry Bultel <thierry.bultel@basystemes.fr>");
+MODULE_DESCRIPTION("RTCAN board driver for Advantech PCI cards");
+MODULE_SUPPORTED_DEVICE("ADV-PCI card CAN controller");
+MODULE_LICENSE("GPL");
+
+struct rtcan_adv_pci {
+    struct pci_dev *pci_dev;
+    struct rtcan_device *slave_dev;
+    void __iomem *conf_addr;
+    void __iomem *base_addr;
+};
+
+/*
+ * According to the datasheet,
+ * internal clock is 1/2 of the external oscillator frequency
+ * which is 16 MHz
+ */
+#define ADV_PCI_CAN_CLOCK (16000000 / 2)
+
+/*
+ * Output control register
+  Depends on the board configuration
+ */
+
+#define ADV_PCI_OCR (SJA_OCR_MODE_NORMAL    |    \
+                     SJA_OCR_TX0_PUSHPULL    |    \
+                     SJA_OCR_TX1_PUSHPULL    |    \
+                     SJA_OCR_TX1_INVERT)
+
+/*
+ * In the CDR register, you should set CBP to 1.
+ */
+#define ADV_PCI_CDR (SJA_CDR_CBP | SJA_CDR_CAN_MODE)
+
+
+#define ADV_PCI_VENDOR_ID 0x13fe
+
+#define CHANNEL_SINGLE 0 /* this is a single channel device */
+#define CHANNEL_MASTER 1 /* multi channel device, this device is master */
+#define CHANNEL_SLAVE  2 /* multi channel device, this is slave */
+
+#define ADV_PCI_DEVICE(device_id)\
+    { ADV_PCI_VENDOR_ID, device_id, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 }
+
+static DEFINE_PCI_DEVICE_TABLE(adv_pci_tbl) = {
+    ADV_PCI_DEVICE(0x1680),
+    ADV_PCI_DEVICE(0x3680),
+    ADV_PCI_DEVICE(0x2052),
+    ADV_PCI_DEVICE(0x1681),
+    ADV_PCI_DEVICE(0xc001),
+    ADV_PCI_DEVICE(0xc002),
+    ADV_PCI_DEVICE(0xc004),
+    ADV_PCI_DEVICE(0xc101),
+    ADV_PCI_DEVICE(0xc102),
+    ADV_PCI_DEVICE(0xc104),
+    /* required last entry */
+    { }
+};
+
+MODULE_DEVICE_TABLE(pci, adv_pci_tbl);
+
+static u8 rtcan_adv_pci_read_reg(struct rtcan_device *dev, int port)
+{
+    struct rtcan_adv_pci *board = (struct rtcan_adv_pci *)dev->board_priv;
+    return ioread8(board->base_addr + port);
+}
+
+static void rtcan_adv_pci_write_reg(struct rtcan_device *dev, int port, 
u8 data)
+{
+    struct rtcan_adv_pci *board = (struct rtcan_adv_pci *)dev->board_priv;
+    iowrite8(data, board->base_addr + port);
+}
+
+static void rtcan_adv_pci_del_chan(
+    struct pci_dev *pdev,
+    struct rtcan_device *dev)
+{
+    struct rtcan_adv_pci *board;
+
+    if (!dev)
+        return;
+
+    board = (struct rtcan_adv_pci *)dev->board_priv;
+
+    rtcan_sja1000_unregister(dev);
+
+    pci_iounmap(pdev, board->base_addr);
+
+    rtcan_dev_free(dev);
+}
+
+
+static int rtcan_adv_pci_add_chan(
+    struct pci_dev *pdev,
+    int    channel,
+    unsigned int bar,
+    unsigned int offset,
+    struct rtcan_device **master_dev)
+{
+    struct rtcan_device *dev;
+    struct rtcan_sja1000 *chip;
+    struct rtcan_adv_pci *board;
+    void __iomem *base_addr;
+    int ret;
+
+    dev = rtcan_dev_alloc(sizeof(struct rtcan_sja1000),
+                          sizeof(struct rtcan_adv_pci));
+    if (dev == NULL)
+        return -ENOMEM;
+
+    chip  = (struct rtcan_sja1000 *)dev->priv;
+    board = (struct rtcan_adv_pci *)dev->board_priv;
+
+    base_addr = pci_iomap(pdev, bar, ADV_PCI_BASE_SIZE) + offset;
+
+    board->pci_dev = pdev;
+    board->conf_addr = NULL;
+    board->base_addr = base_addr;
+
+    if (channel == CHANNEL_SLAVE) {
+        struct rtcan_adv_pci *master_board = \
+            (struct rtcan_adv_pci *)(*master_dev)->board_priv;
+        master_board->slave_dev = dev;
+    }
+
+    dev->board_name = adv_pci_board_name;
+
+    chip->read_reg  = rtcan_adv_pci_read_reg;
+    chip->write_reg = rtcan_adv_pci_write_reg;
+
+    /* Clock frequency in Hz */
+    dev->can_sys_clock = ADV_PCI_CAN_CLOCK;
+
+    /* Output control register */
+    chip->ocr = ADV_PCI_OCR;
+
+    /* Clock divider register */
+    chip->cdr = ADV_PCI_CDR;
+
+    strncpy(dev->name, RTCAN_DEV_NAME, IFNAMSIZ);
+
+    /* Make sure SJA1000 is in reset mode */
+    chip->write_reg(dev, SJA_MOD, SJA_MOD_RM);
+    /* Set PeliCAN mode */
+    chip->write_reg(dev, SJA_CDR, SJA_CDR_CAN_MODE);
+
+    /* check if mode is set */
+    ret = chip->read_reg(dev, SJA_CDR);
+
+    if (ret != SJA_CDR_CAN_MODE)
+        return -EIO;
+
+    /* Register and setup interrupt handling */
+    chip->irq_flags = RTDM_IRQTYPE_SHARED;
+    chip->irq_num   = pdev->irq;
+
+    RTCAN_DBG("%s: base_addr=%p conf_addr=%p irq=%d ocr=%#x cdr=%#x\n",
+              RTCAN_DRV_NAME, board->base_addr, board->conf_addr,
+              chip->irq_num, chip->ocr, chip->cdr);
+
+    /* Register SJA1000 device */
+    ret = rtcan_sja1000_register(dev);
+
+    if (ret) {
+        printk(KERN_ERR "ERROR %d while trying to register SJA1000 
device!\n",
+               ret);
+        goto failure;
+    }
+
+    if (channel != CHANNEL_SLAVE)
+        *master_dev = dev;
+
+    return 0;
+
+failure:
+    rtcan_dev_free(dev);
+
+    return ret;
+}
+
+static int __devinit adv_pci_init_one(
+    struct pci_dev *pdev,
+    const struct pci_device_id *ent)
+{
+    int ret, channel;
+    unsigned int nb_ports = 0;
+    unsigned int bar = 0;
+    unsigned int bar_flag = 0;
+    unsigned int offset = 0;
+    unsigned int ix;
+
+    struct rtcan_device *master_dev = NULL;
+
+    dev_info(&pdev->dev, "RTCAN Registering card");
+
+    ret = pci_enable_device(pdev);
+    if (ret)
+        goto failure;
+
+    dev_info(&pdev->dev, "RTCAN Detected Advantech PCI card at slot #%i\n",
+             PCI_SLOT(pdev->devfn));
+
+    ret = pci_request_regions(pdev, RTCAN_DRV_NAME);
+    if (ret)
+        goto failure;
+
+    switch (pdev->device) {
+    case 0xc001:
+    case 0xc002:
+    case 0xc004:
+    case 0xc101:
+    case 0xc102:
+    case 0xc104:
+        nb_ports = pdev->device & 0x7;
+        offset   = 0x100;
+        bar      = 0;
+        break;
+    case 0x1680:
+    case 0x2052:
+        nb_ports = 2;
+        bar         = 2;
+        bar_flag = 1;
+        break;
+    case 0x1681:
+        nb_ports = 1;
+        bar      = 2;
+        bar_flag = 1;
+        break;
+    default:
+        break;
+    }
+
+    if (nb_ports > 1)
+        channel = CHANNEL_MASTER;
+    else
+        channel = CHANNEL_SINGLE;
+
+    RTCAN_DBG("%s: Initializing device %04x:%04x:%04x\n",\
+              RTCAN_DRV_NAME, \
+              pdev->vendor, \
+              pdev->device, \
+              pdev->subsystem_device);
+
+    ret = rtcan_adv_pci_add_chan(pdev, channel, bar, offset, &master_dev);
+    if (ret)
+        goto failure_iounmap;
+
+    /* register slave channel, if any    */
+
+    for (ix = 1; ix < nb_ports; ix++) {
+        ret = rtcan_adv_pci_add_chan(pdev,
+                                     CHANNEL_SLAVE,
+                                     bar + (bar_flag ? ix : 0),
+                                     offset*ix,
+ &master_dev);
+        if (ret)
+            goto failure_iounmap;
+    }
+
+    pci_set_drvdata(pdev, master_dev);
+
+    return 0;
+
+failure_iounmap:
+
+    if (master_dev)
+        rtcan_adv_pci_del_chan(pdev, master_dev);
+
+    pci_release_regions(pdev);
+
+failure:
+    return ret;
+}
+
+static void __devexit adv_pci_remove_one(struct pci_dev *pdev)
+{
+    struct rtcan_device *dev = pci_get_drvdata(pdev);
+    struct rtcan_adv_pci *board = (struct rtcan_adv_pci *)dev->board_priv;
+
+    if (board->slave_dev)
+        rtcan_adv_pci_del_chan(pdev, board->slave_dev);
+
+    rtcan_adv_pci_del_chan(pdev, dev);
+
+    pci_release_regions(pdev);
+    pci_disable_device(pdev);
+    pci_set_drvdata(pdev, NULL);
+}
+
+static struct pci_driver rtcan_adv_pci_driver = {
+    .name        = RTCAN_DRV_NAME,
+    .id_table    = adv_pci_tbl,
+    .probe        = adv_pci_init_one,
+    .remove        = __devexit_p(adv_pci_remove_one),
+};
+
+static int __init rtcan_adv_pci_init(void)
+{
+    return pci_register_driver(&rtcan_adv_pci_driver);
+}
+
+
+static void __exit rtcan_adv_pci_exit(void)
+{
+    pci_unregister_driver(&rtcan_adv_pci_driver);
+}
+
+
+module_init(rtcan_adv_pci_init);
+module_exit(rtcan_adv_pci_exit);

Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter

[Thierry Bultel]

Hi Wolfgang,

How are you ?

I have taken some time, to take (almost) all your remarks into account 
about my candidate patch.
Please see below, the new version, inline.

- code style / checkpatch : all ERRORs fixed, it remains a spurious "80 
lines" WARNING that I do not agree with.
- multiple devices: The driver now (theoretically) supports as many 
devices as its original linux version
- plx highjack: fixed, this was a bug, actually.

The patch is generated vs xenomai-2.6.0, because it is my testing 
environnement. I will unfortunately and
unlikely not have time to perform some tests with a more recent version.
I hope this is not a blocking issue and that you will be able to 
integrate it anyway.

Another topic, please, I would like to know if you have an estimate date 
for the availability of the RTDM
drivers for Flexcan, and the IMX6 serial as well ?

Cheers,
Thierry


diff -pruN xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/Kconfig 
xenomai-2.6.0/ksrc/drivers/can/sja1000/Kconfig
--- xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/Kconfig    2011-10-18 
20:17:18.000000000 +0200
+++ xenomai-2.6.0/ksrc/drivers/can/sja1000/Kconfig    2012-07-16 
16:44:12.606904812 +0200
@@ -41,6 +41,14 @@ config XENO_DRIVERS_CAN_SJA1000_IXXAT_PC
      the second channel working, Xenomai's shared interrupt support
      must be enabled.

+config XENO_DRIVERS_CAN_SJA1000_ADV_PCI
+    depends on XENO_DRIVERS_CAN_SJA1000 && PCI
+    tristate "ADVANTECH PCI Card"
+    help
+
+    This driver is for the ADVANTECH PCI cards (1 or more channels)
+
+
  config XENO_DRIVERS_CAN_SJA1000_PLX_PCI
      depends on XENO_DRIVERS_CAN_SJA1000 && PCI
      tristate "PLX90xx PCI-bridge based Cards"
diff -pruN xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/Makefile 
xenomai-2.6.0/ksrc/drivers/can/sja1000/Makefile
--- xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/Makefile    2011-10-18 
20:17:18.000000000 +0200
+++ xenomai-2.6.0/ksrc/drivers/can/sja1000/Makefile    2012-07-16 
14:00:38.682836737 +0200
@@ -9,6 +9,7 @@ obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PE
  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PEAK_DNG) += xeno_can_peak_dng.o
  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PLX_PCI) += xeno_can_plx_pci.o
  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_IXXAT_PCI) += xeno_can_ixxat_pci.o
+obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ADV_PCI) += xeno_can_adv_pci.o
  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_EMS_PCI) += xeno_can_ems_pci.o
  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ESD_PCI) += xeno_can_esd_pci.o
  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ISA) += xeno_can_isa.o
@@ -19,6 +20,7 @@ xeno_can_peak_pci-y := rtcan_peak_pci.o
  xeno_can_peak_dng-y := rtcan_peak_dng.o
  xeno_can_plx_pci-y := rtcan_plx_pci.o
  xeno_can_ixxat_pci-y := rtcan_ixxat_pci.o
+xeno_can_adv_pci-y := rtcan_adv_pci.o
  xeno_can_ems_pci-y := rtcan_ems_pci.o
  xeno_can_esd_pci-y := rtcan_esd_pci.o
  xeno_can_isa-y := rtcan_isa.o
@@ -35,6 +37,7 @@ obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PE
  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PEAK_DNG) += xeno_can_peak_dng.o
  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PLX_PCI) += xeno_can_plx_pci.o
  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_IXXAT_PCI) += xeno_can_ixxat_pci.o
+obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ADV_PCI) += xeno_can_adv_pci.o
  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_EMS_PCI) += xeno_can_ems_pci.o
  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ESD_PCI) += xeno_can_esd_pci.o
  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ISA) += xeno_can_isa.o
@@ -47,6 +50,7 @@ xeno_can_peak_pci-objs := rtcan_peak_pci
  xeno_can_peak_dng-objs := rtcan_peak_dng.o
  xeno_can_plx_pci-objs := rtcan_plx_pci.o
  xeno_can_ixxat_pci-objs := rtcan_ixxat_pci.o
+xeno_can_adv_pci-objs := rtcan_adv_pci.o
  xeno_can_ems_pci-objs := rtcan_ems_pci.o
  xeno_can_esd_pci-objs := rtcan_esd_pci.o
  xeno_can_isa-objs := rtcan_isa.o
@@ -73,6 +77,9 @@ xeno_can_plx_pci.o: $(xeno_can_plx_pci-o
  xeno_can_ixxat_pci.o: $(xeno_can_ixxat_pci-objs)
      $(LD) -r -o $@ $(xeno_can_ixxat_pci-objs)

+xeno_can_adv_pci.o: $(xeno_can_adv_pci-objs)
+    $(LD) -r -o $@ $(xeno_can_adv_pci-objs)
+
  xeno_can_ems_pci.o: $(xeno_can_ems_pci-objs)
      $(LD) -r -o $@ $(xeno_can_ems_pci-objs)

diff -pruN xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/rtcan_adv_pci.c 
xenomai-2.6.0/ksrc/drivers/can/sja1000/rtcan_adv_pci.c
--- xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/rtcan_adv_pci.c    
1970-01-01 01:00:00.000000000 +0100
+++ xenomai-2.6.0/ksrc/drivers/can/sja1000/rtcan_adv_pci.c    2012-07-16 
16:58:14.175087435 +0200
@@ -0,0 +1,351 @@
+/*
+ * Copyright (C) 2006 Wolfgang Grandegger <wg@grandegger.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ */
+
+#include <linux/module.h>
+#include <linux/ioport.h>
+#include <linux/delay.h>
+#include <linux/pci.h>
+#include <linux/io.h>
+
+#include <rtdm/rtdm_driver.h>
+
+#define ADV_PCI_BASE_SIZE    0x80
+
+/* CAN device profile */
+#include <rtdm/rtcan.h>
+#include <rtcan_dev.h>
+#include <rtcan_raw.h>
+#include <rtcan_internal.h>
+#include <rtcan_sja1000.h>
+#include <rtcan_sja1000_regs.h>
+
+#define RTCAN_DEV_NAME "rtcan%d"
+#define RTCAN_DRV_NAME "ADV-PCI-CAN"
+
+static char *adv_pci_board_name = "ADV-PCI";
+
+MODULE_AUTHOR("Thierry Bultel <thierry.bultel@basystemes.fr>");
+MODULE_DESCRIPTION("RTCAN board driver for Advantech PCI cards");
+MODULE_SUPPORTED_DEVICE("ADV-PCI card CAN controller");
+MODULE_LICENSE("GPL");
+
+struct rtcan_adv_pci {
+    struct pci_dev *pci_dev;
+    struct rtcan_device *slave_dev;
+    void __iomem *conf_addr;
+    void __iomem *base_addr;
+};
+
+/*
+ * According to the datasheet,
+ * internal clock is 1/2 of the external oscillator frequency
+ * which is 16 MHz
+ */
+#define ADV_PCI_CAN_CLOCK (16000000 / 2)
+
+/*
+ * Output control register
+  Depends on the board configuration
+ */
+
+#define ADV_PCI_OCR (SJA_OCR_MODE_NORMAL    |    \
+                     SJA_OCR_TX0_PUSHPULL    |    \
+                     SJA_OCR_TX1_PUSHPULL    |    \
+                     SJA_OCR_TX1_INVERT)
+
+/*
+ * In the CDR register, you should set CBP to 1.
+ */
+#define ADV_PCI_CDR (SJA_CDR_CBP | SJA_CDR_CAN_MODE)
+
+
+#define ADV_PCI_VENDOR_ID 0x13fe
+
+#define CHANNEL_SINGLE 0 /* this is a single channel device */
+#define CHANNEL_MASTER 1 /* multi channel device, this device is master */
+#define CHANNEL_SLAVE  2 /* multi channel device, this is slave */
+
+#define ADV_PCI_DEVICE(device_id)\
+    { ADV_PCI_VENDOR_ID, device_id, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 }
+
+static DEFINE_PCI_DEVICE_TABLE(adv_pci_tbl) = {
+    ADV_PCI_DEVICE(0x1680),
+    ADV_PCI_DEVICE(0x3680),
+    ADV_PCI_DEVICE(0x2052),
+    ADV_PCI_DEVICE(0x1681),
+    ADV_PCI_DEVICE(0xc001),
+    ADV_PCI_DEVICE(0xc002),
+    ADV_PCI_DEVICE(0xc004),
+    ADV_PCI_DEVICE(0xc101),
+    ADV_PCI_DEVICE(0xc102),
+    ADV_PCI_DEVICE(0xc104),
+    /* required last entry */
+    { }
+};
+
+MODULE_DEVICE_TABLE(pci, adv_pci_tbl);
+
+static u8 rtcan_adv_pci_read_reg(struct rtcan_device *dev, int port)
+{
+    struct rtcan_adv_pci *board = (struct rtcan_adv_pci *)dev->board_priv;
+    return ioread8(board->base_addr + port);
+}
+
+static void rtcan_adv_pci_write_reg(struct rtcan_device *dev, int port, 
u8 data)
+{
+    struct rtcan_adv_pci *board = (struct rtcan_adv_pci *)dev->board_priv;
+    iowrite8(data, board->base_addr + port);
+}
+
+static void rtcan_adv_pci_del_chan(
+    struct pci_dev *pdev,
+    struct rtcan_device *dev)
+{
+    struct rtcan_adv_pci *board;
+
+    if (!dev)
+        return;
+
+    board = (struct rtcan_adv_pci *)dev->board_priv;
+
+    rtcan_sja1000_unregister(dev);
+
+    pci_iounmap(pdev, board->base_addr);
+
+    rtcan_dev_free(dev);
+}
+
+
+static int rtcan_adv_pci_add_chan(
+    struct pci_dev *pdev,
+    int    channel,
+    unsigned int bar,
+    unsigned int offset,
+    struct rtcan_device **master_dev)
+{
+    struct rtcan_device *dev;
+    struct rtcan_sja1000 *chip;
+    struct rtcan_adv_pci *board;
+    void __iomem *base_addr;
+    int ret;
+
+    dev = rtcan_dev_alloc(sizeof(struct rtcan_sja1000),
+                          sizeof(struct rtcan_adv_pci));
+    if (dev == NULL)
+        return -ENOMEM;
+
+    chip  = (struct rtcan_sja1000 *)dev->priv;
+    board = (struct rtcan_adv_pci *)dev->board_priv;
+
+    base_addr = pci_iomap(pdev, bar, ADV_PCI_BASE_SIZE) + offset;
+
+    board->pci_dev = pdev;
+    board->conf_addr = NULL;
+    board->base_addr = base_addr;
+
+    if (channel == CHANNEL_SLAVE) {
+        struct rtcan_adv_pci *master_board = \
+            (struct rtcan_adv_pci *)(*master_dev)->board_priv;
+        master_board->slave_dev = dev;
+    }
+
+    dev->board_name = adv_pci_board_name;
+
+    chip->read_reg  = rtcan_adv_pci_read_reg;
+    chip->write_reg = rtcan_adv_pci_write_reg;
+
+    /* Clock frequency in Hz */
+    dev->can_sys_clock = ADV_PCI_CAN_CLOCK;
+
+    /* Output control register */
+    chip->ocr = ADV_PCI_OCR;
+
+    /* Clock divider register */
+    chip->cdr = ADV_PCI_CDR;
+
+    strncpy(dev->name, RTCAN_DEV_NAME, IFNAMSIZ);
+
+    /* Make sure SJA1000 is in reset mode */
+    chip->write_reg(dev, SJA_MOD, SJA_MOD_RM);
+    /* Set PeliCAN mode */
+    chip->write_reg(dev, SJA_CDR, SJA_CDR_CAN_MODE);
+
+    /* check if mode is set */
+    ret = chip->read_reg(dev, SJA_CDR);
+
+    if (ret != SJA_CDR_CAN_MODE)
+        return -EIO;
+
+    /* Register and setup interrupt handling */
+    chip->irq_flags = RTDM_IRQTYPE_SHARED;
+    chip->irq_num   = pdev->irq;
+
+    RTCAN_DBG("%s: base_addr=%p conf_addr=%p irq=%d ocr=%#x cdr=%#x\n",
+              RTCAN_DRV_NAME, board->base_addr, board->conf_addr,
+              chip->irq_num, chip->ocr, chip->cdr);
+
+    /* Register SJA1000 device */
+    ret = rtcan_sja1000_register(dev);
+
+    if (ret) {
+        printk(KERN_ERR "ERROR %d while trying to register SJA1000 
device!\n",
+               ret);
+        goto failure;
+    }
+
+    if (channel != CHANNEL_SLAVE)
+        *master_dev = dev;
+
+    return 0;
+
+failure:
+    rtcan_dev_free(dev);
+
+    return ret;
+}
+
+static int __devinit adv_pci_init_one(
+    struct pci_dev *pdev,
+    const struct pci_device_id *ent)
+{
+    int ret, channel;
+    unsigned int nb_ports = 0;
+    unsigned int bar = 0;
+    unsigned int bar_flag = 0;
+    unsigned int offset = 0;
+    unsigned int ix;
+
+    struct rtcan_device *master_dev = NULL;
+
+    dev_info(&pdev->dev, "RTCAN Registering card");
+
+    ret = pci_enable_device(pdev);
+    if (ret)
+        goto failure;
+
+    dev_info(&pdev->dev, "RTCAN Detected Advantech PCI card at slot #%i\n",
+             PCI_SLOT(pdev->devfn));
+
+    ret = pci_request_regions(pdev, RTCAN_DRV_NAME);
+    if (ret)
+        goto failure;
+
+    switch (pdev->device) {
+    case 0xc001:
+    case 0xc002:
+    case 0xc004:
+    case 0xc101:
+    case 0xc102:
+    case 0xc104:
+        nb_ports = pdev->device & 0x7;
+        offset   = 0x100;
+        bar      = 0;
+        break;
+    case 0x1680:
+    case 0x2052:
+        nb_ports = 2;
+        bar         = 2;
+        bar_flag = 1;
+        break;
+    case 0x1681:
+        nb_ports = 1;
+        bar      = 2;
+        bar_flag = 1;
+        break;
+    default:
+        break;
+    }
+
+    if (nb_ports > 1)
+        channel = CHANNEL_MASTER;
+    else
+        channel = CHANNEL_SINGLE;
+
+    RTCAN_DBG("%s: Initializing device %04x:%04x:%04x\n",\
+              RTCAN_DRV_NAME, \
+              pdev->vendor, \
+              pdev->device, \
+              pdev->subsystem_device);
+
+    ret = rtcan_adv_pci_add_chan(pdev, channel, bar, offset, &master_dev);
+    if (ret)
+        goto failure_iounmap;
+
+    /* register slave channel, if any    */
+
+    for (ix = 1; ix < nb_ports; ix++) {
+        ret = rtcan_adv_pci_add_chan(pdev,
+                                     CHANNEL_SLAVE,
+                                     bar + (bar_flag ? ix : 0),
+                                     offset*ix,
+ &master_dev);
+        if (ret)
+            goto failure_iounmap;
+    }
+
+    pci_set_drvdata(pdev, master_dev);
+
+    return 0;
+
+failure_iounmap:
+
+    if (master_dev)
+        rtcan_adv_pci_del_chan(pdev, master_dev);
+
+    pci_release_regions(pdev);
+
+failure:
+    return ret;
+}
+
+static void __devexit adv_pci_remove_one(struct pci_dev *pdev)
+{
+    struct rtcan_device *dev = pci_get_drvdata(pdev);
+    struct rtcan_adv_pci *board = (struct rtcan_adv_pci *)dev->board_priv;
+
+    if (board->slave_dev)
+        rtcan_adv_pci_del_chan(pdev, board->slave_dev);
+
+    rtcan_adv_pci_del_chan(pdev, dev);
+
+    pci_release_regions(pdev);
+    pci_disable_device(pdev);
+    pci_set_drvdata(pdev, NULL);
+}
+
+static struct pci_driver rtcan_adv_pci_driver = {
+    .name        = RTCAN_DRV_NAME,
+    .id_table    = adv_pci_tbl,
+    .probe        = adv_pci_init_one,
+    .remove        = __devexit_p(adv_pci_remove_one),
+};
+
+static int __init rtcan_adv_pci_init(void)
+{
+    return pci_register_driver(&rtcan_adv_pci_driver);
+}
+
+
+static void __exit rtcan_adv_pci_exit(void)
+{
+    pci_unregister_driver(&rtcan_adv_pci_driver);
+}
+
+
+module_init(rtcan_adv_pci_init);
+module_exit(rtcan_adv_pci_exit);

Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter

[Wolfgang Grandegger]

Hi Thierry,

thanks for your cleanup...

On 07/16/2012 05:07 PM, Thierry Bultel wrote:
> Hi Wolfgang,
> 
> How are you ?
> 
> I have taken some time, to take (almost) all your remarks into account
> about my candidate patch.
> Please see below, the new version, inline.

Unfortunately, the patch is white space mangled. Please use
git-send-email or tune your mail client not to break lines, etc.

> - code style / checkpatch : all ERRORs fixed, it remains a spurious "80
> lines" WARNING that I do not agree with.

I think the preferred coding style for Xenomai nowadays is the Linux
one. Therefore please use *tabs*" for indention (even if old RTCAN code
doesn't). This should also make checkpatch.pl even more happier.

> - multiple devices: The driver now (theoretically) supports as many
> devices as its original linux version

Could you mention them in the Kconfig help? Just in case you know the
names of these devices.

> - plx highjack: fixed, this was a bug, actually.
> 
> The patch is generated vs xenomai-2.6.0, because it is my testing
> environnement. I will unfortunately and
> unlikely not have time to perform some tests with a more recent version.
> I hope this is not a blocking issue and that you will be able to
> integrate it anyway.

Should not be a real problem.

> Another topic, please, I would like to know if you have an estimate date
> for the availability of the RTDM
> drivers for Flexcan, and the IMX6 serial as well ?

As you might have realized, there is some interest on a RTCAN Flexcan
driver, which I'm currently working on. Hope to have something ready
this or next week. The same for publishing my rt_imx_uart driver.

Wolfgang.

Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter

[Wolfgang Grandegger]

Hi Thierry,

thanks for your cleanup...

On 07/16/2012 05:07 PM, Thierry Bultel wrote:
> Hi Wolfgang,
> 
> How are you ?
> 
> I have taken some time, to take (almost) all your remarks into account
> about my candidate patch.
> Please see below, the new version, inline.

Unfortunately, the patch is white space mangled. Please use
git-send-email or tune your mail client not to break lines, etc.

> - code style / checkpatch : all ERRORs fixed, it remains a spurious "80
> lines" WARNING that I do not agree with.

I think the preferred coding style for Xenomai nowadays is the Linux
one. Therefore please use *tabs*" for indention (even if old RTCAN code
doesn't). This should also make checkpatch.pl even more happier.

> - multiple devices: The driver now (theoretically) supports as many
> devices as its original linux version

Could you mention them in the Kconfig help? Just in case you know the
names of these devices.

> - plx highjack: fixed, this was a bug, actually.
> 
> The patch is generated vs xenomai-2.6.0, because it is my testing
> environnement. I will unfortunately and
> unlikely not have time to perform some tests with a more recent version.
> I hope this is not a blocking issue and that you will be able to
> integrate it anyway.

Should not be a real problem.

> Another topic, please, I would like to know if you have an estimate date
> for the availability of the RTDM
> drivers for Flexcan, and the IMX6 serial as well ?

As you might have realized, there is some interest on a RTCAN Flexcan
driver, which I'm currently working on. Hope to have something ready
this or next week. The same for publishing my rt_imx_uart driver.

Wolfgang.

Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter

[Thierry Bultel]

Hi Wolfgang,

After some tuning of thunderbird things should get better now.

As you required, I have added a comment in Kconfig, about the 1680U which
is the only board I now about. 

Regarding the "80 chars" stuff, what I meant is that it remains a single
location where checkpatch complains, but I do not know how to fix it;
line is actually only 64 chars long at most so that is a little puzzling. 

WARNING: line over 80 characters
#361: FILE: ksrc/drivers/can/sja1000/rtcan_adv_pci.c:293:
+									 CHANNEL_SLAVE,

WARNING: line over 80 characters
#362: FILE: ksrc/drivers/can/sja1000/rtcan_adv_pci.c:294:
+									 bar + (bar_flag ? ix : 0),

WARNING: line over 80 characters
#363: FILE: ksrc/drivers/can/sja1000/rtcan_adv_pci.c:295:
+									 offset*ix,

WARNING: line over 80 characters
#364: FILE: ksrc/drivers/can/sja1000/rtcan_adv_pci.c:296:
+									 &master_dev);


All the idents are done with tabs (it was the case in the previous mail as well but thunderbird broke it).
so checkpatch is happy for everything else.

Thanks in advance for the integration;
Cheers
Thierry


diff -pruN xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/Kconfig xenomai-2.6.0/ksrc/drivers/can/sja1000/Kconfig
--- xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/Kconfig	2011-10-18 20:17:18.000000000 +0200
+++ xenomai-2.6.0/ksrc/drivers/can/sja1000/Kconfig	2012-07-18 11:46:44.578890869 +0200
@@ -41,6 +41,15 @@ config XENO_DRIVERS_CAN_SJA1000_IXXAT_PC
 	the second channel working, Xenomai's shared interrupt support
 	must be enabled.
 
+config XENO_DRIVERS_CAN_SJA1000_ADV_PCI
+	depends on XENO_DRIVERS_CAN_SJA1000 && PCI
+	tristate "ADVANTECH PCI Card"
+	help
+
+	This driver is for the ADVANTECH PCI cards (1 or more channels)
+	It supports the 1680U and some other ones.
+
+
 config XENO_DRIVERS_CAN_SJA1000_PLX_PCI
 	depends on XENO_DRIVERS_CAN_SJA1000 && PCI
 	tristate "PLX90xx PCI-bridge based Cards"
diff -pruN xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/Makefile xenomai-2.6.0/ksrc/drivers/can/sja1000/Makefile
--- xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/Makefile	2011-10-18 20:17:18.000000000 +0200
+++ xenomai-2.6.0/ksrc/drivers/can/sja1000/Makefile	2012-07-16 14:00:38.682836737 +0200
@@ -9,6 +9,7 @@ obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PE
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PEAK_DNG) += xeno_can_peak_dng.o
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PLX_PCI) += xeno_can_plx_pci.o
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_IXXAT_PCI) += xeno_can_ixxat_pci.o
+obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ADV_PCI) += xeno_can_adv_pci.o
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_EMS_PCI) += xeno_can_ems_pci.o
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ESD_PCI) += xeno_can_esd_pci.o
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ISA) += xeno_can_isa.o
@@ -19,6 +20,7 @@ xeno_can_peak_pci-y := rtcan_peak_pci.o
 xeno_can_peak_dng-y := rtcan_peak_dng.o
 xeno_can_plx_pci-y := rtcan_plx_pci.o
 xeno_can_ixxat_pci-y := rtcan_ixxat_pci.o
+xeno_can_adv_pci-y := rtcan_adv_pci.o
 xeno_can_ems_pci-y := rtcan_ems_pci.o
 xeno_can_esd_pci-y := rtcan_esd_pci.o
 xeno_can_isa-y := rtcan_isa.o
@@ -35,6 +37,7 @@ obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PE
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PEAK_DNG) += xeno_can_peak_dng.o 
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PLX_PCI) += xeno_can_plx_pci.o
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_IXXAT_PCI) += xeno_can_ixxat_pci.o
+obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ADV_PCI) += xeno_can_adv_pci.o
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_EMS_PCI) += xeno_can_ems_pci.o
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ESD_PCI) += xeno_can_esd_pci.o
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ISA) += xeno_can_isa.o 
@@ -47,6 +50,7 @@ xeno_can_peak_pci-objs := rtcan_peak_pci
 xeno_can_peak_dng-objs := rtcan_peak_dng.o
 xeno_can_plx_pci-objs := rtcan_plx_pci.o
 xeno_can_ixxat_pci-objs := rtcan_ixxat_pci.o
+xeno_can_adv_pci-objs := rtcan_adv_pci.o
 xeno_can_ems_pci-objs := rtcan_ems_pci.o
 xeno_can_esd_pci-objs := rtcan_esd_pci.o
 xeno_can_isa-objs := rtcan_isa.o
@@ -73,6 +77,9 @@ xeno_can_plx_pci.o: $(xeno_can_plx_pci-o
 xeno_can_ixxat_pci.o: $(xeno_can_ixxat_pci-objs)
 	$(LD) -r -o $@ $(xeno_can_ixxat_pci-objs)
 
+xeno_can_adv_pci.o: $(xeno_can_adv_pci-objs)
+	$(LD) -r -o $@ $(xeno_can_adv_pci-objs)
+
 xeno_can_ems_pci.o: $(xeno_can_ems_pci-objs)
 	$(LD) -r -o $@ $(xeno_can_ems_pci-objs)
 
diff -pruN xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/rtcan_adv_pci.c xenomai-2.6.0/ksrc/drivers/can/sja1000/rtcan_adv_pci.c
--- xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/rtcan_adv_pci.c	1970-01-01 01:00:00.000000000 +0100
+++ xenomai-2.6.0/ksrc/drivers/can/sja1000/rtcan_adv_pci.c	2012-07-16 16:58:14.175087435 +0200
@@ -0,0 +1,351 @@
+/*
+ * Copyright (C) 2006 Wolfgang Grandegger <wg@grandegger.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ */
+
+#include <linux/module.h>
+#include <linux/ioport.h>
+#include <linux/delay.h>
+#include <linux/pci.h>
+#include <linux/io.h>
+
+#include <rtdm/rtdm_driver.h>
+
+#define ADV_PCI_BASE_SIZE	0x80
+
+/* CAN device profile */
+#include <rtdm/rtcan.h>
+#include <rtcan_dev.h>
+#include <rtcan_raw.h>
+#include <rtcan_internal.h>
+#include <rtcan_sja1000.h>
+#include <rtcan_sja1000_regs.h>
+
+#define RTCAN_DEV_NAME "rtcan%d"
+#define RTCAN_DRV_NAME "ADV-PCI-CAN"
+
+static char *adv_pci_board_name = "ADV-PCI";
+
+MODULE_AUTHOR("Thierry Bultel <thierry.bultel@basystemes.fr>");
+MODULE_DESCRIPTION("RTCAN board driver for Advantech PCI cards");
+MODULE_SUPPORTED_DEVICE("ADV-PCI card CAN controller");
+MODULE_LICENSE("GPL");
+
+struct rtcan_adv_pci {
+	struct pci_dev *pci_dev;
+	struct rtcan_device *slave_dev;
+	void __iomem *conf_addr;
+	void __iomem *base_addr;
+};
+
+/*
+ * According to the datasheet,
+ * internal clock is 1/2 of the external oscillator frequency
+ * which is 16 MHz
+ */
+#define ADV_PCI_CAN_CLOCK (16000000 / 2)
+
+/*
+ * Output control register
+  Depends on the board configuration
+ */
+
+#define ADV_PCI_OCR (SJA_OCR_MODE_NORMAL	|	\
+					 SJA_OCR_TX0_PUSHPULL	|	\
+					 SJA_OCR_TX1_PUSHPULL	|	\
+					 SJA_OCR_TX1_INVERT)
+
+/*
+ * In the CDR register, you should set CBP to 1.
+ */
+#define ADV_PCI_CDR (SJA_CDR_CBP | SJA_CDR_CAN_MODE)
+
+
+#define ADV_PCI_VENDOR_ID 0x13fe
+
+#define CHANNEL_SINGLE 0 /* this is a single channel device */
+#define CHANNEL_MASTER 1 /* multi channel device, this device is master */
+#define CHANNEL_SLAVE  2 /* multi channel device, this is slave */
+
+#define ADV_PCI_DEVICE(device_id)\
+	{ ADV_PCI_VENDOR_ID, device_id, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 }
+
+static DEFINE_PCI_DEVICE_TABLE(adv_pci_tbl) = {
+	ADV_PCI_DEVICE(0x1680),
+	ADV_PCI_DEVICE(0x3680),
+	ADV_PCI_DEVICE(0x2052),
+	ADV_PCI_DEVICE(0x1681),
+	ADV_PCI_DEVICE(0xc001),
+	ADV_PCI_DEVICE(0xc002),
+	ADV_PCI_DEVICE(0xc004),
+	ADV_PCI_DEVICE(0xc101),
+	ADV_PCI_DEVICE(0xc102),
+	ADV_PCI_DEVICE(0xc104),
+	/* required last entry */
+	{ }
+};
+
+MODULE_DEVICE_TABLE(pci, adv_pci_tbl);
+
+static u8 rtcan_adv_pci_read_reg(struct rtcan_device *dev, int port)
+{
+	struct rtcan_adv_pci *board = (struct rtcan_adv_pci *)dev->board_priv;
+	return ioread8(board->base_addr + port);
+}
+
+static void rtcan_adv_pci_write_reg(struct rtcan_device *dev, int port, u8 data)
+{
+	struct rtcan_adv_pci *board = (struct rtcan_adv_pci *)dev->board_priv;
+	iowrite8(data, board->base_addr + port);
+}
+
+static void rtcan_adv_pci_del_chan(
+	struct pci_dev *pdev,
+	struct rtcan_device *dev)
+{
+	struct rtcan_adv_pci *board;
+
+	if (!dev)
+		return;
+
+	board = (struct rtcan_adv_pci *)dev->board_priv;
+
+	rtcan_sja1000_unregister(dev);
+
+	pci_iounmap(pdev, board->base_addr);
+
+	rtcan_dev_free(dev);
+}
+
+
+static int rtcan_adv_pci_add_chan(
+	struct pci_dev *pdev,
+	int	channel,
+	unsigned int bar,
+	unsigned int offset,
+	struct rtcan_device **master_dev)
+{
+	struct rtcan_device *dev;
+	struct rtcan_sja1000 *chip;
+	struct rtcan_adv_pci *board;
+	void __iomem *base_addr;
+	int ret;
+
+	dev = rtcan_dev_alloc(sizeof(struct rtcan_sja1000),
+						  sizeof(struct rtcan_adv_pci));
+	if (dev == NULL)
+		return -ENOMEM;
+
+	chip  = (struct rtcan_sja1000 *)dev->priv;
+	board = (struct rtcan_adv_pci *)dev->board_priv;
+
+	base_addr = pci_iomap(pdev, bar, ADV_PCI_BASE_SIZE) + offset;
+
+	board->pci_dev = pdev;
+	board->conf_addr = NULL;
+	board->base_addr = base_addr;
+
+	if (channel == CHANNEL_SLAVE) {
+		struct rtcan_adv_pci *master_board = \
+			(struct rtcan_adv_pci *)(*master_dev)->board_priv;
+		master_board->slave_dev = dev;
+	}
+
+	dev->board_name = adv_pci_board_name;
+
+	chip->read_reg  = rtcan_adv_pci_read_reg;
+	chip->write_reg = rtcan_adv_pci_write_reg;
+
+	/* Clock frequency in Hz */
+	dev->can_sys_clock = ADV_PCI_CAN_CLOCK;
+
+	/* Output control register */
+	chip->ocr = ADV_PCI_OCR;
+
+	/* Clock divider register */
+	chip->cdr = ADV_PCI_CDR;
+
+	strncpy(dev->name, RTCAN_DEV_NAME, IFNAMSIZ);
+
+    /* Make sure SJA1000 is in reset mode */
+	chip->write_reg(dev, SJA_MOD, SJA_MOD_RM);
+	/* Set PeliCAN mode */
+	chip->write_reg(dev, SJA_CDR, SJA_CDR_CAN_MODE);
+
+	/* check if mode is set */
+	ret = chip->read_reg(dev, SJA_CDR);
+
+	if (ret != SJA_CDR_CAN_MODE)
+		return -EIO;
+
+	/* Register and setup interrupt handling */
+	chip->irq_flags = RTDM_IRQTYPE_SHARED;
+	chip->irq_num   = pdev->irq;
+
+	RTCAN_DBG("%s: base_addr=%p conf_addr=%p irq=%d ocr=%#x cdr=%#x\n",
+			  RTCAN_DRV_NAME, board->base_addr, board->conf_addr,
+			  chip->irq_num, chip->ocr, chip->cdr);
+
+	/* Register SJA1000 device */
+	ret = rtcan_sja1000_register(dev);
+
+	if (ret) {
+		printk(KERN_ERR "ERROR %d while trying to register SJA1000 device!\n",
+			   ret);
+		goto failure;
+	}
+
+	if (channel != CHANNEL_SLAVE)
+		*master_dev = dev;
+
+	return 0;
+
+failure:
+	rtcan_dev_free(dev);
+
+	return ret;
+}
+
+static int __devinit adv_pci_init_one(
+	struct pci_dev *pdev,
+	const struct pci_device_id *ent)
+{
+	int ret, channel;
+	unsigned int nb_ports = 0;
+	unsigned int bar = 0;
+	unsigned int bar_flag = 0;
+	unsigned int offset = 0;
+	unsigned int ix;
+
+	struct rtcan_device *master_dev = NULL;
+
+	dev_info(&pdev->dev, "RTCAN Registering card");
+
+	ret = pci_enable_device(pdev);
+	if (ret)
+		goto failure;
+
+	dev_info(&pdev->dev, "RTCAN Detected Advantech PCI card at slot #%i\n",
+			 PCI_SLOT(pdev->devfn));
+
+	ret = pci_request_regions(pdev, RTCAN_DRV_NAME);
+	if (ret)
+		goto failure;
+
+	switch (pdev->device) {
+	case 0xc001:
+	case 0xc002:
+	case 0xc004:
+	case 0xc101:
+	case 0xc102:
+	case 0xc104:
+		nb_ports = pdev->device & 0x7;
+		offset   = 0x100;
+		bar      = 0;
+		break;
+	case 0x1680:
+	case 0x2052:
+		nb_ports = 2;
+		bar		 = 2;
+		bar_flag = 1;
+		break;
+	case 0x1681:
+		nb_ports = 1;
+		bar      = 2;
+		bar_flag = 1;
+		break;
+	default:
+		break;
+	}
+
+	if (nb_ports > 1)
+		channel = CHANNEL_MASTER;
+	else
+		channel = CHANNEL_SINGLE;
+
+	RTCAN_DBG("%s: Initializing device %04x:%04x:%04x\n",\
+			  RTCAN_DRV_NAME, \
+			  pdev->vendor, \
+			  pdev->device, \
+			  pdev->subsystem_device);
+
+	ret = rtcan_adv_pci_add_chan(pdev, channel, bar, offset, &master_dev);
+	if (ret)
+		goto failure_iounmap;
+
+	/* register slave channel, if any	*/
+
+	for (ix = 1; ix < nb_ports; ix++) {
+		ret = rtcan_adv_pci_add_chan(pdev,
+									 CHANNEL_SLAVE,
+									 bar + (bar_flag ? ix : 0),
+									 offset*ix,
+									 &master_dev);
+		if (ret)
+			goto failure_iounmap;
+	}
+
+	pci_set_drvdata(pdev, master_dev);
+
+	return 0;
+
+failure_iounmap:
+
+	if (master_dev)
+		rtcan_adv_pci_del_chan(pdev, master_dev);
+
+	pci_release_regions(pdev);
+
+failure:
+	return ret;
+}
+
+static void __devexit adv_pci_remove_one(struct pci_dev *pdev)
+{
+	struct rtcan_device *dev = pci_get_drvdata(pdev);
+	struct rtcan_adv_pci *board = (struct rtcan_adv_pci *)dev->board_priv;
+
+	if (board->slave_dev)
+		rtcan_adv_pci_del_chan(pdev, board->slave_dev);
+
+	rtcan_adv_pci_del_chan(pdev, dev);
+
+	pci_release_regions(pdev);
+	pci_disable_device(pdev);
+	pci_set_drvdata(pdev, NULL);
+}
+
+static struct pci_driver rtcan_adv_pci_driver = {
+	.name		= RTCAN_DRV_NAME,
+	.id_table	= adv_pci_tbl,
+	.probe		= adv_pci_init_one,
+	.remove		= __devexit_p(adv_pci_remove_one),
+};
+
+static int __init rtcan_adv_pci_init(void)
+{
+	return pci_register_driver(&rtcan_adv_pci_driver);
+}
+
+
+static void __exit rtcan_adv_pci_exit(void)
+{
+	pci_unregister_driver(&rtcan_adv_pci_driver);
+}
+
+
+module_init(rtcan_adv_pci_init);
+module_exit(rtcan_adv_pci_exit);

Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter

[Thierry Bultel]

Hi Wolfgang,

After some tuning of thunderbird things should get better now.

As you required, I have added a comment in Kconfig, about the 1680U which
is the only board I now about. 

Regarding the "80 chars" stuff, what I meant is that it remains a single
location where checkpatch complains, but I do not know how to fix it;
line is actually only 64 chars long at most so that is a little puzzling. 

WARNING: line over 80 characters
#361: FILE: ksrc/drivers/can/sja1000/rtcan_adv_pci.c:293:
+									 CHANNEL_SLAVE,

WARNING: line over 80 characters
#362: FILE: ksrc/drivers/can/sja1000/rtcan_adv_pci.c:294:
+									 bar + (bar_flag ? ix : 0),

WARNING: line over 80 characters
#363: FILE: ksrc/drivers/can/sja1000/rtcan_adv_pci.c:295:
+									 offset*ix,

WARNING: line over 80 characters
#364: FILE: ksrc/drivers/can/sja1000/rtcan_adv_pci.c:296:
+									 &master_dev);


All the idents are done with tabs (it was the case in the previous mail as well but thunderbird broke it).
so checkpatch is happy for everything else.

Thanks in advance for the integration;
Cheers
Thierry


diff -pruN xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/Kconfig xenomai-2.6.0/ksrc/drivers/can/sja1000/Kconfig
--- xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/Kconfig	2011-10-18 20:17:18.000000000 +0200
+++ xenomai-2.6.0/ksrc/drivers/can/sja1000/Kconfig	2012-07-18 11:46:44.578890869 +0200
@@ -41,6 +41,15 @@ config XENO_DRIVERS_CAN_SJA1000_IXXAT_PC
 	the second channel working, Xenomai's shared interrupt support
 	must be enabled.
 
+config XENO_DRIVERS_CAN_SJA1000_ADV_PCI
+	depends on XENO_DRIVERS_CAN_SJA1000 && PCI
+	tristate "ADVANTECH PCI Card"
+	help
+
+	This driver is for the ADVANTECH PCI cards (1 or more channels)
+	It supports the 1680U and some other ones.
+
+
 config XENO_DRIVERS_CAN_SJA1000_PLX_PCI
 	depends on XENO_DRIVERS_CAN_SJA1000 && PCI
 	tristate "PLX90xx PCI-bridge based Cards"
diff -pruN xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/Makefile xenomai-2.6.0/ksrc/drivers/can/sja1000/Makefile
--- xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/Makefile	2011-10-18 20:17:18.000000000 +0200
+++ xenomai-2.6.0/ksrc/drivers/can/sja1000/Makefile	2012-07-16 14:00:38.682836737 +0200
@@ -9,6 +9,7 @@ obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PE
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PEAK_DNG) += xeno_can_peak_dng.o
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PLX_PCI) += xeno_can_plx_pci.o
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_IXXAT_PCI) += xeno_can_ixxat_pci.o
+obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ADV_PCI) += xeno_can_adv_pci.o
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_EMS_PCI) += xeno_can_ems_pci.o
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ESD_PCI) += xeno_can_esd_pci.o
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ISA) += xeno_can_isa.o
@@ -19,6 +20,7 @@ xeno_can_peak_pci-y := rtcan_peak_pci.o
 xeno_can_peak_dng-y := rtcan_peak_dng.o
 xeno_can_plx_pci-y := rtcan_plx_pci.o
 xeno_can_ixxat_pci-y := rtcan_ixxat_pci.o
+xeno_can_adv_pci-y := rtcan_adv_pci.o
 xeno_can_ems_pci-y := rtcan_ems_pci.o
 xeno_can_esd_pci-y := rtcan_esd_pci.o
 xeno_can_isa-y := rtcan_isa.o
@@ -35,6 +37,7 @@ obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PE
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PEAK_DNG) += xeno_can_peak_dng.o 
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PLX_PCI) += xeno_can_plx_pci.o
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_IXXAT_PCI) += xeno_can_ixxat_pci.o
+obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ADV_PCI) += xeno_can_adv_pci.o
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_EMS_PCI) += xeno_can_ems_pci.o
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ESD_PCI) += xeno_can_esd_pci.o
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ISA) += xeno_can_isa.o 
@@ -47,6 +50,7 @@ xeno_can_peak_pci-objs := rtcan_peak_pci
 xeno_can_peak_dng-objs := rtcan_peak_dng.o
 xeno_can_plx_pci-objs := rtcan_plx_pci.o
 xeno_can_ixxat_pci-objs := rtcan_ixxat_pci.o
+xeno_can_adv_pci-objs := rtcan_adv_pci.o
 xeno_can_ems_pci-objs := rtcan_ems_pci.o
 xeno_can_esd_pci-objs := rtcan_esd_pci.o
 xeno_can_isa-objs := rtcan_isa.o
@@ -73,6 +77,9 @@ xeno_can_plx_pci.o: $(xeno_can_plx_pci-o
 xeno_can_ixxat_pci.o: $(xeno_can_ixxat_pci-objs)
 	$(LD) -r -o $@ $(xeno_can_ixxat_pci-objs)
 
+xeno_can_adv_pci.o: $(xeno_can_adv_pci-objs)
+	$(LD) -r -o $@ $(xeno_can_adv_pci-objs)
+
 xeno_can_ems_pci.o: $(xeno_can_ems_pci-objs)
 	$(LD) -r -o $@ $(xeno_can_ems_pci-objs)
 
diff -pruN xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/rtcan_adv_pci.c xenomai-2.6.0/ksrc/drivers/can/sja1000/rtcan_adv_pci.c
--- xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/rtcan_adv_pci.c	1970-01-01 01:00:00.000000000 +0100
+++ xenomai-2.6.0/ksrc/drivers/can/sja1000/rtcan_adv_pci.c	2012-07-16 16:58:14.175087435 +0200
@@ -0,0 +1,351 @@
+/*
+ * Copyright (C) 2006 Wolfgang Grandegger <wg@grandegger.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ */
+
+#include <linux/module.h>
+#include <linux/ioport.h>
+#include <linux/delay.h>
+#include <linux/pci.h>
+#include <linux/io.h>
+
+#include <rtdm/rtdm_driver.h>
+
+#define ADV_PCI_BASE_SIZE	0x80
+
+/* CAN device profile */
+#include <rtdm/rtcan.h>
+#include <rtcan_dev.h>
+#include <rtcan_raw.h>
+#include <rtcan_internal.h>
+#include <rtcan_sja1000.h>
+#include <rtcan_sja1000_regs.h>
+
+#define RTCAN_DEV_NAME "rtcan%d"
+#define RTCAN_DRV_NAME "ADV-PCI-CAN"
+
+static char *adv_pci_board_name = "ADV-PCI";
+
+MODULE_AUTHOR("Thierry Bultel <thierry.bultel@basystemes.fr>");
+MODULE_DESCRIPTION("RTCAN board driver for Advantech PCI cards");
+MODULE_SUPPORTED_DEVICE("ADV-PCI card CAN controller");
+MODULE_LICENSE("GPL");
+
+struct rtcan_adv_pci {
+	struct pci_dev *pci_dev;
+	struct rtcan_device *slave_dev;
+	void __iomem *conf_addr;
+	void __iomem *base_addr;
+};
+
+/*
+ * According to the datasheet,
+ * internal clock is 1/2 of the external oscillator frequency
+ * which is 16 MHz
+ */
+#define ADV_PCI_CAN_CLOCK (16000000 / 2)
+
+/*
+ * Output control register
+  Depends on the board configuration
+ */
+
+#define ADV_PCI_OCR (SJA_OCR_MODE_NORMAL	|	\
+					 SJA_OCR_TX0_PUSHPULL	|	\
+					 SJA_OCR_TX1_PUSHPULL	|	\
+					 SJA_OCR_TX1_INVERT)
+
+/*
+ * In the CDR register, you should set CBP to 1.
+ */
+#define ADV_PCI_CDR (SJA_CDR_CBP | SJA_CDR_CAN_MODE)
+
+
+#define ADV_PCI_VENDOR_ID 0x13fe
+
+#define CHANNEL_SINGLE 0 /* this is a single channel device */
+#define CHANNEL_MASTER 1 /* multi channel device, this device is master */
+#define CHANNEL_SLAVE  2 /* multi channel device, this is slave */
+
+#define ADV_PCI_DEVICE(device_id)\
+	{ ADV_PCI_VENDOR_ID, device_id, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 }
+
+static DEFINE_PCI_DEVICE_TABLE(adv_pci_tbl) = {
+	ADV_PCI_DEVICE(0x1680),
+	ADV_PCI_DEVICE(0x3680),
+	ADV_PCI_DEVICE(0x2052),
+	ADV_PCI_DEVICE(0x1681),
+	ADV_PCI_DEVICE(0xc001),
+	ADV_PCI_DEVICE(0xc002),
+	ADV_PCI_DEVICE(0xc004),
+	ADV_PCI_DEVICE(0xc101),
+	ADV_PCI_DEVICE(0xc102),
+	ADV_PCI_DEVICE(0xc104),
+	/* required last entry */
+	{ }
+};
+
+MODULE_DEVICE_TABLE(pci, adv_pci_tbl);
+
+static u8 rtcan_adv_pci_read_reg(struct rtcan_device *dev, int port)
+{
+	struct rtcan_adv_pci *board = (struct rtcan_adv_pci *)dev->board_priv;
+	return ioread8(board->base_addr + port);
+}
+
+static void rtcan_adv_pci_write_reg(struct rtcan_device *dev, int port, u8 data)
+{
+	struct rtcan_adv_pci *board = (struct rtcan_adv_pci *)dev->board_priv;
+	iowrite8(data, board->base_addr + port);
+}
+
+static void rtcan_adv_pci_del_chan(
+	struct pci_dev *pdev,
+	struct rtcan_device *dev)
+{
+	struct rtcan_adv_pci *board;
+
+	if (!dev)
+		return;
+
+	board = (struct rtcan_adv_pci *)dev->board_priv;
+
+	rtcan_sja1000_unregister(dev);
+
+	pci_iounmap(pdev, board->base_addr);
+
+	rtcan_dev_free(dev);
+}
+
+
+static int rtcan_adv_pci_add_chan(
+	struct pci_dev *pdev,
+	int	channel,
+	unsigned int bar,
+	unsigned int offset,
+	struct rtcan_device **master_dev)
+{
+	struct rtcan_device *dev;
+	struct rtcan_sja1000 *chip;
+	struct rtcan_adv_pci *board;
+	void __iomem *base_addr;
+	int ret;
+
+	dev = rtcan_dev_alloc(sizeof(struct rtcan_sja1000),
+						  sizeof(struct rtcan_adv_pci));
+	if (dev == NULL)
+		return -ENOMEM;
+
+	chip  = (struct rtcan_sja1000 *)dev->priv;
+	board = (struct rtcan_adv_pci *)dev->board_priv;
+
+	base_addr = pci_iomap(pdev, bar, ADV_PCI_BASE_SIZE) + offset;
+
+	board->pci_dev = pdev;
+	board->conf_addr = NULL;
+	board->base_addr = base_addr;
+
+	if (channel == CHANNEL_SLAVE) {
+		struct rtcan_adv_pci *master_board = \
+			(struct rtcan_adv_pci *)(*master_dev)->board_priv;
+		master_board->slave_dev = dev;
+	}
+
+	dev->board_name = adv_pci_board_name;
+
+	chip->read_reg  = rtcan_adv_pci_read_reg;
+	chip->write_reg = rtcan_adv_pci_write_reg;
+
+	/* Clock frequency in Hz */
+	dev->can_sys_clock = ADV_PCI_CAN_CLOCK;
+
+	/* Output control register */
+	chip->ocr = ADV_PCI_OCR;
+
+	/* Clock divider register */
+	chip->cdr = ADV_PCI_CDR;
+
+	strncpy(dev->name, RTCAN_DEV_NAME, IFNAMSIZ);
+
+    /* Make sure SJA1000 is in reset mode */
+	chip->write_reg(dev, SJA_MOD, SJA_MOD_RM);
+	/* Set PeliCAN mode */
+	chip->write_reg(dev, SJA_CDR, SJA_CDR_CAN_MODE);
+
+	/* check if mode is set */
+	ret = chip->read_reg(dev, SJA_CDR);
+
+	if (ret != SJA_CDR_CAN_MODE)
+		return -EIO;
+
+	/* Register and setup interrupt handling */
+	chip->irq_flags = RTDM_IRQTYPE_SHARED;
+	chip->irq_num   = pdev->irq;
+
+	RTCAN_DBG("%s: base_addr=%p conf_addr=%p irq=%d ocr=%#x cdr=%#x\n",
+			  RTCAN_DRV_NAME, board->base_addr, board->conf_addr,
+			  chip->irq_num, chip->ocr, chip->cdr);
+
+	/* Register SJA1000 device */
+	ret = rtcan_sja1000_register(dev);
+
+	if (ret) {
+		printk(KERN_ERR "ERROR %d while trying to register SJA1000 device!\n",
+			   ret);
+		goto failure;
+	}
+
+	if (channel != CHANNEL_SLAVE)
+		*master_dev = dev;
+
+	return 0;
+
+failure:
+	rtcan_dev_free(dev);
+
+	return ret;
+}
+
+static int __devinit adv_pci_init_one(
+	struct pci_dev *pdev,
+	const struct pci_device_id *ent)
+{
+	int ret, channel;
+	unsigned int nb_ports = 0;
+	unsigned int bar = 0;
+	unsigned int bar_flag = 0;
+	unsigned int offset = 0;
+	unsigned int ix;
+
+	struct rtcan_device *master_dev = NULL;
+
+	dev_info(&pdev->dev, "RTCAN Registering card");
+
+	ret = pci_enable_device(pdev);
+	if (ret)
+		goto failure;
+
+	dev_info(&pdev->dev, "RTCAN Detected Advantech PCI card at slot #%i\n",
+			 PCI_SLOT(pdev->devfn));
+
+	ret = pci_request_regions(pdev, RTCAN_DRV_NAME);
+	if (ret)
+		goto failure;
+
+	switch (pdev->device) {
+	case 0xc001:
+	case 0xc002:
+	case 0xc004:
+	case 0xc101:
+	case 0xc102:
+	case 0xc104:
+		nb_ports = pdev->device & 0x7;
+		offset   = 0x100;
+		bar      = 0;
+		break;
+	case 0x1680:
+	case 0x2052:
+		nb_ports = 2;
+		bar		 = 2;
+		bar_flag = 1;
+		break;
+	case 0x1681:
+		nb_ports = 1;
+		bar      = 2;
+		bar_flag = 1;
+		break;
+	default:
+		break;
+	}
+
+	if (nb_ports > 1)
+		channel = CHANNEL_MASTER;
+	else
+		channel = CHANNEL_SINGLE;
+
+	RTCAN_DBG("%s: Initializing device %04x:%04x:%04x\n",\
+			  RTCAN_DRV_NAME, \
+			  pdev->vendor, \
+			  pdev->device, \
+			  pdev->subsystem_device);
+
+	ret = rtcan_adv_pci_add_chan(pdev, channel, bar, offset, &master_dev);
+	if (ret)
+		goto failure_iounmap;
+
+	/* register slave channel, if any	*/
+
+	for (ix = 1; ix < nb_ports; ix++) {
+		ret = rtcan_adv_pci_add_chan(pdev,
+									 CHANNEL_SLAVE,
+									 bar + (bar_flag ? ix : 0),
+									 offset*ix,
+									 &master_dev);
+		if (ret)
+			goto failure_iounmap;
+	}
+
+	pci_set_drvdata(pdev, master_dev);
+
+	return 0;
+
+failure_iounmap:
+
+	if (master_dev)
+		rtcan_adv_pci_del_chan(pdev, master_dev);
+
+	pci_release_regions(pdev);
+
+failure:
+	return ret;
+}
+
+static void __devexit adv_pci_remove_one(struct pci_dev *pdev)
+{
+	struct rtcan_device *dev = pci_get_drvdata(pdev);
+	struct rtcan_adv_pci *board = (struct rtcan_adv_pci *)dev->board_priv;
+
+	if (board->slave_dev)
+		rtcan_adv_pci_del_chan(pdev, board->slave_dev);
+
+	rtcan_adv_pci_del_chan(pdev, dev);
+
+	pci_release_regions(pdev);
+	pci_disable_device(pdev);
+	pci_set_drvdata(pdev, NULL);
+}
+
+static struct pci_driver rtcan_adv_pci_driver = {
+	.name		= RTCAN_DRV_NAME,
+	.id_table	= adv_pci_tbl,
+	.probe		= adv_pci_init_one,
+	.remove		= __devexit_p(adv_pci_remove_one),
+};
+
+static int __init rtcan_adv_pci_init(void)
+{
+	return pci_register_driver(&rtcan_adv_pci_driver);
+}
+
+
+static void __exit rtcan_adv_pci_exit(void)
+{
+	pci_unregister_driver(&rtcan_adv_pci_driver);
+}
+
+
+module_init(rtcan_adv_pci_init);
+module_exit(rtcan_adv_pci_exit);

Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter

[Wolfgang Grandegger]

Hi Thierry,

I removed the Xenomai ML as it is off-topic there...

On 07/18/2012 12:00 PM, Thierry Bultel wrote:
> Hi Wolfgang,
> 
> After some tuning of thunderbird things should get better now.

Yes, you are close... just a few coding style issues remaining.

[FYI: for Thunderbird I use the add-on "Toggle Word Wrap".]

> As you required, I have added a comment in Kconfig, about the 1680U which
> is the only board I now about. 
> 
> Regarding the "80 chars" stuff, what I meant is that it remains a single
> location where checkpatch complains, but I do not know how to fix it;
> line is actually only 64 chars long at most so that is a little puzzling. 
> 
> WARNING: line over 80 characters
> #361: FILE: ksrc/drivers/can/sja1000/rtcan_adv_pci.c:293:
> +									 CHANNEL_SLAVE,

Here I count 9 tabs plus 15 chars = 87 chars! A tab is 8 chars for Linux!

> All the idents are done with tabs (it was the case in the previous mail as well but thunderbird broke it).
> so checkpatch is happy for everything else.

The tabs still look wired...

> Thanks in advance for the integration;
> Cheers
> Thierry
> 
> 
> diff -pruN xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/Kconfig xenomai-2.6.0/ksrc/drivers/can/sja1000/Kconfig
> --- xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/Kconfig	2011-10-18 20:17:18.000000000 +0200
> +++ xenomai-2.6.0/ksrc/drivers/can/sja1000/Kconfig	2012-07-18 11:46:44.578890869 +0200
> @@ -41,6 +41,15 @@ config XENO_DRIVERS_CAN_SJA1000_IXXAT_PC
>  	the second channel working, Xenomai's shared interrupt support
>  	must be enabled.
>  
> +config XENO_DRIVERS_CAN_SJA1000_ADV_PCI
> +	depends on XENO_DRIVERS_CAN_SJA1000 && PCI
> +	tristate "ADVANTECH PCI Card"
> +	help
> +
> +	This driver is for the ADVANTECH PCI cards (1 or more channels)
> +	It supports the 1680U and some other ones.
> +
> +
>  config XENO_DRIVERS_CAN_SJA1000_PLX_PCI
>  	depends on XENO_DRIVERS_CAN_SJA1000 && PCI
>  	tristate "PLX90xx PCI-bridge based Cards"
> diff -pruN xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/Makefile xenomai-2.6.0/ksrc/drivers/can/sja1000/Makefile
> --- xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/Makefile	2011-10-18 20:17:18.000000000 +0200
> +++ xenomai-2.6.0/ksrc/drivers/can/sja1000/Makefile	2012-07-16 14:00:38.682836737 +0200
> @@ -9,6 +9,7 @@ obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PE
>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PEAK_DNG) += xeno_can_peak_dng.o
>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PLX_PCI) += xeno_can_plx_pci.o
>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_IXXAT_PCI) += xeno_can_ixxat_pci.o
> +obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ADV_PCI) += xeno_can_adv_pci.o
>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_EMS_PCI) += xeno_can_ems_pci.o
>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ESD_PCI) += xeno_can_esd_pci.o
>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ISA) += xeno_can_isa.o
> @@ -19,6 +20,7 @@ xeno_can_peak_pci-y := rtcan_peak_pci.o
>  xeno_can_peak_dng-y := rtcan_peak_dng.o
>  xeno_can_plx_pci-y := rtcan_plx_pci.o
>  xeno_can_ixxat_pci-y := rtcan_ixxat_pci.o
> +xeno_can_adv_pci-y := rtcan_adv_pci.o
>  xeno_can_ems_pci-y := rtcan_ems_pci.o
>  xeno_can_esd_pci-y := rtcan_esd_pci.o
>  xeno_can_isa-y := rtcan_isa.o
> @@ -35,6 +37,7 @@ obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PE
>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PEAK_DNG) += xeno_can_peak_dng.o 
>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PLX_PCI) += xeno_can_plx_pci.o
>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_IXXAT_PCI) += xeno_can_ixxat_pci.o
> +obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ADV_PCI) += xeno_can_adv_pci.o
>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_EMS_PCI) += xeno_can_ems_pci.o
>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ESD_PCI) += xeno_can_esd_pci.o
>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ISA) += xeno_can_isa.o 
> @@ -47,6 +50,7 @@ xeno_can_peak_pci-objs := rtcan_peak_pci
>  xeno_can_peak_dng-objs := rtcan_peak_dng.o
>  xeno_can_plx_pci-objs := rtcan_plx_pci.o
>  xeno_can_ixxat_pci-objs := rtcan_ixxat_pci.o
> +xeno_can_adv_pci-objs := rtcan_adv_pci.o
>  xeno_can_ems_pci-objs := rtcan_ems_pci.o
>  xeno_can_esd_pci-objs := rtcan_esd_pci.o
>  xeno_can_isa-objs := rtcan_isa.o
> @@ -73,6 +77,9 @@ xeno_can_plx_pci.o: $(xeno_can_plx_pci-o
>  xeno_can_ixxat_pci.o: $(xeno_can_ixxat_pci-objs)
>  	$(LD) -r -o $@ $(xeno_can_ixxat_pci-objs)
>  
> +xeno_can_adv_pci.o: $(xeno_can_adv_pci-objs)
> +	$(LD) -r -o $@ $(xeno_can_adv_pci-objs)
> +
>  xeno_can_ems_pci.o: $(xeno_can_ems_pci-objs)
>  	$(LD) -r -o $@ $(xeno_can_ems_pci-objs)
>  
> diff -pruN xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/rtcan_adv_pci.c xenomai-2.6.0/ksrc/drivers/can/sja1000/rtcan_adv_pci.c
> --- xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/rtcan_adv_pci.c	1970-01-01 01:00:00.000000000 +0100
> +++ xenomai-2.6.0/ksrc/drivers/can/sja1000/rtcan_adv_pci.c	2012-07-16 16:58:14.175087435 +0200
> @@ -0,0 +1,351 @@
> +/*
> + * Copyright (C) 2006 Wolfgang Grandegger <wg@grandegger.com>

Your copyright seems to be missing.

> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software Foundation,
> + * Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/ioport.h>
> +#include <linux/delay.h>
> +#include <linux/pci.h>
> +#include <linux/io.h>
> +
> +#include <rtdm/rtdm_driver.h>
> +
> +#define ADV_PCI_BASE_SIZE	0x80
> +
> +/* CAN device profile */
> +#include <rtdm/rtcan.h>
> +#include <rtcan_dev.h>
> +#include <rtcan_raw.h>
> +#include <rtcan_internal.h>
> +#include <rtcan_sja1000.h>
> +#include <rtcan_sja1000_regs.h>
> +
> +#define RTCAN_DEV_NAME "rtcan%d"
> +#define RTCAN_DRV_NAME "ADV-PCI-CAN"
> +
> +static char *adv_pci_board_name = "ADV-PCI";
> +
> +MODULE_AUTHOR("Thierry Bultel <thierry.bultel@basystemes.fr>");
> +MODULE_DESCRIPTION("RTCAN board driver for Advantech PCI cards");
> +MODULE_SUPPORTED_DEVICE("ADV-PCI card CAN controller");
> +MODULE_LICENSE("GPL");
> +
> +struct rtcan_adv_pci {
> +	struct pci_dev *pci_dev;
> +	struct rtcan_device *slave_dev;
> +	void __iomem *conf_addr;
> +	void __iomem *base_addr;
> +};
> +
> +/*
> + * According to the datasheet,
> + * internal clock is 1/2 of the external oscillator frequency
> + * which is 16 MHz
> + */
> +#define ADV_PCI_CAN_CLOCK (16000000 / 2)
> +
> +/*
> + * Output control register
> +  Depends on the board configuration
> + */
> +
> +#define ADV_PCI_OCR (SJA_OCR_MODE_NORMAL	|	\
> +					 SJA_OCR_TX0_PUSHPULL	|	\
> +					 SJA_OCR_TX1_PUSHPULL	|	\
> +					 SJA_OCR_TX1_INVERT)
> +

Less tabs please...

> +/*
> + * In the CDR register, you should set CBP to 1.
> + */
> +#define ADV_PCI_CDR (SJA_CDR_CBP | SJA_CDR_CAN_MODE)
> +
> +
> +#define ADV_PCI_VENDOR_ID 0x13fe
> +
> +#define CHANNEL_SINGLE 0 /* this is a single channel device */
> +#define CHANNEL_MASTER 1 /* multi channel device, this device is master */
> +#define CHANNEL_SLAVE  2 /* multi channel device, this is slave */
> +
> +#define ADV_PCI_DEVICE(device_id)\
> +	{ ADV_PCI_VENDOR_ID, device_id, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 }
> +
> +static DEFINE_PCI_DEVICE_TABLE(adv_pci_tbl) = {
> +	ADV_PCI_DEVICE(0x1680),
> +	ADV_PCI_DEVICE(0x3680),
> +	ADV_PCI_DEVICE(0x2052),
> +	ADV_PCI_DEVICE(0x1681),
> +	ADV_PCI_DEVICE(0xc001),
> +	ADV_PCI_DEVICE(0xc002),
> +	ADV_PCI_DEVICE(0xc004),
> +	ADV_PCI_DEVICE(0xc101),
> +	ADV_PCI_DEVICE(0xc102),
> +	ADV_PCI_DEVICE(0xc104),
> +	/* required last entry */
> +	{ }
> +};
> +
> +MODULE_DEVICE_TABLE(pci, adv_pci_tbl);
> +
> +static u8 rtcan_adv_pci_read_reg(struct rtcan_device *dev, int port)
> +{
> +	struct rtcan_adv_pci *board = (struct rtcan_adv_pci *)dev->board_priv;
> +	return ioread8(board->base_addr + port);
> +}
> +
> +static void rtcan_adv_pci_write_reg(struct rtcan_device *dev, int port, u8 data)
> +{
> +	struct rtcan_adv_pci *board = (struct rtcan_adv_pci *)dev->board_priv;

Add an empty line here please.

> +	iowrite8(data, board->base_addr + port);
> +}
> +
> +static void rtcan_adv_pci_del_chan(
> +	struct pci_dev *pdev,
> +	struct rtcan_device *dev)
> +{
> +	struct rtcan_adv_pci *board;
> +
> +	if (!dev)
> +		return;
> +
> +	board = (struct rtcan_adv_pci *)dev->board_priv;
> +
> +	rtcan_sja1000_unregister(dev);
> +
> +	pci_iounmap(pdev, board->base_addr);
> +
> +	rtcan_dev_free(dev);
> +}
> +
> +
> +static int rtcan_adv_pci_add_chan(
> +	struct pci_dev *pdev,
> +	int	channel,

Please don't use tabs for alignment. Just one space if fine.

> +	unsigned int bar,
> +	unsigned int offset,
> +	struct rtcan_device **master_dev)
> +{
> +	struct rtcan_device *dev;
> +	struct rtcan_sja1000 *chip;
> +	struct rtcan_adv_pci *board;
> +	void __iomem *base_addr;
> +	int ret;
> +
> +	dev = rtcan_dev_alloc(sizeof(struct rtcan_sja1000),
> +						  sizeof(struct rtcan_adv_pci));

Less tabs!

> +	if (dev == NULL)
> +		return -ENOMEM;
> +
> +	chip  = (struct rtcan_sja1000 *)dev->priv;
> +	board = (struct rtcan_adv_pci *)dev->board_priv;
> +
> +	base_addr = pci_iomap(pdev, bar, ADV_PCI_BASE_SIZE) + offset;
> +
> +	board->pci_dev = pdev;
> +	board->conf_addr = NULL;
> +	board->base_addr = base_addr;
> +
> +	if (channel == CHANNEL_SLAVE) {
> +		struct rtcan_adv_pci *master_board = \
> +			(struct rtcan_adv_pci *)(*master_dev)->board_priv;
> +		master_board->slave_dev = dev;
> +	}
> +
> +	dev->board_name = adv_pci_board_name;
> +
> +	chip->read_reg  = rtcan_adv_pci_read_reg;
> +	chip->write_reg = rtcan_adv_pci_write_reg;
> +
> +	/* Clock frequency in Hz */
> +	dev->can_sys_clock = ADV_PCI_CAN_CLOCK;
> +
> +	/* Output control register */
> +	chip->ocr = ADV_PCI_OCR;
> +
> +	/* Clock divider register */
> +	chip->cdr = ADV_PCI_CDR;
> +
> +	strncpy(dev->name, RTCAN_DEV_NAME, IFNAMSIZ);
> +
> +    /* Make sure SJA1000 is in reset mode */

Tabs, please!

> +	chip->write_reg(dev, SJA_MOD, SJA_MOD_RM);
> +	/* Set PeliCAN mode */
> +	chip->write_reg(dev, SJA_CDR, SJA_CDR_CAN_MODE);
> +
> +	/* check if mode is set */
> +	ret = chip->read_reg(dev, SJA_CDR);
> +
> +	if (ret != SJA_CDR_CAN_MODE)
> +		return -EIO;
> +
> +	/* Register and setup interrupt handling */
> +	chip->irq_flags = RTDM_IRQTYPE_SHARED;
> +	chip->irq_num   = pdev->irq;

See my comment about alignment above.

> +
> +	RTCAN_DBG("%s: base_addr=%p conf_addr=%p irq=%d ocr=%#x cdr=%#x\n",
> +			  RTCAN_DRV_NAME, board->base_addr, board->conf_addr,
> +			  chip->irq_num, chip->ocr, chip->cdr);

Tabs plus spaces? Please use a consistant indention style.

> +
> +	/* Register SJA1000 device */
> +	ret = rtcan_sja1000_register(dev);
> +

Remove this empty line please!

> +	if (ret) {
> +		printk(KERN_ERR "ERROR %d while trying to register SJA1000 device!\n",
> +			   ret);
> +		goto failure;
> +	}
> +
> +	if (channel != CHANNEL_SLAVE)
> +		*master_dev = dev;
> +
> +	return 0;
> +
> +failure:
> +	rtcan_dev_free(dev);
> +
> +	return ret;
> +}
> +
> +static int __devinit adv_pci_init_one(
> +	struct pci_dev *pdev,

Should go one line up. This style is not used in Linux! Also true for
the functions above.

> +	const struct pci_device_id *ent)
> +{
> +	int ret, channel;
> +	unsigned int nb_ports = 0;
> +	unsigned int bar = 0;
> +	unsigned int bar_flag = 0;
> +	unsigned int offset = 0;
> +	unsigned int ix;
> +
> +	struct rtcan_device *master_dev = NULL;
> +
> +	dev_info(&pdev->dev, "RTCAN Registering card");
> +
> +	ret = pci_enable_device(pdev);
> +	if (ret)
> +		goto failure;
> +
> +	dev_info(&pdev->dev, "RTCAN Detected Advantech PCI card at slot #%i\n",
> +			 PCI_SLOT(pdev->devfn));
> +
> +	ret = pci_request_regions(pdev, RTCAN_DRV_NAME);
> +	if (ret)
> +		goto failure;
> +
> +	switch (pdev->device) {
> +	case 0xc001:
> +	case 0xc002:
> +	case 0xc004:
> +	case 0xc101:
> +	case 0xc102:
> +	case 0xc104:
> +		nb_ports = pdev->device & 0x7;
> +		offset   = 0x100;
> +		bar      = 0;
> +		break;
> +	case 0x1680:
> +	case 0x2052:
> +		nb_ports = 2;
> +		bar		 = 2;

No tabs. Just one space before and after "="!

> +		bar_flag = 1;
> +		break;
> +	case 0x1681:
> +		nb_ports = 1;
> +		bar      = 2;
> +		bar_flag = 1;
> +		break;
> +	default:

Is this an error case?

> +		break;
> +	}
> +
> +	if (nb_ports > 1)
> +		channel = CHANNEL_MASTER;
> +	else
> +		channel = CHANNEL_SINGLE;
> +
> +	RTCAN_DBG("%s: Initializing device %04x:%04x:%04x\n",\
> +			  RTCAN_DRV_NAME, \
> +			  pdev->vendor, \
> +			  pdev->device, \
> +			  pdev->subsystem_device);

You do not need the backslashes!

> +	ret = rtcan_adv_pci_add_chan(pdev, channel, bar, offset, &master_dev);
> +	if (ret)
> +		goto failure_iounmap;
> +
> +	/* register slave channel, if any	*/
> +
> +	for (ix = 1; ix < nb_ports; ix++) {
> +		ret = rtcan_adv_pci_add_chan(pdev,
> +									 CHANNEL_SLAVE,
> +									 bar + (bar_flag ? ix : 0),
> +									 offset*ix,
> +									 &master_dev);

Too much tabs! Spaces before and after "*"!

> +		if (ret)
> +			goto failure_iounmap;
> +	}
> +
> +	pci_set_drvdata(pdev, master_dev);
> +
> +	return 0;
> +
> +failure_iounmap:
> +
> +	if (master_dev)
> +		rtcan_adv_pci_del_chan(pdev, master_dev);
> +
> +	pci_release_regions(pdev);
> +
> +failure:
> +	return ret;
> +}
> +
> +static void __devexit adv_pci_remove_one(struct pci_dev *pdev)
> +{
> +	struct rtcan_device *dev = pci_get_drvdata(pdev);
> +	struct rtcan_adv_pci *board = (struct rtcan_adv_pci *)dev->board_priv;
> +
> +	if (board->slave_dev)
> +		rtcan_adv_pci_del_chan(pdev, board->slave_dev);
> +
> +	rtcan_adv_pci_del_chan(pdev, dev);
> +
> +	pci_release_regions(pdev);
> +	pci_disable_device(pdev);
> +	pci_set_drvdata(pdev, NULL);
> +}
> +
> +static struct pci_driver rtcan_adv_pci_driver = {
> +	.name		= RTCAN_DRV_NAME,
> +	.id_table	= adv_pci_tbl,
> +	.probe		= adv_pci_init_one,
> +	.remove		= __devexit_p(adv_pci_remove_one),

Don't align with tabs, just use one space before and after "=".

> +};
> +
> +static int __init rtcan_adv_pci_init(void)
> +{
> +	return pci_register_driver(&rtcan_adv_pci_driver);
> +}
> +
> +

Remove one empty line.

> +static void __exit rtcan_adv_pci_exit(void)
> +{
> +	pci_unregister_driver(&rtcan_adv_pci_driver);
> +}
> +
> +
Ditto.

> +module_init(rtcan_adv_pci_init);
> +module_exit(rtcan_adv_pci_exit);

Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter

[Wolfgang Grandegger]

On 07/18/2012 01:19 PM, Wolfgang Grandegger wrote:
> Hi Thierry,
> 
> I removed the Xenomai ML as it is off-topic there...

Oops, it's the other way round. Swapping mailing lists...

> On 07/18/2012 12:00 PM, Thierry Bultel wrote:
>> Hi Wolfgang,
>>
>> After some tuning of thunderbird things should get better now.
> 
> Yes, you are close... just a few coding style issues remaining.
> 
> [FYI: for Thunderbird I use the add-on "Toggle Word Wrap".]
> 
>> As you required, I have added a comment in Kconfig, about the 1680U which
>> is the only board I now about. 
>>
>> Regarding the "80 chars" stuff, what I meant is that it remains a single
>> location where checkpatch complains, but I do not know how to fix it;
>> line is actually only 64 chars long at most so that is a little puzzling. 
>>
>> WARNING: line over 80 characters
>> #361: FILE: ksrc/drivers/can/sja1000/rtcan_adv_pci.c:293:
>> +									 CHANNEL_SLAVE,
> 
> Here I count 9 tabs plus 15 chars = 87 chars! A tab is 8 chars for Linux!
> 
>> All the idents are done with tabs (it was the case in the previous mail as well but thunderbird broke it).
>> so checkpatch is happy for everything else.
> 
> The tabs still look wired...
> 
>> Thanks in advance for the integration;
>> Cheers
>> Thierry
>>
>>
>> diff -pruN xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/Kconfig xenomai-2.6.0/ksrc/drivers/can/sja1000/Kconfig
>> --- xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/Kconfig	2011-10-18 20:17:18.000000000 +0200
>> +++ xenomai-2.6.0/ksrc/drivers/can/sja1000/Kconfig	2012-07-18 11:46:44.578890869 +0200
>> @@ -41,6 +41,15 @@ config XENO_DRIVERS_CAN_SJA1000_IXXAT_PC
>>  	the second channel working, Xenomai's shared interrupt support
>>  	must be enabled.
>>  
>> +config XENO_DRIVERS_CAN_SJA1000_ADV_PCI
>> +	depends on XENO_DRIVERS_CAN_SJA1000 && PCI
>> +	tristate "ADVANTECH PCI Card"
>> +	help
>> +
>> +	This driver is for the ADVANTECH PCI cards (1 or more channels)
>> +	It supports the 1680U and some other ones.
>> +
>> +
>>  config XENO_DRIVERS_CAN_SJA1000_PLX_PCI
>>  	depends on XENO_DRIVERS_CAN_SJA1000 && PCI
>>  	tristate "PLX90xx PCI-bridge based Cards"
>> diff -pruN xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/Makefile xenomai-2.6.0/ksrc/drivers/can/sja1000/Makefile
>> --- xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/Makefile	2011-10-18 20:17:18.000000000 +0200
>> +++ xenomai-2.6.0/ksrc/drivers/can/sja1000/Makefile	2012-07-16 14:00:38.682836737 +0200
>> @@ -9,6 +9,7 @@ obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PE
>>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PEAK_DNG) += xeno_can_peak_dng.o
>>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PLX_PCI) += xeno_can_plx_pci.o
>>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_IXXAT_PCI) += xeno_can_ixxat_pci.o
>> +obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ADV_PCI) += xeno_can_adv_pci.o
>>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_EMS_PCI) += xeno_can_ems_pci.o
>>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ESD_PCI) += xeno_can_esd_pci.o
>>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ISA) += xeno_can_isa.o
>> @@ -19,6 +20,7 @@ xeno_can_peak_pci-y := rtcan_peak_pci.o
>>  xeno_can_peak_dng-y := rtcan_peak_dng.o
>>  xeno_can_plx_pci-y := rtcan_plx_pci.o
>>  xeno_can_ixxat_pci-y := rtcan_ixxat_pci.o
>> +xeno_can_adv_pci-y := rtcan_adv_pci.o
>>  xeno_can_ems_pci-y := rtcan_ems_pci.o
>>  xeno_can_esd_pci-y := rtcan_esd_pci.o
>>  xeno_can_isa-y := rtcan_isa.o
>> @@ -35,6 +37,7 @@ obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PE
>>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PEAK_DNG) += xeno_can_peak_dng.o 
>>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PLX_PCI) += xeno_can_plx_pci.o
>>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_IXXAT_PCI) += xeno_can_ixxat_pci.o
>> +obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ADV_PCI) += xeno_can_adv_pci.o
>>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_EMS_PCI) += xeno_can_ems_pci.o
>>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ESD_PCI) += xeno_can_esd_pci.o
>>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ISA) += xeno_can_isa.o 
>> @@ -47,6 +50,7 @@ xeno_can_peak_pci-objs := rtcan_peak_pci
>>  xeno_can_peak_dng-objs := rtcan_peak_dng.o
>>  xeno_can_plx_pci-objs := rtcan_plx_pci.o
>>  xeno_can_ixxat_pci-objs := rtcan_ixxat_pci.o
>> +xeno_can_adv_pci-objs := rtcan_adv_pci.o
>>  xeno_can_ems_pci-objs := rtcan_ems_pci.o
>>  xeno_can_esd_pci-objs := rtcan_esd_pci.o
>>  xeno_can_isa-objs := rtcan_isa.o
>> @@ -73,6 +77,9 @@ xeno_can_plx_pci.o: $(xeno_can_plx_pci-o
>>  xeno_can_ixxat_pci.o: $(xeno_can_ixxat_pci-objs)
>>  	$(LD) -r -o $@ $(xeno_can_ixxat_pci-objs)
>>  
>> +xeno_can_adv_pci.o: $(xeno_can_adv_pci-objs)
>> +	$(LD) -r -o $@ $(xeno_can_adv_pci-objs)
>> +
>>  xeno_can_ems_pci.o: $(xeno_can_ems_pci-objs)
>>  	$(LD) -r -o $@ $(xeno_can_ems_pci-objs)
>>  
>> diff -pruN xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/rtcan_adv_pci.c xenomai-2.6.0/ksrc/drivers/can/sja1000/rtcan_adv_pci.c
>> --- xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/rtcan_adv_pci.c	1970-01-01 01:00:00.000000000 +0100
>> +++ xenomai-2.6.0/ksrc/drivers/can/sja1000/rtcan_adv_pci.c	2012-07-16 16:58:14.175087435 +0200
>> @@ -0,0 +1,351 @@
>> +/*
>> + * Copyright (C) 2006 Wolfgang Grandegger <wg@grandegger.com>
> 
> Your copyright seems to be missing.
> 
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful, but
>> + * WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software Foundation,
>> + * Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/ioport.h>
>> +#include <linux/delay.h>
>> +#include <linux/pci.h>
>> +#include <linux/io.h>
>> +
>> +#include <rtdm/rtdm_driver.h>
>> +
>> +#define ADV_PCI_BASE_SIZE	0x80
>> +
>> +/* CAN device profile */
>> +#include <rtdm/rtcan.h>
>> +#include <rtcan_dev.h>
>> +#include <rtcan_raw.h>
>> +#include <rtcan_internal.h>
>> +#include <rtcan_sja1000.h>
>> +#include <rtcan_sja1000_regs.h>
>> +
>> +#define RTCAN_DEV_NAME "rtcan%d"
>> +#define RTCAN_DRV_NAME "ADV-PCI-CAN"
>> +
>> +static char *adv_pci_board_name = "ADV-PCI";
>> +
>> +MODULE_AUTHOR("Thierry Bultel <thierry.bultel@basystemes.fr>");
>> +MODULE_DESCRIPTION("RTCAN board driver for Advantech PCI cards");
>> +MODULE_SUPPORTED_DEVICE("ADV-PCI card CAN controller");
>> +MODULE_LICENSE("GPL");
>> +
>> +struct rtcan_adv_pci {
>> +	struct pci_dev *pci_dev;
>> +	struct rtcan_device *slave_dev;
>> +	void __iomem *conf_addr;
>> +	void __iomem *base_addr;
>> +};
>> +
>> +/*
>> + * According to the datasheet,
>> + * internal clock is 1/2 of the external oscillator frequency
>> + * which is 16 MHz
>> + */
>> +#define ADV_PCI_CAN_CLOCK (16000000 / 2)
>> +
>> +/*
>> + * Output control register
>> +  Depends on the board configuration
>> + */
>> +
>> +#define ADV_PCI_OCR (SJA_OCR_MODE_NORMAL	|	\
>> +					 SJA_OCR_TX0_PUSHPULL	|	\
>> +					 SJA_OCR_TX1_PUSHPULL	|	\
>> +					 SJA_OCR_TX1_INVERT)
>> +
> 
> Less tabs please...
> 
>> +/*
>> + * In the CDR register, you should set CBP to 1.
>> + */
>> +#define ADV_PCI_CDR (SJA_CDR_CBP | SJA_CDR_CAN_MODE)
>> +
>> +
>> +#define ADV_PCI_VENDOR_ID 0x13fe
>> +
>> +#define CHANNEL_SINGLE 0 /* this is a single channel device */
>> +#define CHANNEL_MASTER 1 /* multi channel device, this device is master */
>> +#define CHANNEL_SLAVE  2 /* multi channel device, this is slave */
>> +
>> +#define ADV_PCI_DEVICE(device_id)\
>> +	{ ADV_PCI_VENDOR_ID, device_id, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 }
>> +
>> +static DEFINE_PCI_DEVICE_TABLE(adv_pci_tbl) = {
>> +	ADV_PCI_DEVICE(0x1680),
>> +	ADV_PCI_DEVICE(0x3680),
>> +	ADV_PCI_DEVICE(0x2052),
>> +	ADV_PCI_DEVICE(0x1681),
>> +	ADV_PCI_DEVICE(0xc001),
>> +	ADV_PCI_DEVICE(0xc002),
>> +	ADV_PCI_DEVICE(0xc004),
>> +	ADV_PCI_DEVICE(0xc101),
>> +	ADV_PCI_DEVICE(0xc102),
>> +	ADV_PCI_DEVICE(0xc104),
>> +	/* required last entry */
>> +	{ }
>> +};
>> +
>> +MODULE_DEVICE_TABLE(pci, adv_pci_tbl);
>> +
>> +static u8 rtcan_adv_pci_read_reg(struct rtcan_device *dev, int port)
>> +{
>> +	struct rtcan_adv_pci *board = (struct rtcan_adv_pci *)dev->board_priv;
>> +	return ioread8(board->base_addr + port);
>> +}
>> +
>> +static void rtcan_adv_pci_write_reg(struct rtcan_device *dev, int port, u8 data)
>> +{
>> +	struct rtcan_adv_pci *board = (struct rtcan_adv_pci *)dev->board_priv;
> 
> Add an empty line here please.
> 
>> +	iowrite8(data, board->base_addr + port);
>> +}
>> +
>> +static void rtcan_adv_pci_del_chan(
>> +	struct pci_dev *pdev,
>> +	struct rtcan_device *dev)
>> +{
>> +	struct rtcan_adv_pci *board;
>> +
>> +	if (!dev)
>> +		return;
>> +
>> +	board = (struct rtcan_adv_pci *)dev->board_priv;
>> +
>> +	rtcan_sja1000_unregister(dev);
>> +
>> +	pci_iounmap(pdev, board->base_addr);
>> +
>> +	rtcan_dev_free(dev);
>> +}
>> +
>> +
>> +static int rtcan_adv_pci_add_chan(
>> +	struct pci_dev *pdev,
>> +	int	channel,
> 
> Please don't use tabs for alignment. Just one space if fine.
> 
>> +	unsigned int bar,
>> +	unsigned int offset,
>> +	struct rtcan_device **master_dev)
>> +{
>> +	struct rtcan_device *dev;
>> +	struct rtcan_sja1000 *chip;
>> +	struct rtcan_adv_pci *board;
>> +	void __iomem *base_addr;
>> +	int ret;
>> +
>> +	dev = rtcan_dev_alloc(sizeof(struct rtcan_sja1000),
>> +						  sizeof(struct rtcan_adv_pci));
> 
> Less tabs!
> 
>> +	if (dev == NULL)
>> +		return -ENOMEM;
>> +
>> +	chip  = (struct rtcan_sja1000 *)dev->priv;
>> +	board = (struct rtcan_adv_pci *)dev->board_priv;
>> +
>> +	base_addr = pci_iomap(pdev, bar, ADV_PCI_BASE_SIZE) + offset;
>> +
>> +	board->pci_dev = pdev;
>> +	board->conf_addr = NULL;
>> +	board->base_addr = base_addr;
>> +
>> +	if (channel == CHANNEL_SLAVE) {
>> +		struct rtcan_adv_pci *master_board = \
>> +			(struct rtcan_adv_pci *)(*master_dev)->board_priv;
>> +		master_board->slave_dev = dev;
>> +	}
>> +
>> +	dev->board_name = adv_pci_board_name;
>> +
>> +	chip->read_reg  = rtcan_adv_pci_read_reg;
>> +	chip->write_reg = rtcan_adv_pci_write_reg;
>> +
>> +	/* Clock frequency in Hz */
>> +	dev->can_sys_clock = ADV_PCI_CAN_CLOCK;
>> +
>> +	/* Output control register */
>> +	chip->ocr = ADV_PCI_OCR;
>> +
>> +	/* Clock divider register */
>> +	chip->cdr = ADV_PCI_CDR;
>> +
>> +	strncpy(dev->name, RTCAN_DEV_NAME, IFNAMSIZ);
>> +
>> +    /* Make sure SJA1000 is in reset mode */
> 
> Tabs, please!
> 
>> +	chip->write_reg(dev, SJA_MOD, SJA_MOD_RM);
>> +	/* Set PeliCAN mode */
>> +	chip->write_reg(dev, SJA_CDR, SJA_CDR_CAN_MODE);
>> +
>> +	/* check if mode is set */
>> +	ret = chip->read_reg(dev, SJA_CDR);
>> +
>> +	if (ret != SJA_CDR_CAN_MODE)
>> +		return -EIO;
>> +
>> +	/* Register and setup interrupt handling */
>> +	chip->irq_flags = RTDM_IRQTYPE_SHARED;
>> +	chip->irq_num   = pdev->irq;
> 
> See my comment about alignment above.
> 
>> +
>> +	RTCAN_DBG("%s: base_addr=%p conf_addr=%p irq=%d ocr=%#x cdr=%#x\n",
>> +			  RTCAN_DRV_NAME, board->base_addr, board->conf_addr,
>> +			  chip->irq_num, chip->ocr, chip->cdr);
> 
> Tabs plus spaces? Please use a consistant indention style.
> 
>> +
>> +	/* Register SJA1000 device */
>> +	ret = rtcan_sja1000_register(dev);
>> +
> 
> Remove this empty line please!
> 
>> +	if (ret) {
>> +		printk(KERN_ERR "ERROR %d while trying to register SJA1000 device!\n",
>> +			   ret);
>> +		goto failure;
>> +	}
>> +
>> +	if (channel != CHANNEL_SLAVE)
>> +		*master_dev = dev;
>> +
>> +	return 0;
>> +
>> +failure:
>> +	rtcan_dev_free(dev);
>> +
>> +	return ret;
>> +}
>> +
>> +static int __devinit adv_pci_init_one(
>> +	struct pci_dev *pdev,
> 
> Should go one line up. This style is not used in Linux! Also true for
> the functions above.
> 
>> +	const struct pci_device_id *ent)
>> +{
>> +	int ret, channel;
>> +	unsigned int nb_ports = 0;
>> +	unsigned int bar = 0;
>> +	unsigned int bar_flag = 0;
>> +	unsigned int offset = 0;
>> +	unsigned int ix;
>> +
>> +	struct rtcan_device *master_dev = NULL;
>> +
>> +	dev_info(&pdev->dev, "RTCAN Registering card");
>> +
>> +	ret = pci_enable_device(pdev);
>> +	if (ret)
>> +		goto failure;
>> +
>> +	dev_info(&pdev->dev, "RTCAN Detected Advantech PCI card at slot #%i\n",
>> +			 PCI_SLOT(pdev->devfn));
>> +
>> +	ret = pci_request_regions(pdev, RTCAN_DRV_NAME);
>> +	if (ret)
>> +		goto failure;
>> +
>> +	switch (pdev->device) {
>> +	case 0xc001:
>> +	case 0xc002:
>> +	case 0xc004:
>> +	case 0xc101:
>> +	case 0xc102:
>> +	case 0xc104:
>> +		nb_ports = pdev->device & 0x7;
>> +		offset   = 0x100;
>> +		bar      = 0;
>> +		break;
>> +	case 0x1680:
>> +	case 0x2052:
>> +		nb_ports = 2;
>> +		bar		 = 2;
> 
> No tabs. Just one space before and after "="!
> 
>> +		bar_flag = 1;
>> +		break;
>> +	case 0x1681:
>> +		nb_ports = 1;
>> +		bar      = 2;
>> +		bar_flag = 1;
>> +		break;
>> +	default:
> 
> Is this an error case?
> 
>> +		break;
>> +	}
>> +
>> +	if (nb_ports > 1)
>> +		channel = CHANNEL_MASTER;
>> +	else
>> +		channel = CHANNEL_SINGLE;
>> +
>> +	RTCAN_DBG("%s: Initializing device %04x:%04x:%04x\n",\
>> +			  RTCAN_DRV_NAME, \
>> +			  pdev->vendor, \
>> +			  pdev->device, \
>> +			  pdev->subsystem_device);
> 
> You do not need the backslashes!
> 
>> +	ret = rtcan_adv_pci_add_chan(pdev, channel, bar, offset, &master_dev);
>> +	if (ret)
>> +		goto failure_iounmap;
>> +
>> +	/* register slave channel, if any	*/
>> +
>> +	for (ix = 1; ix < nb_ports; ix++) {
>> +		ret = rtcan_adv_pci_add_chan(pdev,
>> +									 CHANNEL_SLAVE,
>> +									 bar + (bar_flag ? ix : 0),
>> +									 offset*ix,
>> +									 &master_dev);
> 
> Too much tabs! Spaces before and after "*"!
> 
>> +		if (ret)
>> +			goto failure_iounmap;
>> +	}
>> +
>> +	pci_set_drvdata(pdev, master_dev);
>> +
>> +	return 0;
>> +
>> +failure_iounmap:
>> +
>> +	if (master_dev)
>> +		rtcan_adv_pci_del_chan(pdev, master_dev);
>> +
>> +	pci_release_regions(pdev);
>> +
>> +failure:
>> +	return ret;
>> +}
>> +
>> +static void __devexit adv_pci_remove_one(struct pci_dev *pdev)
>> +{
>> +	struct rtcan_device *dev = pci_get_drvdata(pdev);
>> +	struct rtcan_adv_pci *board = (struct rtcan_adv_pci *)dev->board_priv;
>> +
>> +	if (board->slave_dev)
>> +		rtcan_adv_pci_del_chan(pdev, board->slave_dev);
>> +
>> +	rtcan_adv_pci_del_chan(pdev, dev);
>> +
>> +	pci_release_regions(pdev);
>> +	pci_disable_device(pdev);
>> +	pci_set_drvdata(pdev, NULL);
>> +}
>> +
>> +static struct pci_driver rtcan_adv_pci_driver = {
>> +	.name		= RTCAN_DRV_NAME,
>> +	.id_table	= adv_pci_tbl,
>> +	.probe		= adv_pci_init_one,
>> +	.remove		= __devexit_p(adv_pci_remove_one),
> 
> Don't align with tabs, just use one space before and after "=".
> 
>> +};
>> +
>> +static int __init rtcan_adv_pci_init(void)
>> +{
>> +	return pci_register_driver(&rtcan_adv_pci_driver);
>> +}
>> +
>> +
> 
> Remove one empty line.
> 
>> +static void __exit rtcan_adv_pci_exit(void)
>> +{
>> +	pci_unregister_driver(&rtcan_adv_pci_driver);
>> +}
>> +
>> +
> Ditto.
> 
>> +module_init(rtcan_adv_pci_init);
>> +module_exit(rtcan_adv_pci_exit);
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-can" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter

[Wolfgang Grandegger]

On 07/18/2012 01:19 PM, Wolfgang Grandegger wrote:
> Hi Thierry,
> 
> I removed the Xenomai ML as it is off-topic there...

Oops, it's the other way round. Swapping mailing lists now.

Sorry for the noise.

Wolfgang.

Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter

[Thierry Bultel]

Wolfgang

- Fixed all the tabs (vim was my friend), all my indents were based on 4 chars display, thus the strange format and the 80 chars issue
- Added copyright
- Fixed missing, or unwanted empty lines
- Function prototypes fixed to linux code style
- '"' and '*' surrounded with 1 space left and right
- 'default' case fixed; treated as an error
- backslashes removed

checkpatch now only complains about the Signed-off-by stuff; I have also checked with a more recent checkpatch version (3.2.9)
than my 2.6.38.8, it has shown spurious whitespaces, and also one space between function name and parenthesis,
I have fixed those issues as well.

Cheers
Thierry


diff -pruN xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/Kconfig xenomai-2.6.0/ksrc/drivers/can/sja1000/Kconfig
--- xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/Kconfig	2011-10-18 20:17:18.000000000 +0200
+++ xenomai-2.6.0/ksrc/drivers/can/sja1000/Kconfig	2012-07-18 11:46:44.578890869 +0200
@@ -41,6 +41,15 @@ config XENO_DRIVERS_CAN_SJA1000_IXXAT_PC
 	the second channel working, Xenomai's shared interrupt support
 	must be enabled.
 
+config XENO_DRIVERS_CAN_SJA1000_ADV_PCI
+	depends on XENO_DRIVERS_CAN_SJA1000 && PCI
+	tristate "ADVANTECH PCI Card"
+	help
+
+	This driver is for the ADVANTECH PCI cards (1 or more channels)
+	It supports the 1680U and some other ones.
+
+
 config XENO_DRIVERS_CAN_SJA1000_PLX_PCI
 	depends on XENO_DRIVERS_CAN_SJA1000 && PCI
 	tristate "PLX90xx PCI-bridge based Cards"
diff -pruN xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/Makefile xenomai-2.6.0/ksrc/drivers/can/sja1000/Makefile
--- xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/Makefile	2011-10-18 20:17:18.000000000 +0200
+++ xenomai-2.6.0/ksrc/drivers/can/sja1000/Makefile	2012-07-16 14:00:38.682836737 +0200
@@ -9,6 +9,7 @@ obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PE
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PEAK_DNG) += xeno_can_peak_dng.o
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PLX_PCI) += xeno_can_plx_pci.o
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_IXXAT_PCI) += xeno_can_ixxat_pci.o
+obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ADV_PCI) += xeno_can_adv_pci.o
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_EMS_PCI) += xeno_can_ems_pci.o
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ESD_PCI) += xeno_can_esd_pci.o
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ISA) += xeno_can_isa.o
@@ -19,6 +20,7 @@ xeno_can_peak_pci-y := rtcan_peak_pci.o
 xeno_can_peak_dng-y := rtcan_peak_dng.o
 xeno_can_plx_pci-y := rtcan_plx_pci.o
 xeno_can_ixxat_pci-y := rtcan_ixxat_pci.o
+xeno_can_adv_pci-y := rtcan_adv_pci.o
 xeno_can_ems_pci-y := rtcan_ems_pci.o
 xeno_can_esd_pci-y := rtcan_esd_pci.o
 xeno_can_isa-y := rtcan_isa.o
@@ -35,6 +37,7 @@ obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PE
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PEAK_DNG) += xeno_can_peak_dng.o 
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PLX_PCI) += xeno_can_plx_pci.o
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_IXXAT_PCI) += xeno_can_ixxat_pci.o
+obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ADV_PCI) += xeno_can_adv_pci.o
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_EMS_PCI) += xeno_can_ems_pci.o
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ESD_PCI) += xeno_can_esd_pci.o
 obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ISA) += xeno_can_isa.o 
@@ -47,6 +50,7 @@ xeno_can_peak_pci-objs := rtcan_peak_pci
 xeno_can_peak_dng-objs := rtcan_peak_dng.o
 xeno_can_plx_pci-objs := rtcan_plx_pci.o
 xeno_can_ixxat_pci-objs := rtcan_ixxat_pci.o
+xeno_can_adv_pci-objs := rtcan_adv_pci.o
 xeno_can_ems_pci-objs := rtcan_ems_pci.o
 xeno_can_esd_pci-objs := rtcan_esd_pci.o
 xeno_can_isa-objs := rtcan_isa.o
@@ -73,6 +77,9 @@ xeno_can_plx_pci.o: $(xeno_can_plx_pci-o
 xeno_can_ixxat_pci.o: $(xeno_can_ixxat_pci-objs)
 	$(LD) -r -o $@ $(xeno_can_ixxat_pci-objs)
 
+xeno_can_adv_pci.o: $(xeno_can_adv_pci-objs)
+	$(LD) -r -o $@ $(xeno_can_adv_pci-objs)
+
 xeno_can_ems_pci.o: $(xeno_can_ems_pci-objs)
 	$(LD) -r -o $@ $(xeno_can_ems_pci-objs)
 
diff -pruN xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/rtcan_adv_pci.c xenomai-2.6.0/ksrc/drivers/can/sja1000/rtcan_adv_pci.c
--- xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/rtcan_adv_pci.c	1970-01-01 01:00:00.000000000 +0100
+++ xenomai-2.6.0/ksrc/drivers/can/sja1000/rtcan_adv_pci.c	2012-07-18 14:21:46.831088314 +0200
@@ -0,0 +1,347 @@
+/*
+ * Copyright (C) 2006 Wolfgang Grandegger <wg@grandegger.com>
+ *
+ * Copyright (C) 2012 Thierry Bultel <thierry.bultel@basystemes.fr>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ */
+
+#include <linux/module.h>
+#include <linux/ioport.h>
+#include <linux/delay.h>
+#include <linux/pci.h>
+#include <linux/io.h>
+
+#include <rtdm/rtdm_driver.h>
+
+#define ADV_PCI_BASE_SIZE	0x80
+
+/* CAN device profile */
+#include <rtdm/rtcan.h>
+#include <rtcan_dev.h>
+#include <rtcan_raw.h>
+#include <rtcan_internal.h>
+#include <rtcan_sja1000.h>
+#include <rtcan_sja1000_regs.h>
+
+#define RTCAN_DEV_NAME "rtcan%d"
+#define RTCAN_DRV_NAME "ADV-PCI-CAN"
+
+static char *adv_pci_board_name = "ADV-PCI";
+
+MODULE_AUTHOR("Thierry Bultel <thierry.bultel@basystemes.fr>");
+MODULE_DESCRIPTION("RTCAN board driver for Advantech PCI cards");
+MODULE_SUPPORTED_DEVICE("ADV-PCI card CAN controller");
+MODULE_LICENSE("GPL");
+
+struct rtcan_adv_pci {
+	struct pci_dev *pci_dev;
+	struct rtcan_device *slave_dev;
+	void __iomem *conf_addr;
+	void __iomem *base_addr;
+};
+
+/*
+ * According to the datasheet,
+ * internal clock is 1/2 of the external oscillator frequency
+ * which is 16 MHz
+ */
+#define ADV_PCI_CAN_CLOCK (16000000 / 2)
+
+/*
+ * Output control register
+  Depends on the board configuration
+ */
+
+#define ADV_PCI_OCR (SJA_OCR_MODE_NORMAL	|\
+		     SJA_OCR_TX0_PUSHPULL	|\
+		     SJA_OCR_TX1_PUSHPULL	|\
+		     SJA_OCR_TX1_INVERT)
+
+/*
+ * In the CDR register, you should set CBP to 1.
+ */
+#define ADV_PCI_CDR (SJA_CDR_CBP | SJA_CDR_CAN_MODE)
+
+#define ADV_PCI_VENDOR_ID 0x13fe
+
+#define CHANNEL_SINGLE 0 /* this is a single channel device */
+#define CHANNEL_MASTER 1 /* multi channel device, this device is master */
+#define CHANNEL_SLAVE  2 /* multi channel device, this is slave */
+
+#define ADV_PCI_DEVICE(device_id)\
+	{ ADV_PCI_VENDOR_ID, device_id, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 }
+
+static DEFINE_PCI_DEVICE_TABLE(adv_pci_tbl) = {
+	ADV_PCI_DEVICE(0x1680),
+	ADV_PCI_DEVICE(0x3680),
+	ADV_PCI_DEVICE(0x2052),
+	ADV_PCI_DEVICE(0x1681),
+	ADV_PCI_DEVICE(0xc001),
+	ADV_PCI_DEVICE(0xc002),
+	ADV_PCI_DEVICE(0xc004),
+	ADV_PCI_DEVICE(0xc101),
+	ADV_PCI_DEVICE(0xc102),
+	ADV_PCI_DEVICE(0xc104),
+	/* required last entry */
+	{ }
+};
+
+MODULE_DEVICE_TABLE(pci, adv_pci_tbl);
+
+static u8 rtcan_adv_pci_read_reg(struct rtcan_device *dev, int port)
+{
+	struct rtcan_adv_pci *board = (struct rtcan_adv_pci *)dev->board_priv;
+	return ioread8(board->base_addr + port);
+}
+
+static void rtcan_adv_pci_write_reg(struct rtcan_device *dev, int port, u8 data)
+{
+	struct rtcan_adv_pci *board = (struct rtcan_adv_pci *)dev->board_priv;
+
+	iowrite8(data, board->base_addr + port);
+}
+
+static void rtcan_adv_pci_del_chan(struct pci_dev *pdev,
+				   struct rtcan_device *dev)
+{
+	struct rtcan_adv_pci *board;
+
+	if (!dev)
+		return;
+
+	board = (struct rtcan_adv_pci *)dev->board_priv;
+
+	rtcan_sja1000_unregister(dev);
+
+	pci_iounmap(pdev, board->base_addr);
+
+	rtcan_dev_free(dev);
+}
+
+
+static int rtcan_adv_pci_add_chan(struct pci_dev *pdev,
+				  int channel,
+				  unsigned int bar,
+				  unsigned int offset,
+				  struct rtcan_device **master_dev)
+{
+	struct rtcan_device *dev;
+	struct rtcan_sja1000 *chip;
+	struct rtcan_adv_pci *board;
+	void __iomem *base_addr;
+	int ret;
+
+	dev = rtcan_dev_alloc(sizeof(struct rtcan_sja1000),
+			      sizeof(struct rtcan_adv_pci));
+	if (dev == NULL)
+		return -ENOMEM;
+
+	chip  = (struct rtcan_sja1000 *)dev->priv;
+	board = (struct rtcan_adv_pci *)dev->board_priv;
+
+	base_addr = pci_iomap(pdev, bar, ADV_PCI_BASE_SIZE) + offset;
+
+	board->pci_dev = pdev;
+	board->conf_addr = NULL;
+	board->base_addr = base_addr;
+
+	if (channel == CHANNEL_SLAVE) {
+		struct rtcan_adv_pci *master_board =
+			(struct rtcan_adv_pci *)(*master_dev)->board_priv;
+		master_board->slave_dev = dev;
+	}
+
+	dev->board_name = adv_pci_board_name;
+
+	chip->read_reg  = rtcan_adv_pci_read_reg;
+	chip->write_reg = rtcan_adv_pci_write_reg;
+
+	/* Clock frequency in Hz */
+	dev->can_sys_clock = ADV_PCI_CAN_CLOCK;
+
+	/* Output control register */
+	chip->ocr = ADV_PCI_OCR;
+
+	/* Clock divider register */
+	chip->cdr = ADV_PCI_CDR;
+
+	strncpy(dev->name, RTCAN_DEV_NAME, IFNAMSIZ);
+
+	/* Make sure SJA1000 is in reset mode */
+	chip->write_reg(dev, SJA_MOD, SJA_MOD_RM);
+	/* Set PeliCAN mode */
+	chip->write_reg(dev, SJA_CDR, SJA_CDR_CAN_MODE);
+
+	/* check if mode is set */
+	ret = chip->read_reg(dev, SJA_CDR);
+	if (ret != SJA_CDR_CAN_MODE)
+		return -EIO;
+
+	/* Register and setup interrupt handling */
+	chip->irq_flags = RTDM_IRQTYPE_SHARED;
+	chip->irq_num = pdev->irq;
+
+	RTCAN_DBG("%s: base_addr=%p conf_addr=%p irq=%d ocr=%#x cdr=%#x\n",
+		   RTCAN_DRV_NAME, board->base_addr, board->conf_addr,
+		   chip->irq_num, chip->ocr, chip->cdr);
+
+	/* Register SJA1000 device */
+	ret = rtcan_sja1000_register(dev);
+	if (ret) {
+		printk(KERN_ERR "ERROR %d while trying to register SJA1000 device!\n",
+		       ret);
+		goto failure;
+	}
+
+	if (channel != CHANNEL_SLAVE)
+		*master_dev = dev;
+
+	return 0;
+
+failure:
+	rtcan_dev_free(dev);
+
+	return ret;
+}
+
+static int __devinit adv_pci_init_one(struct pci_dev *pdev,
+				      const struct pci_device_id *ent)
+{
+	int ret, channel;
+	unsigned int nb_ports = 0;
+	unsigned int bar = 0;
+	unsigned int bar_flag = 0;
+	unsigned int offset = 0;
+	unsigned int ix;
+
+	struct rtcan_device *master_dev = NULL;
+
+	dev_info(&pdev->dev, "RTCAN Registering card");
+
+	ret = pci_enable_device(pdev);
+	if (ret)
+		goto failure;
+
+	dev_info(&pdev->dev, "RTCAN Detected Advantech PCI card at slot #%i\n",
+			 PCI_SLOT(pdev->devfn));
+
+	ret = pci_request_regions(pdev, RTCAN_DRV_NAME);
+	if (ret)
+		goto failure;
+
+	switch (pdev->device) {
+	case 0xc001:
+	case 0xc002:
+	case 0xc004:
+	case 0xc101:
+	case 0xc102:
+	case 0xc104:
+		nb_ports = pdev->device & 0x7;
+		offset = 0x100;
+		bar = 0;
+		break;
+	case 0x1680:
+	case 0x2052:
+		nb_ports = 2;
+		bar = 2;
+		bar_flag = 1;
+		break;
+	case 0x1681:
+		nb_ports = 1;
+		bar = 2;
+		bar_flag = 1;
+		break;
+	default:
+		goto failure_iounmap;
+		break;
+	}
+
+	if (nb_ports > 1)
+		channel = CHANNEL_MASTER;
+	else
+		channel = CHANNEL_SINGLE;
+
+	RTCAN_DBG("%s: Initializing device %04x:%04x:%04x\n",
+		   RTCAN_DRV_NAME,
+		   pdev->vendor,
+		   pdev->device,
+		   pdev->subsystem_device);
+
+	ret = rtcan_adv_pci_add_chan(pdev, channel, bar, offset, &master_dev);
+	if (ret)
+		goto failure_iounmap;
+
+	/* register slave channel, if any	*/
+
+	for (ix = 1; ix < nb_ports; ix++) {
+		ret = rtcan_adv_pci_add_chan(pdev,
+					     CHANNEL_SLAVE,
+					     bar + (bar_flag ? ix : 0),
+					     offset * ix,
+					     &master_dev);
+		if (ret)
+			goto failure_iounmap;
+	}
+
+	pci_set_drvdata(pdev, master_dev);
+
+	return 0;
+
+failure_iounmap:
+
+	if (master_dev)
+		rtcan_adv_pci_del_chan(pdev, master_dev);
+
+	pci_release_regions(pdev);
+
+failure:
+	return ret;
+}
+
+static void __devexit adv_pci_remove_one(struct pci_dev *pdev)
+{
+	struct rtcan_device *dev = pci_get_drvdata(pdev);
+	struct rtcan_adv_pci *board = (struct rtcan_adv_pci *)dev->board_priv;
+
+	if (board->slave_dev)
+		rtcan_adv_pci_del_chan(pdev, board->slave_dev);
+
+	rtcan_adv_pci_del_chan(pdev, dev);
+
+	pci_release_regions(pdev);
+	pci_disable_device(pdev);
+	pci_set_drvdata(pdev, NULL);
+}
+
+static struct pci_driver rtcan_adv_pci_driver = {
+	.name = RTCAN_DRV_NAME,
+	.id_table = adv_pci_tbl,
+	.probe = adv_pci_init_one,
+	.remove = __devexit_p(adv_pci_remove_one),
+};
+
+static int __init rtcan_adv_pci_init(void)
+{
+	return pci_register_driver(&rtcan_adv_pci_driver);
+}
+
+static void __exit rtcan_adv_pci_exit(void)
+{
+	pci_unregister_driver(&rtcan_adv_pci_driver);
+}
+
+module_init(rtcan_adv_pci_init);
+module_exit(rtcan_adv_pci_exit);

Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter

[Wolfgang Grandegger]

Hi Thierry,

now the message went to the wrong mailing list, sorry, my fault! CC goes
now to the Xenomai mailing list.

On 07/18/2012 02:39 PM, Thierry Bultel wrote:
> Wolfgang
> 
> - Fixed all the tabs (vim was my friend), all my indents were based on 4 chars display, thus the strange format and the 80 chars issue
> - Added copyright
> - Fixed missing, or unwanted empty lines
> - Function prototypes fixed to linux code style
> - '"' and '*' surrounded with 1 space left and right
> - 'default' case fixed; treated as an error
> - backslashes removed

Thanks.

> checkpatch now only complains about the Signed-off-by stuff; I have also checked with a more recent checkpatch version (3.2.9)
> than my 2.6.38.8, it has shown spurious whitespaces, and also one space between function name and parenthesis,
> I have fixed those issues as well.

Ah, I forgot about the "signed-off-by" stuff. You should send the patch
as a *separate* mail with the subject "[PATCH] rtcan: ...". In the mail
header there should be a commit message explaining what this patch is
good for followed by your "signed-off-by" line. You will find similar
patches in this mailing list. This will allow us to directly apply the
patch.

At a closer look I realized a few more issues...

> diff -pruN xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/Kconfig xenomai-2.6.0/ksrc/drivers/can/sja1000/Kconfig
> --- xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/Kconfig	2011-10-18 20:17:18.000000000 +0200
> +++ xenomai-2.6.0/ksrc/drivers/can/sja1000/Kconfig	2012-07-18 11:46:44.578890869 +0200
> @@ -41,6 +41,15 @@ config XENO_DRIVERS_CAN_SJA1000_IXXAT_PC
>  	the second channel working, Xenomai's shared interrupt support
>  	must be enabled.
>  
> +config XENO_DRIVERS_CAN_SJA1000_ADV_PCI
> +	depends on XENO_DRIVERS_CAN_SJA1000 && PCI
> +	tristate "ADVANTECH PCI Card"

s/Card/Cards/

> +	help
> +
> +	This driver is for the ADVANTECH PCI cards (1 or more channels)
> +	It supports the 1680U and some other ones.
> +
> +
>  config XENO_DRIVERS_CAN_SJA1000_PLX_PCI
>  	depends on XENO_DRIVERS_CAN_SJA1000 && PCI
>  	tristate "PLX90xx PCI-bridge based Cards"
> diff -pruN xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/Makefile xenomai-2.6.0/ksrc/drivers/can/sja1000/Makefile
> --- xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/Makefile	2011-10-18 20:17:18.000000000 +0200
> +++ xenomai-2.6.0/ksrc/drivers/can/sja1000/Makefile	2012-07-16 14:00:38.682836737 +0200
> @@ -9,6 +9,7 @@ obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PE
>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PEAK_DNG) += xeno_can_peak_dng.o
>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PLX_PCI) += xeno_can_plx_pci.o
>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_IXXAT_PCI) += xeno_can_ixxat_pci.o
> +obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ADV_PCI) += xeno_can_adv_pci.o
>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_EMS_PCI) += xeno_can_ems_pci.o
>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ESD_PCI) += xeno_can_esd_pci.o
>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ISA) += xeno_can_isa.o
> @@ -19,6 +20,7 @@ xeno_can_peak_pci-y := rtcan_peak_pci.o
>  xeno_can_peak_dng-y := rtcan_peak_dng.o
>  xeno_can_plx_pci-y := rtcan_plx_pci.o
>  xeno_can_ixxat_pci-y := rtcan_ixxat_pci.o
> +xeno_can_adv_pci-y := rtcan_adv_pci.o
>  xeno_can_ems_pci-y := rtcan_ems_pci.o
>  xeno_can_esd_pci-y := rtcan_esd_pci.o
>  xeno_can_isa-y := rtcan_isa.o
> @@ -35,6 +37,7 @@ obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PE
>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PEAK_DNG) += xeno_can_peak_dng.o 
>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_PLX_PCI) += xeno_can_plx_pci.o
>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_IXXAT_PCI) += xeno_can_ixxat_pci.o
> +obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ADV_PCI) += xeno_can_adv_pci.o
>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_EMS_PCI) += xeno_can_ems_pci.o
>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ESD_PCI) += xeno_can_esd_pci.o
>  obj-$(CONFIG_XENO_DRIVERS_CAN_SJA1000_ISA) += xeno_can_isa.o 
> @@ -47,6 +50,7 @@ xeno_can_peak_pci-objs := rtcan_peak_pci
>  xeno_can_peak_dng-objs := rtcan_peak_dng.o
>  xeno_can_plx_pci-objs := rtcan_plx_pci.o
>  xeno_can_ixxat_pci-objs := rtcan_ixxat_pci.o
> +xeno_can_adv_pci-objs := rtcan_adv_pci.o
>  xeno_can_ems_pci-objs := rtcan_ems_pci.o
>  xeno_can_esd_pci-objs := rtcan_esd_pci.o
>  xeno_can_isa-objs := rtcan_isa.o
> @@ -73,6 +77,9 @@ xeno_can_plx_pci.o: $(xeno_can_plx_pci-o
>  xeno_can_ixxat_pci.o: $(xeno_can_ixxat_pci-objs)
>  	$(LD) -r -o $@ $(xeno_can_ixxat_pci-objs)
>  
> +xeno_can_adv_pci.o: $(xeno_can_adv_pci-objs)
> +	$(LD) -r -o $@ $(xeno_can_adv_pci-objs)
> +
>  xeno_can_ems_pci.o: $(xeno_can_ems_pci-objs)
>  	$(LD) -r -o $@ $(xeno_can_ems_pci-objs)
>  
> diff -pruN xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/rtcan_adv_pci.c xenomai-2.6.0/ksrc/drivers/can/sja1000/rtcan_adv_pci.c
> --- xenomai-2.6.0-orig/ksrc/drivers/can/sja1000/rtcan_adv_pci.c	1970-01-01 01:00:00.000000000 +0100
> +++ xenomai-2.6.0/ksrc/drivers/can/sja1000/rtcan_adv_pci.c	2012-07-18 14:21:46.831088314 +0200
> @@ -0,0 +1,347 @@
> +/*
> + * Copyright (C) 2006 Wolfgang Grandegger <wg@grandegger.com>
> + *
> + * Copyright (C) 2012 Thierry Bultel <thierry.bultel@basystemes.fr>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software Foundation,
> + * Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/ioport.h>
> +#include <linux/delay.h>
> +#include <linux/pci.h>
> +#include <linux/io.h>
> +
> +#include <rtdm/rtdm_driver.h>
> +
> +#define ADV_PCI_BASE_SIZE	0x80
> +
> +/* CAN device profile */
> +#include <rtdm/rtcan.h>
> +#include <rtcan_dev.h>
> +#include <rtcan_raw.h>
> +#include <rtcan_internal.h>
> +#include <rtcan_sja1000.h>
> +#include <rtcan_sja1000_regs.h>
> +
> +#define RTCAN_DEV_NAME "rtcan%d"
> +#define RTCAN_DRV_NAME "ADV-PCI-CAN"
> +
> +static char *adv_pci_board_name = "ADV-PCI";
> +
> +MODULE_AUTHOR("Thierry Bultel <thierry.bultel@basystemes.fr>");
> +MODULE_DESCRIPTION("RTCAN board driver for Advantech PCI cards");
> +MODULE_SUPPORTED_DEVICE("ADV-PCI card CAN controller");
> +MODULE_LICENSE("GPL");
> +
> +struct rtcan_adv_pci {
> +	struct pci_dev *pci_dev;
> +	struct rtcan_device *slave_dev;
> +	void __iomem *conf_addr;
> +	void __iomem *base_addr;
> +};
> +
> +/*
> + * According to the datasheet,
> + * internal clock is 1/2 of the external oscillator frequency
> + * which is 16 MHz
> + */
> +#define ADV_PCI_CAN_CLOCK (16000000 / 2)
> +
> +/*
> + * Output control register
> +  Depends on the board configuration
> + */
> +
> +#define ADV_PCI_OCR (SJA_OCR_MODE_NORMAL	|\
> +		     SJA_OCR_TX0_PUSHPULL	|\
> +		     SJA_OCR_TX1_PUSHPULL	|\
> +		     SJA_OCR_TX1_INVERT)
> +
> +/*
> + * In the CDR register, you should set CBP to 1.
> + */
> +#define ADV_PCI_CDR (SJA_CDR_CBP | SJA_CDR_CAN_MODE)
> +
> +#define ADV_PCI_VENDOR_ID 0x13fe
> +
> +#define CHANNEL_SINGLE 0 /* this is a single channel device */
> +#define CHANNEL_MASTER 1 /* multi channel device, this device is master */
> +#define CHANNEL_SLAVE  2 /* multi channel device, this is slave */
> +
> +#define ADV_PCI_DEVICE(device_id)\
> +	{ ADV_PCI_VENDOR_ID, device_id, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 }
> +
> +static DEFINE_PCI_DEVICE_TABLE(adv_pci_tbl) = {
> +	ADV_PCI_DEVICE(0x1680),
> +	ADV_PCI_DEVICE(0x3680),
> +	ADV_PCI_DEVICE(0x2052),
> +	ADV_PCI_DEVICE(0x1681),
> +	ADV_PCI_DEVICE(0xc001),
> +	ADV_PCI_DEVICE(0xc002),
> +	ADV_PCI_DEVICE(0xc004),
> +	ADV_PCI_DEVICE(0xc101),
> +	ADV_PCI_DEVICE(0xc102),
> +	ADV_PCI_DEVICE(0xc104),
> +	/* required last entry */
> +	{ }
> +};
> +
> +MODULE_DEVICE_TABLE(pci, adv_pci_tbl);
> +
> +static u8 rtcan_adv_pci_read_reg(struct rtcan_device *dev, int port)
> +{
> +	struct rtcan_adv_pci *board = (struct rtcan_adv_pci *)dev->board_priv;

Add an empty line here as well.

> +	return ioread8(board->base_addr + port);
> +}
> +
> +static void rtcan_adv_pci_write_reg(struct rtcan_device *dev, int port, u8 data)
> +{
> +	struct rtcan_adv_pci *board = (struct rtcan_adv_pci *)dev->board_priv;
> +
> +	iowrite8(data, board->base_addr + port);
> +}
> +
> +static void rtcan_adv_pci_del_chan(struct pci_dev *pdev,
> +				   struct rtcan_device *dev)
> +{
> +	struct rtcan_adv_pci *board;
> +
> +	if (!dev)
> +		return;
> +
> +	board = (struct rtcan_adv_pci *)dev->board_priv;
> +
> +	rtcan_sja1000_unregister(dev);
> +
> +	pci_iounmap(pdev, board->base_addr);
> +
> +	rtcan_dev_free(dev);
> +}
> +
> +
> +static int rtcan_adv_pci_add_chan(struct pci_dev *pdev,
> +				  int channel,
> +				  unsigned int bar,
> +				  unsigned int offset,
> +				  struct rtcan_device **master_dev)
> +{
> +	struct rtcan_device *dev;
> +	struct rtcan_sja1000 *chip;
> +	struct rtcan_adv_pci *board;
> +	void __iomem *base_addr;
> +	int ret;
> +
> +	dev = rtcan_dev_alloc(sizeof(struct rtcan_sja1000),
> +			      sizeof(struct rtcan_adv_pci));
> +	if (dev == NULL)
> +		return -ENOMEM;
> +
> +	chip  = (struct rtcan_sja1000 *)dev->priv;
> +	board = (struct rtcan_adv_pci *)dev->board_priv;
> +
> +	base_addr = pci_iomap(pdev, bar, ADV_PCI_BASE_SIZE) + offset;

For each channel, you map the same address space! Shouldn't that be done
not only once? You should also check the return code of pci_iomap().

> +
> +	board->pci_dev = pdev;
> +	board->conf_addr = NULL;
> +	board->base_addr = base_addr;
> +
> +	if (channel == CHANNEL_SLAVE) {
> +		struct rtcan_adv_pci *master_board =
> +			(struct rtcan_adv_pci *)(*master_dev)->board_priv;
> +		master_board->slave_dev = dev;
> +	}
> +
> +	dev->board_name = adv_pci_board_name;
> +
> +	chip->read_reg  = rtcan_adv_pci_read_reg;
> +	chip->write_reg = rtcan_adv_pci_write_reg;
> +
> +	/* Clock frequency in Hz */
> +	dev->can_sys_clock = ADV_PCI_CAN_CLOCK;
> +
> +	/* Output control register */
> +	chip->ocr = ADV_PCI_OCR;
> +
> +	/* Clock divider register */
> +	chip->cdr = ADV_PCI_CDR;
> +
> +	strncpy(dev->name, RTCAN_DEV_NAME, IFNAMSIZ);
> +
> +	/* Make sure SJA1000 is in reset mode */
> +	chip->write_reg(dev, SJA_MOD, SJA_MOD_RM);
> +	/* Set PeliCAN mode */
> +	chip->write_reg(dev, SJA_CDR, SJA_CDR_CAN_MODE);
> +
> +	/* check if mode is set */
> +	ret = chip->read_reg(dev, SJA_CDR);
> +	if (ret != SJA_CDR_CAN_MODE)
> +		return -EIO;

What about pci_iounmap() and rtcan_dev_free()? goto?

> +
> +	/* Register and setup interrupt handling */
> +	chip->irq_flags = RTDM_IRQTYPE_SHARED;
> +	chip->irq_num = pdev->irq;
> +
> +	RTCAN_DBG("%s: base_addr=%p conf_addr=%p irq=%d ocr=%#x cdr=%#x\n",
> +		   RTCAN_DRV_NAME, board->base_addr, board->conf_addr,
> +		   chip->irq_num, chip->ocr, chip->cdr);
> +
> +	/* Register SJA1000 device */
> +	ret = rtcan_sja1000_register(dev);
> +	if (ret) {
> +		printk(KERN_ERR "ERROR %d while trying to register SJA1000 device!\n",
> +		       ret);
> +		goto failure;
> +	}
> +
> +	if (channel != CHANNEL_SLAVE)
> +		*master_dev = dev;
> +
> +	return 0;
> +
> +failure:
> +	rtcan_dev_free(dev);

What about pci_iounmap()?

> +
> +	return ret;
> +}
> +
> +static int __devinit adv_pci_init_one(struct pci_dev *pdev,
> +				      const struct pci_device_id *ent)
> +{
> +	int ret, channel;
> +	unsigned int nb_ports = 0;
> +	unsigned int bar = 0;
> +	unsigned int bar_flag = 0;
> +	unsigned int offset = 0;
> +	unsigned int ix;
> +
> +	struct rtcan_device *master_dev = NULL;
> +
> +	dev_info(&pdev->dev, "RTCAN Registering card");
> +
> +	ret = pci_enable_device(pdev);
> +	if (ret)
> +		goto failure;
> +
> +	dev_info(&pdev->dev, "RTCAN Detected Advantech PCI card at slot #%i\n",

s/Detected/detected ?

> +			 PCI_SLOT(pdev->devfn));
> +
> +	ret = pci_request_regions(pdev, RTCAN_DRV_NAME);
> +	if (ret)
> +		goto failure;
> +
> +	switch (pdev->device) {
> +	case 0xc001:
> +	case 0xc002:
> +	case 0xc004:
> +	case 0xc101:
> +	case 0xc102:
> +	case 0xc104:
> +		nb_ports = pdev->device & 0x7;
> +		offset = 0x100;
> +		bar = 0;
> +		break;
> +	case 0x1680:
> +	case 0x2052:
> +		nb_ports = 2;
> +		bar = 2;
> +		bar_flag = 1;
> +		break;
> +	case 0x1681:
> +		nb_ports = 1;
> +		bar = 2;
> +		bar_flag = 1;
> +		break;
> +	default:
> +		goto failure_iounmap;
> +		break;

break will never be reached.

> +	}
> +
> +	if (nb_ports > 1)
> +		channel = CHANNEL_MASTER;
> +	else
> +		channel = CHANNEL_SINGLE;
> +
> +	RTCAN_DBG("%s: Initializing device %04x:%04x:%04x\n",
> +		   RTCAN_DRV_NAME,
> +		   pdev->vendor,
> +		   pdev->device,
> +		   pdev->subsystem_device);
> +
> +	ret = rtcan_adv_pci_add_chan(pdev, channel, bar, offset, &master_dev);
> +	if (ret)
> +		goto failure_iounmap;
> +
> +	/* register slave channel, if any	*/

s+	*/+ */+ ?

> +
> +	for (ix = 1; ix < nb_ports; ix++) {
> +		ret = rtcan_adv_pci_add_chan(pdev,
> +					     CHANNEL_SLAVE,
> +					     bar + (bar_flag ? ix : 0),
> +					     offset * ix,
> +					     &master_dev);
> +		if (ret)
> +			goto failure_iounmap;
> +	}
> +
> +	pci_set_drvdata(pdev, master_dev);
> +
> +	return 0;
> +
> +failure_iounmap:
> +
> +	if (master_dev)
> +		rtcan_adv_pci_del_chan(pdev, master_dev);
> +
> +	pci_release_regions(pdev);

pci_disable_device(pdev)?

> +
> +failure:
> +	return ret;
> +}
> +
> +static void __devexit adv_pci_remove_one(struct pci_dev *pdev)
> +{
> +	struct rtcan_device *dev = pci_get_drvdata(pdev);
> +	struct rtcan_adv_pci *board = (struct rtcan_adv_pci *)dev->board_priv;
> +
> +	if (board->slave_dev)
> +		rtcan_adv_pci_del_chan(pdev, board->slave_dev);
> +
> +	rtcan_adv_pci_del_chan(pdev, dev);
> +
> +	pci_release_regions(pdev);
> +	pci_disable_device(pdev);
> +	pci_set_drvdata(pdev, NULL);
> +}
> +
> +static struct pci_driver rtcan_adv_pci_driver = {
> +	.name = RTCAN_DRV_NAME,
> +	.id_table = adv_pci_tbl,
> +	.probe = adv_pci_init_one,
> +	.remove = __devexit_p(adv_pci_remove_one),
> +};
> +
> +static int __init rtcan_adv_pci_init(void)
> +{
> +	return pci_register_driver(&rtcan_adv_pci_driver);
> +}
> +
> +static void __exit rtcan_adv_pci_exit(void)
> +{
> +	pci_unregister_driver(&rtcan_adv_pci_driver);
> +}
> +
> +module_init(rtcan_adv_pci_init);
> +module_exit(rtcan_adv_pci_exit);

Thanks for your patience.

Wolfgang.