linux-kernel - Re: [PATCH 2/9] dt-bindings: ASoC: Add chv3-i2s

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 25 Apr 2023 18:01:17 +0200
From:   Paweł Anikiel <pan@...ihalf.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc:     alsa-devel@...a-project.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, lgirdwood@...il.com,
        broonie@...nel.org, perex@...ex.cz, tiwai@...e.com,
        robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
        dinguyen@...nel.org, lars@...afoo.de, nuno.sa@...log.com,
        upstream@...ihalf.com
Subject: Re: [PATCH 2/9] dt-bindings: ASoC: Add chv3-i2s

On Fri, Apr 14, 2023 at 7:00 PM Krzysztof Kozlowski
<krzysztof.kozlowski@...aro.org> 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?

>
>
> >
> > Signed-off-by: Paweł Anikiel <pan@...ihalf.com>
> > ---
> >  .../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@...ihalf.com>
> > +
> > +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?

>
> > +  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@...60300 {
> > +        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>;

Regards,
Paweł