On Thu, 2019-06-06 at 11:23 -0700, Stephen Boyd wrote:

Quoting Nicolas Saenz Julienne (2019-06-06 11:10:04)

quoted

On Thu, 2019-06-06 at 10:36 -0700, Stephen Boyd wrote:

quoted

Quoting Nicolas Saenz Julienne (2019-06-06 10:22:16)

quoted

Hi Stephen,
Thanks for the review.

On Thu, 2019-06-06 at 10:09 -0700, Stephen Boyd wrote:

quoted

Quoting Nicolas Saenz Julienne (2019-06-06 07:22:56)

quoted

diff --git a/drivers/cpufreq/raspberrypi-cpufreq.c

b/drivers/cpufreq/raspberrypi-cpufreq.c
new file mode 100644
index 000000000000..99b59d5a50aa

--- /dev/null
+++ b/drivers/cpufreq/raspberrypi-cpufreq.c

[...]

quoted

+
+/*
+ * Since the driver depends on clk-raspberrypi, which may return
EPROBE_DEFER,
+ * all the activity is performed in the probe, which may be defered
as
well.
+ */
+static struct platform_driver raspberrypi_cpufreq_driver = {
+       .driver = {
+               .name = "raspberrypi-cpufreq",
+       },
+       .probe          = raspberrypi_cpufreq_probe,
+       .remove         = raspberrypi_cpufreq_remove,
+};
+module_platform_driver(raspberrypi_cpufreq_driver);

How does this driver probe? Do you have a node in DT named
raspberrypi-cpufreq that matches and probes this? I would think this
would follow the drivers/cpufreq/cpufreq-dt-platdev.c design where
it's
an initcall that probes the board compatible string.

Or, if it depends on clk-raspberrypi probing, maybe it could create
the
platform device in that drivers probe function.

Well you just reviewed that patch :)

Ok. So what's your plan?

So as discussed previously with the RPi mantainers, they preferred for the
platform device for raspberrypi-clk to be created by the firmware interface
driver. IIRC Stefan said it was more flexible and the approach used with
RPi's
hwmon driver already. Also, it's not really clear whether this driver really
fits the device tree as it wouldn't be describing hardware.

As far as raspberrypi-cpufreq is concerned the max and min frequencies are
configurable in the firmware. So we can't really integrate cpufreq into the
device tree as we need to create the opp table dynamically. Hence the
dedicated
driver. On top of that the CPU might not have a clock during the init
process,
as both the firmware interface and raspberrypi-clk can be compiled as
modules.
So I decided the simplest solution was to create the raspberrypi-cpufreq
platform device at the end of raspberrypi-clk's probe.

Once raspberrypi-cpufreq is loaded it queries the min/max frequencies,
populates the CPU's opp table and creates an instance of cpufreq-dt. Which
finally can operate, without the need of any dt info, as opp tables are
populated and CPUs have a clock.

I hope this makes it a little more clear :).

Yes, thanks. I see that largely follows the commit description so it
looks OK to me.

Thanks!