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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 7 Oct 2015 17:19:53 +0100
From:	Jon Hunter <jonathanh@...dia.com>
To:	Stephen Warren <swarren@...dotorg.org>
CC:	Laxman Dewangan <ldewangan@...dia.com>,
	Vinod Koul <vinod.koul@...el.com>,
	Thierry Reding <thierry.reding@...il.com>,
	Alexandre Courbot <gnurou@...il.com>,
	Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	"Arnd Bergmann" <arnd@...db.de>, <dmaengine@...r.kernel.org>,
	<linux-tegra@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V2 1/2] Documentation: DT: Add binding documentation for
 NVIDIA ADMA


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@...02d0000 {
>>      ...
>>      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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/