linux-kernel - Re: [PATCH v9 7/7] clk: qcom: Add APCS clock controller support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20171026043919.GM1575@tuxbook>
Date:   Wed, 25 Oct 2017 21:39:19 -0700
From:   Bjorn Andersson <bjorn.andersson@...aro.org>
To:     Georgi Djakov <georgi.djakov@...aro.org>
Cc:     sboyd@...eaurora.org, jassisinghbrar@...il.com, robh+dt@...nel.org,
        mturquette@...libre.com, linux-clk@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        devicetree@...r.kernel.org
Subject: Re: [PATCH v9 7/7] clk: qcom: Add APCS clock controller support

On Thu 21 Sep 09:49 PDT 2017, Georgi Djakov wrote:

> Add a driver for the APCS clock controller. It is part of the APCS
> hardware block, which among other things implements also a combined
> mux and half integer divider functionality. It can choose between a
> fixed-rate clock or the dedicated APCS (A53) PLL. The source and the
> divider can be set both at the same time.
> 
> This is required for enabling CPU frequency scaling on MSM8916-based
> platforms.

As stated in the binding patch I think you should describe the "two"
parts in one node and probably add this code to the existing driver,
rather than spawning a new device.

[..]
> diff --git a/drivers/clk/qcom/apcs-msm8916.c b/drivers/clk/qcom/apcs-msm8916.c
[..]
> +#include "clk-regmap.h"
> +#include "clk-regmap-mux-div.h"

These two include files might cause some issues, but I would prefer that
you bake this code into the existing apcs driver.

[..]
> +static int __init qcom_apcs_msm8916_clk_init(void)
> +{
> +	return platform_driver_register(&qcom_apcs_msm8916_clk_driver);
> +}
> +core_initcall(qcom_apcs_msm8916_clk_init);

NB. The a53 clock is a builtin_platform_driver(), i.e. device_initcall()
the clock will never be available at core_initcall(), so the
devm_clk_get() should always hit a probe defer. Use
module_platform_driver() instead.

> +
> +static void __exit qcom_apcs_msm8916_clk_exit(void)
> +{
> +	platform_driver_unregister(&qcom_apcs_msm8916_clk_driver);
> +}
> +module_exit(qcom_apcs_msm8916_clk_exit);

Regards,
Bjorn