lkml.org 
[lkml]   [2022]   [Aug]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 1/3] dt-bindings: sound: Add Apple MCA I2S transceiver
From
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@cutebit.org>
> +
> +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

\
 
 \ /
  Last update: 2022-08-09 10:17    [W:0.093 / U:4.764 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site