All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alan Cox <alan@lxorguk.ukuu.org.uk>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: Preston Fick <pffick@gmail.com>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-serial@vger.kernel.org, preston.fick@silabs.com
Subject: Re: [PATCH 3/3] usb: cp210x: Add ioctl for GPIO support
Date: Wed, 2 May 2012 23:10:27 +0100	[thread overview]
Message-ID: <20120502231027.1c45105d@pyramind.ukuu.org.uk> (raw)
In-Reply-To: <20120502215226.GD27281@kroah.com>

On Wed, 2 May 2012 14:52:26 -0700
Greg KH <gregkh@linuxfoundation.org> wrote:

> On Wed, May 02, 2012 at 09:49:01PM +0100, Alan Cox wrote:
> > > Actually, why can't you use the GPIO subsystem for something like this?
> > > Can't you export your device as both a usb-serial device and a gpio
> > > device and have things work properly that way?
> > 
> > You still need the ioctls even then in order to discover the gpio
> > numbers
> 
> What discovery?  The device knows what gpio values it has in it, and
> should be able to register those with the gpio subsystem.

You open /dev/ttyUSB0

Ok now in your user application how are you going to find which gpio
numbers to use that are associated with this specific port, and how is
udev going to do that to manage permissions ?

So you need an ioctl to give you the range that is mapped to this (or a
sysfs node, but the sysfs node makes the security problem pretty
much insoluble)

> > and having done that youi've got potential races with unload
> > when you try and open them. You've also got permissions considerations
> > and synchronization between gpio and data problems.
> 
> That can be handled in the driver itself, if it really is a problem (the
> existing patch sure didn't handle any of that at all, so I'm guessing
> either it wasn't considered, or it isn't a problem.)

Not reliably

	open /dev/ttyUSB0 [or sysfs node]
	read gpio numbers
	close 

	open gpio foo

Oh dear... so random shell scripting user is going to screw up horribly.

> > It's not a good way to go. It might make sense in some platforms to
> > expose them as both but its not a good general model.
> 
> Why isn't the gpio subsystem a good general model?  I thought that is
> what it was created to solve?

gpio provides a flat abstraction for arbitary pins on a platform. It's
really oriented to fairly fixed system stuff. It doesn't provide a useful
abstraction for extra carrier lines.

Imagine if RTS and DTR were driven by GPIO pins instead of the tty
layer. Many devices use them as extra magic GPIO lines not as tty control
lines, so its an equivalent argument.

> > I'm currently favouring adding some 'additional control line' bits to
> > termiox.
> 
> Yes, but that's only good for usb-serial devices that also have gpio
> pins on the controller side.  Which seems pretty limited to me.
> 
> If I have a userspace program, and I want to use GPIO, I shouldn't have
> to care/know that the pins are really on the end of a USB->serial
> bridge, or somewhere hanging off of a SOC, the same userspace api should
> "just work", right?

For certain applications that makes more sense, which is why I said you
may well want to expose it as both. However you still need both, and
ideally we need a standardised pattern of assignments for line
disciplines that use the extra control lines (which has come up recently
for another device with extra lines)

I think it basically boils down to this

If you have a serial port with some gpio lines that drive arbitary
unrelated electronics then the gpio interface is handy because you can
use the same code logic as if it was wired to other pins elsewhere

If you are using them as part of the tty interface as extra control lines
(eg for smartcard protocols) then you want them driven via the tty
interface and doubly so once we add some of the smartcard/sim ldisc
support.

Hence we really need to expose them both ways because end users are doing
both things with the gpio pins on these ports.

So I'd suggest we expose them via termiox bits and also via the tty
providing gpio range info in a standardised way.

Alan

  reply	other threads:[~2012-05-02 22:07 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-01  4:06 [PATCH 1/3] usb: cp210x: Corrected USB request type definitions Preston Fick
2012-05-01  4:06 ` [PATCH 2/3] usb: cp210x: Added in support to get and store part number Preston Fick
2012-05-01 16:16   ` Sergei Shtylyov
2012-05-02 20:04     ` Greg KH
2012-05-02 20:04       ` Greg KH
2012-05-01  4:06 ` [PATCH 3/3] usb: cp210x: Add ioctl for GPIO support Preston Fick
2012-05-02 20:03   ` Greg KH
2012-05-02 20:49     ` Alan Cox
2012-05-02 21:52       ` Greg KH
2012-05-02 22:10         ` Alan Cox [this message]
2012-05-02 22:27           ` Greg KH
2012-05-02 22:59             ` Alan Cox
2012-05-03  8:58 ` [PATCH 1/3] usb: cp210x: Corrected USB request type definitions Bjørn Mork
2012-05-08 13:56   ` Preston Fick
2012-05-08 13:56     ` Preston Fick

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120502231027.1c45105d@pyramind.ukuu.org.uk \
    --to=alan@lxorguk.ukuu.org.uk \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=pffick@gmail.com \
    --cc=preston.fick@silabs.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.