lkml.org 
[lkml]   [2015]   [Oct]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH V2 1/2] Documentation: DT: Add binding documentation for NVIDIA ADMA
    From
    Date
    On 10/08/2015 03:58 AM, Jon Hunter wrote:
    >
    > On 07/10/15 20:36, Stephen Warren wrote:
    >> On 10/07/2015 10:19 AM, Jon Hunter wrote:
    >>>
    >>> On 07/10/15 17:09, Stephen Warren wrote:
    >>>> On 10/07/2015 02:43 AM, Jon Hunter wrote:
    >>>>>
    >>>>> On 07/10/15 00:04, Stephen Warren wrote:
    >>>>>> On 10/05/2015 06:10 AM, Jon Hunter wrote:
    >>>>>>> Add device-tree binding documentation for the Tegra210 Audio DMA
    >>>>>>> controller.
    >>>>>>
    >>>>>>> diff --git a/Documentation/devicetree/bindings/dma/tegra210-adma.txt
    >>>>>>> b/Documentation/devicetree/bindings/dma/tegra210-adma.txt
    >>>>>>
    >>>>>>> +- #dma-cells : Must be <2>. The first cell denotes the transmit or
    >>>>>>> + receive request number and should be between 1 and the maximum
    >>>>>>> number
    >>>>>>> + of requests supported (see properties "dma-rx-requests" and
    >>>>>>> + "dma-tx-requests"). This value corresponds to the
    >>>>>>> RX/TX_REQUEST_SELECT
    >>>>>>> + fields in the ADMA_CHn_CTRL register. The second cell denotes
    >>>>>>> whether
    >>>>>>> + the channel is a receive or transmit channel and must be either 2
    >>>>>>> for
    >>>>>>> + a receive channel and 4 for a transmit channel. These values
    >>>>>>> correspond
    >>>>>>> + to the TRANSFER_DIRECTION field of the ADMA_CHn_CTRL register.
    >>>>>>
    >>>>>> Is it typical to encode the direction into the dma cells? I would have
    >>>>>> thought the client would provide that information at run-time when
    >>>>>> requesting a DMA channel.
    >>>>>
    >>>>> I have seen other dma bindings that do [0]. If we don't put the
    >>>>> direction in the client binding, then it would appear as ...
    >>>>>
    >>>>> tegra_admaif: admaif@0x702d0000 {
    >>>>> ...
    >>>>> dmas = <&adma 1>, <&adma 1>, <&adma 2>, <&adma 2>,
    >>>>> <&adma 3>, <&adma 3>, <&adma 4>, <&adma 4>,
    >>>>> <&adma 5>, <&adma 5>, <&adma 6>, <&adma 6>,
    >>>>> <&adma 7>, <&adma 7>, <&adma 8>, <&adma 8>,
    >>>>> <&adma 9>, <&adma 9>, <&adma 10>, <&adma 10>;
    >>>>> dma-names = "rx1", "tx1", "rx2", "tx2", "rx3", "tx3",
    >>>>> "rx4", "tx4", "rx5", "tx5", "rx6", "tx6",
    >>>>> "rx7", "tx7", "rx8", "tx8", "rx9", "tx9",
    >>>>> "rx10", "tx10";
    >>>>> ...
    >>>>> };
    >>>>>
    >>>>> ... where "rxN" and "txN" appear to use the same request, but the
    >>>>> reality is that "rxN" is using rx-request-N and "txN" is using
    >>>>> tx-request-N. Arnd questioned this before. Obviously I can explain this
    >>>>> in the binding document if the above is preferred. However, given that
    >>>>> they are named "rx1", "rx2", etc, why not put the direction in the
    >>>>> binding?
    >>>>
    >>>> Why would we need to duplicate the request IDs? I'd expect to have the
    >>>> following property content:
    >>>>
    >>>> dmas = <&adma 1>, <&adma 2>, <&adma 3>, ...;
    >>>> dma-names = "1", "2", "3"...;
    >>>>
    >>>> *and* not have a cell to represent the direction in DT. After all, the
    >>>> direction of the channel is 100% implied by the use-case (and hence
    >>>> defined by the DMA client's own DT binding); it is known by the client
    >>>> driver and can be supplied at run-time.
    >>>
    >>> Right, but what does the 1, 2, 3, etc in the specifier represent?
    >>
    >> Aren't they the ADMAIF FIFO IDs?
    >
    > Yes.
    >
    >> We know the set/number of ADMAIF FIFOs, and each FIFO needs a request
    >> selector ID. The list of those can be indexed by the identity of the
    >> FIFO that is accessed via DMA.
    >
    > Right, but you have 10 RX FIFOs and 10 TX FIFOs. The FIFOs are
    > unidirectional. This means that instead of having 20 FIFOs from 1-20
    > (yes the FIFOs start from 1 and not 0), you have RX FIFOs from 1-10 and
    > TX FIFOs from 1-10.
    >
    >> Thinking about this more, I think actually that the dmas/dma-names
    >> property example that you posted above is exactly what is required here.
    >
    > Ok, good.
    >
    >> The counter-example I wrote makes this assumption and hence is invalid.
    >> The ADMAIF binding should not assume that the RX and TX request selector
    >> IDs are identical. As such, dmas/dma-names should have both an RX and TX
    >> entry for each ADMAIF FIFO.
    >
    > Yes.
    >
    >> Still, there's no need to encode the DMA direction into the #dma-cells.
    >> The client code will know that if it wants to configure DMA into (TX to)
    >> FIFO ID 5, it must query dma-names entry "tx5", and simply use whatever
    >> is in the DT. When it passes that DMA specifier to the DMA API, the
    >> ADMAIF driver knows that it will be for TX, and can pass that
    >> information to the DMA code.
    >
    > That's fine. From my perspective I don't have a strong objection either
    > way, however, I can see that given that the name indicates rx or tx,
    > then the direction in the binding could be seen as redundant.
    >
    > So to confirm you are happy with the client bindings being as follows?
    >
    > tegra_admaif: admaif@0x702d0000 {
    > ...
    > dmas = <&adma 1>, <&adma 1>, <&adma 2>, <&adma 2>,
    > <&adma 3>, <&adma 3>, <&adma 4>, <&adma 4>,
    > <&adma 5>, <&adma 5>, <&adma 6>, <&adma 6>,
    > <&adma 7>, <&adma 7>, <&adma 8>, <&adma 8>,
    > <&adma 9>, <&adma 9>, <&adma 10>, <&adma 10>;
    > dma-names = "rx1", "tx1", "rx2", "tx2", "rx3", "tx3",
    > "rx4", "tx4", "rx5", "tx5", "rx6", "tx6",
    > "rx7", "tx7", "rx8", "tx8", "rx9", "tx9",
    > "rx10", "tx10";
    > ...
    > };

    Yes, that looks good for the client binding.



    \
     
     \ /
      Last update: 2015-10-08 16:41    [W:6.207 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site