* [PATCH v3] tty: serial: samsung_tty: loopback mode support [not found] <CGME20220629004356epcas2p408afcd3d19d926a86d98e887e25e93bc@epcas2p4.samsung.com> @ 2022-06-29 0:41 ` Chanho Park 2022-07-11 12:31 ` Marek Szyprowski 0 siblings, 1 reply; 5+ messages in thread From: Chanho Park @ 2022-06-29 0:41 UTC (permalink / raw) To: Krzysztof Kozlowski, Greg Kroah-Hartman Cc: Jiri Slaby, Alim Akhtar, Hector Martin, Jaewon Kim, Vincent Whitchurch, Ilpo Järvinen, linux-samsung-soc, linux-serial, linux-arm-kernel, Chanho Park 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_ in order to avoid confusion. Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> Signed-off-by: Chanho Park <chanho61.park@samsung.com> --- Changes from v2: - Add Ilop and Krzysztof's R-B tags - Drop linux-serial-test related commit messages - Wrap commit messages at 72 columns 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(-) 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 | \ -- 2.36.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3] tty: serial: samsung_tty: loopback mode support 2022-06-29 0:41 ` [PATCH v3] tty: serial: samsung_tty: loopback mode support Chanho Park @ 2022-07-11 12:31 ` Marek Szyprowski 2022-07-12 0:44 ` Chanho Park 0 siblings, 1 reply; 5+ messages in thread From: Marek Szyprowski @ 2022-07-11 12:31 UTC (permalink / raw) To: Chanho Park, Krzysztof Kozlowski, Greg Kroah-Hartman Cc: Jiri Slaby, Alim Akhtar, Hector Martin, Jaewon Kim, Vincent Whitchurch, Ilpo Järvinen, linux-samsung-soc, linux-serial, linux-arm-kernel Hi Chanho, On 29.06.2022 02:41, 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_ in order to avoid confusion. > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > Signed-off-by: Chanho Park <chanho61.park@samsung.com> > --- > Changes from v2: > - Add Ilop and Krzysztof's R-B tags > - Drop linux-serial-test related commit messages > - Wrap commit messages at 72 columns > > 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(-) > > 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); S3C2410_UCON must be modified at least under the local_irq_save() or port lock, otherwise it breaks kernel console operation on older Exynos SoCs (especially when DMA mode is used). The above read-modify-write sequence should be replaced with the following pattern: if (mctrl & TIOCM_LOOP) s3c24xx_set_bit(port, S3C2410_UCON_LOOPBACK, S3C2410_UCON); else s3c24xx_clear_bit(port, S3C2410_UCON_LOOPBACK, S3C2410_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 | \ Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH v3] tty: serial: samsung_tty: loopback mode support 2022-07-11 12:31 ` Marek Szyprowski @ 2022-07-12 0:44 ` Chanho Park 2022-07-12 13:58 ` Marek Szyprowski 0 siblings, 1 reply; 5+ messages in thread From: Chanho Park @ 2022-07-12 0:44 UTC (permalink / raw) To: 'Marek Szyprowski', 'Krzysztof Kozlowski', 'Greg Kroah-Hartman' Cc: 'Jiri Slaby', 'Alim Akhtar', 'Hector Martin', 'Jaewon Kim', 'Vincent Whitchurch', 'Ilpo Järvinen', linux-samsung-soc, linux-serial, linux-arm-kernel > > + if (mctrl & TIOCM_LOOP) > > + ucon |= S3C2410_UCON_LOOPBACK; > > + else > > + ucon &= ~S3C2410_UCON_LOOPBACK; > > + > > + wr_regl(port, S3C2410_UCON, ucon); > > S3C2410_UCON must be modified at least under the local_irq_save() or port > lock, otherwise it breaks kernel console operation on older Exynos SoCs > (especially when DMA mode is used). > > The above read-modify-write sequence should be replaced with the following > pattern: > > if (mctrl & TIOCM_LOOP) > s3c24xx_set_bit(port, S3C2410_UCON_LOOPBACK, S3C2410_UCON); > else > s3c24xx_clear_bit(port, S3C2410_UCON_LOOPBACK, S3C2410_UCON); All the set_mctrl() call can be protected by spin_[un]lock_irq[save/restore] with port->lock. So, I think it is not required. Best Regards, Chanho Park ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] tty: serial: samsung_tty: loopback mode support 2022-07-12 0:44 ` Chanho Park @ 2022-07-12 13:58 ` Marek Szyprowski 2022-07-12 23:55 ` Chanho Park 0 siblings, 1 reply; 5+ messages in thread From: Marek Szyprowski @ 2022-07-12 13:58 UTC (permalink / raw) To: Chanho Park, 'Krzysztof Kozlowski', 'Greg Kroah-Hartman' Cc: 'Jiri Slaby', 'Alim Akhtar', 'Hector Martin', 'Jaewon Kim', 'Vincent Whitchurch', 'Ilpo Järvinen', linux-samsung-soc, linux-serial, linux-arm-kernel On 12.07.2022 02:44, Chanho Park wrote: >>> + if (mctrl & TIOCM_LOOP) >>> + ucon |= S3C2410_UCON_LOOPBACK; >>> + else >>> + ucon &= ~S3C2410_UCON_LOOPBACK; >>> + >>> + wr_regl(port, S3C2410_UCON, ucon); >> S3C2410_UCON must be modified at least under the local_irq_save() or port >> lock, otherwise it breaks kernel console operation on older Exynos SoCs >> (especially when DMA mode is used). >> >> The above read-modify-write sequence should be replaced with the following >> pattern: >> >> if (mctrl & TIOCM_LOOP) >> s3c24xx_set_bit(port, S3C2410_UCON_LOOPBACK, S3C2410_UCON); >> else >> s3c24xx_clear_bit(port, S3C2410_UCON_LOOPBACK, S3C2410_UCON); > All the set_mctrl() call can be protected by spin_[un]lock_irq[save/restore] with port->lock. > So, I think it is not required. Right. The problem is elsewhere. You used rd_reg(port, S3C2410_UCON) instead of rd_regl(port, S3C2410_UCON) what always zeroed upper bits. I will send a fix in a few minutes. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH v3] tty: serial: samsung_tty: loopback mode support 2022-07-12 13:58 ` Marek Szyprowski @ 2022-07-12 23:55 ` Chanho Park 0 siblings, 0 replies; 5+ messages in thread From: Chanho Park @ 2022-07-12 23:55 UTC (permalink / raw) To: 'Marek Szyprowski', 'Krzysztof Kozlowski', 'Greg Kroah-Hartman' Cc: 'Jiri Slaby', 'Alim Akhtar', 'Hector Martin', 'Jaewon Kim', 'Vincent Whitchurch', 'Ilpo Järvinen', linux-samsung-soc, linux-serial, linux-arm-kernel > Right. The problem is elsewhere. You used rd_reg(port, S3C2410_UCON) > instead of rd_regl(port, S3C2410_UCON) what always zeroed upper bits. I > will send a fix in a few minutes. Oops. You're right. Thanks for pointing it out and send the fix. Best Regards, Chanho Park ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-07-12 23:55 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20220629004356epcas2p408afcd3d19d926a86d98e887e25e93bc@epcas2p4.samsung.com>
2022-06-29 0:41 ` [PATCH v3] tty: serial: samsung_tty: loopback mode support Chanho Park
2022-07-11 12:31 ` Marek Szyprowski
2022-07-12 0:44 ` Chanho Park
2022-07-12 13:58 ` Marek Szyprowski
2022-07-12 23:55 ` Chanho Park
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).