Re: [PATCH V6 1/1] usb:serial: Add Fintek F81532/534 driver

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

 



Hi,

Oliver Neukum 於 2015/11/4 下午 04:38 寫道:
On Wed, 2015-11-04 at 16:19 +0800, Peter Hung wrote:
Hi

Oliver Neukum 於 2015/11/3 下午 06:03 寫道:
On Tue, 2015-11-03 at 11:51 +0800, Peter Hung wrote:
+	for (i = 0; i < F81534_NUM_PORT; ++i)
+		atomic_set(&serial_priv->port_active[i], 0);

Should be ATOMIC_INIT()


ATOMIC_INIT() seems to be used only for variable initializer, It cant be
used for dynamic allocation. Should I change it to a normal boolean
flag protecting with spin_lock ?

No, if it doesn't work, use the current code.

OK, I'll use current code.

+static void f81534_compare_msr(struct usb_serial_port *port, u8 *msr,

Is the point of passing a pointer to msr locking?

+				bool is_port_open)

This function is used only with URB callback function. The *msr is
reported by H/W with newest MSR. The USB-Serial generic system will
re-submit read URB when callback complete. So this function should
run once on the same time.

Yes, so why don't you pass an u8 as opposed to a pointer to an u8?

I'll re-write it from u8* to u8.

+static int f81534_tiocmget(struct tty_struct *tty)
+{
+	struct usb_serial_port *port = tty->driver_data;
+	struct f81534_port_private *port_priv = usb_get_serial_port_data(port);
+	unsigned long flags;
+	int r;
+	u8 msr, mcr;
+
+	/*
+	 * We'll avoid to direct read MSR register. The IC will read the MSR
+	 * changed and report it f81534_process_per_serial_block() by
+	 * F81534_TOKEN_MSR_CHANGE.
+	 *
+	 * When this device in heavy loading (e.g., BurnInTest Loopback Test)
+	 * The report of MSR register will delay received a bit. It's due to
+	 * MSR interrupt is lowest priority in 16550A. So we decide to sleep
+	 * a little time to pass the test.
+	 */
+	if (schedule_timeout_interruptible(
+			msecs_to_jiffies(F81534_DELAY_READ_MSR))) {
+		dev_info(&port->dev, "%s: breaked !!\n", __func__);
+	}

Is the delay necessary or isn't it?
If it is necessary you should do something about the signal.


We add this delay due to stress test (Loop-back & 921600bps with
BurnInTest). It'll receive MSR with some delay when connecting with
DTR-DSR & RTS/CTS, but the delay smaller than 10ms. So we decided to
delay some time to pass the test.

OK, but how do you guarantee the delay you need if you get a signal,
which would abort the delay?


Hmm, you are right. It seems to replace *_interruptible to
*_killable and return -EINTR to guarantee not abort by normal
signal.


Thanks for your advices
--
With Best Regards,
Peter Hung
--
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