* [PATCH 00/11] serial: pic32: cleanup
@ 2022-05-03 6:31 Jiri Slaby
2022-05-03 6:31 ` [PATCH 01/11] serial: pic32: remove unused items from the header Jiri Slaby
` (10 more replies)
0 siblings, 11 replies; 12+ messages in thread
From: Jiri Slaby @ 2022-05-03 6:31 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, Jiri Slaby
This series:
* ! re-enables disabled irqs in pic32_uart_startup() (*)
* removes unused/unneeded stuff
* merges a separate .h into .c
* avoids potentially broken macros
* performs other cleanups
(*) I'm not sure how this driver could work until now. Maybe there is
some coincidental enablement of irqs I don't see. A contact to people
from microchip would be highly appreciated to check with them.
Jiri Slaby (11):
serial: pic32: remove unused items from the header
serial: pic32: move header content to .c
serial: pic32: remove constants from struct pic32_sport
serial: pic32: simplify clk handling
serial: pic32: simplify pic32_sport::enable_tx_irq handling
serial: pic32: remove pic32_get_port() macro
serial: pic32: convert to_pic32_sport() to an inline
serial: pic32: don't assign pic32_sport::cts_gpio twice
serial: pic32: don't zero members of kzalloc-ated structure
serial: pic32: free up irq names correctly
serial: pic32: restore disabled irqs in pic32_uart_startup()
drivers/tty/serial/pic32_uart.c | 159 +++++++++++++++++++++++---------
drivers/tty/serial/pic32_uart.h | 125 -------------------------
2 files changed, 116 insertions(+), 168 deletions(-)
delete mode 100644 drivers/tty/serial/pic32_uart.h
--
2.36.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 01/11] serial: pic32: remove unused items from the header
2022-05-03 6:31 [PATCH 00/11] serial: pic32: cleanup Jiri Slaby
@ 2022-05-03 6:31 ` Jiri Slaby
2022-05-03 6:31 ` [PATCH 02/11] serial: pic32: move header content to .c Jiri Slaby
` (9 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Jiri Slaby @ 2022-05-03 6:31 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, Jiri Slaby
struct pic32_console_opt and its support are unused in pic32. So remove
all that.
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
drivers/tty/serial/pic32_uart.h | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/drivers/tty/serial/pic32_uart.h b/drivers/tty/serial/pic32_uart.h
index b15639cc336b..a7dba1d57236 100644
--- a/drivers/tty/serial/pic32_uart.h
+++ b/drivers/tty/serial/pic32_uart.h
@@ -20,13 +20,6 @@
#define PIC32_UART_RX 0x30
#define PIC32_UART_BRG 0x40
-struct pic32_console_opt {
- int baud;
- int parity;
- int bits;
- int flow;
-};
-
/* struct pic32_sport - pic32 serial port descriptor
* @port: uart port descriptor
* @idx: port index
@@ -44,7 +37,6 @@ struct pic32_console_opt {
**/
struct pic32_sport {
struct uart_port port;
- struct pic32_console_opt opt;
int idx;
int irq_fault;
@@ -68,7 +60,6 @@ struct pic32_sport {
};
#define to_pic32_sport(c) container_of(c, struct pic32_sport, port)
#define pic32_get_port(sport) (&sport->port)
-#define pic32_get_opt(sport) (&sport->opt)
#define tx_irq_enabled(sport) (sport->enable_tx_irq)
static inline void pic32_uart_writel(struct pic32_sport *sport,
--
2.36.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 02/11] serial: pic32: move header content to .c
2022-05-03 6:31 [PATCH 00/11] serial: pic32: cleanup Jiri Slaby
2022-05-03 6:31 ` [PATCH 01/11] serial: pic32: remove unused items from the header Jiri Slaby
@ 2022-05-03 6:31 ` Jiri Slaby
2022-05-03 6:31 ` [PATCH 03/11] serial: pic32: remove constants from struct pic32_sport Jiri Slaby
` (8 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Jiri Slaby @ 2022-05-03 6:31 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, Jiri Slaby
There is no point keeping the header content separated. So move the
content to the appropriate source file.
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
drivers/tty/serial/pic32_uart.c | 104 +++++++++++++++++++++++++++-
drivers/tty/serial/pic32_uart.h | 116 --------------------------------
2 files changed, 103 insertions(+), 117 deletions(-)
delete mode 100644 drivers/tty/serial/pic32_uart.h
diff --git a/drivers/tty/serial/pic32_uart.c b/drivers/tty/serial/pic32_uart.c
index b7a3a1b959b1..a1b8c05f3d46 100644
--- a/drivers/tty/serial/pic32_uart.c
+++ b/drivers/tty/serial/pic32_uart.c
@@ -25,13 +25,115 @@
#include <linux/delay.h>
#include <asm/mach-pic32/pic32.h>
-#include "pic32_uart.h"
/* UART name and device definitions */
#define PIC32_DEV_NAME "pic32-uart"
#define PIC32_MAX_UARTS 6
#define PIC32_SDEV_NAME "ttyPIC"
+#define PIC32_UART_DFLT_BRATE 9600
+#define PIC32_UART_TX_FIFO_DEPTH 8
+#define PIC32_UART_RX_FIFO_DEPTH 8
+
+#define PIC32_UART_MODE 0x00
+#define PIC32_UART_STA 0x10
+#define PIC32_UART_TX 0x20
+#define PIC32_UART_RX 0x30
+#define PIC32_UART_BRG 0x40
+
+/* struct pic32_sport - pic32 serial port descriptor
+ * @port: uart port descriptor
+ * @idx: port index
+ * @irq_fault: virtual fault interrupt number
+ * @irqflags_fault: flags related to fault irq
+ * @irq_fault_name: irq fault name
+ * @irq_rx: virtual rx interrupt number
+ * @irqflags_rx: flags related to rx irq
+ * @irq_rx_name: irq rx name
+ * @irq_tx: virtual tx interrupt number
+ * @irqflags_tx: : flags related to tx irq
+ * @irq_tx_name: irq tx name
+ * @cts_gpio: clear to send gpio
+ * @dev: device descriptor
+ **/
+struct pic32_sport {
+ struct uart_port port;
+ int idx;
+
+ int irq_fault;
+ int irqflags_fault;
+ const char *irq_fault_name;
+ int irq_rx;
+ int irqflags_rx;
+ const char *irq_rx_name;
+ int irq_tx;
+ int irqflags_tx;
+ const char *irq_tx_name;
+ u8 enable_tx_irq;
+
+ bool hw_flow_ctrl;
+ int cts_gpio;
+
+ int ref_clk;
+ struct clk *clk;
+
+ struct device *dev;
+};
+#define to_pic32_sport(c) container_of(c, struct pic32_sport, port)
+#define pic32_get_port(sport) (&sport->port)
+#define tx_irq_enabled(sport) (sport->enable_tx_irq)
+
+static inline void pic32_uart_writel(struct pic32_sport *sport,
+ u32 reg, u32 val)
+{
+ struct uart_port *port = pic32_get_port(sport);
+
+ __raw_writel(val, port->membase + reg);
+}
+
+static inline u32 pic32_uart_readl(struct pic32_sport *sport, u32 reg)
+{
+ struct uart_port *port = pic32_get_port(sport);
+
+ return __raw_readl(port->membase + reg);
+}
+
+/* pic32 uart mode register bits */
+#define PIC32_UART_MODE_ON BIT(15)
+#define PIC32_UART_MODE_FRZ BIT(14)
+#define PIC32_UART_MODE_SIDL BIT(13)
+#define PIC32_UART_MODE_IREN BIT(12)
+#define PIC32_UART_MODE_RTSMD BIT(11)
+#define PIC32_UART_MODE_RESV1 BIT(10)
+#define PIC32_UART_MODE_UEN1 BIT(9)
+#define PIC32_UART_MODE_UEN0 BIT(8)
+#define PIC32_UART_MODE_WAKE BIT(7)
+#define PIC32_UART_MODE_LPBK BIT(6)
+#define PIC32_UART_MODE_ABAUD BIT(5)
+#define PIC32_UART_MODE_RXINV BIT(4)
+#define PIC32_UART_MODE_BRGH BIT(3)
+#define PIC32_UART_MODE_PDSEL1 BIT(2)
+#define PIC32_UART_MODE_PDSEL0 BIT(1)
+#define PIC32_UART_MODE_STSEL BIT(0)
+
+/* pic32 uart status register bits */
+#define PIC32_UART_STA_UTXISEL1 BIT(15)
+#define PIC32_UART_STA_UTXISEL0 BIT(14)
+#define PIC32_UART_STA_UTXINV BIT(13)
+#define PIC32_UART_STA_URXEN BIT(12)
+#define PIC32_UART_STA_UTXBRK BIT(11)
+#define PIC32_UART_STA_UTXEN BIT(10)
+#define PIC32_UART_STA_UTXBF BIT(9)
+#define PIC32_UART_STA_TRMT BIT(8)
+#define PIC32_UART_STA_URXISEL1 BIT(7)
+#define PIC32_UART_STA_URXISEL0 BIT(6)
+#define PIC32_UART_STA_ADDEN BIT(5)
+#define PIC32_UART_STA_RIDLE BIT(4)
+#define PIC32_UART_STA_PERR BIT(3)
+#define PIC32_UART_STA_FERR BIT(2)
+#define PIC32_UART_STA_OERR BIT(1)
+#define PIC32_UART_STA_URXDA BIT(0)
+
/* pic32_sport pointer for console use */
static struct pic32_sport *pic32_sports[PIC32_MAX_UARTS];
diff --git a/drivers/tty/serial/pic32_uart.h b/drivers/tty/serial/pic32_uart.h
deleted file mode 100644
index a7dba1d57236..000000000000
--- a/drivers/tty/serial/pic32_uart.h
+++ /dev/null
@@ -1,116 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0+ */
-/*
- * PIC32 Integrated Serial Driver.
- *
- * Copyright (C) 2015 Microchip Technology, Inc.
- *
- * Authors:
- * Sorin-Andrei Pistirica <andrei.pistirica@microchip.com>
- */
-#ifndef __DT_PIC32_UART_H__
-#define __DT_PIC32_UART_H__
-
-#define PIC32_UART_DFLT_BRATE (9600)
-#define PIC32_UART_TX_FIFO_DEPTH (8)
-#define PIC32_UART_RX_FIFO_DEPTH (8)
-
-#define PIC32_UART_MODE 0x00
-#define PIC32_UART_STA 0x10
-#define PIC32_UART_TX 0x20
-#define PIC32_UART_RX 0x30
-#define PIC32_UART_BRG 0x40
-
-/* struct pic32_sport - pic32 serial port descriptor
- * @port: uart port descriptor
- * @idx: port index
- * @irq_fault: virtual fault interrupt number
- * @irqflags_fault: flags related to fault irq
- * @irq_fault_name: irq fault name
- * @irq_rx: virtual rx interrupt number
- * @irqflags_rx: flags related to rx irq
- * @irq_rx_name: irq rx name
- * @irq_tx: virtual tx interrupt number
- * @irqflags_tx: : flags related to tx irq
- * @irq_tx_name: irq tx name
- * @cts_gpio: clear to send gpio
- * @dev: device descriptor
- **/
-struct pic32_sport {
- struct uart_port port;
- int idx;
-
- int irq_fault;
- int irqflags_fault;
- const char *irq_fault_name;
- int irq_rx;
- int irqflags_rx;
- const char *irq_rx_name;
- int irq_tx;
- int irqflags_tx;
- const char *irq_tx_name;
- u8 enable_tx_irq;
-
- bool hw_flow_ctrl;
- int cts_gpio;
-
- int ref_clk;
- struct clk *clk;
-
- struct device *dev;
-};
-#define to_pic32_sport(c) container_of(c, struct pic32_sport, port)
-#define pic32_get_port(sport) (&sport->port)
-#define tx_irq_enabled(sport) (sport->enable_tx_irq)
-
-static inline void pic32_uart_writel(struct pic32_sport *sport,
- u32 reg, u32 val)
-{
- struct uart_port *port = pic32_get_port(sport);
-
- __raw_writel(val, port->membase + reg);
-}
-
-static inline u32 pic32_uart_readl(struct pic32_sport *sport, u32 reg)
-{
- struct uart_port *port = pic32_get_port(sport);
-
- return __raw_readl(port->membase + reg);
-}
-
-/* pic32 uart mode register bits */
-#define PIC32_UART_MODE_ON BIT(15)
-#define PIC32_UART_MODE_FRZ BIT(14)
-#define PIC32_UART_MODE_SIDL BIT(13)
-#define PIC32_UART_MODE_IREN BIT(12)
-#define PIC32_UART_MODE_RTSMD BIT(11)
-#define PIC32_UART_MODE_RESV1 BIT(10)
-#define PIC32_UART_MODE_UEN1 BIT(9)
-#define PIC32_UART_MODE_UEN0 BIT(8)
-#define PIC32_UART_MODE_WAKE BIT(7)
-#define PIC32_UART_MODE_LPBK BIT(6)
-#define PIC32_UART_MODE_ABAUD BIT(5)
-#define PIC32_UART_MODE_RXINV BIT(4)
-#define PIC32_UART_MODE_BRGH BIT(3)
-#define PIC32_UART_MODE_PDSEL1 BIT(2)
-#define PIC32_UART_MODE_PDSEL0 BIT(1)
-#define PIC32_UART_MODE_STSEL BIT(0)
-
-/* pic32 uart status register bits */
-#define PIC32_UART_STA_UTXISEL1 BIT(15)
-#define PIC32_UART_STA_UTXISEL0 BIT(14)
-#define PIC32_UART_STA_UTXINV BIT(13)
-#define PIC32_UART_STA_URXEN BIT(12)
-#define PIC32_UART_STA_UTXBRK BIT(11)
-#define PIC32_UART_STA_UTXEN BIT(10)
-#define PIC32_UART_STA_UTXBF BIT(9)
-#define PIC32_UART_STA_TRMT BIT(8)
-#define PIC32_UART_STA_URXISEL1 BIT(7)
-#define PIC32_UART_STA_URXISEL0 BIT(6)
-#define PIC32_UART_STA_ADDEN BIT(5)
-#define PIC32_UART_STA_RIDLE BIT(4)
-#define PIC32_UART_STA_PERR BIT(3)
-#define PIC32_UART_STA_FERR BIT(2)
-#define PIC32_UART_STA_OERR BIT(1)
-#define PIC32_UART_STA_URXDA BIT(0)
-
-#endif /* __DT_PIC32_UART_H__ */
--
2.36.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 03/11] serial: pic32: remove constants from struct pic32_sport
2022-05-03 6:31 [PATCH 00/11] serial: pic32: cleanup Jiri Slaby
2022-05-03 6:31 ` [PATCH 01/11] serial: pic32: remove unused items from the header Jiri Slaby
2022-05-03 6:31 ` [PATCH 02/11] serial: pic32: move header content to .c Jiri Slaby
@ 2022-05-03 6:31 ` Jiri Slaby
2022-05-03 6:31 ` [PATCH 04/11] serial: pic32: simplify clk handling Jiri Slaby
` (7 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Jiri Slaby @ 2022-05-03 6:31 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, Jiri Slaby
All the irqflags_* in struct pic32_sport are set to IRQF_NO_THREAD and
never updated. So remove pic32_sport::irqflags_* and use the flag
directly.
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
drivers/tty/serial/pic32_uart.c | 15 +++------------
1 file changed, 3 insertions(+), 12 deletions(-)
diff --git a/drivers/tty/serial/pic32_uart.c b/drivers/tty/serial/pic32_uart.c
index a1b8c05f3d46..1e8ff6004e8e 100644
--- a/drivers/tty/serial/pic32_uart.c
+++ b/drivers/tty/serial/pic32_uart.c
@@ -45,13 +45,10 @@
* @port: uart port descriptor
* @idx: port index
* @irq_fault: virtual fault interrupt number
- * @irqflags_fault: flags related to fault irq
* @irq_fault_name: irq fault name
* @irq_rx: virtual rx interrupt number
- * @irqflags_rx: flags related to rx irq
* @irq_rx_name: irq rx name
* @irq_tx: virtual tx interrupt number
- * @irqflags_tx: : flags related to tx irq
* @irq_tx_name: irq tx name
* @cts_gpio: clear to send gpio
* @dev: device descriptor
@@ -61,13 +58,10 @@ struct pic32_sport {
int idx;
int irq_fault;
- int irqflags_fault;
const char *irq_fault_name;
int irq_rx;
- int irqflags_rx;
const char *irq_rx_name;
int irq_tx;
- int irqflags_tx;
const char *irq_tx_name;
u8 enable_tx_irq;
@@ -533,7 +527,7 @@ static int pic32_uart_startup(struct uart_port *port)
}
irq_set_status_flags(sport->irq_fault, IRQ_NOAUTOEN);
ret = request_irq(sport->irq_fault, pic32_uart_fault_interrupt,
- sport->irqflags_fault, sport->irq_fault_name, port);
+ IRQF_NO_THREAD, sport->irq_fault_name, port);
if (ret) {
dev_err(port->dev, "%s: request irq(%d) err! ret:%d name:%s\n",
__func__, sport->irq_fault, ret,
@@ -551,7 +545,7 @@ static int pic32_uart_startup(struct uart_port *port)
}
irq_set_status_flags(sport->irq_rx, IRQ_NOAUTOEN);
ret = request_irq(sport->irq_rx, pic32_uart_rx_interrupt,
- sport->irqflags_rx, sport->irq_rx_name, port);
+ IRQF_NO_THREAD, sport->irq_rx_name, port);
if (ret) {
dev_err(port->dev, "%s: request irq(%d) err! ret:%d name:%s\n",
__func__, sport->irq_rx, ret,
@@ -569,7 +563,7 @@ static int pic32_uart_startup(struct uart_port *port)
}
irq_set_status_flags(sport->irq_tx, IRQ_NOAUTOEN);
ret = request_irq(sport->irq_tx, pic32_uart_tx_interrupt,
- sport->irqflags_tx, sport->irq_tx_name, port);
+ IRQF_NO_THREAD, sport->irq_tx_name, port);
if (ret) {
dev_err(port->dev, "%s: request irq(%d) err! ret:%d name:%s\n",
__func__, sport->irq_tx, ret,
@@ -918,11 +912,8 @@ static int pic32_uart_probe(struct platform_device *pdev)
sport->idx = uart_idx;
sport->irq_fault = irq_of_parse_and_map(np, 0);
- sport->irqflags_fault = IRQF_NO_THREAD;
sport->irq_rx = irq_of_parse_and_map(np, 1);
- sport->irqflags_rx = IRQF_NO_THREAD;
sport->irq_tx = irq_of_parse_and_map(np, 2);
- sport->irqflags_tx = IRQF_NO_THREAD;
sport->clk = devm_clk_get(&pdev->dev, NULL);
sport->cts_gpio = -EINVAL;
sport->dev = &pdev->dev;
--
2.36.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 04/11] serial: pic32: simplify clk handling
2022-05-03 6:31 [PATCH 00/11] serial: pic32: cleanup Jiri Slaby
` (2 preceding siblings ...)
2022-05-03 6:31 ` [PATCH 03/11] serial: pic32: remove constants from struct pic32_sport Jiri Slaby
@ 2022-05-03 6:31 ` Jiri Slaby
2022-05-03 6:31 ` [PATCH 05/11] serial: pic32: simplify pic32_sport::enable_tx_irq handling Jiri Slaby
` (6 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Jiri Slaby @ 2022-05-03 6:31 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, Jiri Slaby
struct pic32_sport::ref_clk is only set, but not read. That means we can
remove it. And when we do so, pic32_enable_clock() and
pic32_disable_clock() are simple wrappers around clk_prepare_enable()
and clk_disable_unprepare() respectively. So we can remove the former
two from the code and replace it by the latter two.
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
drivers/tty/serial/pic32_uart.c | 28 +++++-----------------------
1 file changed, 5 insertions(+), 23 deletions(-)
diff --git a/drivers/tty/serial/pic32_uart.c b/drivers/tty/serial/pic32_uart.c
index 1e8ff6004e8e..42269e96b3f8 100644
--- a/drivers/tty/serial/pic32_uart.c
+++ b/drivers/tty/serial/pic32_uart.c
@@ -68,7 +68,6 @@ struct pic32_sport {
bool hw_flow_ctrl;
int cts_gpio;
- int ref_clk;
struct clk *clk;
struct device *dev;
@@ -138,23 +137,6 @@ static inline void pic32_wait_deplete_txbuf(struct pic32_sport *sport)
udelay(1);
}
-static inline int pic32_enable_clock(struct pic32_sport *sport)
-{
- int ret = clk_prepare_enable(sport->clk);
-
- if (ret)
- return ret;
-
- sport->ref_clk++;
- return 0;
-}
-
-static inline void pic32_disable_clock(struct pic32_sport *sport)
-{
- sport->ref_clk--;
- clk_disable_unprepare(sport->clk);
-}
-
/* serial core request to check if uart tx buffer is empty */
static unsigned int pic32_uart_tx_empty(struct uart_port *port)
{
@@ -491,7 +473,7 @@ static int pic32_uart_startup(struct uart_port *port)
local_irq_save(flags);
- ret = pic32_enable_clock(sport);
+ ret = clk_prepare_enable(sport->clk);
if (ret) {
local_irq_restore(flags);
goto out_done;
@@ -611,7 +593,7 @@ static void pic32_uart_shutdown(struct uart_port *port)
spin_lock_irqsave(&port->lock, flags);
pic32_uart_dsbl_and_mask(port);
spin_unlock_irqrestore(&port->lock, flags);
- pic32_disable_clock(sport);
+ clk_disable_unprepare(sport->clk);
/* free all 3 interrupts for this UART */
free_irq(sport->irq_fault, port);
@@ -835,7 +817,7 @@ static int pic32_console_setup(struct console *co, char *options)
return -ENODEV;
port = pic32_get_port(sport);
- ret = pic32_enable_clock(sport);
+ ret = clk_prepare_enable(sport->clk);
if (ret)
return ret;
@@ -965,7 +947,7 @@ static int pic32_uart_probe(struct platform_device *pdev)
/* The peripheral clock has been enabled by console_setup,
* so disable it till the port is used.
*/
- pic32_disable_clock(sport);
+ clk_disable_unprepare(sport->clk);
}
#endif
@@ -986,7 +968,7 @@ static int pic32_uart_remove(struct platform_device *pdev)
struct pic32_sport *sport = to_pic32_sport(port);
uart_remove_one_port(&pic32_uart_driver, port);
- pic32_disable_clock(sport);
+ clk_disable_unprepare(sport->clk);
platform_set_drvdata(pdev, NULL);
pic32_sports[sport->idx] = NULL;
--
2.36.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 05/11] serial: pic32: simplify pic32_sport::enable_tx_irq handling
2022-05-03 6:31 [PATCH 00/11] serial: pic32: cleanup Jiri Slaby
` (3 preceding siblings ...)
2022-05-03 6:31 ` [PATCH 04/11] serial: pic32: simplify clk handling Jiri Slaby
@ 2022-05-03 6:31 ` Jiri Slaby
2022-05-03 6:31 ` [PATCH 06/11] serial: pic32: remove pic32_get_port() macro Jiri Slaby
` (5 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Jiri Slaby @ 2022-05-03 6:31 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, Jiri Slaby
Make it a bool, so use true+false. And remove the wrap-around macro --
i.e. access the member directly.
It makes the code more obvious.
BTW the macro did not have 'sport' in parentheses, so it was potentially
problematic wrt expansion.
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
drivers/tty/serial/pic32_uart.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/serial/pic32_uart.c b/drivers/tty/serial/pic32_uart.c
index 42269e96b3f8..a6d548d5833d 100644
--- a/drivers/tty/serial/pic32_uart.c
+++ b/drivers/tty/serial/pic32_uart.c
@@ -63,7 +63,7 @@ struct pic32_sport {
const char *irq_rx_name;
int irq_tx;
const char *irq_tx_name;
- u8 enable_tx_irq;
+ bool enable_tx_irq;
bool hw_flow_ctrl;
int cts_gpio;
@@ -74,7 +74,6 @@ struct pic32_sport {
};
#define to_pic32_sport(c) container_of(c, struct pic32_sport, port)
#define pic32_get_port(sport) (&sport->port)
-#define tx_irq_enabled(sport) (sport->enable_tx_irq)
static inline void pic32_uart_writel(struct pic32_sport *sport,
u32 reg, u32 val)
@@ -195,16 +194,16 @@ static unsigned int pic32_uart_get_mctrl(struct uart_port *port)
*/
static inline void pic32_uart_irqtxen(struct pic32_sport *sport, u8 en)
{
- if (en && !tx_irq_enabled(sport)) {
+ if (en && !sport->enable_tx_irq) {
enable_irq(sport->irq_tx);
- tx_irq_enabled(sport) = 1;
- } else if (!en && tx_irq_enabled(sport)) {
+ sport->enable_tx_irq = true;
+ } else if (!en && sport->enable_tx_irq) {
/* use disable_irq_nosync() and not disable_irq() to avoid self
* imposed deadlock by not waiting for irq handler to end,
* since this callback is called from interrupt context.
*/
disable_irq_nosync(sport->irq_tx);
- tx_irq_enabled(sport) = 0;
+ sport->enable_tx_irq = false;
}
}
@@ -497,7 +496,7 @@ static int pic32_uart_startup(struct uart_port *port)
* For each irq request_irq() is called with interrupt disabled.
* And the irq is enabled as soon as we are ready to handle them.
*/
- tx_irq_enabled(sport) = 0;
+ sport->enable_tx_irq = false;
sport->irq_fault_name = kasprintf(GFP_KERNEL, "%s%d-fault",
pic32_uart_type(port),
--
2.36.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 06/11] serial: pic32: remove pic32_get_port() macro
2022-05-03 6:31 [PATCH 00/11] serial: pic32: cleanup Jiri Slaby
` (4 preceding siblings ...)
2022-05-03 6:31 ` [PATCH 05/11] serial: pic32: simplify pic32_sport::enable_tx_irq handling Jiri Slaby
@ 2022-05-03 6:31 ` Jiri Slaby
2022-05-03 6:31 ` [PATCH 07/11] serial: pic32: convert to_pic32_sport() to an inline Jiri Slaby
` (4 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Jiri Slaby @ 2022-05-03 6:31 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, Jiri Slaby
It's just &sport->port. First, sport was not in parenthesis, so macro
expansion could be an issue. Second, it's so simple, that we can expand
the macro and make the code really straightforward.
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
drivers/tty/serial/pic32_uart.c | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/drivers/tty/serial/pic32_uart.c b/drivers/tty/serial/pic32_uart.c
index a6d548d5833d..32a86b12f203 100644
--- a/drivers/tty/serial/pic32_uart.c
+++ b/drivers/tty/serial/pic32_uart.c
@@ -73,21 +73,16 @@ struct pic32_sport {
struct device *dev;
};
#define to_pic32_sport(c) container_of(c, struct pic32_sport, port)
-#define pic32_get_port(sport) (&sport->port)
static inline void pic32_uart_writel(struct pic32_sport *sport,
u32 reg, u32 val)
{
- struct uart_port *port = pic32_get_port(sport);
-
- __raw_writel(val, port->membase + reg);
+ __raw_writel(val, sport->port.membase + reg);
}
static inline u32 pic32_uart_readl(struct pic32_sport *sport, u32 reg)
{
- struct uart_port *port = pic32_get_port(sport);
-
- return __raw_readl(port->membase + reg);
+ return __raw_readl(sport->port.membase + reg);
}
/* pic32 uart mode register bits */
@@ -789,10 +784,9 @@ static void pic32_console_write(struct console *co, const char *s,
unsigned int count)
{
struct pic32_sport *sport = pic32_sports[co->index];
- struct uart_port *port = pic32_get_port(sport);
/* call uart helper to deal with \r\n */
- uart_console_write(port, s, count, pic32_console_putchar);
+ uart_console_write(&sport->port, s, count, pic32_console_putchar);
}
/* console core request to setup given console, find matching uart
@@ -801,7 +795,6 @@ static void pic32_console_write(struct console *co, const char *s,
static int pic32_console_setup(struct console *co, char *options)
{
struct pic32_sport *sport;
- struct uart_port *port = NULL;
int baud = 115200;
int bits = 8;
int parity = 'n';
@@ -814,7 +807,6 @@ static int pic32_console_setup(struct console *co, char *options)
sport = pic32_sports[co->index];
if (!sport)
return -ENODEV;
- port = pic32_get_port(sport);
ret = clk_prepare_enable(sport->clk);
if (ret)
@@ -823,7 +815,7 @@ static int pic32_console_setup(struct console *co, char *options)
if (options)
uart_parse_options(options, &baud, &parity, &bits, &flow);
- return uart_set_options(port, co, baud, parity, bits, flow);
+ return uart_set_options(&sport->port, co, baud, parity, bits, flow);
}
static struct uart_driver pic32_uart_driver;
--
2.36.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 07/11] serial: pic32: convert to_pic32_sport() to an inline
2022-05-03 6:31 [PATCH 00/11] serial: pic32: cleanup Jiri Slaby
` (5 preceding siblings ...)
2022-05-03 6:31 ` [PATCH 06/11] serial: pic32: remove pic32_get_port() macro Jiri Slaby
@ 2022-05-03 6:31 ` Jiri Slaby
2022-05-03 6:31 ` [PATCH 08/11] serial: pic32: don't assign pic32_sport::cts_gpio twice Jiri Slaby
` (3 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Jiri Slaby @ 2022-05-03 6:31 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, Jiri Slaby
'c' is not in wrapped in parentheses in the to_pic32_sport() macro, so
it might be problematic wrt macro expansion. Using an inline is always
safer in these cases. Both type-wise and macro-expansion-wise. So switch
the macro to an inline.
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
drivers/tty/serial/pic32_uart.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/pic32_uart.c b/drivers/tty/serial/pic32_uart.c
index 32a86b12f203..c3b4fd0b5b76 100644
--- a/drivers/tty/serial/pic32_uart.c
+++ b/drivers/tty/serial/pic32_uart.c
@@ -72,7 +72,11 @@ struct pic32_sport {
struct device *dev;
};
-#define to_pic32_sport(c) container_of(c, struct pic32_sport, port)
+
+static inline struct pic32_sport *to_pic32_sport(struct uart_port *port)
+{
+ return container_of(port, struct pic32_sport, port);
+}
static inline void pic32_uart_writel(struct pic32_sport *sport,
u32 reg, u32 val)
--
2.36.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 08/11] serial: pic32: don't assign pic32_sport::cts_gpio twice
2022-05-03 6:31 [PATCH 00/11] serial: pic32: cleanup Jiri Slaby
` (6 preceding siblings ...)
2022-05-03 6:31 ` [PATCH 07/11] serial: pic32: convert to_pic32_sport() to an inline Jiri Slaby
@ 2022-05-03 6:31 ` Jiri Slaby
2022-05-03 6:31 ` [PATCH 09/11] serial: pic32: don't zero members of kzalloc-ated structure Jiri Slaby
` (2 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Jiri Slaby @ 2022-05-03 6:31 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, Jiri Slaby
sport->cts_gpio is first assigned -EINVAL and few lines below using
of_get_named_gpio(). Remove the first (useless) assignment.
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
drivers/tty/serial/pic32_uart.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/tty/serial/pic32_uart.c b/drivers/tty/serial/pic32_uart.c
index c3b4fd0b5b76..08e79d7f34f7 100644
--- a/drivers/tty/serial/pic32_uart.c
+++ b/drivers/tty/serial/pic32_uart.c
@@ -892,7 +892,6 @@ static int pic32_uart_probe(struct platform_device *pdev)
sport->irq_rx = irq_of_parse_and_map(np, 1);
sport->irq_tx = irq_of_parse_and_map(np, 2);
sport->clk = devm_clk_get(&pdev->dev, NULL);
- sport->cts_gpio = -EINVAL;
sport->dev = &pdev->dev;
/* Hardware flow control: gpios
--
2.36.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 09/11] serial: pic32: don't zero members of kzalloc-ated structure
2022-05-03 6:31 [PATCH 00/11] serial: pic32: cleanup Jiri Slaby
` (7 preceding siblings ...)
2022-05-03 6:31 ` [PATCH 08/11] serial: pic32: don't assign pic32_sport::cts_gpio twice Jiri Slaby
@ 2022-05-03 6:31 ` Jiri Slaby
2022-05-03 6:31 ` [PATCH 10/11] serial: pic32: free up irq names correctly Jiri Slaby
2022-05-03 6:31 ` [PATCH 11/11] serial: pic32: restore disabled irqs in pic32_uart_startup() Jiri Slaby
10 siblings, 0 replies; 12+ messages in thread
From: Jiri Slaby @ 2022-05-03 6:31 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, Jiri Slaby
struct pic32_sport (sport) has just been kzallocated. So there is no
need to zero its member (sport->port) now.
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
drivers/tty/serial/pic32_uart.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/tty/serial/pic32_uart.c b/drivers/tty/serial/pic32_uart.c
index 08e79d7f34f7..990603fe8a8d 100644
--- a/drivers/tty/serial/pic32_uart.c
+++ b/drivers/tty/serial/pic32_uart.c
@@ -919,7 +919,6 @@ static int pic32_uart_probe(struct platform_device *pdev)
pic32_sports[uart_idx] = sport;
port = &sport->port;
- memset(port, 0, sizeof(*port));
port->iotype = UPIO_MEM;
port->mapbase = res_mem->start;
port->ops = &pic32_uart_ops;
--
2.36.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 10/11] serial: pic32: free up irq names correctly
2022-05-03 6:31 [PATCH 00/11] serial: pic32: cleanup Jiri Slaby
` (8 preceding siblings ...)
2022-05-03 6:31 ` [PATCH 09/11] serial: pic32: don't zero members of kzalloc-ated structure Jiri Slaby
@ 2022-05-03 6:31 ` Jiri Slaby
2022-05-03 6:31 ` [PATCH 11/11] serial: pic32: restore disabled irqs in pic32_uart_startup() Jiri Slaby
10 siblings, 0 replies; 12+ messages in thread
From: Jiri Slaby @ 2022-05-03 6:31 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, Jiri Slaby
struct pic32_sport contains built-up names for irqs. These are freed
only in error path of pic32_uart_startup(). And even there, the freeing
happens before free_irq().
So fix this by:
* moving frees after free_irq(), and
* add frees to pic32_uart_shutdown() -- the opposite of
pic32_uart_startup().
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
drivers/tty/serial/pic32_uart.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/pic32_uart.c b/drivers/tty/serial/pic32_uart.c
index 990603fe8a8d..c5584628f8c4 100644
--- a/drivers/tty/serial/pic32_uart.c
+++ b/drivers/tty/serial/pic32_uart.c
@@ -569,14 +569,14 @@ static int pic32_uart_startup(struct uart_port *port)
return 0;
out_t:
- kfree(sport->irq_tx_name);
free_irq(sport->irq_tx, port);
+ kfree(sport->irq_tx_name);
out_r:
- kfree(sport->irq_rx_name);
free_irq(sport->irq_rx, port);
+ kfree(sport->irq_rx_name);
out_f:
- kfree(sport->irq_fault_name);
free_irq(sport->irq_fault, port);
+ kfree(sport->irq_fault_name);
out_done:
return ret;
}
@@ -595,8 +595,11 @@ static void pic32_uart_shutdown(struct uart_port *port)
/* free all 3 interrupts for this UART */
free_irq(sport->irq_fault, port);
+ kfree(sport->irq_fault_name);
free_irq(sport->irq_tx, port);
+ kfree(sport->irq_tx_name);
free_irq(sport->irq_rx, port);
+ kfree(sport->irq_rx_name);
}
/* serial core request to change current uart setting */
--
2.36.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 11/11] serial: pic32: restore disabled irqs in pic32_uart_startup()
2022-05-03 6:31 [PATCH 00/11] serial: pic32: cleanup Jiri Slaby
` (9 preceding siblings ...)
2022-05-03 6:31 ` [PATCH 10/11] serial: pic32: free up irq names correctly Jiri Slaby
@ 2022-05-03 6:31 ` Jiri Slaby
10 siblings, 0 replies; 12+ messages in thread
From: Jiri Slaby @ 2022-05-03 6:31 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, Jiri Slaby, Andrei Pistirica
pic32_uart_startup() disables interrupts by local_irq_save(). But the
function never enables them. The serial core only holds a mutex, so irqs
are not restored.
So how could this driver work? This irq handling was already present in
the driver's initial commit 157b9394709ed (serial: pic32_uart: Add PIC32
UART driver).
So is it a candidate for removal? Anyone has a contact to the author:
Andrei Pistirica (I believe the one below -- @microchip.com -- will
bounce)? Or to someone else @microchip.com?
Cc: Andrei Pistirica <andrei.pistirica@microchip.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
drivers/tty/serial/pic32_uart.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/tty/serial/pic32_uart.c b/drivers/tty/serial/pic32_uart.c
index c5584628f8c4..b399aac530fe 100644
--- a/drivers/tty/serial/pic32_uart.c
+++ b/drivers/tty/serial/pic32_uart.c
@@ -564,6 +564,8 @@ static int pic32_uart_startup(struct uart_port *port)
/* enable all interrupts and eanable uart */
pic32_uart_en_and_unmask(port);
+ local_irq_restore(flags);
+
enable_irq(sport->irq_rx);
return 0;
--
2.36.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-05-03 6:32 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-03 6:31 [PATCH 00/11] serial: pic32: cleanup Jiri Slaby
2022-05-03 6:31 ` [PATCH 01/11] serial: pic32: remove unused items from the header Jiri Slaby
2022-05-03 6:31 ` [PATCH 02/11] serial: pic32: move header content to .c Jiri Slaby
2022-05-03 6:31 ` [PATCH 03/11] serial: pic32: remove constants from struct pic32_sport Jiri Slaby
2022-05-03 6:31 ` [PATCH 04/11] serial: pic32: simplify clk handling Jiri Slaby
2022-05-03 6:31 ` [PATCH 05/11] serial: pic32: simplify pic32_sport::enable_tx_irq handling Jiri Slaby
2022-05-03 6:31 ` [PATCH 06/11] serial: pic32: remove pic32_get_port() macro Jiri Slaby
2022-05-03 6:31 ` [PATCH 07/11] serial: pic32: convert to_pic32_sport() to an inline Jiri Slaby
2022-05-03 6:31 ` [PATCH 08/11] serial: pic32: don't assign pic32_sport::cts_gpio twice Jiri Slaby
2022-05-03 6:31 ` [PATCH 09/11] serial: pic32: don't zero members of kzalloc-ated structure Jiri Slaby
2022-05-03 6:31 ` [PATCH 10/11] serial: pic32: free up irq names correctly Jiri Slaby
2022-05-03 6:31 ` [PATCH 11/11] serial: pic32: restore disabled irqs in pic32_uart_startup() Jiri Slaby
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).