lkml.org 
[lkml]   [2016]   [Nov]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RESEND/PATCH v6 3/3] clk: qcom: Add A53 clock driver
From
Date
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
>

\
 
 \ /
  Last update: 2016-11-11 18:27    [W:0.087 / U:0.272 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site