[PATCH v2 0/7] cpufreq support for Raspberry Pi

[PATCH v2 0/7] cpufreq support for Raspberry Pi

[Nicolas Saenz Julienne]

Hi all,
this aims at adding cpufreq support to the Raspberry Pi family of
boards.

The series first factors out 'pllb' from clk-bcm2385 and creates a new
clk driver that operates it over RPi's firmware interface[1]. We are
forced to do so as the firmware 'owns' the pll and we're not allowed to
change through the register interface directly as we might race with the
over-temperature and under-voltage protections provided by the firmware.

Next it creates a minimal cpufreq driver that populates the CPU's opp
table, and registers cpufreq-dt. Which is needed as the firmware
controls the max and min frequencies available.

This was tested on a RPi3b+ and RPI2b, both using multi_v7_defconfig and
arm64's defconfig.

@Eric: Even though some things change, namely to properly support being
       built as a module, I still added your Acked-by as it didn't
       change the overall design. I hope it's OK.

That's all,
kind regards,
Nicolas

[1] https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface

---

Changes since v1:
  - Enabled by default on the whole family of devices
  - Added/Fixed module support
  - clk device now registered by firmware driver
  - raspberrypi-cpufreq device now registered by clk driver
  - Reimplemented clk rounding unsing determine_rate()
  - Enabled in configs for arm and arm64

Changes since RFC:
  - Move firmware clk device into own driver

Nicolas Saenz Julienne (7):
  clk: bcm2835: remove pllb
  clk: bcm283x: add driver interfacing with Raspberry Pi's firmware
  firmware: raspberrypi: register clk device
  cpufreq: add driver for Raspbery Pi
  clk: raspberrypi: register platform device for raspberrypi-cpufreq
  ARM: defconfig: enable cpufreq driver for RPi
  arm64: defconfig: enable cpufreq support for RPi3

 arch/arm/configs/bcm2835_defconfig    |   9 +
 arch/arm/configs/multi_v7_defconfig   |   2 +
 arch/arm64/configs/defconfig          |   2 +
 drivers/clk/bcm/Kconfig               |   7 +
 drivers/clk/bcm/Makefile              |   1 +
 drivers/clk/bcm/clk-bcm2835.c         |  28 +--
 drivers/clk/bcm/clk-raspberrypi.c     | 316 ++++++++++++++++++++++++++
 drivers/cpufreq/Kconfig.arm           |   8 +
 drivers/cpufreq/Makefile              |   1 +
 drivers/cpufreq/raspberrypi-cpufreq.c | 100 ++++++++
 drivers/firmware/raspberrypi.c        |  10 +
 11 files changed, 460 insertions(+), 24 deletions(-)
 create mode 100644 drivers/clk/bcm/clk-raspberrypi.c
 create mode 100644 drivers/cpufreq/raspberrypi-cpufreq.c

-- 
2.21.0

[PATCH v2 1/7] clk: bcm2835: remove pllb

[Nicolas Saenz Julienne]

Raspberry Pi's firmware controls this pll, we should use the firmware
interface to access it.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Acked-by: Eric Anholt <eric@anholt.net>

---

Changes since v1:
  - Add comment to explain why pllb isn't there anymore

 drivers/clk/bcm/clk-bcm2835.c | 28 ++++------------------------
 1 file changed, 4 insertions(+), 24 deletions(-)

diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index 770bb01f523e..867ae3c20041 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -1651,30 +1651,10 @@ static const struct bcm2835_clk_desc clk_desc_array[] = {
 		.fixed_divider = 1,
 		.flags = CLK_SET_RATE_PARENT),
 
-	/* PLLB is used for the ARM's clock. */
-	[BCM2835_PLLB]		= REGISTER_PLL(
-		.name = "pllb",
-		.cm_ctrl_reg = CM_PLLB,
-		.a2w_ctrl_reg = A2W_PLLB_CTRL,
-		.frac_reg = A2W_PLLB_FRAC,
-		.ana_reg_base = A2W_PLLB_ANA0,
-		.reference_enable_mask = A2W_XOSC_CTRL_PLLB_ENABLE,
-		.lock_mask = CM_LOCK_FLOCKB,
-
-		.ana = &bcm2835_ana_default,
-
-		.min_rate = 600000000u,
-		.max_rate = 3000000000u,
-		.max_fb_rate = BCM2835_MAX_FB_RATE),
-	[BCM2835_PLLB_ARM]	= REGISTER_PLL_DIV(
-		.name = "pllb_arm",
-		.source_pll = "pllb",
-		.cm_reg = CM_PLLB,
-		.a2w_reg = A2W_PLLB_ARM,
-		.load_mask = CM_PLLB_LOADARM,
-		.hold_mask = CM_PLLB_HOLDARM,
-		.fixed_divider = 1,
-		.flags = CLK_SET_RATE_PARENT),
+	/*
+	 * PLLB is used for the ARM's clock. Controlled by firmware, see
+	 * clk-raspberrypi.c.
+	 */
 
 	/*
 	 * PLLC is the core PLL, used to drive the core VPU clock.
-- 
2.21.0

[PATCH v2 2/7] clk: bcm283x: add driver interfacing with Raspberry Pi's firmware

[Nicolas Saenz Julienne]

Raspberry Pi's firmware offers an interface though which update it's
clock's frequencies. This is specially useful in order to change the CPU
clock (pllb_arm) which is 'owned' by the firmware and we're unable to
scale using the register interface provided by clk-bcm2835.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Acked-by: Eric Anholt <eric@anholt.net>

---

Changes since v1:
  - Use BIT()
  - Add Kconfig entry, with compile test
  - remove prepare/unprepare
  - Fix uninitialized init.name in pllb registration
  - Add MODULE_ALIAS()
  - Use determine_rate() instead of round_rate()
  - Add small introduction explaining need for driver

 drivers/clk/bcm/Kconfig           |   7 +
 drivers/clk/bcm/Makefile          |   1 +
 drivers/clk/bcm/clk-raspberrypi.c | 302 ++++++++++++++++++++++++++++++
 3 files changed, 310 insertions(+)
 create mode 100644 drivers/clk/bcm/clk-raspberrypi.c

diff --git a/drivers/clk/bcm/Kconfig b/drivers/clk/bcm/Kconfig
index 29ee7b776cd4..a4a2775d65e1 100644
--- a/drivers/clk/bcm/Kconfig
+++ b/drivers/clk/bcm/Kconfig
@@ -64,3 +64,10 @@ config CLK_BCM_SR
 	default ARCH_BCM_IPROC
 	help
 	  Enable common clock framework support for the Broadcom Stingray SoC
+
+config CLK_RASPBERRYPI
+	tristate "Raspberry Pi firmware based clock support"
+	depends on RASPBERRYPI_FIRMWARE || (COMPILE_TEST && !RASPBERRYPI_FIRMWARE)
+	help
+	  Enable common clock framework support for Raspberry Pi's firmware
+	  dependent clocks
diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile
index 002661d39128..eb7159099d82 100644
--- a/drivers/clk/bcm/Makefile
+++ b/drivers/clk/bcm/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_CLK_BCM_KONA)	+= clk-bcm21664.o
 obj-$(CONFIG_COMMON_CLK_IPROC)	+= clk-iproc-armpll.o clk-iproc-pll.o clk-iproc-asiu.o
 obj-$(CONFIG_ARCH_BCM2835)	+= clk-bcm2835.o
 obj-$(CONFIG_ARCH_BCM2835)	+= clk-bcm2835-aux.o
+obj-$(CONFIG_CLK_RASPBERRYPI)	+= clk-raspberrypi.o
 obj-$(CONFIG_ARCH_BCM_53573)	+= clk-bcm53573-ilp.o
 obj-$(CONFIG_CLK_BCM_CYGNUS)	+= clk-cygnus.o
 obj-$(CONFIG_CLK_BCM_HR2)	+= clk-hr2.o
diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c
new file mode 100644
index 000000000000..b1365cf19f3a
--- /dev/null
+++ b/drivers/clk/bcm/clk-raspberrypi.c
@@ -0,0 +1,302 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Raspberry Pi driver for firmware controlled clocks
+ *
+ * Even though clk-bcm2835 provides an interface to the harware registers for
+ * the system clocks we've had to factor out 'pllb' as the firmware 'owns' it.
+ * We're not allowed to change it directly as we might race with the
+ * over-temperature and under-voltage protections provided by the firmware.
+ *
+ * Copyright (C) 2019 Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
+ */
+
+#include <linux/clkdev.h>
+#include <linux/clk-provider.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#include <soc/bcm2835/raspberrypi-firmware.h>
+
+#define RPI_FIRMWARE_ARM_CLK_ID		0x000000003
+
+#define RPI_FIRMWARE_STATE_ENABLE_BIT	BIT(0)
+#define RPI_FIRMWARE_STATE_WAIT_BIT	BIT(1)
+
+/*
+ * Even though the firmware interface alters 'pllb' the frequencies are
+ * provided as per 'pllb_arm'. We need to scale before passing them trough.
+ */
+#define RPI_FIRMWARE_PLLB_ARM_DIV_RATE	2
+
+#define A2W_PLL_FRAC_BITS		20
+
+struct raspberrypi_clk {
+	struct device *dev;
+	struct rpi_firmware *firmware;
+
+	unsigned long min_rate;
+	unsigned long max_rate;
+
+	struct clk_hw pllb;
+	struct clk_hw *pllb_arm;
+	struct clk_lookup *pllb_arm_lookup;
+};
+
+/*
+ * Structure of the message passed to Raspberry Pi's firmware in order to
+ * change clock rates. The 'disable_turbo' option is only available to the ARM
+ * clock (pllb) which we enable by default as turbo mode will alter multiple
+ * clocks at once.
+ *
+ * Even though we're able to access the clock registers directly we're bound to
+ * use the firmware interface as the firmware ultimately takes care of
+ * mitigating overheating/undervoltage situations and we would be changing
+ * frequencies behind his back.
+ *
+ * For more information on the firmware interface check:
+ * https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface
+ */
+struct raspberrypi_firmware_prop {
+	__le32 id;
+	__le32 val;
+	__le32 disable_turbo;
+} __packed;
+
+static int raspberrypi_clock_property(struct rpi_firmware *firmware, u32 tag,
+				      u32 clk, u32 *val)
+{
+	struct raspberrypi_firmware_prop msg = {
+		.id = clk,
+		.val = *val,
+		.disable_turbo = 1,
+	};
+	int ret;
+
+	ret = rpi_firmware_property(firmware, tag, &msg, sizeof(msg));
+	if (ret)
+		return ret;
+
+	*val = msg.val;
+
+	return 0;
+}
+
+static int raspberrypi_fw_pll_is_on(struct clk_hw *hw)
+{
+	struct raspberrypi_clk *rpi = container_of(hw, struct raspberrypi_clk,
+						   pllb);
+	u32 val = 0;
+	int ret;
+
+	ret = raspberrypi_clock_property(rpi->firmware,
+					 RPI_FIRMWARE_GET_CLOCK_STATE,
+					 RPI_FIRMWARE_ARM_CLK_ID, &val);
+	if (ret)
+		return 0;
+
+	return !!(val & RPI_FIRMWARE_STATE_ENABLE_BIT);
+}
+
+
+static unsigned long raspberrypi_fw_pll_get_rate(struct clk_hw *hw,
+						 unsigned long parent_rate)
+{
+	struct raspberrypi_clk *rpi = container_of(hw, struct raspberrypi_clk,
+						   pllb);
+	u32 val = 0;
+	int ret;
+
+	ret = raspberrypi_clock_property(rpi->firmware,
+					 RPI_FIRMWARE_GET_CLOCK_RATE,
+					 RPI_FIRMWARE_ARM_CLK_ID,
+					 &val);
+	if (ret)
+		return ret;
+
+	return val * RPI_FIRMWARE_PLLB_ARM_DIV_RATE;
+}
+
+static int raspberrypi_fw_pll_set_rate(struct clk_hw *hw, unsigned long rate,
+				       unsigned long parent_rate)
+{
+	struct raspberrypi_clk *rpi = container_of(hw, struct raspberrypi_clk,
+						   pllb);
+	u32 new_rate = rate / RPI_FIRMWARE_PLLB_ARM_DIV_RATE;
+	int ret;
+
+	ret = raspberrypi_clock_property(rpi->firmware,
+					 RPI_FIRMWARE_SET_CLOCK_RATE,
+					 RPI_FIRMWARE_ARM_CLK_ID,
+					 &new_rate);
+	if (ret)
+		dev_err_ratelimited(rpi->dev, "Failed to change %s frequency: %d",
+				    clk_hw_get_name(hw), ret);
+
+	return ret;
+}
+
+/*
+ * Sadly there is no firmware rate rounding interface. We borrowed it from
+ * clk-bcm2835.
+ */
+static int raspberrypi_pll_determine_rate(struct clk_hw *hw,
+				           struct clk_rate_request *req)
+{
+	struct raspberrypi_clk *rpi = container_of(hw, struct raspberrypi_clk,
+						   pllb);
+	u64 div, final_rate;
+	u32 ndiv, fdiv;
+
+	/* We can't use req->rate directly as it would overflow */
+	final_rate = clamp(req->rate, rpi->min_rate, rpi->max_rate);
+
+	div = (u64)final_rate << A2W_PLL_FRAC_BITS;
+	do_div(div, req->best_parent_rate);
+
+	ndiv = div >> A2W_PLL_FRAC_BITS;
+	fdiv = div & ((1 << A2W_PLL_FRAC_BITS) - 1);
+
+	final_rate = ((u64)req->best_parent_rate *
+					((ndiv << A2W_PLL_FRAC_BITS) + fdiv));
+
+	req->rate = final_rate >> A2W_PLL_FRAC_BITS;
+
+	return 0;
+}
+
+static const struct clk_ops raspberrypi_firmware_pll_clk_ops = {
+	.is_prepared = raspberrypi_fw_pll_is_on,
+	.recalc_rate = raspberrypi_fw_pll_get_rate,
+	.set_rate = raspberrypi_fw_pll_set_rate,
+	.determine_rate = raspberrypi_pll_determine_rate,
+};
+
+static int raspberrypi_register_pllb(struct raspberrypi_clk *rpi)
+{
+	u32 min_rate = 0, max_rate = 0;
+	struct clk_init_data init;
+	int ret;
+
+	memset(&init, 0, sizeof(init));
+
+	/* All of the PLLs derive from the external oscillator. */
+	init.parent_names = (const char *[]){ "osc" };
+	init.num_parents = 1;
+	init.name = "pllb";
+	init.ops = &raspberrypi_firmware_pll_clk_ops;
+	init.flags = CLK_GET_RATE_NOCACHE | CLK_IGNORE_UNUSED;
+
+	/* Get min & max rates set by the firmware */
+	ret = raspberrypi_clock_property(rpi->firmware,
+					 RPI_FIRMWARE_GET_MIN_CLOCK_RATE,
+					 RPI_FIRMWARE_ARM_CLK_ID,
+					 &min_rate);
+	if (ret) {
+		dev_err(rpi->dev, "Failed to get %s min freq: %d\n",
+			init.name, ret);
+		return ret;
+	}
+
+	ret = raspberrypi_clock_property(rpi->firmware,
+					 RPI_FIRMWARE_GET_MAX_CLOCK_RATE,
+					 RPI_FIRMWARE_ARM_CLK_ID,
+					 &max_rate);
+	if (ret) {
+		dev_err(rpi->dev, "Failed to get %s max freq: %d\n",
+			init.name, ret);
+		return ret;
+	}
+
+	if (!min_rate || !max_rate) {
+		dev_err(rpi->dev, "Unexpected frequency range: min %u, max %u\n",
+			min_rate, max_rate);
+		return -EINVAL;
+	}
+
+	dev_info(rpi->dev, "CPU frequency range: min %u, max %u\n",
+		 min_rate, max_rate);
+
+	rpi->min_rate = min_rate * RPI_FIRMWARE_PLLB_ARM_DIV_RATE;
+	rpi->max_rate = max_rate * RPI_FIRMWARE_PLLB_ARM_DIV_RATE;
+
+	rpi->pllb.init = &init;
+
+	return devm_clk_hw_register(rpi->dev, &rpi->pllb);
+}
+
+static int raspberrypi_register_pllb_arm(struct raspberrypi_clk *rpi)
+{
+	rpi->pllb_arm = clk_hw_register_fixed_factor(rpi->dev,
+				"pllb_arm", "pllb",
+				CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
+				1, 2);
+	if (IS_ERR(rpi->pllb_arm)) {
+		dev_err(rpi->dev, "Failed to initialize pllb_arm\n");
+		return PTR_ERR(rpi->pllb_arm);
+	}
+
+	rpi->pllb_arm_lookup = clkdev_hw_create(rpi->pllb_arm, NULL, "cpu0");
+	if (!rpi->pllb_arm_lookup) {
+		dev_err(rpi->dev, "Failed to initialize pllb_arm_lookup\n");
+		clk_hw_unregister_fixed_factor(rpi->pllb_arm);
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+static int raspberrypi_clk_probe(struct platform_device *pdev)
+{
+	struct device_node *firmware_node;
+	struct device *dev = &pdev->dev;
+	struct rpi_firmware *firmware;
+	struct raspberrypi_clk *rpi;
+	int ret;
+
+	firmware_node = of_find_compatible_node(NULL, NULL,
+					"raspberrypi,bcm2835-firmware");
+	if (!firmware_node) {
+		dev_err(dev, "Missing firmware node\n");
+		return -ENOENT;
+	}
+
+	firmware = rpi_firmware_get(firmware_node);
+	of_node_put(firmware_node);
+	if (!firmware)
+		return -EPROBE_DEFER;
+
+	rpi = devm_kzalloc(dev, sizeof(*rpi), GFP_KERNEL);
+	if (!rpi)
+		return -ENOMEM;
+
+	rpi->dev = dev;
+	rpi->firmware = firmware;
+
+	ret = raspberrypi_register_pllb(rpi);
+	if (ret) {
+		dev_err(dev, "Failed to initialize pllb, %d\n", ret);
+		return ret;
+	}
+
+	ret = raspberrypi_register_pllb_arm(rpi);
+	if (ret) {
+		dev_err(dev, "Failed to initialize pllb_arm, %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static struct platform_driver raspberrypi_clk_driver = {
+	.driver = {
+		.name = "raspberrypi-clk",
+	},
+	.probe          = raspberrypi_clk_probe,
+};
+module_platform_driver(raspberrypi_clk_driver);
+
+MODULE_AUTHOR("Nicolas Saenz Julienne <nsaenzjulienne@suse.de>");
+MODULE_DESCRIPTION("Raspberry Pi firmware clock driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:raspberrypi-clk");
-- 
2.21.0

[PATCH v2 3/7] firmware: raspberrypi: register clk device

[Nicolas Saenz Julienne]

Since clk-raspberrypi is tied to the VC4 firmware instead of particular
hardware it's registration should be performed by the firmware driver.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Acked-by: Eric Anholt <eric@anholt.net>
---
 drivers/firmware/raspberrypi.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
index 61be15d9df7d..da26a584dca0 100644
--- a/drivers/firmware/raspberrypi.c
+++ b/drivers/firmware/raspberrypi.c
@@ -20,6 +20,7 @@
 #define MBOX_CHAN_PROPERTY		8
 
 static struct platform_device *rpi_hwmon;
+static struct platform_device *rpi_clk;
 
 struct rpi_firmware {
 	struct mbox_client cl;
@@ -207,6 +208,12 @@ rpi_register_hwmon_driver(struct device *dev, struct rpi_firmware *fw)
 						  -1, NULL, 0);
 }
 
+static void rpi_register_clk_driver(struct device *dev)
+{
+	rpi_clk = platform_device_register_data(dev, "raspberrypi-clk",
+						-1, NULL, 0);
+}
+
 static int rpi_firmware_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -234,6 +241,7 @@ static int rpi_firmware_probe(struct platform_device *pdev)
 
 	rpi_firmware_print_firmware_revision(fw);
 	rpi_register_hwmon_driver(dev, fw);
+	rpi_register_clk_driver(dev);
 
 	return 0;
 }
@@ -254,6 +262,8 @@ static int rpi_firmware_remove(struct platform_device *pdev)
 
 	platform_device_unregister(rpi_hwmon);
 	rpi_hwmon = NULL;
+	platform_device_unregister(rpi_clk);
+	rpi_clk = NULL;
 	mbox_free_channel(fw->chan);
 
 	return 0;
-- 
2.21.0

[PATCH v2 4/7] cpufreq: add driver for Raspbery Pi

[Nicolas Saenz Julienne]

Raspberry Pi's firmware offers and interface though which update it's
performance requirements. It allows us to request for specific runtime
frequencies, which the firmware might or might not respect, depending on
the firmware configuration and thermals.

As the maximum and minimum frequencies are configurable in the firmware
there is no way to know in advance their values. So the Raspberry Pi
cpufreq driver queries them, builds an opp frequency table to then
launch cpufreq-dt.

Also, as the firmware interface might be configured as a module, making
the cpu clock unavailable during init, this implements a full fledged
driver, as opposed to most drivers registering cpufreq-dt, which only
make use of an init routine.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Acked-by: Eric Anholt <eric@anholt.net>

---

Changes since v1:
  - Remove compatible checks
  - Add module support, now full fledged driver
  - Use NULL in clk_get()

 drivers/cpufreq/Kconfig.arm           |   8 +++
 drivers/cpufreq/Makefile              |   1 +
 drivers/cpufreq/raspberrypi-cpufreq.c | 100 ++++++++++++++++++++++++++
 3 files changed, 109 insertions(+)
 create mode 100644 drivers/cpufreq/raspberrypi-cpufreq.c

diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index f8129edc145e..5e9204d443ff 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -133,6 +133,14 @@ config ARM_QCOM_CPUFREQ_HW
 	  The driver implements the cpufreq interface for this HW engine.
 	  Say Y if you want to support CPUFreq HW.
 
+config ARM_RASPBERRYPI_CPUFREQ
+	tristate "Raspberry Pi cpufreq support"
+	depends on CLK_RASPBERRYPI || COMPILE_TEST
+	help
+	  This adds the CPUFreq driver for Raspberry Pi
+
+	  If in doubt, say N.
+
 config ARM_S3C_CPUFREQ
 	bool
 	help
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index 689b26c6f949..121c1acb66c0 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -64,6 +64,7 @@ obj-$(CONFIG_ARM_PXA2xx_CPUFREQ)	+= pxa2xx-cpufreq.o
 obj-$(CONFIG_PXA3xx)			+= pxa3xx-cpufreq.o
 obj-$(CONFIG_ARM_QCOM_CPUFREQ_HW)	+= qcom-cpufreq-hw.o
 obj-$(CONFIG_ARM_QCOM_CPUFREQ_KRYO)	+= qcom-cpufreq-kryo.o
+obj-$(CONFIG_ARM_RASPBERRYPI_CPUFREQ) 	+= raspberrypi-cpufreq.o
 obj-$(CONFIG_ARM_S3C2410_CPUFREQ)	+= s3c2410-cpufreq.o
 obj-$(CONFIG_ARM_S3C2412_CPUFREQ)	+= s3c2412-cpufreq.o
 obj-$(CONFIG_ARM_S3C2416_CPUFREQ)	+= s3c2416-cpufreq.o
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
@@ -0,0 +1,100 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Raspberry Pi cpufreq driver
+ *
+ * Copyright (C) 2019, Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
+ */
+
+#include <linux/clk.h>
+#include <linux/cpu.h>
+#include <linux/cpufreq.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_opp.h>
+
+static struct platform_device *cpufreq_dt;
+
+static int raspberrypi_cpufreq_probe(struct platform_device *pdev)
+{
+	struct device *cpu_dev;
+	unsigned long min, max;
+	unsigned long rate;
+	struct clk *clk;
+	int ret;
+
+	cpu_dev = get_cpu_device(0);
+	if (!cpu_dev) {
+		pr_err("Cannot get CPU for cpufreq driver\n");
+		return -ENODEV;
+	}
+
+	clk = clk_get(cpu_dev, NULL);
+	if (IS_ERR(clk)) {
+		dev_err(cpu_dev, "Cannot get clock for CPU0\n");
+		return PTR_ERR(clk);
+	}
+
+	/*
+	 * The max and min frequencies are configurable in the Raspberry Pi
+	 * firmware, so we query them at runtime
+	 */
+	min = clk_round_rate(clk, 0);
+	max = clk_round_rate(clk, ULONG_MAX);
+	clk_put(clk);
+
+	for (rate = min; rate < max; rate += 100000000) {
+		ret = dev_pm_opp_add(cpu_dev, rate, 0);
+		if (ret)
+			goto remove_opp;
+	}
+
+	ret = dev_pm_opp_add(cpu_dev, max, 0);
+	if (ret)
+		goto remove_opp;
+
+	cpufreq_dt = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
+	ret = PTR_ERR_OR_ZERO(cpufreq_dt);
+	if (ret) {
+		dev_err(cpu_dev, "Failed to create platform device, %d\n", ret);
+		goto remove_opp;
+	}
+
+	return 0;
+
+remove_opp:
+	dev_pm_opp_remove_all_dynamic(cpu_dev);
+
+	return ret;
+}
+
+static int raspberrypi_cpufreq_remove(struct platform_device *pdev)
+{
+	struct device *cpu_dev;
+
+	cpu_dev = get_cpu_device(0);
+	if (cpu_dev)
+		dev_pm_opp_remove_all_dynamic(cpu_dev);
+
+	platform_device_unregister(cpufreq_dt);
+	cpufreq_dt = NULL;
+
+	return 0;
+}
+
+/*
+ * 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);
+
+MODULE_AUTHOR("Nicolas Saenz Julienne <nsaenzjulienne@suse.de");
+MODULE_DESCRIPTION("Raspberry Pi cpufreq driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:raspberrypi-cpufreq");
-- 
2.21.0

[PATCH v2 5/7] clk: raspberrypi: register platform device for raspberrypi-cpufreq

[Nicolas Saenz Julienne]

As 'clk-raspberrypi' depends on RPi's firmware interface, which might be
configured as a module, the cpu clock might not be available for the
cpufreq driver during it's init process. So we register the
'raspberrypi-cpufreq' platform device after the probe sequence succeeds.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Acked-by: Eric Anholt <eric@anholt.net>
---
 drivers/clk/bcm/clk-raspberrypi.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c
index b1365cf19f3a..052296b5fbe4 100644
--- a/drivers/clk/bcm/clk-raspberrypi.c
+++ b/drivers/clk/bcm/clk-raspberrypi.c
@@ -63,6 +63,8 @@ struct raspberrypi_firmware_prop {
 	__le32 disable_turbo;
 } __packed;
 
+static struct platform_device *rpi_cpufreq;
+
 static int raspberrypi_clock_property(struct rpi_firmware *firmware, u32 tag,
 				      u32 clk, u32 *val)
 {
@@ -285,6 +287,17 @@ static int raspberrypi_clk_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	rpi_cpufreq = platform_device_register_data(dev, "raspberrypi-cpufreq",
+						    -1, NULL, 0);
+
+	return 0;
+}
+
+static int raspberrypi_clk_remove(struct platform_device *pdev)
+{
+	platform_device_unregister(rpi_cpufreq);
+	rpi_cpufreq = NULL;
+
 	return 0;
 }
 
@@ -293,6 +306,7 @@ static struct platform_driver raspberrypi_clk_driver = {
 		.name = "raspberrypi-clk",
 	},
 	.probe          = raspberrypi_clk_probe,
+	.remove		= raspberrypi_clk_remove,
 };
 module_platform_driver(raspberrypi_clk_driver);
 
-- 
2.21.0

[PATCH v2 6/7] ARM: defconfig: enable cpufreq driver for RPi

[Nicolas Saenz Julienne]

This enables on both multi_v7_defconfig and bcm2835_defconfig the new
firmware based clock and cpufreq drivers for the Raspberry Pi platform.

In the case of bcm2835_defconfig, as the cpufreq subsystem was disabled,
the subsystem configuration was copied from multi_v7_defconfig (default
governor, statistics, etc...).

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
 arch/arm/configs/bcm2835_defconfig  | 9 +++++++++
 arch/arm/configs/multi_v7_defconfig | 2 ++
 2 files changed, 11 insertions(+)

diff --git a/arch/arm/configs/bcm2835_defconfig b/arch/arm/configs/bcm2835_defconfig
index dcf7610cfe55..3fd90bfd5fec 100644
--- a/arch/arm/configs/bcm2835_defconfig
+++ b/arch/arm/configs/bcm2835_defconfig
@@ -37,6 +37,14 @@ CONFIG_CMA=y
 CONFIG_SECCOMP=y
 CONFIG_KEXEC=y
 CONFIG_CRASH_DUMP=y
+CONFIG_CPU_FREQ=y
+CONFIG_CPU_FREQ_STAT=y
+CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND=y
+CONFIG_CPU_FREQ_GOV_POWERSAVE=m
+CONFIG_CPU_FREQ_GOV_USERSPACE=m
+CONFIG_CPU_FREQ_GOV_CONSERVATIVE=m
+CONFIG_CPUFREQ_DT=y
+CONFIG_ARM_RASPBERRYPI_CPUFREQ=y
 CONFIG_VFP=y
 # CONFIG_CORE_DUMP_DEFAULT_ELF_HEADERS is not set
 # CONFIG_SUSPEND is not set
@@ -132,6 +140,7 @@ CONFIG_DMA_BCM2835=y
 CONFIG_STAGING=y
 CONFIG_SND_BCM2835=m
 CONFIG_VIDEO_BCM2835=m
+CONFIG_CLK_RASPBERRYPI=y
 CONFIG_MAILBOX=y
 CONFIG_BCM2835_MBOX=y
 # CONFIG_IOMMU_SUPPORT is not set
diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index 6b748f214eae..0fd60a83f768 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -102,6 +102,7 @@ CONFIG_CPU_FREQ_GOV_CONSERVATIVE=m
 CONFIG_CPU_FREQ_GOV_SCHEDUTIL=y
 CONFIG_CPUFREQ_DT=y
 CONFIG_ARM_IMX6Q_CPUFREQ=y
+CONFIG_ARM_RASPBERRYPI_CPUFREQ=y
 CONFIG_QORIQ_CPUFREQ=y
 CONFIG_CPU_IDLE=y
 CONFIG_ARM_CPUIDLE=y
@@ -899,6 +900,7 @@ CONFIG_STAGING_BOARD=y
 CONFIG_COMMON_CLK_MAX77686=y
 CONFIG_COMMON_CLK_RK808=m
 CONFIG_COMMON_CLK_S2MPS11=m
+CONFIG_CLK_RASPBERRYPI=y
 CONFIG_COMMON_CLK_QCOM=y
 CONFIG_QCOM_CLK_RPM=y
 CONFIG_APQ_MMCC_8084=y
-- 
2.21.0

[PATCH v2 7/7] arm64: defconfig: enable cpufreq support for RPi3

[Nicolas Saenz Julienne]

This enables both the new firmware clock driver and cpufreq driver
available for the RPi3 family of boards.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
 arch/arm64/configs/defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 4d583514258c..3b7baffb3087 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -82,6 +82,7 @@ CONFIG_CPUFREQ_DT=y
 CONFIG_ACPI_CPPC_CPUFREQ=m
 CONFIG_ARM_ARMADA_37XX_CPUFREQ=y
 CONFIG_ARM_SCPI_CPUFREQ=y
+CONFIG_ARM_RASPBERRYPI_CPUFREQ=y
 CONFIG_ARM_TEGRA186_CPUFREQ=y
 CONFIG_ARM_SCPI_PROTOCOL=y
 CONFIG_RASPBERRYPI_FIRMWARE=y
@@ -639,6 +640,7 @@ CONFIG_COMMON_CLK_CS2000_CP=y
 CONFIG_COMMON_CLK_S2MPS11=y
 CONFIG_CLK_QORIQ=y
 CONFIG_COMMON_CLK_PWM=y
+CONFIG_CLK_RASPBERRYPI=y
 CONFIG_CLK_IMX8MQ=y
 CONFIG_CLK_IMX8QXP=y
 CONFIG_TI_SCI_CLK=y
-- 
2.21.0

Re: [PATCH v2 2/7] clk: bcm283x: add driver interfacing with Raspberry Pi's firmware

[Nicolas Saenz Julienne]

[-- Attachment #1: Type: text/plain, Size: 11780 bytes --]

On Thu, 2019-06-06 at 16:22 +0200, Nicolas Saenz Julienne wrote:
> Raspberry Pi's firmware offers an interface though which update it's
> clock's frequencies. This is specially useful in order to change the CPU
> clock (pllb_arm) which is 'owned' by the firmware and we're unable to
> scale using the register interface provided by clk-bcm2835.
> 
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> Acked-by: Eric Anholt <eric@anholt.net>
> 
> ---
> 
> Changes since v1:
>   - Use BIT()
>   - Add Kconfig entry, with compile test
>   - remove prepare/unprepare
>   - Fix uninitialized init.name in pllb registration
>   - Add MODULE_ALIAS()
>   - Use determine_rate() instead of round_rate()
>   - Add small introduction explaining need for driver
> 
>  drivers/clk/bcm/Kconfig           |   7 +
>  drivers/clk/bcm/Makefile          |   1 +
>  drivers/clk/bcm/clk-raspberrypi.c | 302 ++++++++++++++++++++++++++++++
>  3 files changed, 310 insertions(+)
>  create mode 100644 drivers/clk/bcm/clk-raspberrypi.c
> 
> diff --git a/drivers/clk/bcm/Kconfig b/drivers/clk/bcm/Kconfig
> index 29ee7b776cd4..a4a2775d65e1 100644
> --- a/drivers/clk/bcm/Kconfig
> +++ b/drivers/clk/bcm/Kconfig
> @@ -64,3 +64,10 @@ config CLK_BCM_SR
>  	default ARCH_BCM_IPROC
>  	help
>  	  Enable common clock framework support for the Broadcom Stingray SoC
> +
> +config CLK_RASPBERRYPI
> +	tristate "Raspberry Pi firmware based clock support"
> +	depends on RASPBERRYPI_FIRMWARE || (COMPILE_TEST &&
> !RASPBERRYPI_FIRMWARE)
> +	help
> +	  Enable common clock framework support for Raspberry Pi's firmware
> +	  dependent clocks
> diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile
> index 002661d39128..eb7159099d82 100644
> --- a/drivers/clk/bcm/Makefile
> +++ b/drivers/clk/bcm/Makefile
> @@ -7,6 +7,7 @@ obj-$(CONFIG_CLK_BCM_KONA)	+= clk-bcm21664.o
>  obj-$(CONFIG_COMMON_CLK_IPROC)	+= clk-iproc-armpll.o clk-iproc-pll.o
> clk-iproc-asiu.o
>  obj-$(CONFIG_ARCH_BCM2835)	+= clk-bcm2835.o
>  obj-$(CONFIG_ARCH_BCM2835)	+= clk-bcm2835-aux.o
> +obj-$(CONFIG_CLK_RASPBERRYPI)	+= clk-raspberrypi.o
>  obj-$(CONFIG_ARCH_BCM_53573)	+= clk-bcm53573-ilp.o
>  obj-$(CONFIG_CLK_BCM_CYGNUS)	+= clk-cygnus.o
>  obj-$(CONFIG_CLK_BCM_HR2)	+= clk-hr2.o
> diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-
> raspberrypi.c
> new file mode 100644
> index 000000000000..b1365cf19f3a
> --- /dev/null
> +++ b/drivers/clk/bcm/clk-raspberrypi.c
> @@ -0,0 +1,302 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Raspberry Pi driver for firmware controlled clocks
> + *
> + * Even though clk-bcm2835 provides an interface to the harware registers for
> + * the system clocks we've had to factor out 'pllb' as the firmware 'owns'
> it.
> + * We're not allowed to change it directly as we might race with the
> + * over-temperature and under-voltage protections provided by the firmware.
> + *
> + * Copyright (C) 2019 Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> + */
> +
> +#include <linux/clkdev.h>
> +#include <linux/clk-provider.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +
> +#include <soc/bcm2835/raspberrypi-firmware.h>
> +
> +#define RPI_FIRMWARE_ARM_CLK_ID		0x000000003
> +
> +#define RPI_FIRMWARE_STATE_ENABLE_BIT	BIT(0)
> +#define RPI_FIRMWARE_STATE_WAIT_BIT	BIT(1)
> +
> +/*
> + * Even though the firmware interface alters 'pllb' the frequencies are
> + * provided as per 'pllb_arm'. We need to scale before passing them trough.
> + */
> +#define RPI_FIRMWARE_PLLB_ARM_DIV_RATE	2
> +
> +#define A2W_PLL_FRAC_BITS		20
> +
> +struct raspberrypi_clk {
> +	struct device *dev;
> +	struct rpi_firmware *firmware;
> +
> +	unsigned long min_rate;
> +	unsigned long max_rate;
> +
> +	struct clk_hw pllb;
> +	struct clk_hw *pllb_arm;
> +	struct clk_lookup *pllb_arm_lookup;
> +};
> +
> +/*
> + * Structure of the message passed to Raspberry Pi's firmware in order to
> + * change clock rates. The 'disable_turbo' option is only available to the
> ARM
> + * clock (pllb) which we enable by default as turbo mode will alter multiple
> + * clocks at once.
> + *
> + * Even though we're able to access the clock registers directly we're bound
> to
> + * use the firmware interface as the firmware ultimately takes care of
> + * mitigating overheating/undervoltage situations and we would be changing
> + * frequencies behind his back.
> + *
> + * For more information on the firmware interface check:
> + * https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface
> + */
> +struct raspberrypi_firmware_prop {
> +	__le32 id;
> +	__le32 val;
> +	__le32 disable_turbo;
> +} __packed;
> +
> +static int raspberrypi_clock_property(struct rpi_firmware *firmware, u32 tag,
> +				      u32 clk, u32 *val)
> +{
> +	struct raspberrypi_firmware_prop msg = {
> +		.id = clk,
> +		.val = *val,
> +		.disable_turbo = 1,
> +	};
> +	int ret;
> +
> +	ret = rpi_firmware_property(firmware, tag, &msg, sizeof(msg));
> +	if (ret)
> +		return ret;
> +
> +	*val = msg.val;
> +
> +	return 0;
> +}
> +
> +static int raspberrypi_fw_pll_is_on(struct clk_hw *hw)
> +{
> +	struct raspberrypi_clk *rpi = container_of(hw, struct raspberrypi_clk,
> +						   pllb);
> +	u32 val = 0;
> +	int ret;
> +
> +	ret = raspberrypi_clock_property(rpi->firmware,
> +					 RPI_FIRMWARE_GET_CLOCK_STATE,
> +					 RPI_FIRMWARE_ARM_CLK_ID, &val);
> +	if (ret)
> +		return 0;
> +
> +	return !!(val & RPI_FIRMWARE_STATE_ENABLE_BIT);
> +}
> +
> +
> +static unsigned long raspberrypi_fw_pll_get_rate(struct clk_hw *hw,
> +						 unsigned long parent_rate)
> +{
> +	struct raspberrypi_clk *rpi = container_of(hw, struct raspberrypi_clk,
> +						   pllb);
> +	u32 val = 0;
> +	int ret;
> +
> +	ret = raspberrypi_clock_property(rpi->firmware,
> +					 RPI_FIRMWARE_GET_CLOCK_RATE,
> +					 RPI_FIRMWARE_ARM_CLK_ID,
> +					 &val);
> +	if (ret)
> +		return ret;
> +
> +	return val * RPI_FIRMWARE_PLLB_ARM_DIV_RATE;
> +}
> +
> +static int raspberrypi_fw_pll_set_rate(struct clk_hw *hw, unsigned long rate,
> +				       unsigned long parent_rate)
> +{
> +	struct raspberrypi_clk *rpi = container_of(hw, struct raspberrypi_clk,
> +						   pllb);
> +	u32 new_rate = rate / RPI_FIRMWARE_PLLB_ARM_DIV_RATE;
> +	int ret;
> +
> +	ret = raspberrypi_clock_property(rpi->firmware,
> +					 RPI_FIRMWARE_SET_CLOCK_RATE,
> +					 RPI_FIRMWARE_ARM_CLK_ID,
> +					 &new_rate);
> +	if (ret)
> +		dev_err_ratelimited(rpi->dev, "Failed to change %s frequency:
> %d",
> +				    clk_hw_get_name(hw), ret);
> +
> +	return ret;
> +}
> +
> +/*
> + * Sadly there is no firmware rate rounding interface. We borrowed it from
> + * clk-bcm2835.
> + */
> +static int raspberrypi_pll_determine_rate(struct clk_hw *hw,
> +				           struct clk_rate_request *req)
> +{
> +	struct raspberrypi_clk *rpi = container_of(hw, struct raspberrypi_clk,
> +						   pllb);
> +	u64 div, final_rate;
> +	u32 ndiv, fdiv;
> +
> +	/* We can't use req->rate directly as it would overflow */
> +	final_rate = clamp(req->rate, rpi->min_rate, rpi->max_rate);
> +
> +	div = (u64)final_rate << A2W_PLL_FRAC_BITS;
> +	do_div(div, req->best_parent_rate);
> +
> +	ndiv = div >> A2W_PLL_FRAC_BITS;
> +	fdiv = div & ((1 << A2W_PLL_FRAC_BITS) - 1);
> +
> +	final_rate = ((u64)req->best_parent_rate *
> +					((ndiv << A2W_PLL_FRAC_BITS) + fdiv));
> +
> +	req->rate = final_rate >> A2W_PLL_FRAC_BITS;
> +
> +	return 0;
> +}
> +
> +static const struct clk_ops raspberrypi_firmware_pll_clk_ops = {
> +	.is_prepared = raspberrypi_fw_pll_is_on,
> +	.recalc_rate = raspberrypi_fw_pll_get_rate,
> +	.set_rate = raspberrypi_fw_pll_set_rate,
> +	.determine_rate = raspberrypi_pll_determine_rate,
> +};
> +
> +static int raspberrypi_register_pllb(struct raspberrypi_clk *rpi)
> +{
> +	u32 min_rate = 0, max_rate = 0;
> +	struct clk_init_data init;
> +	int ret;
> +
> +	memset(&init, 0, sizeof(init));
> +
> +	/* All of the PLLs derive from the external oscillator. */
> +	init.parent_names = (const char *[]){ "osc" };
> +	init.num_parents = 1;
> +	init.name = "pllb";
> +	init.ops = &raspberrypi_firmware_pll_clk_ops;
> +	init.flags = CLK_GET_RATE_NOCACHE | CLK_IGNORE_UNUSED;
> +
> +	/* Get min & max rates set by the firmware */
> +	ret = raspberrypi_clock_property(rpi->firmware,
> +					 RPI_FIRMWARE_GET_MIN_CLOCK_RATE,
> +					 RPI_FIRMWARE_ARM_CLK_ID,
> +					 &min_rate);
> +	if (ret) {
> +		dev_err(rpi->dev, "Failed to get %s min freq: %d\n",
> +			init.name, ret);
> +		return ret;
> +	}
> +
> +	ret = raspberrypi_clock_property(rpi->firmware,
> +					 RPI_FIRMWARE_GET_MAX_CLOCK_RATE,
> +					 RPI_FIRMWARE_ARM_CLK_ID,
> +					 &max_rate);
> +	if (ret) {
> +		dev_err(rpi->dev, "Failed to get %s max freq: %d\n",
> +			init.name, ret);
> +		return ret;
> +	}
> +
> +	if (!min_rate || !max_rate) {
> +		dev_err(rpi->dev, "Unexpected frequency range: min %u, max
> %u\n",
> +			min_rate, max_rate);
> +		return -EINVAL;
> +	}
> +
> +	dev_info(rpi->dev, "CPU frequency range: min %u, max %u\n",
> +		 min_rate, max_rate);
> +
> +	rpi->min_rate = min_rate * RPI_FIRMWARE_PLLB_ARM_DIV_RATE;
> +	rpi->max_rate = max_rate * RPI_FIRMWARE_PLLB_ARM_DIV_RATE;
> +
> +	rpi->pllb.init = &init;
> +
> +	return devm_clk_hw_register(rpi->dev, &rpi->pllb);
> +}
> +
> +static int raspberrypi_register_pllb_arm(struct raspberrypi_clk *rpi)
> +{
> +	rpi->pllb_arm = clk_hw_register_fixed_factor(rpi->dev,
> +				"pllb_arm", "pllb",
> +				CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
> +				1, 2);
> +	if (IS_ERR(rpi->pllb_arm)) {
> +		dev_err(rpi->dev, "Failed to initialize pllb_arm\n");
> +		return PTR_ERR(rpi->pllb_arm);
> +	}
> +
> +	rpi->pllb_arm_lookup = clkdev_hw_create(rpi->pllb_arm, NULL, "cpu0");
> +	if (!rpi->pllb_arm_lookup) {
> +		dev_err(rpi->dev, "Failed to initialize pllb_arm_lookup\n");
> +		clk_hw_unregister_fixed_factor(rpi->pllb_arm);
> +		return -ENOMEM;
> +	}
> +
> +	return 0;
> +}
> +
> +static int raspberrypi_clk_probe(struct platform_device *pdev)
> +{
> +	struct device_node *firmware_node;
> +	struct device *dev = &pdev->dev;
> +	struct rpi_firmware *firmware;
> +	struct raspberrypi_clk *rpi;
> +	int ret;
> +
> +	firmware_node = of_find_compatible_node(NULL, NULL,
> +					"raspberrypi,bcm2835-firmware");
> +	if (!firmware_node) {
> +		dev_err(dev, "Missing firmware node\n");
> +		return -ENOENT;
> +	}
> +
> +	firmware = rpi_firmware_get(firmware_node);
> +	of_node_put(firmware_node);
> +	if (!firmware)
> +		return -EPROBE_DEFER;
> +
> +	rpi = devm_kzalloc(dev, sizeof(*rpi), GFP_KERNEL);
> +	if (!rpi)
> +		return -ENOMEM;
> +
> +	rpi->dev = dev;
> +	rpi->firmware = firmware;
> +
> +	ret = raspberrypi_register_pllb(rpi);
> +	if (ret) {
> +		dev_err(dev, "Failed to initialize pllb, %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = raspberrypi_register_pllb_arm(rpi);
> +	if (ret) {
> +		dev_err(dev, "Failed to initialize pllb_arm, %d\n", ret);

Sorry Stefan, I missed removing this message. It's done for the next revision.

> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static struct platform_driver raspberrypi_clk_driver = {
> +	.driver = {
> +		.name = "raspberrypi-clk",
> +	},
> +	.probe          = raspberrypi_clk_probe,
> +};
> +module_platform_driver(raspberrypi_clk_driver);
> +
> +MODULE_AUTHOR("Nicolas Saenz Julienne <nsaenzjulienne@suse.de>");
> +MODULE_DESCRIPTION("Raspberry Pi firmware clock driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:raspberrypi-clk");


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 5/7] clk: raspberrypi: register platform device for raspberrypi-cpufreq

[Stephen Boyd]

Quoting Nicolas Saenz Julienne (2019-06-06 07:22:58)
> diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c
> index b1365cf19f3a..052296b5fbe4 100644
> --- a/drivers/clk/bcm/clk-raspberrypi.c
> +++ b/drivers/clk/bcm/clk-raspberrypi.c
> @@ -63,6 +63,8 @@ struct raspberrypi_firmware_prop {
>         __le32 disable_turbo;
>  } __packed;
>  
> +static struct platform_device *rpi_cpufreq;

Why can't this be stored in platform driver data?

> +
>  static int raspberrypi_clock_property(struct rpi_firmware *firmware, u32 tag,
>                                       u32 clk, u32 *val)
>  {
> @@ -285,6 +287,17 @@ static int raspberrypi_clk_probe(struct platform_device *pdev)
>                 return ret;
>         }
>  
> +       rpi_cpufreq = platform_device_register_data(dev, "raspberrypi-cpufreq",
> +                                                   -1, NULL, 0);
> +
> +       return 0;
> +}
> +
> +static int raspberrypi_clk_remove(struct platform_device *pdev)
> +{
> +       platform_device_unregister(rpi_cpufreq);
> +       rpi_cpufreq = NULL;

This assignment to NULL looks unnecessary.

> +
>         return 0;
>  }
>  

Re: [PATCH v2 4/7] cpufreq: add driver for Raspbery Pi

[Stephen Boyd]

Quoting Nicolas Saenz Julienne (2019-06-06 07:22:56)
> 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
[...]
> +
> +/*
> + * 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.

> +
> +MODULE_AUTHOR("Nicolas Saenz Julienne <nsaenzjulienne@suse.de");
> +MODULE_DESCRIPTION("Raspberry Pi cpufreq driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:raspberrypi-cpufreq");

I don't think the module alias is needed anymore.

Re: [PATCH v2 5/7] clk: raspberrypi: register platform device for raspberrypi-cpufreq

[Nicolas Saenz Julienne]

[-- Attachment #1: Type: text/plain, Size: 1781 bytes --]

On Thu, 2019-06-06 at 10:05 -0700, Stephen Boyd wrote:
> Quoting Nicolas Saenz Julienne (2019-06-06 07:22:58)
> > diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-
> > raspberrypi.c
> > index b1365cf19f3a..052296b5fbe4 100644
> > --- a/drivers/clk/bcm/clk-raspberrypi.c
> > +++ b/drivers/clk/bcm/clk-raspberrypi.c
> > @@ -63,6 +63,8 @@ struct raspberrypi_firmware_prop {
> >         __le32 disable_turbo;
> >  } __packed;
> >  
> > +static struct platform_device *rpi_cpufreq;
> 
> Why can't this be stored in platform driver data?

It actually could, I just followed the same pattern as the code found in patch
#3. I'll update it in the next version if you prefer it. 

> 
> > +
> >  static int raspberrypi_clock_property(struct rpi_firmware *firmware, u32
> > tag,
> >                                       u32 clk, u32 *val)
> >  {
> > @@ -285,6 +287,17 @@ static int raspberrypi_clk_probe(struct platform_device
> > *pdev)
> >                 return ret;
> >         }
> >  
> > +       rpi_cpufreq = platform_device_register_data(dev, "raspberrypi-
> > cpufreq",
> > +                                                   -1, NULL, 0);
> > +
> > +       return 0;
> > +}
> > +
> > +static int raspberrypi_clk_remove(struct platform_device *pdev)
> > +{
> > +       platform_device_unregister(rpi_cpufreq);
> > +       rpi_cpufreq = NULL;
> 
> This assignment to NULL looks unnecessary.
> 

Same as above, but now that you pointed out, it's true it doesn't have any
effect

> > +
> >         return 0;
> >  }
> >  
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 5/7] clk: raspberrypi: register platform device for raspberrypi-cpufreq

[Stephen Boyd]

Quoting Nicolas Saenz Julienne (2019-06-06 10:16:56)
> On Thu, 2019-06-06 at 10:05 -0700, Stephen Boyd wrote:
> > Quoting Nicolas Saenz Julienne (2019-06-06 07:22:58)
> > > diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-
> > > raspberrypi.c
> > > index b1365cf19f3a..052296b5fbe4 100644
> > > --- a/drivers/clk/bcm/clk-raspberrypi.c
> > > +++ b/drivers/clk/bcm/clk-raspberrypi.c
> > > @@ -63,6 +63,8 @@ struct raspberrypi_firmware_prop {
> > >         __le32 disable_turbo;
> > >  } __packed;
> > >  
> > > +static struct platform_device *rpi_cpufreq;
> > 
> > Why can't this be stored in platform driver data?
> 
> It actually could, I just followed the same pattern as the code found in patch
> #3. I'll update it in the next version if you prefer it. 
> 

Yes please. It reduces global state.

Re: [PATCH v2 4/7] cpufreq: add driver for Raspbery Pi

[Nicolas Saenz Julienne]

[-- Attachment #1: Type: text/plain, Size: 1758 bytes --]

Hi Stephen,
Thanks for the review.

On Thu, 2019-06-06 at 10:09 -0700, Stephen Boyd wrote:
> Quoting Nicolas Saenz Julienne (2019-06-06 07:22:56)
> > 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
> [...]
> > +
> > +/*
> > + * 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 :)

> > +
> > +MODULE_AUTHOR("Nicolas Saenz Julienne <nsaenzjulienne@suse.de");
> > +MODULE_DESCRIPTION("Raspberry Pi cpufreq driver");
> > +MODULE_LICENSE("GPL");
> > +MODULE_ALIAS("platform:raspberrypi-cpufreq");
> 
> I don't think the module alias is needed anymore.

That's surprising. I remember the driver not being loaded by udev without it.

Regards,
Nicolas


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 4/7] cpufreq: add driver for Raspbery Pi

[Stephen Boyd]

Quoting Nicolas Saenz Julienne (2019-06-06 10:22:16)
> Hi Stephen,
> Thanks for the review.
> 
> On Thu, 2019-06-06 at 10:09 -0700, Stephen Boyd wrote:
> > Quoting Nicolas Saenz Julienne (2019-06-06 07:22:56)
> > > 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
> > [...]
> > > +
> > > +/*
> > > + * 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?

> 
> > > +
> > > +MODULE_AUTHOR("Nicolas Saenz Julienne <nsaenzjulienne@suse.de");
> > > +MODULE_DESCRIPTION("Raspberry Pi cpufreq driver");
> > > +MODULE_LICENSE("GPL");
> > > +MODULE_ALIAS("platform:raspberrypi-cpufreq");
> > 
> > I don't think the module alias is needed anymore.
> 
> That's surprising. I remember the driver not being loaded by udev without it.
> 

Maybe I'm wrong. Could be not needed for DT based platform devices with
an OF table.

Re: [PATCH v2 4/7] cpufreq: add driver for Raspbery Pi

[Nicolas Saenz Julienne]

[-- Attachment #1: Type: text/plain, Size: 3459 bytes --]

On Thu, 2019-06-06 at 10:36 -0700, Stephen Boyd wrote:
> Quoting Nicolas Saenz Julienne (2019-06-06 10:22:16)
> > Hi Stephen,
> > Thanks for the review.
> > 
> > On Thu, 2019-06-06 at 10:09 -0700, Stephen Boyd wrote:
> > > Quoting Nicolas Saenz Julienne (2019-06-06 07:22:56)
> > > > 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
> > > [...]
> > > > +
> > > > +/*
> > > > + * 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 :).

> > > > +
> > > > +MODULE_AUTHOR("Nicolas Saenz Julienne <nsaenzjulienne@suse.de");
> > > > +MODULE_DESCRIPTION("Raspberry Pi cpufreq driver");
> > > > +MODULE_LICENSE("GPL");
> > > > +MODULE_ALIAS("platform:raspberrypi-cpufreq");
> > > 
> > > I don't think the module alias is needed anymore.
> > 
> > That's surprising. I remember the driver not being loaded by udev without
> > it.
> > 
> 
> Maybe I'm wrong. Could be not needed for DT based platform devices with
> an OF table.

As explained in the previous paragraph, I'm not using DT.

Regards,
Nicolas


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 4/7] cpufreq: add driver for Raspbery Pi

[Stephen Boyd]

Quoting Nicolas Saenz Julienne (2019-06-06 11:10:04)
> On Thu, 2019-06-06 at 10:36 -0700, Stephen Boyd wrote:
> > Quoting Nicolas Saenz Julienne (2019-06-06 10:22:16)
> > > Hi Stephen,
> > > Thanks for the review.
> > > 
> > > On Thu, 2019-06-06 at 10:09 -0700, Stephen Boyd wrote:
> > > > Quoting Nicolas Saenz Julienne (2019-06-06 07:22:56)
> > > > > 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
> > > > [...]
> > > > > +
> > > > > +/*
> > > > > + * 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.

Re: [PATCH v2 4/7] cpufreq: add driver for Raspbery Pi

[Nicolas Saenz Julienne]

[-- Attachment #1: Type: text/plain, Size: 3410 bytes --]

On Thu, 2019-06-06 at 11:23 -0700, Stephen Boyd wrote:
> Quoting Nicolas Saenz Julienne (2019-06-06 11:10:04)
> > On Thu, 2019-06-06 at 10:36 -0700, Stephen Boyd wrote:
> > > Quoting Nicolas Saenz Julienne (2019-06-06 10:22:16)
> > > > Hi Stephen,
> > > > Thanks for the review.
> > > > 
> > > > On Thu, 2019-06-06 at 10:09 -0700, Stephen Boyd wrote:
> > > > > Quoting Nicolas Saenz Julienne (2019-06-06 07:22:56)
> > > > > > 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
> > > > > [...]
> > > > > > +
> > > > > > +/*
> > > > > > + * 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!


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 4/7] cpufreq: add driver for Raspbery Pi

[Viresh Kumar]

On 06-06-19, 11:23, Stephen Boyd wrote:
> Yes, thanks. I see that largely follows the commit description so it
> looks OK to me.

Do you want to provide your Reviewed/Acked-by tag before I apply it ?

-- 
viresh

Re: [PATCH v2 4/7] cpufreq: add driver for Raspbery Pi

[Stefan Wahren]

Hi Viresh,

Am 07.06.19 um 05:09 schrieb Viresh Kumar:
> On 06-06-19, 11:23, Stephen Boyd wrote:
>> Yes, thanks. I see that largely follows the commit description so it
>> looks OK to me.
> Do you want to provide your Reviewed/Acked-by tag before I apply it ?

Nicolas wanted to send a V3 of this series and as a platform maintainer
i need some time for testing this version.

Stefan

Re: [PATCH v2 2/7] clk: bcm283x: add driver interfacing with Raspberry Pi's firmware

[Stefan Wahren]

Hi Nicolas,

Am 06.06.19 um 16:22 schrieb Nicolas Saenz Julienne:
> Raspberry Pi's firmware offers an interface though which update it's
> clock's frequencies. This is specially useful in order to change the CPU
> clock (pllb_arm) which is 'owned' by the firmware and we're unable to
> scale using the register interface provided by clk-bcm2835.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> Acked-by: Eric Anholt <eric@anholt.net>
>
> ---
>
> Changes since v1:
>   - Use BIT()
>   - Add Kconfig entry, with compile test
>   - remove prepare/unprepare
>   - Fix uninitialized init.name in pllb registration
>   - Add MODULE_ALIAS()
>   - Use determine_rate() instead of round_rate()
>   - Add small introduction explaining need for driver
>
>  drivers/clk/bcm/Kconfig           |   7 +
>  drivers/clk/bcm/Makefile          |   1 +
>  drivers/clk/bcm/clk-raspberrypi.c | 302 ++++++++++++++++++++++++++++++
>  3 files changed, 310 insertions(+)
>  create mode 100644 drivers/clk/bcm/clk-raspberrypi.c
>
> diff --git a/drivers/clk/bcm/Kconfig b/drivers/clk/bcm/Kconfig
> index 29ee7b776cd4..a4a2775d65e1 100644
> --- a/drivers/clk/bcm/Kconfig
> +++ b/drivers/clk/bcm/Kconfig
> @@ -64,3 +64,10 @@ config CLK_BCM_SR
>  	default ARCH_BCM_IPROC
>  	help
>  	  Enable common clock framework support for the Broadcom Stingray SoC
> +
> +config CLK_RASPBERRYPI
> +	tristate "Raspberry Pi firmware based clock support"
> +	depends on RASPBERRYPI_FIRMWARE || (COMPILE_TEST && !RASPBERRYPI_FIRMWARE)
> +	help
> +	  Enable common clock framework support for Raspberry Pi's firmware
> +	  dependent clocks
> diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile
> index 002661d39128..eb7159099d82 100644
> --- a/drivers/clk/bcm/Makefile
> +++ b/drivers/clk/bcm/Makefile
> @@ -7,6 +7,7 @@ obj-$(CONFIG_CLK_BCM_KONA)	+= clk-bcm21664.o
>  obj-$(CONFIG_COMMON_CLK_IPROC)	+= clk-iproc-armpll.o clk-iproc-pll.o clk-iproc-asiu.o
>  obj-$(CONFIG_ARCH_BCM2835)	+= clk-bcm2835.o
>  obj-$(CONFIG_ARCH_BCM2835)	+= clk-bcm2835-aux.o
> +obj-$(CONFIG_CLK_RASPBERRYPI)	+= clk-raspberrypi.o
>  obj-$(CONFIG_ARCH_BCM_53573)	+= clk-bcm53573-ilp.o
>  obj-$(CONFIG_CLK_BCM_CYGNUS)	+= clk-cygnus.o
>  obj-$(CONFIG_CLK_BCM_HR2)	+= clk-hr2.o

not your fault but you better rebase your next version on linux-next
because Florian's latest patches ("clk: bcm: Make BCM2835 clock drivers
selectable") collide with this patch.

Checkpatch gives the following output about this patch:

WARNING: 'harware' may be misspelled - perhaps 'hardware'?

#58: FILE: drivers/clk/bcm/clk-raspberrypi.c:5:
+ * Even though clk-bcm2835 provides an interface to the harware
registers for

ERROR: code indent should use tabs where possible
#197: FILE: drivers/clk/bcm/clk-raspberrypi.c:144:
+^I^I^I^I           struct clk_rate_request *req)$

Re: [PATCH v2 2/7] clk: bcm283x: add driver interfacing with Raspberry Pi's firmware

[Nicolas Saenz Julienne]

[-- Attachment #1: Type: text/plain, Size: 3188 bytes --]

On Fri, 2019-06-07 at 11:26 +0200, Stefan Wahren wrote:
> Hi Nicolas,
> 
> Am 06.06.19 um 16:22 schrieb Nicolas Saenz Julienne:
> > Raspberry Pi's firmware offers an interface though which update it's
> > clock's frequencies. This is specially useful in order to change the CPU
> > clock (pllb_arm) which is 'owned' by the firmware and we're unable to
> > scale using the register interface provided by clk-bcm2835.
> > 
> > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > Acked-by: Eric Anholt <eric@anholt.net>
> > 
> > ---
> > 
> > Changes since v1:
> >   - Use BIT()
> >   - Add Kconfig entry, with compile test
> >   - remove prepare/unprepare
> >   - Fix uninitialized init.name in pllb registration
> >   - Add MODULE_ALIAS()
> >   - Use determine_rate() instead of round_rate()
> >   - Add small introduction explaining need for driver
> > 
> >  drivers/clk/bcm/Kconfig           |   7 +
> >  drivers/clk/bcm/Makefile          |   1 +
> >  drivers/clk/bcm/clk-raspberrypi.c | 302 ++++++++++++++++++++++++++++++
> >  3 files changed, 310 insertions(+)
> >  create mode 100644 drivers/clk/bcm/clk-raspberrypi.c
> > 
> > diff --git a/drivers/clk/bcm/Kconfig b/drivers/clk/bcm/Kconfig
> > index 29ee7b776cd4..a4a2775d65e1 100644
> > --- a/drivers/clk/bcm/Kconfig
> > +++ b/drivers/clk/bcm/Kconfig
> > @@ -64,3 +64,10 @@ config CLK_BCM_SR
> >  	default ARCH_BCM_IPROC
> >  	help
> >  	  Enable common clock framework support for the Broadcom Stingray SoC
> > +
> > +config CLK_RASPBERRYPI
> > +	tristate "Raspberry Pi firmware based clock support"
> > +	depends on RASPBERRYPI_FIRMWARE || (COMPILE_TEST &&
> > !RASPBERRYPI_FIRMWARE)
> > +	help
> > +	  Enable common clock framework support for Raspberry Pi's firmware
> > +	  dependent clocks
> > diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile
> > index 002661d39128..eb7159099d82 100644
> > --- a/drivers/clk/bcm/Makefile
> > +++ b/drivers/clk/bcm/Makefile
> > @@ -7,6 +7,7 @@ obj-$(CONFIG_CLK_BCM_KONA)	+= clk-bcm21664.o
> >  obj-$(CONFIG_COMMON_CLK_IPROC)	+= clk-iproc-armpll.o clk-iproc-pll.o
> > clk-iproc-asiu.o
> >  obj-$(CONFIG_ARCH_BCM2835)	+= clk-bcm2835.o
> >  obj-$(CONFIG_ARCH_BCM2835)	+= clk-bcm2835-aux.o
> > +obj-$(CONFIG_CLK_RASPBERRYPI)	+= clk-raspberrypi.o
> >  obj-$(CONFIG_ARCH_BCM_53573)	+= clk-bcm53573-ilp.o
> >  obj-$(CONFIG_CLK_BCM_CYGNUS)	+= clk-cygnus.o
> >  obj-$(CONFIG_CLK_BCM_HR2)	+= clk-hr2.o
> 
> not your fault but you better rebase your next version on linux-next
> because Florian's latest patches ("clk: bcm: Make BCM2835 clock drivers
> selectable") collide with this patch.
> 
> Checkpatch gives the following output about this patch:
> 
> WARNING: 'harware' may be misspelled - perhaps 'hardware'?
> 
> #58: FILE: drivers/clk/bcm/clk-raspberrypi.c:5:
> + * Even though clk-bcm2835 provides an interface to the harware
> registers for
> 
> ERROR: code indent should use tabs where possible
> #197: FILE: drivers/clk/bcm/clk-raspberrypi.c:144:
> +^I^I^I^I           struct clk_rate_request *req)$

Noted, thanks.

As this seems fairly calm, should I send v3 or wait little more?


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 2/7] clk: bcm283x: add driver interfacing with Raspberry Pi's firmware

[Stefan Wahren]

Am 07.06.19 um 11:42 schrieb Nicolas Saenz Julienne:
> On Fri, 2019-06-07 at 11:26 +0200, Stefan Wahren wrote:
>> Hi Nicolas,
>>
>> Am 06.06.19 um 16:22 schrieb Nicolas Saenz Julienne:
>>> Raspberry Pi's firmware offers an interface though which update it's
>>> clock's frequencies. This is specially useful in order to change the CPU
>>> clock (pllb_arm) which is 'owned' by the firmware and we're unable to
>>> scale using the register interface provided by clk-bcm2835.
>>>
>>> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>>> Acked-by: Eric Anholt <eric@anholt.net>
>>>
>>> ---
>>>
>>> Changes since v1:
>>>   - Use BIT()
>>>   - Add Kconfig entry, with compile test
>>>   - remove prepare/unprepare
>>>   - Fix uninitialized init.name in pllb registration
>>>   - Add MODULE_ALIAS()
>>>   - Use determine_rate() instead of round_rate()
>>>   - Add small introduction explaining need for driver
>>>
>>>  drivers/clk/bcm/Kconfig           |   7 +
>>>  drivers/clk/bcm/Makefile          |   1 +
>>>  drivers/clk/bcm/clk-raspberrypi.c | 302 ++++++++++++++++++++++++++++++
>>>  3 files changed, 310 insertions(+)
>>>  create mode 100644 drivers/clk/bcm/clk-raspberrypi.c
>>>
>>> diff --git a/drivers/clk/bcm/Kconfig b/drivers/clk/bcm/Kconfig
>>> index 29ee7b776cd4..a4a2775d65e1 100644
>>> --- a/drivers/clk/bcm/Kconfig
>>> +++ b/drivers/clk/bcm/Kconfig
>>> @@ -64,3 +64,10 @@ config CLK_BCM_SR
>>>  	default ARCH_BCM_IPROC
>>>  	help
>>>  	  Enable common clock framework support for the Broadcom Stingray SoC
>>> +
>>> +config CLK_RASPBERRYPI
>>> +	tristate "Raspberry Pi firmware based clock support"
>>> +	depends on RASPBERRYPI_FIRMWARE || (COMPILE_TEST &&
>>> !RASPBERRYPI_FIRMWARE)
>>> +	help
>>> +	  Enable common clock framework support for Raspberry Pi's firmware
>>> +	  dependent clocks
>>> diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile
>>> index 002661d39128..eb7159099d82 100644
>>> --- a/drivers/clk/bcm/Makefile
>>> +++ b/drivers/clk/bcm/Makefile
>>> @@ -7,6 +7,7 @@ obj-$(CONFIG_CLK_BCM_KONA)	+= clk-bcm21664.o
>>>  obj-$(CONFIG_COMMON_CLK_IPROC)	+= clk-iproc-armpll.o clk-iproc-pll.o
>>> clk-iproc-asiu.o
>>>  obj-$(CONFIG_ARCH_BCM2835)	+= clk-bcm2835.o
>>>  obj-$(CONFIG_ARCH_BCM2835)	+= clk-bcm2835-aux.o
>>> +obj-$(CONFIG_CLK_RASPBERRYPI)	+= clk-raspberrypi.o
>>>  obj-$(CONFIG_ARCH_BCM_53573)	+= clk-bcm53573-ilp.o
>>>  obj-$(CONFIG_CLK_BCM_CYGNUS)	+= clk-cygnus.o
>>>  obj-$(CONFIG_CLK_BCM_HR2)	+= clk-hr2.o
>> not your fault but you better rebase your next version on linux-next
>> because Florian's latest patches ("clk: bcm: Make BCM2835 clock drivers
>> selectable") collide with this patch.
>>
>> Checkpatch gives the following output about this patch:
>>
>> WARNING: 'harware' may be misspelled - perhaps 'hardware'?
>>
>> #58: FILE: drivers/clk/bcm/clk-raspberrypi.c:5:
>> + * Even though clk-bcm2835 provides an interface to the harware
>> registers for
>>
>> ERROR: code indent should use tabs where possible
>> #197: FILE: drivers/clk/bcm/clk-raspberrypi.c:144:
>> +^I^I^I^I           struct clk_rate_request *req)$
> Noted, thanks.
>
> As this seems fairly calm, should I send v3 or wait little more?
>
Please wait until i'm finished with testing.

If i don't gave you a feedback until Tuesday, send out your next
version. Okay?

Re: [PATCH v2 7/7] arm64: defconfig: enable cpufreq support for RPi3

[Stefan Wahren]

Hi Nicolas,

Am 06.06.19 um 16:23 schrieb Nicolas Saenz Julienne:
> This enables both the new firmware clock driver and cpufreq driver
> available for the RPi3 family of boards.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> ---
>  arch/arm64/configs/defconfig | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index 4d583514258c..3b7baffb3087 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -82,6 +82,7 @@ CONFIG_CPUFREQ_DT=y
>  CONFIG_ACPI_CPPC_CPUFREQ=m
>  CONFIG_ARM_ARMADA_37XX_CPUFREQ=y
>  CONFIG_ARM_SCPI_CPUFREQ=y
> +CONFIG_ARM_RASPBERRYPI_CPUFREQ=y

the arm64 kernel tends to get very big, so i suggested to build it as a
kernel module.

Any reason why you choose to make it builtin?

>  CONFIG_ARM_TEGRA186_CPUFREQ=y
>  CONFIG_ARM_SCPI_PROTOCOL=y
>  CONFIG_RASPBERRYPI_FIRMWARE=y
> @@ -639,6 +640,7 @@ CONFIG_COMMON_CLK_CS2000_CP=y
>  CONFIG_COMMON_CLK_S2MPS11=y
>  CONFIG_CLK_QORIQ=y
>  CONFIG_COMMON_CLK_PWM=y
> +CONFIG_CLK_RASPBERRYPI=y
>  CONFIG_CLK_IMX8MQ=y
>  CONFIG_CLK_IMX8QXP=y
>  CONFIG_TI_SCI_CLK=y

Re: [PATCH v2 7/7] arm64: defconfig: enable cpufreq support for RPi3

[Nicolas Saenz Julienne]

[-- Attachment #1: Type: text/plain, Size: 1017 bytes --]

On Fri, 2019-06-07 at 12:19 +0200, Stefan Wahren wrote:
> Hi Nicolas,
> 
> Am 06.06.19 um 16:23 schrieb Nicolas Saenz Julienne:
> > This enables both the new firmware clock driver and cpufreq driver
> > available for the RPi3 family of boards.
> > 
> > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > ---
> >  arch/arm64/configs/defconfig | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> > index 4d583514258c..3b7baffb3087 100644
> > --- a/arch/arm64/configs/defconfig
> > +++ b/arch/arm64/configs/defconfig
> > @@ -82,6 +82,7 @@ CONFIG_CPUFREQ_DT=y
> >  CONFIG_ACPI_CPPC_CPUFREQ=m
> >  CONFIG_ARM_ARMADA_37XX_CPUFREQ=y
> >  CONFIG_ARM_SCPI_CPUFREQ=y
> > +CONFIG_ARM_RASPBERRYPI_CPUFREQ=y
> 
> the arm64 kernel tends to get very big, so i suggested to build it as a
> kernel module.
> 
> Any reason why you choose to make it builtin?

Not really, I missed your suggestion. I'll fix in v3.


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 6/7] ARM: defconfig: enable cpufreq driver for RPi

[Stefan Wahren]

Hi Nicolas,

Am 06.06.19 um 16:22 schrieb Nicolas Saenz Julienne:
> This enables on both multi_v7_defconfig and bcm2835_defconfig the new
> firmware based clock and cpufreq drivers for the Raspberry Pi platform.
>
> In the case of bcm2835_defconfig, as the cpufreq subsystem was disabled,
> the subsystem configuration was copied from multi_v7_defconfig (default
> governor, statistics, etc...).
sorry i didn't made any suggestions for this.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> ---
>  arch/arm/configs/bcm2835_defconfig  | 9 +++++++++
>  arch/arm/configs/multi_v7_defconfig | 2 ++
>  2 files changed, 11 insertions(+)
>
> diff --git a/arch/arm/configs/bcm2835_defconfig b/arch/arm/configs/bcm2835_defconfig
> index dcf7610cfe55..3fd90bfd5fec 100644
> --- a/arch/arm/configs/bcm2835_defconfig
> +++ b/arch/arm/configs/bcm2835_defconfig
> @@ -37,6 +37,14 @@ CONFIG_CMA=y
>  CONFIG_SECCOMP=y
>  CONFIG_KEXEC=y
>  CONFIG_CRASH_DUMP=y
> +CONFIG_CPU_FREQ=y
> +CONFIG_CPU_FREQ_STAT=y
> +CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND=y
From my understanding ondemand isn't the best fit for Raspberry Pi. I
would prefer conservative because of the delays through the mailbox
interface but it's not a really strong opinion.
> +CONFIG_CPU_FREQ_GOV_POWERSAVE=m
> +CONFIG_CPU_FREQ_GOV_USERSPACE=m
> +CONFIG_CPU_FREQ_GOV_CONSERVATIVE=m

Please make them builtin in this case.

After that you can have make Acked-by for this patch.

> +CONFIG_CPUFREQ_DT=y
> +CONFIG_ARM_RASPBERRYPI_CPUFREQ=y
>  CONFIG_VFP=y
>  # CONFIG_CORE_DUMP_DEFAULT_ELF_HEADERS is not set
>  # CONFIG_SUSPEND is not set
> @@ -132,6 +140,7 @@ CONFIG_DMA_BCM2835=y
>  CONFIG_STAGING=y
>  CONFIG_SND_BCM2835=m
>  CONFIG_VIDEO_BCM2835=m
> +CONFIG_CLK_RASPBERRYPI=y
>  CONFIG_MAILBOX=y
>  CONFIG_BCM2835_MBOX=y
>  # CONFIG_IOMMU_SUPPORT is not set
> diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
> index 6b748f214eae..0fd60a83f768 100644
> --- a/arch/arm/configs/multi_v7_defconfig
> +++ b/arch/arm/configs/multi_v7_defconfig
> @@ -102,6 +102,7 @@ CONFIG_CPU_FREQ_GOV_CONSERVATIVE=m
>  CONFIG_CPU_FREQ_GOV_SCHEDUTIL=y
>  CONFIG_CPUFREQ_DT=y
>  CONFIG_ARM_IMX6Q_CPUFREQ=y
> +CONFIG_ARM_RASPBERRYPI_CPUFREQ=y
>  CONFIG_QORIQ_CPUFREQ=y
>  CONFIG_CPU_IDLE=y
>  CONFIG_ARM_CPUIDLE=y
> @@ -899,6 +900,7 @@ CONFIG_STAGING_BOARD=y
>  CONFIG_COMMON_CLK_MAX77686=y
>  CONFIG_COMMON_CLK_RK808=m
>  CONFIG_COMMON_CLK_S2MPS11=m
> +CONFIG_CLK_RASPBERRYPI=y
>  CONFIG_COMMON_CLK_QCOM=y
>  CONFIG_QCOM_CLK_RPM=y
>  CONFIG_APQ_MMCC_8084=y

Re: [PATCH v2 4/7] cpufreq: add driver for Raspbery Pi

[Stefan Wahren]

Hi Nicolas,

Am 06.06.19 um 16:22 schrieb Nicolas Saenz Julienne:
> Raspberry Pi's firmware offers and interface though which update it's
> performance requirements. It allows us to request for specific runtime
> frequencies, which the firmware might or might not respect, depending on
> the firmware configuration and thermals.
>
> As the maximum and minimum frequencies are configurable in the firmware
> there is no way to know in advance their values. So the Raspberry Pi
> cpufreq driver queries them, builds an opp frequency table to then
> launch cpufreq-dt.
>
> Also, as the firmware interface might be configured as a module, making
> the cpu clock unavailable during init, this implements a full fledged
> driver, as opposed to most drivers registering cpufreq-dt, which only
> make use of an init routine.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> Acked-by: Eric Anholt <eric@anholt.net>
>
> ---
>
> Changes since v1:
>   - Remove compatible checks
>   - Add module support, now full fledged driver
>   - Use NULL in clk_get()
>
>  drivers/cpufreq/Kconfig.arm           |   8 +++
>  drivers/cpufreq/Makefile              |   1 +
>  drivers/cpufreq/raspberrypi-cpufreq.c | 100 ++++++++++++++++++++++++++
>  3 files changed, 109 insertions(+)
>  create mode 100644 drivers/cpufreq/raspberrypi-cpufreq.c
>
> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index f8129edc145e..5e9204d443ff 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -133,6 +133,14 @@ config ARM_QCOM_CPUFREQ_HW
>  	  The driver implements the cpufreq interface for this HW engine.
>  	  Say Y if you want to support CPUFreq HW.
>  
> +config ARM_RASPBERRYPI_CPUFREQ
> +	tristate "Raspberry Pi cpufreq support"
> +	depends on CLK_RASPBERRYPI || COMPILE_TEST
> +	help
> +	  This adds the CPUFreq driver for Raspberry Pi
> +
> +	  If in doubt, say N.
> +
>  config ARM_S3C_CPUFREQ
>  	bool
>  	help
> diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
> index 689b26c6f949..121c1acb66c0 100644
> --- a/drivers/cpufreq/Makefile
> +++ b/drivers/cpufreq/Makefile
> @@ -64,6 +64,7 @@ obj-$(CONFIG_ARM_PXA2xx_CPUFREQ)	+= pxa2xx-cpufreq.o
>  obj-$(CONFIG_PXA3xx)			+= pxa3xx-cpufreq.o
>  obj-$(CONFIG_ARM_QCOM_CPUFREQ_HW)	+= qcom-cpufreq-hw.o
>  obj-$(CONFIG_ARM_QCOM_CPUFREQ_KRYO)	+= qcom-cpufreq-kryo.o
> +obj-$(CONFIG_ARM_RASPBERRYPI_CPUFREQ) 	+= raspberrypi-cpufreq.o
>  obj-$(CONFIG_ARM_S3C2410_CPUFREQ)	+= s3c2410-cpufreq.o
>  obj-$(CONFIG_ARM_S3C2412_CPUFREQ)	+= s3c2412-cpufreq.o
>  obj-$(CONFIG_ARM_S3C2416_CPUFREQ)	+= s3c2416-cpufreq.o
> 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
> @@ -0,0 +1,100 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Raspberry Pi cpufreq driver
> + *
> + * Copyright (C) 2019, Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/cpu.h>
> +#include <linux/cpufreq.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_opp.h>
> +
> +static struct platform_device *cpufreq_dt;
> +
> +static int raspberrypi_cpufreq_probe(struct platform_device *pdev)
> +{
> +	struct device *cpu_dev;
> +	unsigned long min, max;
> +	unsigned long rate;
> +	struct clk *clk;
> +	int ret;
> +
> +	cpu_dev = get_cpu_device(0);
> +	if (!cpu_dev) {
> +		pr_err("Cannot get CPU for cpufreq driver\n");
> +		return -ENODEV;
> +	}
> +
> +	clk = clk_get(cpu_dev, NULL);
> +	if (IS_ERR(clk)) {
> +		dev_err(cpu_dev, "Cannot get clock for CPU0\n");
> +		return PTR_ERR(clk);
> +	}
> +
> +	/*
> +	 * The max and min frequencies are configurable in the Raspberry Pi
> +	 * firmware, so we query them at runtime
> +	 */
> +	min = clk_round_rate(clk, 0);
> +	max = clk_round_rate(clk, ULONG_MAX);
> +	clk_put(clk);
> +
> +	for (rate = min; rate < max; rate += 100000000) {
> +		ret = dev_pm_opp_add(cpu_dev, rate, 0);
> +		if (ret)
> +			goto remove_opp;
> +	}

i played a little bit with my Raspberry Pi Zero W and this series. Looks
fine so far.

Sorry for this nitpicking, but i expect user questions about the
differences between sysfs and vcgencmd measure_clock.

scaling_available_frequencies gives

699999 799999 899999 999999

but vcgencmd measure_clock return the rounded up values.

I know we shouldn't fake anything, but adding the OPPs rounded up may
avoid confusion.

Stefan

Re: [PATCH v2 4/7] cpufreq: add driver for Raspbery Pi

[Nicolas Saenz Julienne]

[-- Attachment #1: Type: text/plain, Size: 5044 bytes --]

On Fri, 2019-06-07 at 13:42 +0200, Stefan Wahren wrote:
> Hi Nicolas,
> 
> Am 06.06.19 um 16:22 schrieb Nicolas Saenz Julienne:
> > Raspberry Pi's firmware offers and interface though which update it's
> > performance requirements. It allows us to request for specific runtime
> > frequencies, which the firmware might or might not respect, depending on
> > the firmware configuration and thermals.
> > 
> > As the maximum and minimum frequencies are configurable in the firmware
> > there is no way to know in advance their values. So the Raspberry Pi
> > cpufreq driver queries them, builds an opp frequency table to then
> > launch cpufreq-dt.
> > 
> > Also, as the firmware interface might be configured as a module, making
> > the cpu clock unavailable during init, this implements a full fledged
> > driver, as opposed to most drivers registering cpufreq-dt, which only
> > make use of an init routine.
> > 
> > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > Acked-by: Eric Anholt <eric@anholt.net>
> > 
> > ---
> > 
> > Changes since v1:
> >   - Remove compatible checks
> >   - Add module support, now full fledged driver
> >   - Use NULL in clk_get()
> > 
> >  drivers/cpufreq/Kconfig.arm           |   8 +++
> >  drivers/cpufreq/Makefile              |   1 +
> >  drivers/cpufreq/raspberrypi-cpufreq.c | 100 ++++++++++++++++++++++++++
> >  3 files changed, 109 insertions(+)
> >  create mode 100644 drivers/cpufreq/raspberrypi-cpufreq.c
> > 
> > diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> > index f8129edc145e..5e9204d443ff 100644
> > --- a/drivers/cpufreq/Kconfig.arm
> > +++ b/drivers/cpufreq/Kconfig.arm
> > @@ -133,6 +133,14 @@ config ARM_QCOM_CPUFREQ_HW
> >  	  The driver implements the cpufreq interface for this HW engine.
> >  	  Say Y if you want to support CPUFreq HW.
> >  
> > +config ARM_RASPBERRYPI_CPUFREQ
> > +	tristate "Raspberry Pi cpufreq support"
> > +	depends on CLK_RASPBERRYPI || COMPILE_TEST
> > +	help
> > +	  This adds the CPUFreq driver for Raspberry Pi
> > +
> > +	  If in doubt, say N.
> > +
> >  config ARM_S3C_CPUFREQ
> >  	bool
> >  	help
> > diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
> > index 689b26c6f949..121c1acb66c0 100644
> > --- a/drivers/cpufreq/Makefile
> > +++ b/drivers/cpufreq/Makefile
> > @@ -64,6 +64,7 @@ obj-$(CONFIG_ARM_PXA2xx_CPUFREQ)	+= pxa2xx-cpufreq.o
> >  obj-$(CONFIG_PXA3xx)			+= pxa3xx-cpufreq.o
> >  obj-$(CONFIG_ARM_QCOM_CPUFREQ_HW)	+= qcom-cpufreq-hw.o
> >  obj-$(CONFIG_ARM_QCOM_CPUFREQ_KRYO)	+= qcom-cpufreq-kryo.o
> > +obj-$(CONFIG_ARM_RASPBERRYPI_CPUFREQ) 	+= raspberrypi-cpufreq.o
> >  obj-$(CONFIG_ARM_S3C2410_CPUFREQ)	+= s3c2410-cpufreq.o
> >  obj-$(CONFIG_ARM_S3C2412_CPUFREQ)	+= s3c2412-cpufreq.o
> >  obj-$(CONFIG_ARM_S3C2416_CPUFREQ)	+= s3c2416-cpufreq.o
> > 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
> > @@ -0,0 +1,100 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Raspberry Pi cpufreq driver
> > + *
> > + * Copyright (C) 2019, Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/cpu.h>
> > +#include <linux/cpufreq.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_opp.h>
> > +
> > +static struct platform_device *cpufreq_dt;
> > +
> > +static int raspberrypi_cpufreq_probe(struct platform_device *pdev)
> > +{
> > +	struct device *cpu_dev;
> > +	unsigned long min, max;
> > +	unsigned long rate;
> > +	struct clk *clk;
> > +	int ret;
> > +
> > +	cpu_dev = get_cpu_device(0);
> > +	if (!cpu_dev) {
> > +		pr_err("Cannot get CPU for cpufreq driver\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	clk = clk_get(cpu_dev, NULL);
> > +	if (IS_ERR(clk)) {
> > +		dev_err(cpu_dev, "Cannot get clock for CPU0\n");
> > +		return PTR_ERR(clk);
> > +	}
> > +
> > +	/*
> > +	 * The max and min frequencies are configurable in the Raspberry Pi
> > +	 * firmware, so we query them at runtime
> > +	 */
> > +	min = clk_round_rate(clk, 0);
> > +	max = clk_round_rate(clk, ULONG_MAX);
> > +	clk_put(clk);
> > +
> > +	for (rate = min; rate < max; rate += 100000000) {
> > +		ret = dev_pm_opp_add(cpu_dev, rate, 0);
> > +		if (ret)
> > +			goto remove_opp;
> > +	}
> 
> i played a little bit with my Raspberry Pi Zero W and this series. Looks
> fine so far.
> 
> Sorry for this nitpicking, but i expect user questions about the
> differences between sysfs and vcgencmd measure_clock.
> 
> scaling_available_frequencies gives
> 
> 699999 799999 899999 999999
> 
> but vcgencmd measure_clock return the rounded up values.
> 
> I know we shouldn't fake anything, but adding the OPPs rounded up may
> avoid confusion.
> 
> Stefan

Agree, I'll change this in v3.


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 4/7] cpufreq: add driver for Raspbery Pi

[Stephen Boyd]

Quoting Stefan Wahren (2019-06-07 02:13:54)
> Hi Viresh,
> 
> Am 07.06.19 um 05:09 schrieb Viresh Kumar:
> > On 06-06-19, 11:23, Stephen Boyd wrote:
> >> Yes, thanks. I see that largely follows the commit description so it
> >> looks OK to me.
> > Do you want to provide your Reviewed/Acked-by tag before I apply it ?
> 
> Nicolas wanted to send a V3 of this series and as a platform maintainer
> i need some time for testing this version.
> 

You can add my review tag.

Reviewed-by: Stephen Boyd <sboyd@kernel.org>

Re: [PATCH v2 0/7] cpufreq support for Raspberry Pi

[Stefan Wahren]

Am 06.06.19 um 16:22 schrieb Nicolas Saenz Julienne:
> Hi all,
> this aims at adding cpufreq support to the Raspberry Pi family of
> boards.
>
> The series first factors out 'pllb' from clk-bcm2385 and creates a new
> clk driver that operates it over RPi's firmware interface[1]. We are
> forced to do so as the firmware 'owns' the pll and we're not allowed to
> change through the register interface directly as we might race with the
> over-temperature and under-voltage protections provided by the firmware.
>
> Next it creates a minimal cpufreq driver that populates the CPU's opp
> table, and registers cpufreq-dt. Which is needed as the firmware
> controls the max and min frequencies available.

Here some figures from the Raspberry Pi 3 B+ as before/after comparison:

Dhrystone Benchmark 2.1 A7 32 Bit

 600 MHz, w/o Turbo (1): 1216.11 VAX MIPS
1400 MHz, w/o Turbo (2): 2839.67 VAX MIPS
1400 MHz, w   Turbo (3): 2839.45 VAX MIPS

Whetstone Single Precision C Benchmark  vfpv4 32 Bit

 600 MHz, w/o Turbo: 454.565 MWIPS
1400 MHz, w/o Turbo: 1062.494 MWIPS
1400 MHz, w   Turbo: 1061.723 MWIPS

Power consumption (32 bit, without Ethernet) with load ( cat /dev/zero )

 600 MHz, w/o Turbo: 2.48 W
1400 MHz, w/o Turbo: 3.2 W
1400 MHz, w   Turbo: 3.15 W

Note 1: This is the maximum performance before enabling any cpufreq driver.

Note 2: This is the maximum performance after enabling V2 of the cpufreq
driver
http://lists.infradead.org/pipermail/linux-arm-kernel/2019-June/657768.html

Note 3: This is the maximum performance after enabling the initial
cpufreq driver
http://lists.infradead.org/pipermail/linux-rpi-kernel/2019-April/008634.html