Re: [PATCH 1/2] dma: add Qualcomm Technologies HIDMA management driver
From: Mark Rutland
Date: Fri Oct 30 2015 - 11:00:42 EST
On Thu, Oct 29, 2015 at 11:08:12PM -0400, Sinan Kaya wrote:
> The Qualcomm Technologies HIDMA device has been designed
> to support virtualization technology. The driver has been
> divided into two to follow the hardware design. The management
> driver is executed in hypervisor context and is the main
> managment for all channels provided by the device. The
> channel driver is exected in the guest OS context.
>
> Signed-off-by: Sinan Kaya <okaya@xxxxxxxxxxxxxx>
> ---
> .../devicetree/bindings/dma/qcom_hidma_mgmt.txt | 42 +
> drivers/dma/Kconfig | 11 +
> drivers/dma/Makefile | 1 +
> drivers/dma/qcom_hidma_mgmt.c | 868 +++++++++++++++++++++
> 4 files changed, 922 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
> create mode 100644 drivers/dma/qcom_hidma_mgmt.c
>
> diff --git a/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt b/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
> new file mode 100644
> index 0000000..81674ab
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
> @@ -0,0 +1,42 @@
> +Qualcomm Technologies HIDMA Management interface
> +
> +Required properties:
> +- compatible: must contain "qcom,hidma_mgmt"
s/_/-/ in property names and compatible strings.
Per your rationale for splitting the device nodes, surely the
relationship between the two needs to be described?
The GIC has a similar split, yet we use a single node, with the
hypervisor portions being optional.
What if there were multiple instances?
> +- reg: Address range for DMA device
> +- interrupts: Should contain the one interrupt shared by all channels
> +- nr-channels: Number of channels supported by this DMA controller.
> +- max-write: Maximum write burst in bytes. A memcpy requested is
> + fragmented to multiples of this amount.
> +- max-read: Maximum read burst in bytes. A memcpy request is
> + fragmented to multiples of this amount.
Thse would be better named as max-read-burst-bytes and
min-read-burst-bytes.
> +- max-wxactions: Maximum write transactions to perform in a burst
> +- max-rdactions: Maximum read transactions to perform in a burst
max-{write,read}-transactions
> +- max-memset-limit: Maximum memset limit
I have no idea what this is meant to mean.
> +- ch-priority-#n: Priority of the channel
> +- ch-weight-#n: Round robin weight of the channel
These need better descriptions. They sound like configuration options.
[...]
> +#if IS_ENABLED(CONFIG_ACPI)
> +static const struct acpi_device_id qcom_hidma_mgmt_acpi_ids[] = {
> + {"QCOM8060"},
> + {},
> +};
> +#endif
How do DMA engines work with ACPI?
How are client relationships defined?
THanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/