Re: [PATCH V5 6/8] USB: f81232: clarify f81232_ioctl()
Hello, Johan Hovold 於 2015/2/9 下午 04:42 寫道: The value you should be returning is jiffies_to_msecs(port-port.closing_wait) / 10, unless the value is ASYNC_CLOSING_WAIT_NONE in which case you simply return that, and similarly for close_delay. I'll try to fix it, or reuse default value next version. Make sure to update the commit log for the next revision so that it describes what you actually do. I will probably not have time to review this version this week I'm afraid. OK, I'll review it again, fix and resend it after Chinese New Year (2/23+). Thanks all seniors. -- With Best Regards, Peter Hung -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V5 6/8] USB: f81232: clarify f81232_ioctl()
On Mon, Feb 09, 2015 at 02:59:12PM +0800, Peter Hung wrote: Hello, Sergei Shtylyov 於 2015/2/6 下午 08:21 寫道: We extract TIOCGSERIAL section in f81232_ioctl() to f81232_get_serial_info() to make it clarify You're also changing 'ser.baud_rate' from 460800 to 115200. And explicitly overriding some previously initialized to 0 fields. F81232 max baudrate is only 115200bps, so I set it for 1.8432MHz/16 = 115200. We had add some closing time referenced from serial_core.c. The default value is: port-close_delay = HZ / 2; /* .5 seconds */ port-closing_wait= 30 * HZ;/* 30 seconds */ We had increasing close_delay about 10x to port-close_delay = 5 * HZ ; You're never changing anything, you're just reporting an incorrect value to userspace here. The value you should be returning is jiffies_to_msecs(port-port.closing_wait) / 10, unless the value is ASYNC_CLOSING_WAIT_NONE in which case you simply return that, and similarly for close_delay. The f81232_set_mctrl() replace set_control_lines() to do MCR control so we clean-up the set_control_lines() function. I don't see where are you doing this... This text is my patch V5 5/8 second section. I had wrong operation of copy paste. It's doesn't need for this patch, sorry for it. Make sure to update the commit log for the next revision so that it describes what you actually do. I will probably not have time to review this version this week I'm afraid. Johan -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V5 6/8] USB: f81232: clarify f81232_ioctl()
Hello, Sergei Shtylyov 於 2015/2/6 下午 08:21 寫道: We extract TIOCGSERIAL section in f81232_ioctl() to f81232_get_serial_info() to make it clarify You're also changing 'ser.baud_rate' from 460800 to 115200. And explicitly overriding some previously initialized to 0 fields. F81232 max baudrate is only 115200bps, so I set it for 1.8432MHz/16 = 115200. We had add some closing time referenced from serial_core.c. The default value is: port-close_delay = HZ / 2; /* .5 seconds */ port-closing_wait= 30 * HZ;/* 30 seconds */ We had increasing close_delay about 10x to port-close_delay = 5 * HZ ; The f81232_set_mctrl() replace set_control_lines() to do MCR control so we clean-up the set_control_lines() function. I don't see where are you doing this... This text is my patch V5 5/8 second section. I had wrong operation of copy paste. It's doesn't need for this patch, sorry for it. -- With Best Regards, Peter Hung -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V5 6/8] USB: f81232: clarify f81232_ioctl()
Hello.
On 2/6/2015 12:46 PM, Peter Hung wrote:
We extract TIOCGSERIAL section in f81232_ioctl() to f81232_get_serial_info()
to make it clarify
You're also changing 'ser.baud_rate' from 460800 to 115200. And explicitly
overriding some previously initialized to 0 fields.
The f81232_set_mctrl() replace set_control_lines() to do MCR control
so we clean-up the set_control_lines() function.
I don't see where are you doing this...
Signed-off-by: Peter Hung hpeter+linux_ker...@gmail.com
---
drivers/usb/serial/f81232.c | 34 +++---
1 file changed, 23 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index e1cdf42..4dddb44 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -516,24 +516,36 @@ static int f81232_carrier_raised(struct usb_serial_port
*port)
return 0;
}
+static int f81232_get_serial_info(struct usb_serial_port *port,
+ unsigned long arg)
+{
+ struct serial_struct ser;
+
+ memset(ser, 0, sizeof(ser));
+
+ ser.flags = ASYNC_SKIP_TEST | ASYNC_AUTO_IRQ;
+ ser.xmit_fifo_size = port-bulk_out_size;
+ ser.close_delay = 5 * HZ;
+ ser.closing_wait= 30 * HZ;
+ ser.type = PORT_16550A;
+ ser.line = port-minor;
+ ser.port = port-port_number;
+ ser.baud_base = 115200;
+
+ if (copy_to_user((void __user *)arg, ser, sizeof(ser)))
+ return -EFAULT;
+
+ return 0;
+}
+
static int f81232_ioctl(struct tty_struct *tty,
unsigned int cmd, unsigned long arg)
{
- struct serial_struct ser;
struct usb_serial_port *port = tty-driver_data;
switch (cmd) {
case TIOCGSERIAL:
- memset(ser, 0, sizeof ser);
- ser.type = PORT_16654;
- ser.line = port-minor;
- ser.port = port-port_number;
- ser.baud_base = 460800;
-
- if (copy_to_user((void __user *)arg, ser, sizeof ser))
- return -EFAULT;
-
- return 0;
+ return f81232_get_serial_info(port, arg);
default:
break;
}
WBR, Sergei
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
