Re: [PATCH] Omnikey Cardman 4040 driver (UPDATE)

2005-09-06 Thread Harald Welte
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)

2005-09-06 Thread Harald Welte
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

2005-09-05 Thread Jesper Juhl
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

2005-09-05 Thread Jesper Juhl
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

2005-09-05 Thread Christoph Hellwig
> +#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

2005-09-05 Thread Harald Welte
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

2005-09-05 Thread Harald Welte
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

2005-09-05 Thread Christoph Hellwig
 +#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

2005-09-05 Thread Jesper Juhl
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

2005-09-05 Thread Jesper Juhl
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