Re: Add TIOCGICOUNT functionality to pl2303 driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Dec 18, 2010 at 01:40:07PM -0500, Vijay Kiran Kamuju wrote:
> Hi,
> 
>         Developer Certificate of Origin :
> 
>         (a) The contribution was created in whole or in part by me and I
>             have the right to submit it under the open source license
>             indicated in the file; or
> 
>         (b) The contribution is based upon previous work that, to the best
>             of my knowledge, is covered under an appropriate open source
>             license and I have the right under that license to submit that
>             work with modifications, whether created in whole or in part
>             by me, under the same open source license (unless I am
>             permitted to submit under a different license), as indicated
>             in the file; or
> 
>         (c) The contribution was provided directly to me by some other
>             person who certified (a), (b) or (c) and I have not modified
>             it.
> 
>         (d) I understand and agree that this project and the contribution
>             are public and that a record of the contribution (including all
>             personal information I submit with it, including my sign-off) is
>             maintained indefinitely and may be redistributed consistent with
>             this project or the open source license(s) involved.
> 
>         [infyquest@xxxxxxxxx: add TIOCGICOUNT functionality to pl2303
> driver]
>         Signed-off-by: Vijay Kiran Kamuju <infyquest@xxxxxxxxx>

You don't have to quote the whole thing at all, all you need is a simple
"Signed-off-by:" line at the end of your changelog comment in the body
of your email, right before the patch.


> @@ -711,6 +739,7 @@ static void pl2303_update_line_status(st
>  static void pl2303_read_int_callback(struct urb *urb)
>  {
>  	struct usb_serial_port *port =  urb->context;
> +	struct pl2303_private *priv = usb_get_serial_port_data(port);
>  	unsigned char *data = urb->transfer_buffer;
>  	unsigned int actual_length = urb->actual_length;
>  	int status = urb->status;
> @@ -738,6 +767,10 @@ static void pl2303_read_int_callback(str
>  	usb_serial_debug_data(debug, &port->dev, __func__,
>  			      urb->actual_length, urb->transfer_buffer);
>  
> +	spin_lock(&priv->lock);
> +	priv->icount.rx += urb->actual_length;
> +	spin_unlock(&priv->lock);
> +
>  	pl2303_update_line_status(port, data, actual_length);

Why not just update icount.rx in the pl2303_update_line_status() call?

Actually, I don't think this is right at all, this was not data that was
sent or received, it is status information about the UART settings.

You also aren't setting any other of the icount variables, leaving them
all to be 0, which really isn't helpful.  Please set them to their
proper values so that userspace can rely on them being correct.  Right
now it is better to not provide the ioctl at all instead of providing
incorrect data.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux