On 09/08/2022 01:41, Martin Povišer wrote:
> Add binding schema for MCA I2S transceiver found on Apple M1 and other
> chips.
Thank you for your patch. There is something to discuss/improve.
> +title: Apple MCA I2S transceiver
> +
> +description: |
> + MCA is an I2S transceiver peripheral found on M1 and other Apple chips. It is
> + composed of a number of identical clusters which can operate independently
> + or in an interlinked fashion. Up to 6 clusters have been seen on an MCA.
> +
> +maintainers:
> + - Martin Povišer <povik+lin@xxxxxxxxxxx>
> +
> +properties:
> + compatible:
> + items:
> + - enum:
> + - apple,t8103-mca
> + - apple,t6000-mca
How about alphabetical order?
> + - const: apple,mca
> +
> + reg:
> + items:
> + - description: Register region of the MCA clusters proper
> + - description: Register region of the DMA glue and its FIFOs
> +
> + interrupts:
> + minItems: 4
> + maxItems: 6
> + description:
> + One interrupt per each cluster
> +
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 0
> +
> + dmas:
> + minItems: 16
> + maxItems: 24
> + description:
> + DMA channels corresponding to the SERDES units in the peripheral. They are
> + listed in groups of four per cluster, and within the group they are given
> + as associated to the TXA, RXA, TXB, RXB units.
> +
> + dma-names:
> + minItems: 16
> + maxItems: 24
> + items:
> + pattern: '^(tx|rx)[0-5][ab]$'
Use consistent quotes (everywhere " or ').
Describe the items because otherwise you allow any order. The list will
be unfortunately quite long, but still readable enough.
> + description: |
> + Names for the DMA channels: 'tx'/'rx', then cluster number, then 'a'/'b'
> + based on the associated SERDES unit.
> +
> + clocks:
> + minItems: 4
> + maxItems: 6
> + description:
> + Clusters' input reference clock.
> +
> + resets:
> + maxItems: 1
> +
> + power-domains:
> + minItems: 5
> + maxItems: 7
> + description:
> + First a general power domain for register access, then the power
> + domains of individual clusters for their operation.
> +
> + "#sound-dai-cells":
> + const: 1
> +
> +required:
> + - compatible
> + - reg
> + - dmas
> + - dma-names
> + - clocks
> + - power-domains
> + - '#sound-dai-cells'
Use consistent quotes (everywhere " or ').
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + mca: mca@9b600000 {
You called it I2S transceiver but isn't it also actually I2S controller?
If yes, then the node name should be probably "i2s".
> + compatible = "apple,t6000-mca", "apple,mca";
> + reg = <0x9b600000 0x10000>,
> + <0x9b200000 0x20000>;
> +
Best regards,
Krzysztof
