Re: [PATCH v3 4/4] arm64: dts: qcom: sm8550: add UART11 node

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Hardware/Device Tree inconsistency: The uart11 node references the wrong operating-points-v2 table.
- [Medium] Missing Pinctrl State: The `qup_uart11_cts_rts` state is omitted, forcing boards to open-code it.
- [Medium] Incorrect Pin Bias: The `qup_uart11_default` pinctrl state configures pins with `bias-disable` instead of `bias-pull-up`.
--

commit 3269a824736c1b0b41ba9f1674a29c444d6c2b01
Author: Alexandre Hamamdjian <azkali.limited@gmail.com>

arm64: dts: qcom: sm8550: add UART11 node

This commit adds the QUPv3_2 SE3 High Speed UART (UART11) controller node
and its default pinctrl state to the sm8550 device tree.

> diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi b/arch/arm64/boot/dts/qcom/sm8550.dtsi
> index a9c678fc9cb2e..3e71701b18ff0 100644
> --- a/arch/arm64/boot/dts/qcom/sm8550.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi
> @@ -1084,6 +1084,24 @@ &config_noc SLAVE_QUP_2 QCOM_ICC_TAG_ACTIVE_ONLY>,
>  				status = "disabled";
>  			};
>  
> +			uart11: serial@88c000 {
> +				compatible = "qcom,geni-uart";
> +				reg = <0 0x0088c000 0 0x4000>;
> +				clock-names = "se";
> +				clocks = <&gcc GCC_QUPV3_WRAP2_S3_CLK>;
> +				interrupts = <GIC_SPI 585 IRQ_TYPE_LEVEL_HIGH 0>;
> +				pinctrl-names = "default";
> +				pinctrl-0 = <&qup_uart11_default>;
> +				interconnects = <&clk_virt MASTER_QUP_CORE_2 QCOM_ICC_TAG_ALWAYS
> +						 &clk_virt SLAVE_QUP_CORE_2 QCOM_ICC_TAG_ALWAYS>,
> +						<&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ACTIVE_ONLY
> +						 &config_noc SLAVE_QUP_2 QCOM_ICC_TAG_ACTIVE_ONLY>;
> +				interconnect-names = "qup-core", "qup-config";
> +				operating-points-v2 = <&qup_opp_table_100mhz>;

Is this the correct OPP table for this Serial Engine block?

The sibling spi11 and i2c11 nodes, which represent the same physical SE block
at the same address, use operating-points-v2 = <&qup_opp_table_120mhz>;.

A mismatch could lead to the UART mode voting for incorrect voltages or
failing to reach required frequencies.

> +				power-domains = <&rpmhpd RPMHPD_CX>;
> +				status = "disabled";
> +			};
> +

[ ... ]

> @@ -5102,6 +5120,14 @@ qup_uart7_default: qup-uart7-default-state {
>  				bias-disable;
>  			};
>  
> +			qup_uart11_default: qup-uart11-default-state {
> +				/* TX, RX */
> +				pins = "gpio70", "gpio71";
> +				function = "qup2_se3";
> +				drive-strength = <2>;
> +				bias-disable;

Should these pins be configured with a pull-up?

Other high-speed UARTs on this SoC (like qup_uart14_default) typically use
bias-pull-up for TX/RX lines. Using bias-disable deviates from the
established pattern for high-speed UARTs and could cause signal integrity
issues on boards lacking external pull-up resistors.

> +			};
> +

Could a qup_uart11_cts_rts state be added here as well?

The commit message states the goal is to prevent boards from open-coding the
controller configuration. Since high-speed UARTs often require hardware flow
control, boards that need it will still have to open-code the CTS/RTS pins
(gpio68 and gpio69) if the state is omitted from this file.

>  			qup_uart14_default: qup-uart14-default-state {
>  				/* TX, RX */
>  				pins = "gpio78", "gpio79";

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260517-pocketds-v3-0-d5910c801756@gmail.com?part=4