Re: Re: [TEGRA194_CPUFREQ Patch 1/3] firmware: tegra: adding function to get BPMP data
From: Sumit Gupta
Date: Wed Apr 29 2020 - 04:21:31 EST
On 27/04/20 12:48 PM, Thierry Reding wrote:
On Tue, Apr 07, 2020 at 12:05:20PM +0200, Thierry Reding wrote:
On Wed, Dec 04, 2019 at 03:21:38PM +0530, Viresh Kumar wrote:
On 04-12-19, 10:33, Thierry Reding wrote:
Yeah, the code that registers this device is in drivers/base/cpu.c in
register_cpu(). It even retrieves the device tree node for the CPU from
device tree and stores it in cpu->dev.of_node, so we should be able to
just pass &cpu->dev to tegra_bpmp_get() in order to retrieve a reference
to the BPMP.
That said, I'm wondering if perhaps we could just add a compatible
string to the /cpus node for cases like this where we don't have an
actual device representing the CPU complex. There are a number of CPU
frequency drivers that register dummy devices just so that they have
something to bind a driver to.
If we allow the /cpus node to represent the CPU complex (if no other
"device" does that yet), we can add a compatible string and have the
cpufreq driver match on that.
Of course this would be slightly difficult to retrofit into existing
drivers because they'd need to remain backwards compatible with existing
device trees. But it would allow future drivers to do this a little more
elegantly. For some SoCs this may not matter, but especially once you
start depending on additional resources this would come in handy.
Adding Rob and the device tree mailing list for feedback on this idea.
Took some time to find this thread, but something around this was
suggested by Rafael earlier.
https://lore.kernel.org/lkml/8139001.Q4eV8YG1Il@xxxxxxxxxxxxxx/
I gave this a try and came up with the following:
--- >8 ---
diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
index f4ede86e32b4..e4462f95f0b3 100644
--- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
@@ -1764,6 +1764,9 @@ bpmp_thermal: thermal {
};
cpus {
+ compatible = "nvidia,tegra194-ccplex";
+ nvidia,bpmp = <&bpmp>;
+
#address-cells = <1>;
#size-cells = <0>;
--- >8 ---
Now I can do something rougly like this, although I have a more complete
patch locally that also gets rid of all the global variables because we
now actually have a struct platform_device that we can anchor everything
at:
--- >8 ---
static const struct of_device_id tegra194_cpufreq_of_match[] = {
{ .compatible = "nvidia,tegra194-ccplex", },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, tegra194_cpufreq_of_match);
static struct platform_driver tegra194_ccplex_driver = {
.driver = {
.name = "tegra194-cpufreq",
.of_match_table = tegra194_cpufreq_of_match,
},
.probe = tegra194_cpufreq_probe,
.remove = tegra194_cpufreq_remove,
};
module_platform_driver(tegra194_ccplex_driver);
--- >8 ---
I don't think that's exactly what Rafael (Cc'ed) had in mind, since the
above thread seems to have mostly talked about binding a driver to each
individual CPU.
But this seems a lot better than having to instantiate a device from
scratch just so that a driver can bind to it and it allows additional
properties to be associated with the CCPLEX device.
Rob, any thoughts on this from a device tree point of view? The /cpus
bindings don't mention the compatible property, but there doesn't seem
to be anything in the bindings that would prohibit its use.
If we can agree on that, I can forward my local changes to Sumit for
inclusion or reference.
Rob, do you see any reason why we shouldn't be able to use a compatible
string in the /cpus node for devices such as Tegra194 where there is no
dedicated hardware block for the CCPLEX?
Thierry
Ping.
Please suggest if we can add compatible string in the '/cpus' node.
If not then i propose accepting the current patch to get BPMP data
without adding any property in respective driver's DT node.
We can push separate patch later if we need to add compatible string in
the '/cpus' node or create new DT node for cpufreq.
Regards,
Sumit