| Message ID | 20220628124050.144398-1-chanho61.park@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] tty: serial: samsung_tty: loopback mode support | expand |
On Tue, 28 Jun 2022, Chanho Park wrote: > Internal loopback mode can be supported by setting > UCON register's Loopback Mode bit. The mode & bit can be supported since > s3c2410 and later SoCs. The prefix of LOOPBACK / BIT(5) naming should be > also changed to S3C2410_ to avoid confusion. > > We can test it by linux-serial-test program > with -k option. The tool will set TIOCM_LOOP mode when the option is > specified. > -k, --loopback Use internal hardware loop back > > Signed-off-by: Chanho Park <chanho61.park@samsung.com> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> You should have included Krzysztof's Reviewed-by when you sent this next version so that it won't get lost. I suspect Greg will again complain that you didn't address his comment about the out of the place "-k, --loopback ..." line.
On Tue, Jun 28, 2022 at 09:40:50PM +0900, Chanho Park wrote: > Internal loopback mode can be supported by setting > UCON register's Loopback Mode bit. The mode & bit can be supported since > s3c2410 and later SoCs. The prefix of LOOPBACK / BIT(5) naming should be > also changed to S3C2410_ to avoid confusion. > > We can test it by linux-serial-test program > with -k option. The tool will set TIOCM_LOOP mode when the option is > specified. > -k, --loopback Use internal hardware loop back Again, what does this line mean in a changelog text? And properly wrap your lines at 72 columns. thanks, greg k-h
> > Internal loopback mode can be supported by setting UCON register's > > Loopback Mode bit. The mode & bit can be supported since > > s3c2410 and later SoCs. The prefix of LOOPBACK / BIT(5) naming should > > be also changed to S3C2410_ to avoid confusion. > > > > We can test it by linux-serial-test program with -k option. The tool > > will set TIOCM_LOOP mode when the option is specified. > > -k, --loopback Use internal hardware loop back > > > > Signed-off-by: Chanho Park <chanho61.park@samsung.com> > > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> Thanks for your review. > > You should have included Krzysztof's Reviewed-by when you sent this next > version so that it won't get lost. I thought the codes have been changed since v1 so I'm not convinced I can put his R-B tag. > > I suspect Greg will again complain that you didn't address his comment > about the out of the place "-k, --loopback ..." line. The commit messages seems to be not necessary. I'll remove them next patch. Best Regards, Chanho Park
> > We can test it by linux-serial-test program with -k option. The tool > > will set TIOCM_LOOP mode when the option is specified. > > -k, --loopback Use internal hardware loop back > > Again, what does this line mean in a changelog text? I put the option to refer how the tool can use. I can see it is not related with this commit. I'll remove them next patch. > > And properly wrap your lines at 72 columns. I'll keep that in mind. Thanks for your review :) Best Regards, Chanho Park
diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c index d5ca904def34..03ef4ff506fd 100644 --- a/drivers/tty/serial/samsung_tty.c +++ b/drivers/tty/serial/samsung_tty.c @@ -1012,6 +1012,7 @@ static unsigned int s3c24xx_serial_get_mctrl(struct uart_port *port) 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 +1020,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 |= S3C2410_UCON_LOOPBACK; + else + ucon &= ~S3C2410_UCON_LOOPBACK; + + wr_regl(port, S3C2410_UCON, ucon); } static void s3c24xx_serial_break_ctl(struct uart_port *port, int break_state) diff --git a/include/linux/serial_s3c.h b/include/linux/serial_s3c.h index dec15f5b3dec..1672cf0810ef 100644 --- a/include/linux/serial_s3c.h +++ b/include/linux/serial_s3c.h @@ -83,7 +83,7 @@ #define S3C2410_UCON_RXIRQMODE (1<<0) #define S3C2410_UCON_RXFIFO_TOI (1<<7) #define S3C2443_UCON_RXERR_IRQEN (1<<6) -#define S3C2443_UCON_LOOPBACK (1<<5) +#define S3C2410_UCON_LOOPBACK (1<<5) #define S3C2410_UCON_DEFAULT (S3C2410_UCON_TXILEVEL | \ S3C2410_UCON_RXILEVEL | \
Internal loopback mode can be supported by setting UCON register's Loopback Mode bit. The mode & bit can be supported since s3c2410 and later SoCs. The prefix of LOOPBACK / BIT(5) naming should be also changed to S3C2410_ to avoid confusion. We can test it by linux-serial-test program with -k option. The tool will set TIOCM_LOOP mode when the option is specified. -k, --loopback Use internal hardware loop back Signed-off-by: Chanho Park <chanho61.park@samsung.com> --- Changes from v1: - Drop TIOCM_LOOP return from s3c24xx_serial_get_mctrl as pointed out by Ilpo. Documentation/driver-api/serial/driver.rst indicates the bit is only for set_mctrl. - Change the loopback bit definition from S3C2443_UCON_LOOPBACK to S3C2410_UCON_LOOPBACK because it has been supported since s3c2410. - Remove the unnecessary footnote and put a blank line before Signed-off-by drivers/tty/serial/samsung_tty.c | 8 ++++++++ include/linux/serial_s3c.h | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-)