Re: [PATCH] char/pcmcia: add scr24x_cs chip card interface driver

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Mon, Oct 10, 2016 at 05:58:15PM +0200, Lubomir Rintel wrote:
> This implements only the very basic protocol "Mode A", just to make the
> device functional. Patches to implement "Mode C" that uses better bulking
> and is interrupt-driver may follow.
> 
> The device essentially speaks the same protocol as USB CCID devices do over
> the bulk endpoints. The driver exchanges the command submissions and
> responses over a plain read()/write() interface, compatible with legacy
> OpenCT's pcmcia_block driver.
> 
> Patches for the newer CCID driver are available:
> https://github.com/lkundrak/CCID/tree/lr/pcmcia_block
> 
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> ---
>  MAINTAINERS                     |   5 +
>  drivers/char/pcmcia/Kconfig     |  11 ++
>  drivers/char/pcmcia/Makefile    |   1 +
>  drivers/char/pcmcia/scr24x_cs.c | 357 ++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 374 insertions(+)
>  create mode 100644 drivers/char/pcmcia/scr24x_cs.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5327bbe..8dc6a9f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10608,6 +10608,11 @@ W:	http://www.sunplus.com
>  S:	Supported
>  F:	arch/score/
>  
> +SCR24X CHIP CARD INTERFACE DRIVER
> +M:	Lubomir Rintel <lkundrak@v3.sk>
> +S:	Supported
> +F:	drivers/char/pcmcia/scr24x_cs.c
> +
>  SYSTEM CONTROL & POWER INTERFACE (SCPI) Message Protocol drivers
>  M:	Sudeep Holla <sudeep.holla@arm.com>
>  L:	linux-arm-kernel@lists.infradead.org
> diff --git a/drivers/char/pcmcia/Kconfig b/drivers/char/pcmcia/Kconfig
> index 8d3dfb0..1d1e7da 100644
> --- a/drivers/char/pcmcia/Kconfig
> +++ b/drivers/char/pcmcia/Kconfig
> @@ -43,6 +43,17 @@ config CARDMAN_4040
>  	  (http://www.omnikey.com/), or a current development version of OpenCT
>  	  (http://www.opensc-project.org/opensc).
>  
> +config SCR24X
> +	tristate "SCR24x Chip Card Interface support"
> +	depends on PCMCIA
> +	help
> +	  Enable support for the SCR24x PCMCIA Chip Card Interface.
> +
> +	  To compile this driver as a module, choose M here.
> +	  The module will be called scr24x_cs..
> +
> +	  If unsure say N.

A new PCMCIA driver?  What decade is this?  :)

> +
>  config IPWIRELESS
>  	tristate "IPWireless 3G UMTS PCMCIA card support"
>  	depends on PCMCIA && NETDEVICES && TTY
> diff --git a/drivers/char/pcmcia/Makefile b/drivers/char/pcmcia/Makefile
> index 0aae209..5b836bc 100644
> --- a/drivers/char/pcmcia/Makefile
> +++ b/drivers/char/pcmcia/Makefile
> @@ -7,3 +7,4 @@
>  obj-$(CONFIG_SYNCLINK_CS) += synclink_cs.o
>  obj-$(CONFIG_CARDMAN_4000) += cm4000_cs.o
>  obj-$(CONFIG_CARDMAN_4040) += cm4040_cs.o
> +obj-$(CONFIG_SCR24X) += scr24x_cs.o
> diff --git a/drivers/char/pcmcia/scr24x_cs.c b/drivers/char/pcmcia/scr24x_cs.c
> new file mode 100644
> index 0000000..24379ac
> --- /dev/null
> +++ b/drivers/char/pcmcia/scr24x_cs.c
> @@ -0,0 +1,357 @@
> +/*
> + * SCR24x PCMCIA Smart Card Reader Driver
> + *
> + * Copyright (C) 2005-2006 TL Sudheendran
> + * Copyright (C) 2016 Lubomir Rintel
> + *
> + * Derived from "scr24x_v4.2.6_Release.tar.gz" driver by TL Sudheendran.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2, or (at your option)
> + * any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; see the file COPYING.  If not, write to
> + * the Free Software Foundation, 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/delay.h>
> +#include <linux/cdev.h>
> +
> +#include <pcmcia/cistpl.h>
> +#include <pcmcia/ds.h>
> +
> +#define CCID_HEADER_SIZE	10
> +#define CCID_LENGTH_OFFSET	1
> +#define CCID_MAX_LEN		271
> +
> +#define SCR24X_DATA(n)		(1 + n)
> +#define SCR24X_CMD_STATUS	7
> +#define CMD_START		0x40
> +#define CMD_WRITE_BYTE		0x41
> +#define CMD_READ_BYTE		0x42
> +#define STATUS_BUSY		0x80
> +
> +struct scr24x_dev {
> +	struct device *dev;
> +	struct cdev c_dev;
> +	unsigned char buf[CCID_MAX_LEN];
> +	int devno;
> +	struct mutex lock;
> +	struct kref refcnt;
> +	u8 __iomem *regs;
> +};
> +
> +#define SCR24X_DEVS 8
> +static DECLARE_BITMAP(scr24x_minors, SCR24X_DEVS);
> +
> +static struct class *scr24x_class;
> +dev_t scr24x_devt;

global variable?  And why do you need a char device for this type of
hardware?  Isn't there already an existing interface for this device
class?


> +
> +static void scr24x_delete(struct kref *kref)
> +{
> +	struct scr24x_dev *dev = container_of(kref, struct scr24x_dev,
> +								refcnt);
> +
> +	kfree(dev);
> +}
> +
> +static int scr24x_wait_ready(struct scr24x_dev *dev)
> +{
> +	u_char status;
> +	int timeout = 100;
> +
> +	do {
> +		status = ioread8(dev->regs + SCR24X_CMD_STATUS);
> +		if (!(status & STATUS_BUSY))
> +			return 0;
> +
> +		msleep(20);
> +	} while (--timeout);
> +
> +	return -EIO;
> +}
> +
> +static int scr24x_open(struct inode *inode, struct file *filp)
> +{
> +	struct scr24x_dev *dev = container_of(inode->i_cdev,
> +				struct scr24x_dev, c_dev);
> +
> +	kref_get(&dev->refcnt);
> +	filp->private_data = dev;
> +
> +	return nonseekable_open(inode, filp);
> +}
> +
> +static int scr24x_release(struct inode *inode, struct file *filp)
> +{
> +	struct scr24x_dev *dev = filp->private_data;
> +
> +	kref_put(&dev->refcnt, scr24x_delete);

No locking?

thanks,

greg k-h