Re: [PATCH v1 2/2] LRW UART: serial: add driver for the LRW UART

From: Krzysztof Kozlowski <krzk@kernel.org>
To: LiuQingtao <qtliu@mail.ustc.edu.cn>, gregkh@linuxfoundation.org
Cc: jirislaby@kernel.org, robh@kernel.org, krzk+dt@kernel.org,
	conor+dt@kernel.org, neil.armstrong@linaro.org,
	bjorn.andersson@oss.qualcomm.com, marex@nabladev.com,
	dev@kael-k.io, mani@kernel.org,
	prabhakar.mahadev-lad.rj@bp.renesas.com,
	linux-serial@vger.kernel.org, linux-riscv@lists.infradead.org,
	liu.wenhong35@zte.com.cn, liu.qingtao2@zte.com.cn,
	hu.yuye@zte.com.cn, dai.hualiang@zte.com.cn,
	deng.weixian@zte.com.cn, jia.yunxiang@zte.com.cn,
	bai.lu5@zte.com.cn, yang.susheng@zte.com.cn,
	shen.lin1@zte.com.cn, tan.hu@zte.com.cn, zuo.jiang@zte.com.cn
Subject: Re: [PATCH v1 2/2] LRW UART: serial: add driver for the LRW UART
Date: Fri, 13 Feb 2026 11:02:57 +0100	[thread overview]
Message-ID: <3387b1bd-c945-4b9d-915a-baf9412f656f@kernel.org> (raw)
In-Reply-To: <20260213093334.9217-3-qtliu@mail.ustc.edu.cn>

On 13/02/2026 10:33, LiuQingtao wrote:
> From: Wenhong Liu <liu.wenhong35@zte.com.cn>
> 
> This commit introduces a serial driver for the LRW UART controller

Please do not use "This commit/patch/change", but imperative mood. See
longer explanation here:
https://elixir.bootlin.com/linux/v6.16/source/Documentation/process/submitting-patches.rst#L94

> 
> Key features implemented:
> - Support for FIFO mode (16-byte depth)
> - Baud rate configuration
> - Standard asynchronous communication formats:
>   * Data bits: 5, 6, 7, 8, 9 bits
>   * Parity: odd, even, fixed, none
>   * Stop bits: 1 or 2 bits
> - Hardware flow control (RTS/CTS)
> - Multiple interrupt reporting mechanisms
> 
> Signed-off-by: Wenhong Liu <liu.wenhong35@zte.com.cn>
> Signed-off-by: Qingtao Liu <liu.qingtao2@zte.com.cn>

Mismatched DCO/SoB.

Run checkpatch on your code.

> ---
>  MAINTAINERS                      |    3 +
>  drivers/tty/serial/Kconfig       |   33 +
>  drivers/tty/serial/Makefile      |    1 +
>  drivers/tty/serial/lrw_uart.c    | 2822 ++++++++++++++++++++++++++++++
>  include/uapi/linux/serial_core.h |    3 +
>  5 files changed, 2862 insertions(+)
>  create mode 100644 drivers/tty/serial/lrw_uart.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ad6acbe24544..a97fbd205f75 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15041,6 +15041,9 @@ R:	Qingtao Liu <liu.qingtao2@zte.com.cn>
>  L:	linux-serial@vger.kernel.org
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/serial/lrw,lrw-uart.yaml
> +F:	drivers/tty/serial/Kconfig
> +F:	drivers/tty/serial/Makefile

Why do you claim you maintain these files?

> +F:	drivers/tty/serial/lrw_uart.c
>  

...

> +
> +static int lrw_uart_probe(struct platform_device *pdev)
> +{
> +	struct lrw_uart_port *sup;
> +	struct resource *r;
> +	int portnr, ret;
> +	unsigned int clk;
> +	unsigned int baudrate;
> +
> +	/*
> +	 * Check the mandatory baud rate parameter in the DT node early
> +	 * so that we can easily exit with the error.
> +	 */
> +	if (pdev->dev.of_node) {
> +		struct device_node *np = pdev->dev.of_node;
> +
> +		ret = of_property_read_u32(np, "current-speed", &baudrate);

Test your code/DTS - there is no such property allowed and you would see
warnings on DTS.

Best regards,
Krzysztof

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv