linux-kernel - Re: [PATCH v7 2/3] DMA: Freescale: Add new 8-channel DMA engine device tree nodes

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: <521541EE.4010303@wwwdotorg.org>
Date:	Wed, 21 Aug 2013 16:40:46 -0600
From:	Stephen Warren <swarren@...dotorg.org>
To:	hongbo.zhang@...escale.com
CC:	vinod.koul@...el.com, djbw@...com, leoli@...escale.com,
	scottwood@...escale.com, linuxppc-dev@...ts.ozlabs.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v7 2/3] DMA: Freescale: Add new 8-channel DMA engine device
 tree nodes

On 07/29/2013 04:49 AM, hongbo.zhang@...escale.com wrote:
> Freescale QorIQ T4 and B4 introduce new 8-channel DMA engines, this patch adds
> the device tree nodes for them.

> diff --git a/Documentation/devicetree/bindings/powerpc/fsl/dma.txt b/Documentation/devicetree/bindings/powerpc/fsl/dma.txt

> +** Freescale Elo3 DMA Controller
> +   This is EloPlus controller with 8 channels, used in Freescale Txxx and Bxxx
> +   series chips, such as t1040, t4240, b4860.
> +
> +Required properties:
> +
> +- compatible        : must include "fsl,elo3-dma"

This should probably list all the SoC-specific compatible values too.

> +- ranges            : describes the mapping between the address space of the
> +                      DMA channels and the address space of the DMA controller

Oh, so looking at the example, this is simply about being able to write
the reg value in the child nodes more easily without having to write out
the full based address of the controller in each child node.

I don't think the binding document should require this; all the binding
document should care about is that the child nodes have a valid reg
value. Whether that reg value is <0x100100 0x80> without a ranges in the
top-level DMA nor or whether that reg value is <0x0 0x80> with a ranges
value in the top-level DMA node isn't something that the binding should
specify. Either way will work equally without affecting a driver for the
DMA controller; the parsing of reg with/without a ranges property is
more of a core part of DT than anything to do with this binding.

> +- DMA channel nodes:
> +        - compatible        : must include "fsl,eloplus-dma-channel"

Why do the channel nodes even need a compatible value? Presumably the
driver for the top-level DMA node will scan these dma-channel nodes to
extract the information it needs and will simply assume that all these
nodes are DMA channel nodes rather than something else? I suppose this
doesn't hurt, it just seems unnecessary unless you foresee other child
nodes types existing in the future and hence a need to differentiate
different types of nodes.

> +        - reg               : <registers mapping for channel>
> +        - interrupts        : <interrupt mapping for DMA channel IRQ>

s/interrupts/specifier/

> +Example:
> +dma@...300 {
> +	#address-cells = <1>;
> +	#size-cells = <1>;

Those weren't mentioned in the required properties list above. Perhaps
they're considered such a core part of DT functionality that it's not
necessary, yet some other binding documents do include these properties
in the list of required properties.
--
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/