Re: [PATCH 2/2] arm64: dts: qcom: purwa-iot-evk: Add eDP display overlay

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `/delete-property/ #sound-dai-cells;` directive moved into the `.dtso` overlay is a silent no-op, falsely exposing an audio DAI.
- [Low] The `pmk8550_pwm` node is left enabled in the base DTS after its only consumer was moved to the overlay.
--

--- Patch [2]: [PATCH 2/2] arm64: dts: qcom: purwa-iot-evk: Add eDP display overlay ---
commit 28cbb74b5accd1eed8eb8f23fae1e5b1e310ff55
Author: Anand Tiwari <[email protected]>

arm64: dts: qcom: purwa-iot-evk: Add eDP display overlay
    
Move the eDP panel configuration and related power, backlight, and pinctrl
nodes into a separate overlay. Keep the base DTB suitable for headless
variants and provide a composite DTB for headed variants.

> diff --git a/arch/arm64/boot/dts/qcom/purwa-iot-evk-edp.dtso b/arch/arm64/boot/dts/qcom/purwa-iot-evk-edp.dtso
> new file mode 100644
> index 0000000000000..759d05342627f
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/purwa-iot-evk-edp.dtso

[ ... ]

> +&mdss_dp3 {
> +	/delete-property/ #sound-dai-cells;

[Severity: Medium]
Is this property deletion effective in an overlay?

The Device Tree Overlay standard and fdtoverlay mechanism do not natively
support property deletions at merge-time. When compiled with dtc, the
/delete-property/ directive is discarded because it has no target within the
local fragment scope.

Will this result in the mdss_dp3 node continuing to inherit the
#sound-dai-cells = <0> property from hamoa.dtsi, falsely exposing an audio
DAI to the audio framework when probed?

[ ... ]

> diff --git a/arch/arm64/boot/dts/qcom/purwa-iot-evk.dts b/arch/arm64/boot/dts/qcom/purwa-iot-evk.dts
> index ad503beec1d3d..6dde925025dd8 100644
> --- a/arch/arm64/boot/dts/qcom/purwa-iot-evk.dts
> +++ b/arch/arm64/boot/dts/qcom/purwa-iot-evk.dts

[ ... ]

> @@ -1169,13 +1076,6 @@ usb0_pwr_1p15_reg_en: usb0-pwr-1p15-reg-en-state {
>  	};
>  };
>  
> -&pmk8550_gpios {
> -	edp_bl_pwm: edp-bl-pwm-state {
> -		pins = "gpio5";
> -		function = "func3";
> -	};
> -};
> -
>  &pmk8550_pwm {
>  	status = "okay";
>  };

[Severity: Low]
Should the pmk8550_pwm node's status be removed or set to "disabled" in the
base device tree now that its consumer is moved?

Since the only consumer of this PWM (the eDP backlight) was moved to the
overlay, leaving it with status = "okay" here unnecessarily probes the
controller on headless variants of the EVK. 

The overlay also redundantly re-enables it.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2