Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1240292 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH V2 1/2] Documentation: DT: Add binding documentation for  NVIDIA ADMA Jon Hunter <jonathanh@nvidia.com> - 2015-10-06 11:20 +0200
    Re: [PATCH V2 1/2] Documentation: DT: Add binding documentation for  NVIDIA ADMA Stephen Warren <swarren@wwwdotorg.org> - 2015-10-07 01:00 +0200
      Re: [PATCH V2 1/2] Documentation: DT: Add binding documentation for  NVIDIA ADMA Jon Hunter <jonathanh@nvidia.com> - 2015-10-07 17:30 +0200
        Re: [PATCH V2 1/2] Documentation: DT: Add binding documentation for  NVIDIA ADMA Stephen Warren <swarren@wwwdotorg.org> - 2015-10-07 18:10 +0200
          Re: [PATCH V2 1/2] Documentation: DT: Add binding documentation for  NVIDIA ADMA Mark Rutland <mark.rutland@arm.com> - 2015-10-07 18:40 +0200

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

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.

>> +- dma-rx-requests: Must be 10. Defines the number of receive request
>> +  signals supported by the DMA controller.
>> +- dma-tx-requests: Must be 10. Defines the number of transmit request
>> +  signals supported by the DMA controller.
> 
> Likewise for these two?

For these I am not sure if they will change in the future and so yes I
could simply place these in the driver. The register would need to
change in the future for these to change and so that would mean that the
driver would need to be updated. So may be this does not make sense.

Cheers
Jon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1241063

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.

Even if you don't want to do that (e.g. so the driver can work on future 
HW without code changes), then you can always add this property in the 
future if needed; if the property is present the value in it will be 
used, but if missing, any driver should default to the value in use 
for/on HW  before the binding update.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1241598

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1241642

On 10/07/2015 09:26 AM, Jon Hunter wrote:
>
> 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?

It seems to be a matter of preference.

I think DT is mainly about identifying which HW is present. Facets of 
the HW that are fixed (100% derivable from the compatible value) belong 
in the driver. If we took the opposite view, we should put a description 
of every register/field layout in the DT in case they change on new HW 
and we don't want to edit the driver code to take account of that, and 
in fact should put byte code into the DT since simple data might not be 
enough to allow us not to edit the driver. Facets that vary between 
boards, use-cases, etc. are the main benefit of DT in my opinion, not 
slow-moving data specific to SoCs.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1241676

[...]

> >>>>>+- 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?
> 
> It seems to be a matter of preference.
> 
> I think DT is mainly about identifying which HW is present. Facets
> of the HW that are fixed (100% derivable from the compatible value)
> belong in the driver. If we took the opposite view, we should put a
> description of every register/field layout in the DT in case they
> change on new HW and we don't want to edit the driver code to take
> account of that, and in fact should put byte code into the DT since
> simple data might not be enough to allow us not to edit the driver.
> Facets that vary between boards, use-cases, etc. are the main
> benefit of DT in my opinion, not slow-moving data specific to SoCs.

I think it depends on what the difference is as opposed to how quickly
it changes.

If there's some known-configurable but non-probeable property of an IP
block, then describing that explicitly makes sense if you can predict
what the other configurations will look like in terms of the programming
interface (e.g. the register layout). That gives you a degree of forward
compatibility, with less churn and a smaller driver.

If the register layout changes in a non-predictable fashion, you cannot
capture that in a binding a need more code. There will always be
elements which are non-predictable and need changes, but that need not
be the default case.

Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web