Re: [PATCH] [SERIAL] qe-uart: add support for Freescale QUICCEngine UART

On Jan 15, 2008, at 9:03 AM, Timur Tabi wrote:

Kumar Gala wrote:

quoted

what is the define for?  you commented all the others :)

I thought you said there were no more issues? :-)

I lied, get use to it ;)

(I was going to apply the patch and thus scanned over it).

quoted

quoted

+#define UCC_WAIT_CLOSING 100

This is how long to wait after receiving a close request for  
characters to be sent out before actually closing the device.  I  
just used the same value as the CPM serial driver.

Yeah, I just meant a comment to this fact would be nice.

quoted

quoted

+    u16 tx_state;       /* 0xC6, TX state */

__be16?

Oops.

quoted

quoted

+/*
+ * Set the modem control lines
+ *
+ * We currently don't support setting modem control lines, but  
this function
+ * needs to exist, otherwise the kernel will panic.
+ */
+void qe_uart_set_mctrl(struct uart_port *port, unsigned int mctrl)
+{
+}

What's the issue w/support of this?  (maybe a comment related to  
why its not supported -- if not here in the git commit message)

I didn't want to add any serial functionality that wasn't in the CPM  
serial driver, mostly because I don't really understand the  
technical details of serial communication at this level.  I figured  
if it's okay for the CPM driver not to handle modem control, then  
the QE driver doesn't have to do it either.

This is fine, can we make it clear that we aren't implementing them  
because we didn't need them not because of some board issue.  (Looking  
at the 8250.c driver it looks like this has to do with CTS support --  
which the QE UCC supports, we just aren't using).

quoted

quoted

+static void qe_uart_enable_ms(struct uart_port *port)
+{

same question as above.

Same answer!

I suppose you want me to post another version of this patch?

yes.

- k