diff mbox series

[1/4] ASoC: dt-bindings: wcd934x: add bindings for Headset Button detection

Message ID 20210510101201.7281-2-srinivas.kandagatla@linaro.org
State Changes Requested
Headers show
Series ASoC: codecs: wcd934x: add Headset and button detection support | expand

Checks

Context Check Description
robh/checkpatch warning total: 0 errors, 1 warnings, 77 lines checked
robh/dt-meta-schema success
robh/dtbs-check fail build log

Commit Message

Srinivas Kandagatla May 10, 2021, 10:11 a.m. UTC
Add bindings required for Multi Button Headset detection.
WCD934x support Headsets with upto 8 buttons including, impedance measurement
on both L/R Headset speakers and cross connection detection.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 .../bindings/sound/qcom,wcd934x.yaml          | 65 +++++++++++++++++++
 1 file changed, 65 insertions(+)

Comments

Rob Herring May 10, 2021, 4:45 p.m. UTC | #1
On Mon, May 10, 2021 at 11:11:58AM +0100, Srinivas Kandagatla wrote:
> Add bindings required for Multi Button Headset detection.
> WCD934x support Headsets with upto 8 buttons including, impedance measurement
> on both L/R Headset speakers and cross connection detection.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>  .../bindings/sound/qcom,wcd934x.yaml          | 65 +++++++++++++++++++
>  1 file changed, 65 insertions(+)

A search tells me this is not v1... 

> 
> diff --git a/Documentation/devicetree/bindings/sound/qcom,wcd934x.yaml b/Documentation/devicetree/bindings/sound/qcom,wcd934x.yaml
> index e8f716b5f875..b25c6ca4e97c 100644
> --- a/Documentation/devicetree/bindings/sound/qcom,wcd934x.yaml
> +++ b/Documentation/devicetree/bindings/sound/qcom,wcd934x.yaml
> @@ -77,6 +77,64 @@ properties:
>      minimum: 1800000
>      maximum: 2850000
>  
> +  qcom,hphl-jack-type-normally-closed:
> +    description: Indicates that HPHL jack switch type is normally closed
> +    type: boolean
> +
> +  qcom,ground-jack-type-normally-closed:
> +    description: Indicates that Headset Ground switch type is normally closed
> +    type: boolean

I asked before if 'normally closed' was the more common case and you 
said yes. So I'd expect 'open' here, but now you've changed the 
polarity of the property. And now not present is not described at all. 

> +
> +  qcom,mbhc-headset-vthreshold-microvolt:
> +    description: Voltage threshold value for headset detection
> +    minimum: 0
> +    maximum: 2850000
> +
> +  qcom,mbhc-headphone-vthreshold-microvolt:
> +    description: Voltage threshold value for headphone detection
> +    minimum: 0
> +    maximum: 2850000
> +
> +  qcom,mbhc-button0-vthreshold-microvolt:
> +    description: Voltage threshold value for headset button0
> +    minimum: 0
> +    maximum: 500000
> +
> +  qcom,mbhc-button1-vthreshold-microvolt:
> +    description: Voltage threshold value for headset button1
> +    minimum: 0
> +    maximum: 500000
> +
> +  qcom,mbhc-button2-vthreshold-microvolt:
> +    description: Voltage threshold value for headset button2
> +    minimum: 0
> +    maximum: 500000
> +
> +  qcom,mbhc-button3-vthreshold-microvolt:
> +    description: Voltage threshold value for headset button3
> +    minimum: 0
> +    maximum: 500000
> +
> +  qcom,mbhc-button4-vthreshold-microvolt:
> +    description: Voltage threshold value for headset button4
> +    minimum: 0
> +    maximum: 500000
> +
> +  qcom,mbhc-button5-vthreshold-microvolt:
> +    description: Voltage threshold value for headset button5
> +    minimum: 0
> +    maximum: 500000
> +
> +  qcom,mbhc-button6-vthreshold-microvolt:
> +    description: Voltage threshold value for headset button6
> +    minimum: 0
> +    maximum: 500000
> +
> +  qcom,mbhc-button7-vthreshold-microvolt:
> +    description: Voltage threshold value headset button7
> +    minimum: 0
> +    maximum: 500000

These can all be a single pattern property: 
'^qcom,mbhc-button[0-7]-vthreshold-microvolt$'

Are there inter-dependencies between these properties? Are 0-7 
meaningful or just an index? For the latter case, why not just make this 
an array?

> +
>    clock-output-names:
>      const: mclk
>  
> @@ -159,6 +217,13 @@ examples:
>          qcom,micbias2-microvolt = <1800000>;
>          qcom,micbias3-microvolt = <1800000>;
>          qcom,micbias4-microvolt = <1800000>;
> +        qcom,hphl-jack-type-normally-closed;
> +        qcom,ground-jack-type-normally-closed;
> +        qcom,mbhc-button0-vthreshold-microvolt = <75000>;
> +        qcom,mbhc-button1-vthreshold-microvolt = <150000>;
> +        qcom,mbhc-button2-vthreshold-microvolt = <237000>;
> +        qcom,mbhc-headset-vthreshold-microvolt = <1700000>;
> +        qcom,mbhc-headphone-vthreshold-microvolt = <50000>;
>          clock-names = "extclk";
>          clocks = <&rpmhcc 2>;
>  
> -- 
> 2.21.0
>
Srinivas Kandagatla May 11, 2021, 9:02 a.m. UTC | #2
On 10/05/2021 17:45, Rob Herring wrote:
> On Mon, May 10, 2021 at 11:11:58AM +0100, Srinivas Kandagatla wrote:
>> Add bindings required for Multi Button Headset detection.
>> WCD934x support Headsets with upto 8 buttons including, impedance measurement
>> on both L/R Headset speakers and cross connection detection.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> ---
>>   .../bindings/sound/qcom,wcd934x.yaml          | 65 +++++++++++++++++++
>>   1 file changed, 65 insertions(+)
> 
> A search tells me this is not v1...
> 

I did submit something for wcd9335 in the past but not for wcd934x which 
is still v1.

Most of these codecs have same MBHC block so the mbhc bindings look same 
across them.


>>
>> diff --git a/Documentation/devicetree/bindings/sound/qcom,wcd934x.yaml b/Documentation/devicetree/bindings/sound/qcom,wcd934x.yaml
>> index e8f716b5f875..b25c6ca4e97c 100644
>> --- a/Documentation/devicetree/bindings/sound/qcom,wcd934x.yaml
>> +++ b/Documentation/devicetree/bindings/sound/qcom,wcd934x.yaml
>> @@ -77,6 +77,64 @@ properties:
>>       minimum: 1800000
>>       maximum: 2850000
>>   
>> +  qcom,hphl-jack-type-normally-closed:
>> +    description: Indicates that HPHL jack switch type is normally closed
>> +    type: boolean
>> +
>> +  qcom,ground-jack-type-normally-closed:
>> +    description: Indicates that Headset Ground switch type is normally closed
>> +    type: boolean
> 
> I asked before if 'normally closed' was the more common case and you
> said yes. So I'd expect 'open' here, but now you've changed the
> polarity of the property. And now not present is not described at all.

Yes "normally closed" is the normal use case however Banajit commented 
on these binding with normally-open at https://lkml.org/lkml/2018/9/26/410

Ground pin may not be present on most of the headset jacks.
So we can not consider the headset jacks as Normally closed when this 
property is not specified in the device tree.

So having binding with normally-closed makes it much safer to use.

> 
>> +
>> +  qcom,mbhc-headset-vthreshold-microvolt:
>> +    description: Voltage threshold value for headset detection
>> +    minimum: 0
>> +    maximum: 2850000
>> +
>> +  qcom,mbhc-headphone-vthreshold-microvolt:
>> +    description: Voltage threshold value for headphone detection
>> +    minimum: 0
>> +    maximum: 2850000
>> +
>> +  qcom,mbhc-button0-vthreshold-microvolt:
>> +    description: Voltage threshold value for headset button0
>> +    minimum: 0
>> +    maximum: 500000
>> +
>> +  qcom,mbhc-button1-vthreshold-microvolt:
>> +    description: Voltage threshold value for headset button1
>> +    minimum: 0
>> +    maximum: 500000
>> +
>> +  qcom,mbhc-button2-vthreshold-microvolt:
>> +    description: Voltage threshold value for headset button2
>> +    minimum: 0
>> +    maximum: 500000
>> +
>> +  qcom,mbhc-button3-vthreshold-microvolt:
>> +    description: Voltage threshold value for headset button3
>> +    minimum: 0
>> +    maximum: 500000
>> +
>> +  qcom,mbhc-button4-vthreshold-microvolt:
>> +    description: Voltage threshold value for headset button4
>> +    minimum: 0
>> +    maximum: 500000
>> +
>> +  qcom,mbhc-button5-vthreshold-microvolt:
>> +    description: Voltage threshold value for headset button5
>> +    minimum: 0
>> +    maximum: 500000
>> +
>> +  qcom,mbhc-button6-vthreshold-microvolt:
>> +    description: Voltage threshold value for headset button6
>> +    minimum: 0
>> +    maximum: 500000
>> +
>> +  qcom,mbhc-button7-vthreshold-microvolt:
>> +    description: Voltage threshold value headset button7
>> +    minimum: 0
>> +    maximum: 500000
> 
> These can all be a single pattern property:
> '^qcom,mbhc-button[0-7]-vthreshold-microvolt$'

Thanks for the hint, this works, will change it in next version.

--srini
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/sound/qcom,wcd934x.yaml b/Documentation/devicetree/bindings/sound/qcom,wcd934x.yaml
index e8f716b5f875..b25c6ca4e97c 100644
--- a/Documentation/devicetree/bindings/sound/qcom,wcd934x.yaml
+++ b/Documentation/devicetree/bindings/sound/qcom,wcd934x.yaml
@@ -77,6 +77,64 @@  properties:
     minimum: 1800000
     maximum: 2850000
 
+  qcom,hphl-jack-type-normally-closed:
+    description: Indicates that HPHL jack switch type is normally closed
+    type: boolean
+
+  qcom,ground-jack-type-normally-closed:
+    description: Indicates that Headset Ground switch type is normally closed
+    type: boolean
+
+  qcom,mbhc-headset-vthreshold-microvolt:
+    description: Voltage threshold value for headset detection
+    minimum: 0
+    maximum: 2850000
+
+  qcom,mbhc-headphone-vthreshold-microvolt:
+    description: Voltage threshold value for headphone detection
+    minimum: 0
+    maximum: 2850000
+
+  qcom,mbhc-button0-vthreshold-microvolt:
+    description: Voltage threshold value for headset button0
+    minimum: 0
+    maximum: 500000
+
+  qcom,mbhc-button1-vthreshold-microvolt:
+    description: Voltage threshold value for headset button1
+    minimum: 0
+    maximum: 500000
+
+  qcom,mbhc-button2-vthreshold-microvolt:
+    description: Voltage threshold value for headset button2
+    minimum: 0
+    maximum: 500000
+
+  qcom,mbhc-button3-vthreshold-microvolt:
+    description: Voltage threshold value for headset button3
+    minimum: 0
+    maximum: 500000
+
+  qcom,mbhc-button4-vthreshold-microvolt:
+    description: Voltage threshold value for headset button4
+    minimum: 0
+    maximum: 500000
+
+  qcom,mbhc-button5-vthreshold-microvolt:
+    description: Voltage threshold value for headset button5
+    minimum: 0
+    maximum: 500000
+
+  qcom,mbhc-button6-vthreshold-microvolt:
+    description: Voltage threshold value for headset button6
+    minimum: 0
+    maximum: 500000
+
+  qcom,mbhc-button7-vthreshold-microvolt:
+    description: Voltage threshold value headset button7
+    minimum: 0
+    maximum: 500000
+
   clock-output-names:
     const: mclk
 
@@ -159,6 +217,13 @@  examples:
         qcom,micbias2-microvolt = <1800000>;
         qcom,micbias3-microvolt = <1800000>;
         qcom,micbias4-microvolt = <1800000>;
+        qcom,hphl-jack-type-normally-closed;
+        qcom,ground-jack-type-normally-closed;
+        qcom,mbhc-button0-vthreshold-microvolt = <75000>;
+        qcom,mbhc-button1-vthreshold-microvolt = <150000>;
+        qcom,mbhc-button2-vthreshold-microvolt = <237000>;
+        qcom,mbhc-headset-vthreshold-microvolt = <1700000>;
+        qcom,mbhc-headphone-vthreshold-microvolt = <50000>;
         clock-names = "extclk";
         clocks = <&rpmhcc 2>;