Re: [PATCH] tty: serial: samsung_tty: loopback mode support

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Mon, 27 Jun 2022, Chanho Park wrote:

> Internal loopback mode can be supported by setting
> S3C2443_UCON_LOOPBACK bit. The mode & bit can be supported since
> s3c2410 and later SoCs.

In that case, why is the  LOOPBACK define named 2443 and not 2410???

The change looks fine otherwise.

I note though that many of the current drivers won't return TOICM_LOOP 
from ->get_mctrl() but I don't think it's exactly wrong to return it 
either. Perhaps lack of returning it is due to 
Documentation/driver-api/serial/driver.rst not including TOICM_LOOP in 
get_mctrl's list of information but only in set_mctrl's one.

-- 
 i.

> We can test it by linux-serial-test program[1]
> with -k option. It will set TIOCM_LOOP mode during test.
> 
> -k, --loopback     Use internal hardware loop back
> 
> [1]: https://github.com/cbrake/linux-serial-test
> Signed-off-by: Chanho Park <chanho61.park@samsung.com>
> ---
>  drivers/tty/serial/samsung_tty.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
> index d5ca904def34..d7d035cd95c0 100644
> --- a/drivers/tty/serial/samsung_tty.c
> +++ b/drivers/tty/serial/samsung_tty.c
> @@ -1002,16 +1002,22 @@ static unsigned int s3c24xx_serial_tx_empty(struct uart_port *port)
>  static unsigned int s3c24xx_serial_get_mctrl(struct uart_port *port)
>  {
>  	unsigned int umstat = rd_reg(port, S3C2410_UMSTAT);
> +	unsigned int ucon = rd_reg(port, S3C2410_UCON);
> +	unsigned int mctrl = TIOCM_CAR | TIOCM_DSR;
>  
>  	if (umstat & S3C2410_UMSTAT_CTS)
> -		return TIOCM_CAR | TIOCM_DSR | TIOCM_CTS;
> -	else
> -		return TIOCM_CAR | TIOCM_DSR;
> +		mctrl |= TIOCM_CTS;
> +
> +	if (ucon & S3C2443_UCON_LOOPBACK)
> +		mctrl |= TIOCM_LOOP;
> +
> +	return mctrl;
>  }
>  
>  static void s3c24xx_serial_set_mctrl(struct uart_port *port, unsigned int mctrl)
>  {
>  	unsigned int umcon = rd_regl(port, S3C2410_UMCON);
> +	unsigned int ucon = rd_reg(port, S3C2410_UCON);
>  
>  	if (mctrl & TIOCM_RTS)
>  		umcon |= S3C2410_UMCOM_RTS_LOW;
> @@ -1019,6 +1025,13 @@ static void s3c24xx_serial_set_mctrl(struct uart_port *port, unsigned int mctrl)
>  		umcon &= ~S3C2410_UMCOM_RTS_LOW;
>  
>  	wr_regl(port, S3C2410_UMCON, umcon);
> +
> +	if (mctrl & TIOCM_LOOP)
> +		ucon |= S3C2443_UCON_LOOPBACK;
> +	else
> +		ucon &= ~S3C2443_UCON_LOOPBACK;
> +
> +	wr_regl(port, S3C2410_UCON, ucon);
>  }
>  
>  static void s3c24xx_serial_break_ctl(struct uart_port *port, int break_state)
>