Ok this is my suggestion based on GregKH comments and a couple of others
plus some other driver and ldisc stuff that is pending
- register the gpio lines with the gpio layer dynamically
- put them in the right place in the device tree (I'll let Greg advise on
the best way to do that bit), plus make them visible via the ioctls for
convenience and as they will be needed anyway in kernel
That provides the user space API
After that I'll add the hooks to the core tty layer code which allow an
ldisc to adjust the gpio lines.
For that we'll need
struct tty_gpio {
u32 base;
u16 num;
u16 reserved;
#define NR_TTY_GPIOMAP 8
u16 map[NR_TTY_GPIOMAP];
u32 reserved2[4];
};
and
tty->gpiomap
which will be NULL for most users.
Plus
struct tty_gpio d;
ioctl(tty, TIOCGPIO, &d)
and
ioctl(tty, TIOCSGPIO, &d)
where the only bits that can be updated will be the map.
So the normal use case from user space would be
struct tty_gpio d;
int fd = open("/dev/ttyUSB0", O_RDWR);
ioctl(tty, TIOCSGPIO, &d);
stuff using the gpio driver interfaces
close(fd);
And setting up for a kernel ldisc something like
/* Set a GPIO to LDISC signal mapping for ISO7816 */
ioctl(tty, TIOCGPIO, &d);
d.map[TTY_GPIO_ISO7816_RESET] = d.base;
d.map]TTY_GPIO_ISO7816_VCC] = d.base + 1;
ioctl(tty, TIOCSGPIO, &d);
/* Switch to the ldisc */
ld = N_ISO7816;
ioctl(tty, TCSETD, &ld);
and we can then abstract all the wiring details away to keep the ldisc
portable.
Thoughts ?
Alan
Hi Alan -
Thanks for this detailed explanation - I'll continue to look deeper into this, a lot of this is new to me. If Greg is in agreement here, then I will work on this as a solution to our GPIO support and submit a series of patches for this when it's ready and tested.
Kind Regards -
Preston
-----Original Message-----
From: Alan Cox [mailto:[email protected]]
Sent: Thursday, May 03, 2012 3:35 PM
To: Preston Fick
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Preston Fick; [email protected]
Subject: Re: [PATCH] usb: cp210x: Added support for GPIO (CP2103/4/5)
Ok this is my suggestion based on GregKH comments and a couple of others
plus some other driver and ldisc stuff that is pending
- register the gpio lines with the gpio layer dynamically
- put them in the right place in the device tree (I'll let Greg advise on
the best way to do that bit), plus make them visible via the ioctls for
convenience and as they will be needed anyway in kernel
That provides the user space API
After that I'll add the hooks to the core tty layer code which allow an
ldisc to adjust the gpio lines.
For that we'll need
struct tty_gpio {
u32 base;
u16 num;
u16 reserved;
#define NR_TTY_GPIOMAP 8
u16 map[NR_TTY_GPIOMAP];
u32 reserved2[4];
};
and
tty->gpiomap
which will be NULL for most users.
Plus
struct tty_gpio d;
ioctl(tty, TIOCGPIO, &d)
and
ioctl(tty, TIOCSGPIO, &d)
where the only bits that can be updated will be the map.
So the normal use case from user space would be
struct tty_gpio d;
int fd = open("/dev/ttyUSB0", O_RDWR);
ioctl(tty, TIOCSGPIO, &d);
stuff using the gpio driver interfaces
close(fd);
And setting up for a kernel ldisc something like
/* Set a GPIO to LDISC signal mapping for ISO7816 */
ioctl(tty, TIOCGPIO, &d);
d.map[TTY_GPIO_ISO7816_RESET] = d.base;
d.map]TTY_GPIO_ISO7816_VCC] = d.base + 1;
ioctl(tty, TIOCSGPIO, &d);
/* Switch to the ldisc */
ld = N_ISO7816;
ioctl(tty, TCSETD, &ld);
and we can then abstract all the wiring details away to keep the ldisc
portable.
Thoughts ?
Alan
This message (including any attachments) is intended only for the use of the individual or entity to which it is addressed and may contain information that is non-public, proprietary, privileged, confidential, and exempt from disclosure under applicable law or may constitute as attorney work product. If you are not the intended recipient, you are hereby notified that any use, dissemination, distribution, or copying of this communication is strictly prohibited. If you have received this communication in error, notify us immediately by telephone and (i) destroy this message if a facsimile or (ii) delete this message immediately if this is an electronic communication.
Thank you.
On Thu, May 03, 2012 at 09:34:56PM +0100, Alan Cox wrote:
>
> Ok this is my suggestion based on GregKH comments and a couple of others
> plus some other driver and ldisc stuff that is pending
>
> - register the gpio lines with the gpio layer dynamically
> - put them in the right place in the device tree (I'll let Greg advise on
> the best way to do that bit), plus make them visible via the ioctls for
> convenience and as they will be needed anyway in kernel
Hang them off of the USB-serial device?
> That provides the user space API
>
> After that I'll add the hooks to the core tty layer code which allow an
> ldisc to adjust the gpio lines.
>
> For that we'll need
>
> struct tty_gpio {
> u32 base;
> u16 num;
> u16 reserved;
> #define NR_TTY_GPIOMAP 8
> u16 map[NR_TTY_GPIOMAP];
> u32 reserved2[4];
> };
>
> and
>
> tty->gpiomap
>
> which will be NULL for most users.
>
>
> Plus
>
> struct tty_gpio d;
> ioctl(tty, TIOCGPIO, &d)
>
> and
>
> ioctl(tty, TIOCSGPIO, &d)
>
> where the only bits that can be updated will be the map.
>
>
>
> So the normal use case from user space would be
>
> struct tty_gpio d;
> int fd = open("/dev/ttyUSB0", O_RDWR);
> ioctl(tty, TIOCSGPIO, &d);
>
> stuff using the gpio driver interfaces
>
> close(fd);
>
>
> And setting up for a kernel ldisc something like
>
>
> /* Set a GPIO to LDISC signal mapping for ISO7816 */
> ioctl(tty, TIOCGPIO, &d);
> d.map[TTY_GPIO_ISO7816_RESET] = d.base;
> d.map]TTY_GPIO_ISO7816_VCC] = d.base + 1;
> ioctl(tty, TIOCSGPIO, &d);
>
> /* Switch to the ldisc */
> ld = N_ISO7816;
> ioctl(tty, TCSETD, &ld);
>
>
> and we can then abstract all the wiring details away to keep the ldisc
> portable.
>
>
> Thoughts ?
Will the new ldisc mess with the tty stuff to prevent "normal" serial
data and line settings from being handled properly? If not, that all
looks very good to me, thanks for working this out.
greg k-h
> Will the new ldisc mess with the tty stuff to prevent "normal" serial
> data and line settings from being handled properly? If not, that all
> looks very good to me, thanks for working this out.
It'll have no effect on that. What it will do is provide a way for things
like ISO7816 ldiscs to use the extra pins (as they can call from the
ldisc into the gpio layer) and to know the mapping of the extra control
wires, which varies by device.
Alan
On Sat, May 05, 2012 at 12:01:37PM +0100, Alan Cox wrote:
> > Will the new ldisc mess with the tty stuff to prevent "normal" serial
> > data and line settings from being handled properly? If not, that all
> > looks very good to me, thanks for working this out.
>
> It'll have no effect on that. What it will do is provide a way for things
> like ISO7816 ldiscs to use the extra pins (as they can call from the
> ldisc into the gpio layer) and to know the mapping of the extra control
> wires, which varies by device.
Ok, that sounds good to me.
So the patch looks like this, which seems nice and compact (UNTESTED)
commit 4164f9b7074e682fe71dad3b57e78521ef9df492
Author: Alan Cox <[email protected]>
Date: Wed May 16 15:13:02 2012 +0100
tty: Add a gpio helper set
Various tty devices have additional control lines which are sometimes used
as GPIO pins and sometimes also tied with the serial port to implement
protocols such as ISO7816.
This code provides a kernel interface for querying the GPIO range of a tty,
and to describe the mapping between GPIO pins and control lines. The latter
will be needed for some upcoming line discipline support.
[Proposal do not merge yet]
Not-Signed-off-by: Alan Cox <[email protected]>
diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c
index a1b9a2f..60550e7 100644
--- a/drivers/tty/tty_ioctl.c
+++ b/drivers/tty/tty_ioctl.c
@@ -1071,6 +1071,39 @@ int tty_mode_ioctl(struct tty_struct *tty, struct file *file,
case TCSETXF:
return set_termiox(real_tty, p, TERMIOS_FLUSH);
#endif
+#ifdef TCGGPIO
+ case TCGGPIO: {
+ struct tcgpio gpio;
+
+ if (tty->gpio == NULL)
+ return -EOPNOTSUPP;
+ mutex_lock(&real_tty->termios_mutex);
+ memset(&gpio, 0, sizeof(gpio));
+ gpio.base = tty->gpio->base;
+ gpio.num = tty->gpio->num;
+ memcpy(gpio.map, tty->gpio->map, sizeof(gpio.map));
+ mutex_unlock(&real_tty->termios_mutex);
+ if (copy_to_user(p, &gpio, sizeof(gpio)))
+ return -EFAULT;
+ return 0;
+ }
+ case TCSGPIO:
+ {
+ struct tcgpio gpio;
+
+ if (tty->gpio == NULL)
+ return -EOPNOTSUPP;
+ if (copy_from_user(&gpio, p, sizeof(gpio)))
+ return -EFAULT;
+ mutex_lock(&real_tty->termios_mutex);
+ memcpy(tty->gpio->map, gpio.map, sizeof(tty->gpio->map));
+ /* An ldisc can see this by watching the ioctl go through
+ but we may want to add a hook */
+ mutex_unlock(&real_tty->termios_mutex);
+ return 0;
+ }
+
+#endif
case TIOCGSOFTCAR:
copy_termios(real_tty, &kterm);
ret = put_user((kterm.c_cflag & CLOCAL) ? 1 : 0,
diff --git a/include/asm-generic/ioctls.h b/include/asm-generic/ioctls.h
index 199975f..fee17d3 100644
--- a/include/asm-generic/ioctls.h
+++ b/include/asm-generic/ioctls.h
@@ -74,6 +74,8 @@
#define TCSETXW 0x5435
#define TIOCSIG _IOW('T', 0x36, int) /* pty: generate signal */
#define TIOCVHANGUP 0x5437
+#define TCGGPIO _IOR('T', 0x38, struct tcgpio)
+#define TCSGPIO _IOW('T', 0x39, struct tcgpio)
#define FIONCLEX 0x5450
#define FIOCLEX 0x5451
diff --git a/include/asm-generic/termios.h b/include/asm-generic/termios.h
index d0922ad..3adda38 100644
--- a/include/asm-generic/termios.h
+++ b/include/asm-generic/termios.h
@@ -18,6 +18,18 @@ struct winsize {
unsigned short ws_ypixel;
};
+
+/* GPIO handling */
+#define NR_TTY_GPIOMAP 8
+struct tcgpio /* User copied version */
+{
+ u32 base;
+ u16 num;
+ u16 reserved;
+ u32 map[NR_TTY_GPIOMAP];
+ u32 reserved2[6];
+};
+
#define NCC 8
struct termio {
unsigned short c_iflag; /* input mode flags */
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 9f47ab5..19daf03 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -243,6 +243,18 @@ struct tty_port {
};
/*
+ * GPIO block, optional
+ */
+
+struct tty_kgpio /* Kernel gpio struct */
+{
+ u32 base;
+ u16 num;
+ u16 reserved;
+ u32 map[NR_TTY_GPIOMAP];
+};
+
+/*
* Where all of the state associated with a tty is kept while the tty
* is open. Since the termios state should be kept even if the tty
* has been closed --- for things like the baud rate, etc --- it is
@@ -331,6 +343,8 @@ struct tty_struct {
/* If the tty has a pending do_SAK, queue it here - akpm */
struct work_struct SAK_work;
struct tty_port *port;
+
+ struct tty_kgpio *gpio; /* Optional */
};
/* Each of a tty's open files has private_data pointing to tty_file_private */
On Wed, May 16, 2012 at 04:33:47PM +0100, Alan Cox wrote:
> So the patch looks like this, which seems nice and compact (UNTESTED)
>
> commit 4164f9b7074e682fe71dad3b57e78521ef9df492
> Author: Alan Cox <[email protected]>
> Date: Wed May 16 15:13:02 2012 +0100
>
> tty: Add a gpio helper set
>
> Various tty devices have additional control lines which are sometimes used
> as GPIO pins and sometimes also tied with the serial port to implement
> protocols such as ISO7816.
>
> This code provides a kernel interface for querying the GPIO range of a tty,
> and to describe the mapping between GPIO pins and control lines. The latter
> will be needed for some upcoming line discipline support.
>
> [Proposal do not merge yet]
>
> Not-Signed-off-by: Alan Cox <[email protected]>
Wow, that looks really nice and tiny, if that's all that is needed in
the core, that's great.
> diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c
> index a1b9a2f..60550e7 100644
> --- a/drivers/tty/tty_ioctl.c
> +++ b/drivers/tty/tty_ioctl.c
> @@ -1071,6 +1071,39 @@ int tty_mode_ioctl(struct tty_struct *tty, struct file *file,
> case TCSETXF:
> return set_termiox(real_tty, p, TERMIOS_FLUSH);
> #endif
> +#ifdef TCGGPIO
> + case TCGGPIO: {
> + struct tcgpio gpio;
> +
> + if (tty->gpio == NULL)
> + return -EOPNOTSUPP;
> + mutex_lock(&real_tty->termios_mutex);
> + memset(&gpio, 0, sizeof(gpio));
> + gpio.base = tty->gpio->base;
> + gpio.num = tty->gpio->num;
> + memcpy(gpio.map, tty->gpio->map, sizeof(gpio.map));
> + mutex_unlock(&real_tty->termios_mutex);
> + if (copy_to_user(p, &gpio, sizeof(gpio)))
> + return -EFAULT;
> + return 0;
> + }
> + case TCSGPIO:
> + {
> + struct tcgpio gpio;
> +
> + if (tty->gpio == NULL)
> + return -EOPNOTSUPP;
> + if (copy_from_user(&gpio, p, sizeof(gpio)))
> + return -EFAULT;
> + mutex_lock(&real_tty->termios_mutex);
> + memcpy(tty->gpio->map, gpio.map, sizeof(tty->gpio->map));
> + /* An ldisc can see this by watching the ioctl go through
> + but we may want to add a hook */
> + mutex_unlock(&real_tty->termios_mutex);
> + return 0;
So how would the lower tty driver get the ioctl to know to now set these
values to the hardware? I think we at least need a hook for that,
right? Or would that go through the ldisc?
> + }
> +
> +#endif
> case TIOCGSOFTCAR:
> copy_termios(real_tty, &kterm);
> ret = put_user((kterm.c_cflag & CLOCAL) ? 1 : 0,
> diff --git a/include/asm-generic/ioctls.h b/include/asm-generic/ioctls.h
> index 199975f..fee17d3 100644
> --- a/include/asm-generic/ioctls.h
> +++ b/include/asm-generic/ioctls.h
> @@ -74,6 +74,8 @@
> #define TCSETXW 0x5435
> #define TIOCSIG _IOW('T', 0x36, int) /* pty: generate signal */
> #define TIOCVHANGUP 0x5437
> +#define TCGGPIO _IOR('T', 0x38, struct tcgpio)
> +#define TCSGPIO _IOW('T', 0x39, struct tcgpio)
>
> #define FIONCLEX 0x5450
> #define FIOCLEX 0x5451
> diff --git a/include/asm-generic/termios.h b/include/asm-generic/termios.h
> index d0922ad..3adda38 100644
> --- a/include/asm-generic/termios.h
> +++ b/include/asm-generic/termios.h
> @@ -18,6 +18,18 @@ struct winsize {
> unsigned short ws_ypixel;
> };
>
> +
> +/* GPIO handling */
> +#define NR_TTY_GPIOMAP 8
> +struct tcgpio /* User copied version */
> +{
> + u32 base;
> + u16 num;
> + u16 reserved;
> + u32 map[NR_TTY_GPIOMAP];
> + u32 reserved2[6];
> +};
__u32 and friends instead?
greg k-h