[PATCH 1/1] tty: serial: use dmaengine_desc_set_callback()

[PATCH 1/1] tty: serial: use dmaengine_desc_set_callback()

[Frank.Li]

From: Frank Li <Frank.Li@nxp.com>

Replace open-coded assignments to callback and callback_param fields
of struct dma_async_tx_descriptor with calls to the
dmaengine_desc_set_callback() helper across serial drivers.

No functional change intended. Prepare to make struct
dma_async_tx_descriptor private to the DMA engine.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/tty/serial/8250/8250_dma.c  | 6 ++----
 drivers/tty/serial/8250/8250_mtk.c  | 3 +--
 drivers/tty/serial/8250/8250_omap.c | 6 ++----
 drivers/tty/serial/amba-pl011.c     | 6 ++----
 drivers/tty/serial/atmel_serial.c   | 6 ++----
 drivers/tty/serial/fsl_lpuart.c     | 6 ++----
 drivers/tty/serial/imx.c            | 6 ++----
 drivers/tty/serial/msm_serial.c     | 6 ++----
 drivers/tty/serial/mxs-auart.c      | 6 ++----
 drivers/tty/serial/pch_uart.c       | 6 ++----
 drivers/tty/serial/samsung_tty.c    | 6 ++----
 drivers/tty/serial/serial-tegra.c   | 6 ++----
 drivers/tty/serial/sh-sci.c         | 9 +++------
 drivers/tty/serial/sprd_serial.c    | 3 +--
 drivers/tty/serial/stm32-usart.c    | 6 ++----
 15 files changed, 29 insertions(+), 58 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dma.c b/drivers/tty/serial/8250/8250_dma.c
index 1da605cc86011..7d42afe1ce33b 100644
--- a/drivers/tty/serial/8250/8250_dma.c
+++ b/drivers/tty/serial/8250/8250_dma.c
@@ -131,8 +131,7 @@ int serial8250_tx_dma(struct uart_8250_port *p)
 	}
 
 	dma->tx_running = 1;
-	desc->callback = __dma_tx_complete;
-	desc->callback_param = p;
+	dmaengine_desc_set_callback(desc, __dma_tx_complete, NULL, p);
 
 	dma->tx_cookie = dmaengine_submit(desc);
 
@@ -197,8 +196,7 @@ int serial8250_rx_dma(struct uart_8250_port *p)
 		return -EBUSY;
 
 	dma->rx_running = 1;
-	desc->callback = dma_rx_complete;
-	desc->callback_param = p;
+	dmaengine_desc_set_callback(desc, dma_rx_complete, NULL, p);
 
 	dma->rx_cookie = dmaengine_submit(desc);
 
diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
index d1fb917b762a1..442856b3fe6d9 100644
--- a/drivers/tty/serial/8250/8250_mtk.c
+++ b/drivers/tty/serial/8250/8250_mtk.c
@@ -147,8 +147,7 @@ static void mtk8250_rx_dma(struct uart_8250_port *up)
 		return;
 	}
 
-	desc->callback = mtk8250_dma_rx_complete;
-	desc->callback_param = up;
+	dmaengine_desc_set_callback(desc, mtk8250_dma_rx_complete, NULL, up);
 
 	dma->rx_cookie = dmaengine_submit(desc);
 
diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index 29a203ed93fd8..9bdc0f00dec5a 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -1048,8 +1048,7 @@ static int omap_8250_rx_dma(struct uart_8250_port *p)
 	}
 
 	dma->rx_running = 1;
-	desc->callback = __dma_rx_complete;
-	desc->callback_param = p;
+	dmaengine_desc_set_callback(desc, __dma_rx_complete, NULL, p);
 
 	dma->rx_cookie = dmaengine_submit(desc);
 
@@ -1192,8 +1191,7 @@ static int omap_8250_tx_dma(struct uart_8250_port *p)
 	dma->tx_size = sg_dma_len(&sg);
 	dma->tx_running = 1;
 
-	desc->callback = omap_8250_dma_tx_complete;
-	desc->callback_param = p;
+	dmaengine_desc_set_callback(desc, omap_8250_dma_tx_complete, NULL, p);
 
 	dma->tx_cookie = dmaengine_submit(desc);
 
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index e6f652905ca6f..e7373cd11b621 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -715,8 +715,7 @@ static int pl011_dma_tx_refill(struct uart_amba_port *uap)
 	}
 
 	/* Some data to go along to the callback */
-	desc->callback = pl011_dma_tx_callback;
-	desc->callback_param = uap;
+	dmaengine_desc_set_callback(desc, pl011_dma_tx_callback, NULL, uap);
 
 	/* All errors should happen at prepare time */
 	dmaengine_submit(desc);
@@ -906,8 +905,7 @@ static int pl011_dma_rx_trigger_dma(struct uart_amba_port *uap)
 	}
 
 	/* Some data to go along to the callback */
-	desc->callback = pl011_dma_rx_callback;
-	desc->callback_param = uap;
+	dmaengine_desc_set_callback(desc, pl011_dma_rx_callback, NULL, uap);
 	dmarx->cookie = dmaengine_submit(desc);
 	dma_async_issue_pending(rxchan);
 
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index c3880259e3bce..886280bc8464a 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -993,8 +993,7 @@ static void atmel_tx_dma(struct uart_port *port)
 					   UART_XMIT_SIZE, DMA_TO_DEVICE);
 
 		atmel_port->desc_tx = desc;
-		desc->callback = atmel_complete_tx_dma;
-		desc->callback_param = atmel_port;
+		dmaengine_desc_set_callback(desc, atmel_complete_tx_dma, NULL, atmel_port);
 		atmel_port->cookie_tx = dmaengine_submit(desc);
 		if (dma_submit_error(atmel_port->cookie_tx)) {
 			dev_err(port->dev, "dma_submit_error %d\n",
@@ -1242,8 +1241,7 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
 		dev_err(port->dev, "Preparing DMA cyclic failed\n");
 		goto chan_err;
 	}
-	desc->callback = atmel_complete_rx_dma;
-	desc->callback_param = port;
+	dmaengine_desc_set_callback(desc, atmel_complete_rx_dma, NULL, port);
 	atmel_port->desc_rx = desc;
 	atmel_port->cookie_rx = dmaengine_submit(desc);
 	if (dma_submit_error(atmel_port->cookie_rx)) {
diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index c8575c9652035..3213519776b35 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -508,8 +508,7 @@ static void lpuart_dma_tx(struct lpuart_port *sport)
 		return;
 	}
 
-	sport->dma_tx_desc->callback = lpuart_dma_tx_complete;
-	sport->dma_tx_desc->callback_param = sport;
+	dmaengine_desc_set_callback(sport->dma_tx_desc, lpuart_dma_tx_complete, NULL, sport);
 	sport->dma_tx_in_progress = true;
 	sport->dma_tx_cookie = dmaengine_submit(sport->dma_tx_desc);
 	dma_async_issue_pending(chan);
@@ -1407,8 +1406,7 @@ static inline int lpuart_start_rx_dma(struct lpuart_port *sport)
 		goto err_unmap_sg;
 	}
 
-	sport->dma_rx_desc->callback = lpuart_dma_rx_complete;
-	sport->dma_rx_desc->callback_param = sport;
+	dmaengine_desc_set_callback(sport->dma_rx_desc, lpuart_dma_rx_complete, NULL, sport);
 	sport->dma_rx_cookie = dmaengine_submit(sport->dma_rx_desc);
 	dma_async_issue_pending(chan);
 
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 986fefb45c632..c83da7c48b675 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -655,8 +655,7 @@ static void imx_uart_dma_tx(struct imx_port *sport)
 		dev_err(dev, "We cannot prepare for the TX slave dma!\n");
 		return;
 	}
-	desc->callback = imx_uart_dma_tx_callback;
-	desc->callback_param = sport;
+	dmaengine_desc_set_callback(desc, imx_uart_dma_tx_callback, NULL, sport);
 
 	dev_dbg(dev, "TX: prepare to send %u bytes by DMA.\n", sport->tx_bytes);
 
@@ -1294,8 +1293,7 @@ static int imx_uart_start_rx_dma(struct imx_port *sport)
 		dev_err(dev, "We cannot prepare for the RX slave dma!\n");
 		return -EINVAL;
 	}
-	desc->callback = imx_uart_dma_rx_callback;
-	desc->callback_param = sport;
+	dmaengine_desc_set_callback(desc, imx_uart_dma_rx_callback, NULL, sport);
 
 	dev_dbg(dev, "RX: prepare for the DMA.\n");
 	sport->dma_is_rxing = 1;
diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
index bfa44b01c3e97..62c7872f9ffbd 100644
--- a/drivers/tty/serial/msm_serial.c
+++ b/drivers/tty/serial/msm_serial.c
@@ -521,8 +521,7 @@ static int msm_handle_tx_dma(struct msm_port *msm_port, unsigned int count)
 		goto unmap;
 	}
 
-	dma->desc->callback = msm_complete_tx_dma;
-	dma->desc->callback_param = msm_port;
+	dmaengine_desc_set_callback(dma->desc, msm_complete_tx_dma, NULL, msm_port);
 
 	dma->cookie = dmaengine_submit(dma->desc);
 	ret = dma_submit_error(dma->cookie);
@@ -642,8 +641,7 @@ static void msm_start_rx_dma(struct msm_port *msm_port)
 	if (!dma->desc)
 		goto unmap;
 
-	dma->desc->callback = msm_complete_rx_dma;
-	dma->desc->callback_param = msm_port;
+	dmaengine_desc_set_callback(dma->desc, msm_complete_rx_dma, NULL, msm_port);
 
 	dma->cookie = dmaengine_submit(dma->desc);
 	ret = dma_submit_error(dma->cookie);
diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
index 697318dbb146b..0b39dd2d653ee 100644
--- a/drivers/tty/serial/mxs-auart.c
+++ b/drivers/tty/serial/mxs-auart.c
@@ -559,8 +559,7 @@ static int mxs_auart_dma_tx(struct mxs_auart_port *s, int size)
 	}
 
 	/* [3] : submit the DMA */
-	desc->callback = dma_tx_callback;
-	desc->callback_param = s;
+	dmaengine_desc_set_callback(desc, dma_tx_callback, NULL, s);
 	dmaengine_submit(desc);
 	dma_async_issue_pending(channel);
 	return 0;
@@ -858,8 +857,7 @@ static int mxs_auart_dma_prep_rx(struct mxs_auart_port *s)
 	}
 
 	/* [3] : submit the DMA, but do not issue it. */
-	desc->callback = dma_rx_callback;
-	desc->callback_param = s;
+	dmaengine_desc_set_callback(desc, dma_rx_callback, NULL, s);
 	dmaengine_submit(desc);
 	dma_async_issue_pending(channel);
 	return 0;
diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
index 9505d255f09c5..8f3ffe39a5857 100644
--- a/drivers/tty/serial/pch_uart.c
+++ b/drivers/tty/serial/pch_uart.c
@@ -809,8 +809,7 @@ static int dma_handle_rx(struct eg20t_port *priv)
 		return 0;
 
 	priv->desc_rx = desc;
-	desc->callback = pch_dma_rx_complete;
-	desc->callback_param = priv;
+	dmaengine_desc_set_callback(desc, pch_dma_rx_complete, NULL, priv);
 	dmaengine_submit(desc);
 	dma_async_issue_pending(priv->chan_rx);
 
@@ -968,8 +967,7 @@ static unsigned int dma_handle_tx(struct eg20t_port *priv)
 	}
 	dma_sync_sg_for_device(port->dev, priv->sg_tx_p, num, DMA_TO_DEVICE);
 	priv->desc_tx = desc;
-	desc->callback = pch_dma_tx_complete;
-	desc->callback_param = priv;
+	dmaengine_desc_set_callback(desc, pch_dma_tx_complete, NULL, priv);
 
 	dmaengine_submit(desc);
 
diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
index 640f85e2d636b..3f82a83998e1c 100644
--- a/drivers/tty/serial/samsung_tty.c
+++ b/drivers/tty/serial/samsung_tty.c
@@ -447,8 +447,7 @@ static int s3c24xx_serial_start_tx_dma(struct s3c24xx_uart_port *ourport,
 		return -EIO;
 	}
 
-	dma->tx_desc->callback = s3c24xx_serial_tx_dma_complete;
-	dma->tx_desc->callback_param = ourport;
+	dmaengine_desc_set_callback(dma->tx_desc, s3c24xx_serial_tx_dma_complete, NULL, ourport);
 	dma->tx_bytes_requested = dma->tx_size;
 
 	ourport->tx_in_progress = S3C24XX_TX_DMA;
@@ -642,8 +641,7 @@ static void s3c64xx_start_rx_dma(struct s3c24xx_uart_port *ourport)
 		return;
 	}
 
-	dma->rx_desc->callback = s3c24xx_serial_rx_dma_complete;
-	dma->rx_desc->callback_param = ourport;
+	dmaengine_desc_set_callback(dma->rx_desc, s3c24xx_serial_rx_dma_complete, NULL, ourport);
 	dma->rx_bytes_requested = dma->rx_size;
 
 	dma->rx_cookie = dmaengine_submit(dma->rx_desc);
diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c
index b0fc4ae3686f8..05ba808bdbaef 100644
--- a/drivers/tty/serial/serial-tegra.c
+++ b/drivers/tty/serial/serial-tegra.c
@@ -568,8 +568,7 @@ static int tegra_uart_start_tx_dma(struct tegra_uart_port *tup,
 		return -EIO;
 	}
 
-	tup->tx_dma_desc->callback = tegra_uart_tx_dma_complete;
-	tup->tx_dma_desc->callback_param = tup;
+	dmaengine_desc_set_callback(tup->tx_dma_desc, tegra_uart_tx_dma_complete, NULL, tup);
 	tup->tx_in_progress = TEGRA_UART_TX_DMA;
 	tup->tx_bytes_requested = tup->tx_bytes;
 	tup->tx_cookie = dmaengine_submit(tup->tx_dma_desc);
@@ -817,8 +816,7 @@ static int tegra_uart_start_rx_dma(struct tegra_uart_port *tup)
 	}
 
 	tup->rx_dma_active = true;
-	tup->rx_dma_desc->callback = tegra_uart_rx_dma_complete;
-	tup->rx_dma_desc->callback_param = tup;
+	dmaengine_desc_set_callback(tup->rx_dma_desc, tegra_uart_rx_dma_complete, NULL, tup);
 	tup->rx_bytes_requested = count;
 	tup->rx_cookie = dmaengine_submit(tup->rx_dma_desc);
 	dma_async_issue_pending(tup->rx_dma_chan);
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index bd58bbfa65aa7..f7b4d35b2d5bb 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1556,8 +1556,7 @@ static void sci_dma_rx_complete(void *arg)
 	if (!desc)
 		goto fail;
 
-	desc->callback = sci_dma_rx_complete;
-	desc->callback_param = s;
+	dmaengine_desc_set_callback(desc, sci_dma_rx_complete, NULL, s);
 	s->cookie_rx[active] = dmaengine_submit(desc);
 	if (dma_submit_error(s->cookie_rx[active]))
 		goto fail;
@@ -1613,8 +1612,7 @@ static int sci_dma_rx_submit(struct sci_port *s, bool port_lock_held)
 		if (!desc)
 			goto fail;
 
-		desc->callback = sci_dma_rx_complete;
-		desc->callback_param = s;
+		dmaengine_desc_set_callback(desc, sci_dma_rx_complete, NULL, s);
 		s->cookie_rx[i] = dmaengine_submit(desc);
 		if (dma_submit_error(s->cookie_rx[i]))
 			goto fail;
@@ -1679,8 +1677,7 @@ static void sci_dma_tx_work_fn(struct work_struct *work)
 	dma_sync_single_for_device(dmaengine_get_dma_device(chan), buf, s->tx_dma_len,
 				   DMA_TO_DEVICE);
 
-	desc->callback = sci_dma_tx_complete;
-	desc->callback_param = s;
+	dmaengine_desc_set_callback(desc, sci_dma_tx_complete, NULL, s);
 	s->cookie_tx = dmaengine_submit(desc);
 	if (dma_submit_error(s->cookie_tx)) {
 		uart_port_unlock_irq(port);
diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c
index 3fdf5939a62c1..3801b0c03482f 100644
--- a/drivers/tty/serial/sprd_serial.c
+++ b/drivers/tty/serial/sprd_serial.c
@@ -303,8 +303,7 @@ static int sprd_uart_dma_submit(struct uart_port *port,
 	if (!dma_des)
 		return -ENODEV;
 
-	dma_des->callback = callback;
-	dma_des->callback_param = port;
+	dmaengine_desc_set_callback(dma_des, callback, NULL, port);
 
 	ud->cookie = dmaengine_submit(dma_des);
 	if (dma_submit_error(ud->cookie))
diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
index ad06b760cfca7..a27b8797ad953 100644
--- a/drivers/tty/serial/stm32-usart.c
+++ b/drivers/tty/serial/stm32-usart.c
@@ -585,8 +585,7 @@ static int stm32_usart_rx_dma_start_or_resume(struct uart_port *port)
 		return -ENODEV;
 	}
 
-	desc->callback = stm32_usart_rx_dma_complete;
-	desc->callback_param = port;
+	dmaengine_desc_set_callback(desc, stm32_usart_rx_dma_complete, NULL, port);
 
 	/* Push current DMA transaction in the pending queue */
 	ret = dma_submit_error(dmaengine_submit(desc));
@@ -753,8 +752,7 @@ static void stm32_usart_transmit_chars_dma(struct uart_port *port)
 	 */
 	stm32port->tx_dma_busy = true;
 
-	desc->callback = stm32_usart_tx_dma_complete;
-	desc->callback_param = port;
+	dmaengine_desc_set_callback(desc, stm32_usart_tx_dma_complete, NULL, port);
 
 	/* Push current DMA TX transaction in the pending queue */
 	/* DMA no yet started, safe to free resources */
-- 
2.43.0

Re: [PATCH 1/1] tty: serial: use dmaengine_desc_set_callback()

[Nicolai Buchwitz]

Hi Frank

On 2.11.2026 18:54, Frank.Li@oss.nxp.com wrote:
> From: Frank Li <Frank.Li@nxp.com>
> 
> Replace open-coded assignments to callback and callback_param fields
> of struct dma_async_tx_descriptor with calls to the
> dmaengine_desc_set_callback() helper across serial drivers.
> 
> No functional change intended. Prepare to make struct
> dma_async_tx_descriptor private to the DMA engine.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>  drivers/tty/serial/8250/8250_dma.c  | 6 ++----
>  drivers/tty/serial/8250/8250_mtk.c  | 3 +--
>  drivers/tty/serial/8250/8250_omap.c | 6 ++----
>  drivers/tty/serial/amba-pl011.c     | 6 ++----
>  drivers/tty/serial/atmel_serial.c   | 6 ++----
>  drivers/tty/serial/fsl_lpuart.c     | 6 ++----
>  drivers/tty/serial/imx.c            | 6 ++----
>  drivers/tty/serial/msm_serial.c     | 6 ++----
>  drivers/tty/serial/mxs-auart.c      | 6 ++----
>  drivers/tty/serial/pch_uart.c       | 6 ++----
>  drivers/tty/serial/samsung_tty.c    | 6 ++----
>  drivers/tty/serial/serial-tegra.c   | 6 ++----
>  drivers/tty/serial/sh-sci.c         | 9 +++------
>  drivers/tty/serial/sprd_serial.c    | 3 +--
>  drivers/tty/serial/stm32-usart.c    | 6 ++----
>  15 files changed, 29 insertions(+), 58 deletions(-)

> [...]

> diff --git a/drivers/tty/serial/amba-pl011.c 
> b/drivers/tty/serial/amba-pl011.c
> index e6f652905ca6f..e7373cd11b621 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -715,8 +715,7 @@ static int pl011_dma_tx_refill(struct 
> uart_amba_port *uap)
>  	}
> 
>  	/* Some data to go along to the callback */
> -	desc->callback = pl011_dma_tx_callback;
> -	desc->callback_param = uap;
> +	dmaengine_desc_set_callback(desc, pl011_dma_tx_callback, NULL, uap);
> 
>  	/* All errors should happen at prepare time */
>  	dmaengine_submit(desc);
> @@ -906,8 +905,7 @@ static int pl011_dma_rx_trigger_dma(struct 
> uart_amba_port *uap)
>  	}
> 
>  	/* Some data to go along to the callback */
> -	desc->callback = pl011_dma_rx_callback;
> -	desc->callback_param = uap;
> +	dmaengine_desc_set_callback(desc, pl011_dma_rx_callback, NULL, uap);
>  	dmarx->cookie = dmaengine_submit(desc);
>  	dma_async_issue_pending(rxchan);

> [...]

Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de> # amba-pl011

Thanks,
Nicolai

Re: [PATCH 1/1] tty: serial: use dmaengine_desc_set_callback()

[Krzysztof Kozlowski]

On 02/11/2026 18:54, Frank.Li@oss.nxp.com wrote:
> From: Frank Li <Frank.Li@nxp.com>
> 
> Replace open-coded assignments to callback and callback_param fields
> of struct dma_async_tx_descriptor with calls to the
> dmaengine_desc_set_callback() helper across serial drivers.
> 
> No functional change intended. Prepare to make struct
> dma_async_tx_descriptor private to the DMA engine.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>  drivers/tty/serial/8250/8250_dma.c  | 6 ++----
>  drivers/tty/serial/8250/8250_mtk.c  | 3 +--
>  drivers/tty/serial/8250/8250_omap.c | 6 ++----
>  drivers/tty/serial/amba-pl011.c     | 6 ++----
>  drivers/tty/serial/atmel_serial.c   | 6 ++----
>  drivers/tty/serial/fsl_lpuart.c     | 6 ++----
>  drivers/tty/serial/imx.c            | 6 ++----
>  drivers/tty/serial/msm_serial.c     | 6 ++----
>  drivers/tty/serial/mxs-auart.c      | 6 ++----
>  drivers/tty/serial/pch_uart.c       | 6 ++----
>  drivers/tty/serial/samsung_tty.c    | 6 ++----
>  drivers/tty/serial/serial-tegra.c   | 6 ++----
>  drivers/tty/serial/sh-sci.c         | 9 +++------
>  drivers/tty/serial/sprd_serial.c    | 3 +--
>  drivers/tty/serial/stm32-usart.c    | 6 ++----
>  15 files changed, 29 insertions(+), 58 deletions(-)

...
I did not look at this code at all...

> index 640f85e2d636b..3f82a83998e1c 100644
> --- a/drivers/tty/serial/samsung_tty.c
> +++ b/drivers/tty/serial/samsung_tty.c
> @@ -447,8 +447,7 @@ static int s3c24xx_serial_start_tx_dma(struct s3c24xx_uart_port *ourport,
>  		return -EIO;
>  	}
>  
> -	dma->tx_desc->callback = s3c24xx_serial_tx_dma_complete;
> -	dma->tx_desc->callback_param = ourport;
> +	dmaengine_desc_set_callback(dma->tx_desc, s3c24xx_serial_tx_dma_complete, NULL, ourport);
>  	dma->tx_bytes_requested = dma->tx_size;
>  
>  	ourport->tx_in_progress = S3C24XX_TX_DMA;
> @@ -642,8 +641,7 @@ static void s3c64xx_start_rx_dma(struct s3c24xx_uart_port *ourport)
>  		return;
>  	}
>  
> -	dma->rx_desc->callback = s3c24xx_serial_rx_dma_complete;
> -	dma->rx_desc->callback_param = ourport;
> +	dmaengine_desc_set_callback(dma->rx_desc, s3c24xx_serial_rx_dma_complete, NULL, ourport);
>  	dma->rx_bytes_requested = dma->rx_size;
>  


... but apparently reviewing only a few lines out of 400 is still reviewing:

https://lore.kernel.org/all/9e39502a-dd17-4fa2-98a3-9019523cb5e8@kernel.org/

Therefore:

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>


Best regards,
Krzysztof

Re: [PATCH 1/1] tty: serial: use dmaengine_desc_set_callback()

[Andy Shevchenko]

On Wed, Nov 04, 2026 at 06:41:08PM +0100, Krzysztof Kozlowski wrote:
> On 02/11/2026 18:54, Frank.Li@oss.nxp.com wrote:
> > 
> > Replace open-coded assignments to callback and callback_param fields
> > of struct dma_async_tx_descriptor with calls to the
> > dmaengine_desc_set_callback() helper across serial drivers.
> > 
> > No functional change intended. Prepare to make struct
> > dma_async_tx_descriptor private to the DMA engine.

...

> I did not look at this code at all...
> 
> > index 640f85e2d636b..3f82a83998e1c 100644
> > --- a/drivers/tty/serial/samsung_tty.c
> > +++ b/drivers/tty/serial/samsung_tty.c
> > @@ -447,8 +447,7 @@ static int s3c24xx_serial_start_tx_dma(struct s3c24xx_uart_port *ourport,
> >  		return -EIO;
> >  	}
> >  
> > -	dma->tx_desc->callback = s3c24xx_serial_tx_dma_complete;
> > -	dma->tx_desc->callback_param = ourport;
> > +	dmaengine_desc_set_callback(dma->tx_desc, s3c24xx_serial_tx_dma_complete, NULL, ourport);
> >  	dma->tx_bytes_requested = dma->tx_size;
> >  
> >  	ourport->tx_in_progress = S3C24XX_TX_DMA;
> > @@ -642,8 +641,7 @@ static void s3c64xx_start_rx_dma(struct s3c24xx_uart_port *ourport)
> >  		return;
> >  	}
> >  
> > -	dma->rx_desc->callback = s3c24xx_serial_rx_dma_complete;
> > -	dma->rx_desc->callback_param = ourport;
> > +	dmaengine_desc_set_callback(dma->rx_desc, s3c24xx_serial_rx_dma_complete, NULL, ourport);
> >  	dma->rx_bytes_requested = dma->rx_size;

> ... but apparently reviewing only a few lines out of 400 is still reviewing:
> 
> https://lore.kernel.org/all/9e39502a-dd17-4fa2-98a3-9019523cb5e8@kernel.org/
> 
> Therefore:
> 
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

That's why I have added a trail to it to show what exactly I has reviewed,
this is a common practice, no?

Does this ring a bell 34d1e754155b ("dt-bindings: iio: drop unneeded quotes")?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/1] tty: serial: use dmaengine_desc_set_callback()

[Geert Uytterhoeven]

Hi Kezysztof,

On Wed, 4 Nov 2026 at 18:41, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On 02/11/2026 18:54, Frank.Li@oss.nxp.com wrote:
> > From: Frank Li <Frank.Li@nxp.com>
> >
> > Replace open-coded assignments to callback and callback_param fields
> > of struct dma_async_tx_descriptor with calls to the
> > dmaengine_desc_set_callback() helper across serial drivers.
> >
> > No functional change intended. Prepare to make struct
> > dma_async_tx_descriptor private to the DMA engine.
> >
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >  drivers/tty/serial/8250/8250_dma.c  | 6 ++----
> >  drivers/tty/serial/8250/8250_mtk.c  | 3 +--
> >  drivers/tty/serial/8250/8250_omap.c | 6 ++----
> >  drivers/tty/serial/amba-pl011.c     | 6 ++----
> >  drivers/tty/serial/atmel_serial.c   | 6 ++----
> >  drivers/tty/serial/fsl_lpuart.c     | 6 ++----
> >  drivers/tty/serial/imx.c            | 6 ++----
> >  drivers/tty/serial/msm_serial.c     | 6 ++----
> >  drivers/tty/serial/mxs-auart.c      | 6 ++----
> >  drivers/tty/serial/pch_uart.c       | 6 ++----
> >  drivers/tty/serial/samsung_tty.c    | 6 ++----
> >  drivers/tty/serial/serial-tegra.c   | 6 ++----
> >  drivers/tty/serial/sh-sci.c         | 9 +++------
> >  drivers/tty/serial/sprd_serial.c    | 3 +--
> >  drivers/tty/serial/stm32-usart.c    | 6 ++----
> >  15 files changed, 29 insertions(+), 58 deletions(-)
>
> ...
> I did not look at this code at all...

[...]

> Therefore:
>
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Then please add "# samsung", to make it clear and obvious.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/1] tty: serial: use dmaengine_desc_set_callback()

[Krzysztof Kozlowski]

On 05/11/2026 10:06, Geert Uytterhoeven wrote:
>> Therefore:
>>
>> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
> 
> Then please add "# samsung", to make it clear and obvious.

That was irony, the linked thread has fuller discussion, including '#
marker' part topic. Although should be moved to workflows (to my v2
patch for submitting-patches discussion), because I still contest the '#
marker' logic.

Reviewer statement of oversight does not allow that, as I explained in
linked thread.

Best regards,
Krzysztof

Re: [PATCH 1/1] tty: serial: use dmaengine_desc_set_callback()

[Andy Shevchenko]

On Thu, Nov 05, 2026 at 10:06:06AM +0100, Geert Uytterhoeven wrote:
> On Wed, 4 Nov 2026 at 18:41, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > On 02/11/2026 18:54, Frank.Li@oss.nxp.com wrote:

> > I did not look at this code at all...

[...]

> > Therefore:
> >
> > Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
> 
> Then please add "# samsung", to make it clear and obvious.

+1

See also 20261105082729.19622-2-krzysztof.kozlowski@oss.qualcomm.com

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/1] tty: serial: use dmaengine_desc_set_callback()

[Geert Uytterhoeven]

On Mon, 2 Nov 2026 at 18:56, <Frank.Li@oss.nxp.com> wrote:
> From: Frank Li <Frank.Li@nxp.com>
>
> Replace open-coded assignments to callback and callback_param fields
> of struct dma_async_tx_descriptor with calls to the
> dmaengine_desc_set_callback() helper across serial drivers.
>
> No functional change intended. Prepare to make struct
> dma_async_tx_descriptor private to the DMA engine.
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>

>  drivers/tty/serial/sh-sci.c         | 9 +++------

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> # sh-sci

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/1] tty: serial: use dmaengine_desc_set_callback()

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1344 bytes --]

On Mon, Nov 02, 2026 at 12:54:48PM -0500, Frank.Li@oss.nxp.com wrote:
> From: Frank Li <Frank.Li@nxp.com>
> 
> Replace open-coded assignments to callback and callback_param fields
> of struct dma_async_tx_descriptor with calls to the
> dmaengine_desc_set_callback() helper across serial drivers.
> 
> No functional change intended. Prepare to make struct
> dma_async_tx_descriptor private to the DMA engine.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>  drivers/tty/serial/8250/8250_dma.c  | 6 ++----
>  drivers/tty/serial/8250/8250_mtk.c  | 3 +--
>  drivers/tty/serial/8250/8250_omap.c | 6 ++----
>  drivers/tty/serial/amba-pl011.c     | 6 ++----
>  drivers/tty/serial/atmel_serial.c   | 6 ++----
>  drivers/tty/serial/fsl_lpuart.c     | 6 ++----
>  drivers/tty/serial/imx.c            | 6 ++----
>  drivers/tty/serial/msm_serial.c     | 6 ++----
>  drivers/tty/serial/mxs-auart.c      | 6 ++----
>  drivers/tty/serial/pch_uart.c       | 6 ++----
>  drivers/tty/serial/samsung_tty.c    | 6 ++----
>  drivers/tty/serial/serial-tegra.c   | 6 ++----
>  drivers/tty/serial/sh-sci.c         | 9 +++------
>  drivers/tty/serial/sprd_serial.c    | 3 +--
>  drivers/tty/serial/stm32-usart.c    | 6 ++----
>  15 files changed, 29 insertions(+), 58 deletions(-)

Reviewed-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/1] tty: serial: use dmaengine_desc_set_callback()

[Andy Shevchenko]

On Thu, Nov 05, 2026 at 01:28:46PM +0100, Thierry Reding wrote:
> On Mon, Nov 02, 2026 at 12:54:48PM -0500, Frank.Li@oss.nxp.com wrote:
> > 
> > Replace open-coded assignments to callback and callback_param fields
> > of struct dma_async_tx_descriptor with calls to the
> > dmaengine_desc_set_callback() helper across serial drivers.
> > 
> > No functional change intended. Prepare to make struct
> > dma_async_tx_descriptor private to the DMA engine.
> > 
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >  drivers/tty/serial/8250/8250_dma.c  | 6 ++----
> >  drivers/tty/serial/8250/8250_mtk.c  | 3 +--
> >  drivers/tty/serial/8250/8250_omap.c | 6 ++----
> >  drivers/tty/serial/amba-pl011.c     | 6 ++----
> >  drivers/tty/serial/atmel_serial.c   | 6 ++----
> >  drivers/tty/serial/fsl_lpuart.c     | 6 ++----
> >  drivers/tty/serial/imx.c            | 6 ++----
> >  drivers/tty/serial/msm_serial.c     | 6 ++----
> >  drivers/tty/serial/mxs-auart.c      | 6 ++----
> >  drivers/tty/serial/pch_uart.c       | 6 ++----
> >  drivers/tty/serial/samsung_tty.c    | 6 ++----
> >  drivers/tty/serial/serial-tegra.c   | 6 ++----
> >  drivers/tty/serial/sh-sci.c         | 9 +++------
> >  drivers/tty/serial/sprd_serial.c    | 3 +--
> >  drivers/tty/serial/stm32-usart.c    | 6 ++----
> >  15 files changed, 29 insertions(+), 58 deletions(-)
> 
> Reviewed-by: Thierry Reding <treding@nvidia.com>

Krzysztof, now look at this case. I would argue that this is even less fit than
# subpart(s) case, because it might imply anything. Again the problem is the
bundle, and for such a case (when tons of independent changes are bundled in
the same stuff, *every* Rb tag has to be explained on what has been reviewed!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/1] tty: serial: use dmaengine_desc_set_callback()

[Krzysztof Kozlowski]

On 05/11/2026 15:17, Andy Shevchenko wrote:
>>> ---
>>>  drivers/tty/serial/8250/8250_dma.c  | 6 ++----
>>>  drivers/tty/serial/8250/8250_mtk.c  | 3 +--
>>>  drivers/tty/serial/8250/8250_omap.c | 6 ++----
>>>  drivers/tty/serial/amba-pl011.c     | 6 ++----
>>>  drivers/tty/serial/atmel_serial.c   | 6 ++----
>>>  drivers/tty/serial/fsl_lpuart.c     | 6 ++----
>>>  drivers/tty/serial/imx.c            | 6 ++----
>>>  drivers/tty/serial/msm_serial.c     | 6 ++----
>>>  drivers/tty/serial/mxs-auart.c      | 6 ++----
>>>  drivers/tty/serial/pch_uart.c       | 6 ++----
>>>  drivers/tty/serial/samsung_tty.c    | 6 ++----
>>>  drivers/tty/serial/serial-tegra.c   | 6 ++----
>>>  drivers/tty/serial/sh-sci.c         | 9 +++------
>>>  drivers/tty/serial/sprd_serial.c    | 3 +--
>>>  drivers/tty/serial/stm32-usart.c    | 6 ++----
>>>  15 files changed, 29 insertions(+), 58 deletions(-)
>>
>> Reviewed-by: Thierry Reding <treding@nvidia.com>
> 
> Krzysztof, now look at this case. I would argue that this is even less fit than
> # subpart(s) case, because it might imply anything. Again the problem is the
> bundle, and for such a case (when tons of independent changes are bundled in
> the same stuff, *every* Rb tag has to be explained on what has been reviewed!

I think this case is obvious - Thierry reviewed this patch and gives Rb
tag, according to reviewer's statement of oversight. I don't see a
problem here or what is questionable.


Best regards,
Krzysztof

Re: [PATCH 1/1] tty: serial: use dmaengine_desc_set_callback()

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2331 bytes --]

On Thu, Nov 05, 2026 at 03:35:20PM +0100, Krzysztof Kozlowski wrote:
> On 05/11/2026 15:17, Andy Shevchenko wrote:
> >>> ---
> >>>  drivers/tty/serial/8250/8250_dma.c  | 6 ++----
> >>>  drivers/tty/serial/8250/8250_mtk.c  | 3 +--
> >>>  drivers/tty/serial/8250/8250_omap.c | 6 ++----
> >>>  drivers/tty/serial/amba-pl011.c     | 6 ++----
> >>>  drivers/tty/serial/atmel_serial.c   | 6 ++----
> >>>  drivers/tty/serial/fsl_lpuart.c     | 6 ++----
> >>>  drivers/tty/serial/imx.c            | 6 ++----
> >>>  drivers/tty/serial/msm_serial.c     | 6 ++----
> >>>  drivers/tty/serial/mxs-auart.c      | 6 ++----
> >>>  drivers/tty/serial/pch_uart.c       | 6 ++----
> >>>  drivers/tty/serial/samsung_tty.c    | 6 ++----
> >>>  drivers/tty/serial/serial-tegra.c   | 6 ++----
> >>>  drivers/tty/serial/sh-sci.c         | 9 +++------
> >>>  drivers/tty/serial/sprd_serial.c    | 3 +--
> >>>  drivers/tty/serial/stm32-usart.c    | 6 ++----
> >>>  15 files changed, 29 insertions(+), 58 deletions(-)
> >>
> >> Reviewed-by: Thierry Reding <treding@nvidia.com>
> > 
> > Krzysztof, now look at this case. I would argue that this is even less fit than
> > # subpart(s) case, because it might imply anything. Again the problem is the
> > bundle, and for such a case (when tons of independent changes are bundled in
> > the same stuff, *every* Rb tag has to be explained on what has been reviewed!
> 
> I think this case is obvious - Thierry reviewed this patch and gives Rb
> tag, according to reviewer's statement of oversight. I don't see a
> problem here or what is questionable.

Correct. I looked at this whole thing and approve of it. If I review
only parts of something at most I'll give an Acked-by. I don't think
I've ever done partial reviews. I don't think you can reasonably do that
because if you don't know what other parts of a patch are doing, how can
you say that a specific hunk does what it's supposed to.

Patches are an atomic set of changes, so I don't think it makes sense to
review only parts of it. If you can't be bothered to look at the
entirety, that's absolutely fine of course, but in that case don't
provide a Reviewed-by.

Or, I suppose, you do and then you bear the consequences of lending
credibility to something that you haven't looked at or understood.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/1] tty: serial: use dmaengine_desc_set_callback()

[Andy Shevchenko]

On Thu, Nov 05, 2026 at 04:34:15PM +0100, Thierry Reding wrote:
> On Thu, Nov 05, 2026 at 03:35:20PM +0100, Krzysztof Kozlowski wrote:
> > On 05/11/2026 15:17, Andy Shevchenko wrote:
> > >>> ---
> > >>>  drivers/tty/serial/8250/8250_dma.c  | 6 ++----
> > >>>  drivers/tty/serial/8250/8250_mtk.c  | 3 +--
> > >>>  drivers/tty/serial/8250/8250_omap.c | 6 ++----
> > >>>  drivers/tty/serial/amba-pl011.c     | 6 ++----
> > >>>  drivers/tty/serial/atmel_serial.c   | 6 ++----
> > >>>  drivers/tty/serial/fsl_lpuart.c     | 6 ++----
> > >>>  drivers/tty/serial/imx.c            | 6 ++----
> > >>>  drivers/tty/serial/msm_serial.c     | 6 ++----
> > >>>  drivers/tty/serial/mxs-auart.c      | 6 ++----
> > >>>  drivers/tty/serial/pch_uart.c       | 6 ++----
> > >>>  drivers/tty/serial/samsung_tty.c    | 6 ++----
> > >>>  drivers/tty/serial/serial-tegra.c   | 6 ++----
> > >>>  drivers/tty/serial/sh-sci.c         | 9 +++------
> > >>>  drivers/tty/serial/sprd_serial.c    | 3 +--
> > >>>  drivers/tty/serial/stm32-usart.c    | 6 ++----
> > >>>  15 files changed, 29 insertions(+), 58 deletions(-)
> > >>
> > >> Reviewed-by: Thierry Reding <treding@nvidia.com>
> > > 
> > > Krzysztof, now look at this case. I would argue that this is even less fit than
> > > # subpart(s) case, because it might imply anything. Again the problem is the
> > > bundle, and for such a case (when tons of independent changes are bundled in
> > > the same stuff, *every* Rb tag has to be explained on what has been reviewed!
> > 
> > I think this case is obvious - Thierry reviewed this patch and gives Rb
> > tag, according to reviewer's statement of oversight. I don't see a
> > problem here or what is questionable.
> 
> Correct. I looked at this whole thing and approve of it.

for this type of bundles it's not obvious.

> If I review only parts of something at most I'll give an Acked-by.

Which is strange, don't you find it? The review is not an acknowledgement.
These are two different operations.

> I don't think I've ever done partial reviews. I don't think you can reasonably do
> that
> because if you don't know what other parts of a patch are doing, how can
> you say that a specific hunk does what it's supposed to.
> 
> Patches are an atomic set of changes,

Of course, not always the case. This patch is a bundle of atomic changes
in a single drop.

> so I don't think it makes sense to review only parts of it. If you can't be
> bothered to look at the entirety, that's absolutely fine of course, but in
> that case don't provide a Reviewed-by.
> 
> Or, I suppose, you do and then you bear the consequences of lending
> credibility to something that you haven't looked at or understood.

No, I review *my* part, that I understand and I confirm.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/1] tty: serial: use dmaengine_desc_set_callback()

[Krzysztof Kozlowski]

On 05/11/2026 17:16, Andy Shevchenko wrote:
> On Thu, Nov 05, 2026 at 04:34:15PM +0100, Thierry Reding wrote:
>> On Thu, Nov 05, 2026 at 03:35:20PM +0100, Krzysztof Kozlowski wrote:
>>> On 05/11/2026 15:17, Andy Shevchenko wrote:
>>>>>> ---
>>>>>>  drivers/tty/serial/8250/8250_dma.c  | 6 ++----
>>>>>>  drivers/tty/serial/8250/8250_mtk.c  | 3 +--
>>>>>>  drivers/tty/serial/8250/8250_omap.c | 6 ++----
>>>>>>  drivers/tty/serial/amba-pl011.c     | 6 ++----
>>>>>>  drivers/tty/serial/atmel_serial.c   | 6 ++----
>>>>>>  drivers/tty/serial/fsl_lpuart.c     | 6 ++----
>>>>>>  drivers/tty/serial/imx.c            | 6 ++----
>>>>>>  drivers/tty/serial/msm_serial.c     | 6 ++----
>>>>>>  drivers/tty/serial/mxs-auart.c      | 6 ++----
>>>>>>  drivers/tty/serial/pch_uart.c       | 6 ++----
>>>>>>  drivers/tty/serial/samsung_tty.c    | 6 ++----
>>>>>>  drivers/tty/serial/serial-tegra.c   | 6 ++----
>>>>>>  drivers/tty/serial/sh-sci.c         | 9 +++------
>>>>>>  drivers/tty/serial/sprd_serial.c    | 3 +--
>>>>>>  drivers/tty/serial/stm32-usart.c    | 6 ++----
>>>>>>  15 files changed, 29 insertions(+), 58 deletions(-)
>>>>>
>>>>> Reviewed-by: Thierry Reding <treding@nvidia.com>
>>>>
>>>> Krzysztof, now look at this case. I would argue that this is even less fit than
>>>> # subpart(s) case, because it might imply anything. Again the problem is the
>>>> bundle, and for such a case (when tons of independent changes are bundled in
>>>> the same stuff, *every* Rb tag has to be explained on what has been reviewed!
>>>
>>> I think this case is obvious - Thierry reviewed this patch and gives Rb
>>> tag, according to reviewer's statement of oversight. I don't see a
>>> problem here or what is questionable.
>>
>> Correct. I looked at this whole thing and approve of it.
> 
> for this type of bundles it's not obvious.

No, it was still 100% obvious. Tag was placed under entire file-list,
even indicating that this was the intention.

> 
>> If I review only parts of something at most I'll give an Acked-by.
> 
> Which is strange, don't you find it? The review is not an acknowledgement.
> These are two different operations.

Andy, you mix review with Reviewer's Statement of Oversight. It clearly
states that it applies for THE patch, which is atomic, one logical
change being reviewed.

Best regards,
Krzysztof

Re: [PATCH 1/1] tty: serial: use dmaengine_desc_set_callback()

[Andy Shevchenko]

On Thu, Nov 05, 2026 at 03:35:20PM +0100, Krzysztof Kozlowski wrote:
> On 05/11/2026 15:17, Andy Shevchenko wrote:
> >>> ---
> >>>  drivers/tty/serial/8250/8250_dma.c  | 6 ++----
> >>>  drivers/tty/serial/8250/8250_mtk.c  | 3 +--
> >>>  drivers/tty/serial/8250/8250_omap.c | 6 ++----
> >>>  drivers/tty/serial/amba-pl011.c     | 6 ++----
> >>>  drivers/tty/serial/atmel_serial.c   | 6 ++----
> >>>  drivers/tty/serial/fsl_lpuart.c     | 6 ++----
> >>>  drivers/tty/serial/imx.c            | 6 ++----
> >>>  drivers/tty/serial/msm_serial.c     | 6 ++----
> >>>  drivers/tty/serial/mxs-auart.c      | 6 ++----
> >>>  drivers/tty/serial/pch_uart.c       | 6 ++----
> >>>  drivers/tty/serial/samsung_tty.c    | 6 ++----
> >>>  drivers/tty/serial/serial-tegra.c   | 6 ++----
> >>>  drivers/tty/serial/sh-sci.c         | 9 +++------
> >>>  drivers/tty/serial/sprd_serial.c    | 3 +--
> >>>  drivers/tty/serial/stm32-usart.c    | 6 ++----
> >>>  15 files changed, 29 insertions(+), 58 deletions(-)
> >>
> >> Reviewed-by: Thierry Reding <treding@nvidia.com>
> > 
> > Krzysztof, now look at this case. I would argue that this is even less fit than
> > # subpart(s) case, because it might imply anything. Again the problem is the
> > bundle, and for such a case (when tons of independent changes are bundled in
> > the same stuff, *every* Rb tag has to be explained on what has been reviewed!
> 
> I think this case is obvious - Thierry reviewed this patch and gives Rb
> tag, according to reviewer's statement of oversight.

It's unclear from a bare tag, It misses that kind of information. Documentation
is also unclear to me as it does *not* describe the case what should I do when
independent things are mixed together.

> I don't see a problem here or what is questionable.

And I obviously see the issue.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/1] tty: serial: use dmaengine_desc_set_callback()

[Krzysztof Kozlowski]

On 05/11/2026 17:20, Andy Shevchenko wrote:
> On Thu, Nov 05, 2026 at 03:35:20PM +0100, Krzysztof Kozlowski wrote:
>> On 05/11/2026 15:17, Andy Shevchenko wrote:
>>>>> ---
>>>>>  drivers/tty/serial/8250/8250_dma.c  | 6 ++----
>>>>>  drivers/tty/serial/8250/8250_mtk.c  | 3 +--
>>>>>  drivers/tty/serial/8250/8250_omap.c | 6 ++----
>>>>>  drivers/tty/serial/amba-pl011.c     | 6 ++----
>>>>>  drivers/tty/serial/atmel_serial.c   | 6 ++----
>>>>>  drivers/tty/serial/fsl_lpuart.c     | 6 ++----
>>>>>  drivers/tty/serial/imx.c            | 6 ++----
>>>>>  drivers/tty/serial/msm_serial.c     | 6 ++----
>>>>>  drivers/tty/serial/mxs-auart.c      | 6 ++----
>>>>>  drivers/tty/serial/pch_uart.c       | 6 ++----
>>>>>  drivers/tty/serial/samsung_tty.c    | 6 ++----
>>>>>  drivers/tty/serial/serial-tegra.c   | 6 ++----
>>>>>  drivers/tty/serial/sh-sci.c         | 9 +++------
>>>>>  drivers/tty/serial/sprd_serial.c    | 3 +--
>>>>>  drivers/tty/serial/stm32-usart.c    | 6 ++----
>>>>>  15 files changed, 29 insertions(+), 58 deletions(-)
>>>>
>>>> Reviewed-by: Thierry Reding <treding@nvidia.com>
>>>
>>> Krzysztof, now look at this case. I would argue that this is even less fit than
>>> # subpart(s) case, because it might imply anything. Again the problem is the
>>> bundle, and for such a case (when tons of independent changes are bundled in
>>> the same stuff, *every* Rb tag has to be explained on what has been reviewed!
>>
>> I think this case is obvious - Thierry reviewed this patch and gives Rb
>> tag, according to reviewer's statement of oversight.
> 
> It's unclear from a bare tag, It misses that kind of information. Documentation
> is also unclear to me as it does *not* describe the case what should I do when

Documentation is clear and I quote to you multiple times, to which you
did not respond.

"Reviewed-by:, instead, indicates that *the patch* has been reviewed and
found acceptable according to the Reviewer's Statement:"

Important: "THE PATCH"

Further:

"I state that:
(a) I have carried out a technical review of *this patch* to evaluate
its appropriateness and readiness for inclusion into  the mainline kernel."

And further:

"A Reviewed-by tag is a statement of opinion that *the patch* is an"

All of these are exhaustive (as in complete) statements leaving no
interpretation whether this is part of patch or entire patch. It is THE
patch, meaning entire patch.

You do not drive your car, as only a wheel. You driver *the car*.

You live in some apartment. *The apartment* is your home, not "piece of
your apartment is your home".

Best regards,
Krzysztof

Re: [PATCH 1/1] tty: serial: use dmaengine_desc_set_callback()

[Andy Shevchenko]

On Thu, Nov 05, 2026 at 05:26:18PM +0100, Krzysztof Kozlowski wrote:
> On 05/11/2026 17:20, Andy Shevchenko wrote:
> > On Thu, Nov 05, 2026 at 03:35:20PM +0100, Krzysztof Kozlowski wrote:
> >> On 05/11/2026 15:17, Andy Shevchenko wrote:
> >>>>> ---
> >>>>>  drivers/tty/serial/8250/8250_dma.c  | 6 ++----
> >>>>>  drivers/tty/serial/8250/8250_mtk.c  | 3 +--
> >>>>>  drivers/tty/serial/8250/8250_omap.c | 6 ++----
> >>>>>  drivers/tty/serial/amba-pl011.c     | 6 ++----
> >>>>>  drivers/tty/serial/atmel_serial.c   | 6 ++----
> >>>>>  drivers/tty/serial/fsl_lpuart.c     | 6 ++----
> >>>>>  drivers/tty/serial/imx.c            | 6 ++----
> >>>>>  drivers/tty/serial/msm_serial.c     | 6 ++----
> >>>>>  drivers/tty/serial/mxs-auart.c      | 6 ++----
> >>>>>  drivers/tty/serial/pch_uart.c       | 6 ++----
> >>>>>  drivers/tty/serial/samsung_tty.c    | 6 ++----
> >>>>>  drivers/tty/serial/serial-tegra.c   | 6 ++----
> >>>>>  drivers/tty/serial/sh-sci.c         | 9 +++------
> >>>>>  drivers/tty/serial/sprd_serial.c    | 3 +--
> >>>>>  drivers/tty/serial/stm32-usart.c    | 6 ++----
> >>>>>  15 files changed, 29 insertions(+), 58 deletions(-)
> >>>>
> >>>> Reviewed-by: Thierry Reding <treding@nvidia.com>
> >>>
> >>> Krzysztof, now look at this case. I would argue that this is even less fit than
> >>> # subpart(s) case, because it might imply anything. Again the problem is the
> >>> bundle, and for such a case (when tons of independent changes are bundled in
> >>> the same stuff, *every* Rb tag has to be explained on what has been reviewed!
> >>
> >> I think this case is obvious - Thierry reviewed this patch and gives Rb
> >> tag, according to reviewer's statement of oversight.
> > 
> > It's unclear from a bare tag, It misses that kind of information. Documentation
> > is also unclear to me as it does *not* describe the case what should I do when
> 
> Documentation is clear and I quote to you multiple times, to which you
> did not respond.
> 
> "Reviewed-by:, instead, indicates that *the patch* has been reviewed and
> found acceptable according to the Reviewer's Statement:"
> 
> Important: "THE PATCH"
> 
> Further:
> 
> "I state that:
> (a) I have carried out a technical review of *this patch* to evaluate
> its appropriateness and readiness for inclusion into  the mainline kernel."
> 
> And further:
> 
> "A Reviewed-by tag is a statement of opinion that *the patch* is an"
> 
> All of these are exhaustive (as in complete) statements leaving no
> interpretation whether this is part of patch or entire patch. It is THE
> patch, meaning entire patch.
> 
> You do not drive your car, as only a wheel. You driver *the car*.
> 
> You live in some apartment. *The apartment* is your home, not "piece of
> your apartment is your home".

And I'm trying to explain my point: "the car" in this case is actually "the cars"!
I drive mine, you driver yours. "The PATCH" is wrong term when it's bundled.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/1] tty: serial: use dmaengine_desc_set_callback()

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2239 bytes --]

On Thu, Nov 05, 2026 at 04:17:53PM +0200, Andy Shevchenko wrote:
> On Thu, Nov 05, 2026 at 01:28:46PM +0100, Thierry Reding wrote:
> > On Mon, Nov 02, 2026 at 12:54:48PM -0500, Frank.Li@oss.nxp.com wrote:
> > > 
> > > Replace open-coded assignments to callback and callback_param fields
> > > of struct dma_async_tx_descriptor with calls to the
> > > dmaengine_desc_set_callback() helper across serial drivers.
> > > 
> > > No functional change intended. Prepare to make struct
> > > dma_async_tx_descriptor private to the DMA engine.
> > > 
> > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > ---
> > >  drivers/tty/serial/8250/8250_dma.c  | 6 ++----
> > >  drivers/tty/serial/8250/8250_mtk.c  | 3 +--
> > >  drivers/tty/serial/8250/8250_omap.c | 6 ++----
> > >  drivers/tty/serial/amba-pl011.c     | 6 ++----
> > >  drivers/tty/serial/atmel_serial.c   | 6 ++----
> > >  drivers/tty/serial/fsl_lpuart.c     | 6 ++----
> > >  drivers/tty/serial/imx.c            | 6 ++----
> > >  drivers/tty/serial/msm_serial.c     | 6 ++----
> > >  drivers/tty/serial/mxs-auart.c      | 6 ++----
> > >  drivers/tty/serial/pch_uart.c       | 6 ++----
> > >  drivers/tty/serial/samsung_tty.c    | 6 ++----
> > >  drivers/tty/serial/serial-tegra.c   | 6 ++----
> > >  drivers/tty/serial/sh-sci.c         | 9 +++------
> > >  drivers/tty/serial/sprd_serial.c    | 3 +--
> > >  drivers/tty/serial/stm32-usart.c    | 6 ++----
> > >  15 files changed, 29 insertions(+), 58 deletions(-)
> > 
> > Reviewed-by: Thierry Reding <treding@nvidia.com>
> 
> Krzysztof, now look at this case. I would argue that this is even less fit than
> # subpart(s) case, because it might imply anything. Again the problem is the
> bundle, and for such a case (when tons of independent changes are bundled in
> the same stuff, *every* Rb tag has to be explained on what has been reviewed!

It may be a bit subtle, but the placement of the tag indicates that it
applies to the entirety of the changes. I didn't crop the list of
changed files, nor did I omit the summary.

That bit of meta-data obviously isn't picked up into the tag line, but
I think the default meaning of Reviewed-by is that it applies to the
entire thing.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/1] tty: serial: use dmaengine_desc_set_callback()

[Andy Shevchenko]

On Thu, Nov 05, 2026 at 04:36:38PM +0100, Thierry Reding wrote:
> On Thu, Nov 05, 2026 at 04:17:53PM +0200, Andy Shevchenko wrote:
> > On Thu, Nov 05, 2026 at 01:28:46PM +0100, Thierry Reding wrote:
> > > On Mon, Nov 02, 2026 at 12:54:48PM -0500, Frank.Li@oss.nxp.com wrote:
> > > > 
> > > > Replace open-coded assignments to callback and callback_param fields
> > > > of struct dma_async_tx_descriptor with calls to the
> > > > dmaengine_desc_set_callback() helper across serial drivers.
> > > > 
> > > > No functional change intended. Prepare to make struct
> > > > dma_async_tx_descriptor private to the DMA engine.
> > > > 
> > > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > > ---
> > > >  drivers/tty/serial/8250/8250_dma.c  | 6 ++----
> > > >  drivers/tty/serial/8250/8250_mtk.c  | 3 +--
> > > >  drivers/tty/serial/8250/8250_omap.c | 6 ++----
> > > >  drivers/tty/serial/amba-pl011.c     | 6 ++----
> > > >  drivers/tty/serial/atmel_serial.c   | 6 ++----
> > > >  drivers/tty/serial/fsl_lpuart.c     | 6 ++----
> > > >  drivers/tty/serial/imx.c            | 6 ++----
> > > >  drivers/tty/serial/msm_serial.c     | 6 ++----
> > > >  drivers/tty/serial/mxs-auart.c      | 6 ++----
> > > >  drivers/tty/serial/pch_uart.c       | 6 ++----
> > > >  drivers/tty/serial/samsung_tty.c    | 6 ++----
> > > >  drivers/tty/serial/serial-tegra.c   | 6 ++----
> > > >  drivers/tty/serial/sh-sci.c         | 9 +++------
> > > >  drivers/tty/serial/sprd_serial.c    | 3 +--
> > > >  drivers/tty/serial/stm32-usart.c    | 6 ++----
> > > >  15 files changed, 29 insertions(+), 58 deletions(-)
> > > 
> > > Reviewed-by: Thierry Reding <treding@nvidia.com>
> > 
> > Krzysztof, now look at this case. I would argue that this is even less fit than
> > # subpart(s) case, because it might imply anything. Again the problem is the
> > bundle, and for such a case (when tons of independent changes are bundled in
> > the same stuff, *every* Rb tag has to be explained on what has been reviewed!
> 
> It may be a bit subtle, but the placement of the tag indicates that it
> applies to the entirety of the changes. I didn't crop the list of
> changed files, nor did I omit the summary.

This doesn't clarify the question. Wordless Rb tag on a bundles is an oddity.

> That bit of meta-data obviously isn't picked up into the tag line, but
> I think the default meaning of Reviewed-by is that it applies to the
> entire thing.

And I interpret it otherwise, because this is a set of independent changes
bundled into a single patch!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/1] tty: serial: use dmaengine_desc_set_callback()

[Krzysztof Kozlowski]

On 05/11/2026 17:18, Andy Shevchenko wrote:
> On Thu, Nov 05, 2026 at 04:36:38PM +0100, Thierry Reding wrote:
>> On Thu, Nov 05, 2026 at 04:17:53PM +0200, Andy Shevchenko wrote:
>>> On Thu, Nov 05, 2026 at 01:28:46PM +0100, Thierry Reding wrote:
>>>> On Mon, Nov 02, 2026 at 12:54:48PM -0500, Frank.Li@oss.nxp.com wrote:
>>>>>
>>>>> Replace open-coded assignments to callback and callback_param fields
>>>>> of struct dma_async_tx_descriptor with calls to the
>>>>> dmaengine_desc_set_callback() helper across serial drivers.
>>>>>
>>>>> No functional change intended. Prepare to make struct
>>>>> dma_async_tx_descriptor private to the DMA engine.
>>>>>
>>>>> Signed-off-by: Frank Li <Frank.Li@nxp.com>
>>>>> ---
>>>>>  drivers/tty/serial/8250/8250_dma.c  | 6 ++----
>>>>>  drivers/tty/serial/8250/8250_mtk.c  | 3 +--
>>>>>  drivers/tty/serial/8250/8250_omap.c | 6 ++----
>>>>>  drivers/tty/serial/amba-pl011.c     | 6 ++----
>>>>>  drivers/tty/serial/atmel_serial.c   | 6 ++----
>>>>>  drivers/tty/serial/fsl_lpuart.c     | 6 ++----
>>>>>  drivers/tty/serial/imx.c            | 6 ++----
>>>>>  drivers/tty/serial/msm_serial.c     | 6 ++----
>>>>>  drivers/tty/serial/mxs-auart.c      | 6 ++----
>>>>>  drivers/tty/serial/pch_uart.c       | 6 ++----
>>>>>  drivers/tty/serial/samsung_tty.c    | 6 ++----
>>>>>  drivers/tty/serial/serial-tegra.c   | 6 ++----
>>>>>  drivers/tty/serial/sh-sci.c         | 9 +++------
>>>>>  drivers/tty/serial/sprd_serial.c    | 3 +--
>>>>>  drivers/tty/serial/stm32-usart.c    | 6 ++----
>>>>>  15 files changed, 29 insertions(+), 58 deletions(-)
>>>>
>>>> Reviewed-by: Thierry Reding <treding@nvidia.com>
>>>
>>> Krzysztof, now look at this case. I would argue that this is even less fit than
>>> # subpart(s) case, because it might imply anything. Again the problem is the
>>> bundle, and for such a case (when tons of independent changes are bundled in
>>> the same stuff, *every* Rb tag has to be explained on what has been reviewed!
>>
>> It may be a bit subtle, but the placement of the tag indicates that it
>> applies to the entirety of the changes. I didn't crop the list of
>> changed files, nor did I omit the summary.
> 
> This doesn't clarify the question. Wordless Rb tag on a bundles is an oddity.

No, it's perfectly normal and typical. I would not expect anything else,
because writing "LGTM" is pointless. We are experienced reviewers and we
do understand what Reviewer's statement of oversight means, thus giving
the tag is obvious, explicit and not odd.

> 
>> That bit of meta-data obviously isn't picked up into the tag line, but
>> I think the default meaning of Reviewed-by is that it applies to the
>> entire thing.
> 
> And I interpret it otherwise, because this is a set of independent changes
> bundled into a single patch!

It's not set of independent changes, but even if it was then it would
not matter. We review the code regardless how we actually judge the
atomicity of a patch.

Best regards,
Krzysztof

Re: [PATCH 1/1] tty: serial: use dmaengine_desc_set_callback()

[Andy Shevchenko]

On Thu, Nov 05, 2026 at 05:21:06PM +0100, Krzysztof Kozlowski wrote:
> On 05/11/2026 17:18, Andy Shevchenko wrote:
> > On Thu, Nov 05, 2026 at 04:36:38PM +0100, Thierry Reding wrote:
> >> On Thu, Nov 05, 2026 at 04:17:53PM +0200, Andy Shevchenko wrote:
> >>> On Thu, Nov 05, 2026 at 01:28:46PM +0100, Thierry Reding wrote:
> >>>> On Mon, Nov 02, 2026 at 12:54:48PM -0500, Frank.Li@oss.nxp.com wrote:
> >>>>>
> >>>>> Replace open-coded assignments to callback and callback_param fields
> >>>>> of struct dma_async_tx_descriptor with calls to the
> >>>>> dmaengine_desc_set_callback() helper across serial drivers.
> >>>>>
> >>>>> No functional change intended. Prepare to make struct
> >>>>> dma_async_tx_descriptor private to the DMA engine.
> >>>>>
> >>>>> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> >>>>> ---
> >>>>>  drivers/tty/serial/8250/8250_dma.c  | 6 ++----
> >>>>>  drivers/tty/serial/8250/8250_mtk.c  | 3 +--
> >>>>>  drivers/tty/serial/8250/8250_omap.c | 6 ++----
> >>>>>  drivers/tty/serial/amba-pl011.c     | 6 ++----
> >>>>>  drivers/tty/serial/atmel_serial.c   | 6 ++----
> >>>>>  drivers/tty/serial/fsl_lpuart.c     | 6 ++----
> >>>>>  drivers/tty/serial/imx.c            | 6 ++----
> >>>>>  drivers/tty/serial/msm_serial.c     | 6 ++----
> >>>>>  drivers/tty/serial/mxs-auart.c      | 6 ++----
> >>>>>  drivers/tty/serial/pch_uart.c       | 6 ++----
> >>>>>  drivers/tty/serial/samsung_tty.c    | 6 ++----
> >>>>>  drivers/tty/serial/serial-tegra.c   | 6 ++----
> >>>>>  drivers/tty/serial/sh-sci.c         | 9 +++------
> >>>>>  drivers/tty/serial/sprd_serial.c    | 3 +--
> >>>>>  drivers/tty/serial/stm32-usart.c    | 6 ++----
> >>>>>  15 files changed, 29 insertions(+), 58 deletions(-)
> >>>>
> >>>> Reviewed-by: Thierry Reding <treding@nvidia.com>
> >>>
> >>> Krzysztof, now look at this case. I would argue that this is even less fit than
> >>> # subpart(s) case, because it might imply anything. Again the problem is the
> >>> bundle, and for such a case (when tons of independent changes are bundled in
> >>> the same stuff, *every* Rb tag has to be explained on what has been reviewed!
> >>
> >> It may be a bit subtle, but the placement of the tag indicates that it
> >> applies to the entirety of the changes. I didn't crop the list of
> >> changed files, nor did I omit the summary.
> > 
> > This doesn't clarify the question. Wordless Rb tag on a bundles is an oddity.
> 
> No, it's perfectly normal and typical. I would not expect anything else,
> because writing "LGTM" is pointless. We are experienced reviewers and we
> do understand what Reviewer's statement of oversight means, thus giving
> the tag is obvious, explicit and not odd.
> 
> > 
> >> That bit of meta-data obviously isn't picked up into the tag line, but
> >> I think the default meaning of Reviewed-by is that it applies to the
> >> entire thing.
> > 
> > And I interpret it otherwise, because this is a set of independent changes
> > bundled into a single patch!
> 
> It's not set of independent changes, but even if it was then it would
> not matter. We review the code regardless how we actually judge the
> atomicity of a patch.

And I confront exactly that! When the patch may be split it has to. This is
seems the outcome of this discussion to me. From now on I think I will NAK
any bundle patch until piece that I in charge to is split from the bundle.

-- 
With Best Regards,
Andy Shevchenko