> +#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/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
| |
 |