Thread (9 messages) 9 messages, 3 authors, 2016-12-06

Re: [RESEND/PATCH v6 3/3] clk: qcom: Add A53 clock driver

From: Georgi Djakov <hidden>
Date: 2016-11-11 17:26:42
Also in: linux-arm-msm, linux-clk, lkml

On 11/03/2016 08:28 PM, Bjorn Andersson wrote:
On Wed 02 Nov 15:55 PDT 2016, Stephen Boyd wrote:
quoted
On 11/02, Bjorn Andersson wrote:
quoted
On Thu 27 Oct 18:54 PDT 2016, Stephen Boyd wrote:
quoted
On 10/19, Georgi Djakov wrote:
quoted
Add a driver for the A53 Clock Controller. It is a hardware block that
implements a combined mux and half integer divider functionality. It can
choose between a fixed-rate clock or the dedicated A53 PLL. The source
and the divider can be set both at the same time.

This is required for enabling CPU frequency scaling on platforms like
MSM8916.
Please Cc DT reviewers.
quoted
Signed-off-by: Georgi Djakov <redacted>
---
 .../devicetree/bindings/clock/qcom,a53cc.txt       |  22 +++
 drivers/clk/qcom/Kconfig                           |   8 ++
 drivers/clk/qcom/Makefile                          |   1 +
 drivers/clk/qcom/a53cc.c                           | 155 +++++++++++++++++++++
 4 files changed, 186 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,a53cc.txt
 create mode 100644 drivers/clk/qcom/a53cc.c
diff --git a/Documentation/devicetree/bindings/clock/qcom,a53cc.txt b/Documentation/devicetree/bindings/clock/qcom,a53cc.txt
new file mode 100644
index 000000000000..a025f062f177
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/qcom,a53cc.txt
@@ -0,0 +1,22 @@
+Qualcomm A53 CPU Clock Controller Binding
+------------------------------------------------
+The A53 CPU Clock Controller is hardware, which provides a combined
+mux and divider functionality for the CPU clocks. It can choose between
+a fixed rate clock and the dedicated A53 PLL.
+
+Required properties :
+- compatible : shall contain:
+
+			"qcom,a53cc"
+
+- reg : shall contain base register location and length
+	of the APCS region
+- #clock-cells : shall contain 1
+
+Example:
+
+	apcs: syscon@b011000 {
+		compatible = "qcom,a53cc", "syscon";
Why is it a syscon? Is that part used?
I use the register at offset 8 for interrupting the other subsystems, so
this must be available as something I can poke.

Which makes me think that this should be described as a "simple-mfd" and
"syscon" with the a53cc node as a child - grabbing the regmap of the
syscon parent, rather then ioremapping the same region again.
That's sort of a question for DT reviewers. The register space
certainly seems like a free for all with a tilt toward power
management of the CPU, similar to how this was done on Krait
based designs.
Right. But this kind of mashup blocks was the reason why simple-mfd was
put in place.
Ok, thanks for the comments. Then i will make it look like this:

	apcs: syscon@b011000 {
		compatible = "syscon", "simple-mfd";
		reg = <0x0b011000 0x1000>;

		a53mux: clock {
			compatible = "qcom,msm8916-a53cc";
			#clock-cells = <1>;
		};
	};

Thanks,
Georgi
quoted
I wonder why we didn't make up some provider/consumer binding for
the "kicking" feature used by SMD/RPM code. Then this could be a
clock provider and a "kick" provider (haha #kick-cells) and the
usage of syscon/regmap wouldn't be mandatory.
I did consider doing that, but had enough dependencies to put in place
as it was.

I'm in favour of us inventing a kicker API and it's found outside out
use cases as well (e.g. virtio/rpmsg).

Regards,
Bjorn
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help