linux-kernel - Re: [PATCH v2 1/4] dt-bindings: Document the STM32 DMA bindings

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: <20151013142240.GC24353@leverpostej>
Date:	Tue, 13 Oct 2015 15:22:40 +0100
From:	Mark Rutland <mark.rutland@....com>
To:	M'boumba Cedric Madianga <cedric.madianga@...il.com>
Cc:	mcoquelin.stm32@...il.com, robh+dt@...nel.org, pawel.moll@....com,
	ijc+devicetree@...lion.org.uk, galak@...eaurora.org,
	linux@....linux.org.uk, vinod.koul@...el.com,
	linux-arm-kernel@...ts.infradead.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, dmaengine@...r.kernel.org
Subject: Re: [PATCH v2 1/4] dt-bindings: Document the STM32 DMA bindings

On Tue, Oct 13, 2015 at 04:00:45PM +0200, M'boumba Cedric Madianga wrote:
> This patch adds documentation of device tree bindings for the STM32 dma
> controller.
> 
> Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@...il.com>
> ---
>  .../devicetree/bindings/dma/stm32-dma.txt          | 98 ++++++++++++++++++++++
>  1 file changed, 98 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/dma/stm32-dma.txt
> 
> diff --git a/Documentation/devicetree/bindings/dma/stm32-dma.txt b/Documentation/devicetree/bindings/dma/stm32-dma.txt
> new file mode 100644
> index 0000000..9ce0d49
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/stm32-dma.txt
> @@ -0,0 +1,98 @@
> +* STMicroelectronics STM32 DMA controller
> +
> +The STM32 DMA is a general-purpose direct memory access controller capable of
> +supporting 8 independent DMA channels. Each channel can have up to 8 requests.
> +
> +Required properties:
> +- compatible: Should be "st,stm32-dma"
> +- reg: Should contain DMA registers location and length. This should include
> +  all of the per-channel registers.
> +- interrupts: Should contain all of the per-channel DMA interrupts.

Please specify the order they must be in.

> +- clocks: Should contain the input clock of the DMA instance.
> +- #dma-cells : Must be <4>. See DMA client paragraph for more details.
> +
> +Optional properties:
> +- resets: Reference to a reset controller asserting the DMA controller
> +- st,mem2mem: boolean; if defined, it indicates that the controller supports
> +  memory-to-memory transfer
> +
> +Example:
> +
> +	dma2: dma-controller@...26400 {
> +		compatible = "st,stm32-dma";
> +		reg = <0x40026400 0x400>;
> +		interrupts = <56>,
> +			     <57>,
> +			     <58>,
> +			     <59>,
> +			     <60>,
> +			     <68>,
> +			     <69>,
> +			     <70>;
> +		clocks = <&clk_hclk>;
> +		#dma-cells = <4>;
> +		st,mem2mem;
> +		resets = <&rcc 150>;
> +	};
> +
> +* DMA client
> +
> +Required properties:
> +- dmas: Comma separated list of dma channel requests
> +- dma-names: Names of the aforementioned requested channels
> +
> +Each dmas request consists of 5 cells:
> +1. A phandle pointing to the STM32 DMA controller
> +2. The channel id
> +3. The request line number
> +4. A 32bit mask specifying the DMA channel configuration
> + -bit 1: Direct Mode Error Interrupt
> +	0x0: disabled
> +	0x1: enabled
> + -bit 2: Transfer Error Interrupt
> +	0x0: disabled
> +	0x1: enabled
> + -bit 3: Half Transfer Mode Error Interrupt
> +	0x0: disabled
> +	0x1: enabled
> + -bit 4: Transfer Complete Interrupt
> +	0x0: disabled
> +	0x1: enabled

Why should this be in the DT?

Surely the driver should be in charge of deciding when to use these?

> + -bit 9: Peripheral Increment Address
> +	0x0: no address increment between transfers
> +	0x1: increment address between transfers
> + -bit 10: Memory Increment Address
> +	0x0: no address increment between transfers
> +	0x1: increment address between transfers

These don't seem like they belong either. Surely this would depend on
the request made, rather than being a fixed property?

> + -bit 15: Peripheral Increment Offset Size
> +	0x0: offset size is linked to the peripheral bus width
> +	0x1: offset size is fixed to 4 (32-bit alignment)

This sounds like it might belong in the DT.

> + -bit 16-17: Priority level
> +	0x0: low
> +	0x1: medium
> +	0x2: high
> +	0x3: very high

What do we do elsewhere in terms of describing prioritisation? It feels
like it would be a dynamic property of the system.

> +5. A 32bit mask specifying the DMA FIFO configuration
> + -bit 0-1: Fifo threshold
> +	0x0: 1/4 full FIFO
> +	0x1: 1/2 full FIFO
> +	0x2: 3/4 full FIFO
> +	0x3:full FIFO

What does this mean?

> + -bit 2: Direct mode
> +	0x0: enabled
> +	0x1: disabled

What does this mean?

> + -bit 7: FIFO Error Interrupt
> +	0x0: disabled
> +	0x1: enabled

As with the other interrupt configuration, this does not look like it
belongs in the DT.

Thanks,
Mark.
--
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/