Re: [PATCH 2/2] arm64: dts: qcom: oneplus-kebab: new device

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] dts: i2c2 controller left disabled preventing PMIC probe
- [Medium] dts: pm8008_default pinctrl state is defined but unreferenced
- [Low] dts: gpio-keys-state pinctrl is dead boilerplate
- [Low] dts: downstream qcom,kona root compatible string
--

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

arm64: dts: qcom: oneplus-kebab: new device

This commit adds the initial device-tree support for the OnePlus 8T. It
configures the framebuffer, battery, I2C devices including the PM8008 PMIC
and fuel gauge, and pin states.

Downstream reference: https://github.com/LineageOS/android_kernel_oneplus_sm8250/blob/lineage-23.2/arch/arm64/boot/dts/vendor/oplus/kona-kebab-overlay.dts

> diff --git a/arch/arm64/boot/dts/qcom/sm8250-oneplus-kebab.dts b/arch/arm64/boot/dts/qcom/sm8250-oneplus-kebab.dts
> new file mode 100644
> index 0000000000000..3e9802db1464b
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sm8250-oneplus-kebab.dts
> @@ -0,0 +1,124 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +/dts-v1/;
> +
> +#include "sm8250-oneplus-common.dtsi"
> +
> +/ {
> +	model = "OnePlus 8T";
> +	compatible = "oneplus,kebab", "qcom,sm8250", "qcom,kona";

[Severity: Low]
Does the inclusion of the downstream "qcom,kona" string here violate the
upstream devicetree bindings for the qcom,sm8250 SoC family? 

It appears this vendor code name is not recognized by upstream schemas.

> +};
> +

[ ... ]

> +&i2c2 {

[Severity: High]
Is the status = "okay"; property missing for this I2C controller?

Because it is disabled by default in the base sm8250.dtsi, without explicitly
enabling it, the bus and its child devices (like the PM8008 PMIC and its
regulators) will likely be ignored and not probed.

> +	pm8008: pmic@8 {
> +		compatible = "qcom,pm8008";
> +		reg = <0x8>;
> +
> +		interrupt-parent = <&tlmm>;
> +		interrupts = <112 IRQ_TYPE_EDGE_RISING>;
> +
> +		reset-gpios = <&tlmm 93 GPIO_ACTIVE_LOW>;

[Severity: Medium]
Should this consumer node reference the pm8008_default pinctrl state using
pinctrl-names and pinctrl-0 properties?

As it stands, the reset pin configuration (gpio93) defined in pm8008_default
won't be applied by the pinctrl subsystem, which might lead to PMIC probe
failure if the reset pin relies on the pull-up to avoid floating.

[ ... ]

> +		pinctrl {
> +			gpio-keys-state {
> +				pins = "gpio1";
> +				function = "normal";
> +			};
> +		};

[Severity: Low]
Is this gpio-keys-state referenced anywhere?

It looks like it might be an unused artifact copied from a DT binding example,
as it has no phandle label and isn't consumed by any other node.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015-kebab_dts-v1-0-1fee1204a18b@fhannenheim.net?part=2