Devicetree
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: erebion <erebion@erebion.eu>
Cc: devicetree@vger.kernel.org, conor+dt@kernel.org, robh@kernel.org,
	dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 5/8] arm64: dts: qcom: sm6350-lena: Enable Bluetooth
Date: Sat, 26 Sep 2026 10:33:18 +0000	[thread overview]
Message-ID: <20260926103318.B09C51F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260926-pdx213-upstream-3-v1-5-1609c18b5c1d@erebion.eu>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dts: silent pinctrl override failures on UART pins due to missing /delete-property/
--

commit bed8cbd842a7138947e9a27eb01373050b147335
Author: erebion <erebion@erebion.eu>

arm64: dts: qcom: sm6350-lena: Enable Bluetooth
    
Configure the required regulators for Bluetooth and enable it on the
Xperia 10 III.

> diff --git a/arch/arm64/boot/dts/qcom/sm6350-sony-xperia-lena-pdx213.dts b/arch/arm64/boot/dts/qcom/sm6350-sony-xperia-lena-pdx213.dts
> index 146c7c5c45ea2..cb749cc121abf 100644
> --- a/arch/arm64/boot/dts/qcom/sm6350-sony-xperia-lena-pdx213.dts
> +++ b/arch/arm64/boot/dts/qcom/sm6350-sony-xperia-lena-pdx213.dts
[ ... ]
> @@ -392,6 +396,40 @@ &qupv3_id_1 {
>  	status = "okay";
>  };
>  
> +/* UART1: HS UART for Bluetooth */
> +&qup_uart1_cts {
> +	/*
> +	 * Configure a bias-bus-hold on CTS to lower power
> +	 * usage when Bluetooth is turned off. Bus hold will
> +	 * maintain a low power state regardless of whether
> +	 * the Bluetooth module drives the pin in either
> +	 * direction or leaves the pin fully unpowered.
> +	 */
> +	bias-bus-hold;
> +};

[Severity: Medium]
Since device tree overlays merge boolean properties rather than replacing them,
both the new and inherited properties will be present in the node.

The generic pinctrl parser evaluates properties in a fixed order, which causes
the inherited bias-disable to be evaluated after bias-bus-hold. Does this need
an explicit /delete-property/ bias-disable; to prevent the bus-hold from
being silently ignored and causing power leakage?

> +
> +&qup_uart1_rts {
> +	/* We'll drive RTS, so no pull */
> +	drive-strength = <2>;
> +	bias-disable;
> +};

[Severity: Medium]
Will this silently leave the pull-down enabled?

Similar to the CTS pin above, if the base node has bias-pull-down, merging
bias-disable doesn't remove it. The parser will evaluate the inherited
bias-pull-down after bias-disable, causing the pull to remain active. 
Should this include /delete-property/ bias-pull-down; to ensure the bias
is actually disabled?

> +
> +&qup_uart1_rx {
> +	/*
> +	 * Configure a pull-up on RX. This is needed to avoid
> +	 * garbage data when the TX pin of the Bluetooth module is
> +	 * in tri-state (module powered off or not driving the
> +	 * signal yet).
> +	 */
> +	bias-pull-up;
> +};
> +
> +&qup_uart1_tx {
> +	/* We'll drive TX, so no pull */
> +	drive-strength = <2>;
> +	bias-disable;
> +};

[Severity: Medium]
Does this face the same property merging issue with the inherited pull-up?

If the base node defines bias-pull-up, it will be evaluated after bias-disable,
silently overriding it. Should this include /delete-property/ bias-pull-up;
to ensure the pin bias is disabled as intended?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260926-pdx213-upstream-3-v1-0-1609c18b5c1d@erebion.eu?part=5

  reply	other threads:[~2026-09-26 10:33 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-26 10:20 [PATCH 0/8] Enable more nodes for the Xperia 10 III erebion
2026-09-26 10:20 ` [PATCH 1/8] arm64: dts: qcom: sm6350-lena: Add flash LED erebion
2026-09-26 10:28   ` sashiko-bot
2026-09-26 10:20 ` [PATCH 2/8] arm64: dts: qcom: sm6350-lena: Add qcom,rmtfs-mem node erebion
2026-09-26 10:20 ` [PATCH 3/8] arm64: dts: qcom: sm6350-lena: Enable ADSP, CDSP & MPSS erebion
2026-09-26 10:31   ` sashiko-bot
2026-09-26 10:20 ` [PATCH 4/8] arm64: dts: qcom: sm6350-lena: Enable IPA erebion
2026-09-26 10:20 ` [PATCH 5/8] arm64: dts: qcom: sm6350-lena: Enable Bluetooth erebion
2026-09-26 10:33   ` sashiko-bot [this message]
2026-09-26 10:20 ` [PATCH 6/8] arm64: dts: qcom: sm6350-lena: Enable wifi erebion
2026-09-26 10:20 ` [PATCH 7/8] arm64: dts: qcom: sm6350-lena: Enable PM7250B erebion
2026-09-26 10:20 ` [PATCH 8/8] arm64: dts: qcom: sm6350-lena: Enable PMK8350 erebion
2026-09-26 10:30   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260926103318.B09C51F000FF@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=erebion@erebion.eu \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox