* [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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ 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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ 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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ 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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ 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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-07-12 23:57 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).