On Fri, Jul 29, 2022 at 12:42 AM Krzysztof Kozlowski < krzysztof.kozlowski@xxxxxxxxxx> wrote: > On 28/07/2022 17:04, Shengjiu Wang wrote: > > Convert the NXP SAI binding to DT schema format using json-schema. > > > > The Synchronous Audio Interface (SAI) provides an interface that > > supports full-duplex serial interfaces with frame synchronization > > formats such as I2S, AC97, TDM, and codec/DSP interfaces. > > > > Signed-off-by: Shengjiu Wang <shengjiu.wang@xxxxxxx> > > --- > > changes in v2 > > - fix exclusive property issue > > - fix order issue of compatible, clock-names, dma-names > > > > Thank you for your patch. There is something to discuss/improve. > > > .../devicetree/bindings/sound/fsl,sai.yaml | 215 ++++++++++++++++++ > > .../devicetree/bindings/sound/fsl-sai.txt | 95 -------- > > 2 files changed, 215 insertions(+), 95 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/sound/fsl,sai.yaml > > delete mode 100644 Documentation/devicetree/bindings/sound/fsl-sai.txt > > > > diff --git a/Documentation/devicetree/bindings/sound/fsl,sai.yaml > b/Documentation/devicetree/bindings/sound/fsl,sai.yaml > > new file mode 100644 > > index 000000000000..3e3d99febd69 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/sound/fsl,sai.yaml > > @@ -0,0 +1,215 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/sound/fsl,sai.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Freescale Synchronous Audio Interface (SAI). > > + > > +maintainers: > > + - Shengjiu Wang <shengjiu.wang@xxxxxxx> > > + > > +description: | > > + The SAI is based on I2S module that used communicating with audio > codecs, > > + which provides a synchronous audio interface that supports fullduplex > > + serial interfaces with frame synchronization such as I2S, AC97, TDM, > and > > + codec/DSP interfaces. > > + > > +properties: > > + compatible: > > + oneOf: > > + - const: fsl,vf610-sai > > + - const: fsl,imx6sx-sai > > + - const: fsl,imx6ul-sai > > + - const: fsl,imx7ulp-sai > > + - const: fsl,imx8mq-sai > > + - const: fsl,imx8qm-sai > > + - const: fsl,imx8ulp-sai > > All these are an enum. > ok, I will update it. > > > + - items: > > + - enum: > > + - fsl,imx8mm-sai > > + - fsl,imx8mn-sai > > + - fsl,imx8mp-sai > > + - const: fsl,imx8mq-sai > > + > > + reg: > > + maxItems: 1 > > + > > + interrupts: > > + items: > > + - description: receive and transmit interrupt > > + > > + dmas: > > + maxItems: 2 > > + > > + dma-names: > > + maxItems: 2 > > + > > + clocks: > > + items: > > + - description: The ipg clock for register access > > + - description: master clock source 0 (obsoleted) > > + - description: master clock source 1 > > + - description: master clock source 2 > > + - description: master clock source 3 > > + - description: PLL clock source for 8kHz series > > + - description: PLL clock source for 11kHz series > > + minItems: 4 > > + > > + clock-names: > > + oneOf: > > + - items: > > + - const: bus > > + - const: mclk0 > > + - const: mclk1 > > + - const: mclk2 > > + - const: mclk3 > > + - const: pll8k > > + - const: pll11k > > + minItems: 4 > > + - items: > > + - const: bus > > + - const: mclk1 > > + - const: mclk2 > > + - const: mclk3 > > + - const: pll8k > > + - const: pll11k > > + minItems: 4 > > + > > + lsb-first: > > + $ref: /schemas/types.yaml#/definitions/flag > > Be consistent, so: > type:boolean > ok, I will add it. > > > + description: | > > + Configures whether the LSB or the MSB is transmitted > > + first for the fifo data. If this property is absent, > > + the MSB is transmitted first as default, or the LSB > > + is transmitted first. > > + > > + big-endian: > > + description: | > > + required if all the SAI registers are big-endian rather than > little-endian. > > + type: boolean > > + > > + fsl,sai-synchronous-rx: > > + $ref: /schemas/types.yaml#/definitions/flag > > type:boolean > > > + description: | > > + SAI will work in the synchronous mode (sync Tx with Rx) which > means > > + both the transmitter and the receiver will send and receive data > by > > + following receiver's bit clocks and frame sync clocks. > > + > > + fsl,sai-asynchronous: > > + $ref: /schemas/types.yaml#/definitions/flag > > type:boolean > > > + description: | > > + SAI will work in the asynchronous mode, which means both > transmitter > > + and receiver will send and receive data by following their own > bit clocks > > + and frame sync clocks separately. > > + If both fsl,sai-asynchronous and fsl,sai-synchronous-rx are > absent, the > > + default synchronous mode (sync Rx with Tx) will be used, which > means both > > + transmitter and receiver will send and receive data by following > clocks > > + of transmitter. > > + > > + fsl,dataline: > > + $ref: /schemas/types.yaml#/definitions/uint32-matrix > > + description: | > > + Configure the dataline. It has 3 value for each configuration > > and how many items in total? > I will add maxitems. > > > + items: > > + items: > > + - description: format Default(0), I2S(1) or PDM(2) > > + enum: [0, 1, 2] > > + - description: dataline mask for 'rx' > > + - description: dataline mask for 'tx' > > + > > + fsl,sai-mclk-direction-output: > > + $ref: /schemas/types.yaml#/definitions/flag > > boolean > > > + description: | > > + SAI will output the SAI MCLK clock. > > + > > + fsl,shared-interrupt: > > + $ref: /schemas/types.yaml#/definitions/flag > > boolean > but the problem is it was not present in previous bindings and a change > in pure conversion was not mentioned/explained in commit msg. > > it is already used by some dts, so add it, I will mention it in commit msg. > > + description: | > > + Interrupt is shared with other modules. > > + > > + "#sound-dai-cells": > > + const: 0 > > Also a new property. If these are already used, please briefly explain > in commit msg the changes to binding from pure conversion. > > it is already used by some dts, so add it, I will mention it in commit msg. > > + > > +allOf: > > + - if: > > + properties: > > + compatible: > > + contains: > > + const: fsl,vf610-sai > > + then: > > + properties: > > + dmas: > > + items: > > + - description: DMA controller phandle and request line for > TX > > + - description: DMA controller phandle and request line for > RX > > + dma-names: > > + items: > > + - const: tx > > + - const: rx > > + else: > > + properties: > > + dmas: > > + items: > > + - description: DMA controller phandle and request line for > RX > > + - description: DMA controller phandle and request line for > TX > > + dma-names: > > + items: > > + - const: rx > > + - const: tx > > + - if: > > + required: > > + - fsl,sai-asynchronous > > + then: > > + properties: > > + fsl,sai-synchronous-rx: false > > + > > +required: > > + - compatible > > + - reg > > + - interrupts > > + - dmas > > + - dma-names > > + - clocks > > + - clock-names > > sound-dai-cells not required and not present in vf610-sai? That's a bit > unusual. Maybe it was missing? > it is optional, some old dts nodes don't use it. > > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/interrupt-controller/arm-gic.h> > > + #include <dt-bindings/clock/vf610-clock.h> > > + sai2: sai@40031000 { > > + compatible = "fsl,vf610-sai"; > > + reg = <0x40031000 0x1000>; > > + interrupts = <86 IRQ_TYPE_LEVEL_HIGH>; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&pinctrl_sai2_1>; > > + clocks = <&clks VF610_CLK_PLATFORM_BUS>, > > + <&clks VF610_CLK_SAI2>, > > + <&clks 0>, <&clks 0>; > > + clock-names = "bus", "mclk1", "mclk2", "mclk3"; > > + dma-names = "tx", "rx"; > > + dmas = <&edma0 0 21>, > > + <&edma0 0 20>; > > + big-endian; > > + lsb-first; > > + }; > > + > > + - | > > + #include <dt-bindings/interrupt-controller/arm-gic.h> > > + #include <dt-bindings/clock/imx8mm-clock.h> > > + sai1: sai@30010000 { > > + #sound-dai-cells = <0>; > > + compatible = "fsl,imx8mm-sai", "fsl,imx8mq-sai"; > > + reg = <0x30010000 0x10000>; > > First compatible, then reg, then the rest of properties. > ok will update it. best regards wang shengjiu > > > + interrupts = <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>; > > + clocks = <&clk IMX8MM_CLK_SAI1_IPG>, > > + <&clk IMX8MM_CLK_DUMMY>, > > + <&clk IMX8MM_CLK_SAI1_ROOT>, > > + <&clk IMX8MM_CLK_DUMMY>, <&clk IMX8MM_CLK_DUMMY>; > > + clock-names = "bus", "mclk0", "mclk1", "mclk2", "mclk3"; > > + dmas = <&sdma2 0 2 0>, <&sdma2 1 2 0>; > > + dma-names = "rx", "tx"; > > + fsl,dataline = <1 0xff 0xff 2 0xff 0x11>; > > + }; > > > Best regards, > Krzysztof >
