linux-kernel - Re: [PATCH 10/16] dmaengine: ti: New driver for K3 UDMA

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]

Message-ID: <7229677c-29c5-8c1f-2218-ff51ed57b8d0@ti.com>
Date:   Thu, 13 Jun 2019 23:40:59 +0300
From:   Peter Ujfalusi <peter.ujfalusi@...com>
To:     Rob Herring <robh@...nel.org>
CC:     <vkoul@...nel.org>, <nm@...com>, <ssantosh@...nel.org>,
        <dan.j.williams@...el.com>, <dmaengine@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <grygorii.strashko@...com>, <lokeshvutla@...com>,
        <t-kristo@...com>, <tony@...mide.com>
Subject: Re: [PATCH 10/16] dmaengine: ti: New driver for K3 UDMA - split#1:
 defines, structs, io func

Rob,

On 13/06/2019 21.43, Rob Herring wrote:
> On Mon, May 06, 2019 at 03:34:50PM +0300, Peter Ujfalusi wrote:
>> Split patch for review containing: defines, structs, io and low level
>> functions and interrupt callbacks.
> 
> Not a useful comment for upstream.

Vinod asked me to split the patch to smaller pieces for review. This is
just a short note on what this part covers. The real commit message
follows under.

>> DMA driver for
>> Texas Instruments K3 NAVSS Unified DMA – Peripheral Root Complex (UDMA-P)
>>
>> The UDMA-P is intended to perform similar (but significantly upgraded) functions
>> as the packet-oriented DMA used on previous SoC devices. The UDMA-P module
>> supports the transmission and reception of various packet types. The UDMA-P is
>> architected to facilitate the segmentation and reassembly of SoC DMA data
>> structure compliant packets to/from smaller data blocks that are natively
>> compatible with the specific requirements of each connected peripheral. Multiple
>> Tx and Rx channels are provided within the DMA which allow multiple segmentation
>> or reassembly operations to be ongoing. The DMA controller maintains state
>> information for each of the channels which allows packet segmentation and
>> reassembly operations to be time division multiplexed between channels in order
>> to share the underlying DMA hardware. An external DMA scheduler is used to
>> control the ordering and rate at which this multiplexing occurs for Transmit
>> operations. The ordering and rate of Receive operations is indirectly controlled
>> by the order in which blocks are pushed into the DMA on the Rx PSI-L interface.
>>
>> The UDMA-P also supports acting as both a UTC and UDMA-C for its internal
>> channels. Channels in the UDMA-P can be configured to be either Packet-Based or
>> Third-Party channels on a channel by channel basis.
>>
>> The initial driver supports:
>> - MEM_TO_MEM (TR mode)
>> - DEV_TO_MEM (Packet / TR mode)
>> - MEM_TO_DEV (Packet / TR mode)
>> - Cyclic (Packet / TR mode)
>> - Metadata for descriptors
>>
>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@...com>
>> ---
>>  drivers/dma/ti/k3-udma.c          | 1008 +++++++++++++++++++++++++++++
>>  drivers/dma/ti/k3-udma.h          |  129 ++++
>>  include/dt-bindings/dma/k3-udma.h |   26 +
> 
> This belongs in the binding patch.

OK, I'll move it.

> 
>>  3 files changed, 1163 insertions(+)
>>  create mode 100644 drivers/dma/ti/k3-udma.c
>>  create mode 100644 drivers/dma/ti/k3-udma.h
>>  create mode 100644 include/dt-bindings/dma/k3-udma.h
> 
>> diff --git a/include/dt-bindings/dma/k3-udma.h b/include/dt-bindings/dma/k3-udma.h
>> new file mode 100644
>> index 000000000000..89ba6a9d4a8f
>> --- /dev/null
>> +++ b/include/dt-bindings/dma/k3-udma.h
>> @@ -0,0 +1,26 @@
>> +#ifndef __DT_TI_UDMA_H
>> +#define __DT_TI_UDMA_H
>> +
>> +#define UDMA_TR_MODE		0
>> +#define UDMA_PKT_MODE		1
>> +
>> +#define UDMA_DIR_TX		0
>> +#define UDMA_DIR_RX		1
>> +
>> +#define PSIL_STATIC_TR_NONE	0
>> +#define PSIL_STATIC_TR_XY	1
>> +#define PSIL_STATIC_TR_MCAN	2
>> +
>> +#define UDMA_PDMA_TR_XY(id)				\
>> +	ti,psil-config##id {				\
>> +		linux,udma-mode = <UDMA_TR_MODE>;	\
>> +		statictr-type = <PSIL_STATIC_TR_XY>;	\
>> +	}
> 
> We don't accept this kind of complex macros in dts files. It obfuscates 
> reading dts files.

I see. I agree that it obfuscates things as you need to look it up in
the header, but as I mentioned regarding to patch 9 we have PDMAs with
22 threads needing 22 psil-config section for the threads. It makes the
reading of the DT a bit hard and also error prone when you populate things.

But I can drop the macro and write all psil-config0...22

Hrm, so we have 23 threads in some PDMA...

Thanks,
- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki