On 6 November 2014 09:12, Alexandre Courbot <acourbot@xxxxxxxxxx> wrote:
> On 10/30/2014 01:22 AM, Tomeu Vizoso wrote:
>>
>> The MC driver needs some timing-specific information to program the EMEM
>> during
>> a rate change of the EMC clock.
>>
>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@xxxxxxxxxxxxx>
>> ---
>> .../memory-controllers/nvidia,tegra-mc.txt | 46
>> +++++++++++++++++++++-
>> 1 file changed, 44 insertions(+), 2 deletions(-)
>>
>> diff --git
>> a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra-mc.txt
>> b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra-mc.txt
>> index f3db93c..8467b8c 100644
>> ---
>> a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra-mc.txt
>> +++
>> b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra-mc.txt
>> @@ -15,9 +15,26 @@ Required properties:
>> This device implements an IOMMU that complies with the generic IOMMU
>> binding.
>> See ../iommu/iommu.txt for details.
>>
>> -Example:
>> ---------
>> +The node should contain a "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 "timings" nodes :
>> +- nvidia,ram-code : Should contain the value of RAM_CODE this timing set
>> is used
>> +for.
>> +
>> +Each "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.
>
>
> In patch 04, a similar sub-node is named "emc-timings". Shouldn't the same
> name be used here for consistency?
Yeah, agreed.
> Also, it seems like there is a need for timing nodes in the MC to have match
> timing nodes in the CAR. It would be nice to add that information in all
> related bindings files.
Well, rather than the timing subnodes in a node having to match the
ones in another nodes, I think that all of them should match the
frequencies at which the EMC can run, which I think is clear enough
like this (already in each of the three bindings):
+Each "timings" node should contain a "timing" subnode for every
supported EMC clock rate.
>> +
>> +Required properties for "timing" nodes :
>> +- clock-frequency : Should contain the memory clock rate in Hz.
>> +- nvidia,emem-configuration : Values to be written to the EMEM register
>> block,
>> +as specified by the board documentation.
>
>
> Could you add some more information about which range of registers we are
> talking about, and whether they must all be specified or only part of them?
>
>
>> +
>> +Example SoC include file:
>> +
>> +/ {
>> mc: memory-controller@0,70019000 {
>> compatible = "nvidia,tegra124-mc";
>> reg = <0x0 0x70019000 0x0 0x1000>;
>> @@ -34,3 +51,28 @@ Example:
>> ...
>> iommus = <&mc TEGRA_SWGROUP_SDMMC1A>;
>> };
>> +};
>> +
>> +Example board file:
>> +
>> +/ {
>> + memory-controller@0,70019000 {
>> + timings@3 {
>> + nvidia,ram-code = <3>;
>> +
>> + timing@12750000 {
>> + clock-frequency = <12750000>;
>> +
>> + nvidia,emem-configuration = <
>> + 0x40040001 /* MC_EMEM_ARB_CFG */
>> + 0x8000000a /*
>> MC_EMEM_ARB_OUTSTANDING_REQ */
>> + 0x00000001 /*
>> MC_EMEM_ARB_TIMING_RCD */
>> + 0x00000001 /*
>> MC_EMEM_ARB_TIMING_RP */
>> + 0x00000002 /*
>> MC_EMEM_ARB_TIMING_RC */
>> + 0x00000000 /*
>> MC_EMEM_ARB_TIMING_RAS */
>> + 0x00000002 /*
>> MC_EMEM_ARB_TIMING_FAW */
>> + >;
>
>
> Looking at the actual board files I suppose this example here is incomplete.
> It would be nice to make this explicit, maybe by adding "..." on a new line
> to indicate more registers are expected.
Sounds good.
Thanks!
Tomeu
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
