Re: [PATCH 2/4] arm64: dts: qcom: sm8450-hdk: Enable I2S for HDMI

[Neil Armstrong <neil.armstrong@linaro.org>]

On 7/28/26 11:10, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] Device Tree Misconfiguration - Pinctrl for SoC I2S interface is incorrectly assigned to the external HDMI bridge node.
> - [Low] DT Schema Violation - Invalid clock-names order for dai@16.
> --
> 
> --- Patch [2]: [PATCH 2/4] arm64: dts: qcom: sm8450-hdk: Enable I2S for HDMI ---
> commit bf4f513e744f0c756ca7054358cd98929fb84150
> Author: Neil Armstrong <neil.armstrong@linaro.org>
> 
> arm64: dts: qcom: sm8450-hdk: Enable I2S for HDMI
> 
> This commit adds the necessary device tree nodes and configurations to enable
> the I2S interface for HDMI audio output on the SM8450-HDK board. It configures
> the pinctrl for I2S, sets up the q6apmbedai DAI, and adds the primary MI2S DAI
> link.
> 
>> diff --git a/arch/arm64/boot/dts/qcom/sm8450-hdk.dts b/arch/arm64/boot/dts/qcom/sm8450-hdk.dts
>> index b37998cd9a2c3..6a6d8346046bd 100644
>> --- a/arch/arm64/boot/dts/qcom/sm8450-hdk.dts
>> +++ b/arch/arm64/boot/dts/qcom/sm8450-hdk.dts
>> @@ -665,7 +665,12 @@ lt9611_codec: hdmi-bridge@2b {
>>   		vcc-supply = <&lt9611_3v3>;
>>   
>>   		pinctrl-names = "default";
>> -		pinctrl-0 = <&lt9611_irq_pin &lt9611_rst_pin>;
>> +		pinctrl-0 = <&lt9611_irq_pin>,
>> +			    <&lt9611_rst_pin>,
>> +			    <&i2s0_default_state>,
>> +			    <&audio_mclk0_default_state>;
> 
> [Severity: Medium]
> Should the SoC I2S and MCLK pin states be assigned to the sound node or CPU
> DAI instead of the external HDMI bridge node?
> 
> By assigning these to the lt9611_codec node, the SoC's audio pin multiplexing
> is tied to the lifecycle of the external display bridge. If the HDMI bridge
> driver defers or unloads, the SoC I2S pins will revert to their unconfigured
> state, which breaks the hardware description paradigm and might cause runtime
> audio issues.

No since the HDMI bridge is the effective user of the pins.

> 
>> +
>> +		#sound-dai-cells = <1>;
>>   
>>   		ports {
>>   			#address-cells = <1>;
> 
> [ ... ]
> 
>> @@ -947,6 +952,19 @@ channel@44b {
>>   	};
>>   };
>>   
>> +&q6apmbedai {
>> +	#address-cells = <1>;
>> +	#size-cells = <0>;
>> +
>> +	dai@16 {
>> +		reg = <PRIMARY_MI2S_RX>;
>> +		clocks = <&q6prmcc LPASS_CLK_ID_MCLK_1 LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
>> +			 <&q6prmcc LPASS_CLK_ID_PRI_MI2S_IBIT LPASS_CLK_ATTRIBUTE_COUPLE_NO>;
>> +		clock-names = "mclk",
>> +			      "bclk";
> 
> [Severity: Low]
> Is this clock-names order correct according to the schema?
> 
> The qcom,q6apm-lpass-dais.yaml schema requires that if two clocks are
> provided, they must be ordered such that index 1 is mclk. Here, bclk is at
> index 1, which will cause make dtbs_check validation to fail.

Good point, I'll fix this

> 
>> +	};
>> +};
>> +
>