From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752250AbbJaRMC (ORCPT ); Sat, 31 Oct 2015 13:12:02 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:46817 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751698AbbJaRL7 (ORCPT ); Sat, 31 Oct 2015 13:11:59 -0400 Subject: Re: [PATCH 1/2] dma: add Qualcomm Technologies HIDMA management driver To: Mark Rutland References: <1446174501-8870-1-git-send-email-okaya@codeaurora.org> <20151030150025.GF31073@leverpostej> Cc: dmaengine@vger.kernel.org, timur@codeaurora.org, cov@codeaurora.org, jcm@redhat.com, Rob Herring , Pawel Moll , Ian Campbell , Kumar Gala , Vinod Koul , Dan Williams , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org From: Sinan Kaya Message-ID: <5634F64B.5000000@codeaurora.org> Date: Sat, 31 Oct 2015 13:11:39 -0400 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <20151030150025.GF31073@leverpostej> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/30/2015 11:00 AM, Mark Rutland wrote: > 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 >> --- >> .../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. > done > Per your rationale for splitting the device nodes, surely the > relationship between the two needs to be described? > I changed the commit message to make it more clear. I also added the same message to the device binding document for reference. > The GIC has a similar split, yet we use a single node, with the > hypervisor portions being optional. > > What if there were multiple instances? > The driver already supports multiple instances. It has been tested against 4 instances. I ended up having 4 management device entities managing 6 different channels each. >> +- 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. ok > >> +- max-wxactions: Maximum write transactions to perform in a burst >> +- max-rdactions: Maximum read transactions to perform in a burst > > max-{write,read}-transactions ok > >> +- max-memset-limit: Maximum memset limit > > I have no idea what this is meant to mean. > old stuff. no memset support anymore. will remove it. >> +- 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. > ok. > [...] > >> +#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. > -- Sinan Kaya Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project