Re: [RESEND PATCH 1/5] dtbindings: qcom_adm: Fix channel specifiers

[Thomas Pedersen <twp@codeaurora.org>]

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 <andy.gross@linaro.org>
>> 
>> 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 <andy.gross@linaro.org>
>> Signed-off-by: Thomas Pedersen <twp@codeaurora.org>
>> ---
>>  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?

Thanks,

-- 
thomas