[PATCH 0/3] add ISO7816 support

[PATCH 0/3] add ISO7816 support

[Ludovic Desroches]

Hi,

This set of patches 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.

Nicolas Ferre (3):
  tty/serial_core: add ISO7816 infrastructure
  tty/serial: atmel: add ISO7816 support
  tty/serial: atmel: manage shutdown in case of RS485 or ISO7816 mode

 drivers/tty/serial/atmel_serial.c | 179 +++++++++++++++++++++++++++++++++++---
 drivers/tty/serial/atmel_serial.h |   3 +-
 drivers/tty/serial/serial_core.c  |  49 +++++++++++
 include/linux/serial_core.h       |   3 +
 include/uapi/asm-generic/ioctls.h |   2 +
 include/uapi/linux/serial.h       |  17 ++++
 6 files changed, 241 insertions(+), 12 deletions(-)

-- 
2.12.2

[PATCH 1/3] tty/serial_core: add ISO7816 infrastructure

[Ludovic Desroches]

From: Nicolas Ferre <nicolas.ferre@microchip.com>

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 <nicolas.ferre@microchip.com>
[ludovic.desroches@microchip.com: squash and rebase, removal of gpios, checkpatch fixes]
Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
---
 drivers/tty/serial/serial_core.c  | 49 +++++++++++++++++++++++++++++++++++++++
 include/linux/serial_core.h       |  3 +++
 include/uapi/asm-generic/ioctls.h |  2 ++
 include/uapi/linux/serial.h       | 17 ++++++++++++++
 4 files changed, 71 insertions(+)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 9c14a453f73c..c89ac59f6f8c 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1301,6 +1301,47 @@ 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;
+
+	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 ret;
+	unsigned long flags;
+
+	if (!port->iso7816_config)
+		return -ENOIOCTLCMD;
+
+	if (copy_from_user(&iso7816, iso7816_user, sizeof(*iso7816_user)))
+		return -EFAULT;
+
+	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 +1426,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..0e5c79726c2d 100644
--- a/include/uapi/asm-generic/ioctls.h
+++ b/include/uapi/asm-generic/ioctls.h
@@ -66,6 +66,8 @@
 #ifndef TIOCSRS485
 #define TIOCSRS485	0x542F
 #endif
+#define TIOCGISO7816	0x5430
+#define TIOCSISO7816	0x5431
 #define TIOCGPTN	_IOR('T', 0x30, unsigned int) /* Get Pty Number (of pty-mux device) */
 #define TIOCSPTLCK	_IOW('T', 0x31, int)  /* Lock/unlock Pty */
 #define TIOCGDEV	_IOR('T', 0x32, unsigned int) /* Get primary device node of /dev/console */
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

[PATCH 2/3] tty/serial: atmel: add ISO7816 support

[Ludovic Desroches]

From: Nicolas Ferre <nicolas.ferre@microchip.com>

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 <nicolas.ferre@microchip.com>
[ludovic.desroches@microchip.com: rebase, add check on fidi ratio, checkpatch fixes]
Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
---
 drivers/tty/serial/atmel_serial.c | 175 +++++++++++++++++++++++++++++++++++---
 drivers/tty/serial/atmel_serial.h |   3 +-
 2 files changed, 167 insertions(+), 11 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 8e4428725848..0118b219f3a8 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,136 @@ 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);
+	dev_dbg(port->dev, "ISO7816 mck_rate = %lld\n", mck_rate);
+	do_div(mck_rate, iso7816conf->clk);
+	cd = mck_rate;
+	dev_dbg(port->dev, "ISO7816 clk = %d. CD = %d\n", iso7816conf->clk, cd);
+	return cd;
+}
+
+static unsigned int atmel_calc_fidi(struct uart_port *port,
+				    struct serial_iso7816 *iso7816conf)
+{
+	u64 fidi = 0;
+
+	dev_dbg(port->dev, "ISO7816 fi(%d) / di(%d)\n",
+		iso7816conf->sc_fi, iso7816conf->sc_di);
+	if (iso7816conf->sc_fi && iso7816conf->sc_di) {
+		fidi = (u64)iso7816conf->sc_fi;
+		do_div(fidi, iso7816conf->sc_di);
+	}
+	dev_dbg(port->dev, "ISO7816 fidi = 0x%x\n", (u32)fidi);
+	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 (!(port->iso7816.flags & SER_ISO7816_ENABLED)) {
+		/* port not yet in iso7816 mode: store configuration */
+		atmel_port->backup_mode = mode;
+		atmel_port->backup_brgr = atmel_uart_readl(port, ATMEL_US_BRGR);
+	}
+
+	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;
+		}
+		/* 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 +614,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 +634,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 +934,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 +1417,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_err(port->dev, "ISO7816 ERROR (0x%08x)\n", pending);
 }
 
 /*
@@ -1373,8 +1512,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 +2239,18 @@ 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) {
+		dev_dbg(port->dev, "Setting UART to ISO7816 in termios\n");
+		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 +2327,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 +2508,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 +2533,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

[PATCH 3/3] tty/serial: atmel: manage shutdown in case of RS485 or ISO7816 mode

[Ludovic Desroches]

From: Nicolas Ferre <nicolas.ferre@microchip.com>

In atmel_shutdown() we call atmel_stop_rx() and atmel_stop_tx() functions.
Prevent the rx restart that is implemented in RS485 or ISO7816 modes when
calling atmel_stop_tx() by using the atomic information tasklet_shutdown
that is already in place for this purpose.

Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>
---
 drivers/tty/serial/atmel_serial.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 0118b219f3a8..e4f877e1f3c6 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -617,7 +617,9 @@ static void atmel_stop_tx(struct uart_port *port)
 	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);
+		if (!atomic_read(&atmel_port->tasklet_shutdown))
+			atmel_start_rx(port);
+
 }
 
 /*
-- 
2.12.2

[PATCH 3/3] tty/serial: atmel: manage shutdown in case of RS485 or ISO7816 mode

[Ludovic Desroches]

From: Nicolas Ferre <nicolas.ferre@microchip.com>

In atmel_shutdown() we call atmel_stop_rx() and atmel_stop_tx() functions.
Prevent the rx restart that is implemented in RS485 or ISO7816 modes when
calling atmel_stop_tx() by using the atomic information tasklet_shutdown
that is already in place for this purpose.

Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>
---
 drivers/tty/serial/atmel_serial.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 0118b219f3a8..e4f877e1f3c6 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -617,7 +617,9 @@ static void atmel_stop_tx(struct uart_port *port)
 	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);
+		if (!atomic_read(&atmel_port->tasklet_shutdown))
+			atmel_start_rx(port);
+
 }
 
 /*
-- 
2.12.2

Re: [PATCH 1/3] tty/serial_core: add ISO7816 infrastructure

[kbuild test robot]

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

Hi Nicolas,

I love your patch! Yet something to improve:

[auto build test ERROR on tty/tty-testing]
[also build test ERROR on v4.18-rc4 next-20180711]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Ludovic-Desroches/add-ISO7816-support/20180712-052207
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
config: sparc64-allyesconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=sparc64 

All errors (new ones prefixed by >>):

   drivers/tty/serial/serial_core.c: In function 'uart_ioctl':
   drivers/tty/serial/serial_core.c:1430:7: error: 'TIOCSISO7816' undeclared (first use in this function); did you mean 'TIOCSRS485'?
     case TIOCSISO7816:
          ^~~~~~~~~~~~
          TIOCSRS485
   drivers/tty/serial/serial_core.c:1430:7: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/tty/serial/serial_core.c:1434:7: error: 'TIOCGISO7816' undeclared (first use in this function); did you mean 'TIOCSISO7816'?
     case TIOCGISO7816:
          ^~~~~~~~~~~~
          TIOCSISO7816

vim +1434 drivers/tty/serial/serial_core.c

  1344	
  1345	/*
  1346	 * Called via sys_ioctl.  We can use spin_lock_irq() here.
  1347	 */
  1348	static int
  1349	uart_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg)
  1350	{
  1351		struct uart_state *state = tty->driver_data;
  1352		struct tty_port *port = &state->port;
  1353		struct uart_port *uport;
  1354		void __user *uarg = (void __user *)arg;
  1355		int ret = -ENOIOCTLCMD;
  1356	
  1357	
  1358		/*
  1359		 * These ioctls don't rely on the hardware to be present.
  1360		 */
  1361		switch (cmd) {
  1362		case TIOCGSERIAL:
  1363			ret = uart_get_info_user(port, uarg);
  1364			break;
  1365	
  1366		case TIOCSSERIAL:
  1367			down_write(&tty->termios_rwsem);
  1368			ret = uart_set_info_user(tty, state, uarg);
  1369			up_write(&tty->termios_rwsem);
  1370			break;
  1371	
  1372		case TIOCSERCONFIG:
  1373			down_write(&tty->termios_rwsem);
  1374			ret = uart_do_autoconfig(tty, state);
  1375			up_write(&tty->termios_rwsem);
  1376			break;
  1377	
  1378		case TIOCSERGWILD: /* obsolete */
  1379		case TIOCSERSWILD: /* obsolete */
  1380			ret = 0;
  1381			break;
  1382		}
  1383	
  1384		if (ret != -ENOIOCTLCMD)
  1385			goto out;
  1386	
  1387		if (tty_io_error(tty)) {
  1388			ret = -EIO;
  1389			goto out;
  1390		}
  1391	
  1392		/*
  1393		 * The following should only be used when hardware is present.
  1394		 */
  1395		switch (cmd) {
  1396		case TIOCMIWAIT:
  1397			ret = uart_wait_modem_status(state, arg);
  1398			break;
  1399		}
  1400	
  1401		if (ret != -ENOIOCTLCMD)
  1402			goto out;
  1403	
  1404		mutex_lock(&port->mutex);
  1405		uport = uart_port_check(state);
  1406	
  1407		if (!uport || tty_io_error(tty)) {
  1408			ret = -EIO;
  1409			goto out_up;
  1410		}
  1411	
  1412		/*
  1413		 * All these rely on hardware being present and need to be
  1414		 * protected against the tty being hung up.
  1415		 */
  1416	
  1417		switch (cmd) {
  1418		case TIOCSERGETLSR: /* Get line status register */
  1419			ret = uart_get_lsr_info(tty, state, uarg);
  1420			break;
  1421	
  1422		case TIOCGRS485:
  1423			ret = uart_get_rs485_config(uport, uarg);
  1424			break;
  1425	
  1426		case TIOCSRS485:
  1427			ret = uart_set_rs485_config(uport, uarg);
  1428			break;
  1429	
> 1430		case TIOCSISO7816:
  1431			ret = uart_set_iso7816_config(state->uart_port, uarg);
  1432			break;
  1433	
> 1434		case TIOCGISO7816:
  1435			ret = uart_get_iso7816_config(state->uart_port, uarg);
  1436			break;
  1437		default:
  1438			if (uport->ops->ioctl)
  1439				ret = uport->ops->ioctl(uport, cmd, arg);
  1440			break;
  1441		}
  1442	out_up:
  1443		mutex_unlock(&port->mutex);
  1444	out:
  1445		return ret;
  1446	}
  1447	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 54391 bytes --]

Re: [PATCH 1/3] tty/serial_core: add ISO7816 infrastructure

[kbuild test robot]

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

Hi Nicolas,

I love your patch! Yet something to improve:

[auto build test ERROR on tty/tty-testing]
[also build test ERROR on v4.18-rc4 next-20180711]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Ludovic-Desroches/add-ISO7816-support/20180712-052207
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
config: xtensa-allmodconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 8.1.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=8.1.0 make.cross ARCH=xtensa 

All errors (new ones prefixed by >>):

   drivers/tty/serial/serial_core.c: In function 'uart_ioctl':
   drivers/tty/serial/serial_core.c:1430:7: error: 'TIOCSISO7816' undeclared (first use in this function); did you mean 'TIOCSRS485'?
     case TIOCSISO7816:
          ^~~~~~~~~~~~
          TIOCSRS485
   drivers/tty/serial/serial_core.c:1430:7: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/tty/serial/serial_core.c:1434:7: error: 'TIOCGISO7816' undeclared (first use in this function); did you mean 'TIOCGRS485'?
     case TIOCGISO7816:
          ^~~~~~~~~~~~
          TIOCGRS485

vim +1434 drivers/tty/serial/serial_core.c

  1344	
  1345	/*
  1346	 * Called via sys_ioctl.  We can use spin_lock_irq() here.
  1347	 */
  1348	static int
  1349	uart_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg)
  1350	{
  1351		struct uart_state *state = tty->driver_data;
  1352		struct tty_port *port = &state->port;
  1353		struct uart_port *uport;
  1354		void __user *uarg = (void __user *)arg;
  1355		int ret = -ENOIOCTLCMD;
  1356	
  1357	
  1358		/*
  1359		 * These ioctls don't rely on the hardware to be present.
  1360		 */
  1361		switch (cmd) {
  1362		case TIOCGSERIAL:
  1363			ret = uart_get_info_user(port, uarg);
  1364			break;
  1365	
  1366		case TIOCSSERIAL:
  1367			down_write(&tty->termios_rwsem);
  1368			ret = uart_set_info_user(tty, state, uarg);
  1369			up_write(&tty->termios_rwsem);
  1370			break;
  1371	
  1372		case TIOCSERCONFIG:
  1373			down_write(&tty->termios_rwsem);
  1374			ret = uart_do_autoconfig(tty, state);
  1375			up_write(&tty->termios_rwsem);
  1376			break;
  1377	
  1378		case TIOCSERGWILD: /* obsolete */
  1379		case TIOCSERSWILD: /* obsolete */
  1380			ret = 0;
  1381			break;
  1382		}
  1383	
  1384		if (ret != -ENOIOCTLCMD)
  1385			goto out;
  1386	
  1387		if (tty_io_error(tty)) {
  1388			ret = -EIO;
  1389			goto out;
  1390		}
  1391	
  1392		/*
  1393		 * The following should only be used when hardware is present.
  1394		 */
  1395		switch (cmd) {
  1396		case TIOCMIWAIT:
  1397			ret = uart_wait_modem_status(state, arg);
  1398			break;
  1399		}
  1400	
  1401		if (ret != -ENOIOCTLCMD)
  1402			goto out;
  1403	
  1404		mutex_lock(&port->mutex);
  1405		uport = uart_port_check(state);
  1406	
  1407		if (!uport || tty_io_error(tty)) {
  1408			ret = -EIO;
  1409			goto out_up;
  1410		}
  1411	
  1412		/*
  1413		 * All these rely on hardware being present and need to be
  1414		 * protected against the tty being hung up.
  1415		 */
  1416	
  1417		switch (cmd) {
  1418		case TIOCSERGETLSR: /* Get line status register */
  1419			ret = uart_get_lsr_info(tty, state, uarg);
  1420			break;
  1421	
  1422		case TIOCGRS485:
  1423			ret = uart_get_rs485_config(uport, uarg);
  1424			break;
  1425	
  1426		case TIOCSRS485:
  1427			ret = uart_set_rs485_config(uport, uarg);
  1428			break;
  1429	
> 1430		case TIOCSISO7816:
  1431			ret = uart_set_iso7816_config(state->uart_port, uarg);
  1432			break;
  1433	
> 1434		case TIOCGISO7816:
  1435			ret = uart_get_iso7816_config(state->uart_port, uarg);
  1436			break;
  1437		default:
  1438			if (uport->ops->ioctl)
  1439				ret = uport->ops->ioctl(uport, cmd, arg);
  1440			break;
  1441		}
  1442	out_up:
  1443		mutex_unlock(&port->mutex);
  1444	out:
  1445		return ret;
  1446	}
  1447	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 54028 bytes --]

Re: [PATCH 1/3] tty/serial_core: add ISO7816 infrastructure

[Greg KH]

On Wed, Jul 11, 2018 at 03:16:36PM +0200, Ludovic Desroches wrote:
> From: Nicolas Ferre <nicolas.ferre@microchip.com>
> 
> 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 <nicolas.ferre@microchip.com>
> [ludovic.desroches@microchip.com: squash and rebase, removal of gpios, checkpatch fixes]
> Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
> ---
>  drivers/tty/serial/serial_core.c  | 49 +++++++++++++++++++++++++++++++++++++++
>  include/linux/serial_core.h       |  3 +++
>  include/uapi/asm-generic/ioctls.h |  2 ++
>  include/uapi/linux/serial.h       | 17 ++++++++++++++
>  4 files changed, 71 insertions(+)

Note, kbuild found build issues with this, please fix up.

Also, here's some comments:

> 
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 9c14a453f73c..c89ac59f6f8c 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -1301,6 +1301,47 @@ 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;
> +
> +	spin_lock_irqsave(&port->lock, flags);
> +	aux = port->iso7816;

Don't you want to check to see if there is a function for iso7816 before
copying it?  Otherwise it's just empty, right?

> +	if (!port->iso7816_config)
> +		return -ENOIOCTLCMD;

Why this error value?

> --- a/include/uapi/asm-generic/ioctls.h
> +++ b/include/uapi/asm-generic/ioctls.h
> @@ -66,6 +66,8 @@
>  #ifndef TIOCSRS485
>  #define TIOCSRS485	0x542F
>  #endif
> +#define TIOCGISO7816	0x5430
> +#define TIOCSISO7816	0x5431

Where did you get these numbers from?

thanks,

greg k-h

Re: [PATCH 3/3] tty/serial: atmel: manage shutdown in case of RS485 or ISO7816 mode

[Greg KH]

On Wed, Jul 11, 2018 at 03:26:23PM +0200, Ludovic Desroches wrote:
> From: Nicolas Ferre <nicolas.ferre@microchip.com>
> 
> In atmel_shutdown() we call atmel_stop_rx() and atmel_stop_tx() functions.
> Prevent the rx restart that is implemented in RS485 or ISO7816 modes when
> calling atmel_stop_tx() by using the atomic information tasklet_shutdown
> that is already in place for this purpose.
> 
> Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>
> ---
>  drivers/tty/serial/atmel_serial.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Did you send this patch twice?

confused,

greg k-h

Re: [PATCH 3/3] tty/serial: atmel: manage shutdown in case of RS485 or ISO7816 mode

[Greg KH]

On Wed, Jul 11, 2018 at 03:16:38PM +0200, Ludovic Desroches wrote:
> From: Nicolas Ferre <nicolas.ferre@microchip.com>
> 
> In atmel_shutdown() we call atmel_stop_rx() and atmel_stop_tx() functions.
> Prevent the rx restart that is implemented in RS485 or ISO7816 modes when
> calling atmel_stop_tx() by using the atomic information tasklet_shutdown
> that is already in place for this purpose.
> 
> Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>

No signed-off-by from you?

> ---
>  drivers/tty/serial/atmel_serial.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 0118b219f3a8..e4f877e1f3c6 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -617,7 +617,9 @@ static void atmel_stop_tx(struct uart_port *port)
>  	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);
> +		if (!atomic_read(&atmel_port->tasklet_shutdown))
> +			atmel_start_rx(port);

Is this really ok?  No locking needed?  What happens if that value is
set right after this?

thanks,

greg k-h

Re: [PATCH 2/3] tty/serial: atmel: add ISO7816 support

[Greg KH]

On Wed, Jul 11, 2018 at 03:16:37PM +0200, Ludovic Desroches wrote:
> From: Nicolas Ferre <nicolas.ferre@microchip.com>
> 
> 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 <nicolas.ferre@microchip.com>
> [ludovic.desroches@microchip.com: rebase, add check on fidi ratio, checkpatch fixes]
> Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
> ---
>  drivers/tty/serial/atmel_serial.c | 175 +++++++++++++++++++++++++++++++++++---
>  drivers/tty/serial/atmel_serial.h |   3 +-
>  2 files changed, 167 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 8e4428725848..0118b219f3a8 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,136 @@ 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);
> +	dev_dbg(port->dev, "ISO7816 mck_rate = %lld\n", mck_rate);
> +	do_div(mck_rate, iso7816conf->clk);
> +	cd = mck_rate;
> +	dev_dbg(port->dev, "ISO7816 clk = %d. CD = %d\n", iso7816conf->clk, cd);
> +	return cd;
> +}
> +
> +static unsigned int atmel_calc_fidi(struct uart_port *port,
> +				    struct serial_iso7816 *iso7816conf)
> +{
> +	u64 fidi = 0;
> +
> +	dev_dbg(port->dev, "ISO7816 fi(%d) / di(%d)\n",
> +		iso7816conf->sc_fi, iso7816conf->sc_di);

Do you still need all of the debugging code?  Shouldn't ftrace give you
all of this?



> +	if (iso7816conf->sc_fi && iso7816conf->sc_di) {
> +		fidi = (u64)iso7816conf->sc_fi;
> +		do_div(fidi, iso7816conf->sc_di);
> +	}
> +	dev_dbg(port->dev, "ISO7816 fidi = 0x%x\n", (u32)fidi);
> +	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 (!(port->iso7816.flags & SER_ISO7816_ENABLED)) {
> +		/* port not yet in iso7816 mode: store configuration */
> +		atmel_port->backup_mode = mode;
> +		atmel_port->backup_brgr = atmel_uart_readl(port, ATMEL_US_BRGR);
> +	}

Are you really validating all of the fields in this structure for valid
values correctly?  It feels like you are missing some here...

> @@ -1281,6 +1417,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_err(port->dev, "ISO7816 ERROR (0x%08x)\n", pending);

What can someone do with this?

If nothing, then why check or tell the user about it?  They will ignore
it :(

thanks,

greg k-h

Re: [PATCH 1/3] tty/serial_core: add ISO7816 infrastructure

[Greg KH]

On Wed, Jul 11, 2018 at 03:16:36PM +0200, Ludovic Desroches wrote:
> From: Nicolas Ferre <nicolas.ferre@microchip.com>
> 
> 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 <nicolas.ferre@microchip.com>
> [ludovic.desroches@microchip.com: squash and rebase, removal of gpios, checkpatch fixes]
> Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
> ---
>  drivers/tty/serial/serial_core.c  | 49 +++++++++++++++++++++++++++++++++++++++
>  include/linux/serial_core.h       |  3 +++
>  include/uapi/asm-generic/ioctls.h |  2 ++
>  include/uapi/linux/serial.h       | 17 ++++++++++++++
>  4 files changed, 71 insertions(+)
> 
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 9c14a453f73c..c89ac59f6f8c 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -1301,6 +1301,47 @@ 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;
> +
> +	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 ret;
> +	unsigned long flags;
> +
> +	if (!port->iso7816_config)
> +		return -ENOIOCTLCMD;
> +
> +	if (copy_from_user(&iso7816, iso7816_user, sizeof(*iso7816_user)))
> +		return -EFAULT;
> +
> +	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 +1426,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..0e5c79726c2d 100644
> --- a/include/uapi/asm-generic/ioctls.h
> +++ b/include/uapi/asm-generic/ioctls.h
> @@ -66,6 +66,8 @@
>  #ifndef TIOCSRS485
>  #define TIOCSRS485	0x542F
>  #endif
> +#define TIOCGISO7816	0x5430
> +#define TIOCSISO7816	0x5431
>  #define TIOCGPTN	_IOR('T', 0x30, unsigned int) /* Get Pty Number (of pty-mux device) */
>  #define TIOCSPTLCK	_IOW('T', 0x31, int)  /* Lock/unlock Pty */
>  #define TIOCGDEV	_IOR('T', 0x32, unsigned int) /* Get primary device node of /dev/console */
> 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];

You need to verify that reserved[] is all set to 0, otherwise people
will put crud in there and you can never use it for anything in the
future.

thanks,

greg k-h

Re: [PATCH 3/3] tty/serial: atmel: manage shutdown in case of RS485 or ISO7816 mode

[Ludovic Desroches]

On Thu, Jul 12, 2018 at 04:58:27PM +0200, Greg KH wrote:
> On Wed, Jul 11, 2018 at 03:26:23PM +0200, Ludovic Desroches wrote:
> > From: Nicolas Ferre <nicolas.ferre@microchip.com>
> > 
> > In atmel_shutdown() we call atmel_stop_rx() and atmel_stop_tx() functions.
> > Prevent the rx restart that is implemented in RS485 or ISO7816 modes when
> > calling atmel_stop_tx() by using the atomic information tasklet_shutdown
> > that is already in place for this purpose.
> > 
> > Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>
> > ---
> >  drivers/tty/serial/atmel_serial.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> Did you send this patch twice?
> 
> confused,

Yes sorry, as I didn't receive patch 3/3, I resend it. At the end, it
was sent twice.

Regards

Ludovic

> 
> greg k-h
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/3] tty/serial_core: add ISO7816 infrastructure

[Ludovic Desroches]

On Thu, Jul 12, 2018 at 05:02:29PM +0200, Greg KH wrote:
> On Wed, Jul 11, 2018 at 03:16:36PM +0200, Ludovic Desroches wrote:
> > From: Nicolas Ferre <nicolas.ferre@microchip.com>
> > 
> > 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 <nicolas.ferre@microchip.com>
> > [ludovic.desroches@microchip.com: squash and rebase, removal of gpios, checkpatch fixes]
> > Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
> > ---
> >  drivers/tty/serial/serial_core.c  | 49 +++++++++++++++++++++++++++++++++++++++
> >  include/linux/serial_core.h       |  3 +++
> >  include/uapi/asm-generic/ioctls.h |  2 ++
> >  include/uapi/linux/serial.h       | 17 ++++++++++++++
> >  4 files changed, 71 insertions(+)
> > 
> > diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> > index 9c14a453f73c..c89ac59f6f8c 100644
> > --- a/drivers/tty/serial/serial_core.c
> > +++ b/drivers/tty/serial/serial_core.c
> > @@ -1301,6 +1301,47 @@ 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;
> > +
> > +	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 ret;
> > +	unsigned long flags;
> > +
> > +	if (!port->iso7816_config)
> > +		return -ENOIOCTLCMD;
> > +
> > +	if (copy_from_user(&iso7816, iso7816_user, sizeof(*iso7816_user)))
> > +		return -EFAULT;
> > +
> > +	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 +1426,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..0e5c79726c2d 100644
> > --- a/include/uapi/asm-generic/ioctls.h
> > +++ b/include/uapi/asm-generic/ioctls.h
> > @@ -66,6 +66,8 @@
> >  #ifndef TIOCSRS485
> >  #define TIOCSRS485	0x542F
> >  #endif
> > +#define TIOCGISO7816	0x5430
> > +#define TIOCSISO7816	0x5431
> >  #define TIOCGPTN	_IOR('T', 0x30, unsigned int) /* Get Pty Number (of pty-mux device) */
> >  #define TIOCSPTLCK	_IOW('T', 0x31, int)  /* Lock/unlock Pty */
> >  #define TIOCGDEV	_IOR('T', 0x32, unsigned int) /* Get primary device node of /dev/console */
> > 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];
> 
> You need to verify that reserved[] is all set to 0, otherwise people
> will put crud in there and you can never use it for anything in the
> future.

Should I just verify or force it to 0?

Regards

Ludovic

> 
> thanks,
> 
> greg k-h
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/3] tty/serial_core: add ISO7816 infrastructure

[Ludovic Desroches]

On Thu, Jul 12, 2018 at 04:58:08PM +0200, Greg KH wrote:
> On Wed, Jul 11, 2018 at 03:16:36PM +0200, Ludovic Desroches wrote:
> > From: Nicolas Ferre <nicolas.ferre@microchip.com>
> > 
> > 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 <nicolas.ferre@microchip.com>
> > [ludovic.desroches@microchip.com: squash and rebase, removal of gpios, checkpatch fixes]
> > Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
> > ---
> >  drivers/tty/serial/serial_core.c  | 49 +++++++++++++++++++++++++++++++++++++++
> >  include/linux/serial_core.h       |  3 +++
> >  include/uapi/asm-generic/ioctls.h |  2 ++
> >  include/uapi/linux/serial.h       | 17 ++++++++++++++
> >  4 files changed, 71 insertions(+)
> 
> Note, kbuild found build issues with this, please fix up.
> 
> Also, here's some comments:
> 
> > 
> > diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> > index 9c14a453f73c..c89ac59f6f8c 100644
> > --- a/drivers/tty/serial/serial_core.c
> > +++ b/drivers/tty/serial/serial_core.c
> > @@ -1301,6 +1301,47 @@ 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;
> > +
> > +	spin_lock_irqsave(&port->lock, flags);
> > +	aux = port->iso7816;
> 
> Don't you want to check to see if there is a function for iso7816 before
> copying it?  Otherwise it's just empty, right?

I'll add the check.

> 
> > +	if (!port->iso7816_config)
> > +		return -ENOIOCTLCMD;
> 
> Why this error value?
> 

It was a mimic of RS485.

> > --- a/include/uapi/asm-generic/ioctls.h
> > +++ b/include/uapi/asm-generic/ioctls.h
> > @@ -66,6 +66,8 @@
> >  #ifndef TIOCSRS485
> >  #define TIOCSRS485	0x542F
> >  #endif
> > +#define TIOCGISO7816	0x5430
> > +#define TIOCSISO7816	0x5431
> 
> Where did you get these numbers from?

I will use the macros to create numbers. Any rules about nr choice?

Regards

Ludovic

> 
> thanks,
> 
> greg k-h

Re: [PATCH 1/3] tty/serial_core: add ISO7816 infrastructure

[Greg KH]

On Fri, Jul 13, 2018 at 09:56:48AM +0200, Ludovic Desroches wrote:
> On Thu, Jul 12, 2018 at 05:02:29PM +0200, Greg KH wrote:
> > On Wed, Jul 11, 2018 at 03:16:36PM +0200, Ludovic Desroches wrote:
> > > From: Nicolas Ferre <nicolas.ferre@microchip.com>
> > > 
> > > 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 <nicolas.ferre@microchip.com>
> > > [ludovic.desroches@microchip.com: squash and rebase, removal of gpios, checkpatch fixes]
> > > Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
> > > ---
> > >  drivers/tty/serial/serial_core.c  | 49 +++++++++++++++++++++++++++++++++++++++
> > >  include/linux/serial_core.h       |  3 +++
> > >  include/uapi/asm-generic/ioctls.h |  2 ++
> > >  include/uapi/linux/serial.h       | 17 ++++++++++++++
> > >  4 files changed, 71 insertions(+)
> > > 
> > > diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> > > index 9c14a453f73c..c89ac59f6f8c 100644
> > > --- a/drivers/tty/serial/serial_core.c
> > > +++ b/drivers/tty/serial/serial_core.c
> > > @@ -1301,6 +1301,47 @@ 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;
> > > +
> > > +	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 ret;
> > > +	unsigned long flags;
> > > +
> > > +	if (!port->iso7816_config)
> > > +		return -ENOIOCTLCMD;
> > > +
> > > +	if (copy_from_user(&iso7816, iso7816_user, sizeof(*iso7816_user)))
> > > +		return -EFAULT;
> > > +
> > > +	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 +1426,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..0e5c79726c2d 100644
> > > --- a/include/uapi/asm-generic/ioctls.h
> > > +++ b/include/uapi/asm-generic/ioctls.h
> > > @@ -66,6 +66,8 @@
> > >  #ifndef TIOCSRS485
> > >  #define TIOCSRS485	0x542F
> > >  #endif
> > > +#define TIOCGISO7816	0x5430
> > > +#define TIOCSISO7816	0x5431
> > >  #define TIOCGPTN	_IOR('T', 0x30, unsigned int) /* Get Pty Number (of pty-mux device) */
> > >  #define TIOCSPTLCK	_IOW('T', 0x31, int)  /* Lock/unlock Pty */
> > >  #define TIOCGDEV	_IOR('T', 0x32, unsigned int) /* Get primary device node of /dev/console */
> > > 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];
> > 
> > You need to verify that reserved[] is all set to 0, otherwise people
> > will put crud in there and you can never use it for anything in the
> > future.
> 
> Should I just verify or force it to 0?

Verify please, and then abort if they are not set to 0.  Otherwise if
userspace puts odd stuff in there, and then later you decide to use it
as new fields, that old code will start to do odd things.

thanks,

greg k-h

Re: [PATCH 1/3] tty/serial_core: add ISO7816 infrastructure

[Alan Cox]

> >   
> > > +	if (!port->iso7816_config)
> > > +		return -ENOIOCTLCMD;  
> > 
> > Why this error value?
> >   
> 
> It was a mimic of RS485.

Which is what you want - it means the upper tty layer knows to offer the
ioctl to other places and then return appropriately.

Alan