Re: [PATCH 1/6] arm64: dts: qcom: sc8280xp-x13s: disable soundcard

[Krzysztof Kozlowski]

On 02/01/2023 16:07, Johan Hovold wrote:
> On Mon, Jan 02, 2023 at 01:25:38PM +0100, Krzysztof Kozlowski wrote:
>> On 02/01/2023 11:50, Johan Hovold wrote:
>>> Driver support for the X13s soundcard is not yet in place so disable it
>>> for now to avoid probe failures such as:
>>>
>>> [   11.077727] qcom-prm gprsvc:service:2:2: DSP returned error[100100f] 1
>>> [   11.077926] rx_macro: probe of 3200000.rxmacro failed with error -22
>>> [   21.221104] platform 3210000.soundwire-controller: deferred probe pending
>>>
>>> Signed-off-by: Johan Hovold <johan+linaro@xxxxxxxxxx>
>>> ---
>>>  .../boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts  | 12 ++++++++++--
>>>  1 file changed, 10 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
>>> index 0201c6776746..97ff74d5095e 100644
>>> --- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
>>> +++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
>>> @@ -649,6 +649,8 @@ wcd938x: codec {
>>>  		qcom,mbhc-headphone-vthreshold-microvolt = <50000>;
>>>  		qcom,rx-device = <&wcd_rx>;
>>>  		qcom,tx-device = <&wcd_tx>;
>>> +
>>> +		status = "disabled";
>>>  	};
>>>  };
>>>  
>>> @@ -669,6 +671,8 @@ &sound {
>>>  		"TX DMIC2", "MIC BIAS3",
>>>  		"TX SWR_ADC1", "ADC2_OUTPUT";
>>>  
>>> +	status = "disabled";
>>> +
>>>  	wcd-playback-dai-link {
>>>  		link-name = "WCD Playback";
>>>  		cpu {
>>> @@ -731,6 +735,8 @@ codec {
>>>  };
>>>  
>>>  &swr0 {
>>> +	status = "disabled";
>>> +
>>>  	left_spkr: wsa8830-left@0,1 {
>>>  		compatible = "sdw10217020200";
>>>  		reg = <0 1>;
>>> @@ -757,7 +763,7 @@ right_spkr: wsa8830-right@0,2{
>>>  };
>>>  
>>>  &swr1 {
>>> -	status = "okay";
>>> +	status = "disabled";
>>>  
>>>  	wcd_rx: wcd9380-rx@0,4 {
>>>  		compatible = "sdw20217010d00";
>>> @@ -767,7 +773,7 @@ wcd_rx: wcd9380-rx@0,4 {
>>>  };
>>>  
>>>  &swr2 {
>>> -	status = "okay";
>>> +	status = "disabled";
>>
>> That's a double disable.
>>
>>>  
>>>  	wcd_tx: wcd9380-tx@0,3 {
>>>  		compatible = "sdw20217010d00";
>>> @@ -781,6 +787,8 @@ &vamacro {
>>>  	pinctrl-names = "default";
>>>  	vdd-micb-supply = <&vreg_s10b>;
>>>  	qcom,dmic-sample-rate = <600000>;
>>> +
>>> +	status = "disabled";
>>
>> That's a double disable.
> 
> Yes, that's on purpose. We're temporarily disabling these nodes instead
> of reverting the series which should not have been merged.

I don't get why disabling something twice is anyhow related to
"temporarily disable". One disable is enough for temporary or permanent
disables.

> 
> Once we have driver support, these properties will be updated again.

Linux kernel is not the only consumer of DTS, thus having or not having
the support in the kernel is not reason to disable pieces of it.
Assuming the DTS is correct, of course, because maybe that's the problem?

Best regards,
Krzysztof