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

From: Krzysztof Kozlowski
Date: Thu Jul 28 2022 - 08:23:07 EST


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