From 2227f4c0afed9fef940ace67d1482417eb876316 Mon Sep 17 00:00:00 2001 From: Sam Protsenko Date: Tue, 7 Nov 2023 11:34:17 -0600 Subject: serial: s5p: Fix clk_get_by_index() error code check clk_get_by_index() returns negative number on error. Assigning it to unsigned int makes the subsequent "ret < 0" check always false, leading in turn to possible unhandled errors. Change 'ret' variable type to signed int so the code checks and handles clk_get_by_index() return code properly. Signed-off-by: Sam Protsenko Fixes: cf75cdf96ef2 ("serial: s5p: use clock api to get clock rate") Signed-off-by: Minkyu Kang --- drivers/serial/serial_s5p.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/serial') diff --git a/drivers/serial/serial_s5p.c b/drivers/serial/serial_s5p.c index 7aeb8c0..fe52580 100644 --- a/drivers/serial/serial_s5p.c +++ b/drivers/serial/serial_s5p.c @@ -118,7 +118,7 @@ int s5p_serial_setbrg(struct udevice *dev, int baudrate) #if IS_ENABLED(CONFIG_CLK_EXYNOS) || IS_ENABLED(CONFIG_ARCH_APPLE) struct clk clk; - u32 ret; + int ret; ret = clk_get_by_index(dev, 1, &clk); if (ret < 0) -- cgit v1.1 From a0615ffc99a5c5e23b9fe5a8116fc265483be2bd Mon Sep 17 00:00:00 2001 From: Sam Protsenko Date: Tue, 7 Nov 2023 13:05:58 -0600 Subject: serial: s5p: Remove common.h inclusion It's not really needed here anymore. Remove it, as common.h is going away at some point. Signed-off-by: Sam Protsenko Reviewed-by: Simon Glass Signed-off-by: Minkyu Kang --- drivers/serial/serial_s5p.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/serial') diff --git a/drivers/serial/serial_s5p.c b/drivers/serial/serial_s5p.c index fe52580..1772155 100644 --- a/drivers/serial/serial_s5p.c +++ b/drivers/serial/serial_s5p.c @@ -7,7 +7,6 @@ * based on drivers/serial/s3c64xx.c */ -#include #include #include #include -- cgit v1.1 From 5ad21de6bae0a6d51d4b0ff8eedfb795ed2d4581 Mon Sep 17 00:00:00 2001 From: Sam Protsenko Date: Tue, 7 Nov 2023 13:05:59 -0600 Subject: serial: s5p: Use livetree API to get "id" property Use dev_read_u8_default() instead of fdtdec_get_int() to read the "id" property from device tree, as suggested in [1]. dev_* API is already used in this driver, so there is no reason to stick to fdtdec_* API. This also fixes checkpatch warning: WARNING: Use the livetree API (dev_read_...) [1] doc/develop/driver-model/livetree.rst Signed-off-by: Sam Protsenko Reviewed-by: Simon Glass Signed-off-by: Minkyu Kang --- drivers/serial/serial_s5p.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'drivers/serial') diff --git a/drivers/serial/serial_s5p.c b/drivers/serial/serial_s5p.c index 1772155..c57bdd0 100644 --- a/drivers/serial/serial_s5p.c +++ b/drivers/serial/serial_s5p.c @@ -20,8 +20,6 @@ #include #include -DECLARE_GLOBAL_DATA_PTR; - enum { PORT_S5P = 0, PORT_S5L @@ -220,8 +218,7 @@ static int s5p_serial_of_to_plat(struct udevice *dev) plat->reg = (struct s5p_uart *)addr; plat->reg_width = dev_read_u32_default(dev, "reg-io-width", 1); - plat->port_id = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev), - "id", dev_seq(dev)); + plat->port_id = dev_read_u8_default(dev, "id", dev_seq(dev)); if (port_type == PORT_S5L) { plat->rx_fifo_count_shift = S5L_RX_FIFO_COUNT_SHIFT; -- cgit v1.1 From e79f630dbf67a402aff7b34f743064eeb2569ea1 Mon Sep 17 00:00:00 2001 From: Sam Protsenko Date: Tue, 7 Nov 2023 13:06:00 -0600 Subject: serial: s5p: Use named constants for register values Get rid of magic numbers in s5p_serial_init() when writing to UART registers. While at it, use BIT() macro for existing constants when appropriate. No functional change. Signed-off-by: Sam Protsenko Reviewed-by: Simon Glass Signed-off-by: Minkyu Kang --- drivers/serial/serial_s5p.c | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) (limited to 'drivers/serial') diff --git a/drivers/serial/serial_s5p.c b/drivers/serial/serial_s5p.c index c57bdd0..6d316cc 100644 --- a/drivers/serial/serial_s5p.c +++ b/drivers/serial/serial_s5p.c @@ -25,19 +25,28 @@ enum { PORT_S5L }; +#define UFCON_FIFO_EN BIT(0) +#define UFCON_RX_FIFO_RESET BIT(1) +#define UMCON_RESET_VAL 0x0 +#define ULCON_WORD_8_BIT 0x3 +#define UCON_RX_IRQ_OR_POLLING BIT(0) +#define UCON_TX_IRQ_OR_POLLING BIT(2) +#define UCON_RX_ERR_IRQ_EN BIT(6) +#define UCON_TX_IRQ_LEVEL BIT(9) + #define S5L_RX_FIFO_COUNT_SHIFT 0 #define S5L_RX_FIFO_COUNT_MASK (0xf << S5L_RX_FIFO_COUNT_SHIFT) -#define S5L_RX_FIFO_FULL (1 << 8) +#define S5L_RX_FIFO_FULL BIT(8) #define S5L_TX_FIFO_COUNT_SHIFT 4 #define S5L_TX_FIFO_COUNT_MASK (0xf << S5L_TX_FIFO_COUNT_SHIFT) -#define S5L_TX_FIFO_FULL (1 << 9) +#define S5L_TX_FIFO_FULL BIT(9) #define S5P_RX_FIFO_COUNT_SHIFT 0 #define S5P_RX_FIFO_COUNT_MASK (0xff << S5P_RX_FIFO_COUNT_SHIFT) -#define S5P_RX_FIFO_FULL (1 << 8) +#define S5P_RX_FIFO_FULL BIT(8) #define S5P_TX_FIFO_COUNT_SHIFT 16 #define S5P_TX_FIFO_COUNT_MASK (0xff << S5P_TX_FIFO_COUNT_SHIFT) -#define S5P_TX_FIFO_FULL (1 << 24) +#define S5P_TX_FIFO_FULL BIT(24) /* Information about a serial port */ struct s5p_serial_plat { @@ -80,13 +89,15 @@ static const int udivslot[] = { static void __maybe_unused s5p_serial_init(struct s5p_uart *uart) { - /* enable FIFOs, auto clear Rx FIFO */ - writel(0x3, &uart->ufcon); - writel(0, &uart->umcon); - /* 8N1 */ - writel(0x3, &uart->ulcon); + /* Enable FIFOs, auto clear Rx FIFO */ + writel(UFCON_FIFO_EN | UFCON_RX_FIFO_RESET, &uart->ufcon); + /* No auto flow control, disable nRTS signal */ + writel(UMCON_RESET_VAL, &uart->umcon); + /* 8N1, no parity bit */ + writel(ULCON_WORD_8_BIT, &uart->ulcon); /* No interrupts, no DMA, pure polling */ - writel(0x245, &uart->ucon); + writel(UCON_RX_IRQ_OR_POLLING | UCON_TX_IRQ_OR_POLLING | + UCON_RX_ERR_IRQ_EN | UCON_TX_IRQ_LEVEL, &uart->ucon); } static void __maybe_unused s5p_serial_baud(struct s5p_uart *uart, u8 reg_width, -- cgit v1.1 From a627f2802a71afe7fec28d2a9b514dc37f1b8f20 Mon Sep 17 00:00:00 2001 From: Sam Protsenko Date: Tue, 7 Nov 2023 13:06:01 -0600 Subject: serial: s5p: Improve coding style Just some minor style fixes. No functional change. Signed-off-by: Sam Protsenko Reviewed-by: Simon Glass Signed-off-by: Minkyu Kang --- drivers/serial/serial_s5p.c | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) (limited to 'drivers/serial') diff --git a/drivers/serial/serial_s5p.c b/drivers/serial/serial_s5p.c index 6d316cc..c24d9bc 100644 --- a/drivers/serial/serial_s5p.c +++ b/drivers/serial/serial_s5p.c @@ -50,9 +50,9 @@ enum { /* Information about a serial port */ struct s5p_serial_plat { - struct s5p_uart *reg; /* address of registers in physical memory */ - u8 reg_width; /* register width */ - u8 port_id; /* uart port number */ + struct s5p_uart *reg; /* address of registers in physical memory */ + u8 reg_width; /* register width */ + u8 port_id; /* uart port number */ u8 rx_fifo_count_shift; u8 tx_fifo_count_shift; u32 rx_fifo_count_mask; @@ -65,7 +65,7 @@ struct s5p_serial_plat { * The coefficient, used to calculate the baudrate on S5P UARTs is * calculated as * C = UBRDIV * 16 + number_of_set_bits_in_UDIVSLOT - * however, section 31.6.11 of the datasheet doesn't recomment using 1 for 1, + * however, section 31.6.11 of the datasheet doesn't recommend using 1 for 1, * 3 for 2, ... (2^n - 1) for n, instead, they suggest using these constants: */ static const int udivslot[] = { @@ -251,10 +251,10 @@ static int s5p_serial_of_to_plat(struct udevice *dev) } static const struct dm_serial_ops s5p_serial_ops = { - .putc = s5p_serial_putc, - .pending = s5p_serial_pending, - .getc = s5p_serial_getc, - .setbrg = s5p_serial_setbrg, + .putc = s5p_serial_putc, + .pending = s5p_serial_pending, + .getc = s5p_serial_getc, + .setbrg = s5p_serial_setbrg, }; static const struct udevice_id s5p_serial_ids[] = { @@ -264,13 +264,13 @@ static const struct udevice_id s5p_serial_ids[] = { }; U_BOOT_DRIVER(serial_s5p) = { - .name = "serial_s5p", - .id = UCLASS_SERIAL, - .of_match = s5p_serial_ids, - .of_to_plat = s5p_serial_of_to_plat, + .name = "serial_s5p", + .id = UCLASS_SERIAL, + .of_match = s5p_serial_ids, + .of_to_plat = s5p_serial_of_to_plat, .plat_auto = sizeof(struct s5p_serial_plat), - .probe = s5p_serial_probe, - .ops = &s5p_serial_ops, + .probe = s5p_serial_probe, + .ops = &s5p_serial_ops, }; #endif @@ -298,10 +298,12 @@ static inline void _debug_uart_putc(int ch) struct s5p_uart *uart = (struct s5p_uart *)CONFIG_VAL(DEBUG_UART_BASE); #if IS_ENABLED(CONFIG_ARCH_APPLE) - while (readl(&uart->ufstat) & S5L_TX_FIFO_FULL); + while (readl(&uart->ufstat) & S5L_TX_FIFO_FULL) + ; writel(ch, &uart->utxh); #else - while (readl(&uart->ufstat) & S5P_TX_FIFO_FULL); + while (readl(&uart->ufstat) & S5P_TX_FIFO_FULL) + ; writeb(ch, &uart->utxh); #endif } -- cgit v1.1 From 33e7ca5a9b6a0dc1c894c14fd8e29b816f9a6f55 Mon Sep 17 00:00:00 2001 From: Sam Protsenko Date: Tue, 7 Nov 2023 14:13:49 -0600 Subject: serial: s5p: Use dev_read_addr_ptr() to get base address As the address read from device tree is being cast to a pointer, it's better to use dev_read_addr_ptr() API for getting that address. The more detailed explanation can be found in commit a12a73b66476 ("drivers: use dev_read_addr_ptr when cast to pointer"). Signed-off-by: Sam Protsenko Reviewed-by: Simon Glass Signed-off-by: Minkyu Kang --- drivers/serial/serial_s5p.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'drivers/serial') diff --git a/drivers/serial/serial_s5p.c b/drivers/serial/serial_s5p.c index c24d9bc..7d04dcf 100644 --- a/drivers/serial/serial_s5p.c +++ b/drivers/serial/serial_s5p.c @@ -221,13 +221,11 @@ static int s5p_serial_of_to_plat(struct udevice *dev) { struct s5p_serial_plat *plat = dev_get_plat(dev); const ulong port_type = dev_get_driver_data(dev); - fdt_addr_t addr; - addr = dev_read_addr(dev); - if (addr == FDT_ADDR_T_NONE) + plat->reg = dev_read_addr_ptr(dev); + if (!plat->reg) return -EINVAL; - plat->reg = (struct s5p_uart *)addr; plat->reg_width = dev_read_u32_default(dev, "reg-io-width", 1); plat->port_id = dev_read_u8_default(dev, "id", dev_seq(dev)); -- cgit v1.1