Re: [PATCH v3 04/13] of: document new emc-timings subnode in nvidia,tegra124-car

[Tomeu Vizoso <tomeu.vizoso@collabora.com>]

On 11/07/2014 06:31 AM, Alexandre Courbot wrote:
> On 11/07/2014 12:12 AM, Rob Herring wrote:
>> On Thu, Nov 6, 2014 at 12:37 AM, Alexandre Courbot <acourbot@nvidia.com> wrote:
>>> On 10/30/2014 01:22 AM, Tomeu Vizoso wrote:
>>>>
>>>> The EMC clock needs some extra information for changing its rate.
>>>>
>>>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>>>> ---
>>>>    .../bindings/clock/nvidia,tegra124-car.txt         | 46
>>>> +++++++++++++++++++++-
>>>>    1 file changed, 44 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/clock/nvidia,tegra124-car.txt
>>>> b/Documentation/devicetree/bindings/clock/nvidia,tegra124-car.txt
>>>> index ded5d62..42e0588 100644
>>>> --- a/Documentation/devicetree/bindings/clock/nvidia,tegra124-car.txt
>>>> +++ b/Documentation/devicetree/bindings/clock/nvidia,tegra124-car.txt
>>>> @@ -19,12 +19,35 @@ Required properties :
>>>>      In clock consumers, this cell represents the bit number in the CAR's
>>>>      array of CLK_RST_CONTROLLER_RST_DEVICES_* registers.
>>>>
>>>> +The node should contain a "emc-timings" subnode for each supported RAM
>>>> type (see
>>>> +field RAM_CODE in register PMC_STRAPPING_OPT_A), with its unit address
>>>> being its
>>>> +RAM_CODE.
>>>> +
>>>> +Required properties for "emc-timings" nodes :
>>>> +- nvidia,ram-code : Should contain the value of RAM_CODE this timing set
>>>> +  is used for.
>>>> +
>>>> +Each "emc-timings" node should contain a "timing" subnode for every
>>>> supported
>>>> +EMC clock rate. The "timing" subnodes should have the clock rate in Hz as
>>>> their
>>>> +unit address.
>>>
>>>
>>> This seems to be a quite liberal use of unit addresses (same in the next
>>> patch) - is this allowed by DT?
>>
>> No, unit address should match a reg property.
> 
> Mmm, would you have any suggestion as to how this can be fixed? Right 
> now what I can think of is either to replace the "clock-frequency" 
> property by "reg" (which would be confusing), or to use a different 
> naming scheme, e.g. timing-12750000. IIUC the naming is not essential 
> for properly parsing these nodes, so maybe the second solution is the 
> way to go?

Yeah, there seems to be a precedent for embedding the frequency in the
node name in the TI DTs, e.g.: virt_12000000_ck.

Thanks,

Tomeu