Re: [RESEND/PATCH v6 3/3] clk: qcom: Add A53 clock driver
From: Georgi Djakov
Date: Fri Nov 11 2016 - 12:26:45 EST
On 11/03/2016 08:28 PM, Bjorn Andersson wrote:
On Wed 02 Nov 15:55 PDT 2016, Stephen Boyd wrote:
On 11/02, Bjorn Andersson wrote:
On Thu 27 Oct 18:54 PDT 2016, Stephen Boyd wrote:
On 10/19, Georgi Djakov wrote:
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.
Signed-off-by: Georgi Djakov <georgi.djakov@xxxxxxxxxx>
---
.../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
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