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

[Krzysztof Kozlowski <krzk@kernel.org>]

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