[PATCH v2 0/2] tty: serial: samsung_tty: simple cleanups

[PATCH v2 0/2] tty: serial: samsung_tty: simple cleanups

[André Draszik]

While looking through the samsung tty driver, I've spotted a few things that
can be simplified by removing unused function arguments and by avoiding some
duplicated variables and casting.

There are no functional changes here.

Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
Changes in v2:
- fix -Wdiscarded-qualifiers warnings
- collect tags
- Link to v1: https://lore.kernel.org/r/20240806-samsung-tty-cleanup-v1-0-a68d3abf31fe@linaro.org

---
André Draszik (2):
      tty: serial: samsung_tty: drop unused argument to irq handlers
      tty: serial: samsung_tty: cast the interrupt's void *id just once

 drivers/tty/serial/samsung_tty.c | 33 ++++++++++++++-------------------
 1 file changed, 14 insertions(+), 19 deletions(-)
---
base-commit: 1e391b34f6aa043c7afa40a2103163a0ef06d179
change-id: 20240806-samsung-tty-cleanup-ffae1515a284

Best regards,
-- 
André Draszik <andre.draszik@linaro.org>

[PATCH v2 1/2] tty: serial: samsung_tty: drop unused argument to irq handlers

[André Draszik]

The 'irq' argument is not used in any of the callees, we can just drop
it and simplify the code.

No functional changes.

Reviewed-by: Tudor Ambarus <tudor.ambarus@linaro.org>
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
 drivers/tty/serial/samsung_tty.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
index dc35eb77d2ef..1c6d0ffe5649 100644
--- a/drivers/tty/serial/samsung_tty.c
+++ b/drivers/tty/serial/samsung_tty.c
@@ -855,7 +855,7 @@ static irqreturn_t s3c24xx_serial_rx_chars_pio(void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static irqreturn_t s3c24xx_serial_rx_irq(int irq, void *dev_id)
+static irqreturn_t s3c24xx_serial_rx_irq(void *dev_id)
 {
 	struct s3c24xx_uart_port *ourport = dev_id;
 
@@ -928,7 +928,7 @@ static void s3c24xx_serial_tx_chars(struct s3c24xx_uart_port *ourport)
 		s3c24xx_serial_stop_tx(port);
 }
 
-static irqreturn_t s3c24xx_serial_tx_irq(int irq, void *id)
+static irqreturn_t s3c24xx_serial_tx_irq(void *id)
 {
 	struct s3c24xx_uart_port *ourport = id;
 	struct uart_port *port = &ourport->port;
@@ -950,11 +950,11 @@ static irqreturn_t s3c64xx_serial_handle_irq(int irq, void *id)
 	irqreturn_t ret = IRQ_HANDLED;
 
 	if (pend & S3C64XX_UINTM_RXD_MSK) {
-		ret = s3c24xx_serial_rx_irq(irq, id);
+		ret = s3c24xx_serial_rx_irq(id);
 		wr_regl(port, S3C64XX_UINTP, S3C64XX_UINTM_RXD_MSK);
 	}
 	if (pend & S3C64XX_UINTM_TXD_MSK) {
-		ret = s3c24xx_serial_tx_irq(irq, id);
+		ret = s3c24xx_serial_tx_irq(id);
 		wr_regl(port, S3C64XX_UINTP, S3C64XX_UINTM_TXD_MSK);
 	}
 	return ret;
@@ -971,11 +971,11 @@ static irqreturn_t apple_serial_handle_irq(int irq, void *id)
 	if (pend & (APPLE_S5L_UTRSTAT_RXTHRESH | APPLE_S5L_UTRSTAT_RXTO)) {
 		wr_regl(port, S3C2410_UTRSTAT,
 			APPLE_S5L_UTRSTAT_RXTHRESH | APPLE_S5L_UTRSTAT_RXTO);
-		ret = s3c24xx_serial_rx_irq(irq, id);
+		ret = s3c24xx_serial_rx_irq(id);
 	}
 	if (pend & APPLE_S5L_UTRSTAT_TXTHRESH) {
 		wr_regl(port, S3C2410_UTRSTAT, APPLE_S5L_UTRSTAT_TXTHRESH);
-		ret = s3c24xx_serial_tx_irq(irq, id);
+		ret = s3c24xx_serial_tx_irq(id);
 	}
 
 	return ret;

-- 
2.46.0.rc2.264.g509ed76dc8-goog

[PATCH v2 2/2] tty: serial: samsung_tty: cast the interrupt's void *id just once

[André Draszik]

The interrupt handler routines and helpers are casting the 'void *'
pointer to 'struct exynos_uart_port *' all over the place.

There is no need for that, we can do the casting once and keep passing
the 'struct exynos_uart_port *', simplifying the code and saving a few
lines of code.

No functional changes.

Reviewed-by: Tudor Ambarus <tudor.ambarus@linaro.org>
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
v2: fix -Wdiscarded-qualifiers, sorry
---
 drivers/tty/serial/samsung_tty.c | 33 ++++++++++++++-------------------
 1 file changed, 14 insertions(+), 19 deletions(-)

diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
index 1c6d0ffe5649..951e16c87471 100644
--- a/drivers/tty/serial/samsung_tty.c
+++ b/drivers/tty/serial/samsung_tty.c
@@ -707,9 +707,8 @@ static void enable_rx_pio(struct s3c24xx_uart_port *ourport)
 
 static void s3c24xx_serial_rx_drain_fifo(struct s3c24xx_uart_port *ourport);
 
-static irqreturn_t s3c24xx_serial_rx_chars_dma(void *dev_id)
+static irqreturn_t s3c24xx_serial_rx_chars_dma(struct s3c24xx_uart_port *ourport)
 {
-	struct s3c24xx_uart_port *ourport = dev_id;
 	struct uart_port *port = &ourport->port;
 	struct s3c24xx_uart_dma *dma = ourport->dma;
 	struct tty_struct *tty = tty_port_tty_get(&ourport->port.state->port);
@@ -843,9 +842,8 @@ static void s3c24xx_serial_rx_drain_fifo(struct s3c24xx_uart_port *ourport)
 	tty_flip_buffer_push(&port->state->port);
 }
 
-static irqreturn_t s3c24xx_serial_rx_chars_pio(void *dev_id)
+static irqreturn_t s3c24xx_serial_rx_chars_pio(struct s3c24xx_uart_port *ourport)
 {
-	struct s3c24xx_uart_port *ourport = dev_id;
 	struct uart_port *port = &ourport->port;
 
 	uart_port_lock(port);
@@ -855,13 +853,11 @@ static irqreturn_t s3c24xx_serial_rx_chars_pio(void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static irqreturn_t s3c24xx_serial_rx_irq(void *dev_id)
+static irqreturn_t s3c24xx_serial_rx_irq(struct s3c24xx_uart_port *ourport)
 {
-	struct s3c24xx_uart_port *ourport = dev_id;
-
 	if (ourport->dma && ourport->dma->rx_chan)
-		return s3c24xx_serial_rx_chars_dma(dev_id);
-	return s3c24xx_serial_rx_chars_pio(dev_id);
+		return s3c24xx_serial_rx_chars_dma(ourport);
+	return s3c24xx_serial_rx_chars_pio(ourport);
 }
 
 static void s3c24xx_serial_tx_chars(struct s3c24xx_uart_port *ourport)
@@ -928,9 +924,8 @@ static void s3c24xx_serial_tx_chars(struct s3c24xx_uart_port *ourport)
 		s3c24xx_serial_stop_tx(port);
 }
 
-static irqreturn_t s3c24xx_serial_tx_irq(void *id)
+static irqreturn_t s3c24xx_serial_tx_irq(struct s3c24xx_uart_port *ourport)
 {
-	struct s3c24xx_uart_port *ourport = id;
 	struct uart_port *port = &ourport->port;
 
 	uart_port_lock(port);
@@ -944,17 +939,17 @@ static irqreturn_t s3c24xx_serial_tx_irq(void *id)
 /* interrupt handler for s3c64xx and later SoC's.*/
 static irqreturn_t s3c64xx_serial_handle_irq(int irq, void *id)
 {
-	const struct s3c24xx_uart_port *ourport = id;
-	const struct uart_port *port = &ourport->port;
+	struct s3c24xx_uart_port *ourport = id;
+	struct uart_port *port = &ourport->port;
 	u32 pend = rd_regl(port, S3C64XX_UINTP);
 	irqreturn_t ret = IRQ_HANDLED;
 
 	if (pend & S3C64XX_UINTM_RXD_MSK) {
-		ret = s3c24xx_serial_rx_irq(id);
+		ret = s3c24xx_serial_rx_irq(ourport);
 		wr_regl(port, S3C64XX_UINTP, S3C64XX_UINTM_RXD_MSK);
 	}
 	if (pend & S3C64XX_UINTM_TXD_MSK) {
-		ret = s3c24xx_serial_tx_irq(id);
+		ret = s3c24xx_serial_tx_irq(ourport);
 		wr_regl(port, S3C64XX_UINTP, S3C64XX_UINTM_TXD_MSK);
 	}
 	return ret;
@@ -963,19 +958,19 @@ static irqreturn_t s3c64xx_serial_handle_irq(int irq, void *id)
 /* interrupt handler for Apple SoC's.*/
 static irqreturn_t apple_serial_handle_irq(int irq, void *id)
 {
-	const struct s3c24xx_uart_port *ourport = id;
-	const struct uart_port *port = &ourport->port;
+	struct s3c24xx_uart_port *ourport = id;
+	struct uart_port *port = &ourport->port;
 	u32 pend = rd_regl(port, S3C2410_UTRSTAT);
 	irqreturn_t ret = IRQ_NONE;
 
 	if (pend & (APPLE_S5L_UTRSTAT_RXTHRESH | APPLE_S5L_UTRSTAT_RXTO)) {
 		wr_regl(port, S3C2410_UTRSTAT,
 			APPLE_S5L_UTRSTAT_RXTHRESH | APPLE_S5L_UTRSTAT_RXTO);
-		ret = s3c24xx_serial_rx_irq(id);
+		ret = s3c24xx_serial_rx_irq(ourport);
 	}
 	if (pend & APPLE_S5L_UTRSTAT_TXTHRESH) {
 		wr_regl(port, S3C2410_UTRSTAT, APPLE_S5L_UTRSTAT_TXTHRESH);
-		ret = s3c24xx_serial_tx_irq(id);
+		ret = s3c24xx_serial_tx_irq(ourport);
 	}
 
 	return ret;

-- 
2.46.0.rc2.264.g509ed76dc8-goog

Re: [PATCH v2 2/2] tty: serial: samsung_tty: cast the interrupt's void *id just once

[Jiri Slaby]

On 07. 08. 24, 13:58, André Draszik wrote:
> The interrupt handler routines and helpers are casting the 'void *'
> pointer to 'struct exynos_uart_port *' all over the place.
> 
> There is no need for that, we can do the casting once and keep passing
> the 'struct exynos_uart_port *', simplifying the code and saving a few
> lines of code.
> 
> No functional changes.
...
> @@ -944,17 +939,17 @@ static irqreturn_t s3c24xx_serial_tx_irq(void *id)
>   /* interrupt handler for s3c64xx and later SoC's.*/
>   static irqreturn_t s3c64xx_serial_handle_irq(int irq, void *id)
>   {
> -	const struct s3c24xx_uart_port *ourport = id;
> -	const struct uart_port *port = &ourport->port;
> +	struct s3c24xx_uart_port *ourport = id;
> +	struct uart_port *port = &ourport->port;
>   	u32 pend = rd_regl(port, S3C64XX_UINTP);
>   	irqreturn_t ret = IRQ_HANDLED;
>   
>   	if (pend & S3C64XX_UINTM_RXD_MSK) {
> -		ret = s3c24xx_serial_rx_irq(id);
> +		ret = s3c24xx_serial_rx_irq(ourport);
>   		wr_regl(port, S3C64XX_UINTP, S3C64XX_UINTM_RXD_MSK);
>   	}
>   	if (pend & S3C64XX_UINTM_TXD_MSK) {
> -		ret = s3c24xx_serial_tx_irq(id);
> +		ret = s3c24xx_serial_tx_irq(ourport);
>   		wr_regl(port, S3C64XX_UINTP, S3C64XX_UINTM_TXD_MSK);
>   	}
>   	return ret;
> @@ -963,19 +958,19 @@ static irqreturn_t s3c64xx_serial_handle_irq(int irq, void *id)
>   /* interrupt handler for Apple SoC's.*/
>   static irqreturn_t apple_serial_handle_irq(int irq, void *id)
>   {
> -	const struct s3c24xx_uart_port *ourport = id;
> -	const struct uart_port *port = &ourport->port;
> +	struct s3c24xx_uart_port *ourport = id;
> +	struct uart_port *port = &ourport->port;

No need to remove const from port here and above, right? (Only from 
ourport.)

Other than that, LGTM.

thanks,
-- 
js
suse labs

Re: [PATCH v2 2/2] tty: serial: samsung_tty: cast the interrupt's void *id just once

[André Draszik]

On Thu, 2024-08-08 at 09:31 +0200, Jiri Slaby wrote:
> On 07. 08. 24, 13:58, André Draszik wrote:
> > The interrupt handler routines and helpers are casting the 'void *'
> > pointer to 'struct exynos_uart_port *' all over the place.
> > 
> > There is no need for that, we can do the casting once and keep passing
> > the 'struct exynos_uart_port *', simplifying the code and saving a few
> > lines of code.
> > 
> > No functional changes.
> ...
> > @@ -944,17 +939,17 @@ static irqreturn_t s3c24xx_serial_tx_irq(void *id)
> >   /* interrupt handler for s3c64xx and later SoC's.*/
> >   static irqreturn_t s3c64xx_serial_handle_irq(int irq, void *id)
> >   {
> > -	const struct s3c24xx_uart_port *ourport = id;
> > -	const struct uart_port *port = &ourport->port;
> > +	struct s3c24xx_uart_port *ourport = id;
> > +	struct uart_port *port = &ourport->port;
> >   	u32 pend = rd_regl(port, S3C64XX_UINTP);
> >   	irqreturn_t ret = IRQ_HANDLED;
> >   
> >   	if (pend & S3C64XX_UINTM_RXD_MSK) {
> > -		ret = s3c24xx_serial_rx_irq(id);
> > +		ret = s3c24xx_serial_rx_irq(ourport);
> >   		wr_regl(port, S3C64XX_UINTP, S3C64XX_UINTM_RXD_MSK);
> >   	}
> >   	if (pend & S3C64XX_UINTM_TXD_MSK) {
> > -		ret = s3c24xx_serial_tx_irq(id);
> > +		ret = s3c24xx_serial_tx_irq(ourport);
> >   		wr_regl(port, S3C64XX_UINTP, S3C64XX_UINTM_TXD_MSK);
> >   	}
> >   	return ret;
> > @@ -963,19 +958,19 @@ static irqreturn_t s3c64xx_serial_handle_irq(int irq, void *id)
> >   /* interrupt handler for Apple SoC's.*/
> >   static irqreturn_t apple_serial_handle_irq(int irq, void *id)
> >   {
> > -	const struct s3c24xx_uart_port *ourport = id;
> > -	const struct uart_port *port = &ourport->port;
> > +	struct s3c24xx_uart_port *ourport = id;
> > +	struct uart_port *port = &ourport->port;
> 
> No need to remove const from port here and above, right? (Only from 
> ourport.)

Jiri, you're right of course.

Thanks,
A.