* [PATCH 1/1] serial: 8250: revert UART_CAP_NOTEMT changes
@ 2022-04-19 13:39 Ilpo Järvinen
2022-04-19 14:24 ` Uwe Kleine-König
0 siblings, 1 reply; 3+ messages in thread
From: Ilpo Järvinen @ 2022-04-19 13:39 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Uwe Kleine-König, linux-serial, Eric Tremblay, Lukas Wunner,
kernel, Heiko Stuebner, Andy Shevchenko, Jiri Slaby,
Giulio Benetti
[-- Attachment #1: Type: text/plain, Size: 7887 bytes --]
This reverts UART_CAP_NOTEMT commit and driver changes depending
on it:
f6f586102ad1 (serial: 8250: Handle UART without interrupt on TEMT
using em485)
296385fe1275 (serial: 8250: Add UART_CAP_NOTEMT on PORT_16550A_FSL64)
bec1f1b66a6a (serial: 8250: add compatible for fsl,16550-FIFO64)
The UART_CAP_NOTEMT code added in f6f586102add1 (serial: 8250:
Handle UART without interrupt on TEMT using em485) containts math
overflow for 32-bit archs. In addition, the approach used in it
is unnecessarily complicated requiring a dedicated timer just for
notemt. A simpler approach for providing UART_CAP_NOTEMT already
exists (patches 1-2):
https://lore.kernel.org/linux-serial/20220411083321.9131-3-ilpo.jarvinen@linux.intel.com/T/#u
Thus, simply revert the UART_CAP_NOTEMT changes for now.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/tty/serial/8250/8250.h | 1 -
drivers/tty/serial/8250/8250_of.c | 2 -
drivers/tty/serial/8250/8250_port.c | 78 ++---------------------------
include/linux/serial_8250.h | 2 -
4 files changed, 3 insertions(+), 80 deletions(-)
diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index 39ffeb37786f..db784ace25d8 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -83,7 +83,6 @@ struct serial8250_config {
#define UART_CAP_MINI BIT(17) /* Mini UART on BCM283X family lacks:
* STOP PARITY EPAR SPAR WLEN5 WLEN6
*/
-#define UART_CAP_NOTEMT BIT(18) /* UART without interrupt on TEMT available */
#define UART_BUG_QUOT BIT(0) /* UART has buggy quot LSB */
#define UART_BUG_TXEN BIT(1) /* UART has buggy TX IIR status */
diff --git a/drivers/tty/serial/8250/8250_of.c b/drivers/tty/serial/8250/8250_of.c
index 5a699a1aa79c..be8626234627 100644
--- a/drivers/tty/serial/8250/8250_of.c
+++ b/drivers/tty/serial/8250/8250_of.c
@@ -326,8 +326,6 @@ static const struct of_device_id of_platform_serial_table[] = {
.data = (void *)PORT_ALTR_16550_F64, },
{ .compatible = "altr,16550-FIFO128",
.data = (void *)PORT_ALTR_16550_F128, },
- { .compatible = "fsl,16550-FIFO64",
- .data = (void *)PORT_16550A_FSL64, },
{ .compatible = "mediatek,mtk-btif",
.data = (void *)PORT_MTK_BTIF, },
{ .compatible = "mrvl,mmp-uart",
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index c1f24e88ef09..318af6f13605 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -263,7 +263,7 @@ static const struct serial8250_config uart_config[] = {
.tx_loadsz = 63,
.fcr = UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10 |
UART_FCR7_64BYTE,
- .flags = UART_CAP_FIFO | UART_CAP_NOTEMT,
+ .flags = UART_CAP_FIFO,
},
[PORT_RT2880] = {
.name = "Palmchip BK-3103",
@@ -571,21 +571,8 @@ static void serial8250_clear_fifos(struct uart_8250_port *p)
}
}
-static inline void serial8250_em485_update_temt_delay(struct uart_8250_port *p,
- unsigned int cflag, unsigned int baud)
-{
- unsigned int bits;
-
- if (!p->em485)
- return;
-
- bits = tty_get_frame_size(cflag);
- p->em485->no_temt_delay = DIV_ROUND_UP(bits * NSEC_PER_SEC, baud);
-}
-
static enum hrtimer_restart serial8250_em485_handle_start_tx(struct hrtimer *t);
static enum hrtimer_restart serial8250_em485_handle_stop_tx(struct hrtimer *t);
-static enum hrtimer_restart serial8250_em485_handle_no_temt(struct hrtimer *t);
void serial8250_clear_and_reinit_fifos(struct uart_8250_port *p)
{
@@ -644,16 +631,6 @@ static int serial8250_em485_init(struct uart_8250_port *p)
HRTIMER_MODE_REL);
hrtimer_init(&p->em485->start_tx_timer, CLOCK_MONOTONIC,
HRTIMER_MODE_REL);
-
- if (p->capabilities & UART_CAP_NOTEMT) {
- struct tty_struct *tty = p->port.state->port.tty;
-
- serial8250_em485_update_temt_delay(p, tty->termios.c_cflag,
- tty_get_baud_rate(tty));
- hrtimer_init(&p->em485->no_temt_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
- p->em485->no_temt_timer.function = &serial8250_em485_handle_no_temt;
- }
-
p->em485->stop_tx_timer.function = &serial8250_em485_handle_stop_tx;
p->em485->start_tx_timer.function = &serial8250_em485_handle_start_tx;
p->em485->port = p;
@@ -685,7 +662,6 @@ void serial8250_em485_destroy(struct uart_8250_port *p)
hrtimer_cancel(&p->em485->start_tx_timer);
hrtimer_cancel(&p->em485->stop_tx_timer);
- hrtimer_cancel(&p->em485->no_temt_timer);
kfree(p->em485);
p->em485 = NULL;
@@ -1528,11 +1504,6 @@ static void start_hrtimer_ms(struct hrtimer *hrt, unsigned long msec)
hrtimer_start(hrt, ms_to_ktime(msec), HRTIMER_MODE_REL);
}
-static void start_hrtimer_ns(struct hrtimer *hrt, unsigned long nsec)
-{
- hrtimer_start(hrt, ns_to_ktime(nsec), HRTIMER_MODE_REL);
-}
-
static void __stop_tx_rs485(struct uart_8250_port *p)
{
struct uart_8250_em485 *em485 = p->em485;
@@ -1564,33 +1535,14 @@ static inline void __stop_tx(struct uart_8250_port *p)
if (em485) {
unsigned char lsr = serial_in(p, UART_LSR);
-
- p->lsr_saved_flags |= lsr & LSR_SAVE_FLAGS;
-
/*
- * To provide required timing and allow FIFO transfer,
+ * To provide required timeing and allow FIFO transfer,
* __stop_tx_rs485() must be called only when both FIFO and
* shift register are empty. It is for device driver to enable
* interrupt on TEMT.
*/
- if ((lsr & BOTH_EMPTY) != BOTH_EMPTY) {
- if (!(p->capabilities & UART_CAP_NOTEMT))
- /* __stop_tx will be called again once TEMT triggers */
- return;
-
- if (!(lsr & UART_LSR_THRE))
- /* __stop_tx will be called again once THRE triggers */
- return;
-
- /*
- * On devices with no TEMT interrupt available, start
- * a timer for a byte time. The timer will recall
- * __stop_tx().
- */
- em485->active_timer = &em485->no_temt_timer;
- start_hrtimer_ns(&em485->no_temt_timer, em485->no_temt_delay);
+ if ((lsr & BOTH_EMPTY) != BOTH_EMPTY)
return;
- }
__stop_tx_rs485(p);
}
@@ -1701,27 +1653,6 @@ static inline void start_tx_rs485(struct uart_port *port)
__start_tx(port);
}
-static enum hrtimer_restart serial8250_em485_handle_no_temt(struct hrtimer *t)
-{
- struct uart_8250_em485 *em485;
- struct uart_8250_port *p;
- unsigned long flags;
-
- em485 = container_of(t, struct uart_8250_em485, no_temt_timer);
- p = em485->port;
-
- serial8250_rpm_get(p);
- spin_lock_irqsave(&p->port.lock, flags);
- if (em485->active_timer == &em485->no_temt_timer) {
- em485->active_timer = NULL;
- __stop_tx(p);
- }
-
- spin_unlock_irqrestore(&p->port.lock, flags);
- serial8250_rpm_put(p);
- return HRTIMER_NORESTART;
-}
-
static enum hrtimer_restart serial8250_em485_handle_start_tx(struct hrtimer *t)
{
struct uart_8250_em485 *em485 = container_of(t, struct uart_8250_em485,
@@ -2927,9 +2858,6 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
serial8250_set_divisor(port, baud, quot, frac);
- if (up->capabilities & UART_CAP_NOTEMT)
- serial8250_em485_update_temt_delay(up, termios->c_cflag, baud);
-
/*
* LCR DLAB must be set to enable 64-byte FIFO mode. If the FCR
* is written without DLAB set, this mode will be disabled.
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index de135852107c..ff84a3ed10ea 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -79,9 +79,7 @@ struct uart_8250_ops {
struct uart_8250_em485 {
struct hrtimer start_tx_timer; /* "rs485 start tx" timer */
struct hrtimer stop_tx_timer; /* "rs485 stop tx" timer */
- struct hrtimer no_temt_timer; /* "rs485 no TEMT interrupt" timer */
struct hrtimer *active_timer; /* pointer to active timer */
- unsigned long no_temt_delay; /* Delay for no_temt_timer */
struct uart_8250_port *port; /* for hrtimer callbacks */
unsigned int tx_stopped:1; /* tx is currently stopped */
};
--
2.30.2
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH 1/1] serial: 8250: revert UART_CAP_NOTEMT changes
2022-04-19 13:39 [PATCH 1/1] serial: 8250: revert UART_CAP_NOTEMT changes Ilpo Järvinen
@ 2022-04-19 14:24 ` Uwe Kleine-König
2022-04-20 8:17 ` Ilpo Järvinen
0 siblings, 1 reply; 3+ messages in thread
From: Uwe Kleine-König @ 2022-04-19 14:24 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Greg Kroah-Hartman, linux-serial, Eric Tremblay, Heiko Stuebner,
Lukas Wunner, kernel, Andy Shevchenko, Jiri Slaby,
Giulio Benetti
[-- Attachment #1: Type: text/plain, Size: 1494 bytes --]
Hello,
On Tue, Apr 19, 2022 at 04:39:49PM +0300, Ilpo Järvinen wrote:
>
> This reverts UART_CAP_NOTEMT commit and driver changes depending
> on it:
> f6f586102ad1 (serial: 8250: Handle UART without interrupt on TEMT
> using em485)
> 296385fe1275 (serial: 8250: Add UART_CAP_NOTEMT on PORT_16550A_FSL64)
> bec1f1b66a6a (serial: 8250: add compatible for fsl,16550-FIFO64)
>
> The UART_CAP_NOTEMT code added in f6f586102add1 (serial: 8250:
> Handle UART without interrupt on TEMT using em485) containts math
> overflow for 32-bit archs. In addition, the approach used in it
> is unnecessarily complicated requiring a dedicated timer just for
> notemt. A simpler approach for providing UART_CAP_NOTEMT already
> exists (patches 1-2):
> https://lore.kernel.org/linux-serial/20220411083321.9131-3-ilpo.jarvinen@linux.intel.com/T/#u
> Thus, simply revert the UART_CAP_NOTEMT changes for now.
>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Oh I wasn't aware that Greg picked that up. OK for me to revert.
I wonder however if it's nice to revert three patches in one commit. I
would have just reverted f6f586102ad1 and kept the define
UART_CAP_NOTEMT such that the other two patches are noops until your
fixed series comes in. Just my 0.02€.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] serial: 8250: revert UART_CAP_NOTEMT changes
2022-04-19 14:24 ` Uwe Kleine-König
@ 2022-04-20 8:17 ` Ilpo Järvinen
0 siblings, 0 replies; 3+ messages in thread
From: Ilpo Järvinen @ 2022-04-20 8:17 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Greg Kroah-Hartman, linux-serial, Eric Tremblay, Heiko Stuebner,
Lukas Wunner, kernel, Andy Shevchenko, Jiri Slaby,
Giulio Benetti
[-- Attachment #1: Type: text/plain, Size: 1547 bytes --]
On Tue, 19 Apr 2022, Uwe Kleine-König wrote:
> Hello,
>
> On Tue, Apr 19, 2022 at 04:39:49PM +0300, Ilpo Järvinen wrote:
> >
> > This reverts UART_CAP_NOTEMT commit and driver changes depending
> > on it:
> > f6f586102ad1 (serial: 8250: Handle UART without interrupt on TEMT
> > using em485)
> > 296385fe1275 (serial: 8250: Add UART_CAP_NOTEMT on PORT_16550A_FSL64)
> > bec1f1b66a6a (serial: 8250: add compatible for fsl,16550-FIFO64)
> >
> > The UART_CAP_NOTEMT code added in f6f586102add1 (serial: 8250:
> > Handle UART without interrupt on TEMT using em485) containts math
> > overflow for 32-bit archs. In addition, the approach used in it
> > is unnecessarily complicated requiring a dedicated timer just for
> > notemt. A simpler approach for providing UART_CAP_NOTEMT already
> > exists (patches 1-2):
> > https://lore.kernel.org/linux-serial/20220411083321.9131-3-ilpo.jarvinen@linux.intel.com/T/#u
> > Thus, simply revert the UART_CAP_NOTEMT changes for now.
> >
> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>
> Oh I wasn't aware that Greg picked that up. OK for me to revert.
>
> I wonder however if it's nice to revert three patches in one commit. I
> would have just reverted f6f586102ad1 and kept the define
> UART_CAP_NOTEMT such that the other two patches are noops until your
> fixed series comes in. Just my 0.02€.
I was thinking along the same lines but was a little worried Greg would
be against such a solution. ...I'll send v2 with only that single commit
reverted.
--
i.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-04-20 8:17 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-19 13:39 [PATCH 1/1] serial: 8250: revert UART_CAP_NOTEMT changes Ilpo Järvinen
2022-04-19 14:24 ` Uwe Kleine-König
2022-04-20 8:17 ` Ilpo Järvinen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.