Re: [PATCH RESEND v4 2/3] arm64: dts: qcom: Add sm8250-oneplus-common.dtsi

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] arm64: dts: qcom: duplicate KEY_VOLUMEDOWN on gpio-keys and pon_resin
- [Low] arm64: dts: qcom: unused vreg_s6c_0p88 regulator in sm8250-oneplus-common
--

commit 564ff98ec1e681e7bcc2a9e910efbd58280623b5
Author: Frieder Hannenheim <git@fhannenheim.net>

arm64: dts: qcom: Add sm8250-oneplus-common.dtsi

This commit adds the common device tree include for OnePlus devices
based on the Qualcomm SM8250 SoC.

> diff --git a/arch/arm64/boot/dts/qcom/sm8250-oneplus-common.dtsi b/arch/arm64/boot/dts/qcom/sm8250-oneplus-common.dtsi
> new file mode 100644
> index 0000000000000..6d7793c792f05
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sm8250-oneplus-common.dtsi

[ ... ]

> +	vreg_s6c_0p88: smpc6-regulator {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vreg_s6c_0p88";
> +		regulator-min-microvolt = <880000>;
> +		regulator-max-microvolt = <880000>;
> +		regulator-always-on;
> +		vin-supply = <&vph_pwr>;
> +	};
> +};
> +
> +&apps_rsc {
> +	regulators-0 {
> +		compatible = "qcom,pm8150-rpmh-regulators";
> +		qcom,pmic-id = "a";
> +		vdd-s1-supply = <&vph_pwr>;
> +		vdd-s2-supply = <&vph_pwr>;
> +		vdd-s3-supply = <&vph_pwr>;
> +		vdd-s4-supply = <&vph_pwr>;
> +		vdd-s5-supply = <&vph_pwr>;
> +		vdd-s6-supply = <&vph_pwr>;
> +		vdd-s7-supply = <&vph_pwr>;
> +		vdd-s8-supply = <&vph_pwr>;
> +		vdd-s9-supply = <&vph_pwr>;
> +		vdd-s10-supply = <&vph_pwr>;
> +		vdd-l2-l10-supply = <&vreg_bob>;
> +		vdd-l3-l4-l5-l18-supply = <&vreg_s6a_0p95>;

[Severity: Low]
Is vreg_s6c_0p88 intended to be connected to regulators-0 via
vdd-l1-l8-l11-supply = <&vreg_s6c_0p88>, or can smpc6-regulator be dropped?

Right now vreg_s6c_0p88 is defined in the root node, but nothing in
sm8250-oneplus-common.dtsi or sm8250-oneplus-kebab.dts references it.

[ ... ]

> +&pm8150_gpios {
> +	vol_up_n: vol-up-n-state {
> +		pins = "gpio6";
> +		function = "normal";
> +		power-source = <1>;
> +		input-enable;
> +		bias-pull-up;
> +	};
> +
> +	vol_down_l: vol-down-l-state {
> +		pins = "gpio7";
> +		function = "normal";
> +		power-source = <1>;
> +		input-enable;
> +		bias-pull-up;
> +	};
> +};
> +
> +&pon {
> +	mode-bootloader = <0x2>;
> +	mode-recovery = <0x1>;
> +};
> +
> +&pon_pwrkey {
> +	status = "okay";
> +};
> +
> +&pon_resin {
> +	linux,code = <KEY_VOLUMEDOWN>;
> +
> +	status = "okay";
> +};

[Severity: Medium]
As reported by Sashiko on v3 [1], why is KEY_VOLUMEDOWN defined in both
gpio_keys (on pm8150_gpios 7) and pon_resin?

In gpio_keys, key-vol-down configures pm8150_gpios 7 with KEY_VOLUMEDOWN:

	gpio_keys: gpio-keys {
		...
		pinctrl-0 = <&vol_down_l &vol_up_n>;
		pinctrl-names = "default";

		key-vol-down {
			label = "Volume Down";
			linux,code = <KEY_VOLUMEDOWN>;
			gpios = <&pm8150_gpios 7 GPIO_ACTIVE_LOW>;
			debounce-interval = <15>;
			linux,can-disable;
			wakeup-source;
		};

At the same time, pon_resin is enabled with KEY_VOLUMEDOWN. On SM8250 with
PM8150, Volume Down is normally routed to PON RESIN_N, and
Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml along with
drivers/pinctrl/qcom/pinctrl-spmi-gpio.c list gpio7 as a pin hole on
qcom,pm8150-gpio.

Should key-vol-down and vol_down_l be removed so that Volume Down is only
handled by pon_resin?

[1] https://lore.kernel.org/r/20261017-kebab_dts-v3-2-5fc3ca52a383@fhannenheim.net

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105-kebab_dts-v4-0-c6bbe6ded92e@fhannenheim.net?part=2