On 25/04/2023 18:01, Paweł Anikiel wrote: > On Fri, Apr 14, 2023 at 7:00 PM Krzysztof Kozlowski > <krzysztof.kozlowski@xxxxxxxxxx> wrote: >> >> On 14/04/2023 16:01, Paweł Anikiel wrote: >>> Add binding for chv3-i2s device. >> >> Your subject needs improvements: >> 1. ASoC goes before bindings >> 2. You miss some meaningful title of device. "chv3-i2s" can be anything, >> so add Google or Google Chameleon. Or use entire compatible. > > Would "ASoC: dt-bindings: Add Google Chameleon v3 I2S device" be better? Yes, thanks. > >> >> >>> >>> Signed-off-by: Paweł Anikiel <pan@xxxxxxxxxxxx> >>> --- >>> .../bindings/sound/google,chv3-i2s.yaml | 42 +++++++++++++++++++ >>> 1 file changed, 42 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/sound/google,chv3-i2s.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/sound/google,chv3-i2s.yaml b/Documentation/devicetree/bindings/sound/google,chv3-i2s.yaml >>> new file mode 100644 >>> index 000000000000..6f49cf059ac5 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/sound/google,chv3-i2s.yaml >>> @@ -0,0 +1,42 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/sound/google,chv3-i2s.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Google Chameleon v3 I2S device >>> + >>> +maintainers: >>> + - Paweł Anikiel <pan@xxxxxxxxxxxx> >>> + >>> +description: | >>> + I2S device for the Google Chameleon v3. The device handles both RX >>> + and TX using a producer/consumer ring buffer design. >>> + >>> +properties: >>> + compatible: >>> + const: google,chv3-i2s >> >> Missing blank line. >> >> Is chv3 the name of your SoC? Where are the SoC bindings? What's exactly >> the versioning scheme for it (compatibles must be specific, not generic). > > The Chameleon v3 is based around an Intel Arria 10 SoC FPGA. The i2s > device is implemented inside the FPGA. Does this case require SoC > bindings? No, I was mistaken. I somehow get impression that's for Pixel... Sorry for the noise. > >> >>> + reg: >>> + items: >>> + - description: core registers >>> + - description: irq registers >> >> As well... >> >>> + interrupts: >>> + maxItems: 1 >>> + >>> +required: >>> + - compatible >>> + - reg >>> + - interrupts >>> + >>> +additionalProperties: false >>> + >>> +examples: >>> + - | >>> + #include <dt-bindings/interrupt-controller/irq.h> >>> + >>> + i2s0: i2s@c0060300 { >>> + compatible = "google,chv3-i2s"; >>> + reg = <0xc0060300 0x100>, >>> + <0xc0060f00 0x10>; >>> + interrupts = <0 20 IRQ_TYPE_LEVEL_HIGH>; >> >> Isn't 0 also a known define? > > Do you mean this? > interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>; Yes, please. Best regards, Krzysztof