Re: [PATCH V2 1/2] Documentation: DT: Add binding documentation for NVIDIA ADMA

[Jon Hunter <jonathanh@nvidia.com>]

On 06/10/15 23:57, Stephen Warren wrote:
> On 10/06/2015 03:16 AM, Jon Hunter wrote:
>>
>> On 05/10/15 14:12, Mark Rutland wrote:
>>> On Mon, Oct 05, 2015 at 01:10:06PM +0100, Jon Hunter wrote:
>>>> Add device-tree binding documentation for the Tegra210 Audio DMA
>>>> controller.
>>>>
>>>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>>>> ---
>>>>   .../devicetree/bindings/dma/tegra210-adma.txt      | 63
>>>> ++++++++++++++++++++++
>>>>   1 file changed, 63 insertions(+)
>>>>   create mode 100644
>>>> Documentation/devicetree/bindings/dma/tegra210-adma.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/dma/tegra210-adma.txt
>>>> b/Documentation/devicetree/bindings/dma/tegra210-adma.txt
>>>> new file mode 100644
>>>> index 000000000000..df0e46868a63
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/dma/tegra210-adma.txt
>>>> @@ -0,0 +1,63 @@
>>>> +* NVIDIA Tegra Audio DMA (ADMA) controller
>>>> +
>>>> +Required properties:
>>>> +- compatible: Must be "nvidia,tegra210-adma".
>>>> +- reg: Should contain DMA registers location and length. This
>>>> should be
>>>> +  a single entry that includes all of the per-channel registers in one
>>>> +  contiguous bank.
>>>> +- interrupt-parent: Phandle to the interrupt parent controller.
>>>> +- interrupts: Should contain all of the per-channel DMA interrupts in
>>>> +  ascending order with respect to the DMA channel index.
>>>> +- clocks: Must contain one entry for the ADMA module clock,
>>>> "adma_ape".
>>>> +- clock-names: Must contain the entry "adma_ape".
>>>> +- dma-channels: Must be 22. Defines the number of DMA channels
>>>> supported
>>>> +  by the DMA controller.
>>>
>>> If this has to be a fixed value, why is it necessary? Why does the
>>> driver not just know this?
>>>
>>> Are there other instances of this IP block where this differs?
>>
>> So this will change for future devices and yes it may seem silly now to
>> have something that fixed and appears to be constant but I was trying to
>> future proof the binding. May be the comment should read "For tegra210
>> must be 22", however, I thought the compatible string would imply this.
> 
> Typically you'd want a table in the driver that maps from compatible
> value to the set of per-SoC data that's associated with the compatible
> value. Then, you don't need to put this data into the DT.

Yes I have seen that which I was not sure that I was a fan of, given
that we have DT and its purpose is to describe the hardware. So may be
the problem I have is deciding on which hardware parameters should be
described in DT versus those that should be place in the driver itself.
I am not sure if there is a rule of thumb for this type of thing?

Cheers
Jon