Re: [[PATCH v2] 1/2] clk: qcom: clk-rpmh: Add QCOM RPMh clock driver

2018-04-13 Thread Taniya Das 

Hello Bjorn,

Thank you for the review comments.

On 4/10/2018 11:09 PM, Bjorn Andersson wrote:

On Sun 08 Apr 03:32 PDT 2018, Taniya Das wrote:


From: Amit Nischal 

Add the RPMh clock driver to control the RPMh managed clock resources on
some of the Qualcomm Technologies, Inc. SoCs.

Signed-off-by: David Collins 
Signed-off-by: Amit Nischal 
Signed-off-by: Taniya Das 
---
  drivers/clk/qcom/Kconfig  |   9 +
  drivers/clk/qcom/Makefile |   1 +
  drivers/clk/qcom/clk-rpmh.c   | 394 ++
  include/dt-bindings/clock/qcom,rpmh.h |  25 +++
  4 files changed, 429 insertions(+)
  create mode 100644 drivers/clk/qcom/clk-rpmh.c
  create mode 100644 include/dt-bindings/clock/qcom,rpmh.h

diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index fbf4532..3697a6a 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -112,6 +112,15 @@ config IPQ_GCC_8074
  i2c, USB, SD/eMMC, etc. Select this for the root clock
  of ipq8074.
  
+config MSM_CLK_RPMH


QCOM_CLK_RPMH



Would update it to use "QCOM_CLK_RPMH".


+   tristate "RPMh Clock Driver"
+   depends on COMMON_CLK_QCOM && QCOM_RPMH
+   help
+RPMh manages shared resources on some Qualcomm Technologies, Inc.
+SoCs. It accepts requests from other hardware subsystems via RSC.
+Say Y if you want to support the clocks exposed by RPMh on
+platforms such as sdm845.
+
  config MSM_GCC_8660
tristate "MSM8660 Global Clock Controller"
depends on COMMON_CLK_QCOM
diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index 230332c..b7da05b 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_IPQ_GCC_8074) += gcc-ipq8074.o
  obj-$(CONFIG_IPQ_LCC_806X) += lcc-ipq806x.o
  obj-$(CONFIG_MDM_GCC_9615) += gcc-mdm9615.o
  obj-$(CONFIG_MDM_LCC_9615) += lcc-mdm9615.o
+obj-$(CONFIG_MSM_CLK_RPMH) += clk-rpmh.o
  obj-$(CONFIG_MSM_GCC_8660) += gcc-msm8660.o
  obj-$(CONFIG_MSM_GCC_8916) += gcc-msm8916.o
  obj-$(CONFIG_MSM_GCC_8960) += gcc-msm8960.o
diff --git a/drivers/clk/qcom/clk-rpmh.c b/drivers/clk/qcom/clk-rpmh.c
new file mode 100644
index 000..763401f
--- /dev/null
+++ b/drivers/clk/qcom/clk-rpmh.c
@@ -0,0 +1,394 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2017-2018, The Linux Foundation. All rights reserved.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 


Unused


+#include 
+#include 
+
+#include 
+
+#include "common.h"
+#include "clk-regmap.h"


Unused



I would remove the unused header includes.


+
+#define CLK_RPMH_ARC_EN_OFFSET 0
+#define CLK_RPMH_VRM_EN_OFFSET 4
+#define CLK_RPMH_VRM_OFF_VAL 0
+#define CLK_RPMH_VRM_ON_VAL 1


Use a bool (true/false) rather than lugging around these two defines.


I would remove these in the next patch.




+#define CLK_RPMH_APPS_RSC_AO_STATE_MASK (BIT(RPMH_WAKE_ONLY_STATE) | \
+BIT(RPMH_ACTIVE_ONLY_STATE))
+#define CLK_RPMH_APPS_RSC_STATE_MASK (BIT(RPMH_WAKE_ONLY_STATE) | \
+ BIT(RPMH_ACTIVE_ONLY_STATE) | \
+ BIT(RPMH_SLEEP_STATE))
+struct clk_rpmh {
+   struct clk_hw hw;
+   const char *res_name;
+   u32 res_addr;
+   u32 res_en_offset;
+   u32 res_on_val;
+   u32 res_off_val;


res_off_val is 0 for all clocks.



They could change if required, so is the reason for keeping it.


+   u32 state;
+   u32 aggr_state;
+   u32 last_sent_aggr_state;


Through the use of some local variables you shouldn't have to lug around
aggr_state and last_sent_aggr_state.



We need to check if the last state and the current state requested, 
has_state_changed().



+   u32 valid_state_mask;
+   struct rpmh_client *rpmh_client;
+   unsigned long rate;
+   struct clk_rpmh *peer;
+};
+
+struct rpmh_cc {
+   struct clk_onecell_data data;
+   struct clk *clks[];
+};
+
+struct clk_rpmh_desc {
+   struct clk_hw **clks;
+   size_t num_clks;
+};
+
+static DEFINE_MUTEX(rpmh_clk_lock);
+
+#define __DEFINE_CLK_RPMH(_platform, _name, _name_active, _res_name, \
+ _res_en_offset, _res_on, _res_off, _rate, \
+ _state_mask, _state_on_mask)\
+   static struct clk_rpmh _platform##_##_name_active;\
+   static struct clk_rpmh _platform##_##_name = {\
+   .res_name = _res_name,\
+   .res_en_offset = _res_en_offset,  \
+   .res_on_val = _res_on,\
+   .res_off_val = _res_off,  \
+

Re: [[PATCH v2] 1/2] clk: qcom: clk-rpmh: Add QCOM RPMh clock driver

2018-04-13 Thread Taniya Das 

Hello Bjorn,

Thank you for the review comments.

On 4/10/2018 11:09 PM, Bjorn Andersson wrote:

On Sun 08 Apr 03:32 PDT 2018, Taniya Das wrote:


From: Amit Nischal 

Add the RPMh clock driver to control the RPMh managed clock resources on
some of the Qualcomm Technologies, Inc. SoCs.

Signed-off-by: David Collins 
Signed-off-by: Amit Nischal 
Signed-off-by: Taniya Das 
---
  drivers/clk/qcom/Kconfig  |   9 +
  drivers/clk/qcom/Makefile |   1 +
  drivers/clk/qcom/clk-rpmh.c   | 394 ++
  include/dt-bindings/clock/qcom,rpmh.h |  25 +++
  4 files changed, 429 insertions(+)
  create mode 100644 drivers/clk/qcom/clk-rpmh.c
  create mode 100644 include/dt-bindings/clock/qcom,rpmh.h

diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index fbf4532..3697a6a 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -112,6 +112,15 @@ config IPQ_GCC_8074
  i2c, USB, SD/eMMC, etc. Select this for the root clock
  of ipq8074.
  
+config MSM_CLK_RPMH


QCOM_CLK_RPMH



Would update it to use "QCOM_CLK_RPMH".


+   tristate "RPMh Clock Driver"
+   depends on COMMON_CLK_QCOM && QCOM_RPMH
+   help
+RPMh manages shared resources on some Qualcomm Technologies, Inc.
+SoCs. It accepts requests from other hardware subsystems via RSC.
+Say Y if you want to support the clocks exposed by RPMh on
+platforms such as sdm845.
+
  config MSM_GCC_8660
tristate "MSM8660 Global Clock Controller"
depends on COMMON_CLK_QCOM
diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index 230332c..b7da05b 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_IPQ_GCC_8074) += gcc-ipq8074.o
  obj-$(CONFIG_IPQ_LCC_806X) += lcc-ipq806x.o
  obj-$(CONFIG_MDM_GCC_9615) += gcc-mdm9615.o
  obj-$(CONFIG_MDM_LCC_9615) += lcc-mdm9615.o
+obj-$(CONFIG_MSM_CLK_RPMH) += clk-rpmh.o
  obj-$(CONFIG_MSM_GCC_8660) += gcc-msm8660.o
  obj-$(CONFIG_MSM_GCC_8916) += gcc-msm8916.o
  obj-$(CONFIG_MSM_GCC_8960) += gcc-msm8960.o
diff --git a/drivers/clk/qcom/clk-rpmh.c b/drivers/clk/qcom/clk-rpmh.c
new file mode 100644
index 000..763401f
--- /dev/null
+++ b/drivers/clk/qcom/clk-rpmh.c
@@ -0,0 +1,394 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2017-2018, The Linux Foundation. All rights reserved.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 


Unused


+#include 
+#include 
+
+#include 
+
+#include "common.h"
+#include "clk-regmap.h"


Unused



I would remove the unused header includes.


+
+#define CLK_RPMH_ARC_EN_OFFSET 0
+#define CLK_RPMH_VRM_EN_OFFSET 4
+#define CLK_RPMH_VRM_OFF_VAL 0
+#define CLK_RPMH_VRM_ON_VAL 1


Use a bool (true/false) rather than lugging around these two defines.


I would remove these in the next patch.




+#define CLK_RPMH_APPS_RSC_AO_STATE_MASK (BIT(RPMH_WAKE_ONLY_STATE) | \
+BIT(RPMH_ACTIVE_ONLY_STATE))
+#define CLK_RPMH_APPS_RSC_STATE_MASK (BIT(RPMH_WAKE_ONLY_STATE) | \
+ BIT(RPMH_ACTIVE_ONLY_STATE) | \
+ BIT(RPMH_SLEEP_STATE))
+struct clk_rpmh {
+   struct clk_hw hw;
+   const char *res_name;
+   u32 res_addr;
+   u32 res_en_offset;
+   u32 res_on_val;
+   u32 res_off_val;


res_off_val is 0 for all clocks.



They could change if required, so is the reason for keeping it.


+   u32 state;
+   u32 aggr_state;
+   u32 last_sent_aggr_state;


Through the use of some local variables you shouldn't have to lug around
aggr_state and last_sent_aggr_state.



We need to check if the last state and the current state requested, 
has_state_changed().



+   u32 valid_state_mask;
+   struct rpmh_client *rpmh_client;
+   unsigned long rate;
+   struct clk_rpmh *peer;
+};
+
+struct rpmh_cc {
+   struct clk_onecell_data data;
+   struct clk *clks[];
+};
+
+struct clk_rpmh_desc {
+   struct clk_hw **clks;
+   size_t num_clks;
+};
+
+static DEFINE_MUTEX(rpmh_clk_lock);
+
+#define __DEFINE_CLK_RPMH(_platform, _name, _name_active, _res_name, \
+ _res_en_offset, _res_on, _res_off, _rate, \
+ _state_mask, _state_on_mask)\
+   static struct clk_rpmh _platform##_##_name_active;\
+   static struct clk_rpmh _platform##_##_name = {\
+   .res_name = _res_name,\
+   .res_en_offset = _res_en_offset,  \
+   .res_on_val = _res_on,\
+   .res_off_val = _res_off,  \
+   .rate = _rate,\
+   .peer =

Re: [[PATCH v2] 1/2] clk: qcom: clk-rpmh: Add QCOM RPMh clock driver

2018-04-13 Thread Taniya Das 

Hello Rob,

Thank you for the review comments.

On 4/13/2018 10:07 PM, Rob Herring wrote:

On Sun, Apr 08, 2018 at 04:02:12PM +0530, Taniya Das wrote:

From: Amit Nischal 

Add the RPMh clock driver to control the RPMh managed clock resources on
some of the Qualcomm Technologies, Inc. SoCs.

Signed-off-by: David Collins 
Signed-off-by: Amit Nischal 
Signed-off-by: Taniya Das 
---
  drivers/clk/qcom/Kconfig  |   9 +
  drivers/clk/qcom/Makefile |   1 +
  drivers/clk/qcom/clk-rpmh.c   | 394 ++



  include/dt-bindings/clock/qcom,rpmh.h |  25 +++


This goes with the binding patch which should come first in the series.



I would make the change in the next patch series.


  4 files changed, 429 insertions(+)
  create mode 100644 drivers/clk/qcom/clk-rpmh.c
  create mode 100644 include/dt-bindings/clock/qcom,rpmh.h


--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.

--

Re: [[PATCH v2] 1/2] clk: qcom: clk-rpmh: Add QCOM RPMh clock driver

2018-04-13 Thread Taniya Das 

Hello Rob,

Thank you for the review comments.

On 4/13/2018 10:07 PM, Rob Herring wrote:

On Sun, Apr 08, 2018 at 04:02:12PM +0530, Taniya Das wrote:

From: Amit Nischal 

Add the RPMh clock driver to control the RPMh managed clock resources on
some of the Qualcomm Technologies, Inc. SoCs.

Signed-off-by: David Collins 
Signed-off-by: Amit Nischal 
Signed-off-by: Taniya Das 
---
  drivers/clk/qcom/Kconfig  |   9 +
  drivers/clk/qcom/Makefile |   1 +
  drivers/clk/qcom/clk-rpmh.c   | 394 ++



  include/dt-bindings/clock/qcom,rpmh.h |  25 +++


This goes with the binding patch which should come first in the series.



I would make the change in the next patch series.


  4 files changed, 429 insertions(+)
  create mode 100644 drivers/clk/qcom/clk-rpmh.c
  create mode 100644 include/dt-bindings/clock/qcom,rpmh.h


--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.

--

Re: [[PATCH v2] 1/2] clk: qcom: clk-rpmh: Add QCOM RPMh clock driver

2018-04-13 Thread Rob Herring 
On Sun, Apr 08, 2018 at 04:02:12PM +0530, Taniya Das wrote:
> From: Amit Nischal 
> 
> Add the RPMh clock driver to control the RPMh managed clock resources on
> some of the Qualcomm Technologies, Inc. SoCs.
> 
> Signed-off-by: David Collins 
> Signed-off-by: Amit Nischal 
> Signed-off-by: Taniya Das 
> ---
>  drivers/clk/qcom/Kconfig  |   9 +
>  drivers/clk/qcom/Makefile |   1 +
>  drivers/clk/qcom/clk-rpmh.c   | 394 
> ++

>  include/dt-bindings/clock/qcom,rpmh.h |  25 +++

This goes with the binding patch which should come first in the series.

>  4 files changed, 429 insertions(+)
>  create mode 100644 drivers/clk/qcom/clk-rpmh.c
>  create mode 100644 include/dt-bindings/clock/qcom,rpmh.h

Re: [[PATCH v2] 1/2] clk: qcom: clk-rpmh: Add QCOM RPMh clock driver

2018-04-13 Thread Rob Herring 
On Sun, Apr 08, 2018 at 04:02:12PM +0530, Taniya Das wrote:
> From: Amit Nischal 
> 
> Add the RPMh clock driver to control the RPMh managed clock resources on
> some of the Qualcomm Technologies, Inc. SoCs.
> 
> Signed-off-by: David Collins 
> Signed-off-by: Amit Nischal 
> Signed-off-by: Taniya Das 
> ---
>  drivers/clk/qcom/Kconfig  |   9 +
>  drivers/clk/qcom/Makefile |   1 +
>  drivers/clk/qcom/clk-rpmh.c   | 394 
> ++

>  include/dt-bindings/clock/qcom,rpmh.h |  25 +++

This goes with the binding patch which should come first in the series.

>  4 files changed, 429 insertions(+)
>  create mode 100644 drivers/clk/qcom/clk-rpmh.c
>  create mode 100644 include/dt-bindings/clock/qcom,rpmh.h

Re: [[PATCH v2] 1/2] clk: qcom: clk-rpmh: Add QCOM RPMh clock driver

2018-04-10 Thread Bjorn Andersson 
On Sun 08 Apr 03:32 PDT 2018, Taniya Das wrote:

> From: Amit Nischal 
> 
> Add the RPMh clock driver to control the RPMh managed clock resources on
> some of the Qualcomm Technologies, Inc. SoCs.
> 
> Signed-off-by: David Collins 
> Signed-off-by: Amit Nischal 
> Signed-off-by: Taniya Das 
> ---
>  drivers/clk/qcom/Kconfig  |   9 +
>  drivers/clk/qcom/Makefile |   1 +
>  drivers/clk/qcom/clk-rpmh.c   | 394 
> ++
>  include/dt-bindings/clock/qcom,rpmh.h |  25 +++
>  4 files changed, 429 insertions(+)
>  create mode 100644 drivers/clk/qcom/clk-rpmh.c
>  create mode 100644 include/dt-bindings/clock/qcom,rpmh.h
> 
> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
> index fbf4532..3697a6a 100644
> --- a/drivers/clk/qcom/Kconfig
> +++ b/drivers/clk/qcom/Kconfig
> @@ -112,6 +112,15 @@ config IPQ_GCC_8074
> i2c, USB, SD/eMMC, etc. Select this for the root clock
> of ipq8074.
>  
> +config MSM_CLK_RPMH

QCOM_CLK_RPMH

> + tristate "RPMh Clock Driver"
> + depends on COMMON_CLK_QCOM && QCOM_RPMH
> + help
> +  RPMh manages shared resources on some Qualcomm Technologies, Inc.
> +  SoCs. It accepts requests from other hardware subsystems via RSC.
> +  Say Y if you want to support the clocks exposed by RPMh on
> +  platforms such as sdm845.
> +
>  config MSM_GCC_8660
>   tristate "MSM8660 Global Clock Controller"
>   depends on COMMON_CLK_QCOM
> diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
> index 230332c..b7da05b 100644
> --- a/drivers/clk/qcom/Makefile
> +++ b/drivers/clk/qcom/Makefile
> @@ -23,6 +23,7 @@ obj-$(CONFIG_IPQ_GCC_8074) += gcc-ipq8074.o
>  obj-$(CONFIG_IPQ_LCC_806X) += lcc-ipq806x.o
>  obj-$(CONFIG_MDM_GCC_9615) += gcc-mdm9615.o
>  obj-$(CONFIG_MDM_LCC_9615) += lcc-mdm9615.o
> +obj-$(CONFIG_MSM_CLK_RPMH) += clk-rpmh.o
>  obj-$(CONFIG_MSM_GCC_8660) += gcc-msm8660.o
>  obj-$(CONFIG_MSM_GCC_8916) += gcc-msm8916.o
>  obj-$(CONFIG_MSM_GCC_8960) += gcc-msm8960.o
> diff --git a/drivers/clk/qcom/clk-rpmh.c b/drivers/clk/qcom/clk-rpmh.c
> new file mode 100644
> index 000..763401f
> --- /dev/null
> +++ b/drivers/clk/qcom/clk-rpmh.c
> @@ -0,0 +1,394 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2017-2018, The Linux Foundation. All rights reserved.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

Unused

> +#include 
> +#include 
> +
> +#include 
> +
> +#include "common.h"
> +#include "clk-regmap.h"

Unused

> +
> +#define CLK_RPMH_ARC_EN_OFFSET 0
> +#define CLK_RPMH_VRM_EN_OFFSET 4
> +#define CLK_RPMH_VRM_OFF_VAL 0
> +#define CLK_RPMH_VRM_ON_VAL 1

Use a bool (true/false) rather than lugging around these two defines.

> +#define CLK_RPMH_APPS_RSC_AO_STATE_MASK (BIT(RPMH_WAKE_ONLY_STATE) | \
> +  BIT(RPMH_ACTIVE_ONLY_STATE))
> +#define CLK_RPMH_APPS_RSC_STATE_MASK (BIT(RPMH_WAKE_ONLY_STATE) | \
> +   BIT(RPMH_ACTIVE_ONLY_STATE) | \
> +   BIT(RPMH_SLEEP_STATE))
> +struct clk_rpmh {
> + struct clk_hw hw;
> + const char *res_name;
> + u32 res_addr;
> + u32 res_en_offset;
> + u32 res_on_val;
> + u32 res_off_val;

res_off_val is 0 for all clocks.

> + u32 state;
> + u32 aggr_state;
> + u32 last_sent_aggr_state;

Through the use of some local variables you shouldn't have to lug around
aggr_state and last_sent_aggr_state.

> + u32 valid_state_mask;
> + struct rpmh_client *rpmh_client;
> + unsigned long rate;
> + struct clk_rpmh *peer;
> +};
> +
> +struct rpmh_cc {
> + struct clk_onecell_data data;
> + struct clk *clks[];
> +};
> +
> +struct clk_rpmh_desc {
> + struct clk_hw **clks;
> + size_t num_clks;
> +};
> +
> +static DEFINE_MUTEX(rpmh_clk_lock);
> +
> +#define __DEFINE_CLK_RPMH(_platform, _name, _name_active, _res_name, \
> +   _res_en_offset, _res_on, _res_off, _rate, \
> +   _state_mask, _state_on_mask)\
> + static struct clk_rpmh _platform##_##_name_active;\
> + static struct clk_rpmh _platform##_##_name = {\
> + .res_name = _res_name,\
> + .res_en_offset = _res_en_offset,  \
> + .res_on_val = _res_on,\
> + .res_off_val = _res_off,  \
> + .rate = _rate,\
> + .peer = &_platform##_##_name_active,  \
> + .valid_state_mask = _state_mask,  \

This is always

Re: [[PATCH v2] 1/2] clk: qcom: clk-rpmh: Add QCOM RPMh clock driver

2018-04-10 Thread Bjorn Andersson 
On Sun 08 Apr 03:32 PDT 2018, Taniya Das wrote:

> From: Amit Nischal 
> 
> Add the RPMh clock driver to control the RPMh managed clock resources on
> some of the Qualcomm Technologies, Inc. SoCs.
> 
> Signed-off-by: David Collins 
> Signed-off-by: Amit Nischal 
> Signed-off-by: Taniya Das 
> ---
>  drivers/clk/qcom/Kconfig  |   9 +
>  drivers/clk/qcom/Makefile |   1 +
>  drivers/clk/qcom/clk-rpmh.c   | 394 
> ++
>  include/dt-bindings/clock/qcom,rpmh.h |  25 +++
>  4 files changed, 429 insertions(+)
>  create mode 100644 drivers/clk/qcom/clk-rpmh.c
>  create mode 100644 include/dt-bindings/clock/qcom,rpmh.h
> 
> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
> index fbf4532..3697a6a 100644
> --- a/drivers/clk/qcom/Kconfig
> +++ b/drivers/clk/qcom/Kconfig
> @@ -112,6 +112,15 @@ config IPQ_GCC_8074
> i2c, USB, SD/eMMC, etc. Select this for the root clock
> of ipq8074.
>  
> +config MSM_CLK_RPMH

QCOM_CLK_RPMH

> + tristate "RPMh Clock Driver"
> + depends on COMMON_CLK_QCOM && QCOM_RPMH
> + help
> +  RPMh manages shared resources on some Qualcomm Technologies, Inc.
> +  SoCs. It accepts requests from other hardware subsystems via RSC.
> +  Say Y if you want to support the clocks exposed by RPMh on
> +  platforms such as sdm845.
> +
>  config MSM_GCC_8660
>   tristate "MSM8660 Global Clock Controller"
>   depends on COMMON_CLK_QCOM
> diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
> index 230332c..b7da05b 100644
> --- a/drivers/clk/qcom/Makefile
> +++ b/drivers/clk/qcom/Makefile
> @@ -23,6 +23,7 @@ obj-$(CONFIG_IPQ_GCC_8074) += gcc-ipq8074.o
>  obj-$(CONFIG_IPQ_LCC_806X) += lcc-ipq806x.o
>  obj-$(CONFIG_MDM_GCC_9615) += gcc-mdm9615.o
>  obj-$(CONFIG_MDM_LCC_9615) += lcc-mdm9615.o
> +obj-$(CONFIG_MSM_CLK_RPMH) += clk-rpmh.o
>  obj-$(CONFIG_MSM_GCC_8660) += gcc-msm8660.o
>  obj-$(CONFIG_MSM_GCC_8916) += gcc-msm8916.o
>  obj-$(CONFIG_MSM_GCC_8960) += gcc-msm8960.o
> diff --git a/drivers/clk/qcom/clk-rpmh.c b/drivers/clk/qcom/clk-rpmh.c
> new file mode 100644
> index 000..763401f
> --- /dev/null
> +++ b/drivers/clk/qcom/clk-rpmh.c
> @@ -0,0 +1,394 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2017-2018, The Linux Foundation. All rights reserved.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

Unused

> +#include 
> +#include 
> +
> +#include 
> +
> +#include "common.h"
> +#include "clk-regmap.h"

Unused

> +
> +#define CLK_RPMH_ARC_EN_OFFSET 0
> +#define CLK_RPMH_VRM_EN_OFFSET 4
> +#define CLK_RPMH_VRM_OFF_VAL 0
> +#define CLK_RPMH_VRM_ON_VAL 1

Use a bool (true/false) rather than lugging around these two defines.

> +#define CLK_RPMH_APPS_RSC_AO_STATE_MASK (BIT(RPMH_WAKE_ONLY_STATE) | \
> +  BIT(RPMH_ACTIVE_ONLY_STATE))
> +#define CLK_RPMH_APPS_RSC_STATE_MASK (BIT(RPMH_WAKE_ONLY_STATE) | \
> +   BIT(RPMH_ACTIVE_ONLY_STATE) | \
> +   BIT(RPMH_SLEEP_STATE))
> +struct clk_rpmh {
> + struct clk_hw hw;
> + const char *res_name;
> + u32 res_addr;
> + u32 res_en_offset;
> + u32 res_on_val;
> + u32 res_off_val;

res_off_val is 0 for all clocks.

> + u32 state;
> + u32 aggr_state;
> + u32 last_sent_aggr_state;

Through the use of some local variables you shouldn't have to lug around
aggr_state and last_sent_aggr_state.

> + u32 valid_state_mask;
> + struct rpmh_client *rpmh_client;
> + unsigned long rate;
> + struct clk_rpmh *peer;
> +};
> +
> +struct rpmh_cc {
> + struct clk_onecell_data data;
> + struct clk *clks[];
> +};
> +
> +struct clk_rpmh_desc {
> + struct clk_hw **clks;
> + size_t num_clks;
> +};
> +
> +static DEFINE_MUTEX(rpmh_clk_lock);
> +
> +#define __DEFINE_CLK_RPMH(_platform, _name, _name_active, _res_name, \
> +   _res_en_offset, _res_on, _res_off, _rate, \
> +   _state_mask, _state_on_mask)\
> + static struct clk_rpmh _platform##_##_name_active;\
> + static struct clk_rpmh _platform##_##_name = {\
> + .res_name = _res_name,\
> + .res_en_offset = _res_en_offset,  \
> + .res_on_val = _res_on,\
> + .res_off_val = _res_off,  \
> + .rate = _rate,\
> + .peer = &_platform##_##_name_active,  \
> + .valid_state_mask = _state_mask,  \

This is always CLK_RPMH_APPS_RSC_STATE_MASK

> + .hw.init = &(struct clk_init_data){   \