Thread (45 messages) 45 messages, 7 authors, 2017-10-05

Re: [PATCH v1 3/5] dt-bindings: Add DT bindings for NVIDIA Tegra AHB DMA controller

From: Dmitry Osipenko <digetx@gmail.com>
Date: 2017-10-03 17:04:19
Also in: linux-clk, linux-devicetree, lkml

On 03.10.2017 18:38, Stephen Warren wrote:
On 10/03/2017 04:32 AM, Jon Hunter wrote:
quoted

On 03/10/17 00:02, Dmitry Osipenko wrote:
quoted
On 02.10.2017 20:05, Stephen Warren wrote:
quoted
On 09/29/2017 09:11 PM, Dmitry Osipenko wrote:
quoted
On 29.09.2017 22:30, Stephen Warren wrote:
quoted
On 09/27/2017 02:34 AM, Jon Hunter wrote:
quoted
On 27/09/17 02:57, Dmitry Osipenko wrote:
quoted
On 26.09.2017 17:50, Jon Hunter wrote:
quoted
On 26/09/17 00:22, Dmitry Osipenko wrote:
quoted
Document DT bindings for NVIDIA Tegra AHB DMA controller that presents
on Tegra20/30 SoC's.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
    .../bindings/dma/nvidia,tegra20-ahbdma.txt         | 23
++++++++++++++++++++++
    1 file changed, 23 insertions(+)
    create mode 100644
Documentation/devicetree/bindings/dma/nvidia,tegra20-ahbdma.txt

diff --git
a/Documentation/devicetree/bindings/dma/nvidia,tegra20-ahbdma.txt
b/Documentation/devicetree/bindings/dma/nvidia,tegra20-ahbdma.txt
new file mode 100644
index 000000000000..2af9aa76ae11
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/nvidia,tegra20-ahbdma.txt
@@ -0,0 +1,23 @@
+* NVIDIA Tegra AHB DMA controller
+
+Required properties:
+- compatible:    Must be "nvidia,tegra20-ahbdma"
+- reg:        Should contain registers base address and length.
+- interrupts:    Should contain one entry, DMA controller interrupt.
+- clocks:    Should contain one entry, DMA controller clock.
+- resets :    Should contain one entry, DMA controller reset.
+- #dma-cells:    Should be <1>. The cell represents DMA request select
value
+        for the peripheral. For more details consult the Tegra TRM's
+        documentation, in particular AHB DMA channel control register
+        REQ_SEL field.
What about the TRIG_SEL field? Do we need to handle this here as well?
Actually, DMA transfer trigger isn't related a hardware description. It's
up to
software to decide what trigger to select. So it shouldn't be in the
binding.
I think it could be, if say a board wanted a GPIO to trigger a transfer.
quoted
And I think the same applies to requester... any objections?
Well, the REQ_SEL should definitely be in the binding.

Laxman, Stephen, what are your thoughts on the TRIG_SEL field? Looks
like we never bothered with it for the APB DMA and so maybe no ones uses
this.
I don't think TRIG_SEL should be in the binding, at least at present. While
TRIG_SEL certainly is something used to configure the transfer, I believe the
semantics of the current DMA binding only cover DMA transfers that are
initiated
when SW desires, rather than being a combination of after SW programs the
transfer plus some other HW event. So, we always use a default/hard-coded
TRIG_SEL value. As such, there's no need for a TRIG_SEL value in DT. There's
certainly no known use-case that requires a non-default TRIG_SEL value at
present. We could add an extra #dma-cells value later if we find a use for
it,
and the semantics of that use-case make sense to add it to the DMA specifier,
rather than some other separate higher-level property/driver/...
Thank you for the comment. If we'd want to extend the binding further with the
trigger, how to differentiate trigger from the requester in a case of a single
#data-cell?

Of course realistically a chance that the further extension would be needed is
very-very low, so we may defer the efforts to solve that question and for now
make driver aware of the potential #dma-cells extension.
The request selector cell isn't optional, so is always present. If we later add
an optional trig_sel cell, we'll either have:

#dma-cells=<1>: req_sel

or:

#dma-cells=<2>: req_sel, trig_sel
Why request sel. couldn't be optional? Could you please elaborate a bit more?
The documentation currently says it's mandatory, and DT bindings must be evolved
in a backwards-compatible fashion.
quoted
quoted
I think possible options are:

#dma-cells=<1>: req_sel
#dma-cells=<1>: trig_sel
With the above, how would you know that it is the req_sel or trig_sel
that is specified?
Also, if req_sel were optional, then it'd be impossible to distinguish between
those cases, so we can't design a binding like that. In general, when adding
extra optional cells to an #xxx-cells style binding, then whenever cell N is
present, all cells before cell N must be present even if optional.
I've checked how extending of #dma-cells looks like and it is indeed a good
variant since it preserves backward compatibility. Thank you and Jon for the
comments and suggestions, I'll send out v2 soon.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help