linux-kernel - Re: [PATCH 2/5] ASoC: dt-bindings: document WCD939x Audio Codec

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: <b637c287-93e5-4214-9275-80fac3c6181b@linaro.org>
Date:   Tue, 28 Nov 2023 09:59:23 +0100
From:   Neil Armstrong <neil.armstrong@...aro.org>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        Banajit Goswami <bgoswami@...cinc.com>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>
Cc:     linux-arm-msm@...r.kernel.org, alsa-devel@...a-project.org,
        linux-sound@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/5] ASoC: dt-bindings: document WCD939x Audio Codec

On 24/11/2023 09:33, Krzysztof Kozlowski wrote:
> On 23/11/2023 15:49, Neil Armstrong wrote:
> 
>> +  Qualcomm WCD9390/WCD9395 Codec is a standalone Hi-Fi audio codec IC.
>> +  It has RX and TX Soundwire slave devices.
>> +  The WCD9390/WCD9395 IC has a functionally separate USB-C Mux subsystem
>> +  accessible over an I2C interface.
>> +  The Audio Headphone and Microphone data path between the Codec and the USB-C Mux
>> +  subsystems are external to the IC, thus requiring DT port-endpoint graph description
>> +  to handle USB-C altmode & orientation switching for Audio Accessory Mode.
>> +
>> +allOf:
>> +  - $ref: dai-common.yaml#
>> +  - $ref: qcom,wcd93xx-common.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - qcom,wcd9390-codec
>> +      - qcom,wcd9395-codec
> 
> 9395 should be compatible with 9390, so please express it with a list
> using fallback. I know that earlier wcd93xx do not follow that concept,
> but maybe we will fix them some point as well.

I don't get why this would be needed, yes their are compatible but still
two separate ICs with different internal capabilities.

It the first time I get such request for new documentation

> 
>> +
>> +  mode-switch:
>> +    description: Flag the port as possible handle of altmode switching
>> +    type: boolean
>> +
>> +  orientation-switch:
>> +    description: Flag the port as possible handler of orientation switching
>> +    type: boolean
>> +
>> +  port:
>> +    $ref: /schemas/graph.yaml#/properties/port
>> +    description:
>> +      A port node to link the WCD939x Codec node to USB MUX subsystems for the
>> +      purpose of handling altmode muxing and orientation switching to detecte and
>> +      enable Audio Accessory Mode.
>> +
>> +required:
>> +  - compatible
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> +  - |
>> +    codec {
>> +        compatible = "qcom,wcd9390-codec";
>> +        reset-gpios = <&tlmm 32 0>;
> 
> Please define for the GPIO flag.

Ack

> 
>> +        #sound-dai-cells = <1>;
>> +        qcom,tx-device = <&wcd939x_tx>;
>> +        qcom,rx-device = <&wcd939x_rx>;
>> +        qcom,micbias1-microvolt = <1800000>;
>> +        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-buttons-vthreshold-microvolt = <75000 150000 237000 500000 500000 500000 500000 500000>;
>> +        qcom,mbhc-headphone-vthreshold-microvolt = <50000>;
>> +    };
>> +
>> +    /* ... */
>> +
>> +    soundwire@...0000 {
>> +        #address-cells = <2>;
>> +        #size-cells = <0>;
>> +        reg = <0x03210000 0x2000>;
>> +        wcd939x_rx: codec@0,4 {
>> +            compatible = "sdw20217010e00";
>> +            reg  = <0 4>;
> 
> Just one space before '='

Ack

> 
>> +            qcom,rx-port-mapping = <1 2 3 4 5 6>;
>> +        };
>> +    };
>> +
>> +    soundwire@...0000 {
>> +        #address-cells = <2>;
>> +        #size-cells = <0>;
>> +        reg = <0x03230000 0x2000>;
>> +        wcd938x_tx: codec@0,3 {
>> +            compatible = "sdw20217010e00";
>> +            reg  = <0 3>;
> 
> Ditto

Ack

Thanks,
Neil

> 
> 
> Best regards,
> Krzysztof
>