Re: [PATCH] New: Omnikey CardMan 4040 PCMCIA Driver

Re: [PATCH] New: Omnikey CardMan 4040 PCMCIA Driver

[Chase Venters]

> Below you can find a driver for the Omnikey CardMan 4040 PCMCIA
> Smartcard Reader.

Someone correct me if I'm wrong, but wouldn't these #defines be a problem with 
the new HZ flexibility:

#define	CCID_DRIVER_BULK_DEFAULT_TIMEOUT  	(150*HZ)
#define	CCID_DRIVER_ASYNC_POWERUP_TIMEOUT 	(35*HZ)
#define	CCID_DRIVER_MINIMUM_TIMEOUT 		(3*HZ)
#define READ_WRITE_BUFFER_SIZE 512
#define POLL_LOOP_COUNT				1000

/* how often to poll for fifo status change */
#define POLL_PERIOD 				(HZ/100)

In particular, 2.6.13 allows a HZ of 100, which would define POLL_PERIOD to 0. 
Your later calls to mod_timer would be setting cmx_poll_timer to the current 
value of jiffies. 

Also, you've got a typo in the comments:

* 	- adhere to linux kenrel coding style and policies

Forgive me if I'm way off - I'm just now getting my feet wet in kernel 
development. Just making comments based on what I (think) I know at this 
point.

Best Regards,
Chase Venters

Re: [PATCH] New: Omnikey CardMan 4040 PCMCIA Driver

[Alexey Dobriyan]

On Sun, Sep 04, 2005 at 12:12:18PM +0200, Harald Welte wrote:
> Below you can find a driver for the Omnikey CardMan 4040 PCMCIA
> Smartcard Reader.  

> --- /dev/null
> +++ b/drivers/char/pcmcia/cm4040_cs.c

> +#include <linux/config.h>

Not needed.

> +static volatile char *version =

Can we lose all volatile and register keywords?

> +typedef struct reader_dev_t {
> +	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 int 	  	buffer_status;
> +                                
> +	unsigned int      	fTimerExpired;
> +	struct timer_list	timer;
> +	unsigned long     	timeout;
> +	unsigned char     	sBuf[READ_WRITE_BUFFER_SIZE];
> +	unsigned char     	rBuf[READ_WRITE_BUFFER_SIZE];
> +	struct task_struct 	*owner;	
> +} reader_dev_t;

And typedefs too.

	struct reader_dev {
	
	};

> +static ssize_t cmx_read(struct file *filp,char *buf,size_t count,loff_t *ppos)

char __user *buf

> +	ulBytesToRead = 5 + 
> +			(0x000000FF&((char)dev->rBuf[1])) + 
> +			(0x0000FF00&((char)dev->rBuf[2] << 8)) + 
> +			(0x00FF0000&((char)dev->rBuf[3] << 16)) + 
> +			(0xFF000000&((char)dev->rBuf[4] << 24));

	ulBytesToRead = 5 + le32_to_cpu(*(__le32 *)&dev->rBuf[1]);

> +	ulMin = (count < (ulBytesToRead+5))?count:(ulBytesToRead+5);

	ulMin = min(count, ulBytesToRead + 5);

> +	copy_to_user(buf, dev->rBuf, ulMin);

Can fail.

> +static ssize_t cmx_write(struct file *filp,const char *buf,size_t count,

const char __user *buf

> +			 loff_t *ppos)

> +	copy_from_user(dev->sBuf, buf, uiBytesToWrite);

Can fail.

> +static int cmx_ioctl(struct inode *inode,struct file *filp,unsigned int cmd,
> +			unsigned long arg)
> +{
> +	dev_link_t *link;
> +	int rc, size;
> +
> +	link=dev_table[MINOR(inode->i_rdev)];
> +	if (!(DEV_OK(link))) {
> +		DEBUG(4, "DEV_OK false\n");
> +		return -ENODEV;
> +	}
> +	if (_IOC_TYPE(cmd)!=CM_IOC_MAGIC) {
> +		DEBUG(4,"ioctype mismatch\n");
> +		return -EINVAL;
> +	}
> +	if (_IOC_NR(cmd)>CM_IOC_MAXNR) {
> +		DEBUG(4,"iocnr mismatch\n");
> +		return -EINVAL;
> +	} 
> +	size = _IOC_SIZE(cmd);
> +	rc = 0;
> +	DEBUG(4,"iocdir=%.4x iocr=%.4x iocw=%.4x iocsize=%d cmd=%.4x\n",
> +		_IOC_DIR(cmd),_IOC_READ,_IOC_WRITE,size,cmd);
> +
> +	if (_IOC_DIR(cmd)&_IOC_READ) {
> +		if (!access_ok(VERIFY_WRITE, (void *)arg, size))
> +			return -EFAULT;
> +	}
> +	if (_IOC_DIR(cmd)&_IOC_WRITE) {
> +		if (!access_ok(VERIFY_READ, (void *)arg, size))
> +			return -EFAULT;
> +	}
> +
> +	return rc;
> +}

Whoo, empty ioctl handler.

> +static void reader_release(u_long arg)

> +	link = (dev_link_t *)arg;

You do

	reader_release((unsigned long)link);

somewhere above and below.

> +static void reader_detach_by_devno(int devno,dev_link_t *link)

> +		reader_release((u_long)link);

Like this.

Re: [PATCH] New: Omnikey CardMan 4040 PCMCIA Driver

[Nish Aravamudan]

On 9/3/05, Chase Venters <chase.venters@clientec.com> wrote:
> > Below you can find a driver for the Omnikey CardMan 4040 PCMCIA
> > Smartcard Reader.
> 
> Someone correct me if I'm wrong, but wouldn't these #defines be a problem 
> with the new HZ flexibility:
> 
> #define CCID_DRIVER_BULK_DEFAULT_TIMEOUT        (150*HZ)
> #define CCID_DRIVER_ASYNC_POWERUP_TIMEOUT       (35*HZ)
> #define CCID_DRIVER_MINIMUM_TIMEOUT             (3*HZ)
> #define READ_WRITE_BUFFER_SIZE 512
> #define POLL_LOOP_COUNT                         1000

These are all fine. Although I am a bit suspicious of 150 second
timeouts; but if that is the hardware...

> /* how often to poll for fifo status change */
> #define POLL_PERIOD                             (HZ/100)

This needs to be msecs_to_jiffies(10), please.

> In particular, 2.6.13 allows a HZ of 100, which would define POLL_PERIOD to 0.

Um, 100/100 = 1, not 0?

> Your later calls to mod_timer would be setting cmx_poll_timer to the current
> value of jiffies.

Which is technically ok, because HZ=100, a
     jiffies + 0
or
     jiffies + 1
timeout request will both result in the soft-timer being expired at
the *next* timer interrupt. Regardless, you're right, and
msecs_to_jiffies() will cover it.

> Also, you've got a typo in the comments:
> 
> *       - adhere to linux kenrel coding style and policies
> 
> Forgive me if I'm way off - I'm just now getting my feet wet in kernel
> development. Just making comments based on what I (think) I know at this
> point.

Of bigger concern to me is the use of the sleep_on() family of
functions, all of which are deprecated.

Thanks,
Nish

Re: [PATCH] New: Omnikey CardMan 4040 PCMCIA Driver

[Chase Venters]

> Um, 100/100 = 1, not 0?

Oh my... it's been a long day. 

Regards,
Chase Venters

Re: [PATCH] New: Omnikey CardMan 4040 PCMCIA Driver

[Jesper Juhl]

On 9/4/05, Harald Welte <laforge@gnumonks.org> wrote:
> On Sun, Sep 04, 2005 at 12:12:18PM +0200, Harald Welte wrote:
> > Hi!
> >
> > Below you can find a driver for the Omnikey CardMan 4040 PCMCIA
> > Smartcard Reader.
> 
> Sorry, the patch was missing a "cg-add" of the header file.  Please use
> the patch below.

It would be so much nicer if the patch actually was "below" - that is
"inline in the email as opposed to as an attachment". Having to first
save an attachment and then cut'n'paste from it is a pain.

Anyway, a few comments below :

+#define DEBUG(n, x, args...) do { if (pc_debug >= (n)) 			    \


line longer than 80 chars. Please adhere to CodingStyle and keep lines
<80 chars.
There's more than one occourance of this.

+static inline int cmx_waitForBulkOutReady(reader_dev_t *dev)


Why TheStudlyCaps ?  Please keep function names lowercase. There are
more instances of this, only pointing out one.

+        register int i;

+	register int iobase = dev->link.io.BasePort1;


Please use only tabs for indentation (line 1 of the above is indented
with spaces).

+	for (i=0; i < POLL_LOOP_COUNT; i++) {


for (i = 0; i < POLL_LOOP_COUNT; i++) {

+        if (rc != 1)


Again spaces used for indentation, please fix all that up to use tabs.

+	unsigned long ulBytesToRead;


lowercase prefered also for variables.

+	for (i=0; i<5; i++) {


for (i = 0; i < 5; i++) {

+			DEBUG(5,"cmx_waitForBulkInReady rc=%.2x\n",rc);


Space after ","s please : DEBUG(5, "cmx_waitForBulkInReady rc=%.2x\n", rc);

+	ulMin = (count < (ulBytesToRead+5))?count:(ulBytesToRead+5);


needs spaces : 
ulMin = (count < (ulBytesToRead + 5)) ? count : (ulBytesToRead + 5);

+	reader_dev_t *dev=(reader_dev_t *)filp->private_data;


reader_dev_t *dev = (reader_dev_t *)filp->private_data;


+static int cmx_open (struct inode *inode, struct file *filp)


get rid of the space before the opening paren : 
static int cmx_open(struct inode *inode, struct file *filp)


+	for (rc = pcmcia_get_first_tuple(handle, &tuple);

+	     rc == CS_SUCCESS;

+	     rc = pcmcia_get_next_tuple(handle, &tuple)) {

...
+		if (parse.cftable_entry.io.nwin) {

+			link->io.BasePort1 = parse.cftable_entry.io.win[0].base;

+			link->io.NumPorts1 = parse.cftable_entry.io.win[0].len;

+			link->io.Attributes1 = IO_DATA_PATH_WIDTH_AUTO;

+			if(!(parse.cftable_entry.io.flags & CISTPL_IO_8BIT))

+				link->io.Attributes1 = IO_DATA_PATH_WIDTH_16;

...
+		}
+	}

How about not having to indent so deep by rewriting that as

	for (rc = pcmcia_get_first_tuple(handle, &tuple);
	     rc == CS_SUCCESS;
	     rc = pcmcia_get_next_tuple(handle, &tuple)) {
...
		if (!parse.cftable_entry.io.nwin)
			continue;

		link->io.BasePort1 = parse.cftable_entry.io.win[0].base;
		link->io.NumPorts1 = parse.cftable_entry.io.win[0].len;
		link->io.Attributes1 = IO_DATA_PATH_WIDTH_AUTO;
		if(!(parse.cftable_entry.io.flags & CISTPL_IO_8BIT))
			link->io.Attributes1 = IO_DATA_PATH_WIDTH_16;
...
	}


+        link->conf.IntType = 00000002;


more spaces used for indentation. Not going to point out any more of these.

+	cmx_poll_timer.function = &cmx_do_poll;


shouldn't this be 
	 cmx_poll_timer.function = cmx_do_poll;
???

+	int i;

+	DEBUG(3, "-> reader_detach(link=%p\n", link);


please have a blank line between variable declarations and other statements.



-- 
Jesper Juhl <jesper.juhl@gmail.com>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please      http://www.expita.com/nomime.html

Re: [PATCH] New: Omnikey CardMan 4040 PCMCIA Driver

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 487 bytes --]

Thanks for your comments, Alexey.

I've now incorprorated all of the requested changes and am testing the
driver.  If everything is still  fine, I'll repost later today.

-- 
- Harald Welte <laforge@gnumonks.org>          	        http://gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] New: Omnikey CardMan 4040 PCMCIA Driver

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 1415 bytes --]

Hi Nish, 

thanks for your comments.

On Sat, Sep 03, 2005 at 03:13:43PM -0700, Nish Aravamudan wrote:
> On 9/3/05, Chase Venters <chase.venters@clientec.com> wrote:
> > > Below you can find a driver for the Omnikey CardMan 4040 PCMCIA
> > > Smartcard Reader.
> > 
> > #define CCID_DRIVER_BULK_DEFAULT_TIMEOUT        (150*HZ)
> 
> These are all fine. Although I am a bit suspicious of 150 second
> timeouts; but if that is the hardware...

That's a definition from the original vendor-supplied driver.
Unfortunately there's no hardware documentation, so I can't verify it.
But generally speaking, serial smart cards can really be slow, so I
think it could make sense.

> > /* how often to poll for fifo status change */
> > #define POLL_PERIOD                             (HZ/100)
> 
> This needs to be msecs_to_jiffies(10), please.

thanks, changed in my local tree now.

> Of bigger concern to me is the use of the sleep_on() family of
> functions, all of which are deprecated.

Ok, I'm working on replacing the respective code with
wait_event_interruptible_timeout().

-- 
- Harald Welte <laforge@gnumonks.org>          	        http://gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

[PATCH] New: Omnikey CardMan 4040 PCMCIA Driver

[Harald Welte]

[-- Attachment #1.1: Type: text/plain, Size: 815 bytes --]

Hi!

Below you can find a driver for the Omnikey CardMan 4040 PCMCIA
Smartcard Reader.  

It's based on some source code originally made available by the vendor
(as BSD/GPL dual licensed code), but has undergone significant changes
to make it more compliant with the general kernel community coding
practise.

As this is the first PCMCIA driver that I'm involved in, please let me
know if I missed something.

If there are no objections, I'd like to see it included in mainline.
Thanks!

-- 
- Harald Welte <laforge@gnumonks.org>          	        http://gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)

[-- Attachment #1.2: cm4040-driver.patch --]
[-- Type: text/plain, Size: 25952 bytes --]

Omnikey CardMan 4040 PCMCIA Smartcard reader driver

Signed-off-by: Harald Welte <laforge@netfilter.org>

---
commit 04ec3ca3e3c6fd6d88c508b6ebe32726ef109367
tree e27a5ccafbcebda6ebfe90036016f0e76dd93137
parent c4ab879b6ef599bf88d19b9b145878ef73400ce7
author Harald Welte <laforge@netfilter.org> So, 04 Sep 2005 11:59:37 +0200
committer Harald Welte <laforge@netfilter.org> So, 04 Sep 2005 11:59:37 +0200

 MAINTAINERS                     |    5 
 drivers/char/pcmcia/Kconfig     |   13 +
 drivers/char/pcmcia/Makefile    |    1 
 drivers/char/pcmcia/cm4040_cs.c |  900 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 919 insertions(+), 0 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1737,6 +1737,11 @@ L:	linux-tr@linuxtr.net
 W:	http://www.linuxtr.net
 S:	Maintained
 
+OMNIKEY CARDMAN 4040 DRIVER
+P:	Harald Welte
+M:	laforge@gnumonks.org
+S:	Maintained
+
 ONSTREAM SCSI TAPE DRIVER
 P:	Willem Riede
 M:	osst@riede.org
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,900 @@
+ /*
+ * A driver for the Omnikey PCMCIA smartcard reader CardMan 4040
+ *
+ * (c) 2000-2004 Omnikey AG (http://www.omnikey.com/)
+ *
+ * (C) 2005 Harald Welte <laforge@gnumonks.org>
+ * 	- add support for poll()
+ * 	- driver cleanup
+ * 	- add waitqueues
+ * 	- adhere to linux kenrel coding style and policies
+ * 	- support 2.6.13 "new style" pcmcia interface
+ *
+ * All rights reserved, Dual BSD/GPL Licensed.
+ */
+
+/* #define PCMCIA_DEBUG 6 */
+
+#include <linux/config.h>
+#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 <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 cmx_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 volatile char *version =
+"OMNIKEY CardMan 4040 v1.1.0gm3 - All bugs added by Harald Welte";
+
+#define	CCID_DRIVER_BULK_DEFAULT_TIMEOUT  	(150*HZ)
+#define	CCID_DRIVER_ASYNC_POWERUP_TIMEOUT 	(35*HZ)
+#define	CCID_DRIVER_MINIMUM_TIMEOUT 		(3*HZ)
+#define READ_WRITE_BUFFER_SIZE 512
+#define POLL_LOOP_COUNT				1000
+
+/* how often to poll for fifo status change */
+#define POLL_PERIOD 				(HZ/100)
+
+static void reader_release(u_long arg);
+static void reader_detach(dev_link_t *link);
+
+static int major;	
+
+#define		BS_READABLE	0x01
+#define		BS_WRITABLE	0x02
+
+typedef struct reader_dev_t {
+	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 int 	  	buffer_status;
+                                
+	unsigned int      	fTimerExpired;
+	struct timer_list	timer;
+	unsigned long     	timeout;
+	unsigned char     	sBuf[READ_WRITE_BUFFER_SIZE];
+	unsigned char     	rBuf[READ_WRITE_BUFFER_SIZE];
+	struct task_struct 	*owner;	
+} reader_dev_t;
+
+static dev_info_t dev_info = MODULE_NAME;
+static dev_link_t *dev_table[CM_MAX_DEV] = { NULL, };
+
+static struct timer_list cmx_poll_timer;
+
+#if (!defined PCMCIA_DEBUG) || (PCMCIA_DEBUG < 7)
+#define	xoutb	outb
+#define	xinb	inb
+#else
+static inline void xoutb(unsigned char val,unsigned short port)
+{
+	DEBUG(7, "outb(val=%.2x,port=%.4x)\n", val, port);
+	outb(val,port);
+}
+
+static unsigned char xinb(unsigned short port)
+{
+	unsigned char val;
+
+	val = inb(port);
+	DEBUG(7, "%.2x=inb(%.4x)\n", val, port);
+	return val;
+}
+#endif
+
+/* poll the device fifo status register.  not to be confused with
+ * the poll syscall. */
+static void cmx_do_poll(unsigned long dummy)
+{
+	unsigned int i;
+	/* walk through all devices */	
+	for (i = 0; dev_table[i]; i++) {
+		dev_link_t *dl = dev_table[i];
+		reader_dev_t *dev = dl->priv;
+		unsigned int obs = xinb(dl->io.BasePort1
+					+ REG_OFFSET_BUFFER_STATUS);
+
+		dev->buffer_status = 0;
+
+		if ((obs & BSR_BULK_IN_FULL) == BSR_BULK_IN_FULL) {
+			dev->buffer_status |= BS_READABLE;
+			DEBUG(4, "waking up read_wait\n");
+			wake_up_interruptible(&dev->read_wait);
+		}
+
+		if ((obs & BSR_BULK_OUT_FULL) == 0) {
+			dev->buffer_status |= BS_WRITABLE;
+			DEBUG(4, "waking up write_wait\n");
+			wake_up_interruptible(&dev->write_wait);
+		}
+
+		if (dev->buffer_status)
+			wake_up_interruptible(&dev->poll_wait);
+	}
+
+	if (atomic_read(&cmx_num_devices_open))
+		mod_timer(&cmx_poll_timer, jiffies + POLL_PERIOD);
+}
+
+static loff_t cmx_llseek(struct file * filp, loff_t off, int whence)
+{
+	return -ESPIPE;
+}
+
+static inline int cmx_waitForBulkOutReady(reader_dev_t *dev)
+{
+        register int i;
+	register int iobase = dev->link.io.BasePort1;
+
+	for (i=0; i < POLL_LOOP_COUNT; i++) {
+		if ((xinb(iobase + REG_OFFSET_BUFFER_STATUS)
+		    & BSR_BULK_OUT_FULL) == 0) {
+			DEBUG(4, "BulkOut empty\n");
+			return 1;
+		}
+	}
+
+	interruptible_sleep_on_timeout(&dev->write_wait, dev->timeout);
+	if (dev->buffer_status & BS_WRITABLE) {
+		DEBUG(4, "woke up: BulkOut empty\n");
+		return 1;
+	}
+
+	DEBUG(4, "woke up: BulkOut full, returning 0 :(\n");
+	return 0;
+}
+
+/* Write to Sync Control Register */
+static inline int cmx_writeSync(unsigned char val,reader_dev_t *dev)
+{
+	register int iobase = dev->link.io.BasePort1;
+	int rc;
+
+	rc = cmx_waitForBulkOutReady(dev);
+        if (rc != 1)
+		return 0;
+
+	xoutb(val,iobase + REG_OFFSET_SYNC_CONTROL);
+	rc = cmx_waitForBulkOutReady(dev);
+        if (rc != 1)
+		return 0;
+
+	return 1;
+}
+
+static inline int cmx_waitForBulkInReady(reader_dev_t *dev)
+{
+        register int i;
+	register int iobase = dev->link.io.BasePort1;
+
+	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\n");
+			return 1;
+		}
+	}
+
+	DEBUG(4, "interruptible_sleep_on_timeout(read_wait, timeout=%ld\n", dev->timeout);
+	interruptible_sleep_on_timeout(&dev->read_wait, dev->timeout);
+	if (dev->buffer_status & BS_READABLE) {
+		DEBUG(4, "woke up: BulkIn full\n");
+		return 1;
+	}
+
+	DEBUG(4, "woke up: BulkIn not full, returning 0 :(\n");
+	return 0;
+}
+
+static ssize_t cmx_read(struct file *filp,char *buf,size_t count,loff_t *ppos)
+{
+	register reader_dev_t *dev=(reader_dev_t *)filp->private_data;
+	register int iobase=dev->link.io.BasePort1;
+	unsigned long ulBytesToRead;
+	unsigned long i;
+	unsigned long ulMin;
+	int rc;
+	unsigned char uc;
+
+	DEBUG(2, "-> cmx_read(%s,%d)\n", current->comm,current->pid);
+
+	if (count==0)
+		return 0;
+
+        if (count < 10)
+		return -EFAULT;
+
+	if (filp->f_flags & O_NONBLOCK) { 
+		DEBUG(4, "filep->f_flags O_NONBLOCK set\n");
+		DEBUG(4, "<- cmx_read (failure)\n");
+		return -EAGAIN;
+	}
+
+	if ((dev->link.state & DEV_PRESENT)==0)	
+		return -ENODEV;
+
+	for (i=0; i<5; i++) {
+		rc = cmx_waitForBulkInReady(dev);
+		if (rc != 1) {
+			DEBUG(5,"cmx_waitForBulkInReady rc=%.2x\n",rc);
+			DEBUG(2, "<- cmx_read (failed)\n");
+			return -EIO;
+		}
+	  	dev->rBuf[i] = xinb(iobase + REG_OFFSET_BULK_IN);
+#ifdef PCMCIA_DEBUG
+		if (pc_debug >= 6)
+			printk(KERN_DEBUG "%lu:%2x ", i, dev->rBuf[i]);
+	}
+	printk("\n");
+#else
+	}
+#endif
+
+	ulBytesToRead = 5 + 
+			(0x000000FF&((char)dev->rBuf[1])) + 
+			(0x0000FF00&((char)dev->rBuf[2] << 8)) + 
+			(0x00FF0000&((char)dev->rBuf[3] << 16)) + 
+			(0xFF000000&((char)dev->rBuf[4] << 24));
+
+	DEBUG(6, "BytesToRead=%lu\n", ulBytesToRead);
+
+	ulMin = (count < (ulBytesToRead+5))?count:(ulBytesToRead+5);
+
+	DEBUG(6, "Min=%lu\n", ulMin);
+
+	for (i=0; i < (ulMin-5); i++) {
+		rc = cmx_waitForBulkInReady(dev);
+		if (rc != 1) {
+			DEBUG(5,"cmx_waitForBulkInReady rc=%.2x\n",rc);
+			DEBUG(2, "<- cmx_read (failed)\n");
+			return -EIO;
+		}
+		dev->rBuf[i+5] = xinb(iobase + REG_OFFSET_BULK_IN);
+		DEBUG(6, "%lu:%2x ", i, dev->rBuf[i]);
+	}
+	DEBUG(6, "\n");
+
+	*ppos = ulMin;
+	copy_to_user(buf, dev->rBuf, ulMin);
+
+
+	rc = cmx_waitForBulkInReady(dev);
+	if (rc != 1) {
+		DEBUG(5,"cmx_waitForBulkInReady rc=%.2x\n",rc);
+		DEBUG(2, "<- cmx_read (failed)\n");
+		return -EIO;
+	}
+
+	rc = cmx_writeSync(SCR_READER_TO_HOST_DONE, dev);
+
+	if (rc != 1) {
+		DEBUG(5,"cmx_writeSync c=%.2x\n",rc);
+		DEBUG(2, "<- cmx_read (failed)\n");
+		return -EIO;
+	}
+
+	uc = xinb(iobase + REG_OFFSET_BULK_IN);
+
+	DEBUG(2,"<- cmx_read (successfully)\n");
+	return ulMin;
+}
+
+static ssize_t cmx_write(struct file *filp,const char *buf,size_t count,
+			 loff_t *ppos)
+{
+	register reader_dev_t *dev=(reader_dev_t *)filp->private_data;
+	register int iobase=dev->link.io.BasePort1;
+	ssize_t rc;
+	int i;
+	unsigned int uiBytesToWrite; 
+
+	DEBUG(2, "-> cmx_write(%s,%d)\n", current->comm, current->pid);
+
+	if (count == 0) {
+		DEBUG(2, "<- cmx_write nothing to do (successfully)\n");
+		return 0;
+	}
+
+	if (count < 5) {
+		DEBUG(2, "<- cmx_write buffersize=%Zd < 5\n", count);
+		return -EIO;
+	}
+
+	if (filp->f_flags & O_NONBLOCK) { 
+		DEBUG(4, "filep->f_flags O_NONBLOCK set\n");
+		DEBUG(4, "<- cmx_write (failure)\n");
+		return -EAGAIN;
+        }
+
+	if ((dev->link.state & DEV_PRESENT) == 0)	
+		return -ENODEV;
+
+	uiBytesToWrite = count;
+	copy_from_user(dev->sBuf, buf, uiBytesToWrite);
+
+	switch (dev->sBuf[0]) {
+		case CMD_PC_TO_RDR_XFRBLOCK:
+		case CMD_PC_TO_RDR_SECURE:
+		case CMD_PC_TO_RDR_TEST_SECURE:
+		case CMD_PC_TO_RDR_OK_SECURE:
+			dev->timeout = CCID_DRIVER_BULK_DEFAULT_TIMEOUT;
+			break;
+
+		case CMD_PC_TO_RDR_ICCPOWERON:
+			dev->timeout = CCID_DRIVER_ASYNC_POWERUP_TIMEOUT;
+			break;
+
+		case CMD_PC_TO_RDR_GETSLOTSTATUS:
+		case CMD_PC_TO_RDR_ICCPOWEROFF:
+		case CMD_PC_TO_RDR_GETPARAMETERS:
+		case CMD_PC_TO_RDR_RESETPARAMETERS:  
+		case CMD_PC_TO_RDR_SETPARAMETERS:
+		case CMD_PC_TO_RDR_ESCAPE:
+		case CMD_PC_TO_RDR_ICCCLOCK:
+		default:
+			dev->timeout = CCID_DRIVER_MINIMUM_TIMEOUT;
+			break;
+	}
+
+	rc = cmx_writeSync(SCR_HOST_TO_READER_START, dev);
+
+	DEBUG(4, "start \n");
+
+	for (i=0; i < uiBytesToWrite; i++) {
+		rc = cmx_waitForBulkOutReady(dev);
+		if (rc != 1) {
+			DEBUG(5, "cmx_waitForBulkOutReady rc=%.2Zx\n", rc);
+			DEBUG(2, "<- cmx_write (failed)\n");
+			return -EIO;
+          	}
+	 
+		xoutb(dev->sBuf[i],iobase + REG_OFFSET_BULK_OUT);
+		DEBUG(4, "%.2x ", dev->sBuf[i]);
+	}
+	DEBUG(4, "end\n");
+
+	rc = cmx_writeSync(SCR_HOST_TO_READER_DONE, dev);
+
+	if (rc != 1) {
+		DEBUG(5, "cmx_writeSync c=%.2Zx\n", rc);
+		DEBUG(2, "<- cmx_write (failed)\n");
+		return -EIO;
+	}
+
+	DEBUG(2, "<- cmx_write (successfully)\n");
+	return count;
+}
+
+static unsigned int cmx_poll(struct file *filp, poll_table *wait)
+{
+	reader_dev_t *dev=(reader_dev_t *)filp->private_data;
+	unsigned int mask = 0;
+
+	poll_wait(filp, &dev->poll_wait, wait);
+
+	if (dev->buffer_status & BS_READABLE)
+		mask |= POLLIN | POLLRDNORM;
+	if (dev->buffer_status & BS_WRITABLE)
+		mask |= POLLOUT | POLLWRNORM;
+
+	return mask;
+}
+
+static int cmx_ioctl(struct inode *inode,struct file *filp,unsigned int cmd,
+			unsigned long arg)
+{
+	dev_link_t *link;
+	int rc, size;
+
+	link=dev_table[MINOR(inode->i_rdev)];
+	if (!(DEV_OK(link))) {
+		DEBUG(4, "DEV_OK false\n");
+		return -ENODEV;
+	}
+	if (_IOC_TYPE(cmd)!=CM_IOC_MAGIC) {
+		DEBUG(4,"ioctype mismatch\n");
+		return -EINVAL;
+	}
+	if (_IOC_NR(cmd)>CM_IOC_MAXNR) {
+		DEBUG(4,"iocnr mismatch\n");
+		return -EINVAL;
+	} 
+	size = _IOC_SIZE(cmd);
+	rc = 0;
+	DEBUG(4,"iocdir=%.4x iocr=%.4x iocw=%.4x iocsize=%d cmd=%.4x\n",
+		_IOC_DIR(cmd),_IOC_READ,_IOC_WRITE,size,cmd);
+
+	if (_IOC_DIR(cmd)&_IOC_READ) {
+		if (!access_ok(VERIFY_WRITE, (void *)arg, size))
+			return -EFAULT;
+	}
+	if (_IOC_DIR(cmd)&_IOC_WRITE) {
+		if (!access_ok(VERIFY_READ, (void *)arg, size))
+			return -EFAULT;
+	}
+
+	return rc;
+}
+
+static int cmx_open (struct inode *inode, struct file *filp)
+{
+	reader_dev_t *dev;
+	dev_link_t *link;
+	int i;
+
+	DEBUG(2, "-> cmx_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);
+	if (i >= CM_MAX_DEV) {
+		DEBUG(4, "MAX_DEV reached\n");
+		DEBUG(4, "<- cmx_open (failure)\n");
+		return -ENODEV;
+	}
+	link = dev_table[MINOR(inode->i_rdev)];
+	if (link == NULL || !(DEV_OK(link))) {
+		DEBUG(4, "link== NULL || DEV_OK false\n");
+		DEBUG(4, "<- cmx_open (failure)\n");
+		return -ENODEV;
+	}
+	if (link->open) {
+		DEBUG(4, "DEVICE BUSY\n");
+		DEBUG(4, "<- cmx_open (failure)\n");
+		return -EBUSY;
+	}
+
+	dev = (reader_dev_t *)link->priv;
+	filp->private_data = dev;
+
+	if (filp->f_flags & O_NONBLOCK) { 
+		DEBUG(4, "filep->f_flags O_NONBLOCK set\n");
+		DEBUG(4, "<- cmx_open (failure)\n");
+		return -EAGAIN;
+        }
+
+	dev->owner = current;
+	link->open = 1;		
+
+	atomic_inc(&cmx_num_devices_open);
+	mod_timer(&cmx_poll_timer, jiffies + POLL_PERIOD);
+
+	DEBUG(2, "<- cmx_open (successfully)\n");
+	return 0;
+}
+
+static int cmx_close(struct inode *inode,struct file *filp)
+{
+	reader_dev_t *dev;
+	dev_link_t *link;
+	int i;
+
+	DEBUG(2, "-> cmx_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 = (reader_dev_t *)link->priv;
+
+	link->open = 0;
+	wake_up(&dev->devq);	
+
+	atomic_dec(&cmx_num_devices_open);
+
+	DEBUG(2, "<- cmx_close\n");
+	return 0;
+}
+
+static void cmx_reader_release(dev_link_t *link)
+{
+	reader_dev_t *dev = (reader_dev_t *)link->priv;
+
+	DEBUG(3, "-> cmx_reader_release\n"); 
+	while (link->open) {
+		DEBUG(3, KERN_INFO MODULE_NAME ": delaying release until "
+		      "process '%s', pid %d has terminated\n",
+		      dev->owner->comm,dev->owner->pid);
+ 		wait_event(dev->devq, (link->open == 0));
+	}
+	DEBUG(3, "<- cmx_reader_release\n"); 
+	return;
+}
+
+static void reader_config(dev_link_t *link, int devno)
+{
+	client_handle_t handle;
+	reader_dev_t *dev;
+	tuple_t tuple;
+	cisparse_t parse;
+	config_info_t conf;
+	u_char buf[64];
+	int fail_fn,fail_rc;
+	int rc;
+
+	DEBUG(2, "-> reader_config\n");
+
+	handle = link->handle;
+
+	tuple.DesiredTuple = CISTPL_CONFIG;
+	tuple.Attributes = 0;
+	tuple.TupleData = buf;
+	tuple.TupleDataMax = sizeof(buf);
+ 	tuple.TupleOffset = 0;
+
+	if ((fail_rc = pcmcia_get_first_tuple(handle,&tuple)) != CS_SUCCESS) {
+		fail_fn = GetFirstTuple;
+		goto cs_failed;
+	}
+	if ((fail_rc = pcmcia_get_tuple_data(handle,&tuple)) != CS_SUCCESS) {
+		fail_fn = GetTupleData;
+		goto cs_failed;
+	}
+	if ((fail_rc = pcmcia_parse_tuple(handle,&tuple,&parse))
+							!= CS_SUCCESS) {
+		fail_fn = ParseTuple;
+		goto cs_failed;
+	}
+	if ((fail_rc = pcmcia_get_configuration_info(handle,&conf))
+							!= CS_SUCCESS) {
+		fail_fn = GetConfigurationInfo;
+		goto cs_failed;
+	}
+
+	link->state |= DEV_CONFIG;
+	link->conf.ConfigBase = parse.config.base;
+	link->conf.Present = parse.config.rmask[0];
+	link->conf.Vcc = conf.Vcc;
+	DEBUG(2, "link->conf.Vcc=%d\n", link->conf.Vcc);
+
+	link->io.BasePort2 = 0;
+	link->io.NumPorts2 = 0;
+	link->io.Attributes2 = 0;
+	tuple.DesiredTuple = CISTPL_CFTABLE_ENTRY;
+	for (rc = pcmcia_get_first_tuple(handle, &tuple);
+	     rc == CS_SUCCESS;
+	     rc = pcmcia_get_next_tuple(handle, &tuple)) {
+ 		DEBUG(2, "Examing CIS Tuple!\n");
+		rc = pcmcia_get_tuple_data(handle, &tuple);
+		if (rc != CS_SUCCESS)
+			continue;
+		rc = pcmcia_parse_tuple(handle, &tuple, &parse);
+		if (rc != CS_SUCCESS)
+			continue;
+
+		DEBUG(2, "tupleIndex=%d\n", parse.cftable_entry.index);
+		link->conf.ConfigIndex = parse.cftable_entry.index;
+		
+		if (parse.cftable_entry.io.nwin) {
+			link->io.BasePort1 = parse.cftable_entry.io.win[0].base;
+			link->io.NumPorts1 = parse.cftable_entry.io.win[0].len;
+			link->io.Attributes1 = IO_DATA_PATH_WIDTH_AUTO;
+			if(!(parse.cftable_entry.io.flags & CISTPL_IO_8BIT))
+				link->io.Attributes1 = IO_DATA_PATH_WIDTH_16;
+			if(!(parse.cftable_entry.io.flags & CISTPL_IO_16BIT))
+				link->io.Attributes1 = IO_DATA_PATH_WIDTH_8;
+			link->io.IOAddrLines = parse.cftable_entry.io.flags
+						& CISTPL_IO_LINES_MASK;
+			DEBUG(2,"io.BasePort1=%.4x\n", link->io.BasePort1);
+			DEBUG(2,"io.NumPorts1=%.4x\n", link->io.NumPorts1);
+			DEBUG(2,"io.BasePort2=%.4x\n", link->io.BasePort2);
+			DEBUG(2,"io.NumPorts2=%.4x\n", link->io.NumPorts2);
+			DEBUG(2,"io.IOAddrLines=%.4x\n", 
+			      link->io.IOAddrLines);
+			rc = pcmcia_request_io(handle, &link->io);
+			if (rc == CS_SUCCESS) {
+				DEBUG(2, "RequestIO OK\n");
+				break; 
+			} else 
+				DEBUG(2, "RequestIO failed\n");
+		}
+	}
+	if (rc != CS_SUCCESS) {
+		DEBUG(2, "Couldn't configure reader\n");
+		goto cs_release;
+	}
+
+        link->conf.IntType = 00000002;
+
+	if ((fail_rc = pcmcia_request_configuration(handle,&link->conf))
+								!=CS_SUCCESS) {
+		fail_fn = RequestConfiguration;
+		DEBUG(1, "pcmcia_request_configuration failed 0x%x\n", fail_rc);
+		goto cs_release;
+	}
+
+	DEBUG(2, "RequestConfiguration OK\n");
+
+	dev = link->priv;
+	sprintf(dev->node.dev_name, DEVICE_NAME "%d", devno);
+	dev->node.major = major;
+	dev->node.minor = devno;
+	dev->node.next = NULL;
+	link->dev = &dev->node;
+	link->state &= ~DEV_CONFIG_PENDING;
+
+	DEBUG(2, "device " DEVICE_NAME "%d at 0x%.4x-0x%.4x\n", devno,
+	      link->io.BasePort1, link->io.BasePort1+link->io.NumPorts1);
+	DEBUG(2, "<- reader_config (succ)\n");
+
+	return;
+
+cs_failed:
+	cs_error(handle, fail_fn, fail_rc);
+cs_release:
+	reader_release((u_long)link);
+	link->state &= ~DEV_CONFIG_PENDING;
+	DEBUG(2, "<- reader_config (failure)\n");
+}
+
+static int reader_event(event_t event, int priority,
+			event_callback_args_t *args)
+{
+	dev_link_t *link;
+	reader_dev_t *dev;
+	int devno;
+
+	DEBUG(3,"-> reader_event\n");
+	link = args->client_data;
+	dev = link->priv;
+	for (devno = 0; devno < CM_MAX_DEV; devno++) {
+		if (dev_table[devno]==link)
+			break;
+	}
+	if (devno == CM_MAX_DEV)
+		return CS_BAD_ADAPTER;
+
+	switch (event) {
+		case CS_EVENT_CARD_INSERTION:
+			DEBUG(5, "CS_EVENT_CARD_INSERTION\n");
+			link->state |= DEV_PRESENT | DEV_CONFIG_PENDING;
+			reader_config(link,devno);
+			break;
+		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;
+		
+		case CS_EVENT_CARD_RESET:
+	        	DEBUG(5, "CS_EVENT_CARD_RESET\n");
+			if ((link->state & DEV_CONFIG)) {
+				DEBUG(5, "cmx: RequestConfiguration\n");
+		  		pcmcia_request_configuration(link->handle,
+							     &link->conf);
+			}
+			break;
+		default:
+			DEBUG(5, "reader_event: unknown event %.2x\n", event);
+			break;
+	}
+	DEBUG(3, "<- reader_event\n");
+	return CS_SUCCESS;
+}
+
+static void reader_release(u_long arg)
+{
+	dev_link_t *link;
+	int rc;
+
+	DEBUG(3, "-> reader_release\n");
+	link = (dev_link_t *)arg;
+	cmx_reader_release(link->priv); 
+	rc = pcmcia_release_configuration(link->handle);
+	if (rc != CS_SUCCESS)
+		DEBUG(5, "couldn't ReleaseConfiguration "
+		      "reasoncode=%.2x\n", rc);
+	rc = pcmcia_release_io(link->handle, &link->io);
+	if (rc != CS_SUCCESS)
+		DEBUG(5, "couldn't ReleaseIO reasoncode=%.2x\n", rc);
+
+	DEBUG(3, "<- reader_release\n");
+}
+
+static dev_link_t *reader_attach(void)
+{
+	reader_dev_t *dev;
+	dev_link_t *link;
+	client_reg_t client_reg;
+	int i;
+
+	DEBUG(3, "reader_attach\n");
+	for (i=0; i < CM_MAX_DEV; i++) {
+		if (dev_table[i] == NULL)
+			break;
+	}
+
+	if (i == CM_MAX_DEV) {
+		printk(KERN_NOTICE "all devices in use\n");
+		return NULL;
+	}
+	
+	DEBUG(5, "create reader device instance\n");
+	dev = kmalloc(sizeof(reader_dev_t), GFP_KERNEL);
+	if (dev == NULL)
+		return NULL;
+
+	memset(dev, 0, sizeof(reader_dev_t));
+	dev->timeout = CCID_DRIVER_MINIMUM_TIMEOUT;
+	dev->fTimerExpired = 0;
+
+	link = &dev->link;
+	link->priv = dev;
+
+	link->conf.IntType = INT_MEMORY_AND_IO;
+	dev_table[i] = link;
+
+	
+	DEBUG(5, "Register with Card Services\n");
+	client_reg.dev_info = &dev_info;
+	client_reg.Attributes = INFO_IO_CLIENT | INFO_CARD_SHARE;
+	client_reg.EventMask=
+		CS_EVENT_CARD_INSERTION | CS_EVENT_CARD_REMOVAL |
+		CS_EVENT_RESET_PHYSICAL | CS_EVENT_CARD_RESET |
+		CS_EVENT_PM_SUSPEND | CS_EVENT_PM_RESUME;
+	client_reg.Version = 0x0210;
+	client_reg.event_callback_args.client_data = link;
+	i = pcmcia_register_client(&link->handle, &client_reg);
+	if (i) {
+		cs_error(link->handle, RegisterClient, i);
+		reader_detach(link);
+		return NULL;
+	}
+	init_waitqueue_head(&dev->devq);
+	init_waitqueue_head(&dev->poll_wait);
+	init_waitqueue_head(&dev->read_wait);
+	init_waitqueue_head(&dev->write_wait);
+	init_timer(&cmx_poll_timer);
+	cmx_poll_timer.function = &cmx_do_poll;
+
+	return link;
+}
+
+static void reader_detach_by_devno(int devno,dev_link_t *link)
+{
+	reader_dev_t *dev=link->priv;
+
+	DEBUG(3, "-> detach_by_devno(devno=%d)\n", devno);
+	if (link->state & DEV_CONFIG) {
+		DEBUG(5, "device still configured (try to release it)\n");
+		reader_release((u_long)link);
+	}
+
+	pcmcia_deregister_client(link->handle);
+	dev_table[devno] = NULL;
+	DEBUG(5, "freeing dev=%p\n", dev);
+	kfree(dev);
+	DEBUG(3, "<- detach_by-devno\n");
+	return;
+}
+
+static void reader_detach(dev_link_t *link)
+{
+	int i;
+	DEBUG(3, "-> reader_detach(link=%p\n", link);
+	/* find device */
+	for(i=0; i < CM_MAX_DEV; i++) {
+		if (dev_table[i] == link)
+			break;
+	}
+	if (i == CM_MAX_DEV) {
+		printk(KERN_WARNING MODULE_NAME 
+			": detach for unkown device aborted\n");
+		return;
+	}
+	reader_detach_by_devno(i, link);
+	DEBUG(3, "<- reader_detach\n");
+	return;
+}
+
+static struct file_operations reader_fops = {
+	.owner		= THIS_MODULE,
+	.llseek		= cmx_llseek,
+	.read		= cmx_read,
+	.write		= cmx_write,
+	.ioctl		= cmx_ioctl,
+	.open		= cmx_open,
+	.release	= cmx_close,
+	.poll		= cmx_poll,
+};
+
+static struct pcmcia_device_id cm4040_ids[] = {
+	PCMCIA_DEVICE_MANF_CARD(0x0223, 0x0200),
+	PCMCIA_DEVICE_PROD_ID12("OMNIKEY", "CardMan 4040", 
+				0xE32CDD8C, 0x8F23318B),
+	PCMCIA_DEVICE_NULL,
+};
+MODULE_DEVICE_TABLE(pcmcia, cm4040_ids);
+
+static struct pcmcia_driver reader_driver = {
+  	.owner		= THIS_MODULE,
+  	.drv		= {
+		.name	= "cm4040_cs",
+	},
+	.attach		= reader_attach,
+	.detach		= reader_detach,
+	.event		= reader_event,
+	.id_table	= cm4040_ids,
+};
+
+static int __init cmx_init(void)
+{
+	printk(KERN_INFO "%s\n", version);
+	pcmcia_register_driver(&reader_driver);
+	major = register_chrdev(0, DEVICE_NAME, &reader_fops);
+	if (major < 0) {
+		printk(KERN_WARNING MODULE_NAME
+			": could not get major number\n");
+		return -1;
+	}
+	return 0;
+}
+
+static void __exit cmx_exit(void)
+{
+	int i;
+
+	printk(KERN_INFO MODULE_NAME ": unloading\n");
+	pcmcia_unregister_driver(&reader_driver);  
+	for (i=0; i < CM_MAX_DEV; i++) {
+		if (dev_table[i])
+			reader_detach_by_devno(i, dev_table[i]);
+	}
+	unregister_chrdev(major, DEVICE_NAME);
+}
+
+module_init(cmx_init);
+module_exit(cmx_exit);
+MODULE_LICENSE("Dual BSD/GPL");

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] New: Omnikey CardMan 4040 PCMCIA Driver

[Harald Welte]

[-- Attachment #1.1: Type: text/plain, Size: 568 bytes --]

On Sun, Sep 04, 2005 at 12:12:18PM +0200, Harald Welte wrote:
> Hi!
> 
> Below you can find a driver for the Omnikey CardMan 4040 PCMCIA
> Smartcard Reader.  

Sorry, the patch was missing a "cg-add" of the header file.  Please use
the patch below.
-- 
- Harald Welte <laforge@gnumonks.org>          	        http://gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)

[-- Attachment #1.2: cm4040-driver.patch --]
[-- Type: text/plain, Size: 27813 bytes --]

Add Omnikey CardMan 4040 Driver

Signed-off-by: Harald Welte <laforge@netfilter.org>

---
commit 0e760a5785ebb83b932d104679cc2b2b4070b1c1
tree 46d71d0c6b41b441a1a8d725b741450f1334296e
parent c4ab879b6ef599bf88d19b9b145878ef73400ce7
author Harald Welte <laforge@netfilter.org> So, 04 Sep 2005 13:05:44 +0200
committer Harald Welte <laforge@netfilter.org> So, 04 Sep 2005 13:05:44 +0200

 MAINTAINERS                     |    5 
 drivers/char/pcmcia/Kconfig     |   13 +
 drivers/char/pcmcia/Makefile    |    1 
 drivers/char/pcmcia/cm4040_cs.c |  900 +++++++++++++++++++++++++++++++++++++++
 drivers/char/pcmcia/cm4040_cs.h |   52 ++
 5 files changed, 971 insertions(+), 0 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1737,6 +1737,11 @@ L:	linux-tr@linuxtr.net
 W:	http://www.linuxtr.net
 S:	Maintained
 
+OMNIKEY CARDMAN 4040 DRIVER
+P:	Harald Welte
+M:	laforge@gnumonks.org
+S:	Maintained
+
 ONSTREAM SCSI TAPE DRIVER
 P:	Willem Riede
 M:	osst@riede.org
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,900 @@
+ /*
+ * A driver for the Omnikey PCMCIA smartcard reader CardMan 4040
+ *
+ * (c) 2000-2004 Omnikey AG (http://www.omnikey.com/)
+ *
+ * (C) 2005 Harald Welte <laforge@gnumonks.org>
+ * 	- add support for poll()
+ * 	- driver cleanup
+ * 	- add waitqueues
+ * 	- adhere to linux kenrel coding style and policies
+ * 	- support 2.6.13 "new style" pcmcia interface
+ *
+ * All rights reserved, Dual BSD/GPL Licensed.
+ */
+
+/* #define PCMCIA_DEBUG 6 */
+
+#include <linux/config.h>
+#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 <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 cmx_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 volatile char *version =
+"OMNIKEY CardMan 4040 v1.1.0gm3 - All bugs added by Harald Welte";
+
+#define	CCID_DRIVER_BULK_DEFAULT_TIMEOUT  	(150*HZ)
+#define	CCID_DRIVER_ASYNC_POWERUP_TIMEOUT 	(35*HZ)
+#define	CCID_DRIVER_MINIMUM_TIMEOUT 		(3*HZ)
+#define READ_WRITE_BUFFER_SIZE 512
+#define POLL_LOOP_COUNT				1000
+
+/* how often to poll for fifo status change */
+#define POLL_PERIOD 				(HZ/100)
+
+static void reader_release(u_long arg);
+static void reader_detach(dev_link_t *link);
+
+static int major;	
+
+#define		BS_READABLE	0x01
+#define		BS_WRITABLE	0x02
+
+typedef struct reader_dev_t {
+	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 int 	  	buffer_status;
+                                
+	unsigned int      	fTimerExpired;
+	struct timer_list	timer;
+	unsigned long     	timeout;
+	unsigned char     	sBuf[READ_WRITE_BUFFER_SIZE];
+	unsigned char     	rBuf[READ_WRITE_BUFFER_SIZE];
+	struct task_struct 	*owner;	
+} reader_dev_t;
+
+static dev_info_t dev_info = MODULE_NAME;
+static dev_link_t *dev_table[CM_MAX_DEV] = { NULL, };
+
+static struct timer_list cmx_poll_timer;
+
+#if (!defined PCMCIA_DEBUG) || (PCMCIA_DEBUG < 7)
+#define	xoutb	outb
+#define	xinb	inb
+#else
+static inline void xoutb(unsigned char val,unsigned short port)
+{
+	DEBUG(7, "outb(val=%.2x,port=%.4x)\n", val, port);
+	outb(val,port);
+}
+
+static unsigned char xinb(unsigned short port)
+{
+	unsigned char val;
+
+	val = inb(port);
+	DEBUG(7, "%.2x=inb(%.4x)\n", val, port);
+	return val;
+}
+#endif
+
+/* poll the device fifo status register.  not to be confused with
+ * the poll syscall. */
+static void cmx_do_poll(unsigned long dummy)
+{
+	unsigned int i;
+	/* walk through all devices */	
+	for (i = 0; dev_table[i]; i++) {
+		dev_link_t *dl = dev_table[i];
+		reader_dev_t *dev = dl->priv;
+		unsigned int obs = xinb(dl->io.BasePort1
+					+ REG_OFFSET_BUFFER_STATUS);
+
+		dev->buffer_status = 0;
+
+		if ((obs & BSR_BULK_IN_FULL) == BSR_BULK_IN_FULL) {
+			dev->buffer_status |= BS_READABLE;
+			DEBUG(4, "waking up read_wait\n");
+			wake_up_interruptible(&dev->read_wait);
+		}
+
+		if ((obs & BSR_BULK_OUT_FULL) == 0) {
+			dev->buffer_status |= BS_WRITABLE;
+			DEBUG(4, "waking up write_wait\n");
+			wake_up_interruptible(&dev->write_wait);
+		}
+
+		if (dev->buffer_status)
+			wake_up_interruptible(&dev->poll_wait);
+	}
+
+	if (atomic_read(&cmx_num_devices_open))
+		mod_timer(&cmx_poll_timer, jiffies + POLL_PERIOD);
+}
+
+static loff_t cmx_llseek(struct file * filp, loff_t off, int whence)
+{
+	return -ESPIPE;
+}
+
+static inline int cmx_waitForBulkOutReady(reader_dev_t *dev)
+{
+        register int i;
+	register int iobase = dev->link.io.BasePort1;
+
+	for (i=0; i < POLL_LOOP_COUNT; i++) {
+		if ((xinb(iobase + REG_OFFSET_BUFFER_STATUS)
+		    & BSR_BULK_OUT_FULL) == 0) {
+			DEBUG(4, "BulkOut empty\n");
+			return 1;
+		}
+	}
+
+	interruptible_sleep_on_timeout(&dev->write_wait, dev->timeout);
+	if (dev->buffer_status & BS_WRITABLE) {
+		DEBUG(4, "woke up: BulkOut empty\n");
+		return 1;
+	}
+
+	DEBUG(4, "woke up: BulkOut full, returning 0 :(\n");
+	return 0;
+}
+
+/* Write to Sync Control Register */
+static inline int cmx_writeSync(unsigned char val,reader_dev_t *dev)
+{
+	register int iobase = dev->link.io.BasePort1;
+	int rc;
+
+	rc = cmx_waitForBulkOutReady(dev);
+        if (rc != 1)
+		return 0;
+
+	xoutb(val,iobase + REG_OFFSET_SYNC_CONTROL);
+	rc = cmx_waitForBulkOutReady(dev);
+        if (rc != 1)
+		return 0;
+
+	return 1;
+}
+
+static inline int cmx_waitForBulkInReady(reader_dev_t *dev)
+{
+        register int i;
+	register int iobase = dev->link.io.BasePort1;
+
+	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\n");
+			return 1;
+		}
+	}
+
+	DEBUG(4, "interruptible_sleep_on_timeout(read_wait, timeout=%ld\n", dev->timeout);
+	interruptible_sleep_on_timeout(&dev->read_wait, dev->timeout);
+	if (dev->buffer_status & BS_READABLE) {
+		DEBUG(4, "woke up: BulkIn full\n");
+		return 1;
+	}
+
+	DEBUG(4, "woke up: BulkIn not full, returning 0 :(\n");
+	return 0;
+}
+
+static ssize_t cmx_read(struct file *filp,char *buf,size_t count,loff_t *ppos)
+{
+	register reader_dev_t *dev=(reader_dev_t *)filp->private_data;
+	register int iobase=dev->link.io.BasePort1;
+	unsigned long ulBytesToRead;
+	unsigned long i;
+	unsigned long ulMin;
+	int rc;
+	unsigned char uc;
+
+	DEBUG(2, "-> cmx_read(%s,%d)\n", current->comm,current->pid);
+
+	if (count==0)
+		return 0;
+
+        if (count < 10)
+		return -EFAULT;
+
+	if (filp->f_flags & O_NONBLOCK) { 
+		DEBUG(4, "filep->f_flags O_NONBLOCK set\n");
+		DEBUG(4, "<- cmx_read (failure)\n");
+		return -EAGAIN;
+	}
+
+	if ((dev->link.state & DEV_PRESENT)==0)	
+		return -ENODEV;
+
+	for (i=0; i<5; i++) {
+		rc = cmx_waitForBulkInReady(dev);
+		if (rc != 1) {
+			DEBUG(5,"cmx_waitForBulkInReady rc=%.2x\n",rc);
+			DEBUG(2, "<- cmx_read (failed)\n");
+			return -EIO;
+		}
+	  	dev->rBuf[i] = xinb(iobase + REG_OFFSET_BULK_IN);
+#ifdef PCMCIA_DEBUG
+		if (pc_debug >= 6)
+			printk(KERN_DEBUG "%lu:%2x ", i, dev->rBuf[i]);
+	}
+	printk("\n");
+#else
+	}
+#endif
+
+	ulBytesToRead = 5 + 
+			(0x000000FF&((char)dev->rBuf[1])) + 
+			(0x0000FF00&((char)dev->rBuf[2] << 8)) + 
+			(0x00FF0000&((char)dev->rBuf[3] << 16)) + 
+			(0xFF000000&((char)dev->rBuf[4] << 24));
+
+	DEBUG(6, "BytesToRead=%lu\n", ulBytesToRead);
+
+	ulMin = (count < (ulBytesToRead+5))?count:(ulBytesToRead+5);
+
+	DEBUG(6, "Min=%lu\n", ulMin);
+
+	for (i=0; i < (ulMin-5); i++) {
+		rc = cmx_waitForBulkInReady(dev);
+		if (rc != 1) {
+			DEBUG(5,"cmx_waitForBulkInReady rc=%.2x\n",rc);
+			DEBUG(2, "<- cmx_read (failed)\n");
+			return -EIO;
+		}
+		dev->rBuf[i+5] = xinb(iobase + REG_OFFSET_BULK_IN);
+		DEBUG(6, "%lu:%2x ", i, dev->rBuf[i]);
+	}
+	DEBUG(6, "\n");
+
+	*ppos = ulMin;
+	copy_to_user(buf, dev->rBuf, ulMin);
+
+
+	rc = cmx_waitForBulkInReady(dev);
+	if (rc != 1) {
+		DEBUG(5,"cmx_waitForBulkInReady rc=%.2x\n",rc);
+		DEBUG(2, "<- cmx_read (failed)\n");
+		return -EIO;
+	}
+
+	rc = cmx_writeSync(SCR_READER_TO_HOST_DONE, dev);
+
+	if (rc != 1) {
+		DEBUG(5,"cmx_writeSync c=%.2x\n",rc);
+		DEBUG(2, "<- cmx_read (failed)\n");
+		return -EIO;
+	}
+
+	uc = xinb(iobase + REG_OFFSET_BULK_IN);
+
+	DEBUG(2,"<- cmx_read (successfully)\n");
+	return ulMin;
+}
+
+static ssize_t cmx_write(struct file *filp,const char *buf,size_t count,
+			 loff_t *ppos)
+{
+	register reader_dev_t *dev=(reader_dev_t *)filp->private_data;
+	register int iobase=dev->link.io.BasePort1;
+	ssize_t rc;
+	int i;
+	unsigned int uiBytesToWrite; 
+
+	DEBUG(2, "-> cmx_write(%s,%d)\n", current->comm, current->pid);
+
+	if (count == 0) {
+		DEBUG(2, "<- cmx_write nothing to do (successfully)\n");
+		return 0;
+	}
+
+	if (count < 5) {
+		DEBUG(2, "<- cmx_write buffersize=%Zd < 5\n", count);
+		return -EIO;
+	}
+
+	if (filp->f_flags & O_NONBLOCK) { 
+		DEBUG(4, "filep->f_flags O_NONBLOCK set\n");
+		DEBUG(4, "<- cmx_write (failure)\n");
+		return -EAGAIN;
+        }
+
+	if ((dev->link.state & DEV_PRESENT) == 0)	
+		return -ENODEV;
+
+	uiBytesToWrite = count;
+	copy_from_user(dev->sBuf, buf, uiBytesToWrite);
+
+	switch (dev->sBuf[0]) {
+		case CMD_PC_TO_RDR_XFRBLOCK:
+		case CMD_PC_TO_RDR_SECURE:
+		case CMD_PC_TO_RDR_TEST_SECURE:
+		case CMD_PC_TO_RDR_OK_SECURE:
+			dev->timeout = CCID_DRIVER_BULK_DEFAULT_TIMEOUT;
+			break;
+
+		case CMD_PC_TO_RDR_ICCPOWERON:
+			dev->timeout = CCID_DRIVER_ASYNC_POWERUP_TIMEOUT;
+			break;
+
+		case CMD_PC_TO_RDR_GETSLOTSTATUS:
+		case CMD_PC_TO_RDR_ICCPOWEROFF:
+		case CMD_PC_TO_RDR_GETPARAMETERS:
+		case CMD_PC_TO_RDR_RESETPARAMETERS:  
+		case CMD_PC_TO_RDR_SETPARAMETERS:
+		case CMD_PC_TO_RDR_ESCAPE:
+		case CMD_PC_TO_RDR_ICCCLOCK:
+		default:
+			dev->timeout = CCID_DRIVER_MINIMUM_TIMEOUT;
+			break;
+	}
+
+	rc = cmx_writeSync(SCR_HOST_TO_READER_START, dev);
+
+	DEBUG(4, "start \n");
+
+	for (i=0; i < uiBytesToWrite; i++) {
+		rc = cmx_waitForBulkOutReady(dev);
+		if (rc != 1) {
+			DEBUG(5, "cmx_waitForBulkOutReady rc=%.2Zx\n", rc);
+			DEBUG(2, "<- cmx_write (failed)\n");
+			return -EIO;
+          	}
+	 
+		xoutb(dev->sBuf[i],iobase + REG_OFFSET_BULK_OUT);
+		DEBUG(4, "%.2x ", dev->sBuf[i]);
+	}
+	DEBUG(4, "end\n");
+
+	rc = cmx_writeSync(SCR_HOST_TO_READER_DONE, dev);
+
+	if (rc != 1) {
+		DEBUG(5, "cmx_writeSync c=%.2Zx\n", rc);
+		DEBUG(2, "<- cmx_write (failed)\n");
+		return -EIO;
+	}
+
+	DEBUG(2, "<- cmx_write (successfully)\n");
+	return count;
+}
+
+static unsigned int cmx_poll(struct file *filp, poll_table *wait)
+{
+	reader_dev_t *dev=(reader_dev_t *)filp->private_data;
+	unsigned int mask = 0;
+
+	poll_wait(filp, &dev->poll_wait, wait);
+
+	if (dev->buffer_status & BS_READABLE)
+		mask |= POLLIN | POLLRDNORM;
+	if (dev->buffer_status & BS_WRITABLE)
+		mask |= POLLOUT | POLLWRNORM;
+
+	return mask;
+}
+
+static int cmx_ioctl(struct inode *inode,struct file *filp,unsigned int cmd,
+			unsigned long arg)
+{
+	dev_link_t *link;
+	int rc, size;
+
+	link=dev_table[MINOR(inode->i_rdev)];
+	if (!(DEV_OK(link))) {
+		DEBUG(4, "DEV_OK false\n");
+		return -ENODEV;
+	}
+	if (_IOC_TYPE(cmd)!=CM_IOC_MAGIC) {
+		DEBUG(4,"ioctype mismatch\n");
+		return -EINVAL;
+	}
+	if (_IOC_NR(cmd)>CM_IOC_MAXNR) {
+		DEBUG(4,"iocnr mismatch\n");
+		return -EINVAL;
+	} 
+	size = _IOC_SIZE(cmd);
+	rc = 0;
+	DEBUG(4,"iocdir=%.4x iocr=%.4x iocw=%.4x iocsize=%d cmd=%.4x\n",
+		_IOC_DIR(cmd),_IOC_READ,_IOC_WRITE,size,cmd);
+
+	if (_IOC_DIR(cmd)&_IOC_READ) {
+		if (!access_ok(VERIFY_WRITE, (void *)arg, size))
+			return -EFAULT;
+	}
+	if (_IOC_DIR(cmd)&_IOC_WRITE) {
+		if (!access_ok(VERIFY_READ, (void *)arg, size))
+			return -EFAULT;
+	}
+
+	return rc;
+}
+
+static int cmx_open (struct inode *inode, struct file *filp)
+{
+	reader_dev_t *dev;
+	dev_link_t *link;
+	int i;
+
+	DEBUG(2, "-> cmx_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);
+	if (i >= CM_MAX_DEV) {
+		DEBUG(4, "MAX_DEV reached\n");
+		DEBUG(4, "<- cmx_open (failure)\n");
+		return -ENODEV;
+	}
+	link = dev_table[MINOR(inode->i_rdev)];
+	if (link == NULL || !(DEV_OK(link))) {
+		DEBUG(4, "link== NULL || DEV_OK false\n");
+		DEBUG(4, "<- cmx_open (failure)\n");
+		return -ENODEV;
+	}
+	if (link->open) {
+		DEBUG(4, "DEVICE BUSY\n");
+		DEBUG(4, "<- cmx_open (failure)\n");
+		return -EBUSY;
+	}
+
+	dev = (reader_dev_t *)link->priv;
+	filp->private_data = dev;
+
+	if (filp->f_flags & O_NONBLOCK) { 
+		DEBUG(4, "filep->f_flags O_NONBLOCK set\n");
+		DEBUG(4, "<- cmx_open (failure)\n");
+		return -EAGAIN;
+        }
+
+	dev->owner = current;
+	link->open = 1;		
+
+	atomic_inc(&cmx_num_devices_open);
+	mod_timer(&cmx_poll_timer, jiffies + POLL_PERIOD);
+
+	DEBUG(2, "<- cmx_open (successfully)\n");
+	return 0;
+}
+
+static int cmx_close(struct inode *inode,struct file *filp)
+{
+	reader_dev_t *dev;
+	dev_link_t *link;
+	int i;
+
+	DEBUG(2, "-> cmx_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 = (reader_dev_t *)link->priv;
+
+	link->open = 0;
+	wake_up(&dev->devq);	
+
+	atomic_dec(&cmx_num_devices_open);
+
+	DEBUG(2, "<- cmx_close\n");
+	return 0;
+}
+
+static void cmx_reader_release(dev_link_t *link)
+{
+	reader_dev_t *dev = (reader_dev_t *)link->priv;
+
+	DEBUG(3, "-> cmx_reader_release\n"); 
+	while (link->open) {
+		DEBUG(3, KERN_INFO MODULE_NAME ": delaying release until "
+		      "process '%s', pid %d has terminated\n",
+		      dev->owner->comm,dev->owner->pid);
+ 		wait_event(dev->devq, (link->open == 0));
+	}
+	DEBUG(3, "<- cmx_reader_release\n"); 
+	return;
+}
+
+static void reader_config(dev_link_t *link, int devno)
+{
+	client_handle_t handle;
+	reader_dev_t *dev;
+	tuple_t tuple;
+	cisparse_t parse;
+	config_info_t conf;
+	u_char buf[64];
+	int fail_fn,fail_rc;
+	int rc;
+
+	DEBUG(2, "-> reader_config\n");
+
+	handle = link->handle;
+
+	tuple.DesiredTuple = CISTPL_CONFIG;
+	tuple.Attributes = 0;
+	tuple.TupleData = buf;
+	tuple.TupleDataMax = sizeof(buf);
+ 	tuple.TupleOffset = 0;
+
+	if ((fail_rc = pcmcia_get_first_tuple(handle,&tuple)) != CS_SUCCESS) {
+		fail_fn = GetFirstTuple;
+		goto cs_failed;
+	}
+	if ((fail_rc = pcmcia_get_tuple_data(handle,&tuple)) != CS_SUCCESS) {
+		fail_fn = GetTupleData;
+		goto cs_failed;
+	}
+	if ((fail_rc = pcmcia_parse_tuple(handle,&tuple,&parse))
+							!= CS_SUCCESS) {
+		fail_fn = ParseTuple;
+		goto cs_failed;
+	}
+	if ((fail_rc = pcmcia_get_configuration_info(handle,&conf))
+							!= CS_SUCCESS) {
+		fail_fn = GetConfigurationInfo;
+		goto cs_failed;
+	}
+
+	link->state |= DEV_CONFIG;
+	link->conf.ConfigBase = parse.config.base;
+	link->conf.Present = parse.config.rmask[0];
+	link->conf.Vcc = conf.Vcc;
+	DEBUG(2, "link->conf.Vcc=%d\n", link->conf.Vcc);
+
+	link->io.BasePort2 = 0;
+	link->io.NumPorts2 = 0;
+	link->io.Attributes2 = 0;
+	tuple.DesiredTuple = CISTPL_CFTABLE_ENTRY;
+	for (rc = pcmcia_get_first_tuple(handle, &tuple);
+	     rc == CS_SUCCESS;
+	     rc = pcmcia_get_next_tuple(handle, &tuple)) {
+ 		DEBUG(2, "Examing CIS Tuple!\n");
+		rc = pcmcia_get_tuple_data(handle, &tuple);
+		if (rc != CS_SUCCESS)
+			continue;
+		rc = pcmcia_parse_tuple(handle, &tuple, &parse);
+		if (rc != CS_SUCCESS)
+			continue;
+
+		DEBUG(2, "tupleIndex=%d\n", parse.cftable_entry.index);
+		link->conf.ConfigIndex = parse.cftable_entry.index;
+		
+		if (parse.cftable_entry.io.nwin) {
+			link->io.BasePort1 = parse.cftable_entry.io.win[0].base;
+			link->io.NumPorts1 = parse.cftable_entry.io.win[0].len;
+			link->io.Attributes1 = IO_DATA_PATH_WIDTH_AUTO;
+			if(!(parse.cftable_entry.io.flags & CISTPL_IO_8BIT))
+				link->io.Attributes1 = IO_DATA_PATH_WIDTH_16;
+			if(!(parse.cftable_entry.io.flags & CISTPL_IO_16BIT))
+				link->io.Attributes1 = IO_DATA_PATH_WIDTH_8;
+			link->io.IOAddrLines = parse.cftable_entry.io.flags
+						& CISTPL_IO_LINES_MASK;
+			DEBUG(2,"io.BasePort1=%.4x\n", link->io.BasePort1);
+			DEBUG(2,"io.NumPorts1=%.4x\n", link->io.NumPorts1);
+			DEBUG(2,"io.BasePort2=%.4x\n", link->io.BasePort2);
+			DEBUG(2,"io.NumPorts2=%.4x\n", link->io.NumPorts2);
+			DEBUG(2,"io.IOAddrLines=%.4x\n", 
+			      link->io.IOAddrLines);
+			rc = pcmcia_request_io(handle, &link->io);
+			if (rc == CS_SUCCESS) {
+				DEBUG(2, "RequestIO OK\n");
+				break; 
+			} else 
+				DEBUG(2, "RequestIO failed\n");
+		}
+	}
+	if (rc != CS_SUCCESS) {
+		DEBUG(2, "Couldn't configure reader\n");
+		goto cs_release;
+	}
+
+        link->conf.IntType = 00000002;
+
+	if ((fail_rc = pcmcia_request_configuration(handle,&link->conf))
+								!=CS_SUCCESS) {
+		fail_fn = RequestConfiguration;
+		DEBUG(1, "pcmcia_request_configuration failed 0x%x\n", fail_rc);
+		goto cs_release;
+	}
+
+	DEBUG(2, "RequestConfiguration OK\n");
+
+	dev = link->priv;
+	sprintf(dev->node.dev_name, DEVICE_NAME "%d", devno);
+	dev->node.major = major;
+	dev->node.minor = devno;
+	dev->node.next = NULL;
+	link->dev = &dev->node;
+	link->state &= ~DEV_CONFIG_PENDING;
+
+	DEBUG(2, "device " DEVICE_NAME "%d at 0x%.4x-0x%.4x\n", devno,
+	      link->io.BasePort1, link->io.BasePort1+link->io.NumPorts1);
+	DEBUG(2, "<- reader_config (succ)\n");
+
+	return;
+
+cs_failed:
+	cs_error(handle, fail_fn, fail_rc);
+cs_release:
+	reader_release((u_long)link);
+	link->state &= ~DEV_CONFIG_PENDING;
+	DEBUG(2, "<- reader_config (failure)\n");
+}
+
+static int reader_event(event_t event, int priority,
+			event_callback_args_t *args)
+{
+	dev_link_t *link;
+	reader_dev_t *dev;
+	int devno;
+
+	DEBUG(3,"-> reader_event\n");
+	link = args->client_data;
+	dev = link->priv;
+	for (devno = 0; devno < CM_MAX_DEV; devno++) {
+		if (dev_table[devno]==link)
+			break;
+	}
+	if (devno == CM_MAX_DEV)
+		return CS_BAD_ADAPTER;
+
+	switch (event) {
+		case CS_EVENT_CARD_INSERTION:
+			DEBUG(5, "CS_EVENT_CARD_INSERTION\n");
+			link->state |= DEV_PRESENT | DEV_CONFIG_PENDING;
+			reader_config(link,devno);
+			break;
+		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;
+		
+		case CS_EVENT_CARD_RESET:
+	        	DEBUG(5, "CS_EVENT_CARD_RESET\n");
+			if ((link->state & DEV_CONFIG)) {
+				DEBUG(5, "cmx: RequestConfiguration\n");
+		  		pcmcia_request_configuration(link->handle,
+							     &link->conf);
+			}
+			break;
+		default:
+			DEBUG(5, "reader_event: unknown event %.2x\n", event);
+			break;
+	}
+	DEBUG(3, "<- reader_event\n");
+	return CS_SUCCESS;
+}
+
+static void reader_release(u_long arg)
+{
+	dev_link_t *link;
+	int rc;
+
+	DEBUG(3, "-> reader_release\n");
+	link = (dev_link_t *)arg;
+	cmx_reader_release(link->priv); 
+	rc = pcmcia_release_configuration(link->handle);
+	if (rc != CS_SUCCESS)
+		DEBUG(5, "couldn't ReleaseConfiguration "
+		      "reasoncode=%.2x\n", rc);
+	rc = pcmcia_release_io(link->handle, &link->io);
+	if (rc != CS_SUCCESS)
+		DEBUG(5, "couldn't ReleaseIO reasoncode=%.2x\n", rc);
+
+	DEBUG(3, "<- reader_release\n");
+}
+
+static dev_link_t *reader_attach(void)
+{
+	reader_dev_t *dev;
+	dev_link_t *link;
+	client_reg_t client_reg;
+	int i;
+
+	DEBUG(3, "reader_attach\n");
+	for (i=0; i < CM_MAX_DEV; i++) {
+		if (dev_table[i] == NULL)
+			break;
+	}
+
+	if (i == CM_MAX_DEV) {
+		printk(KERN_NOTICE "all devices in use\n");
+		return NULL;
+	}
+	
+	DEBUG(5, "create reader device instance\n");
+	dev = kmalloc(sizeof(reader_dev_t), GFP_KERNEL);
+	if (dev == NULL)
+		return NULL;
+
+	memset(dev, 0, sizeof(reader_dev_t));
+	dev->timeout = CCID_DRIVER_MINIMUM_TIMEOUT;
+	dev->fTimerExpired = 0;
+
+	link = &dev->link;
+	link->priv = dev;
+
+	link->conf.IntType = INT_MEMORY_AND_IO;
+	dev_table[i] = link;
+
+	
+	DEBUG(5, "Register with Card Services\n");
+	client_reg.dev_info = &dev_info;
+	client_reg.Attributes = INFO_IO_CLIENT | INFO_CARD_SHARE;
+	client_reg.EventMask=
+		CS_EVENT_CARD_INSERTION | CS_EVENT_CARD_REMOVAL |
+		CS_EVENT_RESET_PHYSICAL | CS_EVENT_CARD_RESET |
+		CS_EVENT_PM_SUSPEND | CS_EVENT_PM_RESUME;
+	client_reg.Version = 0x0210;
+	client_reg.event_callback_args.client_data = link;
+	i = pcmcia_register_client(&link->handle, &client_reg);
+	if (i) {
+		cs_error(link->handle, RegisterClient, i);
+		reader_detach(link);
+		return NULL;
+	}
+	init_waitqueue_head(&dev->devq);
+	init_waitqueue_head(&dev->poll_wait);
+	init_waitqueue_head(&dev->read_wait);
+	init_waitqueue_head(&dev->write_wait);
+	init_timer(&cmx_poll_timer);
+	cmx_poll_timer.function = &cmx_do_poll;
+
+	return link;
+}
+
+static void reader_detach_by_devno(int devno,dev_link_t *link)
+{
+	reader_dev_t *dev=link->priv;
+
+	DEBUG(3, "-> detach_by_devno(devno=%d)\n", devno);
+	if (link->state & DEV_CONFIG) {
+		DEBUG(5, "device still configured (try to release it)\n");
+		reader_release((u_long)link);
+	}
+
+	pcmcia_deregister_client(link->handle);
+	dev_table[devno] = NULL;
+	DEBUG(5, "freeing dev=%p\n", dev);
+	kfree(dev);
+	DEBUG(3, "<- detach_by-devno\n");
+	return;
+}
+
+static void reader_detach(dev_link_t *link)
+{
+	int i;
+	DEBUG(3, "-> reader_detach(link=%p\n", link);
+	/* find device */
+	for(i=0; i < CM_MAX_DEV; i++) {
+		if (dev_table[i] == link)
+			break;
+	}
+	if (i == CM_MAX_DEV) {
+		printk(KERN_WARNING MODULE_NAME 
+			": detach for unkown device aborted\n");
+		return;
+	}
+	reader_detach_by_devno(i, link);
+	DEBUG(3, "<- reader_detach\n");
+	return;
+}
+
+static struct file_operations reader_fops = {
+	.owner		= THIS_MODULE,
+	.llseek		= cmx_llseek,
+	.read		= cmx_read,
+	.write		= cmx_write,
+	.ioctl		= cmx_ioctl,
+	.open		= cmx_open,
+	.release	= cmx_close,
+	.poll		= cmx_poll,
+};
+
+static struct pcmcia_device_id cm4040_ids[] = {
+	PCMCIA_DEVICE_MANF_CARD(0x0223, 0x0200),
+	PCMCIA_DEVICE_PROD_ID12("OMNIKEY", "CardMan 4040", 
+				0xE32CDD8C, 0x8F23318B),
+	PCMCIA_DEVICE_NULL,
+};
+MODULE_DEVICE_TABLE(pcmcia, cm4040_ids);
+
+static struct pcmcia_driver reader_driver = {
+  	.owner		= THIS_MODULE,
+  	.drv		= {
+		.name	= "cm4040_cs",
+	},
+	.attach		= reader_attach,
+	.detach		= reader_detach,
+	.event		= reader_event,
+	.id_table	= cm4040_ids,
+};
+
+static int __init cmx_init(void)
+{
+	printk(KERN_INFO "%s\n", version);
+	pcmcia_register_driver(&reader_driver);
+	major = register_chrdev(0, DEVICE_NAME, &reader_fops);
+	if (major < 0) {
+		printk(KERN_WARNING MODULE_NAME
+			": could not get major number\n");
+		return -1;
+	}
+	return 0;
+}
+
+static void __exit cmx_exit(void)
+{
+	int i;
+
+	printk(KERN_INFO MODULE_NAME ": unloading\n");
+	pcmcia_unregister_driver(&reader_driver);  
+	for (i=0; i < CM_MAX_DEV; i++) {
+		if (dev_table[i])
+			reader_detach_by_devno(i, dev_table[i]);
+	}
+	unregister_chrdev(major, DEVICE_NAME);
+}
+
+module_init(cmx_init);
+module_exit(cmx_exit);
+MODULE_LICENSE("Dual BSD/GPL");
diff --git a/drivers/char/pcmcia/cm4040_cs.h b/drivers/char/pcmcia/cm4040_cs.h
new file mode 100644
--- /dev/null
+++ b/drivers/char/pcmcia/cm4040_cs.h
@@ -0,0 +1,52 @@
+#ifndef	_CM4040_H_
+#define	_CM4040_H_
+
+#define	CM_MAX_DEV		4
+
+#define	CM_IOC_MAGIC		'c'
+#define	CM_IOC_MAXNR	        255
+
+#define CM_IOSDBGLVL            _IOW(CM_IOC_MAGIC, 250, int*) 
+
+#define	DEVICE_NAME		"cmx"
+#define	MODULE_NAME		"cm4040_cs"
+
+#define REG_OFFSET_BULK_OUT      0
+#define REG_OFFSET_BULK_IN       0
+#define REG_OFFSET_BUFFER_STATUS 1
+#define REG_OFFSET_SYNC_CONTROL  2
+
+#define BSR_BULK_IN_FULL  0x02
+#define BSR_BULK_OUT_FULL 0x01
+
+#define SCR_HOST_TO_READER_START 0x80
+#define SCR_ABORT                0x40
+#define SCR_EN_NOTIFY            0x20
+#define SCR_ACK_NOTIFY           0x10
+#define SCR_READER_TO_HOST_DONE  0x08
+#define SCR_HOST_TO_READER_DONE  0x04
+#define SCR_PULSE_INTERRUPT      0x02
+#define SCR_POWER_DOWN           0x01
+
+
+#define  CMD_PC_TO_RDR_ICCPOWERON       0x62
+#define  CMD_PC_TO_RDR_GETSLOTSTATUS    0x65
+#define  CMD_PC_TO_RDR_ICCPOWEROFF      0x63
+#define  CMD_PC_TO_RDR_SECURE           0x69
+#define  CMD_PC_TO_RDR_GETPARAMETERS    0x6C
+#define  CMD_PC_TO_RDR_RESETPARAMETERS  0x6D
+#define  CMD_PC_TO_RDR_SETPARAMETERS    0x61
+#define  CMD_PC_TO_RDR_XFRBLOCK         0x6F
+#define  CMD_PC_TO_RDR_ESCAPE           0x6B
+#define  CMD_PC_TO_RDR_ICCCLOCK         0x6E
+#define  CMD_PC_TO_RDR_TEST_SECURE      0x74
+#define  CMD_PC_TO_RDR_OK_SECURE        0x89
+
+
+#define  CMD_RDR_TO_PC_SLOTSTATUS         0x81
+#define  CMD_RDR_TO_PC_DATABLOCK          0x80
+#define  CMD_RDR_TO_PC_PARAMETERS         0x82
+#define  CMD_RDR_TO_PC_ESCAPE             0x83
+#define  CMD_RDR_TO_PC_OK_SECURE          0x89
+
+#endif	/* _CM4040_H_ */

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] New: Omnikey CardMan 4040 PCMCIA Driver

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 1573 bytes --]

On Sat, Sep 03, 2005 at 04:27:00PM -0500, Chase Venters wrote:
> > Below you can find a driver for the Omnikey CardMan 4040 PCMCIA
> > Smartcard Reader.
> 
> Someone correct me if I'm wrong, but wouldn't these #defines be a problem with 
> the new HZ flexibility:
> 
> #define	CCID_DRIVER_BULK_DEFAULT_TIMEOUT  	(150*HZ)
> #define	CCID_DRIVER_ASYNC_POWERUP_TIMEOUT 	(35*HZ)
> #define	CCID_DRIVER_MINIMUM_TIMEOUT 		(3*HZ)

The defines above certainly have no problems.  They want to wait for
multiples of seconds.

> /* how often to poll for fifo status change */
> #define POLL_PERIOD 				(HZ/100)
> 
> In particular, 2.6.13 allows a HZ of 100, which would define POLL_PERIOD to 0. 
> Your later calls to mod_timer would be setting cmx_poll_timer to the current 
> value of jiffies. 

100/100 == 1.  As far as my limited math skills go, only 0 divided by
something can give a result of zero ;)

So yes, the code would poll every 1/100th of a second, even with HZ=100.  

Obviously, if HZ would ever go below 100, the code above would provide
some problems.  I'm not sure what the future plans with HZ are, but I'll
add an #error statement in case HZ goes smaller than that.

> Also, you've got a typo in the comments:

thanks.

-- 
- Harald Welte <laforge@gnumonks.org>          	        http://gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] New: Omnikey CardMan 4040 PCMCIA Driver

[Ingo Oeser]

[-- Attachment #1: Type: text/plain, Size: 425 bytes --]

On Sunday 04 September 2005 12:12, Harald Welte wrote:
> cmx_llseek

just use

	return nonseekable_open(inode, filp);

as your last statement in cmx_open() instead of

	return 0;

to really disable any file pointer positioning (e.g. pwrite/pread too).

Addtionally cmx_llseek() is implement already as "no_llseek()"
by the VFS, so you delete it from the driver an use no_llseek() from
the VFS instead.


Regards

Ingo Oeser


[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] New: Omnikey CardMan 4040 PCMCIA Driver

[Horst von Brand]

Jesper Juhl <jesper.juhl@gmail.com> wrote:
> On 9/4/05, Harald Welte <laforge@gnumonks.org> wrote:
> > On Sun, Sep 04, 2005 at 12:12:18PM +0200, Harald Welte wrote:
> > > Hi!
> > >
> > > Below you can find a driver for the Omnikey CardMan 4040 PCMCIA
> > > Smartcard Reader.
> > 
> > Sorry, the patch was missing a "cg-add" of the header file.  Please use
> > the patch below.
> 
> It would be so much nicer if the patch actually was "below" - that is
> "inline in the email as opposed to as an attachment". Having to first
> save an attachment and then cut'n'paste from it is a pain.
> 
> Anyway, a few comments below :

[...]

> +	unsigned long ulBytesToRead;
> 
> 
> lowercase prefered also for variables.

Also, "encoding" the type (ul) into the variable name is nonsense.

[...]

> +	ulMin = (count < (ulBytesToRead+5))?count:(ulBytesToRead+5);

Again.
-- 
Dr. Horst H. von Brand                   User #22616 counter.li.org
Departamento de Informatica                     Fono: +56 32 654431
Universidad Tecnica Federico Santa Maria              +56 32 654239
Casilla 110-V, Valparaiso, Chile                Fax:  +56 32 797513

Re: [PATCH] New: Omnikey CardMan 4040 PCMCIA Driver

[Jesper Juhl]

On 9/4/05, Horst von Brand <vonbrand@inf.utfsm.cl> wrote:
> Jesper Juhl <jesper.juhl@gmail.com> wrote:
> > On 9/4/05, Harald Welte <laforge@gnumonks.org> wrote:
> > > On Sun, Sep 04, 2005 at 12:12:18PM +0200, Harald Welte wrote:
> > > > Hi!
> > > >
> > > > Below you can find a driver for the Omnikey CardMan 4040 PCMCIA
> > > > Smartcard Reader.
> > >
> > > Sorry, the patch was missing a "cg-add" of the header file.  Please use
> > > the patch below.
> >
> > It would be so much nicer if the patch actually was "below" - that is
> > "inline in the email as opposed to as an attachment". Having to first
> > save an attachment and then cut'n'paste from it is a pain.
> >
> > Anyway, a few comments below :
> 
> [...]
> 
> > +     unsigned long ulBytesToRead;
> >
> >
> > lowercase prefered also for variables.
> 
> Also, "encoding" the type (ul) into the variable name is nonsense.
> 
Agreed, and it's even mentioned in CodingStyle (ok, it talks about
functions, but the same goes for variables):

...
Encoding the type of a function into the name (so-called Hungarian
notation) is brain damaged - the compiler knows the types anyway and can
check those, and it only confuses the programmer.  No wonder MicroSoft
makes buggy programs.

LOCAL variable names should be short, and to the point.  If you have
some random integer loop counter, it should probably be called "i".
Calling it "loop_counter" is non-productive, if there is no chance of it
being mis-understood.  Similarly, "tmp" can be just about any type of
variable that is used to hold a temporary value.
...



-- 
Jesper Juhl <jesper.juhl@gmail.com>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please      http://www.expita.com/nomime.html

Re: [PATCH] New: Omnikey CardMan 4040 PCMCIA Driver

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 786 bytes --]

On Sun, Sep 04, 2005 at 02:58:27PM +0200, Ingo Oeser wrote:

> just use
> 	return nonseekable_open(inode, filp);
> 
> to really disable any file pointer positioning (e.g. pwrite/pread too).
> 
> Addtionally cmx_llseek() is implement already as "no_llseek()"
> by the VFS, so you delete it from the driver an use no_llseek() from
> the VFS instead.

great, thanks,  I've merged your suggested changes into my local tree.
Stay tuned for a re-submit later today.

-- 
- Harald Welte <laforge@gnumonks.org>          	        http://gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] New: Omnikey CardMan 4040 PCMCIA Driver

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 3103 bytes --]

On Sun, Sep 04, 2005 at 12:27:20AM +0200, Jesper Juhl wrote:
> On 9/4/05, Harald Welte <laforge@gnumonks.org> wrote:
> > On Sun, Sep 04, 2005 at 12:12:18PM +0200, Harald Welte wrote:
> > > Hi!
> > >
> > > Below you can find a driver for the Omnikey CardMan 4040 PCMCIA
> > > Smartcard Reader.
> > 
> > Sorry, the patch was missing a "cg-add" of the header file.  Please use
> > the patch below.
> 
> It would be so much nicer if the patch actually was "below" - that is
> "inline in the email as opposed to as an attachment". Having to first
> save an attachment and then cut'n'paste from it is a pain.

This is a neverending discussion.  A number of kernel develpoers I know
prefer attachements these days.  It's a matter of your email client, and
why it doesn't just append a "plaintext" attachment at the bottom of
your mail and rather display you the "save as" icon/button/whatever.

> Anyway, a few comments below :
> 
> +#define DEBUG(n, x, args...) do { if (pc_debug >= (n)) 			    \
> 
> 
> line longer than 80 chars. Please adhere to CodingStyle and keep lines
> <80 chars.

The line is _not_ longer than 80 chars, at least not if you remove the
"+" from the beginning of the patch and you hve 8 chars wide

> There's more than one occourance of this.

there was one occurrence in the file which I have missed, thanks.

> +static inline int cmx_waitForBulkOutReady(reader_dev_t *dev)
> 
> 
> Why TheStudlyCaps ?  Please keep function names lowercase. There are
> more instances of this, only pointing out one.

Yes, there are.  My initial idea was to diverge as little as possible
from the original vendor driver, making it easy to pull in changes from
their tree in the future, should it be neccessarry.

However, it has diverged that much now, it doesn't really matter whether
I also rename the functions, too.  Please stay tuned for the next
revision of the patch.

> Please use only tabs for indentation (line 1 of the above is indented
> with spaces).

thanks, should have catched all of them now.

> lowercase prefered also for variables.

done

> Space after ","s please : DEBUG(5, "cmx_waitForBulkInReady rc=%.2x\n", rc);

done

> get rid of the space before the opening paren : 
> static int cmx_open(struct inode *inode, struct file *filp)

done

> How about not having to indent so deep by rewriting that as

done

> +	cmx_poll_timer.function = &cmx_do_poll;
> 
> shouldn't this be 
> 	 cmx_poll_timer.function = cmx_do_poll;
> ???

I don't really know what difference it would make.  My understanding of
'c' is that both cases would take the address of the function.

My personal taste is rather to explicitly indicate this with an '&' than
let the compiler implicitly take the address.

-- 
- Harald Welte <laforge@gnumonks.org>          	        http://gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] New: Omnikey CardMan 4040 PCMCIA Driver

[Roland Dreier]

    Harald> Obviously, if HZ would ever go below 100, the code above
    Harald> would provide some problems.  I'm not sure what the future
    Harald> plans with HZ are, but I'll add an #error statement in
    Harald> case HZ goes smaller than that.

It might be simpler just to define it to msecs_to_jiffies(10).

 - R.

Re: [PATCH] New: Omnikey CardMan 4040 PCMCIA Driver

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 778 bytes --]

On Tue, Sep 06, 2005 at 09:15:10AM -0700, Roland Dreier wrote:
>     Harald> Obviously, if HZ would ever go below 100, the code above
>     Harald> would provide some problems.  I'm not sure what the future
>     Harald> plans with HZ are, but I'll add an #error statement in
>     Harald> case HZ goes smaller than that.
> 
> It might be simpler just to define it to msecs_to_jiffies(10).

That's what I did in the last version that was posted to lkml ;)

-- 
- Harald Welte <laforge@gnumonks.org>          	        http://gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]