2020-04-03 16:52:37

by Eddie James

[permalink] [raw]
Subject: [PATCH v8 1/5] dt-bindings: soc: Add Aspeed XDMA Engine

Document the bindings for the Aspeed AST25XX and AST26XX XDMA engine.

Signed-off-by: Eddie James <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
Reviewed-by: Andrew Jeffery <[email protected]>
---
Changes since v6:
- Add PCI-E root complex reset documentation
- Add reset-names documentation
- Switch to memory-region phandle instead of memory regs

.../devicetree/bindings/soc/aspeed/xdma.txt | 44 +++++++++++++++++++
MAINTAINERS | 6 +++
2 files changed, 50 insertions(+)
create mode 100644 Documentation/devicetree/bindings/soc/aspeed/xdma.txt

diff --git a/Documentation/devicetree/bindings/soc/aspeed/xdma.txt b/Documentation/devicetree/bindings/soc/aspeed/xdma.txt
new file mode 100644
index 000000000000..c3d7cdeb1db6
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/aspeed/xdma.txt
@@ -0,0 +1,44 @@
+Aspeed AST25XX and AST26XX XDMA Engine
+
+The XDMA Engine embedded in the AST2500 and AST2600 SOCs can perform automatic
+DMA operations over PCI between the SOC (acting as a BMC) and a host processor.
+
+Required properties:
+ - compatible : must be "aspeed,ast2500-xdma" or
+ "aspeed,ast2600-xdma"
+ - reg : contains the address and size of the memory region
+ associated with the XDMA engine registers
+ - clocks : clock specifier for the clock associated with the
+ XDMA engine
+ - resets : reset specifier for the syscon reset associated with
+ the XDMA engine, and, for the AST2600, a second reset
+ associated with the PCI-E root complex
+ - reset-names : required only for the AST2600; must be "dev, "rc"
+ - interrupts-extended : two interrupt cells; the first specifies the global
+ interrupt for the XDMA engine and the second
+ specifies the PCI-E reset or PERST interrupt.
+ - aspeed,scu : a phandle to the syscon node for the system control
+ unit of the SOC
+ - memory-region : a phandle to the reserved memory region to be used by
+ the XDMA engine for DMA operations
+
+Optional properties:
+ - pcie-device : should be either "bmc" or "vga", corresponding to
+ which device should be used by the XDMA engine for
+ DMA operations. If this property is not set, the XDMA
+ engine will use the BMC PCI-E device.
+
+Example:
+
+ xdma@1e6e7000 {
+ compatible = "aspeed,ast2600-xdma";
+ reg = <0x1e6e7000 0x100>;
+ clocks = <&syscon ASPEED_CLK_GATE_BCLK>;
+ resets = <&syscon ASPEED_RESET_DEV_XDMA>, <&syscon ASPEED_RESET_RC_XDMA>;
+ reset-names = "dev", "rc";
+ interrupts-extended = <&gic GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
+ <&scu_ic0 ASPEED_AST2600_SCU_IC0_PCIE_PERST_LO_TO_HI>;
+ aspeed,scu = <&syscon>;
+ pcie-device = "bmc";
+ memory-region = <&vga_memory>;
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index 245a96316636..dcc0907b2aa5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2771,6 +2771,12 @@ S: Maintained
F: drivers/media/platform/aspeed-video.c
F: Documentation/devicetree/bindings/media/aspeed-video.txt

+ASPEED XDMA ENGINE DRIVER
+M: Eddie James <[email protected]>
+L: [email protected] (moderated for non-subscribers)
+S: Maintained
+F: Documentation/devicetree/bindings/soc/aspeed/xdma.txt
+
ASUS NOTEBOOKS AND EEEPC ACPI/WMI EXTRAS DRIVERS
M: Corentin Chary <[email protected]>
L: [email protected]
--
2.24.0


2020-04-05 07:13:37

by Andrew Jeffery

[permalink] [raw]
Subject: Re: [PATCH v8 1/5] dt-bindings: soc: Add Aspeed XDMA Engine



On Sat, 4 Apr 2020, at 02:58, Eddie James wrote:
> Document the bindings for the Aspeed AST25XX and AST26XX XDMA engine.
>
> Signed-off-by: Eddie James <[email protected]>
> Reviewed-by: Rob Herring <[email protected]>
> Reviewed-by: Andrew Jeffery <[email protected]>
> ---
> Changes since v6:
> - Add PCI-E root complex reset documentation
> - Add reset-names documentation
> - Switch to memory-region phandle instead of memory regs
>
> .../devicetree/bindings/soc/aspeed/xdma.txt | 44 +++++++++++++++++++
> MAINTAINERS | 6 +++
> 2 files changed, 50 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/soc/aspeed/xdma.txt
>
> diff --git a/Documentation/devicetree/bindings/soc/aspeed/xdma.txt
> b/Documentation/devicetree/bindings/soc/aspeed/xdma.txt
> new file mode 100644
> index 000000000000..c3d7cdeb1db6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/aspeed/xdma.txt
> @@ -0,0 +1,44 @@
> +Aspeed AST25XX and AST26XX XDMA Engine
> +
> +The XDMA Engine embedded in the AST2500 and AST2600 SOCs can perform
> automatic
> +DMA operations over PCI between the SOC (acting as a BMC) and a host
> processor.
> +
> +Required properties:
> + - compatible : must be "aspeed,ast2500-xdma" or
> + "aspeed,ast2600-xdma"
> + - reg : contains the address and size of the memory region
> + associated with the XDMA engine registers
> + - clocks : clock specifier for the clock associated with the
> + XDMA engine
> + - resets : reset specifier for the syscon reset associated with
> + the XDMA engine, and, for the AST2600, a second reset
> + associated with the PCI-E root complex
> + - reset-names : required only for the AST2600; must be "dev, "rc"

Bit of a nit, but I find "root-complex" to not be so onerous as to shorten it to "rc".
Is there a need to be so economical? Contracting it makes it harder to read.
While we're here we might also expand "dev" to "device".

Thoughts?

Also you're missing the closing quote on "dev".

> + - interrupts-extended : two interrupt cells; the first specifies the
> global
> + interrupt for the XDMA engine and the second
> + specifies the PCI-E reset or PERST interrupt.
> + - aspeed,scu : a phandle to the syscon node for the system control
> + unit of the SOC
> + - memory-region : a phandle to the reserved memory region to be used
> by
> + the XDMA engine for DMA operations

Might have been worth dropping the tags for this patch given the changes. I
feel Rob will welcome the switch to describing the memory with a phandle
to a reserved region, but removing his tag makes it easier for him to protest :)

Andrew