From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Boyd Subject: Re: [RESEND/PATCH v6 3/3] clk: qcom: Add A53 clock driver Date: Mon, 14 Nov 2016 14:21:22 -0800 Message-ID: <20161114203426.GN5177@codeaurora.org> References: <20161019132816.31073-4-georgi.djakov@linaro.org> <20161028015438.GG16026@codeaurora.org> <20161102205910.GQ25787@tuxbot> <20161102225520.GW16026@codeaurora.org> <20161103182834.GR25787@tuxbot> <549f87fe-7be9-14b4-8e34-86f7f8dad94e@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <549f87fe-7be9-14b4-8e34-86f7f8dad94e-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Georgi Djakov Cc: Bjorn Andersson , mturquette-rdvid1DuHRBWk0Htik3J/w@public.gmane.org, linux-clk-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring List-Id: devicetree@vger.kernel.org On 11/11, Georgi Djakov wrote: > 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 > >>>>>--- > >>>>> .../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). > > I'd rather we did this kicker API as well. That way we don't need to make a syscon and a simple-mfd to get software to work properly. Don't other silicon vendors need a kicker API as well? How are they kicking remote processors in other places? GPIOs? -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html