Re: [PATCH v4 1/4] serial: mxs-auart: clean get_mctrl and set_mctrl

[Richard Genoud <richard.genoud@gmail.com>]

On 10/10/2014 18:53, Janusz Uzycki wrote:
> Russell King:
> The only thing which the .get_mctrl method is supposed to do is
> to return the state of the /input/ lines, which are CTS, DCD, DSR, RI.
> The output line state is stored in port->mctrl, and is added to
> the returned value by serial_core when it's required.
> RTS output state should not be returned from the .get_mctrl method.
> 
> This patch removes ctrl variable from mxs_auart_port
> and removes useless reading back RTS line.
> 
> The ctrl variable in mxs_auart_port duplicated mctrl,
> member of uart_port structure in serial_core.h.
> The removed code from mxs_auart_set_mctrl() and mxs_auart_get_mctrl
> duplicated uart_update_mctrl() and uart_tiocmget()
> in serial_core.c.
> 
> Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl>
> ---
> v3 -> v4 changelog:
> [PATCH 1/4] serial: mxs-auart: clean get_mctrl and set_mctrl
> * renamed from "serial: mxs-auart: ctrl removed from mxs_auart_port"
> * mxs_auart_get_mctrl() read back RTS line. It is removed too.
> 
> v2 -> v3 changelog:
> The patch was introduced:
> new [PATCH 1/4] serial: mxs-auart: ctrl removed from mxs_auart_port
> * the ctrl variable duplicated mctrl, member of uart_port structure
>   in serial_core.h
> * the code duplicated uart_update_mctrl() and uart_tiocmget()
>   in serial_core.c
> * mxs_auart_get_mctrl() reads back RTS line. It could be removed too
>   but not sure.
> 
> ---
>  drivers/tty/serial/mxs-auart.c | 14 +-------------
>  1 file changed, 1 insertion(+), 13 deletions(-)
> 
> diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
> index e838e84..2d49901 100644
> --- a/drivers/tty/serial/mxs-auart.c
> +++ b/drivers/tty/serial/mxs-auart.c
> @@ -143,7 +143,6 @@ struct mxs_auart_port {
>  #define MXS_AUART_DMA_RX_READY	3  /* bit 3 */
>  #define MXS_AUART_RTSCTS	4  /* bit 4 */
>  	unsigned long flags;
> -	unsigned int ctrl;
>  	enum mxs_auart_type devtype;
>  
>  	unsigned int irq;
> @@ -406,8 +405,6 @@ static void mxs_auart_release_port(struct uart_port *u)
>  
>  static void mxs_auart_set_mctrl(struct uart_port *u, unsigned mctrl)
>  {
> -	struct mxs_auart_port *s = to_auart_port(u);
> -
>  	u32 ctrl = readl(u->membase + AUART_CTRL2);
>  
>  	ctrl &= ~(AUART_CTRL2_RTSEN | AUART_CTRL2_RTS);
> @@ -418,24 +415,17 @@ static void mxs_auart_set_mctrl(struct uart_port *u, unsigned mctrl)
>  			ctrl |= AUART_CTRL2_RTS;
>  	}
>  
> -	s->ctrl = mctrl;
>  	writel(ctrl, u->membase + AUART_CTRL2);
>  }
>  
>  static u32 mxs_auart_get_mctrl(struct uart_port *u)
>  {
> -	struct mxs_auart_port *s = to_auart_port(u);
>  	u32 stat = readl(u->membase + AUART_STAT);
> -	int ctrl2 = readl(u->membase + AUART_CTRL2);
> -	u32 mctrl = s->ctrl;
> +	u32 mctrl = 0;
>  
> -	mctrl &= ~TIOCM_CTS;
>  	if (stat & AUART_STAT_CTS)
>  		mctrl |= TIOCM_CTS;
>  
> -	if (ctrl2 & AUART_CTRL2_RTS)
> -		mctrl |= TIOCM_RTS;
> -
>  	return mctrl;
>  }
>  
> @@ -1071,8 +1061,6 @@ static int mxs_auart_probe(struct platform_device *pdev)
>  	s->port.type = PORT_IMX;
>  	s->port.dev = s->dev = &pdev->dev;
>  
> -	s->ctrl = 0;
> -
>  	s->irq = platform_get_irq(pdev, 0);
>  	s->port.irq = s->irq;
>  	ret = request_irq(s->irq, mxs_auart_irq_handle, 0, dev_name(&pdev->dev), s);
> 
Seems good to me.

Reviewed-by: Richard Genoud <richard.genoud@gmail.com>