* [Xenomai-core] [PATCH] rtcan_mem - CAN driver for memory mapped SJA1000 controllers
@ 2006-09-12 11:30 Matthias Fuchs
2006-09-12 12:21 ` Jan Kiszka
2006-09-12 12:22 ` Wolfgang Grandegger
0 siblings, 2 replies; 13+ messages in thread
From: Matthias Fuchs @ 2006-09-12 11:30 UTC (permalink / raw)
To: xenomai
[-- Attachment #1: Type: text/plain, Size: 598 bytes --]
Hi,
attached you will find a patch that adds support for memory mapped SJA1000 CAN
controllers as they often can be found on embedded boards. The driver is
based on the rtmen_isa driver.
The driver has been tested on esd's embedded PowerPC boards with AMCC PPC405
CPUs.
Thanks to Jan for giving me some introduction to Xenomai during a nightly
session last friday.
There's one thing a I am not very satisfied with :-) Why passing half of the
external clock frequency to the module. Because of compatiblity reasons I
kept this behavior of the clock paramter from the ISA driver.
Matthias
[-- Attachment #2: rtcan_mem.patch --]
[-- Type: text/x-diff, Size: 8304 bytes --]
Index: ksrc/drivers/can/sja1000/Kconfig
===================================================================
--- ksrc/drivers/can/sja1000/Kconfig (revision 1564)
+++ ksrc/drivers/can/sja1000/Kconfig (working copy)
@@ -13,6 +13,16 @@
int "Maximum number of ISA devices"
default 4
+config XENO_DRIVERS_RTCAN_SJA1000_MEM
+ depends on XENO_DRIVERS_RTCAN_SJA1000
+ tristate "Memory mapped controllers"
+ default n
+
+config XENO_DRIVERS_RTCAN_SJA1000_MEM_MAX_DEV
+ depends on XENO_DRIVERS_RTCAN_SJA1000_MEM
+ int "Maximum number of memory mapped controllers"
+ default 4
+
config XENO_DRIVERS_RTCAN_SJA1000_PEAK_PCI
depends on XENO_DRIVERS_RTCAN_SJA1000
tristate "PEAK PCI Card"
Index: ksrc/drivers/can/sja1000/rtcan_mem.c
===================================================================
--- ksrc/drivers/can/sja1000/rtcan_mem.c (revision 0)
+++ ksrc/drivers/can/sja1000/rtcan_mem.c (revision 0)
@@ -0,0 +1,206 @@
+/*
+ * Copyright (C) 2006 Matthias Fuchs <matthias.fuchs@domain.hid>,
+ * Jan Kiszka <jan.kiszka@domain.hid>
+ *
+ * RTCAN driver for memory mapped SJA1000 CAN controller
+ * This code has been tested on esd's CPCI405/EPPC405 PPC405 systems.
+ *
+ * This driver is derived from the rtcan-isa driver by
+ * Wolfgang Grandegger and Sebastian Smolorz.
+ *
+ * Copyright (C) 2006 Wolfgang Grandegger <wg@domain.hid>
+ * Copyright (C) 2005, 2006 Sebastian Smolorz
+ * <Sebastian.Smolorz@domain.hid>
+ *
+ *
+ * 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; eitherer 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 <rtdm/rtdm_driver.h>
+
+#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 "sja1000-mem"
+
+#ifdef CONFIG_XENO_DRIVERS_RTCAN_MEM_MAX_DEVS
+#define RTCAN_MEM_MAX_DEV CONFIG_XENO_DRIVERS_RTCAN_MEM_MAX_DEV
+#else
+#define RTCAN_MEM_MAX_DEV 2
+#endif
+
+static char *mem_board_name = "mem mapped";
+
+MODULE_AUTHOR("Matthias Fuchs <matthias.fuchs@domain.hid>");
+MODULE_DESCRIPTION("RTCAN driver for memory mapped SJA1000 controller");
+MODULE_SUPPORTED_DEVICE("mem mapped");
+MODULE_LICENSE("GPL");
+
+static u32 mem[RTCAN_MEM_MAX_DEV];
+static int irq[RTCAN_MEM_MAX_DEV];
+static u32 clock[RTCAN_MEM_MAX_DEV];
+static u8 ocr[RTCAN_MEM_MAX_DEV];
+static u8 cdr[RTCAN_MEM_MAX_DEV];
+
+compat_module_int_param_array(mem, RTCAN_MEM_MAX_DEV);
+compat_module_int_param_array(irq, RTCAN_MEM_MAX_DEV);
+compat_module_int_param_array(clock, RTCAN_MEM_MAX_DEV);
+compat_module_byte_param_array(ocr, RTCAN_MEM_MAX_DEV);
+compat_module_byte_param_array(cdr, RTCAN_MEM_MAX_DEV);
+
+MODULE_PARM_DESC(mem, "The io-memory address");
+MODULE_PARM_DESC(irq, "The interrupt number");
+MODULE_PARM_DESC(clock, "CAN system clock frequency (default 8 MHz)");
+MODULE_PARM_DESC(ocr, "Value of output control register (default 0x1a)");
+MODULE_PARM_DESC(cdr, "Value of clock divider register (default 0xc8");
+
+#define RTCAN_MEM_RANGE 0x80
+
+struct rtcan_mem
+{
+ u8 *mem;
+};
+
+static struct rtcan_device *rtcan_mem_devs[RTCAN_MEM_MAX_DEV];
+
+static u8 rtcan_mem_readreg(struct rtcan_device *dev, int reg)
+{
+ struct rtcan_mem *board = (struct rtcan_mem *)dev->board_priv;
+ return readb(board->mem + reg);
+}
+
+static void rtcan_mem_writereg(struct rtcan_device *dev, int reg, u8 val)
+{
+ struct rtcan_mem *board = (struct rtcan_mem *)dev->board_priv;
+ writeb(val, board->mem + reg);
+}
+
+int __init rtcan_mem_init_one(int idx)
+{
+ struct rtcan_device *dev;
+ struct rtcan_sja1000 *chip;
+ struct rtcan_mem *board;
+ int ret;
+
+ if ((dev = rtcan_dev_alloc(sizeof(struct rtcan_sja1000),
+ sizeof(struct rtcan_mem))) == NULL)
+ return -ENOMEM;
+
+ chip = (struct rtcan_sja1000 *)dev->priv;
+ board = (struct rtcan_mem *)dev->board_priv;
+
+ dev->board_name = mem_board_name;
+
+ chip->irq_num = irq[idx];
+ chip->irq_flags = RTDM_IRQTYPE_SHARED;
+ chip->read_reg = rtcan_mem_readreg;
+ chip->write_reg = rtcan_mem_writereg;
+
+ /* ioremap io memory */
+ if (!(board->mem = (u8*)ioremap(mem[idx], RTCAN_MEM_RANGE))) {
+ ret = -EBUSY;
+ goto out_dev_free;
+ }
+
+ /* Clock frequency in Hz */
+ if (clock[idx])
+ dev->can_sys_clock = clock[idx];
+ else
+ dev->can_sys_clock = 8000000; /* 16/2 MHz */
+
+ /* Output control register */
+ if (ocr[idx])
+ chip->ocr = ocr[idx];
+ else
+ chip->ocr = SJA_OCR_MODE_NORMAL | SJA_OCR_TX0_PUSHPULL;
+
+ if (cdr[idx])
+ chip->cdr = cdr[idx];
+ else
+ chip->cdr = SJA_CDR_CAN_MODE | SJA_CDR_CLK_OFF | SJA_CDR_CBP;
+
+ strncpy(dev->name, RTCAN_DEV_NAME, IFNAMSIZ);
+
+ ret = rtcan_sja1000_register(dev);
+ if (ret) {
+ printk(KERN_ERR "ERROR %d while trying to register SJA1000 device!\n",
+ ret);
+ goto out_free_region;
+ }
+
+ rtcan_mem_devs[idx] = dev;
+ return 0;
+
+ out_free_region:
+ iounmap(board->mem);
+
+ out_dev_free:
+ rtcan_dev_free(dev);
+
+ return ret;
+}
+
+/** Init module */
+static int __init rtcan_mem_init(void)
+{
+ int i;
+ int ret;
+ int done = 0;
+ for (i = 0;
+ i < RTCAN_MEM_MAX_DEV && mem[i] != 0;
+ i++) {
+
+ if ((ret = rtcan_mem_init_one(i) != 0)) {
+ return ret;
+ }
+ done++;
+ }
+ if (done)
+ return 0;
+ printk(KERN_ERR "ERROR! No devices specified! "
+ "Use mem=<base_addr1>[,...] irq=<irq1>[,...]\n");
+
+ return -EINVAL;
+}
+
+
+/** Cleanup module */
+static void __exit rtcan_mem_exit(void)
+{
+ int i;
+ struct rtcan_device *dev;
+ u8 *mem;
+
+ for (i = 0; i < RTCAN_MEM_MAX_DEV; i++) {
+ dev = rtcan_mem_devs[i];
+ if (!dev)
+ continue;
+ mem = ((struct rtcan_mem *)dev->board_priv)->mem;
+ rtcan_sja1000_unregister(dev);
+ iounmap(mem);
+ }
+}
+
+module_init(rtcan_mem_init);
+module_exit(rtcan_mem_exit);
Index: ksrc/drivers/can/sja1000/Makefile
===================================================================
--- ksrc/drivers/can/sja1000/Makefile (revision 1564)
+++ ksrc/drivers/can/sja1000/Makefile (working copy)
@@ -8,11 +8,13 @@
obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_PEAK_PCI) += xeno_rtcan_peak_pci.o
obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_PEAK_DNG) += xeno_rtcan_peak_dng.o
obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_ISA) += xeno_rtcan_isa.o
+obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_MEM) += xeno_rtcan_mem.o
xeno_rtcan_sja1000-y := rtcan_sja1000.o rtcan_sja1000_proc.o
xeno_rtcan_peak_pci-y := rtcan_peak_pci.o
xeno_rtcan_peak_dng-y := rtcan_peak_dng.o
xeno_rtcan_isa-y := rtcan_isa.o
+xeno_rtcan_mem-y := rtcan_mem.o
else
@@ -24,6 +26,7 @@
obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_PEAK_PCI) += xeno_rtcan_peak_pci.o
obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_PEAK_DNG) += xeno_rtcan_peak_dng.o
obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_ISA) += xeno_rtcan_isa.o
+obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_MEM) += xeno_rtcan_mem.o
list-multi := xeno_rtcan_sja1000.o
@@ -31,6 +34,7 @@
xeno_rtcan_peak_pci-objs := rtcan_peak_pci.o
xeno_rtcan_peak_dng-objs := rtcan_peak_dng.o
xeno_rtcan_isa-objs := rtcan_isa.o
+xeno_rtcan_mem-objs := rtcan_mem.o
export-objs := $(xeno_rtcan_sja1000-objs)
@@ -49,4 +53,7 @@
xeno_rtcan_isa.o: $(xeno_rtcan_isa-objs)
$(LD) -r -o $@ $(xeno_rtcan_isa-objs)
+
+xeno_rtcan_mem.o: $(xeno_rtcan_mem-objs)
+ $(LD) -r -o $@ $(xeno_rtcan_mem-objs)
endif
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Xenomai-core] [PATCH] rtcan_mem - CAN driver for memory mapped SJA1000 controllers
2006-09-12 11:30 [Xenomai-core] [PATCH] rtcan_mem - CAN driver for memory mapped SJA1000 controllers Matthias Fuchs
@ 2006-09-12 12:21 ` Jan Kiszka
2006-09-12 12:22 ` Wolfgang Grandegger
1 sibling, 0 replies; 13+ messages in thread
From: Jan Kiszka @ 2006-09-12 12:21 UTC (permalink / raw)
To: Matthias Fuchs; +Cc: xenomai
[-- Attachment #1: Type: text/plain, Size: 11418 bytes --]
Matthias Fuchs wrote:
> Hi,
>
> attached you will find a patch that adds support for memory mapped SJA1000 CAN
> controllers as they often can be found on embedded boards. The driver is
> based on the rtmen_isa driver.
Thanks for your patch! Below are only cleanup comments, not all directed
to you as some of them originate back to the rtcan_isa driver.
>
> The driver has been tested on esd's embedded PowerPC boards with AMCC PPC405
> CPUs.
>
> Thanks to Jan for giving me some introduction to Xenomai during a nightly
> session last friday.
>
> There's one thing a I am not very satisfied with :-) Why passing half of the
> external clock frequency to the module. Because of compatiblity reasons I
> kept this behavior of the clock paramter from the ISA driver.
Wolfgang promised to rework this. I was also already about hacking some
can_sys_clock = clock[idx]/2 patch, but hesitated when I noticed that
the output in rtcan_raw_dev.c will print half of the provided frequency
then. Some smart documentation concept for those frequencies is required
first.
>
> Matthias
>
>
> ------------------------------------------------------------------------
>
> Index: ksrc/drivers/can/sja1000/Kconfig
> ===================================================================
> --- ksrc/drivers/can/sja1000/Kconfig (revision 1564)
> +++ ksrc/drivers/can/sja1000/Kconfig (working copy)
> @@ -13,6 +13,16 @@
> int "Maximum number of ISA devices"
> default 4
>
> +config XENO_DRIVERS_RTCAN_SJA1000_MEM
> + depends on XENO_DRIVERS_RTCAN_SJA1000
> + tristate "Memory mapped controllers"
> + default n
> +
> +config XENO_DRIVERS_RTCAN_SJA1000_MEM_MAX_DEV
> + depends on XENO_DRIVERS_RTCAN_SJA1000_MEM
> + int "Maximum number of memory mapped controllers"
> + default 4
> +
> config XENO_DRIVERS_RTCAN_SJA1000_PEAK_PCI
> depends on XENO_DRIVERS_RTCAN_SJA1000
> tristate "PEAK PCI Card"
> Index: ksrc/drivers/can/sja1000/rtcan_mem.c
> ===================================================================
> --- ksrc/drivers/can/sja1000/rtcan_mem.c (revision 0)
> +++ ksrc/drivers/can/sja1000/rtcan_mem.c (revision 0)
> @@ -0,0 +1,206 @@
> +/*
> + * Copyright (C) 2006 Matthias Fuchs <matthias.fuchs@domain.hidom>,
> + * Jan Kiszka <jan.kiszka@domain.hid>
> + *
> + * RTCAN driver for memory mapped SJA1000 CAN controller
> + * This code has been tested on esd's CPCI405/EPPC405 PPC405 systems.
> + *
> + * This driver is derived from the rtcan-isa driver by
> + * Wolfgang Grandegger and Sebastian Smolorz.
> + *
> + * Copyright (C) 2006 Wolfgang Grandegger <wg@domain.hid>
> + * Copyright (C) 2005, 2006 Sebastian Smolorz
> + * <Sebastian.Smolorz@domain.hid>
> + *
> + *
> + * 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; eitherer 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 <rtdm/rtdm_driver.h>
> +
> +#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 "sja1000-mem"
You don't use RTCAN_DRV_NAME, so we can kill it.
But this line triggered an idea about a new naming scheme for our CAN
drivers (the shorter, the better):
xeno_can (core)
xeno_can_virt (virtual CAN bus)
xeno_sja1000 (SJA1000 core)
xeno_sja1000_port (or _io)
xeno_sja1000_mem
xeno_sja1000_pci (generalised peak_pci if feasible)
xeno_sja1000_peakdng (Peak parport dongle)
xeno_mscan
Moreover, file and CONFIG names should be shortened. I see no need for
rtcan-prefixes if the file reside in the drivers/can folder, or for
XENO_DRIVER_RTCAN when XENO already expresses "RT" (=>XENO_DRIVER_CAN).
> +
> +#ifdef CONFIG_XENO_DRIVERS_RTCAN_MEM_MAX_DEVS
> +#define RTCAN_MEM_MAX_DEV CONFIG_XENO_DRIVERS_RTCAN_MEM_MAX_DEV
> +#else
> +#define RTCAN_MEM_MAX_DEV 2
> +#endif
This #ifdef is redundant (but copy&paste for rtcan_isa): there should be
no undefined CONFIG_XENO_DRIVERS_RTCAN_MEM_MAX_DEVS if this driver is
enabled for compilation.
> +
> +static char *mem_board_name = "mem mapped";
> +
> +MODULE_AUTHOR("Matthias Fuchs <matthias.fuchs@domain.hid>");
> +MODULE_DESCRIPTION("RTCAN driver for memory mapped SJA1000 controller");
> +MODULE_SUPPORTED_DEVICE("mem mapped");
> +MODULE_LICENSE("GPL");
> +
> +static u32 mem[RTCAN_MEM_MAX_DEV];
> +static int irq[RTCAN_MEM_MAX_DEV];
> +static u32 clock[RTCAN_MEM_MAX_DEV];
> +static u8 ocr[RTCAN_MEM_MAX_DEV];
> +static u8 cdr[RTCAN_MEM_MAX_DEV];
> +
> +compat_module_int_param_array(mem, RTCAN_MEM_MAX_DEV);
> +compat_module_int_param_array(irq, RTCAN_MEM_MAX_DEV);
> +compat_module_int_param_array(clock, RTCAN_MEM_MAX_DEV);
> +compat_module_byte_param_array(ocr, RTCAN_MEM_MAX_DEV);
> +compat_module_byte_param_array(cdr, RTCAN_MEM_MAX_DEV);
> +
> +MODULE_PARM_DESC(mem, "The io-memory address");
> +MODULE_PARM_DESC(irq, "The interrupt number");
> +MODULE_PARM_DESC(clock, "CAN system clock frequency (default 8 MHz)");
> +MODULE_PARM_DESC(ocr, "Value of output control register (default 0x1a)");
> +MODULE_PARM_DESC(cdr, "Value of clock divider register (default 0xc8");
> +
> +#define RTCAN_MEM_RANGE 0x80
> +
> +struct rtcan_mem
> +{
> + u8 *mem;
> +};
> +
> +static struct rtcan_device *rtcan_mem_devs[RTCAN_MEM_MAX_DEV];
> +
> +static u8 rtcan_mem_readreg(struct rtcan_device *dev, int reg)
> +{
> + struct rtcan_mem *board = (struct rtcan_mem *)dev->board_priv;
> + return readb(board->mem + reg);
> +}
> +
> +static void rtcan_mem_writereg(struct rtcan_device *dev, int reg, u8 val)
> +{
> + struct rtcan_mem *board = (struct rtcan_mem *)dev->board_priv;
> + writeb(val, board->mem + reg);
> +}
> +
> +int __init rtcan_mem_init_one(int idx)
> +{
> + struct rtcan_device *dev;
> + struct rtcan_sja1000 *chip;
> + struct rtcan_mem *board;
> + int ret;
> +
> + if ((dev = rtcan_dev_alloc(sizeof(struct rtcan_sja1000),
> + sizeof(struct rtcan_mem))) == NULL)
> + return -ENOMEM;
> +
> + chip = (struct rtcan_sja1000 *)dev->priv;
> + board = (struct rtcan_mem *)dev->board_priv;
> +
> + dev->board_name = mem_board_name;
> +
> + chip->irq_num = irq[idx];
> + chip->irq_flags = RTDM_IRQTYPE_SHARED;
> + chip->read_reg = rtcan_mem_readreg;
> + chip->write_reg = rtcan_mem_writereg;
> +
> + /* ioremap io memory */
> + if (!(board->mem = (u8*)ioremap(mem[idx], RTCAN_MEM_RANGE))) {
> + ret = -EBUSY;
> + goto out_dev_free;
> + }
> +
> + /* Clock frequency in Hz */
> + if (clock[idx])
> + dev->can_sys_clock = clock[idx];
> + else
> + dev->can_sys_clock = 8000000; /* 16/2 MHz */
> +
> + /* Output control register */
> + if (ocr[idx])
> + chip->ocr = ocr[idx];
> + else
> + chip->ocr = SJA_OCR_MODE_NORMAL | SJA_OCR_TX0_PUSHPULL;
> +
> + if (cdr[idx])
> + chip->cdr = cdr[idx];
> + else
> + chip->cdr = SJA_CDR_CAN_MODE | SJA_CDR_CLK_OFF | SJA_CDR_CBP;
> +
> + strncpy(dev->name, RTCAN_DEV_NAME, IFNAMSIZ);
> +
> + ret = rtcan_sja1000_register(dev);
> + if (ret) {
> + printk(KERN_ERR "ERROR %d while trying to register SJA1000 device!\n",
> + ret);
> + goto out_free_region;
> + }
> +
> + rtcan_mem_devs[idx] = dev;
> + return 0;
> +
> + out_free_region:
> + iounmap(board->mem);
> +
> + out_dev_free:
> + rtcan_dev_free(dev);
> +
> + return ret;
> +}
> +
> +/** Init module */
> +static int __init rtcan_mem_init(void)
> +{
> + int i;
> + int ret;
> + int done = 0;
Not nicely formatted. BTW, as this is new code to be included, we should
take the chance and convert it to kernel style right from the beginning. :)
> + for (i = 0;
> + i < RTCAN_MEM_MAX_DEV && mem[i] != 0;
> + i++) {
> +
> + if ((ret = rtcan_mem_init_one(i) != 0)) {
> + return ret;
> + }
> + done++;
> + }
> + if (done)
> + return 0;
> + printk(KERN_ERR "ERROR! No devices specified! "
> + "Use mem=<base_addr1>[,...] irq=<irq1>[,...]\n");
> +
> + return -EINVAL;
> +}
> +
> +
> +/** Cleanup module */
> +static void __exit rtcan_mem_exit(void)
> +{
> + int i;
> + struct rtcan_device *dev;
> + u8 *mem;
> +
> + for (i = 0; i < RTCAN_MEM_MAX_DEV; i++) {
> + dev = rtcan_mem_devs[i];
> + if (!dev)
> + continue;
> + mem = ((struct rtcan_mem *)dev->board_priv)->mem;
> + rtcan_sja1000_unregister(dev);
> + iounmap(mem);
> + }
> +}
> +
> +module_init(rtcan_mem_init);
> +module_exit(rtcan_mem_exit);
> Index: ksrc/drivers/can/sja1000/Makefile
> ===================================================================
> --- ksrc/drivers/can/sja1000/Makefile (revision 1564)
> +++ ksrc/drivers/can/sja1000/Makefile (working copy)
> @@ -8,11 +8,13 @@
> obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_PEAK_PCI) += xeno_rtcan_peak_pci.o
> obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_PEAK_DNG) += xeno_rtcan_peak_dng.o
> obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_ISA) += xeno_rtcan_isa.o
> +obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_MEM) += xeno_rtcan_mem.o
>
> xeno_rtcan_sja1000-y := rtcan_sja1000.o rtcan_sja1000_proc.o
> xeno_rtcan_peak_pci-y := rtcan_peak_pci.o
> xeno_rtcan_peak_dng-y := rtcan_peak_dng.o
> xeno_rtcan_isa-y := rtcan_isa.o
> +xeno_rtcan_mem-y := rtcan_mem.o
>
> else
>
> @@ -24,6 +26,7 @@
> obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_PEAK_PCI) += xeno_rtcan_peak_pci.o
> obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_PEAK_DNG) += xeno_rtcan_peak_dng.o
> obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_ISA) += xeno_rtcan_isa.o
> +obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_MEM) += xeno_rtcan_mem.o
>
> list-multi := xeno_rtcan_sja1000.o
>
> @@ -31,6 +34,7 @@
> xeno_rtcan_peak_pci-objs := rtcan_peak_pci.o
> xeno_rtcan_peak_dng-objs := rtcan_peak_dng.o
> xeno_rtcan_isa-objs := rtcan_isa.o
> +xeno_rtcan_mem-objs := rtcan_mem.o
>
> export-objs := $(xeno_rtcan_sja1000-objs)
>
> @@ -49,4 +53,7 @@
>
> xeno_rtcan_isa.o: $(xeno_rtcan_isa-objs)
> $(LD) -r -o $@ $(xeno_rtcan_isa-objs)
> +
> +xeno_rtcan_mem.o: $(xeno_rtcan_mem-objs)
> + $(LD) -r -o $@ $(xeno_rtcan_mem-objs)
> endif
>
Patch for scripts/Modules.frag is missing (=> built-in compilation under
2.4) - just like for rtcan_isa... :-/
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Xenomai-core] [PATCH] rtcan_mem - CAN driver for memory mapped SJA1000 controllers
2006-09-12 11:30 [Xenomai-core] [PATCH] rtcan_mem - CAN driver for memory mapped SJA1000 controllers Matthias Fuchs
2006-09-12 12:21 ` Jan Kiszka
@ 2006-09-12 12:22 ` Wolfgang Grandegger
2006-09-12 12:38 ` Jan Kiszka
` (2 more replies)
1 sibling, 3 replies; 13+ messages in thread
From: Wolfgang Grandegger @ 2006-09-12 12:22 UTC (permalink / raw)
To: Matthias Fuchs; +Cc: xenomai
[-- Attachment #1: Type: text/plain, Size: 10248 bytes --]
Hi Matthias,
Matthias Fuchs wrote:
> Hi,
>
> attached you will find a patch that adds support for memory mapped SJA1000 CAN
> controllers as they often can be found on embedded boards. The driver is
> based on the rtmen_isa driver.
What about using request_mem_region()? While looking to the driver I now
realize, that it's mainly duplicated code. Does it not make more sense
to make a combined io/mem driver. If io address < 32K it's an io driver
else a mem driver.
> The driver has been tested on esd's embedded PowerPC boards with AMCC PPC405
> CPUs.
>
> Thanks to Jan for giving me some introduction to Xenomai during a nightly
> session last friday.
>
> There's one thing a I am not very satisfied with :-) Why passing half of the
> external clock frequency to the module. Because of compatiblity reasons I
> kept this behavior of the clock paramter from the ISA driver.
The attached patch fixes this and replaces the module parameter "isa"
with "io". I also tend to rename the driver into rtcan_io instead
rtcan_isa if we keep it.
Wolfgang.
> Matthias
>
>
> ------------------------------------------------------------------------
>
> Index: ksrc/drivers/can/sja1000/Kconfig
> ===================================================================
> --- ksrc/drivers/can/sja1000/Kconfig (revision 1564)
> +++ ksrc/drivers/can/sja1000/Kconfig (working copy)
> @@ -13,6 +13,16 @@
> int "Maximum number of ISA devices"
> default 4
>
> +config XENO_DRIVERS_RTCAN_SJA1000_MEM
> + depends on XENO_DRIVERS_RTCAN_SJA1000
> + tristate "Memory mapped controllers"
> + default n
> +
> +config XENO_DRIVERS_RTCAN_SJA1000_MEM_MAX_DEV
> + depends on XENO_DRIVERS_RTCAN_SJA1000_MEM
> + int "Maximum number of memory mapped controllers"
> + default 4
> +
> config XENO_DRIVERS_RTCAN_SJA1000_PEAK_PCI
> depends on XENO_DRIVERS_RTCAN_SJA1000
> tristate "PEAK PCI Card"
> Index: ksrc/drivers/can/sja1000/rtcan_mem.c
> ===================================================================
> --- ksrc/drivers/can/sja1000/rtcan_mem.c (revision 0)
> +++ ksrc/drivers/can/sja1000/rtcan_mem.c (revision 0)
> @@ -0,0 +1,206 @@
> +/*
> + * Copyright (C) 2006 Matthias Fuchs <matthias.fuchs@domain.hid>,
> + * Jan Kiszka <jan.kiszka@domain.hid>
> + *
> + * RTCAN driver for memory mapped SJA1000 CAN controller
> + * This code has been tested on esd's CPCI405/EPPC405 PPC405 systems.
> + *
> + * This driver is derived from the rtcan-isa driver by
> + * Wolfgang Grandegger and Sebastian Smolorz.
> + *
> + * Copyright (C) 2006 Wolfgang Grandegger <wg@domain.hid>
> + * Copyright (C) 2005, 2006 Sebastian Smolorz
> + * <Sebastian.Smolorz@domain.hid>
> + *
> + *
> + * 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; eitherer 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 <rtdm/rtdm_driver.h>
> +
> +#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 "sja1000-mem"
> +
> +#ifdef CONFIG_XENO_DRIVERS_RTCAN_MEM_MAX_DEVS
> +#define RTCAN_MEM_MAX_DEV CONFIG_XENO_DRIVERS_RTCAN_MEM_MAX_DEV
> +#else
> +#define RTCAN_MEM_MAX_DEV 2
> +#endif
> +
> +static char *mem_board_name = "mem mapped";
> +
> +MODULE_AUTHOR("Matthias Fuchs <matthias.fuchs@domain.hid>");
> +MODULE_DESCRIPTION("RTCAN driver for memory mapped SJA1000 controller");
> +MODULE_SUPPORTED_DEVICE("mem mapped");
> +MODULE_LICENSE("GPL");
> +
> +static u32 mem[RTCAN_MEM_MAX_DEV];
> +static int irq[RTCAN_MEM_MAX_DEV];
> +static u32 clock[RTCAN_MEM_MAX_DEV];
> +static u8 ocr[RTCAN_MEM_MAX_DEV];
> +static u8 cdr[RTCAN_MEM_MAX_DEV];
> +
> +compat_module_int_param_array(mem, RTCAN_MEM_MAX_DEV);
> +compat_module_int_param_array(irq, RTCAN_MEM_MAX_DEV);
> +compat_module_int_param_array(clock, RTCAN_MEM_MAX_DEV);
> +compat_module_byte_param_array(ocr, RTCAN_MEM_MAX_DEV);
> +compat_module_byte_param_array(cdr, RTCAN_MEM_MAX_DEV);
> +
> +MODULE_PARM_DESC(mem, "The io-memory address");
> +MODULE_PARM_DESC(irq, "The interrupt number");
> +MODULE_PARM_DESC(clock, "CAN system clock frequency (default 8 MHz)");
> +MODULE_PARM_DESC(ocr, "Value of output control register (default 0x1a)");
> +MODULE_PARM_DESC(cdr, "Value of clock divider register (default 0xc8");
> +
> +#define RTCAN_MEM_RANGE 0x80
> +
> +struct rtcan_mem
> +{
> + u8 *mem;
> +};
> +
> +static struct rtcan_device *rtcan_mem_devs[RTCAN_MEM_MAX_DEV];
> +
> +static u8 rtcan_mem_readreg(struct rtcan_device *dev, int reg)
> +{
> + struct rtcan_mem *board = (struct rtcan_mem *)dev->board_priv;
> + return readb(board->mem + reg);
> +}
> +
> +static void rtcan_mem_writereg(struct rtcan_device *dev, int reg, u8 val)
> +{
> + struct rtcan_mem *board = (struct rtcan_mem *)dev->board_priv;
> + writeb(val, board->mem + reg);
> +}
> +
> +int __init rtcan_mem_init_one(int idx)
> +{
> + struct rtcan_device *dev;
> + struct rtcan_sja1000 *chip;
> + struct rtcan_mem *board;
> + int ret;
> +
> + if ((dev = rtcan_dev_alloc(sizeof(struct rtcan_sja1000),
> + sizeof(struct rtcan_mem))) == NULL)
> + return -ENOMEM;
> +
> + chip = (struct rtcan_sja1000 *)dev->priv;
> + board = (struct rtcan_mem *)dev->board_priv;
> +
> + dev->board_name = mem_board_name;
> +
> + chip->irq_num = irq[idx];
> + chip->irq_flags = RTDM_IRQTYPE_SHARED;
> + chip->read_reg = rtcan_mem_readreg;
> + chip->write_reg = rtcan_mem_writereg;
> +
> + /* ioremap io memory */
> + if (!(board->mem = (u8*)ioremap(mem[idx], RTCAN_MEM_RANGE))) {
> + ret = -EBUSY;
> + goto out_dev_free;
> + }
> +
> + /* Clock frequency in Hz */
> + if (clock[idx])
> + dev->can_sys_clock = clock[idx];
> + else
> + dev->can_sys_clock = 8000000; /* 16/2 MHz */
> +
> + /* Output control register */
> + if (ocr[idx])
> + chip->ocr = ocr[idx];
> + else
> + chip->ocr = SJA_OCR_MODE_NORMAL | SJA_OCR_TX0_PUSHPULL;
> +
> + if (cdr[idx])
> + chip->cdr = cdr[idx];
> + else
> + chip->cdr = SJA_CDR_CAN_MODE | SJA_CDR_CLK_OFF | SJA_CDR_CBP;
> +
> + strncpy(dev->name, RTCAN_DEV_NAME, IFNAMSIZ);
> +
> + ret = rtcan_sja1000_register(dev);
> + if (ret) {
> + printk(KERN_ERR "ERROR %d while trying to register SJA1000 device!\n",
> + ret);
> + goto out_free_region;
> + }
> +
> + rtcan_mem_devs[idx] = dev;
> + return 0;
> +
> + out_free_region:
> + iounmap(board->mem);
> +
> + out_dev_free:
> + rtcan_dev_free(dev);
> +
> + return ret;
> +}
> +
> +/** Init module */
> +static int __init rtcan_mem_init(void)
> +{
> + int i;
> + int ret;
> + int done = 0;
> + for (i = 0;
> + i < RTCAN_MEM_MAX_DEV && mem[i] != 0;
> + i++) {
> +
> + if ((ret = rtcan_mem_init_one(i) != 0)) {
> + return ret;
> + }
> + done++;
> + }
> + if (done)
> + return 0;
> + printk(KERN_ERR "ERROR! No devices specified! "
> + "Use mem=<base_addr1>[,...] irq=<irq1>[,...]\n");
> +
> + return -EINVAL;
> +}
> +
> +
> +/** Cleanup module */
> +static void __exit rtcan_mem_exit(void)
> +{
> + int i;
> + struct rtcan_device *dev;
> + u8 *mem;
> +
> + for (i = 0; i < RTCAN_MEM_MAX_DEV; i++) {
> + dev = rtcan_mem_devs[i];
> + if (!dev)
> + continue;
> + mem = ((struct rtcan_mem *)dev->board_priv)->mem;
> + rtcan_sja1000_unregister(dev);
> + iounmap(mem);
> + }
> +}
> +
> +module_init(rtcan_mem_init);
> +module_exit(rtcan_mem_exit);
> Index: ksrc/drivers/can/sja1000/Makefile
> ===================================================================
> --- ksrc/drivers/can/sja1000/Makefile (revision 1564)
> +++ ksrc/drivers/can/sja1000/Makefile (working copy)
> @@ -8,11 +8,13 @@
> obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_PEAK_PCI) += xeno_rtcan_peak_pci.o
> obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_PEAK_DNG) += xeno_rtcan_peak_dng.o
> obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_ISA) += xeno_rtcan_isa.o
> +obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_MEM) += xeno_rtcan_mem.o
>
> xeno_rtcan_sja1000-y := rtcan_sja1000.o rtcan_sja1000_proc.o
> xeno_rtcan_peak_pci-y := rtcan_peak_pci.o
> xeno_rtcan_peak_dng-y := rtcan_peak_dng.o
> xeno_rtcan_isa-y := rtcan_isa.o
> +xeno_rtcan_mem-y := rtcan_mem.o
>
> else
>
> @@ -24,6 +26,7 @@
> obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_PEAK_PCI) += xeno_rtcan_peak_pci.o
> obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_PEAK_DNG) += xeno_rtcan_peak_dng.o
> obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_ISA) += xeno_rtcan_isa.o
> +obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_MEM) += xeno_rtcan_mem.o
>
> list-multi := xeno_rtcan_sja1000.o
>
> @@ -31,6 +34,7 @@
> xeno_rtcan_peak_pci-objs := rtcan_peak_pci.o
> xeno_rtcan_peak_dng-objs := rtcan_peak_dng.o
> xeno_rtcan_isa-objs := rtcan_isa.o
> +xeno_rtcan_mem-objs := rtcan_mem.o
>
> export-objs := $(xeno_rtcan_sja1000-objs)
>
> @@ -49,4 +53,7 @@
>
> xeno_rtcan_isa.o: $(xeno_rtcan_isa-objs)
> $(LD) -r -o $@ $(xeno_rtcan_isa-objs)
> +
> +xeno_rtcan_mem.o: $(xeno_rtcan_mem-objs)
> + $(LD) -r -o $@ $(xeno_rtcan_mem-objs)
> endif
>
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> Xenomai-core mailing list
> Xenomai-core@domain.hid
> https://mail.gna.org/listinfo/xenomai-core
[-- Attachment #2: xenomai-can-sja1000-canclk.patch --]
[-- Type: text/x-patch, Size: 3464 bytes --]
+ diff -u xenomai/ksrc/drivers/can/sja1000/rtcan_isa.c.CANCLK xenomai/ksrc/drivers/can/sja1000/rtcan_isa.c
--- xenomai/ksrc/drivers/can/sja1000/rtcan_isa.c.CANCLK 2006-09-10 09:10:46.000000000 +0200
+++ xenomai/ksrc/drivers/can/sja1000/rtcan_isa.c 2006-09-11 13:06:19.000000000 +0200
@@ -49,21 +49,21 @@
MODULE_SUPPORTED_DEVICE("ISA board");
MODULE_LICENSE("GPL");
-static u16 isa[RTCAN_ISA_MAX_DEV];
+static u16 io[RTCAN_ISA_MAX_DEV];
static int irq[RTCAN_ISA_MAX_DEV];
static u32 clock[RTCAN_ISA_MAX_DEV];
static u8 ocr[RTCAN_ISA_MAX_DEV];
static u8 cdr[RTCAN_ISA_MAX_DEV];
-compat_module_short_param_array(isa, RTCAN_ISA_MAX_DEV);
+compat_module_short_param_array(io, RTCAN_ISA_MAX_DEV);
compat_module_int_param_array(irq, RTCAN_ISA_MAX_DEV);
compat_module_int_param_array(clock, RTCAN_ISA_MAX_DEV);
compat_module_byte_param_array(ocr, RTCAN_ISA_MAX_DEV);
compat_module_byte_param_array(cdr, RTCAN_ISA_MAX_DEV);
-MODULE_PARM_DESC(isa, "The io-port address");
+MODULE_PARM_DESC(io, "The io-port address");
MODULE_PARM_DESC(irq, "The interrupt number");
-MODULE_PARM_DESC(clock, "CAN system clock frequency (default 8 MHz)");
+MODULE_PARM_DESC(clock, "External clock frequency (default 16 MHz)");
MODULE_PARM_DESC(ocr, "Value of output control register (default 0x1a)");
MODULE_PARM_DESC(cdr, "Value of clock divider register (default 0xc8");
@@ -71,7 +71,7 @@
struct rtcan_isa
{
- u16 isa;
+ u16 io;
};
static struct rtcan_device *rtcan_isa_devs[RTCAN_ISA_MAX_DEV];
@@ -79,13 +79,13 @@
static u8 rtcan_isa_readreg(struct rtcan_device *dev, int port)
{
struct rtcan_isa *board = (struct rtcan_isa *)dev->board_priv;
- return inb(board->isa + port);
+ return inb(board->io + port);
}
static void rtcan_isa_writereg(struct rtcan_device *dev, int port, u8 val)
{
struct rtcan_isa *board = (struct rtcan_isa *)dev->board_priv;
- outb(val, board->isa + port);
+ outb(val, board->io + port);
}
@@ -105,7 +105,7 @@
dev->board_name = isa_board_name;
- board->isa = isa[idx];
+ board->io = io[idx];
chip->irq_num = irq[idx];
chip->irq_flags = RTDM_IRQTYPE_SHARED | RTDM_IRQTYPE_EDGE;
@@ -114,14 +114,14 @@
chip->write_reg = rtcan_isa_writereg;
/* Check and request I/O ports */
- if (!request_region(board->isa, RTCAN_ISA_PORT_SIZE, RTCAN_DRV_NAME)) {
+ if (!request_region(board->io, RTCAN_ISA_PORT_SIZE, RTCAN_DRV_NAME)) {
ret = -EBUSY;
goto out_dev_free;
}
/* Clock frequency in Hz */
if (clock[idx])
- dev->can_sys_clock = clock[idx];
+ dev->can_sys_clock = clock[idx] / 2;
else
dev->can_sys_clock = 8000000; /* 16/2 MHz */
@@ -149,7 +149,7 @@
return 0;
out_free_region:
- release_region(board->isa, RTCAN_ISA_PORT_SIZE);
+ release_region(board->io, RTCAN_ISA_PORT_SIZE);
out_dev_free:
rtcan_dev_free(dev);
@@ -160,11 +160,10 @@
/** Init module */
static int __init rtcan_isa_init(void)
{
- int i;
- int ret;
- int done = 0;
+ int i, ret, done;
+
for (i = 0;
- i < RTCAN_ISA_MAX_DEV && isa[i] != 0;
+ i < RTCAN_ISA_MAX_DEV && io[i] != 0;
i++) {
if ((ret = rtcan_isa_init_one(i) != 0)) {
@@ -175,7 +174,7 @@
if (done)
return 0;
printk(KERN_ERR "ERROR! No devices specified! "
- "Use isa=<port1>[,...] irq=<irq1>[,...]\n");
+ "Use io=<port1>[,...] irq=<irq1>[,...]\n");
return -EINVAL;
}
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Xenomai-core] [PATCH] rtcan_mem - CAN driver for memory mapped SJA1000 controllers
2006-09-12 12:22 ` Wolfgang Grandegger
@ 2006-09-12 12:38 ` Jan Kiszka
2006-09-12 13:52 ` Wolfgang Grandegger
2006-09-12 13:13 ` Matthias Fuchs
2006-09-12 16:07 ` Matthias Fuchs
2 siblings, 1 reply; 13+ messages in thread
From: Jan Kiszka @ 2006-09-12 12:38 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: xenomai
[-- Attachment #1: Type: text/plain, Size: 1658 bytes --]
Wolfgang Grandegger wrote:
> Hi Matthias,
>
> Matthias Fuchs wrote:
>> Hi,
>>
>> attached you will find a patch that adds support for memory mapped
>> SJA1000 CAN controllers as they often can be found on embedded boards.
>> The driver is based on the rtmen_isa driver.
>
> What about using request_mem_region()?
(and this would also give RTCAN_DRV_NAME some sense again :))
> While looking to the driver I now
> realize, that it's mainly duplicated code. Does it not make more sense
> to make a combined io/mem driver. If io address < 32K it's an io driver
> else a mem driver.
And provide two sets of readreg/writereg? What about differences in
chip->irq_flags, are they always like io=edge, mem=level? What about
defaults for CDR and OCR? Are the arbitrary anyway or do they correlate
somehow to the access type?
When we do not find answers right now (maybe in other Linux CAN
stacks?), we may postpone the merge and keep is separated until more
hardware pops up with more use-cases.
>
>> The driver has been tested on esd's embedded PowerPC boards with AMCC
>> PPC405 CPUs.
>>
>> Thanks to Jan for giving me some introduction to Xenomai during a
>> nightly session last friday.
>>
>> There's one thing a I am not very satisfied with :-) Why passing half
>> of the external clock frequency to the module. Because of compatiblity
>> reasons I kept this behavior of the clock paramter from the ISA driver.
>
> The attached patch fixes this and replaces the module parameter "isa"
> with "io". I also tend to rename the driver into rtcan_io instead
> rtcan_isa if we keep it.
>
> Wolfgang.
>
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Xenomai-core] [PATCH] rtcan_mem - CAN driver for memory mapped SJA1000 controllers
2006-09-12 12:22 ` Wolfgang Grandegger
2006-09-12 12:38 ` Jan Kiszka
@ 2006-09-12 13:13 ` Matthias Fuchs
2006-09-12 13:57 ` Wolfgang Grandegger
2006-09-12 16:07 ` Matthias Fuchs
2 siblings, 1 reply; 13+ messages in thread
From: Matthias Fuchs @ 2006-09-12 13:13 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: xenomai
Hi Wolfgang,
On Tuesday 12 September 2006 14:22, Wolfgang Grandegger wrote:
> What about using request_mem_region()? While looking to the driver I now
... will be added, of course.
> realize, that it's mainly duplicated code. Does it not make more sense
> to make a combined io/mem driver. If io address < 32K it's an io driver
> else a mem driver.
Yes, that's possible. We can also decide on the used module parameter (mem or
io) what is to do. Passing both results in an error.
You are right that its duplicated code. But the driver is very simple. It
nearly does nothing but mapping, implementing register access callbacks,
registering and cleanup when unloading. But the driver differs in all these
points from the isa/io colleague. I like shot an simple drivers, so that you
do not have to care about breaking the isa/io part when modifying the mem
part.
>
> > There's one thing a I am not very satisfied with :-) Why passing half of
the
> > external clock frequency to the module. Because of compatiblity reasons I
> > kept this behavior of the clock paramter from the ISA driver.
>
> The attached patch fixes this and replaces the module parameter "isa"
> with "io". I also tend to rename the driver into rtcan_io instead
> rtcan_isa if we keep it.
Sounds good.
Matthias
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Xenomai-core] [PATCH] rtcan_mem - CAN driver for memory mapped SJA1000 controllers
2006-09-12 12:38 ` Jan Kiszka
@ 2006-09-12 13:52 ` Wolfgang Grandegger
0 siblings, 0 replies; 13+ messages in thread
From: Wolfgang Grandegger @ 2006-09-12 13:52 UTC (permalink / raw)
To: Jan Kiszka; +Cc: xenomai
Jan Kiszka wrote:
> Wolfgang Grandegger wrote:
>> Hi Matthias,
>>
>> Matthias Fuchs wrote:
>>> Hi,
>>>
>>> attached you will find a patch that adds support for memory mapped
>>> SJA1000 CAN controllers as they often can be found on embedded boards.
>>> The driver is based on the rtmen_isa driver.
>> What about using request_mem_region()?
>
> (and this would also give RTCAN_DRV_NAME some sense again :))
>
>> While looking to the driver I now
>> realize, that it's mainly duplicated code. Does it not make more sense
>> to make a combined io/mem driver. If io address < 32K it's an io driver
>> else a mem driver.
>
> And provide two sets of readreg/writereg? What about differences in
Yes.
> chip->irq_flags, are they always like io=edge, mem=level? What about
> defaults for CDR and OCR? Are the arbitrary anyway or do they correlate
> somehow to the access type?
Another module parameter for the irq_flags makes sense, indeed. OCR and
CDR do not affect the access type. It more defines some electrical
characteristics.
/* Output control register */
enum SJA1000_PELI_OCR {
SJA_OCR_MODE_BIPHASE = 0,
SJA_OCR_MODE_TEST = 1,
SJA_OCR_MODE_NORMAL = 2,
SJA_OCR_MODE_CLOCK = 3,
SJA_OCR_TX0_INVERT = 1<<2,
SJA_OCR_TX0_PULLDOWN = 1<<3,
SJA_OCR_TX0_PULLUP = 2<<3,
SJA_OCR_TX0_PUSHPULL = 3<<3,
SJA_OCR_TX1_INVERT = 1<<5,
SJA_OCR_TX1_PULLDOWN = 1<<6,
SJA_OCR_TX1_PULLUP = 2<<6,
SJA_OCR_TX1_PUSHPULL = 3<<6
};
enum SJA1000_PELI_CDR {
SJA_CDR_CLK_OFF = 1<<3, /* Clock off (CLKOUT pin) */
SJA_CDR_CBP = 1<<6, /* CAN input comparator bypass */
SJA_CDR_CAN_MODE = 1<<7 /* CAN mode: 1 = PeliCAN */
};
> When we do not find answers right now (maybe in other Linux CAN
> stacks?), we may postpone the merge and keep is separated until more
> hardware pops up with more use-cases.
No hurry, of course.
>>> The driver has been tested on esd's embedded PowerPC boards with AMCC
>>> PPC405 CPUs.
>>>
>>> Thanks to Jan for giving me some introduction to Xenomai during a
>>> nightly session last friday.
>>>
>>> There's one thing a I am not very satisfied with :-) Why passing half
>>> of the external clock frequency to the module. Because of compatiblity
>>> reasons I kept this behavior of the clock paramter from the ISA driver.
>> The attached patch fixes this and replaces the module parameter "isa"
>> with "io". I also tend to rename the driver into rtcan_io instead
>> rtcan_isa if we keep it.
>>
>> Wolfgang.
>>
>
> Jan
Wolfgang.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Xenomai-core] [PATCH] rtcan_mem - CAN driver for memory mapped SJA1000 controllers
2006-09-12 13:13 ` Matthias Fuchs
@ 2006-09-12 13:57 ` Wolfgang Grandegger
2006-09-12 14:25 ` Jan Kiszka
0 siblings, 1 reply; 13+ messages in thread
From: Wolfgang Grandegger @ 2006-09-12 13:57 UTC (permalink / raw)
To: Matthias Fuchs; +Cc: xenomai
Matthias Fuchs wrote:
> Hi Wolfgang,
>
> On Tuesday 12 September 2006 14:22, Wolfgang Grandegger wrote:
>> What about using request_mem_region()? While looking to the driver I now
> ... will be added, of course.
>> realize, that it's mainly duplicated code. Does it not make more sense
>> to make a combined io/mem driver. If io address < 32K it's an io driver
>> else a mem driver.
> Yes, that's possible. We can also decide on the used module parameter (mem or
> io) what is to do. Passing both results in an error.
>
> You are right that its duplicated code. But the driver is very simple. It
> nearly does nothing but mapping, implementing register access callbacks,
> registering and cleanup when unloading. But the driver differs in all these
> points from the isa/io colleague. I like shot an simple drivers, so that you
> do not have to care about breaking the isa/io part when modifying the mem
> part.
>
>>> There's one thing a I am not very satisfied with :-) Why passing half of
> the
>>> external clock frequency to the module. Because of compatiblity reasons I
>>> kept this behavior of the clock paramter from the ISA driver.
>> The attached patch fixes this and replaces the module parameter "isa"
>> with "io". I also tend to rename the driver into rtcan_io instead
>> rtcan_isa if we keep it.
> Sounds good.
Jan, could you apply this patch to avoid further confusion? The real CAN
clock frequency listed also in the proc file system is half of the
oscillator clock. Some more doc might help to avoid further confusion.
Wolfgang.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Xenomai-core] [PATCH] rtcan_mem - CAN driver for memory mapped SJA1000 controllers
2006-09-12 13:57 ` Wolfgang Grandegger
@ 2006-09-12 14:25 ` Jan Kiszka
0 siblings, 0 replies; 13+ messages in thread
From: Jan Kiszka @ 2006-09-12 14:25 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: xenomai
[-- Attachment #1: Type: text/plain, Size: 1720 bytes --]
Wolfgang Grandegger wrote:
> Matthias Fuchs wrote:
>> Hi Wolfgang,
>>
>> On Tuesday 12 September 2006 14:22, Wolfgang Grandegger wrote:
>>> What about using request_mem_region()? While looking to the driver I now
>> ... will be added, of course.
>>> realize, that it's mainly duplicated code. Does it not make more
>>> sense to make a combined io/mem driver. If io address < 32K it's an
>>> io driver
>>> else a mem driver.
>> Yes, that's possible. We can also decide on the used module parameter
>> (mem or io) what is to do. Passing both results in an error.
>>
>> You are right that its duplicated code. But the driver is very simple.
>> It nearly does nothing but mapping, implementing register access
>> callbacks, registering and cleanup when unloading. But the driver
>> differs in all these points from the isa/io colleague. I like shot an
>> simple drivers, so that you do not have to care about breaking the
>> isa/io part when modifying the mem part.
>>
>>>> There's one thing a I am not very satisfied with :-) Why passing
>>>> half of
>> the
>>>> external clock frequency to the module. Because of compatiblity
>>>> reasons I kept this behavior of the clock paramter from the ISA driver.
>>> The attached patch fixes this and replaces the module parameter "isa"
>>> with "io". I also tend to rename the driver into rtcan_io instead
>>> rtcan_isa if we keep it.
>> Sounds good.
>
> Jan, could you apply this patch to avoid further confusion? The real CAN
> clock frequency listed also in the proc file system is half of the
> oscillator clock. Some more doc might help to avoid further confusion.
>
Done (with minor modification to avoid compiler warning).
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Xenomai-core] [PATCH] rtcan_mem - CAN driver for memory mapped SJA1000 controllers
2006-09-12 12:22 ` Wolfgang Grandegger
2006-09-12 12:38 ` Jan Kiszka
2006-09-12 13:13 ` Matthias Fuchs
@ 2006-09-12 16:07 ` Matthias Fuchs
2006-09-12 16:46 ` Wolfgang Grandegger
2 siblings, 1 reply; 13+ messages in thread
From: Matthias Fuchs @ 2006-09-12 16:07 UTC (permalink / raw)
To: xenomai; +Cc: Jan Kiszka
[-- Attachment #1: Type: text/plain, Size: 920 bytes --]
Hi,
here's the 2nd try:
- updated external clock frequency handling
- use request_mem_region() before remapping memory
- renamed local variable mem to vmem in clean loop because it shadowed a
module parameter
Now it's on you to decide if we should merge and/or rename. I wouldn't merge
because of my former arguments. But do not care about me:-)
Matthias
On Tuesday 12 September 2006 14:22, Wolfgang Grandegger wrote:
> Hi Matthias,
>
> Matthias Fuchs wrote:
> > Hi,
> >
> > attached you will find a patch that adds support for memory mapped
> > SJA1000 CAN controllers as they often can be found on embedded boards.
> > The driver is based on the rtmen_isa driver.
>
> What about using request_mem_region()? While looking to the driver I now
> realize, that it's mainly duplicated code. Does it not make more sense
> to make a combined io/mem driver. If io address < 32K it's an io driver
> else a mem driver.
>
[-- Attachment #2: rtcan_mem_2.patch --]
[-- Type: text/x-diff, Size: 8544 bytes --]
Index: ksrc/drivers/can/sja1000/Kconfig
===================================================================
--- ksrc/drivers/can/sja1000/Kconfig (revision 1564)
+++ ksrc/drivers/can/sja1000/Kconfig (working copy)
@@ -13,6 +13,16 @@
int "Maximum number of ISA devices"
default 4
+config XENO_DRIVERS_RTCAN_SJA1000_MEM
+ depends on XENO_DRIVERS_RTCAN_SJA1000
+ tristate "Memory mapped controllers"
+ default n
+
+config XENO_DRIVERS_RTCAN_SJA1000_MEM_MAX_DEV
+ depends on XENO_DRIVERS_RTCAN_SJA1000_MEM
+ int "Maximum number of memory mapped controllers"
+ default 4
+
config XENO_DRIVERS_RTCAN_SJA1000_PEAK_PCI
depends on XENO_DRIVERS_RTCAN_SJA1000
tristate "PEAK PCI Card"
Index: ksrc/drivers/can/sja1000/rtcan_mem.c
===================================================================
--- ksrc/drivers/can/sja1000/rtcan_mem.c (revision 0)
+++ ksrc/drivers/can/sja1000/rtcan_mem.c (revision 0)
@@ -0,0 +1,213 @@
+/*
+ * Copyright (C) 2006 Matthias Fuchs <matthias.fuchs@domain.hid>,
+ * Jan Kiszka <jan.kiszka@domain.hid>
+ *
+ * RTCAN driver for memory mapped SJA1000 CAN controller
+ * This code has been tested on esd's CPCI405/EPPC405 PPC405 systems.
+ *
+ * This driver is derived from the rtcan-isa driver by
+ * Wolfgang Grandegger and Sebastian Smolorz.
+ *
+ * Copyright (C) 2006 Wolfgang Grandegger <wg@domain.hid>
+ * Copyright (C) 2005, 2006 Sebastian Smolorz
+ * <Sebastian.Smolorz@domain.hid>
+ *
+ *
+ * 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; eitherer 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 <rtdm/rtdm_driver.h>
+
+#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 "sja1000-mem"
+
+#ifdef CONFIG_XENO_DRIVERS_RTCAN_MEM_MAX_DEVS
+#define RTCAN_MEM_MAX_DEV CONFIG_XENO_DRIVERS_RTCAN_MEM_MAX_DEV
+#else
+#define RTCAN_MEM_MAX_DEV 2
+#endif
+
+static char *mem_board_name = "mem mapped";
+
+MODULE_AUTHOR("Matthias Fuchs <matthias.fuchs@domain.hid>");
+MODULE_DESCRIPTION("RTCAN driver for memory mapped SJA1000 controller");
+MODULE_SUPPORTED_DEVICE("mem mapped");
+MODULE_LICENSE("GPL");
+
+static u32 mem[RTCAN_MEM_MAX_DEV];
+static int irq[RTCAN_MEM_MAX_DEV];
+static u32 clock[RTCAN_MEM_MAX_DEV];
+static u8 ocr[RTCAN_MEM_MAX_DEV];
+static u8 cdr[RTCAN_MEM_MAX_DEV];
+
+compat_module_int_param_array(mem, RTCAN_MEM_MAX_DEV);
+compat_module_int_param_array(irq, RTCAN_MEM_MAX_DEV);
+compat_module_int_param_array(clock, RTCAN_MEM_MAX_DEV);
+compat_module_byte_param_array(ocr, RTCAN_MEM_MAX_DEV);
+compat_module_byte_param_array(cdr, RTCAN_MEM_MAX_DEV);
+
+MODULE_PARM_DESC(mem, "The io-memory address");
+MODULE_PARM_DESC(irq, "The interrupt number");
+MODULE_PARM_DESC(clock, "External clock frequency (default 16 MHz)");
+MODULE_PARM_DESC(ocr, "Value of output control register (default 0x1a)");
+MODULE_PARM_DESC(cdr, "Value of clock divider register (default 0xc8");
+
+#define RTCAN_MEM_RANGE 0x80
+
+struct rtcan_mem
+{
+ u8 *vmem;
+};
+
+static struct rtcan_device *rtcan_mem_devs[RTCAN_MEM_MAX_DEV];
+
+static u8 rtcan_mem_readreg(struct rtcan_device *dev, int reg)
+{
+ struct rtcan_mem *board = (struct rtcan_mem *)dev->board_priv;
+ return readb(board->vmem + reg);
+}
+
+static void rtcan_mem_writereg(struct rtcan_device *dev, int reg, u8 val)
+{
+ struct rtcan_mem *board = (struct rtcan_mem *)dev->board_priv;
+ writeb(val, board->vmem + reg);
+}
+
+int __init rtcan_mem_init_one(int idx)
+{
+ struct rtcan_device *dev;
+ struct rtcan_sja1000 *chip;
+ struct rtcan_mem *board;
+ int ret;
+
+ if ((dev = rtcan_dev_alloc(sizeof(struct rtcan_sja1000),
+ sizeof(struct rtcan_mem))) == NULL)
+ return -ENOMEM;
+
+ chip = (struct rtcan_sja1000 *)dev->priv;
+ board = (struct rtcan_mem *)dev->board_priv;
+
+ dev->board_name = mem_board_name;
+
+ chip->irq_num = irq[idx];
+ chip->irq_flags = RTDM_IRQTYPE_SHARED;
+ chip->read_reg = rtcan_mem_readreg;
+ chip->write_reg = rtcan_mem_writereg;
+
+ if (!request_mem_region(mem[idx], RTCAN_MEM_RANGE, RTCAN_DRV_NAME)) {
+ ret = -EBUSY;
+ goto out_dev_free;
+ }
+
+ /* ioremap io memory */
+ if (!(board->vmem = (u8*)ioremap(mem[idx], RTCAN_MEM_RANGE))) {
+ release_mem_region(mem[idx], RTCAN_MEM_RANGE);
+ ret = -EBUSY;
+ goto out_dev_free;
+ }
+
+ /* Clock frequency in Hz */
+ if (clock[idx])
+ dev->can_sys_clock = clock[idx] / 2;
+ else
+ dev->can_sys_clock = 8000000; /* 16/2 MHz */
+
+ /* Output control register */
+ if (ocr[idx])
+ chip->ocr = ocr[idx];
+ else
+ chip->ocr = SJA_OCR_MODE_NORMAL | SJA_OCR_TX0_PUSHPULL;
+
+ if (cdr[idx])
+ chip->cdr = cdr[idx];
+ else
+ chip->cdr = SJA_CDR_CAN_MODE | SJA_CDR_CLK_OFF | SJA_CDR_CBP;
+
+ strncpy(dev->name, RTCAN_DEV_NAME, IFNAMSIZ);
+
+ ret = rtcan_sja1000_register(dev);
+ if (ret) {
+ printk(KERN_ERR "ERROR %d while trying to register SJA1000 device!\n",
+ ret);
+ goto out_free_region;
+ }
+
+ rtcan_mem_devs[idx] = dev;
+ return 0;
+
+ out_free_region:
+ iounmap(board->vmem);
+
+ out_dev_free:
+ rtcan_dev_free(dev);
+
+ return ret;
+}
+
+/** Init module */
+static int __init rtcan_mem_init(void)
+{
+ int i;
+ int ret;
+ int done = 0;
+ for (i = 0;
+ i < RTCAN_MEM_MAX_DEV && mem[i] != 0;
+ i++) {
+
+ if ((ret = rtcan_mem_init_one(i) != 0)) {
+ return ret;
+ }
+ done++;
+ }
+ if (done)
+ return 0;
+ printk(KERN_ERR "ERROR! No devices specified! "
+ "Use mem=<base_addr1>[,...] irq=<irq1>[,...]\n");
+
+ return -EINVAL;
+}
+
+
+/** Cleanup module */
+static void __exit rtcan_mem_exit(void)
+{
+ int i;
+ struct rtcan_device *dev;
+ u8 *vmem;
+
+ for (i = 0; i < RTCAN_MEM_MAX_DEV; i++) {
+ dev = rtcan_mem_devs[i];
+ if (!dev)
+ continue;
+ vmem = ((struct rtcan_mem *)dev->board_priv)->vmem;
+ rtcan_sja1000_unregister(dev);
+ iounmap(vmem);
+ release_mem_region(mem[i], RTCAN_MEM_RANGE);
+ }
+}
+
+module_init(rtcan_mem_init);
+module_exit(rtcan_mem_exit);
Index: ksrc/drivers/can/sja1000/Makefile
===================================================================
--- ksrc/drivers/can/sja1000/Makefile (revision 1564)
+++ ksrc/drivers/can/sja1000/Makefile (working copy)
@@ -8,11 +8,13 @@
obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_PEAK_PCI) += xeno_rtcan_peak_pci.o
obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_PEAK_DNG) += xeno_rtcan_peak_dng.o
obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_ISA) += xeno_rtcan_isa.o
+obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_MEM) += xeno_rtcan_mem.o
xeno_rtcan_sja1000-y := rtcan_sja1000.o rtcan_sja1000_proc.o
xeno_rtcan_peak_pci-y := rtcan_peak_pci.o
xeno_rtcan_peak_dng-y := rtcan_peak_dng.o
xeno_rtcan_isa-y := rtcan_isa.o
+xeno_rtcan_mem-y := rtcan_mem.o
else
@@ -24,6 +26,7 @@
obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_PEAK_PCI) += xeno_rtcan_peak_pci.o
obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_PEAK_DNG) += xeno_rtcan_peak_dng.o
obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_ISA) += xeno_rtcan_isa.o
+obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_MEM) += xeno_rtcan_mem.o
list-multi := xeno_rtcan_sja1000.o
@@ -31,6 +34,7 @@
xeno_rtcan_peak_pci-objs := rtcan_peak_pci.o
xeno_rtcan_peak_dng-objs := rtcan_peak_dng.o
xeno_rtcan_isa-objs := rtcan_isa.o
+xeno_rtcan_mem-objs := rtcan_mem.o
export-objs := $(xeno_rtcan_sja1000-objs)
@@ -49,4 +53,7 @@
xeno_rtcan_isa.o: $(xeno_rtcan_isa-objs)
$(LD) -r -o $@ $(xeno_rtcan_isa-objs)
+
+xeno_rtcan_mem.o: $(xeno_rtcan_mem-objs)
+ $(LD) -r -o $@ $(xeno_rtcan_mem-objs)
endif
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Xenomai-core] [PATCH] rtcan_mem - CAN driver for memory mapped SJA1000 controllers
2006-09-12 16:07 ` Matthias Fuchs
@ 2006-09-12 16:46 ` Wolfgang Grandegger
2006-09-12 18:37 ` Jan Kiszka
2006-09-12 19:05 ` Matthias Fuchs
0 siblings, 2 replies; 13+ messages in thread
From: Wolfgang Grandegger @ 2006-09-12 16:46 UTC (permalink / raw)
To: Matthias Fuchs; +Cc: Jan Kiszka, xenomai
Matthias Fuchs wrote:
> Hi,
>
> here's the 2nd try:
> - updated external clock frequency handling
> - use request_mem_region() before remapping memory
> - renamed local variable mem to vmem in clean loop because it shadowed a
> module parameter
I have one more comment, sorry. I think you should use:
volatile void __iomem *vmem;
and the cleanup in case of error is not OK. See below.
> Now it's on you to decide if we should merge and/or rename. I wouldn't merge
> because of my former arguments. But do not care about me:-)
Fine for me for the time being.
> Matthias
Wolfgang
> On Tuesday 12 September 2006 14:22, Wolfgang Grandegger wrote:
>> Hi Matthias,
>>
>> Matthias Fuchs wrote:
>>> Hi,
>>>
>>> attached you will find a patch that adds support for memory mapped
>>> SJA1000 CAN controllers as they often can be found on embedded boards.
>>> The driver is based on the rtmen_isa driver.
>> What about using request_mem_region()? While looking to the driver I now
>> realize, that it's mainly duplicated code. Does it not make more sense
>> to make a combined io/mem driver. If io address < 32K it's an io driver
>> else a mem driver.
>>
>>
>> ------------------------------------------------------------------------
>>
>> Index: ksrc/drivers/can/sja1000/Kconfig
>> ===================================================================
>> --- ksrc/drivers/can/sja1000/Kconfig (revision 1564)
>> +++ ksrc/drivers/can/sja1000/Kconfig (working copy)
>> @@ -13,6 +13,16 @@
>> int "Maximum number of ISA devices"
>> default 4
>>
>> +config XENO_DRIVERS_RTCAN_SJA1000_MEM
>> + depends on XENO_DRIVERS_RTCAN_SJA1000
>> + tristate "Memory mapped controllers"
>> + default n
>> +
>> +config XENO_DRIVERS_RTCAN_SJA1000_MEM_MAX_DEV
>> + depends on XENO_DRIVERS_RTCAN_SJA1000_MEM
>> + int "Maximum number of memory mapped controllers"
>> + default 4
>> +
>> config XENO_DRIVERS_RTCAN_SJA1000_PEAK_PCI
>> depends on XENO_DRIVERS_RTCAN_SJA1000
>> tristate "PEAK PCI Card"
>> Index: ksrc/drivers/can/sja1000/rtcan_mem.c
>> ===================================================================
>> --- ksrc/drivers/can/sja1000/rtcan_mem.c (revision 0)
>> +++ ksrc/drivers/can/sja1000/rtcan_mem.c (revision 0)
>> @@ -0,0 +1,213 @@
>> +/*
>> + * Copyright (C) 2006 Matthias Fuchs <matthias.fuchs@domain.hid>,
>> + * Jan Kiszka <jan.kiszka@domain.hid>
>> + *
>> + * RTCAN driver for memory mapped SJA1000 CAN controller
>> + * This code has been tested on esd's CPCI405/EPPC405 PPC405 systems.
>> + *
>> + * This driver is derived from the rtcan-isa driver by
>> + * Wolfgang Grandegger and Sebastian Smolorz.
>> + *
>> + * Copyright (C) 2006 Wolfgang Grandegger <wg@domain.hid>
>> + * Copyright (C) 2005, 2006 Sebastian Smolorz
>> + * <Sebastian.Smolorz@domain.hid>
>> + *
>> + *
>> + * 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; eitherer 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 <rtdm/rtdm_driver.h>
>> +
>> +#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 "sja1000-mem"
>> +
>> +#ifdef CONFIG_XENO_DRIVERS_RTCAN_MEM_MAX_DEVS
>> +#define RTCAN_MEM_MAX_DEV CONFIG_XENO_DRIVERS_RTCAN_MEM_MAX_DEV
>> +#else
>> +#define RTCAN_MEM_MAX_DEV 2
>> +#endif
>> +
>> +static char *mem_board_name = "mem mapped";
>> +
>> +MODULE_AUTHOR("Matthias Fuchs <matthias.fuchs@domain.hid>");
>> +MODULE_DESCRIPTION("RTCAN driver for memory mapped SJA1000 controller");
>> +MODULE_SUPPORTED_DEVICE("mem mapped");
>> +MODULE_LICENSE("GPL");
>> +
>> +static u32 mem[RTCAN_MEM_MAX_DEV];
>> +static int irq[RTCAN_MEM_MAX_DEV];
>> +static u32 clock[RTCAN_MEM_MAX_DEV];
>> +static u8 ocr[RTCAN_MEM_MAX_DEV];
>> +static u8 cdr[RTCAN_MEM_MAX_DEV];
>> +
>> +compat_module_int_param_array(mem, RTCAN_MEM_MAX_DEV);
>> +compat_module_int_param_array(irq, RTCAN_MEM_MAX_DEV);
>> +compat_module_int_param_array(clock, RTCAN_MEM_MAX_DEV);
>> +compat_module_byte_param_array(ocr, RTCAN_MEM_MAX_DEV);
>> +compat_module_byte_param_array(cdr, RTCAN_MEM_MAX_DEV);
>> +
>> +MODULE_PARM_DESC(mem, "The io-memory address");
>> +MODULE_PARM_DESC(irq, "The interrupt number");
>> +MODULE_PARM_DESC(clock, "External clock frequency (default 16 MHz)");
>> +MODULE_PARM_DESC(ocr, "Value of output control register (default 0x1a)");
>> +MODULE_PARM_DESC(cdr, "Value of clock divider register (default 0xc8");
>> +
>> +#define RTCAN_MEM_RANGE 0x80
>> +
>> +struct rtcan_mem
>> +{
>> + u8 *vmem;
>> +};
>> +
>> +static struct rtcan_device *rtcan_mem_devs[RTCAN_MEM_MAX_DEV];
>> +
>> +static u8 rtcan_mem_readreg(struct rtcan_device *dev, int reg)
>> +{
>> + struct rtcan_mem *board = (struct rtcan_mem *)dev->board_priv;
>> + return readb(board->vmem + reg);
>> +}
>> +
>> +static void rtcan_mem_writereg(struct rtcan_device *dev, int reg, u8 val)
>> +{
>> + struct rtcan_mem *board = (struct rtcan_mem *)dev->board_priv;
>> + writeb(val, board->vmem + reg);
>> +}
>> +
>> +int __init rtcan_mem_init_one(int idx)
>> +{
>> + struct rtcan_device *dev;
>> + struct rtcan_sja1000 *chip;
>> + struct rtcan_mem *board;
>> + int ret;
>> +
>> + if ((dev = rtcan_dev_alloc(sizeof(struct rtcan_sja1000),
>> + sizeof(struct rtcan_mem))) == NULL)
>> + return -ENOMEM;
>> +
>> + chip = (struct rtcan_sja1000 *)dev->priv;
>> + board = (struct rtcan_mem *)dev->board_priv;
>> +
>> + dev->board_name = mem_board_name;
>> +
>> + chip->irq_num = irq[idx];
>> + chip->irq_flags = RTDM_IRQTYPE_SHARED;
>> + chip->read_reg = rtcan_mem_readreg;
>> + chip->write_reg = rtcan_mem_writereg;
>> +
>> + if (!request_mem_region(mem[idx], RTCAN_MEM_RANGE, RTCAN_DRV_NAME)) {
>> + ret = -EBUSY;
>> + goto out_dev_free;
>> + }
>> +
>> + /* ioremap io memory */
>> + if (!(board->vmem = (u8*)ioremap(mem[idx], RTCAN_MEM_RANGE))) {
>> + release_mem_region(mem[idx], RTCAN_MEM_RANGE);
>> + ret = -EBUSY;
goto out_release_mem;
>> + }
>> +
>> + /* Clock frequency in Hz */
>> + if (clock[idx])
>> + dev->can_sys_clock = clock[idx] / 2;
>> + else
>> + dev->can_sys_clock = 8000000; /* 16/2 MHz */
>> +
>> + /* Output control register */
>> + if (ocr[idx])
>> + chip->ocr = ocr[idx];
>> + else
>> + chip->ocr = SJA_OCR_MODE_NORMAL | SJA_OCR_TX0_PUSHPULL;
>> +
>> + if (cdr[idx])
>> + chip->cdr = cdr[idx];
>> + else
>> + chip->cdr = SJA_CDR_CAN_MODE | SJA_CDR_CLK_OFF | SJA_CDR_CBP;
>> +
>> + strncpy(dev->name, RTCAN_DEV_NAME, IFNAMSIZ);
>> +
>> + ret = rtcan_sja1000_register(dev);
>> + if (ret) {
>> + printk(KERN_ERR "ERROR %d while trying to register SJA1000 device!\n",
>> + ret);
goto out_iounmap;
>> + }
>> +
>> + rtcan_mem_devs[idx] = dev;
>> + return 0;
>> +
>> + out_iounmap:
>> + iounmap(board->vmem);
out_release_mem:
release_mem_region(mem[idx], RTCAN_MEM_RANGE);
>> +
>> + out_dev_free:
>> + rtcan_dev_free(dev);
>> +
>> + return ret;
>> +}
>> +
>> +/** Init module */
>> +static int __init rtcan_mem_init(void)
>> +{
>> + int i;
>> + int ret;
>> + int done = 0;
>> + for (i = 0;
>> + i < RTCAN_MEM_MAX_DEV && mem[i] != 0;
>> + i++) {
>> +
>> + if ((ret = rtcan_mem_init_one(i) != 0)) {
>> + return ret;
>> + }
>> + done++;
>> + }
>> + if (done)
>> + return 0;
>> + printk(KERN_ERR "ERROR! No devices specified! "
>> + "Use mem=<base_addr1>[,...] irq=<irq1>[,...]\n");
>> +
>> + return -EINVAL;
>> +}
>> +
>> +
>> +/** Cleanup module */
>> +static void __exit rtcan_mem_exit(void)
>> +{
>> + int i;
>> + struct rtcan_device *dev;
>> + u8 *vmem;
>> +
>> + for (i = 0; i < RTCAN_MEM_MAX_DEV; i++) {
>> + dev = rtcan_mem_devs[i];
>> + if (!dev)
>> + continue;
>> + vmem = ((struct rtcan_mem *)dev->board_priv)->vmem;
>> + rtcan_sja1000_unregister(dev);
>> + iounmap(vmem);
>> + release_mem_region(mem[i], RTCAN_MEM_RANGE);
>> + }
>> +}
>> +
>> +module_init(rtcan_mem_init);
>> +module_exit(rtcan_mem_exit);
>> Index: ksrc/drivers/can/sja1000/Makefile
>> ===================================================================
>> --- ksrc/drivers/can/sja1000/Makefile (revision 1564)
>> +++ ksrc/drivers/can/sja1000/Makefile (working copy)
>> @@ -8,11 +8,13 @@
>> obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_PEAK_PCI) += xeno_rtcan_peak_pci.o
>> obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_PEAK_DNG) += xeno_rtcan_peak_dng.o
>> obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_ISA) += xeno_rtcan_isa.o
>> +obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_MEM) += xeno_rtcan_mem.o
>>
>> xeno_rtcan_sja1000-y := rtcan_sja1000.o rtcan_sja1000_proc.o
>> xeno_rtcan_peak_pci-y := rtcan_peak_pci.o
>> xeno_rtcan_peak_dng-y := rtcan_peak_dng.o
>> xeno_rtcan_isa-y := rtcan_isa.o
>> +xeno_rtcan_mem-y := rtcan_mem.o
>>
>> else
>>
>> @@ -24,6 +26,7 @@
>> obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_PEAK_PCI) += xeno_rtcan_peak_pci.o
>> obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_PEAK_DNG) += xeno_rtcan_peak_dng.o
>> obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_ISA) += xeno_rtcan_isa.o
>> +obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_MEM) += xeno_rtcan_mem.o
>>
>> list-multi := xeno_rtcan_sja1000.o
>>
>> @@ -31,6 +34,7 @@
>> xeno_rtcan_peak_pci-objs := rtcan_peak_pci.o
>> xeno_rtcan_peak_dng-objs := rtcan_peak_dng.o
>> xeno_rtcan_isa-objs := rtcan_isa.o
>> +xeno_rtcan_mem-objs := rtcan_mem.o
>>
>> export-objs := $(xeno_rtcan_sja1000-objs)
>>
>> @@ -49,4 +53,7 @@
>>
>> xeno_rtcan_isa.o: $(xeno_rtcan_isa-objs)
>> $(LD) -r -o $@ $(xeno_rtcan_isa-objs)
>> +
>> +xeno_rtcan_mem.o: $(xeno_rtcan_mem-objs)
>> + $(LD) -r -o $@ $(xeno_rtcan_mem-objs)
>> endif
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Xenomai-core] [PATCH] rtcan_mem - CAN driver for memory mapped SJA1000 controllers
2006-09-12 16:46 ` Wolfgang Grandegger
@ 2006-09-12 18:37 ` Jan Kiszka
2006-09-12 19:05 ` Matthias Fuchs
1 sibling, 0 replies; 13+ messages in thread
From: Jan Kiszka @ 2006-09-12 18:37 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: xenomai
[-- Attachment #1: Type: text/plain, Size: 576 bytes --]
Wolfgang Grandegger wrote:
> Matthias Fuchs wrote:
>> Hi,
>>
>> here's the 2nd try:
>> - updated external clock frequency handling
>> - use request_mem_region() before remapping memory
>> - renamed local variable mem to vmem in clean loop because it shadowed
>> a
>> module parameter
>
> I have one more comment, sorry. I think you should use:
>
> volatile void __iomem *vmem;
>
> and the cleanup in case of error is not OK. See below.
Could someone post a new, full patch so that I don't mess anything up on
the way into SVN?
Thanks,
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Xenomai-core] [PATCH] rtcan_mem - CAN driver for memory mapped SJA1000 controllers
2006-09-12 16:46 ` Wolfgang Grandegger
2006-09-12 18:37 ` Jan Kiszka
@ 2006-09-12 19:05 ` Matthias Fuchs
2006-09-12 20:42 ` Jan Kiszka
1 sibling, 1 reply; 13+ messages in thread
From: Matthias Fuchs @ 2006-09-12 19:05 UTC (permalink / raw)
To: xenomai; +Cc: Jan Kiszka
[-- Attachment #1: Type: text/plain, Size: 11789 bytes --]
On Tuesday 12 September 2006 18:46, Wolfgang Grandegger wrote:
> Matthias Fuchs wrote:
> > Hi,
> >
> > here's the 2nd try:
> > - updated external clock frequency handling
> > - use request_mem_region() before remapping memory
> > - renamed local variable mem to vmem in clean loop because it shadowed a
> > module parameter
>
> I have one more comment, sorry. I think you should use:
>
> volatile void __iomem *vmem;
__iomem is available since kernel version 2.4.28 and it is defined for
documentation only. But the xenomai readme names the Denx 2.4.25'based tree
as the 2.4 refererence. And that tree also defines __iomem.
So, objection allowed.
> and the cleanup in case of error is not OK. See below.
Fixed.
New patch attached.
Matthias
>
> > Now it's on you to decide if we should merge and/or rename. I wouldn't
> > merge because of my former arguments. But do not care about me:-)
>
> Fine for me for the time being.
>
> > Matthias
>
> Wolfgang
>
> > On Tuesday 12 September 2006 14:22, Wolfgang Grandegger wrote:
> >> Hi Matthias,
> >>
> >> Matthias Fuchs wrote:
> >>> Hi,
> >>>
> >>> attached you will find a patch that adds support for memory mapped
> >>> SJA1000 CAN controllers as they often can be found on embedded boards.
> >>> The driver is based on the rtmen_isa driver.
> >>
> >> What about using request_mem_region()? While looking to the driver I now
> >> realize, that it's mainly duplicated code. Does it not make more sense
> >> to make a combined io/mem driver. If io address < 32K it's an io driver
> >> else a mem driver.
> >>
> >>
> >> ------------------------------------------------------------------------
> >>
> >> Index: ksrc/drivers/can/sja1000/Kconfig
> >> ===================================================================
> >> --- ksrc/drivers/can/sja1000/Kconfig (revision 1564)
> >> +++ ksrc/drivers/can/sja1000/Kconfig (working copy)
> >> @@ -13,6 +13,16 @@
> >> int "Maximum number of ISA devices"
> >> default 4
> >>
> >> +config XENO_DRIVERS_RTCAN_SJA1000_MEM
> >> + depends on XENO_DRIVERS_RTCAN_SJA1000
> >> + tristate "Memory mapped controllers"
> >> + default n
> >> +
> >> +config XENO_DRIVERS_RTCAN_SJA1000_MEM_MAX_DEV
> >> + depends on XENO_DRIVERS_RTCAN_SJA1000_MEM
> >> + int "Maximum number of memory mapped controllers"
> >> + default 4
> >> +
> >> config XENO_DRIVERS_RTCAN_SJA1000_PEAK_PCI
> >> depends on XENO_DRIVERS_RTCAN_SJA1000
> >> tristate "PEAK PCI Card"
> >> Index: ksrc/drivers/can/sja1000/rtcan_mem.c
> >> ===================================================================
> >> --- ksrc/drivers/can/sja1000/rtcan_mem.c (revision 0)
> >> +++ ksrc/drivers/can/sja1000/rtcan_mem.c (revision 0)
> >> @@ -0,0 +1,213 @@
> >> +/*
> >> + * Copyright (C) 2006 Matthias Fuchs
> >> <matthias.fuchs@domain.hid>, + * Jan Kiszka
> >> <jan.kiszka@domain.hid>
> >> + *
> >> + * RTCAN driver for memory mapped SJA1000 CAN controller
> >> + * This code has been tested on esd's CPCI405/EPPC405 PPC405 systems.
> >> + *
> >> + * This driver is derived from the rtcan-isa driver by
> >> + * Wolfgang Grandegger and Sebastian Smolorz.
> >> + *
> >> + * Copyright (C) 2006 Wolfgang Grandegger <wg@domain.hid>
> >> + * Copyright (C) 2005, 2006 Sebastian Smolorz
> >> + * <Sebastian.Smolorz@domain.hid>
> >> + *
> >> + *
> >> + * 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; eitherer 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 <rtdm/rtdm_driver.h>
> >> +
> >> +#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 "sja1000-mem"
> >> +
> >> +#ifdef CONFIG_XENO_DRIVERS_RTCAN_MEM_MAX_DEVS
> >> +#define RTCAN_MEM_MAX_DEV CONFIG_XENO_DRIVERS_RTCAN_MEM_MAX_DEV
> >> +#else
> >> +#define RTCAN_MEM_MAX_DEV 2
> >> +#endif
> >> +
> >> +static char *mem_board_name = "mem mapped";
> >> +
> >> +MODULE_AUTHOR("Matthias Fuchs <matthias.fuchs@domain.hid>");
> >> +MODULE_DESCRIPTION("RTCAN driver for memory mapped SJA1000
> >> controller"); +MODULE_SUPPORTED_DEVICE("mem mapped");
> >> +MODULE_LICENSE("GPL");
> >> +
> >> +static u32 mem[RTCAN_MEM_MAX_DEV];
> >> +static int irq[RTCAN_MEM_MAX_DEV];
> >> +static u32 clock[RTCAN_MEM_MAX_DEV];
> >> +static u8 ocr[RTCAN_MEM_MAX_DEV];
> >> +static u8 cdr[RTCAN_MEM_MAX_DEV];
> >> +
> >> +compat_module_int_param_array(mem, RTCAN_MEM_MAX_DEV);
> >> +compat_module_int_param_array(irq, RTCAN_MEM_MAX_DEV);
> >> +compat_module_int_param_array(clock, RTCAN_MEM_MAX_DEV);
> >> +compat_module_byte_param_array(ocr, RTCAN_MEM_MAX_DEV);
> >> +compat_module_byte_param_array(cdr, RTCAN_MEM_MAX_DEV);
> >> +
> >> +MODULE_PARM_DESC(mem, "The io-memory address");
> >> +MODULE_PARM_DESC(irq, "The interrupt number");
> >> +MODULE_PARM_DESC(clock, "External clock frequency (default 16 MHz)");
> >> +MODULE_PARM_DESC(ocr, "Value of output control register (default
> >> 0x1a)"); +MODULE_PARM_DESC(cdr, "Value of clock divider register
> >> (default 0xc8"); +
> >> +#define RTCAN_MEM_RANGE 0x80
> >> +
> >> +struct rtcan_mem
> >> +{
> >> + u8 *vmem;
> >> +};
> >> +
> >> +static struct rtcan_device *rtcan_mem_devs[RTCAN_MEM_MAX_DEV];
> >> +
> >> +static u8 rtcan_mem_readreg(struct rtcan_device *dev, int reg)
> >> +{
> >> + struct rtcan_mem *board = (struct rtcan_mem *)dev->board_priv;
> >> + return readb(board->vmem + reg);
> >> +}
> >> +
> >> +static void rtcan_mem_writereg(struct rtcan_device *dev, int reg, u8
> >> val) +{
> >> + struct rtcan_mem *board = (struct rtcan_mem *)dev->board_priv;
> >> + writeb(val, board->vmem + reg);
> >> +}
> >> +
> >> +int __init rtcan_mem_init_one(int idx)
> >> +{
> >> + struct rtcan_device *dev;
> >> + struct rtcan_sja1000 *chip;
> >> + struct rtcan_mem *board;
> >> + int ret;
> >> +
> >> + if ((dev = rtcan_dev_alloc(sizeof(struct rtcan_sja1000),
> >> + sizeof(struct rtcan_mem))) == NULL)
> >> + return -ENOMEM;
> >> +
> >> + chip = (struct rtcan_sja1000 *)dev->priv;
> >> + board = (struct rtcan_mem *)dev->board_priv;
> >> +
> >> + dev->board_name = mem_board_name;
> >> +
> >> + chip->irq_num = irq[idx];
> >> + chip->irq_flags = RTDM_IRQTYPE_SHARED;
> >> + chip->read_reg = rtcan_mem_readreg;
> >> + chip->write_reg = rtcan_mem_writereg;
> >> +
> >> + if (!request_mem_region(mem[idx], RTCAN_MEM_RANGE, RTCAN_DRV_NAME))
> >> { + ret = -EBUSY;
> >> + goto out_dev_free;
> >> + }
> >> +
> >> + /* ioremap io memory */
> >> + if (!(board->vmem = (u8*)ioremap(mem[idx], RTCAN_MEM_RANGE))) {
> >> + release_mem_region(mem[idx], RTCAN_MEM_RANGE);
> >> + ret = -EBUSY;
>
> goto out_release_mem;
>
> >> + }
> >> +
> >> + /* Clock frequency in Hz */
> >> + if (clock[idx])
> >> + dev->can_sys_clock = clock[idx] / 2;
> >> + else
> >> + dev->can_sys_clock = 8000000; /* 16/2 MHz */
> >> +
> >> + /* Output control register */
> >> + if (ocr[idx])
> >> + chip->ocr = ocr[idx];
> >> + else
> >> + chip->ocr = SJA_OCR_MODE_NORMAL | SJA_OCR_TX0_PUSHPULL;
> >> +
> >> + if (cdr[idx])
> >> + chip->cdr = cdr[idx];
> >> + else
> >> + chip->cdr = SJA_CDR_CAN_MODE | SJA_CDR_CLK_OFF | SJA_CDR_CBP;
> >> +
> >> + strncpy(dev->name, RTCAN_DEV_NAME, IFNAMSIZ);
> >> +
> >> + ret = rtcan_sja1000_register(dev);
> >> + if (ret) {
> >> + printk(KERN_ERR "ERROR %d while trying to register SJA1000 device!\n",
> >> + ret);
>
> goto out_iounmap;
>
> >> + }
> >> +
> >> + rtcan_mem_devs[idx] = dev;
> >> + return 0;
> >> +
> >> + out_iounmap:
> >> + iounmap(board->vmem);
>
> out_release_mem:
> release_mem_region(mem[idx], RTCAN_MEM_RANGE);
>
> >> +
> >> + out_dev_free:
> >> + rtcan_dev_free(dev);
> >> +
> >> + return ret;
> >> +}
> >> +
> >> +/** Init module */
> >> +static int __init rtcan_mem_init(void)
> >> +{
> >> + int i;
> >> + int ret;
> >> + int done = 0;
> >> + for (i = 0;
> >> + i < RTCAN_MEM_MAX_DEV && mem[i] != 0;
> >> + i++) {
> >> +
> >> + if ((ret = rtcan_mem_init_one(i) != 0)) {
> >> + return ret;
> >> + }
> >> + done++;
> >> + }
> >> + if (done)
> >> + return 0;
> >> + printk(KERN_ERR "ERROR! No devices specified! "
> >> + "Use mem=<base_addr1>[,...] irq=<irq1>[,...]\n");
> >> +
> >> + return -EINVAL;
> >> +}
> >> +
> >> +
> >> +/** Cleanup module */
> >> +static void __exit rtcan_mem_exit(void)
> >> +{
> >> + int i;
> >> + struct rtcan_device *dev;
> >> + u8 *vmem;
> >> +
> >> + for (i = 0; i < RTCAN_MEM_MAX_DEV; i++) {
> >> + dev = rtcan_mem_devs[i];
> >> + if (!dev)
> >> + continue;
> >> + vmem = ((struct rtcan_mem *)dev->board_priv)->vmem;
> >> + rtcan_sja1000_unregister(dev);
> >> + iounmap(vmem);
> >> + release_mem_region(mem[i], RTCAN_MEM_RANGE);
> >> + }
> >> +}
> >> +
> >> +module_init(rtcan_mem_init);
> >> +module_exit(rtcan_mem_exit);
> >> Index: ksrc/drivers/can/sja1000/Makefile
> >> ===================================================================
> >> --- ksrc/drivers/can/sja1000/Makefile (revision 1564)
> >> +++ ksrc/drivers/can/sja1000/Makefile (working copy)
> >> @@ -8,11 +8,13 @@
> >> obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_PEAK_PCI) +=
> >> xeno_rtcan_peak_pci.o obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_PEAK_DNG)
> >> += xeno_rtcan_peak_dng.o obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_ISA) +=
> >> xeno_rtcan_isa.o +obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_MEM) +=
> >> xeno_rtcan_mem.o
> >>
> >> xeno_rtcan_sja1000-y := rtcan_sja1000.o rtcan_sja1000_proc.o
> >> xeno_rtcan_peak_pci-y := rtcan_peak_pci.o
> >> xeno_rtcan_peak_dng-y := rtcan_peak_dng.o
> >> xeno_rtcan_isa-y := rtcan_isa.o
> >> +xeno_rtcan_mem-y := rtcan_mem.o
> >>
> >> else
> >>
> >> @@ -24,6 +26,7 @@
> >> obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_PEAK_PCI) +=
> >> xeno_rtcan_peak_pci.o obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_PEAK_DNG)
> >> += xeno_rtcan_peak_dng.o obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_ISA) +=
> >> xeno_rtcan_isa.o +obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_MEM) +=
> >> xeno_rtcan_mem.o
> >>
> >> list-multi := xeno_rtcan_sja1000.o
> >>
> >> @@ -31,6 +34,7 @@
> >> xeno_rtcan_peak_pci-objs := rtcan_peak_pci.o
> >> xeno_rtcan_peak_dng-objs := rtcan_peak_dng.o
> >> xeno_rtcan_isa-objs := rtcan_isa.o
> >> +xeno_rtcan_mem-objs := rtcan_mem.o
> >>
> >> export-objs := $(xeno_rtcan_sja1000-objs)
> >>
> >> @@ -49,4 +53,7 @@
> >>
> >> xeno_rtcan_isa.o: $(xeno_rtcan_isa-objs)
> >> $(LD) -r -o $@ $(xeno_rtcan_isa-objs)
> >> +
> >> +xeno_rtcan_mem.o: $(xeno_rtcan_mem-objs)
> >> + $(LD) -r -o $@ $(xeno_rtcan_mem-objs)
> >> endif
>
> _______________________________________________
> Xenomai-core mailing list
> Xenomai-core@domain.hid
> https://mail.gna.org/listinfo/xenomai-core
[-- Attachment #2: rtcan_mem_3.patch --]
[-- Type: text/x-diff, Size: 8589 bytes --]
Index: ksrc/drivers/can/sja1000/Kconfig
===================================================================
--- ksrc/drivers/can/sja1000/Kconfig (revision 1564)
+++ ksrc/drivers/can/sja1000/Kconfig (working copy)
@@ -13,6 +13,16 @@
int "Maximum number of ISA devices"
default 4
+config XENO_DRIVERS_RTCAN_SJA1000_MEM
+ depends on XENO_DRIVERS_RTCAN_SJA1000
+ tristate "Memory mapped controllers"
+ default n
+
+config XENO_DRIVERS_RTCAN_SJA1000_MEM_MAX_DEV
+ depends on XENO_DRIVERS_RTCAN_SJA1000_MEM
+ int "Maximum number of memory mapped controllers"
+ default 4
+
config XENO_DRIVERS_RTCAN_SJA1000_PEAK_PCI
depends on XENO_DRIVERS_RTCAN_SJA1000
tristate "PEAK PCI Card"
Index: ksrc/drivers/can/sja1000/rtcan_mem.c
===================================================================
--- ksrc/drivers/can/sja1000/rtcan_mem.c (revision 0)
+++ ksrc/drivers/can/sja1000/rtcan_mem.c (revision 0)
@@ -0,0 +1,215 @@
+/*
+ * Copyright (C) 2006 Matthias Fuchs <matthias.fuchs@domain.hid>,
+ * Jan Kiszka <jan.kiszka@domain.hid>
+ *
+ * RTCAN driver for memory mapped SJA1000 CAN controller
+ * This code has been tested on esd's CPCI405/EPPC405 PPC405 systems.
+ *
+ * This driver is derived from the rtcan-isa driver by
+ * Wolfgang Grandegger and Sebastian Smolorz.
+ *
+ * Copyright (C) 2006 Wolfgang Grandegger <wg@domain.hid>
+ * Copyright (C) 2005, 2006 Sebastian Smolorz
+ * <Sebastian.Smolorz@domain.hid>
+ *
+ *
+ * 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; eitherer 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 <rtdm/rtdm_driver.h>
+
+#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 "sja1000-mem"
+
+#ifdef CONFIG_XENO_DRIVERS_RTCAN_MEM_MAX_DEVS
+#define RTCAN_MEM_MAX_DEV CONFIG_XENO_DRIVERS_RTCAN_MEM_MAX_DEV
+#else
+#define RTCAN_MEM_MAX_DEV 2
+#endif
+
+static char *mem_board_name = "mem mapped";
+
+MODULE_AUTHOR("Matthias Fuchs <matthias.fuchs@domain.hid>");
+MODULE_DESCRIPTION("RTCAN driver for memory mapped SJA1000 controller");
+MODULE_SUPPORTED_DEVICE("mem mapped");
+MODULE_LICENSE("GPL");
+
+static u32 mem[RTCAN_MEM_MAX_DEV];
+static int irq[RTCAN_MEM_MAX_DEV];
+static u32 clock[RTCAN_MEM_MAX_DEV];
+static u8 ocr[RTCAN_MEM_MAX_DEV];
+static u8 cdr[RTCAN_MEM_MAX_DEV];
+
+compat_module_int_param_array(mem, RTCAN_MEM_MAX_DEV);
+compat_module_int_param_array(irq, RTCAN_MEM_MAX_DEV);
+compat_module_int_param_array(clock, RTCAN_MEM_MAX_DEV);
+compat_module_byte_param_array(ocr, RTCAN_MEM_MAX_DEV);
+compat_module_byte_param_array(cdr, RTCAN_MEM_MAX_DEV);
+
+MODULE_PARM_DESC(mem, "The io-memory address");
+MODULE_PARM_DESC(irq, "The interrupt number");
+MODULE_PARM_DESC(clock, "External clock frequency (default 16 MHz)");
+MODULE_PARM_DESC(ocr, "Value of output control register (default 0x1a)");
+MODULE_PARM_DESC(cdr, "Value of clock divider register (default 0xc8");
+
+#define RTCAN_MEM_RANGE 0x80
+
+struct rtcan_mem
+{
+ volatile void __iomem *vmem;
+};
+
+static struct rtcan_device *rtcan_mem_devs[RTCAN_MEM_MAX_DEV];
+
+static u8 rtcan_mem_readreg(struct rtcan_device *dev, int reg)
+{
+ struct rtcan_mem *board = (struct rtcan_mem *)dev->board_priv;
+ return readb(board->vmem + reg);
+}
+
+static void rtcan_mem_writereg(struct rtcan_device *dev, int reg, u8 val)
+{
+ struct rtcan_mem *board = (struct rtcan_mem *)dev->board_priv;
+ writeb(val, board->vmem + reg);
+}
+
+int __init rtcan_mem_init_one(int idx)
+{
+ struct rtcan_device *dev;
+ struct rtcan_sja1000 *chip;
+ struct rtcan_mem *board;
+ int ret;
+
+ if ((dev = rtcan_dev_alloc(sizeof(struct rtcan_sja1000),
+ sizeof(struct rtcan_mem))) == NULL)
+ return -ENOMEM;
+
+ chip = (struct rtcan_sja1000 *)dev->priv;
+ board = (struct rtcan_mem *)dev->board_priv;
+
+ dev->board_name = mem_board_name;
+
+ chip->irq_num = irq[idx];
+ chip->irq_flags = RTDM_IRQTYPE_SHARED;
+ chip->read_reg = rtcan_mem_readreg;
+ chip->write_reg = rtcan_mem_writereg;
+
+ if (!request_mem_region(mem[idx], RTCAN_MEM_RANGE, RTCAN_DRV_NAME)) {
+ ret = -EBUSY;
+ goto out_dev_free;
+ }
+
+ /* ioremap io memory */
+ if (!(board->vmem = ioremap(mem[idx], RTCAN_MEM_RANGE))) {
+ ret = -EBUSY;
+ goto out_release_mem;
+ }
+
+ /* Clock frequency in Hz */
+ if (clock[idx])
+ dev->can_sys_clock = clock[idx] / 2;
+ else
+ dev->can_sys_clock = 8000000; /* 16/2 MHz */
+
+ /* Output control register */
+ if (ocr[idx])
+ chip->ocr = ocr[idx];
+ else
+ chip->ocr = SJA_OCR_MODE_NORMAL | SJA_OCR_TX0_PUSHPULL;
+
+ if (cdr[idx])
+ chip->cdr = cdr[idx];
+ else
+ chip->cdr = SJA_CDR_CAN_MODE | SJA_CDR_CLK_OFF | SJA_CDR_CBP;
+
+ strncpy(dev->name, RTCAN_DEV_NAME, IFNAMSIZ);
+
+ ret = rtcan_sja1000_register(dev);
+ if (ret) {
+ printk(KERN_ERR "ERROR %d while trying to register SJA1000 device!\n",
+ ret);
+ goto out_iounmap;
+ }
+
+ rtcan_mem_devs[idx] = dev;
+ return 0;
+
+ out_iounmap:
+ iounmap(board->vmem);
+
+ out_release_mem:
+ release_mem_region(mem[idx], RTCAN_MEM_RANGE);
+
+ out_dev_free:
+ rtcan_dev_free(dev);
+
+ return ret;
+}
+
+/** Init module */
+static int __init rtcan_mem_init(void)
+{
+ int i;
+ int ret;
+ int done = 0;
+ for (i = 0;
+ i < RTCAN_MEM_MAX_DEV && mem[i] != 0;
+ i++) {
+
+ if ((ret = rtcan_mem_init_one(i) != 0)) {
+ return ret;
+ }
+ done++;
+ }
+ if (done)
+ return 0;
+ printk(KERN_ERR "ERROR! No devices specified! "
+ "Use mem=<base_addr1>[,...] irq=<irq1>[,...]\n");
+
+ return -EINVAL;
+}
+
+
+/** Cleanup module */
+static void __exit rtcan_mem_exit(void)
+{
+ int i;
+ struct rtcan_device *dev;
+ volatile void __iomem *vmem;
+
+ for (i = 0; i < RTCAN_MEM_MAX_DEV; i++) {
+ dev = rtcan_mem_devs[i];
+ if (!dev)
+ continue;
+ vmem = ((struct rtcan_mem *)dev->board_priv)->vmem;
+ rtcan_sja1000_unregister(dev);
+ iounmap(vmem);
+ release_mem_region(mem[i], RTCAN_MEM_RANGE);
+ }
+}
+
+module_init(rtcan_mem_init);
+module_exit(rtcan_mem_exit);
Index: ksrc/drivers/can/sja1000/Makefile
===================================================================
--- ksrc/drivers/can/sja1000/Makefile (revision 1564)
+++ ksrc/drivers/can/sja1000/Makefile (working copy)
@@ -8,11 +8,13 @@
obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_PEAK_PCI) += xeno_rtcan_peak_pci.o
obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_PEAK_DNG) += xeno_rtcan_peak_dng.o
obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_ISA) += xeno_rtcan_isa.o
+obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_MEM) += xeno_rtcan_mem.o
xeno_rtcan_sja1000-y := rtcan_sja1000.o rtcan_sja1000_proc.o
xeno_rtcan_peak_pci-y := rtcan_peak_pci.o
xeno_rtcan_peak_dng-y := rtcan_peak_dng.o
xeno_rtcan_isa-y := rtcan_isa.o
+xeno_rtcan_mem-y := rtcan_mem.o
else
@@ -24,6 +26,7 @@
obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_PEAK_PCI) += xeno_rtcan_peak_pci.o
obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_PEAK_DNG) += xeno_rtcan_peak_dng.o
obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_ISA) += xeno_rtcan_isa.o
+obj-$(CONFIG_XENO_DRIVERS_RTCAN_SJA1000_MEM) += xeno_rtcan_mem.o
list-multi := xeno_rtcan_sja1000.o
@@ -31,6 +34,7 @@
xeno_rtcan_peak_pci-objs := rtcan_peak_pci.o
xeno_rtcan_peak_dng-objs := rtcan_peak_dng.o
xeno_rtcan_isa-objs := rtcan_isa.o
+xeno_rtcan_mem-objs := rtcan_mem.o
export-objs := $(xeno_rtcan_sja1000-objs)
@@ -49,4 +53,7 @@
xeno_rtcan_isa.o: $(xeno_rtcan_isa-objs)
$(LD) -r -o $@ $(xeno_rtcan_isa-objs)
+
+xeno_rtcan_mem.o: $(xeno_rtcan_mem-objs)
+ $(LD) -r -o $@ $(xeno_rtcan_mem-objs)
endif
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Xenomai-core] [PATCH] rtcan_mem - CAN driver for memory mapped SJA1000 controllers
2006-09-12 19:05 ` Matthias Fuchs
@ 2006-09-12 20:42 ` Jan Kiszka
0 siblings, 0 replies; 13+ messages in thread
From: Jan Kiszka @ 2006-09-12 20:42 UTC (permalink / raw)
To: Matthias Fuchs; +Cc: xenomai
[-- Attachment #1: Type: text/plain, Size: 1309 bytes --]
Matthias Fuchs wrote:
> On Tuesday 12 September 2006 18:46, Wolfgang Grandegger wrote:
>> Matthias Fuchs wrote:
>>> Hi,
>>>
>>> here's the 2nd try:
>>> - updated external clock frequency handling
>>> - use request_mem_region() before remapping memory
>>> - renamed local variable mem to vmem in clean loop because it shadowed a
>>> module parameter
>> I have one more comment, sorry. I think you should use:
>>
>> volatile void __iomem *vmem;
> __iomem is available since kernel version 2.4.28 and it is defined for
> documentation only.
I guess it's for sparse (haven't used it yet).
> But the xenomai readme names the Denx 2.4.25'based tree
> as the 2.4 refererence. And that tree also defines __iomem.
> So, objection allowed.
>
>> and the cleanup in case of error is not OK. See below.
> Fixed.
>
> New patch attached.
Ok, thanks again. I just pushed it in with rev. 1598. I reformatted the
code to kernel style and fixed another inherited bug of the error
roll-back during init. The same happened to rtcan_isa.
Wolfgang, I added the Peak dongle to 2.4 config. Compiles fine, so I
assume you simply forgot that one. Also building-in of all CAN driver
should work now.
If I broke anything, please complain (I only compile-tested so far).
Uff.
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 249 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2006-09-12 20:42 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-09-12 11:30 [Xenomai-core] [PATCH] rtcan_mem - CAN driver for memory mapped SJA1000 controllers Matthias Fuchs
2006-09-12 12:21 ` Jan Kiszka
2006-09-12 12:22 ` Wolfgang Grandegger
2006-09-12 12:38 ` Jan Kiszka
2006-09-12 13:52 ` Wolfgang Grandegger
2006-09-12 13:13 ` Matthias Fuchs
2006-09-12 13:57 ` Wolfgang Grandegger
2006-09-12 14:25 ` Jan Kiszka
2006-09-12 16:07 ` Matthias Fuchs
2006-09-12 16:46 ` Wolfgang Grandegger
2006-09-12 18:37 ` Jan Kiszka
2006-09-12 19:05 ` Matthias Fuchs
2006-09-12 20:42 ` Jan Kiszka
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.