Re: [PATCH RFC v11 5/6] dma: mpc512x: add device tree binding document
On Fri, 2014-04-18 at 15:29 +0400, Alexander Popov wrote: 2014-04-17 0:44 GMT+04:00 Gerhard Sittig g...@denx.de: On Tue, 2014-04-15 at 14:54 +0400, Alexander Popov wrote: +- reg: Address and size of the DMA controller's register set +- interrupts: Interrupt for the DMA controller. Generic interrupt client node + is described in interrupt-controller/interrupts.txt 'interrupts' only works in combinations with 'interrupt-parent', that actual .dts files don't have the latter in the nodes is an implementation detail but not a binding's requirement Excuse me, I didn't understand your point. and an alternative method of specifying interrupts was introduced recently, a reference to the common binding without naming one specific property name could be most appropriate Excuse me, I haven't found such an example. The 'interrupts' property is not enough in itself, it always needs the 'interrupt-parent' property, too. Because the parent (the interrupt controller) often is the same for multiple interrupts, it usually gets factored out into a parent node in the tree, and thus often gets missed in discussions. Still the 'interrupt-parent' is strictly required for 'interrupts' to work. Splitting both properties and putting them into nodes that are rather distant from each other is just an implementation detail of .dts files. This should not be reflected in bindings. The 'interrupts-extended' property was introduced only recently. See bindings/interrupt-controller/interrupts.txt for details. Although the motivation was to reference several parents from one client node, I very much like the idea of having all of the interrupt spec within a single property. Being explicit is a good thing, especially in setups with cascades. I consider this approach an improvement in readability and maintenance. You might just want to document in the binding that interrupt specs are required (or optional), which interrupts these are (the above text could be sufficient if there is only one interrupt for this IP block), and refer to the common binding for the syntax. virtually yours Gerhard Sittig -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: off...@denx.de ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH RFC v11 5/6] dma: mpc512x: add device tree binding document
Thanks for your reply, Gerhard
2014-04-17 0:44 GMT+04:00 Gerhard Sittig g...@denx.de:
On Tue, 2014-04-15 at 14:54 +0400, Alexander Popov wrote:
Introduce a device tree binding document for the MPC512x DMA controller
Signed-off-by: Gerhard Sittig g...@denx.de
Signed-off-by: Alexander Popov a13xp0p0...@gmail.com
I'm not certain whether the attribution is right. Is the S-o-b
appropriate when the patch is not from me? As I've stated
before, it's OK if you pick up and extend what I provide, but
please don't pretend that I wrote what you did,
Thanks. I've read the corresponding part of
Documentation/SubmittingPatches once again and now I see
my mistake.
and don't pretend
that I ACKed or passed along your submission when I didn't.
I didn't have any malicious intent.
This binding certainly needs further improvement to become a good
one. As I've communicated in the past, I was rather ignorant
back then when I wrote v1 and v2 of the RFC. We have learned
something in the meantime. Though I admit having gone silent
after several review iterations. Assumed you would pick up
information that showed up several times on public lists.
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/mpc512x-dma.txt
@@ -0,0 +1,51 @@
+* Freescale MPC512x and MPC8308 DMA Controller
+
+The DMA controller in the Freescale MPC512x and MPC8308 SoCs can move
+blocks of memory contents between memory and peripherals or
+from memory to memory.
+
+Refer to the Generic DMA Controller and DMA request bindings in
+the dma/dma.txt file for a more detailed description of binding.
+
+* DMA controller
+
+Required properties:
+- compatible: Should be one of
+ fsl,mpc5121-dma
+ fsl,mpc8308-dma, fsl,mpc5121-dma
is this a duplicate? looks funny, needs a fix
or is it a requirement that for MPC8308 you need to provide both
compatible strings? that would be wrong, as MPC8308 certainly is
not an MPC5121
a quick search reveals: the drivers/dma/mpc512x_dma.c Linux
driver implementation is wrong, it should match on both strings;
expecting the MPC8308 to disguise as an MPC5121 when it's not is
inappropriate (and only went unnoticed because of missing
bindings, I guess)
I can try to fix that and add a new patch to the series.
+- reg: Address and size of the DMA controller's register set
+- interrupts: Interrupt for the DMA controller. Generic interrupt client
node
+ is described in interrupt-controller/interrupts.txt
'interrupts' only works in combinations with 'interrupt-parent',
that actual .dts files don't have the latter in the nodes is an
implementation detail but not a binding's requirement
Excuse me, I didn't understand your point.
and an alternative method of specifying interrupts was introduced
recently, a reference to the common binding without naming one
specific property name could be most appropriate
Excuse me, I haven't found such an example.
+
+Optional properties:
+- #dma-cells: The length of the DMA specifier, must be 1 since
+ the DMA controller uses a fixed assignment of request lines
+ per channel. Refer to dma/dma.txt for the detailed description
+ of this property
I'm afraid that a generic/common document does not and cannot
describe the specific semantics of this provider's cells
Ok, I see.
this binding should explicitly mention that the number of cells
needs to be one, and that this one cell is the DMA channel (which
translates to peripheral request line), because these
assigments are fixed in hardware
Ok.
+
+Example:
+
+ dma0: dma@14000 {
+ compatible = fsl,mpc5121-dma;
+ reg = 0x14000 0x1800;
+ interrupts = 65 0x8;
+ #dma-cells = 1;
+ };
+
+* DMA client
the DMA provider's binding probably need not discuss client
specs, a reference to the common binding should suffice if it's
appropriate at all
Ok.
Best regards,
Alexander
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH RFC v11 5/6] dma: mpc512x: add device tree binding document
On Tue, 2014-04-15 at 14:54 +0400, Alexander Popov wrote:
Introduce a device tree binding document for the MPC512x DMA controller
Signed-off-by: Gerhard Sittig g...@denx.de
Signed-off-by: Alexander Popov a13xp0p0...@gmail.com
I'm not certain whether the attribution is right. Is the S-o-b
appropriate when the patch is not from me? As I've stated
before, it's OK if you pick up and extend what I provide, but
please don't pretend that I wrote what you did, and don't pretend
that I ACKed or passed along your submission when I didn't.
This binding certainly needs further improvement to become a good
one. As I've communicated in the past, I was rather ignorant
back then when I wrote v1 and v2 of the RFC. We have learned
something in the meantime. Though I admit having gone silent
after several review iterations. Assumed you would pick up
information that showed up several times on public lists.
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/mpc512x-dma.txt
@@ -0,0 +1,51 @@
+* Freescale MPC512x and MPC8308 DMA Controller
+
+The DMA controller in the Freescale MPC512x and MPC8308 SoCs can move
+blocks of memory contents between memory and peripherals or
+from memory to memory.
+
+Refer to the Generic DMA Controller and DMA request bindings in
+the dma/dma.txt file for a more detailed description of binding.
+
+* DMA controller
+
+Required properties:
+- compatible: Should be one of
+ fsl,mpc5121-dma
+ fsl,mpc8308-dma, fsl,mpc5121-dma
is this a duplicate? looks funny, needs a fix
or is it a requirement that for MPC8308 you need to provide both
compatible strings? that would be wrong, as MPC8308 certainly is
not an MPC5121
a quick search reveals: the drivers/dma/mpc512x_dma.c Linux
driver implementation is wrong, it should match on both strings;
expecting the MPC8308 to disguise as an MPC5121 when it's not is
inappropriate (and only went unnoticed because of missing
bindings, I guess)
+- reg: Address and size of the DMA controller's register set
+- interrupts: Interrupt for the DMA controller. Generic interrupt client node
+ is described in interrupt-controller/interrupts.txt
'interrupts' only works in combinations with 'interrupt-parent',
that actual .dts files don't have the latter in the nodes is an
implementation detail but not a binding's requirement
and an alternative method of specifying interrupts was introduced
recently, a reference to the common binding without naming one
specific property name could be most appropriate
+
+Optional properties:
+- #dma-cells: The length of the DMA specifier, must be 1 since
+ the DMA controller uses a fixed assignment of request lines
+ per channel. Refer to dma/dma.txt for the detailed description
+ of this property
I'm afraid that a generic/common document does not and cannot
describe the specific semantics of this provider's cells
this binding should explicitly mention that the number of cells
needs to be one, and that this one cell is the DMA channel (which
translates to peripheral request line), because these
assigments are fixed in hardware
+
+Example:
+
+ dma0: dma@14000 {
+ compatible = fsl,mpc5121-dma;
+ reg = 0x14000 0x1800;
+ interrupts = 65 0x8;
+ #dma-cells = 1;
+ };
+
+* DMA client
the DMA provider's binding probably need not discuss client
specs, a reference to the common binding should suffice if it's
appropriate at all
virtually yours
Gerhard Sittig
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: off...@denx.de
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH RFC v11 5/6] dma: mpc512x: add device tree binding document
Introduce a device tree binding document for the MPC512x DMA controller
Signed-off-by: Gerhard Sittig g...@denx.de
Signed-off-by: Alexander Popov a13xp0p0...@gmail.com
---
.../devicetree/bindings/dma/mpc512x-dma.txt| 51 ++
1 file changed, 51 insertions(+)
create mode 100644 Documentation/devicetree/bindings/dma/mpc512x-dma.txt
diff --git a/Documentation/devicetree/bindings/dma/mpc512x-dma.txt
b/Documentation/devicetree/bindings/dma/mpc512x-dma.txt
new file mode 100644
index 000..92eb0d9
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/mpc512x-dma.txt
@@ -0,0 +1,51 @@
+* Freescale MPC512x and MPC8308 DMA Controller
+
+The DMA controller in the Freescale MPC512x and MPC8308 SoCs can move
+blocks of memory contents between memory and peripherals or
+from memory to memory.
+
+Refer to the Generic DMA Controller and DMA request bindings in
+the dma/dma.txt file for a more detailed description of binding.
+
+* DMA controller
+
+Required properties:
+- compatible: Should be one of
+ fsl,mpc5121-dma
+ fsl,mpc8308-dma, fsl,mpc5121-dma
+- reg: Address and size of the DMA controller's register set
+- interrupts: Interrupt for the DMA controller. Generic interrupt client node
+ is described in interrupt-controller/interrupts.txt
+
+Optional properties:
+- #dma-cells: The length of the DMA specifier, must be 1 since
+ the DMA controller uses a fixed assignment of request lines
+ per channel. Refer to dma/dma.txt for the detailed description
+ of this property
+
+Example:
+
+ dma0: dma@14000 {
+ compatible = fsl,mpc5121-dma;
+ reg = 0x14000 0x1800;
+ interrupts = 65 0x8;
+ #dma-cells = 1;
+ };
+
+* DMA client
+
+Required properties:
+- dmas: List of one or more DMA request specifiers. One DMA request specifier
+ consists of a phandle to the DMA controller node followed by
+ the integer specifying the request line
+- dma-names: Contains an identifier string for each DMA request specifier.
+ Refer to dma/dma.txt for the description of this property
+
+Example:
+
+ sdhc@1500 {
+ compatible = fsl,mpc5121-sdhc;
+ /* ... */
+ dmas = dma0 30;
+ dma-names = rx-tx;
+ };
--
1.8.4.2
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
