Re: [PATCH V3 4/8] memory: tegra: Add Tegra210 EMC clock driver

Joseph Lo
2019-05-14T09:22:26Z

On 5/14/19 12:54 AM, Dmitry Osipenko wrote:
> 10.05.2019 11:47, Joseph Lo пишет:
>> This is the initial patch for Tegra210 EMC clock driver, which doesn't
>> include the support code and detail sequence for clock scaling yet.
>>
>> The driver is designed to support LPDDR4 SDRAM. Because of the LPDDR4
>> devices need to do initial time training before it can be used, the
>> firmware will help to do that at early boot stage. Then, the trained
>> table for the rates we support will pass to the kernel via DT. So the
>> driver can get the trained table for clock scaling support.
>>
>> For the higher rate support (above 800MHz), the periodic training is
>> needed for the timing compensation. So basically, two methodologies for
>> clock scaling are supported, one is following the clock changing
>> sequence to update the EMC table to EMC registers and another is if the
>> rate needs periodic training, then we will start a timer to do that
>> periodically until it scales to the lower rate.
>>
>> Based on the work of Peter De Schrijver <pdeschrijver@nvidia.com>.
>>
>> Signed-off-by: Joseph Lo <josephl@nvidia.com>
>> ---
>> v3:
>> - address almost all the comments from the previous version
>> - remove the DT parser of EMC table
>> - The EMC table is passing as a binary blob now.
>> ---
snip.
>> +#ifdef CONFIG_DEBUG_FS
>> +static int emc_stats_show(struct seq_file *s, void *data)
>> +{
>> + int i;
>> + struct tegra_emc *emc = (struct tegra_emc *)s->private;
>
> There is no need for casting of the void*.
>
>> +
>> + if (!emc->emc_table_size || !seq)
>> + return 0;
>> +
>> + emc_last_stats_update(TEGRA_EMC_MAX_FREQS);
>> +
>> + seq_printf(s, "%-10s %-10s\n", "rate kHz", "time");
>> + for (i = 0; i < emc->emc_table_size; i++) {
>> + if (emc_clk_sel[i].input == NULL)
>> + continue;
>> +
>> + seq_printf(s, "%-10u %-10llu\n",
>> + emc->emc_table[i].rate,
>> + jiffies_64_to_clock_t(
>> + emc_stats.time_at_clock[i]));
>> + }
>> + seq_printf(s, "%-15s %llu\n", "transitions:",
>> + emc_stats.clkchange_count);
>> + seq_printf(s, "%-15s %llu\n", "time-stamp:",
>> + jiffies_64_to_clock_t(emc_stats.last_update));
>
> Devfreq subsystem has the transition stats too and it is a bit more advanced than what you have here.
>
> cat /sys/class/devfreq/devfreq0/trans_stat
> From : To
> : 50000000 100000000 150000000 300000000 600000000 time(ms)
> * 50000000: 0 4 4 0 46 118096210
> 100000000: 21 0 0 0 5 15460
> 150000000: 10 9 0 0 10560 20213940
> 300000000: 13 6 5058 0 22375 6848690
> 600000000: 11 7 5517 27452 0 11958990
> Total transition : 71098
>
> Hence I'm questioning the necessity of the debug-info duplication.
>

Okay, will remove the statistic data in the next version. BTW, I think
we will use Interconnect framework for the EMC BW manager for the Tegra
chips >= T210.

Thanks,
Joseph