Re: [PATCH 1/2] ASoC: dt-bindings: Add sample format conversion

[Krzysztof Kozlowski]

On 27/07/2022 16:26, Sameer Pujar wrote:
> Presently "convert-channels" and "convert-rate" DT bindings are available
> for channel and rate fixups respectively.
> 
> Similarly add "convert-sample-format" binding to fixup DAI sample format
> as well. This is added to simple-card and audio-graph based sound cards.
> 
> Signed-off-by: Sameer Pujar <spujar@xxxxxxxxxx>
> Cc: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
> ---
>  .../devicetree/bindings/sound/audio-graph-port.yaml | 21 +++++++++++++++++++++
>  .../devicetree/bindings/sound/audio-graph.yaml      | 11 +++++++++++
>  .../devicetree/bindings/sound/simple-card.yaml      | 15 +++++++++++++++
>  3 files changed, 47 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/sound/audio-graph-port.yaml b/Documentation/devicetree/bindings/sound/audio-graph-port.yaml
> index 5c36867..d7491038 100644
> --- a/Documentation/devicetree/bindings/sound/audio-graph-port.yaml
> +++ b/Documentation/devicetree/bindings/sound/audio-graph-port.yaml
> @@ -24,6 +24,17 @@ properties:
>    convert-channels:
>      description: CPU to Codec rate channels.
>      $ref: /schemas/types.yaml#/definitions/uint32

Missing blank line.

> +  convert-sample-format:
> +    description: CPU to Codec sample format.
> +      Value    Format
> +        0        S8
> +        2        S16_LE
> +        6        S24_LE
> +        10       S32_LE
> +        32       S24_3LE

Why some random set of integers? Just use string, must more descriptive
and you do not embed programming model to DT... unless there is some
reason for this approach?

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 2, 6, 10, 32]
> +
>  patternProperties:
>    "^endpoint(@[0-9a-f]+)?":
>      $ref: /schemas/graph.yaml#/$defs/endpoint-base
> @@ -70,6 +81,16 @@ patternProperties:
>        convert-channels:
>          description: CPU to Codec rate channels.
>          $ref: /schemas/types.yaml#/definitions/uint32

Ditto

> +      convert-sample-format:
> +        description: CPU to Codec sample format.
> +          Value    Format
> +            0        S8
> +            2        S16_LE
> +            6        S24_LE
> +            10       S32_LE
> +            32       S24_3LE
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        enum: [0, 2, 6, 10, 32]

Ditto


>  
>        dai-tdm-slot-width-map:
>          description: Mapping of sample widths to slot widths. For hardware
> diff --git a/Documentation/devicetree/bindings/sound/audio-graph.yaml b/Documentation/devicetree/bindings/sound/audio-graph.yaml
> index 4b46794..df8a994 100644
> --- a/Documentation/devicetree/bindings/sound/audio-graph.yaml
> +++ b/Documentation/devicetree/bindings/sound/audio-graph.yaml
> @@ -32,6 +32,17 @@ properties:
>    convert-channels:
>      description: CPU to Codec rate channels.
>      $ref: /schemas/types.yaml#/definitions/uint32

Ditto

> +  convert-sample-format:
> +    description: CPU to Codec sample format.
> +      Value    Format
> +        0        S8
> +        2        S16_LE
> +        6        S24_LE
> +        10       S32_LE
> +        32       S24_3LE

Ditto


> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 2, 6, 10, 32]
> +
>    pa-gpios:
>      maxItems: 1
>    hp-det-gpio:
> diff --git a/Documentation/devicetree/bindings/sound/simple-card.yaml b/Documentation/devicetree/bindings/sound/simple-card.yaml
> index b261d49..6decd73 100644
> --- a/Documentation/devicetree/bindings/sound/simple-card.yaml
> +++ b/Documentation/devicetree/bindings/sound/simple-card.yaml
> @@ -77,6 +77,17 @@ definitions:
>      description: CPU to Codec rate channels.
>      $ref: /schemas/types.yaml#/definitions/uint32
>  
> +  convert-sample-format:
> +    description: CPU to Codec sample format.
> +      Value    Format
> +        0        S8
> +        2        S16_LE
> +        6        S24_LE
> +        10       S32_LE
> +        32       S24_3LE
> +    $ref: /schemas/types.yaml#/definitions/uint32

Wait, you now put the same property in four different places. Don't you
have something which could be shared?



Best regards,
Krzysztof