* Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter
@ 2012-07-03 20:29 ` Wolfgang Grandegger
0 siblings, 0 replies; 21+ messages in thread
From: Wolfgang Grandegger @ 2012-07-03 20:29 UTC (permalink / raw)
To: Thierry BULTEL; +Cc: Linux-CAN, xenomai
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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter
2012-07-03 20:29 ` Wolfgang Grandegger
(?)
@ 2012-07-04 6:24 ` dietmar.schindler
2012-07-04 6:54 ` Wolfgang Grandegger
-1 siblings, 1 reply; 21+ messages in thread
From: dietmar.schindler @ 2012-07-04 6:24 UTC (permalink / raw)
Cc: xenomai
> 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
________________________________________
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter
2012-07-04 6:24 ` dietmar.schindler
@ 2012-07-04 6:54 ` Wolfgang Grandegger
0 siblings, 0 replies; 21+ messages in thread
From: Wolfgang Grandegger @ 2012-07-04 6:54 UTC (permalink / raw)
To: dietmar.schindler; +Cc: xenomai
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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter
2012-07-03 20:29 ` Wolfgang Grandegger
(?)
(?)
@ 2012-07-16 15:07 ` Thierry Bultel
2012-07-17 8:39 ` Wolfgang Grandegger
-1 siblings, 1 reply; 21+ messages in thread
From: Thierry Bultel @ 2012-07-16 15:07 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: Linux-CAN, xenomai
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);
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter
2012-07-16 15:07 ` Thierry Bultel
@ 2012-07-17 8:39 ` Wolfgang Grandegger
0 siblings, 0 replies; 21+ messages in thread
From: Wolfgang Grandegger @ 2012-07-17 8:39 UTC (permalink / raw)
To: Thierry Bultel; +Cc: xenomai, Linux-CAN
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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter
@ 2012-07-17 8:39 ` Wolfgang Grandegger
0 siblings, 0 replies; 21+ messages in thread
From: Wolfgang Grandegger @ 2012-07-17 8:39 UTC (permalink / raw)
To: Thierry Bultel; +Cc: Linux-CAN, xenomai
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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter
2012-07-17 8:39 ` Wolfgang Grandegger
@ 2012-07-18 10:00 ` Thierry Bultel
-1 siblings, 0 replies; 21+ messages in thread
From: Thierry Bultel @ 2012-07-18 10:00 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: xenomai, Linux-CAN
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);
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter
@ 2012-07-18 10:00 ` Thierry Bultel
0 siblings, 0 replies; 21+ messages in thread
From: Thierry Bultel @ 2012-07-18 10:00 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: Linux-CAN, xenomai
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);
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter
2012-07-18 10:00 ` Thierry Bultel
(?)
@ 2012-07-18 11:19 ` Wolfgang Grandegger
2012-07-18 11:29 ` Wolfgang Grandegger
` (2 more replies)
-1 siblings, 3 replies; 21+ messages in thread
From: Wolfgang Grandegger @ 2012-07-18 11:19 UTC (permalink / raw)
To: Thierry Bultel; +Cc: Linux-CAN
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);
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter
2012-07-18 11:19 ` Wolfgang Grandegger
@ 2012-07-18 11:29 ` Wolfgang Grandegger
2012-07-18 11:31 ` Wolfgang Grandegger
2012-07-18 12:39 ` Thierry Bultel
2 siblings, 0 replies; 21+ messages in thread
From: Wolfgang Grandegger @ 2012-07-18 11:29 UTC (permalink / raw)
To: Thierry Bultel; +Cc: Xenomai
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
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter
2012-07-18 11:19 ` Wolfgang Grandegger
2012-07-18 11:29 ` Wolfgang Grandegger
@ 2012-07-18 11:31 ` Wolfgang Grandegger
2012-07-18 12:39 ` Thierry Bultel
2 siblings, 0 replies; 21+ messages in thread
From: Wolfgang Grandegger @ 2012-07-18 11:31 UTC (permalink / raw)
To: Thierry Bultel; +Cc: Linux-CAN
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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter
2012-07-18 11:19 ` Wolfgang Grandegger
2012-07-18 11:29 ` Wolfgang Grandegger
2012-07-18 11:31 ` Wolfgang Grandegger
@ 2012-07-18 12:39 ` Thierry Bultel
2012-07-18 13:28 ` Wolfgang Grandegger
2 siblings, 1 reply; 21+ messages in thread
From: Thierry Bultel @ 2012-07-18 12:39 UTC (permalink / raw)
To: Wolfgang Grandegger, Linux-CAN
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);
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter
2012-07-18 12:39 ` Thierry Bultel
@ 2012-07-18 13:28 ` Wolfgang Grandegger
0 siblings, 0 replies; 21+ messages in thread
From: Wolfgang Grandegger @ 2012-07-18 13:28 UTC (permalink / raw)
To: Thierry Bultel; +Cc: Xenomai
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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Xenomai] RTDM driver for the Advantech 1680U CANbus Adapter
2012-07-03 20:29 ` Wolfgang Grandegger
` (2 preceding siblings ...)
(?)
@ 2012-07-16 15:07 ` Thierry Bultel
-1 siblings, 0 replies; 21+ messages in thread
From: Thierry Bultel @ 2012-07-16 15:07 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: xenomai, Linux-CAN
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);
^ permalink raw reply [flat|nested] 21+ messages in thread