Re: [PATCH] Omnikey Cardman 4040 driver (UPDATE)
Hi hch, thanks for your feedback. An updated version of the patch is at the bottom of this mail. On Mon, Sep 05, 2005 at 08:06:35PM +0100, Christoph Hellwig wrote: > > +#include > > I don't think you need this one. done > > +#include > > you shouldn't need this one. done > > +static atomic_t cm4040_num_devices_open; > > + > > +#ifdef PCMCIA_DEBUG > > +static int pc_debug = PCMCIA_DEBUG; > > +module_param(pc_debug, int, 0600); > > +#define DEBUG(n, x, args...) do { if (pc_debug >= (n)) > > \ > > + printk(KERN_DEBUG "%s:%s:" x, MODULE_NAME, \ > > +__FUNCTION__, ##args); } while (0) > > +#else > > +#define DEBUG(n, args...) > > +#endif > > What about just using pr_debug (or dev_dbg where you have a struct device > handy) I really appreciate the multiple debug levels of the driver, sorry. However, I have converted the DEBUG macro to use dev_printk as a compromise. > Please make the poll timer per device. We generally try to avoid > global state, and this allows to get rid of the opencount tracking aswell. done. > > +static ssize_t cm4040_read(struct file *filp, char __user *buf, > > + size_t count, loff_t *ppos) > > +{ > > + struct reader_dev *dev = (struct reader_dev *) filp->private_data; > > no need to case a void pointer. well, but good style in my opioion anyway. Anway, I don't really mind, so it has been removed. > > + if (count < 10) > > + return -EFAULT; > > + > > + if (filp->f_flags & O_NONBLOCK) { > > + DEBUG(4, "filep->f_flags O_NONBLOCK set\n"); > > + DEBUG(2, "<- cm4040_read (failure)\n"); > > + return -EAGAIN; > > + } > > this sounds rather pointless. letting an O_NONBLOCK open fail all > the time doesn't sound like a good idea. what about fcntl enabling NONBLOCK after the open? Other character drivers (such as rtc) do the same, btw. Is there any way how I can tell the VFS to make any nonblocking operation fail? > please use iminor. done. > given that you fail O_NONLOCK in open already the code above makes even > less sense. see my comment above. > > + dev->owner = current; > > this doesn't make a lot of sense and seems to be only used in > debug code, I'd suggest killing it. done. > you should be able to use file->private_data here. done > I think these events became methods of their own recently, not sure > if it hit -mm or mainline yet. I cannot find anything like that in mainline, thus my code remains unchanged for now. diff --git a/MAINTAINERS b/MAINTAINERS --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1737,6 +1737,11 @@ L: [EMAIL PROTECTED] W: http://www.linuxtr.net S: Maintained +OMNIKEY CARDMAN 4040 DRIVER +P: Harald Welte +M: [EMAIL PROTECTED] +S: Maintained + ONSTREAM SCSI TAPE DRIVER P: Willem Riede M: [EMAIL PROTECTED] diff --git a/drivers/char/pcmcia/Kconfig b/drivers/char/pcmcia/Kconfig --- a/drivers/char/pcmcia/Kconfig +++ b/drivers/char/pcmcia/Kconfig @@ -18,5 +18,18 @@ config SYNCLINK_CS The module will be called synclinkmp. If you want to do that, say M here. +config CARDMAN_4040 + tristate "Omnikey CardMan 4040 support" + depends on PCMCIA + help + Enable support for the Omnikey CardMan 4040 PCMCIA Smartcard + reader. + + This card is basically a USB CCID device connected to a FIFO + in I/O space. To use the kernel driver, you will need either the + PC/SC ifdhandler provided from the Omnikey homepage + (http://www.omnikey.com/), or a current development version of OpenCT + (http://www.opensc.org/). + endmenu diff --git a/drivers/char/pcmcia/Makefile b/drivers/char/pcmcia/Makefile --- a/drivers/char/pcmcia/Makefile +++ b/drivers/char/pcmcia/Makefile @@ -5,3 +5,4 @@ # obj-$(CONFIG_SYNCLINK_CS) += synclink_cs.o +obj-$(CONFIG_CARDMAN_4040) += cm4040_cs.o diff --git a/drivers/char/pcmcia/cm4040_cs.c b/drivers/char/pcmcia/cm4040_cs.c new file mode 100644 --- /dev/null +++ b/drivers/char/pcmcia/cm4040_cs.c @@ -0,0 +1,835 @@ +/* + * A driver for the Omnikey PCMCIA smartcard reader CardMan 4040 + * + * (c) 2000-2004 Omnikey AG (http://www.omnikey.com/) + * + * (C) 2005 Harald Welte <[EMAIL PROTECTED]> + * - add support for poll() + * - driver cleanup + * - add waitqueues + * - adhere to linux kernel coding style and policies + * - support 2.6.13 "new style" pcmcia interface + * + * The device basically is a USB CCID compliant device that has been + * attached to an I/O-Mapped FIFO. + * + * All rights reserved, Dual BSD/GPL Licensed. + */ + +/* #define PCMCIA_DEBUG 6 */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include + +#include "cm4040_cs.h" + + +#ifdef PCMCIA_DEBUG +#define reader_to_dev(x)
Re: [PATCH] Omnikey Cardman 4040 driver (UPDATE)
Hi hch, thanks for your feedback. An updated version of the patch is at the bottom of this mail. On Mon, Sep 05, 2005 at 08:06:35PM +0100, Christoph Hellwig wrote: +#include linux/version.h I don't think you need this one. done +#include pcmcia/version.h you shouldn't need this one. done +static atomic_t cm4040_num_devices_open; + +#ifdef PCMCIA_DEBUG +static int pc_debug = PCMCIA_DEBUG; +module_param(pc_debug, int, 0600); +#define DEBUG(n, x, args...) do { if (pc_debug = (n)) \ + printk(KERN_DEBUG %s:%s: x, MODULE_NAME, \ +__FUNCTION__, ##args); } while (0) +#else +#define DEBUG(n, args...) +#endif What about just using pr_debug (or dev_dbg where you have a struct device handy) I really appreciate the multiple debug levels of the driver, sorry. However, I have converted the DEBUG macro to use dev_printk as a compromise. Please make the poll timer per device. We generally try to avoid global state, and this allows to get rid of the opencount tracking aswell. done. +static ssize_t cm4040_read(struct file *filp, char __user *buf, + size_t count, loff_t *ppos) +{ + struct reader_dev *dev = (struct reader_dev *) filp-private_data; no need to case a void pointer. well, but good style in my opioion anyway. Anway, I don't really mind, so it has been removed. + if (count 10) + return -EFAULT; + + if (filp-f_flags O_NONBLOCK) { + DEBUG(4, filep-f_flags O_NONBLOCK set\n); + DEBUG(2, - cm4040_read (failure)\n); + return -EAGAIN; + } this sounds rather pointless. letting an O_NONBLOCK open fail all the time doesn't sound like a good idea. what about fcntl enabling NONBLOCK after the open? Other character drivers (such as rtc) do the same, btw. Is there any way how I can tell the VFS to make any nonblocking operation fail? please use iminor. done. given that you fail O_NONLOCK in open already the code above makes even less sense. see my comment above. + dev-owner = current; this doesn't make a lot of sense and seems to be only used in debug code, I'd suggest killing it. done. you should be able to use file-private_data here. done I think these events became methods of their own recently, not sure if it hit -mm or mainline yet. I cannot find anything like that in mainline, thus my code remains unchanged for now. diff --git a/MAINTAINERS b/MAINTAINERS --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1737,6 +1737,11 @@ L: [EMAIL PROTECTED] W: http://www.linuxtr.net S: Maintained +OMNIKEY CARDMAN 4040 DRIVER +P: Harald Welte +M: [EMAIL PROTECTED] +S: Maintained + ONSTREAM SCSI TAPE DRIVER P: Willem Riede M: [EMAIL PROTECTED] diff --git a/drivers/char/pcmcia/Kconfig b/drivers/char/pcmcia/Kconfig --- a/drivers/char/pcmcia/Kconfig +++ b/drivers/char/pcmcia/Kconfig @@ -18,5 +18,18 @@ config SYNCLINK_CS The module will be called synclinkmp. If you want to do that, say M here. +config CARDMAN_4040 + tristate Omnikey CardMan 4040 support + depends on PCMCIA + help + Enable support for the Omnikey CardMan 4040 PCMCIA Smartcard + reader. + + This card is basically a USB CCID device connected to a FIFO + in I/O space. To use the kernel driver, you will need either the + PC/SC ifdhandler provided from the Omnikey homepage + (http://www.omnikey.com/), or a current development version of OpenCT + (http://www.opensc.org/). + endmenu diff --git a/drivers/char/pcmcia/Makefile b/drivers/char/pcmcia/Makefile --- a/drivers/char/pcmcia/Makefile +++ b/drivers/char/pcmcia/Makefile @@ -5,3 +5,4 @@ # obj-$(CONFIG_SYNCLINK_CS) += synclink_cs.o +obj-$(CONFIG_CARDMAN_4040) += cm4040_cs.o diff --git a/drivers/char/pcmcia/cm4040_cs.c b/drivers/char/pcmcia/cm4040_cs.c new file mode 100644 --- /dev/null +++ b/drivers/char/pcmcia/cm4040_cs.c @@ -0,0 +1,835 @@ +/* + * A driver for the Omnikey PCMCIA smartcard reader CardMan 4040 + * + * (c) 2000-2004 Omnikey AG (http://www.omnikey.com/) + * + * (C) 2005 Harald Welte [EMAIL PROTECTED] + * - add support for poll() + * - driver cleanup + * - add waitqueues + * - adhere to linux kernel coding style and policies + * - support 2.6.13 new style pcmcia interface + * + * The device basically is a USB CCID compliant device that has been + * attached to an I/O-Mapped FIFO. + * + * All rights reserved, Dual BSD/GPL Licensed. + */ + +/* #define PCMCIA_DEBUG 6 */ + +#include linux/kernel.h +#include linux/module.h +#include linux/slab.h +#include linux/init.h +#include linux/fs.h +#include linux/delay.h +#include linux/poll.h +#include linux/wait.h +#include asm/uaccess.h +#include asm/io.h + +#include pcmcia/cs_types.h +#include pcmcia/cs.h +#include pcmcia/cistpl.h +#include
Re: [PATCH] Omnikey Cardman 4040 driver
On Monday 05 September 2005 22:05, Jesper Juhl wrote: > On Monday 05 September 2005 21:54, Harald Welte wrote: > > Hi! > > > > I've now incorporated all the suggested changes (thanks once again on > > the many comments received). The resulting driver has been tested and > > works fine. > > > > Please consider applying the driver to the mainline tree, thanks. > > > What did you diff this against? When I applied it to the 2.6.13 source I got > : > > patching file MAINTAINERS > Hunk #1 succeeded at 1730 (offset -7 lines). > patching file drivers/char/pcmcia/Kconfig > patching file drivers/char/pcmcia/Makefile > patching file drivers/char/pcmcia/cm4040_cs.c > patching file drivers/char/pcmcia/cm4040_cs.h > > Anyway, I did a small cleanup. > Removed all instances of trailing whitespace ( sed -r s/"[ \t]+$"/""/ ). > Removed some excessive (IMHO) use of blank lines. > A few CodingStyle related whitespace fixes (spaces after "," etc). > Removed some pointless casts. > > Hope this is useful to you (applies on top of the version you just posted). > Actually, forget the previous patch, I forgot a few things. Improved patch can be found below (a few extra cleanups of spacing and make the header look a little more pretty). /Jesper Juhl --- drivers/char/pcmcia/cm4040_cs.c.orig2005-09-05 21:39:05.0 +0200 +++ drivers/char/pcmcia/cm4040_cs.c 2005-09-05 22:20:38.0 +0200 @@ -1,4 +1,4 @@ - /* +/* * A driver for the Omnikey PCMCIA smartcard reader CardMan 4040 * * (c) 2000-2004 Omnikey AG (http://www.omnikey.com/) @@ -68,28 +68,25 @@ static char *version = static void reader_release(dev_link_t *link); static void reader_detach(dev_link_t *link); -static int major; +static int major; #defineBS_READABLE 0x01 #defineBS_WRITABLE 0x02 struct reader_dev { - dev_link_t link; - dev_node_t node; - wait_queue_head_t devq; - + dev_link_t link; + dev_node_t node; + wait_queue_head_t devq; wait_queue_head_t poll_wait; wait_queue_head_t read_wait; wait_queue_head_t write_wait; - unsigned intbuffer_status; - unsigned inttimer_expired; struct timer_list timer; unsigned long timeout; unsigned char s_buf[READ_WRITE_BUFFER_SIZE]; unsigned char r_buf[READ_WRITE_BUFFER_SIZE]; - struct task_struct *owner; + struct task_struct *owner; }; static dev_info_t dev_info = MODULE_NAME; @@ -104,7 +101,7 @@ static struct timer_list cm4040_poll_tim static inline void xoutb(unsigned char val, unsigned short port) { DEBUG(7, "outb(val=%.2x,port=%.4x)\n", val, port); - outb(val,port); + outb(val, port); } static inline unsigned char xinb(unsigned short port) @@ -122,7 +119,7 @@ static inline unsigned char xinb(unsigne static void cm4040_do_poll(unsigned long dummy) { unsigned int i; - /* walk through all devices */ + /* walk through all devices */ for (i = 0; dev_table[i]; i++) { dev_link_t *dl = dev_table[i]; struct reader_dev *dev = dl->priv; @@ -156,7 +153,7 @@ static int wait_for_bulk_out_ready(struc int i, rc; int iobase = dev->link.io.BasePort1; - for (i=0; i < POLL_LOOP_COUNT; i++) { + for (i = 0; i < POLL_LOOP_COUNT; i++) { if ((xinb(iobase + REG_OFFSET_BUFFER_STATUS) & BSR_BULK_OUT_FULL) == 0) { DEBUG(4, "BulkOut empty (i=%d)\n", i); @@ -191,7 +188,7 @@ static int write_sync_reg(unsigned char if (rc <= 0) return rc; - xoutb(val,iobase + REG_OFFSET_SYNC_CONTROL); + xoutb(val, iobase + REG_OFFSET_SYNC_CONTROL); rc = wait_for_bulk_out_ready(dev); if (rc <= 0) return rc; @@ -204,7 +201,7 @@ static int wait_for_bulk_in_ready(struct int i, rc; int iobase = dev->link.io.BasePort1; - for (i=0; i < POLL_LOOP_COUNT; i++) { + for (i = 0; i < POLL_LOOP_COUNT; i++) { if ((xinb(iobase + REG_OFFSET_BUFFER_STATUS) & BSR_BULK_IN_FULL) == BSR_BULK_IN_FULL) { DEBUG(3, "BulkIn full (i=%d)\n", i); @@ -215,7 +212,7 @@ static int wait_for_bulk_in_ready(struct DEBUG(4, "wait_event_interruptible_timeout(timeout=%ld\n", dev->timeout); rc = wait_event_interruptible_timeout(dev->read_wait, - test_and_clear_bit(BS_READABLE, + test_and_clear_bit(BS_READABLE, >buffer_status), dev->timeout); if (rc > 0) @@ -231,7
Re: [PATCH] Omnikey Cardman 4040 driver
On Monday 05 September 2005 21:54, Harald Welte wrote: > Hi! > > I've now incorporated all the suggested changes (thanks once again on > the many comments received). The resulting driver has been tested and > works fine. > > Please consider applying the driver to the mainline tree, thanks. > What did you diff this against? When I applied it to the 2.6.13 source I got : patching file MAINTAINERS Hunk #1 succeeded at 1730 (offset -7 lines). patching file drivers/char/pcmcia/Kconfig patching file drivers/char/pcmcia/Makefile patching file drivers/char/pcmcia/cm4040_cs.c patching file drivers/char/pcmcia/cm4040_cs.h Anyway, I did a small cleanup. Removed all instances of trailing whitespace ( sed -r s/"[ \t]+$"/""/ ). Removed some excessive (IMHO) use of blank lines. A few CodingStyle related whitespace fixes (spaces after "," etc). Removed some pointless casts. Hope this is useful to you (applies on top of the version you just posted). /Jesper Juhl --- drivers/char/pcmcia/cm4040_cs.c.orig2005-09-05 21:39:05.0 +0200 +++ drivers/char/pcmcia/cm4040_cs.c 2005-09-05 22:01:19.0 +0200 @@ -1,4 +1,4 @@ - /* +/* * A driver for the Omnikey PCMCIA smartcard reader CardMan 4040 * * (c) 2000-2004 Omnikey AG (http://www.omnikey.com/) @@ -68,28 +68,25 @@ static char *version = static void reader_release(dev_link_t *link); static void reader_detach(dev_link_t *link); -static int major; +static int major; #defineBS_READABLE 0x01 #defineBS_WRITABLE 0x02 struct reader_dev { - dev_link_t link; - dev_node_t node; - wait_queue_head_t devq; - + dev_link_t link; + dev_node_t node; + wait_queue_head_t devq; wait_queue_head_t poll_wait; wait_queue_head_t read_wait; wait_queue_head_t write_wait; - unsigned intbuffer_status; - unsigned inttimer_expired; struct timer_list timer; unsigned long timeout; unsigned char s_buf[READ_WRITE_BUFFER_SIZE]; unsigned char r_buf[READ_WRITE_BUFFER_SIZE]; - struct task_struct *owner; + struct task_struct *owner; }; static dev_info_t dev_info = MODULE_NAME; @@ -104,7 +101,7 @@ static struct timer_list cm4040_poll_tim static inline void xoutb(unsigned char val, unsigned short port) { DEBUG(7, "outb(val=%.2x,port=%.4x)\n", val, port); - outb(val,port); + outb(val, port); } static inline unsigned char xinb(unsigned short port) @@ -122,7 +119,7 @@ static inline unsigned char xinb(unsigne static void cm4040_do_poll(unsigned long dummy) { unsigned int i; - /* walk through all devices */ + /* walk through all devices */ for (i = 0; dev_table[i]; i++) { dev_link_t *dl = dev_table[i]; struct reader_dev *dev = dl->priv; @@ -156,7 +153,7 @@ static int wait_for_bulk_out_ready(struc int i, rc; int iobase = dev->link.io.BasePort1; - for (i=0; i < POLL_LOOP_COUNT; i++) { + for (i = 0; i < POLL_LOOP_COUNT; i++) { if ((xinb(iobase + REG_OFFSET_BUFFER_STATUS) & BSR_BULK_OUT_FULL) == 0) { DEBUG(4, "BulkOut empty (i=%d)\n", i); @@ -191,7 +188,7 @@ static int write_sync_reg(unsigned char if (rc <= 0) return rc; - xoutb(val,iobase + REG_OFFSET_SYNC_CONTROL); + xoutb(val, iobase + REG_OFFSET_SYNC_CONTROL); rc = wait_for_bulk_out_ready(dev); if (rc <= 0) return rc; @@ -204,7 +201,7 @@ static int wait_for_bulk_in_ready(struct int i, rc; int iobase = dev->link.io.BasePort1; - for (i=0; i < POLL_LOOP_COUNT; i++) { + for (i = 0; i < POLL_LOOP_COUNT; i++) { if ((xinb(iobase + REG_OFFSET_BUFFER_STATUS) & BSR_BULK_IN_FULL) == BSR_BULK_IN_FULL) { DEBUG(3, "BulkIn full (i=%d)\n", i); @@ -215,7 +212,7 @@ static int wait_for_bulk_in_ready(struct DEBUG(4, "wait_event_interruptible_timeout(timeout=%ld\n", dev->timeout); rc = wait_event_interruptible_timeout(dev->read_wait, - test_and_clear_bit(BS_READABLE, + test_and_clear_bit(BS_READABLE, >buffer_status), dev->timeout); if (rc > 0) @@ -231,7 +228,7 @@ static int wait_for_bulk_in_ready(struct static ssize_t cm4040_read(struct file *filp, char __user *buf, size_t count, loff_t *ppos) { - struct reader_dev *dev = (struct reader_dev *) filp->private_data; + struct reader_dev *dev =
Re: [PATCH] Omnikey Cardman 4040 driver
> +#include I don't think you need this one. > +#include you shouldn't need this one. > +static atomic_t cm4040_num_devices_open; > + > +#ifdef PCMCIA_DEBUG > +static int pc_debug = PCMCIA_DEBUG; > +module_param(pc_debug, int, 0600); > +#define DEBUG(n, x, args...) do { if (pc_debug >= (n)) > \ > + printk(KERN_DEBUG "%s:%s:" x, MODULE_NAME, \ > + __FUNCTION__, ##args); } while (0) > +#else > +#define DEBUG(n, args...) > +#endif What about just using pr_debug (or dev_dbg where you have a struct device handy) > +/* poll the device fifo status register. not to be confused with > + * the poll syscall. */ > +static void cm4040_do_poll(unsigned long dummy) > +{ > + unsigned int i; > + /* walk through all devices */ > + for (i = 0; dev_table[i]; i++) { Please make the poll timer per device. We generally try to avoid global state, and this allows to get rid of the opencount tracking aswell. > +static ssize_t cm4040_read(struct file *filp, char __user *buf, > + size_t count, loff_t *ppos) > +{ > + struct reader_dev *dev = (struct reader_dev *) filp->private_data; no need to case a void pointer. > + if (count < 10) > + return -EFAULT; > + > + if (filp->f_flags & O_NONBLOCK) { > + DEBUG(4, "filep->f_flags O_NONBLOCK set\n"); > + DEBUG(2, "<- cm4040_read (failure)\n"); > + return -EAGAIN; > + } this sounds rather pointless. letting an O_NONBLOCK open fail all the time doesn't sound like a good idea. > +static int cm4040_open(struct inode *inode, struct file *filp) > +{ > + struct reader_dev *dev; > + dev_link_t *link; > + int i; > + > + DEBUG(2, "-> cm4040_open(device=%d.%d process=%s,%d)\n", > + MAJOR(inode->i_rdev), MINOR(inode->i_rdev), > + current->comm, current->pid); > + > + i = MINOR(inode->i_rdev); please use iminor. > + if (filp->f_flags & O_NONBLOCK) { > + DEBUG(4, "filep->f_flags O_NONBLOCK set\n"); > + DEBUG(4, "<- cm4040_open (failure)\n"); > + return -EAGAIN; > + } given that you fail O_NONLOCK in open already the code above makes even less sense. > + > + dev->owner = current; this doesn't make a lot of sense and seems to be only used in debug code, I'd suggest killing it. > +static int cm4040_close(struct inode *inode,struct file *filp) > +{ > + struct reader_dev *dev; > + dev_link_t *link; > + int i; > + > + DEBUG(2, "-> cm4040_close(maj/min=%d.%d)\n", > + MAJOR(inode->i_rdev), MINOR(inode->i_rdev)); > + > + i = MINOR(inode->i_rdev); > + if (i >= CM_MAX_DEV) > + return -ENODEV; > + > + link = dev_table[MINOR(inode->i_rdev)]; > + if (link == NULL) > + return -ENODEV; > + > + dev = (struct reader_dev *) link->priv; you should be able to use file->private_data here. > + case CS_EVENT_CARD_REMOVAL: > + DEBUG(5, "CS_EVENT_CARD_REMOVAL\n"); > + link->state &= ~DEV_PRESENT; > + break; > + case CS_EVENT_PM_SUSPEND: > + DEBUG(5, "CS_EVENT_PM_SUSPEND " > + "(fall-through to CS_EVENT_RESET_PHYSICAL)\n"); > + link->state |= DEV_SUSPEND; > + > + case CS_EVENT_RESET_PHYSICAL: > + DEBUG(5, "CS_EVENT_RESET_PHYSICAL\n"); > + if (link->state & DEV_CONFIG) { > + DEBUG(5, "ReleaseConfiguration\n"); > + pcmcia_release_configuration(link->handle); > + } > + break; > + case CS_EVENT_PM_RESUME: > + DEBUG(5, "CS_EVENT_PM_RESUME " > + "(fall-through to CS_EVENT_CARD_RESET)\n"); > + link->state &= ~DEV_SUSPEND; I think these events became methods of their own recently, not sure if it hit -mm or mainline yet. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Omnikey Cardman 4040 driver
Hi! I've now incorporated all the suggested changes (thanks once again on the many comments received). The resulting driver has been tested and works fine. Please consider applying the driver to the mainline tree, thanks. Add new Omnikey Cardman 4040 smartcard reader driver Signed-off-by: Harald Welte <[EMAIL PROTECTED]> --- commit 1c1cd1c5aba9ae1c0ea32d55c5b25f2370aaeca4 tree 016dec439275ab425575901dca5ee261bbc0aa0f parent c4ab879b6ef599bf88d19b9b145878ef73400ce7 author Harald Welte <[EMAIL PROTECTED]> Mo, 05 Sep 2005 21:47:01 +0200 committer Harald Welte <[EMAIL PROTECTED]> Mo, 05 Sep 2005 21:47:01 +0200 MAINTAINERS |5 drivers/char/pcmcia/Kconfig | 13 + drivers/char/pcmcia/Makefile|1 drivers/char/pcmcia/cm4040_cs.c | 897 +++ drivers/char/pcmcia/cm4040_cs.h | 47 ++ 5 files changed, 963 insertions(+), 0 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1737,6 +1737,11 @@ L: [EMAIL PROTECTED] W: http://www.linuxtr.net S: Maintained +OMNIKEY CARDMAN 4040 DRIVER +P: Harald Welte +M: [EMAIL PROTECTED] +S: Maintained + ONSTREAM SCSI TAPE DRIVER P: Willem Riede M: [EMAIL PROTECTED] diff --git a/drivers/char/pcmcia/Kconfig b/drivers/char/pcmcia/Kconfig --- a/drivers/char/pcmcia/Kconfig +++ b/drivers/char/pcmcia/Kconfig @@ -18,5 +18,18 @@ config SYNCLINK_CS The module will be called synclinkmp. If you want to do that, say M here. +config CARDMAN_4040 + tristate "Omnikey CardMan 4040 support" + depends on PCMCIA + help + Enable support for the Omnikey CardMan 4040 PCMCIA Smartcard + reader. + + This card is basically a USB CCID device connected to a FIFO + in I/O space. To use the kernel driver, you will need either the + PC/SC ifdhandler provided from the Omnikey homepage + (http://www.omnikey.com/), or a current development version of OpenCT + (http://www.opensc.org/). + endmenu diff --git a/drivers/char/pcmcia/Makefile b/drivers/char/pcmcia/Makefile --- a/drivers/char/pcmcia/Makefile +++ b/drivers/char/pcmcia/Makefile @@ -5,3 +5,4 @@ # obj-$(CONFIG_SYNCLINK_CS) += synclink_cs.o +obj-$(CONFIG_CARDMAN_4040) += cm4040_cs.o diff --git a/drivers/char/pcmcia/cm4040_cs.c b/drivers/char/pcmcia/cm4040_cs.c new file mode 100644 --- /dev/null +++ b/drivers/char/pcmcia/cm4040_cs.c @@ -0,0 +1,897 @@ + /* + * A driver for the Omnikey PCMCIA smartcard reader CardMan 4040 + * + * (c) 2000-2004 Omnikey AG (http://www.omnikey.com/) + * + * (C) 2005 Harald Welte <[EMAIL PROTECTED]> + * - add support for poll() + * - driver cleanup + * - add waitqueues + * - adhere to linux kernel coding style and policies + * - support 2.6.13 "new style" pcmcia interface + * + * The device basically is a USB CCID compliant device that has been + * attached to an I/O-Mapped FIFO. + * + * All rights reserved, Dual BSD/GPL Licensed. + */ + +/* #define PCMCIA_DEBUG 6 */ + +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +#include "cm4040_cs.h" + +static atomic_t cm4040_num_devices_open; + +#ifdef PCMCIA_DEBUG +static int pc_debug = PCMCIA_DEBUG; +module_param(pc_debug, int, 0600); +#define DEBUG(n, x, args...) do { if (pc_debug >= (n)) \ + printk(KERN_DEBUG "%s:%s:" x, MODULE_NAME, \ +__FUNCTION__, ##args); } while (0) +#else +#define DEBUG(n, args...) +#endif + +static char *version = +"OMNIKEY CardMan 4040 v1.1.0gm3 - All bugs added by Harald Welte"; + +#defineCCID_DRIVER_BULK_DEFAULT_TIMEOUT(150*HZ) +#defineCCID_DRIVER_ASYNC_POWERUP_TIMEOUT (35*HZ) +#defineCCID_DRIVER_MINIMUM_TIMEOUT (3*HZ) +#define READ_WRITE_BUFFER_SIZE 512 +#define POLL_LOOP_COUNT1000 + +/* how often to poll for fifo status change */ +#define POLL_PERIODmsecs_to_jiffies(10) + +static void reader_release(dev_link_t *link); +static void reader_detach(dev_link_t *link); + +static int major; + +#defineBS_READABLE 0x01 +#defineBS_WRITABLE 0x02 + +struct reader_dev { + dev_link_t link; + dev_node_t node; + wait_queue_head_t devq; + + wait_queue_head_t poll_wait; + wait_queue_head_t read_wait; + wait_queue_head_t write_wait; + + unsigned intbuffer_status; + + unsigned inttimer_expired; + struct timer_list timer; + unsigned long
[PATCH] Omnikey Cardman 4040 driver
Hi! I've now incorporated all the suggested changes (thanks once again on the many comments received). The resulting driver has been tested and works fine. Please consider applying the driver to the mainline tree, thanks. Add new Omnikey Cardman 4040 smartcard reader driver Signed-off-by: Harald Welte [EMAIL PROTECTED] --- commit 1c1cd1c5aba9ae1c0ea32d55c5b25f2370aaeca4 tree 016dec439275ab425575901dca5ee261bbc0aa0f parent c4ab879b6ef599bf88d19b9b145878ef73400ce7 author Harald Welte [EMAIL PROTECTED] Mo, 05 Sep 2005 21:47:01 +0200 committer Harald Welte [EMAIL PROTECTED] Mo, 05 Sep 2005 21:47:01 +0200 MAINTAINERS |5 drivers/char/pcmcia/Kconfig | 13 + drivers/char/pcmcia/Makefile|1 drivers/char/pcmcia/cm4040_cs.c | 897 +++ drivers/char/pcmcia/cm4040_cs.h | 47 ++ 5 files changed, 963 insertions(+), 0 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1737,6 +1737,11 @@ L: [EMAIL PROTECTED] W: http://www.linuxtr.net S: Maintained +OMNIKEY CARDMAN 4040 DRIVER +P: Harald Welte +M: [EMAIL PROTECTED] +S: Maintained + ONSTREAM SCSI TAPE DRIVER P: Willem Riede M: [EMAIL PROTECTED] diff --git a/drivers/char/pcmcia/Kconfig b/drivers/char/pcmcia/Kconfig --- a/drivers/char/pcmcia/Kconfig +++ b/drivers/char/pcmcia/Kconfig @@ -18,5 +18,18 @@ config SYNCLINK_CS The module will be called synclinkmp. If you want to do that, say M here. +config CARDMAN_4040 + tristate Omnikey CardMan 4040 support + depends on PCMCIA + help + Enable support for the Omnikey CardMan 4040 PCMCIA Smartcard + reader. + + This card is basically a USB CCID device connected to a FIFO + in I/O space. To use the kernel driver, you will need either the + PC/SC ifdhandler provided from the Omnikey homepage + (http://www.omnikey.com/), or a current development version of OpenCT + (http://www.opensc.org/). + endmenu diff --git a/drivers/char/pcmcia/Makefile b/drivers/char/pcmcia/Makefile --- a/drivers/char/pcmcia/Makefile +++ b/drivers/char/pcmcia/Makefile @@ -5,3 +5,4 @@ # obj-$(CONFIG_SYNCLINK_CS) += synclink_cs.o +obj-$(CONFIG_CARDMAN_4040) += cm4040_cs.o diff --git a/drivers/char/pcmcia/cm4040_cs.c b/drivers/char/pcmcia/cm4040_cs.c new file mode 100644 --- /dev/null +++ b/drivers/char/pcmcia/cm4040_cs.c @@ -0,0 +1,897 @@ + /* + * A driver for the Omnikey PCMCIA smartcard reader CardMan 4040 + * + * (c) 2000-2004 Omnikey AG (http://www.omnikey.com/) + * + * (C) 2005 Harald Welte [EMAIL PROTECTED] + * - add support for poll() + * - driver cleanup + * - add waitqueues + * - adhere to linux kernel coding style and policies + * - support 2.6.13 new style pcmcia interface + * + * The device basically is a USB CCID compliant device that has been + * attached to an I/O-Mapped FIFO. + * + * All rights reserved, Dual BSD/GPL Licensed. + */ + +/* #define PCMCIA_DEBUG 6 */ + +#include linux/version.h + +#include linux/kernel.h +#include linux/module.h +#include linux/slab.h +#include linux/init.h +#include linux/fs.h +#include linux/delay.h +#include linux/poll.h +#include linux/wait.h +#include asm/uaccess.h +#include asm/io.h + +#include pcmcia/version.h +#include pcmcia/cs_types.h +#include pcmcia/cs.h +#include pcmcia/cistpl.h +#include pcmcia/cisreg.h +#include pcmcia/ciscode.h +#include pcmcia/ds.h + +#include cm4040_cs.h + +static atomic_t cm4040_num_devices_open; + +#ifdef PCMCIA_DEBUG +static int pc_debug = PCMCIA_DEBUG; +module_param(pc_debug, int, 0600); +#define DEBUG(n, x, args...) do { if (pc_debug = (n)) \ + printk(KERN_DEBUG %s:%s: x, MODULE_NAME, \ +__FUNCTION__, ##args); } while (0) +#else +#define DEBUG(n, args...) +#endif + +static char *version = +OMNIKEY CardMan 4040 v1.1.0gm3 - All bugs added by Harald Welte; + +#defineCCID_DRIVER_BULK_DEFAULT_TIMEOUT(150*HZ) +#defineCCID_DRIVER_ASYNC_POWERUP_TIMEOUT (35*HZ) +#defineCCID_DRIVER_MINIMUM_TIMEOUT (3*HZ) +#define READ_WRITE_BUFFER_SIZE 512 +#define POLL_LOOP_COUNT1000 + +/* how often to poll for fifo status change */ +#define POLL_PERIODmsecs_to_jiffies(10) + +static void reader_release(dev_link_t *link); +static void reader_detach(dev_link_t *link); + +static int major; + +#defineBS_READABLE 0x01 +#defineBS_WRITABLE 0x02 + +struct reader_dev { + dev_link_t link; + dev_node_t node; + wait_queue_head_t devq; + + wait_queue_head_t poll_wait; + wait_queue_head_t read_wait;
Re: [PATCH] Omnikey Cardman 4040 driver
+#include linux/version.h I don't think you need this one. +#include pcmcia/version.h you shouldn't need this one. +static atomic_t cm4040_num_devices_open; + +#ifdef PCMCIA_DEBUG +static int pc_debug = PCMCIA_DEBUG; +module_param(pc_debug, int, 0600); +#define DEBUG(n, x, args...) do { if (pc_debug = (n)) \ + printk(KERN_DEBUG %s:%s: x, MODULE_NAME, \ + __FUNCTION__, ##args); } while (0) +#else +#define DEBUG(n, args...) +#endif What about just using pr_debug (or dev_dbg where you have a struct device handy) +/* poll the device fifo status register. not to be confused with + * the poll syscall. */ +static void cm4040_do_poll(unsigned long dummy) +{ + unsigned int i; + /* walk through all devices */ + for (i = 0; dev_table[i]; i++) { Please make the poll timer per device. We generally try to avoid global state, and this allows to get rid of the opencount tracking aswell. +static ssize_t cm4040_read(struct file *filp, char __user *buf, + size_t count, loff_t *ppos) +{ + struct reader_dev *dev = (struct reader_dev *) filp-private_data; no need to case a void pointer. + if (count 10) + return -EFAULT; + + if (filp-f_flags O_NONBLOCK) { + DEBUG(4, filep-f_flags O_NONBLOCK set\n); + DEBUG(2, - cm4040_read (failure)\n); + return -EAGAIN; + } this sounds rather pointless. letting an O_NONBLOCK open fail all the time doesn't sound like a good idea. +static int cm4040_open(struct inode *inode, struct file *filp) +{ + struct reader_dev *dev; + dev_link_t *link; + int i; + + DEBUG(2, - cm4040_open(device=%d.%d process=%s,%d)\n, + MAJOR(inode-i_rdev), MINOR(inode-i_rdev), + current-comm, current-pid); + + i = MINOR(inode-i_rdev); please use iminor. + if (filp-f_flags O_NONBLOCK) { + DEBUG(4, filep-f_flags O_NONBLOCK set\n); + DEBUG(4, - cm4040_open (failure)\n); + return -EAGAIN; + } given that you fail O_NONLOCK in open already the code above makes even less sense. + + dev-owner = current; this doesn't make a lot of sense and seems to be only used in debug code, I'd suggest killing it. +static int cm4040_close(struct inode *inode,struct file *filp) +{ + struct reader_dev *dev; + dev_link_t *link; + int i; + + DEBUG(2, - cm4040_close(maj/min=%d.%d)\n, + MAJOR(inode-i_rdev), MINOR(inode-i_rdev)); + + i = MINOR(inode-i_rdev); + if (i = CM_MAX_DEV) + return -ENODEV; + + link = dev_table[MINOR(inode-i_rdev)]; + if (link == NULL) + return -ENODEV; + + dev = (struct reader_dev *) link-priv; you should be able to use file-private_data here. + case CS_EVENT_CARD_REMOVAL: + DEBUG(5, CS_EVENT_CARD_REMOVAL\n); + link-state = ~DEV_PRESENT; + break; + case CS_EVENT_PM_SUSPEND: + DEBUG(5, CS_EVENT_PM_SUSPEND + (fall-through to CS_EVENT_RESET_PHYSICAL)\n); + link-state |= DEV_SUSPEND; + + case CS_EVENT_RESET_PHYSICAL: + DEBUG(5, CS_EVENT_RESET_PHYSICAL\n); + if (link-state DEV_CONFIG) { + DEBUG(5, ReleaseConfiguration\n); + pcmcia_release_configuration(link-handle); + } + break; + case CS_EVENT_PM_RESUME: + DEBUG(5, CS_EVENT_PM_RESUME + (fall-through to CS_EVENT_CARD_RESET)\n); + link-state = ~DEV_SUSPEND; I think these events became methods of their own recently, not sure if it hit -mm or mainline yet. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Omnikey Cardman 4040 driver
On Monday 05 September 2005 21:54, Harald Welte wrote: Hi! I've now incorporated all the suggested changes (thanks once again on the many comments received). The resulting driver has been tested and works fine. Please consider applying the driver to the mainline tree, thanks. What did you diff this against? When I applied it to the 2.6.13 source I got : patching file MAINTAINERS Hunk #1 succeeded at 1730 (offset -7 lines). patching file drivers/char/pcmcia/Kconfig patching file drivers/char/pcmcia/Makefile patching file drivers/char/pcmcia/cm4040_cs.c patching file drivers/char/pcmcia/cm4040_cs.h Anyway, I did a small cleanup. Removed all instances of trailing whitespace ( sed -r s/[ \t]+$// ). Removed some excessive (IMHO) use of blank lines. A few CodingStyle related whitespace fixes (spaces after , etc). Removed some pointless casts. Hope this is useful to you (applies on top of the version you just posted). /Jesper Juhl --- drivers/char/pcmcia/cm4040_cs.c.orig2005-09-05 21:39:05.0 +0200 +++ drivers/char/pcmcia/cm4040_cs.c 2005-09-05 22:01:19.0 +0200 @@ -1,4 +1,4 @@ - /* +/* * A driver for the Omnikey PCMCIA smartcard reader CardMan 4040 * * (c) 2000-2004 Omnikey AG (http://www.omnikey.com/) @@ -68,28 +68,25 @@ static char *version = static void reader_release(dev_link_t *link); static void reader_detach(dev_link_t *link); -static int major; +static int major; #defineBS_READABLE 0x01 #defineBS_WRITABLE 0x02 struct reader_dev { - dev_link_t link; - dev_node_t node; - wait_queue_head_t devq; - + dev_link_t link; + dev_node_t node; + wait_queue_head_t devq; wait_queue_head_t poll_wait; wait_queue_head_t read_wait; wait_queue_head_t write_wait; - unsigned intbuffer_status; - unsigned inttimer_expired; struct timer_list timer; unsigned long timeout; unsigned char s_buf[READ_WRITE_BUFFER_SIZE]; unsigned char r_buf[READ_WRITE_BUFFER_SIZE]; - struct task_struct *owner; + struct task_struct *owner; }; static dev_info_t dev_info = MODULE_NAME; @@ -104,7 +101,7 @@ static struct timer_list cm4040_poll_tim static inline void xoutb(unsigned char val, unsigned short port) { DEBUG(7, outb(val=%.2x,port=%.4x)\n, val, port); - outb(val,port); + outb(val, port); } static inline unsigned char xinb(unsigned short port) @@ -122,7 +119,7 @@ static inline unsigned char xinb(unsigne static void cm4040_do_poll(unsigned long dummy) { unsigned int i; - /* walk through all devices */ + /* walk through all devices */ for (i = 0; dev_table[i]; i++) { dev_link_t *dl = dev_table[i]; struct reader_dev *dev = dl-priv; @@ -156,7 +153,7 @@ static int wait_for_bulk_out_ready(struc int i, rc; int iobase = dev-link.io.BasePort1; - for (i=0; i POLL_LOOP_COUNT; i++) { + for (i = 0; i POLL_LOOP_COUNT; i++) { if ((xinb(iobase + REG_OFFSET_BUFFER_STATUS) BSR_BULK_OUT_FULL) == 0) { DEBUG(4, BulkOut empty (i=%d)\n, i); @@ -191,7 +188,7 @@ static int write_sync_reg(unsigned char if (rc = 0) return rc; - xoutb(val,iobase + REG_OFFSET_SYNC_CONTROL); + xoutb(val, iobase + REG_OFFSET_SYNC_CONTROL); rc = wait_for_bulk_out_ready(dev); if (rc = 0) return rc; @@ -204,7 +201,7 @@ static int wait_for_bulk_in_ready(struct int i, rc; int iobase = dev-link.io.BasePort1; - for (i=0; i POLL_LOOP_COUNT; i++) { + for (i = 0; i POLL_LOOP_COUNT; i++) { if ((xinb(iobase + REG_OFFSET_BUFFER_STATUS) BSR_BULK_IN_FULL) == BSR_BULK_IN_FULL) { DEBUG(3, BulkIn full (i=%d)\n, i); @@ -215,7 +212,7 @@ static int wait_for_bulk_in_ready(struct DEBUG(4, wait_event_interruptible_timeout(timeout=%ld\n, dev-timeout); rc = wait_event_interruptible_timeout(dev-read_wait, - test_and_clear_bit(BS_READABLE, + test_and_clear_bit(BS_READABLE, dev-buffer_status), dev-timeout); if (rc 0) @@ -231,7 +228,7 @@ static int wait_for_bulk_in_ready(struct static ssize_t cm4040_read(struct file *filp, char __user *buf, size_t count, loff_t *ppos) { - struct reader_dev *dev = (struct reader_dev *) filp-private_data; + struct reader_dev *dev = filp-private_data; int iobase =
Re: [PATCH] Omnikey Cardman 4040 driver
On Monday 05 September 2005 22:05, Jesper Juhl wrote: On Monday 05 September 2005 21:54, Harald Welte wrote: Hi! I've now incorporated all the suggested changes (thanks once again on the many comments received). The resulting driver has been tested and works fine. Please consider applying the driver to the mainline tree, thanks. What did you diff this against? When I applied it to the 2.6.13 source I got : patching file MAINTAINERS Hunk #1 succeeded at 1730 (offset -7 lines). patching file drivers/char/pcmcia/Kconfig patching file drivers/char/pcmcia/Makefile patching file drivers/char/pcmcia/cm4040_cs.c patching file drivers/char/pcmcia/cm4040_cs.h Anyway, I did a small cleanup. Removed all instances of trailing whitespace ( sed -r s/[ \t]+$// ). Removed some excessive (IMHO) use of blank lines. A few CodingStyle related whitespace fixes (spaces after , etc). Removed some pointless casts. Hope this is useful to you (applies on top of the version you just posted). Actually, forget the previous patch, I forgot a few things. Improved patch can be found below (a few extra cleanups of spacing and make the header look a little more pretty). /Jesper Juhl --- drivers/char/pcmcia/cm4040_cs.c.orig2005-09-05 21:39:05.0 +0200 +++ drivers/char/pcmcia/cm4040_cs.c 2005-09-05 22:20:38.0 +0200 @@ -1,4 +1,4 @@ - /* +/* * A driver for the Omnikey PCMCIA smartcard reader CardMan 4040 * * (c) 2000-2004 Omnikey AG (http://www.omnikey.com/) @@ -68,28 +68,25 @@ static char *version = static void reader_release(dev_link_t *link); static void reader_detach(dev_link_t *link); -static int major; +static int major; #defineBS_READABLE 0x01 #defineBS_WRITABLE 0x02 struct reader_dev { - dev_link_t link; - dev_node_t node; - wait_queue_head_t devq; - + dev_link_t link; + dev_node_t node; + wait_queue_head_t devq; wait_queue_head_t poll_wait; wait_queue_head_t read_wait; wait_queue_head_t write_wait; - unsigned intbuffer_status; - unsigned inttimer_expired; struct timer_list timer; unsigned long timeout; unsigned char s_buf[READ_WRITE_BUFFER_SIZE]; unsigned char r_buf[READ_WRITE_BUFFER_SIZE]; - struct task_struct *owner; + struct task_struct *owner; }; static dev_info_t dev_info = MODULE_NAME; @@ -104,7 +101,7 @@ static struct timer_list cm4040_poll_tim static inline void xoutb(unsigned char val, unsigned short port) { DEBUG(7, outb(val=%.2x,port=%.4x)\n, val, port); - outb(val,port); + outb(val, port); } static inline unsigned char xinb(unsigned short port) @@ -122,7 +119,7 @@ static inline unsigned char xinb(unsigne static void cm4040_do_poll(unsigned long dummy) { unsigned int i; - /* walk through all devices */ + /* walk through all devices */ for (i = 0; dev_table[i]; i++) { dev_link_t *dl = dev_table[i]; struct reader_dev *dev = dl-priv; @@ -156,7 +153,7 @@ static int wait_for_bulk_out_ready(struc int i, rc; int iobase = dev-link.io.BasePort1; - for (i=0; i POLL_LOOP_COUNT; i++) { + for (i = 0; i POLL_LOOP_COUNT; i++) { if ((xinb(iobase + REG_OFFSET_BUFFER_STATUS) BSR_BULK_OUT_FULL) == 0) { DEBUG(4, BulkOut empty (i=%d)\n, i); @@ -191,7 +188,7 @@ static int write_sync_reg(unsigned char if (rc = 0) return rc; - xoutb(val,iobase + REG_OFFSET_SYNC_CONTROL); + xoutb(val, iobase + REG_OFFSET_SYNC_CONTROL); rc = wait_for_bulk_out_ready(dev); if (rc = 0) return rc; @@ -204,7 +201,7 @@ static int wait_for_bulk_in_ready(struct int i, rc; int iobase = dev-link.io.BasePort1; - for (i=0; i POLL_LOOP_COUNT; i++) { + for (i = 0; i POLL_LOOP_COUNT; i++) { if ((xinb(iobase + REG_OFFSET_BUFFER_STATUS) BSR_BULK_IN_FULL) == BSR_BULK_IN_FULL) { DEBUG(3, BulkIn full (i=%d)\n, i); @@ -215,7 +212,7 @@ static int wait_for_bulk_in_ready(struct DEBUG(4, wait_event_interruptible_timeout(timeout=%ld\n, dev-timeout); rc = wait_event_interruptible_timeout(dev-read_wait, - test_and_clear_bit(BS_READABLE, + test_and_clear_bit(BS_READABLE, dev-buffer_status), dev-timeout); if (rc 0) @@ -231,7 +228,7 @@ static int wait_for_bulk_in_ready(struct static