Re: [PATCH v7 1/2] Documentation: DT: dma: Add Xilinx zynqmp dma device tree binding documentation

[Lars-Peter Clausen]

On 04/28/2016 11:00 AM, Appana Durga Kedareswara Rao wrote:
> Hi Lars,
> 
> 	Thanks for the review...
> 
>> -----Original Message-----
>> From: Lars-Peter Clausen [mailto:lars@xxxxxxxxxx]
>> Sent: Wednesday, April 27, 2016 6:10 PM
>> To: Appana Durga Kedareswara Rao <appanad@xxxxxxxxxx>;
>> robh+dt@xxxxxxxxxx; pawel.moll@xxxxxxx; mark.rutland@xxxxxxx;
>> ijc+devicetree@xxxxxxxxxxxxxx; galak@xxxxxxxxxxxxxx; Michal Simek
>> <michals@xxxxxxxxxx>; Soren Brinkmann <sorenb@xxxxxxxxxx>;
>> vinod.koul@xxxxxxxxx; dan.j.williams@xxxxxxxxx; moritz.fischer@xxxxxxxxx;
>> laurent.pinchart@xxxxxxxxxxxxxxxx; luis@xxxxxxxxxxxxxxxxx; Anirudha
>> Sarangi <anirudh@xxxxxxxxxx>; Punnaiah Choudary Kalluri
>> <punnaia@xxxxxxxxxx>
>> Cc: devicetree@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-
>> kernel@xxxxxxxxxxxxxxx; dmaengine@xxxxxxxxxxxxxxx
>> Subject: Re: [PATCH v7 1/2] Documentation: DT: dma: Add Xilinx zynqmp dma
>> device tree binding documentation
>>
>> On 04/27/2016 09:33 AM, Appana Durga Kedareswara Rao wrote:
>>> Hi Lars,
>>>
>>>> -----Original Message-----
>>>> From: Lars-Peter Clausen [mailto:lars@xxxxxxxxxx]
>>>> Sent: Wednesday, April 27, 2016 12:42 PM
>>>> To: Appana Durga Kedareswara Rao <appanad@xxxxxxxxxx>;
>>>> robh+dt@xxxxxxxxxx; pawel.moll@xxxxxxx; mark.rutland@xxxxxxx;
>>>> ijc+devicetree@xxxxxxxxxxxxxx; galak@xxxxxxxxxxxxxx; Michal Simek
>>>> <michals@xxxxxxxxxx>; Soren Brinkmann <sorenb@xxxxxxxxxx>;
>>>> vinod.koul@xxxxxxxxx; dan.j.williams@xxxxxxxxx; Appana Durga
>>>> Kedareswara Rao <appanad@xxxxxxxxxx>; moritz.fischer@xxxxxxxxx;
>>>> laurent.pinchart@xxxxxxxxxxxxxxxx; luis@xxxxxxxxxxxxxxxxx; Anirudha
>>>> Sarangi <anirudh@xxxxxxxxxx>; Punnaiah Choudary Kalluri
>>>> <punnaia@xxxxxxxxxx>
>>>> Cc: devicetree@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
>>>> linux- kernel@xxxxxxxxxxxxxxx; dmaengine@xxxxxxxxxxxxxxx
>>>> Subject: Re: [PATCH v7 1/2] Documentation: DT: dma: Add Xilinx zynqmp
>>>> dma device tree binding documentation
>>>>
>>>> On 04/27/2016 09:05 AM, Kedareswara rao Appana wrote:
>>>> [...]
>>>>> +- xlnx,include-sg	: Indicates the controller to operate in simple or
>>>>> +			  scatter gather dma mode
>>>>> +- xlnx,ratectrl		: Scheduling interval in terms of clock cycles for
>>>>> +			  source AXI transaction
>>>>> +- xlnx,overfetch	: Tells whether the channel is allowed to over
>>>>> +			  fetch the data
>>>>> +- xlnx,src-issue	: Number of AXI outstanding transactions on source
>>>> side
>>>>> +- xlnx,src-burst-len	: AXI length for data read. Support only power of
>>>>> +			  2 byte values.
>>>>> +- xlnx,dst-burst-len	: AXI length for data write. Support only power of
>>>>
>>>> These are all software runtime configuration parameters that you'd
>>>> want to change at runtime depending on which peripheral you are
>>>> targeting with a specific DMA transfer. These really do not belong into the
>> devicetree.
>>>
>>> You mean to have a separate config structure in the driver and handle
>>> the above parameters Through that structure???
>>>
>>> I understand that above will work for slave dma transfer types what
>>> about memory to memory Transfers where we don't have provision to the use
>> this parameters...
>>
>> These parameters are just as application specific as e.g. the DMA
>> source/destination address or the DMA transfer length. If you want to use the
>> DMA controller in a different configuration you'd have to re-compile the DTB
>> and reboot your board, that is not really practical. Especially considering that
>> you'd typically have multiple applications using the DMA controller in different
>> configurations concurrently. In general if I have to reconfigure the DT depending
>> on what application software is running something is fundamentally broken.
>>
>> Derive these parameters at runtime depending on the requested transfer. E.g.
>> some transfer types only work in SG mode, others only work in non-SG modes.
>> For those which can work in both modes choose the one that is more efficient.
>> Similar for the other parameters.
> 
> Ok will fix in the next version...
> Will use module_params for the above properties.

Sorry, but that is just as broken. You need to derive those parameters from
the DMA transfer as they are transfer specific.