From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Pedersen Subject: Re: [RESEND PATCH 1/5] dtbindings: qcom_adm: Fix channel specifiers Date: Thu, 28 Jul 2016 15:16:07 -0700 Message-ID: <952bef3d58349bb7df262bd23e7a17f3@codeaurora.org> References: <1467150186-11427-1-git-send-email-twp@codeaurora.org> <1467150186-11427-2-git-send-email-twp@codeaurora.org> <20160629210606.GB16832@hector.attlocal.net> <847cb2110a7980eed819100bd07e2054@codeaurora.org> <20160701190841.GA7295@hector.attlocal.net> Reply-To: twp@codeaurora.org Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from smtp.codeaurora.org ([198.145.29.96]:59728 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752102AbcG1WQJ (ORCPT ); Thu, 28 Jul 2016 18:16:09 -0400 In-Reply-To: <20160701190841.GA7295@hector.attlocal.net> Sender: linux-arm-msm-owner@vger.kernel.org List-Id: linux-arm-msm@vger.kernel.org To: Andy Gross Cc: linux-arm-msm@vger.kernel.org, linux@qca.qualcomm.com, Vinod Koul , Rob Herring , Mark Rutland , linux-arm-msm-owner@vger.kernel.org On 2016-07-01 12:08, Andy Gross wrote: > On Fri, Jul 01, 2016 at 10:50:51AM -0700, Thomas Pedersen wrote: >> Hi Andy, >> >> On 2016-06-29 14:06, Andy Gross wrote: >> >On Tue, Jun 28, 2016 at 02:43:02PM -0700, Thomas Pedersen wrote: >> >>From: Andy Gross >> >> >> >>This patch removes the crci information from the dma >> >>channel property. At least one client device requires >> >>using more than one CRCI value for a channel. This does >> >>not match the current binding and the crci information >> >>needs to be removed. >> >> >> >>Instead, the client device will provide this information >> >>via other means. >> >> >> >>Signed-off-by: Andy Gross >> >>Signed-off-by: Thomas Pedersen >> >>--- >> >> Documentation/devicetree/bindings/dma/qcom_adm.txt | 16 >> >>++++++---------- >> >> 1 file changed, 6 insertions(+), 10 deletions(-) >> >> >> >>diff --git a/Documentation/devicetree/bindings/dma/qcom_adm.txt >> >>b/Documentation/devicetree/bindings/dma/qcom_adm.txt >> >>index 9bcab91..38d45f8 100644 >> >>--- a/Documentation/devicetree/bindings/dma/qcom_adm.txt >> >>+++ b/Documentation/devicetree/bindings/dma/qcom_adm.txt >> >>@@ -4,8 +4,7 @@ Required properties: >> >> - compatible: must contain "qcom,adm" for IPQ/APQ8064 and MSM8960 >> >> - reg: Address range for DMA registers >> >> - interrupts: Should contain one interrupt shared by all channels >> >>-- #dma-cells: must be <2>. First cell denotes the channel number. >> >>Second cell >> >>- denotes CRCI (client rate control interface) flow control assignment. >> >>+- #dma-cells: must be <1>. First cell denotes the channel number. >> > >> >I've actually been thinking more about this. The crci being specified in >> >the >> >slave config is probably the wrong approach. What we really need is each >> >physical DMA channel to allow for virtual channels that allow for a CRCI >> >setting >> >(0 being no flow control). This would require the properties to continue >> >to be >> >2 for the dma definition. >> >> AFAICT the cmd-crci and data-crci properties in the NAND controller >> client >> are >> really just the slave_id for the DMA engine. Flow control is decided >> by some >> other heuristic such as whether it's a read/write/etc. command. >> >> >This would also require clients to get multiple references to the same DMA >> >channel if they require some communications with and without flow control. >> > >> >For NAND, this would mean having two channels for dma. One for flow >> >controlled >> >and one without. >> >> So the NAND DT would look something like: >> >> dmas = <&adm_dma 3 0>, <%&adm_dma 3 1>; >> dma-names = "rxtx", "rxtx_fc"; >> qcom,cmd-crci = <15>; >> qcom,data-crci = <3>; >> >> ? We'd still need the cmd-crci and data-crci properties for the >> slave_id, >> unless >> I'm confusing something? > > No, what we would have are separate channels for cmd and data that > would be > virtual channels sharing the same hardware channel. At least that is > what I am > thinking. So > > dmas = <&adm 3 0>, <&adm 3 15>, <&adm 3 3>; > dma-names = "rxtx", "rxtx-cmd", "rxtx-data"; # insert better names > here > > All three would use channel 3, but the virtual channel would know about > the > crci. CRCI = 0 is no flow control. > > I need to look at the nand a little harder to see how they formulate > the dma > requests. Did you have a chance to look into this? If not, can you provide some more specific pointers (like would these virtual channels be in the NAND controller or ADM driver?), and I can give it a try? -- thomas