diff mbox

[v8,3/3] mailbox: qcom: Add support for APCS clock controller

Message ID 20170623161533.20449-4-georgi.djakov@linaro.org
State Not Applicable, archived
Headers show

Commit Message

Georgi Djakov June 23, 2017, 4:15 p.m. UTC 
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.

Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
---
 .../bindings/mailbox/qcom,apcs-kpss-global.txt     |   5 +
 drivers/mailbox/qcom-apcs-ipc-mailbox.c            | 122 +++++++++++++++++++++
 2 files changed, 127 insertions(+)

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Bjorn Andersson June 23, 2017, 5:45 p.m. UTC | #1 
On Fri 23 Jun 09:15 PDT 2017, Georgi Djakov wrote:

> +static int msm8916_register_clk(struct device *dev, void __iomem *base)
> +{
[..]
> +	regmap = devm_regmap_init_mmio(dev, base, &a53cc_regmap_config);
> +	if (IS_ERR(regmap)) {
> +		ret = PTR_ERR(regmap);
> +		dev_err(dev, "failed to init regmap mmio: %d\n", ret);
> +		goto err;
> +	}

I think it would be cleaner if you create the regmap in probe() and we
use that throughout the driver - rather than using two different access
mechanism.

> +
> +	a53cc->clkr.regmap = regmap;
> +
> +	ret = devm_clk_register_regmap(dev, &a53cc->clkr);
> +	if (ret) {
> +		dev_err(dev, "failed to register regmap clock: %d\n", ret);
> +		goto err;
> +	}
> +
> +	ret = of_clk_add_hw_provider(dev->of_node, of_clk_hw_simple_get,
> +				     &a53cc->clkr.hw);
> +	if (ret) {
> +		dev_err(dev, "failed to add clock provider: %d\n", ret);
> +		goto err;
> +	}
> +
> +	return 0;
> +
> +err:
> +	clk_notifier_unregister(pclk, &a53cc->clk_nb);
> +	return ret;
> +}
> +
>  static int qcom_apcs_ipc_probe(struct platform_device *pdev)
>  {
> +	struct device_node *np = pdev->dev.of_node;
>  	struct qcom_apcs_ipc *apcs;
>  	struct resource *res;
>  	unsigned long offset;
> @@ -63,6 +178,13 @@ static int qcom_apcs_ipc_probe(struct platform_device *pdev)
>  	if (IS_ERR(base))
>  		return PTR_ERR(base);
>  
> +	if (of_device_is_compatible(np, "qcom,msm8916-apcs-kpss-global")) {
> +		/* register the APCS mux and divider clock */
> +		ret = msm8916_register_clk(&pdev->dev, base);
> +		if (ret)
> +			return ret;
> +	}
> +

Don't you need to clean up anything in the below error path and in
remove()?

>  	offset = (unsigned long)of_device_get_match_data(&pdev->dev);
>  
>  	apcs->reg = base + offset;

Other than that I think this looks reasonable.

Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
kernel test robot June 25, 2017, 1:24 a.m. UTC | #2 
Hi Georgi,

[auto build test ERROR on next-20170619]
[cannot apply to clk/clk-next robh/for-next linus/master v4.12-rc6 v4.12-rc5 v4.12-rc4 v4.12-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Georgi-Djakov/Add-support-for-Qualcomm-A53-CPU-clock/20170625-063544
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 6.2.0
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=ia64 

All error/warnings (new ones prefixed by >>):

   In file included from drivers/mailbox/qcom-apcs-ipc-mailbox.c:26:0:
>> drivers/mailbox/../clk/qcom/clk-regmap.h:31:16: error: field 'hw' has incomplete type
     struct clk_hw hw;
                   ^~
   drivers/mailbox/qcom-apcs-ipc-mailbox.c: In function 'msm8916_register_clk':
>> drivers/mailbox/qcom-apcs-ipc-mailbox.c:101:9: error: variable 'init' has initializer but incomplete type
     struct clk_init_data init = { };
            ^~~~~~~~~~~~~
>> drivers/mailbox/qcom-apcs-ipc-mailbox.c:101:23: error: storage size of 'init' isn't known
     struct clk_init_data init = { };
                          ^~~~
>> drivers/mailbox/qcom-apcs-ipc-mailbox.c:119:15: error: 'CLK_SET_RATE_PARENT' undeclared (first use in this function)
     init.flags = CLK_SET_RATE_PARENT;
                  ^~~~~~~~~~~~~~~~~~~
   drivers/mailbox/qcom-apcs-ipc-mailbox.c:119:15: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/mailbox/qcom-apcs-ipc-mailbox.c:122:9: error: implicit declaration of function '__clk_lookup' [-Werror=implicit-function-declaration]
     pclk = __clk_lookup(gpll0_a53cc[1]);
            ^~~~~~~~~~~~
>> drivers/mailbox/qcom-apcs-ipc-mailbox.c:122:7: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
     pclk = __clk_lookup(gpll0_a53cc[1]);
          ^
>> drivers/mailbox/qcom-apcs-ipc-mailbox.c:148:8: error: implicit declaration of function 'of_clk_add_hw_provider' [-Werror=implicit-function-declaration]
     ret = of_clk_add_hw_provider(dev->of_node, of_clk_hw_simple_get,
           ^~~~~~~~~~~~~~~~~~~~~~
>> drivers/mailbox/qcom-apcs-ipc-mailbox.c:148:45: error: 'of_clk_hw_simple_get' undeclared (first use in this function)
     ret = of_clk_add_hw_provider(dev->of_node, of_clk_hw_simple_get,
                                                ^~~~~~~~~~~~~~~~~~~~
   drivers/mailbox/qcom-apcs-ipc-mailbox.c:101:23: warning: unused variable 'init' [-Wunused-variable]
     struct clk_init_data init = { };
                          ^~~~
   cc1: some warnings being treated as errors

vim +/hw +31 drivers/mailbox/../clk/qcom/clk-regmap.h

085d7a45 Stephen Boyd 2014-01-15  25   * @enable_reg: register when using regmap enable/disable ops
085d7a45 Stephen Boyd 2014-01-15  26   * @enable_mask: mask when using regmap enable/disable ops
085d7a45 Stephen Boyd 2014-01-15  27   * @enable_is_inverted: flag to indicate set enable_mask bits to disable
085d7a45 Stephen Boyd 2014-01-15  28   *                      when using clock_enable_regmap and friends APIs.
085d7a45 Stephen Boyd 2014-01-15  29   */
085d7a45 Stephen Boyd 2014-01-15  30  struct clk_regmap {
085d7a45 Stephen Boyd 2014-01-15 @31  	struct clk_hw hw;
085d7a45 Stephen Boyd 2014-01-15  32  	struct regmap *regmap;
085d7a45 Stephen Boyd 2014-01-15  33  	unsigned int enable_reg;
085d7a45 Stephen Boyd 2014-01-15  34  	unsigned int enable_mask;

:::::: The code at line 31 was first introduced by commit
:::::: 085d7a455444f4d425371ee3c8a273c6e1b522db clk: qcom: Add a regmap type clock struct

:::::: TO: Stephen Boyd <sboyd@codeaurora.org>
:::::: CC: Mike Turquette <mturquette@linaro.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
kernel test robot June 25, 2017, 4:50 a.m. UTC | #3 
Hi Georgi,

[auto build test ERROR on next-20170619]
[cannot apply to clk/clk-next robh/for-next linus/master v4.12-rc6 v4.12-rc5 v4.12-rc4 v4.12-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Georgi-Djakov/Add-support-for-Qualcomm-A53-CPU-clock/20170625-063544
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

>> ERROR: "__mux_div_set_src_div" [drivers/mailbox/qcom-apcs-ipc-mailbox.ko] undefined!
>> ERROR: "__clk_lookup" [drivers/mailbox/qcom-apcs-ipc-mailbox.ko] undefined!
>> ERROR: "clk_regmap_mux_div_ops" [drivers/mailbox/qcom-apcs-ipc-mailbox.ko] undefined!

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Rob Herring (Arm) June 26, 2017, 7:40 p.m. UTC | #4 
On Fri, Jun 23, 2017 at 07:15:33PM +0300, 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.
> 
> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
> ---
>  .../bindings/mailbox/qcom,apcs-kpss-global.txt     |   5 +

Acked-by: Rob Herring <robh@kernel.org>

>  drivers/mailbox/qcom-apcs-ipc-mailbox.c            | 122 +++++++++++++++++++++
>  2 files changed, 127 insertions(+)
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Boyd June 26, 2017, 10:47 p.m. UTC | #5 
On 06/23, Georgi Djakov wrote:
> diff --git a/drivers/mailbox/qcom-apcs-ipc-mailbox.c b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
> index 9924c6d7f05d..da363c6580da 100644
> --- a/drivers/mailbox/qcom-apcs-ipc-mailbox.c
> +++ b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
> @@ -11,6 +11,8 @@
>   * GNU General Public License for more details.
>   */
>  
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/io.h>
> @@ -19,6 +21,34 @@
>  #include <linux/of_platform.h>
>  #include <linux/platform_device.h>
>  #include <linux/mailbox_controller.h>
> +#include <linux/regmap.h>
> +
> +#include "../clk/qcom/clk-regmap.h"
> +#include "../clk/qcom/clk-regmap-mux-div.h"

Why?

> +
> +
> +static int msm8916_register_clk(struct device *dev, void __iomem *base)
> +{
> +	struct clk_regmap_mux_div *a53cc;
> +	struct clk *pclk;
> +	struct regmap *regmap;
> +	struct clk_init_data init = { };
> +	int ret;
> +
> +	a53cc = devm_kzalloc(dev, sizeof(*a53cc), GFP_KERNEL);
> +	if (!a53cc)
> +		return -ENOMEM;
> +
> +	a53cc->reg_offset = 0x50;
> +	a53cc->hid_width = 5;
> +	a53cc->hid_shift = 0;
> +	a53cc->src_width = 3;
> +	a53cc->src_shift = 8;
> +	a53cc->parent_map = gpll0_a53cc_map;
> +
> +	init.name = "a53mux";
> +	init.parent_names = gpll0_a53cc;
> +	init.num_parents = 2;

ARRAY_SIZE(gpll0_a53cc) instead of 2 please

> +	init.ops = &clk_regmap_mux_div_ops;
> +	init.flags = CLK_SET_RATE_PARENT;
> +	a53cc->clkr.hw.init = &init;
> +
> +	pclk = __clk_lookup(gpll0_a53cc[1]);

Urgh.. ok. We can't clk_get()?

> +	if (!pclk)
> +		return -EPROBE_DEFER;
> +
> +	a53cc->clk_nb.notifier_call = a53cc_notifier_cb;
> +	ret = clk_notifier_register(pclk, &a53cc->clk_nb);
> +	if (ret) {
> +		dev_err(dev, "failed to register clock notifier: %d\n", ret);
> +		return ret;
> +	}
> +
> +	regmap = devm_regmap_init_mmio(dev, base, &a53cc_regmap_config);
> +	if (IS_ERR(regmap)) {
> +		ret = PTR_ERR(regmap);
> +		dev_err(dev, "failed to init regmap mmio: %d\n", ret);
> +		goto err;
> +	}
> +
> +	a53cc->clkr.regmap = regmap;
> +
> +	ret = devm_clk_register_regmap(dev, &a53cc->clkr);

Regmap is not a requirement to work with the qcom clk driver.

> +	if (ret) {
> +		dev_err(dev, "failed to register regmap clock: %d\n", ret);
> +		goto err;
> +	}
> +
> +	ret = of_clk_add_hw_provider(dev->of_node, of_clk_hw_simple_get,
> +				     &a53cc->clkr.hw);
> +	if (ret) {
> +		dev_err(dev, "failed to add clock provider: %d\n", ret);
> +		goto err;
> +	}
> +
> +	return 0;
> +
> +err:
> +	clk_notifier_unregister(pclk, &a53cc->clk_nb);
> +	return ret;
> +}
> +
>  static int qcom_apcs_ipc_probe(struct platform_device *pdev)
>  {
> +	struct device_node *np = pdev->dev.of_node;
>  	struct qcom_apcs_ipc *apcs;
>  	struct resource *res;
>  	unsigned long offset;
> @@ -63,6 +178,13 @@ static int qcom_apcs_ipc_probe(struct platform_device *pdev)
>  	if (IS_ERR(base))
>  		return PTR_ERR(base);
>  
> +	if (of_device_is_compatible(np, "qcom,msm8916-apcs-kpss-global")) {
> +		/* register the APCS mux and divider clock */
> +		ret = msm8916_register_clk(&pdev->dev, base);

Register a child platform device here instead of creating clks in the
same driver?

> +		if (ret)
> +			return ret;
> +	}
> +
>  	offset = (unsigned long)of_device_get_match_data(&pdev->dev);
>  
>  	apcs->reg = base + offset;
Georgi Djakov June 27, 2017, 4:21 p.m. UTC | #6 
Hi Bjorn,

On 06/23/2017 08:45 PM, Bjorn Andersson wrote:
> On Fri 23 Jun 09:15 PDT 2017, Georgi Djakov wrote:
> 
>> +static int msm8916_register_clk(struct device *dev, void __iomem *base)
>> +{
> [..]
>> +	regmap = devm_regmap_init_mmio(dev, base, &a53cc_regmap_config);
>> +	if (IS_ERR(regmap)) {
>> +		ret = PTR_ERR(regmap);
>> +		dev_err(dev, "failed to init regmap mmio: %d\n", ret);
>> +		goto err;
>> +	}
> 
> I think it would be cleaner if you create the regmap in probe() and we
> use that throughout the driver - rather than using two different access
> mechanism.

Ok agree, will make sure its consistent.

> 
>> +
>> +	a53cc->clkr.regmap = regmap;
>> +
>> +	ret = devm_clk_register_regmap(dev, &a53cc->clkr);
>> +	if (ret) {
>> +		dev_err(dev, "failed to register regmap clock: %d\n", ret);
>> +		goto err;
>> +	}
>> +
>> +	ret = of_clk_add_hw_provider(dev->of_node, of_clk_hw_simple_get,
>> +				     &a53cc->clkr.hw);
>> +	if (ret) {
>> +		dev_err(dev, "failed to add clock provider: %d\n", ret);
>> +		goto err;
>> +	}
>> +
>> +	return 0;
>> +
>> +err:
>> +	clk_notifier_unregister(pclk, &a53cc->clk_nb);
>> +	return ret;
>> +}
>> +
>>  static int qcom_apcs_ipc_probe(struct platform_device *pdev)
>>  {
>> +	struct device_node *np = pdev->dev.of_node;
>>  	struct qcom_apcs_ipc *apcs;
>>  	struct resource *res;
>>  	unsigned long offset;
>> @@ -63,6 +178,13 @@ static int qcom_apcs_ipc_probe(struct platform_device *pdev)
>>  	if (IS_ERR(base))
>>  		return PTR_ERR(base);
>>  
>> +	if (of_device_is_compatible(np, "qcom,msm8916-apcs-kpss-global")) {
>> +		/* register the APCS mux and divider clock */
>> +		ret = msm8916_register_clk(&pdev->dev, base);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
> 
> Don't you need to clean up anything in the below error path and in
> remove()?

Right, will take care of it.

> 
>>  	offset = (unsigned long)of_device_get_match_data(&pdev->dev);
>>  
>>  	apcs->reg = base + offset;
> 
> Other than that I think this looks reasonable.

Thanks for your time!

BR,
Georgi

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Georgi Djakov June 27, 2017, 4:24 p.m. UTC | #7 
Hi Stephen,

On 06/27/2017 01:47 AM, Stephen Boyd wrote:
> On 06/23, Georgi Djakov wrote:
>> diff --git a/drivers/mailbox/qcom-apcs-ipc-mailbox.c b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
>> index 9924c6d7f05d..da363c6580da 100644
>> --- a/drivers/mailbox/qcom-apcs-ipc-mailbox.c
>> +++ b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
>> @@ -11,6 +11,8 @@
>>   * GNU General Public License for more details.
>>   */
>>  
>> +#include <linux/clk.h>
>> +#include <linux/clk-provider.h>
>>  #include <linux/kernel.h>
>>  #include <linux/module.h>
>>  #include <linux/io.h>
>> @@ -19,6 +21,34 @@
>>  #include <linux/of_platform.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/mailbox_controller.h>
>> +#include <linux/regmap.h>
>> +
>> +#include "../clk/qcom/clk-regmap.h"
>> +#include "../clk/qcom/clk-regmap-mux-div.h"
> 
> Why?
> 
>> +
>> +
>> +static int msm8916_register_clk(struct device *dev, void __iomem *base)
>> +{
>> +	struct clk_regmap_mux_div *a53cc;
>> +	struct clk *pclk;
>> +	struct regmap *regmap;
>> +	struct clk_init_data init = { };
>> +	int ret;
>> +
>> +	a53cc = devm_kzalloc(dev, sizeof(*a53cc), GFP_KERNEL);
>> +	if (!a53cc)
>> +		return -ENOMEM;
>> +
>> +	a53cc->reg_offset = 0x50;
>> +	a53cc->hid_width = 5;
>> +	a53cc->hid_shift = 0;
>> +	a53cc->src_width = 3;
>> +	a53cc->src_shift = 8;
>> +	a53cc->parent_map = gpll0_a53cc_map;
>> +
>> +	init.name = "a53mux";
>> +	init.parent_names = gpll0_a53cc;
>> +	init.num_parents = 2;
> 
> ARRAY_SIZE(gpll0_a53cc) instead of 2 please

Ok.

> 
>> +	init.ops = &clk_regmap_mux_div_ops;
>> +	init.flags = CLK_SET_RATE_PARENT;
>> +	a53cc->clkr.hw.init = &init;
>> +
>> +	pclk = __clk_lookup(gpll0_a53cc[1]);
> 
> Urgh.. ok. We can't clk_get()?

Ok, will do.

> 
>> +	if (!pclk)
>> +		return -EPROBE_DEFER;
>> +
>> +	a53cc->clk_nb.notifier_call = a53cc_notifier_cb;
>> +	ret = clk_notifier_register(pclk, &a53cc->clk_nb);
>> +	if (ret) {
>> +		dev_err(dev, "failed to register clock notifier: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	regmap = devm_regmap_init_mmio(dev, base, &a53cc_regmap_config);
>> +	if (IS_ERR(regmap)) {
>> +		ret = PTR_ERR(regmap);
>> +		dev_err(dev, "failed to init regmap mmio: %d\n", ret);
>> +		goto err;
>> +	}
>> +
>> +	a53cc->clkr.regmap = regmap;
>> +
>> +	ret = devm_clk_register_regmap(dev, &a53cc->clkr);
> 
> Regmap is not a requirement to work with the qcom clk driver.
> 

The other option then is to drop the regmap-mux-div patch and switch
to readl()/writel().

>> +	if (ret) {
>> +		dev_err(dev, "failed to register regmap clock: %d\n", ret);
>> +		goto err;
>> +	}
>> +
>> +	ret = of_clk_add_hw_provider(dev->of_node, of_clk_hw_simple_get,
>> +				     &a53cc->clkr.hw);
>> +	if (ret) {
>> +		dev_err(dev, "failed to add clock provider: %d\n", ret);
>> +		goto err;
>> +	}
>> +
>> +	return 0;
>> +
>> +err:
>> +	clk_notifier_unregister(pclk, &a53cc->clk_nb);
>> +	return ret;
>> +}
>> +
>>  static int qcom_apcs_ipc_probe(struct platform_device *pdev)
>>  {
>> +	struct device_node *np = pdev->dev.of_node;
>>  	struct qcom_apcs_ipc *apcs;
>>  	struct resource *res;
>>  	unsigned long offset;
>> @@ -63,6 +178,13 @@ static int qcom_apcs_ipc_probe(struct platform_device *pdev)
>>  	if (IS_ERR(base))
>>  		return PTR_ERR(base);
>>  
>> +	if (of_device_is_compatible(np, "qcom,msm8916-apcs-kpss-global")) {
>> +		/* register the APCS mux and divider clock */
>> +		ret = msm8916_register_clk(&pdev->dev, base);
> 
> Register a child platform device here instead of creating clks in the
> same driver?

Ok, we can register a child platform device and create a separate driver
in drivers/clk. If we are dropping regmap, then we can also move the
mux-div clk ops and clk registration into the same file. I will give it
a try.

Thanks,
Georgi

> 
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>>  	offset = (unsigned long)of_device_get_match_data(&pdev->dev);
>>  
>>  	apcs->reg = base + offset;
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt
index fb961c310f44..2432be307083 100644
--- a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt
+++ b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt
@@ -21,6 +21,11 @@  platforms.
 	Value type: <u32>
 	Definition: as described in mailbox.txt, must be 1
 
+- #clock-cells:
+	Usage: required for msm8916 platforms
+	Value type: <u32>
+	Definition: as described in clock-bindings.txt, must be 0
+
 
 = EXAMPLE
 The following example describes the APCS HMSS found in MSM8996 and part of the
diff --git a/drivers/mailbox/qcom-apcs-ipc-mailbox.c b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
index 9924c6d7f05d..da363c6580da 100644
--- a/drivers/mailbox/qcom-apcs-ipc-mailbox.c
+++ b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
@@ -11,6 +11,8 @@ 
  * GNU General Public License for more details.
  */
 
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/io.h>
@@ -19,6 +21,34 @@ 
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/mailbox_controller.h>
+#include <linux/regmap.h>
+
+#include "../clk/qcom/clk-regmap.h"
+#include "../clk/qcom/clk-regmap-mux-div.h"
+
+enum {
+	P_GPLL0,
+	P_A53PLL,
+};
+
+static const struct parent_map gpll0_a53cc_map[] = {
+	{ P_GPLL0, 4 },
+	{ P_A53PLL, 5 },
+};
+
+static const char * const gpll0_a53cc[] = {
+	"gpll0_vote",
+	"a53pll",
+};
+
+static const struct regmap_config a53cc_regmap_config = {
+	.reg_bits		= 32,
+	.reg_stride		= 4,
+	.val_bits		= 32,
+	.max_register		= 0x1000,
+	.fast_io		= true,
+	.val_format_endian	= REGMAP_ENDIAN_LITTLE,
+};
 
 #define QCOM_APCS_IPC_BITS	32
 
@@ -45,8 +75,93 @@  static const struct mbox_chan_ops qcom_apcs_ipc_ops = {
 	.send_data = qcom_apcs_ipc_send_data,
 };
 
+/*
+ * We use the notifier function for switching to a temporary safe configuration
+ * (mux and divider), while the A53 PLL is reconfigured.
+ */
+static int a53cc_notifier_cb(struct notifier_block *nb, unsigned long event,
+			     void *data)
+{
+	int ret = 0;
+	struct clk_regmap_mux_div *md = container_of(nb,
+						     struct clk_regmap_mux_div,
+						     clk_nb);
+	if (event == PRE_RATE_CHANGE)
+		/* set the mux and divider to safe frequency (400mhz) */
+		ret = __mux_div_set_src_div(md, 4, 3);
+
+	return notifier_from_errno(ret);
+}
+
+static int msm8916_register_clk(struct device *dev, void __iomem *base)
+{
+	struct clk_regmap_mux_div *a53cc;
+	struct clk *pclk;
+	struct regmap *regmap;
+	struct clk_init_data init = { };
+	int ret;
+
+	a53cc = devm_kzalloc(dev, sizeof(*a53cc), GFP_KERNEL);
+	if (!a53cc)
+		return -ENOMEM;
+
+	a53cc->reg_offset = 0x50;
+	a53cc->hid_width = 5;
+	a53cc->hid_shift = 0;
+	a53cc->src_width = 3;
+	a53cc->src_shift = 8;
+	a53cc->parent_map = gpll0_a53cc_map;
+
+	init.name = "a53mux";
+	init.parent_names = gpll0_a53cc;
+	init.num_parents = 2;
+	init.ops = &clk_regmap_mux_div_ops;
+	init.flags = CLK_SET_RATE_PARENT;
+	a53cc->clkr.hw.init = &init;
+
+	pclk = __clk_lookup(gpll0_a53cc[1]);
+	if (!pclk)
+		return -EPROBE_DEFER;
+
+	a53cc->clk_nb.notifier_call = a53cc_notifier_cb;
+	ret = clk_notifier_register(pclk, &a53cc->clk_nb);
+	if (ret) {
+		dev_err(dev, "failed to register clock notifier: %d\n", ret);
+		return ret;
+	}
+
+	regmap = devm_regmap_init_mmio(dev, base, &a53cc_regmap_config);
+	if (IS_ERR(regmap)) {
+		ret = PTR_ERR(regmap);
+		dev_err(dev, "failed to init regmap mmio: %d\n", ret);
+		goto err;
+	}
+
+	a53cc->clkr.regmap = regmap;
+
+	ret = devm_clk_register_regmap(dev, &a53cc->clkr);
+	if (ret) {
+		dev_err(dev, "failed to register regmap clock: %d\n", ret);
+		goto err;
+	}
+
+	ret = of_clk_add_hw_provider(dev->of_node, of_clk_hw_simple_get,
+				     &a53cc->clkr.hw);
+	if (ret) {
+		dev_err(dev, "failed to add clock provider: %d\n", ret);
+		goto err;
+	}
+
+	return 0;
+
+err:
+	clk_notifier_unregister(pclk, &a53cc->clk_nb);
+	return ret;
+}
+
 static int qcom_apcs_ipc_probe(struct platform_device *pdev)
 {
+	struct device_node *np = pdev->dev.of_node;
 	struct qcom_apcs_ipc *apcs;
 	struct resource *res;
 	unsigned long offset;
@@ -63,6 +178,13 @@  static int qcom_apcs_ipc_probe(struct platform_device *pdev)
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
+	if (of_device_is_compatible(np, "qcom,msm8916-apcs-kpss-global")) {
+		/* register the APCS mux and divider clock */
+		ret = msm8916_register_clk(&pdev->dev, base);
+		if (ret)
+			return ret;
+	}
+
 	offset = (unsigned long)of_device_get_match_data(&pdev->dev);
 
 	apcs->reg = base + offset;