[PATCH v2 0/2] add ISO7816 support

[PATCH v2 0/2] add ISO7816 support

[Ludovic Desroches]

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

[PATCH v2 0/2] add ISO7816 support

[Ludovic Desroches]

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

[PATCH v2 1/2] 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>
Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
---
 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

[PATCH v2 1/2] 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>
Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
---
 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

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

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 4392 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-rc5 next-20180719]
[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/tty-serial_core-add-ISO7816-infrastructure/20180719-183102
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
config: parisc-generic-64bit_defconfig (attached as .config)
compiler: hppa64-linux-gnu-gcc (GCC) 7.3.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.3.0 make.cross ARCH=parisc 

All errors (new ones prefixed by >>):

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

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

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

---
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: 19052 bytes --]

[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

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

[PATCH v2 2/2] 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>
Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
---
 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

[PATCH v2 2/2] 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>
Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
---
 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

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

[Richard Genoud]

Hi Ludovic,

On 19/07/2018 10:47, 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>
> Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
> ---
>  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.

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

[Richard Genoud]

Hi Ludovic,

On 19/07/2018 10:47, 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>
> Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
> ---
>  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.

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

[Ludovic Desroches]

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 <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>
> > Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
> > ---
> >  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

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

[Ludovic Desroches]

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 <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>
> > Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
> > ---
> >  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

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

[Neil Armstrong]

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(-)
> 

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

[Neil Armstrong]

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(-)
> 

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

[Ludovic Desroches]

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(-)
> > 
> 

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

[Ludovic Desroches]

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(-)
> > 
>