Messages in this thread | | | Subject | Re: [RESEND/PATCH v6 3/3] clk: qcom: Add A53 clock driver | From | Georgi Djakov <> | Date | Fri, 11 Nov 2016 19:26:33 +0200 |
| |
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@linaro.org> >>>>> --- >>>>> .../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 >
| |