Re: [PATCH 1/1] mct_u232: IOCTL implementation

Re: [PATCH 1/1] mct_u232: IOCTL implementation

[Tsozik]

Pete,

My apology, reading Greg's post I realized that 2.6.35 (distributed with Fedora 14) is indeed outdated. I just checked out 2.6.37-rc7 tree and saw references to get_icount function. I will definetely address your concern,

Thank you again,
 Vadim.

--- On Sun, 12/26/10, Tsozik <tsozik@yahoo.com> wrote:

> From: Tsozik <tsozik@yahoo.com>
> Subject: Re: [PATCH 1/1] mct_u232: IOCTL implementation
> To: "Pete Zaitcev" <zaitcev@redhat.com>
> Cc: "Greg Kroah-Hartman" <gregkh@suse.de>, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
> Date: Sunday, December 26, 2010, 2:41 PM
> Pete,
> 
> Many thanks for your comment/concern. I borrowed an
> TIOCGICOUNT implementation from usb/serial/io_ti.c:
> 
>         case TIOCGICOUNT:
>                
> dbg("%s - (%d) TIOCGICOUNT RX=%d, TX=%d", __func__,
>                
>      port->number,
> edge_port->icount.rx, edge_port->icount.tx);
>                 if
> (copy_to_user((void __user *)arg,
> &edge_port->icount,
>                
>                
> sizeof(edge_port->icount)))
>                
>         return -EFAULT;
>                
> return 0;
>         }
> 
> There are 2 similar TIOCGICOUNT implementations listed in
> 
> ark3116.c
> io_edgeport.c
> io_ti.c
> mos7720.c
> mos7840.c
> ti_usb_3410_5052.c
> 
> files under usb/serial/ directory. One based on io_ti.c and
> another based on io_edgeport.c. I borrowed one from io_ti.c,
> since it looked more effecient to me. I searched for any
> mention of get_icount function under linux-2.6.35 and didn't
> find any file which declared or called this function:
> 
> [vtsozik@SR71 linux-2.6.35]$ find . -type f -name '*.[c,h]'
> | xargs grep get_icount
> [vtsozik@SR71 linux-2.6.35]$
> 
> I'm wondering if you could give me a bit more information
> on this. Otherwise I would really prefer to proceed with
> something that already exists and tested. If by some reason
> you believe that alternative implementation from
> io_edgeport.c (please see code snippet below) should be used
> please let me know. Again I didn't see any reason for extra
> copy.
> 
>         case TIOCGICOUNT:
>                
> cnow = edge_port->icount;
>                
> memset(&icount, 0, sizeof(icount));
>                
> icount.cts = cnow.cts;
>                
> icount.dsr = cnow.dsr;
>                
> icount.rng = cnow.rng;
>                
> icount.dcd = cnow.dcd;
>                
> icount.rx = cnow.rx;
>                
> icount.tx = cnow.tx;
>                
> icount.frame = cnow.frame;
>                
> icount.overrun = cnow.overrun;
>                
> icount.parity = cnow.parity;
>                
> icount.brk = cnow.brk;
>                
> icount.buf_overrun = cnow.buf_overrun;
> 
>                
> dbg("%s (%d) TIOCGICOUNT RX=%d, TX=%d",
>                
>                
> __func__,  port->number, icount.rx, icount.tx);
>                 if
> (copy_to_user((void __user *)arg, &icount,
> sizeof(icount)))
>                
>         return -EFAULT;
>                
> return 0;
>         }
> 
> Thank you in advance,
> Vadim.
> 
> --- On Sun, 12/26/10, Pete Zaitcev <zaitcev@redhat.com>
> wrote:
> 
> > From: Pete Zaitcev <zaitcev@redhat.com>
> > Subject: Re: [PATCH 1/1] mct_u232: IOCTL
> implementation
> > To: "Tsozik" <tsozik@yahoo.com>
> > Cc: "Greg Kroah-Hartman" <gregkh@suse.de>,
> linux-usb@vger.kernel.org,
> linux-kernel@vger.kernel.org
> > Date: Sunday, December 26, 2010, 12:49 PM
> > On Sat, 25 Dec 2010 21:39:39 -0800
> > (PST)
> > Tsozik <tsozik@yahoo.com>
> > wrote:
> > 
> > > +++ mct_u232.c  2010-12-25 21:44:57.714640343
> > -0500
> > > +static int  mct_u232_ioctl(struct tty_struct
> > *tty, struct file *file,
> > > +             
> >           unsigned int cmd,
> > unsigned long arg)
> > > +{
> > > +       case TIOCGICOUNT:
> > > +             
> >   dbg("%s - (%d) TIOCGICOUNT RX=%d, TX=%d",
> __func__,
> > > +             
> >       port->number,
> > mct_u232_port->icount.rx,
> mct_u232_port->icount.tx);
> > > +             
> >   if (copy_to_user((void __user *)arg,
> > &mct_u232_port->icount,
> > > +             
> >       sizeof(mct_u232_port->icount)))
> > > +             
> >           return -EFAULT;
> > 
> > This looks suspicious. Didn't we relocate the
> machinery for
> > TIOCGICOUNT
> > into a generic place? Please examine how
> ->get_icount
> > works before
> > hand-rolling the ioctl.
> > 
> > -- Pete
> > 
> 
> 
> 
> 


      

Re: [PATCH 1/1] mct_u232: IOCTL implementation

[Pete Zaitcev]

On Sun, 26 Dec 2010 14:14:44 -0800 (PST)
Tsozik <tsozik@yahoo.com> wrote:

> My apology, reading Greg's post I realized that 2.6.35 (distributed with
> Fedora 14) is indeed outdated. I just checked out 2.6.37-rc7 tree and saw
> references to get_icount function. I will definetely address your concern,

Thanks. I figured there was a reason for us to do this (if nothing else,
to get rid of casts with __user). So, you might as well align the code
with the current practice.

Please try to be more careful with spaces and tabs. I didn't want to
focus on such small things right from beginning, but they are considered
important.

Yours,
-- Pete

Re: [PATCH 1/1] mct_u232: IOCTL implementation

[Tsozik]

Jesper,

Many thanks for the info. After reading your post I realized that Greg meant that change log comments will be stored in git repository comment section like in any other source control system (e.g. cvs, rcs, ...). So no need to duplicate them in actual file. I will only provide comments in description section of canonical patch format. I'm also wondering if it's OK to provide "git diffs" instead of diffs as a part of patch email? 

Thank you in advance,
 Vadim.

--- On Sun, 12/26/10, Jesper Juhl <jj@chaosbits.net> wrote:

> From: Jesper Juhl <jj@chaosbits.net>
> Subject: Re: [PATCH 1/1] mct_u232: IOCTL implementation
> To: "Tsozik" <tsozik@yahoo.com>
> Cc: "Randy Dunlap" <rdunlap@xenotime.net>, "Greg KH" <gregkh@suse.de>, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
> Date: Sunday, December 26, 2010, 5:03 PM
> On Sun, 26 Dec 2010, Tsozik wrote:
> 
> > Randy,
> > 
> > Greg referred to git as to place where change log
> comments are now 
> > checked in. I googled it and found that one can also
> check out latest 
> > kernel source code as opposed to tarball download.
> However, You're 
> > right, looking at 
> > http://kernelnewbies.org/UpstreamMerge/SubmittingPatches
> documentation I 
> > don't see how one can use git to check in comments or
> code directly to 
> > git repository,
> > 
> 
> Only Linus can check code into the final official tree.
> But, you can use 
> git to get a copy of the latest Linus tree (and many other
> trees run by 
> various maintainers and others). Git is also useful for
> maintaining your 
> own local branch with your changes.
> 
> The way it usually works is that you create a patch and
> send it off to 
> some sub-system maintainer via email patch ('git diff'
> generated usually) 
> or via email pull request for your personal repository. The
> sub-system 
> maintainer then merges your patch with his tree and, at a
> later date, 
> submits his tree (including your patch) to Linus.
> 
> Changelog comments are tracked with git. Once your patch is
> accepted it 
> will eventually make its way to Linus' tree and your
> changelog comments in 
> the patch mail (or git commit message - if you send a pull
> request) will 
> eventually turn into a git commit message in the official
> tree. So there's 
> no need to add changelog info in the files themselves.
> Check http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=shortlog
> 
> (or git log after cloning the tree) to see commit
> messages.
> 
> 
> -- 
> Jesper Juhl <jj@chaosbits.net> 
>           http://www.chaosbits.net/
> Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
> Plain text mails only, please.
> 
> 


      

Re: [PATCH 1/1] mct_u232: IOCTL implementation

[Jesper Juhl]

On Sun, 26 Dec 2010, Tsozik wrote:

> Randy,
> 
> Greg referred to git as to place where change log comments are now 
> checked in. I googled it and found that one can also check out latest 
> kernel source code as opposed to tarball download. However, You're 
> right, looking at 
> http://kernelnewbies.org/UpstreamMerge/SubmittingPatches documentation I 
> don't see how one can use git to check in comments or code directly to 
> git repository,
> 

Only Linus can check code into the final official tree. But, you can use 
git to get a copy of the latest Linus tree (and many other trees run by 
various maintainers and others). Git is also useful for maintaining your 
own local branch with your changes.

The way it usually works is that you create a patch and send it off to 
some sub-system maintainer via email patch ('git diff' generated usually) 
or via email pull request for your personal repository. The sub-system 
maintainer then merges your patch with his tree and, at a later date, 
submits his tree (including your patch) to Linus.

Changelog comments are tracked with git. Once your patch is accepted it 
will eventually make its way to Linus' tree and your changelog comments in 
the patch mail (or git commit message - if you send a pull request) will 
eventually turn into a git commit message in the official tree. So there's 
no need to add changelog info in the files themselves.
Check http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=shortlog 
(or git log after cloning the tree) to see commit messages.


-- 
Jesper Juhl <jj@chaosbits.net>            http://www.chaosbits.net/
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please.

Re: [PATCH 1/1] mct_u232: IOCTL implementation

[Tsozik]

Randy,

Greg referred to git as to place where change log comments are now checked in. I googled it and found that one can also check out latest kernel source code as opposed to tarball download. However, You're right, looking at http://kernelnewbies.org/UpstreamMerge/SubmittingPatches documentation I don't see how one can use git to check in comments or code directly to git repository,

Thank you,
Vadim.


--- On Sun, 12/26/10, Randy Dunlap <rdunlap@xenotime.net> wrote:

> From: Randy Dunlap <rdunlap@xenotime.net>
> Subject: Re: [PATCH 1/1] mct_u232: IOCTL implementation
> To: "Tsozik" <tsozik@yahoo.com>
> Cc: "Greg KH" <gregkh@suse.de>, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
> Date: Sunday, December 26, 2010, 4:30 PM
> On Sun, 26 Dec 2010 11:51:28 -0800
> (PST) Tsozik wrote:
> 
> > Greg,
> > 
> > Many thanks for your reply. I will try to install git
> and download the latest release candidate. I see that
> 2.6.37.rc7 is already available. I think I will also hope to
> find get_icount there to address Pete's concern,
> 
> 
> You can use git if you want to, but it certainly is not
> required for this.
> 
> ---
> ~Randy
> *** Remember to use Documentation/SubmitChecklist when
> testing your code ***
> desserts:  http://www.xenotime.net/linux/recipes/
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


      

Re: [PATCH 1/1] mct_u232: IOCTL implementation

[Randy Dunlap]

On Sun, 26 Dec 2010 11:51:28 -0800 (PST) Tsozik wrote:

> Greg,
> 
> Many thanks for your reply. I will try to install git and download the latest release candidate. I see that 2.6.37.rc7 is already available. I think I will also hope to find get_icount there to address Pete's concern,


You can use git if you want to, but it certainly is not required for this.

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
desserts:  http://www.xenotime.net/linux/recipes/

Re: [PATCH 1/1] mct_u232: IOCTL implementation

[Tsozik]

Greg,

Many thanks for your reply. I will try to install git and download the latest release candidate. I see that 2.6.37.rc7 is already available. I think I will also hope to find get_icount there to address Pete's concern,

Thank you again,
 Vadim.

--- On Sun, 12/26/10, Greg KH <gregkh@suse.de> wrote:

> From: Greg KH <gregkh@suse.de>
> Subject: Re: [PATCH 1/1] mct_u232: IOCTL implementation
> To: "Tsozik" <tsozik@yahoo.com>
> Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
> Date: Sunday, December 26, 2010, 1:41 PM
> On Sat, Dec 25, 2010 at 09:39:39PM
> -0800, Tsozik wrote:
> > From: Vadim Tsozik <tsozik@yahoo.com>
> > 
> > Added mct_u232_ioctl function and implemented
> TIOCMIWAIT and TIOCGICOUNT commands. MCT u232 p9 is one of a
> few usb to serail adapters which converts USB +/-5v voltage
> levels to COM +/-15 voltages. So it can also power COM
> interfaced devices. This makes it very usable for legacy COM
> interfaced data-acquisition hardware. I tested new
> implementation with AWARE Electronics RM-60 radiation meter,
> which sends pulse via RNG COM line whenever new particle is
> registered.
> > 
> > Patch below is based on
> linux-2.6.35.10-72.fc14.x86_64.
> 
> Care to make sure this applies on a kernel that we can
> actually apply
> patches to (i.e 2.6.37-rc6?)  The .35 kernel was a
> long time ago.
> 
> > 
> > Signed-off-by: Vadim Tsozik <tsozik@yahoo.com>
> > 
> > ---
> > --- original/mct_u232.c 2010-12-25 21:31:13.744174626
> -0500
> > +++ mct_u232.c  2010-12-25 21:44:57.714640343
> -0500
> 
> Care to read over Documentation/SubmittingPatches which
> shows you the
> proper patch level to make a patch against?
> 
> > @@ -24,6 +24,12 @@
> >   *   Basic tests have
> been performed with minicom/zmodem transfers and
> >   *   modem dialing
> under Linux 2.4.0-test10 (for me it works fine).
> >   *
> > + * 24-Apr-2010 Vadim Tsozik <tsozik@yahoo.com>
> > + *   - Added implementation of
> 'TIOCMIWAIT' and 'TIOCGICOUNT' ioctls.
> > + *     This routines are
> necessary if you use mct u232 p9 as data
> > + *     acquisition interface.
> These routines were tested with RM-60 AWARE
> > + *     Electronics Radiation
> Monitor.
> > + *
> 
> No need to ever add an entry here, your changelog
> information is now
> stored in git,  not in the files themselves.
> 
> >   * 04-Nov-2003 Bill Marr <marr at
> flex dot com>
> >   *   - Mimic Windows
> driver by sending 2 USB 'device request' messages
> >   *     following
> normal 'baud rate change' message.  This allows data to
> be
> > @@ -78,6 +84,8 @@
> >  #include <asm/unaligned.h>
> >  #include <linux/usb.h>
> >  #include <linux/usb/serial.h>
> > +#include <linux/serial.h>
> > +#include <linux/ioctl.h>
> >  #include "mct_u232.h"
> > 
> >  /*
> > @@ -104,6 +112,8 @@ static void
> mct_u232_break_ctl(struct tt
> >  static int  mct_u232_tiocmget(struct
> tty_struct *tty, struct file *file);
> >  static int  mct_u232_tiocmset(struct
> tty_struct *tty, struct file *file,
> >               
>          unsigned int set,
> unsigned int clear);
> > +static int  mct_u232_ioctl(struct tty_struct
> *tty, struct file *file,
> > +             
>           unsigned int cmd,
> unsigned long arg);
> >  static void mct_u232_throttle(struct tty_struct
> *tty);
> >  static void mct_u232_unthrottle(struct
> tty_struct *tty);
> > 
> > @@ -150,6 +160,7 @@ static struct usb_serial_driver
> mct_u232
> >         .tiocmset
> =          mct_u232_tiocmset,
> >         .attach = 
>           mct_u232_startup,
> >         .release = 
>          mct_u232_release,
> > +       .ioctl =   
>          mct_u232_ioctl,
> 
> Your email client converted the tabs to spaces, making this
> patch
> unable to apply.  Please fix your email client and
> also run your patch
> through the scripts/checkpatch.pl script before sending it
> to fix up the
> formatting errors that are in it.
> 
> Care to try again after doing this, and addressing Pete's
> concerns?
> 
> thanks,
> 
> greg k-h
> 


      

Re: [PATCH 1/1] mct_u232: IOCTL implementation

[Tsozik]

Pete,

Many thanks for your comment/concern. I borrowed an TIOCGICOUNT implementation from usb/serial/io_ti.c:

        case TIOCGICOUNT:
                dbg("%s - (%d) TIOCGICOUNT RX=%d, TX=%d", __func__,
                     port->number, edge_port->icount.rx, edge_port->icount.tx);
                if (copy_to_user((void __user *)arg, &edge_port->icount,
                                sizeof(edge_port->icount)))
                        return -EFAULT;
                return 0;
        }

There are 2 similar TIOCGICOUNT implementations listed in

ark3116.c
io_edgeport.c
io_ti.c
mos7720.c
mos7840.c
ti_usb_3410_5052.c

files under usb/serial/ directory. One based on io_ti.c and another based on io_edgeport.c. I borrowed one from io_ti.c, since it looked more effecient to me. I searched for any mention of get_icount function under linux-2.6.35 and didn't find any file which declared or called this function:

[vtsozik@SR71 linux-2.6.35]$ find . -type f -name '*.[c,h]' | xargs grep get_icount
[vtsozik@SR71 linux-2.6.35]$

I'm wondering if you could give me a bit more information on this. Otherwise I would really prefer to proceed with something that already exists and tested. If by some reason you believe that alternative implementation from io_edgeport.c (please see code snippet below) should be used please let me know. Again I didn't see any reason for extra copy.

        case TIOCGICOUNT:
                cnow = edge_port->icount;
                memset(&icount, 0, sizeof(icount));
                icount.cts = cnow.cts;
                icount.dsr = cnow.dsr;
                icount.rng = cnow.rng;
                icount.dcd = cnow.dcd;
                icount.rx = cnow.rx;
                icount.tx = cnow.tx;
                icount.frame = cnow.frame;
                icount.overrun = cnow.overrun;
                icount.parity = cnow.parity;
                icount.brk = cnow.brk;
                icount.buf_overrun = cnow.buf_overrun;

                dbg("%s (%d) TIOCGICOUNT RX=%d, TX=%d",
                                __func__,  port->number, icount.rx, icount.tx);
                if (copy_to_user((void __user *)arg, &icount, sizeof(icount)))
                        return -EFAULT;
                return 0;
        }

Thank you in advance,
Vadim.

--- On Sun, 12/26/10, Pete Zaitcev <zaitcev@redhat.com> wrote:

> From: Pete Zaitcev <zaitcev@redhat.com>
> Subject: Re: [PATCH 1/1] mct_u232: IOCTL implementation
> To: "Tsozik" <tsozik@yahoo.com>
> Cc: "Greg Kroah-Hartman" <gregkh@suse.de>, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
> Date: Sunday, December 26, 2010, 12:49 PM
> On Sat, 25 Dec 2010 21:39:39 -0800
> (PST)
> Tsozik <tsozik@yahoo.com>
> wrote:
> 
> > +++ mct_u232.c  2010-12-25 21:44:57.714640343
> -0500
> > +static int  mct_u232_ioctl(struct tty_struct
> *tty, struct file *file,
> > +             
>           unsigned int cmd,
> unsigned long arg)
> > +{
> > +       case TIOCGICOUNT:
> > +             
>   dbg("%s - (%d) TIOCGICOUNT RX=%d, TX=%d", __func__,
> > +             
>       port->number,
> mct_u232_port->icount.rx, mct_u232_port->icount.tx);
> > +             
>   if (copy_to_user((void __user *)arg,
> &mct_u232_port->icount,
> > +             
>       sizeof(mct_u232_port->icount)))
> > +             
>           return -EFAULT;
> 
> This looks suspicious. Didn't we relocate the machinery for
> TIOCGICOUNT
> into a generic place? Please examine how ->get_icount
> works before
> hand-rolling the ioctl.
> 
> -- Pete
> 


      

Re: [PATCH 1/1] mct_u232: IOCTL implementation

[Greg KH]

On Sat, Dec 25, 2010 at 09:39:39PM -0800, Tsozik wrote:
> From: Vadim Tsozik <tsozik@yahoo.com>
> 
> Added mct_u232_ioctl function and implemented TIOCMIWAIT and TIOCGICOUNT commands. MCT u232 p9 is one of a few usb to serail adapters which converts USB +/-5v voltage levels to COM +/-15 voltages. So it can also power COM interfaced devices. This makes it very usable for legacy COM interfaced data-acquisition hardware. I tested new implementation with AWARE Electronics RM-60 radiation meter, which sends pulse via RNG COM line whenever new particle is registered.
> 
> Patch below is based on linux-2.6.35.10-72.fc14.x86_64.

Care to make sure this applies on a kernel that we can actually apply
patches to (i.e 2.6.37-rc6?)  The .35 kernel was a long time ago.

> 
> Signed-off-by: Vadim Tsozik <tsozik@yahoo.com>
> 
> ---
> --- original/mct_u232.c 2010-12-25 21:31:13.744174626 -0500
> +++ mct_u232.c  2010-12-25 21:44:57.714640343 -0500

Care to read over Documentation/SubmittingPatches which shows you the
proper patch level to make a patch against?

> @@ -24,6 +24,12 @@
>   *   Basic tests have been performed with minicom/zmodem transfers and
>   *   modem dialing under Linux 2.4.0-test10 (for me it works fine).
>   *
> + * 24-Apr-2010 Vadim Tsozik <tsozik@yahoo.com>
> + *   - Added implementation of 'TIOCMIWAIT' and 'TIOCGICOUNT' ioctls.
> + *     This routines are necessary if you use mct u232 p9 as data
> + *     acquisition interface. These routines were tested with RM-60 AWARE
> + *     Electronics Radiation Monitor.
> + *

No need to ever add an entry here, your changelog information is now
stored in git,  not in the files themselves.

>   * 04-Nov-2003 Bill Marr <marr at flex dot com>
>   *   - Mimic Windows driver by sending 2 USB 'device request' messages
>   *     following normal 'baud rate change' message.  This allows data to be
> @@ -78,6 +84,8 @@
>  #include <asm/unaligned.h>
>  #include <linux/usb.h>
>  #include <linux/usb/serial.h>
> +#include <linux/serial.h>
> +#include <linux/ioctl.h>
>  #include "mct_u232.h"
> 
>  /*
> @@ -104,6 +112,8 @@ static void mct_u232_break_ctl(struct tt
>  static int  mct_u232_tiocmget(struct tty_struct *tty, struct file *file);
>  static int  mct_u232_tiocmset(struct tty_struct *tty, struct file *file,
>                         unsigned int set, unsigned int clear);
> +static int  mct_u232_ioctl(struct tty_struct *tty, struct file *file,
> +                        unsigned int cmd, unsigned long arg);
>  static void mct_u232_throttle(struct tty_struct *tty);
>  static void mct_u232_unthrottle(struct tty_struct *tty);
> 
> @@ -150,6 +160,7 @@ static struct usb_serial_driver mct_u232
>         .tiocmset =          mct_u232_tiocmset,
>         .attach =            mct_u232_startup,
>         .release =           mct_u232_release,
> +       .ioctl =             mct_u232_ioctl,

Your email client converted the tabs to spaces, making this patch
unable to apply.  Please fix your email client and also run your patch
through the scripts/checkpatch.pl script before sending it to fix up the
formatting errors that are in it.

Care to try again after doing this, and addressing Pete's concerns?

thanks,

greg k-h

Re: [PATCH 1/1] mct_u232: IOCTL implementation

[Pete Zaitcev]

On Sat, 25 Dec 2010 21:39:39 -0800 (PST)
Tsozik <tsozik@yahoo.com> wrote:

> +++ mct_u232.c  2010-12-25 21:44:57.714640343 -0500
> +static int  mct_u232_ioctl(struct tty_struct *tty, struct file *file,
> +                        unsigned int cmd, unsigned long arg)
> +{
> +       case TIOCGICOUNT:
> +                dbg("%s - (%d) TIOCGICOUNT RX=%d, TX=%d", __func__,
> +                    port->number, mct_u232_port->icount.rx, mct_u232_port->icount.tx);
> +                if (copy_to_user((void __user *)arg, &mct_u232_port->icount,
> +                    sizeof(mct_u232_port->icount)))
> +                        return -EFAULT;

This looks suspicious. Didn't we relocate the machinery for TIOCGICOUNT
into a generic place? Please examine how ->get_icount works before
hand-rolling the ioctl.

-- Pete

[PATCH 1/1] mct_u232: IOCTL implementation

[Tsozik]

From: Vadim Tsozik <tsozik@yahoo.com>

Added mct_u232_ioctl function and implemented TIOCMIWAIT and TIOCGICOUNT commands. MCT u232 p9 is one of a few usb to serail adapters which converts USB +/-5v voltage levels to COM +/-15 voltages. So it can also power COM interfaced devices. This makes it very usable for legacy COM interfaced data-acquisition hardware. I tested new implementation with AWARE Electronics RM-60 radiation meter, which sends pulse via RNG COM line whenever new particle is registered.

Patch below is based on linux-2.6.35.10-72.fc14.x86_64.

Signed-off-by: Vadim Tsozik <tsozik@yahoo.com>

---
--- original/mct_u232.c 2010-12-25 21:31:13.744174626 -0500
+++ mct_u232.c  2010-12-25 21:44:57.714640343 -0500
@@ -24,6 +24,12 @@
  *   Basic tests have been performed with minicom/zmodem transfers and
  *   modem dialing under Linux 2.4.0-test10 (for me it works fine).
  *
+ * 24-Apr-2010 Vadim Tsozik <tsozik@yahoo.com>
+ *   - Added implementation of 'TIOCMIWAIT' and 'TIOCGICOUNT' ioctls.
+ *     This routines are necessary if you use mct u232 p9 as data
+ *     acquisition interface. These routines were tested with RM-60 AWARE
+ *     Electronics Radiation Monitor.
+ *
  * 04-Nov-2003 Bill Marr <marr at flex dot com>
  *   - Mimic Windows driver by sending 2 USB 'device request' messages
  *     following normal 'baud rate change' message.  This allows data to be
@@ -78,6 +84,8 @@
 #include <asm/unaligned.h>
 #include <linux/usb.h>
 #include <linux/usb/serial.h>
+#include <linux/serial.h>
+#include <linux/ioctl.h>
 #include "mct_u232.h"

 /*
@@ -104,6 +112,8 @@ static void mct_u232_break_ctl(struct tt
 static int  mct_u232_tiocmget(struct tty_struct *tty, struct file *file);
 static int  mct_u232_tiocmset(struct tty_struct *tty, struct file *file,
                        unsigned int set, unsigned int clear);
+static int  mct_u232_ioctl(struct tty_struct *tty, struct file *file,
+                        unsigned int cmd, unsigned long arg);
 static void mct_u232_throttle(struct tty_struct *tty);
 static void mct_u232_unthrottle(struct tty_struct *tty);

@@ -150,6 +160,7 @@ static struct usb_serial_driver mct_u232
        .tiocmset =          mct_u232_tiocmset,
        .attach =            mct_u232_startup,
        .release =           mct_u232_release,
+       .ioctl =             mct_u232_ioctl,
 };


@@ -160,6 +171,8 @@ struct mct_u232_private {
        unsigned char        last_lsr;      /* Line Status Register */
        unsigned char        last_msr;      /* Modem Status Register */
        unsigned int         rx_flags;      /* Throttling flags */
+        struct async_icount  icount;
+       wait_queue_head_t    msr_wait;      /* for handling sleeping while waiting for msr change to happen */
 };

 #define THROTTLED              0x01
@@ -386,27 +399,41 @@ static int mct_u232_get_modem_stat(struc
        return rc;
 } /* mct_u232_get_modem_stat */

-static void mct_u232_msr_to_state(unsigned int *control_state,
-                                               unsigned char msr)
+static void mct_u232_msr_to_state(struct mct_u232_private *priv)
 {
-       /* Translate Control Line states */
-       if (msr & MCT_U232_MSR_DSR)
-               *control_state |=  TIOCM_DSR;
-       else
-               *control_state &= ~TIOCM_DSR;
-       if (msr & MCT_U232_MSR_CTS)
-               *control_state |=  TIOCM_CTS;
-       else
-               *control_state &= ~TIOCM_CTS;
-       if (msr & MCT_U232_MSR_RI)
-               *control_state |=  TIOCM_RI;
-       else
-               *control_state &= ~TIOCM_RI;
-       if (msr & MCT_U232_MSR_CD)
-               *control_state |=  TIOCM_CD;
-       else
-               *control_state &= ~TIOCM_CD;
-       dbg("msr_to_state: msr=0x%x ==> state=0x%x", msr, *control_state);
+        unsigned char msr = priv->last_msr;
+        unsigned int *control_state = &priv->control_state;
+        struct async_icount *icount = &priv->icount;
+
+        /* Translate Control Line states */
+        if (msr & MCT_U232_MSR_DSR) {
+         *control_state |=  TIOCM_DSR;
+         icount->dsr++;
+        } else {
+         *control_state &= ~TIOCM_DSR;
+        }
+        if (msr & MCT_U232_MSR_CTS) {
+         *control_state |=  TIOCM_CTS;
+         icount->cts++;
+        } else {
+         *control_state &= ~TIOCM_CTS;
+        }
+        if (msr & MCT_U232_MSR_RI) {
+         *control_state |=  TIOCM_RI;
+         icount->rng++;
+        } else {
+         *control_state &= ~TIOCM_RI;
+        }
+        if (msr & MCT_U232_MSR_CD) {
+         *control_state |=  TIOCM_CD;
+         icount->dcd++;
+        } else {
+         *control_state &= ~TIOCM_CD;
+        }
+
+        dbg("msr_to_state: msr=0x%x ==> state=0x%x", msr, *control_state);
+
+        wake_up_interruptible(&priv->msr_wait);
 } /* mct_u232_msr_to_state */

 /*
@@ -422,6 +449,7 @@ static int mct_u232_startup(struct usb_s
        if (!priv)
                return -ENOMEM;
        spin_lock_init(&priv->lock);
+       init_waitqueue_head(&priv->msr_wait);
        usb_set_serial_port_data(serial->port[0], priv);

        init_waitqueue_head(&serial->port[0]->write_wait);
@@ -498,7 +526,7 @@ static int  mct_u232_open(struct tty_str
        mct_u232_get_modem_stat(serial, &last_msr);
        spin_lock_irqsave(&priv->lock, flags);
        priv->last_msr = last_msr;
-       mct_u232_msr_to_state(&priv->control_state, priv->last_msr);
+       mct_u232_msr_to_state(priv);
        spin_unlock_irqrestore(&priv->lock, flags);

        port->read_urb->dev = port->serial->dev;
@@ -616,7 +644,7 @@ static void mct_u232_read_int_callback(s
        priv->last_msr = data[MCT_U232_MSR_INDEX];

        /* Record Control Line states */
-       mct_u232_msr_to_state(&priv->control_state, priv->last_msr);
+       mct_u232_msr_to_state(priv);

 #if 0
        /* Not yet handled. See belkin_sa.c for further information */
@@ -823,7 +851,6 @@ static void mct_u232_throttle(struct tty
        }
 }

-
 static void mct_u232_unthrottle(struct tty_struct *tty)
 {
        struct usb_serial_port *port = tty->driver_data;
@@ -844,6 +871,57 @@ static void mct_u232_unthrottle(struct t
        }
 }

+static int  mct_u232_ioctl(struct tty_struct *tty, struct file *file,
+                        unsigned int cmd, unsigned long arg)
+{
+       DEFINE_WAIT(wait);
+       struct usb_serial_port *port = tty->driver_data;
+       struct mct_u232_private *mct_u232_port = usb_get_serial_port_data(port);
+       struct async_icount cnow, cprev;
+
+       dbg("%s - port %d, cmd = 0x%x", __func__, port->number, cmd);
+
+       switch (cmd) {
+
+       case TIOCMIWAIT:
+
+               dbg("%s (%d) TIOCMIWAIT", __func__,  port->number);
+
+               cprev = mct_u232_port->icount;
+               for ( ; ; ) {
+                       prepare_to_wait(&mct_u232_port->msr_wait,
+                                               &wait, TASK_INTERRUPTIBLE);
+                       schedule();
+                       finish_wait(&mct_u232_port->msr_wait, &wait);
+                       /* see if a signal did it */
+                       if (signal_pending(current))
+                               return -ERESTARTSYS;
+                       cnow = mct_u232_port->icount;
+                       if (cnow.rng == cprev.rng && cnow.dsr == cprev.dsr &&
+                           cnow.dcd == cprev.dcd && cnow.cts == cprev.cts)
+                               return -EIO; /* no change => error */
+                       if (((arg & TIOCM_RNG) && (cnow.rng != cprev.rng)) ||
+                           ((arg & TIOCM_DSR) && (cnow.dsr != cprev.dsr)) ||
+                           ((arg & TIOCM_CD)  && (cnow.dcd != cprev.dcd)) ||
+                           ((arg & TIOCM_CTS) && (cnow.cts != cprev.cts))) {
+                               return 0;
+                       }
+                       cprev = cnow;
+               }
+               /* NOTREACHED */
+               break;
+
+       case TIOCGICOUNT:
+                dbg("%s - (%d) TIOCGICOUNT RX=%d, TX=%d", __func__,
+                    port->number, mct_u232_port->icount.rx, mct_u232_port->icount.tx);
+                if (copy_to_user((void __user *)arg, &mct_u232_port->icount,
+                    sizeof(mct_u232_port->icount)))
+                        return -EFAULT;
+                return 0;
+       }
+       return -ENOIOCTLCMD;
+}
+
 static int __init mct_u232_init(void)
 {
        int retval;