| Message ID | 604d92036e0936443290e68a2226f935fb348113.1506380746.git.digetx@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On 26/09/17 00:22, Dmitry Osipenko wrote: > 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? Cheers Jon
On 26.09.2017 17:50, Jon Hunter wrote: > > On 26/09/17 00:22, Dmitry Osipenko wrote: >> 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? > I've followed APB DMA here, that HW also has TRIG_SEL but ignores it for some reason. I think technically it should be present in the binding, yeah.
On 26.09.2017 17:50, Jon Hunter wrote: > > On 26/09/17 00:22, Dmitry Osipenko wrote: >> 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. And I think the same applies to requester... any objections?
On 27/09/17 02:57, Dmitry Osipenko wrote: > On 26.09.2017 17:50, Jon Hunter wrote: >> >> On 26/09/17 00:22, Dmitry Osipenko wrote: >>> 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. > 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. Cheers Jon
On 27.09.2017 11:34, Jon Hunter wrote: > > On 27/09/17 02:57, Dmitry Osipenko wrote: >> On 26.09.2017 17:50, Jon Hunter wrote: >>> >>> On 26/09/17 00:22, Dmitry Osipenko wrote: >>>> 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. > GPIO isn't a very good example, there is no "GPIO" trigger. To me all triggers are software-defined, so that software could create transfer chains. >> And I think the same applies to requester... any objections? > > Well, the REQ_SEL should definitely be in the binding. > Okay. > 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. >
On 27/09/17 13:12, Dmitry Osipenko wrote: > On 27.09.2017 11:34, Jon Hunter wrote: >> >> On 27/09/17 02:57, Dmitry Osipenko wrote: >>> On 26.09.2017 17:50, Jon Hunter wrote: >>>> >>>> On 26/09/17 00:22, Dmitry Osipenko wrote: >>>>> 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. >> > > GPIO isn't a very good example, there is no "GPIO" trigger. To me all triggers > are software-defined, so that software could create transfer chains. TRM shows the following in the APBDMA_TRIG_REG_0 ... "XRQ_A: XRQ.A (GPIOA) (Hardware initiated DMA request)" Jon
On 27/09/17 14:44, Jon Hunter wrote: > > On 27/09/17 13:12, Dmitry Osipenko wrote: >> On 27.09.2017 11:34, Jon Hunter wrote: >>> >>> On 27/09/17 02:57, Dmitry Osipenko wrote: >>>> On 26.09.2017 17:50, Jon Hunter wrote: >>>>> >>>>> On 26/09/17 00:22, Dmitry Osipenko wrote: >>>>>> 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. >>> >> >> GPIO isn't a very good example, there is no "GPIO" trigger. To me all triggers >> are software-defined, so that software could create transfer chains. > > TRM shows the following in the APBDMA_TRIG_REG_0 ... > > "XRQ_A: XRQ.A (GPIOA) (Hardware initiated DMA request)" Furthermore there are timer and hw-semaphore triggers as well. Jon
On 27.09.2017 16:46, Jon Hunter wrote: > > On 27/09/17 14:44, Jon Hunter wrote: >> >> On 27/09/17 13:12, Dmitry Osipenko wrote: >>> On 27.09.2017 11:34, Jon Hunter wrote: >>>> >>>> On 27/09/17 02:57, Dmitry Osipenko wrote: >>>>> On 26.09.2017 17:50, Jon Hunter wrote: >>>>>> >>>>>> On 26/09/17 00:22, Dmitry Osipenko wrote: >>>>>>> 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. >>>> >>> >>> GPIO isn't a very good example, there is no "GPIO" trigger. To me all triggers >>> are software-defined, so that software could create transfer chains. >> >> TRM shows the following in the APBDMA_TRIG_REG_0 ... >> >> "XRQ_A: XRQ.A (GPIOA) (Hardware initiated DMA request)" > > Furthermore there are timer and hw-semaphore triggers as well. > Aha, I wasn't sure about what XRQ is. AHB DMA doesn't have XRQ.A as a trigger, but XRQ.C/D which I suppose corresponds to GPIO C/D. Timer and hw-semaphore are more questionable, aren't semaphores software-only triggerable?
On 09/27, Jon Hunter wrote: > > > Well, the REQ_SEL should definitely be in the binding. > > Laxman, Stephen, what are your thoughts on the TRIG_SEL field? Looks Wrong Stephen?
On 28/09/17 00:32, Stephen Boyd wrote: > On 09/27, Jon Hunter wrote: >> >> >> Well, the REQ_SEL should definitely be in the binding. >> >> Laxman, Stephen, what are your thoughts on the TRIG_SEL field? Looks > > Wrong Stephen? Indeed! With all the folks in copy, I assumed we had the right one :-) Cheers Jon
On 09/27/2017 02:34 AM, Jon Hunter wrote: > > On 27/09/17 02:57, Dmitry Osipenko wrote: >> On 26.09.2017 17:50, Jon Hunter wrote: >>> >>> On 26/09/17 00:22, Dmitry Osipenko wrote: >>>> 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. > >> 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/... -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 29.09.2017 22:30, Stephen Warren wrote: > On 09/27/2017 02:34 AM, Jon Hunter wrote: >> >> On 27/09/17 02:57, Dmitry Osipenko wrote: >>> On 26.09.2017 17:50, Jon Hunter wrote: >>>> >>>> On 26/09/17 00:22, Dmitry Osipenko wrote: >>>>> 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. >> >>> 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. -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/29/2017 09:11 PM, Dmitry Osipenko wrote: > On 29.09.2017 22:30, Stephen Warren wrote: >> On 09/27/2017 02:34 AM, Jon Hunter wrote: >>> >>> On 27/09/17 02:57, Dmitry Osipenko wrote: >>>> On 26.09.2017 17:50, Jon Hunter wrote: >>>>> >>>>> On 26/09/17 00:22, Dmitry Osipenko wrote: >>>>>> 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. >>> >>>> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02.10.2017 20:05, Stephen Warren wrote: > On 09/29/2017 09:11 PM, Dmitry Osipenko wrote: >> On 29.09.2017 22:30, Stephen Warren wrote: >>> On 09/27/2017 02:34 AM, Jon Hunter wrote: >>>> >>>> On 27/09/17 02:57, Dmitry Osipenko wrote: >>>>> On 26.09.2017 17:50, Jon Hunter wrote: >>>>>> >>>>>> On 26/09/17 00:22, Dmitry Osipenko wrote: >>>>>>> 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. >>>> >>>>> 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? I think possible options are: #dma-cells=<1>: req_sel #dma-cells=<1>: trig_sel #dma-cells=<2>: req_sel, trig_sel The only difference between request and trigger is that trigger issues the whole transfer, while request only a single burst. Isn't it possible to have a case in HW for the "trigger-only" option? If not or it's a rareness, then I agree that REQ_SEL must be mandatory. -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/10/17 00:02, Dmitry Osipenko wrote: > On 02.10.2017 20:05, Stephen Warren wrote: >> On 09/29/2017 09:11 PM, Dmitry Osipenko wrote: >>> On 29.09.2017 22:30, Stephen Warren wrote: >>>> On 09/27/2017 02:34 AM, Jon Hunter wrote: >>>>> >>>>> On 27/09/17 02:57, Dmitry Osipenko wrote: >>>>>> On 26.09.2017 17:50, Jon Hunter wrote: >>>>>>> >>>>>>> On 26/09/17 00:22, Dmitry Osipenko wrote: >>>>>>>> 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. >>>>> >>>>>> 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? > > 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? > #dma-cells=<2>: req_sel, trig_sel > > The only difference between request and trigger is that trigger issues the whole > transfer, while request only a single burst. Isn't it possible to have a case in > HW for the "trigger-only" option? If not or it's a rareness, then I agree that > REQ_SEL must be mandatory. I think that what Stephen is proposing is that for now we go with '#dma-cells=<1>' and if we ever need to support the trigger cell we could add support for '#dma-cells=<2>'. So with this proposal the 'req_sel' would always be required for both '#dma-cells=<1>' and '#dma-cells=<2>'. Even if the req_sel is not actually used but the 'trig_sel' is, the user would have to set 'req_sel' to some pre-defined value (eg. -1) where we know to ignore it. Cheers Jon
On 03.10.2017 13:32, Jon Hunter wrote: > > > On 03/10/17 00:02, Dmitry Osipenko wrote: >> On 02.10.2017 20:05, Stephen Warren wrote: >>> On 09/29/2017 09:11 PM, Dmitry Osipenko wrote: >>>> On 29.09.2017 22:30, Stephen Warren wrote: >>>>> On 09/27/2017 02:34 AM, Jon Hunter wrote: >>>>>> >>>>>> On 27/09/17 02:57, Dmitry Osipenko wrote: >>>>>>> On 26.09.2017 17:50, Jon Hunter wrote: >>>>>>>> >>>>>>>> On 26/09/17 00:22, Dmitry Osipenko wrote: >>>>>>>>> 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. >>>>>> >>>>>>> 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? >> >> 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? > >> #dma-cells=<2>: req_sel, trig_sel >> >> The only difference between request and trigger is that trigger issues the whole >> transfer, while request only a single burst. Isn't it possible to have a case in >> HW for the "trigger-only" option? If not or it's a rareness, then I agree that >> REQ_SEL must be mandatory. > > I think that what Stephen is proposing is that for now we go with > '#dma-cells=<1>' and if we ever need to support the trigger cell we > could add support for '#dma-cells=<2>'. So with this proposal the > 'req_sel' would always be required for both '#dma-cells=<1>' and > '#dma-cells=<2>'. Even if the req_sel is not actually used but the > 'trig_sel' is, the user would have to set 'req_sel' to some pre-defined > value (eg. -1) where we know to ignore it. > Okay, I see now. Thank you for the clarification, but then we should have that pre-defined value declared in the binding? -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/10/17 13:07, Dmitry Osipenko wrote: > On 03.10.2017 13:32, Jon Hunter wrote: >> >> >> On 03/10/17 00:02, Dmitry Osipenko wrote: >>> On 02.10.2017 20:05, Stephen Warren wrote: >>>> On 09/29/2017 09:11 PM, Dmitry Osipenko wrote: >>>>> On 29.09.2017 22:30, Stephen Warren wrote: >>>>>> On 09/27/2017 02:34 AM, Jon Hunter wrote: >>>>>>> >>>>>>> On 27/09/17 02:57, Dmitry Osipenko wrote: >>>>>>>> On 26.09.2017 17:50, Jon Hunter wrote: >>>>>>>>> >>>>>>>>> On 26/09/17 00:22, Dmitry Osipenko wrote: >>>>>>>>>> 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. >>>>>>> >>>>>>>> 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? >>> >>> 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? >> >>> #dma-cells=<2>: req_sel, trig_sel >>> >>> The only difference between request and trigger is that trigger issues the whole >>> transfer, while request only a single burst. Isn't it possible to have a case in >>> HW for the "trigger-only" option? If not or it's a rareness, then I agree that >>> REQ_SEL must be mandatory. >> >> I think that what Stephen is proposing is that for now we go with >> '#dma-cells=<1>' and if we ever need to support the trigger cell we >> could add support for '#dma-cells=<2>'. So with this proposal the >> 'req_sel' would always be required for both '#dma-cells=<1>' and >> '#dma-cells=<2>'. Even if the req_sel is not actually used but the >> 'trig_sel' is, the user would have to set 'req_sel' to some pre-defined >> value (eg. -1) where we know to ignore it. >> > > Okay, I see now. Thank you for the clarification, but then we should have that > pre-defined value declared in the binding? I would have thought it should be in the dt-binding header. Cheers Jon
On 10/03/2017 04:32 AM, Jon Hunter wrote: > > > On 03/10/17 00:02, Dmitry Osipenko wrote: >> On 02.10.2017 20:05, Stephen Warren wrote: >>> On 09/29/2017 09:11 PM, Dmitry Osipenko wrote: >>>> On 29.09.2017 22:30, Stephen Warren wrote: >>>>> On 09/27/2017 02:34 AM, Jon Hunter wrote: >>>>>> >>>>>> On 27/09/17 02:57, Dmitry Osipenko wrote: >>>>>>> On 26.09.2017 17:50, Jon Hunter wrote: >>>>>>>> >>>>>>>> On 26/09/17 00:22, Dmitry Osipenko wrote: >>>>>>>>> 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. >>>>>> >>>>>>> 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. >> 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. -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03.10.2017 18:38, Stephen Warren wrote: > On 10/03/2017 04:32 AM, Jon Hunter wrote: >> >> >> On 03/10/17 00:02, Dmitry Osipenko wrote: >>> On 02.10.2017 20:05, Stephen Warren wrote: >>>> On 09/29/2017 09:11 PM, Dmitry Osipenko wrote: >>>>> On 29.09.2017 22:30, Stephen Warren wrote: >>>>>> On 09/27/2017 02:34 AM, Jon Hunter wrote: >>>>>>> >>>>>>> On 27/09/17 02:57, Dmitry Osipenko wrote: >>>>>>>> On 26.09.2017 17:50, Jon Hunter wrote: >>>>>>>>> >>>>>>>>> On 26/09/17 00:22, Dmitry Osipenko wrote: >>>>>>>>>> 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. >>>>>>> >>>>>>>> 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. > >>> 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. -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Sep 26, 2017 at 02:22:04AM +0300, Dmitry Osipenko wrote: > 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. > + > +Example: > + > +ahbdma: ahbdma@60008000 { Use standard node names. dma-controller in this case. > + compatible = "nvidia,tegra20-ahbdma"; > + reg = <0x60008000 0x2000>; > + interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&tegra_car TEGRA20_CLK_AHBDMA>; > + resets = <&tegra_car 33>; > + #dma-cells = <1>; > +}; > -- > 2.14.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05.10.2017 23:33, Rob Herring wrote: > On Tue, Sep 26, 2017 at 02:22:04AM +0300, Dmitry Osipenko wrote: >> 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. >> + >> +Example: >> + >> +ahbdma: ahbdma@60008000 { > > Use standard node names. dma-controller in this case. > Okay, I'll change it in v3. Thank you for the comment. >> + compatible = "nvidia,tegra20-ahbdma"; >> + reg = <0x60008000 0x2000>; >> + interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>; >> + clocks = <&tegra_car TEGRA20_CLK_AHBDMA>; >> + resets = <&tegra_car 33>; >> + #dma-cells = <1>; >> +}; >> -- >> 2.14.1 >> -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. + +Example: + +ahbdma: ahbdma@60008000 { + compatible = "nvidia,tegra20-ahbdma"; + reg = <0x60008000 0x2000>; + interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&tegra_car TEGRA20_CLK_AHBDMA>; + resets = <&tegra_car 33>; + #dma-cells = <1>; +};
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