2015-07-20 19:33:27

by Eric Anholt

[permalink] [raw]
Subject: [PATCH v4 0/8] Raspberry Pi clock support

This is a respin of my rpi-dt-clocks series (also available at
https://github.com/anholt/linux/tree/rpi-dt-clocks). New in this
version is mentioning the clk subsystem for drivers/clk/* patches,
some better documentation of behavior in clk-raspberrypi.c, and a new
patch at the tail of the series for updating the EMMC clock.


2015-07-20 19:33:29

by Eric Anholt

[permalink] [raw]
Subject: [PATCH v4 1/8] clk: bcm2835: Add binding docs for the Raspberry Pi clock provider

The hardware clocks are not controllable by the ARM, so we have to
make requests to the firmware to do so from the VPU side. This will
let us replace fixed clocks in our DT with actual clock control (and
correct frequency information).

Signed-off-by: Eric Anholt <[email protected]>
Acked-by: Stephen Warren <[email protected]>
Acked-by: Lee Jones <[email protected]>
---

v2: Include the dt-bindings header in this commit instead of the next
one. Make the clock indices match the firmware clock IDs. Rename
the binding's compat string. Move the firmware phandle to be
under a vendor-specific namespace.
v4: Mention 'clk' in the subject instead of the more generic
dt/bindings.

.../clock/raspberrypi,bcm2835-firmware-clocks.txt | 25 ++++++++++++++++++++++
include/dt-bindings/clk/raspberrypi.h | 23 ++++++++++++++++++++
2 files changed, 48 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/raspberrypi,bcm2835-firmware-clocks.txt
create mode 100644 include/dt-bindings/clk/raspberrypi.h

diff --git a/Documentation/devicetree/bindings/clock/raspberrypi,bcm2835-firmware-clocks.txt b/Documentation/devicetree/bindings/clock/raspberrypi,bcm2835-firmware-clocks.txt
new file mode 100644
index 0000000..0972602
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/raspberrypi,bcm2835-firmware-clocks.txt
@@ -0,0 +1,25 @@
+Raspberry Pi firmware clock provider.
+
+The Raspberry Pi architecture doesn't provide direct access to the
+CLOCKMAN peripheral from the ARM side, so Linux has to make requests
+to the VPU firmware to program them.
+
+This binding uses the common clock binding:
+Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Required properties:
+- compatible: Should be "raspberrypi,bcm2835-firmware-clocks"
+
+- #clock-cells: Shall have value <1>. The permitted clock-specifier
+ values can be found in
+ include/dt-bindings/clk/raspberrypi.h.
+
+- raspberrypi,firmware: Phandle to the firmware driver node.
+
+Example:
+
+firmware_clocks: firmware-clocks {
+ compatible = "raspberrypi,bcm2835-firmware-clocks";
+ #clock-cells = <1>;
+ raspberrypi,firmware = <&firmware>;
+};
diff --git a/include/dt-bindings/clk/raspberrypi.h b/include/dt-bindings/clk/raspberrypi.h
new file mode 100644
index 0000000..ceec90f
--- /dev/null
+++ b/include/dt-bindings/clk/raspberrypi.h
@@ -0,0 +1,23 @@
+#/*
+ * Copyright © 2015 Broadcom
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _DT_BINDINGS_CLK_RASPBERRYPI_H
+#define _DT_BINDINGS_CLK_RASPBERRYPI_H
+
+#define RPI_CLOCK_EMMC 1
+#define RPI_CLOCK_UART0 2
+#define RPI_CLOCK_ARM 3
+#define RPI_CLOCK_CORE 4
+#define RPI_CLOCK_V3D 5
+#define RPI_CLOCK_H264 6
+#define RPI_CLOCK_ISP 7
+#define RPI_CLOCK_SDRAM 8
+#define RPI_CLOCK_PIXEL 9
+#define RPI_CLOCK_PWM 10
+
+#endif
--
2.1.4

2015-07-20 19:35:20

by Eric Anholt

[permalink] [raw]
Subject: [PATCH v4 2/8] clk: Add a Raspberry Pi-specific clock driver.

Unfortunately, the clock manager's registers are not accessible by the
ARM, so we have to request that the firmware modify our clocks for us.

This driver only registers the clocks at the point they are requested
by a client driver. This is partially to support returning
-EPROBE_DEFER when the firmware driver isn't supported yet, but it
also avoids issues with disabling "unused" clocks due to them not yet
being connected to their consumers in the DT.

Signed-off-by: Eric Anholt <[email protected]>
Acked-by: Stephen Warren <[email protected]>
---

v2: Declare the mutex static (from review by Baruch Siach), merge
description and copyright comments.

v3: Update for new rpi firmware API. Update the compatible string.
Make the firmware handle be under a vendor-namespaced property.
Make the DT indices match the firmware clock IDs. Move the driver
under the firmware driver's Kconfig, since it requires it. Move a
container_of() from 2 callers into the callee.

v4: Update commit message to mention subsystem. Add #defines for a
couple of bitfield checks, and use the bitfield check for
rpi_clk_is_on(). Return an error when appropriate in
rpi_clk_is_on(), and return specific rpi_firmware_property()
errors in other places when we were only returning -EINVAL. Add
some comment text describing how return values work from the
firmware.

drivers/clk/Makefile | 1 +
drivers/clk/clk-raspberrypi.c | 259 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 260 insertions(+)
create mode 100644 drivers/clk/clk-raspberrypi.c

diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index c4cf075..ad0a4a5 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_MACH_ASM9260) += clk-asm9260.o
obj-$(CONFIG_COMMON_CLK_AXI_CLKGEN) += clk-axi-clkgen.o
obj-$(CONFIG_ARCH_AXXIA) += clk-axm5516.o
obj-$(CONFIG_ARCH_BCM2835) += clk-bcm2835.o
+obj-$(CONFIG_RASPBERRYPI_FIRMWARE) += clk-raspberrypi.o
obj-$(CONFIG_COMMON_CLK_CDCE706) += clk-cdce706.o
obj-$(CONFIG_ARCH_CLPS711X) += clk-clps711x.o
obj-$(CONFIG_ARCH_EFM32) += clk-efm32gg.o
diff --git a/drivers/clk/clk-raspberrypi.c b/drivers/clk/clk-raspberrypi.c
new file mode 100644
index 0000000..ae35804
--- /dev/null
+++ b/drivers/clk/clk-raspberrypi.c
@@ -0,0 +1,259 @@
+/*
+ * Implements a clock provider for the clocks controlled by the
+ * firmware on Raspberry Pi.
+ *
+ * These clocks are controlled by the CLOCKMAN peripheral in the
+ * hardware, but the ARM doesn't have access to the registers for
+ * them. As a result, we have to call into the firmware to get it to
+ * enable, disable, and set their frequencies.
+ *
+ * We don't have an interface for getting the set of frequencies
+ * available from the hardware. We can request a min/max, but other
+ * than that we have to request a frequency and take what it gives us.
+ *
+ * Copyright © 2015 Broadcom
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <dt-bindings/clk/raspberrypi.h>
+#include <linux/clk-provider.h>
+#include <soc/bcm2835/raspberrypi-firmware.h>
+
+#define RPI_FIRMWARE_CLOCK_STATE_ENABLED (1 << 0)
+#define RPI_FIRMWARE_CLOCK_STATE_ERROR (1 << 1)
+#define RPI_FIRMWARE_SET_CLOCK_RATE_ERROR 0
+
+struct rpi_firmware_clock {
+ /* Clock definitions in our static struct. */
+ const char *name;
+
+ /* The rest are filled in at init time. */
+ struct clk_hw hw;
+ struct device *dev;
+ struct rpi_firmware *firmware;
+};
+
+static struct rpi_firmware_clock rpi_clocks[] = {
+ [RPI_CLOCK_EMMC] = { "emmc" },
+ [RPI_CLOCK_UART0] = { "uart0" },
+ [RPI_CLOCK_ARM] = { "arm" },
+ [RPI_CLOCK_CORE] = { "core" },
+ [RPI_CLOCK_V3D] = { "v3d" },
+ [RPI_CLOCK_H264] = { "h264" },
+ [RPI_CLOCK_ISP] = { "isp" },
+ [RPI_CLOCK_SDRAM] = { "sdram" },
+ [RPI_CLOCK_PIXEL] = { "pixel" },
+ [RPI_CLOCK_PWM] = { "pwm" },
+};
+
+static int rpi_clock_id(struct rpi_firmware_clock *rpi_clk)
+{
+ return rpi_clk - rpi_clocks;
+}
+
+static int rpi_clk_is_on(struct clk_hw *hw)
+{
+ struct rpi_firmware_clock *rpi_clk =
+ container_of(hw, struct rpi_firmware_clock, hw);
+ u32 packet[2];
+ int ret;
+
+ packet[0] = rpi_clock_id(rpi_clk);
+ packet[1] = 0;
+ ret = rpi_firmware_property(rpi_clk->firmware,
+ RPI_FIRMWARE_GET_CLOCK_STATE,
+ &packet, sizeof(packet));
+ /* The second packet field has the new clock state returned in
+ * the low bit, or an error flag in the second bit.
+ */
+ if (ret || (packet[1] & RPI_FIRMWARE_CLOCK_STATE_ERROR)) {
+ dev_err(rpi_clk->dev, "Failed to get clock state\n");
+ return ret ? ret : -EINVAL;
+ }
+
+ return packet[1] & RPI_FIRMWARE_CLOCK_STATE_ENABLED;
+}
+
+static int rpi_clk_set_state(struct clk_hw *hw, bool on)
+{
+ struct rpi_firmware_clock *rpi_clk =
+ container_of(hw, struct rpi_firmware_clock, hw);
+ u32 packet[2];
+ int ret;
+
+ packet[0] = rpi_clock_id(rpi_clk);
+ packet[1] = on;
+ ret = rpi_firmware_property(rpi_clk->firmware,
+ RPI_FIRMWARE_SET_CLOCK_STATE,
+ &packet, sizeof(packet));
+ /* The second packet field has the new clock state returned in
+ * the low bit, or an error flag in the second bit.
+ */
+ if (ret || (packet[1] & RPI_FIRMWARE_CLOCK_STATE_ERROR)) {
+ dev_err(rpi_clk->dev, "Failed to set clock state\n");
+ return ret ? ret : -EINVAL;
+ }
+
+ return 0;
+}
+
+static int rpi_clk_on(struct clk_hw *hw)
+{
+ return rpi_clk_set_state(hw, true);
+}
+
+static void rpi_clk_off(struct clk_hw *hw)
+{
+ rpi_clk_set_state(hw, false);
+}
+
+static unsigned long rpi_clk_get_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct rpi_firmware_clock *rpi_clk =
+ container_of(hw, struct rpi_firmware_clock, hw);
+ u32 packet[2];
+ int ret;
+
+ packet[0] = rpi_clock_id(rpi_clk);
+ packet[1] = 0;
+ ret = rpi_firmware_property(rpi_clk->firmware,
+ RPI_FIRMWARE_GET_CLOCK_RATE,
+ &packet, sizeof(packet));
+ /* Note that the second packet field returns 0 on an unknown
+ * clock error, which would also be a reasonable value for a
+ * clock that's off.
+ */
+ if (ret) {
+ dev_err(rpi_clk->dev, "Failed to get clock rate\n");
+ return 0;
+ }
+
+ return packet[1];
+}
+
+static int rpi_clk_set_rate(struct clk_hw *hw,
+ unsigned long rate, unsigned long parent_rate)
+{
+ struct rpi_firmware_clock *rpi_clk =
+ container_of(hw, struct rpi_firmware_clock, hw);
+ u32 packet[2];
+ int ret;
+
+ packet[0] = rpi_clock_id(rpi_clk);
+ packet[1] = rate;
+ ret = rpi_firmware_property(rpi_clk->firmware,
+ RPI_FIRMWARE_SET_CLOCK_RATE,
+ &packet, sizeof(packet));
+ /* The second packet field has the new clock rate returned, or
+ * 0 on error.
+ */
+ if (ret || packet[1] == RPI_FIRMWARE_SET_CLOCK_RATE_ERROR) {
+ dev_err(rpi_clk->dev, "Failed to set clock rate\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static long rpi_clk_round_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long *parent_rate)
+{
+ /*
+ * The firmware will end up rounding our rate to something,
+ * but we don't have an interface for it. Just return the
+ * requested value, and it'll get updated after the clock gets
+ * set.
+ */
+ return rate;
+}
+
+static struct clk_ops rpi_clk_ops = {
+ .is_prepared = rpi_clk_is_on,
+ .prepare = rpi_clk_on,
+ .unprepare = rpi_clk_off,
+ .recalc_rate = rpi_clk_get_rate,
+ .set_rate = rpi_clk_set_rate,
+ .round_rate = rpi_clk_round_rate,
+};
+
+static DEFINE_MUTEX(delayed_clock_init);
+static struct clk *rpi_firmware_delayed_get_clk(struct of_phandle_args *clkspec,
+ void *_data)
+{
+ struct device_node *of_node = _data;
+ struct platform_device *pdev = of_find_device_by_node(of_node);
+ struct device *dev = &pdev->dev;
+ struct device_node *firmware_node;
+ struct rpi_firmware *firmware;
+ struct clk_init_data init;
+ struct rpi_firmware_clock *rpi_clk;
+ struct clk *ret_clk;
+
+ if (clkspec->args_count != 1) {
+ dev_err(dev, "clock phandle should have 1 argument\n");
+ return ERR_PTR(-ENODEV);
+ }
+
+ if (clkspec->args[0] >= ARRAY_SIZE(rpi_clocks)) {
+ dev_err(dev, "clock phandle index %d out of range\n",
+ clkspec->args[0]);
+ return ERR_PTR(-ENODEV);
+ }
+
+ rpi_clk = &rpi_clocks[clkspec->args[0]];
+ if (!rpi_clk->name) {
+ dev_err(dev, "clock phandle index %d invalid\n",
+ clkspec->args[0]);
+ return ERR_PTR(-ENODEV);
+ }
+
+ firmware_node = of_parse_phandle(of_node, "raspberrypi,firmware", 0);
+ if (!firmware_node) {
+ dev_err(dev, "%s: Missing firmware node\n", rpi_clk->name);
+ return ERR_PTR(-ENODEV);
+ }
+ firmware = rpi_firmware_get(firmware_node);
+ if (!firmware)
+ return ERR_PTR(-EPROBE_DEFER);
+
+ mutex_lock(&delayed_clock_init);
+ if (rpi_clk->hw.clk) {
+ mutex_unlock(&delayed_clock_init);
+ return rpi_clk->hw.clk;
+ }
+ memset(&init, 0, sizeof(init));
+ init.ops = &rpi_clk_ops;
+
+ rpi_clk->firmware = firmware;
+ rpi_clk->dev = dev;
+ rpi_clk->hw.init = &init;
+ init.name = rpi_clk->name;
+ init.flags = CLK_IS_ROOT;
+
+ ret_clk = clk_register(dev, &rpi_clk->hw);
+ mutex_unlock(&delayed_clock_init);
+ if (!IS_ERR(ret_clk))
+ dev_info(dev, "registered %s clock\n", rpi_clk->name);
+ else {
+ dev_err(dev, "%s clock failed to init: %ld\n", rpi_clk->name,
+ PTR_ERR(ret_clk));
+ }
+ return ret_clk;
+}
+
+void __init rpi_firmware_init_clock_provider(struct device_node *node)
+{
+ /* We delay construction of our struct clks until get time,
+ * because we need to be able to return -EPROBE_DEFER if the
+ * firmware driver isn't up yet. clk core doesn't support
+ * re-probing on -EPROBE_DEFER, but callers of clk_get can.
+ */
+ of_clk_add_provider(node, rpi_firmware_delayed_get_clk, node);
+}
+
+CLK_OF_DECLARE(rpi_firmware_clocks, "raspberrypi,bcm2835-firmware-clocks",
+ rpi_firmware_init_clock_provider);
--
2.1.4

2015-07-20 19:33:26

by Eric Anholt

[permalink] [raw]
Subject: [PATCH v4 3/8] ARM: bcm2835: Add DT for the firmware clocks driver.

Signed-off-by: Eric Anholt <[email protected]>
Acked-by: Stephen Warren <[email protected]>
---

v2: Rename our compat string to mention bcm2835, and make our firmware
phandle be under a vendor-namespaced property.

arch/arm/boot/dts/bcm2835-rpi.dtsi | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/bcm2835-rpi.dtsi b/arch/arm/boot/dts/bcm2835-rpi.dtsi
index ab5474e..7cc47fb 100644
--- a/arch/arm/boot/dts/bcm2835-rpi.dtsi
+++ b/arch/arm/boot/dts/bcm2835-rpi.dtsi
@@ -20,6 +20,12 @@
compatible = "raspberrypi,bcm2835-firmware";
mboxes = <&mailbox>;
};
+
+ firmware_clocks: firmware-clocks {
+ compatible = "raspberrypi,bcm2835-firmware-clocks";
+ #clock-cells = <1>;
+ raspberrypi,firmware = <&firmware>;
+ };
};
};

--
2.1.4

2015-07-20 19:33:24

by Eric Anholt

[permalink] [raw]
Subject: [PATCH v4 4/8] ARM: bcm2835: Drop never-used clock-frequency property of uart0.

This appears to have been copy-and-paste from another serial driver's
DT. The driver has never used this value -- instead, the pl011 driver
is getting the fixed 20201000.uart clock from clk-bcm2835.c.

Signed-off-by: Eric Anholt <[email protected]>
Acked-by: Stephen Warren <[email protected]>
---
arch/arm/boot/dts/bcm2835.dtsi | 1 -
1 file changed, 1 deletion(-)

diff --git a/arch/arm/boot/dts/bcm2835.dtsi b/arch/arm/boot/dts/bcm2835.dtsi
index 301c73f..32f5830 100644
--- a/arch/arm/boot/dts/bcm2835.dtsi
+++ b/arch/arm/boot/dts/bcm2835.dtsi
@@ -96,7 +96,6 @@
compatible = "brcm,bcm2835-pl011", "arm,pl011", "arm,primecell";
reg = <0x7e201000 0x1000>;
interrupts = <2 25>;
- clock-frequency = <3000000>;
arm,primecell-periphid = <0x00241011>;
};

--
2.1.4

2015-07-20 19:35:18

by Eric Anholt

[permalink] [raw]
Subject: [PATCH v4 5/8] clk: bcm2835: Drop the fixed sys_pclk.

Nothing uses it, and I can't find any evidence that anything ever has.
Its role is now filled by the core clock in the firmware driver.

Signed-off-by: Eric Anholt <[email protected]>
Acked-by: Stephen Warren <[email protected]>
---
drivers/clk/clk-bcm2835.c | 5 -----
1 file changed, 5 deletions(-)

diff --git a/drivers/clk/clk-bcm2835.c b/drivers/clk/clk-bcm2835.c
index 6b950ca..dd295e4 100644
--- a/drivers/clk/clk-bcm2835.c
+++ b/drivers/clk/clk-bcm2835.c
@@ -32,11 +32,6 @@ void __init bcm2835_init_clocks(void)
struct clk *clk;
int ret;

- clk = clk_register_fixed_rate(NULL, "sys_pclk", NULL, CLK_IS_ROOT,
- 250000000);
- if (IS_ERR(clk))
- pr_err("sys_pclk not registered\n");
-
clk = clk_register_fixed_rate(NULL, "apb_pclk", NULL, CLK_IS_ROOT,
126000000);
if (IS_ERR(clk))
--
2.1.4

2015-07-20 19:34:10

by Eric Anholt

[permalink] [raw]
Subject: [PATCH v4 6/8] ARM: bcm2835: Use the RPi firmware clocks for uart.

This gets us a correct apb_pclk, which previously was accidentally
using the "20201000.uart" clock from clk-bcm2835.c, due to the
fallback clk_get_sys() path.

Signed-off-by: Eric Anholt <[email protected]>
Acked-by: Stephen Warren <[email protected]>
---
arch/arm/boot/dts/bcm2835-rpi.dtsi | 7 +++++++
arch/arm/boot/dts/bcm2835.dtsi | 2 +-
2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/bcm2835-rpi.dtsi b/arch/arm/boot/dts/bcm2835-rpi.dtsi
index 7cc47fb..9549eb8 100644
--- a/arch/arm/boot/dts/bcm2835-rpi.dtsi
+++ b/arch/arm/boot/dts/bcm2835-rpi.dtsi
@@ -1,3 +1,4 @@
+#include <dt-bindings/clk/raspberrypi.h>
#include "bcm2835.dtsi"

/ {
@@ -62,3 +63,9 @@
status = "okay";
bus-width = <4>;
};
+
+&uart0 {
+ clocks = <&firmware_clocks RPI_CLOCK_UART0>,
+ <&firmware_clocks RPI_CLOCK_CORE>;
+ clock-names = "uartclk", "apb_pclk";
+};
diff --git a/arch/arm/boot/dts/bcm2835.dtsi b/arch/arm/boot/dts/bcm2835.dtsi
index 32f5830..5be2862 100644
--- a/arch/arm/boot/dts/bcm2835.dtsi
+++ b/arch/arm/boot/dts/bcm2835.dtsi
@@ -92,7 +92,7 @@
#interrupt-cells = <2>;
};

- uart@7e201000 {
+ uart0: uart@7e201000 {
compatible = "brcm,bcm2835-pl011", "arm,pl011", "arm,primecell";
reg = <0x7e201000 0x1000>;
interrupts = <2 25>;
--
2.1.4

2015-07-20 19:34:55

by Eric Anholt

[permalink] [raw]
Subject: [PATCH v4 7/8] ARM: bcm2835: Tie SPI clock to the core clock rate.

We were previously using a fixed clock declared in the 2835 DT, but
it's actually the core clock, and it might not be the same if you had
adjusted it using the firmware's config.txt.

Signed-off-by: Eric Anholt <[email protected]>
Acked-by: Stephen Warren <[email protected]>
---
arch/arm/boot/dts/bcm2835-rpi.dtsi | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/bcm2835-rpi.dtsi b/arch/arm/boot/dts/bcm2835-rpi.dtsi
index 9549eb8..0a32123 100644
--- a/arch/arm/boot/dts/bcm2835-rpi.dtsi
+++ b/arch/arm/boot/dts/bcm2835-rpi.dtsi
@@ -69,3 +69,7 @@
<&firmware_clocks RPI_CLOCK_CORE>;
clock-names = "uartclk", "apb_pclk";
};
+
+&spi {
+ clocks = <&firmware_clocks RPI_CLOCK_CORE>;
+};
--
2.1.4

2015-07-20 19:35:16

by Eric Anholt

[permalink] [raw]
Subject: [PATCH v4 8/8] ARM: bcm2835: Use the firmware on rpi to get the EMMC clock.

This updates the MMC frequency from 100Mhz to 250Mhz.

Signed-off-by: Eric Anholt <[email protected]>
---
arch/arm/boot/dts/bcm2835-rpi.dtsi | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/bcm2835-rpi.dtsi b/arch/arm/boot/dts/bcm2835-rpi.dtsi
index 0a32123..5d370cb 100644
--- a/arch/arm/boot/dts/bcm2835-rpi.dtsi
+++ b/arch/arm/boot/dts/bcm2835-rpi.dtsi
@@ -62,6 +62,7 @@
&sdhci {
status = "okay";
bus-width = <4>;
+ clocks = <&firmware_clocks RPI_CLOCK_EMMC>;
};

&uart0 {
--
2.1.4

2015-07-24 15:22:10

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v4 6/8] ARM: bcm2835: Use the RPi firmware clocks for uart.

On Mon, 20 Jul 2015, Eric Anholt wrote:

> This gets us a correct apb_pclk, which previously was accidentally
> using the "20201000.uart" clock from clk-bcm2835.c, due to the
> fallback clk_get_sys() path.
>
> Signed-off-by: Eric Anholt <[email protected]>
> Acked-by: Stephen Warren <[email protected]>
> ---
> arch/arm/boot/dts/bcm2835-rpi.dtsi | 7 +++++++
> arch/arm/boot/dts/bcm2835.dtsi | 2 +-
> 2 files changed, 8 insertions(+), 1 deletion(-)

For my own reference:
Acked-by: Lee Jones <[email protected]>

> diff --git a/arch/arm/boot/dts/bcm2835-rpi.dtsi b/arch/arm/boot/dts/bcm2835-rpi.dtsi
> index 7cc47fb..9549eb8 100644
> --- a/arch/arm/boot/dts/bcm2835-rpi.dtsi
> +++ b/arch/arm/boot/dts/bcm2835-rpi.dtsi
> @@ -1,3 +1,4 @@
> +#include <dt-bindings/clk/raspberrypi.h>
> #include "bcm2835.dtsi"
>
> / {
> @@ -62,3 +63,9 @@
> status = "okay";
> bus-width = <4>;
> };
> +
> +&uart0 {
> + clocks = <&firmware_clocks RPI_CLOCK_UART0>,
> + <&firmware_clocks RPI_CLOCK_CORE>;
> + clock-names = "uartclk", "apb_pclk";
> +};
> diff --git a/arch/arm/boot/dts/bcm2835.dtsi b/arch/arm/boot/dts/bcm2835.dtsi
> index 32f5830..5be2862 100644
> --- a/arch/arm/boot/dts/bcm2835.dtsi
> +++ b/arch/arm/boot/dts/bcm2835.dtsi
> @@ -92,7 +92,7 @@
> #interrupt-cells = <2>;
> };
>
> - uart@7e201000 {
> + uart0: uart@7e201000 {
> compatible = "brcm,bcm2835-pl011", "arm,pl011", "arm,primecell";
> reg = <0x7e201000 0x1000>;
> interrupts = <2 25>;

2015-07-24 15:22:28

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v4 8/8] ARM: bcm2835: Use the firmware on rpi to get the EMMC clock.

On Mon, 20 Jul 2015, Eric Anholt wrote:

> This updates the MMC frequency from 100Mhz to 250Mhz.
>
> Signed-off-by: Eric Anholt <[email protected]>
> ---
> arch/arm/boot/dts/bcm2835-rpi.dtsi | 1 +
> 1 file changed, 1 insertion(+)

For my own reference:
Acked-by: Lee Jones <[email protected]>

> diff --git a/arch/arm/boot/dts/bcm2835-rpi.dtsi b/arch/arm/boot/dts/bcm2835-rpi.dtsi
> index 0a32123..5d370cb 100644
> --- a/arch/arm/boot/dts/bcm2835-rpi.dtsi
> +++ b/arch/arm/boot/dts/bcm2835-rpi.dtsi
> @@ -62,6 +62,7 @@
> &sdhci {
> status = "okay";
> bus-width = <4>;
> + clocks = <&firmware_clocks RPI_CLOCK_EMMC>;
> };
>
> &uart0 {

2015-07-24 15:22:56

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v4 4/8] ARM: bcm2835: Drop never-used clock-frequency property of uart0.

On Mon, 20 Jul 2015, Eric Anholt wrote:

> This appears to have been copy-and-paste from another serial driver's
> DT. The driver has never used this value -- instead, the pl011 driver
> is getting the fixed 20201000.uart clock from clk-bcm2835.c.
>
> Signed-off-by: Eric Anholt <[email protected]>
> Acked-by: Stephen Warren <[email protected]>
> ---
> arch/arm/boot/dts/bcm2835.dtsi | 1 -
> 1 file changed, 1 deletion(-)

For my own reference:
Acked-by: Lee Jones <[email protected]>

> diff --git a/arch/arm/boot/dts/bcm2835.dtsi b/arch/arm/boot/dts/bcm2835.dtsi
> index 301c73f..32f5830 100644
> --- a/arch/arm/boot/dts/bcm2835.dtsi
> +++ b/arch/arm/boot/dts/bcm2835.dtsi
> @@ -96,7 +96,6 @@
> compatible = "brcm,bcm2835-pl011", "arm,pl011", "arm,primecell";
> reg = <0x7e201000 0x1000>;
> interrupts = <2 25>;
> - clock-frequency = <3000000>;
> arm,primecell-periphid = <0x00241011>;
> };
>

2015-07-24 15:23:25

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v4 3/8] ARM: bcm2835: Add DT for the firmware clocks driver.

On Mon, 20 Jul 2015, Eric Anholt wrote:

> Signed-off-by: Eric Anholt <[email protected]>
> Acked-by: Stephen Warren <[email protected]>
> ---
>
> v2: Rename our compat string to mention bcm2835, and make our firmware
> phandle be under a vendor-namespaced property.
>
> arch/arm/boot/dts/bcm2835-rpi.dtsi | 6 ++++++
> 1 file changed, 6 insertions(+)

For my own reference:
Acked-by: Lee Jones <[email protected]>

> diff --git a/arch/arm/boot/dts/bcm2835-rpi.dtsi b/arch/arm/boot/dts/bcm2835-rpi.dtsi
> index ab5474e..7cc47fb 100644
> --- a/arch/arm/boot/dts/bcm2835-rpi.dtsi
> +++ b/arch/arm/boot/dts/bcm2835-rpi.dtsi
> @@ -20,6 +20,12 @@
> compatible = "raspberrypi,bcm2835-firmware";
> mboxes = <&mailbox>;
> };
> +
> + firmware_clocks: firmware-clocks {
> + compatible = "raspberrypi,bcm2835-firmware-clocks";
> + #clock-cells = <1>;
> + raspberrypi,firmware = <&firmware>;
> + };
> };
> };
>

2015-07-24 15:23:44

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v4 7/8] ARM: bcm2835: Tie SPI clock to the core clock rate.

On Mon, 20 Jul 2015, Eric Anholt wrote:

> We were previously using a fixed clock declared in the 2835 DT, but
> it's actually the core clock, and it might not be the same if you had
> adjusted it using the firmware's config.txt.
>
> Signed-off-by: Eric Anholt <[email protected]>
> Acked-by: Stephen Warren <[email protected]>
> ---
> arch/arm/boot/dts/bcm2835-rpi.dtsi | 4 ++++
> 1 file changed, 4 insertions(+)

For my own reference:
Acked-by: Lee Jones <[email protected]>

> diff --git a/arch/arm/boot/dts/bcm2835-rpi.dtsi b/arch/arm/boot/dts/bcm2835-rpi.dtsi
> index 9549eb8..0a32123 100644
> --- a/arch/arm/boot/dts/bcm2835-rpi.dtsi
> +++ b/arch/arm/boot/dts/bcm2835-rpi.dtsi
> @@ -69,3 +69,7 @@
> <&firmware_clocks RPI_CLOCK_CORE>;
> clock-names = "uartclk", "apb_pclk";
> };
> +
> +&spi {
> + clocks = <&firmware_clocks RPI_CLOCK_CORE>;
> +};

2015-08-11 21:17:49

by Mike Turquette

[permalink] [raw]
Subject: Re: [PATCH v4 2/8] clk: Add a Raspberry Pi-specific clock driver.

Hi Eric,

Quoting Eric Anholt (2015-07-20 12:33:01)
> +void __init rpi_firmware_init_clock_provider(struct device_node *node)
> +{
> + /* We delay construction of our struct clks until get time,
> + * because we need to be able to return -EPROBE_DEFER if the
> + * firmware driver isn't up yet. clk core doesn't support
> + * re-probing on -EPROBE_DEFER, but callers of clk_get can.
> + */
> + of_clk_add_provider(node, rpi_firmware_delayed_get_clk, node);
> +}
> +
> +CLK_OF_DECLARE(rpi_firmware_clocks, "raspberrypi,bcm2835-firmware-clocks",
> + rpi_firmware_init_clock_provider);

Do you require CLK_OF_DECLARE here? Could this be a platform driver
instead?

Regards,
Mike

> --
> 2.1.4
>

2015-08-11 21:18:56

by Mike Turquette

[permalink] [raw]
Subject: Re: [PATCH v4 1/8] clk: bcm2835: Add binding docs for the Raspberry Pi clock provider

Quoting Eric Anholt (2015-07-20 12:33:00)
> The hardware clocks are not controllable by the ARM, so we have to
> make requests to the firmware to do so from the VPU side. This will
> let us replace fixed clocks in our DT with actual clock control (and
> correct frequency information).
>
> Signed-off-by: Eric Anholt <[email protected]>
> Acked-by: Stephen Warren <[email protected]>
> Acked-by: Lee Jones <[email protected]>

Looks good to me. Do you want the clk patches to go through the clk tree
or do you want them all together? For this patch feel free to add:

Acked-by: Michael Turquette <[email protected]>

Regards,
Mike

> ---
>
> v2: Include the dt-bindings header in this commit instead of the next
> one. Make the clock indices match the firmware clock IDs. Rename
> the binding's compat string. Move the firmware phandle to be
> under a vendor-specific namespace.
> v4: Mention 'clk' in the subject instead of the more generic
> dt/bindings.
>
> .../clock/raspberrypi,bcm2835-firmware-clocks.txt | 25 ++++++++++++++++++++++
> include/dt-bindings/clk/raspberrypi.h | 23 ++++++++++++++++++++
> 2 files changed, 48 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/raspberrypi,bcm2835-firmware-clocks.txt
> create mode 100644 include/dt-bindings/clk/raspberrypi.h
>
> diff --git a/Documentation/devicetree/bindings/clock/raspberrypi,bcm2835-firmware-clocks.txt b/Documentation/devicetree/bindings/clock/raspberrypi,bcm2835-firmware-clocks.txt
> new file mode 100644
> index 0000000..0972602
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/raspberrypi,bcm2835-firmware-clocks.txt
> @@ -0,0 +1,25 @@
> +Raspberry Pi firmware clock provider.
> +
> +The Raspberry Pi architecture doesn't provide direct access to the
> +CLOCKMAN peripheral from the ARM side, so Linux has to make requests
> +to the VPU firmware to program them.
> +
> +This binding uses the common clock binding:
> +Documentation/devicetree/bindings/clock/clock-bindings.txt
> +
> +Required properties:
> +- compatible: Should be "raspberrypi,bcm2835-firmware-clocks"
> +
> +- #clock-cells: Shall have value <1>. The permitted clock-specifier
> + values can be found in
> + include/dt-bindings/clk/raspberrypi.h.
> +
> +- raspberrypi,firmware: Phandle to the firmware driver node.
> +
> +Example:
> +
> +firmware_clocks: firmware-clocks {
> + compatible = "raspberrypi,bcm2835-firmware-clocks";
> + #clock-cells = <1>;
> + raspberrypi,firmware = <&firmware>;
> +};
> diff --git a/include/dt-bindings/clk/raspberrypi.h b/include/dt-bindings/clk/raspberrypi.h
> new file mode 100644
> index 0000000..ceec90f
> --- /dev/null
> +++ b/include/dt-bindings/clk/raspberrypi.h
> @@ -0,0 +1,23 @@
> +#/*
> + * Copyright �� 2015 Broadcom
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef _DT_BINDINGS_CLK_RASPBERRYPI_H
> +#define _DT_BINDINGS_CLK_RASPBERRYPI_H
> +
> +#define RPI_CLOCK_EMMC 1
> +#define RPI_CLOCK_UART0 2
> +#define RPI_CLOCK_ARM 3
> +#define RPI_CLOCK_CORE 4
> +#define RPI_CLOCK_V3D 5
> +#define RPI_CLOCK_H264 6
> +#define RPI_CLOCK_ISP 7
> +#define RPI_CLOCK_SDRAM 8
> +#define RPI_CLOCK_PIXEL 9
> +#define RPI_CLOCK_PWM 10
> +
> +#endif
> --
> 2.1.4
>

2015-08-11 21:20:03

by Mike Turquette

[permalink] [raw]
Subject: Re: [PATCH v4 5/8] clk: bcm2835: Drop the fixed sys_pclk.

Quoting Eric Anholt (2015-07-20 12:33:04)
> Nothing uses it, and I can't find any evidence that anything ever has.
> Its role is now filled by the core clock in the firmware driver.
>
> Signed-off-by: Eric Anholt <[email protected]>
> Acked-by: Stephen Warren <[email protected]>

Applied to clk-next.

Regards,
Mike

> ---
> drivers/clk/clk-bcm2835.c | 5 -----
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/clk/clk-bcm2835.c b/drivers/clk/clk-bcm2835.c
> index 6b950ca..dd295e4 100644
> --- a/drivers/clk/clk-bcm2835.c
> +++ b/drivers/clk/clk-bcm2835.c
> @@ -32,11 +32,6 @@ void __init bcm2835_init_clocks(void)
> struct clk *clk;
> int ret;
>
> - clk = clk_register_fixed_rate(NULL, "sys_pclk", NULL, CLK_IS_ROOT,
> - 250000000);
> - if (IS_ERR(clk))
> - pr_err("sys_pclk not registered\n");
> -
> clk = clk_register_fixed_rate(NULL, "apb_pclk", NULL, CLK_IS_ROOT,
> 126000000);
> if (IS_ERR(clk))
> --
> 2.1.4
>

2015-08-12 17:57:07

by Eric Anholt

[permalink] [raw]
Subject: Re: [PATCH v4 1/8] clk: bcm2835: Add binding docs for the Raspberry Pi clock provider

Michael Turquette <[email protected]> writes:

> Quoting Eric Anholt (2015-07-20 12:33:00)
>> The hardware clocks are not controllable by the ARM, so we have to
>> make requests to the firmware to do so from the VPU side. This will
>> let us replace fixed clocks in our DT with actual clock control (and
>> correct frequency information).
>>
>> Signed-off-by: Eric Anholt <[email protected]>
>> Acked-by: Stephen Warren <[email protected]>
>> Acked-by: Lee Jones <[email protected]>
>
> Looks good to me. Do you want the clk patches to go through the clk tree
> or do you want them all together? For this patch feel free to add:
>
> Acked-by: Michael Turquette <[email protected]>

It would be nice to do them all together in the arm-soc tree, since the
later DT changes require that the clock driver be present. (And, since
this is a dependency of the RPi2 work, it would let us stack that on
top, too).


Attachments:
signature.asc (818.00 B)

2015-08-12 18:04:10

by Eric Anholt

[permalink] [raw]
Subject: Re: [PATCH v4 2/8] clk: Add a Raspberry Pi-specific clock driver.

Michael Turquette <[email protected]> writes:

> Hi Eric,
>
> Quoting Eric Anholt (2015-07-20 12:33:01)
>> +void __init rpi_firmware_init_clock_provider(struct device_node *node)
>> +{
>> + /* We delay construction of our struct clks until get time,
>> + * because we need to be able to return -EPROBE_DEFER if the
>> + * firmware driver isn't up yet. clk core doesn't support
>> + * re-probing on -EPROBE_DEFER, but callers of clk_get can.
>> + */
>> + of_clk_add_provider(node, rpi_firmware_delayed_get_clk, node);
>> +}
>> +
>> +CLK_OF_DECLARE(rpi_firmware_clocks, "raspberrypi,bcm2835-firmware-clocks",
>> + rpi_firmware_init_clock_provider);
>
> Do you require CLK_OF_DECLARE here? Could this be a platform driver
> instead?

I'm not actually sure. The common pattern seemed to be using
CLK_OF_DECLARE (130 files using it versus declaring a struct
platform_driver), and it seems to avoid a whole lot of boilerplate.
What would the advantage be?


Attachments:
signature.asc (818.00 B)

2015-08-12 19:00:30

by Michael Turquette

[permalink] [raw]
Subject: Re: [PATCH v4 2/8] clk: Add a Raspberry Pi-specific clock driver.

Quoting Eric Anholt (2015-08-12 11:04:05)
> Michael Turquette <[email protected]> writes:
>
> > Hi Eric,
> >
> > Quoting Eric Anholt (2015-07-20 12:33:01)
> >> +void __init rpi_firmware_init_clock_provider(struct device_node *node)
> >> +{
> >> + /* We delay construction of our struct clks until get time,
> >> + * because we need to be able to return -EPROBE_DEFER if the
> >> + * firmware driver isn't up yet. clk core doesn't support
> >> + * re-probing on -EPROBE_DEFER, but callers of clk_get can.
> >> + */
> >> + of_clk_add_provider(node, rpi_firmware_delayed_get_clk, node);
> >> +}
> >> +
> >> +CLK_OF_DECLARE(rpi_firmware_clocks, "raspberrypi,bcm2835-firmware-clocks",
> >> + rpi_firmware_init_clock_provider);
> >
> > Do you require CLK_OF_DECLARE here? Could this be a platform driver
> > instead?
>
> I'm not actually sure. The common pattern seemed to be using
> CLK_OF_DECLARE (130 files using it versus declaring a struct
> platform_driver), and it seems to avoid a whole lot of boilerplate.
> What would the advantage be?

Correctly using the Linux driver model, having a nice struct device and
things like supsend and resume handlers, etc.

There is a lot of CLK_OF_DECLARE going on in drivers/clk, but I am
hoping we can fix that. The main reason for CLK_OF_DECLARE is when you
need to register clocks very early (e.g. timers). This is often not the
case.

Regards,
Mike