On Sun, Dec 19, 2010 at 07:18:04AM -0500, Vijay Kiran Kamuju wrote:
> On Sat, Dec 18, 2010 at 7:04 PM, Greg KH <gregkh@xxxxxxx> wrote:
>
> > 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?
> >
> >
> Updated the patch to do this in the pl2303_update_line_status() call
When you resend a patch, it needs to be "clean" with the proper
changelog information and signed-off-by: line. Think of sending a patch
that I don't have to edit at all in order to be able to apply it and you
should be fine.
>
> 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.
> >
> >
> The actual reading is not in the pl2303_read_int_callback() function.
> Hence we are setting the received information/count to the status info
> received rather than the data it received.
Why? You need to count "interrupts" not bytes received here. So it
should be a simple increment of 1 added, not the number of bytes
received.
> 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.
> >
> >
> I have updated the patch to set the variables related to the line status in
> the icount, since there is no modem status register available for this one.
> I hope every thing is ok in this second try.
> If not please let me now.
What about transmit interrupts?
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
