2018-07-19 08:52:43

by Ludovic Desroches

[permalink] [raw]
Subject: [PATCH v2 0/2] add ISO7816 support

Hi,

This patchset adds support for the ISO7816 standard. The USART devices in
Microchip SoCs have an ISO7816 mode. It allows to let the USART managing
the CLK and I/O signals of a smart card.

Changes:
- v2
- uart_get_iso7816_config: check there is an iso7816_config function
- use IOCTL macros to generate the IOCTL number
- check that reserved field is not used
- remove debug logs
- check that the iso7816_config is right before doing any action
- change the error from nack and max iteration status to a debug message
- remove patch 3 as it concerns both rs485 and iso7816 to think more
about the need of adding a lock or not

Nicolas Ferre (2):
tty/serial_core: add ISO7816 infrastructure
tty/serial: atmel: add ISO7816 support

arch/alpha/include/uapi/asm/ioctls.h | 2 +
arch/mips/include/uapi/asm/ioctls.h | 2 +
arch/powerpc/include/uapi/asm/ioctls.h | 2 +
arch/sh/include/uapi/asm/ioctls.h | 2 +
arch/sparc/include/uapi/asm/ioctls.h | 2 +
arch/xtensa/include/uapi/asm/ioctls.h | 2 +
drivers/tty/serial/atmel_serial.c | 170 +++++++++++++++++++++++++++++++--
drivers/tty/serial/atmel_serial.h | 3 +-
drivers/tty/serial/serial_core.c | 60 ++++++++++++
include/linux/serial_core.h | 3 +
include/uapi/asm-generic/ioctls.h | 2 +
include/uapi/linux/serial.h | 17 ++++
12 files changed, 256 insertions(+), 11 deletions(-)

--
2.12.2



2018-07-19 08:52:12

by Ludovic Desroches

[permalink] [raw]
Subject: [PATCH v2 1/2] tty/serial_core: add ISO7816 infrastructure

From: Nicolas Ferre <[email protected]>

Add the ISO7816 ioctl and associated accessors and data structure.
Drivers can then use this common implementation to handle ISO7816.

Signed-off-by: Nicolas Ferre <[email protected]>
Signed-off-by: Ludovic Desroches <[email protected]>
---
arch/alpha/include/uapi/asm/ioctls.h | 2 ++
arch/mips/include/uapi/asm/ioctls.h | 2 ++
arch/powerpc/include/uapi/asm/ioctls.h | 2 ++
arch/sh/include/uapi/asm/ioctls.h | 2 ++
arch/sparc/include/uapi/asm/ioctls.h | 2 ++
arch/xtensa/include/uapi/asm/ioctls.h | 2 ++
drivers/tty/serial/serial_core.c | 60 ++++++++++++++++++++++++++++++++++
include/linux/serial_core.h | 3 ++
include/uapi/asm-generic/ioctls.h | 2 ++
include/uapi/linux/serial.h | 17 ++++++++++
10 files changed, 94 insertions(+)

diff --git a/arch/alpha/include/uapi/asm/ioctls.h b/arch/alpha/include/uapi/asm/ioctls.h
index 3729d92d3fa8..1e9121c9b3c7 100644
--- a/arch/alpha/include/uapi/asm/ioctls.h
+++ b/arch/alpha/include/uapi/asm/ioctls.h
@@ -102,6 +102,8 @@
#define TIOCGPTLCK _IOR('T', 0x39, int) /* Get Pty lock state */
#define TIOCGEXCL _IOR('T', 0x40, int) /* Get exclusive mode state */
#define TIOCGPTPEER _IO('T', 0x41) /* Safely open the slave */
+#define TIOCGISO7816 _IOR('T', 0x42, struct serial_iso7816)
+#define TIOCSISO7816 _IOWR('T', 0x43, struct serial_iso7816)

#define TIOCSERCONFIG 0x5453
#define TIOCSERGWILD 0x5454
diff --git a/arch/mips/include/uapi/asm/ioctls.h b/arch/mips/include/uapi/asm/ioctls.h
index 890245a9f0c4..16aa8a766aec 100644
--- a/arch/mips/include/uapi/asm/ioctls.h
+++ b/arch/mips/include/uapi/asm/ioctls.h
@@ -93,6 +93,8 @@
#define TIOCGPTLCK _IOR('T', 0x39, int) /* Get Pty lock state */
#define TIOCGEXCL _IOR('T', 0x40, int) /* Get exclusive mode state */
#define TIOCGPTPEER _IO('T', 0x41) /* Safely open the slave */
+#define TIOCGISO7816 _IOR('T', 0x42, struct serial_iso7816)
+#define TIOCSISO7816 _IOWR('T', 0x43, struct serial_iso7816)

/* I hope the range from 0x5480 on is free ... */
#define TIOCSCTTY 0x5480 /* become controlling tty */
diff --git a/arch/powerpc/include/uapi/asm/ioctls.h b/arch/powerpc/include/uapi/asm/ioctls.h
index 41b1a5c15734..2c145da3b774 100644
--- a/arch/powerpc/include/uapi/asm/ioctls.h
+++ b/arch/powerpc/include/uapi/asm/ioctls.h
@@ -102,6 +102,8 @@
#define TIOCGPTLCK _IOR('T', 0x39, int) /* Get Pty lock state */
#define TIOCGEXCL _IOR('T', 0x40, int) /* Get exclusive mode state */
#define TIOCGPTPEER _IO('T', 0x41) /* Safely open the slave */
+#define TIOCGISO7816 _IOR('T', 0x42, struct serial_iso7816)
+#define TIOCSISO7816 _IOWR('T', 0x43, struct serial_iso7816)

#define TIOCSERCONFIG 0x5453
#define TIOCSERGWILD 0x5454
diff --git a/arch/sh/include/uapi/asm/ioctls.h b/arch/sh/include/uapi/asm/ioctls.h
index cc62f6f98103..11866d4f60e1 100644
--- a/arch/sh/include/uapi/asm/ioctls.h
+++ b/arch/sh/include/uapi/asm/ioctls.h
@@ -95,6 +95,8 @@
#define TIOCGPTLCK _IOR('T', 0x39, int) /* Get Pty lock state */
#define TIOCGEXCL _IOR('T', 0x40, int) /* Get exclusive mode state */
#define TIOCGPTPEER _IO('T', 0x41) /* Safely open the slave */
+#define TIOCGISO7816 _IOR('T', 0x42, struct serial_iso7816)
+#define TIOCSISO7816 _IOWR('T', 0x43, struct serial_iso7816)

#define TIOCSERCONFIG _IO('T', 83) /* 0x5453 */
#define TIOCSERGWILD _IOR('T', 84, int) /* 0x5454 */
diff --git a/arch/sparc/include/uapi/asm/ioctls.h b/arch/sparc/include/uapi/asm/ioctls.h
index 2df52711e170..7fd2f5873c9e 100644
--- a/arch/sparc/include/uapi/asm/ioctls.h
+++ b/arch/sparc/include/uapi/asm/ioctls.h
@@ -27,6 +27,8 @@
#define TIOCGEXCL _IOR('T', 0x40, int) /* Get exclusive mode state */
#define TIOCGRS485 _IOR('T', 0x41, struct serial_rs485)
#define TIOCSRS485 _IOWR('T', 0x42, struct serial_rs485)
+#define TIOCGISO7816 _IOR('T', 0x43, struct serial_iso7816)
+#define TIOCSISO7816 _IOWR('T', 0x44, struct serial_iso7816)

/* Note that all the ioctls that are not available in Linux have a
* double underscore on the front to: a) avoid some programs to
diff --git a/arch/xtensa/include/uapi/asm/ioctls.h b/arch/xtensa/include/uapi/asm/ioctls.h
index ec43609cbfc5..6d4a87296c95 100644
--- a/arch/xtensa/include/uapi/asm/ioctls.h
+++ b/arch/xtensa/include/uapi/asm/ioctls.h
@@ -107,6 +107,8 @@
#define TIOCGPTLCK _IOR('T', 0x39, int) /* Get Pty lock state */
#define TIOCGEXCL _IOR('T', 0x40, int) /* Get exclusive mode state */
#define TIOCGPTPEER _IO('T', 0x41) /* Safely open the slave */
+#define TIOCGISO7816 _IOR('T', 0x42, struct serial_iso7816)
+#define TIOCSISO7816 _IOWR('T', 0x43, struct serial_iso7816)

#define TIOCSERCONFIG _IO('T', 83)
#define TIOCSERGWILD _IOR('T', 84, int)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 9c14a453f73c..af8d5b12fba9 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1301,6 +1301,58 @@ static int uart_set_rs485_config(struct uart_port *port,
return 0;
}

+static int uart_get_iso7816_config(struct uart_port *port,
+ struct serial_iso7816 __user *iso7816)
+{
+ unsigned long flags;
+ struct serial_iso7816 aux;
+
+ if (!port->iso7816_config)
+ return -ENOIOCTLCMD;
+
+ spin_lock_irqsave(&port->lock, flags);
+ aux = port->iso7816;
+ spin_unlock_irqrestore(&port->lock, flags);
+
+ if (copy_to_user(iso7816, &aux, sizeof(aux)))
+ return -EFAULT;
+
+ return 0;
+}
+
+static int uart_set_iso7816_config(struct uart_port *port,
+ struct serial_iso7816 __user *iso7816_user)
+{
+ struct serial_iso7816 iso7816;
+ int i, ret;
+ unsigned long flags;
+
+ if (!port->iso7816_config)
+ return -ENOIOCTLCMD;
+
+ if (copy_from_user(&iso7816, iso7816_user, sizeof(*iso7816_user)))
+ return -EFAULT;
+
+ /*
+ * There are 5 words reserved for future use. Check that userspace
+ * doesn't put stuff in there to prevent breakages in the future.
+ */
+ for (i = 0; i < 5; i++)
+ if (iso7816.reserved[i])
+ return -EINVAL;
+
+ spin_lock_irqsave(&port->lock, flags);
+ ret = port->iso7816_config(port, &iso7816);
+ spin_unlock_irqrestore(&port->lock, flags);
+ if (ret)
+ return ret;
+
+ if (copy_to_user(iso7816_user, &port->iso7816, sizeof(port->iso7816)))
+ return -EFAULT;
+
+ return 0;
+}
+
/*
* Called via sys_ioctl. We can use spin_lock_irq() here.
*/
@@ -1385,6 +1437,14 @@ uart_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg)
case TIOCSRS485:
ret = uart_set_rs485_config(uport, uarg);
break;
+
+ case TIOCSISO7816:
+ ret = uart_set_iso7816_config(state->uart_port, uarg);
+ break;
+
+ case TIOCGISO7816:
+ ret = uart_get_iso7816_config(state->uart_port, uarg);
+ break;
default:
if (uport->ops->ioctl)
ret = uport->ops->ioctl(uport, cmd, arg);
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 06ea4eeb09ab..d6e7747ffd46 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -137,6 +137,8 @@ struct uart_port {
void (*handle_break)(struct uart_port *);
int (*rs485_config)(struct uart_port *,
struct serial_rs485 *rs485);
+ int (*iso7816_config)(struct uart_port *,
+ struct serial_iso7816 *iso7816);
unsigned int irq; /* irq number */
unsigned long irqflags; /* irq flags */
unsigned int uartclk; /* base uart clock */
@@ -253,6 +255,7 @@ struct uart_port {
struct attribute_group *attr_group; /* port specific attributes */
const struct attribute_group **tty_groups; /* all attributes (serial core use only) */
struct serial_rs485 rs485;
+ struct serial_iso7816 iso7816;
void *private_data; /* generic platform data pointer */
};

diff --git a/include/uapi/asm-generic/ioctls.h b/include/uapi/asm-generic/ioctls.h
index 040651735662..cdc9f4ca8c27 100644
--- a/include/uapi/asm-generic/ioctls.h
+++ b/include/uapi/asm-generic/ioctls.h
@@ -79,6 +79,8 @@
#define TIOCGPTLCK _IOR('T', 0x39, int) /* Get Pty lock state */
#define TIOCGEXCL _IOR('T', 0x40, int) /* Get exclusive mode state */
#define TIOCGPTPEER _IO('T', 0x41) /* Safely open the slave */
+#define TIOCGISO7816 _IOR('T', 0x42, struct serial_iso7816)
+#define TIOCSISO7816 _IOWR('T', 0x43, struct serial_iso7816)

#define FIONCLEX 0x5450
#define FIOCLEX 0x5451
diff --git a/include/uapi/linux/serial.h b/include/uapi/linux/serial.h
index 3fdd0dee8b41..93eb3c496ff1 100644
--- a/include/uapi/linux/serial.h
+++ b/include/uapi/linux/serial.h
@@ -132,4 +132,21 @@ struct serial_rs485 {
are a royal PITA .. */
};

+/*
+ * Serial interface for controlling ISO7816 settings on chips with suitable
+ * support. Set with TIOCSISO7816 and get with TIOCGISO7816 if supported by
+ * your platform.
+ */
+struct serial_iso7816 {
+ __u32 flags; /* ISO7816 feature flags */
+#define SER_ISO7816_ENABLED (1 << 0)
+#define SER_ISO7816_T_PARAM (0x0f << 4)
+#define SER_ISO7816_T(t) (((t) & 0x0f) << 4)
+ __u32 tg;
+ __u32 sc_fi;
+ __u32 sc_di;
+ __u32 clk;
+ __u32 reserved[5];
+};
+
#endif /* _UAPI_LINUX_SERIAL_H */
--
2.12.2


2018-07-19 08:52:17

by Ludovic Desroches

[permalink] [raw]
Subject: [PATCH v2 2/2] tty/serial: atmel: add ISO7816 support

From: Nicolas Ferre <[email protected]>

When mode is set in atmel_config_iso7816() we backup last RS232 mode
for coming back to this mode if requested.
Also allow setup of T=0 and T=1 parameter and basic support in set_termios
function as well.
Report NACK and ITER errors in irq handler.

Signed-off-by: Nicolas Ferre <[email protected]>
Signed-off-by: Ludovic Desroches <[email protected]>
---
drivers/tty/serial/atmel_serial.c | 170 +++++++++++++++++++++++++++++++++++---
drivers/tty/serial/atmel_serial.h | 3 +-
2 files changed, 162 insertions(+), 11 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 8e4428725848..cec958f1e7d4 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -34,6 +34,7 @@
#include <linux/suspend.h>
#include <linux/mm.h>

+#include <asm/div64.h>
#include <asm/io.h>
#include <asm/ioctls.h>

@@ -147,6 +148,8 @@ struct atmel_uart_port {
struct circ_buf rx_ring;

struct mctrl_gpios *gpios;
+ u32 backup_mode; /* MR saved during iso7816 operations */
+ u32 backup_brgr; /* BRGR saved during iso7816 operations */
unsigned int tx_done_mask;
u32 fifo_size;
u32 rts_high;
@@ -362,6 +365,132 @@ static int atmel_config_rs485(struct uart_port *port,
return 0;
}

+static unsigned int atmel_calc_cd(struct uart_port *port,
+ struct serial_iso7816 *iso7816conf)
+{
+ struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
+ unsigned int cd;
+ u64 mck_rate;
+
+ mck_rate = (u64)clk_get_rate(atmel_port->clk);
+ do_div(mck_rate, iso7816conf->clk);
+ cd = mck_rate;
+ return cd;
+}
+
+static unsigned int atmel_calc_fidi(struct uart_port *port,
+ struct serial_iso7816 *iso7816conf)
+{
+ u64 fidi = 0;
+
+ if (iso7816conf->sc_fi && iso7816conf->sc_di) {
+ fidi = (u64)iso7816conf->sc_fi;
+ do_div(fidi, iso7816conf->sc_di);
+ }
+ return (u32)fidi;
+}
+
+/* Enable or disable the iso7816 support */
+/* Called with interrupts disabled */
+static int atmel_config_iso7816(struct uart_port *port,
+ struct serial_iso7816 *iso7816conf)
+{
+ struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
+ unsigned int mode, t;
+ unsigned int cd, fidi;
+ int ret = 0;
+
+ /* Disable RX and TX */
+ atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RXDIS | ATMEL_US_TXDIS);
+ /* Disable interrupts */
+ atmel_uart_writel(port, ATMEL_US_IDR, atmel_port->tx_done_mask);
+
+ mode = atmel_uart_readl(port, ATMEL_US_MR);
+
+ if (iso7816conf->flags & SER_ISO7816_ENABLED) {
+ mode &= ~ATMEL_US_USMODE;
+
+ if ((iso7816conf->flags & SER_ISO7816_T_PARAM)
+ == SER_ISO7816_T(0)) {
+ mode |= ATMEL_US_USMODE_ISO7816_T0;
+ t = 0;
+ } else if ((iso7816conf->flags & SER_ISO7816_T_PARAM)
+ == SER_ISO7816_T(1)) {
+ mode |= ATMEL_US_USMODE_ISO7816_T1;
+ t = 1;
+ } else {
+ dev_warn(port->dev, "ISO7816 Type not supported. Resetting\n");
+ memset(iso7816conf, 0, sizeof(struct serial_iso7816));
+ goto err_out;
+ }
+
+ dev_dbg(port->dev, "Setting USART to ISO7816 mode T%d\n", t);
+
+ mode &= ~(ATMEL_US_USCLKS | ATMEL_US_CHRL
+ | ATMEL_US_NBSTOP | ATMEL_US_PAR);
+
+ /* NACK configuration */
+ if ((iso7816conf->flags & SER_ISO7816_T_PARAM)
+ == SER_ISO7816_T(0))
+ mode |= ATMEL_US_DSNACK;
+ else
+ mode |= ATMEL_US_INACK;
+ /* select mck clock, and output */
+ mode |= ATMEL_US_USCLKS_MCK | ATMEL_US_CLKO;
+ /* set parity for normal/inverse mode + max iterations */
+ mode |= ATMEL_US_PAR_EVEN | ATMEL_US_NBSTOP_1 | (3 << 24);
+
+ cd = atmel_calc_cd(port, iso7816conf);
+ fidi = atmel_calc_fidi(port, iso7816conf);
+ if (fidi < 0) {
+ dev_warn(port->dev, "ISO7816 fidi = 0, Generator generates no signal\n");
+ } else if (fidi == 1 || fidi == 2) {
+ dev_err(port->dev, "ISO7816 fidi = %u, value not supported\n", fidi);
+ ret = -EINVAL;
+ goto err_out;
+ }
+
+ if (!(port->iso7816.flags & SER_ISO7816_ENABLED)) {
+ /* port not yet in iso7816 mode: store configuration */
+ atmel_port->backup_mode = atmel_uart_readl(port, ATMEL_US_MR);
+ atmel_port->backup_brgr = atmel_uart_readl(port, ATMEL_US_BRGR);
+ }
+
+ /* Actually set ISO7816 mode */
+ atmel_uart_writel(port, ATMEL_US_TTGR, iso7816conf->tg);
+ atmel_uart_writel(port, ATMEL_US_BRGR, cd);
+ atmel_uart_writel(port, ATMEL_US_FIDIR, fidi);
+
+ atmel_port->tx_done_mask = ATMEL_US_TXEMPTY | ATMEL_US_NACK | ATMEL_US_ITERATION;
+ } else {
+ dev_dbg(port->dev, "Setting UART to RS232\n");
+ /* back to last RS232 settings */
+ mode = atmel_port->backup_mode;
+ memset(iso7816conf, 0, sizeof(struct serial_iso7816));
+ atmel_uart_writel(port, ATMEL_US_TTGR, 0);
+ atmel_uart_writel(port, ATMEL_US_BRGR, atmel_port->backup_brgr);
+ atmel_uart_writel(port, ATMEL_US_FIDIR, 0x174);
+
+ if (atmel_use_pdc_tx(port))
+ atmel_port->tx_done_mask = ATMEL_US_ENDTX |
+ ATMEL_US_TXBUFE;
+ else
+ atmel_port->tx_done_mask = ATMEL_US_TXRDY;
+ }
+
+ port->iso7816 = *iso7816conf;
+
+ atmel_uart_writel(port, ATMEL_US_MR, mode);
+
+err_out:
+ /* Enable interrupts */
+ atmel_uart_writel(port, ATMEL_US_IER, atmel_port->tx_done_mask);
+ /* Enable RX and TX */
+ atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RXEN | ATMEL_US_TXEN);
+
+ return ret;
+}
+
/*
* Return TIOCSER_TEMT when transmitter FIFO and Shift register is empty.
*/
@@ -481,8 +610,9 @@ static void atmel_stop_tx(struct uart_port *port)
/* Disable interrupts */
atmel_uart_writel(port, ATMEL_US_IDR, atmel_port->tx_done_mask);

- if ((port->rs485.flags & SER_RS485_ENABLED) &&
- !(port->rs485.flags & SER_RS485_RX_DURING_TX))
+ if (((port->rs485.flags & SER_RS485_ENABLED) &&
+ !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
+ port->iso7816.flags & SER_ISO7816_ENABLED)
atmel_start_rx(port);
}

@@ -500,8 +630,9 @@ static void atmel_start_tx(struct uart_port *port)
return;

if (atmel_use_pdc_tx(port) || atmel_use_dma_tx(port))
- if ((port->rs485.flags & SER_RS485_ENABLED) &&
- !(port->rs485.flags & SER_RS485_RX_DURING_TX))
+ if (((port->rs485.flags & SER_RS485_ENABLED) &&
+ !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
+ port->iso7816.flags & SER_ISO7816_ENABLED)
atmel_stop_rx(port);

if (atmel_use_pdc_tx(port))
@@ -799,8 +930,9 @@ static void atmel_complete_tx_dma(void *arg)
*/
if (!uart_circ_empty(xmit))
atmel_tasklet_schedule(atmel_port, &atmel_port->tasklet_tx);
- else if ((port->rs485.flags & SER_RS485_ENABLED) &&
- !(port->rs485.flags & SER_RS485_RX_DURING_TX)) {
+ else if (((port->rs485.flags & SER_RS485_ENABLED) &&
+ !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
+ port->iso7816.flags & SER_ISO7816_ENABLED) {
/* DMA done, stop TX, start RX for RS485 */
atmel_start_rx(port);
}
@@ -1281,6 +1413,9 @@ atmel_handle_status(struct uart_port *port, unsigned int pending,
wake_up_interruptible(&port->state->port.delta_msr_wait);
}
}
+
+ if (pending & (ATMEL_US_NACK | ATMEL_US_ITERATION))
+ dev_dbg(port->dev, "ISO7816 ERROR (0x%08x)\n", pending);
}

/*
@@ -1373,8 +1508,9 @@ static void atmel_tx_pdc(struct uart_port *port)
atmel_uart_writel(port, ATMEL_US_IER,
atmel_port->tx_done_mask);
} else {
- if ((port->rs485.flags & SER_RS485_ENABLED) &&
- !(port->rs485.flags & SER_RS485_RX_DURING_TX)) {
+ if (((port->rs485.flags & SER_RS485_ENABLED) &&
+ !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
+ port->iso7816.flags & SER_ISO7816_ENABLED) {
/* DMA done, stop TX, start RX for RS485 */
atmel_start_rx(port);
}
@@ -2099,6 +2235,17 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
atmel_uart_writel(port, ATMEL_US_TTGR,
port->rs485.delay_rts_after_send);
mode |= ATMEL_US_USMODE_RS485;
+ } else if (port->iso7816.flags & SER_ISO7816_ENABLED) {
+ atmel_uart_writel(port, ATMEL_US_TTGR, port->iso7816.tg);
+ /* select mck clock, and output */
+ mode |= ATMEL_US_USCLKS_MCK | ATMEL_US_CLKO;
+ /* set max iterations */
+ mode |= (3 << 24);
+ if ((port->iso7816.flags & SER_ISO7816_T_PARAM)
+ == SER_ISO7816_T(0))
+ mode |= ATMEL_US_USMODE_ISO7816_T0;
+ else
+ mode |= ATMEL_US_USMODE_ISO7816_T1;
} else if (termios->c_cflag & CRTSCTS) {
/* RS232 with hardware handshake (RTS/CTS) */
if (atmel_use_fifo(port) &&
@@ -2175,7 +2322,8 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
}
quot = cd | fp << ATMEL_US_FP_OFFSET;

- atmel_uart_writel(port, ATMEL_US_BRGR, quot);
+ if (!(port->iso7816.flags & SER_ISO7816_ENABLED))
+ atmel_uart_writel(port, ATMEL_US_BRGR, quot);
atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN | ATMEL_US_RXEN);
atmel_port->tx_stopped = false;
@@ -2355,6 +2503,7 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port,
port->mapbase = pdev->resource[0].start;
port->irq = pdev->resource[1].start;
port->rs485_config = atmel_config_rs485;
+ port->iso7816_config = atmel_config_iso7816;
port->membase = NULL;

memset(&atmel_port->rx_ring, 0, sizeof(atmel_port->rx_ring));
@@ -2379,7 +2528,8 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port,
}

/* Use TXEMPTY for interrupt when rs485 else TXRDY or ENDTX|TXBUFE */
- if (port->rs485.flags & SER_RS485_ENABLED)
+ if (port->rs485.flags & SER_RS485_ENABLED ||
+ port->iso7816.flags & SER_ISO7816_ENABLED)
atmel_port->tx_done_mask = ATMEL_US_TXEMPTY;
else if (atmel_use_pdc_tx(port)) {
port->fifosize = PDC_BUFFER_SIZE;
diff --git a/drivers/tty/serial/atmel_serial.h b/drivers/tty/serial/atmel_serial.h
index ba3a2437cde4..fff51f5fe8bc 100644
--- a/drivers/tty/serial/atmel_serial.h
+++ b/drivers/tty/serial/atmel_serial.h
@@ -124,7 +124,8 @@
#define ATMEL_US_TTGR 0x28 /* Transmitter Timeguard Register */
#define ATMEL_US_TG GENMASK(7, 0) /* Timeguard Value */

-#define ATMEL_US_FIDI 0x40 /* FI DI Ratio Register */
+#define ATMEL_US_FIDIR 0x40 /* FI DI Ratio Register */
+#define ATMEL_US_FIDI GENMASK(15, 0) /* FIDI ratio */
#define ATMEL_US_NER 0x44 /* Number of Errors Register */
#define ATMEL_US_IF 0x4c /* IrDA Filter Register */

--
2.12.2


2018-07-19 09:01:10

by Neil Armstrong

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] add ISO7816 support

Hi Ludovic,

On 19/07/2018 10:47, Ludovic Desroches wrote:
> Hi,
>
> This patchset adds support for the ISO7816 standard. The USART devices in
> Microchip SoCs have an ISO7816 mode. It allows to let the USART managing
> the CLK and I/O signals of a smart card.

Wow, I would have loved to have this at the time...
I'm curious, do you have an example of userspace code using this ?
The ATR rx needs a very weird handling, I'm curious how you managed it.

Thanks,
Neil

>
> Changes:
> - v2
> - uart_get_iso7816_config: check there is an iso7816_config function
> - use IOCTL macros to generate the IOCTL number
> - check that reserved field is not used
> - remove debug logs
> - check that the iso7816_config is right before doing any action
> - change the error from nack and max iteration status to a debug message
> - remove patch 3 as it concerns both rs485 and iso7816 to think more
> about the need of adding a lock or not
>
> Nicolas Ferre (2):
> tty/serial_core: add ISO7816 infrastructure
> tty/serial: atmel: add ISO7816 support
>
> arch/alpha/include/uapi/asm/ioctls.h | 2 +
> arch/mips/include/uapi/asm/ioctls.h | 2 +
> arch/powerpc/include/uapi/asm/ioctls.h | 2 +
> arch/sh/include/uapi/asm/ioctls.h | 2 +
> arch/sparc/include/uapi/asm/ioctls.h | 2 +
> arch/xtensa/include/uapi/asm/ioctls.h | 2 +
> drivers/tty/serial/atmel_serial.c | 170 +++++++++++++++++++++++++++++++--
> drivers/tty/serial/atmel_serial.h | 3 +-
> drivers/tty/serial/serial_core.c | 60 ++++++++++++
> include/linux/serial_core.h | 3 +
> include/uapi/asm-generic/ioctls.h | 2 +
> include/uapi/linux/serial.h | 17 ++++
> 12 files changed, 256 insertions(+), 11 deletions(-)
>


2018-07-19 09:29:00

by Ludovic Desroches

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] add ISO7816 support

Hi Neil,

On Thu, Jul 19, 2018 at 10:59:47AM +0200, Neil Armstrong wrote:
> Hi Ludovic,
>
> On 19/07/2018 10:47, Ludovic Desroches wrote:
> > Hi,
> >
> > This patchset adds support for the ISO7816 standard. The USART devices in
> > Microchip SoCs have an ISO7816 mode. It allows to let the USART managing
> > the CLK and I/O signals of a smart card.
>
> Wow, I would have loved to have this at the time...
> I'm curious, do you have an example of userspace code using this ?
> The ATR rx needs a very weird handling, I'm curious how you managed it.
>

Unfortunately, I have nothing I can give you at the moment. I am doing
some experiments. Before going further, I need to have the interface
with the kernel accepted.

Of course, I can give more details about the experiments I am doing.
First of all, I am not used to ISO7816 so every feedback are
appreciated.

On the userspace part, there is the PCSC Lite library. It needs a
userspace driver. From what I've seen, most of the readers are managed
by the CCID driver. This driver handle different hardware capabilities.
Some hardware are very close to what we provide with our SoC ie. just
sending the data on the line and not manage the procedure bytes.

The CCID driver includes a lot of things related to the ISO7816 protocol
so I tried to plug my code into it to reuse this code. The issue is that I am
not a CCID device. At the moment, I am writing my own PCSC driver and I am
copying/pasting code I need. The USART only manages the CLK and I/O
signals. Others one are handled by the PCSC driver with the help of
GPIOs.

At a time, I was thinking about a CCID driver (which interprets the CCID
header and interacts with the device handling ISO7816) in the kernel to easily
interface with the PCSC CCID driver but I am not sure it's the right way
to go.

Regards

Ludovic


> Thanks,
> Neil
>
> >
> > Changes:
> > - v2
> > - uart_get_iso7816_config: check there is an iso7816_config function
> > - use IOCTL macros to generate the IOCTL number
> > - check that reserved field is not used
> > - remove debug logs
> > - check that the iso7816_config is right before doing any action
> > - change the error from nack and max iteration status to a debug message
> > - remove patch 3 as it concerns both rs485 and iso7816 to think more
> > about the need of adding a lock or not
> >
> > Nicolas Ferre (2):
> > tty/serial_core: add ISO7816 infrastructure
> > tty/serial: atmel: add ISO7816 support
> >
> > arch/alpha/include/uapi/asm/ioctls.h | 2 +
> > arch/mips/include/uapi/asm/ioctls.h | 2 +
> > arch/powerpc/include/uapi/asm/ioctls.h | 2 +
> > arch/sh/include/uapi/asm/ioctls.h | 2 +
> > arch/sparc/include/uapi/asm/ioctls.h | 2 +
> > arch/xtensa/include/uapi/asm/ioctls.h | 2 +
> > drivers/tty/serial/atmel_serial.c | 170 +++++++++++++++++++++++++++++++--
> > drivers/tty/serial/atmel_serial.h | 3 +-
> > drivers/tty/serial/serial_core.c | 60 ++++++++++++
> > include/linux/serial_core.h | 3 +
> > include/uapi/asm-generic/ioctls.h | 2 +
> > include/uapi/linux/serial.h | 17 ++++
> > 12 files changed, 256 insertions(+), 11 deletions(-)
> >
>

2018-07-27 14:48:32

by Richard Genoud

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] tty/serial: atmel: add ISO7816 support

Hi Ludovic,

On 19/07/2018 10:47, Ludovic Desroches wrote:
> From: Nicolas Ferre <[email protected]>
>
> When mode is set in atmel_config_iso7816() we backup last RS232 mode
> for coming back to this mode if requested.
> Also allow setup of T=0 and T=1 parameter and basic support in set_termios
> function as well.
> Report NACK and ITER errors in irq handler.
>
> Signed-off-by: Nicolas Ferre <[email protected]>
> Signed-off-by: Ludovic Desroches <[email protected]>
> ---
> drivers/tty/serial/atmel_serial.c | 170 +++++++++++++++++++++++++++++++++++---
> drivers/tty/serial/atmel_serial.h | 3 +-
> 2 files changed, 162 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 8e4428725848..cec958f1e7d4 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -34,6 +34,7 @@
> #include <linux/suspend.h>
> #include <linux/mm.h>
>
> +#include <asm/div64.h>
> #include <asm/io.h>
> #include <asm/ioctls.h>
>
> @@ -147,6 +148,8 @@ struct atmel_uart_port {
> struct circ_buf rx_ring;
>
> struct mctrl_gpios *gpios;
> + u32 backup_mode; /* MR saved during iso7816 operations */
> + u32 backup_brgr; /* BRGR saved during iso7816 operations */
> unsigned int tx_done_mask;
> u32 fifo_size;
> u32 rts_high;
> @@ -362,6 +365,132 @@ static int atmel_config_rs485(struct uart_port *port,
> return 0;
> }
>
> +static unsigned int atmel_calc_cd(struct uart_port *port,
> + struct serial_iso7816 *iso7816conf)
> +{
> + struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> + unsigned int cd;
> + u64 mck_rate;
> +
> + mck_rate = (u64)clk_get_rate(atmel_port->clk);
> + do_div(mck_rate, iso7816conf->clk);
> + cd = mck_rate;
> + return cd;
> +}
> +
> +static unsigned int atmel_calc_fidi(struct uart_port *port,
> + struct serial_iso7816 *iso7816conf)
> +{
> + u64 fidi = 0;
> +
> + if (iso7816conf->sc_fi && iso7816conf->sc_di) {
> + fidi = (u64)iso7816conf->sc_fi;
> + do_div(fidi, iso7816conf->sc_di);
> + }
> + return (u32)fidi;
> +}
> +
> +/* Enable or disable the iso7816 support */
> +/* Called with interrupts disabled */
> +static int atmel_config_iso7816(struct uart_port *port,
> + struct serial_iso7816 *iso7816conf)
> +{
> + struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> + unsigned int mode, t;
> + unsigned int cd, fidi;
> + int ret = 0;
> +
> + /* Disable RX and TX */
> + atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RXDIS | ATMEL_US_TXDIS);
> + /* Disable interrupts */
> + atmel_uart_writel(port, ATMEL_US_IDR, atmel_port->tx_done_mask);
> +
> + mode = atmel_uart_readl(port, ATMEL_US_MR);
> +
> + if (iso7816conf->flags & SER_ISO7816_ENABLED) {
> + mode &= ~ATMEL_US_USMODE;
> +
> + if ((iso7816conf->flags & SER_ISO7816_T_PARAM)
> + == SER_ISO7816_T(0)) {
> + mode |= ATMEL_US_USMODE_ISO7816_T0;
> + t = 0;
> + } else if ((iso7816conf->flags & SER_ISO7816_T_PARAM)
> + == SER_ISO7816_T(1)) {
> + mode |= ATMEL_US_USMODE_ISO7816_T1;
> + t = 1;
> + } else {
> + dev_warn(port->dev, "ISO7816 Type not supported. Resetting\n");
> + memset(iso7816conf, 0, sizeof(struct serial_iso7816));
> + goto err_out;
> + }
> +
> + dev_dbg(port->dev, "Setting USART to ISO7816 mode T%d\n", t);
> +
> + mode &= ~(ATMEL_US_USCLKS | ATMEL_US_CHRL
> + | ATMEL_US_NBSTOP | ATMEL_US_PAR);
This could be merged in the mode &= line above.

> +
> + /* NACK configuration */
> + if ((iso7816conf->flags & SER_ISO7816_T_PARAM)
> + == SER_ISO7816_T(0))
> + mode |= ATMEL_US_DSNACK;
> + else
> + mode |= ATMEL_US_INACK;
This could be also part of the if () above.

> + /* select mck clock, and output */
> + mode |= ATMEL_US_USCLKS_MCK | ATMEL_US_CLKO;
> + /* set parity for normal/inverse mode + max iterations */
> + mode |= ATMEL_US_PAR_EVEN | ATMEL_US_NBSTOP_1 | (3 << 24);
Is this really needed ?
In the documentation, I found:
"The configuration is 8 data bits, even parity and 1 or 2 stop bits,
regardless of the values programmed in the CHRL, MODE9, PAR and CHMODE
fields."
And, for MAX_ITERATIONS, could you add a macro instead of (x << 24) ?
(ATMEL_US_MAX_ITER mask is already defined).
And why 3 ? Should the user-space be allowed to control the max
automatic iteration ? or is it more like a "same-value-for-every-one"
thing ?

> +
> + cd = atmel_calc_cd(port, iso7816conf);
> + fidi = atmel_calc_fidi(port, iso7816conf);
> + if (fidi < 0) {
I guess you meant (fidi == 0)
Because fidi is unsigned and atmel_calc_fidi() returns also an unsigned.

> + dev_warn(port->dev, "ISO7816 fidi = 0, Generator generates no signal\n");
> + } else if (fidi == 1 || fidi == 2) {
> + dev_err(port->dev, "ISO7816 fidi = %u, value not supported\n", fidi);
> + ret = -EINVAL;
> + goto err_out;
> + }
And you may also want to check upper values ( <2048 or <65536, depending
on the SoC)

> +
> + if (!(port->iso7816.flags & SER_ISO7816_ENABLED)) {
> + /* port not yet in iso7816 mode: store configuration */
> + atmel_port->backup_mode = atmel_uart_readl(port, ATMEL_US_MR);
> + atmel_port->backup_brgr = atmel_uart_readl(port, ATMEL_US_BRGR);
> + }
> +
> + /* Actually set ISO7816 mode */
> + atmel_uart_writel(port, ATMEL_US_TTGR, iso7816conf->tg);
iso7816conf->tg comes from user-space unchecked. AFAIK, max value is 255

> + atmel_uart_writel(port, ATMEL_US_BRGR, cd);
> + atmel_uart_writel(port, ATMEL_US_FIDIR, fidi);
> +
> + atmel_port->tx_done_mask = ATMEL_US_TXEMPTY | ATMEL_US_NACK | ATMEL_US_ITERATION;
> + } else {
> + dev_dbg(port->dev, "Setting UART to RS232\n");
> + /* back to last RS232 settings */
> + mode = atmel_port->backup_mode;
> + memset(iso7816conf, 0, sizeof(struct serial_iso7816));
> + atmel_uart_writel(port, ATMEL_US_TTGR, 0);
> + atmel_uart_writel(port, ATMEL_US_BRGR, atmel_port->backup_brgr);
> + atmel_uart_writel(port, ATMEL_US_FIDIR, 0x174);
> +
> + if (atmel_use_pdc_tx(port))
> + atmel_port->tx_done_mask = ATMEL_US_ENDTX |
> + ATMEL_US_TXBUFE;
> + else
> + atmel_port->tx_done_mask = ATMEL_US_TXRDY;
> + }
> +
> + port->iso7816 = *iso7816conf;
> +
> + atmel_uart_writel(port, ATMEL_US_MR, mode);
> +
> +err_out:
> + /* Enable interrupts */
> + atmel_uart_writel(port, ATMEL_US_IER, atmel_port->tx_done_mask);
> + /* Enable RX and TX */
> + atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RXEN | ATMEL_US_TXEN);
Is it all right to enable RX/TX unconditionally here ?

> +
> + return ret;
> +}
> +
> /*
> * Return TIOCSER_TEMT when transmitter FIFO and Shift register is empty.
> */
> @@ -481,8 +610,9 @@ static void atmel_stop_tx(struct uart_port *port)
> /* Disable interrupts */
> atmel_uart_writel(port, ATMEL_US_IDR, atmel_port->tx_done_mask);
>
> - if ((port->rs485.flags & SER_RS485_ENABLED) &&
> - !(port->rs485.flags & SER_RS485_RX_DURING_TX))
> + if (((port->rs485.flags & SER_RS485_ENABLED) &&
> + !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
> + port->iso7816.flags & SER_ISO7816_ENABLED)
> atmel_start_rx(port);
> }
>
> @@ -500,8 +630,9 @@ static void atmel_start_tx(struct uart_port *port)
> return;
>
> if (atmel_use_pdc_tx(port) || atmel_use_dma_tx(port))
> - if ((port->rs485.flags & SER_RS485_ENABLED) &&
> - !(port->rs485.flags & SER_RS485_RX_DURING_TX))
> + if (((port->rs485.flags & SER_RS485_ENABLED) &&
> + !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
> + port->iso7816.flags & SER_ISO7816_ENABLED)
> atmel_stop_rx(port);
>
> if (atmel_use_pdc_tx(port))
> @@ -799,8 +930,9 @@ static void atmel_complete_tx_dma(void *arg)
> */
> if (!uart_circ_empty(xmit))
> atmel_tasklet_schedule(atmel_port, &atmel_port->tasklet_tx);
> - else if ((port->rs485.flags & SER_RS485_ENABLED) &&
> - !(port->rs485.flags & SER_RS485_RX_DURING_TX)) {
> + else if (((port->rs485.flags & SER_RS485_ENABLED) &&
> + !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
> + port->iso7816.flags & SER_ISO7816_ENABLED) {
> /* DMA done, stop TX, start RX for RS485 */
> atmel_start_rx(port);
> }
> @@ -1281,6 +1413,9 @@ atmel_handle_status(struct uart_port *port, unsigned int pending,
> wake_up_interruptible(&port->state->port.delta_msr_wait);
> }
> }
> +
> + if (pending & (ATMEL_US_NACK | ATMEL_US_ITERATION))
> + dev_dbg(port->dev, "ISO7816 ERROR (0x%08x)\n", pending);
> }
>
> /*
> @@ -1373,8 +1508,9 @@ static void atmel_tx_pdc(struct uart_port *port)
> atmel_uart_writel(port, ATMEL_US_IER,
> atmel_port->tx_done_mask);
> } else {
> - if ((port->rs485.flags & SER_RS485_ENABLED) &&
> - !(port->rs485.flags & SER_RS485_RX_DURING_TX)) {
> + if (((port->rs485.flags & SER_RS485_ENABLED) &&
> + !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
> + port->iso7816.flags & SER_ISO7816_ENABLED) {
> /* DMA done, stop TX, start RX for RS485 */
> atmel_start_rx(port);
> }
> @@ -2099,6 +2235,17 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
> atmel_uart_writel(port, ATMEL_US_TTGR,
> port->rs485.delay_rts_after_send);
> mode |= ATMEL_US_USMODE_RS485;
> + } else if (port->iso7816.flags & SER_ISO7816_ENABLED) {
> + atmel_uart_writel(port, ATMEL_US_TTGR, port->iso7816.tg);
> + /* select mck clock, and output */
> + mode |= ATMEL_US_USCLKS_MCK | ATMEL_US_CLKO;
> + /* set max iterations */
> + mode |= (3 << 24);
Same remark for macro / hardcoded value.

> + if ((port->iso7816.flags & SER_ISO7816_T_PARAM)
> + == SER_ISO7816_T(0))
> + mode |= ATMEL_US_USMODE_ISO7816_T0;
> + else
> + mode |= ATMEL_US_USMODE_ISO7816_T1;
> } else if (termios->c_cflag & CRTSCTS) {
> /* RS232 with hardware handshake (RTS/CTS) */
> if (atmel_use_fifo(port) &&
> @@ -2175,7 +2322,8 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
> }
> quot = cd | fp << ATMEL_US_FP_OFFSET;
>
> - atmel_uart_writel(port, ATMEL_US_BRGR, quot);
> + if (!(port->iso7816.flags & SER_ISO7816_ENABLED))
> + atmel_uart_writel(port, ATMEL_US_BRGR, quot);
> atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
> atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN | ATMEL_US_RXEN);
> atmel_port->tx_stopped = false;
> @@ -2355,6 +2503,7 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port,
> port->mapbase = pdev->resource[0].start;
> port->irq = pdev->resource[1].start;
> port->rs485_config = atmel_config_rs485;
> + port->iso7816_config = atmel_config_iso7816;
> port->membase = NULL;
>
> memset(&atmel_port->rx_ring, 0, sizeof(atmel_port->rx_ring));
> @@ -2379,7 +2528,8 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port,
> }
>
> /* Use TXEMPTY for interrupt when rs485 else TXRDY or ENDTX|TXBUFE */
> - if (port->rs485.flags & SER_RS485_ENABLED)
> + if (port->rs485.flags & SER_RS485_ENABLED ||
> + port->iso7816.flags & SER_ISO7816_ENABLED)
please update the comment above.

> atmel_port->tx_done_mask = ATMEL_US_TXEMPTY;
> else if (atmel_use_pdc_tx(port)) {
> port->fifosize = PDC_BUFFER_SIZE;
> diff --git a/drivers/tty/serial/atmel_serial.h b/drivers/tty/serial/atmel_serial.h
> index ba3a2437cde4..fff51f5fe8bc 100644
> --- a/drivers/tty/serial/atmel_serial.h
> +++ b/drivers/tty/serial/atmel_serial.h
> @@ -124,7 +124,8 @@
> #define ATMEL_US_TTGR 0x28 /* Transmitter Timeguard Register */
> #define ATMEL_US_TG GENMASK(7, 0) /* Timeguard Value */
>
> -#define ATMEL_US_FIDI 0x40 /* FI DI Ratio Register */
> +#define ATMEL_US_FIDIR 0x40 /* FI DI Ratio Register */
> +#define ATMEL_US_FIDI GENMASK(15, 0) /* FIDI ratio */
> #define ATMEL_US_NER 0x44 /* Number of Errors Register */
> #define ATMEL_US_IF 0x4c /* IrDA Filter Register */
>
>

Thanks !

Richard.

2018-08-03 08:52:55

by Ludovic Desroches

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] tty/serial: atmel: add ISO7816 support

Hi Richard,

On Fri, Jul 27, 2018 at 04:39:17PM +0200, Richard Genoud wrote:
> Hi Ludovic,
>
> On 19/07/2018 10:47, Ludovic Desroches wrote:
> > From: Nicolas Ferre <[email protected]>
> >
> > When mode is set in atmel_config_iso7816() we backup last RS232 mode
> > for coming back to this mode if requested.
> > Also allow setup of T=0 and T=1 parameter and basic support in set_termios
> > function as well.
> > Report NACK and ITER errors in irq handler.
> >
> > Signed-off-by: Nicolas Ferre <[email protected]>
> > Signed-off-by: Ludovic Desroches <[email protected]>
> > ---
> > drivers/tty/serial/atmel_serial.c | 170 +++++++++++++++++++++++++++++++++++---
> > drivers/tty/serial/atmel_serial.h | 3 +-
> > 2 files changed, 162 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> > index 8e4428725848..cec958f1e7d4 100644
> > --- a/drivers/tty/serial/atmel_serial.c
> > +++ b/drivers/tty/serial/atmel_serial.c
> > @@ -34,6 +34,7 @@
> > #include <linux/suspend.h>
> > #include <linux/mm.h>
> >
> > +#include <asm/div64.h>
> > #include <asm/io.h>
> > #include <asm/ioctls.h>
> >
> > @@ -147,6 +148,8 @@ struct atmel_uart_port {
> > struct circ_buf rx_ring;
> >
> > struct mctrl_gpios *gpios;
> > + u32 backup_mode; /* MR saved during iso7816 operations */
> > + u32 backup_brgr; /* BRGR saved during iso7816 operations */
> > unsigned int tx_done_mask;
> > u32 fifo_size;
> > u32 rts_high;
> > @@ -362,6 +365,132 @@ static int atmel_config_rs485(struct uart_port *port,
> > return 0;
> > }
> >
> > +static unsigned int atmel_calc_cd(struct uart_port *port,
> > + struct serial_iso7816 *iso7816conf)
> > +{
> > + struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> > + unsigned int cd;
> > + u64 mck_rate;
> > +
> > + mck_rate = (u64)clk_get_rate(atmel_port->clk);
> > + do_div(mck_rate, iso7816conf->clk);
> > + cd = mck_rate;
> > + return cd;
> > +}
> > +
> > +static unsigned int atmel_calc_fidi(struct uart_port *port,
> > + struct serial_iso7816 *iso7816conf)
> > +{
> > + u64 fidi = 0;
> > +
> > + if (iso7816conf->sc_fi && iso7816conf->sc_di) {
> > + fidi = (u64)iso7816conf->sc_fi;
> > + do_div(fidi, iso7816conf->sc_di);
> > + }
> > + return (u32)fidi;
> > +}
> > +
> > +/* Enable or disable the iso7816 support */
> > +/* Called with interrupts disabled */
> > +static int atmel_config_iso7816(struct uart_port *port,
> > + struct serial_iso7816 *iso7816conf)
> > +{
> > + struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> > + unsigned int mode, t;
> > + unsigned int cd, fidi;
> > + int ret = 0;
> > +
> > + /* Disable RX and TX */
> > + atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RXDIS | ATMEL_US_TXDIS);
> > + /* Disable interrupts */
> > + atmel_uart_writel(port, ATMEL_US_IDR, atmel_port->tx_done_mask);
> > +
> > + mode = atmel_uart_readl(port, ATMEL_US_MR);
> > +
> > + if (iso7816conf->flags & SER_ISO7816_ENABLED) {
> > + mode &= ~ATMEL_US_USMODE;
> > +
> > + if ((iso7816conf->flags & SER_ISO7816_T_PARAM)
> > + == SER_ISO7816_T(0)) {
> > + mode |= ATMEL_US_USMODE_ISO7816_T0;
> > + t = 0;
> > + } else if ((iso7816conf->flags & SER_ISO7816_T_PARAM)
> > + == SER_ISO7816_T(1)) {
> > + mode |= ATMEL_US_USMODE_ISO7816_T1;
> > + t = 1;
> > + } else {
> > + dev_warn(port->dev, "ISO7816 Type not supported. Resetting\n");
> > + memset(iso7816conf, 0, sizeof(struct serial_iso7816));
> > + goto err_out;
> > + }
> > +
> > + dev_dbg(port->dev, "Setting USART to ISO7816 mode T%d\n", t);
> > +
> > + mode &= ~(ATMEL_US_USCLKS | ATMEL_US_CHRL
> > + | ATMEL_US_NBSTOP | ATMEL_US_PAR);
> This could be merged in the mode &= line above.
>

Ok

> > +
> > + /* NACK configuration */
> > + if ((iso7816conf->flags & SER_ISO7816_T_PARAM)
> > + == SER_ISO7816_T(0))
> > + mode |= ATMEL_US_DSNACK;
> > + else
> > + mode |= ATMEL_US_INACK;
> This could be also part of the if () above.
>

Ok

> > + /* select mck clock, and output */
> > + mode |= ATMEL_US_USCLKS_MCK | ATMEL_US_CLKO;
> > + /* set parity for normal/inverse mode + max iterations */
> > + mode |= ATMEL_US_PAR_EVEN | ATMEL_US_NBSTOP_1 | (3 << 24);
> Is this really needed ?
> In the documentation, I found:
> "The configuration is 8 data bits, even parity and 1 or 2 stop bits,
> regardless of the values programmed in the CHRL, MODE9, PAR and CHMODE
> fields."

It's not very clear. It is followed by "Parity Bit (PAR) can be used to
transmit in Normal or Inverse mode." and you have to choose 1 or 2 stop
bits.

> And, for MAX_ITERATIONS, could you add a macro instead of (x << 24) ?
> (ATMEL_US_MAX_ITER mask is already defined).

Yes

> And why 3 ? Should the user-space be allowed to control the max
> automatic iteration ? or is it more like a "same-value-for-every-one"
> thing ?
>

It's an arbitrary choice. I have not seen any reference to number of
repetition in the ISO7816 spec but I may miss it. So I would say it's
not the same value for everyone. Is it useful to let the user setting
it? I don't know, maybe we can wait for a request to export this
setting.

> > +
> > + cd = atmel_calc_cd(port, iso7816conf);
> > + fidi = atmel_calc_fidi(port, iso7816conf);
> > + if (fidi < 0) {
> I guess you meant (fidi == 0)
> Because fidi is unsigned and atmel_calc_fidi() returns also an unsigned.

Right.

>
> > + dev_warn(port->dev, "ISO7816 fidi = 0, Generator generates no signal\n");
> > + } else if (fidi == 1 || fidi == 2) {
> > + dev_err(port->dev, "ISO7816 fidi = %u, value not supported\n", fidi);
> > + ret = -EINVAL;
> > + goto err_out;
> > + }
> And you may also want to check upper values ( <2048 or <65536, depending
> on the SoC)
>

I'll do, I assumed that user space won't request weird values but it's
probably better to not trust him.

> > +
> > + if (!(port->iso7816.flags & SER_ISO7816_ENABLED)) {
> > + /* port not yet in iso7816 mode: store configuration */
> > + atmel_port->backup_mode = atmel_uart_readl(port, ATMEL_US_MR);
> > + atmel_port->backup_brgr = atmel_uart_readl(port, ATMEL_US_BRGR);
> > + }
> > +
> > + /* Actually set ISO7816 mode */
> > + atmel_uart_writel(port, ATMEL_US_TTGR, iso7816conf->tg);
> iso7816conf->tg comes from user-space unchecked. AFAIK, max value is 255

Ok I'll check it.

>
> > + atmel_uart_writel(port, ATMEL_US_BRGR, cd);
> > + atmel_uart_writel(port, ATMEL_US_FIDIR, fidi);
> > +
> > + atmel_port->tx_done_mask = ATMEL_US_TXEMPTY | ATMEL_US_NACK | ATMEL_US_ITERATION;
> > + } else {
> > + dev_dbg(port->dev, "Setting UART to RS232\n");
> > + /* back to last RS232 settings */
> > + mode = atmel_port->backup_mode;
> > + memset(iso7816conf, 0, sizeof(struct serial_iso7816));
> > + atmel_uart_writel(port, ATMEL_US_TTGR, 0);
> > + atmel_uart_writel(port, ATMEL_US_BRGR, atmel_port->backup_brgr);
> > + atmel_uart_writel(port, ATMEL_US_FIDIR, 0x174);
> > +
> > + if (atmel_use_pdc_tx(port))
> > + atmel_port->tx_done_mask = ATMEL_US_ENDTX |
> > + ATMEL_US_TXBUFE;
> > + else
> > + atmel_port->tx_done_mask = ATMEL_US_TXRDY;
> > + }
> > +
> > + port->iso7816 = *iso7816conf;
> > +
> > + atmel_uart_writel(port, ATMEL_US_MR, mode);
> > +
> > +err_out:
> > + /* Enable interrupts */
> > + atmel_uart_writel(port, ATMEL_US_IER, atmel_port->tx_done_mask);
> > + /* Enable RX and TX */
> > + atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RXEN | ATMEL_US_TXEN);
> Is it all right to enable RX/TX unconditionally here ?
>

As I didn't see any issue at the moment, I kept it like this. But I have
noticed this warning in the datasheet and I keep it in mind:
"Enabling both the receiver and the transmitter at the same time in
ISO7816 mode maylead to unpredictable results"

> > +
> > + return ret;
> > +}
> > +
> > /*
> > * Return TIOCSER_TEMT when transmitter FIFO and Shift register is empty.
> > */
> > @@ -481,8 +610,9 @@ static void atmel_stop_tx(struct uart_port *port)
> > /* Disable interrupts */
> > atmel_uart_writel(port, ATMEL_US_IDR, atmel_port->tx_done_mask);
> >
> > - if ((port->rs485.flags & SER_RS485_ENABLED) &&
> > - !(port->rs485.flags & SER_RS485_RX_DURING_TX))
> > + if (((port->rs485.flags & SER_RS485_ENABLED) &&
> > + !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
> > + port->iso7816.flags & SER_ISO7816_ENABLED)
> > atmel_start_rx(port);
> > }
> >
> > @@ -500,8 +630,9 @@ static void atmel_start_tx(struct uart_port *port)
> > return;
> >
> > if (atmel_use_pdc_tx(port) || atmel_use_dma_tx(port))
> > - if ((port->rs485.flags & SER_RS485_ENABLED) &&
> > - !(port->rs485.flags & SER_RS485_RX_DURING_TX))
> > + if (((port->rs485.flags & SER_RS485_ENABLED) &&
> > + !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
> > + port->iso7816.flags & SER_ISO7816_ENABLED)
> > atmel_stop_rx(port);
> >
> > if (atmel_use_pdc_tx(port))
> > @@ -799,8 +930,9 @@ static void atmel_complete_tx_dma(void *arg)
> > */
> > if (!uart_circ_empty(xmit))
> > atmel_tasklet_schedule(atmel_port, &atmel_port->tasklet_tx);
> > - else if ((port->rs485.flags & SER_RS485_ENABLED) &&
> > - !(port->rs485.flags & SER_RS485_RX_DURING_TX)) {
> > + else if (((port->rs485.flags & SER_RS485_ENABLED) &&
> > + !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
> > + port->iso7816.flags & SER_ISO7816_ENABLED) {
> > /* DMA done, stop TX, start RX for RS485 */
> > atmel_start_rx(port);
> > }
> > @@ -1281,6 +1413,9 @@ atmel_handle_status(struct uart_port *port, unsigned int pending,
> > wake_up_interruptible(&port->state->port.delta_msr_wait);
> > }
> > }
> > +
> > + if (pending & (ATMEL_US_NACK | ATMEL_US_ITERATION))
> > + dev_dbg(port->dev, "ISO7816 ERROR (0x%08x)\n", pending);
> > }
> >
> > /*
> > @@ -1373,8 +1508,9 @@ static void atmel_tx_pdc(struct uart_port *port)
> > atmel_uart_writel(port, ATMEL_US_IER,
> > atmel_port->tx_done_mask);
> > } else {
> > - if ((port->rs485.flags & SER_RS485_ENABLED) &&
> > - !(port->rs485.flags & SER_RS485_RX_DURING_TX)) {
> > + if (((port->rs485.flags & SER_RS485_ENABLED) &&
> > + !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
> > + port->iso7816.flags & SER_ISO7816_ENABLED) {
> > /* DMA done, stop TX, start RX for RS485 */
> > atmel_start_rx(port);
> > }
> > @@ -2099,6 +2235,17 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
> > atmel_uart_writel(port, ATMEL_US_TTGR,
> > port->rs485.delay_rts_after_send);
> > mode |= ATMEL_US_USMODE_RS485;
> > + } else if (port->iso7816.flags & SER_ISO7816_ENABLED) {
> > + atmel_uart_writel(port, ATMEL_US_TTGR, port->iso7816.tg);
> > + /* select mck clock, and output */
> > + mode |= ATMEL_US_USCLKS_MCK | ATMEL_US_CLKO;
> > + /* set max iterations */
> > + mode |= (3 << 24);
> Same remark for macro / hardcoded value.
>

OK

> > + if ((port->iso7816.flags & SER_ISO7816_T_PARAM)
> > + == SER_ISO7816_T(0))
> > + mode |= ATMEL_US_USMODE_ISO7816_T0;
> > + else
> > + mode |= ATMEL_US_USMODE_ISO7816_T1;
> > } else if (termios->c_cflag & CRTSCTS) {
> > /* RS232 with hardware handshake (RTS/CTS) */
> > if (atmel_use_fifo(port) &&
> > @@ -2175,7 +2322,8 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
> > }
> > quot = cd | fp << ATMEL_US_FP_OFFSET;
> >
> > - atmel_uart_writel(port, ATMEL_US_BRGR, quot);
> > + if (!(port->iso7816.flags & SER_ISO7816_ENABLED))
> > + atmel_uart_writel(port, ATMEL_US_BRGR, quot);
> > atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
> > atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN | ATMEL_US_RXEN);
> > atmel_port->tx_stopped = false;
> > @@ -2355,6 +2503,7 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port,
> > port->mapbase = pdev->resource[0].start;
> > port->irq = pdev->resource[1].start;
> > port->rs485_config = atmel_config_rs485;
> > + port->iso7816_config = atmel_config_iso7816;
> > port->membase = NULL;
> >
> > memset(&atmel_port->rx_ring, 0, sizeof(atmel_port->rx_ring));
> > @@ -2379,7 +2528,8 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port,
> > }
> >
> > /* Use TXEMPTY for interrupt when rs485 else TXRDY or ENDTX|TXBUFE */
> > - if (port->rs485.flags & SER_RS485_ENABLED)
> > + if (port->rs485.flags & SER_RS485_ENABLED ||
> > + port->iso7816.flags & SER_ISO7816_ENABLED)
> please update the comment above.
>

OK

> > atmel_port->tx_done_mask = ATMEL_US_TXEMPTY;
> > else if (atmel_use_pdc_tx(port)) {
> > port->fifosize = PDC_BUFFER_SIZE;
> > diff --git a/drivers/tty/serial/atmel_serial.h b/drivers/tty/serial/atmel_serial.h
> > index ba3a2437cde4..fff51f5fe8bc 100644
> > --- a/drivers/tty/serial/atmel_serial.h
> > +++ b/drivers/tty/serial/atmel_serial.h
> > @@ -124,7 +124,8 @@
> > #define ATMEL_US_TTGR 0x28 /* Transmitter Timeguard Register */
> > #define ATMEL_US_TG GENMASK(7, 0) /* Timeguard Value */
> >
> > -#define ATMEL_US_FIDI 0x40 /* FI DI Ratio Register */
> > +#define ATMEL_US_FIDIR 0x40 /* FI DI Ratio Register */
> > +#define ATMEL_US_FIDI GENMASK(15, 0) /* FIDI ratio */
> > #define ATMEL_US_NER 0x44 /* Number of Errors Register */
> > #define ATMEL_US_IF 0x4c /* IrDA Filter Register */
> >
> >
>
> Thanks !
>
> Richard.

Thanks for your review, I'll send a new version.

Regards

Ludovic