Re: [RFC PATCH 1/4] memory: tegra124-emc: Add EMC driver

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

 



On 06/16/2014 10:02 PM, Stephen Warren wrote:
On 06/16/2014 07:35 AM, Tomeu Vizoso wrote:
+
+Child device nodes describe the memory settings for different configurations and
+clock rates.

How do the child nodes do that? The binding needs to specify the format
of the child node.

Sorry, that file was sent before I had finished removing the bits from downstream that aren't needed yet. There's no current need for any child nodes.

This binding looks quite anaemic vs.
Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-emc.txt; I
would expect that this binding needs all the EMC register data from the
tegra20-emc binding too. Can the two bindings be identical?

There's even less stuff needed right now, as all what ultimately the EMC driver does is call clk_set_rate on the EMC clock. As the T124 EMC driver gains more features, they should get more similar.

Can you explain what the nvidia,mc and nvidia,pmc references are needed
for? Hopefully, this driver isn't going to reach into those devices and
touch their registers directly.

Not really needed, see above.

diff --git a/include/linux/platform_data/tegra_emc.h b/include/linux/platform_data/tegra_emc.h

A header file that defines platform data format isn't the correct place
to put the definitions of public APIs. I'd expect something more like
<linux/tegra-soc.h>.

Sounds better indeed, thanks.

+#ifdef CONFIG_TEGRA124_EMC
+int tegra124_emc_reserve_bandwidth(unsigned int consumer, unsigned long rate);
+void tegra124_emc_set_floor(unsigned long freq);
+void tegra124_emc_set_ceiling(unsigned long freq);
+#else
+int tegra124_emc_reserve_bandwidth(unsigned int consumer, unsigned long rate)
+{ return -ENODEV; }
+void tegra124_emc_set_floor(unsigned long freq)
+{ return; }
+void tegra124_emc_set_ceiling(unsigned long freq)
+{ return; }
+#endif

I'll repeat what I said off-list so that we can have the whole
conversation on the list:

That looks like a custom Tegra-specific API. I think it'd be much better
to integrate this into the common clock framework as a standard clock
constraints API. There are other use-cases for clock constraints besides
EMC scaling (e.g. some in audio on Tegra, and I'm sure many on other
SoCs too).

Yes, I wrote a bit in the cover letter about our requirements and how they map to the CCF. Could you please comment on that?

Thanks,

Tomeu

See https://lkml.org/lkml/2014/5/16/569 for some previous discussion on
this topic.


_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux