| Message ID | 20220808224153.3634-2-povik+lin@cutebit.org |
|---|---|
| State | New |
| Headers | show |
| Series | ASoC platform driver for Apple MCA | expand |
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
> On 9. 8. 2022, at 10:15, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > 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> (...) >> + 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 '). OK > Describe the items because otherwise you allow any order. The list will > be unfortunately quite long, but still readable enough. Well, I would assume the ‘dmas’ property as described above has an implicit natural order, and the dma-names are tied to it. You order it like the other per-cluster properties, and then within the cluster the order is fixed to 'TXA, RXA, TXB, RXB’ (maybe the word ‘respectively’ thrown into the description would have made it clearer). Anyway that’s just discussing my assumptions. I can roll out the items list for ‘dma-names’, if that’s what you mean. Or do you mean explicitly describing the items in ‘dmas’ too? >> + description: | >> + Names for the DMA channels: 'tx'/'rx', then cluster number, then 'a'/'b' >> + based on the associated SERDES unit. >> + (...) >> + >> +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". It’s a peripheral you use to transmit and receive samples over I2S, frankly I don't know the nomenclature. >> + compatible = "apple,t6000-mca", "apple,mca"; >> + reg = <0x9b600000 0x10000>, >> + <0x9b200000 0x20000>; >> + > > > Best regards, > Krzysztof All best, Martin
On 09/08/2022 11:40, Martin Povišer wrote: >> Describe the items because otherwise you allow any order. The list will >> be unfortunately quite long, but still readable enough. > > Well, I would assume the ‘dmas’ property as described above has an implicit > natural order, and the dma-names are tied to it. You order it like the other > per-cluster properties, and then within the cluster the order is fixed to > 'TXA, RXA, TXB, RXB’ (maybe the word ‘respectively’ thrown into the description > would have made it clearer). > > Anyway that’s just discussing my assumptions. I can roll out the items list > for ‘dma-names’, if that’s what you mean. Or do you mean explicitly describing > the items in ‘dmas’ too? The text description of 'dmas' does not mean it will be followed by DTS author. In current bindings DTS author can therefore put any order of dmas/dma-names. Unrolling the dma-names forces this order to be fixed and validated by dtschema. > >>> + description: | >>> + Names for the DMA channels: 'tx'/'rx', then cluster number, then 'a'/'b' >>> + based on the associated SERDES unit. >>> + > > (...) > >>> + >>> +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". > > It’s a peripheral you use to transmit and receive samples over I2S, frankly > I don't know the nomenclature. Looking at other devices, it's i2s. Best regards, Krzysztof
> On 9. 8. 2022, at 10:47, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 09/08/2022 11:40, Martin Povišer wrote: >>> Describe the items because otherwise you allow any order. The list will >>> be unfortunately quite long, but still readable enough. >> >> Well, I would assume the ‘dmas’ property as described above has an implicit >> natural order, and the dma-names are tied to it. You order it like the other >> per-cluster properties, and then within the cluster the order is fixed to >> 'TXA, RXA, TXB, RXB’ (maybe the word ‘respectively’ thrown into the description >> would have made it clearer). >> >> Anyway that’s just discussing my assumptions. I can roll out the items list >> for ‘dma-names’, if that’s what you mean. Or do you mean explicitly describing >> the items in ‘dmas’ too? > > The text description of 'dmas' does not mean it will be followed by DTS > author. In current bindings DTS author can therefore put any order of > dmas/dma-names. Unrolling the dma-names forces this order to be fixed > and validated by dtschema. OK >> >>>> + description: | >>>> + Names for the DMA channels: 'tx'/'rx', then cluster number, then 'a'/'b' >>>> + based on the associated SERDES unit. >>>> + >> >> (...) >> >>>> + >>>> +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". >> >> It’s a peripheral you use to transmit and receive samples over I2S, frankly >> I don't know the nomenclature. > > Looking at other devices, it's i2s. OK, thanks. > Best regards, > Krzysztof Best, Martin
diff --git a/Documentation/devicetree/bindings/sound/apple,mca.yaml b/Documentation/devicetree/bindings/sound/apple,mca.yaml new file mode 100644 index 000000000000..f64119c5d8d1 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/apple,mca.yaml @@ -0,0 +1,109 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/sound/apple,mca.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +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 + - 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]$' + 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' + +additionalProperties: false + +examples: + - | + mca: mca@9b600000 { + compatible = "apple,t6000-mca", "apple,mca"; + reg = <0x9b600000 0x10000>, + <0x9b200000 0x20000>; + + clocks = <&nco 0>, <&nco 1>, <&nco 2>, <&nco 3>; + power-domains = <&ps_audio_p>, <&ps_mca0>, <&ps_mca1>, + <&ps_mca2>, <&ps_mca3>; + dmas = <&admac 0>, <&admac 1>, <&admac 2>, <&admac 3>, + <&admac 4>, <&admac 5>, <&admac 6>, <&admac 7>, + <&admac 8>, <&admac 9>, <&admac 10>, <&admac 11>, + <&admac 12>, <&admac 13>, <&admac 14>, <&admac 15>; + dma-names = "tx0a", "rx0a", "tx0b", "rx0b", + "tx1a", "rx1a", "tx1b", "rx1b", + "tx2a", "rx2a", "tx2b", "rx2b", + "tx3a", "rx3a", "tx3b", "rx3b"; + + #sound-dai-cells = <1>; + };
Add binding schema for MCA I2S transceiver found on Apple M1 and other chips. Signed-off-by: Martin Povišer <povik+lin@cutebit.org> --- .../devicetree/bindings/sound/apple,mca.yaml | 109 ++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/apple,mca.yaml