Re: [PATCH 1/3] dt-bindings: sound: Add Apple MCA I2S transceiver

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