2022-03-07 19:49:21

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 2/6] ASoC: dt-bindings: Document Microchip's PDMC

On 07/03/2022 11:04, Codrin Ciubotariu wrote:
> Add DT bindings for the new Microchip PDMC embedded in sama7g5 SoCs.
>
> Signed-off-by: Codrin Ciubotariu <[email protected]>
> ---
>
> Changes in v2:
> - renamed patch from 'ASoC: add DT bindings for Microchip PDMC' to
> 'ASoC: dt-bindings: Document Microchip's PDMC';
> - renamed yaml file from 'mchp,pdmc.yaml' to 'microchip,pdmc.yaml';
> - used imperative mode in commit description;
> - renamed mchp,pdmc.h to microchip,pdmc.h;
> - fixed 'title' to represent HW;
> - made 'compatible' first property;
> - s/microhpone/microphone;
> - none name in example set to 'sound';
>
> .../bindings/sound/microchip,pdmc.yaml | 99 +++++++++++++++++++
> include/dt-bindings/sound/microchip,pdmc.h | 13 +++
> 2 files changed, 112 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/sound/microchip,pdmc.yaml
> create mode 100644 include/dt-bindings/sound/microchip,pdmc.h
>
> diff --git a/Documentation/devicetree/bindings/sound/microchip,pdmc.yaml b/Documentation/devicetree/bindings/sound/microchip,pdmc.yaml
> new file mode 100644
> index 000000000000..edfa3cbd318e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/microchip,pdmc.yaml
> @@ -0,0 +1,99 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/microchip,pdmc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip Pulse Density Microphone Controller
> +
> +maintainers:
> + - Codrin Ciubotariu <[email protected]>
> +
> +description:
> + The Microchip Pulse Density Microphone Controller (PDMC) interfaces up to 4 digital microphones
> + having Pulse Density Modulated (PDM) outputs.

This exceeds 80 characters.

> +
> +properties:
> + compatible:
> + const: microchip,sama7g5-pdmc
> +
> + "#sound-dai-cells":
> + const: 0
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + clocks:
> + items:
> + - description: Peripheral Bus Clock
> + - description: Generic Clock
> +
> + clock-names:
> + items:
> + - const: pclk
> + - const: gclk
> +
> + dmas:
> + description: RX DMA Channel
> + maxItems: 1
> +
> + dma-names:
> + const: rx
> +
> + microchip,mic-pos:
> + description: |
> + Position of PDM microphones on the DS line and the sampling edge (rising or falling) of the
> + CLK line. A microphone is represented as a pair of DS line and the sampling edge. The first
> + microphone is mapped to channel 0, the second to channel 1, etc.

This is also too long, wrap at 80.

> + $ref: /schemas/types.yaml#/definitions/uint32-matrix
> + items:
> + items:
> + - description: value for DS line
> + - description: value for sampling edge
> + anyOf:
> + - enum:
> + - [0, 0]
> + - [0, 1]
> + - [1, 0]
> + - [1, 1]
> + minItems: 1
> + maxItems: 4
> + uniqueItems: true
> +
> +required:
> + - compatible
> + - "#sound-dai-cells"
> + - reg
> + - interrupts
> + - clocks
> + - clock-names
> + - dmas
> + - dma-names
> + - microchip,mic-pos
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/at91.h>
> + #include <dt-bindings/dma/at91.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/sound/microchip,pdmc.h>
> +
> + pdmc: sound@e1608000 {
> + compatible = "microchip,sama7g5-pdmc";
> + #sound-dai-cells = <0>;
> + reg = <0xe1608000 0x4000>;

In DTS sources: compatible then reg. I mentioned it in previous version.

Best regards,
Krzysztof