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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210510164532.GA241925@robh.at.kernel.org>
Date:   Mon, 10 May 2021 11:45:32 -0500
From:   Rob Herring <robh@...nel.org>
To:     Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
Cc:     broonie@...nel.org, devicetree@...r.kernel.org, perex@...ex.cz,
        alsa-devel@...a-project.org, linux-kernel@...r.kernel.org,
        lgirdwood@...il.com, lee.jones@...aro.org
Subject: Re: [PATCH 1/4] ASoC: dt-bindings: wcd934x: add bindings for Headset
 Button detection

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@...aro.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
>