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

[Neil Armstrong]

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@3210000 {
> > +        #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@3230000 {
> > +        #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