[v1] Add Chameleon v3 ASoC audio

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

Posted by Paweł Anikiel 2 years, 8 months ago

Add binding for chv3-i2s device.

Signed-off-by: Paweł Anikiel <pan@semihalf.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@semihalf.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
+  reg:
+    items:
+      - description: core registers
+      - description: irq registers
+  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>;
+    };
-- 
2.40.0.634.g4ca3ef3211-goog

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

Posted by Krzysztof Kozlowski 2 years, 8 months ago

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.


> 
> Signed-off-by: Paweł Anikiel <pan@semihalf.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@semihalf.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).

> +  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?



Best regards,
Krzysztof

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

Posted by Paweł Anikiel 2 years, 7 months ago

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

Regards,
Paweł

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

Posted by Krzysztof Kozlowski 2 years, 7 months ago

On 25/04/2023 18:01, Paweł Anikiel wrote:
> On Fri, Apr 14, 2023 at 7:00 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.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?

Yes, thanks.

> 
>>
>>
>>>
>>> Signed-off-by: Paweł Anikiel <pan@semihalf.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@semihalf.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?

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