Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1241067 > unrolled thread

Re: [PATCH V2 1/2] Documentation: DT: Add binding documentation for NVIDIA ADMA

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH V2 1/2] Documentation: DT: Add binding documentation for  NVIDIA ADMA Stephen Warren <swarren@wwwdotorg.org> - 2015-10-07 01:10 +0200
    Re: [PATCH V2 1/2] Documentation: DT: Add binding documentation for  NVIDIA ADMA Jon Hunter <jonathanh@nvidia.com> - 2015-10-07 10:50 +0200
      Re: [PATCH V2 1/2] Documentation: DT: Add binding documentation for  NVIDIA ADMA Stephen Warren <swarren@wwwdotorg.org> - 2015-10-07 18:10 +0200
        Re: [PATCH V2 1/2] Documentation: DT: Add binding documentation for  NVIDIA ADMA Jon Hunter <jonathanh@nvidia.com> - 2015-10-07 18:30 +0200
          Re: [PATCH V2 1/2] Documentation: DT: Add binding documentation for  NVIDIA ADMA Stephen Warren <swarren@wwwdotorg.org> - 2015-10-07 21:40 +0200
            Re: [PATCH V2 1/2] Documentation: DT: Add binding documentation for  NVIDIA ADMA Jon Hunter <jonathanh@nvidia.com> - 2015-10-08 12:00 +0200
              Re: [PATCH V2 1/2] Documentation: DT: Add binding documentation for  NVIDIA ADMA Stephen Warren <swarren@wwwdotorg.org> - 2015-10-08 16:30 +0200
                Re: [PATCH V2 1/2] Documentation: DT: Add binding documentation for  NVIDIA ADMA Jon Hunter <jonathanh@nvidia.com> - 2015-10-09 12:30 +0200
                  Re: [PATCH V2 1/2] Documentation: DT: Add binding documentation for  NVIDIA ADMA Stephen Warren <swarren@wwwdotorg.org> - 2015-10-09 17:30 +0200
                    Re: [PATCH V2 1/2] Documentation: DT: Add binding documentation for  NVIDIA ADMA Jon Hunter <jonathanh@nvidia.com> - 2015-10-12 16:00 +0200
                      Re: [PATCH V2 1/2] Documentation: DT: Add binding documentation for  NVIDIA ADMA Stephen Warren <swarren@wwwdotorg.org> - 2015-10-12 20:00 +0200
                        Re: [PATCH V2 1/2] Documentation: DT: Add binding documentation for  NVIDIA ADMA Jon Hunter <jonathanh@nvidia.com> - 2015-10-13 15:00 +0200
      Re: [PATCH V2 1/2] Documentation: DT: Add binding documentation for  NVIDIA ADMA Mark Rutland <mark.rutland@arm.com> - 2015-10-07 18:40 +0200

#1241067 — Re: [PATCH V2 1/2] Documentation: DT: Add binding documentation for NVIDIA ADMA

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1241239

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?

By the way, in the ADMA driver, the driver does check that the direction
specified when configuring the transfer (ie. via
tegra_adma_prep_dma_cyclic()) matches the adma channel being used.

Cheers
Jon

[0]
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/dma/ste-dma40.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1241643

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.

Perhaps the core DMA DT bindings are not designed that way though, in 
which case I suppose the patch you sent makes sense. If so though, that 
seems like a bug in the core DMA DT bindings.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1241665

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? If it
is the request signal then I don't see how this would work because there
are 10 rx request signals and 10 tx requests signal and both are 1-10.
If you look at the ADMA_CH<n>_CTRL_0 register you will see there are a
fields for the TX_REQUEST_SELECT, RX_REQUEST_SELECT and
TRANSFER_DIRECTION. It seems a bit silly to have both TX_REQUEST_SELECT
and RX_REQUEST_SELECT as the channel can only work with one direction at
any given time.

> Perhaps the core DMA DT bindings are not designed that way though, in
> which case I suppose the patch you sent makes sense. If so though, that
> seems like a bug in the core DMA DT bindings.

I think it is more of a nuance in how this DMA controller is configured.

Jon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1241754

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?

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.

Thinking about this more, I think actually that the dmas/dma-names 
property example that you posted above is exactly what is required here. 
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.

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1242139

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";
     ...
};

Jon



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1242411

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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1243225

On 08/10/15 15:27, Stephen Warren wrote:
> On 10/08/2015 03:58 AM, Jon Hunter wrote:

[snip]

>> 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.

One more clarifying question ... should the xlate verify that no other
dma channel is using the same hardware request signal?

I understand that typically the xlate decodes the binding to get the
channel info, but because this is invoked by dmaengine while allocating
a channel, I was wondering if we should prevent dmaengine allocating
more than one channel to be used with the same hardware request? If so,
then passing the direction to the xlate would be necessary (so I can
determine in the xlate that no one else is currently using this, which
is what I currently do).

Alternatively, I could check that no one else is using the request
signal at a later when the transfer is being prepared.

If you are wondering why I am worried about this, I my mind I think that
the driver should be robust enough to check for conflicts in the request
signals used by the various channels.

Jon


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1243475

On 10/09/2015 04:20 AM, Jon Hunter wrote:
>
> On 08/10/15 15:27, Stephen Warren wrote:
>> On 10/08/2015 03:58 AM, Jon Hunter wrote:
>
> [snip]
>
>>> 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.
>
> One more clarifying question ... should the xlate verify that no other
> dma channel is using the same hardware request signal?
>
> I understand that typically the xlate decodes the binding to get the
> channel info, but because this is invoked by dmaengine while allocating
> a channel, I was wondering if we should prevent dmaengine allocating
> more than one channel to be used with the same hardware request? If so,
> then passing the direction to the xlate would be necessary (so I can
> determine in the xlate that no one else is currently using this, which
> is what I currently do).
>
> Alternatively, I could check that no one else is using the request
> signal at a later when the transfer is being prepared.

I think that handling this at prepare/usage time is probably most 
appropriate. That is the time when the resource conflict /actually/ occurs.

The only time when two clients would be given the same DMA request 
signal is if there are multiple different drivers that can DMA into the 
same FIFO in a time-multiplexed fashion. That seems pretty unlikely off 
the top of my head, but I don't think we want to actively ban that, in 
case we come up with a cunning use-case for it.

> If you are wondering why I am worried about this, I my mind I think that
> the driver should be robust enough to check for conflicts in the request
> signals used by the various channels.

Sure, makes sense.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1244711

On 09/10/15 16:26, Stephen Warren wrote:
> On 10/09/2015 04:20 AM, Jon Hunter wrote:
>>
>> On 08/10/15 15:27, Stephen Warren wrote:
>>> On 10/08/2015 03:58 AM, Jon Hunter wrote:
>>
>> [snip]
>>
>>>> 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.
>>
>> One more clarifying question ... should the xlate verify that no other
>> dma channel is using the same hardware request signal?
>>
>> I understand that typically the xlate decodes the binding to get the
>> channel info, but because this is invoked by dmaengine while allocating
>> a channel, I was wondering if we should prevent dmaengine allocating
>> more than one channel to be used with the same hardware request? If so,
>> then passing the direction to the xlate would be necessary (so I can
>> determine in the xlate that no one else is currently using this, which
>> is what I currently do).
>>
>> Alternatively, I could check that no one else is using the request
>> signal at a later when the transfer is being prepared.
> 
> I think that handling this at prepare/usage time is probably most
> appropriate. That is the time when the resource conflict /actually/ occurs.

Although that makes sense, the more I look at this, the more I think it
should be handled during the channel allocate/free phases as it makes
sense to allocate the required resources then. It is probably simpler
and safer too.

> The only time when two clients would be given the same DMA request
> signal is if there are multiple different drivers that can DMA into the
> same FIFO in a time-multiplexed fashion. That seems pretty unlikely off
> the top of my head, but I don't think we want to actively ban that, in
> case we come up with a cunning use-case for it.

I know this is purely an example, but if such a time-multiplexed scheme
was a real use-case, then it would seem more likely to have a shim layer
between the clients that talked to the dmaengine and hence, it would
still only be necessary for one client to interface to a given channel.

What I don't like about the above binding is that someone can request
the dma channel "tx5" and then call dmaengine_prep_dma_cyclic() and say
you know what, I am gonna receive data instead. That seems odd and I
think that such a scenario should be greeted with an error code of
-EINVAL. It seems to me that if channels are uni-directional (in the
sense you either use it for tx or rx), you should request the
appropriate channel for the direction you want and then set the
direction in dmaengine_prep_dma_cyclic() so that it matches and if it
does not then we return an error.

So I still like the idea of the direction of the request being in the
binding so we know what the client intends (sorry to keep changing my
mind). Do you completely deplore the idea?

Jon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1244967

On 10/12/2015 07:55 AM, Jon Hunter wrote:
>
> On 09/10/15 16:26, Stephen Warren wrote:
>> On 10/09/2015 04:20 AM, Jon Hunter wrote:
>>>
>>> On 08/10/15 15:27, Stephen Warren wrote:
>>>> On 10/08/2015 03:58 AM, Jon Hunter wrote:
>>>
>>> [snip]
>>>
>>>>> 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.
>>>
>>> One more clarifying question ... should the xlate verify that no other
>>> dma channel is using the same hardware request signal?
>>>
>>> I understand that typically the xlate decodes the binding to get the
>>> channel info, but because this is invoked by dmaengine while allocating
>>> a channel, I was wondering if we should prevent dmaengine allocating
>>> more than one channel to be used with the same hardware request? If so,
>>> then passing the direction to the xlate would be necessary (so I can
>>> determine in the xlate that no one else is currently using this, which
>>> is what I currently do).
>>>
>>> Alternatively, I could check that no one else is using the request
>>> signal at a later when the transfer is being prepared.
>>
>> I think that handling this at prepare/usage time is probably most
>> appropriate. That is the time when the resource conflict /actually/ occurs.
>
> Although that makes sense, the more I look at this, the more I think it
> should be handled during the channel allocate/free phases as it makes
> sense to allocate the required resources then. It is probably simpler
> and safer too.

I fail to see how it's simpler or safer. Everything is still 100% safe 
if the checks are handled when the channel is actually used. I think 
it's simpler too, since there's less to worry about in DT, and less 
state to carry around in the code.

>> The only time when two clients would be given the same DMA request
>> signal is if there are multiple different drivers that can DMA into the
>> same FIFO in a time-multiplexed fashion. That seems pretty unlikely off
>> the top of my head, but I don't think we want to actively ban that, in
>> case we come up with a cunning use-case for it.
>
> I know this is purely an example, but if such a time-multiplexed scheme
> was a real use-case, then it would seem more likely to have a shim layer
> between the clients that talked to the dmaengine and hence, it would
> still only be necessary for one client to interface to a given channel.

I don't agree at all. There's no reason why clients shouldn't simply go 
to the dmaengine code and request to use channels when they need them. 
Why would a shim layer be needed for that?

> What I don't like about the above binding is that someone can request
> the dma channel "tx5" and then call dmaengine_prep_dma_cyclic() and say
> you know what, I am gonna receive data instead.

It's always possible to write bugs. The DMA binding can't fix that.

> That seems odd and I
> think that such a scenario should be greeted with an error code of
> -EINVAL. It seems to me that if channels are uni-directional (in the
> sense you either use it for tx or rx), you should request the
> appropriate channel for the direction you want and then set the
> direction in dmaengine_prep_dma_cyclic() so that it matches and if it
> does not then we return an error.

Channels (in HW) are uni-directional for a particular transfer, but can 
operate in any arbitrary combination of directions for different transfers.

Do note that the name "tx5" is something 100% isolated to the client of 
the DMA channel and meaningless for the DMA controller itself. This is 
simply a name that the client uses to look up data that it must pass to 
the DMA controller.

> So I still like the idea of the direction of the request being in the
> binding so we know what the client intends (sorry to keep changing my
> mind). Do you completely deplore the idea?

I still believe it's wrong yes.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1245683

On 12/10/15 18:51, Stephen Warren wrote:
> On 10/12/2015 07:55 AM, Jon Hunter wrote:
>>
>> On 09/10/15 16:26, Stephen Warren wrote:
>>> On 10/09/2015 04:20 AM, Jon Hunter wrote:
>>>>
>>>> On 08/10/15 15:27, Stephen Warren wrote:
>>>>> On 10/08/2015 03:58 AM, Jon Hunter wrote:
>>>>
>>>> [snip]
>>>>
>>>>>> 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.
>>>>
>>>> One more clarifying question ... should the xlate verify that no other
>>>> dma channel is using the same hardware request signal?
>>>>
>>>> I understand that typically the xlate decodes the binding to get the
>>>> channel info, but because this is invoked by dmaengine while allocating
>>>> a channel, I was wondering if we should prevent dmaengine allocating
>>>> more than one channel to be used with the same hardware request? If so,
>>>> then passing the direction to the xlate would be necessary (so I can
>>>> determine in the xlate that no one else is currently using this, which
>>>> is what I currently do).
>>>>
>>>> Alternatively, I could check that no one else is using the request
>>>> signal at a later when the transfer is being prepared.
>>>
>>> I think that handling this at prepare/usage time is probably most
>>> appropriate. That is the time when the resource conflict /actually/
>>> occurs.
>>
>> Although that makes sense, the more I look at this, the more I think it
>> should be handled during the channel allocate/free phases as it makes
>> sense to allocate the required resources then. It is probably simpler
>> and safer too.
> 
> I fail to see how it's simpler or safer. Everything is still 100% safe
> if the checks are handled when the channel is actually used. I think
> it's simpler too, since there's less to worry about in DT, and less
> state to carry around in the code.

Safer/simpler in the sense that there are less potential paths to deal
with (ie. completion paths). That's all.

>>> The only time when two clients would be given the same DMA request
>>> signal is if there are multiple different drivers that can DMA into the
>>> same FIFO in a time-multiplexed fashion. That seems pretty unlikely off
>>> the top of my head, but I don't think we want to actively ban that, in
>>> case we come up with a cunning use-case for it.
>>
>> I know this is purely an example, but if such a time-multiplexed scheme
>> was a real use-case, then it would seem more likely to have a shim layer
>> between the clients that talked to the dmaengine and hence, it would
>> still only be necessary for one client to interface to a given channel.
> 
> I don't agree at all. There's no reason why clients shouldn't simply go
> to the dmaengine code and request to use channels when they need them.
> Why would a shim layer be needed for that?

I was just thinking about the granularity of time-multiplexing and
overhead of swapping the sync back and forth versus formatting the
buffer prior to submitting to dmaengine.

>> What I don't like about the above binding is that someone can request
>> the dma channel "tx5" and then call dmaengine_prep_dma_cyclic() and say
>> you know what, I am gonna receive data instead.
> 
> It's always possible to write bugs. The DMA binding can't fix that.

Shame ;-)

>> That seems odd and I
>> think that such a scenario should be greeted with an error code of
>> -EINVAL. It seems to me that if channels are uni-directional (in the
>> sense you either use it for tx or rx), you should request the
>> appropriate channel for the direction you want and then set the
>> direction in dmaengine_prep_dma_cyclic() so that it matches and if it
>> does not then we return an error.
> 
> Channels (in HW) are uni-directional for a particular transfer, but can
> operate in any arbitrary combination of directions for different transfers.
> 
> Do note that the name "tx5" is something 100% isolated to the client of
> the DMA channel and meaningless for the DMA controller itself. This is
> simply a name that the client uses to look up data that it must pass to
> the DMA controller.

Yes.

>> So I still like the idea of the direction of the request being in the
>> binding so we know what the client intends (sorry to keep changing my
>> mind). Do you completely deplore the idea?
> 
> I still believe it's wrong yes.

Won't be my first time ;-)

I was trying to describe the sync and it seems for this DMA, to describe
each hardware sync you would need the index and direction. So putting
both in the binding does not seem entirely bonkers. However, I concede
that in this case the direction is somewhat negligible because as long
as the driver can handle any conflict between clients (w.r.t syncs), the
client just needs the index of the appropriate dma interface.

Jon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1241670

On Wed, Oct 07, 2015 at 09:43:45AM +0100, 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?

The client shouldn't know anything about the format of the specifier. If
the client expects separate rx and tx DMA channels, it should describe
these separately with names as above, regardless of whether the DMA
controller requires TX and RX channels to be described differently.

I don't know if it make sense to also verify this from the DMA
controller's PoV.

Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web