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.cdiff --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