Re: [PATCH v7 3/3] arm64: dts: qcom: Add hamoa Samsung Galaxy Book4 Edge devicetrees

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] dt-bindings: touchscreen@5d node missing interrupts property
- [Medium] sound: incorrect audio routing for right speakers to same macro output
- [Low] dt-bindings: invalid enable-active-low property in regulator-fixed node
- [Low] dt-bindings: pmic-glink missing qcom,sm8550-pmic-glink fallback string
--

commit 2d1c565b5f3cc1fc06395b66ffcf702541cf9a0d
Author: Maxim Storetvedt <mstoretv@cern.ch>

arm64: dts: qcom: Add hamoa Samsung Galaxy Book4 Edge devicetrees
    
This commit adds devicetrees for the 14-inch and 16-inch hamoa SKUs of the
Samsung Galaxy Book4 Edge. It uses a common dtsi derived from nodes that
were able to work on Linux.

Link: https://lore.kernel.org/all/p3mhtj2rp6y2ezuwpd2gu7dwx5cbckfu4s4pazcudi4j2wogtr@4yecb2bkeyms/
Link: https://lore.kernel.org/all/20260629154812.9066-1-mail@etehtsea.me/

> diff --git a/arch/arm64/boot/dts/qcom/x1-samsung-galaxy-book4-edge.dtsi b/arch/arm64/boot/dts/qcom/x1-samsung-galaxy-book4-edge.dtsi
> new file mode 100644
> index 0000000000000..51ec68bbab141
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/x1-samsung-galaxy-book4-edge.dtsi

[ ... ]

> +	pmic-glink {
> +		compatible = "qcom,x1e80100-pmic-glink",
> +			     "qcom,pmic-glink";

[Severity: Low]
Does this violate the pmic-glink devicetree schema? The schema requires that
"qcom,x1e80100-pmic-glink" must be followed by the "qcom,sm8550-pmic-glink"
fallback string.

Omitting the intermediate fallback will trigger dtbs_check warnings, even
though the driver matches "qcom,x1e80100-pmic-glink" directly.

[ ... ]

> +	sound {
> +		compatible = "qcom,x1e80100-sndcard";
> +		model = "X1E80100-GalaxyBook4Edge";
> +		audio-routing = "WooferLeft IN", "WSA WSA_SPK1 OUT",
> +				"TweeterLeft IN", "WSA WSA_SPK2 OUT",
> +				"WooferRight IN", "WSA2 WSA_SPK2 OUT",
> +				"TweeterRight IN", "WSA2 WSA_SPK2 OUT",

[Severity: Medium]
Is the right-side audio routing mapped correctly here? Both "WooferRight IN"
and "TweeterRight IN" are mapped to the same "WSA2 WSA_SPK2 OUT" macro
channel.

It appears "WooferRight IN" should map to "WSA2 WSA_SPK1 OUT" to maintain
discrete stereo/crossover separation for the right side, instead of routing
both to the same output channel.

[ ... ]

> +	vreg_edp_3p3: regulator-edp-3p3 {
> +		compatible = "regulator-fixed";
> +
> +		regulator-name = "VREG_EDP_3P3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +
> +		gpio = <&tlmm 70 GPIO_ACTIVE_LOW>;
> +		enable-active-low;

[Severity: Low]
Is it correct to include the enable-active-low property here?

The regulator-fixed bindings do not support this property (only
enable-active-high is valid). Active-low polarity is already correctly and
implicitly defined by the GPIO_ACTIVE_LOW flag in the gpio specifier. Leaving
this in will trigger a dtbs_check schema validation error.

[ ... ]

> +&i2c8 {
> +	clock-frequency = <400000>;
> +
> +	status = "okay";
> +
> +	touchscreen@5d {
> +		compatible = "hid-over-i2c";
> +		reg = <0x5d>;
> +
> +		hid-descr-addr = <0x1>;
> +		//interrupts-extended = <&tlmm 34 IRQ_TYPE_LEVEL_LOW>;
> +
> +		vdd-supply = <&vreg_misc_3p3>;
> +
> +		//pinctrl-0 = <&ts0_default>;
> +		//pinctrl-names = "default";
> +	};
> +};

[Severity: Medium]
Will this node fail to probe? The hid-over-i2c binding requires the
interrupts or interrupts-extended property.

Since interrupts-extended is commented out but the node remains active (no
status = "disabled"), the i2c-hid driver will unconditionally fail to probe
without an IRQ. This leaves the touchscreen non-functional and the devicetree
schema invalid. Should this node either be properly configured or marked
disabled?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260920184148.31541-1-mstoretv@cern.ch?part=3