On 14. 10. 23, 20:13, Vamshi Gajjela wrote:
From: VAMSHI GAJJELA <vamshigajjela@xxxxxxxxxx>
uart_update_timeout() sets a u64 value to an unsigned int frame_time.
While it can be cast to u32 before assignment, there's a specific case
where frame_time is cast to u64. Since frame_time consistently
participates in u64 arithmetic, its data type is converted to u64 to
eliminate the need for explicit casting.
And make the divisions by the order of magnutude slower for no good
reason? (Hint: have you looked what DIV64_U64_ROUND_UP() looks like on
32bit yet?)
Unless you provide a reason it can overflow in real (in fact you spell
the opposite in the text above):
NACKED-by: Jiri Slaby <jirislaby@xxxxxxxxxx>
Signed-off-by: VAMSHI GAJJELA <vamshigajjela@xxxxxxxxxx>
---
v2:
- use DIV64_U64_ROUND_UP with frame_time
drivers/tty/serial/8250/8250_port.c | 2 +-
include/linux/serial_core.h | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 141627370aab..d1bf794498c4 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1510,7 +1510,7 @@ static inline void __stop_tx(struct uart_8250_port *p)
* rather than after it is fully sent.
* Roughly estimate 1 extra bit here with / 7.
*/
- stop_delay = p->port.frame_time + DIV_ROUND_UP(p->port.frame_time, 7);
+ stop_delay = p->port.frame_time + DIV64_U64_ROUND_UP(p->port.frame_time, 7);
}
__stop_tx_rs485(p, stop_delay);
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index bb6f073bc159..b128513b009a 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -558,7 +558,7 @@ struct uart_port {
bool hw_stopped; /* sw-assisted CTS flow state */
unsigned int mctrl; /* current modem ctrl settings */
- unsigned int frame_time; /* frame timing in ns */
+ unsigned long frame_time; /* frame timing in ns */
unsigned int type; /* port type */
const struct uart_ops *ops;
unsigned int custom_divisor;
@@ -764,7 +764,7 @@ unsigned int uart_get_divisor(struct uart_port *port, unsigned int baud);
*/
static inline unsigned long uart_fifo_timeout(struct uart_port *port)
{
- u64 fifo_timeout = (u64)READ_ONCE(port->frame_time) * port->fifosize;
+ u64 fifo_timeout = READ_ONCE(port->frame_time) * port->fifosize;
/* Add .02 seconds of slop */
fifo_timeout += 20 * NSEC_PER_MSEC;
--
js
suse labs
