2022-03-31 02:29:01

by Martin Povišer

[permalink] [raw]
Subject: [PATCH 1/2] dt-bindings: dma: Add Apple ADMAC

Apple's Audio DMA Controller (ADMAC) is used to fetch and store audio
samples on Apple SoCs from the "Apple Silicon" family.

Signed-off-by: Martin Povišer <[email protected]>
---
.../devicetree/bindings/dma/apple,admac.yaml | 73 +++++++++++++++++++
1 file changed, 73 insertions(+)
create mode 100644 Documentation/devicetree/bindings/dma/apple,admac.yaml

diff --git a/Documentation/devicetree/bindings/dma/apple,admac.yaml b/Documentation/devicetree/bindings/dma/apple,admac.yaml
new file mode 100644
index 000000000000..34f76a9a2983
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/apple,admac.yaml
@@ -0,0 +1,73 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/dma/apple,admac.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Apple Audio DMA Controller (ADMAC)
+
+description: |
+ Apple's Audio DMA Controller (ADMAC) is used to fetch and store
+ audio samples on Apple SoCs from the "Apple Silicon" family.
+
+maintainers:
+ - Martin Povišer <[email protected]>
+
+allOf:
+ - $ref: "dma-controller.yaml#"
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - apple,t6000-admac
+ - apple,t8103-admac
+ - const: apple,admac
+
+ reg:
+ maxItems: 1
+
+ '#dma-cells':
+ const: 1
+
+ apple,internal-irq-destination:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: Index influencing internal routing of the IRQs
+ within the peripheral.
+
+ interrupts:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - '#dma-cells'
+ - dma-channels
+ - apple,internal-irq-destination
+ - interrupts
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/apple-aic.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ dart_sio: iommu@235004000 {
+ compatible = "apple,t8103-dart", "apple,dart";
+ reg = <0x2 0x35004000 0x0 0x4000>;
+ interrupt-parent = <&aic>;
+ interrupts = <AIC_IRQ 635 IRQ_TYPE_LEVEL_HIGH>;
+ #iommu-cells = <1>;
+ };
+
+ admac: dma-controller@238200000 {
+ compatible = "apple,t8103-admac", "apple,admac";
+ reg = <0x2 0x38200000 0x0 0x34000>;
+ dma-channels = <12>;
+ interrupt-parent = <&aic>;
+ interrupts = <AIC_IRQ 626 IRQ_TYPE_LEVEL_HIGH>;
+ #dma-cells = <1>;
+ iommus = <&dart_sio 2>;
+ apple,internal-irq-destination = <1>;
+ };
--
2.33.0


2022-03-31 03:15:18

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: dma: Add Apple ADMAC

On Wed, 30 Mar 2022 18:44:57 +0200, Martin Povišer wrote:
> Apple's Audio DMA Controller (ADMAC) is used to fetch and store audio
> samples on Apple SoCs from the "Apple Silicon" family.
>
> Signed-off-by: Martin Povišer <[email protected]>
> ---
> .../devicetree/bindings/dma/apple,admac.yaml | 73 +++++++++++++++++++
> 1 file changed, 73 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/dma/apple,admac.yaml
>

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dma/apple,admac.example.dt.yaml: example-0: iommu@235004000:reg:0: [2, 889208832, 0, 16384] is too long
From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dma/apple,admac.example.dt.yaml: example-0: dma-controller@238200000:reg:0: [2, 941621248, 0, 212992] is too long
From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dma/apple,admac.example.dt.yaml: iommu@235004000: compatible: ['apple,t8103-dart', 'apple,dart'] is too long
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iommu/apple,dart.yaml
Documentation/devicetree/bindings/dma/apple,admac.example.dt.yaml:0:0: /example-0/iommu@235004000: failed to match any schema with compatible: ['apple,t8103-dart', 'apple,dart']
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dma/apple,admac.example.dt.yaml: dma-controller@238200000: 'dma-channels', 'iommus' do not match any of the regexes: 'pinctrl-[0-9]+'
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dma/apple,admac.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

2022-03-31 05:57:30

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: dma: Add Apple ADMAC

On 30-03-22, 18:44, Martin Povišer wrote:
> Apple's Audio DMA Controller (ADMAC) is used to fetch and store audio
> samples on Apple SoCs from the "Apple Silicon" family.
>
> Signed-off-by: Martin Povišer <[email protected]>
> ---
> .../devicetree/bindings/dma/apple,admac.yaml | 73 +++++++++++++++++++
> 1 file changed, 73 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/dma/apple,admac.yaml
>
> diff --git a/Documentation/devicetree/bindings/dma/apple,admac.yaml b/Documentation/devicetree/bindings/dma/apple,admac.yaml
> new file mode 100644
> index 000000000000..34f76a9a2983
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/apple,admac.yaml
> @@ -0,0 +1,73 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/dma/apple,admac.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Apple Audio DMA Controller (ADMAC)
> +
> +description: |
> + Apple's Audio DMA Controller (ADMAC) is used to fetch and store
> + audio samples on Apple SoCs from the "Apple Silicon" family.
> +
> +maintainers:
> + - Martin Povišer <[email protected]>
> +
> +allOf:
> + - $ref: "dma-controller.yaml#"
> +
> +properties:
> + compatible:
> + items:
> + - enum:
> + - apple,t6000-admac
> + - apple,t8103-admac
> + - const: apple,admac
> +
> + reg:
> + maxItems: 1
> +
> + '#dma-cells':
> + const: 1
> +
> + apple,internal-irq-destination:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: Index influencing internal routing of the IRQs
> + within the peripheral.

do you have more details for this, is this for peripheral and if so
suited to be in dam-cells?

> +
> + interrupts:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> + - '#dma-cells'
> + - dma-channels
> + - apple,internal-irq-destination
> + - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/apple-aic.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + dart_sio: iommu@235004000 {
> + compatible = "apple,t8103-dart", "apple,dart";
> + reg = <0x2 0x35004000 0x0 0x4000>;
> + interrupt-parent = <&aic>;
> + interrupts = <AIC_IRQ 635 IRQ_TYPE_LEVEL_HIGH>;
> + #iommu-cells = <1>;
> + };
> +
> + admac: dma-controller@238200000 {
> + compatible = "apple,t8103-admac", "apple,admac";
> + reg = <0x2 0x38200000 0x0 0x34000>;
> + dma-channels = <12>;
> + interrupt-parent = <&aic>;
> + interrupts = <AIC_IRQ 626 IRQ_TYPE_LEVEL_HIGH>;
> + #dma-cells = <1>;
> + iommus = <&dart_sio 2>;
> + apple,internal-irq-destination = <1>;
> + };
> --
> 2.33.0

--
~Vinod

2022-03-31 07:51:05

by Martin Povišer

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: dma: Add Apple ADMAC


> On 31. 3. 2022, at 7:23, Vinod Koul <[email protected]> wrote:
>
> On 30-03-22, 18:44, Martin Povišer wrote:
>> Apple's Audio DMA Controller (ADMAC) is used to fetch and store audio
>> samples on Apple SoCs from the "Apple Silicon" family.
>>
>> Signed-off-by: Martin Povišer <[email protected]>
>> ---
>> .../devicetree/bindings/dma/apple,admac.yaml | 73 +++++++++++++++++++
>> 1 file changed, 73 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/dma/apple,admac.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/dma/apple,admac.yaml b/Documentation/devicetree/bindings/dma/apple,admac.yaml
>> new file mode 100644
>> index 000000000000..34f76a9a2983
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/dma/apple,admac.yaml

>> + apple,internal-irq-destination:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description: Index influencing internal routing of the IRQs
>> + within the peripheral.
>
> do you have more details for this, is this for peripheral and if so
> suited to be in dam-cells?

By peripheral I meant the DMA controller itself here.

Effectively the controller has four independent IRQ outputs and the driver
needs to know which one we are using. (It need to be the same output even
for different ADMAC instances on one die.)

2022-03-31 11:01:35

by Martin Povišer

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: dma: Add Apple ADMAC


> On 31. 3. 2022, at 8:50, Martin Povišer <[email protected]> wrote:
>
>>
>> On 31. 3. 2022, at 7:23, Vinod Koul <[email protected]> wrote:
>>
>> On 30-03-22, 18:44, Martin Povišer wrote:
>>> Apple's Audio DMA Controller (ADMAC) is used to fetch and store audio
>>> samples on Apple SoCs from the "Apple Silicon" family.
>>>
>>> Signed-off-by: Martin Povišer <[email protected]>
>>> ---
>>> .../devicetree/bindings/dma/apple,admac.yaml | 73 +++++++++++++++++++
>>> 1 file changed, 73 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/dma/apple,admac.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/dma/apple,admac.yaml b/Documentation/devicetree/bindings/dma/apple,admac.yaml
>>> new file mode 100644
>>> index 000000000000..34f76a9a2983
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/dma/apple,admac.yaml
>
>>> + apple,internal-irq-destination:
>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>> + description: Index influencing internal routing of the IRQs
>>> + within the peripheral.
>>
>> do you have more details for this, is this for peripheral and if so
>> suited to be in dam-cells?
>
> By peripheral I meant the DMA controller itself here.
>
> Effectively the controller has four independent IRQ outputs and the driver
> needs to know which one we are using. (It need to be the same output even
> for different ADMAC instances on one die.)

Pardon, got an evil typo there: It need *not* be the same output... (And pardon
the other rich non-plaintext reply...)

2022-03-31 21:00:52

by Martin Povišer

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: dma: Add Apple ADMAC


> On 31. 3. 2022, at 16:10, Vinod Koul <[email protected]> wrote:
>
> On 31-03-22, 09:06, Martin Povišer wrote:
>>
>>> On 31. 3. 2022, at 8:50, Martin Povišer <[email protected]> wrote:
>>>>
>>>> On 31. 3. 2022, at 7:23, Vinod Koul <[email protected]> wrote:
>>>>
>>>> On 30-03-22, 18:44, Martin Povišer wrote:
>>>>> Apple's Audio DMA Controller (ADMAC) is used to fetch and store audio
>>>>> samples on Apple SoCs from the "Apple Silicon" family.
>>>>>
>>>>> Signed-off-by: Martin Povišer <[email protected]>
>>>>> ---
>>>>> .../devicetree/bindings/dma/apple,admac.yaml | 73 +++++++++++++++++++
>>>>> 1 file changed, 73 insertions(+)
>>>>> create mode 100644 Documentation/devicetree/bindings/dma/apple,admac.yaml
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/dma/apple,admac.yaml b/Documentation/devicetree/bindings/dma/apple,admac.yaml
>>>>> new file mode 100644
>>>>> index 000000000000..34f76a9a2983
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/dma/apple,admac.yaml
>>>
>>>>> + apple,internal-irq-destination:
>>>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>>>> + description: Index influencing internal routing of the IRQs
>>>>> + within the peripheral.
>>>>
>>>> do you have more details for this, is this for peripheral and if so
>>>> suited to be in dam-cells?
>>>
>>> By peripheral I meant the DMA controller itself here.
>
> Dmaengine convention is that peripheral is device which we are doing dma
> to/from, like audio controller/fifo here
>
>>> Effectively the controller has four independent IRQ outputs and the driver
>>> needs to know which one we are using. (It need not be the same output even
>>> for different ADMAC instances on one die.)
>
> That smells like a mux to me.. why not use dma-requests for this?

I am not sure that’s right. Reading the dmaengine docs, DMA requests seem to have
to do with the DMA-controller-to-peripheral connection, but the proposed property
tells us which of four independent IRQ outputs of the DMA controller we actually
have in the interrupts= property. That is, it has to do with the DMA-controller-to-CPU
connection.

(I took the liberty of correcting my typo in the quotation.)

>
> --
> ~Vinod

2022-04-01 03:05:08

by Martin Povišer

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: dma: Add Apple ADMAC



> On 31. 3. 2022, at 19:21, Rob Herring <[email protected]> wrote:
>
> On Thu, Mar 31, 2022 at 06:13:53PM +0200, Martin Povišer wrote:
>>
>>> On 31. 3. 2022, at 16:10, Vinod Koul <[email protected]> wrote:
>>>
>>> On 31-03-22, 09:06, Martin Povišer wrote:
>>>>
>>>>> On 31. 3. 2022, at 8:50, Martin Povišer <[email protected]> wrote:
>>>>>>
>>>>>> On 31. 3. 2022, at 7:23, Vinod Koul <[email protected]> wrote:
>>>>>>
>>>>>> On 30-03-22, 18:44, Martin Povišer wrote:
>>>>>>> Apple's Audio DMA Controller (ADMAC) is used to fetch and store audio
>>>>>>> samples on Apple SoCs from the "Apple Silicon" family.
>>>>>>>
>>>>>>> Signed-off-by: Martin Povišer <[email protected]>
>>>>>>> ---
>>>>>>> .../devicetree/bindings/dma/apple,admac.yaml | 73 +++++++++++++++++++
>>>>>>> 1 file changed, 73 insertions(+)
>>>>>>> create mode 100644 Documentation/devicetree/bindings/dma/apple,admac.yaml
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/dma/apple,admac.yaml b/Documentation/devicetree/bindings/dma/apple,admac.yaml
>>>>>>> new file mode 100644
>>>>>>> index 000000000000..34f76a9a2983
>>>>>>> --- /dev/null
>>>>>>> +++ b/Documentation/devicetree/bindings/dma/apple,admac.yaml
>>>>>
>>>>>>> + apple,internal-irq-destination:
>>>>>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>>>>>> + description: Index influencing internal routing of the IRQs
>>>>>>> + within the peripheral.
>>>>>>
>>>>>> do you have more details for this, is this for peripheral and if so
>>>>>> suited to be in dam-cells?
>>>>>
>>>>> By peripheral I meant the DMA controller itself here.
>>>
>>> Dmaengine convention is that peripheral is device which we are doing dma
>>> to/from, like audio controller/fifo here
>>>
>>>>> Effectively the controller has four independent IRQ outputs and the driver
>>>>> needs to know which one we are using. (It need not be the same output even
>>>>> for different ADMAC instances on one die.)
>>>
>>> That smells like a mux to me.. why not use dma-requests for this?
>>
>> I am not sure that’s right. Reading the dmaengine docs, DMA requests seem to have
>> to do with the DMA-controller-to-peripheral connection, but the proposed property
>> tells us which of four independent IRQ outputs of the DMA controller we actually
>> have in the interrupts= property. That is, it has to do with the DMA-controller-to-CPU
>> connection.
>
> Why do they have to be different? IRQF_SHARED doesn't work?

It’s not that the IRQ outputs of different controllers are overlaid. It’s
that e.g. first output of controller A is hooked up to some input of the AP’s
interrupt controller, the third output of controller B is hooked to another
input, but for all we know the other controller outputs lead to nowhere or
to some coprocessor.

> Why can't you request each IRQ until it succeeds?
>
> What happens when there are 5 DMA controllers?
>
> If using more than 1 interrupt will never work or be needed, then I'm
> inclined to say just describe that 1 interrupt. Yes, that goes against
> 'describe all the h/w', but there's always exceptions. I suppose you
> need to know which 'interrupts' index (output) you are using. If so, you
> can do something like this:
>
> interrupts = <-1>, <-1>, <3 0>, <-1>;

That’s actually exactly what I want! In next iteration of the binding I will
drop the vendor property and do that.

>
> Rob

Martin

2022-04-01 15:05:24

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: dma: Add Apple ADMAC

On Thu, Mar 31, 2022 at 06:13:53PM +0200, Martin Povišer wrote:
>
> > On 31. 3. 2022, at 16:10, Vinod Koul <[email protected]> wrote:
> >
> > On 31-03-22, 09:06, Martin Povišer wrote:
> >>
> >>> On 31. 3. 2022, at 8:50, Martin Povišer <[email protected]> wrote:
> >>>>
> >>>> On 31. 3. 2022, at 7:23, Vinod Koul <[email protected]> wrote:
> >>>>
> >>>> On 30-03-22, 18:44, Martin Povišer wrote:
> >>>>> Apple's Audio DMA Controller (ADMAC) is used to fetch and store audio
> >>>>> samples on Apple SoCs from the "Apple Silicon" family.
> >>>>>
> >>>>> Signed-off-by: Martin Povišer <[email protected]>
> >>>>> ---
> >>>>> .../devicetree/bindings/dma/apple,admac.yaml | 73 +++++++++++++++++++
> >>>>> 1 file changed, 73 insertions(+)
> >>>>> create mode 100644 Documentation/devicetree/bindings/dma/apple,admac.yaml
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/dma/apple,admac.yaml b/Documentation/devicetree/bindings/dma/apple,admac.yaml
> >>>>> new file mode 100644
> >>>>> index 000000000000..34f76a9a2983
> >>>>> --- /dev/null
> >>>>> +++ b/Documentation/devicetree/bindings/dma/apple,admac.yaml
> >>>
> >>>>> + apple,internal-irq-destination:
> >>>>> + $ref: /schemas/types.yaml#/definitions/uint32
> >>>>> + description: Index influencing internal routing of the IRQs
> >>>>> + within the peripheral.
> >>>>
> >>>> do you have more details for this, is this for peripheral and if so
> >>>> suited to be in dam-cells?
> >>>
> >>> By peripheral I meant the DMA controller itself here.
> >
> > Dmaengine convention is that peripheral is device which we are doing dma
> > to/from, like audio controller/fifo here
> >
> >>> Effectively the controller has four independent IRQ outputs and the driver
> >>> needs to know which one we are using. (It need not be the same output even
> >>> for different ADMAC instances on one die.)
> >
> > That smells like a mux to me.. why not use dma-requests for this?
>
> I am not sure that’s right. Reading the dmaengine docs, DMA requests seem to have
> to do with the DMA-controller-to-peripheral connection, but the proposed property
> tells us which of four independent IRQ outputs of the DMA controller we actually
> have in the interrupts= property. That is, it has to do with the DMA-controller-to-CPU
> connection.

Why do they have to be different? IRQF_SHARED doesn't work?

Why can't you request each IRQ until it succeeds?

What happens when there are 5 DMA controllers?

If using more than 1 interrupt will never work or be needed, then I'm
inclined to say just describe that 1 interrupt. Yes, that goes against
'describe all the h/w', but there's always exceptions. I suppose you
need to know which 'interrupts' index (output) you are using. If so, you
can do something like this:

interrupts = <-1>, <-1>, <3 0>, <-1>;

Rob

2022-04-01 15:16:02

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: dma: Add Apple ADMAC

On 31-03-22, 09:06, Martin Povišer wrote:
>
> > On 31. 3. 2022, at 8:50, Martin Povišer <[email protected]> wrote:
> >
> >>
> >> On 31. 3. 2022, at 7:23, Vinod Koul <[email protected]> wrote:
> >>
> >> On 30-03-22, 18:44, Martin Povišer wrote:
> >>> Apple's Audio DMA Controller (ADMAC) is used to fetch and store audio
> >>> samples on Apple SoCs from the "Apple Silicon" family.
> >>>
> >>> Signed-off-by: Martin Povišer <[email protected]>
> >>> ---
> >>> .../devicetree/bindings/dma/apple,admac.yaml | 73 +++++++++++++++++++
> >>> 1 file changed, 73 insertions(+)
> >>> create mode 100644 Documentation/devicetree/bindings/dma/apple,admac.yaml
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/dma/apple,admac.yaml b/Documentation/devicetree/bindings/dma/apple,admac.yaml
> >>> new file mode 100644
> >>> index 000000000000..34f76a9a2983
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/dma/apple,admac.yaml
> >
> >>> + apple,internal-irq-destination:
> >>> + $ref: /schemas/types.yaml#/definitions/uint32
> >>> + description: Index influencing internal routing of the IRQs
> >>> + within the peripheral.
> >>
> >> do you have more details for this, is this for peripheral and if so
> >> suited to be in dam-cells?
> >
> > By peripheral I meant the DMA controller itself here.

Dmaengine convention is that peripheral is device which we are doing dma
to/from, like audio controller/fifo here

> > Effectively the controller has four independent IRQ outputs and the driver
> > needs to know which one we are using. (It need to be the same output even
> > for different ADMAC instances on one die.)

That smells like a mux to me.. why not use dma-requests for this?

--
~Vinod