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

[Bjorn Andersson]

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