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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 5/15/19 11:25 PM, Dmitry Osipenko wrote:
15.05.2019 11:42, Joseph Lo пишет:
On 5/15/19 1:04 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@xxxxxxxxxx>.

Signed-off-by: Joseph Lo <josephl@xxxxxxxxxx>
---
snip.
+    if (!seq->set_clock) {
+        seq = NULL;
+        dev_err(&pdev->dev, "Invalid EMC sequence for table Rev. %d\n",
+            emc->emc_table[0].rev);
+        goto emc_clk_register;

Why do you want to register EMC clock if something fails? KMSG will be
flooded with errors coming from clk_set_rate.


See patch 7 in the series, the legacy EMC clock will be removed later,
so we need to register the EMC clock whether the table is ready or not> In that case, I mean if the table is not available, it will still
register EMC clock at the rate that boot loader configured before kernel
booting. So the MC clock can still work as expected, which is under EMC
clock.

And I did test that, couldn't observe any KMSG in that case.

Looks like it kinda should work in the end.

Although it's not good that now MC driver relies on the EMC driver
presence. Maybe it's not the best variant with moving the clock stuff
into the EMC driver?

What about the backwards compatibility for DT that doesn't have the EMC
node?

What if EMC driver is disabled in the kernel's config?

The three questions above are actually one problem here. It's not about MC clock, because it's still available after these changes. And MC driver can still get it in the probe function even the EMC driver isn't there.

The problem is that without EMC driver after these changes. The PLLM will have no client under it, which will cause the PLLM to be disabled in the late init call of "clk_disable_unused". So the system will be stuck.


And lastly.. what stops the MC driver to probe before the EMC? Looks
like MC driver is already in trouble because it's on arch level and the
EMC is on subsys, hence MC will get the orphaned clock and won't
initialize hardware properly on probe.

After this moving, the EMC driver will be always enabled by default. And the DT change is necessary as well. The blob of EMC table is not necessary, because it needs a firmware update. We will update the firmware accordingly after the review settled and release it later.

In case of no EMC table blob, the driver can still be registered, but no scaling function provided.


BTW, how are you testing the EMC driver? Is there T210 devfreq patches
in works? Or what's the user of the EMC on T210?


1. Currently, via debugfs.
2. No, we prefer to use Interconnect framework for that. The evaluation is ongoing. 3. With Interconnect, the devices or peripherals can register on it to request the BW. So we can fine-tune the BW requirements with the latency allowance registers altogether to get better efficiency.

Thanks,
Joseph




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux