On Fri, Jul 13, 2018 at 09:56:48AM +0200, Ludovic Desroches wrote:
> On Thu, Jul 12, 2018 at 05:02:29PM +0200, Greg KH wrote:
> > On Wed, Jul 11, 2018 at 03:16:36PM +0200, Ludovic Desroches wrote:
> > > From: Nicolas Ferre <nicolas.ferre@xxxxxxxxxxxxx>
> > >
> > > 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@xxxxxxxxxxxxx>
> > > [ludovic.desroches@xxxxxxxxxxxxx: squash and rebase, removal of gpios, checkpatch fixes]
> > > Signed-off-by: Ludovic Desroches <ludovic.desroches@xxxxxxxxxxxxx>
> > > ---
> > > drivers/tty/serial/serial_core.c | 49 +++++++++++++++++++++++++++++++++++++++
> > > include/linux/serial_core.h | 3 +++
> > > include/uapi/asm-generic/ioctls.h | 2 ++
> > > include/uapi/linux/serial.h | 17 ++++++++++++++
> > > 4 files changed, 71 insertions(+)
> > >
> > > diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> > > index 9c14a453f73c..c89ac59f6f8c 100644
> > > --- a/drivers/tty/serial/serial_core.c
> > > +++ b/drivers/tty/serial/serial_core.c
> > > @@ -1301,6 +1301,47 @@ static int uart_set_rs485_config(struct uart_port *port,
> > > return 0;
> > > }
> > >
> > > +static int uart_get_iso7816_config(struct uart_port *port,
> > > + struct serial_iso7816 __user *iso7816)
> > > +{
> > > + unsigned long flags;
> > > + struct serial_iso7816 aux;
> > > +
> > > + spin_lock_irqsave(&port->lock, flags);
> > > + aux = port->iso7816;
> > > + spin_unlock_irqrestore(&port->lock, flags);
> > > +
> > > + if (copy_to_user(iso7816, &aux, sizeof(aux)))
> > > + return -EFAULT;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int uart_set_iso7816_config(struct uart_port *port,
> > > + struct serial_iso7816 __user *iso7816_user)
> > > +{
> > > + struct serial_iso7816 iso7816;
> > > + int ret;
> > > + unsigned long flags;
> > > +
> > > + if (!port->iso7816_config)
> > > + return -ENOIOCTLCMD;
> > > +
> > > + if (copy_from_user(&iso7816, iso7816_user, sizeof(*iso7816_user)))
> > > + return -EFAULT;
> > > +
> > > + spin_lock_irqsave(&port->lock, flags);
> > > + ret = port->iso7816_config(port, &iso7816);
> > > + spin_unlock_irqrestore(&port->lock, flags);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + if (copy_to_user(iso7816_user, &port->iso7816, sizeof(port->iso7816)))
> > > + return -EFAULT;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > /*
> > > * Called via sys_ioctl. We can use spin_lock_irq() here.
> > > */
> > > @@ -1385,6 +1426,14 @@ uart_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg)
> > > case TIOCSRS485:
> > > ret = uart_set_rs485_config(uport, uarg);
> > > break;
> > > +
> > > + case TIOCSISO7816:
> > > + ret = uart_set_iso7816_config(state->uart_port, uarg);
> > > + break;
> > > +
> > > + case TIOCGISO7816:
> > > + ret = uart_get_iso7816_config(state->uart_port, uarg);
> > > + break;
> > > default:
> > > if (uport->ops->ioctl)
> > > ret = uport->ops->ioctl(uport, cmd, arg);
> > > diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> > > index 06ea4eeb09ab..d6e7747ffd46 100644
> > > --- a/include/linux/serial_core.h
> > > +++ b/include/linux/serial_core.h
> > > @@ -137,6 +137,8 @@ struct uart_port {
> > > void (*handle_break)(struct uart_port *);
> > > int (*rs485_config)(struct uart_port *,
> > > struct serial_rs485 *rs485);
> > > + int (*iso7816_config)(struct uart_port *,
> > > + struct serial_iso7816 *iso7816);
> > > unsigned int irq; /* irq number */
> > > unsigned long irqflags; /* irq flags */
> > > unsigned int uartclk; /* base uart clock */
> > > @@ -253,6 +255,7 @@ struct uart_port {
> > > struct attribute_group *attr_group; /* port specific attributes */
> > > const struct attribute_group **tty_groups; /* all attributes (serial core use only) */
> > > struct serial_rs485 rs485;
> > > + struct serial_iso7816 iso7816;
> > > void *private_data; /* generic platform data pointer */
> > > };
> > >
> > > diff --git a/include/uapi/asm-generic/ioctls.h b/include/uapi/asm-generic/ioctls.h
> > > index 040651735662..0e5c79726c2d 100644
> > > --- a/include/uapi/asm-generic/ioctls.h
> > > +++ b/include/uapi/asm-generic/ioctls.h
> > > @@ -66,6 +66,8 @@
> > > #ifndef TIOCSRS485
> > > #define TIOCSRS485 0x542F
> > > #endif
> > > +#define TIOCGISO7816 0x5430
> > > +#define TIOCSISO7816 0x5431
> > > #define TIOCGPTN _IOR('T', 0x30, unsigned int) /* Get Pty Number (of pty-mux device) */
> > > #define TIOCSPTLCK _IOW('T', 0x31, int) /* Lock/unlock Pty */
> > > #define TIOCGDEV _IOR('T', 0x32, unsigned int) /* Get primary device node of /dev/console */
> > > diff --git a/include/uapi/linux/serial.h b/include/uapi/linux/serial.h
> > > index 3fdd0dee8b41..93eb3c496ff1 100644
> > > --- a/include/uapi/linux/serial.h
> > > +++ b/include/uapi/linux/serial.h
> > > @@ -132,4 +132,21 @@ struct serial_rs485 {
> > > are a royal PITA .. */
> > > };
> > >
> > > +/*
> > > + * Serial interface for controlling ISO7816 settings on chips with suitable
> > > + * support. Set with TIOCSISO7816 and get with TIOCGISO7816 if supported by
> > > + * your platform.
> > > + */
> > > +struct serial_iso7816 {
> > > + __u32 flags; /* ISO7816 feature flags */
> > > +#define SER_ISO7816_ENABLED (1 << 0)
> > > +#define SER_ISO7816_T_PARAM (0x0f << 4)
> > > +#define SER_ISO7816_T(t) (((t) & 0x0f) << 4)
> > > + __u32 tg;
> > > + __u32 sc_fi;
> > > + __u32 sc_di;
> > > + __u32 clk;
> > > + __u32 reserved[5];
> >
> > You need to verify that reserved[] is all set to 0, otherwise people
> > will put crud in there and you can never use it for anything in the
> > future.
>
> Should I just verify or force it to 0?
Verify please, and then abort if they are not set to 0. Otherwise if
userspace puts odd stuff in there, and then later you decide to use it
as new fields, that old code will start to do odd things.
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
