public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@osdl.org>
To: Harald Welte <laforge@gnumonks.org>
Cc: linux-kernel@vger.kernel.org, torvalds@osdl.org
Subject: Re: [PATCH 2/2] New Omnikey Cardman 4000 driver
Date: Wed, 14 Sep 2005 02:23:14 -0700	[thread overview]
Message-ID: <20050914022314.35eab48d.akpm@osdl.org> (raw)
In-Reply-To: <20050913155333.GZ29695@sunbeam.de.gnumonks.org>

Harald Welte <laforge@gnumonks.org> wrote:
>
> Add new Omnikey Cardman 4000 smartcard reader driver

- All the open-coded mdelays() are wrong:

  #define	T_10MSEC	msecs_to_jiffies(10)
  ...
		mdelay(T_10MSEC);

  mdelay() already takes a jiffies argument.

- terminate_monitor() should use del_timer_sync().



Plus:

- Work around gcc-2.95.x macro expansion bug

- unneeded initialisation of static (yes, it's a bit sleazy, but it shrinks
  the on-disk kernel image)

- Use ARRAY_SIZE

- Unneeded void* typecasts

- Use kzalloc()


diff -puN drivers/char/pcmcia/cm4000_cs.c~new-omnikey-cardman-4000-driver-fixes drivers/char/pcmcia/cm4000_cs.c
--- devel/drivers/char/pcmcia/cm4000_cs.c~new-omnikey-cardman-4000-driver-fixes	2005-09-14 02:20:26.000000000 -0700
+++ devel-akpm/drivers/char/pcmcia/cm4000_cs.c	2005-09-14 02:20:26.000000000 -0700
@@ -51,7 +51,7 @@ module_param(pc_debug, int, 0600);
 #define DEBUGP(n, rdr, x, args...) do { 				\
 	if (pc_debug >= (n))						\
 		dev_printk(KERN_DEBUG, reader_to_dev(rdr), "%s:" x, 	\
-			   __FUNCTION__, ## args);			\
+			   __FUNCTION__ , ## args);			\
 	} while (0)
 #else
 #define DEBUGP(n, rdr, x, args...)
@@ -157,7 +157,7 @@ struct cm4000_dev {
 		/*queue*/ 4*sizeof(wait_queue_head_t))
 
 static dev_info_t dev_info = MODULE_NAME;
-static dev_link_t *dev_table[CM4000_MAX_DEV] = { NULL, };
+static dev_link_t *dev_table[CM4000_MAX_DEV];
 
 /* This table doesn't use spaces after the comma between fields and thus
  * violates CodingStyle.  However, I don't really think wrapping it around will
@@ -470,7 +470,7 @@ static void set_cardparameter(struct cm4
 	      ((dev->baudv - 1) & 0xFF));
 
 	/* set stopbits */
-	for (i = 0; i < sizeof(card_fixups)/sizeof(struct card_fixup); i++) {
+	for (i = 0; i < ARRAY_SIZE(card_fixups); i++) {
 		if (!memcmp(dev->atr, card_fixups[i].atr,
 			    card_fixups[i].atr_len))
 			stopbits = card_fixups[i].stopbits;
@@ -502,9 +502,8 @@ static int set_protocol(struct cm4000_de
 	dev->pts[0] = 0xff;
 	dev->pts[1] = 0x00;
 	tmp = ptsreq->protocol;
-	while ((tmp = (tmp >> 1)) > 0) {
+	while ((tmp = (tmp >> 1)) > 0)
 		dev->pts[1]++;
-	}
 	dev->proto = dev->pts[1];	/* Set new protocol */
 	dev->pts[1] = (0x01 << 4) | (dev->pts[1]);
 
@@ -962,7 +961,7 @@ return_with_timer:
 static ssize_t cmm_read(struct file *filp, __user char *buf, size_t count,
 			loff_t *ppos)
 {
-	struct cm4000_dev *dev = (struct cm4000_dev *) filp->private_data;
+	struct cm4000_dev *dev = filp->private_data;
 	ioaddr_t iobase = dev->link.io.BasePort1;
 	ssize_t rc;
 	int i, j, k;
@@ -1440,7 +1439,7 @@ static void stop_monitor(struct cm4000_d
 static int cmm_ioctl(struct inode *inode, struct file *filp, unsigned int cmd,
 		     unsigned long arg)
 {
-	struct cm4000_dev *dev = (struct cm4000_dev *) filp->private_data;
+	struct cm4000_dev *dev = filp->private_data;
 	ioaddr_t iobase = dev->link.io.BasePort1;
 	dev_link_t *link;
 	int size;
@@ -1673,7 +1672,7 @@ static int cmm_open(struct inode *inode,
 	if (link->open)
 		return -EBUSY;
 
-	dev = (struct cm4000_dev *) link->priv;
+	dev = link->priv;
 	filp->private_data = dev;
 
 	DEBUGP(2, dev, "-> cmm_open(device=%d.%d process=%s,%d)\n",
@@ -1719,7 +1718,7 @@ static int cmm_close(struct inode *inode
 	if (link == NULL)
 		return -ENODEV;
 
-	dev = (struct cm4000_dev *) link->priv;
+	dev = link->priv;
 
 	DEBUGP(2, dev, "-> cmm_close(maj/min=%d.%d)\n",
 	       imajor(inode), minor);
@@ -1737,7 +1736,7 @@ static int cmm_close(struct inode *inode
 
 static void cmm_cm4000_release(dev_link_t * link)
 {
-	struct cm4000_dev *dev = (struct cm4000_dev *) link->priv;
+	struct cm4000_dev *dev = link->priv;
 
 	/* dont terminate the monitor, rather rely on
 	 * close doing that for us.
@@ -1952,11 +1951,10 @@ static dev_link_t *cm4000_attach(void)
 	}
 
 	/* create a new cm4000_cs device */
-	dev = kmalloc(sizeof(struct cm4000_dev), GFP_KERNEL);
+	dev = kzalloc(sizeof(struct cm4000_dev), GFP_KERNEL);
 	if (dev == NULL)
 		return NULL;
 
-	memset(dev, 0, sizeof(struct cm4000_dev));
 	link = &dev->link;
 	link->priv = dev;
 	link->conf.IntType = INT_MEMORY_AND_IO;
_


  reply	other threads:[~2005-09-14  9:23 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-09-13 15:53 [PATCH 2/2] New Omnikey Cardman 4000 driver Harald Welte
2005-09-14  9:23 ` Andrew Morton [this message]
2005-09-14 11:20   ` Stefan Smietanowski
2005-09-14 11:42     ` Arjan van de Ven
2005-09-18 21:22   ` [PATCH] Omnikey CardMan 4000 update Harald Welte

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20050914022314.35eab48d.akpm@osdl.org \
    --to=akpm@osdl.org \
    --cc=laforge@gnumonks.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@osdl.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox