Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753177AbbF2Qrm (ORCPT ); Mon, 29 Jun 2015 12:47:42 -0400 Received: from mo4-p00-ob.smtp.rzone.de ([81.169.146.161]:30648 "EHLO mo4-p00-ob.smtp.rzone.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752625AbbF2Qrd convert rfc822-to-8bit (ORCPT ); Mon, 29 Jun 2015 12:47:33 -0400 X-RZG-AUTH: :JGIXVUS7cutRB/49FwqZ7WcKdUCnXG6JabOfSXKWrat6m8/npw== X-RZG-CLASS-ID: mo00 Content-Type: text/plain; charset=windows-1252 Mime-Version: 1.0 (Mac OS X Mail 7.3 \(1878.6\)) Subject: Re: [PATCH RFC v2 2/3] tty: serial_core: Add hooks for uart slave drivers From: "Dr. H. Nikolaus Schaller" In-Reply-To: <20150628215100.GC21426@localhost.localdomain> Date: Mon, 29 Jun 2015 18:47:22 +0200 Cc: Marek Belisko , jslaby@suse.cz, gregkh@linuxfoundation.org, arnd@arndb.de, peter@hurleysoftware.com, mark.rutland@arm.com, gnomes@lxorguk.ukuu.org.uk, sre@kernel.org, neil@brown.name, grant.likely@linaro.org, linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org, robh+dt@kernel.org, pawel.moll@arm.com Content-Transfer-Encoding: 8BIT Message-Id: <3E4DBCB0-EA87-4DF2-A6B9-6ED0FBA3BD69@goldelico.com> References: <1435520786-31867-1-git-send-email-marek@goldelico.com> <1435520786-31867-3-git-send-email-marek@goldelico.com> <20150628215100.GC21426@localhost.localdomain> To: Sergei Zviagintsev X-Mailer: Apple Mail (2.1878.6) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8658 Lines: 260 Hi, thanks again. Am 28.06.2015 um 23:51 schrieb Sergei Zviagintsev : > Hi, > > Some minor comments below. > > On Sun, Jun 28, 2015 at 09:46:25PM +0200, Marek Belisko wrote: >> From: "H. Nikolaus Schaller" >> >> 1. allow drivers to get notified in mctrl changes >> 2. allow drivers to get notified on rx data (indicating to the driver that >> the connected chip is active) >> >> Signed-off-by: H. Nikolaus Schaller >> Signed-off-by: Marek Belisko >> --- >> drivers/tty/serial/serial_core.c | 102 +++++++++++++++++++++++++++++++++++++-- >> include/linux/serial_core.h | 11 ++++- >> 2 files changed, 107 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c >> index ad61441..c8910c4 100644 >> --- a/drivers/tty/serial/serial_core.c >> +++ b/drivers/tty/serial/serial_core.c >> @@ -163,6 +163,84 @@ err0: >> } >> EXPORT_SYMBOL_GPL(devm_serial_get_uart_by_phandle); >> >> +void uart_register_slave(struct uart_port *uport, void *slave) >> +{ >> + if (!slave) { >> + uart_register_mctrl_notification(uport, NULL); >> + uart_register_rx_notification(uport, NULL, NULL); >> + } >> + uport->slave = slave; >> +} >> + >> +void uart_register_mctrl_notification(struct uart_port *uport, int (*function)(void *slave, int state)) >> +{ >> + uport->mctrl_notification = function; >> +} >> + >> +static int uart_port_startup(struct tty_struct *tty, struct uart_state *state, >> + int init_hw); >> + >> +static void uart_port_shutdown(struct tty_port *port); >> + >> +void uart_register_rx_notification(struct uart_port *uport, int (*function)(void *slave, unsigned int *c), struct ktermios *termios) >> +{ >> + struct uart_state *state = uport->state; >> + struct tty_port *tty_port = &state->port; >> + >> + if (!uport->slave) >> + return; /* slave must be registered first */ >> + >> + uport->rx_notification = function; >> + >> + if (tty_port->count == 0) { > > if (tty_port->count) > return; > >> + if (function) { >> + int retval = 0; > > Useless initialization. Indeed. > >> + >> + uart_change_pm(state, UART_PM_STATE_ON); >> + retval = uport->ops->startup(uport); >> + if (retval == 0 && termios) { > > Indentation is evil in this rather simple function :) > >> + int hw_stopped; >> + /* >> + * Initialise the hardware port settings. >> + */ >> + uport->ops->set_termios(uport, termios, NULL); >> + >> + /* >> + * Set modem status enables based on termios cflag >> + */ >> + spin_lock_irq(&uport->lock); >> + if (termios->c_cflag & CRTSCTS) >> + uport->status |= UPSTAT_CTS_ENABLE; >> + else >> + uport->status &= ~UPSTAT_CTS_ENABLE; >> + >> + if (termios->c_cflag & CLOCAL) >> + uport->status &= ~UPSTAT_DCD_ENABLE; >> + else >> + uport->status |= UPSTAT_DCD_ENABLE; >> + >> + /* reset sw-assisted CTS flow control based on (possibly) new mode */ > > checkpatch.pl will complain about long line :) Ok. > >> + hw_stopped = uport->hw_stopped; >> + uport->hw_stopped = uart_softcts_mode(uport) && >> + !(uport->ops->get_mctrl(uport) & TIOCM_CTS); >> + if (uport->hw_stopped) { >> + if (!hw_stopped) >> + uport->ops->stop_tx(uport); >> + } else { >> + if (hw_stopped) >> + uport->ops->start_tx(uport); >> + } >> + spin_unlock_irq(&uport->lock); >> + } >> + } else > > Usage of braces is against CodingStyle. Ok. > >> + uart_port_shutdown(tty_port); >> + } >> +} >> + >> +EXPORT_SYMBOL_GPL(uart_register_slave); >> +EXPORT_SYMBOL_GPL(uart_register_mctrl_notification); >> +EXPORT_SYMBOL_GPL(uart_register_rx_notification); >> + >> /* >> * This routine is used by the interrupt handler to schedule processing in >> * the software interrupt portion of the driver. >> @@ -220,6 +298,10 @@ uart_update_mctrl(struct uart_port *port, unsigned int set, unsigned int clear) >> port->mctrl = (old & ~clear) | set; >> if (old != port->mctrl) >> port->ops->set_mctrl(port, port->mctrl); >> + >> + if (port->mctrl_notification) >> + (*port->mctrl_notification)(port->slave, port->mctrl); >> + >> spin_unlock_irqrestore(&port->lock, flags); >> } >> >> @@ -259,7 +341,8 @@ static int uart_port_startup(struct tty_struct *tty, struct uart_state *state, >> uart_circ_clear(&state->xmit); >> } >> >> - retval = uport->ops->startup(uport); >> + if (!state->uart_port->rx_notification) >> + retval = uport->ops->startup(uport); >> if (retval == 0) { >> if (uart_console(uport) && uport->cons->cflag) { >> tty->termios.c_cflag = uport->cons->cflag; >> @@ -295,7 +378,7 @@ static int uart_startup(struct tty_struct *tty, struct uart_state *state, >> int init_hw) >> { >> struct tty_port *port = &state->port; >> - int retval; >> + int retval = 0; > > What is the point to initialize `retval?? Probably remains from an intermediate version where we had needed it. Will be removed. > >> >> if (port->flags & ASYNC_INITIALIZED) >> return 0; >> @@ -341,8 +424,12 @@ static void uart_shutdown(struct tty_struct *tty, struct uart_state *state) >> >> if (!tty || (tty->termios.c_cflag & HUPCL)) >> uart_clear_mctrl(uport, TIOCM_DTR | TIOCM_RTS); >> - >> - uart_port_shutdown(port); >> + /* >> + * if we have a slave that has registered for rx_notifications >> + * we do not shut down the uart port to be able to monitor the device >> + */ >> + if (!uport->rx_notification) >> + uart_port_shutdown(port); >> } >> >> /* >> @@ -1489,8 +1576,9 @@ static void uart_close(struct tty_struct *tty, struct file *filp) >> /* >> * At this point, we stop accepting input. To do this, we >> * disable the receive line status interrupts. >> + * Unless a slave driver wants to keep input running >> */ >> - if (port->flags & ASYNC_INITIALIZED) { >> + if (!uport->rx_notification && (port->flags & ASYNC_INITIALIZED)) { >> unsigned long flags; >> spin_lock_irqsave(&uport->lock, flags); >> uport->ops->stop_rx(uport); >> @@ -3018,6 +3106,10 @@ void uart_insert_char(struct uart_port *port, unsigned int status, >> { >> struct tty_port *tport = &port->state->port; >> >> + if (port->rx_notification) >> + if ((*port->rx_notification)(port->slave, &ch)) >> + return; /* slave told us to ignore character */ >> + >> if ((status & port->ignore_status_mask & ~overrun) == 0) >> if (tty_insert_flip_char(tport, ch, flag) == 0) >> ++port->icount.buf_overrun; >> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h >> index ba23718..77029781 100644 >> --- a/include/linux/serial_core.h >> +++ b/include/linux/serial_core.h >> @@ -35,7 +35,7 @@ >> #define uart_console(port) \ >> ((port)->cons && (port)->cons->index == (port)->line) >> #else >> -#define uart_console(port) ({ (void)port; 0; }) >> +#define uart_console(port) (0) >> #endif >> >> struct uart_port; >> @@ -247,7 +247,11 @@ struct uart_port { >> const struct attribute_group **tty_groups; /* all attributes (serial core use only) */ >> struct serial_rs485 rs485; >> void *private_data; /* generic platform data pointer */ >> + /* UART slave support */ >> struct list_head head; /* list of uarts e.g. to look up by phandle */ >> + void *slave; /* optional slave (there can be only one) */ >> + int (*mctrl_notification)(void *slave, int state); >> + int (*rx_notification)(void *slave, unsigned int *c); >> }; >> >> static inline int serial_port_in(struct uart_port *up, int offset) >> @@ -485,4 +489,9 @@ static inline int uart_handle_break(struct uart_port *port) >> */ >> extern struct uart_port *devm_serial_get_uart_by_phandle(struct device *dev, >> const char *phandle, u8 index); >> +/* register to receive notifications */ >> +extern void uart_register_slave(struct uart_port *uart, void *slave); >> +extern void uart_register_mctrl_notification(struct uart_port *uart, int (*function)(void *slave, int state)); >> +extern void uart_register_rx_notification(struct uart_port *uart, int (*function)(void *slave, unsigned int *c), struct ktermios *termios); >> + >> #endif /* LINUX_SERIAL_CORE_H */ >> -- >> 1.9.1 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/